mbox series

[v8,0/4] MAX9286 GMSL Support (+RDACM20)

Message ID 20200417103424.5875-1-kieran.bingham+renesas@ideasonboard.com
Headers show
Series MAX9286 GMSL Support (+RDACM20) | expand

Message

Kieran Bingham April 17, 2020, 10:34 a.m. UTC
This series provides a pair of drivers for the GMSL cameras on the R-Car ADAS
platforms.

These drivers originate from Cogent Embedded, and have been refactored to split
the MAX9286 away from the RDACM20 drivers which were once very tightly coupled.

The MAX9286 is capable of capturing up to 4 streams simultaneously, and while
the V4L2-Multiplexed streams series is not available, this works purely on the
assumption that the receiver will correctly map each of the 4 VCs to separate
video nodes, as the RCar-VIN does.

This driver along with a camera driver for the RDACM20 and the
associated platform support for the Renesas R-Car Salvator-X, and the Eagle-V3M
can be found at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git gmsl/v8

We're very much hoping that we can aim to get the max9286 into the next
merge-window. Please let us know if there are any issues blocking this.

Jacopo Mondi (2):
  dt-bindings: media: i2c: Add bindings for IMI RDACM2x
  media: i2c: Add RDACM20 driver

Kieran Bingham (1):
  media: i2c: Add MAX9286 driver

Laurent Pinchart (1):
  dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286

 .../bindings/media/i2c/imi,rdacm2x-gmsl.yaml  |  161 ++
 .../bindings/media/i2c/maxim,max9286.yaml     |  287 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |    2 +
 MAINTAINERS                                   |   22 +
 drivers/media/i2c/Kconfig                     |   25 +
 drivers/media/i2c/Makefile                    |    3 +
 drivers/media/i2c/max9271.c                   |  330 ++++
 drivers/media/i2c/max9271.h                   |  224 +++
 drivers/media/i2c/max9286.c                   | 1332 +++++++++++++++++
 drivers/media/i2c/rdacm20.c                   |  668 +++++++++
 10 files changed, 3054 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imi,rdacm2x-gmsl.yaml
 create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
 create mode 100644 drivers/media/i2c/max9271.c
 create mode 100644 drivers/media/i2c/max9271.h
 create mode 100644 drivers/media/i2c/max9286.c
 create mode 100644 drivers/media/i2c/rdacm20.c

Comments

Hans Verkuil May 11, 2020, 10:20 a.m. UTC | #1
On 17/04/2020 12:34, Kieran Bingham wrote:
> From: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> The RDACM20 is a GMSL camera supporting 1280x800 resolution images
> developed by IMI based on an Omnivision 10635 sensor and a Maxim MAX9271
> GMSL serializer.
> 
> The GMSL link carries power, control (I2C) and video data over a
> single coax cable.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> ---

<snip>

> diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c
> new file mode 100644
> index 000000000000..37786998878b
> --- /dev/null
> +++ b/drivers/media/i2c/rdacm20.c
> @@ -0,0 +1,668 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * IMI RDACM20 GMSL Camera Driver
> + *
> + * Copyright (C) 2017-2020 Jacopo Mondi
> + * Copyright (C) 2017-2020 Kieran Bingham
> + * Copyright (C) 2017-2019 Laurent Pinchart
> + * Copyright (C) 2017-2019 Niklas Söderlund
> + * Copyright (C) 2016 Renesas Electronics Corporation
> + * Copyright (C) 2015 Cogent Embedded, Inc.
> + */
> +
> +/*
> + * The camera is made of an Omnivision OV10635 sensor connected to a Maxim
> + * MAX9271 GMSL serializer.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/fwnode.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/videodev2.h>
> +
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-subdev.h>
> +
> +#include "max9271.h"
> +
> +#define OV10635_I2C_ADDRESS		0x30
> +
> +#define OV10635_SOFTWARE_RESET		0x0103
> +#define OV10635_PID			0x300a
> +#define OV10635_VER			0x300b
> +#define OV10635_SC_CMMN_SCCB_ID		0x300c
> +#define OV10635_SC_CMMN_SCCB_ID_SELECT	BIT(0)
> +#define OV10635_VERSION			0xa635
> +
> +#define OV10635_WIDTH			1280
> +#define OV10635_HEIGHT			800
> +#define OV10635_FORMAT			MEDIA_BUS_FMT_UYVY8_2X8

This OV10635_FORMAT define was very confusing when I reviewed this code.

Please just use MEDIA_BUS_FMT_UYVY8_2X8 directly instead of introducing
an alias. While reviewing I thought for a moment that OV10635_FORMAT was
somehow a new mediabus format that was added elsewhere. I had to dig into
the code to figure out that it really was an alias.

Regards,

	Hans