[v6,3/9] v4l: platform: Add Renesas CEU driver

Commit Message

Jacopo Mondi Jan. 16, 2018, 9:44 p.m. UTC

Add driver for Renesas Capture Engine Unit (CEU).

The CEU interface supports capturing 'data' (YUV422) and 'images'
(NV[12|21|16|61]).

This driver aims to replace the soc_camera-based sh_mobile_ceu one.

Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
platform GR-Peach.

Tested with ov7725 camera sensor on SH4 platform Migo-R.

Signed-off-by: Jacopo Mondi <[e-mail]>
Reviewed-by: Laurent Pinchart <[e-mail]>
---
 drivers/media/platform/Kconfig       |    9 +
 drivers/media/platform/Makefile      |    1 +
 drivers/media/platform/renesas-ceu.c | 1659 ++++++++++++++++++++++++++++++++++
 3 files changed, 1669 insertions(+)
 create mode 100644 drivers/media/platform/renesas-ceu.c

Comments

Hans Verkuil Jan. 19, 2018, 11:20 a.m. UTC | #1

Some more comments:

On 01/16/18 22:44, Jacopo Mondi wrote:
> Add driver for Renesas Capture Engine Unit (CEU).
> 
> The CEU interface supports capturing 'data' (YUV422) and 'images'
> (NV[12|21|16|61]).
> 
> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> 
> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> platform GR-Peach.
> 
> Tested with ov7725 camera sensor on SH4 platform Migo-R.
> 
> Signed-off-by: Jacopo Mondi <[e-mail]>
> Reviewed-by: Laurent Pinchart <[e-mail]>
> ---
>  drivers/media/platform/Kconfig       |    9 +
>  drivers/media/platform/Makefile      |    1 +
>  drivers/media/platform/renesas-ceu.c | 1659 ++++++++++++++++++++++++++++++++++
>  3 files changed, 1669 insertions(+)
>  create mode 100644 drivers/media/platform/renesas-ceu.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index fd0c998..fe7bd26 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called stm32-dcmi.
>  
> +config VIDEO_RENESAS_CEU
> +	tristate "Renesas Capture Engine Unit (CEU) driver"
> +	depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
> +	depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
> +	select VIDEOBUF2_DMA_CONTIG
> +	select V4L2_FWNODE
> +	---help---
> +	  This is a v4l2 driver for the Renesas CEU Interface
> +
>  source "drivers/media/platform/soc_camera/Kconfig"
>  source "drivers/media/platform/exynos4-is/Kconfig"
>  source "drivers/media/platform/am437x/Kconfig"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 003b0bb..6580a6b 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)		+= sh_vou.o
>  obj-$(CONFIG_SOC_CAMERA)		+= soc_camera/
>  
>  obj-$(CONFIG_VIDEO_RCAR_DRIF)		+= rcar_drif.o
> +obj-$(CONFIG_VIDEO_RENESAS_CEU)		+= renesas-ceu.o
>  obj-$(CONFIG_VIDEO_RENESAS_FCP) 	+= rcar-fcp.o
>  obj-$(CONFIG_VIDEO_RENESAS_FDP1)	+= rcar_fdp1.o
>  obj-$(CONFIG_VIDEO_RENESAS_JPU) 	+= rcar_jpu.o
> diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
> new file mode 100644
> index 0000000..1b8f0ef
> --- /dev/null
> +++ b/drivers/media/platform/renesas-ceu.c
> @@ -0,0 +1,1659 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * V4L2 Driver for Renesas Capture Engine Unit (CEU) interface
> + * Copyright (C) 2017-2018 Jacopo Mondi <[e-mail]>
> + *
> + * Based on soc-camera driver "soc_camera/sh_mobile_ceu_camera.c"
> + * Copyright (C) 2008 Magnus Damm
> + *
> + * Based on V4L2 Driver for PXA camera host - "pxa_camera.c",
> + * Copyright (C) 2006, Sascha Hauer, Pengutronix
> + * Copyright (C) 2008, Guennadi Liakhovetski <[e-mail]>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/time.h>
> +#include <linux/videodev2.h>
> +
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-common.h>
> +#include <media/v4l2-dev.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-image-sizes.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-mediabus.h>
> +#include <media/videobuf2-dma-contig.h>
> +
> +#include <media/drv-intf/renesas-ceu.h>
> +
> +#define DRIVER_NAME	"renesas-ceu"
> +
> +/* CEU registers offsets and masks. */
> +#define CEU_CAPSR	0x00 /* Capture start register			*/
> +#define CEU_CAPCR	0x04 /* Capture control register		*/
> +#define CEU_CAMCR	0x08 /* Capture interface control register	*/
> +#define CEU_CAMOR	0x10 /* Capture interface offset register	*/
> +#define CEU_CAPWR	0x14 /* Capture interface width register	*/
> +#define CEU_CAIFR	0x18 /* Capture interface input format register */
> +#define CEU_CRCNTR	0x28 /* CEU register control register		*/
> +#define CEU_CRCMPR	0x2c /* CEU register forcible control register	*/
> +#define CEU_CFLCR	0x30 /* Capture filter control register		*/
> +#define CEU_CFSZR	0x34 /* Capture filter size clip register	*/
> +#define CEU_CDWDR	0x38 /* Capture destination width register	*/
> +#define CEU_CDAYR	0x3c /* Capture data address Y register		*/
> +#define CEU_CDACR	0x40 /* Capture data address C register		*/
> +#define CEU_CFWCR	0x5c /* Firewall operation control register	*/
> +#define CEU_CDOCR	0x64 /* Capture data output control register	*/
> +#define CEU_CEIER	0x70 /* Capture event interrupt enable register	*/
> +#define CEU_CETCR	0x74 /* Capture event flag clear register	*/
> +#define CEU_CSTSR	0x7c /* Capture status register			*/
> +#define CEU_CSRTR	0x80 /* Capture software reset register		*/
> +
> +/* Data synchronous fetch mode. */
> +#define CEU_CAMCR_JPEG			BIT(4)
> +
> +/* Input components ordering: CEU_CAMCR.DTARY field. */
> +#define CEU_CAMCR_DTARY_8_UYVY		(0x00 << 8)
> +#define CEU_CAMCR_DTARY_8_VYUY		(0x01 << 8)
> +#define CEU_CAMCR_DTARY_8_YUYV		(0x02 << 8)
> +#define CEU_CAMCR_DTARY_8_YVYU		(0x03 << 8)
> +/* TODO: input components ordering for 16 bits input. */
> +
> +/* Bus transfer MTU. */
> +#define CEU_CAPCR_BUS_WIDTH256		(0x3 << 20)
> +
> +/* Bus width configuration. */
> +#define CEU_CAMCR_DTIF_16BITS		BIT(12)
> +
> +/* No downsampling to planar YUV420 in image fetch mode. */
> +#define CEU_CDOCR_NO_DOWSAMPLE		BIT(4)
> +
> +/* Swap all input data in 8-bit, 16-bits and 32-bits units (Figure 46.45). */
> +#define CEU_CDOCR_SWAP_ENDIANNESS	(7)
> +
> +/* Capture reset and enable bits. */
> +#define CEU_CAPSR_CPKIL			BIT(16)
> +#define CEU_CAPSR_CE			BIT(0)
> +
> +/* CEU operating flag bit. */
> +#define CEU_CAPCR_CTNCP			BIT(16)
> +#define CEU_CSTRST_CPTON		BIT(1)
> +
> +/* Platform specific IRQ source flags. */
> +#define CEU_CETCR_ALL_IRQS_RZ		0x397f313
> +#define CEU_CETCR_ALL_IRQS_SH4		0x3d7f313
> +
> +/* Prohibited register access interrupt bit. */
> +#define CEU_CETCR_IGRW			BIT(4)
> +/* One-frame capture end interrupt. */
> +#define CEU_CEIER_CPE			BIT(0)
> +/* VBP error. */
> +#define CEU_CEIER_VBP			BIT(20)
> +#define CEU_CEIER_MASK			(CEU_CEIER_CPE | CEU_CEIER_VBP)
> +
> +#define CEU_MAX_WIDTH	2560
> +#define CEU_MAX_HEIGHT	1920
> +#define CEU_W_MAX(w)	((w) < CEU_MAX_WIDTH ? (w) : CEU_MAX_WIDTH)
> +#define CEU_H_MAX(h)	((h) < CEU_MAX_HEIGHT ? (h) : CEU_MAX_HEIGHT)
> +
> +/*
> + * ceu_bus_fmt - describe a 8-bits yuyv format the sensor can produce
> + *
> + * @mbus_code: bus format code
> + * @fmt_order: CEU_CAMCR.DTARY ordering of input components (Y, Cb, Cr)
> + * @fmt_order_swap: swapped CEU_CAMCR.DTARY ordering of input components
> + *		    (Y, Cr, Cb)
> + * @swapped: does Cr appear before Cb?
> + * @bps: number of bits sent over bus for each sample
> + * @bpp: number of bits per pixels unit
> + */
> +struct ceu_mbus_fmt {
> +	u32	mbus_code;
> +	u32	fmt_order;
> +	u32	fmt_order_swap;
> +	bool	swapped;
> +	u8	bps;
> +	u8	bpp;
> +};
> +
> +/*
> + * ceu_buffer - Link vb2 buffer to the list of available buffers.
> + */
> +struct ceu_buffer {
> +	struct vb2_v4l2_buffer vb;
> +	struct list_head queue;
> +};
> +
> +static inline struct ceu_buffer *vb2_to_ceu(struct vb2_v4l2_buffer *vbuf)
> +{
> +	return container_of(vbuf, struct ceu_buffer, vb);
> +}
> +
> +/*
> + * ceu_subdev - Wraps v4l2 sub-device and provides async subdevice.
> + */
> +struct ceu_subdev {
> +	struct v4l2_subdev *v4l2_sd;
> +	struct v4l2_async_subdev asd;
> +
> +	/* per-subdevice mbus configuration options */
> +	unsigned int mbus_flags;
> +	struct ceu_mbus_fmt mbus_fmt;
> +};
> +
> +static struct ceu_subdev *to_ceu_subdev(struct v4l2_async_subdev *asd)
> +{
> +	return container_of(asd, struct ceu_subdev, asd);
> +}
> +
> +/*
> + * ceu_device - CEU device instance
> + */
> +struct ceu_device {
> +	struct device		*dev;
> +	struct video_device	vdev;
> +	struct v4l2_device	v4l2_dev;
> +
> +	/* subdevices descriptors */
> +	struct ceu_subdev	*subdevs;
> +	/* the subdevice currently in use */
> +	struct ceu_subdev	*sd;
> +	unsigned int		sd_index;
> +	unsigned int		num_sd;
> +
> +	/* platform specific mask with all IRQ sources flagged */
> +	u32			irq_mask;
> +
> +	/* currently configured field and pixel format */
> +	enum v4l2_field	field;
> +	struct v4l2_pix_format_mplane v4l2_pix;
> +
> +	/* async subdev notification helpers */
> +	struct v4l2_async_notifier notifier;
> +	/* pointers to "struct ceu_subdevice -> asd" */
> +	struct v4l2_async_subdev **asds;
> +
> +	/* vb2 queue, capture buffer list and active buffer pointer */
> +	struct vb2_queue	vb2_vq;
> +	struct list_head	capture;
> +	struct vb2_v4l2_buffer	*active;
> +	unsigned int		sequence;
> +
> +	/* mlock - lock access to interface reset and vb2 queue */
> +	struct mutex	mlock;
> +
> +	/* lock - lock access to capture buffer queue and active buffer */
> +	spinlock_t	lock;
> +
> +	/* base - CEU memory base address */
> +	void __iomem	*base;
> +};
> +
> +static inline struct ceu_device *v4l2_to_ceu(struct v4l2_device *v4l2_dev)
> +{
> +	return container_of(v4l2_dev, struct ceu_device, v4l2_dev);
> +}
> +
> +/* --- CEU memory output formats --- */
> +
> +/*
> + * ceu_fmt - describe a memory output format supported by CEU interface.
> + *
> + * @fourcc: memory layout fourcc format code
> + * @bpp: number of bits for each pixel stored in memory
> + */
> +struct ceu_fmt {
> +	u32	fourcc;
> +	u32	bpp;
> +};
> +
> +/*
> + * ceu_format_list - List of supported memory output formats
> + *
> + * If sensor provides any YUYV bus format, all the following planar memory
> + * formats are available thanks to CEU re-ordering and sub-sampling
> + * capabilities.
> + */
> +static const struct ceu_fmt ceu_fmt_list[] = {
> +	{
> +		.fourcc	= V4L2_PIX_FMT_NV16,
> +		.bpp	= 16,
> +	},
> +	{
> +		.fourcc = V4L2_PIX_FMT_NV61,
> +		.bpp	= 16,
> +	},
> +	{
> +		.fourcc	= V4L2_PIX_FMT_NV12,
> +		.bpp	= 12,
> +	},
> +	{
> +		.fourcc	= V4L2_PIX_FMT_NV21,
> +		.bpp	= 12,
> +	},
> +	{
> +		.fourcc	= V4L2_PIX_FMT_YUYV,
> +		.bpp	= 16,
> +	},
> +};
> +
> +static const struct ceu_fmt *get_ceu_fmt_from_fourcc(unsigned int fourcc)
> +{
> +	const struct ceu_fmt *fmt = &ceu_fmt_list[0];
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ceu_fmt_list); i++, fmt++)
> +		if (fmt->fourcc == fourcc)
> +			return fmt;
> +
> +	return NULL;
> +}
> +
> +static bool ceu_fmt_mplane(struct v4l2_pix_format_mplane *pix)
> +{
> +	switch (pix->pixelformat) {
> +	case V4L2_PIX_FMT_YUYV:
> +		return false;
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_NV61:
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV21:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +/* --- CEU HW operations --- */
> +
> +static void ceu_write(struct ceu_device *priv, unsigned int reg_offs, u32 data)
> +{
> +	iowrite32(data, priv->base + reg_offs);
> +}
> +
> +static u32 ceu_read(struct ceu_device *priv, unsigned int reg_offs)
> +{
> +	return ioread32(priv->base + reg_offs);
> +}
> +
> +/*
> + * ceu_soft_reset() - Software reset the CEU interface.
> + * @ceu_device: CEU device.
> + *
> + * Returns 0 for success, -EIO for error.
> + */
> +static int ceu_soft_reset(struct ceu_device *ceudev)
> +{
> +	unsigned int i;
> +
> +	ceu_write(ceudev, CEU_CAPSR, CEU_CAPSR_CPKIL);
> +
> +	for (i = 0; i < 100; i++) {
> +		if (!(ceu_read(ceudev, CEU_CSTSR) & CEU_CSTRST_CPTON))
> +			break;
> +		udelay(1);
> +	}
> +
> +	if (i == 100) {
> +		dev_err(ceudev->dev, "soft reset time out\n");
> +		return -EIO;
> +	}
> +
> +	for (i = 0; i < 100; i++) {
> +		if (!(ceu_read(ceudev, CEU_CAPSR) & CEU_CAPSR_CPKIL))
> +			return 0;
> +		udelay(1);
> +	}
> +
> +	/* If we get here, CEU has not reset properly. */
> +	return -EIO;
> +}
> +
> +/* --- CEU Capture Operations --- */
> +
> +/*
> + * ceu_hw_config() - Configure CEU interface registers.
> + */
> +static int ceu_hw_config(struct ceu_device *ceudev)
> +{
> +	u32 camcr, cdocr, cfzsr, cdwdr, capwr;
> +	struct v4l2_pix_format_mplane *pix = &ceudev->v4l2_pix;
> +	struct ceu_subdev *ceu_sd = ceudev->sd;
> +	struct ceu_mbus_fmt *mbus_fmt = &ceu_sd->mbus_fmt;
> +	unsigned int mbus_flags = ceu_sd->mbus_flags;
> +
> +	/* Start configuring CEU registers */
> +	ceu_write(ceudev, CEU_CAIFR, 0);
> +	ceu_write(ceudev, CEU_CFWCR, 0);
> +	ceu_write(ceudev, CEU_CRCNTR, 0);
> +	ceu_write(ceudev, CEU_CRCMPR, 0);
> +
> +	/* Set the frame capture period for both image capture and data sync. */
> +	capwr = (pix->height << 16) | pix->width * mbus_fmt->bpp / 8;
> +
> +	/*
> +	 * Swap input data endianness by default.
> +	 * In data fetch mode bytes are received in chunks of 8 bytes.
> +	 * D0, D1, D2, D3, D4, D5, D6, D7 (D0 received first)
> +	 * The data is however by default written to memory in reverse order:
> +	 * D7, D6, D5, D4, D3, D2, D1, D0 (D7 written to lowest byte)
> +	 *
> +	 * Use CEU_CDOCR[2:0] to swap data ordering.
> +	 */
> +	cdocr = CEU_CDOCR_SWAP_ENDIANNESS;
> +
> +	/*
> +	 * Configure CAMCR and CDOCR:
> +	 * match input components ordering with memory output format and
> +	 * handle downsampling to YUV420.
> +	 *
> +	 * If the memory output planar format is 'swapped' (Cr before Cb) and
> +	 * input format is not, use the swapped version of CAMCR.DTARY.
> +	 *
> +	 * If the memory output planar format is not 'swapped' (Cb before Cr)
> +	 * and input format is, use the swapped version of CAMCR.DTARY.
> +	 *
> +	 * CEU by default downsample to planar YUV420 (CDCOR[4] = 0).
> +	 * If output is planar YUV422 set CDOCR[4] = 1
> +	 *
> +	 * No downsample for data fetch sync mode.
> +	 */
> +	switch (pix->pixelformat) {
> +	/* Data fetch sync mode */
> +	case V4L2_PIX_FMT_YUYV:
> +		/* TODO: handle YUYV permutations through DTARY bits. */
> +		camcr	= CEU_CAMCR_JPEG;
> +		cdocr	|= CEU_CDOCR_NO_DOWSAMPLE;
> +		cfzsr	= (pix->height << 16) | pix->width;
> +		cdwdr	= pix->plane_fmt[0].bytesperline;
> +		break;
> +
> +	/* Non-swapped planar image capture mode. */
> +	case V4L2_PIX_FMT_NV16:
> +		cdocr	|= CEU_CDOCR_NO_DOWSAMPLE;
> +	case V4L2_PIX_FMT_NV12:
> +		if (mbus_fmt->swapped)
> +			camcr = mbus_fmt->fmt_order_swap;
> +		else
> +			camcr = mbus_fmt->fmt_order;
> +
> +		cfzsr	= (pix->height << 16) | pix->width;
> +		cdwdr	= pix->width;
> +		break;
> +
> +	/* Swapped planar image capture mode. */
> +	case V4L2_PIX_FMT_NV61:
> +		cdocr	|= CEU_CDOCR_NO_DOWSAMPLE;
> +	case V4L2_PIX_FMT_NV21:
> +		if (mbus_fmt->swapped)
> +			camcr = mbus_fmt->fmt_order;
> +		else
> +			camcr = mbus_fmt->fmt_order_swap;
> +
> +		cfzsr	= (pix->height << 16) | pix->width;
> +		cdwdr	= pix->width;
> +		break;
> +	}
> +
> +	camcr |= mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW ? 1 << 1 : 0;
> +	camcr |= mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW ? 1 << 0 : 0;
> +
> +	/* TODO: handle 16 bit bus width with DTIF bit in CAMCR */
> +	ceu_write(ceudev, CEU_CAMCR, camcr);
> +	ceu_write(ceudev, CEU_CDOCR, cdocr);
> +	ceu_write(ceudev, CEU_CAPCR, CEU_CAPCR_BUS_WIDTH256);
> +
> +	/*
> +	 * TODO: make CAMOR offsets configurable.
> +	 * CAMOR wants to know the number of blanks between a VS/HS signal
> +	 * and valid data. This value should actually come from the sensor...
> +	 */
> +	ceu_write(ceudev, CEU_CAMOR, 0);
> +
> +	/* TODO: 16 bit bus width require re-calculation of cdwdr and cfzsr */
> +	ceu_write(ceudev, CEU_CAPWR, capwr);
> +	ceu_write(ceudev, CEU_CFSZR, cfzsr);
> +	ceu_write(ceudev, CEU_CDWDR, cdwdr);
> +
> +	return 0;
> +}
> +
> +/*
> + * ceu_capture() - Trigger start of a capture sequence.
> + *
> + * Program the CEU DMA registers with addresses where to transfer image data.
> + */
> +static int ceu_capture(struct ceu_device *ceudev)
> +{
> +	struct v4l2_pix_format_mplane *pix = &ceudev->v4l2_pix;
> +	dma_addr_t phys_addr_top;
> +
> +	phys_addr_top =
> +		vb2_dma_contig_plane_dma_addr(&ceudev->active->vb2_buf, 0);
> +	ceu_write(ceudev, CEU_CDAYR, phys_addr_top);
> +
> +	/* Ignore CbCr plane for non multi-planar image formats. */
> +	if (ceu_fmt_mplane(pix)) {
> +		phys_addr_top =
> +			vb2_dma_contig_plane_dma_addr(&ceudev->active->vb2_buf,
> +						      1);
> +		ceu_write(ceudev, CEU_CDACR, phys_addr_top);
> +	}
> +
> +	/*
> +	 * Trigger new capture start: once for each frame, as we work in
> +	 * one-frame capture mode.
> +	 */
> +	ceu_write(ceudev, CEU_CAPSR, CEU_CAPSR_CE);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t ceu_irq(int irq, void *data)
> +{
> +	struct ceu_device *ceudev = data;
> +	struct vb2_v4l2_buffer *vbuf;
> +	struct ceu_buffer *buf;
> +	u32 status;
> +
> +	/* Clean interrupt status. */
> +	status = ceu_read(ceudev, CEU_CETCR);
> +	ceu_write(ceudev, CEU_CETCR, ~ceudev->irq_mask);
> +
> +	/* Unexpected interrupt. */
> +	if (!(status & CEU_CEIER_MASK))
> +		return IRQ_NONE;
> +
> +	spin_lock(&ceudev->lock);
> +
> +	/* Stale interrupt from a released buffer, ignore it. */
> +	vbuf = ceudev->active;
> +	if (!vbuf) {
> +		spin_unlock(&ceudev->lock);
> +		return IRQ_HANDLED;
> +	}
> +
> +	/*
> +	 * When a VBP interrupt occurs, no capture end interrupt will occur
> +	 * and the image of that frame is not captured correctly.
> +	 */
> +	if (status & CEU_CEIER_VBP) {
> +		dev_err(ceudev->dev, "VBP interrupt: abort capture\n");
> +		goto error_irq_out;
> +	}
> +
> +	/* Prepare to return the 'previous' buffer. */
> +	vbuf->vb2_buf.timestamp = ktime_get_ns();
> +	vbuf->sequence = ceudev->sequence++;
> +	vbuf->field = ceudev->field;
> +
> +	/* Prepare a new 'active' buffer and trigger a new capture. */
> +	if (!list_empty(&ceudev->capture)) {
> +		buf = list_first_entry(&ceudev->capture, struct ceu_buffer,
> +				       queue);
> +		list_del(&buf->queue);
> +		ceudev->active = &buf->vb;
> +
> +		ceu_capture(ceudev);
> +	}
> +
> +	/* Return the 'previous' buffer. */
> +	vb2_buffer_done(&vbuf->vb2_buf, VB2_BUF_STATE_DONE);
> +
> +	spin_unlock(&ceudev->lock);
> +
> +	return IRQ_HANDLED;
> +
> +error_irq_out:
> +	/* Return the 'previous' buffer and all queued ones. */
> +	vb2_buffer_done(&vbuf->vb2_buf, VB2_BUF_STATE_ERROR);
> +
> +	list_for_each_entry(buf, &ceudev->capture, queue)
> +		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> +
> +	spin_unlock(&ceudev->lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/* --- CEU Videobuf2 operations --- */
> +
> +static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
> +				   unsigned int bpl, unsigned int szimage)
> +{
> +	if (plane->bytesperline < bpl)
> +		plane->bytesperline = bpl;
> +	if (plane->sizeimage < szimage)
> +		plane->sizeimage = szimage;

As mentioned in your cover letter, you do need to check for invalid
bytesperline values. The v4l2-compliance test is to see what happens
when userspace gives insane values, so yes, drivers need to be able
to handle that.

plane->sizeimage is set by the driver, so drop the 'if' before the
assignment.

> +}
> +
> +/*
> + * ceu_calc_plane_sizes() - Fill per-plane 'struct v4l2_plane_pix_format'
> + *			    information according to the currently configured
> + *			    pixel format.
> + * @ceu_device: CEU device.
> + * @ceu_fmt: Active image format.
> + * @pix: Pixel format information (store line width and image sizes)
> + */
> +static void ceu_calc_plane_sizes(struct ceu_device *ceudev,
> +				 const struct ceu_fmt *ceu_fmt,
> +				 struct v4l2_pix_format_mplane *pix)
> +{
> +	unsigned int bpl, szimage;
> +
> +	switch (pix->pixelformat) {
> +	case V4L2_PIX_FMT_YUYV:
> +		pix->num_planes	= 1;
> +		bpl		= pix->width * ceu_fmt->bpp / 8;
> +		szimage		= pix->height * bpl;
> +		ceu_update_plane_sizes(&pix->plane_fmt[0], bpl, szimage);
> +		break;
> +
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV21:
> +		pix->num_planes	= 2;
> +		bpl		= pix->width;
> +		szimage		= pix->height * pix->width;
> +		ceu_update_plane_sizes(&pix->plane_fmt[0], bpl, szimage);
> +		ceu_update_plane_sizes(&pix->plane_fmt[1], bpl, szimage / 2);
> +		break;
> +
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_NV61:
> +	default:
> +		pix->num_planes	= 2;
> +		bpl		= pix->width;
> +		szimage		= pix->height * pix->width;
> +		ceu_update_plane_sizes(&pix->plane_fmt[0], bpl, szimage);
> +		ceu_update_plane_sizes(&pix->plane_fmt[1], bpl, szimage);
> +		break;
> +	}
> +}
> +
> +/*
> + * ceu_vb2_setup() - is called to check whether the driver can accept the
> + *		     requested number of buffers and to fill in plane sizes
> + *		     for the current frame format, if required.
> + */
> +static int ceu_vb2_setup(struct vb2_queue *vq, unsigned int *count,
> +			 unsigned int *num_planes, unsigned int sizes[],
> +			 struct device *alloc_devs[])
> +{
> +	struct ceu_device *ceudev = vb2_get_drv_priv(vq);
> +	struct v4l2_pix_format_mplane *pix = &ceudev->v4l2_pix;
> +	unsigned int i;
> +
> +	/* num_planes is set: just check plane sizes. */
> +	if (*num_planes) {
> +		for (i = 0; i < pix->num_planes; i++)
> +			if (sizes[i] < pix->plane_fmt[i].sizeimage)
> +				return -EINVAL;
> +
> +		return 0;
> +	}
> +
> +	/* num_planes not set: called from REQBUFS, just set plane sizes. */
> +	*num_planes = pix->num_planes;
> +	for (i = 0; i < pix->num_planes; i++)
> +		sizes[i] = pix->plane_fmt[i].sizeimage;
> +
> +	return 0;
> +}
> +
> +static void ceu_vb2_queue(struct vb2_buffer *vb)
> +{
> +	struct ceu_device *ceudev = vb2_get_drv_priv(vb->vb2_queue);
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct ceu_buffer *buf = vb2_to_ceu(vbuf);
> +	unsigned long irqflags;
> +
> +	spin_lock_irqsave(&ceudev->lock, irqflags);
> +	list_add_tail(&buf->queue, &ceudev->capture);
> +	spin_unlock_irqrestore(&ceudev->lock, irqflags);
> +}
> +
> +static int ceu_vb2_prepare(struct vb2_buffer *vb)
> +{
> +	struct ceu_device *ceudev = vb2_get_drv_priv(vb->vb2_queue);
> +	struct v4l2_pix_format_mplane *pix = &ceudev->v4l2_pix;
> +	unsigned int i;
> +
> +	for (i = 0; i < pix->num_planes; i++) {
> +		if (vb2_plane_size(vb, i) < pix->plane_fmt[i].sizeimage) {
> +			dev_err(ceudev->dev,
> +				"Plane size too small (%lu < %u)\n",
> +				vb2_plane_size(vb, i),
> +				pix->plane_fmt[i].sizeimage);
> +			return -EINVAL;
> +		}
> +
> +		vb2_set_plane_payload(vb, i, pix->plane_fmt[i].sizeimage);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ceu_start_streaming(struct vb2_queue *vq, unsigned int count)
> +{
> +	struct ceu_device *ceudev = vb2_get_drv_priv(vq);
> +	struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
> +	struct ceu_buffer *buf;
> +	unsigned long irqflags;
> +	int ret;
> +
> +	/* Program the CEU interface according to the CEU image format. */
> +	ret = ceu_hw_config(ceudev);
> +	if (ret)
> +		goto error_return_bufs;
> +
> +	ret = v4l2_subdev_call(v4l2_sd, video, s_stream, 1);
> +	if (ret && ret != -ENOIOCTLCMD) {
> +		dev_dbg(ceudev->dev,
> +			"Subdevice failed to start streaming: %d\n", ret);
> +		goto error_return_bufs;
> +	}
> +
> +	spin_lock_irqsave(&ceudev->lock, irqflags);
> +	ceudev->sequence = 0;
> +
> +	/* Grab the first available buffer and trigger the first capture. */
> +	buf = list_first_entry(&ceudev->capture, struct ceu_buffer,
> +			       queue);
> +	if (!buf) {
> +		spin_unlock_irqrestore(&ceudev->lock, irqflags);
> +		dev_dbg(ceudev->dev,
> +			"No buffer available for capture.\n");
> +		goto error_stop_sensor;
> +	}
> +
> +	list_del(&buf->queue);
> +	ceudev->active = &buf->vb;
> +
> +	/* Clean and program interrupts for first capture. */
> +	ceu_write(ceudev, CEU_CETCR, ~ceudev->irq_mask);
> +	ceu_write(ceudev, CEU_CEIER, CEU_CEIER_MASK);
> +
> +	ceu_capture(ceudev);
> +
> +	spin_unlock_irqrestore(&ceudev->lock, irqflags);
> +
> +	return 0;
> +
> +error_stop_sensor:
> +	v4l2_subdev_call(v4l2_sd, video, s_stream, 0);
> +
> +error_return_bufs:
> +	spin_lock_irqsave(&ceudev->lock, irqflags);
> +	list_for_each_entry(buf, &ceudev->capture, queue)
> +		vb2_buffer_done(&ceudev->active->vb2_buf,
> +				VB2_BUF_STATE_QUEUED);
> +	ceudev->active = NULL;
> +	spin_unlock_irqrestore(&ceudev->lock, irqflags);
> +
> +	return ret;
> +}
> +
> +static void ceu_stop_streaming(struct vb2_queue *vq)
> +{
> +	struct ceu_device *ceudev = vb2_get_drv_priv(vq);
> +	struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
> +	struct ceu_buffer *buf;
> +	unsigned long irqflags;
> +
> +	/* Clean and disable interrupt sources. */
> +	ceu_write(ceudev, CEU_CETCR,
> +		  ceu_read(ceudev, CEU_CETCR) & ceudev->irq_mask);
> +	ceu_write(ceudev, CEU_CEIER, CEU_CEIER_MASK);
> +
> +	v4l2_subdev_call(v4l2_sd, video, s_stream, 0);
> +
> +	spin_lock_irqsave(&ceudev->lock, irqflags);
> +	if (ceudev->active) {
> +		vb2_buffer_done(&ceudev->active->vb2_buf,
> +				VB2_BUF_STATE_ERROR);
> +		ceudev->active = NULL;
> +	}
> +
> +	/* Release all queued buffers. */
> +	list_for_each_entry(buf, &ceudev->capture, queue)
> +		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> +	INIT_LIST_HEAD(&ceudev->capture);
> +
> +	spin_unlock_irqrestore(&ceudev->lock, irqflags);
> +
> +	ceu_soft_reset(ceudev);
> +}
> +
> +static const struct vb2_ops ceu_vb2_ops = {
> +	.queue_setup		= ceu_vb2_setup,
> +	.buf_queue		= ceu_vb2_queue,
> +	.buf_prepare		= ceu_vb2_prepare,
> +	.wait_prepare		= vb2_ops_wait_prepare,
> +	.wait_finish		= vb2_ops_wait_finish,
> +	.start_streaming	= ceu_start_streaming,
> +	.stop_streaming		= ceu_stop_streaming,
> +};
> +
> +/* --- CEU image formats handling --- */
> +
> +/*
> + * ceu_try_fmt() - test format on CEU and sensor
> + * @ceudev: The CEU device.
> + * @v4l2_fmt: format to test.
> + *
> + * Returns 0 for success, < 0 for errors.
> + */
> +static int ceu_try_fmt(struct ceu_device *ceudev, struct v4l2_format *v4l2_fmt)
> +{
> +	struct ceu_subdev *ceu_sd = ceudev->sd;
> +	struct v4l2_pix_format_mplane *pix = &v4l2_fmt->fmt.pix_mp;
> +	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
> +	struct v4l2_subdev_pad_config pad_cfg;
> +	const struct ceu_fmt *ceu_fmt;
> +	int ret;
> +
> +	struct v4l2_subdev_format sd_format = {
> +		.which = V4L2_SUBDEV_FORMAT_TRY,
> +	};
> +
> +	switch (pix->pixelformat) {
> +	case V4L2_PIX_FMT_YUYV:
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_NV61:
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV21:
> +		break;
> +
> +	default:
> +		pix->pixelformat = V4L2_PIX_FMT_NV16;
> +		break;
> +	}
> +
> +	ceu_fmt = get_ceu_fmt_from_fourcc(pix->pixelformat);
> +
> +	/* CFSZR requires height and width to be 4-pixel aligned. */
> +	v4l_bound_align_image(&pix->width, 2, CEU_MAX_WIDTH, 4,
> +			      &pix->height, 4, CEU_MAX_HEIGHT, 4, 0);
> +
> +	/*
> +	 * Set format on sensor sub device: bus format used to produce memory
> +	 * format is selected at initialization time.
> +	 */
> +	v4l2_fill_mbus_format_mplane(&sd_format.format, pix);
> +	ret = v4l2_subdev_call(v4l2_sd, pad, set_fmt, &pad_cfg, &sd_format);
> +	if (ret)
> +		return ret;
> +
> +	/* Apply size returned by sensor as the CEU can't scale. */
> +	v4l2_fill_pix_format_mplane(pix, &sd_format.format);
> +
> +	/* Calculate per-plane sizes based on image format. */
> +	ceu_calc_plane_sizes(ceudev, ceu_fmt, pix);
> +
> +	return 0;
> +}
> +
> +/*
> + * ceu_set_fmt() - Apply the supplied format to both sensor and CEU
> + */
> +static int ceu_set_fmt(struct ceu_device *ceudev, struct v4l2_format *v4l2_fmt)
> +{
> +	struct ceu_subdev *ceu_sd = ceudev->sd;
> +	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
> +	int ret;
> +
> +	struct v4l2_subdev_format format = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +	};
> +
> +	ret = ceu_try_fmt(ceudev, v4l2_fmt);
> +	if (ret)
> +		return ret;
> +
> +	v4l2_fill_mbus_format_mplane(&format.format, &v4l2_fmt->fmt.pix_mp);
> +	ret = v4l2_subdev_call(v4l2_sd, pad, set_fmt, NULL, &format);
> +	if (ret)
> +		return ret;
> +
> +	ceudev->v4l2_pix = v4l2_fmt->fmt.pix_mp;
> +
> +	return 0;
> +}
> +
> +/*
> + * ceu_set_default_fmt() - Apply default NV16 memory output format with VGA
> + *			   sizes.
> + */
> +static int ceu_set_default_fmt(struct ceu_device *ceudev)
> +{
> +	int ret;
> +
> +	struct v4l2_format v4l2_fmt = {
> +		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> +		.fmt.pix_mp = {
> +			.width		= VGA_WIDTH,
> +			.height		= VGA_HEIGHT,
> +			.field		= V4L2_FIELD_NONE,
> +			.pixelformat	= V4L2_PIX_FMT_NV16,
> +			.num_planes	= 2,
> +			.plane_fmt	= {
> +				[0]	= {
> +					.sizeimage = VGA_WIDTH * VGA_HEIGHT * 2,
> +					.bytesperline = VGA_WIDTH * 2,
> +				},
> +				[1]	= {
> +					.sizeimage = VGA_WIDTH * VGA_HEIGHT * 2,
> +					.bytesperline = VGA_WIDTH * 2,
> +				},
> +			},
> +		},
> +	};
> +
> +	ret = ceu_try_fmt(ceudev, &v4l2_fmt);
> +	if (ret)
> +		return ret;
> +
> +	ceudev->v4l2_pix = v4l2_fmt.fmt.pix_mp;
> +
> +	return 0;
> +}
> +
> +/*
> + * ceu_init_formats() - Query sensor for supported formats and initialize
> + *			CEU supported format list
> + *
> + * Find out if sensor can produce a permutation of 8-bits YUYV bus format.
> + * From a single 8-bits YUYV bus format the CEU can produce several memory
> + * output formats:
> + * - NV[12|21|16|61] through image fetch mode;
> + * - YUYV422 if sensor provides YUYV422
> + *
> + * TODO: Other YUYV422 permutations through data fetch sync mode and DTARY
> + * TODO: Binary data (eg. JPEG) and raw formats through data fetch sync mode
> + */
> +static int ceu_init_formats(struct ceu_device *ceudev)
> +{
> +	struct ceu_subdev *ceu_sd = ceudev->sd;
> +	struct ceu_mbus_fmt *mbus_fmt = &ceu_sd->mbus_fmt;
> +	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
> +	bool yuyv_bus_fmt = false;
> +
> +	struct v4l2_subdev_mbus_code_enum sd_mbus_fmt = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +		.index = 0,
> +	};
> +
> +	/* Find out if sensor can produce any permutation of 8-bits YUYV422. */
> +	while (!yuyv_bus_fmt &&
> +	       !v4l2_subdev_call(v4l2_sd, pad, enum_mbus_code,
> +				 NULL, &sd_mbus_fmt)) {
> +		switch (sd_mbus_fmt.code) {
> +		case MEDIA_BUS_FMT_YUYV8_2X8:
> +		case MEDIA_BUS_FMT_YVYU8_2X8:
> +		case MEDIA_BUS_FMT_UYVY8_2X8:
> +		case MEDIA_BUS_FMT_VYUY8_2X8:
> +			yuyv_bus_fmt = true;
> +			break;
> +		default:
> +			/*
> +			 * Only support 8-bits YUYV bus formats at the moment;
> +			 *
> +			 * TODO: add support for binary formats (data sync
> +			 * fetch mode).
> +			 */
> +			break;
> +		}
> +
> +		sd_mbus_fmt.index++;
> +	}
> +
> +	if (!yuyv_bus_fmt)
> +		return -ENXIO;
> +
> +	/*
> +	 * Save the first encountered YUYV format as "mbus_fmt" and use it
> +	 * to output all planar YUV422 and YUV420 (NV*) formats to memory as
> +	 * well as for data synch fetch mode (YUYV - YVYU etc. ).
> +	 */
> +	mbus_fmt->mbus_code	= sd_mbus_fmt.code;
> +	mbus_fmt->bps		= 8;
> +
> +	/* Annotate the selected bus format components ordering. */
> +	switch (sd_mbus_fmt.code) {
> +	case MEDIA_BUS_FMT_YUYV8_2X8:
> +		mbus_fmt->fmt_order		= CEU_CAMCR_DTARY_8_YUYV;
> +		mbus_fmt->fmt_order_swap	= CEU_CAMCR_DTARY_8_YVYU;
> +		mbus_fmt->swapped		= false;
> +		mbus_fmt->bpp			= 16;
> +		break;
> +
> +	case MEDIA_BUS_FMT_YVYU8_2X8:
> +		mbus_fmt->fmt_order		= CEU_CAMCR_DTARY_8_YVYU;
> +		mbus_fmt->fmt_order_swap	= CEU_CAMCR_DTARY_8_YUYV;
> +		mbus_fmt->swapped		= true;
> +		mbus_fmt->bpp			= 16;
> +		break;
> +
> +	case MEDIA_BUS_FMT_UYVY8_2X8:
> +		mbus_fmt->fmt_order		= CEU_CAMCR_DTARY_8_UYVY;
> +		mbus_fmt->fmt_order_swap	= CEU_CAMCR_DTARY_8_VYUY;
> +		mbus_fmt->swapped		= false;
> +		mbus_fmt->bpp			= 16;
> +		break;
> +
> +	case MEDIA_BUS_FMT_VYUY8_2X8:
> +		mbus_fmt->fmt_order		= CEU_CAMCR_DTARY_8_VYUY;
> +		mbus_fmt->fmt_order_swap	= CEU_CAMCR_DTARY_8_UYVY;
> +		mbus_fmt->swapped		= true;
> +		mbus_fmt->bpp			= 16;
> +		break;
> +	}
> +
> +	ceudev->field = V4L2_FIELD_NONE;
> +
> +	return 0;
> +}
> +
> +/* --- Runtime PM Handlers --- */
> +
> +/*
> + * ceu_runtime_resume() - soft-reset the interface and turn sensor power on.
> + */
> +static int ceu_runtime_resume(struct device *dev)
> +{
> +	struct ceu_device *ceudev = dev_get_drvdata(dev);
> +	struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
> +
> +	v4l2_subdev_call(v4l2_sd, core, s_power, 1);
> +
> +	ceu_soft_reset(ceudev);
> +
> +	return 0;
> +}
> +
> +/*
> + * ceu_runtime_suspend() - disable capture and interrupts and soft-reset.
> + *			   Turn sensor power off.
> + */
> +static int ceu_runtime_suspend(struct device *dev)
> +{
> +	struct ceu_device *ceudev = dev_get_drvdata(dev);
> +	struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
> +
> +	v4l2_subdev_call(v4l2_sd, core, s_power, 0);
> +
> +	ceu_write(ceudev, CEU_CEIER, 0);
> +	ceu_soft_reset(ceudev);
> +
> +	return 0;
> +}
> +
> +/* --- File Operations --- */
> +
> +static int ceu_open(struct file *file)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +	int ret;
> +
> +	ret = v4l2_fh_open(file);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&ceudev->mlock);
> +	/* Causes soft-reset and sensor power on on first open */
> +	pm_runtime_get_sync(ceudev->dev);
> +	mutex_unlock(&ceudev->mlock);
> +
> +	return 0;
> +}
> +
> +static int ceu_release(struct file *file)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +
> +	vb2_fop_release(file);
> +
> +	mutex_lock(&ceudev->mlock);
> +	/* Causes soft-reset and sensor power down on last close */
> +	pm_runtime_put(ceudev->dev);
> +	mutex_unlock(&ceudev->mlock);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_file_operations ceu_fops = {
> +	.owner			= THIS_MODULE,
> +	.open			= ceu_open,
> +	.release		= ceu_release,
> +	.unlocked_ioctl		= video_ioctl2,
> +	.mmap			= vb2_fop_mmap,
> +	.poll			= vb2_fop_poll,
> +};
> +
> +/* --- Video Device IOCTLs --- */
> +
> +static int ceu_querycap(struct file *file, void *priv,
> +			struct v4l2_capability *cap)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +
> +	strlcpy(cap->card, "Renesas CEU", sizeof(cap->card));
> +	strlcpy(cap->driver, DRIVER_NAME, sizeof(cap->driver));
> +	snprintf(cap->bus_info, sizeof(cap->bus_info),
> +		 "platform:renesas-ceu-%s", dev_name(ceudev->dev));
> +
> +	return 0;
> +}
> +
> +static int ceu_enum_fmt_vid_cap(struct file *file, void *priv,
> +				struct v4l2_fmtdesc *f)
> +{
> +	const struct ceu_fmt *fmt;
> +
> +	if (f->index >= ARRAY_SIZE(ceu_fmt_list))
> +		return -EINVAL;
> +
> +	fmt = &ceu_fmt_list[f->index];
> +	f->pixelformat = fmt->fourcc;
> +
> +	return 0;
> +}
> +
> +static int ceu_try_fmt_vid_cap(struct file *file, void *priv,
> +			       struct v4l2_format *f)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +
> +	return ceu_try_fmt(ceudev, f);
> +}
> +
> +static int ceu_s_fmt_vid_cap(struct file *file, void *priv,
> +			     struct v4l2_format *f)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +
> +	if (vb2_is_streaming(&ceudev->vb2_vq))
> +		return -EBUSY;
> +
> +	return ceu_set_fmt(ceudev, f);
> +}
> +
> +static int ceu_g_fmt_vid_cap(struct file *file, void *priv,
> +			     struct v4l2_format *f)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +
> +	f->fmt.pix_mp = ceudev->v4l2_pix;
> +
> +	return 0;
> +}
> +
> +static int ceu_enum_input(struct file *file, void *priv,
> +			  struct v4l2_input *inp)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +	struct ceu_subdev *ceusd;
> +
> +	if (inp->index >= ceudev->num_sd)
> +		return -EINVAL;
> +
> +	ceusd = &ceudev->subdevs[inp->index];
> +
> +	inp->type = V4L2_INPUT_TYPE_CAMERA;
> +	inp->std = 0;
> +	snprintf(inp->name, sizeof(inp->name), "Camera%u: %s",
> +		 inp->index, ceusd->v4l2_sd->name);
> +
> +	return 0;
> +}
> +
> +static int ceu_g_input(struct file *file, void *priv, unsigned int *i)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +
> +	*i = ceudev->sd_index;
> +
> +	return 0;
> +}
> +
> +static int ceu_s_input(struct file *file, void *priv, unsigned int i)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +	struct ceu_subdev *ceu_sd_old;
> +	int ret;
> +
> +	if (i >= ceudev->num_sd)
> +		return -EINVAL;
> +
> +	if (vb2_is_streaming(&ceudev->vb2_vq))
> +		return -EBUSY;
> +
> +	if (i == ceudev->sd_index)
> +		return 0;
> +
> +	ceu_sd_old = ceudev->sd;
> +	ceudev->sd = &ceudev->subdevs[i];
> +
> +	/* Make sure we can generate output image formats. */
> +	ret = ceu_init_formats(ceudev);
> +	if (ret) {
> +		ceudev->sd = ceu_sd_old;
> +		return -EINVAL;
> +	}
> +
> +	/* now that we're sure we can use the sensor, power off the old one. */
> +	v4l2_subdev_call(ceu_sd_old->v4l2_sd, core, s_power, 0);
> +	v4l2_subdev_call(ceudev->sd->v4l2_sd, core, s_power, 1);
> +
> +	ceudev->sd_index = i;
> +
> +	return 0;
> +}
> +
> +static int ceu_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +
> +	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +		return -EINVAL;
> +
> +	return v4l2_subdev_call(ceudev->sd->v4l2_sd, video, g_parm, a);

