Re: [PATCH 2/2] Add basic support for DW CSI-2 Host IPK

From: Sakari Ailus
Date: Thu Nov 17 2016 - 12:15:02 EST


Hi Ramiro,

Thank you for the patchset. Please see my comments below.

If you've got a CSI-2 receiver, why do you use the DV timing presets? What
do you use those for?

On Mon, Nov 14, 2016 at 02:20:23PM +0000, Ramiro Oliveira wrote:
> Add support for basic configuration of the DW CSI-2 Host IPK
>
> Signed-off-by: Ramiro Oliveira <roliveir@xxxxxxxxxxxx>
> ---
> MAINTAINERS | 7 +
> drivers/media/platform/Kconfig | 1 +
> drivers/media/platform/Makefile | 2 +
> drivers/media/platform/dwc/Kconfig | 36 ++
> drivers/media/platform/dwc/Makefile | 3 +
> drivers/media/platform/dwc/dw_mipi_csi.c | 687 +++++++++++++++++++++++
> drivers/media/platform/dwc/dw_mipi_csi.h | 179 ++++++
> drivers/media/platform/dwc/plat_ipk.c | 835 ++++++++++++++++++++++++++++
> drivers/media/platform/dwc/plat_ipk.h | 97 ++++
> drivers/media/platform/dwc/plat_ipk_video.h | 97 ++++
> drivers/media/platform/dwc/video_device.c | 741 ++++++++++++++++++++++++
> drivers/media/platform/dwc/video_device.h | 101 ++++
> 12 files changed, 2786 insertions(+)
> create mode 100644 drivers/media/platform/dwc/Kconfig
> create mode 100644 drivers/media/platform/dwc/Makefile
> create mode 100644 drivers/media/platform/dwc/dw_mipi_csi.c
> create mode 100644 drivers/media/platform/dwc/dw_mipi_csi.h
> create mode 100644 drivers/media/platform/dwc/plat_ipk.c
> create mode 100644 drivers/media/platform/dwc/plat_ipk.h
> create mode 100644 drivers/media/platform/dwc/plat_ipk_video.h
> create mode 100644 drivers/media/platform/dwc/video_device.c
> create mode 100644 drivers/media/platform/dwc/video_device.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6a71422..f54dfd8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10618,6 +10618,13 @@ S: Maintained
> F: drivers/staging/media/st-cec/
> F: Documentation/devicetree/bindings/media/stih-cec.txt
>
> +SYNOPSYS DESIGNWARE CSI-2 HOST IPK DRIVER
> +M: Ramiro Oliveira <roliveir@xxxxxxxxxxxx>
> +L: linux-media@xxxxxxxxxxxxxxx
> +T: git git://linuxtv.org/media_tree.git
> +S: Maintained
> +F: drivers/media/platform/dwc/
> +
> SYNOPSYS DESIGNWARE DMAC DRIVER
> M: Viresh Kumar <vireshk@xxxxxxxxxx>
> M: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 754edbf1..6fef760f 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -120,6 +120,7 @@ source "drivers/media/platform/am437x/Kconfig"
> source "drivers/media/platform/xilinx/Kconfig"
> source "drivers/media/platform/rcar-vin/Kconfig"
> source "drivers/media/platform/atmel/Kconfig"
> +source "drivers/media/platform/dwc/Kconfig"
>
> config VIDEO_TI_CAL
> tristate "TI CAL (Camera Adaptation Layer) driver"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index f842933..623f5d2 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -61,6 +61,8 @@ obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin/
>
> obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel/
>
> +obj-$(CONFIG_VIDEO_DWC) += dwc/
> +
> ccflags-y += -I$(srctree)/drivers/media/i2c
>
> obj-$(CONFIG_VIDEO_MEDIATEK_VPU) += mtk-vpu/
> diff --git a/drivers/media/platform/dwc/Kconfig b/drivers/media/platform/dwc/Kconfig
> new file mode 100644
> index 0000000..fb8533b
> --- /dev/null
> +++ b/drivers/media/platform/dwc/Kconfig
> @@ -0,0 +1,36 @@
> +config VIDEO_DWC
> + bool "Designware Cores CSI-2 IPK (EXPERIMENTAL)"
> + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA
> + help
> + Say Y here to enable support for the DesignWare Cores CSI-2 Host IP
> + Prototyping Kit.
> +
> +if VIDEO_DWC
> +config DWC_PLATFORM
> + tristate "SNPS DWC MIPI CSI2 Host"
> + depends on VIDEO_DWC
> + help
> + This is the V4L2 plaftorm driver driver for the DWC CSI-2 HOST IPK
> +
> + To compile this driver as a module, choose M here
> +
> +
> +config DWC_MIPI_CSI2_HOST
> + tristate "SNPS DWC MIPI CSI2 Host"
> + select GENERIC_PHY
> + depends on VIDEO_DWC
> + help
> + This is a V4L2 driver for SNPS DWC MIPI-CSI2
> +
> + To compile this driver as a module, choose M here
> +
> +config DWC_VIDEO_DEVICE
> + tristate "DWC VIDEO DEVICE"
> + select VIDEOBUF2_VMALLOC
> + select VIDEOBUF2_DMA_CONTIG
> + depends on VIDEO_DWC
> + help
> + This is a V4L2 driver for SNPS Video device
> + To compile this driver as a module, choose M here
> +
> +endif # VIDEO_DWC
> diff --git a/drivers/media/platform/dwc/Makefile b/drivers/media/platform/dwc/Makefile
> new file mode 100644
> index 0000000..75c74b7
> --- /dev/null
> +++ b/drivers/media/platform/dwc/Makefile
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_DWC_PLATFORM) += plat_ipk.o
> +obj-$(CONFIG_DWC_MIPI_CSI2_HOST) += dw_mipi_csi.o
> +obj-$(CONFIG_DWC_VIDEO_DEVICE) += video_device.o
> diff --git a/drivers/media/platform/dwc/dw_mipi_csi.c b/drivers/media/platform/dwc/dw_mipi_csi.c
> new file mode 100644
> index 0000000..1337a5e
> --- /dev/null
> +++ b/drivers/media/platform/dwc/dw_mipi_csi.c
> @@ -0,0 +1,687 @@
> +/*
> + * DWC MIPI CSI-2 Host device driver
> + *
> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
> + * Author: Ramiro Oliveira <ramiro.oliveira@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published
> + * by the Free Software Foundation, either version 2 of the License,
> + * or (at your option) any later version.
> + */
> +
> +
> +#include "dw_mipi_csi.h"
> +
> +/** @short Driver args: debug */
> +static unsigned int debug;
> +module_param(debug, uint, 0644);
> +MODULE_PARM_DESC(debug, "Activates debug info");

There should be no need for module specific debug parameters. Please use
dynamic debug instead.

> +
> +/*
> + * Basic IO read and write operations
> + */
> +
> +/**
> + * @short Video formats supported by the MIPI CSI-2
> + */
> +static const struct mipi_fmt dw_mipi_csi_formats[] = {
> + {
> + .name = "RAW BGGR 8",

Do you use the name field for something? If not, I'd suggest to drop it.

> + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> + .depth = 8,
> + },
> + {
> + .name = "RAW10",
> + .code = MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE,
> + .depth = 10,
> + },
> + {
> + .name = "RGB565",
> + .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
> + .depth = 16,
> + },
> + {
> + .name = "BGR565",
> + .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
> + .depth = 16,
> + },
> + {
> + .name = "RGB888",
> + .code = MEDIA_BUS_FMT_RGB888_2X12_LE,
> + .depth = 24,
> + },
> + {
> + .name = "BGR888",
> + .code = MEDIA_BUS_FMT_RGB888_2X12_BE,
> + .depth = 24,
> + },
> +};
> +
> +static struct mipi_csi_dev *
> +sd_to_mipi_csi_dev(struct v4l2_subdev *sdev)
> +{
> + return container_of(sdev, struct mipi_csi_dev, sd);
> +}
> +
> +void
> +dw_mipi_csi_write(struct mipi_csi_dev *dev,
> + unsigned int address, unsigned int data)
> +{
> + iowrite32(data, dev->base_address + address);
> +}
> +
> +u32
> +dw_mipi_csi_read(struct mipi_csi_dev *dev, unsigned long address)
> +{
> + return ioread32(dev->base_address + address);
> +}
> +
> +void
> +dw_mipi_csi_write_part(struct mipi_csi_dev *dev,
> + unsigned long address, unsigned long data,
> + unsigned char shift, unsigned char width)
> +{
> + u32 mask = (1 << width) - 1;
> + u32 temp = dw_mipi_csi_read(dev, address);
> +
> + temp &= ~(mask << shift);
> + temp |= (data & mask) << shift;
> + dw_mipi_csi_write(dev, address, temp);
> +}
> +
> +static const struct mipi_fmt *
> +find_dw_mipi_csi_format(struct v4l2_mbus_framefmt *mf)
> +{
> + int i;

unsigned int

> +
> + for (i = 0; i < ARRAY_SIZE(dw_mipi_csi_formats); i++)
> + if (mf->code == dw_mipi_csi_formats[i].code)
> + return &dw_mipi_csi_formats[i];
> + return NULL;
> +}
> +
> +int
> +enable_video_output(struct mipi_csi_dev *dev)
> +{
> + return 0;
> +}

This one is unused.

> +
> +int
> +disable_video_output(struct mipi_csi_dev *dev)
> +{
> + phy_power_off(dev->phy);
> + return 0;
> +}

This one as well.

> +
> +static void
> +dw_mipi_csi_reset(struct mipi_csi_dev *dev)
> +{
> + dw_mipi_csi_write(dev, R_CSI2_CTRL_RESETN, 0);
> + /* mdelay(1); */
> + dw_mipi_csi_write(dev, R_CSI2_CTRL_RESETN, 1);
> +}
> +
> +static int
> +dw_mipi_csi_mask_irq_power_off(struct mipi_csi_dev *dev)
> +{
> + /* set only one lane (lane 0) as active (ON) */
> + dw_mipi_csi_write(dev, R_CSI2_N_LANES, 0);
> +
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY_FATAL, 0);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT_FATAL, 0);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_FRAME_FATAL, 0);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY, 0);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT, 0);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_LINE, 0);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_IPI, 0);
> +
> + dw_mipi_csi_write(dev, R_CSI2_CTRL_RESETN, 0);
> +
> + return 0;
> +
> +}
> +
> +static int
> +dw_mipi_csi_hw_stdby(struct mipi_csi_dev *dev)
> +{
> + /* set only one lane (lane 0) as active (ON) */
> + dw_mipi_csi_reset(dev);
> +
> + dw_mipi_csi_write(dev, R_CSI2_N_LANES, 0);
> +
> + phy_init(dev->phy);
> +
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY_FATAL, 0xFFFFFFFF);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT_FATAL, 0xFFFFFFFF);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_FRAME_FATAL, 0xFFFFFFFF);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY, 0xFFFFFFFF);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT, 0xFFFFFFFF);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_LINE, 0xFFFFFFFF);
> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_IPI, 0xFFFFFFFF);
> +
> + return 0;
> +
> +}
> +
> +void

Shouldn't this be static? The same applies to many other symbols as well.

> +dw_mipi_csi_set_ipi_fmt(struct mipi_csi_dev *dev)
> +{
> + switch (dev->fmt->code) {
> + case MEDIA_BUS_FMT_RGB565_2X8_BE:
> + case MEDIA_BUS_FMT_RGB565_2X8_LE:
> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RGB565);
> + v4l2_dbg(1, debug, &dev->sd, "DT: RGB 565");
> + break;
> +
> + case MEDIA_BUS_FMT_RGB888_2X12_LE:
> + case MEDIA_BUS_FMT_RGB888_2X12_BE:
> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RGB888);
> + v4l2_dbg(1, debug, &dev->sd, "DT: RGB 888");
> + break;
> + case MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE:
> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RAW10);
> + v4l2_dbg(1, debug, &dev->sd, "DT: RAW 10");
> + break;
> + case MEDIA_BUS_FMT_SBGGR8_1X8:
> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RAW8);
> + v4l2_dbg(1, debug, &dev->sd, "DT: RAW 8");
> + break;
> + default:
> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RGB565);
> + v4l2_dbg(1, debug, &dev->sd, "Error");
> + break;
> + }
> +}
> +
> +void
> +__dw_mipi_csi_fill_timings(struct mipi_csi_dev *dev,
> + const struct v4l2_bt_timings *bt)
> +{
> +
> + if (bt == NULL)
> + return;
> +
> + dev->hw.hsa = bt->hsync;
> + dev->hw.hbp = bt->hbackporch;
> + dev->hw.hsd = bt->hsync;
> + dev->hw.htotal = bt->height + bt->vfrontporch +
> + bt->vsync + bt->vbackporch;
> + dev->hw.vsa = bt->vsync;
> + dev->hw.vbp = bt->vbackporch;
> + dev->hw.vfp = bt->vfrontporch;
> + dev->hw.vactive = bt->height;
> +}