Look at this v4l2-compliance failure:

fail: v4l2-test-formats.cpp(1162): ret && node->has_frmintervals

This is caused by the fact that the ov7670 driver has this code:

static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
{
        struct v4l2_captureparm *cp = &parms->parm.capture;
        struct ov7670_info *info = to_state(sd);

        if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
                return -EINVAL;

And parms->type is CAPTURE_MPLANE. Just drop this test from the ov7670 driver
in the g/s_parm functions. It shouldn't test for that since a subdev driver
knows nothing about buffer types.

It might be a good idea to check if other subdevs to the same test.

> +}
> +
> +static int ceu_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +
> +	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +		return -EINVAL;
> +
> +	return v4l2_subdev_call(ceudev->sd->v4l2_sd, video, s_parm, a);
> +}
> +
> +static int ceu_enum_framesizes(struct file *file, void *fh,
> +			       struct v4l2_frmsizeenum *fsize)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +	struct ceu_subdev *ceu_sd = ceudev->sd;
> +	const struct ceu_fmt *ceu_fmt;
> +	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
> +	int ret;
> +
> +	struct v4l2_subdev_frame_size_enum fse = {
> +		.code	= ceu_sd->mbus_fmt.mbus_code,
> +		.index	= fsize->index,
> +		.which	= V4L2_SUBDEV_FORMAT_ACTIVE,
> +	};
> +
> +	/* Just check if user supplied pixel format is supported. */
> +	ceu_fmt = get_ceu_fmt_from_fourcc(fsize->pixel_format);
> +	if (!ceu_fmt)
> +		return -EINVAL;
> +
> +	ret = v4l2_subdev_call(v4l2_sd, pad, enum_frame_size,
> +			       NULL, &fse);
> +	if (ret)
> +		return ret;
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> +	fsize->discrete.width = CEU_W_MAX(fse.max_width);
> +	fsize->discrete.height = CEU_H_MAX(fse.max_height);
> +
> +	return 0;
> +}
> +
> +static int ceu_enum_frameintervals(struct file *file, void *fh,
> +				   struct v4l2_frmivalenum *fival)
> +{
> +	struct ceu_device *ceudev = video_drvdata(file);
> +	struct ceu_subdev *ceu_sd = ceudev->sd;
> +	const struct ceu_fmt *ceu_fmt;
> +	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
> +	int ret;
> +
> +	struct v4l2_subdev_frame_interval_enum fie = {
> +		.code	= ceu_sd->mbus_fmt.mbus_code,
> +		.index = fival->index,
> +		.width = fival->width,
> +		.height = fival->height,
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +	};
> +
> +	/* Just check if user supplied pixel format is supported. */
> +	ceu_fmt = get_ceu_fmt_from_fourcc(fival->pixel_format);
> +	if (!ceu_fmt)
> +		return -EINVAL;
> +
> +	ret = v4l2_subdev_call(v4l2_sd, pad, enum_frame_interval, NULL,
> +			       &fie);
> +	if (ret)
> +		return ret;
> +
> +	fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> +	fival->discrete = fie.interval;
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_ioctl_ops ceu_ioctl_ops = {
> +	.vidioc_querycap		= ceu_querycap,
> +
> +	.vidioc_enum_fmt_vid_cap_mplane	= ceu_enum_fmt_vid_cap,
> +	.vidioc_try_fmt_vid_cap_mplane	= ceu_try_fmt_vid_cap,
> +	.vidioc_s_fmt_vid_cap_mplane	= ceu_s_fmt_vid_cap,
> +	.vidioc_g_fmt_vid_cap_mplane	= ceu_g_fmt_vid_cap,
> +
> +	.vidioc_enum_input		= ceu_enum_input,
> +	.vidioc_g_input			= ceu_g_input,
> +	.vidioc_s_input			= ceu_s_input,
> +
> +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> +	.vidioc_querybuf		= vb2_ioctl_querybuf,
> +	.vidioc_qbuf			= vb2_ioctl_qbuf,
> +	.vidioc_expbuf			= vb2_ioctl_expbuf,
> +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
> +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
> +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
> +	.vidioc_streamon		= vb2_ioctl_streamon,
> +	.vidioc_streamoff		= vb2_ioctl_streamoff,
> +
> +	.vidioc_g_parm			= ceu_g_parm,
> +	.vidioc_s_parm			= ceu_s_parm,
> +	.vidioc_enum_framesizes		= ceu_enum_framesizes,
> +	.vidioc_enum_frameintervals	= ceu_enum_frameintervals,

You're missing these ioctls:

        .vidioc_log_status              = v4l2_ctrl_log_status,
        .vidioc_subscribe_event         = v4l2_ctrl_subscribe_event,
        .vidioc_unsubscribe_event       = v4l2_event_unsubscribe,

These missing _event ops are the reason for this compliance failure:

fail: v4l2-test-controls.cpp(782): subscribe event for control 'User Controls' failed

> +};
> +
> +/*
> + * ceu_vdev_release() - release CEU video device memory when last reference
> + *			to this driver is closed
> + */
> +static void ceu_vdev_release(struct video_device *vdev)
> +{
> +	struct ceu_device *ceudev = video_get_drvdata(vdev);
> +
> +	kfree(ceudev);
> +}
> +
> +static int ceu_notify_bound(struct v4l2_async_notifier *notifier,
> +			    struct v4l2_subdev *v4l2_sd,
> +			    struct v4l2_async_subdev *asd)
> +{
> +	struct v4l2_device *v4l2_dev = notifier->v4l2_dev;
> +	struct ceu_device *ceudev = v4l2_to_ceu(v4l2_dev);
> +	struct ceu_subdev *ceu_sd = to_ceu_subdev(asd);
> +
> +	ceu_sd->v4l2_sd = v4l2_sd;
> +	ceudev->num_sd++;
> +
> +	return 0;
> +}
> +
> +static int ceu_notify_complete(struct v4l2_async_notifier *notifier)
> +{
> +	struct v4l2_device *v4l2_dev = notifier->v4l2_dev;
> +	struct ceu_device *ceudev = v4l2_to_ceu(v4l2_dev);
> +	struct video_device *vdev = &ceudev->vdev;
> +	struct vb2_queue *q = &ceudev->vb2_vq;
> +	struct v4l2_subdev *v4l2_sd;
> +	int ret;
> +
> +	/* Initialize vb2 queue. */
> +	q->type			= V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +	q->io_modes		= VB2_MMAP | VB2_DMABUF;
> +	q->drv_priv		= ceudev;
> +	q->ops			= &ceu_vb2_ops;
> +	q->mem_ops		= &vb2_dma_contig_memops;
> +	q->buf_struct_size	= sizeof(struct ceu_buffer);
> +	q->timestamp_flags	= V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	q->min_buffers_needed	= 2;
> +	q->lock			= &ceudev->mlock;
> +	q->dev			= ceudev->v4l2_dev.dev;
> +
> +	ret = vb2_queue_init(q);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Make sure at least one sensor is primary and use it to initialize
> +	 * ceu formats.
> +	 */
> +	if (!ceudev->sd) {
> +		ceudev->sd = &ceudev->subdevs[0];
> +		ceudev->sd_index = 0;
> +	}
> +
> +	v4l2_sd = ceudev->sd->v4l2_sd;
> +
> +	ret = ceu_init_formats(ceudev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ceu_set_default_fmt(ceudev);
> +	if (ret)
> +		return ret;
> +
> +	/* Register the video device. */
> +	strncpy(vdev->name, DRIVER_NAME, strlen(DRIVER_NAME));
> +	vdev->v4l2_dev		= v4l2_dev;
> +	vdev->lock		= &ceudev->mlock;
> +	vdev->queue		= &ceudev->vb2_vq;
> +	vdev->ctrl_handler	= v4l2_sd->ctrl_handler;
> +	vdev->fops		= &ceu_fops;
> +	vdev->ioctl_ops		= &ceu_ioctl_ops;
> +	vdev->release		= ceu_vdev_release;
> +	vdev->device_caps	= V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> +				  V4L2_CAP_STREAMING;
> +	video_set_drvdata(vdev, ceudev);
> +
> +	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> +	if (ret < 0) {
> +		v4l2_err(vdev->v4l2_dev,
> +			 "video_register_device failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_async_notifier_operations ceu_notify_ops = {
> +	.bound		= ceu_notify_bound,
> +	.complete	= ceu_notify_complete,
> +};
> +
> +/*
> + * ceu_init_async_subdevs() - Initialize CEU subdevices and async_subdevs in
> + *			      ceu device. Both DT and platform data parsing use
> + *			      this routine.
> + *
> + * Returns 0 for success, -ENOMEM for failure.
> + */
> +static int ceu_init_async_subdevs(struct ceu_device *ceudev, unsigned int n_sd)
> +{
> +	/* Reserve memory for 'n_sd' ceu_subdev descriptors. */
> +	ceudev->subdevs = devm_kcalloc(ceudev->dev, n_sd,
> +				       sizeof(*ceudev->subdevs), GFP_KERNEL);
> +	if (!ceudev->subdevs)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Reserve memory for 'n_sd' pointers to async_subdevices.
> +	 * ceudev->asds members will point to &ceu_subdev.asd
> +	 */
> +	ceudev->asds = devm_kcalloc(ceudev->dev, n_sd,
> +				    sizeof(*ceudev->asds), GFP_KERNEL);
> +	if (!ceudev->asds)
> +		return -ENOMEM;
> +
> +	ceudev->sd = NULL;
> +	ceudev->sd_index = 0;
> +	ceudev->num_sd = 0;
> +
> +	return 0;
> +}
> +
> +/*
> + * ceu_parse_platform_data() - Initialize async_subdevices using platform
> + *			       device provided data.
> + */
> +static int ceu_parse_platform_data(struct ceu_device *ceudev,
> +				   const struct ceu_platform_data *pdata)
> +{
> +	const struct ceu_async_subdev *async_sd;
> +	struct ceu_subdev *ceu_sd;
> +	unsigned int i;
> +	int ret;
> +
> +	if (pdata->num_subdevs == 0)
> +		return -ENODEV;
> +
> +	ret = ceu_init_async_subdevs(ceudev, pdata->num_subdevs);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < pdata->num_subdevs; i++) {
> +		/* Setup the ceu subdevice and the async subdevice. */
> +		async_sd = &pdata->subdevs[i];
> +		ceu_sd = &ceudev->subdevs[i];
> +
> +		INIT_LIST_HEAD(&ceu_sd->asd.list);
> +
> +		ceu_sd->mbus_flags	= async_sd->flags;
> +		ceu_sd->asd.match_type	= V4L2_ASYNC_MATCH_I2C;
> +		ceu_sd->asd.match.i2c.adapter_id = async_sd->i2c_adapter_id;
> +		ceu_sd->asd.match.i2c.address = async_sd->i2c_address;
> +
> +		ceudev->asds[i] = &ceu_sd->asd;
> +	}
> +
> +	return pdata->num_subdevs;
> +}
> +
> +/*
> + * ceu_parse_dt() - Initialize async_subdevs parsing device tree graph.
> + */
> +static int ceu_parse_dt(struct ceu_device *ceudev)
> +{
> +	struct device_node *of = ceudev->dev->of_node;
> +	struct v4l2_fwnode_endpoint fw_ep;
> +	struct ceu_subdev *ceu_sd;
> +	struct device_node *ep;
> +	unsigned int i;
> +	int num_ep;
> +	int ret;
> +
> +	num_ep = of_graph_get_endpoint_count(of);
> +	if (!num_ep)
> +		return -ENODEV;
> +
> +	ret = ceu_init_async_subdevs(ceudev, num_ep);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < num_ep; i++) {
> +		ep = of_graph_get_endpoint_by_regs(of, 0, i);
> +		if (!ep) {
> +			dev_err(ceudev->dev,
> +				"No subdevice connected on endpoint %u.\n", i);
> +			ret = -ENODEV;
> +			goto error_put_node;
> +		}
> +
> +		ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &fw_ep);
> +		if (ret) {
> +			dev_err(ceudev->dev,
> +				"Unable to parse endpoint #%u.\n", i);
> +			goto error_put_node;
> +		}
> +
> +		if (fw_ep.bus_type != V4L2_MBUS_PARALLEL) {
> +			dev_err(ceudev->dev,
> +				"Only parallel input supported.\n");
> +			ret = -EINVAL;
> +			goto error_put_node;
> +		}
> +
> +		/* Setup the ceu subdevice and the async subdevice. */
> +		ceu_sd = &ceudev->subdevs[i];
> +		INIT_LIST_HEAD(&ceu_sd->asd.list);
> +
> +		ceu_sd->mbus_flags = fw_ep.bus.parallel.flags;
> +		ceu_sd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> +		ceu_sd->asd.match.fwnode.fwnode =
> +			fwnode_graph_get_remote_port_parent(
> +					of_fwnode_handle(ep));
> +
> +		ceudev->asds[i] = &ceu_sd->asd;
> +		of_node_put(ep);
> +	}
> +
> +	return num_ep;
> +
> +error_put_node:
> +	of_node_put(ep);
> +	return ret;
> +}
> +
> +/*
> + * struct ceu_data - Platform specific CEU data
> + * @irq_mask: CETCR mask with all interrupt sources enabled. The mask differs
> + *	      between SH4 and RZ platforms.
> + */
> +struct ceu_data {
> +	u32 irq_mask;
> +};
> +
> +static const struct ceu_data ceu_data_rz = {
> +	.irq_mask = CEU_CETCR_ALL_IRQS_RZ,
> +};
> +
> +static const struct ceu_data ceu_data_sh4 = {
> +	.irq_mask = CEU_CETCR_ALL_IRQS_SH4,
> +};
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id ceu_of_match[] = {
> +	{ .compatible = "renesas,r7s72100-ceu", .data = &ceu_data_rz },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ceu_of_match);
> +#endif
> +
> +static int ceu_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const struct ceu_data *ceu_data;
> +	struct ceu_device *ceudev;
> +	struct resource *res;
> +	unsigned int irq;
> +	int num_subdevs;
> +	int ret;
> +
> +	ceudev = kzalloc(sizeof(*ceudev), GFP_KERNEL);
> +	if (!ceudev)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, ceudev);
> +	ceudev->dev = dev;
> +
> +	INIT_LIST_HEAD(&ceudev->capture);
> +	spin_lock_init(&ceudev->lock);
> +	mutex_init(&ceudev->mlock);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	ceudev->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(ceudev->base))
> +		goto error_free_ceudev;
> +
> +	ret = platform_get_irq(pdev, 0);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to get irq: %d\n", ret);
> +		goto error_free_ceudev;
> +	}
> +	irq = ret;
> +
> +	ret = devm_request_irq(dev, irq, ceu_irq,
> +			       0, dev_name(dev), ceudev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Unable to request CEU interrupt.\n");
> +		goto error_free_ceudev;
> +	}
> +
> +	pm_runtime_enable(dev);
> +
> +	ret = v4l2_device_register(dev, &ceudev->v4l2_dev);
> +	if (ret)
> +		goto error_pm_disable;
> +
> +	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> +		ceu_data = of_match_device(ceu_of_match, dev)->data;
> +		num_subdevs = ceu_parse_dt(ceudev);
> +	} else if (dev->platform_data) {
> +		/* Assume SH4 if booting with platform data. */
> +		ceu_data = &ceu_data_sh4;
> +		num_subdevs = ceu_parse_platform_data(ceudev,
> +						      dev->platform_data);
> +	} else {
> +		num_subdevs = -EINVAL;
> +	}
> +
> +	if (num_subdevs < 0) {
> +		ret = num_subdevs;
> +		goto error_v4l2_unregister;
> +	}
> +	ceudev->irq_mask = ceu_data->irq_mask;
> +
> +	ceudev->notifier.v4l2_dev	= &ceudev->v4l2_dev;
> +	ceudev->notifier.subdevs	= ceudev->asds;
> +	ceudev->notifier.num_subdevs	= num_subdevs;
> +	ceudev->notifier.ops		= &ceu_notify_ops;
> +	ret = v4l2_async_notifier_register(&ceudev->v4l2_dev,
> +					   &ceudev->notifier);
> +	if (ret)
> +		goto error_v4l2_unregister;
> +
> +	dev_info(dev, "Renesas Capture Engine Unit %s\n", dev_name(dev));
> +
> +	return 0;
> +
> +error_v4l2_unregister:
> +	v4l2_device_unregister(&ceudev->v4l2_dev);
> +error_pm_disable:
> +	pm_runtime_disable(dev);
> +error_free_ceudev:
> +	kfree(ceudev);
> +
> +	return ret;
> +}
> +
> +static int ceu_remove(struct platform_device *pdev)
> +{
> +	struct ceu_device *ceudev = platform_get_drvdata(pdev);
> +
> +	pm_runtime_disable(ceudev->dev);
> +
> +	v4l2_async_notifier_unregister(&ceudev->notifier);
> +
> +	v4l2_device_unregister(&ceudev->v4l2_dev);
> +
> +	video_unregister_device(&ceudev->vdev);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops ceu_pm_ops = {
> +	SET_RUNTIME_PM_OPS(ceu_runtime_suspend,
> +			   ceu_runtime_resume,
> +			   NULL)
> +};
> +
> +static struct platform_driver ceu_driver = {
> +	.driver		= {
> +		.name	= DRIVER_NAME,
> +		.pm	= &ceu_pm_ops,
> +		.of_match_table = of_match_ptr(ceu_of_match),
> +	},
> +	.probe		= ceu_probe,
> +	.remove		= ceu_remove,
> +};
> +
> +module_platform_driver(ceu_driver);
> +
> +MODULE_DESCRIPTION("Renesas CEU camera driver");
> +MODULE_AUTHOR("Jacopo Mondi <[e-mail]>");
> +MODULE_LICENSE("GPL v2");
> 

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Laurent Pinchart Jan. 19, 2018, 12:20 p.m. UTC | #2

Hi Hans,

On Friday, 19 January 2018 13:20:19 EET Hans Verkuil wrote:
> On 01/16/18 22:44, Jacopo Mondi wrote:
> > Add driver for Renesas Capture Engine Unit (CEU).
> > 
> > The CEU interface supports capturing 'data' (YUV422) and 'images'
> > (NV[12|21|16|61]).
> > 
> > This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> > 
> > Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> > platform GR-Peach.
> > 
> > Tested with ov7725 camera sensor on SH4 platform Migo-R.
> > 
> > Signed-off-by: Jacopo Mondi <[e-mail]>
> > Reviewed-by: Laurent Pinchart <[e-mail]>
> > ---
> > 
> >  drivers/media/platform/Kconfig       |    9 +
> >  drivers/media/platform/Makefile      |    1 +
> >  drivers/media/platform/renesas-ceu.c | 1659 +++++++++++++++++++++++++++++
> >  3 files changed, 1669 insertions(+)
> >  create mode 100644 drivers/media/platform/renesas-ceu.c

[snip]

> > diff --git a/drivers/media/platform/renesas-ceu.c
> > b/drivers/media/platform/renesas-ceu.c new file mode 100644
> > index 0000000..1b8f0ef
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas-ceu.c

[snip]

> > +static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
> > +				   unsigned int bpl, unsigned int szimage)
> > +{
> > +	if (plane->bytesperline < bpl)
> > +		plane->bytesperline = bpl;
> > +	if (plane->sizeimage < szimage)
> > +		plane->sizeimage = szimage;
> 
> As mentioned in your cover letter, you do need to check for invalid
> bytesperline values. The v4l2-compliance test is to see what happens
> when userspace gives insane values, so yes, drivers need to be able
> to handle that.

What limit would you set, what is an acceptable large value versus an invalid 
large value ? I think we should have rules for this at the API level (or at 
least, if not part of the API, rules that are consistent across drivers).

> plane->sizeimage is set by the driver, so drop the 'if' before the
> assignment.

I don't think that's correct. Userspace should be able to control padding 
lines at the end of the image, the same way it controls padding pixels at the 
end of lines.

> > +}

[snip]

> > +static const struct v4l2_ioctl_ops ceu_ioctl_ops = {
> > +	.vidioc_querycap		= ceu_querycap,
> > +
> > +	.vidioc_enum_fmt_vid_cap_mplane	= ceu_enum_fmt_vid_cap,
> > +	.vidioc_try_fmt_vid_cap_mplane	= ceu_try_fmt_vid_cap,
> > +	.vidioc_s_fmt_vid_cap_mplane	= ceu_s_fmt_vid_cap,
> > +	.vidioc_g_fmt_vid_cap_mplane	= ceu_g_fmt_vid_cap,
> > +
> > +	.vidioc_enum_input		= ceu_enum_input,
> > +	.vidioc_g_input			= ceu_g_input,
> > +	.vidioc_s_input			= ceu_s_input,
> > +
> > +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> > +	.vidioc_querybuf		= vb2_ioctl_querybuf,
> > +	.vidioc_qbuf			= vb2_ioctl_qbuf,
> > +	.vidioc_expbuf			= vb2_ioctl_expbuf,
> > +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
> > +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
> > +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
> > +	.vidioc_streamon		= vb2_ioctl_streamon,
> > +	.vidioc_streamoff		= vb2_ioctl_streamoff,
> > +
> > +	.vidioc_g_parm			= ceu_g_parm,
> > +	.vidioc_s_parm			= ceu_s_parm,
> > +	.vidioc_enum_framesizes		= ceu_enum_framesizes,
> > +	.vidioc_enum_frameintervals	= ceu_enum_frameintervals,
> 
> You're missing these ioctls:
> 
>         .vidioc_log_status              = v4l2_ctrl_log_status,

Is log status mandatory ?

>         .vidioc_subscribe_event         = v4l2_ctrl_subscribe_event,
>         .vidioc_unsubscribe_event       = v4l2_event_unsubscribe,
> 
> These missing _event ops are the reason for this compliance failure:
> 
> fail: v4l2-test-controls.cpp(782): subscribe event for control 'User
> Controls' failed
> > +};

[snip]

Hans Verkuil Jan. 19, 2018, 12:25 p.m. UTC | #3

On 01/19/18 13:20, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday, 19 January 2018 13:20:19 EET Hans Verkuil wrote:
>> On 01/16/18 22:44, Jacopo Mondi wrote:
>>> Add driver for Renesas Capture Engine Unit (CEU).
>>>
>>> The CEU interface supports capturing 'data' (YUV422) and 'images'
>>> (NV[12|21|16|61]).
>>>
>>> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
>>>
>>> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
>>> platform GR-Peach.
>>>
>>> Tested with ov7725 camera sensor on SH4 platform Migo-R.
>>>
>>> Signed-off-by: Jacopo Mondi <[e-mail]>
>>> Reviewed-by: Laurent Pinchart <[e-mail]>
>>> ---
>>>
>>>  drivers/media/platform/Kconfig       |    9 +
>>>  drivers/media/platform/Makefile      |    1 +
>>>  drivers/media/platform/renesas-ceu.c | 1659 +++++++++++++++++++++++++++++
>>>  3 files changed, 1669 insertions(+)
>>>  create mode 100644 drivers/media/platform/renesas-ceu.c
> 
> [snip]
> 
>>> diff --git a/drivers/media/platform/renesas-ceu.c
>>> b/drivers/media/platform/renesas-ceu.c new file mode 100644
>>> index 0000000..1b8f0ef
>>> --- /dev/null
>>> +++ b/drivers/media/platform/renesas-ceu.c
> 
> [snip]
> 
>>> +static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
>>> +				   unsigned int bpl, unsigned int szimage)
>>> +{
>>> +	if (plane->bytesperline < bpl)
>>> +		plane->bytesperline = bpl;
>>> +	if (plane->sizeimage < szimage)
>>> +		plane->sizeimage = szimage;
>>
>> As mentioned in your cover letter, you do need to check for invalid
>> bytesperline values. The v4l2-compliance test is to see what happens
>> when userspace gives insane values, so yes, drivers need to be able
>> to handle that.
> 
> What limit would you set, what is an acceptable large value versus an invalid 
> large value ? I think we should have rules for this at the API level (or at 
> least, if not part of the API, rules that are consistent across drivers).

I would expect this to be the max of what the hardware can support. If
that's really high, then this can be, say, 4 times the width.

Note that there are very few drivers that can handle a user-specified
stride.

>> plane->sizeimage is set by the driver, so drop the 'if' before the
>> assignment.
> 
> I don't think that's correct. Userspace should be able to control padding 
> lines at the end of the image, the same way it controls padding pixels at the 
> end of lines.

If userspace wants larger buffers, then it should use VIDIOC_CREATE_BUFS.

sizeimage is exclusively set by the driver, applications rely on that.

> 
>>> +}
> 
> [snip]
> 
>>> +static const struct v4l2_ioctl_ops ceu_ioctl_ops = {
>>> +	.vidioc_querycap		= ceu_querycap,
>>> +
>>> +	.vidioc_enum_fmt_vid_cap_mplane	= ceu_enum_fmt_vid_cap,
>>> +	.vidioc_try_fmt_vid_cap_mplane	= ceu_try_fmt_vid_cap,
>>> +	.vidioc_s_fmt_vid_cap_mplane	= ceu_s_fmt_vid_cap,
>>> +	.vidioc_g_fmt_vid_cap_mplane	= ceu_g_fmt_vid_cap,
>>> +
>>> +	.vidioc_enum_input		= ceu_enum_input,
>>> +	.vidioc_g_input			= ceu_g_input,
>>> +	.vidioc_s_input			= ceu_s_input,
>>> +
>>> +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
>>> +	.vidioc_querybuf		= vb2_ioctl_querybuf,
>>> +	.vidioc_qbuf			= vb2_ioctl_qbuf,
>>> +	.vidioc_expbuf			= vb2_ioctl_expbuf,
>>> +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
>>> +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
>>> +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
>>> +	.vidioc_streamon		= vb2_ioctl_streamon,
>>> +	.vidioc_streamoff		= vb2_ioctl_streamoff,
>>> +
>>> +	.vidioc_g_parm			= ceu_g_parm,
>>> +	.vidioc_s_parm			= ceu_s_parm,
>>> +	.vidioc_enum_framesizes		= ceu_enum_framesizes,
>>> +	.vidioc_enum_frameintervals	= ceu_enum_frameintervals,
>>
>> You're missing these ioctls:
>>
>>         .vidioc_log_status              = v4l2_ctrl_log_status,
> 
> Is log status mandatory ?

No, but it doesn't hurt to add this one. It comes for free.

> 
>>         .vidioc_subscribe_event         = v4l2_ctrl_subscribe_event,
>>         .vidioc_unsubscribe_event       = v4l2_event_unsubscribe,
>>
>> These missing _event ops are the reason for this compliance failure:
>>
>> fail: v4l2-test-controls.cpp(782): subscribe event for control 'User
>> Controls' failed
>>> +};
> 
> [snip]
> 

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

kernel test robot Jan. 19, 2018, 10:35 p.m. UTC | #4

Hi Jacopo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.15-rc8]
[cannot apply to linuxtv-media/master next-20180119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/Renesas-Capture-Engine-Unit-CEU-V4L2-driver/20180120-053007
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/media/platform/renesas-ceu.c: In function 'ceu_start_streaming':
>> drivers/media/platform/renesas-ceu.c:287:2: warning: 'cdwdr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     iowrite32(data, priv->base + reg_offs);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/renesas-ceu.c:335:27: note: 'cdwdr' was declared here
     u32 camcr, cdocr, cfzsr, cdwdr, capwr;
                              ^~~~~
>> drivers/media/platform/renesas-ceu.c:287:2: warning: 'cfzsr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     iowrite32(data, priv->base + reg_offs);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/renesas-ceu.c:335:20: note: 'cfzsr' was declared here
     u32 camcr, cdocr, cfzsr, cdwdr, capwr;
                       ^~~~~
>> drivers/media/platform/renesas-ceu.c:415:8: warning: 'camcr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     camcr |= mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW ? 1 << 0 : 0;
     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/renesas-ceu.c:335:6: note: 'camcr' was declared here
     u32 camcr, cdocr, cfzsr, cdwdr, capwr;
         ^~~~~
   drivers/media/platform/renesas-ceu.c: In function 'ceu_probe':
>> drivers/media/platform/renesas-ceu.c:1621:9: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return ret;
            ^~~

vim +/cdwdr +287 drivers/media/platform/renesas-ceu.c

   284	
   285	static void ceu_write(struct ceu_device *priv, unsigned int reg_offs, u32 data)
   286	{
 > 287		iowrite32(data, priv->base + reg_offs);
   288	}
   289	
   290	static u32 ceu_read(struct ceu_device *priv, unsigned int reg_offs)
   291	{
   292		return ioread32(priv->base + reg_offs);
   293	}
   294	
   295	/*
   296	 * ceu_soft_reset() - Software reset the CEU interface.
   297	 * @ceu_device: CEU device.
   298	 *
   299	 * Returns 0 for success, -EIO for error.
   300	 */
   301	static int ceu_soft_reset(struct ceu_device *ceudev)
   302	{
   303		unsigned int i;
   304	
   305		ceu_write(ceudev, CEU_CAPSR, CEU_CAPSR_CPKIL);
   306	
   307		for (i = 0; i < 100; i++) {
   308			if (!(ceu_read(ceudev, CEU_CSTSR) & CEU_CSTRST_CPTON))
   309				break;
   310			udelay(1);
   311		}
   312	
   313		if (i == 100) {
   314			dev_err(ceudev->dev, "soft reset time out\n");
   315			return -EIO;
   316		}
   317	
   318		for (i = 0; i < 100; i++) {
   319			if (!(ceu_read(ceudev, CEU_CAPSR) & CEU_CAPSR_CPKIL))
   320				return 0;
   321			udelay(1);
   322		}
   323	
   324		/* If we get here, CEU has not reset properly. */
   325		return -EIO;
   326	}
   327	
   328	/* --- CEU Capture Operations --- */
   329	
   330	/*
   331	 * ceu_hw_config() - Configure CEU interface registers.
   332	 */
   333	static int ceu_hw_config(struct ceu_device *ceudev)
   334	{
 > 335		u32 camcr, cdocr, cfzsr, cdwdr, capwr;
   336		struct v4l2_pix_format_mplane *pix = &ceudev->v4l2_pix;
   337		struct ceu_subdev *ceu_sd = ceudev->sd;
   338		struct ceu_mbus_fmt *mbus_fmt = &ceu_sd->mbus_fmt;
   339		unsigned int mbus_flags = ceu_sd->mbus_flags;
   340	
   341		/* Start configuring CEU registers */
   342		ceu_write(ceudev, CEU_CAIFR, 0);
   343		ceu_write(ceudev, CEU_CFWCR, 0);
   344		ceu_write(ceudev, CEU_CRCNTR, 0);
   345		ceu_write(ceudev, CEU_CRCMPR, 0);
   346	
   347		/* Set the frame capture period for both image capture and data sync. */
   348		capwr = (pix->height << 16) | pix->width * mbus_fmt->bpp / 8;
   349	
   350		/*
   351		 * Swap input data endianness by default.
   352		 * In data fetch mode bytes are received in chunks of 8 bytes.
   353		 * D0, D1, D2, D3, D4, D5, D6, D7 (D0 received first)
   354		 * The data is however by default written to memory in reverse order:
   355		 * D7, D6, D5, D4, D3, D2, D1, D0 (D7 written to lowest byte)
   356		 *
   357		 * Use CEU_CDOCR[2:0] to swap data ordering.
   358		 */
   359		cdocr = CEU_CDOCR_SWAP_ENDIANNESS;
   360	
   361		/*
   362		 * Configure CAMCR and CDOCR:
   363		 * match input components ordering with memory output format and
   364		 * handle downsampling to YUV420.
   365		 *
   366		 * If the memory output planar format is 'swapped' (Cr before Cb) and
   367		 * input format is not, use the swapped version of CAMCR.DTARY.
   368		 *
   369		 * If the memory output planar format is not 'swapped' (Cb before Cr)
   370		 * and input format is, use the swapped version of CAMCR.DTARY.
   371		 *
   372		 * CEU by default downsample to planar YUV420 (CDCOR[4] = 0).
   373		 * If output is planar YUV422 set CDOCR[4] = 1
   374		 *
   375		 * No downsample for data fetch sync mode.
   376		 */
   377		switch (pix->pixelformat) {
   378		/* Data fetch sync mode */
   379		case V4L2_PIX_FMT_YUYV:
   380			/* TODO: handle YUYV permutations through DTARY bits. */
   381			camcr	= CEU_CAMCR_JPEG;
   382			cdocr	|= CEU_CDOCR_NO_DOWSAMPLE;
   383			cfzsr	= (pix->height << 16) | pix->width;
   384			cdwdr	= pix->plane_fmt[0].bytesperline;
   385			break;
   386	
   387		/* Non-swapped planar image capture mode. */
   388		case V4L2_PIX_FMT_NV16:
   389			cdocr	|= CEU_CDOCR_NO_DOWSAMPLE;
   390		case V4L2_PIX_FMT_NV12:
   391			if (mbus_fmt->swapped)
   392				camcr = mbus_fmt->fmt_order_swap;
   393			else
   394				camcr = mbus_fmt->fmt_order;
   395	
   396			cfzsr	= (pix->height << 16) | pix->width;
   397			cdwdr	= pix->width;
   398			break;
   399	
   400		/* Swapped planar image capture mode. */
   401		case V4L2_PIX_FMT_NV61:
   402			cdocr	|= CEU_CDOCR_NO_DOWSAMPLE;
   403		case V4L2_PIX_FMT_NV21:
   404			if (mbus_fmt->swapped)
   405				camcr = mbus_fmt->fmt_order;
   406			else
   407				camcr = mbus_fmt->fmt_order_swap;
   408	
   409			cfzsr	= (pix->height << 16) | pix->width;
   410			cdwdr	= pix->width;
   411			break;
   412		}
   413	
   414		camcr |= mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW ? 1 << 1 : 0;
 > 415		camcr |= mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW ? 1 << 0 : 0;
   416	
   417		/* TODO: handle 16 bit bus width with DTIF bit in CAMCR */
   418		ceu_write(ceudev, CEU_CAMCR, camcr);
   419		ceu_write(ceudev, CEU_CDOCR, cdocr);
   420		ceu_write(ceudev, CEU_CAPCR, CEU_CAPCR_BUS_WIDTH256);
   421	
   422		/*
   423		 * TODO: make CAMOR offsets configurable.
   424		 * CAMOR wants to know the number of blanks between a VS/HS signal
   425		 * and valid data. This value should actually come from the sensor...
   426		 */
   427		ceu_write(ceudev, CEU_CAMOR, 0);
   428	
   429		/* TODO: 16 bit bus width require re-calculation of cdwdr and cfzsr */
   430		ceu_write(ceudev, CEU_CAPWR, capwr);
   431		ceu_write(ceudev, CEU_CFSZR, cfzsr);
   432		ceu_write(ceudev, CEU_CDWDR, cdwdr);
   433	
   434		return 0;
   435	}
   436	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

kernel test robot Jan. 20, 2018, 2:11 a.m. UTC | #5

Hi Jacopo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.15-rc8]
[cannot apply to linuxtv-media/master next-20180119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/Renesas-Capture-Engine-Unit-CEU-V4L2-driver/20180120-053007
config: mn10300-allmodconfig (attached as .config)
compiler: am33_2.0-linux-gcc (GCC) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mn10300 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from include/linux/scatterlist.h:9:0,
                    from include/linux/dma-mapping.h:11,
                    from drivers/media/platform/renesas-ceu.c:16:
   drivers/media/platform/renesas-ceu.c: In function 'ceu_start_streaming':
>> arch/mn10300/include/asm/io.h:63:25: warning: 'cdwdr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     *(volatile u32 *) addr = b;
     ~~~~~~~~~~~~~~~~~~~~~~~^~~
   drivers/media/platform/renesas-ceu.c:335:27: note: 'cdwdr' was declared here
     u32 camcr, cdocr, cfzsr, cdwdr, capwr;
                              ^~~~~
   In file included from include/linux/scatterlist.h:9:0,
                    from include/linux/dma-mapping.h:11,
                    from drivers/media/platform/renesas-ceu.c:16:
>> arch/mn10300/include/asm/io.h:63:25: warning: 'cfzsr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     *(volatile u32 *) addr = b;
     ~~~~~~~~~~~~~~~~~~~~~~~^~~
   drivers/media/platform/renesas-ceu.c:335:20: note: 'cfzsr' was declared here
     u32 camcr, cdocr, cfzsr, cdwdr, capwr;
                       ^~~~~
   drivers/media/platform/renesas-ceu.c:415:8: warning: 'camcr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     camcr |= mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW ? 1 << 0 : 0;
     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/renesas-ceu.c:335:6: note: 'camcr' was declared here
     u32 camcr, cdocr, cfzsr, cdwdr, capwr;
         ^~~~~
   drivers/media/platform/renesas-ceu.c: In function 'ceu_probe':
   drivers/media/platform/renesas-ceu.c:1621:9: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return ret;
            ^~~
--
   In file included from include/linux/scatterlist.h:9:0,
                    from include/linux/dma-mapping.h:11,
                    from drivers/media//platform/renesas-ceu.c:16:
   drivers/media//platform/renesas-ceu.c: In function 'ceu_start_streaming':
>> arch/mn10300/include/asm/io.h:63:25: warning: 'cdwdr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     *(volatile u32 *) addr = b;
     ~~~~~~~~~~~~~~~~~~~~~~~^~~
   drivers/media//platform/renesas-ceu.c:335:27: note: 'cdwdr' was declared here
     u32 camcr, cdocr, cfzsr, cdwdr, capwr;
                              ^~~~~
   In file included from include/linux/scatterlist.h:9:0,
                    from include/linux/dma-mapping.h:11,
                    from drivers/media//platform/renesas-ceu.c:16:
>> arch/mn10300/include/asm/io.h:63:25: warning: 'cfzsr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     *(volatile u32 *) addr = b;
     ~~~~~~~~~~~~~~~~~~~~~~~^~~
   drivers/media//platform/renesas-ceu.c:335:20: note: 'cfzsr' was declared here
     u32 camcr, cdocr, cfzsr, cdwdr, capwr;
                       ^~~~~
   drivers/media//platform/renesas-ceu.c:415:8: warning: 'camcr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     camcr |= mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW ? 1 << 0 : 0;
     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media//platform/renesas-ceu.c:335:6: note: 'camcr' was declared here
     u32 camcr, cdocr, cfzsr, cdwdr, capwr;
         ^~~~~
   drivers/media//platform/renesas-ceu.c: In function 'ceu_probe':
   drivers/media//platform/renesas-ceu.c:1621:9: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return ret;
            ^~~

vim +/cdwdr +63 arch/mn10300/include/asm/io.h

b920de1b include/asm-mn10300/io.h David Howells 2008-02-08  60  
b920de1b include/asm-mn10300/io.h David Howells 2008-02-08  61  static inline void writel(u32 b, volatile void __iomem *addr)
b920de1b include/asm-mn10300/io.h David Howells 2008-02-08  62  {
b920de1b include/asm-mn10300/io.h David Howells 2008-02-08 @63  	*(volatile u32 *) addr = b;
b920de1b include/asm-mn10300/io.h David Howells 2008-02-08  64  }
b920de1b include/asm-mn10300/io.h David Howells 2008-02-08  65  

:::::: The code at line 63 was first introduced by commit
:::::: b920de1b77b72ca9432ac3f97edb26541e65e5dd mn10300: add the MN10300/AM33 architecture to the kernel

:::::: TO: David Howells <[e-mail]>
:::::: CC: Linus Torvalds <[e-mail]>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Jacopo Mondi Jan. 21, 2018, 9:53 a.m. UTC | #6

Hi Hans,

On Fri, Jan 19, 2018 at 12:20:19PM +0100, Hans Verkuil wrote:
> Some more comments:
>

[snip]

> > +/* --- CEU Videobuf2 operations --- */
> > +
> > +static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
> > +				   unsigned int bpl, unsigned int szimage)
> > +{
> > +	if (plane->bytesperline < bpl)
> > +		plane->bytesperline = bpl;
> > +	if (plane->sizeimage < szimage)
> > +		plane->sizeimage = szimage;
>
> As mentioned in your cover letter, you do need to check for invalid
> bytesperline values. The v4l2-compliance test is to see what happens
> when userspace gives insane values, so yes, drivers need to be able
> to handle that.
>
> plane->sizeimage is set by the driver, so drop the 'if' before the
> assignment.

According to what suggested by you and Laurent I'll limit the h size
to the maximum value supported by the HW (I didn't notice this limit was
specified in the HW manual, and it's set to 8188 bytes).

And I will set sizeimage unconditionally.

> >

[snip]

> > +
> > +static int ceu_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> > +{
> > +	struct ceu_device *ceudev = video_drvdata(file);
> > +
> > +	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > +		return -EINVAL;
> > +
> > +	return v4l2_subdev_call(ceudev->sd->v4l2_sd, video, g_parm, a);
>
> Look at this v4l2-compliance failure:
>
> fail: v4l2-test-formats.cpp(1162): ret && node->has_frmintervals
>
> This is caused by the fact that the ov7670 driver has this code:
>
> static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> {
>         struct v4l2_captureparm *cp = &parms->parm.capture;
>         struct ov7670_info *info = to_state(sd);
>
>         if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>                 return -EINVAL;
>
> And parms->type is CAPTURE_MPLANE. Just drop this test from the ov7670 driver
> in the g/s_parm functions. It shouldn't test for that since a subdev driver
> knows nothing about buffer types.
>

I will drop that test in an additional patch part of next iteration of this series,

> It might be a good idea to check if other subdevs to the same test.
>
> > +static const struct v4l2_ioctl_ops ceu_ioctl_ops = {
> > +	.vidioc_querycap		= ceu_querycap,
> > +
> > +	.vidioc_enum_fmt_vid_cap_mplane	= ceu_enum_fmt_vid_cap,
> > +	.vidioc_try_fmt_vid_cap_mplane	= ceu_try_fmt_vid_cap,
> > +	.vidioc_s_fmt_vid_cap_mplane	= ceu_s_fmt_vid_cap,
> > +	.vidioc_g_fmt_vid_cap_mplane	= ceu_g_fmt_vid_cap,
> > +
> > +	.vidioc_enum_input		= ceu_enum_input,
> > +	.vidioc_g_input			= ceu_g_input,
> > +	.vidioc_s_input			= ceu_s_input,
> > +
> > +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> > +	.vidioc_querybuf		= vb2_ioctl_querybuf,
> > +	.vidioc_qbuf			= vb2_ioctl_qbuf,
> > +	.vidioc_expbuf			= vb2_ioctl_expbuf,
> > +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
> > +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
> > +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
> > +	.vidioc_streamon		= vb2_ioctl_streamon,
> > +	.vidioc_streamoff		= vb2_ioctl_streamoff,
> > +
> > +	.vidioc_g_parm			= ceu_g_parm,
> > +	.vidioc_s_parm			= ceu_s_parm,
> > +	.vidioc_enum_framesizes		= ceu_enum_framesizes,
> > +	.vidioc_enum_frameintervals	= ceu_enum_frameintervals,
>
> You're missing these ioctls:
>
>         .vidioc_log_status              = v4l2_ctrl_log_status,
>         .vidioc_subscribe_event         = v4l2_ctrl_subscribe_event,
>         .vidioc_unsubscribe_event       = v4l2_event_unsubscribe,
>
> These missing _event ops are the reason for this compliance failure:
>
> fail: v4l2-test-controls.cpp(782): subscribe event for control 'User Controls' failed
>

Seems like they al comes almost for free! I will add them.

Thanks
   j

> > +};
> > +
> > +/*
> > + * ceu_vdev_release() - release CEU video device memory when last reference
> > + *			to this driver is closed
> > + */
> > +static void ceu_vdev_release(struct video_device *vdev)
> > +{
> > +	struct ceu_device *ceudev = video_get_drvdata(vdev);
> > +
> > +	kfree(ceudev);
> > +}
> > +
> > +static int ceu_notify_bound(struct v4l2_async_notifier *notifier,
> > +			    struct v4l2_subdev *v4l2_sd,
> > +			    struct v4l2_async_subdev *asd)
> > +{
> > +	struct v4l2_device *v4l2_dev = notifier->v4l2_dev;
> > +	struct ceu_device *ceudev = v4l2_to_ceu(v4l2_dev);
> > +	struct ceu_subdev *ceu_sd = to_ceu_subdev(asd);
> > +
> > +	ceu_sd->v4l2_sd = v4l2_sd;
> > +	ceudev->num_sd++;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ceu_notify_complete(struct v4l2_async_notifier *notifier)
> > +{
> > +	struct v4l2_device *v4l2_dev = notifier->v4l2_dev;
> > +	struct ceu_device *ceudev = v4l2_to_ceu(v4l2_dev);
> > +	struct video_device *vdev = &ceudev->vdev;
> > +	struct vb2_queue *q = &ceudev->vb2_vq;
> > +	struct v4l2_subdev *v4l2_sd;
> > +	int ret;
> > +
> > +	/* Initialize vb2 queue. */
> > +	q->type			= V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +	q->io_modes		= VB2_MMAP | VB2_DMABUF;
> > +	q->drv_priv		= ceudev;
> > +	q->ops			= &ceu_vb2_ops;
> > +	q->mem_ops		= &vb2_dma_contig_memops;
> > +	q->buf_struct_size	= sizeof(struct ceu_buffer);
> > +	q->timestamp_flags	= V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > +	q->min_buffers_needed	= 2;
> > +	q->lock			= &ceudev->mlock;
> > +	q->dev			= ceudev->v4l2_dev.dev;
> > +
> > +	ret = vb2_queue_init(q);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/*
> > +	 * Make sure at least one sensor is primary and use it to initialize
> > +	 * ceu formats.
> > +	 */
> > +	if (!ceudev->sd) {
> > +		ceudev->sd = &ceudev->subdevs[0];
> > +		ceudev->sd_index = 0;
> > +	}
> > +
> > +	v4l2_sd = ceudev->sd->v4l2_sd;
> > +
> > +	ret = ceu_init_formats(ceudev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = ceu_set_default_fmt(ceudev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Register the video device. */
> > +	strncpy(vdev->name, DRIVER_NAME, strlen(DRIVER_NAME));
> > +	vdev->v4l2_dev		= v4l2_dev;
> > +	vdev->lock		= &ceudev->mlock;
> > +	vdev->queue		= &ceudev->vb2_vq;
> > +	vdev->ctrl_handler	= v4l2_sd->ctrl_handler;
> > +	vdev->fops		= &ceu_fops;
> > +	vdev->ioctl_ops		= &ceu_ioctl_ops;
> > +	vdev->release		= ceu_vdev_release;
> > +	vdev->device_caps	= V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> > +				  V4L2_CAP_STREAMING;
> > +	video_set_drvdata(vdev, ceudev);
> > +
> > +	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> > +	if (ret < 0) {
> > +		v4l2_err(vdev->v4l2_dev,
> > +			 "video_register_device failed: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct v4l2_async_notifier_operations ceu_notify_ops = {
> > +	.bound		= ceu_notify_bound,
> > +	.complete	= ceu_notify_complete,
> > +};
> > +
> > +/*
> > + * ceu_init_async_subdevs() - Initialize CEU subdevices and async_subdevs in
> > + *			      ceu device. Both DT and platform data parsing use
> > + *			      this routine.
> > + *
> > + * Returns 0 for success, -ENOMEM for failure.
> > + */
> > +static int ceu_init_async_subdevs(struct ceu_device *ceudev, unsigned int n_sd)
> > +{
> > +	/* Reserve memory for 'n_sd' ceu_subdev descriptors. */
> > +	ceudev->subdevs = devm_kcalloc(ceudev->dev, n_sd,
> > +				       sizeof(*ceudev->subdevs), GFP_KERNEL);
> > +	if (!ceudev->subdevs)
> > +		return -ENOMEM;
> > +
> > +	/*
> > +	 * Reserve memory for 'n_sd' pointers to async_subdevices.
> > +	 * ceudev->asds members will point to &ceu_subdev.asd
> > +	 */
> > +	ceudev->asds = devm_kcalloc(ceudev->dev, n_sd,
> > +				    sizeof(*ceudev->asds), GFP_KERNEL);
> > +	if (!ceudev->asds)
> > +		return -ENOMEM;
> > +
> > +	ceudev->sd = NULL;
> > +	ceudev->sd_index = 0;
> > +	ceudev->num_sd = 0;
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * ceu_parse_platform_data() - Initialize async_subdevices using platform
> > + *			       device provided data.
> > + */
> > +static int ceu_parse_platform_data(struct ceu_device *ceudev,
> > +				   const struct ceu_platform_data *pdata)
> > +{
> > +	const struct ceu_async_subdev *async_sd;
> > +	struct ceu_subdev *ceu_sd;
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	if (pdata->num_subdevs == 0)
> > +		return -ENODEV;
> > +
> > +	ret = ceu_init_async_subdevs(ceudev, pdata->num_subdevs);
> > +	if (ret)
> > +		return ret;
> > +
> > +	for (i = 0; i < pdata->num_subdevs; i++) {
> > +		/* Setup the ceu subdevice and the async subdevice. */
> > +		async_sd = &pdata->subdevs[i];
> > +		ceu_sd = &ceudev->subdevs[i];
> > +
> > +		INIT_LIST_HEAD(&ceu_sd->asd.list);
> > +
> > +		ceu_sd->mbus_flags	= async_sd->flags;
> > +		ceu_sd->asd.match_type	= V4L2_ASYNC_MATCH_I2C;
> > +		ceu_sd->asd.match.i2c.adapter_id = async_sd->i2c_adapter_id;
> > +		ceu_sd->asd.match.i2c.address = async_sd->i2c_address;
> > +
> > +		ceudev->asds[i] = &ceu_sd->asd;
> > +	}
> > +
> > +	return pdata->num_subdevs;
> > +}
> > +
> > +/*
> > + * ceu_parse_dt() - Initialize async_subdevs parsing device tree graph.
> > + */
> > +static int ceu_parse_dt(struct ceu_device *ceudev)
> > +{
> > +	struct device_node *of = ceudev->dev->of_node;
> > +	struct v4l2_fwnode_endpoint fw_ep;
> > +	struct ceu_subdev *ceu_sd;
> > +	struct device_node *ep;
> > +	unsigned int i;
> > +	int num_ep;
> > +	int ret;
> > +
> > +	num_ep = of_graph_get_endpoint_count(of);
> > +	if (!num_ep)
> > +		return -ENODEV;
> > +
> > +	ret = ceu_init_async_subdevs(ceudev, num_ep);
> > +	if (ret)
> > +		return ret;
> > +
> > +	for (i = 0; i < num_ep; i++) {
> > +		ep = of_graph_get_endpoint_by_regs(of, 0, i);
> > +		if (!ep) {
> > +			dev_err(ceudev->dev,
> > +				"No subdevice connected on endpoint %u.\n", i);
> > +			ret = -ENODEV;
> > +			goto error_put_node;
> > +		}
> > +
> > +		ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &fw_ep);
> > +		if (ret) {
> > +			dev_err(ceudev->dev,
> > +				"Unable to parse endpoint #%u.\n", i);
> > +			goto error_put_node;
> > +		}
> > +
> > +		if (fw_ep.bus_type != V4L2_MBUS_PARALLEL) {
> > +			dev_err(ceudev->dev,
> > +				"Only parallel input supported.\n");
> > +			ret = -EINVAL;
> > +			goto error_put_node;
> > +		}
> > +
> > +		/* Setup the ceu subdevice and the async subdevice. */
> > +		ceu_sd = &ceudev->subdevs[i];
> > +		INIT_LIST_HEAD(&ceu_sd->asd.list);
> > +
> > +		ceu_sd->mbus_flags = fw_ep.bus.parallel.flags;
> > +		ceu_sd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> > +		ceu_sd->asd.match.fwnode.fwnode =
> > +			fwnode_graph_get_remote_port_parent(
> > +					of_fwnode_handle(ep));
> > +
> > +		ceudev->asds[i] = &ceu_sd->asd;
> > +		of_node_put(ep);
> > +	}
> > +
> > +	return num_ep;
> > +
> > +error_put_node:
> > +	of_node_put(ep);
> > +	return ret;
> > +}
> > +
> > +/*
> > + * struct ceu_data - Platform specific CEU data
> > + * @irq_mask: CETCR mask with all interrupt sources enabled. The mask differs
> > + *	      between SH4 and RZ platforms.
> > + */
> > +struct ceu_data {
> > +	u32 irq_mask;
> > +};
> > +
> > +static const struct ceu_data ceu_data_rz = {
> > +	.irq_mask = CEU_CETCR_ALL_IRQS_RZ,
> > +};
> > +
> > +static const struct ceu_data ceu_data_sh4 = {
> > +	.irq_mask = CEU_CETCR_ALL_IRQS_SH4,
> > +};
> > +
> > +#if IS_ENABLED(CONFIG_OF)
> > +static const struct of_device_id ceu_of_match[] = {
> > +	{ .compatible = "renesas,r7s72100-ceu", .data = &ceu_data_rz },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, ceu_of_match);
> > +#endif
> > +
> > +static int ceu_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	const struct ceu_data *ceu_data;
> > +	struct ceu_device *ceudev;
> > +	struct resource *res;
> > +	unsigned int irq;
> > +	int num_subdevs;
> > +	int ret;
> > +
> > +	ceudev = kzalloc(sizeof(*ceudev), GFP_KERNEL);
> > +	if (!ceudev)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, ceudev);
> > +	ceudev->dev = dev;
> > +
> > +	INIT_LIST_HEAD(&ceudev->capture);
> > +	spin_lock_init(&ceudev->lock);
> > +	mutex_init(&ceudev->mlock);
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	ceudev->base = devm_ioremap_resource(dev, res);
> > +	if (IS_ERR(ceudev->base))
> > +		goto error_free_ceudev;
> > +
> > +	ret = platform_get_irq(pdev, 0);
> > +	if (ret < 0) {
> > +		dev_err(dev, "Failed to get irq: %d\n", ret);
> > +		goto error_free_ceudev;
> > +	}
> > +	irq = ret;
> > +
> > +	ret = devm_request_irq(dev, irq, ceu_irq,
> > +			       0, dev_name(dev), ceudev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Unable to request CEU interrupt.\n");
> > +		goto error_free_ceudev;
> > +	}
> > +
> > +	pm_runtime_enable(dev);
> > +
> > +	ret = v4l2_device_register(dev, &ceudev->v4l2_dev);
> > +	if (ret)
> > +		goto error_pm_disable;
> > +
> > +	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> > +		ceu_data = of_match_device(ceu_of_match, dev)->data;
> > +		num_subdevs = ceu_parse_dt(ceudev);
> > +	} else if (dev->platform_data) {
> > +		/* Assume SH4 if booting with platform data. */
> > +		ceu_data = &ceu_data_sh4;
> > +		num_subdevs = ceu_parse_platform_data(ceudev,
> > +						      dev->platform_data);
> > +	} else {
> > +		num_subdevs = -EINVAL;
> > +	}
> > +
> > +	if (num_subdevs < 0) {
> > +		ret = num_subdevs;
> > +		goto error_v4l2_unregister;
> > +	}
> > +	ceudev->irq_mask = ceu_data->irq_mask;
> > +
> > +	ceudev->notifier.v4l2_dev	= &ceudev->v4l2_dev;
> > +	ceudev->notifier.subdevs	= ceudev->asds;
> > +	ceudev->notifier.num_subdevs	= num_subdevs;
> > +	ceudev->notifier.ops		= &ceu_notify_ops;
> > +	ret = v4l2_async_notifier_register(&ceudev->v4l2_dev,
> > +					   &ceudev->notifier);
> > +	if (ret)
> > +		goto error_v4l2_unregister;
> > +
> > +	dev_info(dev, "Renesas Capture Engine Unit %s\n", dev_name(dev));
> > +
> > +	return 0;
> > +
> > +error_v4l2_unregister:
> > +	v4l2_device_unregister(&ceudev->v4l2_dev);
> > +error_pm_disable:
> > +	pm_runtime_disable(dev);
> > +error_free_ceudev:
> > +	kfree(ceudev);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ceu_remove(struct platform_device *pdev)
> > +{
> > +	struct ceu_device *ceudev = platform_get_drvdata(pdev);
> > +
> > +	pm_runtime_disable(ceudev->dev);
> > +
> > +	v4l2_async_notifier_unregister(&ceudev->notifier);
> > +
> > +	v4l2_device_unregister(&ceudev->v4l2_dev);
> > +
> > +	video_unregister_device(&ceudev->vdev);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct dev_pm_ops ceu_pm_ops = {
> > +	SET_RUNTIME_PM_OPS(ceu_runtime_suspend,
> > +			   ceu_runtime_resume,
> > +			   NULL)
> > +};
> > +
> > +static struct platform_driver ceu_driver = {
> > +	.driver		= {
> > +		.name	= DRIVER_NAME,
> > +		.pm	= &ceu_pm_ops,
> > +		.of_match_table = of_match_ptr(ceu_of_match),
> > +	},
> > +	.probe		= ceu_probe,
> > +	.remove		= ceu_remove,
> > +};
> > +
> > +module_platform_driver(ceu_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas CEU camera driver");
> > +MODULE_AUTHOR("Jacopo Mondi <[e-mail]>");
> > +MODULE_LICENSE("GPL v2");
> >
>
> Regards,
>
> 	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hans Verkuil Jan. 21, 2018, 10:21 a.m. UTC | #7

On 21/01/18 10:53, jacopo mondi wrote:
> Hi Hans,
> 
> On Fri, Jan 19, 2018 at 12:20:19PM +0100, Hans Verkuil wrote:
>> static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>> {
>>         struct v4l2_captureparm *cp = &parms->parm.capture;
>>         struct ov7670_info *info = to_state(sd);
>>
>>         if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>>                 return -EINVAL;
>>
>> And parms->type is CAPTURE_MPLANE. Just drop this test from the ov7670 driver
>> in the g/s_parm functions. It shouldn't test for that since a subdev driver
>> knows nothing about buffer types.
>>
> 
> I will drop that test in an additional patch part of next iteration of this series,

Replace g/s_parm by g/s_frame_interval. Consider g/s_parm for subdev drivers as
deprecated (I'm working on a patch series to replace all g/s_parm uses by
g/s_frame_interval).

Regards,

	Hans

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hans Verkuil Jan. 21, 2018, 10:23 a.m. UTC | #8

On 21/01/18 11:21, Hans Verkuil wrote:
> On 21/01/18 10:53, jacopo mondi wrote:
>> Hi Hans,
>>
>> On Fri, Jan 19, 2018 at 12:20:19PM +0100, Hans Verkuil wrote:
>>> static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>>> {
>>>         struct v4l2_captureparm *cp = &parms->parm.capture;
>>>         struct ov7670_info *info = to_state(sd);
>>>
>>>         if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>>>                 return -EINVAL;
>>>
>>> And parms->type is CAPTURE_MPLANE. Just drop this test from the ov7670 driver
>>> in the g/s_parm functions. It shouldn't test for that since a subdev driver
>>> knows nothing about buffer types.
>>>
>>
>> I will drop that test in an additional patch part of next iteration of this series,
> 
> Replace g/s_parm by g/s_frame_interval. Consider g/s_parm for subdev drivers as
> deprecated (I'm working on a patch series to replace all g/s_parm uses by
> g/s_frame_interval).

Take a look here:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=parm

You probably want to use the patch 'v4l2-common: add g/s_parm helper functions'
for the new ceu driver in your patch series. Feel free to add it.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jacopo Mondi Jan. 21, 2018, 5:29 p.m. UTC | #9

Hi Hans,

On Sun, Jan 21, 2018 at 11:23:12AM +0100, Hans Verkuil wrote:
> On 21/01/18 11:21, Hans Verkuil wrote:
> > On 21/01/18 10:53, jacopo mondi wrote:
> >> Hi Hans,
> >>
> >> On Fri, Jan 19, 2018 at 12:20:19PM +0100, Hans Verkuil wrote:
> >>> static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> >>> {
> >>>         struct v4l2_captureparm *cp = &parms->parm.capture;
> >>>         struct ov7670_info *info = to_state(sd);
> >>>
> >>>         if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >>>                 return -EINVAL;
> >>>
> >>> And parms->type is CAPTURE_MPLANE. Just drop this test from the ov7670 driver
> >>> in the g/s_parm functions. It shouldn't test for that since a subdev driver
> >>> knows nothing about buffer types.
> >>>
> >>
> >> I will drop that test in an additional patch part of next iteration of this series,
> >
> > Replace g/s_parm by g/s_frame_interval. Consider g/s_parm for subdev drivers as
> > deprecated (I'm working on a patch series to replace all g/s_parm uses by
> > g/s_frame_interval).
>
> Take a look here:
>
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=parm
>
> You probably want to use the patch 'v4l2-common: add g/s_parm helper functions'
> for the new ceu driver in your patch series. Feel free to add it.

Thanks, I have now re-based my series on top of your 'parm' branch,
and now I have silenced those errors on bad frame interval.

CEU g/s_parm now look like this:

static int ceu_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
{
	struct ceu_device *ceudev = video_drvdata(file);
	int ret;

	ret = v4l2_g_parm(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
			  ceudev->sd->v4l2_sd, a);
	if (ret)
		return ret;

	a->parm.capture.readbuffers = 0;

	return 0;
}

Very similar to what you've done on other platform drivers in this
commit:
https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=parm&id=a58956ef45cebaa5ce43a5f740fe04517b24a853

I have a question though (please bear with me a little more :)
I had to manually set a->parm.capture.readbuffers to 0 to silence the following
error in v4l2_compliance (which I have now updated to the most recent
remote HEAD):

 fail: v4l2-test-formats.cpp(1114): cap->readbuffers
                test VIDIOC_G/S_PARM: FAIL

		fail_on_test(cap->readbuffers > VIDEO_MAX_FRAME);
		if (!(node->g_caps() & V4L2_CAP_READWRITE))
			fail_on_test(cap->readbuffers);
		else if (node->g_caps() & V4L2_CAP_STREAMING)
			fail_on_test(!cap->readbuffers);

CEU does not support CAP_READWRITE, as it seems atmel-isc/isi do not, so
v4l2-compliance wants to have readbuffers set to 0. I wonder why in
the previously mentioned commit you didn't have to set readbuffers
explicitly to 0 for atmel-isc/isi as I had to for CEU. Will v4l2-compliance
fail if run on atmel-isc/isi with your commit, or am I missing something?

Thanks
   j
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Laurent Pinchart Jan. 22, 2018, 12:52 a.m. UTC | #10

Hi Hans,

On Friday, 19 January 2018 14:25:39 EET Hans Verkuil wrote:
> On 01/19/18 13:20, Laurent Pinchart wrote:
> > On Friday, 19 January 2018 13:20:19 EET Hans Verkuil wrote:
> >> On 01/16/18 22:44, Jacopo Mondi wrote:
> >>> Add driver for Renesas Capture Engine Unit (CEU).
> >>> 
> >>> The CEU interface supports capturing 'data' (YUV422) and 'images'
> >>> (NV[12|21|16|61]).
> >>> 
> >>> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> >>> 
> >>> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> >>> platform GR-Peach.
> >>> 
> >>> Tested with ov7725 camera sensor on SH4 platform Migo-R.
> >>> 
> >>> Signed-off-by: Jacopo Mondi <[e-mail]>
> >>> Reviewed-by: Laurent Pinchart <[e-mail]>
> >>> ---
> >>> 
> >>>  drivers/media/platform/Kconfig       |    9 +
> >>>  drivers/media/platform/Makefile      |    1 +
> >>>  drivers/media/platform/renesas-ceu.c | 1659 +++++++++++++++++++++++++++
> >>>  3 files changed, 1669 insertions(+)
> >>>  create mode 100644 drivers/media/platform/renesas-ceu.c
> > 
> > [snip]
> > 
> >>> diff --git a/drivers/media/platform/renesas-ceu.c
> >>> b/drivers/media/platform/renesas-ceu.c new file mode 100644
> >>> index 0000000..1b8f0ef
> >>> --- /dev/null
> >>> +++ b/drivers/media/platform/renesas-ceu.c
> > 
> > [snip]
> > 
> >>> +static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
> >>> +				   unsigned int bpl, unsigned int szimage)
> >>> +{
> >>> +	if (plane->bytesperline < bpl)
> >>> +		plane->bytesperline = bpl;
> >>> +	if (plane->sizeimage < szimage)
> >>> +		plane->sizeimage = szimage;
> >> 
> >> As mentioned in your cover letter, you do need to check for invalid
> >> bytesperline values. The v4l2-compliance test is to see what happens
> >> when userspace gives insane values, so yes, drivers need to be able
> >> to handle that.
> > 
> > What limit would you set, what is an acceptable large value versus an
> > invalid large value ? I think we should have rules for this at the API
> > level (or at least, if not part of the API, rules that are consistent
> > across drivers).
> 
> I would expect this to be the max of what the hardware can support. If
> that's really high, then this can be, say, 4 times the width.
> 
> Note that there are very few drivers that can handle a user-specified
> stride.

But that's no reason not to handle it here if the hardware permits, right ? 
:-)

> >> plane->sizeimage is set by the driver, so drop the 'if' before the
> >> assignment.
> > 
> > I don't think that's correct. Userspace should be able to control padding
> > lines at the end of the image, the same way it controls padding pixels at
> > the end of lines.
> 
> If userspace wants larger buffers, then it should use VIDIOC_CREATE_BUFS.
> 
> sizeimage is exclusively set by the driver, applications rely on that.

The API documentation is pretty confusing about this.

In pixfmt-v4l2.rst, the field in the v4l2_pix_format structure is documented 
as

      - Size in bytes of the buffer to hold a complete image, set by the
        driver. Usually this is ``bytesperline`` times ``height``. When
        the image consists of variable length compressed data this is the
        maximum number of bytes required to hold an image.

Then in pixfmt-v4l2-mplane.rst, the field in the v4l2_plane_pix_format 
structure is documented as

      - Maximum size in bytes required for image data in this plane.

Finally, in vidioc-create-bufs.rst, we have

The buffers created by this ioctl will have as minimum size the size
defined by the ``format.pix.sizeimage`` field (or the corresponding
fields for other format types). Usually if the ``format.pix.sizeimage``
field is less than the minimum required for the given format, then an
error will be returned since drivers will typically not allow this. If
it is larger, then the value will be used as-is. In other words, the
driver may reject the requested size, but if it is accepted the driver
will use it unchanged.

The VIDIOC_CREATE_BUFS documentation contradicts the v4l2_pix_format 
documentation, as the multiplane case doesn't state anything about who sets 
the sizeimage field. We should clarify the documentation.

> >>> +}
> > 
> > [snip]
> > 
> >>> +static const struct v4l2_ioctl_ops ceu_ioctl_ops = {
> >>> +	.vidioc_querycap		= ceu_querycap,
> >>> +
> >>> +	.vidioc_enum_fmt_vid_cap_mplane	= ceu_enum_fmt_vid_cap,
> >>> +	.vidioc_try_fmt_vid_cap_mplane	= ceu_try_fmt_vid_cap,
> >>> +	.vidioc_s_fmt_vid_cap_mplane	= ceu_s_fmt_vid_cap,
> >>> +	.vidioc_g_fmt_vid_cap_mplane	= ceu_g_fmt_vid_cap,
> >>> +
> >>> +	.vidioc_enum_input		= ceu_enum_input,
> >>> +	.vidioc_g_input			= ceu_g_input,
> >>> +	.vidioc_s_input			= ceu_s_input,
> >>> +
> >>> +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> >>> +	.vidioc_querybuf		= vb2_ioctl_querybuf,
> >>> +	.vidioc_qbuf			= vb2_ioctl_qbuf,
> >>> +	.vidioc_expbuf			= vb2_ioctl_expbuf,
> >>> +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
> >>> +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
> >>> +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
> >>> +	.vidioc_streamon		= vb2_ioctl_streamon,
> >>> +	.vidioc_streamoff		= vb2_ioctl_streamoff,
> >>> +
> >>> +	.vidioc_g_parm			= ceu_g_parm,
> >>> +	.vidioc_s_parm			= ceu_s_parm,
> >>> +	.vidioc_enum_framesizes		= ceu_enum_framesizes,
> >>> +	.vidioc_enum_frameintervals	= ceu_enum_frameintervals,
> >> 
> >> You're missing these ioctls:
> >>         .vidioc_log_status              = v4l2_ctrl_log_status,
> > 
> > Is log status mandatory ?
> 
> No, but it doesn't hurt to add this one. It comes for free.

Possibly a bit out of scope, but now that the standard debugging interface 
seems to be debugfs, wouldn't it make sense to implement log status through 
there instead of as an ioctl ?

> >>         .vidioc_subscribe_event         = v4l2_ctrl_subscribe_event,
> >>         .vidioc_unsubscribe_event       = v4l2_event_unsubscribe,
> >> 
> >> These missing _event ops are the reason for this compliance failure:
> >> 
> >> fail: v4l2-test-controls.cpp(782): subscribe event for control 'User
> >> Controls' failed
> >> 
> >>> +};
> > 
> > [snip]

Hans Verkuil Jan. 22, 2018, 9:07 a.m. UTC | #11

On 22/01/18 01:52, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday, 19 January 2018 14:25:39 EET Hans Verkuil wrote:
>> On 01/19/18 13:20, Laurent Pinchart wrote:
>>> On Friday, 19 January 2018 13:20:19 EET Hans Verkuil wrote:
>>>> On 01/16/18 22:44, Jacopo Mondi wrote:
>>>>> Add driver for Renesas Capture Engine Unit (CEU).
>>>>>
>>>>> The CEU interface supports capturing 'data' (YUV422) and 'images'
>>>>> (NV[12|21|16|61]).
>>>>>
>>>>> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
>>>>>
>>>>> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
>>>>> platform GR-Peach.
>>>>>
>>>>> Tested with ov7725 camera sensor on SH4 platform Migo-R.
>>>>>
>>>>> Signed-off-by: Jacopo Mondi <[e-mail]>
>>>>> Reviewed-by: Laurent Pinchart <[e-mail]>
>>>>> ---
>>>>>
>>>>>  drivers/media/platform/Kconfig       |    9 +
>>>>>  drivers/media/platform/Makefile      |    1 +
>>>>>  drivers/media/platform/renesas-ceu.c | 1659 +++++++++++++++++++++++++++
>>>>>  3 files changed, 1669 insertions(+)
>>>>>  create mode 100644 drivers/media/platform/renesas-ceu.c
>>>
>>> [snip]
>>>
>>>>> diff --git a/drivers/media/platform/renesas-ceu.c
>>>>> b/drivers/media/platform/renesas-ceu.c new file mode 100644
>>>>> index 0000000..1b8f0ef
>>>>> --- /dev/null
>>>>> +++ b/drivers/media/platform/renesas-ceu.c
>>>
>>> [snip]
>>>
>>>>> +static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
>>>>> +				   unsigned int bpl, unsigned int szimage)
>>>>> +{
>>>>> +	if (plane->bytesperline < bpl)
>>>>> +		plane->bytesperline = bpl;
>>>>> +	if (plane->sizeimage < szimage)
>>>>> +		plane->sizeimage = szimage;
>>>>
>>>> As mentioned in your cover letter, you do need to check for invalid
>>>> bytesperline values. The v4l2-compliance test is to see what happens
>>>> when userspace gives insane values, so yes, drivers need to be able
>>>> to handle that.
>>>
>>> What limit would you set, what is an acceptable large value versus an
>>> invalid large value ? I think we should have rules for this at the API
>>> level (or at least, if not part of the API, rules that are consistent
>>> across drivers).
>>
>> I would expect this to be the max of what the hardware can support. If
>> that's really high, then this can be, say, 4 times the width.
>>
>> Note that there are very few drivers that can handle a user-specified
>> stride.
> 
> But that's no reason not to handle it here if the hardware permits, right ? 
> :-)

Certainly. But it is the reason why it's hard to find example code in
existing drivers.

>>>> plane->sizeimage is set by the driver, so drop the 'if' before the
>>>> assignment.
>>>
>>> I don't think that's correct. Userspace should be able to control padding
>>> lines at the end of the image, the same way it controls padding pixels at
>>> the end of lines.
>>
>> If userspace wants larger buffers, then it should use VIDIOC_CREATE_BUFS.
>>
>> sizeimage is exclusively set by the driver, applications rely on that.
> 
> The API documentation is pretty confusing about this.
> 
> In pixfmt-v4l2.rst, the field in the v4l2_pix_format structure is documented 
> as
> 
>       - Size in bytes of the buffer to hold a complete image, set by the
>         driver. Usually this is ``bytesperline`` times ``height``. When
>         the image consists of variable length compressed data this is the
>         maximum number of bytes required to hold an image.
> 
> Then in pixfmt-v4l2-mplane.rst, the field in the v4l2_plane_pix_format 
> structure is documented as
> 
>       - Maximum size in bytes required for image data in this plane.

This should contain the same text as in pixfmt-v4l2.rst.

> 
> Finally, in vidioc-create-bufs.rst, we have
> 
> The buffers created by this ioctl will have as minimum size the size
> defined by the ``format.pix.sizeimage`` field (or the corresponding
> fields for other format types). Usually if the ``format.pix.sizeimage``
> field is less than the minimum required for the given format, then an
> error will be returned since drivers will typically not allow this. If
> it is larger, then the value will be used as-is. In other words, the
> driver may reject the requested size, but if it is accepted the driver
> will use it unchanged.
> 
> The VIDIOC_CREATE_BUFS documentation contradicts the v4l2_pix_format 
> documentation, as the multiplane case doesn't state anything about who sets 
> the sizeimage field. We should clarify the documentation.

The pixfmt-v4l2-mplane.rst is the one that's incomplete. I noticed the same
thing when I looked at it.

> 
>>>>> +}
>>>
>>> [snip]
>>>
>>>>> +static const struct v4l2_ioctl_ops ceu_ioctl_ops = {
>>>>> +	.vidioc_querycap		= ceu_querycap,
>>>>> +
>>>>> +	.vidioc_enum_fmt_vid_cap_mplane	= ceu_enum_fmt_vid_cap,
>>>>> +	.vidioc_try_fmt_vid_cap_mplane	= ceu_try_fmt_vid_cap,
>>>>> +	.vidioc_s_fmt_vid_cap_mplane	= ceu_s_fmt_vid_cap,
>>>>> +	.vidioc_g_fmt_vid_cap_mplane	= ceu_g_fmt_vid_cap,
>>>>> +
>>>>> +	.vidioc_enum_input		= ceu_enum_input,
>>>>> +	.vidioc_g_input			= ceu_g_input,
>>>>> +	.vidioc_s_input			= ceu_s_input,
>>>>> +
>>>>> +	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
>>>>> +	.vidioc_querybuf		= vb2_ioctl_querybuf,
>>>>> +	.vidioc_qbuf			= vb2_ioctl_qbuf,
>>>>> +	.vidioc_expbuf			= vb2_ioctl_expbuf,
>>>>> +	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
>>>>> +	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
>>>>> +	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
>>>>> +	.vidioc_streamon		= vb2_ioctl_streamon,
>>>>> +	.vidioc_streamoff		= vb2_ioctl_streamoff,
>>>>> +
>>>>> +	.vidioc_g_parm			= ceu_g_parm,
>>>>> +	.vidioc_s_parm			= ceu_s_parm,
>>>>> +	.vidioc_enum_framesizes		= ceu_enum_framesizes,
>>>>> +	.vidioc_enum_frameintervals	= ceu_enum_frameintervals,
>>>>
>>>> You're missing these ioctls:
>>>>         .vidioc_log_status              = v4l2_ctrl_log_status,
>>>
>>> Is log status mandatory ?
>>
>> No, but it doesn't hurt to add this one. It comes for free.
> 
> Possibly a bit out of scope, but now that the standard debugging interface 
> seems to be debugfs, wouldn't it make sense to implement log status through 
> there instead of as an ioctl ?

Why? The ioctl has been there for many many years and it works very well (Cisco
uses this on a daily basis). I see no reason to do a lot of work for no good
purpose. It's probably less useful for sensors but for video receivers (HDMI or
analog) I couldn't do without. With HDMI/analog video receivers you never
know what sort of video you receive, so being able to log the current status
is great. We use it in regression tests and error reporting all the time.

Sure, today we'd design this using debugfs, but I don't think it existed when
LOG_STATUS was created. In any case, I see no reason to change this. In fact,
that would break our tests.

Regards,

	Hans

> 
>>>>         .vidioc_subscribe_event         = v4l2_ctrl_subscribe_event,
>>>>         .vidioc_unsubscribe_event       = v4l2_event_unsubscribe,
>>>>
>>>> These missing _event ops are the reason for this compliance failure:
>>>>
>>>> fail: v4l2-test-controls.cpp(782): subscribe event for control 'User
>>>> Controls' failed
>>>>
>>>>> +};
>>>
>>> [snip]
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hans Verkuil Jan. 22, 2018, 9:56 a.m. UTC | #12

On 21/01/18 18:29, jacopo mondi wrote:
> Hi Hans,
> 
> On Sun, Jan 21, 2018 at 11:23:12AM +0100, Hans Verkuil wrote:
>> On 21/01/18 11:21, Hans Verkuil wrote:
>>> On 21/01/18 10:53, jacopo mondi wrote:
>>>> Hi Hans,
>>>>
>>>> On Fri, Jan 19, 2018 at 12:20:19PM +0100, Hans Verkuil wrote:
>>>>> static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>>>>> {
>>>>>         struct v4l2_captureparm *cp = &parms->parm.capture;
>>>>>         struct ov7670_info *info = to_state(sd);
>>>>>
>>>>>         if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>>>>>                 return -EINVAL;
>>>>>
>>>>> And parms->type is CAPTURE_MPLANE. Just drop this test from the ov7670 driver
>>>>> in the g/s_parm functions. It shouldn't test for that since a subdev driver
>>>>> knows nothing about buffer types.
>>>>>
>>>>
>>>> I will drop that test in an additional patch part of next iteration of this series,
>>>
>>> Replace g/s_parm by g/s_frame_interval. Consider g/s_parm for subdev drivers as
>>> deprecated (I'm working on a patch series to replace all g/s_parm uses by
>>> g/s_frame_interval).
>>
>> Take a look here:
>>
>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=parm
>>
>> You probably want to use the patch 'v4l2-common: add g/s_parm helper functions'
>> for the new ceu driver in your patch series. Feel free to add it.
> 
> Thanks, I have now re-based my series on top of your 'parm' branch,
> and now I have silenced those errors on bad frame interval.
> 
> CEU g/s_parm now look like this:
> 
> static int ceu_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
> 	struct ceu_device *ceudev = video_drvdata(file);
> 	int ret;
> 
> 	ret = v4l2_g_parm(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> 			  ceudev->sd->v4l2_sd, a);
> 	if (ret)
> 		return ret;
> 
> 	a->parm.capture.readbuffers = 0;
> 
> 	return 0;
> }
> 
> Very similar to what you've done on other platform drivers in this
> commit:
> https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=parm&id=a58956ef45cebaa5ce43a5f740fe04517b24a853
> 
> I have a question though (please bear with me a little more :)
> I had to manually set a->parm.capture.readbuffers to 0 to silence the following
> error in v4l2_compliance (which I have now updated to the most recent
> remote HEAD):
> 
>  fail: v4l2-test-formats.cpp(1114): cap->readbuffers
>                 test VIDIOC_G/S_PARM: FAIL
> 
> 		fail_on_test(cap->readbuffers > VIDEO_MAX_FRAME);
> 		if (!(node->g_caps() & V4L2_CAP_READWRITE))
> 			fail_on_test(cap->readbuffers);
> 		else if (node->g_caps() & V4L2_CAP_STREAMING)
> 			fail_on_test(!cap->readbuffers);
> 
> CEU does not support CAP_READWRITE, as it seems atmel-isc/isi do not, so
> v4l2-compliance wants to have readbuffers set to 0. I wonder why in
> the previously mentioned commit you didn't have to set readbuffers
> explicitly to 0 for atmel-isc/isi as I had to for CEU. Will v4l2-compliance
> fail if run on atmel-isc/isi with your commit, or am I missing something?

I've reworked the g/s_parm helper functions so they will now check for
the READWRITE capability and set readbuffers accordingly. I'll post a new
version later today.

Thanks for testing this, I missed that corner case.

Regards,

	Hans

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index fd0c998..fe7bd26 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -144,6 +144,15 @@  config VIDEO_STM32_DCMI
 	  To compile this driver as a module, choose M here: the module
 	  will be called stm32-dcmi.
 
+config VIDEO_RENESAS_CEU
+	tristate "Renesas Capture Engine Unit (CEU) driver"
+	depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
+	depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
+	select VIDEOBUF2_DMA_CONTIG
+	select V4L2_FWNODE
+	---help---
+	  This is a v4l2 driver for the Renesas CEU Interface
+
 source "drivers/media/platform/soc_camera/Kconfig"
 source "drivers/media/platform/exynos4-is/Kconfig"
 source "drivers/media/platform/am437x/Kconfig"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 003b0bb..6580a6b 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -62,6 +62,7 @@  obj-$(CONFIG_VIDEO_SH_VOU)		+= sh_vou.o
 obj-$(CONFIG_SOC_CAMERA)		+= soc_camera/
 
 obj-$(CONFIG_VIDEO_RCAR_DRIF)		+= rcar_drif.o
+obj-$(CONFIG_VIDEO_RENESAS_CEU)		+= renesas-ceu.o
 obj-$(CONFIG_VIDEO_RENESAS_FCP) 	+= rcar-fcp.o
 obj-$(CONFIG_VIDEO_RENESAS_FDP1)	+= rcar_fdp1.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU) 	+= rcar_jpu.o
diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
new file mode 100644
index 0000000..1b8f0ef
--- /dev/null
+++ b/drivers/media/platform/renesas-ceu.c
@@ -0,0 +1,1659 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * V4L2 Driver for Renesas Capture Engine Unit (CEU) interface
+ * Copyright (C) 2017-2018 Jacopo Mondi <[e-mail]>
+ *
+ * Based on soc-camera driver "soc_camera/sh_mobile_ceu_camera.c"
+ * Copyright (C) 2008 Magnus Damm
+ *
+ * Based on V4L2 Driver for PXA camera host - "pxa_camera.c",
+ * Copyright (C) 2006, Sascha Hauer, Pengutronix
+ * Copyright (C) 2008, Guennadi Liakhovetski <[e-mail]>
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/time.h>
+#include <linux/videodev2.h>
+
+#include <media/v4l2-async.h>
+#include <media/v4l2-common.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-image-sizes.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-mediabus.h>
+#include <media/videobuf2-dma-contig.h>
+
+#include <media/drv-intf/renesas-ceu.h>
+
+#define DRIVER_NAME	"renesas-ceu"
+
+/* CEU registers offsets and masks. */
+#define CEU_CAPSR	0x00 /* Capture start register			*/
+#define CEU_CAPCR	0x04 /* Capture control register		*/
+#define CEU_CAMCR	0x08 /* Capture interface control register	*/
+#define CEU_CAMOR	0x10 /* Capture interface offset register	*/
+#define CEU_CAPWR	0x14 /* Capture interface width register	*/
+#define CEU_CAIFR	0x18 /* Capture interface input format register */
+#define CEU_CRCNTR	0x28 /* CEU register control register		*/
+#define CEU_CRCMPR	0x2c /* CEU register forcible control register	*/
+#define CEU_CFLCR	0x30 /* Capture filter control register		*/
+#define CEU_CFSZR	0x34 /* Capture filter size clip register	*/
+#define CEU_CDWDR	0x38 /* Capture destination width register	*/
+#define CEU_CDAYR	0x3c /* Capture data address Y register		*/
+#define CEU_CDACR	0x40 /* Capture data address C register		*/
+#define CEU_CFWCR	0x5c /* Firewall operation control register	*/
+#define CEU_CDOCR	0x64 /* Capture data output control register	*/
+#define CEU_CEIER	0x70 /* Capture event interrupt enable register	*/
+#define CEU_CETCR	0x74 /* Capture event flag clear register	*/
+#define CEU_CSTSR	0x7c /* Capture status register			*/
+#define CEU_CSRTR	0x80 /* Capture software reset register		*/
+
+/* Data synchronous fetch mode. */
+#define CEU_CAMCR_JPEG			BIT(4)
+
+/* Input components ordering: CEU_CAMCR.DTARY field. */
+#define CEU_CAMCR_DTARY_8_UYVY		(0x00 << 8)
+#define CEU_CAMCR_DTARY_8_VYUY		(0x01 << 8)
+#define CEU_CAMCR_DTARY_8_YUYV		(0x02 << 8)
+#define CEU_CAMCR_DTARY_8_YVYU		(0x03 << 8)
+/* TODO: input components ordering for 16 bits input. */
+
+/* Bus transfer MTU. */
+#define CEU_CAPCR_BUS_WIDTH256		(0x3 << 20)
+
+/* Bus width configuration. */
+#define CEU_CAMCR_DTIF_16BITS		BIT(12)
+
+/* No downsampling to planar YUV420 in image fetch mode. */
+#define CEU_CDOCR_NO_DOWSAMPLE		BIT(4)
+
+/* Swap all input data in 8-bit, 16-bits and 32-bits units (Figure 46.45). */
+#define CEU_CDOCR_SWAP_ENDIANNESS	(7)
+
+/* Capture reset and enable bits. */
+#define CEU_CAPSR_CPKIL			BIT(16)
+#define CEU_CAPSR_CE			BIT(0)
+
+/* CEU operating flag bit. */
+#define CEU_CAPCR_CTNCP			BIT(16)
+#define CEU_CSTRST_CPTON		BIT(1)
+
+/* Platform specific IRQ source flags. */
+#define CEU_CETCR_ALL_IRQS_RZ		0x397f313
+#define CEU_CETCR_ALL_IRQS_SH4		0x3d7f313
+
+/* Prohibited register access interrupt bit. */
+#define CEU_CETCR_IGRW			BIT(4)
+/* One-frame capture end interrupt. */
+#define CEU_CEIER_CPE			BIT(0)
+/* VBP error. */
+#define CEU_CEIER_VBP			BIT(20)
+#define CEU_CEIER_MASK			(CEU_CEIER_CPE | CEU_CEIER_VBP)
+
+#define CEU_MAX_WIDTH	2560
+#define CEU_MAX_HEIGHT	1920
+#define CEU_W_MAX(w)	((w) < CEU_MAX_WIDTH ? (w) : CEU_MAX_WIDTH)
+#define CEU_H_MAX(h)	((h) < CEU_MAX_HEIGHT ? (h) : CEU_MAX_HEIGHT)
+
+/*
+ * ceu_bus_fmt - describe a 8-bits yuyv format the sensor can produce
+ *
+ * @mbus_code: bus format code
+ * @fmt_order: CEU_CAMCR.DTARY ordering of input components (Y, Cb, Cr)
+ * @fmt_order_swap: swapped CEU_CAMCR.DTARY ordering of input components
+ *		    (Y, Cr, Cb)
+ * @swapped: does Cr appear before Cb?
+ * @bps: number of bits sent over bus for each sample
+ * @bpp: number of bits per pixels unit
+ */
+struct ceu_mbus_fmt {
+	u32	mbus_code;
+	u32	fmt_order;
+	u32	fmt_order_swap;
+	bool	swapped;
+	u8	bps;
+	u8	bpp;
+};
+
+/*
+ * ceu_buffer - Link vb2 buffer to the list of available buffers.
+ */
+struct ceu_buffer {
+	struct vb2_v4l2_buffer vb;
+	struct list_head queue;
+};
+
+static inline struct ceu_buffer *vb2_to_ceu(struct vb2_v4l2_buffer *vbuf)
+{
+	return container_of(vbuf, struct ceu_buffer, vb);
+}
+
+/*
+ * ceu_subdev - Wraps v4l2 sub-device and provides async subdevice.
+ */
+struct ceu_subdev {
+	struct v4l2_subdev *v4l2_sd;
+	struct v4l2_async_subdev asd;
+
+	/* per-subdevice mbus configuration options */
+	unsigned int mbus_flags;
+	struct ceu_mbus_fmt mbus_fmt;
+};
+
+static struct ceu_subdev *to_ceu_subdev(struct v4l2_async_subdev *asd)
+{
+	return container_of(asd, struct ceu_subdev, asd);
+}
+
+/*
+ * ceu_device - CEU device instance
+ */
+struct ceu_device {
+	struct device		*dev;
+	struct video_device	vdev;
+	struct v4l2_device	v4l2_dev;
+
+	/* subdevices descriptors */
+	struct ceu_subdev	*subdevs;
+	/* the subdevice currently in use */
+	struct ceu_subdev	*sd;
+	unsigned int		sd_index;
+	unsigned int		num_sd;
+
+	/* platform specific mask with all IRQ sources flagged */
+	u32			irq_mask;
+
+	/* currently configured field and pixel format */
+	enum v4l2_field	field;
+	struct v4l2_pix_format_mplane v4l2_pix;
+
+	/* async subdev notification helpers */
+	struct v4l2_async_notifier notifier;
+	/* pointers to "struct ceu_subdevice -> asd" */
+	struct v4l2_async_subdev **asds;
+
+	/* vb2 queue, capture buffer list and active buffer pointer */
+	struct vb2_queue	vb2_vq;
+	struct list_head	capture;
+	struct vb2_v4l2_buffer	*active;
+	unsigned int		sequence;
+
+	/* mlock - lock access to interface reset and vb2 queue */
+	struct mutex	mlock;
+
+	/* lock - lock access to capture buffer queue and active buffer */
+	spinlock_t	lock;
+
+	/* base - CEU memory base address */
+	void __iomem	*base;
+};
+
+static inline struct ceu_device *v4l2_to_ceu(struct v4l2_device *v4l2_dev)
+{
+	return container_of(v4l2_dev, struct ceu_device, v4l2_dev);
+}
+
+/* --- CEU memory output formats --- */
+
+/*
+ * ceu_fmt - describe a memory output format supported by CEU interface.
+ *
+ * @fourcc: memory layout fourcc format code
+ * @bpp: number of bits for each pixel stored in memory
+ */
+struct ceu_fmt {
+	u32	fourcc;
+	u32	bpp;
+};
+
+/*
+ * ceu_format_list - List of supported memory output formats
+ *
+ * If sensor provides any YUYV bus format, all the following planar memory
+ * formats are available thanks to CEU re-ordering and sub-sampling
+ * capabilities.
+ */
+static const struct ceu_fmt ceu_fmt_list[] = {
+	{
+		.fourcc	= V4L2_PIX_FMT_NV16,
+		.bpp	= 16,
+	},
+	{
+		.fourcc = V4L2_PIX_FMT_NV61,
+		.bpp	= 16,
+	},
+	{
+		.fourcc	= V4L2_PIX_FMT_NV12,
+		.bpp	= 12,
+	},
+	{
+		.fourcc	= V4L2_PIX_FMT_NV21,
+		.bpp	= 12,
+	},
+	{
+		.fourcc	= V4L2_PIX_FMT_YUYV,
+		.bpp	= 16,
+	},
+};
+
+static const struct ceu_fmt *get_ceu_fmt_from_fourcc(unsigned int fourcc)
+{
+	const struct ceu_fmt *fmt = &ceu_fmt_list[0];
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(ceu_fmt_list); i++, fmt++)
+		if (fmt->fourcc == fourcc)
+			return fmt;
+
+	return NULL;
+}
+
+static bool ceu_fmt_mplane(struct v4l2_pix_format_mplane *pix)
+{
+	switch (pix->pixelformat) {
+	case V4L2_PIX_FMT_YUYV:
+		return false;
+	case V4L2_PIX_FMT_NV16:
+	case V4L2_PIX_FMT_NV61:
+	case V4L2_PIX_FMT_NV12:
+	case V4L2_PIX_FMT_NV21:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/* --- CEU HW operations --- */
+
+static void ceu_write(struct ceu_device *priv, unsigned int reg_offs, u32 data)
+{
+	iowrite32(data, priv->base + reg_offs);
+}
+
+static u32 ceu_read(struct ceu_device *priv, unsigned int reg_offs)
+{
+	return ioread32(priv->base + reg_offs);
+}
+
+/*
+ * ceu_soft_reset() - Software reset the CEU interface.
+ * @ceu_device: CEU device.
+ *
+ * Returns 0 for success, -EIO for error.
+ */
+static int ceu_soft_reset(struct ceu_device *ceudev)
+{
+	unsigned int i;
+
+	ceu_write(ceudev, CEU_CAPSR, CEU_CAPSR_CPKIL);
+
+	for (i = 0; i < 100; i++) {
+		if (!(ceu_read(ceudev, CEU_CSTSR) & CEU_CSTRST_CPTON))
+			break;
+		udelay(1);
+	}
+
+	if (i == 100) {
+		dev_err(ceudev->dev, "soft reset time out\n");
+		return -EIO;
+	}
+
+	for (i = 0; i < 100; i++) {
+		if (!(ceu_read(ceudev, CEU_CAPSR) & CEU_CAPSR_CPKIL))
+			return 0;
+		udelay(1);
+	}
+
+	/* If we get here, CEU has not reset properly. */
+	return -EIO;
+}
+
+/* --- CEU Capture Operations --- */
+
+/*
+ * ceu_hw_config() - Configure CEU interface registers.
+ */
+static int ceu_hw_config(struct ceu_device *ceudev)
+{
+	u32 camcr, cdocr, cfzsr, cdwdr, capwr;
+	struct v4l2_pix_format_mplane *pix = &ceudev->v4l2_pix;
+	struct ceu_subdev *ceu_sd = ceudev->sd;
+	struct ceu_mbus_fmt *mbus_fmt = &ceu_sd->mbus_fmt;
+	unsigned int mbus_flags = ceu_sd->mbus_flags;
+
+	/* Start configuring CEU registers */
+	ceu_write(ceudev, CEU_CAIFR, 0);
+	ceu_write(ceudev, CEU_CFWCR, 0);
+	ceu_write(ceudev, CEU_CRCNTR, 0);
+	ceu_write(ceudev, CEU_CRCMPR, 0);
+
+	/* Set the frame capture period for both image capture and data sync. */
+	capwr = (pix->height << 16) | pix->width * mbus_fmt->bpp / 8;
+
+	/*
+	 * Swap input data endianness by default.
+	 * In data fetch mode bytes are received in chunks of 8 bytes.
+	 * D0, D1, D2, D3, D4, D5, D6, D7 (D0 received first)
+	 * The data is however by default written to memory in reverse order:
+	 * D7, D6, D5, D4, D3, D2, D1, D0 (D7 written to lowest byte)
+	 *
+	 * Use CEU_CDOCR[2:0] to swap data ordering.
+	 */
+	cdocr = CEU_CDOCR_SWAP_ENDIANNESS;
+
+	/*
+	 * Configure CAMCR and CDOCR:
+	 * match input components ordering with memory output format and
+	 * handle downsampling to YUV420.
+	 *
+	 * If the memory output planar format is 'swapped' (Cr before Cb) and
+	 * input format is not, use the swapped version of CAMCR.DTARY.
+	 *
+	 * If the memory output planar format is not 'swapped' (Cb before Cr)
+	 * and input format is, use the swapped version of CAMCR.DTARY.
+	 *
+	 * CEU by default downsample to planar YUV420 (CDCOR[4] = 0).
+	 * If output is planar YUV422 set CDOCR[4] = 1
+	 *
+	 * No downsample for data fetch sync mode.
+	 */
+	switch (pix->pixelformat) {
+	/* Data fetch sync mode */
+	case V4L2_PIX_FMT_YUYV:
+		/* TODO: handle YUYV permutations through DTARY bits. */
+		camcr	= CEU_CAMCR_JPEG;
+		cdocr	|= CEU_CDOCR_NO_DOWSAMPLE;
+		cfzsr	= (pix->height << 16) | pix->width;
+		cdwdr	= pix->plane_fmt[0].bytesperline;
+		break;
+
+	/* Non-swapped planar image capture mode. */
+	case V4L2_PIX_FMT_NV16:
+		cdocr	|= CEU_CDOCR_NO_DOWSAMPLE;
+	case V4L2_PIX_FMT_NV12:
+		if (mbus_fmt->swapped)
+			camcr = mbus_fmt->fmt_order_swap;
+		else
+			camcr = mbus_fmt->fmt_order;
+
+		cfzsr	= (pix->height << 16) | pix->width;
+		cdwdr	= pix->width;
+		break;
+
+	/* Swapped planar image capture mode. */
+	case V4L2_PIX_FMT_NV61:
+		cdocr	|= CEU_CDOCR_NO_DOWSAMPLE;
+	case V4L2_PIX_FMT_NV21:
+		if (mbus_fmt->swapped)
+			camcr = mbus_fmt->fmt_order;
+		else
+			camcr = mbus_fmt->fmt_order_swap;
+
+		cfzsr	= (pix->height << 16) | pix->width;
+		cdwdr	= pix->width;
+		break;
+	}
+
+	camcr |= mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW ? 1 << 1 : 0;
+	camcr |= mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW ? 1 << 0 : 0;
+
+	/* TODO: handle 16 bit bus width with DTIF bit in CAMCR */
+	ceu_write(ceudev, CEU_CAMCR, camcr);
+	ceu_write(ceudev, CEU_CDOCR, cdocr);
+	ceu_write(ceudev, CEU_CAPCR, CEU_CAPCR_BUS_WIDTH256);
+
+	/*
+	 * TODO: make CAMOR offsets configurable.
+	 * CAMOR wants to know the number of blanks between a VS/HS signal
+	 * and valid data. This value should actually come from the sensor...
+	 */
+	ceu_write(ceudev, CEU_CAMOR, 0);
+
+	/* TODO: 16 bit bus width require re-calculation of cdwdr and cfzsr */
+	ceu_write(ceudev, CEU_CAPWR, capwr);
+	ceu_write(ceudev, CEU_CFSZR, cfzsr);
+	ceu_write(ceudev, CEU_CDWDR, cdwdr);
+
+	return 0;
+}
+
+/*
+ * ceu_capture() - Trigger start of a capture sequence.
+ *
+ * Program the CEU DMA registers with addresses where to transfer image data.
+ */
+static int ceu_capture(struct ceu_device *ceudev)
+{
+	struct v4l2_pix_format_mplane *pix = &ceudev->v4l2_pix;
+	dma_addr_t phys_addr_top;
+
+	phys_addr_top =
+		vb2_dma_contig_plane_dma_addr(&ceudev->active->vb2_buf, 0);
+	ceu_write(ceudev, CEU_CDAYR, phys_addr_top);
+
+	/* Ignore CbCr plane for non multi-planar image formats. */
+	if (ceu_fmt_mplane(pix)) {
+		phys_addr_top =
+			vb2_dma_contig_plane_dma_addr(&ceudev->active->vb2_buf,
+						      1);
+		ceu_write(ceudev, CEU_CDACR, phys_addr_top);
+	}
+
+	/*
+	 * Trigger new capture start: once for each frame, as we work in
+	 * one-frame capture mode.
+	 */
+	ceu_write(ceudev, CEU_CAPSR, CEU_CAPSR_CE);
+
+	return 0;
+}
+
+static irqreturn_t ceu_irq(int irq, void *data)
+{
+	struct ceu_device *ceudev = data;
+	struct vb2_v4l2_buffer *vbuf;
+	struct ceu_buffer *buf;
+	u32 status;
+
+	/* Clean interrupt status. */
+	status = ceu_read(ceudev, CEU_CETCR);
+	ceu_write(ceudev, CEU_CETCR, ~ceudev->irq_mask);
+
+	/* Unexpected interrupt. */
+	if (!(status & CEU_CEIER_MASK))
+		return IRQ_NONE;
+
+	spin_lock(&ceudev->lock);
+
+	/* Stale interrupt from a released buffer, ignore it. */
+	vbuf = ceudev->active;
+	if (!vbuf) {
+		spin_unlock(&ceudev->lock);
+		return IRQ_HANDLED;
+	}
+
+	/*
+	 * When a VBP interrupt occurs, no capture end interrupt will occur
+	 * and the image of that frame is not captured correctly.
+	 */
+	if (status & CEU_CEIER_VBP) {
+		dev_err(ceudev->dev, "VBP interrupt: abort capture\n");
+		goto error_irq_out;
+	}
+
+	/* Prepare to return the 'previous' buffer. */
+	vbuf->vb2_buf.timestamp = ktime_get_ns();
+	vbuf->sequence = ceudev->sequence++;
+	vbuf->field = ceudev->field;
+
+	/* Prepare a new 'active' buffer and trigger a new capture. */
+	if (!list_empty(&ceudev->capture)) {
+		buf = list_first_entry(&ceudev->capture, struct ceu_buffer,
+				       queue);
+		list_del(&buf->queue);
+		ceudev->active = &buf->vb;
+
+		ceu_capture(ceudev);
+	}
+
+	/* Return the 'previous' buffer. */
+	vb2_buffer_done(&vbuf->vb2_buf, VB2_BUF_STATE_DONE);
+
+	spin_unlock(&ceudev->lock);
+
+	return IRQ_HANDLED;
+
+error_irq_out:
+	/* Return the 'previous' buffer and all queued ones. */
+	vb2_buffer_done(&vbuf->vb2_buf, VB2_BUF_STATE_ERROR);
+
+	list_for_each_entry(buf, &ceudev->capture, queue)
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+
+	spin_unlock(&ceudev->lock);
+
+	return IRQ_HANDLED;
+}
+
+/* --- CEU Videobuf2 operations --- */
+
+static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
+				   unsigned int bpl, unsigned int szimage)
+{
+	if (plane->bytesperline < bpl)
+		plane->bytesperline = bpl;
+	if (plane->sizeimage < szimage)
+		plane->sizeimage = szimage;
+}
+
+/*
+ * ceu_calc_plane_sizes() - Fill per-plane 'struct v4l2_plane_pix_format'
+ *			    information according to the currently configured
+ *			    pixel format.
+ * @ceu_device: CEU device.
+ * @ceu_fmt: Active image format.
+ * @pix: Pixel format information (store line width and image sizes)
+ */
+static void ceu_calc_plane_sizes(struct ceu_device *ceudev,
+				 const struct ceu_fmt *ceu_fmt,
+				 struct v4l2_pix_format_mplane *pix)
+{
+	unsigned int bpl, szimage;
+
+	switch (pix->pixelformat) {
+	case V4L2_PIX_FMT_YUYV:
+		pix->num_planes	= 1;
+		bpl		= pix->width * ceu_fmt->bpp / 8;
+		szimage		= pix->height * bpl;
+		ceu_update_plane_sizes(&pix->plane_fmt[0], bpl, szimage);
+		break;
+
+	case V4L2_PIX_FMT_NV12:
+	case V4L2_PIX_FMT_NV21:
+		pix->num_planes	= 2;
+		bpl		= pix->width;
+		szimage		= pix->height * pix->width;
+		ceu_update_plane_sizes(&pix->plane_fmt[0], bpl, szimage);
+		ceu_update_plane_sizes(&pix->plane_fmt[1], bpl, szimage / 2);
+		break;
+
+	case V4L2_PIX_FMT_NV16:
+	case V4L2_PIX_FMT_NV61:
+	default:
+		pix->num_planes	= 2;
+		bpl		= pix->width;
+		szimage		= pix->height * pix->width;
+		ceu_update_plane_sizes(&pix->plane_fmt[0], bpl, szimage);
+		ceu_update_plane_sizes(&pix->plane_fmt[1], bpl, szimage);
+		break;
+	}
+}
+
+/*
+ * ceu_vb2_setup() - is called to check whether the driver can accept the
+ *		     requested number of buffers and to fill in plane sizes
+ *		     for the current frame format, if required.
+ */
+static int ceu_vb2_setup(struct vb2_queue *vq, unsigned int *count,
+			 unsigned int *num_planes, unsigned int sizes[],
+			 struct device *alloc_devs[])
+{
+	struct ceu_device *ceudev = vb2_get_drv_priv(vq);
+	struct v4l2_pix_format_mplane *pix = &ceudev->v4l2_pix;
+	unsigned int i;
+
+	/* num_planes is set: just check plane sizes. */
+	if (*num_planes) {
+		for (i = 0; i < pix->num_planes; i++)
+			if (sizes[i] < pix->plane_fmt[i].sizeimage)
+				return -EINVAL;
+
+		return 0;
+	}
+
+	/* num_planes not set: called from REQBUFS, just set plane sizes. */
+	*num_planes = pix->num_planes;
+	for (i = 0; i < pix->num_planes; i++)
+		sizes[i] = pix->plane_fmt[i].sizeimage;
+
+	return 0;
+}
+
+static void ceu_vb2_queue(struct vb2_buffer *vb)
+{
+	struct ceu_device *ceudev = vb2_get_drv_priv(vb->vb2_queue);
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct ceu_buffer *buf = vb2_to_ceu(vbuf);
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&ceudev->lock, irqflags);
+	list_add_tail(&buf->queue, &ceudev->capture);
+	spin_unlock_irqrestore(&ceudev->lock, irqflags);
+}
+
+static int ceu_vb2_prepare(struct vb2_buffer *vb)
+{
+	struct ceu_device *ceudev = vb2_get_drv_priv(vb->vb2_queue);
+	struct v4l2_pix_format_mplane *pix = &ceudev->v4l2_pix;
+	unsigned int i;
+
+	for (i = 0; i < pix->num_planes; i++) {
+		if (vb2_plane_size(vb, i) < pix->plane_fmt[i].sizeimage) {
+			dev_err(ceudev->dev,
+				"Plane size too small (%lu < %u)\n",
+				vb2_plane_size(vb, i),
+				pix->plane_fmt[i].sizeimage);
+			return -EINVAL;
+		}
+
+		vb2_set_plane_payload(vb, i, pix->plane_fmt[i].sizeimage);
+	}
+
+	return 0;
+}
+
+static int ceu_start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+	struct ceu_device *ceudev = vb2_get_drv_priv(vq);
+	struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
+	struct ceu_buffer *buf;
+	unsigned long irqflags;
+	int ret;
+
+	/* Program the CEU interface according to the CEU image format. */
+	ret = ceu_hw_config(ceudev);
+	if (ret)
+		goto error_return_bufs;
+
+	ret = v4l2_subdev_call(v4l2_sd, video, s_stream, 1);
+	if (ret && ret != -ENOIOCTLCMD) {
+		dev_dbg(ceudev->dev,
+			"Subdevice failed to start streaming: %d\n", ret);
+		goto error_return_bufs;
+	}
+
+	spin_lock_irqsave(&ceudev->lock, irqflags);
+	ceudev->sequence = 0;
+
+	/* Grab the first available buffer and trigger the first capture. */
+	buf = list_first_entry(&ceudev->capture, struct ceu_buffer,
+			       queue);
+	if (!buf) {
+		spin_unlock_irqrestore(&ceudev->lock, irqflags);
+		dev_dbg(ceudev->dev,
+			"No buffer available for capture.\n");
+		goto error_stop_sensor;
+	}
+
+	list_del(&buf->queue);
+	ceudev->active = &buf->vb;
+
+	/* Clean and program interrupts for first capture. */
+	ceu_write(ceudev, CEU_CETCR, ~ceudev->irq_mask);
+	ceu_write(ceudev, CEU_CEIER, CEU_CEIER_MASK);
+
+	ceu_capture(ceudev);
+
+	spin_unlock_irqrestore(&ceudev->lock, irqflags);
+
+	return 0;
+
+error_stop_sensor:
+	v4l2_subdev_call(v4l2_sd, video, s_stream, 0);
+
+error_return_bufs:
+	spin_lock_irqsave(&ceudev->lock, irqflags);
+	list_for_each_entry(buf, &ceudev->capture, queue)
+		vb2_buffer_done(&ceudev->active->vb2_buf,
+				VB2_BUF_STATE_QUEUED);
+	ceudev->active = NULL;
+	spin_unlock_irqrestore(&ceudev->lock, irqflags);
+
+	return ret;
+}
+
+static void ceu_stop_streaming(struct vb2_queue *vq)
+{
+	struct ceu_device *ceudev = vb2_get_drv_priv(vq);
+	struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
+	struct ceu_buffer *buf;
+	unsigned long irqflags;
+
+	/* Clean and disable interrupt sources. */
+	ceu_write(ceudev, CEU_CETCR,
+		  ceu_read(ceudev, CEU_CETCR) & ceudev->irq_mask);
+	ceu_write(ceudev, CEU_CEIER, CEU_CEIER_MASK);
+
+	v4l2_subdev_call(v4l2_sd, video, s_stream, 0);
+
+	spin_lock_irqsave(&ceudev->lock, irqflags);
+	if (ceudev->active) {
+		vb2_buffer_done(&ceudev->active->vb2_buf,
+				VB2_BUF_STATE_ERROR);
+		ceudev->active = NULL;
+	}
+
+	/* Release all queued buffers. */
+	list_for_each_entry(buf, &ceudev->capture, queue)
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+	INIT_LIST_HEAD(&ceudev->capture);
+
+	spin_unlock_irqrestore(&ceudev->lock, irqflags);
+
+	ceu_soft_reset(ceudev);
+}
+
+static const struct vb2_ops ceu_vb2_ops = {
+	.queue_setup		= ceu_vb2_setup,
+	.buf_queue		= ceu_vb2_queue,
+	.buf_prepare		= ceu_vb2_prepare,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
+	.start_streaming	= ceu_start_streaming,
+	.stop_streaming		= ceu_stop_streaming,
+};
+
+/* --- CEU image formats handling --- */
+
+/*
+ * ceu_try_fmt() - test format on CEU and sensor
+ * @ceudev: The CEU device.
+ * @v4l2_fmt: format to test.
+ *
+ * Returns 0 for success, < 0 for errors.
+ */
+static int ceu_try_fmt(struct ceu_device *ceudev, struct v4l2_format *v4l2_fmt)
+{
+	struct ceu_subdev *ceu_sd = ceudev->sd;
+	struct v4l2_pix_format_mplane *pix = &v4l2_fmt->fmt.pix_mp;
+	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
+	struct v4l2_subdev_pad_config pad_cfg;
+	const struct ceu_fmt *ceu_fmt;
+	int ret;
+
+	struct v4l2_subdev_format sd_format = {
+		.which = V4L2_SUBDEV_FORMAT_TRY,
+	};
+
+	switch (pix->pixelformat) {
+	case V4L2_PIX_FMT_YUYV:
+	case V4L2_PIX_FMT_NV16:
+	case V4L2_PIX_FMT_NV61:
+	case V4L2_PIX_FMT_NV12:
+	case V4L2_PIX_FMT_NV21:
+		break;
+
+	default:
+		pix->pixelformat = V4L2_PIX_FMT_NV16;
+		break;
+	}
+
+	ceu_fmt = get_ceu_fmt_from_fourcc(pix->pixelformat);
+
+	/* CFSZR requires height and width to be 4-pixel aligned. */
+	v4l_bound_align_image(&pix->width, 2, CEU_MAX_WIDTH, 4,
+			      &pix->height, 4, CEU_MAX_HEIGHT, 4, 0);
+
+	/*
+	 * Set format on sensor sub device: bus format used to produce memory
+	 * format is selected at initialization time.
+	 */
+	v4l2_fill_mbus_format_mplane(&sd_format.format, pix);
+	ret = v4l2_subdev_call(v4l2_sd, pad, set_fmt, &pad_cfg, &sd_format);
+	if (ret)
+		return ret;
+
+	/* Apply size returned by sensor as the CEU can't scale. */
+	v4l2_fill_pix_format_mplane(pix, &sd_format.format);
+
+	/* Calculate per-plane sizes based on image format. */
+	ceu_calc_plane_sizes(ceudev, ceu_fmt, pix);
+
+	return 0;
+}
+
+/*
+ * ceu_set_fmt() - Apply the supplied format to both sensor and CEU
+ */
+static int ceu_set_fmt(struct ceu_device *ceudev, struct v4l2_format *v4l2_fmt)
+{
+	struct ceu_subdev *ceu_sd = ceudev->sd;
+	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
+	int ret;
+
+	struct v4l2_subdev_format format = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+
+	ret = ceu_try_fmt(ceudev, v4l2_fmt);
+	if (ret)
+		return ret;
+
+	v4l2_fill_mbus_format_mplane(&format.format, &v4l2_fmt->fmt.pix_mp);
+	ret = v4l2_subdev_call(v4l2_sd, pad, set_fmt, NULL, &format);
+	if (ret)
+		return ret;
+
+	ceudev->v4l2_pix = v4l2_fmt->fmt.pix_mp;
+
+	return 0;
+}
+
+/*
+ * ceu_set_default_fmt() - Apply default NV16 memory output format with VGA
+ *			   sizes.
+ */
+static int ceu_set_default_fmt(struct ceu_device *ceudev)
+{
+	int ret;
+
+	struct v4l2_format v4l2_fmt = {
+		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+		.fmt.pix_mp = {
+			.width		= VGA_WIDTH,
+			.height		= VGA_HEIGHT,
+			.field		= V4L2_FIELD_NONE,
+			.pixelformat	= V4L2_PIX_FMT_NV16,
+			.num_planes	= 2,
+			.plane_fmt	= {
+				[0]	= {
+					.sizeimage = VGA_WIDTH * VGA_HEIGHT * 2,
+					.bytesperline = VGA_WIDTH * 2,
+				},
+				[1]	= {
+					.sizeimage = VGA_WIDTH * VGA_HEIGHT * 2,
+					.bytesperline = VGA_WIDTH * 2,
+				},
+			},
+		},
+	};
+
+	ret = ceu_try_fmt(ceudev, &v4l2_fmt);
+	if (ret)
+		return ret;
+
+	ceudev->v4l2_pix = v4l2_fmt.fmt.pix_mp;
+
+	return 0;
+}
+
+/*
+ * ceu_init_formats() - Query sensor for supported formats and initialize
+ *			CEU supported format list
+ *
+ * Find out if sensor can produce a permutation of 8-bits YUYV bus format.
+ * From a single 8-bits YUYV bus format the CEU can produce several memory
+ * output formats:
+ * - NV[12|21|16|61] through image fetch mode;
+ * - YUYV422 if sensor provides YUYV422
+ *
+ * TODO: Other YUYV422 permutations through data fetch sync mode and DTARY
+ * TODO: Binary data (eg. JPEG) and raw formats through data fetch sync mode
+ */
+static int ceu_init_formats(struct ceu_device *ceudev)
+{
+	struct ceu_subdev *ceu_sd = ceudev->sd;
+	struct ceu_mbus_fmt *mbus_fmt = &ceu_sd->mbus_fmt;
+	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
+	bool yuyv_bus_fmt = false;
+
+	struct v4l2_subdev_mbus_code_enum sd_mbus_fmt = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+		.index = 0,
+	};
+
+	/* Find out if sensor can produce any permutation of 8-bits YUYV422. */
+	while (!yuyv_bus_fmt &&
+	       !v4l2_subdev_call(v4l2_sd, pad, enum_mbus_code,
+				 NULL, &sd_mbus_fmt)) {
+		switch (sd_mbus_fmt.code) {
+		case MEDIA_BUS_FMT_YUYV8_2X8:
+		case MEDIA_BUS_FMT_YVYU8_2X8:
+		case MEDIA_BUS_FMT_UYVY8_2X8:
+		case MEDIA_BUS_FMT_VYUY8_2X8:
+			yuyv_bus_fmt = true;
+			break;
+		default:
+			/*
+			 * Only support 8-bits YUYV bus formats at the moment;
+			 *
+			 * TODO: add support for binary formats (data sync
+			 * fetch mode).
+			 */
+			break;
+		}
+
+		sd_mbus_fmt.index++;
+	}
+
+	if (!yuyv_bus_fmt)
+		return -ENXIO;
+
+	/*
+	 * Save the first encountered YUYV format as "mbus_fmt" and use it
+	 * to output all planar YUV422 and YUV420 (NV*) formats to memory as
+	 * well as for data synch fetch mode (YUYV - YVYU etc. ).
+	 */
+	mbus_fmt->mbus_code	= sd_mbus_fmt.code;
+	mbus_fmt->bps		= 8;
+
+	/* Annotate the selected bus format components ordering. */
+	switch (sd_mbus_fmt.code) {
+	case MEDIA_BUS_FMT_YUYV8_2X8:
+		mbus_fmt->fmt_order		= CEU_CAMCR_DTARY_8_YUYV;
+		mbus_fmt->fmt_order_swap	= CEU_CAMCR_DTARY_8_YVYU;
+		mbus_fmt->swapped		= false;
+		mbus_fmt->bpp			= 16;
+		break;
+
+	case MEDIA_BUS_FMT_YVYU8_2X8:
+		mbus_fmt->fmt_order		= CEU_CAMCR_DTARY_8_YVYU;
+		mbus_fmt->fmt_order_swap	= CEU_CAMCR_DTARY_8_YUYV;
+		mbus_fmt->swapped		= true;
+		mbus_fmt->bpp			= 16;
+		break;
+
+	case MEDIA_BUS_FMT_UYVY8_2X8:
+		mbus_fmt->fmt_order		= CEU_CAMCR_DTARY_8_UYVY;
+		mbus_fmt->fmt_order_swap	= CEU_CAMCR_DTARY_8_VYUY;
+		mbus_fmt->swapped		= false;
+		mbus_fmt->bpp			= 16;
+		break;
+
+	case MEDIA_BUS_FMT_VYUY8_2X8:
+		mbus_fmt->fmt_order		= CEU_CAMCR_DTARY_8_VYUY;
+		mbus_fmt->fmt_order_swap	= CEU_CAMCR_DTARY_8_UYVY;
+		mbus_fmt->swapped		= true;
+		mbus_fmt->bpp			= 16;
+		break;
+	}
+
+	ceudev->field = V4L2_FIELD_NONE;
+
+	return 0;
+}
+
+/* --- Runtime PM Handlers --- */
+
+/*
+ * ceu_runtime_resume() - soft-reset the interface and turn sensor power on.
+ */
+static int ceu_runtime_resume(struct device *dev)
+{
+	struct ceu_device *ceudev = dev_get_drvdata(dev);
+	struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
+
+	v4l2_subdev_call(v4l2_sd, core, s_power, 1);
+
+	ceu_soft_reset(ceudev);
+
+	return 0;
+}
+
+/*
+ * ceu_runtime_suspend() - disable capture and interrupts and soft-reset.
+ *			   Turn sensor power off.
+ */
+static int ceu_runtime_suspend(struct device *dev)
+{
+	struct ceu_device *ceudev = dev_get_drvdata(dev);
+	struct v4l2_subdev *v4l2_sd = ceudev->sd->v4l2_sd;
+
+	v4l2_subdev_call(v4l2_sd, core, s_power, 0);
+
+	ceu_write(ceudev, CEU_CEIER, 0);
+	ceu_soft_reset(ceudev);
+
+	return 0;
+}
+
+/* --- File Operations --- */
+
+static int ceu_open(struct file *file)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+	int ret;
+
+	ret = v4l2_fh_open(file);
+	if (ret)
+		return ret;
+
+	mutex_lock(&ceudev->mlock);
+	/* Causes soft-reset and sensor power on on first open */
+	pm_runtime_get_sync(ceudev->dev);
+	mutex_unlock(&ceudev->mlock);
+
+	return 0;
+}
+
+static int ceu_release(struct file *file)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+
+	vb2_fop_release(file);
+
+	mutex_lock(&ceudev->mlock);
+	/* Causes soft-reset and sensor power down on last close */
+	pm_runtime_put(ceudev->dev);
+	mutex_unlock(&ceudev->mlock);
+
+	return 0;
+}
+
+static const struct v4l2_file_operations ceu_fops = {
+	.owner			= THIS_MODULE,
+	.open			= ceu_open,
+	.release		= ceu_release,
+	.unlocked_ioctl		= video_ioctl2,
+	.mmap			= vb2_fop_mmap,
+	.poll			= vb2_fop_poll,
+};
+
+/* --- Video Device IOCTLs --- */
+
+static int ceu_querycap(struct file *file, void *priv,
+			struct v4l2_capability *cap)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+
+	strlcpy(cap->card, "Renesas CEU", sizeof(cap->card));
+	strlcpy(cap->driver, DRIVER_NAME, sizeof(cap->driver));
+	snprintf(cap->bus_info, sizeof(cap->bus_info),
+		 "platform:renesas-ceu-%s", dev_name(ceudev->dev));
+
+	return 0;
+}
+
+static int ceu_enum_fmt_vid_cap(struct file *file, void *priv,
+				struct v4l2_fmtdesc *f)
+{
+	const struct ceu_fmt *fmt;
+
+	if (f->index >= ARRAY_SIZE(ceu_fmt_list))
+		return -EINVAL;
+
+	fmt = &ceu_fmt_list[f->index];
+	f->pixelformat = fmt->fourcc;
+
+	return 0;
+}
+
+static int ceu_try_fmt_vid_cap(struct file *file, void *priv,
+			       struct v4l2_format *f)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+
+	return ceu_try_fmt(ceudev, f);
+}
+
+static int ceu_s_fmt_vid_cap(struct file *file, void *priv,
+			     struct v4l2_format *f)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+
+	if (vb2_is_streaming(&ceudev->vb2_vq))
+		return -EBUSY;
+
+	return ceu_set_fmt(ceudev, f);
+}
+
+static int ceu_g_fmt_vid_cap(struct file *file, void *priv,
+			     struct v4l2_format *f)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+
+	f->fmt.pix_mp = ceudev->v4l2_pix;
+
+	return 0;
+}
+
+static int ceu_enum_input(struct file *file, void *priv,
+			  struct v4l2_input *inp)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+	struct ceu_subdev *ceusd;
+
+	if (inp->index >= ceudev->num_sd)
+		return -EINVAL;
+
+	ceusd = &ceudev->subdevs[inp->index];
+
+	inp->type = V4L2_INPUT_TYPE_CAMERA;
+	inp->std = 0;
+	snprintf(inp->name, sizeof(inp->name), "Camera%u: %s",
+		 inp->index, ceusd->v4l2_sd->name);
+
+	return 0;
+}
+
+static int ceu_g_input(struct file *file, void *priv, unsigned int *i)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+
+	*i = ceudev->sd_index;
+
+	return 0;
+}
+
+static int ceu_s_input(struct file *file, void *priv, unsigned int i)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+	struct ceu_subdev *ceu_sd_old;
+	int ret;
+
+	if (i >= ceudev->num_sd)
+		return -EINVAL;
+
+	if (vb2_is_streaming(&ceudev->vb2_vq))
+		return -EBUSY;
+
+	if (i == ceudev->sd_index)
+		return 0;
+
+	ceu_sd_old = ceudev->sd;
+	ceudev->sd = &ceudev->subdevs[i];
+
+	/* Make sure we can generate output image formats. */
+	ret = ceu_init_formats(ceudev);
+	if (ret) {
+		ceudev->sd = ceu_sd_old;
+		return -EINVAL;
+	}
+
+	/* now that we're sure we can use the sensor, power off the old one. */
+	v4l2_subdev_call(ceu_sd_old->v4l2_sd, core, s_power, 0);
+	v4l2_subdev_call(ceudev->sd->v4l2_sd, core, s_power, 1);
+
+	ceudev->sd_index = i;
+
+	return 0;
+}
+
+static int ceu_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		return -EINVAL;
+
+	return v4l2_subdev_call(ceudev->sd->v4l2_sd, video, g_parm, a);
+}
+
+static int ceu_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		return -EINVAL;
+
+	return v4l2_subdev_call(ceudev->sd->v4l2_sd, video, s_parm, a);
+}
+
+static int ceu_enum_framesizes(struct file *file, void *fh,
+			       struct v4l2_frmsizeenum *fsize)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+	struct ceu_subdev *ceu_sd = ceudev->sd;
+	const struct ceu_fmt *ceu_fmt;
+	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
+	int ret;
+
+	struct v4l2_subdev_frame_size_enum fse = {
+		.code	= ceu_sd->mbus_fmt.mbus_code,
+		.index	= fsize->index,
+		.which	= V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+
+	/* Just check if user supplied pixel format is supported. */
+	ceu_fmt = get_ceu_fmt_from_fourcc(fsize->pixel_format);
+	if (!ceu_fmt)
+		return -EINVAL;
+
+	ret = v4l2_subdev_call(v4l2_sd, pad, enum_frame_size,
+			       NULL, &fse);
+	if (ret)
+		return ret;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+	fsize->discrete.width = CEU_W_MAX(fse.max_width);
+	fsize->discrete.height = CEU_H_MAX(fse.max_height);
+
+	return 0;
+}
+
+static int ceu_enum_frameintervals(struct file *file, void *fh,
+				   struct v4l2_frmivalenum *fival)
+{
+	struct ceu_device *ceudev = video_drvdata(file);
+	struct ceu_subdev *ceu_sd = ceudev->sd;
+	const struct ceu_fmt *ceu_fmt;
+	struct v4l2_subdev *v4l2_sd = ceu_sd->v4l2_sd;
+	int ret;
+
+	struct v4l2_subdev_frame_interval_enum fie = {
+		.code	= ceu_sd->mbus_fmt.mbus_code,
+		.index = fival->index,
+		.width = fival->width,
+		.height = fival->height,
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+
+	/* Just check if user supplied pixel format is supported. */
+	ceu_fmt = get_ceu_fmt_from_fourcc(fival->pixel_format);
+	if (!ceu_fmt)
+		return -EINVAL;
+
+	ret = v4l2_subdev_call(v4l2_sd, pad, enum_frame_interval, NULL,
+			       &fie);
+	if (ret)
+		return ret;
+
+	fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
+	fival->discrete = fie.interval;
+
+	return 0;
+}
+
+static const struct v4l2_ioctl_ops ceu_ioctl_ops = {
+	.vidioc_querycap		= ceu_querycap,
+
+	.vidioc_enum_fmt_vid_cap_mplane	= ceu_enum_fmt_vid_cap,
+	.vidioc_try_fmt_vid_cap_mplane	= ceu_try_fmt_vid_cap,
+	.vidioc_s_fmt_vid_cap_mplane	= ceu_s_fmt_vid_cap,
+	.vidioc_g_fmt_vid_cap_mplane	= ceu_g_fmt_vid_cap,
+
+	.vidioc_enum_input		= ceu_enum_input,
+	.vidioc_g_input			= ceu_g_input,
+	.vidioc_s_input			= ceu_s_input,
+
+	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
+	.vidioc_querybuf		= vb2_ioctl_querybuf,
+	.vidioc_qbuf			= vb2_ioctl_qbuf,
+	.vidioc_expbuf			= vb2_ioctl_expbuf,
+	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
+	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
+	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
+	.vidioc_streamon		= vb2_ioctl_streamon,
+	.vidioc_streamoff		= vb2_ioctl_streamoff,
+
+	.vidioc_g_parm			= ceu_g_parm,
+	.vidioc_s_parm			= ceu_s_parm,
+	.vidioc_enum_framesizes		= ceu_enum_framesizes,
+	.vidioc_enum_frameintervals	= ceu_enum_frameintervals,
+};
+
+/*
+ * ceu_vdev_release() - release CEU video device memory when last reference
+ *			to this driver is closed
+ */
+static void ceu_vdev_release(struct video_device *vdev)
+{
+	struct ceu_device *ceudev = video_get_drvdata(vdev);
+
+	kfree(ceudev);
+}
+
+static int ceu_notify_bound(struct v4l2_async_notifier *notifier,
+			    struct v4l2_subdev *v4l2_sd,
+			    struct v4l2_async_subdev *asd)
+{
+	struct v4l2_device *v4l2_dev = notifier->v4l2_dev;
+	struct ceu_device *ceudev = v4l2_to_ceu(v4l2_dev);
+	struct ceu_subdev *ceu_sd = to_ceu_subdev(asd);
+
+	ceu_sd->v4l2_sd = v4l2_sd;
+	ceudev->num_sd++;
+
+	return 0;
+}
+
+static int ceu_notify_complete(struct v4l2_async_notifier *notifier)
+{
+	struct v4l2_device *v4l2_dev = notifier->v4l2_dev;
+	struct ceu_device *ceudev = v4l2_to_ceu(v4l2_dev);
+	struct video_device *vdev = &ceudev->vdev;
+	struct vb2_queue *q = &ceudev->vb2_vq;
+	struct v4l2_subdev *v4l2_sd;
+	int ret;
+
+	/* Initialize vb2 queue. */
+	q->type			= V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+	q->io_modes		= VB2_MMAP | VB2_DMABUF;
+	q->drv_priv		= ceudev;
+	q->ops			= &ceu_vb2_ops;
+	q->mem_ops		= &vb2_dma_contig_memops;
+	q->buf_struct_size	= sizeof(struct ceu_buffer);
+	q->timestamp_flags	= V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->min_buffers_needed	= 2;
+	q->lock			= &ceudev->mlock;
+	q->dev			= ceudev->v4l2_dev.dev;
+
+	ret = vb2_queue_init(q);
+	if (ret)
+		return ret;
+
+	/*
+	 * Make sure at least one sensor is primary and use it to initialize
+	 * ceu formats.
+	 */
+	if (!ceudev->sd) {
+		ceudev->sd = &ceudev->subdevs[0];
+		ceudev->sd_index = 0;
+	}
+
+	v4l2_sd = ceudev->sd->v4l2_sd;
+
+	ret = ceu_init_formats(ceudev);
+	if (ret)
+		return ret;
+
+	ret = ceu_set_default_fmt(ceudev);
+	if (ret)
+		return ret;
+
+	/* Register the video device. */
+	strncpy(vdev->name, DRIVER_NAME, strlen(DRIVER_NAME));
+	vdev->v4l2_dev		= v4l2_dev;
+	vdev->lock		= &ceudev->mlock;
+	vdev->queue		= &ceudev->vb2_vq;
+	vdev->ctrl_handler	= v4l2_sd->ctrl_handler;
+	vdev->fops		= &ceu_fops;
+	vdev->ioctl_ops		= &ceu_ioctl_ops;
+	vdev->release		= ceu_vdev_release;
+	vdev->device_caps	= V4L2_CAP_VIDEO_CAPTURE_MPLANE |
+				  V4L2_CAP_STREAMING;
+	video_set_drvdata(vdev, ceudev);
+
+	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
+	if (ret < 0) {
+		v4l2_err(vdev->v4l2_dev,
+			 "video_register_device failed: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct v4l2_async_notifier_operations ceu_notify_ops = {
+	.bound		= ceu_notify_bound,
+	.complete	= ceu_notify_complete,
+};
+
+/*
+ * ceu_init_async_subdevs() - Initialize CEU subdevices and async_subdevs in
+ *			      ceu device. Both DT and platform data parsing use
+ *			      this routine.
+ *
+ * Returns 0 for success, -ENOMEM for failure.
+ */
+static int ceu_init_async_subdevs(struct ceu_device *ceudev, unsigned int n_sd)
+{
+	/* Reserve memory for 'n_sd' ceu_subdev descriptors. */
+	ceudev->subdevs = devm_kcalloc(ceudev->dev, n_sd,
+				       sizeof(*ceudev->subdevs), GFP_KERNEL);
+	if (!ceudev->subdevs)
+		return -ENOMEM;
+
+	/*
+	 * Reserve memory for 'n_sd' pointers to async_subdevices.
+	 * ceudev->asds members will point to &ceu_subdev.asd
+	 */
+	ceudev->asds = devm_kcalloc(ceudev->dev, n_sd,
+				    sizeof(*ceudev->asds), GFP_KERNEL);
+	if (!ceudev->asds)
+		return -ENOMEM;
+
+	ceudev->sd = NULL;
+	ceudev->sd_index = 0;
+	ceudev->num_sd = 0;
+
+	return 0;
+}
+
+/*
+ * ceu_parse_platform_data() - Initialize async_subdevices using platform
+ *			       device provided data.
+ */
+static int ceu_parse_platform_data(struct ceu_device *ceudev,
+				   const struct ceu_platform_data *pdata)
+{
+	const struct ceu_async_subdev *async_sd;
+	struct ceu_subdev *ceu_sd;
+	unsigned int i;
+	int ret;
+
+	if (pdata->num_subdevs == 0)
+		return -ENODEV;
+
+	ret = ceu_init_async_subdevs(ceudev, pdata->num_subdevs);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < pdata->num_subdevs; i++) {
+		/* Setup the ceu subdevice and the async subdevice. */
+		async_sd = &pdata->subdevs[i];
+		ceu_sd = &ceudev->subdevs[i];
+
+		INIT_LIST_HEAD(&ceu_sd->asd.list);
+
+		ceu_sd->mbus_flags	= async_sd->flags;
+		ceu_sd->asd.match_type	= V4L2_ASYNC_MATCH_I2C;
+		ceu_sd->asd.match.i2c.adapter_id = async_sd->i2c_adapter_id;
+		ceu_sd->asd.match.i2c.address = async_sd->i2c_address;
+
+		ceudev->asds[i] = &ceu_sd->asd;
+	}
+
+	return pdata->num_subdevs;
+}
+
+/*
+ * ceu_parse_dt() - Initialize async_subdevs parsing device tree graph.
+ */
+static int ceu_parse_dt(struct ceu_device *ceudev)
+{
+	struct device_node *of = ceudev->dev->of_node;
+	struct v4l2_fwnode_endpoint fw_ep;
+	struct ceu_subdev *ceu_sd;
+	struct device_node *ep;
+	unsigned int i;
+	int num_ep;
+	int ret;
+
+	num_ep = of_graph_get_endpoint_count(of);
+	if (!num_ep)
+		return -ENODEV;
+
+	ret = ceu_init_async_subdevs(ceudev, num_ep);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < num_ep; i++) {
+		ep = of_graph_get_endpoint_by_regs(of, 0, i);
+		if (!ep) {
+			dev_err(ceudev->dev,
+				"No subdevice connected on endpoint %u.\n", i);
+			ret = -ENODEV;
+			goto error_put_node;
+		}
+
+		ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &fw_ep);
+		if (ret) {
+			dev_err(ceudev->dev,
+				"Unable to parse endpoint #%u.\n", i);
+			goto error_put_node;
+		}
+
+		if (fw_ep.bus_type != V4L2_MBUS_PARALLEL) {
+			dev_err(ceudev->dev,
+				"Only parallel input supported.\n");
+			ret = -EINVAL;
+			goto error_put_node;
+		}
+
+		/* Setup the ceu subdevice and the async subdevice. */
+		ceu_sd = &ceudev->subdevs[i];
+		INIT_LIST_HEAD(&ceu_sd->asd.list);
+
+		ceu_sd->mbus_flags = fw_ep.bus.parallel.flags;
+		ceu_sd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
+		ceu_sd->asd.match.fwnode.fwnode =
+			fwnode_graph_get_remote_port_parent(
+					of_fwnode_handle(ep));
+
+		ceudev->asds[i] = &ceu_sd->asd;
+		of_node_put(ep);
+	}
+
+	return num_ep;
+
+error_put_node:
+	of_node_put(ep);
+	return ret;
+}
+
+/*
+ * struct ceu_data - Platform specific CEU data
+ * @irq_mask: CETCR mask with all interrupt sources enabled. The mask differs
+ *	      between SH4 and RZ platforms.
+ */
+struct ceu_data {
+	u32 irq_mask;
+};
+
+static const struct ceu_data ceu_data_rz = {
+	.irq_mask = CEU_CETCR_ALL_IRQS_RZ,
+};
+
+static const struct ceu_data ceu_data_sh4 = {
+	.irq_mask = CEU_CETCR_ALL_IRQS_SH4,
+};
+
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id ceu_of_match[] = {
+	{ .compatible = "renesas,r7s72100-ceu", .data = &ceu_data_rz },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ceu_of_match);
+#endif
+
+static int ceu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct ceu_data *ceu_data;
+	struct ceu_device *ceudev;
+	struct resource *res;
+	unsigned int irq;
+	int num_subdevs;
+	int ret;
+
+	ceudev = kzalloc(sizeof(*ceudev), GFP_KERNEL);
+	if (!ceudev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, ceudev);
+	ceudev->dev = dev;
+
+	INIT_LIST_HEAD(&ceudev->capture);
+	spin_lock_init(&ceudev->lock);
+	mutex_init(&ceudev->mlock);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ceudev->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ceudev->base))
+		goto error_free_ceudev;
+
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0) {
+		dev_err(dev, "Failed to get irq: %d\n", ret);
+		goto error_free_ceudev;
+	}
+	irq = ret;
+
+	ret = devm_request_irq(dev, irq, ceu_irq,
+			       0, dev_name(dev), ceudev);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to request CEU interrupt.\n");
+		goto error_free_ceudev;
+	}
+
+	pm_runtime_enable(dev);
+
+	ret = v4l2_device_register(dev, &ceudev->v4l2_dev);
+	if (ret)
+		goto error_pm_disable;
+
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		ceu_data = of_match_device(ceu_of_match, dev)->data;
+		num_subdevs = ceu_parse_dt(ceudev);
+	} else if (dev->platform_data) {
+		/* Assume SH4 if booting with platform data. */
+		ceu_data = &ceu_data_sh4;
+		num_subdevs = ceu_parse_platform_data(ceudev,
+						      dev->platform_data);
+	} else {
+		num_subdevs = -EINVAL;
+	}
+
+	if (num_subdevs < 0) {
+		ret = num_subdevs;
+		goto error_v4l2_unregister;
+	}
+	ceudev->irq_mask = ceu_data->irq_mask;
+
+	ceudev->notifier.v4l2_dev	= &ceudev->v4l2_dev;
+	ceudev->notifier.subdevs	= ceudev->asds;
+	ceudev->notifier.num_subdevs	= num_subdevs;
+	ceudev->notifier.ops		= &ceu_notify_ops;
+	ret = v4l2_async_notifier_register(&ceudev->v4l2_dev,
+					   &ceudev->notifier);
+	if (ret)
+		goto error_v4l2_unregister;
+
+	dev_info(dev, "Renesas Capture Engine Unit %s\n", dev_name(dev));
+
+	return 0;
+
+error_v4l2_unregister:
+	v4l2_device_unregister(&ceudev->v4l2_dev);
+error_pm_disable:
+	pm_runtime_disable(dev);
+error_free_ceudev:
+	kfree(ceudev);
+
+	return ret;
+}
+
+static int ceu_remove(struct platform_device *pdev)
+{
+	struct ceu_device *ceudev = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(ceudev->dev);
+
+	v4l2_async_notifier_unregister(&ceudev->notifier);
+
+	v4l2_device_unregister(&ceudev->v4l2_dev);
+
+	video_unregister_device(&ceudev->vdev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops ceu_pm_ops = {
+	SET_RUNTIME_PM_OPS(ceu_runtime_suspend,
+			   ceu_runtime_resume,
+			   NULL)
+};
+
+static struct platform_driver ceu_driver = {
+	.driver		= {
+		.name	= DRIVER_NAME,
+		.pm	= &ceu_pm_ops,
+		.of_match_table = of_match_ptr(ceu_of_match),
+	},
+	.probe		= ceu_probe,
+	.remove		= ceu_remove,
+};
+
+module_platform_driver(ceu_driver);
+
+MODULE_DESCRIPTION("Renesas CEU camera driver");
+MODULE_AUTHOR("Jacopo Mondi <[e-mail]>");
+MODULE_LICENSE("GPL v2");