There seem to be quite a few unused functions here.

> +
> +void
> +dw_mipi_csi_start(struct mipi_csi_dev *dev)
> +{
> + const struct v4l2_bt_timings *bt = &v4l2_dv_timings_presets[0].bt;
> +
> + __dw_mipi_csi_fill_timings(dev, bt);
> +
> + dw_mipi_csi_write(dev, R_CSI2_N_LANES, (dev->hw.num_lanes - 1));
> + v4l2_dbg(1, debug, &dev->sd, "N Lanes: %d\n", dev->hw.num_lanes);
> +
> + /*IPI Related Configuration */
> + if ((dev->hw.output_type == IPI_OUT)
> + || (dev->hw.output_type == BOTH_OUT)) {
> +
> + dw_mipi_csi_write_part(dev, R_CSI2_IPI_MODE, dev->hw.ipi_mode,
> + 0, 1);
> + v4l2_dbg(1, debug, &dev->sd, "IPI MODE: %d\n",
> + dev->hw.ipi_mode);
> +
> + dw_mipi_csi_write_part(dev, R_CSI2_IPI_MODE,
> + dev->hw.ipi_color_mode, 8, 1);
> + v4l2_dbg(1, debug, &dev->sd, "Color Mode: %d\n",
> + dev->hw.ipi_color_mode);
> +
> + dw_mipi_csi_write(dev, R_CSI2_IPI_VCID, dev->hw.virtual_ch);
> + v4l2_dbg(1, debug, &dev->sd, "Virtual Channel: %d\n",
> + dev->hw.virtual_ch);
> +
> + dw_mipi_csi_write_part(dev, R_CSI2_IPI_MEM_FLUSH,
> + dev->hw.ipi_auto_flush, 8, 1);
> + v4l2_dbg(1, debug, &dev->sd, "Auto-flush: %d\n",
> + dev->hw.ipi_auto_flush);
> +
> + dw_mipi_csi_write(dev, R_CSI2_IPI_HSA_TIME, dev->hw.hsa);
> + v4l2_dbg(1, debug, &dev->sd, "HSA: %d\n", dev->hw.hsa);
> +
> + dw_mipi_csi_write(dev, R_CSI2_IPI_HBP_TIME, dev->hw.hbp);
> + v4l2_dbg(1, debug, &dev->sd, "HBP: %d\n", dev->hw.hbp);
> +
> + dw_mipi_csi_write(dev, R_CSI2_IPI_HSD_TIME, dev->hw.hsd);
> + v4l2_dbg(1, debug, &dev->sd, "HSD: %d\n", dev->hw.hsd);
> +
> + if (dev->hw.ipi_mode == AUTO_TIMING) {
> + dw_mipi_csi_write(dev, R_CSI2_IPI_HLINE_TIME,
> + dev->hw.htotal);
> + v4l2_dbg(1, debug, &dev->sd, "H total: %d\n",
> + dev->hw.htotal);
> +
> + dw_mipi_csi_write(dev, R_CSI2_IPI_VSA_LINES,
> + dev->hw.vsa);
> + v4l2_dbg(1, debug, &dev->sd, "VSA: %d\n", dev->hw.vsa);
> +
> + dw_mipi_csi_write(dev, R_CSI2_IPI_VBP_LINES,
> + dev->hw.vbp);
> + v4l2_dbg(1, debug, &dev->sd, "VBP: %d\n", dev->hw.vbp);
> +
> + dw_mipi_csi_write(dev, R_CSI2_IPI_VFP_LINES,
> + dev->hw.vfp);
> + v4l2_dbg(1, debug, &dev->sd, "VFP: %d\n", dev->hw.vfp);
> +
> + dw_mipi_csi_write(dev, R_CSI2_IPI_VACTIVE_LINES,
> + dev->hw.vactive);
> + v4l2_dbg(1, debug, &dev->sd, "V Active: %d\n",
> + dev->hw.vactive);
> + }
> + }
> +
> + phy_power_on(dev->phy);
> +}
> +
> +static int
> +dw_mipi_csi_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + if (code->index >= ARRAY_SIZE(dw_mipi_csi_formats))
> + return -EINVAL;
> +
> + code->code = dw_mipi_csi_formats[code->index].code;
> + return 0;
> +}
> +
> +static struct mipi_fmt const *
> +dw_mipi_csi_try_format(struct v4l2_mbus_framefmt *mf)
> +{
> + struct mipi_fmt const *fmt;
> +
> + fmt = find_dw_mipi_csi_format(mf);
> + if (fmt == NULL)
> + fmt = &dw_mipi_csi_formats[0];
> +
> + mf->code = fmt->code;
> + return fmt;
> +}
> +
> +static struct v4l2_mbus_framefmt *
> +__dw_mipi_csi_get_format(struct mipi_csi_dev *dev,
> + struct v4l2_subdev_pad_config *cfg,
> + enum v4l2_subdev_format_whence which)
> +{
> + if (which == V4L2_SUBDEV_FORMAT_TRY)
> + return cfg ? v4l2_subdev_get_try_format(&dev->sd, cfg,
> + 0) : NULL;
> +
> + return &dev->format;
> +}
> +
> +static int
> +dw_mipi_csi_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct mipi_csi_dev *dev = sd_to_mipi_csi_dev(sd);
> + struct mipi_fmt const *dev_fmt;
> + struct v4l2_mbus_framefmt *mf;
> + int i = 0;

unsigned int

> + const struct v4l2_bt_timings *bt_r = &v4l2_dv_timings_presets[0].bt;
> +
> + mf = __dw_mipi_csi_get_format(dev, cfg, fmt->which);
> +
> + dev_fmt = dw_mipi_csi_try_format(&fmt->format);
> + if (dev_fmt) {
> + *mf = fmt->format;
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> + dev->fmt = dev_fmt;
> + dw_mipi_csi_set_ipi_fmt(dev);
> + }
> + while (v4l2_dv_timings_presets[i].bt.width) {
> + const struct v4l2_bt_timings *bt =
> + &v4l2_dv_timings_presets[i].bt;
> + if (mf->width == bt->width && mf->height == bt->width) {
> + __dw_mipi_csi_fill_timings(dev, bt);
> + return 0;
> + }
> + i++;
> + }
> +
> + __dw_mipi_csi_fill_timings(dev, bt_r);
> + return 0;
> +
> +}
> +
> +static int
> +dw_mipi_csi_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct mipi_csi_dev *dev = sd_to_mipi_csi_dev(sd);
> + struct v4l2_mbus_framefmt *mf;
> +
> + mf = __dw_mipi_csi_get_format(dev, cfg, fmt->which);
> + if (!mf)
> + return -EINVAL;
> +
> + mutex_lock(&dev->lock);
> + fmt->format = *mf;
> + mutex_unlock(&dev->lock);
> + return 0;
> +}
> +
> +static int
> +dw_mipi_csi_s_power(struct v4l2_subdev *sd, int on)
> +{
> + struct mipi_csi_dev *dev = sd_to_mipi_csi_dev(sd);
> +
> + if (on) {
> + dw_mipi_csi_hw_stdby(dev);
> + dw_mipi_csi_start(dev);
> + } else {
> + dw_mipi_csi_mask_irq_power_off(dev);
> + }
> +
> + return 0;
> +}
> +
> +static int
> +dw_mipi_csi_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + struct v4l2_mbus_framefmt *format =
> + v4l2_subdev_get_try_format(sd, fh->pad, 0);
> +
> + format->colorspace = V4L2_COLORSPACE_SRGB;
> + format->code = dw_mipi_csi_formats[0].code;
> + format->width = MIN_WIDTH;
> + format->height = MIN_HEIGHT;
> + format->field = V4L2_FIELD_NONE;
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_internal_ops dw_mipi_csi_sd_internal_ops = {
> + .open = dw_mipi_csi_open,
> +};
> +
> +static struct v4l2_subdev_core_ops dw_mipi_csi_core_ops = {
> + .s_power = dw_mipi_csi_s_power,
> +};
> +
> +static struct v4l2_subdev_pad_ops dw_mipi_csi_pad_ops = {
> + .enum_mbus_code = dw_mipi_csi_enum_mbus_code,
> + .get_fmt = dw_mipi_csi_get_fmt,
> + .set_fmt = dw_mipi_csi_set_fmt,
> +};
> +
> +static struct v4l2_subdev_ops dw_mipi_csi_subdev_ops = {
> + .core = &dw_mipi_csi_core_ops,
> + .pad = &dw_mipi_csi_pad_ops,
> +};
> +
> +static irqreturn_t
> +dw_mipi_csi_irq1(int irq, void *dev_id)
> +{
> +
> + struct mipi_csi_dev *dev = dev_id;
> + u32 int_status, ind_status;
> + unsigned long flags;
> +
> + int_status = dw_mipi_csi_read(dev, R_CSI2_INTERRUPT);
> + spin_lock_irqsave(&dev->slock, flags);
> +
> + if (int_status & CSI2_INT_PHY_FATAL) {
> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PHY_FATAL);
> + pr_info_ratelimited("%08X CSI INT PHY FATAL: %08X\n",
> + (uint32_t) dev->base_address, ind_status);

I don't think you should use pr_info() for interrupts. This is basically
debug information, isn't it?

> + }
> +
> + if (int_status & CSI2_INT_PKT_FATAL) {
> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PKT_FATAL);
> + pr_info_ratelimited("%08X CSI INT PKT FATAL: %08X\n",
> + (uint32_t) dev->base_address, ind_status);
> + }
> +
> + if (int_status & CSI2_INT_FRAME_FATAL) {
> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_FRAME_FATAL);
> + pr_info_ratelimited("%08X CSI INT FRAME FATAL: %08X\n",
> + (uint32_t) dev->base_address, ind_status);
> + }
> +
> + if (int_status & CSI2_INT_PHY) {
> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PHY);
> + pr_info_ratelimited("%08X CSI INT PHY: %08X\n",
> + (uint32_t) dev->base_address, ind_status);
> + }
> +
> + if (int_status & CSI2_INT_PKT) {
> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PKT);
> + pr_info_ratelimited("%08X CSI INT PKT: %08X\n",
> + (uint32_t) dev->base_address, ind_status);
> + }
> +
> + if (int_status & CSI2_INT_LINE) {
> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_LINE);
> + pr_info_ratelimited("%08X CSI INT LINE: %08X\n",
> + (uint32_t) dev->base_address, ind_status);
> + }
> +
> + if (int_status & CSI2_INT_IPI) {
> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_IPI);
> + pr_info_ratelimited("%08X CSI INT IPI: %08X\n",
> + (uint32_t) dev->base_address, ind_status);
> + }
> + spin_unlock_irqrestore(&dev->slock, flags);
> + return IRQ_HANDLED;
> +}
> +
> +static int
> +dw_mipi_csi_parse_dt(struct platform_device *pdev, struct mipi_csi_dev *dev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + int reg;
> + int ret = 0;
> +
> + /* Device tree information */
> + ret = of_property_read_u32(node, "data-lanes", &dev->hw.num_lanes);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't read data-lanes\n");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "output-type", &dev->hw.output_type);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't read output-type\n");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "ipi-mode", &dev->hw.ipi_mode);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't read ipi-mode\n");
> + return ret;
> + }
> +
> + ret =
> + of_property_read_u32(node, "ipi-auto-flush",
> + &dev->hw.ipi_auto_flush);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't read ipi-auto-flush\n");
> + return ret;
> + }
> +
> + ret =
> + of_property_read_u32(node, "ipi-color-mode",
> + &dev->hw.ipi_color_mode);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't read ipi-color-mode\n");
> + return ret;
> + }
> +
> + ret =
> + of_property_read_u32(node, "virtual-channel", &dev->hw.virtual_ch);

Is this some thing that should come from DT? Sure, we don't really have
proper support for virtual channels right now, but could you use zero for
now?

> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't read virtual-channel\n");
> + return ret;
> + }
> +
> + node = of_get_child_by_name(node, "port");
> + if (!node)
> + return -EINVAL;
> +
> + ret = of_property_read_u32(node, "reg", &reg);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't read reg value\n");
> + return ret;
> + }
> + dev->index = reg - 1;
> +
> + if (dev->index >= CSI_MAX_ENTITIES)
> + return -ENXIO;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id dw_mipi_csi_of_match[];
> +
> +/**
> + * @short Initialization routine - Entry point of the driver
> + * @param[in] pdev pointer to the platform device structure
> + * @return 0 on success and a negative number on failure
> + * Refer to Linux errors.
> + */
> +static int mipi_csi_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *of_id;
> + struct device *dev = &pdev->dev;
> + struct resource *res = NULL;
> + struct mipi_csi_dev *mipi_csi;
> + int ret = -ENOMEM;
> +
> + dev_info(&pdev->dev, "Installing MIPI CSI-2 module\n");
> +
> + dev_dbg(&pdev->dev, "Device registration\n");

At least one of these is redundant. I'd just remove both.

> + mipi_csi = devm_kzalloc(dev, sizeof(*mipi_csi), GFP_KERNEL);
> + if (!dev)
> + return -ENOMEM;
> +
> + mutex_init(&mipi_csi->lock);
> + spin_lock_init(&mipi_csi->slock);
> + mipi_csi->pdev = pdev;
> +
> + of_id = of_match_node(dw_mipi_csi_of_match, dev->of_node);
> + if (WARN_ON(of_id == NULL))
> + return -EINVAL;
> +
> + ret = dw_mipi_csi_parse_dt(pdev, mipi_csi);
> + if (ret < 0)
> + return ret;
> +
> + dev_info(dev, "Request DPHY\n");
> + mipi_csi->phy = devm_phy_get(dev, "csi2-dphy");
> + if (IS_ERR(mipi_csi->phy))
> + return PTR_ERR(mipi_csi->phy);

It'd be better to tell what's wrong when there's an error instead of
printing a series of messages even when all is well.

> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mipi_csi->base_address = devm_ioremap_resource(dev, res);
> +
> + if (IS_ERR(mipi_csi->base_address))
> + return PTR_ERR(mipi_csi->base_address);
> +
> + mipi_csi->ctrl_irq_number = platform_get_irq(pdev, 0);
> + if (mipi_csi->ctrl_irq_number <= 0) {
> + dev_err(dev, "IRQ number not set.\n");
> + return mipi_csi->ctrl_irq_number;
> + }
> +
> + ret = devm_request_irq(dev, mipi_csi->ctrl_irq_number,
> + dw_mipi_csi_irq1, IRQF_SHARED,
> + dev_name(dev), mipi_csi);
> + if (ret) {
> + dev_err(dev, "IRQ failed\n");
> + goto end;
> + }
> +
> + v4l2_subdev_init(&mipi_csi->sd, &dw_mipi_csi_subdev_ops);
> + mipi_csi->sd.owner = THIS_MODULE;
> + snprintf(mipi_csi->sd.name, sizeof(mipi_csi->sd.name), "%s.%d",
> + CSI_DEVICE_NAME, mipi_csi->index);
> + mipi_csi->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + mipi_csi->fmt = &dw_mipi_csi_formats[0];
> +
> + mipi_csi->format.code = dw_mipi_csi_formats[0].code;
> + mipi_csi->format.width = MIN_WIDTH;
> + mipi_csi->format.height = MIN_HEIGHT;
> +
> + mipi_csi->pads[CSI_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> + mipi_csi->pads[CSI_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> + ret = media_entity_pads_init(&mipi_csi->sd.entity,
> + CSI_PADS_NUM, mipi_csi->pads);
> +
> + if (ret < 0) {
> + dev_err(dev, "Media Entity init failed\n");
> + goto entity_cleanup;
> + }
> +
> + /* This allows to retrieve the platform device id by the host driver */
> + v4l2_set_subdevdata(&mipi_csi->sd, pdev);
> +
> + /* .. and a pointer to the subdev. */
> + platform_set_drvdata(pdev, &mipi_csi->sd);
> +
> + dw_mipi_csi_mask_irq_power_off(mipi_csi);
> + dev_info(dev, "DW MIPI CSI-2 Host registered successfully\n");
> + return 0;
> +
> +entity_cleanup:
> + media_entity_cleanup(&mipi_csi->sd.entity);
> +end:
> + return ret;
> +}
> +
> +/**
> + * @short Exit routine - Exit point of the driver
> + * @param[in] pdev pointer to the platform device structure
> + * @return 0 on success and a negative number on failure
> + * Refer to Linux errors.
> + */
> +static int mipi_csi_remove(struct platform_device *pdev)
> +{
> + struct v4l2_subdev *sd = platform_get_drvdata(pdev);
> + struct mipi_csi_dev *mipi_csi = sd_to_mipi_csi_dev(sd);
> +
> + dev_dbg(&pdev->dev, "Removing MIPI CSI-2 module\n");
> + media_entity_cleanup(&mipi_csi->sd.entity);
> +
> + return 0;
> +}
> +
> +/**
> + * @short of_device_id structure
> + */
> +static const struct of_device_id dw_mipi_csi_of_match[] = {
> + {
> + .compatible = "snps,dw-mipi-csi"},
> + { /* sentinel */ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, dw_mipi_csi_of_match);
> +
> +/**
> + * @short Platform driver structure
> + */
> +static struct platform_driver __refdata dw_mipi_csi_pdrv = {
> + .remove = mipi_csi_remove,
> + .probe = mipi_csi_probe,
> + .driver = {
> + .name = CSI_DEVICE_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = dw_mipi_csi_of_match,
> + },
> +};
> +
> +module_platform_driver(dw_mipi_csi_pdrv);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Synopsys DW MIPI CSI-2 Host driver");
> diff --git a/drivers/media/platform/dwc/dw_mipi_csi.h b/drivers/media/platform/dwc/dw_mipi_csi.h
> new file mode 100644
> index 0000000..5e4c48c
> --- /dev/null
> +++ b/drivers/media/platform/dwc/dw_mipi_csi.h
> @@ -0,0 +1,179 @@
> +/*
> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef DW_MIPI_CSI_H_
> +#define DW_MIPI_CSI_H_
> +
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_graph.h>
> +#include <linux/delay.h>
> +#include <linux/wait.h>
> +#include <linux/string.h>
> +#include <linux/phy/phy.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-dv-timings.h>
> +#include <linux/videodev2.h>
> +#include <linux/io.h>

Please arrange alphabetically. Same for other files.

> +
> +#include "plat_ipk_video.h"
> +
> +#define CSI_DEVICE_NAME "dw-mipi-csi"
> +
> +/** @short DWC MIPI CSI-2 register addresses*/
> +enum register_addresses {
> + R_CSI2_VERSION = 0x00,
> + R_CSI2_N_LANES = 0x04,
> + R_CSI2_CTRL_RESETN = 0x08,
> + R_CSI2_INTERRUPT = 0x0C,
> + R_CSI2_DATA_IDS_1 = 0x10,
> + R_CSI2_DATA_IDS_2 = 0x14,
> + R_CSI2_IPI_MODE = 0x80,
> + R_CSI2_IPI_VCID = 0x84,
> + R_CSI2_IPI_DATA_TYPE = 0x88,
> + R_CSI2_IPI_MEM_FLUSH = 0x8C,
> + R_CSI2_IPI_HSA_TIME = 0x90,
> + R_CSI2_IPI_HBP_TIME = 0x94,
> + R_CSI2_IPI_HSD_TIME = 0x98,
> + R_CSI2_IPI_HLINE_TIME = 0x9C,
> + R_CSI2_IPI_VSA_LINES = 0xB0,
> + R_CSI2_IPI_VBP_LINES = 0xB4,
> + R_CSI2_IPI_VFP_LINES = 0xB8,
> + R_CSI2_IPI_VACTIVE_LINES = 0xBC,
> + R_CSI2_INT_PHY_FATAL = 0xe0,
> + R_CSI2_MASK_INT_PHY_FATAL = 0xe4,
> + R_CSI2_FORCE_INT_PHY_FATAL = 0xe8,
> + R_CSI2_INT_PKT_FATAL = 0xf0,
> + R_CSI2_MASK_INT_PKT_FATAL = 0xf4,
> + R_CSI2_FORCE_INT_PKT_FATAL = 0xf8,
> + R_CSI2_INT_FRAME_FATAL = 0x100,
> + R_CSI2_MASK_INT_FRAME_FATAL = 0x104,
> + R_CSI2_FORCE_INT_FRAME_FATAL = 0x108,
> + R_CSI2_INT_PHY = 0x110,
> + R_CSI2_MASK_INT_PHY = 0x114,
> + R_CSI2_FORCE_INT_PHY = 0x118,
> + R_CSI2_INT_PKT = 0x120,
> + R_CSI2_MASK_INT_PKT = 0x124,
> + R_CSI2_FORCE_INT_PKT = 0x128,
> + R_CSI2_INT_LINE = 0x130,
> + R_CSI2_MASK_INT_LINE = 0x134,
> + R_CSI2_FORCE_INT_LINE = 0x138,
> + R_CSI2_INT_IPI = 0x140,
> + R_CSI2_MASK_INT_IPI = 0x144,
> + R_CSI2_FORCE_INT_IPI = 0x148
> +};
> +
> +/** @short IPI Data Types */
> +enum data_type {
> + CSI_2_YUV420_8 = 0x18,
> + CSI_2_YUV420_10 = 0x19,
> + CSI_2_YUV420_8_LEG = 0x1A,
> + CSI_2_YUV420_8_SHIFT = 0x1C,
> + CSI_2_YUV420_10_SHIFT = 0x1D,
> + CSI_2_YUV422_8 = 0x1E,
> + CSI_2_YUV422_10 = 0x1F,
> + CSI_2_RGB444 = 0x20,
> + CSI_2_RGB555 = 0x21,
> + CSI_2_RGB565 = 0x22,
> + CSI_2_RGB666 = 0x23,
> + CSI_2_RGB888 = 0x24,
> + CSI_2_RAW6 = 0x28,
> + CSI_2_RAW7 = 0x29,
> + CSI_2_RAW8 = 0x2A,
> + CSI_2_RAW10 = 0x2B,
> + CSI_2_RAW12 = 0x2C,
> + CSI_2_RAW14 = 0x2D,
> +};
> +
> +/** @short Interrupt Masks */
> +enum interrupt_type {
> + CSI2_INT_PHY_FATAL = 1 << 0,
> + CSI2_INT_PKT_FATAL = 1 << 1,
> + CSI2_INT_FRAME_FATAL = 1 << 2,
> + CSI2_INT_PHY = 1 << 16,
> + CSI2_INT_PKT = 1 << 17,
> + CSI2_INT_LINE = 1 << 18,
> + CSI2_INT_IPI = 1 << 19,
> +
> +};
> +
> +/** @short DWC MIPI CSI-2 output types*/
> +enum output_type {
> + IPI_OUT = 0,
> + IDI_OUT = 1,
> + BOTH_OUT = 2
> +};
> +
> +/** @short IPI output types*/
> +enum ipi_output_type {
> + CAMERA_TIMING = 0,
> + AUTO_TIMING = 1
> +};
> +
> +/**
> + * @short Format template
> + */
> +struct mipi_fmt {
> + const char *name;
> + u32 code;
> + u8 depth;
> +};
> +
> +struct csi_hw {
> +
> + uint32_t num_lanes;
> + uint32_t output_type;
> +
> + /*IPI Info */
> + uint32_t ipi_mode;
> + uint32_t ipi_color_mode;
> + uint32_t ipi_auto_flush;
> + uint32_t virtual_ch;
> +
> + uint32_t hsa;
> + uint32_t hbp;
> + uint32_t hsd;
> + uint32_t htotal;
> +
> + uint32_t vsa;
> + uint32_t vbp;
> + uint32_t vfp;
> + uint32_t vactive;
> +};
> +
> +/**
> + * @short Structure to embed device driver information
> + */
> +struct mipi_csi_dev {
> + struct v4l2_subdev sd;
> + struct video_device vdev;
> +
> + struct mutex lock;
> + spinlock_t slock;
> + struct media_pad pads[CSI_PADS_NUM];
> + struct platform_device *pdev;
> + u8 index;
> +
> + /** Store current format */
> + const struct mipi_fmt *fmt;
> + struct v4l2_mbus_framefmt format;
> +
> + /** Device Tree Information */
> + void __iomem *base_address;
> + uint32_t ctrl_irq_number;
> +
> + struct csi_hw hw;
> +
> + struct phy *phy;
> +};
> +
> +#endif /* DW_MIPI_CSI */
> diff --git a/drivers/media/platform/dwc/plat_ipk.c b/drivers/media/platform/dwc/plat_ipk.c
> new file mode 100644
> index 0000000..48b9e36
> --- /dev/null
> +++ b/drivers/media/platform/dwc/plat_ipk.c
> @@ -0,0 +1,835 @@
> +/**
> + * DWC MIPI CSI-2 Host IPK platform device driver
> + *
> + * Based on Omnivision OV7670 Camera Driver
> + * Copyright (C) 2011 - 2013 Samsung Electronics Co., Ltd.
> + * Author: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> + *
> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
> + * Author: Ramiro Oliveira <ramiro.oliveira@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published
> + * by the Free Software Foundation, either version 2 of the License,
> + * or (at your option) any later version.
> + */
> +
> +#include "plat_ipk.h"
> +#include "video_device.h"
> +#include "dw_mipi_csi.h"
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-subdev.h>
> +
> +static int
> +__plat_ipk_pipeline_s_format(struct plat_ipk_media_pipeline *ep,
> + struct v4l2_subdev_format *fmt)
> +{
> +
> + struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
> + static const u8 seq[IDX_MAX] = {IDX_SENSOR, IDX_CSI, IDX_VDEV};
> +
> + fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
> + v4l2_subdev_call(p->subdevs[seq[IDX_CSI]], pad, set_fmt, NULL, fmt);
> +
> + return 0;
> +}
> +
> +static void
> +plat_ipk_pipeline_prepare(struct plat_ipk_pipeline *p, struct media_entity *me)
> +{
> + struct v4l2_subdev *sd;
> + int i = 0;

unsigned int?

> +
> + for (i = 0; i < IDX_MAX; i++)
> + p->subdevs[i] = NULL;
> +
> + while (1) {
> + struct media_pad *pad = NULL;
> +
> + for (i = 0; i < me->num_pads; i++) {
> + struct media_pad *spad = &me->pads[i];
> +
> + if (!(spad->flags & MEDIA_PAD_FL_SINK))
> + continue;
> +
> + pad = media_entity_remote_pad(spad);
> + if (pad)
> + break;
> + }
> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
> + break;
> +
> + sd = media_entity_to_v4l2_subdev(pad->entity);
> +
> + switch (sd->grp_id) {
> + case GRP_ID_SENSOR:
> + p->subdevs[IDX_SENSOR] = sd;
> + break;
> + case GRP_ID_CSI:
> + p->subdevs[IDX_CSI] = sd;
> + break;
> + case GRP_ID_VIDEODEV:
> + p->subdevs[IDX_VDEV] = sd;
> + break;
> + default:
> + break;
> + }
> + me = &sd->entity;
> + if (me->num_pads == 1)
> + break;
> + }
> +}
> +
> +static int __subdev_set_power(struct v4l2_subdev *sd, int on)
> +{
> + int *use_count;
> + int ret;
> +
> + if (sd == NULL) {
> + pr_info("null subdev\n");
> + return -ENXIO;
> + }
> + use_count = &sd->entity.use_count;
> + if (on && (*use_count)++ > 0)
> + return 0;
> + else if (!on && (*use_count == 0 || --(*use_count) > 0))
> + return 0;
> +
> + pr_debug("%d %s !\n", on, sd->entity.name);

Please do use dev_*() family of functions instead.

> + ret = v4l2_subdev_call(sd, core, s_power, on);
> +
> + return ret != -ENOIOCTLCMD ? ret : 0;
> +}
> +
> +static int plat_ipk_pipeline_s_power(struct plat_ipk_pipeline *p, bool on)
> +{
> + static const u8 seq[IDX_MAX] = {IDX_CSI, IDX_SENSOR, IDX_VDEV};
> + int i, ret = 0;
> +
> + for (i = 0; i < IDX_MAX; i++) {
> + unsigned int idx = seq[i];
> +
> + if (p->subdevs[idx] == NULL)
> + pr_info("No device registered on %d\n", idx);
> + else {
> + ret = __subdev_set_power(p->subdevs[idx], on);
> + if (ret < 0 && ret != -ENXIO)
> + goto error;
> + }
> + }
> + return 0;
> +error:
> + for (; i >= 0; i--) {
> + unsigned int idx = seq[i];
> +
> + __subdev_set_power(p->subdevs[idx], !on);
> + }
> + return ret;
> +}
> +
> +static int
> +__plat_ipk_pipeline_open(struct plat_ipk_media_pipeline *ep,
> + struct media_entity *me, bool prepare)
> +{
> + struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
> + int ret;
> +
> + if (WARN_ON(p == NULL || me == NULL))
> + return -EINVAL;
> +
> + if (prepare)
> + plat_ipk_pipeline_prepare(p, me);
> +
> + ret = plat_ipk_pipeline_s_power(p, 1);
> + if (!ret)
> + return 0;
> +
> + return ret;
> +}
> +
> +static int __plat_ipk_pipeline_close(struct plat_ipk_media_pipeline *ep)
> +{
> + struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
> + int ret;
> +
> + ret = plat_ipk_pipeline_s_power(p, 0);
> +
> + return ret == -ENXIO ? 0 : ret;
> +}
> +
> +static int
> +__plat_ipk_pipeline_s_stream(struct plat_ipk_media_pipeline *ep, bool on)
> +{
> + static const u8 seq[IDX_MAX] = {IDX_SENSOR, IDX_CSI, IDX_VDEV};
> + struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
> + int i, ret = 0;
> +
> + for (i = 0; i < IDX_MAX; i++) {
> + unsigned int idx = seq[i];
> +
> + if (p->subdevs[idx] == NULL)
> + pr_debug("No device registered on %d\n", idx);
> + else {
> + ret =
> + v4l2_subdev_call(p->subdevs[idx], video, s_stream,
> + on);
> +
> + if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
> + goto error;
> + }
> + }
> + return 0;
> +error:
> + for (; i >= 0; i--) {
> + unsigned int idx = seq[i];
> +
> + v4l2_subdev_call(p->subdevs[idx], video, s_stream, !on);
> + }
> + return ret;
> +}
> +
> +static const struct plat_ipk_media_pipeline_ops plat_ipk_pipeline_ops = {
> + .open = __plat_ipk_pipeline_open,
> + .close = __plat_ipk_pipeline_close,
> + .set_format = __plat_ipk_pipeline_s_format,
> + .set_stream = __plat_ipk_pipeline_s_stream,
> +};
> +
> +static struct plat_ipk_media_pipeline *
> +plat_ipk_pipeline_create(struct plat_ipk_dev *plat_ipk)
> +{
> + struct plat_ipk_pipeline *p;
> +
> + p = kzalloc(sizeof(*p), GFP_KERNEL);
> + if (!p)
> + return NULL;
> +
> + list_add_tail(&p->list, &plat_ipk->pipelines);
> +
> + p->ep.ops = &plat_ipk_pipeline_ops;
> + return &p->ep;
> +}
> +
> +static void
> +plat_ipk_pipelines_free(struct plat_ipk_dev *plat_ipk)
> +{
> + while (!list_empty(&plat_ipk->pipelines)) {
> + struct plat_ipk_pipeline *p;
> +
> + p = list_entry(plat_ipk->pipelines.next, typeof(*p), list);
> + list_del(&p->list);
> + kfree(p);
> + }
> +}
> +
> +static int
> +plat_ipk_parse_port_node(struct plat_ipk_dev *plat_ipk,
> + struct device_node *port, unsigned int index)
> +{
> + struct device_node *rem, *ep;
> + struct v4l2_of_endpoint endpoint;
> + struct plat_ipk_source_info *pd = &plat_ipk->sensor[index].pdata;
> +
> + /* Assume here a port node can have only one endpoint node. */
> + ep = of_get_next_child(port, NULL);
> + if (!ep)
> + return 0;
> +
> + v4l2_of_parse_endpoint(ep, &endpoint);
> + if (WARN_ON(endpoint.base.port == 0) || index >= PLAT_MAX_SENSORS)
> + return -EINVAL;
> +
> + pd->mux_id = endpoint.base.port - 1;
> +
> + rem = of_graph_get_remote_port_parent(ep);
> + of_node_put(ep);
> + if (rem == NULL) {
> + v4l2_info(&plat_ipk->v4l2_dev,
> + "Remote device at %s not found\n", ep->full_name);
> + return 0;
> + }
> +
> + if (WARN_ON(index >= ARRAY_SIZE(plat_ipk->sensor)))
> + return -EINVAL;
> +
> + plat_ipk->sensor[index].asd.match_type = V4L2_ASYNC_MATCH_OF;
> + plat_ipk->sensor[index].asd.match.of.node = rem;
> + plat_ipk->async_subdevs[index] = &plat_ipk->sensor[index].asd;
> +
> + plat_ipk->num_sensors++;
> +
> + of_node_put(rem);
> + return 0;
> +}
> +
> +
> +static int plat_ipk_register_sensor_entities(struct plat_ipk_dev *plat_ipk)
> +{
> + struct device_node *parent = plat_ipk->pdev->dev.of_node;
> + struct device_node *node;
> + int index = 0;
> + int ret;
> +
> + plat_ipk->num_sensors = 0;
> +
> + for_each_available_child_of_node(parent, node) {
> + struct device_node *port;
> +
> + if (of_node_cmp(node->name, "csi2"))
> + continue;
> + port = of_get_next_child(node, NULL);
> + if (!port)
> + continue;
> +
> + ret = plat_ipk_parse_port_node(plat_ipk, port, index);
> + if (ret < 0)
> + return ret;
> + index++;
> + }
> + return 0;
> +}
> +
> +static int
> +__of_get_port_id(struct device_node *np)
> +{
> + u32 reg = 0;
> +
> + np = of_get_child_by_name(np, "port");
> + if (!np)
> + return -EINVAL;
> + of_property_read_u32(np, "reg", &reg);
> +
> + return reg - 1;
> +}
> +
> +static int register_videodev_entity(struct plat_ipk_dev *plat_ipk,
> + struct video_device_dev *vid_dev)
> +{
> + struct v4l2_subdev *sd;
> + struct plat_ipk_media_pipeline *ep;
> + int ret;
> +
> + sd = &vid_dev->subdev;
> + sd->grp_id = GRP_ID_VIDEODEV;
> +
> + ep = plat_ipk_pipeline_create(plat_ipk);
> + if (!ep)
> + return -ENOMEM;
> +
> + v4l2_set_subdev_hostdata(sd, ep);
> +
> + ret = v4l2_device_register_subdev(&plat_ipk->v4l2_dev, sd);
> + if (!ret)
> + plat_ipk->vid_dev = vid_dev;
> + else
> + v4l2_err(&plat_ipk->v4l2_dev,
> + "Failed to register Video Device\n");
> + return ret;
> +}
> +
> +static int register_mipi_csi_entity(struct plat_ipk_dev *plat_ipk,
> + struct platform_device *pdev, struct v4l2_subdev *sd)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + int id, ret;
> +
> + id = node ? __of_get_port_id(node) : max(0, pdev->id);
> +
> + if (WARN_ON(id < 0 || id >= CSI_MAX_ENTITIES))
> + return -ENOENT;
> +
> + if (WARN_ON(plat_ipk->mipi_csi[id].sd))
> + return -EBUSY;
> +
> + sd->grp_id = GRP_ID_CSI;
> + ret = v4l2_device_register_subdev(&plat_ipk->v4l2_dev, sd);
> +
> + if (!ret)
> + plat_ipk->mipi_csi[id].sd = sd;
> + else
> + v4l2_err(&plat_ipk->v4l2_dev,
> + "Failed to register MIPI-CSI.%d (%d)\n", id, ret);
> + return ret;
> +}
> +
> +static int plat_ipk_register_platform_entity(struct plat_ipk_dev *plat_ipk,
> + struct platform_device *pdev, int plat_entity)
> +{
> + struct device *dev = &pdev->dev;
> + int ret = -EPROBE_DEFER;
> + void *drvdata;
> +
> +

Extra newline.

> + device_lock(dev);
> + if (!dev->driver || !try_module_get(dev->driver->owner))
> + goto dev_unlock;
> +
> + drvdata = dev_get_drvdata(dev);
> +
> + if (drvdata) {
> + switch (plat_entity) {
> + case IDX_VDEV:
> + ret = register_videodev_entity(plat_ipk, drvdata);
> + break;
> + case IDX_CSI:
> + ret = register_mipi_csi_entity(plat_ipk, pdev, drvdata);
> + break;
> + default:
> + ret = -ENODEV;
> + }
> + } else
> + dev_err(&plat_ipk->pdev->dev, "%s no drvdata\n", dev_name(dev));
> + module_put(dev->driver->owner);
> +dev_unlock:
> + device_unlock(dev);
> + if (ret == -EPROBE_DEFER)
> + dev_info(&plat_ipk->pdev->dev,
> + "deferring %s device registration\n", dev_name(dev));
> + else if (ret < 0)
> + dev_err(&plat_ipk->pdev->dev,
> + "%s device registration failed (%d)\n", dev_name(dev),
> + ret);
> + return ret;
> +}
> +
> +static int
> +plat_ipk_register_platform_entities(struct plat_ipk_dev *plat_ipk,
> + struct device_node *parent)
> +{
> + struct device_node *node;
> + int ret = 0;
> +
> + for_each_available_child_of_node(parent, node) {
> + struct platform_device *pdev;
> + int plat_entity = -1;
> +
> + pdev = of_find_device_by_node(node);
> + if (!pdev)
> + continue;
> +
> + if (!strcmp(node->name, VIDEODEV_OF_NODE_NAME))
> + plat_entity = IDX_VDEV;
> + else if (!strcmp(node->name, CSI_OF_NODE_NAME))
> + plat_entity = IDX_CSI;
> +
> + if (plat_entity >= 0)
> + ret = plat_ipk_register_platform_entity(plat_ipk, pdev,
> + plat_entity);
> + put_device(&pdev->dev);
> + if (ret < 0)
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static void
> +plat_ipk_unregister_entities(struct plat_ipk_dev *plat_ipk)
> +{
> + int i;
> + struct video_device_dev *dev = plat_ipk->vid_dev;
> +
> + if (dev == NULL)
> + return;
> + v4l2_device_unregister_subdev(&dev->subdev);
> + dev->ve.pipe = NULL;
> + plat_ipk->vid_dev = NULL;
> +
> + for (i = 0; i < CSI_MAX_ENTITIES; i++) {
> + if (plat_ipk->mipi_csi[i].sd == NULL)
> + continue;
> + v4l2_device_unregister_subdev(plat_ipk->mipi_csi[i].sd);
> + plat_ipk->mipi_csi[i].sd = NULL;
> + }
> +
> + v4l2_info(&plat_ipk->v4l2_dev, "Unregistered all entities\n");
> +}
> +
> +static int
> +__plat_ipk_create_videodev_sink_links(struct plat_ipk_dev *plat_ipk,
> + struct media_entity *source,
> + int pad)
> +{
> + struct media_entity *sink;
> + int ret = 0;
> +
> + if (!plat_ipk->vid_dev)
> + return 0;
> +
> + sink = &plat_ipk->vid_dev->subdev.entity;
> + ret = media_create_pad_link(source, pad, sink,
> + CSI_PAD_SOURCE, MEDIA_LNK_FL_ENABLED);
> + if (ret)
> + return ret;
> +
> + ret = media_entity_call(sink, link_setup, &sink->pads[0],
> + &source->pads[pad], 0);

What's the purpose of calling link_setup() here?

> + if (ret)
> + return 0;
> +
> + v4l2_info(&plat_ipk->v4l2_dev, "created link [%s] -> [%s]\n",
> + source->name, sink->name);
> +
> + return 0;
> +}
> +
> +
> +static int
> +__plat_ipk_create_videodev_source_links(struct plat_ipk_dev *plat_ipk)
> +{
> + struct media_entity *source, *sink;
> + int ret = 0;
> +
> + struct video_device_dev *vid_dev = plat_ipk->vid_dev;
> +
> + if (vid_dev == NULL)
> + return -ENODEV;
> +
> + source = &vid_dev->subdev.entity;
> + sink = &vid_dev->ve.vdev.entity;
> +
> + ret = media_create_pad_link(source, VIDEO_DEV_SD_PAD_SOURCE_DMA,
> + sink, 0, MEDIA_LNK_FL_ENABLED);
> +
> + v4l2_info(&plat_ipk->v4l2_dev, "created link [%s] -> [%s]\n",
> + source->name, sink->name);
> + return ret;
> +}
> +
> +static int
> +plat_ipk_create_links(struct plat_ipk_dev *plat_ipk)
> +{
> + struct v4l2_subdev *csi_sensor[CSI_MAX_ENTITIES] = { NULL };
> + struct v4l2_subdev *sensor, *csi;
> + struct media_entity *source;
> + struct plat_ipk_source_info *pdata;
> + int i, pad, ret = 0;
> +
> + for (i = 0; i < plat_ipk->num_sensors; i++) {
> + if (plat_ipk->sensor[i].subdev == NULL)
> + continue;
> +
> + sensor = plat_ipk->sensor[i].subdev;
> + pdata = v4l2_get_subdev_hostdata(sensor);
> + if (!pdata)
> + continue;
> +
> + source = NULL;
> +
> + csi = plat_ipk->mipi_csi[pdata->mux_id].sd;
> + if (WARN(csi == NULL, "MIPI-CSI interface specified but dw-mipi-csi module is not loaded!\n"))
> + return -EINVAL;
> +
> + pad = sensor->entity.num_pads - 1;
> + ret = media_create_pad_link(&sensor->entity, pad,
> + &csi->entity, CSI_PAD_SINK,
> + MEDIA_LNK_FL_IMMUTABLE |
> + MEDIA_LNK_FL_ENABLED);
> +
> + if (ret)
> + return ret;
> + v4l2_info(&plat_ipk->v4l2_dev, "created link [%s] -> [%s]\n",
> + sensor->entity.name, csi->entity.name);

dev_dbg() ?

> +
> + csi_sensor[pdata->mux_id] = sensor;
> + }
> +
> + for (i = 0; i < CSI_MAX_ENTITIES; i++) {
> + if (plat_ipk->mipi_csi[i].sd == NULL) {
> + pr_info("no link\n");
> + continue;
> + }
> +
> + source = &plat_ipk->mipi_csi[i].sd->entity;
> + pad = VIDEO_DEV_SD_PAD_SINK_CSI;
> +
> + ret = __plat_ipk_create_videodev_sink_links(plat_ipk, source,
> + pad);
> + }
> +
> + ret = __plat_ipk_create_videodev_source_links(plat_ipk);
> + if (ret < 0)
> + return ret;
> +
> + return ret;
> +}
> +
> +static int __plat_ipk_modify_pipeline(struct media_entity *entity, bool enable)
> +{
> + struct plat_ipk_video_entity *ve;
> + struct plat_ipk_pipeline *p;
> + struct video_device *vdev;
> + int ret;
> +
> + vdev = media_entity_to_video_device(entity);
> +
> + if (vdev->entity.use_count == 0)
> + return 0;
> +
> + ve = vdev_to_plat_ipk_video_entity(vdev);
> + p = to_plat_ipk_pipeline(ve->pipe);
> +
> + if (enable)
> + ret = __plat_ipk_pipeline_open(ve->pipe, entity, true);
> + else
> + ret = __plat_ipk_pipeline_close(ve->pipe);
> +
> + if (ret == 0 && !enable)
> + memset(p->subdevs, 0, sizeof(p->subdevs));
> +
> + return ret;
> +}
> +
> +
> +static int
> +__plat_ipk_modify_pipelines(struct media_entity *entity, bool enable,
> + struct media_entity_graph *graph)
> +{
> + struct media_entity *entity_err = entity;
> + int ret;
> +
> + media_entity_graph_walk_start(graph, entity);
> +
> + while ((entity = media_entity_graph_walk_next(graph))) {
> + if (!is_media_entity_v4l2_video_device(entity))
> + continue;
> +
> + ret = __plat_ipk_modify_pipeline(entity, enable);
> +
> + if (ret < 0)
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + media_entity_graph_walk_start(graph, entity_err);
> +
> + while ((entity_err = media_entity_graph_walk_next(graph))) {
> + if (!is_media_entity_v4l2_video_device(entity_err))
> + continue;
> +
> + __plat_ipk_modify_pipeline(entity_err, !enable);
> +
> + if (entity_err == entity)
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static int
> +plat_ipk_link_notify(struct media_link *link, unsigned int flags,
> + unsigned int notification)
> +{
> + struct media_entity_graph *graph =
> + &container_of(link->graph_obj.mdev, struct plat_ipk_dev,
> + media_dev)->link_setup_graph;
> + struct media_entity *sink = link->sink->entity;
> + int ret = 0;
> +
> + pr_debug("Link notify\n");

This probably made sense at development time but should be removed now.

> +
> + if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH) {
> + ret = media_entity_graph_walk_init(graph, link->graph_obj.mdev);
> + if (ret)
> + return ret;
> + if (!(flags & MEDIA_LNK_FL_ENABLED))
> + ret = __plat_ipk_modify_pipelines(sink, false, graph);
> +
> + } else if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH) {
> + if (link->flags & MEDIA_LNK_FL_ENABLED)
> + ret = __plat_ipk_modify_pipelines(sink, true, graph);
> + media_entity_graph_walk_cleanup(graph);
> + }
> +
> + return ret ? -EPIPE : 0;
> +}

Missing newline.

> +static const struct media_device_ops plat_ipk_media_ops = {
> + .link_notify = plat_ipk_link_notify,
> +};
> +
> +
> +static int
> +subdev_notifier_bound(struct v4l2_async_notifier *notifier,
> + struct v4l2_subdev *subdev, struct v4l2_async_subdev *asd)
> +{
> + struct plat_ipk_dev *plat_ipk = notifier_to_plat_ipk(notifier);
> + struct plat_ipk_sensor_info *si = NULL;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(plat_ipk->sensor); i++)
> + if (plat_ipk->sensor[i].asd.match.of.node ==
> + subdev->dev->of_node)
> + si = &plat_ipk->sensor[i];
> +
> + if (si == NULL)
> + return -EINVAL;
> +
> + v4l2_set_subdev_hostdata(subdev, &si->pdata);
> +
> + subdev->grp_id = GRP_ID_SENSOR;
> +
> + si->subdev = subdev;
> +
> + v4l2_info(&plat_ipk->v4l2_dev, "Registered sensor subdevice: %s (%d)\n",
> + subdev->name, plat_ipk->num_sensors);
> +
> + plat_ipk->num_sensors++;
> +
> + return 0;
> +}
> +
> +static int
> +subdev_notifier_complete(struct v4l2_async_notifier *notifier)
> +{
> + struct plat_ipk_dev *plat_ipk = notifier_to_plat_ipk(notifier);
> + int ret;
> +
> + mutex_lock(&plat_ipk->media_dev.graph_mutex);
> +
> + ret = plat_ipk_create_links(plat_ipk);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = v4l2_device_register_subdev_nodes(&plat_ipk->v4l2_dev);
> +unlock:
> + mutex_unlock(&plat_ipk->media_dev.graph_mutex);
> + if (ret < 0)
> + return ret;
> +
> + return media_device_register(&plat_ipk->media_dev);
> +}
> +
> +static const struct of_device_id plat_ipk_of_match[];

This is redundant.

> +
> +static int plat_ipk_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct v4l2_device *v4l2_dev;
> + struct plat_ipk_dev *plat_ipk;
> + int ret;
> +
> + dev_info(dev, "Installing DW MIPI CSI-2 IPK Platform module\n");
> +
> + plat_ipk = devm_kzalloc(dev, sizeof(*plat_ipk), GFP_KERNEL);
> + if (!plat_ipk)
> + return -ENOMEM;
> +
> + spin_lock_init(&plat_ipk->slock);
> + INIT_LIST_HEAD(&plat_ipk->pipelines);
> + plat_ipk->pdev = pdev;
> +
> + strlcpy(plat_ipk->media_dev.model, "SNPS IPK Platform",
> + sizeof(plat_ipk->media_dev.model));
> + plat_ipk->media_dev.ops = &plat_ipk_media_ops;
> + plat_ipk->media_dev.dev = dev;
> +
> + v4l2_dev = &plat_ipk->v4l2_dev;
> + v4l2_dev->mdev = &plat_ipk->media_dev;
> + strlcpy(v4l2_dev->name, "plat-ipk", sizeof(v4l2_dev->name));
> +
> + media_device_init(&plat_ipk->media_dev);
> +
> + ret = v4l2_device_register(dev, &plat_ipk->v4l2_dev);
> + if (ret < 0) {
> + v4l2_err(v4l2_dev, "Failed to register v4l2_device: %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, plat_ipk);
> +
> + ret = plat_ipk_register_platform_entities(plat_ipk, dev->of_node);
> + if (ret)
> + goto err_m_ent;
> +
> + ret = plat_ipk_register_sensor_entities(plat_ipk);
> + if (ret)
> + goto err_m_ent;
> +
> + if (plat_ipk->num_sensors > 0) {
> + plat_ipk->subdev_notifier.subdevs = plat_ipk->async_subdevs;
> + plat_ipk->subdev_notifier.num_subdevs = plat_ipk->num_sensors;
> + plat_ipk->subdev_notifier.bound = subdev_notifier_bound;
> + plat_ipk->subdev_notifier.complete = subdev_notifier_complete;
> + plat_ipk->num_sensors = 0;
> +
> + ret = v4l2_async_notifier_register(&plat_ipk->v4l2_dev,
> + &plat_ipk->subdev_notifier);
> + if (ret)
> + goto err_m_ent;
> + }
> +
> + return 0;
> +
> +err_m_ent:
> + plat_ipk_unregister_entities(plat_ipk);
> + media_device_unregister(&plat_ipk->media_dev);
> + media_device_cleanup(&plat_ipk->media_dev);
> + v4l2_device_unregister(&plat_ipk->v4l2_dev);
> + return ret;
> +}
> +
> +static int plat_ipk_remove(struct platform_device *pdev)
> +{
> + struct plat_ipk_dev *dev = platform_get_drvdata(pdev);
> +
> + if (!dev)
> + return 0;

Wouldn't !dev here require a driver bug? If probe() succeeds, then dev is
non-zero, right?

> +
> + v4l2_async_notifier_unregister(&dev->subdev_notifier);
> +
> + v4l2_device_unregister(&dev->v4l2_dev);
> + plat_ipk_unregister_entities(dev);
> + plat_ipk_pipelines_free(dev);
> + media_device_unregister(&dev->media_dev);
> + media_device_cleanup(&dev->media_dev);
> +
> + dev_info(&pdev->dev, "Driver removed\n");
> +
> + return 0;
> +}
> +
> +/**
> + * @short of_device_id structure
> + */
> +static const struct of_device_id plat_ipk_of_match[] = {
> + {.compatible = "snps,plat-ipk"},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(of, plat_ipk_of_match);
> +
> +/**
> + * @short Platform driver structure
> + */
> +static struct platform_driver plat_ipk_pdrv = {
> + .remove = plat_ipk_remove,
> + .probe = plat_ipk_probe,
> + .driver = {
> + .name = "snps,plat-ipk",
> + .owner = THIS_MODULE,
> + .of_match_table = plat_ipk_of_match,
> + },
> +};
> +
> +static int __init
> +plat_ipk_init(void)
> +{
> + request_module("dw-mipi-csi");

Why do you need this?

> +
> + return platform_driver_register(&plat_ipk_pdrv);
> +}
> +
> +static void __exit
> +plat_ipk_exit(void)
> +{
> + platform_driver_unregister(&plat_ipk_pdrv);
> +}
> +
> +module_init(plat_ipk_init);
> +module_exit(plat_ipk_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Platform driver for MIPI CSI-2 Host IPK");
> diff --git a/drivers/media/platform/dwc/plat_ipk.h b/drivers/media/platform/dwc/plat_ipk.h
> new file mode 100644
> index 0000000..ef569eb
> --- /dev/null
> +++ b/drivers/media/platform/dwc/plat_ipk.h
> @@ -0,0 +1,97 @@
> +/*
> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef PLAT_IPK_H_
> +#define PLAT_IPK_H_
> +
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/list.h>
> +#include <linux/string.h>
> +#include <media/v4l2-device.h>
> +#include <linux/videodev2.h>
> +#include <media/media-entity.h>
> +
> +#include "plat_ipk_video.h"
> +
> +#define VIDEODEV_OF_NODE_NAME "video-device"
> +#define CSI_OF_NODE_NAME "csi2"
> +
> +enum plat_ipk_subdev_index {
> + IDX_SENSOR,
> + IDX_CSI,
> + IDX_VDEV,
> + IDX_MAX,
> +};
> +
> +struct plat_ipk_sensor_info {
> + struct plat_ipk_source_info pdata;
> + struct v4l2_async_subdev asd;
> + struct v4l2_subdev *subdev;
> + struct mipi_csi_dev *host;
> +};
> +
> +struct plat_ipk_pipeline {
> + struct plat_ipk_media_pipeline ep;
> + struct list_head list;
> + struct media_entity *vdev_entity;
> + struct v4l2_subdev *subdevs[IDX_MAX];
> +};
> +
> +#define to_plat_ipk_pipeline(_ep)\
> + container_of(_ep, struct plat_ipk_pipeline, ep)
> +
> +struct mipi_csi_info {
> + struct v4l2_subdev *sd;
> + int id;
> +};
> +
> +/**
> + * @short Structure to embed device driver information
> + */
> +struct plat_ipk_dev {
> + struct mipi_csi_info mipi_csi[CSI_MAX_ENTITIES];
> + struct video_device_dev *vid_dev;
> + struct device *dev;
> + struct media_device media_dev;
> + struct v4l2_device v4l2_dev;
> + struct platform_device *pdev;
> + struct plat_ipk_sensor_info sensor[PLAT_MAX_SENSORS];
> + struct v4l2_async_notifier subdev_notifier;
> + struct v4l2_async_subdev *async_subdevs[PLAT_MAX_SENSORS];
> + spinlock_t slock;
> + struct list_head pipelines;
> + int num_sensors;
> + struct media_entity_graph link_setup_graph;
> +};
> +
> +static inline struct plat_ipk_dev *
> +entity_to_plat_ipk_mdev(struct media_entity *me)
> +{
> + return me->graph_obj.mdev == NULL ? NULL :
> + container_of(me->graph_obj.mdev, struct plat_ipk_dev, media_dev);
> +}
> +
> +static inline struct plat_ipk_dev *
> +notifier_to_plat_ipk(struct v4l2_async_notifier *n)
> +{
> + return container_of(n, struct plat_ipk_dev, subdev_notifier);
> +}
> +
> +static inline void
> +plat_ipk_graph_unlock(struct plat_ipk_video_entity *ve)
> +{
> + mutex_unlock(&ve->vdev.entity.graph_obj.mdev->graph_mutex);
> +}
> +
> +#endif /* PLAT_IPK_H_ */
> diff --git a/drivers/media/platform/dwc/plat_ipk_video.h b/drivers/media/platform/dwc/plat_ipk_video.h
> new file mode 100644
> index 0000000..6bfc9f8
> --- /dev/null
> +++ b/drivers/media/platform/dwc/plat_ipk_video.h
> @@ -0,0 +1,97 @@
> +/*
> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef PLAT_IPK_INCLUDES_H_
> +#define PLAT_IPK_INCLUDES_H_
> +
> +#include <media/media-entity.h>
> +#include <media/v4l2-dev.h>
> +#include <media/v4l2-mediabus.h>
> +#include <media/v4l2-subdev.h>
> +
> +/*
> + * The subdevices' group IDs.
> + */
> +
> +#define MAX_WIDTH 3280
> +#define MAX_HEIGHT 1852
> +
> +#define MIN_WIDTH 640
> +#define MIN_HEIGHT 480
> +
> +#define GRP_ID_SENSOR (10)
> +#define GRP_ID_CSI (20)
> +#define GRP_ID_VIDEODEV (30)
> +
> +#define CSI_MAX_ENTITIES 1
> +#define PLAT_MAX_SENSORS 1
> +
> +enum video_dev_pads {
> + VIDEO_DEV_SD_PAD_SINK_CSI = 0,
> + VIDEO_DEV_SD_PAD_SOURCE_DMA = 1,
> + VIDEO_DEV_SD_PADS_NUM = 2,
> +};
> +
> +enum mipi_csi_pads {
> + CSI_PAD_SINK = 0,
> + CSI_PAD_SOURCE = 1,
> + CSI_PADS_NUM = 2,
> +};
> +
> +struct plat_ipk_source_info {
> + u16 flags;
> + u16 mux_id;
> +};
> +
> +struct plat_ipk_fmt {
> + u32 mbus_code;
> + char *name;
> + u32 fourcc;
> + u8 depth;
> +};
> +
> +struct plat_ipk_media_pipeline;
> +
> +/*
> + * Media pipeline operations to be called from within a video node, i.e. the
> + * last entity within the pipeline. Implemented by related media device driver.
> + */
> +struct plat_ipk_media_pipeline_ops {
> + int (*prepare)(struct plat_ipk_media_pipeline *p,
> + struct media_entity *me);
> + int (*unprepare)(struct plat_ipk_media_pipeline *p);
> + int (*open)(struct plat_ipk_media_pipeline *p,
> + struct media_entity *me, bool resume);
> + int (*close)(struct plat_ipk_media_pipeline *p);
> + int (*set_stream)(struct plat_ipk_media_pipeline *p, bool state);
> + int (*set_format)(struct plat_ipk_media_pipeline *p,
> + struct v4l2_subdev_format *fmt);
> +};
> +
> +struct plat_ipk_video_entity {
> + struct video_device vdev;
> + struct plat_ipk_media_pipeline *pipe;
> +};
> +
> +struct plat_ipk_media_pipeline {
> + struct media_pipeline mp;
> + const struct plat_ipk_media_pipeline_ops *ops;
> +};
> +
> +static inline struct plat_ipk_video_entity *
> +vdev_to_plat_ipk_video_entity(struct video_device *vdev)
> +{
> + return container_of(vdev, struct plat_ipk_video_entity, vdev);
> +}
> +
> +#define plat_ipk_pipeline_call(ent, op, args...)\
> + (!(ent) ? -ENOENT : (((ent)->pipe->ops && (ent)->pipe->ops->op) ? \
> + (ent)->pipe->ops->op(((ent)->pipe), ##args) : -ENOIOCTLCMD)) \
> +
> +
> +#endif /* PLAT_IPK_INCLUDES_H_ */
> diff --git a/drivers/media/platform/dwc/video_device.c b/drivers/media/platform/dwc/video_device.c
> new file mode 100644
> index 0000000..d827426
> --- /dev/null
> +++ b/drivers/media/platform/dwc/video_device.c
> @@ -0,0 +1,741 @@
> +/*
> + * DWC MIPI CSI-2 Host IPK video device device driver
> + *
> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
> + * Author: Ramiro Oliveira <ramiro.oliveira@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published
> + * by the Free Software Foundation, either version 2 of the License,
> + * or (at your option) any later version.
> + */
> +
> +#include "video_device.h"
> +
> +const struct plat_ipk_fmt vid_dev_formats[] = {
> + {
> + .name = "RGB888",
> + .fourcc = V4L2_PIX_FMT_BGR24,
> + .depth = 24,
> + .mbus_code = MEDIA_BUS_FMT_RGB888_2X12_LE,
> + }, {
> + .name = "RGB565",
> + .fourcc = V4L2_PIX_FMT_RGB565,
> + .depth = 16,
> + .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE,
> + },
> +};
> +
> +const struct plat_ipk_fmt *
> +vid_dev_find_format(struct v4l2_format *f, int index)
> +{
> + const struct plat_ipk_fmt *fmt = NULL;
> + unsigned int i;
> +
> + if (index >= (int) ARRAY_SIZE(vid_dev_formats))
> + return NULL;
> +
> + for (i = 0; i < ARRAY_SIZE(vid_dev_formats); ++i) {
> + fmt = &vid_dev_formats[i];
> + if (fmt->fourcc == f->fmt.pix.pixelformat)
> + return fmt;
> + }
> + return NULL;
> +}
> +
> +/*
> + * Video node ioctl operations
> + */
> +static int
> +vidioc_querycap(struct file *file, void *priv, struct v4l2_capability *cap)
> +{
> + struct video_device_dev *vid_dev = video_drvdata(file);
> +
> + strlcpy(cap->driver, VIDEO_DEVICE_NAME, sizeof(cap->driver));
> + strlcpy(cap->card, VIDEO_DEVICE_NAME, sizeof(cap->card));
> + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> + dev_name(&vid_dev->pdev->dev));
> +
> + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> + return 0;
> +}
> +
> +int
> +vidioc_enum_fmt_vid_cap(struct file *file, void *priv, struct v4l2_fmtdesc *f)
> +{
> + const struct plat_ipk_fmt *p_fmt;
> +
> + if (f->index >= ARRAY_SIZE(vid_dev_formats))
> + return -EINVAL;
> +
> + p_fmt = &vid_dev_formats[f->index];
> +
> + strlcpy(f->description, p_fmt->name, sizeof(f->description));
> + f->pixelformat = p_fmt->fourcc;
> +
> + return 0;
> +}
> +
> +int vidioc_g_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
> +{
> + struct video_device_dev *dev = video_drvdata(file);
> +
> + memcpy(&f->fmt.pix, &dev->format.fmt.pix,
> + sizeof(struct v4l2_pix_format));
> +
> + return 0;
> +}
> +
> +static int
> +vidioc_try_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
> +{
> + const struct plat_ipk_fmt *fmt;
> +
> + fmt = vid_dev_find_format(f, -1);
> + if (!fmt) {
> + f->fmt.pix.pixelformat = V4L2_PIX_FMT_RGB565;
> + fmt = vid_dev_find_format(f, -1);
> + }
> +
> + f->fmt.pix.field = V4L2_FIELD_NONE;
> + v4l_bound_align_image(&f->fmt.pix.width, 48, MAX_WIDTH, 2,
> + &f->fmt.pix.height, 32, MAX_HEIGHT, 0, 0);
> +
> + f->fmt.pix.bytesperline = (f->fmt.pix.width * fmt->depth) >> 3;
> + f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> + return 0;
> +}
> +
> +int vidioc_s_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
> +{
> + struct video_device_dev *dev = video_drvdata(file);
> + int ret;
> + struct v4l2_subdev_format fmt;
> +
> + if (vb2_is_busy(&dev->vb_queue))
> + return -EBUSY;
> +
> + ret = vidioc_try_fmt_vid_cap(file, dev, f);
> + if (ret)
> + return ret;
> +
> + dev->fmt = vid_dev_find_format(f, -1);
> + pixel_format(dev) = f->fmt.pix.pixelformat;
> + width(dev) = f->fmt.pix.width;
> + height(dev) = f->fmt.pix.height;
> + bytes_per_line(dev) = width(dev) * dev->fmt->depth / 8;
> + size_image(dev) = height(dev) * bytes_per_line(dev);
> +
> + fmt.format.colorspace = V4L2_COLORSPACE_SRGB;
> + fmt.format.code = dev->fmt->mbus_code;
> +
> + fmt.format.width = width(dev);
> + fmt.format.height = height(dev);
> +
> + ret = plat_ipk_pipeline_call(&dev->ve, set_format, &fmt);
> +
> + return 0;
> +}
> +
> +int vidioc_enum_framesizes(struct file *file, void *fh,
> + struct v4l2_frmsizeenum *fsize)
> +{
> + static const struct v4l2_frmsize_stepwise sizes = {
> + 48, MAX_WIDTH, 4,
> + 32, MAX_HEIGHT, 1
> + };
> + int i;
> +
> + if (fsize->index)
> + return -EINVAL;
> + for (i = 0; i < ARRAY_SIZE(vid_dev_formats); i++)
> + if (vid_dev_formats[i].fourcc == fsize->pixel_format)
> + break;
> + if (i == ARRAY_SIZE(vid_dev_formats))
> + return -EINVAL;
> + fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> + fsize->stepwise = sizes;
> + return 0;
> +}
> +
> +int vidioc_enum_input(struct file *file, void *priv, struct v4l2_input *input)
> +{
> + if (input->index != 0)
> + return -EINVAL;
> +
> + input->type = V4L2_INPUT_TYPE_CAMERA;
> + input->std = V4L2_STD_ALL; /* Not sure what should go here */
> + strcpy(input->name, "Camera");
> + return 0;
> +}
> +
> +int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
> +{
> + *i = 0;
> + return 0;
> +}
> +
> +int vidioc_s_input(struct file *file, void *priv, unsigned int i)
> +{
> + if (i != 0)
> + return -EINVAL;
> + return 0;
> +}
> +
> +int vidioc_g_std(struct file *file, void *fh, v4l2_std_id *norm)
> +{
> + *norm = V4L2_STD_NTSC_M;
> + return 0;
> +}
> +
> +int vidioc_s_std(struct file *file, void *fh, v4l2_std_id a)
> +{
> + return 0;
> +}
> +
> +static int
> +vid_dev_pipeline_validate(struct video_device_dev *vid_dev)
> +{
> + return 0;
> +}
> +
> +static int
> +vid_dev_streamon(struct file *file, void *priv, enum v4l2_buf_type type)
> +{
> + struct video_device_dev *vid_dev = video_drvdata(file);
> + struct media_entity *entity = &vid_dev->ve.vdev.entity;
> + int ret;
> +
> + ret = media_entity_pipeline_start(entity, &vid_dev->ve.pipe->mp);
> + if (ret < 0)
> + return ret;
> +
> + ret = vid_dev_pipeline_validate(vid_dev);
> + if (ret < 0)
> + goto err_p_stop;
> +
> + vb2_ioctl_streamon(file, priv, type);
> + if (!ret)
> + return ret;
> +
> +err_p_stop:
> + media_entity_pipeline_stop(entity);
> + return 0;
> +}
> +
> +static int
> +vid_dev_streamoff(struct file *file, void *priv, enum v4l2_buf_type type)
> +{
> + struct video_device_dev *vid_dev = video_drvdata(file);
> + int ret;
> +
> + ret = vb2_ioctl_streamoff(file, priv, type);
> + if (ret < 0)
> + return ret;
> +
> + media_entity_pipeline_stop(&vid_dev->ve.vdev.entity);
> + return 0;
> +}
> +
> +static const struct v4l2_ioctl_ops vid_dev_ioctl_ops = {
> + .vidioc_querycap = vidioc_querycap,
> + .vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
> + .vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
> + .vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
> + .vidioc_enum_framesizes = vidioc_enum_framesizes,
> + .vidioc_enum_input = vidioc_enum_input,
> + .vidioc_g_input = vidioc_g_input,
> + .vidioc_s_input = vidioc_s_input,
> +
> + .vidioc_reqbufs = vb2_ioctl_reqbufs,
> + .vidioc_create_bufs = vb2_ioctl_create_bufs,
> + .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
> + .vidioc_querybuf = vb2_ioctl_querybuf,
> + .vidioc_qbuf = vb2_ioctl_qbuf,
> + .vidioc_dqbuf = vb2_ioctl_dqbuf,
> + .vidioc_streamon = vid_dev_streamon,
> + .vidioc_streamoff = vid_dev_streamoff,
> +};
> +
> +/* Capture subdev media entity operations */
> +static int
> +vid_dev_link_setup(struct media_entity *entity,
> + const struct media_pad *local,
> + const struct media_pad *remote, u32 flags)
> +{
> + return 0;
> +}
> +
> +static const struct media_entity_operations vid_dev_subdev_media_ops = {
> + .link_setup = vid_dev_link_setup,
> +};
> +
> +static int
> +vid_dev_open(struct file *file)
> +{
> + struct video_device_dev *vid_dev = video_drvdata(file);
> + struct media_entity *me = &vid_dev->ve.vdev.entity;
> + int ret;
> +
> + mutex_lock(&vid_dev->lock);
> +
> + ret = v4l2_fh_open(file);
> + if (ret < 0)
> + goto unlock;
> +
> + if (!v4l2_fh_is_singular_file(file))
> + goto unlock;
> +
> + mutex_lock(&me->graph_obj.mdev->graph_mutex);
> +
> + ret = plat_ipk_pipeline_call(&vid_dev->ve, open, me, true);
> + if (ret == 0)
> + me->use_count++;
> +
> + mutex_unlock(&me->graph_obj.mdev->graph_mutex);
> +
> + if (!ret)
> + goto unlock;
> +
> + v4l2_fh_release(file);
> +unlock:
> + mutex_unlock(&vid_dev->lock);
> + return ret;
> +}
> +
> +static int
> +vid_dev_release(struct file *file)
> +{
> + struct video_device_dev *vid_dev = video_drvdata(file);
> + struct media_entity *entity = &vid_dev->ve.vdev.entity;
> +
> + mutex_lock(&vid_dev->lock);
> +
> + if (v4l2_fh_is_singular_file(file)) {
> + plat_ipk_pipeline_call(&vid_dev->ve, close);
> + mutex_lock(&entity->graph_obj.mdev->graph_mutex);
> + entity->use_count--;
> + mutex_unlock(&entity->graph_obj.mdev->graph_mutex);
> + }
> +
> + _vb2_fop_release(file, NULL);
> +
> + mutex_unlock(&vid_dev->lock);
> + return 0;
> +}
> +
> +static const struct v4l2_file_operations vid_dev_fops = {
> + .owner = THIS_MODULE,
> + .open = vid_dev_open,
> + .release = vid_dev_release,
> + .write = vb2_fop_write,
> + .read = vb2_fop_read,
> + .poll = vb2_fop_poll,
> + .unlocked_ioctl = video_ioctl2,
> + .mmap = vb2_fop_mmap,
> +};
> +
> +/*
> + * VideoBuffer2 operations
> + */
> +
> +void fill_buffer(struct video_device_dev *dev, struct rx_buffer *buf,
> + int buf_num, unsigned long flags)
> +{
> + int size = 0;
> + void *vbuf = NULL;
> +
> + if (&buf->vb == NULL)
> + return;
> +
> + size = vb2_plane_size(&buf->vb.vb2_buf, 0);
> + vbuf = vb2_plane_vaddr(&buf->vb.vb2_buf, 0);
> +
> + if (vbuf) {
> + spin_unlock_irqrestore(&dev->slock, flags);
> +
> + memcpy(vbuf, dev->dma_buf[buf_num].cpu_addr, size);
> +
> + spin_lock_irqsave(&dev->slock, flags);
> +
> + buf->vb.field = dev->format.fmt.pix.field;
> + buf->vb.sequence++;
> + buf->vb.vb2_buf.timestamp = ktime_get_ns();
> + }
> + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
> +}
> +
> +static void buffer_copy_process(void *param)
> +{
> + struct video_device_dev *dev = (struct video_device_dev *) param;
> + unsigned long flags;
> + struct dmaqueue *dma_q = &dev->vidq;
> + struct rx_buffer *buf = NULL;
> +
> + spin_lock_irqsave(&dev->slock, flags);
> +
> + if (!list_empty(&dma_q->active)) {
> + buf = list_entry(dma_q->active.next, struct rx_buffer, list);
> + list_del(&buf->list);
> + fill_buffer(dev, buf, dev->last_idx, flags);
> + }
> +
> + spin_unlock_irqrestore(&dev->slock, flags);
> +}
> +
> +static inline struct rx_buffer *to_rx_buffer(struct vb2_v4l2_buffer *vb2)
> +{
> + return container_of(vb2, struct rx_buffer, vb);
> +}
> +
> +int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
> + unsigned int *nplanes, unsigned int sizes[],
> + struct device *alloc_devs[])
> +{
> + struct video_device_dev *dev = vb2_get_drv_priv(vq);
> + unsigned long size = 0;
> + int i;
> +
> + size = size_image(dev);
> + if (size == 0)
> + return -EINVAL;
> +
> + *nbuffers = N_BUFFERS;
> +
> + for (i = 0; i < N_BUFFERS; i++) {
> + dev->dma_buf[i].cpu_addr = dma_alloc_coherent(&dev->pdev->dev,
> + size_image(dev),
> + &dev->
> + dma_buf
> + [i].dma_addr,
> + GFP_KERNEL);
> + }
> +
> + *nplanes = 1;
> + sizes[0] = size;
> +
> + return 0;
> +}
> +
> +int buffer_prepare(struct vb2_buffer *vb)
> +{
> + struct rx_buffer *buf = NULL;
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + int size = 0;
> +
> + if (vb == NULL) {
> + pr_warn("%s:vb2_buffer is null\n", FUNC_NAME);
> + return 0;
> + }
> +
> + buf = to_rx_buffer(vbuf);
> +
> + size = vb2_plane_size(&buf->vb.vb2_buf, 0);
> + vb2_set_plane_payload(&buf->vb.vb2_buf, 0, size);
> +
> + INIT_LIST_HEAD(&buf->list);
> + return 0;
> +}
> +
> +void buffer_queue(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct video_device_dev *dev = NULL;
> + struct rx_buffer *buf = NULL;
> + struct dmaqueue *vidq = NULL;
> + struct dma_async_tx_descriptor *desc;
> + u32 flags;
> +
> + if (vb == NULL) {
> + pr_warn("%s:vb2_buffer is null\n", FUNC_NAME);
> + return;
> + }
> +
> + dev = vb2_get_drv_priv(vb->vb2_queue);
> + buf = to_rx_buffer(vbuf);
> + vidq = &dev->vidq;
> +
> + flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
> + dev->xt.dir = DMA_DEV_TO_MEM;
> + dev->xt.src_sgl = false;
> + dev->xt.dst_inc = false;
> + dev->xt.dst_sgl = true;
> + dev->xt.dst_start = dev->dma_buf[dev->idx].dma_addr;
> +
> + dev->last_idx = dev->idx;
> + dev->idx++;
> + if (dev->idx >= N_BUFFERS)
> + dev->idx = 0;
> +
> + dev->xt.frame_size = 1;
> + dev->sgl[0].size = bytes_per_line(dev);
> + dev->sgl[0].icg = 0;
> + dev->xt.numf = height(dev);
> +
> + desc = dmaengine_prep_interleaved_dma(dev->dma, &dev->xt, flags);
> + if (!desc) {
> + pr_err("Failed to prepare DMA transfer\n");
> + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> + return;
> + }
> +
> + desc->callback = buffer_copy_process;
> + desc->callback_param = dev;
> +
> + spin_lock(&dev->slock);
> + list_add_tail(&buf->list, &vidq->active);
> + spin_unlock(&dev->slock);
> +
> + dmaengine_submit(desc);
> +
> + if (vb2_is_streaming(&dev->vb_queue))
> + dma_async_issue_pending(dev->dma);
> +}
> +
> +int start_streaming(struct vb2_queue *vq, unsigned int count)
> +{
> + struct video_device_dev *dev = vb2_get_drv_priv(vq);
> +
> + dma_async_issue_pending(dev->dma);
> +
> + return 0;
> +}
> +
> +void stop_streaming(struct vb2_queue *vq)
> +{
> + struct video_device_dev *dev = vb2_get_drv_priv(vq);
> + struct dmaqueue *dma_q = &dev->vidq;
> +
> + /* Stop and reset the DMA engine. */
> + dmaengine_terminate_all(dev->dma);
> +
> + while (!list_empty(&dma_q->active)) {
> + struct rx_buffer *buf;
> +
> + buf = list_entry(dma_q->active.next, struct rx_buffer, list);
> + if (buf) {
> + list_del(&buf->list);
> + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> + }
> + }
> + list_del_init(&dev->vidq.active);
> +}
> +
> +static const struct vb2_ops vb2_video_qops = {
> + .queue_setup = queue_setup,
> + .buf_prepare = buffer_prepare,
> + .buf_queue = buffer_queue,
> + .start_streaming = start_streaming,
> + .stop_streaming = stop_streaming,
> + .wait_prepare = vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
> +};
> +
> +static int vid_dev_subdev_s_power(struct v4l2_subdev *sd, int on)
> +{
> + return 0;

I think you could just omit defining the function if it does nothing. The
same goes for core ops.

> +}
> +
> +static int vid_dev_subdev_registered(struct v4l2_subdev *sd)
> +{
> + struct video_device_dev *vid_dev = v4l2_get_subdevdata(sd);
> + struct vb2_queue *q = &vid_dev->vb_queue;
> + struct video_device *vfd = &vid_dev->ve.vdev;
> + int ret;
> +
> + memset(vfd, 0, sizeof(*vfd));
> +
> + snprintf(vfd->name, sizeof(vfd->name), VIDEO_DEVICE_NAME);

How about strlcpy()?

> +
> + vfd->fops = &vid_dev_fops;
> + vfd->ioctl_ops = &vid_dev_ioctl_ops;
> + vfd->v4l2_dev = sd->v4l2_dev;
> + vfd->minor = -1;
> + vfd->release = video_device_release_empty;
> + vfd->queue = q;
> +
> + INIT_LIST_HEAD(&vid_dev->vidq.active);
> + init_waitqueue_head(&vid_dev->vidq.wq);
> + memset(q, 0, sizeof(*q));
> + q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> + q->io_modes = VB2_MMAP | VB2_USERPTR;
> + q->ops = &vb2_video_qops;
> + q->mem_ops = &vb2_vmalloc_memops;
> + q->buf_struct_size = sizeof(struct rx_buffer);
> + q->drv_priv = vid_dev;
> + q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> + q->lock = &vid_dev->lock;
> +
> + ret = vb2_queue_init(q);
> + if (ret < 0)
> + return ret;
> +
> + vid_dev->vd_pad.flags = MEDIA_PAD_FL_SINK;
> + ret = media_entity_pads_init(&vfd->entity, 1, &vid_dev->vd_pad);
> + if (ret < 0)
> + return ret;
> +
> + video_set_drvdata(vfd, vid_dev);
> + vid_dev->ve.pipe = v4l2_get_subdev_hostdata(sd);
> +
> + ret = video_register_device(vfd, VFL_TYPE_GRABBER, -1);
> + if (ret < 0) {
> + media_entity_cleanup(&vfd->entity);
> + vid_dev->ve.pipe = NULL;
> + return ret;
> + }
> +
> + v4l2_info(sd->v4l2_dev, "Registered %s as /dev/%s\n",
> + vfd->name, video_device_node_name(vfd));
> + return 0;
> +}
> +
> +static void vid_dev_subdev_unregistered(struct v4l2_subdev *sd)
> +{
> + struct video_device_dev *vid_dev = v4l2_get_subdevdata(sd);
> +
> + if (vid_dev == NULL)
> + return;

Can this happen?

> +
> + mutex_lock(&vid_dev->lock);
> +
> + if (video_is_registered(&vid_dev->ve.vdev)) {
> + video_unregister_device(&vid_dev->ve.vdev);
> + media_entity_cleanup(&vid_dev->ve.vdev.entity);
> + vid_dev->ve.pipe = NULL;
> + }
> +
> + mutex_unlock(&vid_dev->lock);
> +}
> +
> +static const struct v4l2_subdev_internal_ops vid_dev_subdev_internal_ops = {
> + .registered = vid_dev_subdev_registered,
> + .unregistered = vid_dev_subdev_unregistered,
> +};
> +
> +static const struct v4l2_subdev_core_ops vid_dev_subdev_core_ops = {
> + .s_power = vid_dev_subdev_s_power,
> +};
> +
> +static struct v4l2_subdev_ops vid_dev_subdev_ops = {
> + .core = &vid_dev_subdev_core_ops,
> +};
> +
> +static int vid_dev_create_capture_subdev(struct video_device_dev *vid_dev)
> +{
> + struct v4l2_subdev *sd = &vid_dev->subdev;
> + int ret;
> +
> + v4l2_subdev_init(sd, &vid_dev_subdev_ops);
> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + snprintf(sd->name, sizeof(sd->name), "Capture device");
> +
> + vid_dev->subdev_pads[VIDEO_DEV_SD_PAD_SINK_CSI].flags =
> + MEDIA_PAD_FL_SOURCE;
> + vid_dev->subdev_pads[VIDEO_DEV_SD_PAD_SOURCE_DMA].flags =
> + MEDIA_PAD_FL_SINK;
> + ret =

No need for a line break.

> + media_entity_pads_init(&sd->entity, VIDEO_DEV_SD_PADS_NUM,
> + vid_dev->subdev_pads);
> + if (ret)
> + return ret;
> +
> + sd->internal_ops = &vid_dev_subdev_internal_ops;
> + sd->entity.ops = &vid_dev_subdev_media_ops;
> + sd->owner = THIS_MODULE;
> + v4l2_set_subdevdata(sd, vid_dev);
> +
> + return 0;
> +}
> +
> +static void vid_dev_unregister_subdev(struct video_device_dev *vid_dev)
> +{
> + struct v4l2_subdev *sd = &vid_dev->subdev;
> +
> + v4l2_device_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> + v4l2_set_subdevdata(sd, NULL);
> +}
> +
> +static const struct of_device_id vid_dev_of_match[];
> +
> +static int vid_dev_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct of_device_id *of_id;
> + int ret = 0;
> + struct video_device_dev *vid_dev;
> +
> + dev_info(dev, "Installing IPK Video Device module\n");
> +
> + if (!dev->of_node)
> + return -ENODEV;
> +
> + vid_dev = devm_kzalloc(dev, sizeof(*vid_dev), GFP_KERNEL);
> + if (!vid_dev)
> + return -ENOMEM;
> +
> + of_id = of_match_node(vid_dev_of_match, dev->of_node);
> + if (WARN_ON(of_id == NULL))
> + return -EINVAL;
> +
> + vid_dev->pdev = pdev;
> +
> + spin_lock_init(&vid_dev->slock);
> + mutex_init(&vid_dev->lock);
> +
> + dev_info(&pdev->dev, "Requesting DMA\n");
> + vid_dev->dma = dma_request_slave_channel(&pdev->dev, "vdma0");
> + if (vid_dev->dma == NULL) {
> + dev_err(&pdev->dev, "no VDMA channel found\n");
> + ret = -ENODEV;
> + goto end;
> + }
> +
> + ret = vid_dev_create_capture_subdev(vid_dev);
> + if (ret)
> + goto end;
> +
> + platform_set_drvdata(pdev, vid_dev);
> +
> + dev_info(dev, "Video Device registered successfully\n");
> + return 0;
> +end:
> + dev_err(dev, "Video Device not registered!!\n");
> + return ret;
> +}
> +
> +static int vid_dev_remove(struct platform_device *pdev)
> +{
> + struct video_device_dev *dev = platform_get_drvdata(pdev);
> +
> + vid_dev_unregister_subdev(dev);
> + dev_info(&pdev->dev, "Driver removed\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id vid_dev_of_match[] = {
> + {.compatible = "snps,video-device"},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(of, vid_dev_of_match);
> +
> +static struct platform_driver __refdata vid_dev_pdrv = {
> + .remove = vid_dev_remove,
> + .probe = vid_dev_probe,
> + .driver = {
> + .name = VIDEO_DEVICE_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = vid_dev_of_match,
> + },
> +};
> +
> +module_platform_driver(vid_dev_pdrv);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Driver for configuring DMA and Video Device");
> diff --git a/drivers/media/platform/dwc/video_device.h b/drivers/media/platform/dwc/video_device.h
> new file mode 100644
> index 0000000..e828d4b
> --- /dev/null
> +++ b/drivers/media/platform/dwc/video_device.h
> @@ -0,0 +1,101 @@
> +/*
> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef VIDEO_DEVICE_H_
> +#define VIDEO_DEVICE_H_
> +
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/delay.h>
> +#include <linux/list.h>
> +#include <linux/wait.h>
> +#include <linux/string.h>
> +#include <linux/videodev2.h>
> +#include <linux/dma/xilinx_dma.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-fh.h>
> +#include <media/v4l2-common.h>
> +#include <media/videobuf2-vmalloc.h>
> +#include <media/media-entity.h>
> +#include <linux/io.h>
> +
> +#include "plat_ipk_video.h"
> +
> +#define N_BUFFERS 3
> +
> +#define VIDEO_DEVICE_NAME "video-device"
> +
> +#define FUNC_NAME __func__
> +
> +struct rx_buffer {
> + /** @short Buffer for video frames */
> + struct vb2_v4l2_buffer vb;
> + struct list_head list;
> +
> + dma_addr_t dma_addr;
> + void *cpu_addr;
> +};
> +
> +struct dmaqueue {
> + struct list_head active;
> + wait_queue_head_t wq;
> +};
> +
> +/**
> + * @short Structure to embed device driver information
> + */
> +struct video_device_dev {
> + struct platform_device *pdev;
> + struct v4l2_device *v4l2_dev;
> + struct v4l2_subdev subdev;
> + struct media_pad vd_pad;
> + struct media_pad subdev_pads[VIDEO_DEV_SD_PADS_NUM];
> + struct mutex lock;
> + spinlock_t slock;
> + struct plat_ipk_video_entity ve;
> + struct v4l2_format format;
> + struct v4l2_pix_format pix_format;
> + const struct plat_ipk_fmt *fmt;
> + unsigned long *alloc_ctx;
> +
> + /* Buffer and DMA */
> + struct vb2_queue vb_queue;
> + int idx;
> + int last_idx;
> + struct dmaqueue vidq;
> + struct rx_buffer dma_buf[N_BUFFERS];
> + struct dma_chan *dma;
> + struct dma_interleaved_template xt;
> + struct data_chunk sgl[1];
> +};
> +
> +/**
> + * @short Defines to simplify the code reading
> + */
> +
> +#define pixel_format(dev) \
> + dev->format.fmt.pix.pixelformat
> +#define bytes_per_line(dev) \
> + dev->format.fmt.pix.bytesperline
> +#define width(dev) \
> + dev->format.fmt.pix.width
> +#define height(dev) \
> + dev->format.fmt.pix.height
> +#define size_image(dev) \
> + dev->format.fmt.pix.sizeimage

How about referring to the field directly, or define a local variable for a
pointer to dev->format.fmt.fix in functions it's needed.

> +
> +const struct plat_ipk_fmt *vid_dev_find_format(struct v4l2_format *f,
> + int index);
> +
> +#endif /* VIDEO_DEVICE_H_ */

--
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx