cron job: media_tree daily build: WARNINGS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Sat Apr  8 05:00:18 CEST 2017
media-tree git hash:2f65ec0567f77b75f459c98426053a3787af356a
media_build git hash:   0d47a8527df46dc53923513c4055d76060ec2aaa
v4l-utils git hash: 08572e7db2120bc45db732d02409dfd3346b8e51
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: OK
linux-3.12.67-i686: OK
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9-i686: OK
linux-4.10.1-i686: OK
linux-4.11-rc1-i686: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: OK
linux-3.12.67-x86_64: OK
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9-x86_64: WARNINGS
linux-4.10.1-x86_64: WARNINGS
linux-4.11-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

Re: [PATCH v2 1/8] v4l: flash led class: Use fwnode_handle instead of device_node in init

[kbuild test robot]

Hi Sakari,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.11-rc5]
[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/Sakari-Ailus/v4l-flash-led-class-Use-fwnode_handle-instead-of-device_node-in-init/20170408-051139
base:   git://linuxtv.org/media_tree.git master
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
wget 
https://raw.githubusercontent.com/01org/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=tile 

Note: the 
linux-review/Sakari-Ailus/v4l-flash-led-class-Use-fwnode_handle-instead-of-device_node-in-init/20170408-051139
 HEAD 86b5e432c352250540fcd89854fa7ad55ba9e749 builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/media/v4l2-core/v4l2-flash-led-class.c: In function 
'v4l2_flash_init':
>> drivers/media/v4l2-core/v4l2-flash-led-class.c:642:4: error: 'struct 
>> v4l2_subdev' has no member named 'fwnode'
   drivers/media/v4l2-core/v4l2-flash-led-class.c:642:2: error: implicit 
declaration of function 'dev_fwnode'
   drivers/media/v4l2-core/v4l2-flash-led-class.c:642:25: warning: 
pointer/integer type mismatch in conditional expression [enabled by default]
   drivers/media/v4l2-core/v4l2-flash-led-class.c:658:2: error: implicit 
declaration of function 'fwnode_handle_get'
   drivers/media/v4l2-core/v4l2-flash-led-class.c:658:22: error: 'struct 
v4l2_subdev' has no member named 'fwnode'
   drivers/media/v4l2-core/v4l2-flash-led-class.c:667:22: error: 'struct 
v4l2_subdev' has no member named 'fwnode'
   drivers/media/v4l2-core/v4l2-flash-led-class.c: In function 
'v4l2_flash_release':
   drivers/media/v4l2-core/v4l2-flash-led-class.c:687:22: error: 'struct 
v4l2_subdev' has no member named 'fwnode'
   cc1: some warnings being treated as errors

vim +642 drivers/media/v4l2-core/v4l2-flash-led-class.c

   636  
   637  sd = _flash->sd;
   638  v4l2_flash->fled_cdev = fled_cdev;
   639  v4l2_flash->iled_cdev = iled_cdev;
   640  v4l2_flash->ops = ops;
   641  sd->dev = dev;
 > 642  sd->fwnode = fwn ? fwn : dev_fwnode(led_cdev->dev);
   643  v4l2_subdev_init(sd, _flash_subdev_ops);
   644  sd->internal_ops = _flash_subdev_internal_ops;
   645  sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;

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


.config.gz
Description: application/gzip

[PATCH] m2m-deinterlace: don't return zero on failure paths in deinterlace_probe()

[Alexey Khoroshilov]

If DMA does not support INTERLEAVE, deinterlace_probe() breaks off
initialization, releases dma channel, but returns zero.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/media/platform/m2m-deinterlace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/m2m-deinterlace.c 
b/drivers/media/platform/m2m-deinterlace.c
index bedc7cc4c7d6..980066b8d32a 100644
--- a/drivers/media/platform/m2m-deinterlace.c
+++ b/drivers/media/platform/m2m-deinterlace.c
@@ -1017,6 +1017,7 @@ static int deinterlace_probe(struct platform_device *pdev)
 
if (!dma_has_cap(DMA_INTERLEAVE, pcdev->dma_chan->device->cap_mask)) {
dev_err(>dev, "DMA does not support INTERLEAVE\n");
+   ret = -ENODEV;
goto rel_dma;
}
 
-- 
2.7.4

Re: [PATCH v2 7/8] docs-rst: media: Switch documentation to V4L2 fwnode API

[Sakari Ailus]

Hi Laurent,

On Fri, Apr 07, 2017 at 12:59:01PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thursday 06 Apr 2017 16:12:09 Sakari Ailus wrote:
> > Instead of including the V4L2 OF header in ReST documentation, use the
> > V4L2 fwnode header instead.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  Documentation/media/kapi/v4l2-core.rst   | 2 +-
> >  Documentation/media/kapi/v4l2-fwnode.rst | 3 +++
> >  Documentation/media/kapi/v4l2-of.rst | 3 ---
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >  create mode 100644 Documentation/media/kapi/v4l2-fwnode.rst
> >  delete mode 100644 Documentation/media/kapi/v4l2-of.rst
> > 
> > diff --git a/Documentation/media/kapi/v4l2-core.rst
> > b/Documentation/media/kapi/v4l2-core.rst index e967715..1bc8a14 100644
> > --- a/Documentation/media/kapi/v4l2-core.rst
> > +++ b/Documentation/media/kapi/v4l2-core.rst
> > @@ -19,7 +19,7 @@ Video2Linux devices
> >  v4l2-mc
> >  v4l2-mediabus
> >  v4l2-mem2mem
> > -v4l2-of
> > +v4l2-fwnode
> 
> I wonder whether we should keep this alphabetically sorted.

It's not fully sorted at the moment --- see tuner / common below. It'd be
good if it was though. I'll add a patch for that.

> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart 

Thanks!

> 
> >  v4l2-rect
> >  v4l2-tuner
> >  v4l2-common
> > diff --git a/Documentation/media/kapi/v4l2-fwnode.rst
> > b/Documentation/media/kapi/v4l2-fwnode.rst new file mode 100644
> > index 000..6c8bccd
> > --- /dev/null
> > +++ b/Documentation/media/kapi/v4l2-fwnode.rst
> > @@ -0,0 +1,3 @@
> > +V4L2 fwnode kAPI
> > +
> > +.. kernel-doc:: include/media/v4l2-fwnode.h
> > diff --git a/Documentation/media/kapi/v4l2-of.rst
> > b/Documentation/media/kapi/v4l2-of.rst deleted file mode 100644
> > index 1ddf76b..000
> > --- a/Documentation/media/kapi/v4l2-of.rst
> > +++ /dev/null
> > @@ -1,3 +0,0 @@
> > -V4L2 Open Firmware kAPI
> > -^^^
> > -.. kernel-doc:: include/media/v4l2-of.h
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v2 6/8] v4l: media/drv-intf/soc_mediabus.h: include dependent header file

[Sakari Ailus]

Hi Laurent,

On Fri, Apr 07, 2017 at 01:01:29PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thursday 06 Apr 2017 16:12:08 Sakari Ailus wrote:
> > media/drv-intf/soc_mediabus.h does depend on struct v4l2_mbus_config which
> > is defined in media/v4l2-mediabus.h. Include it.
> > 
> > Signed-off-by: Sakari Ailus 
> 
> Was this provided indirectly before, through v4l2-of.h perhaps ? If so, 
> shouldn't this patch be moved before 5/8 ? Apart from that,
> 
> Reviewed-by: Laurent Pinchart 

I tried compiling with and without this patch and see no difference. I could
miss something but the more likely case is that the reason why I wrote this
patch has ceased to exist. I'll drop it from the set, at least for now.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v2 5/8] v4l: Switch from V4L2 OF not V4L2 fwnode API

[Sakari Ailus]

Hi Laurent,

On Fri, Apr 07, 2017 at 02:09:16PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Friday 07 Apr 2017 13:58:06 Sakari Ailus wrote:
> > On Fri, Apr 07, 2017 at 01:32:54PM +0300, Laurent Pinchart wrote:
> > > On Thursday 06 Apr 2017 16:12:07 Sakari Ailus wrote:
> > > > Switch users of the v4l2_of_ APIs to the more generic v4l2_fwnode_ APIs.
> > > > 
> > > > Existing OF matching continues to be supported. omap3isp and smiapp
> > > > drivers are converted to fwnode matching as well.
> > > > 
> > > > Signed-off-by: Sakari Ailus 
> > > > Acked-by: Benoit Parrot  # i2c/ov2569.c,
> > > > am437x/am437x-vpfe.c and ti-vpe/cal.c ---
> > > > 
> > > >  drivers/media/i2c/Kconfig  |  9 
> > > >  drivers/media/i2c/adv7604.c|  7 +--
> > > >  drivers/media/i2c/mt9v032.c|  7 +--
> > > >  drivers/media/i2c/ov2659.c |  8 +--
> > > >  drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  7 +--
> > > >  drivers/media/i2c/s5k5baf.c|  6 +--
> > > >  drivers/media/i2c/smiapp/Kconfig   |  1 +
> > > >  drivers/media/i2c/smiapp/smiapp-core.c | 29 ++-
> > > >  drivers/media/i2c/tc358743.c   | 11 ++--
> > > >  drivers/media/i2c/tvp514x.c|  6 +--
> > > >  drivers/media/i2c/tvp5150.c|  7 +--
> > > >  drivers/media/i2c/tvp7002.c|  6 +--
> > > >  drivers/media/platform/Kconfig |  3 ++
> > > >  drivers/media/platform/am437x/Kconfig  |  1 +
> > > >  drivers/media/platform/am437x/am437x-vpfe.c|  8 +--
> > > >  drivers/media/platform/atmel/Kconfig   |  1 +
> > > >  drivers/media/platform/atmel/atmel-isc.c   |  8 +--
> > > >  drivers/media/platform/exynos4-is/Kconfig  |  2 +
> > > >  drivers/media/platform/exynos4-is/media-dev.c  |  6 +--
> > > >  drivers/media/platform/exynos4-is/mipi-csis.c  |  6 +--
> > > >  drivers/media/platform/omap3isp/isp.c  | 71  +-
> > > >  drivers/media/platform/pxa_camera.c|  7 +--
> > > >  drivers/media/platform/rcar-vin/Kconfig|  1 +
> > > >  drivers/media/platform/rcar-vin/rcar-core.c|  6 +--
> > > >  drivers/media/platform/soc_camera/Kconfig  |  1 +
> > > >  drivers/media/platform/soc_camera/atmel-isi.c  |  7 +--
> > > >  drivers/media/platform/soc_camera/soc_camera.c |  3 +-
> > > >  drivers/media/platform/ti-vpe/cal.c| 11 ++--
> > > >  drivers/media/platform/xilinx/Kconfig  |  1 +
> > > >  drivers/media/platform/xilinx/xilinx-vipp.c| 59  +-
> > > >  include/media/v4l2-fwnode.h|  4 +-
> > > >  31 files changed, 176 insertions(+), 134 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > > index cee1dae..6b2423a 100644
> > > > --- a/drivers/media/i2c/Kconfig
> > > > +++ b/drivers/media/i2c/Kconfig
> > > > @@ -210,6 +210,7 @@ config VIDEO_ADV7604
> > > > 
> > > > depends on GPIOLIB || COMPILE_TEST
> > > > select HDMI
> > > > select MEDIA_CEC_EDID
> > > > 
> > > > +   select V4L2_FWNODE
> > > 
> > > What happens when building the driver on a platform that includes neither
> > > OF nor ACPI support ?
> > 
> > You need either in practice, also for the V4L2 fwnode to be meaningful.
> > 
> > Do you have something in particular in mind?
> 
> I will obviously need either OF or ACPI to use the fwnode API, but some 
> drivers still support platform data (either on non-OF embedded systems, or 
> when the I2C device is part of a PCI card for instance). Compile-testing is 
> also a use case I'm concerned about.

Ah, so essentially compiling a driver using V4L2 fwnode with both ACPI and
OF disabled? I don't know if there are such drivers right now but that's a
good point in general.

> 
> [snip]
> 
> > > > diff --git a/drivers/media/platform/omap3isp/isp.c
> > > > b/drivers/media/platform/omap3isp/isp.c index 084ecf4a..95850b9 100644
> > > > --- a/drivers/media/platform/omap3isp/isp.c
> > > > +++ b/drivers/media/platform/omap3isp/isp.c
> > > 
> > > [snip]
> > > 
> > > > @@ -2024,43 +2025,42 @@ enum isp_of_phy {
> > > > ISP_OF_PHY_CSIPHY2,
> > > >  };
> > > > 
> > > > -static int isp_of_parse_node(struct device *dev, struct device_node
> > > > *node,
> > > > -struct isp_async_subdev *isd)
> > > > +static int isp_fwnode_parse(struct device *dev, struct fwnode_handle
> > > > *fwn,
> > > > +   struct isp_async_subdev *isd)
> > > >  {
> > > > struct isp_bus_cfg *buscfg = >bus;
> > > > -   struct v4l2_of_endpoint vep;
> > > > +   struct v4l2_fwnode_endpoint vfwn;
> > > 
> > > vfwn is confusing to me, I think the variable name should show that it
> > > refers to an endpoint.
> > 
> > How about adding ep to tell it's an endpoint?
> 
> I'd name is vep or endpoint.

I'll 

[PATCH v2 5/7] [media] vimc: cap: Support several image formats

[Helen Koike]

Allow user space to change the image format as the frame size, the
pixel format, colorspace, quantization, field YCbCr encoding
and the transfer function

Signed-off-by: Helen Koike 

---

Changes in v2:
[media] vimc: cap: Support several image formats
- this is a new commit in the serie (the old one was splitted in two)
- allow user space to change all fields from struct v4l2_pix_format
  (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
- link_validate and try_fmt: also checks colospace, quantization, 
field, xfer_func, ycbcr_enc
- add struct v4l2_pix_format fmt_default
- add enum_framesizes
- enum_fmt_vid_cap: enumerate all formats from vimc_pix_map_table
- add mode dev_dbg


---
 drivers/media/platform/vimc/vimc-capture.c | 167 -
 1 file changed, 139 insertions(+), 28 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c 
b/drivers/media/platform/vimc/vimc-capture.c
index 93f6a09..a6441f7 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -40,6 +40,16 @@ struct vimc_cap_device {
struct media_pipeline pipe;
 };
 
+static const struct v4l2_pix_format fmt_default = {
+   .width = 640,
+   .height = 480,
+   .pixelformat = V4L2_PIX_FMT_RGB24,
+   .field = V4L2_FIELD_NONE,
+   .colorspace = V4L2_COLORSPACE_SRGB,
+   .quantization = V4L2_QUANTIZATION_FULL_RANGE,
+   .xfer_func = V4L2_XFER_FUNC_SRGB,
+};
+
 struct vimc_cap_buffer {
/*
 * struct vb2_v4l2_buffer must be the first element
@@ -64,7 +74,7 @@ static int vimc_cap_querycap(struct file *file, void *priv,
return 0;
 }
 
-static int vimc_cap_fmt_vid_cap(struct file *file, void *priv,
+static int vimc_cap_g_fmt_vid_cap(struct file *file, void *priv,
  struct v4l2_format *f)
 {
struct vimc_cap_device *vcap = video_drvdata(file);
@@ -74,16 +84,112 @@ static int vimc_cap_fmt_vid_cap(struct file *file, void 
*priv,
return 0;
 }
 
+static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv,
+   struct v4l2_format *f)
+{
+   struct v4l2_pix_format *format = >fmt.pix;
+   const struct vimc_pix_map *vpix;
+
+   format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH,
+   VIMC_FRAME_MAX_WIDTH);
+   format->height = clamp_t(u32, format->height, VIMC_FRAME_MIN_HEIGHT,
+VIMC_FRAME_MAX_HEIGHT);
+
+   /* Don't accept a pixelformat that is not on the table */
+   vpix = vimc_pix_map_by_pixelformat(format->pixelformat);
+   if (!vpix) {
+   format->pixelformat = fmt_default.pixelformat;
+   vpix = vimc_pix_map_by_pixelformat(format->pixelformat);
+   }
+   /* TODO: Add support for custom bytesperline values */
+   format->bytesperline = format->width * vpix->bpp;
+   format->sizeimage = format->bytesperline * format->height;
+
+   if (format->field == V4L2_FIELD_ANY)
+   format->field = fmt_default.field;
+
+   /* Check if values are out of range */
+   if (format->colorspace == V4L2_COLORSPACE_DEFAULT
+   || format->colorspace > V4L2_COLORSPACE_DCI_P3)
+   format->colorspace = fmt_default.colorspace;
+   if (format->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT
+   || format->ycbcr_enc > V4L2_YCBCR_ENC_SMPTE240M)
+   format->ycbcr_enc = fmt_default.ycbcr_enc;
+   if (format->quantization == V4L2_QUANTIZATION_DEFAULT
+   || format->quantization > V4L2_QUANTIZATION_LIM_RANGE)
+   format->quantization = fmt_default.quantization;
+   if (format->xfer_func == V4L2_XFER_FUNC_DEFAULT
+   || format->xfer_func > V4L2_XFER_FUNC_SMPTE2084)
+   format->xfer_func = fmt_default.xfer_func;
+
+   return 0;
+}
+
+static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
+ struct v4l2_format *f)
+{
+   struct vimc_cap_device *vcap = video_drvdata(file);
+
+   /* Do not change the format while stream is on */
+   if (vb2_is_busy(>queue))
+   return -EBUSY;
+
+   vimc_cap_try_fmt_vid_cap(file, priv, f);
+
+   dev_dbg(vcap->vdev.v4l2_dev->dev, "%s: format update: "
+   "old:%dx%d (0x%x, %d, %d, %d, %d) "
+   "new:%dx%d (0x%x, %d, %d, %d, %d)\n", vcap->vdev.name,
+   /* old */
+   vcap->format.width, vcap->format.height,
+   vcap->format.pixelformat, vcap->format.colorspace,
+   vcap->format.quantization, vcap->format.xfer_func,
+   vcap->format.ycbcr_enc,
+   /* new */
+   f->fmt.pix.width, f->fmt.pix.height,
+   f->fmt.pix.pixelformat, f->fmt.pix.colorspace,
+   f->fmt.pix.quantization, 

[PATCH v2 6/7] [media] vimc: deb: Add debayer filter

[Helen Koike]

Implement the debayer filter and integrate it with the core

Signed-off-by: Helen Koike 

---

Changes in v2:
[media] vimc: deb: Add debayer filter
- Using MEDIA_ENT_F_ATV_DECODER in function
- remove v4l2_dev and dev from vimc_deb_device struct
- src fmt propagates from the sink
- coding style
- remove redundant else if statements
- check end of enum and remove BUG_ON
- enum frame size with min and max values
- set/try fmt
- remove unecessary include freezer.h
- check pad types on create
- return EBUSY when trying to set the format while stream is on
- remove vsd struct
- add IS_SRC and IS_SINK macros
- add deb_mean_win_size as a parameter of the module
- check set_fmt default parameters for quantization, colorspace ...
- add more dev_dbg


---
 drivers/media/platform/vimc/Makefile   |   2 +-
 drivers/media/platform/vimc/vimc-core.c|   6 +-
 drivers/media/platform/vimc/vimc-core.h|   2 +
 drivers/media/platform/vimc/vimc-debayer.c | 573 +
 drivers/media/platform/vimc/vimc-debayer.h |  28 ++
 5 files changed, 609 insertions(+), 2 deletions(-)
 create mode 100644 drivers/media/platform/vimc/vimc-debayer.c
 create mode 100644 drivers/media/platform/vimc/vimc-debayer.h

diff --git a/drivers/media/platform/vimc/Makefile 
b/drivers/media/platform/vimc/Makefile
index c45195e..a6708f9 100644
--- a/drivers/media/platform/vimc/Makefile
+++ b/drivers/media/platform/vimc/Makefile
@@ -1,3 +1,3 @@
-vimc-objs := vimc-core.o vimc-capture.o vimc-sensor.o
+vimc-objs := vimc-core.o vimc-capture.o vimc-debayer.o vimc-sensor.o
 
 obj-$(CONFIG_VIDEO_VIMC) += vimc.o
diff --git a/drivers/media/platform/vimc/vimc-core.c 
b/drivers/media/platform/vimc/vimc-core.c
index bc4b1bb..51cbbf6 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -23,6 +23,7 @@
 
 #include "vimc-capture.h"
 #include "vimc-core.h"
+#include "vimc-debayer.h"
 #include "vimc-sensor.h"
 
 #define VIMC_PDEV_NAME "vimc"
@@ -637,9 +638,12 @@ static int vimc_device_register(struct vimc_device *vimc)
create_func = vimc_cap_create;
break;
 
+   case VIMC_ENT_NODE_DEBAYER:
+   create_func = vimc_deb_create;
+   break;
+
/* TODO: Instantiate the specific topology node */
case VIMC_ENT_NODE_INPUT:
-   case VIMC_ENT_NODE_DEBAYER:
case VIMC_ENT_NODE_SCALER:
default:
/*
diff --git a/drivers/media/platform/vimc/vimc-core.h 
b/drivers/media/platform/vimc/vimc-core.h
index 2146672..2e621fe 100644
--- a/drivers/media/platform/vimc/vimc-core.h
+++ b/drivers/media/platform/vimc/vimc-core.h
@@ -26,6 +26,8 @@
 #define VIMC_FRAME_MIN_WIDTH 16
 #define VIMC_FRAME_MIN_HEIGHT 16
 
+#define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp)
+
 /**
  * struct vimc_pix_map - maps media bus code with v4l2 pixel format
  *
diff --git a/drivers/media/platform/vimc/vimc-debayer.c 
b/drivers/media/platform/vimc/vimc-debayer.c
new file mode 100644
index 000..24e5952
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -0,0 +1,573 @@
+/*
+ * vimc-debayer.c Virtual Media Controller Driver
+ *
+ * Copyright (C) 2015-2017 Helen Koike 
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "vimc-debayer.h"
+
+static unsigned int deb_mean_win_size = 3;
+module_param(deb_mean_win_size, uint, );
+MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the mean.\n"
+   "NOTE: the window size need to be an odd number, as the main pixel "
+   "stays in the center of the window, otherwise the next odd number "
+   "is considered");
+
+#define IS_SINK(pad) (!pad)
+#define IS_SRC(pad)  (pad)
+
+enum vimc_deb_rgb_colors {
+   VIMC_DEB_RED = 0,
+   VIMC_DEB_GREEN = 1,
+   VIMC_DEB_BLUE = 2,
+};
+
+struct vimc_deb_pix_map {
+   u32 code;
+   enum vimc_deb_rgb_colors order[2][2];
+};
+
+struct vimc_deb_device {
+   struct vimc_ent_device ved;
+   struct v4l2_subdev sd;
+   /* The active format */
+   struct v4l2_mbus_framefmt sink_fmt;
+   u32 src_code;
+   void (*set_rgb_src)(struct vimc_deb_device *vdeb, unsigned int lin,
+ 

[PATCH v2 7/7] [media] vimc: sca: Add scaler

[Helen Koike]

Implement scaler and integrated with the core

Signed-off-by: Helen Koike 

---

Changes in v2:
[media] vimc: sca: Add scaler
- Add function MEDIA_ENT_F_IO_V4L
- remove v4l2_dev and dev
- s/sink_mbus_fmt/sink_fmt
- remove BUG_ON, remove redundant if else, rewrite TODO, check end of 
enum
- rm src_width/height, enum fsize with min and max values
- set/try fmt
- remove unecessary include freezer.h
- core: add bayer boolean in pixel table
- coding style
- fix bug in enum frame size
- check pad types on create
- return EBUSY when trying to set the format while stream is on
- remove vsd struct
- add IS_SRC and IS_SINK macros
- add sca_mult as a parameter of the module
- check set_fmt default parameters for quantization, colorspace ...
- add more dev_dbg


---
 drivers/media/platform/vimc/Makefile  |   3 +-
 drivers/media/platform/vimc/vimc-core.c   |  33 ++-
 drivers/media/platform/vimc/vimc-core.h   |   1 +
 drivers/media/platform/vimc/vimc-scaler.c | 426 ++
 drivers/media/platform/vimc/vimc-scaler.h |  28 ++
 5 files changed, 489 insertions(+), 2 deletions(-)
 create mode 100644 drivers/media/platform/vimc/vimc-scaler.c
 create mode 100644 drivers/media/platform/vimc/vimc-scaler.h

diff --git a/drivers/media/platform/vimc/Makefile 
b/drivers/media/platform/vimc/Makefile
index a6708f9..f13a594 100644
--- a/drivers/media/platform/vimc/Makefile
+++ b/drivers/media/platform/vimc/Makefile
@@ -1,3 +1,4 @@
-vimc-objs := vimc-core.o vimc-capture.o vimc-debayer.o vimc-sensor.o
+vimc-objs := vimc-core.o vimc-capture.o vimc-debayer.o vimc-scaler.o \
+   vimc-sensor.o
 
 obj-$(CONFIG_VIDEO_VIMC) += vimc.o
diff --git a/drivers/media/platform/vimc/vimc-core.c 
b/drivers/media/platform/vimc/vimc-core.c
index 51cbbf6..3a04db2 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -24,6 +24,7 @@
 #include "vimc-capture.h"
 #include "vimc-core.h"
 #include "vimc-debayer.h"
+#include "vimc-scaler.h"
 #include "vimc-sensor.h"
 
 #define VIMC_PDEV_NAME "vimc"
@@ -198,6 +199,10 @@ static const struct vimc_pipeline_config pipe_cfg = {
 
 /* -- 
*/
 
+/*
+ * NOTE: non-bayer formats need to come first (necessary for enum_mbus_code
+ * in the scaler)
+ */
 static const struct vimc_pix_map vimc_pix_map_list[] = {
/* TODO: add all missing formats */
 
@@ -206,16 +211,19 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
.code = MEDIA_BUS_FMT_BGR888_1X24,
.pixelformat = V4L2_PIX_FMT_BGR24,
.bpp = 3,
+   .bayer = false,
},
{
.code = MEDIA_BUS_FMT_RGB888_1X24,
.pixelformat = V4L2_PIX_FMT_RGB24,
.bpp = 3,
+   .bayer = false,
},
{
.code = MEDIA_BUS_FMT_ARGB_1X32,
.pixelformat = V4L2_PIX_FMT_ARGB32,
.bpp = 4,
+   .bayer = false,
},
 
/* Bayer formats */
@@ -223,41 +231,49 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
.code = MEDIA_BUS_FMT_SBGGR8_1X8,
.pixelformat = V4L2_PIX_FMT_SBGGR8,
.bpp = 1,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SGBRG8_1X8,
.pixelformat = V4L2_PIX_FMT_SGBRG8,
.bpp = 1,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SGRBG8_1X8,
.pixelformat = V4L2_PIX_FMT_SGRBG8,
.bpp = 1,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SRGGB8_1X8,
.pixelformat = V4L2_PIX_FMT_SRGGB8,
.bpp = 1,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SBGGR10_1X10,
.pixelformat = V4L2_PIX_FMT_SBGGR10,
.bpp = 2,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SGBRG10_1X10,
.pixelformat = V4L2_PIX_FMT_SGBRG10,
.bpp = 2,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SGRBG10_1X10,
.pixelformat = V4L2_PIX_FMT_SGRBG10,
.bpp = 2,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SRGGB10_1X10,
.pixelformat = V4L2_PIX_FMT_SRGGB10,
.bpp = 2,
+   .bayer = true,
},
 
/* 10bit raw bayer a-law compressed to 8 bits */
@@ -265,21 +281,25 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
.code = MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8,
.pixelformat = 

[PATCH v2 2/7] [media] vimc: Add vimc_ent_sd_* helper functions

[Helen Koike]

As all the subdevices in the topology will be initialized in the same
way, to avoid code repetition the vimc_ent_sd_{register, unregister}
helper functions were created

Signed-off-by: Helen Koike 

---

Changes in v2:
[media] vimc: Add vimc_ent_sd_* helper functions
- Comments in vimc_ent_sd_init
- Update vimc_ent_sd_init with upstream code as media_entity_pads_init
(instead of media_entity_init), entity->function intead of entity->type
- Add missing vimc_pads_cleanup in vimc_ent_sd_cleanup
- remove subdevice v4l2_dev and dev fields
- change unregister order in vimc_ent_sd_cleanup
- rename vimc_ent_sd_{init,cleanup} to vimc_ent_sd_{register,unregister}
- remove struct vimc_ent_subdevice, use ved and sd directly
- don't impose struct vimc_sen_device to declare ved and sd struct first
- add kernel docs


---
 drivers/media/platform/vimc/vimc-core.c   | 66 +++
 drivers/media/platform/vimc/vimc-core.h   | 39 ++
 drivers/media/platform/vimc/vimc-sensor.c | 58 +--
 3 files changed, 114 insertions(+), 49 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-core.c 
b/drivers/media/platform/vimc/vimc-core.c
index bc107da..15fa311 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -416,6 +416,72 @@ struct media_pad *vimc_pads_init(u16 num_pads, const 
unsigned long *pads_flag)
return pads;
 }
 
+static const struct media_entity_operations vimc_ent_sd_mops = {
+   .link_validate = v4l2_subdev_link_validate,
+};
+
+int vimc_ent_sd_register(struct vimc_ent_device *ved,
+struct v4l2_subdev *sd,
+struct v4l2_device *v4l2_dev,
+const char *const name,
+u32 function,
+u16 num_pads,
+const unsigned long *pads_flag,
+const struct v4l2_subdev_ops *sd_ops,
+void (*sd_destroy)(struct vimc_ent_device *))
+{
+   int ret;
+
+   /* Allocate the pads */
+   ved->pads = vimc_pads_init(num_pads, pads_flag);
+   if (IS_ERR(ved->pads))
+   return PTR_ERR(ved->pads);
+
+   /* Fill the vimc_ent_device struct */
+   ved->destroy = sd_destroy;
+   ved->ent = >entity;
+
+   /* Initialize the subdev */
+   v4l2_subdev_init(sd, sd_ops);
+   sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
+   sd->entity.ops = _ent_sd_mops;
+   sd->owner = THIS_MODULE;
+   strlcpy(sd->name, name, sizeof(sd->name));
+   v4l2_set_subdevdata(sd, ved);
+
+   /* Expose this subdev to user space */
+   sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+   /* Initialize the media entity */
+   ret = media_entity_pads_init(>entity, num_pads, ved->pads);
+   if (ret)
+   goto err_clean_pads;
+
+   /* Register the subdev with the v4l2 and the media framework */
+   ret = v4l2_device_register_subdev(v4l2_dev, sd);
+   if (ret) {
+   dev_err(v4l2_dev->dev,
+   "%s: subdev register failed (err=%d)\n",
+   name, ret);
+   goto err_clean_m_ent;
+   }
+
+   return 0;
+
+err_clean_m_ent:
+   media_entity_cleanup(>entity);
+err_clean_pads:
+   vimc_pads_cleanup(ved->pads);
+   return ret;
+}
+
+void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev 
*sd)
+{
+   v4l2_device_unregister_subdev(sd);
+   media_entity_cleanup(ved->ent);
+   vimc_pads_cleanup(ved->pads);
+}
+
 /*
  * TODO: remove this function when all the
  * entities specific code are implemented
diff --git a/drivers/media/platform/vimc/vimc-core.h 
b/drivers/media/platform/vimc/vimc-core.h
index 4525d23..92c4729 100644
--- a/drivers/media/platform/vimc/vimc-core.h
+++ b/drivers/media/platform/vimc/vimc-core.h
@@ -109,4 +109,43 @@ const struct vimc_pix_map *vimc_pix_map_by_code(u32 code);
  */
 const struct vimc_pix_map *vimc_pix_map_by_pixelformat(u32 pixelformat);
 
+/**
+ * vimc_ent_sd_register - initialize and register a subdev node
+ *
+ * @ved:   the vimc_ent_device struct to be initialize
+ * @sd:the v4l2_subdev struct to be initialize and registered
+ * @v4l2_dev:  the v4l2 device to register the v4l2_subdev
+ * @name:  name of the sub-device. Please notice that the name must be
+ * unique.
+ * @function:  media entity function defined by MEDIA_ENT_F_* macros
+ * @num_pads:  number of pads to initialize
+ * @pads_flag: flags to use in each pad
+ * @sd_ops:pointer to  v4l2_subdev_ops.
+ * @sd_destroy:callback to destroy the node
+ *
+ * Helper function initialize and register the struct vimc_ent_device and 
struct
+ * v4l2_subdev which represents a subdev node in the topology
+ */
+int vimc_ent_sd_register(struct 

[PATCH v2 0/7] [media]: vimc: Virtual Media Control VPU's

[Helen Koike]

This patch series improves the current video processing units in vimc
(by adding more controls to the sensor and capture node, allowing the
user to configure different frame formats) and also adds a debayer
and a scaler node.
The debayer transforms the bayer format image received in its sink pad
to a bayer format by averaging the pixels within a mean window.
The scaler only scales up the image for now.

This patch series depends on commit "[media] vimc: Virtual Media Controller 
core, capture and sensor"
and it is available at:
https://github.com/helen-fornazier/opw-staging/tree/z/sent/vimc/vpu/v2

Changes in v2:
[media] vimc: sen: Integrate the tpg on the sensor
- Fix include location
- Select V4L2_TPG in Kconfig
- configure tpg on streamon only
- rm BUG_ON
- coding style
[media] vimc: Add vimc_ent_sd_* helper functions
- Comments in vimc_ent_sd_init
- Update vimc_ent_sd_init with upstream code as media_entity_pads_init
(instead of media_entity_init), entity->function intead of entity->type
- Add missing vimc_pads_cleanup in vimc_ent_sd_cleanup
- remove subdevice v4l2_dev and dev fields
- change unregister order in vimc_ent_sd_cleanup
- rename vimc_ent_sd_{init,cleanup} to vimc_ent_sd_{register,unregister}
- remove struct vimc_ent_subdevice, use ved and sd directly
- don't impose struct vimc_sen_device to declare ved and sd struct first
- add kernel docs
[media] vimc: Add vimc_pipeline_s_stream in the core
- Use is_media_entity_v4l2_subdev instead of comparing with the old
entity->type
- Fix comments style
- add kernel-docs
- call s_stream across all sink pads
[media] vimc: sen: Support several image formats
- this is a new commit in the serie (the old one was splitted in two)
- add init_cfg to initialize try_fmt
- reorder code in vimc_sen_set_fmt
- allow user space to change all fields from struct v4l2_mbus_framefmt
  (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
- merge with patch for the enum_mbus_code and enum_frame_size
- change commit message
- add vimc_pix_map_by_index
- rename MIN/MAX macros
- check set_fmt default parameters for quantization, colorspace ...
[media] vimc: cap: Support several image formats
- this is a new commit in the serie (the old one was splitted in two)
- allow user space to change all fields from struct v4l2_pix_format
  (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
- link_validate and try_fmt: also checks colospace, quantization, 
field, xfer_func, ycbcr_enc
- add struct v4l2_pix_format fmt_default
- add enum_framesizes
- enum_fmt_vid_cap: enumerate all formats from vimc_pix_map_table
- add mode dev_dbg
[media] vimc: deb: Add debayer filter
- Using MEDIA_ENT_F_ATV_DECODER in function
- remove v4l2_dev and dev from vimc_deb_device struct
- src fmt propagates from the sink
- coding style
- remove redundant else if statements
- check end of enum and remove BUG_ON
- enum frame size with min and max values
- set/try fmt
- remove unecessary include freezer.h
- check pad types on create
- return EBUSY when trying to set the format while stream is on
- remove vsd struct
- add IS_SRC and IS_SINK macros
- add deb_mean_win_size as a parameter of the module
- check set_fmt default parameters for quantization, colorspace ...
- add more dev_dbg
[media] vimc: sca: Add scaler
- Add function MEDIA_ENT_F_IO_V4L
- remove v4l2_dev and dev
- s/sink_mbus_fmt/sink_fmt
- remove BUG_ON, remove redundant if else, rewrite TODO, check end of 
enum
- rm src_width/height, enum fsize with min and max values
- set/try fmt
- remove unecessary include freezer.h
- core: add bayer boolean in pixel table
- coding style
- fix bug in enum frame size
- check pad types on create
- return EBUSY when trying to set the format while stream is on
- remove vsd struct
- add IS_SRC and IS_SINK macros
- add sca_mult as a parameter of the module
- check set_fmt default parameters for quantization, colorspace ...
- add more dev_dbg

Helen Koike (7):
  [media] vimc: sen: Integrate the tpg on the sensor
  [media] vimc: Add vimc_ent_sd_* helper functions
  [media] vimc: Add vimc_pipeline_s_stream in the core
  [media] vimc: sen: Support several image formats
  [media] vimc: cap: Support several image formats
  [media] vimc: deb: Add debayer filter
  [media] vimc: sca: Add scaler

 drivers/media/platform/vimc/Kconfig|   1 +
 drivers/media/platform/vimc/Makefile   |   3 +-
 drivers/media/platform/vimc/vimc-capture.c | 196 +++---

[PATCH v2 1/7] [media] vimc: sen: Integrate the tpg on the sensor

[Helen Koike]

Initialize the test pattern generator on the sensor
Generate a colored bar image instead of a grey one

Signed-off-by: Helen Koike 

---

Changes in v2:
[media] vimc: sen: Integrate the tpg on the sensor
- Fix include location
- Select V4L2_TPG in Kconfig
- configure tpg on streamon only
- rm BUG_ON
- coding style


---
 drivers/media/platform/vimc/Kconfig   |  1 +
 drivers/media/platform/vimc/vimc-sensor.c | 43 +--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vimc/Kconfig 
b/drivers/media/platform/vimc/Kconfig
index dd285fa..df124d4 100644
--- a/drivers/media/platform/vimc/Kconfig
+++ b/drivers/media/platform/vimc/Kconfig
@@ -2,6 +2,7 @@ config VIDEO_VIMC
tristate "Virtual Media Controller Driver (VIMC)"
depends on VIDEO_DEV && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
select VIDEOBUF2_VMALLOC
+   select VIDEO_V4L2_TPG
default n
---help---
  Skeleton driver for Virtual Media Controller
diff --git a/drivers/media/platform/vimc/vimc-sensor.c 
b/drivers/media/platform/vimc/vimc-sensor.c
index 591f6a4..9154322 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -20,12 +20,16 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "vimc-sensor.h"
 
+#define VIMC_SEN_FRAME_MAX_WIDTH 4096
+
 struct vimc_sen_device {
struct vimc_ent_device ved;
struct v4l2_subdev sd;
+   struct tpg_data tpg;
struct task_struct *kthread_sen;
u8 *frame;
/* The active format */
@@ -84,6 +88,28 @@ static int vimc_sen_get_fmt(struct v4l2_subdev *sd,
return 0;
 }
 
+static void vimc_sen_tpg_s_format(struct vimc_sen_device *vsen)
+{
+   const struct vimc_pix_map *vpix =
+   vimc_pix_map_by_code(vsen->mbus_format.code);
+
+   tpg_reset_source(>tpg, vsen->mbus_format.width,
+vsen->mbus_format.height, vsen->mbus_format.field);
+   tpg_s_bytesperline(>tpg, 0,
+  vsen->mbus_format.width * vpix->bpp);
+   tpg_s_buf_height(>tpg, vsen->mbus_format.height);
+   tpg_s_fourcc(>tpg, vpix->pixelformat);
+   /* TODO: check why the tpg_s_field need this third argument if
+* it is already receiving the field
+*/
+   tpg_s_field(>tpg, vsen->mbus_format.field,
+   vsen->mbus_format.field == V4L2_FIELD_ALTERNATE);
+   tpg_s_colorspace(>tpg, vsen->mbus_format.colorspace);
+   tpg_s_ycbcr_enc(>tpg, vsen->mbus_format.ycbcr_enc);
+   tpg_s_quantization(>tpg, vsen->mbus_format.quantization);
+   tpg_s_xfer_func(>tpg, vsen->mbus_format.xfer_func);
+}
+
 static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
.enum_mbus_code = vimc_sen_enum_mbus_code,
.enum_frame_size= vimc_sen_enum_frame_size,
@@ -110,7 +136,7 @@ static int vimc_thread_sen(void *data)
if (kthread_should_stop())
break;
 
-   memset(vsen->frame, 100, vsen->frame_size);
+   tpg_fill_plane_buffer(>tpg, V4L2_STD_PAL, 0, vsen->frame);
 
/* Send the frame to all source pads */
for (i = 0; i < vsen->sd.entity.num_pads; i++)
@@ -159,6 +185,9 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int 
enable)
vsen->frame = NULL;
return PTR_ERR(vsen->kthread_sen);
}
+
+   /* configure the test pattern generator */
+   vimc_sen_tpg_s_format(vsen);
} else {
if (!vsen->kthread_sen)
return -EINVAL;
@@ -189,6 +218,7 @@ static void vimc_sen_destroy(struct vimc_ent_device *ved)
struct vimc_sen_device *vsen =
container_of(ved, struct vimc_sen_device, ved);
 
+   tpg_free(>tpg);
v4l2_device_unregister_subdev(>sd);
media_entity_cleanup(ved->ent);
kfree(vsen);
@@ -254,17 +284,26 @@ struct vimc_ent_device *vimc_sen_create(struct 
v4l2_device *v4l2_dev,
vsen->mbus_format.quantization = V4L2_QUANTIZATION_FULL_RANGE;
vsen->mbus_format.xfer_func = V4L2_XFER_FUNC_SRGB;
 
+   /* Initialize the test pattern generator */
+   tpg_init(>tpg, vsen->mbus_format.width,
+vsen->mbus_format.height);
+   ret = tpg_alloc(>tpg, VIMC_SEN_FRAME_MAX_WIDTH);
+   if (ret)
+   goto err_clean_m_ent;
+
/* Register the subdev with the v4l2 and the media framework */
ret = v4l2_device_register_subdev(v4l2_dev, >sd);
if (ret) {
dev_err(vsen->sd.v4l2_dev->dev,
"%s: subdev register failed (err=%d)\n",
vsen->sd.name, ret);
-   goto err_clean_m_ent;
+   goto err_free_tpg;
}
 
return >ved;
 

[PATCH v2 4/7] [media] vimc: sen: Support several image formats

[Helen Koike]

Allow user space to change the image format as the frame size, the
media bus pixel format, colorspace, quantization, field YCbCr encoding
and the transfer function

Signed-off-by: Helen Koike 

---

Changes in v2:
[media] vimc: sen: Support several image formats
- this is a new commit in the serie (the old one was splitted in two)
- add init_cfg to initialize try_fmt
- reorder code in vimc_sen_set_fmt
- allow user space to change all fields from struct v4l2_mbus_framefmt
  (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
- merge with patch for the enum_mbus_code and enum_frame_size
- change commit message
- add vimc_pix_map_by_index
- rename MIN/MAX macros
- check set_fmt default parameters for quantization, colorspace ...


---
 drivers/media/platform/vimc/vimc-core.c   |   8 ++
 drivers/media/platform/vimc/vimc-core.h   |  12 +++
 drivers/media/platform/vimc/vimc-sensor.c | 143 --
 3 files changed, 134 insertions(+), 29 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-core.c 
b/drivers/media/platform/vimc/vimc-core.c
index 7c23735..bc4b1bb 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -324,6 +324,14 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
},
 };
 
+const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i)
+{
+   if (i >= ARRAY_SIZE(vimc_pix_map_list))
+   return NULL;
+
+   return _pix_map_list[i];
+}
+
 const struct vimc_pix_map *vimc_pix_map_by_code(u32 code)
 {
unsigned int i;
diff --git a/drivers/media/platform/vimc/vimc-core.h 
b/drivers/media/platform/vimc/vimc-core.h
index 8c3d401..2146672 100644
--- a/drivers/media/platform/vimc/vimc-core.h
+++ b/drivers/media/platform/vimc/vimc-core.h
@@ -21,6 +21,11 @@
 #include 
 #include 
 
+#define VIMC_FRAME_MAX_WIDTH 4096
+#define VIMC_FRAME_MAX_HEIGHT 2160
+#define VIMC_FRAME_MIN_WIDTH 16
+#define VIMC_FRAME_MIN_HEIGHT 16
+
 /**
  * struct vimc_pix_map - maps media bus code with v4l2 pixel format
  *
@@ -107,6 +112,13 @@ static inline void vimc_pads_cleanup(struct media_pad 
*pads)
 int vimc_pipeline_s_stream(struct media_entity *ent, int enable);
 
 /**
+ * vimc_pix_map_by_index - get vimc_pix_map struct by its index
+ *
+ * @i: index of the vimc_pix_map struct in vimc_pix_map_list
+ */
+const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i);
+
+/**
  * vimc_pix_map_by_code - get vimc_pix_map struct by media bus code
  *
  * @code:  media bus format code defined by MEDIA_BUS_FMT_* macros
diff --git a/drivers/media/platform/vimc/vimc-sensor.c 
b/drivers/media/platform/vimc/vimc-sensor.c
index abb2172..c86b4e6 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -24,8 +24,6 @@
 
 #include "vimc-sensor.h"
 
-#define VIMC_SEN_FRAME_MAX_WIDTH 4096
-
 struct vimc_sen_device {
struct vimc_ent_device ved;
struct v4l2_subdev sd;
@@ -37,18 +35,41 @@ struct vimc_sen_device {
int frame_size;
 };
 
+static const struct v4l2_mbus_framefmt fmt_default = {
+   .width = 640,
+   .height = 480,
+   .code = MEDIA_BUS_FMT_RGB888_1X24,
+   .field = V4L2_FIELD_NONE,
+   .colorspace = V4L2_COLORSPACE_SRGB,
+   .quantization = V4L2_QUANTIZATION_FULL_RANGE,
+   .xfer_func = V4L2_XFER_FUNC_SRGB,
+};
+
+static int vimc_sen_init_cfg(struct v4l2_subdev *sd,
+struct v4l2_subdev_pad_config *cfg)
+{
+   unsigned int i;
+
+   for (i = 0; i < sd->entity.num_pads; i++) {
+   struct v4l2_mbus_framefmt *mf;
+
+   mf = v4l2_subdev_get_try_format(sd, cfg, i);
+   *mf = fmt_default;
+   }
+
+   return 0;
+}
+
 static int vimc_sen_enum_mbus_code(struct v4l2_subdev *sd,
   struct v4l2_subdev_pad_config *cfg,
   struct v4l2_subdev_mbus_code_enum *code)
 {
-   struct vimc_sen_device *vsen =
-   container_of(sd, struct vimc_sen_device, sd);
+   const struct vimc_pix_map *vpix = vimc_pix_map_by_index(code->index);
 
-   /* TODO: Add support for other codes */
-   if (code->index)
+   if (!vpix)
return -EINVAL;
 
-   code->code = vsen->mbus_format.code;
+   code->code = vpix->code;
 
return 0;
 }
@@ -57,33 +78,34 @@ static int vimc_sen_enum_frame_size(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_frame_size_enum *fse)
 {
-   struct vimc_sen_device *vsen =
-   container_of(sd, struct vimc_sen_device, sd);
+   const struct vimc_pix_map *vpix;
 
-   /* TODO: Add support to other formats */
if (fse->index)

[PATCH v2 3/7] [media] vimc: Add vimc_pipeline_s_stream in the core

[Helen Koike]

Move the vimc_cap_pipeline_s_stream from the vimc-cap.c to vimc-core.c
as this core will be reused by other subdevices to activate the stream
in their directly connected nodes

Signed-off-by: Helen Koike 

---

Changes in v2:
[media] vimc: Add vimc_pipeline_s_stream in the core
- Use is_media_entity_v4l2_subdev instead of comparing with the old
entity->type
- Fix comments style
- add kernel-docs
- call s_stream across all sink pads


---
 drivers/media/platform/vimc/vimc-capture.c | 29 ++-
 drivers/media/platform/vimc/vimc-core.c| 32 ++
 drivers/media/platform/vimc/vimc-core.h| 11 ++
 3 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c 
b/drivers/media/platform/vimc/vimc-capture.c
index 9adb06d..93f6a09 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -132,31 +132,6 @@ static void vimc_cap_return_all_buffers(struct 
vimc_cap_device *vcap,
spin_unlock(>qlock);
 }
 
-static int vimc_cap_pipeline_s_stream(struct vimc_cap_device *vcap, int enable)
-{
-   struct v4l2_subdev *sd;
-   struct media_pad *pad;
-   int ret;
-
-   /* Start the stream in the subdevice direct connected */
-   pad = media_entity_remote_pad(>vdev.entity.pads[0]);
-
-   /*
-* if it is a raw node from vimc-core, there is nothing to activate
-* TODO: remove this when there are no more raw nodes in the
-* core and return error instead
-*/
-   if (pad->entity->obj_type == MEDIA_ENTITY_TYPE_BASE)
-   return 0;
-
-   sd = media_entity_to_v4l2_subdev(pad->entity);
-   ret = v4l2_subdev_call(sd, video, s_stream, enable);
-   if (ret && ret != -ENOIOCTLCMD)
-   return ret;
-
-   return 0;
-}
-
 static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
@@ -173,7 +148,7 @@ static int vimc_cap_start_streaming(struct vb2_queue *vq, 
unsigned int count)
}
 
/* Enable streaming from the pipe */
-   ret = vimc_cap_pipeline_s_stream(vcap, 1);
+   ret = vimc_pipeline_s_stream(>vdev.entity, 1);
if (ret) {
media_pipeline_stop(entity);
vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
@@ -192,7 +167,7 @@ static void vimc_cap_stop_streaming(struct vb2_queue *vq)
struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
 
/* Disable streaming from the pipe */
-   vimc_cap_pipeline_s_stream(vcap, 0);
+   vimc_pipeline_s_stream(>vdev.entity, 0);
 
/* Stop the media pipeline */
media_pipeline_stop(>vdev.entity);
diff --git a/drivers/media/platform/vimc/vimc-core.c 
b/drivers/media/platform/vimc/vimc-core.c
index 15fa311..7c23735 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -396,6 +396,38 @@ static void vimc_device_unregister(struct vimc_device 
*vimc)
media_device_cleanup(>mdev);
 }
 
+int vimc_pipeline_s_stream(struct media_entity *ent, int enable)
+{
+   struct v4l2_subdev *sd;
+   struct media_pad *pad;
+   unsigned int i;
+   int ret;
+
+   for (i = 0; i < ent->num_pads; i++) {
+   if (ent->pads[i].flags & MEDIA_PAD_FL_SOURCE)
+   continue;
+
+   /* Start the stream in the subdevice direct connected */
+   pad = media_entity_remote_pad(>pads[i]);
+
+   /*
+* if this is a raw node from vimc-core, then there is
+* nothing to activate
+* TODO: remove this when there are no more raw nodes in the
+* core and return error instead
+*/
+   if (pad->entity->obj_type == MEDIA_ENTITY_TYPE_BASE)
+   continue;
+
+   sd = media_entity_to_v4l2_subdev(pad->entity);
+   ret = v4l2_subdev_call(sd, video, s_stream, enable);
+   if (ret && ret != -ENOIOCTLCMD)
+   return ret;
+   }
+
+   return 0;
+}
+
 /* Helper function to allocate and initialize pads */
 struct media_pad *vimc_pads_init(u16 num_pads, const unsigned long *pads_flag)
 {
diff --git a/drivers/media/platform/vimc/vimc-core.h 
b/drivers/media/platform/vimc/vimc-core.h
index 92c4729..8c3d401 100644
--- a/drivers/media/platform/vimc/vimc-core.h
+++ b/drivers/media/platform/vimc/vimc-core.h
@@ -96,6 +96,17 @@ static inline void vimc_pads_cleanup(struct media_pad *pads)
 }
 
 /**
+ * vimc_pipeline_s_stream - start stream through the pipeline
+ *
+ * @ent:   the pointer to struct media_entity for the node
+ * @enable:1 to start the stream and 0 to stop
+ *
+ * Helper function to call the s_stream of the subdevices connected
+ * in all 

Re: [PATCH v2 4/8] v4l: async: Provide interoperability between OF and fwnode matching

[Sakari Ailus]

Hi Laurent,

On Fri, Apr 07, 2017 at 01:07:48PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thursday 06 Apr 2017 16:12:06 Sakari Ailus wrote:
> > OF and fwnode support are separated in V4L2 and individual drivers may
> > implement one of them. Sub-devices do not match with a notifier
> > expecting sub-devices with fwnodes, nor the other way around.
> 
> Shouldn't we instead convert all drivers to fwnode matching ? What's missing 
> after the mass conversion in patch 5/8 ?

A lot of drivers use the OF frame work and thus do not deal with fwnodes
directly. I haven't entirely converted them to use the fwnode API since
making additional, unnecessary changes increases the likelihood of errors.

> 
> > Fix this by checking for sub-device's of_node field in fwnode match and
> > fwnode field in OF match.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/v4l2-async.c | 26 +++---
> >  include/media/v4l2-async.h   |  2 +-
> >  2 files changed, 24 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c
> > b/drivers/media/v4l2-core/v4l2-async.c index 384ad5e..7f5d804 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -14,6 +14,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -40,15 +41,34 @@ static bool match_devname(struct v4l2_subdev *sd,
> > return !strcmp(asd->match.device_name.name, dev_name(sd->dev));
> >  }
> > 
> > +static bool fwnode_cmp(struct fwnode_handle *one,
> > +  struct fwnode_handle *theother)
> > +{
> > +   if (!one || !theother)
> > +   return false;
> > +
> > +   if (one->type != theother->type)
> > +   return false;
> > +
> > +   if (is_of_node(one))
> > +   return !of_node_cmp(of_node_full_name(to_of_node(one)),
> > +   of_node_full_name(to_of_node(theother)));
> > +   else
> > +   return one == theother;
> > +}
> > +
> >  static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> > {
> > -   return !of_node_cmp(of_node_full_name(sd->of_node),
> > -   of_node_full_name(asd->match.of.node));
> > +   return fwnode_cmp(sd->of_node ?
> > + of_fwnode_handle(sd->of_node) : sd->fwnode,
> > + of_fwnode_handle(asd->match.of.node));
> >  }
> > 
> >  static bool match_fwnode(struct v4l2_subdev *sd, struct v4l2_async_subdev
> > *asd)
> >  {
> > -   return sd->fwnode == asd->match.fwnode.fwn;
> > +   return fwnode_cmp(sd->of_node ?
> > + of_fwnode_handle(sd->of_node) : sd->fwnode,
> > +  asd->match.fwnode.fwn);
> >  }
> > 
> >  static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev
> > *asd) diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > index 8f552d2..df8b682 100644
> > --- a/include/media/v4l2-async.h
> > +++ b/include/media/v4l2-async.h
> > @@ -57,7 +57,7 @@ struct v4l2_async_subdev {
> > enum v4l2_async_match_type match_type;
> > union {
> > struct {
> > -   const struct device_node *node;
> > +   struct device_node *node;
> 
> That seems to be a bit of a hack :-( I'd rather make everything const and 
> cast 
> to non-const pointers explicitly where the API requires us to. Or, better, 
> add 
> a to_of_node_const() function.

I'll see what I can do to the matter, but if you don't mind, I'll base it on
this patchset.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v2 3/8] v4l: async: Add fwnode match support

[Sakari Ailus]

On Fri, Apr 07, 2017 at 01:47:11PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Friday 07 Apr 2017 13:45:09 Sakari Ailus wrote:
> > On Fri, Apr 07, 2017 at 01:04:47PM +0300, Laurent Pinchart wrote:
> > > > @@ -58,6 +60,9 @@ struct v4l2_async_subdev {
> > > > const struct device_node *node;
> > > > } of;
> > > > struct {
> > > > +   struct fwnode_handle *fwn;
> > > 
> > > Shouldn't this be const ?
> > 
> > I thought the same, but a lot of functions that operate on fwnode_handle
> > take a non-const argument. I attempted changing that, but it starts a
> > cascade of unavoidable changes elsewhere. That's not very well suitable for
> > this patchset.
> 
> fwnode is young, we should try to fix it instead of propagating issues :-)

I'm not arguing this would be how I'd prefer things to be. Adding a
dependency to another kernel framework would mean postponing the set. This
can well be fixed later on as well.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v2 2/8] v4l: fwnode: Support generic fwnode for parsing standardised properties

[Sakari Ailus]

On Fri, Apr 07, 2017 at 01:36:34PM +0300, Sakari Ailus wrote:
...
> > > + if (is_of_node(fwn)) {
> > > + if (of_node_cmp(to_of_node(fwn)->name, "ports") == 0)
> > > + fwn = fwnode_get_next_parent(fwn);
> > > + } else {
> > > + /* The "ports" node is always there in ACPI. */

This comment is actually wrong and does not reflect the current
implementation anymore. I'll fix that as well.

> > > + fwn = fwnode_get_next_parent(fwn);
> > > + }

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

[PATCH v10] [media] vimc: Virtual Media Controller core, capture and sensor

[Helen Koike]

First version of the Virtual Media Controller.
Add a simple version of the core of the driver, the capture and
sensor nodes in the topology, generating a grey image in a hardcoded
format.

Signed-off-by: Helen Koike 
Reviewed-by: Hans Verkuil 

---

Patch based in media/master tree, and available here:
https://github.com/helen-fornazier/opw-staging/tree/z/sent/vimc/skel/v10

Changes since v9:
- reuturn -EPIPE instead of -EINVAL in link_validate when formats don't
match
- remove check for vb2_is_busy in vimc_cap_process_frame as it can cause
a dead lock with stop_streaming ioctl
- add vb2_ioctl_prepare_buf in vimc_cap_ioctl_ops
- add reviewd-by tag from Hans
- update MAINTAINERS file

Changes since v8:
- fix vimc_sen_enum_mbus_code: return EINVAL if code->index > 0
- remove input ioctls from vimc-capture.c as v4l2 intends to have default
input ioctls in the core
- change kernel-docs for vimc_pix_map_by_* so the line fits in 80
characters

Changes since v7:
- remove left over union in struct vimc_ent_device
- remove v4l2_dev pointer from vimc_sen_device structure
- remove unused dev parameter from vimc_propagate_frame()
- remove struct device *dev from struct vimc_cap_device and vimc_sen_device
- in vimc_sen_create: call media_entity_pads_init() after v4l2_subdev_init()
to avoid double initialization of vsen->sd.entity.name
- in vimc_sen_destroy: move media_entity_cleanup to be after 
v4l2_device_unregister_subdev
- rename video_device back to vdev instead of vd
- adjust copyright with range 2015-2017
- add node names in dev_err prints for vimc-capture.c and vimc-sensor.c
- remove prefix "cap" in dev_dbg as it already uses the name of the node

Changes since v6:
- add kernel-docs in vimc-core.h
- reorder list_del call in vimc_cap_return_all_buffers()
- call media_pipeline_stop in vimc_cap_start_streaming when fail
- remove DMA comment (left over from the sample driver)
- remove vb2_set_plane_payload call in vimc_cap_buffer_prepare
- remove format verification in vimc_cap_link_validate
- set vimc_pix_map_list as static
- use MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN in vimc_raw_create()
- register media device after creating the topology and unregister it before 
destroying the topology
- replace devm_kzalloc by kzalloc for allocating struct vimc_device
- uset TASK_UNINTERRUPTIBLE for vimc_thread_sen
- do not allow the creation of a sensor with no pads
- add more verbose description in Kconfig
- change copyright to 2017
- arrange includes: number before letters
- remove v4l2_dev pointer from vimc_cap_device structure
- coding style adjustments
- remove entity variable in vimc_cap_pipeline_s_stream
- declare and assign variables in vimc_cap_link_validate
- declare vimc_dev_release() and vimc_pdev closer to vimc_init()
- remove pad check in vimc_sen_enum_{mbus_code,frame_size} that is already 
performed by v4l2-subdev.c
- fix multiline comments to start with /*
- reorder variable declaration in functions
- remove pad and subdevice type check in vimc_cap_pipeline_s_stream
- add a note that sensor nodes can can have more then one source pad
- remove the use of entity->use_count in the core and attach the ved structure 
in sd or vd, use
  ent->obj_type to know which structure (sd or vd) to use
- rename vdev (video_device) to vd to be similar to sd (subdev)

Changes since v5:
- Fix message "Entity type for entity Sensor A was not initialized!"
  by initializing the sensor entity.function after the calling
  v4l2_subded_init
- populate device_caps in vimc_cap_create instead of in
  vimc_cap_querycap
- Fix typo in vimc-core.c s/de/the

Changes since v4:
- coding style fixes
- remove BUG_ON
- change copyright to 2016
- depens on VIDEO_V4L2_SUBDEV_API instead of select
- remove assignement of V4L2_CAP_DEVICE_CAPS
- s/vimc_cap_g_fmt_vid_cap/vimc_cap_fmt_vid_cap
- fix vimc_cap_queue_setup declaration type
- remove wrong buffer size check and add it in vimc_cap_buffer_prepare
- vimc_cap_create: remove unecessary check if v4l2_dev or v4l2_dev->dev is null
- vimc_cap_create: only allow a single pad
- vimc_sen_create: only allow source pads, remove unecessary source pads
checks in vimc_thread_sen

Changes since v3: fix rmmod crash and built-in compile
- Re-order unregister calls in vimc_device_unregister function (remove
rmmod issue)
- Call media_device_unregister_entity in vimc_raw_destroy
- Add depends on VIDEO_DEV && VIDEO_V4L2 and select VIDEOBUF2_VMALLOC
- Check *nplanes in queue_setup (this remove v4l2-compliance fail)
- Include  in vimc-sensor.c
- Move include of  from vimc-core.c to vimc-core.h
- Generate 60 frames per sec instead of 1 in the sensor

Changes since v2: update with current media master tree
- Add struct media_pipeline in vimc_cap_device
- Use vb2_v4l2_buffer instead of vb2_buffer
- Typos
- Remove usage of entity->type and use entity->function instead
- Remove fmt argument from queue setup
- Use ktime_get_ns instead of v4l2_get_timestamp
- Iterate over link's 

Re: [PATCH] [media] media-entity: only call dev_dbg_obj if mdev is not NULL

[Helen Koike]

Hi Sakari,

On 2017-04-07 04:40 AM, Sakari Ailus wrote:

Hi Helen,

On Thu, Apr 06, 2017 at 04:32:00PM -0300, Helen Koike wrote:

Fix kernel Oops NULL pointer deference
Call dev_dbg_obj only after checking if gobj->mdev is not NULL

Signed-off-by: Helen Koike 
---
 drivers/media/media-entity.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 5640ca2..bc44193 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -199,12 +199,12 @@ void media_gobj_create(struct media_device *mdev,

 void media_gobj_destroy(struct media_gobj *gobj)
 {
-   dev_dbg_obj(__func__, gobj);
-
/* Do nothing if the object is not linked. */
if (gobj->mdev == NULL)
return;

+   dev_dbg_obj(__func__, gobj);
+
gobj->mdev->topology_version++;

/* Remove the object from mdev list */


Where is media_gobj_destroy() called with an object with NULL mdev?

I do not object to the change, but would like to know because I don't think
it's supposed to happen.



This happens when media_device_unregister(mdev) is called before 
unregistering the subdevices v4l2_device_unregister_subdev(sd) (which 
should be possible).


v4l2_device_unregister_subdev(sd) ends up calling v4l2_device_release() 
that calls media_device_unregister_entity() again (previously called by 
media_device_unregister(mdev)


Helen



There are issues though, until the patches fixing object referencing are
finished and merged. Unfortunately I haven't been able to work on those
recently, will pick them up again soon...

RE: [PATCH v3] usb: document that URB transfer_buffer should be aligned

[David Laight]

From: Mauro Carvalho Chehab
> Sent: 05 April 2017 14:53
> Several host controllers, commonly found on ARM, like dwc2,
> require buffers that are CPU-word aligned for they to work.
> 
> Failing to do that will cause buffer overflows at the caller
> drivers, with could cause data corruption.
> 
> Such data corruption was found, in practice, with the uvcdriver.
> 
> Document it.

How does this in any way solve the problem.

Ethernet frames will be misaligned, however hard it may be
the usb drivers will have to handle it.

David

Re: [PATCH 1/3] staging: atomisp: remove enable_isp_irq function and add disable_isp_irq

[Alan Cox]

On Fri, 2017-04-07 at 14:56 +0900, Daeseok Youn wrote:
> Enable/Disable ISP irq is switched with "enable" parameter of
> enable_isp_irq(). It would be better splited to two such as
> enable_isp_irq()/disable_isp_irq().
> 
> But the enable_isp_irq() is no use in atomisp_cmd.c file.
> So remove the enable_isp_irq() function and add
> disable_isp_irq function only.

All 3 added to my tree - thanks

Alan

Re: [PATCH v2 2/8] v4l: fwnode: Support generic fwnode for parsing standardised properties

[Sakari Ailus]

Hi Laurent,

On Fri, Apr 07, 2017 at 01:54:58PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Friday 07 Apr 2017 13:36:34 Sakari Ailus wrote:
> > On Fri, Apr 07, 2017 at 12:44:27PM +0300, Laurent Pinchart wrote:
> > > On Thursday 06 Apr 2017 16:12:04 Sakari Ailus wrote:
> > > > The fwnode_handle is a more generic way than OF device_node to describe
> > > > firmware nodes. Instead of the OF API, use more generic fwnode API to
> > > > obtain the same information.
> > > 
> > > I would mention that this is a copy of v4l2-of.c with the OF API replaced
> > > with the fwnode API.
> > 
> > I'll add that to the description.
> > 
> > > > As the V4L2 fwnode support will be required by a small minority of e.g.
> > > > ACPI based systems (the same might actually go for OF), make this a
> > > > module instead of embedding it in the videodev module.
> > > > 
> > > > Signed-off-by: Sakari Ailus 
> > > > ---
> > > > 
> > > >  drivers/media/v4l2-core/Kconfig   |   3 +
> > > >  drivers/media/v4l2-core/Makefile  |   1 +
> > > >  drivers/media/v4l2-core/v4l2-fwnode.c | 353 +++
> > > >  include/media/v4l2-fwnode.h   | 104 ++
> > > >  4 files changed, 461 insertions(+)
> > > >  create mode 100644 drivers/media/v4l2-core/v4l2-fwnode.c
> > > >  create mode 100644 include/media/v4l2-fwnode.h
> 
> [snip]
> 
> > > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > b/drivers/media/v4l2-core/v4l2-fwnode.c new file mode 100644
> > > > index 000..4f69b11
> > > > --- /dev/null
> > > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > > @@ -0,0 +1,353 @@
> > > > +/*
> > > > + * V4L2 fwnode binding parsing library
> > > > + *
> > > > + * Copyright (c) 2016 Intel Corporation.
> > > > + * Author: Sakari Ailus 
> > > > + *
> > > > + * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
> > > > + * Author: Sylwester Nawrocki 
> > > > + *
> > > > + * Copyright (C) 2012 Renesas Electronics Corp.
> > > > + * Author: Guennadi Liakhovetski 
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of version 2 of the GNU General Public License as
> > > > + * published by the Free Software Foundation.
> > > > + */
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +#include 
> > > > +
> > > > +static int v4l2_fwnode_endpoint_parse_csi_bus(struct fwnode_handle
> > > > *fwn,
> > > > + struct 
> > > > v4l2_fwnode_endpoint
> > > > *vfwn)
> > > > +{
> > > > +   struct v4l2_fwnode_bus_mipi_csi2 *bus = >bus.mipi_csi2;
> > > > +   bool have_clk_lane = false;
> > > > +   unsigned int flags = 0, lanes_used = 0;
> > > > +   unsigned int i;
> > > > +   u32 v;
> > > > +   int rval;
> > > 
> > > I would have used "ret" instead of "rval" ;-)
> > 
> > I know. But
> > 
> > 1) there's no established convention in the file and
> > 
> > 2) "rval" has the benefit is easier to look up; one doesn't find a plethora
> > of "return something". Therefore it is better than "ret" for the purpose.
> 
> The solution to that is
> 
> /ret\>
> 
> (and, of course, switching to vim :-D)

What's "\>" for?

I don't think you can convince people to switch to vim this way. :-)

> 
> [snip]
> 
> > > > +/*
> > > > + * v4l2_fwnode_endpoint_free() - free the V4L2 fwnode acquired by
> > > > + * v4l2_fwnode_endpoint_alloc_parse()
> > > > + * @fwn - the V4L2 fwnode the resources of which are to be released
> > > 
> > > Mayeb "the V4L2 fwnode whose resources are to be released" ?
> > > 
> > > > + *
> > > > + * It is safe to call this function with NULL argument or on an
> > > 
> > > s/on an/on a/
> > 
> > Yes.
> > 
> > > > + * V4L2 fwnode the parsing of which failed.
> > > 
> > > "whose parsing failed" ?
> > 
> > Any particular reason? Do you like "whose"? :-)
> 
> "of which" sounds dubious in this context, but please consult a native 
> English 
> speaker in case of doubt.

"Whose" is the possessive form of "who". Albeit nowadays it could probably
be used for other purposes as well.

In my opinion "of which" is perfectly appropriate language here.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

[PATCH] add blank line after declarations

[Manny Vindiola]

Add blank line after variable declarations as part of checkpatch.pl style fixup.

Signed-off-by: Manny Vindiola 
---
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
index 996d1bd..48b9604 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
@@ -56,6 +56,7 @@ static int tpg_set_fmt(struct v4l2_subdev *sd,
   struct v4l2_subdev_format *format)
 {
struct v4l2_mbus_framefmt *fmt = >format;
+
if (format->pad)
return -EINVAL;
/* only raw8 grbg is supported by TPG */
-- 
2.7.4

Re: [RFC v2 07/11] vb2: dma-contig: Remove redundant sgt_base field

[Sakari Ailus]

Hi Shuah,

On Mon, Mar 27, 2017 at 04:51:40PM -0600, Shuah Khan wrote:
> On Thu, Dec 15, 2016 at 6:24 PM, Laurent Pinchart
>  wrote:
> > From: Sakari Ailus 
> >
> > The struct vb2_dc_buf contains two struct sg_table fields: sgt_base and
> > dma_sgt. The former is used by DMA-BUF buffers whereas the latter is used
> > by USERPTR.
> >
> > Unify the two, leaving dma_sgt.
> 
> I think this patch should be split in two.
> 
> 1. Unifying dma_sgt and sgt_base
> 
> >
> > MMAP buffers do not need cache flushing since they have been allocated
> > using dma_alloc_coherent().
> 
> 2. That uses vec to check for checking for no flush needed condition.

I can split this, sure. A non-NULL vec indicates a USERPTR buffer. Before
this patch, non-NULL buf->dma_sgt did the same.

> 
> >
> > Signed-off-by: Sakari Ailus 
> > ---
> > Changes since v1:
> >
> > - Test for MMAP or DMABUF type through the vec field instead of the now
> >   gone vma field.
> 
> What is this gone vma field? Did I miss a patch in the series that
> makes this change? This check that is changed used dma_sgt and
> db_attach vma
> 

The field existed on bc0195aad0daa2ad5b0d76cce22b167bc3435590, i.e. v4.2-rc2
from which the earlier version of this patch was rebased from.

> These comments don't agree with the code change.
> 
> > - Move the vec field to a USERPTR section in struct vb2_dc_buf, where
> >   the vma field was located.
> > ---
> >  drivers/media/v4l2-core/videobuf2-dma-contig.c | 25 
> > +
> >  1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
> > b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > index fb6a177be461..2a00d12ffee2 100644
> > --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > @@ -30,12 +30,13 @@ struct vb2_dc_buf {
> > unsigned long   attrs;
> > enum dma_data_direction dma_dir;
> > struct sg_table *dma_sgt;
> > -   struct frame_vector *vec;
> >
> > /* MMAP related */
> > struct vb2_vmarea_handler   handler;
> > atomic_trefcount;
> > -   struct sg_table *sgt_base;
> > +
> > +   /* USERPTR related */
> > +   struct frame_vector *vec;
> >
> > /* DMABUF related */
> > struct dma_buf_attachment   *db_attach;
> > @@ -95,7 +96,7 @@ static void vb2_dc_prepare(void *buf_priv)
> > struct sg_table *sgt = buf->dma_sgt;
> >
> > /* DMABUF exporter will flush the cache for us */
> > -   if (!sgt || buf->db_attach)
> > +   if (!buf->vec)
> > return;
> 
> With the unification dma_sgt is valid for MMAP buffers after 
> vb2_dma_sg_alloc()
> if dma_sgt is not null, sync happens - the patch description doesn't seem to 
> be
> in sync with the change.

I'm not sure what you're referring to. The condition for sync is changed to
use buf->vec instead, i.e. the functionality is not affected.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [Patch v4 12/12] Documention: v4l: Documentation for HEVC CIDs

[Smitha T Murthy]

On Fri, 2017-04-07 at 12:26 +0200, Sylwester Nawrocki wrote:
> On 04/07/2017 12:03 PM, Smitha T Murthy wrote:
> >>> +``V4L2_CID_MPEG_VIDEO_HEVC_LF``
> >>> +Indicates loop filtering. Control ID 0 indicates loop filtering
> >>> +is enabled and when set to 1 indicates no filter.
>  >>
> >> "Setting this control to 0 enables loop filtering, setting this control
> >> to 1 disables loop filtering." ?
> >>
> >> Couldn't the meaning be inverted, so setting the control to 0 disables
> >> the loop filtering?
> >>
> >>From register point of view, this control value needs be 0 to enable
> > loop filtering.
> 
> OK, this is true for our specific hardware/firmware implementation.
> 
> In general, for this user space interface I would rather define that
> boolean control so value 1 enables LF and value 0 disables LF.
> The driver could simply negate the value when writing registers.
> 
> BTW we might need to specify type of the control here as Hans suggested
> commenting on other control.
> 
> --
> Regards,
> Sylwester
> 
Ok I will change the implementation from the driver for this control.
Yes in the next version I will add the type of the control for all.

Thank you for the review.

Regards,
Smitha

Re: [PATCHv6 03/10] exynos_hdmi: add CEC notifier support

[Andrzej Hajda]

On 31.03.2017 14:20, Hans Verkuil wrote:
> From: Hans Verkuil 
>
> Implement the CEC notifier support to allow CEC drivers to
> be informed when there is a new physical address.
>
> Signed-off-by: Hans Verkuil 
> Tested-by: Marek Szyprowski 
> Acked-by: Daniel Vetter 
> Acked-by: Krzysztof Kozlowski 

Reviewed-by: Andrzej Hajda 

 --
Regards
Andrzej

> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 19 +--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 88ccc0469316..bc4c8d0a66f4 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -43,6 +43,8 @@
>  
>  #include 
>  
> +#include 
> +
>  #include "exynos_drm_drv.h"
>  #include "exynos_drm_crtc.h"
>  
> @@ -119,6 +121,7 @@ struct hdmi_context {
>   booldvi_mode;
>   struct delayed_work hotplug_work;
>   struct drm_display_mode current_mode;
> + struct cec_notifier *notifier;
>   const struct hdmi_driver_data   *drv_data;
>  
>   void __iomem*regs;
> @@ -822,6 +825,7 @@ static enum drm_connector_status hdmi_detect(struct 
> drm_connector *connector,
>   if (gpiod_get_value(hdata->hpd_gpio))
>   return connector_status_connected;
>  
> + cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
>   return connector_status_disconnected;
>  }
>  
> @@ -860,6 +864,7 @@ static int hdmi_get_modes(struct drm_connector *connector)
>   edid->width_cm, edid->height_cm);
>  
>   drm_mode_connector_update_edid_property(connector, edid);
> + cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);
>  
>   ret = drm_add_edid_modes(connector, edid);
>  
> @@ -1503,6 +1508,7 @@ static void hdmi_disable(struct drm_encoder *encoder)
>   if (funcs && funcs->disable)
>   (*funcs->disable)(crtc);
>  
> + cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
>   cancel_delayed_work(>hotplug_work);
>  
>   hdmiphy_disable(hdata);
> @@ -1878,15 +1884,22 @@ static int hdmi_probe(struct platform_device *pdev)
>   }
>   }
>  
> + hdata->notifier = cec_notifier_get(>dev);
> + if (hdata->notifier == NULL) {
> + ret = -ENOMEM;
> + goto err_hdmiphy;
> + }
> +
>   pm_runtime_enable(dev);
>  
>   ret = component_add(>dev, _component_ops);
>   if (ret)
> - goto err_disable_pm_runtime;
> + goto err_notifier_put;
>  
>   return ret;
>  
> -err_disable_pm_runtime:
> +err_notifier_put:
> + cec_notifier_put(hdata->notifier);
>   pm_runtime_disable(dev);
>  
>  err_hdmiphy:
> @@ -1905,9 +1918,11 @@ static int hdmi_remove(struct platform_device *pdev)
>   struct hdmi_context *hdata = platform_get_drvdata(pdev);
>  
>   cancel_delayed_work_sync(>hotplug_work);
> + cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
>  
>   component_del(>dev, _component_ops);
>  
> + cec_notifier_put(hdata->notifier);
>   pm_runtime_disable(>dev);
>  
>   if (!IS_ERR(hdata->reg_hdmi_en))

Re: [RFC, v2, 10/11] vb2: dma-contig: Let drivers decide DMA attrs of MMAP and USERPTR bufs

[Laurent Pinchart]

Hi Ricky,

On Monday 26 Dec 2016 15:58:07 Ricky Liang wrote:
> On Fri, Dec 16, 2016 at 9:24 AM, Laurent Pinchart wrote:
> > From: Sakari Ailus 
> > 
> > The desirable DMA attributes are not generic for all devices using
> > Videobuf2 contiguous DMA ops. Let the drivers decide.
> > 
> > This change also results in MMAP buffers always having an sg_table
> > (dma_sgt field).
> > 
> > Also arrange the header files alphabetically.
> > 
> > As a result, also the DMA-BUF exporter must provide ops for synchronising
> > the cache. This adds begin_cpu_access and end_cpu_access ops to
> > vb2_dc_dmabuf_ops.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> > 
> >  drivers/media/v4l2-core/videobuf2-dma-contig.c | 66 +
> >  1 file changed, 56 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > b/drivers/media/v4l2-core/videobuf2-dma-contig.c index
> > d503647ea522..a0e88ad93f07 100644
> > --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c

[snip]

> > @@ -115,8 +115,11 @@ static void vb2_dc_prepare(void *buf_priv)
> > struct vb2_dc_buf *buf = buf_priv;
> > struct sg_table *sgt = buf->dma_sgt;
> > 
> > -   /* DMABUF exporter will flush the cache for us */
> > -   if (!buf->vec)
> > +   /*
> > +* DMABUF exporter will flush the cache for us; only USERPTR
> > +* and MMAP buffers with non-coherent memory will be flushed.
> > +*/
> > +   if (!(buf->attrs & DMA_ATTR_NON_CONSISTENT))
> 
> Should here be "if (!buf->vec || !(buf->attrs & DMA_ATTR_NON_CONSISTENT))" ?

I don't think so. buf->vec indicates that the buffer is using USERPTR. The 
check would thus return immediately for everything that is not USERPTR. What 
we want to do is return for DMABUF, and for MMAP and USERPTR buffers that 
don't have the DMA_ATTR_NON_CONSISTENT attribute set. As DMABUF buffers never 
have that attribute set (because attrs is set in vb2_dc_alloc, which is not 
called for DMABUF buffers), we can check the flag only.

> > return;
> > 
> > dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 5/8] v4l: Switch from V4L2 OF not V4L2 fwnode API

[Laurent Pinchart]

Hi Sakari,

On Friday 07 Apr 2017 13:58:06 Sakari Ailus wrote:
> On Fri, Apr 07, 2017 at 01:32:54PM +0300, Laurent Pinchart wrote:
> > On Thursday 06 Apr 2017 16:12:07 Sakari Ailus wrote:
> > > Switch users of the v4l2_of_ APIs to the more generic v4l2_fwnode_ APIs.
> > > 
> > > Existing OF matching continues to be supported. omap3isp and smiapp
> > > drivers are converted to fwnode matching as well.
> > > 
> > > Signed-off-by: Sakari Ailus 
> > > Acked-by: Benoit Parrot  # i2c/ov2569.c,
> > > am437x/am437x-vpfe.c and ti-vpe/cal.c ---
> > > 
> > >  drivers/media/i2c/Kconfig  |  9 
> > >  drivers/media/i2c/adv7604.c|  7 +--
> > >  drivers/media/i2c/mt9v032.c|  7 +--
> > >  drivers/media/i2c/ov2659.c |  8 +--
> > >  drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  7 +--
> > >  drivers/media/i2c/s5k5baf.c|  6 +--
> > >  drivers/media/i2c/smiapp/Kconfig   |  1 +
> > >  drivers/media/i2c/smiapp/smiapp-core.c | 29 ++-
> > >  drivers/media/i2c/tc358743.c   | 11 ++--
> > >  drivers/media/i2c/tvp514x.c|  6 +--
> > >  drivers/media/i2c/tvp5150.c|  7 +--
> > >  drivers/media/i2c/tvp7002.c|  6 +--
> > >  drivers/media/platform/Kconfig |  3 ++
> > >  drivers/media/platform/am437x/Kconfig  |  1 +
> > >  drivers/media/platform/am437x/am437x-vpfe.c|  8 +--
> > >  drivers/media/platform/atmel/Kconfig   |  1 +
> > >  drivers/media/platform/atmel/atmel-isc.c   |  8 +--
> > >  drivers/media/platform/exynos4-is/Kconfig  |  2 +
> > >  drivers/media/platform/exynos4-is/media-dev.c  |  6 +--
> > >  drivers/media/platform/exynos4-is/mipi-csis.c  |  6 +--
> > >  drivers/media/platform/omap3isp/isp.c  | 71  +-
> > >  drivers/media/platform/pxa_camera.c|  7 +--
> > >  drivers/media/platform/rcar-vin/Kconfig|  1 +
> > >  drivers/media/platform/rcar-vin/rcar-core.c|  6 +--
> > >  drivers/media/platform/soc_camera/Kconfig  |  1 +
> > >  drivers/media/platform/soc_camera/atmel-isi.c  |  7 +--
> > >  drivers/media/platform/soc_camera/soc_camera.c |  3 +-
> > >  drivers/media/platform/ti-vpe/cal.c| 11 ++--
> > >  drivers/media/platform/xilinx/Kconfig  |  1 +
> > >  drivers/media/platform/xilinx/xilinx-vipp.c| 59  +-
> > >  include/media/v4l2-fwnode.h|  4 +-
> > >  31 files changed, 176 insertions(+), 134 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > index cee1dae..6b2423a 100644
> > > --- a/drivers/media/i2c/Kconfig
> > > +++ b/drivers/media/i2c/Kconfig
> > > @@ -210,6 +210,7 @@ config VIDEO_ADV7604
> > > 
> > >   depends on GPIOLIB || COMPILE_TEST
> > >   select HDMI
> > >   select MEDIA_CEC_EDID
> > > 
> > > + select V4L2_FWNODE
> > 
> > What happens when building the driver on a platform that includes neither
> > OF nor ACPI support ?
> 
> You need either in practice, also for the V4L2 fwnode to be meaningful.
> 
> Do you have something in particular in mind?

I will obviously need either OF or ACPI to use the fwnode API, but some 
drivers still support platform data (either on non-OF embedded systems, or 
when the I2C device is part of a PCI card for instance). Compile-testing is 
also a use case I'm concerned about.

[snip]

> > > diff --git a/drivers/media/platform/omap3isp/isp.c
> > > b/drivers/media/platform/omap3isp/isp.c index 084ecf4a..95850b9 100644
> > > --- a/drivers/media/platform/omap3isp/isp.c
> > > +++ b/drivers/media/platform/omap3isp/isp.c
> > 
> > [snip]
> > 
> > > @@ -2024,43 +2025,42 @@ enum isp_of_phy {
> > >   ISP_OF_PHY_CSIPHY2,
> > >  };
> > > 
> > > -static int isp_of_parse_node(struct device *dev, struct device_node
> > > *node,
> > > -  struct isp_async_subdev *isd)
> > > +static int isp_fwnode_parse(struct device *dev, struct fwnode_handle
> > > *fwn,
> > > + struct isp_async_subdev *isd)
> > >  {
> > >   struct isp_bus_cfg *buscfg = >bus;
> > > - struct v4l2_of_endpoint vep;
> > > + struct v4l2_fwnode_endpoint vfwn;
> > 
> > vfwn is confusing to me, I think the variable name should show that it
> > refers to an endpoint.
> 
> How about adding ep to tell it's an endpoint?

I'd name is vep or endpoint.

> > >   unsigned int i;
> > >   int ret;
> > > 
> > > - ret = v4l2_of_parse_endpoint(node, );
> > > + ret = v4l2_fwnode_endpoint_parse(fwn, );
> > >   if (ret)
> > >   return ret;
> > > 
> > > - dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
> > > - vep.base.port);
> > > + dev_dbg(dev, "interface %u\n", vfwn.base.port);
> > 
> > Is there no way to keep the node name in the error message ?
> 
> There's no generic fwnode means to do something similar currently, possibly
> because 

Re: [PATCH v2 5/8] v4l: Switch from V4L2 OF not V4L2 fwnode API

[Sakari Ailus]

Hi Laurent,

On Fri, Apr 07, 2017 at 01:32:54PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thursday 06 Apr 2017 16:12:07 Sakari Ailus wrote:
> > Switch users of the v4l2_of_ APIs to the more generic v4l2_fwnode_ APIs.
> > 
> > Existing OF matching continues to be supported. omap3isp and smiapp
> > drivers are converted to fwnode matching as well.
> > 
> > Signed-off-by: Sakari Ailus 
> > Acked-by: Benoit Parrot  # i2c/ov2569.c,
> > am437x/am437x-vpfe.c and ti-vpe/cal.c ---
> >  drivers/media/i2c/Kconfig  |  9 
> >  drivers/media/i2c/adv7604.c|  7 +--
> >  drivers/media/i2c/mt9v032.c|  7 +--
> >  drivers/media/i2c/ov2659.c |  8 +--
> >  drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  7 +--
> >  drivers/media/i2c/s5k5baf.c|  6 +--
> >  drivers/media/i2c/smiapp/Kconfig   |  1 +
> >  drivers/media/i2c/smiapp/smiapp-core.c | 29 ++-
> >  drivers/media/i2c/tc358743.c   | 11 ++--
> >  drivers/media/i2c/tvp514x.c|  6 +--
> >  drivers/media/i2c/tvp5150.c|  7 +--
> >  drivers/media/i2c/tvp7002.c|  6 +--
> >  drivers/media/platform/Kconfig |  3 ++
> >  drivers/media/platform/am437x/Kconfig  |  1 +
> >  drivers/media/platform/am437x/am437x-vpfe.c|  8 +--
> >  drivers/media/platform/atmel/Kconfig   |  1 +
> >  drivers/media/platform/atmel/atmel-isc.c   |  8 +--
> >  drivers/media/platform/exynos4-is/Kconfig  |  2 +
> >  drivers/media/platform/exynos4-is/media-dev.c  |  6 +--
> >  drivers/media/platform/exynos4-is/mipi-csis.c  |  6 +--
> >  drivers/media/platform/omap3isp/isp.c  | 71 +++---
> >  drivers/media/platform/pxa_camera.c|  7 +--
> >  drivers/media/platform/rcar-vin/Kconfig|  1 +
> >  drivers/media/platform/rcar-vin/rcar-core.c|  6 +--
> >  drivers/media/platform/soc_camera/Kconfig  |  1 +
> >  drivers/media/platform/soc_camera/atmel-isi.c  |  7 +--
> >  drivers/media/platform/soc_camera/soc_camera.c |  3 +-
> >  drivers/media/platform/ti-vpe/cal.c| 11 ++--
> >  drivers/media/platform/xilinx/Kconfig  |  1 +
> >  drivers/media/platform/xilinx/xilinx-vipp.c| 59 +++--
> >  include/media/v4l2-fwnode.h|  4 +-
> >  31 files changed, 176 insertions(+), 134 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index cee1dae..6b2423a 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -210,6 +210,7 @@ config VIDEO_ADV7604
> > depends on GPIOLIB || COMPILE_TEST
> > select HDMI
> > select MEDIA_CEC_EDID
> > +   select V4L2_FWNODE
> 
> What happens when building the driver on a platform that includes neither OF 
> nor ACPI support ?

You need either in practice, also for the V4L2 fwnode to be meaningful.

Do you have something in particular in mind?

> 
> > ---help---
> >   Support for the Analog Devices ADV7604 video decoder.
> > 
> 
> [snip]
> 
> How have you checked that you haven't missed any entry in the Kconfig files ?

I made one Kconfig change per driver. :-)

> 
> [snip]
> 
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c index 084ecf4a..95850b9 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> 
> [snip]
> 
> > @@ -2024,43 +2025,42 @@ enum isp_of_phy {
> > ISP_OF_PHY_CSIPHY2,
> >  };
> > 
> > -static int isp_of_parse_node(struct device *dev, struct device_node *node,
> > -struct isp_async_subdev *isd)
> > +static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn,
> > +   struct isp_async_subdev *isd)
> >  {
> > struct isp_bus_cfg *buscfg = >bus;
> > -   struct v4l2_of_endpoint vep;
> > +   struct v4l2_fwnode_endpoint vfwn;
> 
> vfwn is confusing to me, I think the variable name should show that it refers 
> to an endpoint.

How about adding ep to tell it's an endpoint?

> 
> > unsigned int i;
> > int ret;
> > 
> > -   ret = v4l2_of_parse_endpoint(node, );
> > +   ret = v4l2_fwnode_endpoint_parse(fwn, );
> > if (ret)
> > return ret;
> > 
> > -   dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
> > -   vep.base.port);
> > +   dev_dbg(dev, "interface %u\n", vfwn.base.port);
> 
> Is there no way to keep the node name in the error message ?

There's no generic fwnode means to do something similar currently, possibly
because I understand ACPI doesn't do that. One could check whether the node
is an OF node and then use the full_name field but I wonder if it's worth
it.

> 
> 
> [snip]
> 
> > @@ -2094,18 +2094,17 @@ static int isp_of_parse_node(struct device *dev,
> 

Re: [PATCH v2 2/8] v4l: fwnode: Support generic fwnode for parsing standardised properties

[Laurent Pinchart]

Hi Sakari,

On Friday 07 Apr 2017 13:36:34 Sakari Ailus wrote:
> On Fri, Apr 07, 2017 at 12:44:27PM +0300, Laurent Pinchart wrote:
> > On Thursday 06 Apr 2017 16:12:04 Sakari Ailus wrote:
> > > The fwnode_handle is a more generic way than OF device_node to describe
> > > firmware nodes. Instead of the OF API, use more generic fwnode API to
> > > obtain the same information.
> > 
> > I would mention that this is a copy of v4l2-of.c with the OF API replaced
> > with the fwnode API.
> 
> I'll add that to the description.
> 
> > > As the V4L2 fwnode support will be required by a small minority of e.g.
> > > ACPI based systems (the same might actually go for OF), make this a
> > > module instead of embedding it in the videodev module.
> > > 
> > > Signed-off-by: Sakari Ailus 
> > > ---
> > > 
> > >  drivers/media/v4l2-core/Kconfig   |   3 +
> > >  drivers/media/v4l2-core/Makefile  |   1 +
> > >  drivers/media/v4l2-core/v4l2-fwnode.c | 353 +++
> > >  include/media/v4l2-fwnode.h   | 104 ++
> > >  4 files changed, 461 insertions(+)
> > >  create mode 100644 drivers/media/v4l2-core/v4l2-fwnode.c
> > >  create mode 100644 include/media/v4l2-fwnode.h

[snip]

> > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > b/drivers/media/v4l2-core/v4l2-fwnode.c new file mode 100644
> > > index 000..4f69b11
> > > --- /dev/null
> > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > @@ -0,0 +1,353 @@
> > > +/*
> > > + * V4L2 fwnode binding parsing library
> > > + *
> > > + * Copyright (c) 2016 Intel Corporation.
> > > + * Author: Sakari Ailus 
> > > + *
> > > + * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
> > > + * Author: Sylwester Nawrocki 
> > > + *
> > > + * Copyright (C) 2012 Renesas Electronics Corp.
> > > + * Author: Guennadi Liakhovetski 
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of version 2 of the GNU General Public License as
> > > + * published by the Free Software Foundation.
> > > + */
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include 
> > > +
> > > +static int v4l2_fwnode_endpoint_parse_csi_bus(struct fwnode_handle
> > > *fwn,
> > > +   struct v4l2_fwnode_endpoint
> > > *vfwn)
> > > +{
> > > + struct v4l2_fwnode_bus_mipi_csi2 *bus = >bus.mipi_csi2;
> > > + bool have_clk_lane = false;
> > > + unsigned int flags = 0, lanes_used = 0;
> > > + unsigned int i;
> > > + u32 v;
> > > + int rval;
> > 
> > I would have used "ret" instead of "rval" ;-)
> 
> I know. But
> 
> 1) there's no established convention in the file and
> 
> 2) "rval" has the benefit is easier to look up; one doesn't find a plethora
> of "return something". Therefore it is better than "ret" for the purpose.

The solution to that is

/ret\>

(and, of course, switching to vim :-D)

[snip]

> > > +/*
> > > + * v4l2_fwnode_endpoint_free() - free the V4L2 fwnode acquired by
> > > + * v4l2_fwnode_endpoint_alloc_parse()
> > > + * @fwn - the V4L2 fwnode the resources of which are to be released
> > 
> > Mayeb "the V4L2 fwnode whose resources are to be released" ?
> > 
> > > + *
> > > + * It is safe to call this function with NULL argument or on an
> > 
> > s/on an/on a/
> 
> Yes.
> 
> > > + * V4L2 fwnode the parsing of which failed.
> > 
> > "whose parsing failed" ?
> 
> Any particular reason? Do you like "whose"? :-)

"of which" sounds dubious in this context, but please consult a native English 
speaker in case of doubt.

[snip]

> > > +/**
> > > + * v4l2_fwnode_endpoint_parse_link() - parse a link between two
> > > endpoints
> > > + * @node: pointer to the fwnode at the local end of the link
> > 
> > The parameter is called __fwn. I believe you should rename it to fwn,
> > otherwise the documentation will look weird.
> > 
> > As explained before, you should mention that this is an endpoint fwnode
> > handle.
> 
> Agreed.
> 
> > > + * @link: pointer to the V4L2 fwnode link data structure
> > > + *
> > > + * Fill the link structure with the local and remote nodes and port
> > > numbers.
> > > + * The local_node and remote_node fields are set to point to the local
> > > and
> > > + * remote port's parent nodes respectively (the port parent node being
> > > the
> > > + * parent node of the port node if that node isn't a 'ports' node, or
> > > the
> > > + * grand-parent node of the port node otherwise).
> > > + *
> > > + * A reference is taken to both the local and remote nodes, the caller
> > > + * must use v4l2_fwnode_endpoint_put_link() to drop the references
> > > + * when done with the link.
> > 
> > Just curious, is there a reason to wrap earlier than the 80 columns limit
> > ?
> > 
> >  * A reference is taken to both the local and remote nodes, the caller
> >  

Re: [PATCH v2 3/8] v4l: async: Add fwnode match support

[Laurent Pinchart]

Hi Sakari,

On Friday 07 Apr 2017 13:45:09 Sakari Ailus wrote:
> On Fri, Apr 07, 2017 at 01:04:47PM +0300, Laurent Pinchart wrote:
> > > @@ -58,6 +60,9 @@ struct v4l2_async_subdev {
> > >   const struct device_node *node;
> > >   } of;
> > >   struct {
> > > + struct fwnode_handle *fwn;
> > 
> > Shouldn't this be const ?
> 
> I thought the same, but a lot of functions that operate on fwnode_handle
> take a non-const argument. I attempted changing that, but it starts a
> cascade of unavoidable changes elsewhere. That's not very well suitable for
> this patchset.

fwnode is young, we should try to fix it instead of propagating issues :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/8] v4l: async: Add fwnode match support

[Sakari Ailus]

Hi Laurent,

On Fri, Apr 07, 2017 at 01:04:47PM +0300, Laurent Pinchart wrote:
> > @@ -58,6 +60,9 @@ struct v4l2_async_subdev {
> > const struct device_node *node;
> > } of;
> > struct {
> > +   struct fwnode_handle *fwn;
> 
> Shouldn't this be const ?

I thought the same, but a lot of functions that operate on fwnode_handle
take a non-const argument. I attempted changing that, but it starts a
cascade of unavoidable changes elsewhere. That's not very well suitable for
this patchset.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v2 3/8] v4l: async: Add fwnode match support

[Sakari Ailus]

Hi Laurent,

On Fri, Apr 07, 2017 at 12:49:02PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thursday 06 Apr 2017 16:12:05 Sakari Ailus wrote:
> > Add fwnode matching to complement OF node matching. And fwnode may also be
> > an OF node.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/v4l2-async.c | 12 
> >  include/media/v4l2-async.h   |  5 +
> >  include/media/v4l2-subdev.h  |  3 +++
> >  3 files changed, 20 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c
> > b/drivers/media/v4l2-core/v4l2-async.c index 96cc733..384ad5e 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -46,6 +46,11 @@ static bool match_of(struct v4l2_subdev *sd, struct
> > v4l2_async_subdev *asd) of_node_full_name(asd->match.of.node));
> >  }
> > 
> > +static bool match_fwnode(struct v4l2_subdev *sd, struct v4l2_async_subdev
> > *asd)
> > +{
> > +   return sd->fwnode == asd->match.fwnode.fwn;
> > +}
> > +
> >  static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev
> > *asd) {
> > if (!asd->match.custom.match)
> > @@ -80,6 +85,9 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct
> > v4l2_async_notifier * case V4L2_ASYNC_MATCH_OF:
> > match = match_of;
> > break;
> > +   case V4L2_ASYNC_MATCH_FWNODE:
> > +   match = match_fwnode;
> > +   break;
> > default:
> > /* Cannot happen, unless someone breaks us */
> > WARN_ON(true);
> > @@ -158,6 +166,7 @@ int v4l2_async_notifier_register(struct v4l2_device
> > *v4l2_dev, case V4L2_ASYNC_MATCH_DEVNAME:
> > case V4L2_ASYNC_MATCH_I2C:
> > case V4L2_ASYNC_MATCH_OF:
> > +   case V4L2_ASYNC_MATCH_FWNODE:
> > break;
> > default:
> > dev_err(notifier->v4l2_dev ? notifier->v4l2_dev->dev : 
> NULL,
> > @@ -282,6 +291,9 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> >  */
> > if (!sd->of_node && sd->dev)
> > sd->of_node = sd->dev->of_node;
> > +   if (!sd->fwnode && sd->dev)
> > +   sd->fwnode = sd->dev->of_node ?
> > +   >dev->of_node->fwnode : sd->dev->fwnode;
> > 
> > mutex_lock(_lock);
> > 
> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > index 8e2a236..8f552d2 100644
> > --- a/include/media/v4l2-async.h
> > +++ b/include/media/v4l2-async.h
> > @@ -32,6 +32,7 @@ struct v4l2_async_notifier;
> >   * @V4L2_ASYNC_MATCH_DEVNAME: Match will use the device name
> >   * @V4L2_ASYNC_MATCH_I2C: Match will check for I2C adapter ID and address
> >   * @V4L2_ASYNC_MATCH_OF: Match will use OF node
> > + * @V4L2_ASYNC_MATCH_FWNODE: Match will use firmware node
> >   *
> >   * This enum is used by the asyncrhronous sub-device logic to define the
> >   * algorithm that will be used to match an asynchronous device.
> > @@ -41,6 +42,7 @@ enum v4l2_async_match_type {
> > V4L2_ASYNC_MATCH_DEVNAME,
> > V4L2_ASYNC_MATCH_I2C,
> > V4L2_ASYNC_MATCH_OF,
> > +   V4L2_ASYNC_MATCH_FWNODE,
> >  };
> > 
> >  /**
> > @@ -58,6 +60,9 @@ struct v4l2_async_subdev {
> > const struct device_node *node;
> > } of;
> > struct {
> > +   struct fwnode_handle *fwn;
> 
> I'd name this "node". The rationale is that code should be as independent as 
> possible of whether we use device_node or fwnode_handle. Naming both variable 
> "node" helps in that regard, and is in my opinion easier to read. This 
> applies 
> to the other patches in the series too.

What you're proposing doesn't really change that: you'll have to be aware of
which one you have anyway. Variables pointing to fwnode_handle are often
called fwn as well --- as node is used for struct device_node.

In other words, I prefer to keep it as it is.

> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart 
> 
> > +   } fwnode;
> > +   struct {
> > const char *name;
> > } device_name;
> > struct {
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index 0ab1c5d..5f1669c 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -788,6 +788,8 @@ struct v4l2_subdev_platform_data {
> >   * @devnode: subdev device node
> >   * @dev: pointer to the physical device, if any
> >   * @of_node: The device_node of the subdev, usually the same as
> > dev->of_node. + * @fwnode: The fwnode_handle of the subdev, usually the
> > same as
> > + * either dev->of_node->fwnode or dev->fwnode (whichever is non-
> NULL).
> >   * @async_list: Links this subdev to a global subdev_list or
> > @notifier->done *   list.
> >   * @asd: Pointer to 

Re: [PATCH v2 2/8] v4l: fwnode: Support generic fwnode for parsing standardised properties

[Sakari Ailus]

Hi Laurent,

On Fri, Apr 07, 2017 at 12:44:27PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thursday 06 Apr 2017 16:12:04 Sakari Ailus wrote:
> > The fwnode_handle is a more generic way than OF device_node to describe
> > firmware nodes. Instead of the OF API, use more generic fwnode API to
> > obtain the same information.
> 
> I would mention that this is a copy of v4l2-of.c with the OF API replaced 
> with 
> the fwnode API.

I'll add that to the description.

> 
> > As the V4L2 fwnode support will be required by a small minority of e.g.
> > ACPI based systems (the same might actually go for OF), make this a module
> > instead of embedding it in the videodev module.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/Kconfig   |   3 +
> >  drivers/media/v4l2-core/Makefile  |   1 +
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 353 +++
> >  include/media/v4l2-fwnode.h   | 104 ++
> >  4 files changed, 461 insertions(+)
> >  create mode 100644 drivers/media/v4l2-core/v4l2-fwnode.c
> >  create mode 100644 include/media/v4l2-fwnode.h
> > 
> > diff --git a/drivers/media/v4l2-core/Kconfig
> > b/drivers/media/v4l2-core/Kconfig index 6b1b78f..a35c336 100644
> > --- a/drivers/media/v4l2-core/Kconfig
> > +++ b/drivers/media/v4l2-core/Kconfig
> > @@ -55,6 +55,9 @@ config V4L2_FLASH_LED_CLASS
> > 
> >   When in doubt, say N.
> > 
> > +config V4L2_FWNODE
> > +   tristate
> > +
> >  # Used by drivers that need Videobuf modules
> >  config VIDEOBUF_GEN
> > tristate
> > diff --git a/drivers/media/v4l2-core/Makefile
> > b/drivers/media/v4l2-core/Makefile index 795a535..cf77a63 100644
> > --- a/drivers/media/v4l2-core/Makefile
> > +++ b/drivers/media/v4l2-core/Makefile
> > @@ -13,6 +13,7 @@ endif
> >  ifeq ($(CONFIG_OF),y)
> >videodev-objs += v4l2-of.o
> >  endif
> > +obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o
> >  ifeq ($(CONFIG_TRACEPOINTS),y)
> >videodev-objs += vb2-trace.o v4l2-trace.o
> >  endif
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > b/drivers/media/v4l2-core/v4l2-fwnode.c new file mode 100644
> > index 000..4f69b11
> > --- /dev/null
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -0,0 +1,353 @@
> > +/*
> > + * V4L2 fwnode binding parsing library
> > + *
> > + * Copyright (c) 2016 Intel Corporation.
> > + * Author: Sakari Ailus 
> > + *
> > + * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
> > + * Author: Sylwester Nawrocki 
> > + *
> > + * Copyright (C) 2012 Renesas Electronics Corp.
> > + * Author: Guennadi Liakhovetski 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of version 2 of the GNU General Public License as
> > + * published by the Free Software Foundation.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +
> > +static int v4l2_fwnode_endpoint_parse_csi_bus(struct fwnode_handle *fwn,
> > + struct v4l2_fwnode_endpoint 
> *vfwn)
> > +{
> > +   struct v4l2_fwnode_bus_mipi_csi2 *bus = >bus.mipi_csi2;
> > +   bool have_clk_lane = false;
> > +   unsigned int flags = 0, lanes_used = 0;
> > +   unsigned int i;
> > +   u32 v;
> > +   int rval;
> 
> I would have used "ret" instead of "rval" ;-)

I know. But

1) there's no established convention in the file and

2) "rval" has the benefit is easier to look up; one doesn't find a plethora
of "return something". Therefore it is better than "ret" for the purpose.

> 
> > +   rval = fwnode_property_read_u32_array(fwn, "data-lanes", NULL, 0);
> > +   if (rval > 0) {
> > +   u32 array[ARRAY_SIZE(bus->data_lanes)];
> > +
> > +   bus->num_data_lanes =
> > +   min_t(int, ARRAY_SIZE(bus->data_lanes), rval);
> > +
> > +   fwnode_property_read_u32_array(
> > +   fwn, "data-lanes", array, bus->num_data_lanes);
> 
> Is there anything wrong with the usual way to wrap code ?
> 
>   fwnode_property_read_u32_array(fwn, "data-lanes", array,
>  bus->num_data_lanes);

I'll change the occurrences of that whenever feasible.

> 
> > +
> > +   for (i = 0; i < bus->num_data_lanes; i++) {
> > +   if (lanes_used & BIT(array[i]))
> > +   pr_warn("duplicated lane %u in data-lanes\n",
> > +   array[i]);
> > +   lanes_used |= BIT(array[i]);
> > +
> > +   bus->data_lanes[i] = array[i];
> > +   }
> > +   }
> > +
> > +   rval = fwnode_property_read_u32_array(fwn, "lane-polarities", NULL, 
> 0);
> > +   if (rval > 0) {
> > +   u32 array[ARRAY_SIZE(bus->lane_polarities)];
> > +
> > +   

Re: [PATCH v2 5/8] v4l: Switch from V4L2 OF not V4L2 fwnode API

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 06 Apr 2017 16:12:07 Sakari Ailus wrote:
> Switch users of the v4l2_of_ APIs to the more generic v4l2_fwnode_ APIs.
> 
> Existing OF matching continues to be supported. omap3isp and smiapp
> drivers are converted to fwnode matching as well.
> 
> Signed-off-by: Sakari Ailus 
> Acked-by: Benoit Parrot  # i2c/ov2569.c,
> am437x/am437x-vpfe.c and ti-vpe/cal.c ---
>  drivers/media/i2c/Kconfig  |  9 
>  drivers/media/i2c/adv7604.c|  7 +--
>  drivers/media/i2c/mt9v032.c|  7 +--
>  drivers/media/i2c/ov2659.c |  8 +--
>  drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  7 +--
>  drivers/media/i2c/s5k5baf.c|  6 +--
>  drivers/media/i2c/smiapp/Kconfig   |  1 +
>  drivers/media/i2c/smiapp/smiapp-core.c | 29 ++-
>  drivers/media/i2c/tc358743.c   | 11 ++--
>  drivers/media/i2c/tvp514x.c|  6 +--
>  drivers/media/i2c/tvp5150.c|  7 +--
>  drivers/media/i2c/tvp7002.c|  6 +--
>  drivers/media/platform/Kconfig |  3 ++
>  drivers/media/platform/am437x/Kconfig  |  1 +
>  drivers/media/platform/am437x/am437x-vpfe.c|  8 +--
>  drivers/media/platform/atmel/Kconfig   |  1 +
>  drivers/media/platform/atmel/atmel-isc.c   |  8 +--
>  drivers/media/platform/exynos4-is/Kconfig  |  2 +
>  drivers/media/platform/exynos4-is/media-dev.c  |  6 +--
>  drivers/media/platform/exynos4-is/mipi-csis.c  |  6 +--
>  drivers/media/platform/omap3isp/isp.c  | 71 +++---
>  drivers/media/platform/pxa_camera.c|  7 +--
>  drivers/media/platform/rcar-vin/Kconfig|  1 +
>  drivers/media/platform/rcar-vin/rcar-core.c|  6 +--
>  drivers/media/platform/soc_camera/Kconfig  |  1 +
>  drivers/media/platform/soc_camera/atmel-isi.c  |  7 +--
>  drivers/media/platform/soc_camera/soc_camera.c |  3 +-
>  drivers/media/platform/ti-vpe/cal.c| 11 ++--
>  drivers/media/platform/xilinx/Kconfig  |  1 +
>  drivers/media/platform/xilinx/xilinx-vipp.c| 59 +++--
>  include/media/v4l2-fwnode.h|  4 +-
>  31 files changed, 176 insertions(+), 134 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index cee1dae..6b2423a 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -210,6 +210,7 @@ config VIDEO_ADV7604
>   depends on GPIOLIB || COMPILE_TEST
>   select HDMI
>   select MEDIA_CEC_EDID
> + select V4L2_FWNODE

What happens when building the driver on a platform that includes neither OF 
nor ACPI support ?

>   ---help---
> Support for the Analog Devices ADV7604 video decoder.
> 

[snip]

How have you checked that you haven't missed any entry in the Kconfig files ?

[snip]

> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 084ecf4a..95850b9 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c

[snip]

> @@ -2024,43 +2025,42 @@ enum isp_of_phy {
>   ISP_OF_PHY_CSIPHY2,
>  };
> 
> -static int isp_of_parse_node(struct device *dev, struct device_node *node,
> -  struct isp_async_subdev *isd)
> +static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn,
> + struct isp_async_subdev *isd)
>  {
>   struct isp_bus_cfg *buscfg = >bus;
> - struct v4l2_of_endpoint vep;
> + struct v4l2_fwnode_endpoint vfwn;

vfwn is confusing to me, I think the variable name should show that it refers 
to an endpoint.

>   unsigned int i;
>   int ret;
> 
> - ret = v4l2_of_parse_endpoint(node, );
> + ret = v4l2_fwnode_endpoint_parse(fwn, );
>   if (ret)
>   return ret;
> 
> - dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
> - vep.base.port);
> + dev_dbg(dev, "interface %u\n", vfwn.base.port);

Is there no way to keep the node name in the error message ?


[snip]

> @@ -2094,18 +2094,17 @@ static int isp_of_parse_node(struct device *dev,
> struct device_node *node, break;
> 
>   default:
> - dev_warn(dev, "%s: invalid interface %u\n", node->full_name,
> -  vep.base.port);
> + dev_warn(dev, "invalid interface %u\n", vfwn.base.port);

Ditto.

>   break;
>   }
> 
>   return 0;
>  }
> 
> -static int isp_of_parse_nodes(struct device *dev,
> -   struct v4l2_async_notifier *notifier)
> +static int isp_fwnodes_parse(struct device *dev,
> +  struct v4l2_async_notifier *notifier)
>  {
> - struct device_node *node = NULL;
> + struct fwnode_handle *fwn = NULL;

As explained in the review of another patch from the same 

Re: [Patch v4 12/12] Documention: v4l: Documentation for HEVC CIDs

[Sylwester Nawrocki]

On 04/07/2017 12:03 PM, Smitha T Murthy wrote:

+``V4L2_CID_MPEG_VIDEO_HEVC_LF``
+Indicates loop filtering. Control ID 0 indicates loop filtering
+is enabled and when set to 1 indicates no filter.

>>

"Setting this control to 0 enables loop filtering, setting this control
to 1 disables loop filtering." ?

Couldn't the meaning be inverted, so setting the control to 0 disables
the loop filtering?

From register point of view, this control value needs be 0 to enable

loop filtering.


OK, this is true for our specific hardware/firmware implementation.

In general, for this user space interface I would rather define that
boolean control so value 1 enables LF and value 0 disables LF.
The driver could simply negate the value when writing registers.

BTW we might need to specify type of the control here as Hans suggested
commenting on other control.

--
Regards,
Sylwester

Re: [PATCH v2 1/8] v4l: flash led class: Use fwnode_handle instead of device_node in init

[Sakari Ailus]

Hi Laurent,

Thank you for the review!

On Fri, Apr 07, 2017 at 11:49:56AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thursday 06 Apr 2017 16:12:03 Sakari Ailus wrote:
> > Pass the more generic fwnode_handle to the init function than the
> > device_node.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/leds/leds-aat1290.c|  5 +++--
> >  drivers/leds/leds-max77693.c   |  5 +++--
> >  drivers/media/v4l2-core/v4l2-flash-led-class.c | 11 ++-
> >  include/media/v4l2-flash-led-class.h   |  4 ++--
> >  4 files changed, 14 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
> > index def3cf9..a21e192 100644
> > --- a/drivers/leds/leds-aat1290.c
> > +++ b/drivers/leds/leds-aat1290.c
> > @@ -503,8 +503,9 @@ static int aat1290_led_probe(struct platform_device
> > *pdev) aat1290_init_v4l2_flash_config(led, _cfg, _sd_cfg);
> > 
> > /* Create V4L2 Flash subdev. */
> > -   led->v4l2_flash = v4l2_flash_init(dev, sub_node, fled_cdev, NULL,
> > - _flash_ops, _sd_cfg);
> > +   led->v4l2_flash = v4l2_flash_init(dev, of_fwnode_handle(sub_node),
> > + fled_cdev, NULL, _flash_ops,
> > + _sd_cfg);
> > if (IS_ERR(led->v4l2_flash)) {
> > ret = PTR_ERR(led->v4l2_flash);
> > goto error_v4l2_flash_init;
> > diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
> > index 1eb58ef..2d3062d 100644
> > --- a/drivers/leds/leds-max77693.c
> > +++ b/drivers/leds/leds-max77693.c
> > @@ -930,8 +930,9 @@ static int max77693_register_led(struct max77693_sub_led
> > *sub_led, max77693_init_v4l2_flash_config(sub_led, led_cfg, _sd_cfg);
> > 
> > /* Register in the V4L2 subsystem. */
> > -   sub_led->v4l2_flash = v4l2_flash_init(dev, sub_node, fled_cdev, NULL,
> > - _flash_ops, _sd_cfg);
> > +   sub_led->v4l2_flash = v4l2_flash_init(dev, of_fwnode_handle(sub_node),
> > + fled_cdev, NULL, 
> _flash_ops,
> > + _sd_cfg);
> > if (IS_ERR(sub_led->v4l2_flash)) {
> > ret = PTR_ERR(sub_led->v4l2_flash);
> > goto err_v4l2_flash_init;
> > diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c
> > b/drivers/media/v4l2-core/v4l2-flash-led-class.c index 794e563..f430c89
> > 100644
> > --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c
> > +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
> > @@ -13,6 +13,7 @@
> >  #include 
> >  #include 
> >  #include 
> 
> I think you can drop linux/of.h.

Correct. Will fix.

> 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -612,7 +613,7 @@ static const struct v4l2_subdev_internal_ops
> > v4l2_flash_subdev_internal_ops = { static const struct v4l2_subdev_ops
> > v4l2_flash_subdev_ops;
> > 
> >  struct v4l2_flash *v4l2_flash_init(
> > -   struct device *dev, struct device_node *of_node,
> > +   struct device *dev, struct fwnode_handle *fwn,
> > struct led_classdev_flash *fled_cdev,
> > struct led_classdev_flash *iled_cdev,
> > const struct v4l2_flash_ops *ops,
> > @@ -638,7 +639,7 @@ struct v4l2_flash *v4l2_flash_init(
> > v4l2_flash->iled_cdev = iled_cdev;
> > v4l2_flash->ops = ops;
> > sd->dev = dev;
> > -   sd->of_node = of_node ? of_node : led_cdev->dev->of_node;
> > +   sd->fwnode = fwn ? fwn : dev_fwnode(led_cdev->dev);
> 
> v4l2_subdev will only have a fwnode field in patch 3/8.

I think I've rearranged the set after writing the patch and missed the
dependency. I'll move it third unless there are other dependencies.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v2 4/8] v4l: async: Provide interoperability between OF and fwnode matching

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 06 Apr 2017 16:12:06 Sakari Ailus wrote:
> OF and fwnode support are separated in V4L2 and individual drivers may
> implement one of them. Sub-devices do not match with a notifier
> expecting sub-devices with fwnodes, nor the other way around.

Shouldn't we instead convert all drivers to fwnode matching ? What's missing 
after the mass conversion in patch 5/8 ?

> Fix this by checking for sub-device's of_node field in fwnode match and
> fwnode field in OF match.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 26 +++---
>  include/media/v4l2-async.h   |  2 +-
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> b/drivers/media/v4l2-core/v4l2-async.c index 384ad5e..7f5d804 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -40,15 +41,34 @@ static bool match_devname(struct v4l2_subdev *sd,
>   return !strcmp(asd->match.device_name.name, dev_name(sd->dev));
>  }
> 
> +static bool fwnode_cmp(struct fwnode_handle *one,
> +struct fwnode_handle *theother)
> +{
> + if (!one || !theother)
> + return false;
> +
> + if (one->type != theother->type)
> + return false;
> +
> + if (is_of_node(one))
> + return !of_node_cmp(of_node_full_name(to_of_node(one)),
> + of_node_full_name(to_of_node(theother)));
> + else
> + return one == theother;
> +}
> +
>  static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> {
> - return !of_node_cmp(of_node_full_name(sd->of_node),
> - of_node_full_name(asd->match.of.node));
> + return fwnode_cmp(sd->of_node ?
> +   of_fwnode_handle(sd->of_node) : sd->fwnode,
> +   of_fwnode_handle(asd->match.of.node));
>  }
> 
>  static bool match_fwnode(struct v4l2_subdev *sd, struct v4l2_async_subdev
> *asd)
>  {
> - return sd->fwnode == asd->match.fwnode.fwn;
> + return fwnode_cmp(sd->of_node ?
> +   of_fwnode_handle(sd->of_node) : sd->fwnode,
> +asd->match.fwnode.fwn);
>  }
> 
>  static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev
> *asd) diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 8f552d2..df8b682 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -57,7 +57,7 @@ struct v4l2_async_subdev {
>   enum v4l2_async_match_type match_type;
>   union {
>   struct {
> - const struct device_node *node;
> + struct device_node *node;

That seems to be a bit of a hack :-( I'd rather make everything const and cast 
to non-const pointers explicitly where the API requires us to. Or, better, add 
a to_of_node_const() function.

>   } of;
>   struct {
>   struct fwnode_handle *fwn;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/8] v4l: async: Add fwnode match support

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

One more small comment below.

On Thursday 06 Apr 2017 16:12:05 Sakari Ailus wrote:
> Add fwnode matching to complement OF node matching. And fwnode may also be
> an OF node.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 12 
>  include/media/v4l2-async.h   |  5 +
>  include/media/v4l2-subdev.h  |  3 +++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> b/drivers/media/v4l2-core/v4l2-async.c index 96cc733..384ad5e 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -46,6 +46,11 @@ static bool match_of(struct v4l2_subdev *sd, struct
> v4l2_async_subdev *asd) of_node_full_name(asd->match.of.node));
>  }
> 
> +static bool match_fwnode(struct v4l2_subdev *sd, struct v4l2_async_subdev
> *asd)
> +{
> + return sd->fwnode == asd->match.fwnode.fwn;
> +}
> +
>  static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev
> *asd) {
>   if (!asd->match.custom.match)
> @@ -80,6 +85,9 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct
> v4l2_async_notifier * case V4L2_ASYNC_MATCH_OF:
>   match = match_of;
>   break;
> + case V4L2_ASYNC_MATCH_FWNODE:
> + match = match_fwnode;
> + break;
>   default:
>   /* Cannot happen, unless someone breaks us */
>   WARN_ON(true);
> @@ -158,6 +166,7 @@ int v4l2_async_notifier_register(struct v4l2_device
> *v4l2_dev, case V4L2_ASYNC_MATCH_DEVNAME:
>   case V4L2_ASYNC_MATCH_I2C:
>   case V4L2_ASYNC_MATCH_OF:
> + case V4L2_ASYNC_MATCH_FWNODE:
>   break;
>   default:
>   dev_err(notifier->v4l2_dev ? notifier->v4l2_dev->dev : 
NULL,
> @@ -282,6 +291,9 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>*/
>   if (!sd->of_node && sd->dev)
>   sd->of_node = sd->dev->of_node;
> + if (!sd->fwnode && sd->dev)
> + sd->fwnode = sd->dev->of_node ?
> + >dev->of_node->fwnode : sd->dev->fwnode;
> 
>   mutex_lock(_lock);
> 
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 8e2a236..8f552d2 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -32,6 +32,7 @@ struct v4l2_async_notifier;
>   * @V4L2_ASYNC_MATCH_DEVNAME: Match will use the device name
>   * @V4L2_ASYNC_MATCH_I2C: Match will check for I2C adapter ID and address
>   * @V4L2_ASYNC_MATCH_OF: Match will use OF node
> + * @V4L2_ASYNC_MATCH_FWNODE: Match will use firmware node
>   *
>   * This enum is used by the asyncrhronous sub-device logic to define the
>   * algorithm that will be used to match an asynchronous device.
> @@ -41,6 +42,7 @@ enum v4l2_async_match_type {
>   V4L2_ASYNC_MATCH_DEVNAME,
>   V4L2_ASYNC_MATCH_I2C,
>   V4L2_ASYNC_MATCH_OF,
> + V4L2_ASYNC_MATCH_FWNODE,
>  };
> 
>  /**
> @@ -58,6 +60,9 @@ struct v4l2_async_subdev {
>   const struct device_node *node;
>   } of;
>   struct {
> + struct fwnode_handle *fwn;

Shouldn't this be const ?

> + } fwnode;
> + struct {
>   const char *name;
>   } device_name;
>   struct {
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 0ab1c5d..5f1669c 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -788,6 +788,8 @@ struct v4l2_subdev_platform_data {
>   * @devnode: subdev device node
>   * @dev: pointer to the physical device, if any
>   * @of_node: The device_node of the subdev, usually the same as
> dev->of_node.
> + * @fwnode: The fwnode_handle of the subdev, usually the same as
> + *   either dev->of_node->fwnode or dev->fwnode (whichever is non-
NULL).
>   * @async_list: Links this subdev to a global subdev_list or
> @notifier->done * list.
>   * @asd: Pointer to respective  v4l2_async_subdev.
> @@ -819,6 +821,7 @@ struct v4l2_subdev {
>   struct video_device *devnode;
>   struct device *dev;
>   struct device_node *of_node;
> + struct fwnode_handle *fwnode;
>   struct list_head async_list;
>   struct v4l2_async_subdev *asd;
>   struct v4l2_async_notifier *notifier;

-- 
Regards,

Laurent Pinchart

Re: [Patch v4 12/12] Documention: v4l: Documentation for HEVC CIDs

[Smitha T Murthy]

On Thu, 2017-04-06 at 16:47 +0200, Sylwester Nawrocki wrote:
> On 04/06/2017 08:11 AM, Smitha T Murthy wrote:
> > Added V4l2 controls for HEVC encoder
> 
> s/HEVC/H.265/HEVC ?
> 
Ok I will change it.

> > Signed-off-by: Smitha T Murthy 
> > ---
> >  Documentation/media/uapi/v4l/extended-controls.rst | 391 
> > +
> >  1 file changed, 391 insertions(+)
> >
> > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
> > b/Documentation/media/uapi/v4l/extended-controls.rst
> > index abb1057..85a668d 100644
> > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > @@ -1960,6 +1960,397 @@ enum v4l2_vp8_golden_frame_sel -
> >  1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
> >
> >
> > +HEVC Control Reference
> 
> Perhaps "High Efficiency Video Coding (HEVC/H.265) Control Reference" ?
> 
I will change it.

> > +-
> > +
> > +The HEVC controls include controls for encoding parameters of HEVC video
> > +codec.
> 
> s/'HEVC'/'HEVC/H.265' ?
> 
Ok I will change it.
> > +
> > +
> > +.. _hevc-control-id:
> > +
> > +HEVC Control IDs
> 
> s/'HEVC'/'HEVC/H.265' ?
> 
I will change it.

> > +^^^
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP``
> > +Minimum quantization parameter for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP``
> > +Maximum quantization parameter for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP``
> > +Quantization parameter for an I frame for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP``
> > +Quantization parameter for a P frame for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP``
> > +Quantization parameter for a B frame for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_QP``
> > +HIERARCHICAL_QP allows host to specify the quantization parameter 
> > values
> > +for each temporal layer through HIERARCHICAL_QP_LAYER. This is valid 
> > only
> > +if HIERARCHICAL_CODING_LAYER is greater than 1.
> > +
> > +.. _v4l2-hevc-hierarchical-coding-type:
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_TYPE``
> > +(enum)
> > +
> > +enum v4l2_mpeg_video_hevc_hier_coding_type -
> > +Selects the hierarchical coding type for encoding. Possible values are:
> > +
> > +.. raw:: latex
> > +
> > +\begin{adjustbox}{width=\columnwidth}
> > +
> > +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> > +
> > +.. flat-table::
> > +:header-rows:  0
> > +:stub-columns: 0
> > +
> > +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_B``
> > +  - Use the B frame for hierarchical coding.
> > +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_P``
> > +  - Use the P frame for hierarchical coding.
> > +
> > +.. raw:: latex
> > +
> > +\end{adjustbox}
> > +
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER``
> > +Selects the hierarchical coding layer. In normal encoding
> > +(non-hierarchial coding), it should be zero. Possible values are 0 ~ 6.
> > +0 indicates HIERARCHICAL CODING LAYER 0, 1 indicates HIERARCHICAL 
> > CODING
> > +LAYER 1 and so on.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER_QP``
> > +Indicates the hierarchical coding layer quantization parameter.
> > +For HEVC it can have a value of 0-51. Hence in the control value passed
> > +the LSB 16 bits will indicate the quantization parameter. The MSB 16 
> > bit
> > +will pass the layer(0-6) it is meant for.
> > +
> > +.. _v4l2-hevc-profile:
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_PROFILE``
> > +(enum)
> > +
> > +enum v4l2_mpeg_video_hevc_profile -
> > +Select the desired profile for HEVC encoder.
> > +
> > +.. raw:: latex
> > +
> > +\begin{adjustbox}{width=\columnwidth}
> > +
> > +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> > +
> > +.. flat-table::
> > +:header-rows:  0
> > +:stub-columns: 0
> > +
> > +* - ``V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN``
> > +  - Main profile.
> > +* - ``V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE``
> > +  - Main still picture profile.
> > +
> > +.. raw:: latex
> > +
> > +\end{adjustbox}
> > +
> > +
> > +.. _v4l2-hevc-level:
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_LEVEL``
> > +(enum)
> > +
> > +enum v4l2_mpeg_video_hevc_level -
> > +Select the desired level for HEVC encoder.
> > +
> > +.. raw:: latex
> > +
> > +\begin{adjustbox}{width=\columnwidth}
> > +
> > +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> > +
> > +.. flat-table::
> > +   :header-rows:  0
> > +:stub-columns: 0
> > +
> > +* - ``V4L2_MPEG_VIDEO_HEVC_LEVEL_1``
> > +  - Level 1.0
> > +* - ``V4L2_MPEG_VIDEO_HEVC_LEVEL_2``
> > +  - Level 2.0
> > +* - ``V4L2_MPEG_VIDEO_HEVC_LEVEL_2_1``
> > +  - Level 2.1
> > +* - ``V4L2_MPEG_VIDEO_HEVC_LEVEL_3``
> > +  - Level 3.0
> > +* - ``V4L2_MPEG_VIDEO_HEVC_LEVEL_3_1``
> > +  - Level 3.1
> > +* - 

Re: [PATCH v2 6/8] v4l: media/drv-intf/soc_mediabus.h: include dependent header file

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 06 Apr 2017 16:12:08 Sakari Ailus wrote:
> media/drv-intf/soc_mediabus.h does depend on struct v4l2_mbus_config which
> is defined in media/v4l2-mediabus.h. Include it.
> 
> Signed-off-by: Sakari Ailus 

Was this provided indirectly before, through v4l2-of.h perhaps ? If so, 
shouldn't this patch be moved before 5/8 ? Apart from that,

Reviewed-by: Laurent Pinchart 

> ---
>  include/media/drv-intf/soc_mediabus.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/media/drv-intf/soc_mediabus.h
> b/include/media/drv-intf/soc_mediabus.h index 2ff7737..0449788 100644
> --- a/include/media/drv-intf/soc_mediabus.h
> +++ b/include/media/drv-intf/soc_mediabus.h
> @@ -14,6 +14,8 @@
>  #include 
>  #include 
> 
> +#include 
> +
>  /**
>   * enum soc_mbus_packing - data packing types on the media-bus
>   * @SOC_MBUS_PACKING_NONE:   no packing, bit-for-bit transfer to RAM, one

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 7/8] docs-rst: media: Switch documentation to V4L2 fwnode API

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 06 Apr 2017 16:12:09 Sakari Ailus wrote:
> Instead of including the V4L2 OF header in ReST documentation, use the
> V4L2 fwnode header instead.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  Documentation/media/kapi/v4l2-core.rst   | 2 +-
>  Documentation/media/kapi/v4l2-fwnode.rst | 3 +++
>  Documentation/media/kapi/v4l2-of.rst | 3 ---
>  3 files changed, 4 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/media/kapi/v4l2-fwnode.rst
>  delete mode 100644 Documentation/media/kapi/v4l2-of.rst
> 
> diff --git a/Documentation/media/kapi/v4l2-core.rst
> b/Documentation/media/kapi/v4l2-core.rst index e967715..1bc8a14 100644
> --- a/Documentation/media/kapi/v4l2-core.rst
> +++ b/Documentation/media/kapi/v4l2-core.rst
> @@ -19,7 +19,7 @@ Video2Linux devices
>  v4l2-mc
>  v4l2-mediabus
>  v4l2-mem2mem
> -v4l2-of
> +v4l2-fwnode

I wonder whether we should keep this alphabetically sorted.

Apart from that,

Reviewed-by: Laurent Pinchart 

>  v4l2-rect
>  v4l2-tuner
>  v4l2-common
> diff --git a/Documentation/media/kapi/v4l2-fwnode.rst
> b/Documentation/media/kapi/v4l2-fwnode.rst new file mode 100644
> index 000..6c8bccd
> --- /dev/null
> +++ b/Documentation/media/kapi/v4l2-fwnode.rst
> @@ -0,0 +1,3 @@
> +V4L2 fwnode kAPI
> +
> +.. kernel-doc:: include/media/v4l2-fwnode.h
> diff --git a/Documentation/media/kapi/v4l2-of.rst
> b/Documentation/media/kapi/v4l2-of.rst deleted file mode 100644
> index 1ddf76b..000
> --- a/Documentation/media/kapi/v4l2-of.rst
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -V4L2 Open Firmware kAPI
> -^^^
> -.. kernel-doc:: include/media/v4l2-of.h

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 8/8] v4l: Remove V4L2 OF framework in favour of V4L2 fwnode framework

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 06 Apr 2017 16:12:10 Sakari Ailus wrote:
> All drivers have been converted from V4L2 OF to V4L2 fwnode. The V4L2 OF
> framework is now unused. Remove it.
> 
> Signed-off-by: Sakari Ailus 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/media/v4l2-core/Makefile  |   3 -
>  drivers/media/v4l2-core/v4l2-of.c | 327 ---
>  include/media/v4l2-of.h   | 128 ---
>  3 files changed, 458 deletions(-)
>  delete mode 100644 drivers/media/v4l2-core/v4l2-of.c
>  delete mode 100644 include/media/v4l2-of.h

-- 
Regards,

Laurent Pinchart

Re: [PATCH v9] [media] vimc: Virtual Media Controller core, capture and sensor

[Hans Verkuil]

Just a few small comments for v10:

On 04/04/2017 12:16 AM, Helen Koike wrote:
> First version of the Virtual Media Controller.
> Add a simple version of the core of the driver, the capture and
> sensor nodes in the topology, generating a grey image in a hardcoded
> format.
> 
> Signed-off-by: Helen Koike 
> 
> ---
> 
> Patch based in media/master tree, and available here:
> https://github.com/helen-fornazier/opw-staging/tree/vimc/devel/v8
> 
> Changes since v8:
> - fix vimc_sen_enum_mbus_code: return EINVAL if code->index > 0
> - remove input ioctls from vimc-capture.c as v4l2 intends to have default
> input ioctls in the core
> - change kernel-docs for vimc_pix_map_by_* so the line fits in 80
> characters
> 
> Changes since v7:
> - remove left over union in struct vimc_ent_device
> - remove v4l2_dev pointer from vimc_sen_device structure
> - remove unused dev parameter from vimc_propagate_frame()
> - remove struct device *dev from struct vimc_cap_device and vimc_sen_device
> - in vimc_sen_create: call media_entity_pads_init() after v4l2_subdev_init()
> to avoid double initialization of vsen->sd.entity.name
> - in vimc_sen_destroy: move media_entity_cleanup to be after 
> v4l2_device_unregister_subdev
> - rename video_device back to vdev instead of vd
> - adjust copyright with range 2015-2017
> - add node names in dev_err prints for vimc-capture.c and vimc-sensor.c
> - remove prefix "cap" in dev_dbg as it already uses the name of the node
> 
> Changes since v6:
> - add kernel-docs in vimc-core.h
> - reorder list_del call in vimc_cap_return_all_buffers()
> - call media_pipeline_stop in vimc_cap_start_streaming when fail
> - remove DMA comment (left over from the sample driver)
> - remove vb2_set_plane_payload call in vimc_cap_buffer_prepare
> - remove format verification in vimc_cap_link_validate
> - set vimc_pix_map_list as static
> - use MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN in vimc_raw_create()
> - register media device after creating the topology and unregister it before 
> destroying the topology
> - replace devm_kzalloc by kzalloc for allocating struct vimc_device
> - uset TASK_UNINTERRUPTIBLE for vimc_thread_sen
> - do not allow the creation of a sensor with no pads
> - add more verbose description in Kconfig
> - change copyright to 2017
> - arrange includes: number before letters
> - remove v4l2_dev pointer from vimc_cap_device structure
> - coding style adjustments
> - remove entity variable in vimc_cap_pipeline_s_stream
> - declare and assign variables in vimc_cap_link_validate
> - declare vimc_dev_release() and vimc_pdev closer to vimc_init()
> - remove pad check in vimc_sen_enum_{mbus_code,frame_size} that is already 
> performed by v4l2-subdev.c
> - fix multiline comments to start with /*
> - reorder variable declaration in functions
> - remove pad and subdevice type check in vimc_cap_pipeline_s_stream
> - add a note that sensor nodes can can have more then one source pad
> - remove the use of entity->use_count in the core and attach the ved 
> structure in sd or vd, use
>   ent->obj_type to know which structure (sd or vd) to use
> - rename vdev (video_device) to vd to be similar to sd (subdev)
> 
> Changes since v5:
> - Fix message "Entity type for entity Sensor A was not initialized!"
>   by initializing the sensor entity.function after the calling
>   v4l2_subded_init
> - populate device_caps in vimc_cap_create instead of in
>   vimc_cap_querycap
> - Fix typo in vimc-core.c s/de/the
> 
> Changes since v4:
> - coding style fixes
> - remove BUG_ON
> - change copyright to 2016
> - depens on VIDEO_V4L2_SUBDEV_API instead of select
> - remove assignement of V4L2_CAP_DEVICE_CAPS
> - s/vimc_cap_g_fmt_vid_cap/vimc_cap_fmt_vid_cap
> - fix vimc_cap_queue_setup declaration type
> - remove wrong buffer size check and add it in vimc_cap_buffer_prepare
> - vimc_cap_create: remove unecessary check if v4l2_dev or v4l2_dev->dev is 
> null
> - vimc_cap_create: only allow a single pad
> - vimc_sen_create: only allow source pads, remove unecessary source pads
> checks in vimc_thread_sen
> 
> Changes since v3: fix rmmod crash and built-in compile
> - Re-order unregister calls in vimc_device_unregister function (remove
> rmmod issue)
> - Call media_device_unregister_entity in vimc_raw_destroy
> - Add depends on VIDEO_DEV && VIDEO_V4L2 and select VIDEOBUF2_VMALLOC
> - Check *nplanes in queue_setup (this remove v4l2-compliance fail)
> - Include  in vimc-sensor.c
> - Move include of  from vimc-core.c to vimc-core.h
> - Generate 60 frames per sec instead of 1 in the sensor
> 
> Changes since v2: update with current media master tree
> - Add struct media_pipeline in vimc_cap_device
> - Use vb2_v4l2_buffer instead of vb2_buffer
> - Typos
> - Remove usage of entity->type and use entity->function instead
> - Remove fmt argument from queue setup
> - Use ktime_get_ns instead of v4l2_get_timestamp
> - Iterate over link's list using list_for_each_entry
> - Use media_device_{init, cleanup}
> - Use 

Re: [Patch v4 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder

[Smitha T Murthy]

On Fri, 2017-04-07 at 11:25 +0200, Sylwester Nawrocki wrote:
> On 04/07/2017 10:31 AM, Smitha T Murthy wrote:
> > On Thu, 2017-04-06 at 15:14 +0200, Sylwester Nawrocki wrote:
> >> On 04/06/2017 08:11 AM, Smitha T Murthy wrote:
> >>> @@ -775,6 +832,47 @@ const char *v4l2_ctrl_get_name(u32 id)
> >>>   case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:return "VPX 
> >>> P-Frame QP Value";
> >>>   case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:   return "VPX 
> >>> Profile";
> >>>
> >>> + /* HEVC controls */
> >> [...]
> >>> + case V4L2_CID_MPEG_VIDEO_HEVC_LF_SLICE_BOUNDARY:return "HEVC LF 
> >>> Across Slice Boundary or Not";
> >> Please make sure the names are no longer than 31 characters to avoid
> >> truncation during control enumeration in user space.
> >> Data structures like struct v4l2_queryctrl, struct v4l2_query_ext_ctrl
> >> have only 32 bytes long array dedicated for the control name.
>  >
> > I will try to make the names less than 31 characters long without losing
> > the context. But there are many control names in this file which are
> > longer than 31 characters like
> > V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP,
> > V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_REF_PERIOD etc so I assumed it was
> > alright to have such long names. But I will shorten them as per your
> > suggestion.
> 
> Apologies if it wasn't clean enough but my comment referred to the
> length of the character string being returned (e.g. "HEVC LF Across
> Slice Boundary or Not") and not to the name of the enum.
> 
> --
> Regards,
> Sylwester
> 
Sorry I misunderstood I will take care of it in the next version.

Thank you,
Smitha

Re: [PATCH RFC 1/2] [media] v4l2: add V4L2_INPUT_TYPE_DEFAULT

[Laurent Pinchart]

Hi Hans,

On Friday 07 Apr 2017 11:46:48 Hans Verkuil wrote:
> On 04/04/2017 03:22 PM, Sakari Ailus wrote:
> > On Mon, Apr 03, 2017 at 12:11:54PM -0300, Helen Koike wrote:
> >> On 2017-03-31 06:57 AM, Mauro Carvalho Chehab wrote:
> >>> Em Fri, 31 Mar 2017 10:29:04 +0200 Hans Verkuil escreveu:
>  On 30/03/17 18:02, Helen Koike wrote:
> > Add V4L2_INPUT_TYPE_DEFAULT and helpers functions for input ioctls to
> > be used when no inputs are available in the device
> > 
> > Signed-off-by: Helen Koike 
> > ---
> > drivers/media/v4l2-core/v4l2-ioctl.c | 27 +++
> > include/media/v4l2-ioctl.h   | 26 ++
> > include/uapi/linux/videodev2.h   |  1 +
> > 3 files changed, 54 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c index 0c3f238..ccaf04b 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -2573,6 +2573,33 @@ struct mutex *v4l2_ioctl_get_lock(struct
> > video_device *vdev, unsigned cmd)
> > return vdev->lock;
> > }
> > 
> > +int v4l2_ioctl_enum_input_default(struct file *file, void *priv,
> > + struct v4l2_input *i)
> > +{
> > +   if (i->index > 0)
> > +   return -EINVAL;
> > +
> > +   memset(i, 0, sizeof(*i));
> > +   i->type = V4L2_INPUT_TYPE_DEFAULT;
> > +   strlcpy(i->name, "Default", sizeof(i->name));
> > +
> > +   return 0;
> > +}
> > +EXPORT_SYMBOL(v4l2_ioctl_enum_input_default);
> > +
> > +int v4l2_ioctl_g_input_default(struct file *file, void *priv,
> > unsigned int *i)
> > +{
> > +   *i = 0;
> > +   return 0;
> > +}
> > +EXPORT_SYMBOL(v4l2_ioctl_g_input_default);
> > +
> > +int v4l2_ioctl_s_input_default(struct file *file, void *priv,
> > unsigned int i)
> > +{
> > +   return i ? -EINVAL : 0;
> > +}
> > +EXPORT_SYMBOL(v4l2_ioctl_s_input_default);
> > +
> > /* Common ioctl debug function. This function can be used by
> >external ioctl messages as well as internal V4L ioctl */
> > 
> > void v4l_printk_ioctl(const char *prefix, unsigned int cmd)
> > diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> > index 6cd94e5..accc470 100644
> > --- a/include/media/v4l2-ioctl.h
> > +++ b/include/media/v4l2-ioctl.h
> > @@ -652,6 +652,32 @@ struct video_device;
> >  */
> > 
> > struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned
> > int cmd);
> > +
> > +/**
> > + * v4l2_ioctl_enum_input_default - v4l2 ioctl helper for
> > VIDIOC_ENUM_INPUT ioctl
> > + *
> > + * Plug this function in vidioc_enum_input field of the struct
> > v4l2_ioctl_ops to
> > + * enumerate a single input as V4L2_INPUT_TYPE_DEFAULT
> > + */
> > +int v4l2_ioctl_enum_input_default(struct file *file, void *priv,
> > + struct v4l2_input *i);
> > +
> > +/**
> > + * v4l2_ioctl_g_input_default - v4l2 ioctl helper for VIDIOC_G_INPUT
> > ioctl
> > + *
> > + * Plug this function in vidioc_g_input field of the struct
> > v4l2_ioctl_ops
> > + * when using v4l2_ioctl_enum_input_default
> > + */
> > +int v4l2_ioctl_g_input_default(struct file *file, void *priv,
> > unsigned int *i);
> > +
> > +/**
> > + * v4l2_ioctl_s_input_default - v4l2 ioctl helper for VIDIOC_S_INPUT
> > ioctl
> > + *
> > + * Plug this function in vidioc_s_input field of the struct
> > v4l2_ioctl_ops
> > + * when using v4l2_ioctl_enum_input_default
> > + */
> > +int v4l2_ioctl_s_input_default(struct file *file, void *priv,
> > unsigned int i);
> > +
> > /* names for fancy debug output */
> > extern const char *v4l2_field_names[];
> > extern const char *v4l2_type_names[];
> > diff --git a/include/uapi/linux/videodev2.h
> > b/include/uapi/linux/videodev2.h index 316be62..c10bbde 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -1477,6 +1477,7 @@ struct v4l2_input {
> > };
> > 
> > /*  Values for the 'type' field */
> > +#define V4L2_INPUT_TYPE_DEFAULT0
>  
>  I don't think we should add a new type here.
> >>> 
> >>> I second that. Just replied the same thing on a comment from Sakari to
> >>> patch 2/2.
> >>> 
>  The whole point of this exercise is to
>  allow existing apps to work, and existing apps expect a TYPE_CAMERA.
>  
>  BTW, don't read to much in the term 'CAMERA': it's really a catch all
>  for any video stream, whether it is from a sensor, composite input,
>  HDMI, etc.
>  
>  The description for 

Re: [PATCH v2 3/8] v4l: async: Add fwnode match support

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 06 Apr 2017 16:12:05 Sakari Ailus wrote:
> Add fwnode matching to complement OF node matching. And fwnode may also be
> an OF node.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 12 
>  include/media/v4l2-async.h   |  5 +
>  include/media/v4l2-subdev.h  |  3 +++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> b/drivers/media/v4l2-core/v4l2-async.c index 96cc733..384ad5e 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -46,6 +46,11 @@ static bool match_of(struct v4l2_subdev *sd, struct
> v4l2_async_subdev *asd) of_node_full_name(asd->match.of.node));
>  }
> 
> +static bool match_fwnode(struct v4l2_subdev *sd, struct v4l2_async_subdev
> *asd)
> +{
> + return sd->fwnode == asd->match.fwnode.fwn;
> +}
> +
>  static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev
> *asd) {
>   if (!asd->match.custom.match)
> @@ -80,6 +85,9 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct
> v4l2_async_notifier * case V4L2_ASYNC_MATCH_OF:
>   match = match_of;
>   break;
> + case V4L2_ASYNC_MATCH_FWNODE:
> + match = match_fwnode;
> + break;
>   default:
>   /* Cannot happen, unless someone breaks us */
>   WARN_ON(true);
> @@ -158,6 +166,7 @@ int v4l2_async_notifier_register(struct v4l2_device
> *v4l2_dev, case V4L2_ASYNC_MATCH_DEVNAME:
>   case V4L2_ASYNC_MATCH_I2C:
>   case V4L2_ASYNC_MATCH_OF:
> + case V4L2_ASYNC_MATCH_FWNODE:
>   break;
>   default:
>   dev_err(notifier->v4l2_dev ? notifier->v4l2_dev->dev : 
NULL,
> @@ -282,6 +291,9 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>*/
>   if (!sd->of_node && sd->dev)
>   sd->of_node = sd->dev->of_node;
> + if (!sd->fwnode && sd->dev)
> + sd->fwnode = sd->dev->of_node ?
> + >dev->of_node->fwnode : sd->dev->fwnode;
> 
>   mutex_lock(_lock);
> 
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 8e2a236..8f552d2 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -32,6 +32,7 @@ struct v4l2_async_notifier;
>   * @V4L2_ASYNC_MATCH_DEVNAME: Match will use the device name
>   * @V4L2_ASYNC_MATCH_I2C: Match will check for I2C adapter ID and address
>   * @V4L2_ASYNC_MATCH_OF: Match will use OF node
> + * @V4L2_ASYNC_MATCH_FWNODE: Match will use firmware node
>   *
>   * This enum is used by the asyncrhronous sub-device logic to define the
>   * algorithm that will be used to match an asynchronous device.
> @@ -41,6 +42,7 @@ enum v4l2_async_match_type {
>   V4L2_ASYNC_MATCH_DEVNAME,
>   V4L2_ASYNC_MATCH_I2C,
>   V4L2_ASYNC_MATCH_OF,
> + V4L2_ASYNC_MATCH_FWNODE,
>  };
> 
>  /**
> @@ -58,6 +60,9 @@ struct v4l2_async_subdev {
>   const struct device_node *node;
>   } of;
>   struct {
> + struct fwnode_handle *fwn;

I'd name this "node". The rationale is that code should be as independent as 
possible of whether we use device_node or fwnode_handle. Naming both variable 
"node" helps in that regard, and is in my opinion easier to read. This applies 
to the other patches in the series too.

Apart from that,

Reviewed-by: Laurent Pinchart 

> + } fwnode;
> + struct {
>   const char *name;
>   } device_name;
>   struct {
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 0ab1c5d..5f1669c 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -788,6 +788,8 @@ struct v4l2_subdev_platform_data {
>   * @devnode: subdev device node
>   * @dev: pointer to the physical device, if any
>   * @of_node: The device_node of the subdev, usually the same as
> dev->of_node. + * @fwnode: The fwnode_handle of the subdev, usually the
> same as
> + *   either dev->of_node->fwnode or dev->fwnode (whichever is non-
NULL).
>   * @async_list: Links this subdev to a global subdev_list or
> @notifier->done * list.
>   * @asd: Pointer to respective  v4l2_async_subdev.
> @@ -819,6 +821,7 @@ struct v4l2_subdev {
>   struct video_device *devnode;
>   struct device *dev;
>   struct device_node *of_node;
> + struct fwnode_handle *fwnode;
>   struct list_head async_list;
>   struct v4l2_async_subdev *asd;
>   struct v4l2_async_notifier *notifier;

-- 
Regards,

Laurent Pinchart

Re: [PATCH RFC 1/2] [media] v4l2: add V4L2_INPUT_TYPE_DEFAULT

[Hans Verkuil]

On 04/04/2017 03:22 PM, Sakari Ailus wrote:
> Hi Helen,
> 
> On Mon, Apr 03, 2017 at 12:11:54PM -0300, Helen Koike wrote:
>> Hi,
>>
>> On 2017-03-31 06:57 AM, Mauro Carvalho Chehab wrote:
>>> Em Fri, 31 Mar 2017 10:29:04 +0200
>>> Hans Verkuil  escreveu:
>>>
 On 30/03/17 18:02, Helen Koike wrote:
> Add V4L2_INPUT_TYPE_DEFAULT and helpers functions for input ioctls to be
> used when no inputs are available in the device
>
> Signed-off-by: Helen Koike 
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 27 +++
> include/media/v4l2-ioctl.h   | 26 ++
> include/uapi/linux/videodev2.h   |  1 +
> 3 files changed, 54 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 0c3f238..ccaf04b 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2573,6 +2573,33 @@ struct mutex *v4l2_ioctl_get_lock(struct 
> video_device *vdev, unsigned cmd)
>   return vdev->lock;
> }
>
> +int v4l2_ioctl_enum_input_default(struct file *file, void *priv,
> +   struct v4l2_input *i)
> +{
> + if (i->index > 0)
> + return -EINVAL;
> +
> + memset(i, 0, sizeof(*i));
> + i->type = V4L2_INPUT_TYPE_DEFAULT;
> + strlcpy(i->name, "Default", sizeof(i->name));
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(v4l2_ioctl_enum_input_default);
> +
> +int v4l2_ioctl_g_input_default(struct file *file, void *priv, unsigned 
> int *i)
> +{
> + *i = 0;
> + return 0;
> +}
> +EXPORT_SYMBOL(v4l2_ioctl_g_input_default);
> +
> +int v4l2_ioctl_s_input_default(struct file *file, void *priv, unsigned 
> int i)
> +{
> + return i ? -EINVAL : 0;
> +}
> +EXPORT_SYMBOL(v4l2_ioctl_s_input_default);
> +
> /* Common ioctl debug function. This function can be used by
>external ioctl messages as well as internal V4L ioctl */
> void v4l_printk_ioctl(const char *prefix, unsigned int cmd)
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index 6cd94e5..accc470 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -652,6 +652,32 @@ struct video_device;
>  */
> struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned int 
> cmd);
>
> +
> +/**
> + * v4l2_ioctl_enum_input_default - v4l2 ioctl helper for 
> VIDIOC_ENUM_INPUT ioctl
> + *
> + * Plug this function in vidioc_enum_input field of the struct 
> v4l2_ioctl_ops to
> + * enumerate a single input as V4L2_INPUT_TYPE_DEFAULT
> + */
> +int v4l2_ioctl_enum_input_default(struct file *file, void *priv,
> +   struct v4l2_input *i);
> +
> +/**
> + * v4l2_ioctl_g_input_default - v4l2 ioctl helper for VIDIOC_G_INPUT 
> ioctl
> + *
> + * Plug this function in vidioc_g_input field of the struct 
> v4l2_ioctl_ops
> + * when using v4l2_ioctl_enum_input_default
> + */
> +int v4l2_ioctl_g_input_default(struct file *file, void *priv, unsigned 
> int *i);
> +
> +/**
> + * v4l2_ioctl_s_input_default - v4l2 ioctl helper for VIDIOC_S_INPUT 
> ioctl
> + *
> + * Plug this function in vidioc_s_input field of the struct 
> v4l2_ioctl_ops
> + * when using v4l2_ioctl_enum_input_default
> + */
> +int v4l2_ioctl_s_input_default(struct file *file, void *priv, unsigned 
> int i);
> +
> /* names for fancy debug output */
> extern const char *v4l2_field_names[];
> extern const char *v4l2_type_names[];
> diff --git a/include/uapi/linux/videodev2.h 
> b/include/uapi/linux/videodev2.h
> index 316be62..c10bbde 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1477,6 +1477,7 @@ struct v4l2_input {
> };
>
> /*  Values for the 'type' field */
> +#define V4L2_INPUT_TYPE_DEFAULT  0

 I don't think we should add a new type here.
>>>
>>> I second that. Just replied the same thing on a comment from Sakari to
>>> patch 2/2.
>>>
 The whole point of this exercise is to
 allow existing apps to work, and existing apps expect a TYPE_CAMERA.

 BTW, don't read to much in the term 'CAMERA': it's really a catch all for 
 any video
 stream, whether it is from a sensor, composite input, HDMI, etc.

 The description for V4L2_INPUT_TYPE_CAMERA in the spec is hopelessly out 
 of date :-(
>>>
>>> Yeah, we always used "CAMERA" to mean NOT_TUNER.
>>>
 Rather than creating a new type I would add a new V4L2_IN_CAP_MC 
 capability that
 indicates that this input is controlled via the media controller. That 
 makes much
 more sense 

Re: [PATCH v2 2/8] v4l: fwnode: Support generic fwnode for parsing standardised properties

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 06 Apr 2017 16:12:04 Sakari Ailus wrote:
> The fwnode_handle is a more generic way than OF device_node to describe
> firmware nodes. Instead of the OF API, use more generic fwnode API to
> obtain the same information.

I would mention that this is a copy of v4l2-of.c with the OF API replaced with 
the fwnode API.

> As the V4L2 fwnode support will be required by a small minority of e.g.
> ACPI based systems (the same might actually go for OF), make this a module
> instead of embedding it in the videodev module.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/Kconfig   |   3 +
>  drivers/media/v4l2-core/Makefile  |   1 +
>  drivers/media/v4l2-core/v4l2-fwnode.c | 353 +++
>  include/media/v4l2-fwnode.h   | 104 ++
>  4 files changed, 461 insertions(+)
>  create mode 100644 drivers/media/v4l2-core/v4l2-fwnode.c
>  create mode 100644 include/media/v4l2-fwnode.h
> 
> diff --git a/drivers/media/v4l2-core/Kconfig
> b/drivers/media/v4l2-core/Kconfig index 6b1b78f..a35c336 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -55,6 +55,9 @@ config V4L2_FLASH_LED_CLASS
> 
> When in doubt, say N.
> 
> +config V4L2_FWNODE
> + tristate
> +
>  # Used by drivers that need Videobuf modules
>  config VIDEOBUF_GEN
>   tristate
> diff --git a/drivers/media/v4l2-core/Makefile
> b/drivers/media/v4l2-core/Makefile index 795a535..cf77a63 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -13,6 +13,7 @@ endif
>  ifeq ($(CONFIG_OF),y)
>videodev-objs += v4l2-of.o
>  endif
> +obj-$(CONFIG_V4L2_FWNODE) += v4l2-fwnode.o
>  ifeq ($(CONFIG_TRACEPOINTS),y)
>videodev-objs += vb2-trace.o v4l2-trace.o
>  endif
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> b/drivers/media/v4l2-core/v4l2-fwnode.c new file mode 100644
> index 000..4f69b11
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -0,0 +1,353 @@
> +/*
> + * V4L2 fwnode binding parsing library
> + *
> + * Copyright (c) 2016 Intel Corporation.
> + * Author: Sakari Ailus 
> + *
> + * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
> + * Author: Sylwester Nawrocki 
> + *
> + * Copyright (C) 2012 Renesas Electronics Corp.
> + * Author: Guennadi Liakhovetski 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +static int v4l2_fwnode_endpoint_parse_csi_bus(struct fwnode_handle *fwn,
> +   struct v4l2_fwnode_endpoint 
*vfwn)
> +{
> + struct v4l2_fwnode_bus_mipi_csi2 *bus = >bus.mipi_csi2;
> + bool have_clk_lane = false;
> + unsigned int flags = 0, lanes_used = 0;
> + unsigned int i;
> + u32 v;
> + int rval;

I would have used "ret" instead of "rval" ;-)

> + rval = fwnode_property_read_u32_array(fwn, "data-lanes", NULL, 0);
> + if (rval > 0) {
> + u32 array[ARRAY_SIZE(bus->data_lanes)];
> +
> + bus->num_data_lanes =
> + min_t(int, ARRAY_SIZE(bus->data_lanes), rval);
> +
> + fwnode_property_read_u32_array(
> + fwn, "data-lanes", array, bus->num_data_lanes);

Is there anything wrong with the usual way to wrap code ?

fwnode_property_read_u32_array(fwn, "data-lanes", array,
   bus->num_data_lanes);

> +
> + for (i = 0; i < bus->num_data_lanes; i++) {
> + if (lanes_used & BIT(array[i]))
> + pr_warn("duplicated lane %u in data-lanes\n",
> + array[i]);
> + lanes_used |= BIT(array[i]);
> +
> + bus->data_lanes[i] = array[i];
> + }
> + }
> +
> + rval = fwnode_property_read_u32_array(fwn, "lane-polarities", NULL, 
0);
> + if (rval > 0) {
> + u32 array[ARRAY_SIZE(bus->lane_polarities)];
> +
> + if (rval < 1 + bus->num_data_lanes /* clock + data */) {
> + pr_warn("too few lane-polarities entries (need %u, got 
%u)\n",
> + 1 + bus->num_data_lanes, rval);
> + return -EINVAL;
> + }
> +
> + fwnode_property_read_u32_array(
> + fwn, "lane-polarities", array, 1 + bus-
>num_data_lanes);

Ditto with

fwnode_property_read_u32_array(fwn, "lane-polarities", array,
   1 + bus->num_data_lanes);

> + for (i = 

Re: [PATCH RFC 1/2] [media] v4l2: add V4L2_INPUT_TYPE_DEFAULT

[Hans Verkuil]

On 03/31/2017 11:57 AM, Mauro Carvalho Chehab wrote:
> Em Fri, 31 Mar 2017 10:29:04 +0200
> Hans Verkuil  escreveu:
> 
>> On 30/03/17 18:02, Helen Koike wrote:
>>> Add V4L2_INPUT_TYPE_DEFAULT and helpers functions for input ioctls to be
>>> used when no inputs are available in the device
>>>
>>> Signed-off-by: Helen Koike 
>>> ---
>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 27 +++
>>>  include/media/v4l2-ioctl.h   | 26 ++
>>>  include/uapi/linux/videodev2.h   |  1 +
>>>  3 files changed, 54 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
>>> b/drivers/media/v4l2-core/v4l2-ioctl.c
>>> index 0c3f238..ccaf04b 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>> @@ -2573,6 +2573,33 @@ struct mutex *v4l2_ioctl_get_lock(struct 
>>> video_device *vdev, unsigned cmd)
>>> return vdev->lock;
>>>  }
>>>  
>>> +int v4l2_ioctl_enum_input_default(struct file *file, void *priv,
>>> + struct v4l2_input *i)
>>> +{
>>> +   if (i->index > 0)
>>> +   return -EINVAL;
>>> +
>>> +   memset(i, 0, sizeof(*i));
>>> +   i->type = V4L2_INPUT_TYPE_DEFAULT;
>>> +   strlcpy(i->name, "Default", sizeof(i->name));
>>> +
>>> +   return 0;
>>> +}
>>> +EXPORT_SYMBOL(v4l2_ioctl_enum_input_default);
>>> +
>>> +int v4l2_ioctl_g_input_default(struct file *file, void *priv, unsigned int 
>>> *i)
>>> +{
>>> +   *i = 0;
>>> +   return 0;
>>> +}
>>> +EXPORT_SYMBOL(v4l2_ioctl_g_input_default);
>>> +
>>> +int v4l2_ioctl_s_input_default(struct file *file, void *priv, unsigned int 
>>> i)
>>> +{
>>> +   return i ? -EINVAL : 0;
>>> +}
>>> +EXPORT_SYMBOL(v4l2_ioctl_s_input_default);
>>> +
>>>  /* Common ioctl debug function. This function can be used by
>>> external ioctl messages as well as internal V4L ioctl */
>>>  void v4l_printk_ioctl(const char *prefix, unsigned int cmd)
>>> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
>>> index 6cd94e5..accc470 100644
>>> --- a/include/media/v4l2-ioctl.h
>>> +++ b/include/media/v4l2-ioctl.h
>>> @@ -652,6 +652,32 @@ struct video_device;
>>>   */
>>>  struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned int 
>>> cmd);
>>>  
>>> +
>>> +/**
>>> + * v4l2_ioctl_enum_input_default - v4l2 ioctl helper for VIDIOC_ENUM_INPUT 
>>> ioctl
>>> + *
>>> + * Plug this function in vidioc_enum_input field of the struct 
>>> v4l2_ioctl_ops to
>>> + * enumerate a single input as V4L2_INPUT_TYPE_DEFAULT
>>> + */
>>> +int v4l2_ioctl_enum_input_default(struct file *file, void *priv,
>>> + struct v4l2_input *i);
>>> +
>>> +/**
>>> + * v4l2_ioctl_g_input_default - v4l2 ioctl helper for VIDIOC_G_INPUT ioctl
>>> + *
>>> + * Plug this function in vidioc_g_input field of the struct v4l2_ioctl_ops
>>> + * when using v4l2_ioctl_enum_input_default
>>> + */
>>> +int v4l2_ioctl_g_input_default(struct file *file, void *priv, unsigned int 
>>> *i);
>>> +
>>> +/**
>>> + * v4l2_ioctl_s_input_default - v4l2 ioctl helper for VIDIOC_S_INPUT ioctl
>>> + *
>>> + * Plug this function in vidioc_s_input field of the struct v4l2_ioctl_ops
>>> + * when using v4l2_ioctl_enum_input_default
>>> + */
>>> +int v4l2_ioctl_s_input_default(struct file *file, void *priv, unsigned int 
>>> i);
>>> +
>>>  /* names for fancy debug output */
>>>  extern const char *v4l2_field_names[];
>>>  extern const char *v4l2_type_names[];
>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>> index 316be62..c10bbde 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -1477,6 +1477,7 @@ struct v4l2_input {
>>>  };
>>>  
>>>  /*  Values for the 'type' field */
>>> +#define V4L2_INPUT_TYPE_DEFAULT0  
>>
>> I don't think we should add a new type here.
> 
> I second that. Just replied the same thing on a comment from Sakari to
> patch 2/2.
> 
>> The whole point of this exercise is to
>> allow existing apps to work, and existing apps expect a TYPE_CAMERA.
>>
>> BTW, don't read to much in the term 'CAMERA': it's really a catch all for 
>> any video
>> stream, whether it is from a sensor, composite input, HDMI, etc.
>>
>> The description for V4L2_INPUT_TYPE_CAMERA in the spec is hopelessly out of 
>> date :-(
> 
> Yeah, we always used "CAMERA" to mean NOT_TUNER.
> 
>> Rather than creating a new type I would add a new V4L2_IN_CAP_MC capability 
>> that
>> indicates that this input is controlled via the media controller. That makes 
>> much
>> more sense and it wouldn't potentially break applications.
>>
>> Exactly the same can be done for outputs as well: add V4L2_OUT_CAP_MC and use
>> V4L2_OUTPUT_TYPE_ANALOG as the output type (again, a horrible outdated name 
>> and the
>> spec is again out of date).
> 
> I don't see any sense on distinguishing IN and OUT for MC. I mean: should
> we ever allow that any driver to have 

Re: [Patch v4 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder

[Sylwester Nawrocki]

On 04/07/2017 10:31 AM, Smitha T Murthy wrote:

On Thu, 2017-04-06 at 15:14 +0200, Sylwester Nawrocki wrote:

On 04/06/2017 08:11 AM, Smitha T Murthy wrote:

@@ -775,6 +832,47 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:return "VPX P-Frame 
QP Value";
case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:   return "VPX 
Profile";

+   /* HEVC controls */

[...]

+   case V4L2_CID_MPEG_VIDEO_HEVC_LF_SLICE_BOUNDARY:return "HEVC LF 
Across Slice Boundary or Not";

Please make sure the names are no longer than 31 characters to avoid
truncation during control enumeration in user space.
Data structures like struct v4l2_queryctrl, struct v4l2_query_ext_ctrl
have only 32 bytes long array dedicated for the control name.

>

I will try to make the names less than 31 characters long without losing
the context. But there are many control names in this file which are
longer than 31 characters like
V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP,
V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_REF_PERIOD etc so I assumed it was
alright to have such long names. But I will shorten them as per your
suggestion.


Apologies if it wasn't clean enough but my comment referred to the
length of the character string being returned (e.g. "HEVC LF Across
Slice Boundary or Not") and not to the name of the enum.

--
Regards,
Sylwester

Re: [PATCH v2 1/8] v4l: flash led class: Use fwnode_handle instead of device_node in init

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 06 Apr 2017 16:12:03 Sakari Ailus wrote:
> Pass the more generic fwnode_handle to the init function than the
> device_node.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/leds/leds-aat1290.c|  5 +++--
>  drivers/leds/leds-max77693.c   |  5 +++--
>  drivers/media/v4l2-core/v4l2-flash-led-class.c | 11 ++-
>  include/media/v4l2-flash-led-class.h   |  4 ++--
>  4 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
> index def3cf9..a21e192 100644
> --- a/drivers/leds/leds-aat1290.c
> +++ b/drivers/leds/leds-aat1290.c
> @@ -503,8 +503,9 @@ static int aat1290_led_probe(struct platform_device
> *pdev) aat1290_init_v4l2_flash_config(led, _cfg, _sd_cfg);
> 
>   /* Create V4L2 Flash subdev. */
> - led->v4l2_flash = v4l2_flash_init(dev, sub_node, fled_cdev, NULL,
> -   _flash_ops, _sd_cfg);
> + led->v4l2_flash = v4l2_flash_init(dev, of_fwnode_handle(sub_node),
> +   fled_cdev, NULL, _flash_ops,
> +   _sd_cfg);
>   if (IS_ERR(led->v4l2_flash)) {
>   ret = PTR_ERR(led->v4l2_flash);
>   goto error_v4l2_flash_init;
> diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
> index 1eb58ef..2d3062d 100644
> --- a/drivers/leds/leds-max77693.c
> +++ b/drivers/leds/leds-max77693.c
> @@ -930,8 +930,9 @@ static int max77693_register_led(struct max77693_sub_led
> *sub_led, max77693_init_v4l2_flash_config(sub_led, led_cfg, _sd_cfg);
> 
>   /* Register in the V4L2 subsystem. */
> - sub_led->v4l2_flash = v4l2_flash_init(dev, sub_node, fled_cdev, NULL,
> -   _flash_ops, _sd_cfg);
> + sub_led->v4l2_flash = v4l2_flash_init(dev, of_fwnode_handle(sub_node),
> +   fled_cdev, NULL, 
_flash_ops,
> +   _sd_cfg);
>   if (IS_ERR(sub_led->v4l2_flash)) {
>   ret = PTR_ERR(sub_led->v4l2_flash);
>   goto err_v4l2_flash_init;
> diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c
> b/drivers/media/v4l2-core/v4l2-flash-led-class.c index 794e563..f430c89
> 100644
> --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c
> +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 

I think you can drop linux/of.h.

> +#include 
>  #include 
>  #include 
>  #include 
> @@ -612,7 +613,7 @@ static const struct v4l2_subdev_internal_ops
> v4l2_flash_subdev_internal_ops = { static const struct v4l2_subdev_ops
> v4l2_flash_subdev_ops;
> 
>  struct v4l2_flash *v4l2_flash_init(
> - struct device *dev, struct device_node *of_node,
> + struct device *dev, struct fwnode_handle *fwn,
>   struct led_classdev_flash *fled_cdev,
>   struct led_classdev_flash *iled_cdev,
>   const struct v4l2_flash_ops *ops,
> @@ -638,7 +639,7 @@ struct v4l2_flash *v4l2_flash_init(
>   v4l2_flash->iled_cdev = iled_cdev;
>   v4l2_flash->ops = ops;
>   sd->dev = dev;
> - sd->of_node = of_node ? of_node : led_cdev->dev->of_node;
> + sd->fwnode = fwn ? fwn : dev_fwnode(led_cdev->dev);

v4l2_subdev will only have a fwnode field in patch 3/8.

>   v4l2_subdev_init(sd, _flash_subdev_ops);
>   sd->internal_ops = _flash_subdev_internal_ops;
>   sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> @@ -654,7 +655,7 @@ struct v4l2_flash *v4l2_flash_init(
>   if (ret < 0)
>   goto err_init_controls;
> 
> - of_node_get(sd->of_node);
> + fwnode_handle_get(sd->fwnode);
> 
>   ret = v4l2_async_register_subdev(sd);
>   if (ret < 0)
> @@ -663,7 +664,7 @@ struct v4l2_flash *v4l2_flash_init(
>   return v4l2_flash;
> 
>  err_async_register_sd:
> - of_node_put(sd->of_node);
> + fwnode_handle_put(sd->fwnode);
>   v4l2_ctrl_handler_free(sd->ctrl_handler);
>  err_init_controls:
>   media_entity_cleanup(>entity);
> @@ -683,7 +684,7 @@ void v4l2_flash_release(struct v4l2_flash *v4l2_flash)
> 
>   v4l2_async_unregister_subdev(sd);
> 
> - of_node_put(sd->of_node);
> + fwnode_handle_put(sd->fwnode);
> 
>   v4l2_ctrl_handler_free(sd->ctrl_handler);
>   media_entity_cleanup(>entity);
> diff --git a/include/media/v4l2-flash-led-class.h
> b/include/media/v4l2-flash-led-class.h index b0fe4d6..5695853 100644
> --- a/include/media/v4l2-flash-led-class.h
> +++ b/include/media/v4l2-flash-led-class.h
> @@ -108,7 +108,7 @@ static inline struct v4l2_flash
> *v4l2_ctrl_to_v4l2_flash(struct v4l2_ctrl *c) /**
>   * v4l2_flash_init - initialize V4L2 flash led sub-device
>   * @dev: flash device, e.g. an I2C device
> - * @of_node: of_node of the LED, may be NULL if the same as device's
> + * @fwn: fwnode_handle of the LED, may be 

Re: [Patch v4 12/12] Documention: v4l: Documentation for HEVC CIDs

[Smitha T Murthy]

On Thu, 2017-04-06 at 15:33 +0200, Hans Verkuil wrote:
> On 04/06/2017 08:11 AM, Smitha T Murthy wrote:
> > Added V4l2 controls for HEVC encoder
> > 
> > Signed-off-by: Smitha T Murthy 
> 
> General comment: don't forget to build the pdf and check that as well.
> 
Yes I will build it and check before pushing the next version.

> > ---
> >  Documentation/media/uapi/v4l/extended-controls.rst | 391 
> > +
> >  1 file changed, 391 insertions(+)
> > 
> > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
> > b/Documentation/media/uapi/v4l/extended-controls.rst
> > index abb1057..85a668d 100644
> > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > @@ -1960,6 +1960,397 @@ enum v4l2_vp8_golden_frame_sel -
> >  1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
> >  
> >  
> > +HEVC Control Reference
> > +-
> > +
> > +The HEVC controls include controls for encoding parameters of HEVC video
> > +codec.
> > +
> > +
> > +.. _hevc-control-id:
> > +
> > +HEVC Control IDs
> > +^^^
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP``
> 
> You need to add the type of the control in parenthesis. It is probably 
> '(integer)'
> for this one. Just follow what is done for other controls.
> 
I had missed it. I will add them in next version.

> > +Minimum quantization parameter for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP``
> > +Maximum quantization parameter for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP``
> > +Quantization parameter for an I frame for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP``
> > +Quantization parameter for a P frame for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP``
> > +Quantization parameter for a B frame for HEVC.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_QP``
> > +HIERARCHICAL_QP allows host to specify the quantization parameter 
> > values
> > +for each temporal layer through HIERARCHICAL_QP_LAYER. This is valid 
> > only
> > +if HIERARCHICAL_CODING_LAYER is greater than 1.
> > +
> > +.. _v4l2-hevc-hierarchical-coding-type:
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_TYPE``
> > +(enum)
> > +
> > +enum v4l2_mpeg_video_hevc_hier_coding_type -
> > +Selects the hierarchical coding type for encoding. Possible values are:
> > +
> > +.. raw:: latex
> > +
> > +\begin{adjustbox}{width=\columnwidth}
> > +
> > +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> > +
> > +.. flat-table::
> > +:header-rows:  0
> > +:stub-columns: 0
> > +
> > +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_B``
> > +  - Use the B frame for hierarchical coding.
> > +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_P``
> > +  - Use the P frame for hierarchical coding.
> > +
> > +.. raw:: latex
> > +
> > +\end{adjustbox}
> > +
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER``
> > +Selects the hierarchical coding layer. In normal encoding
> > +(non-hierarchial coding), it should be zero. Possible values are 0 ~ 6.
> > +0 indicates HIERARCHICAL CODING LAYER 0, 1 indicates HIERARCHICAL 
> > CODING
> > +LAYER 1 and so on.
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER_QP``
> > +Indicates the hierarchical coding layer quantization parameter.
> > +For HEVC it can have a value of 0-51. Hence in the control value passed
> > +the LSB 16 bits will indicate the quantization parameter. The MSB 16 
> > bit
> > +will pass the layer(0-6) it is meant for.
> > +
> > +.. _v4l2-hevc-profile:
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_PROFILE``
> > +(enum)
> > +
> > +enum v4l2_mpeg_video_hevc_profile -
> > +Select the desired profile for HEVC encoder.
> > +
> > +.. raw:: latex
> > +
> > +\begin{adjustbox}{width=\columnwidth}
> > +
> > +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> > +
> > +.. flat-table::
> > +:header-rows:  0
> > +:stub-columns: 0
> > +
> > +* - ``V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN``
> > +  - Main profile.
> > +* - ``V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE``
> > +  - Main still picture profile.
> > +
> > +.. raw:: latex
> > +
> > +\end{adjustbox}
> > +
> > +
> > +.. _v4l2-hevc-level:
> > +
> > +``V4L2_CID_MPEG_VIDEO_HEVC_LEVEL``
> > +(enum)
> > +
> > +enum v4l2_mpeg_video_hevc_level -
> > +Select the desired level for HEVC encoder.
> > +
> > +.. raw:: latex
> > +
> > +\begin{adjustbox}{width=\columnwidth}
> > +
> > +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> > +
> > +.. flat-table::
> > +   :header-rows:  0
> > +:stub-columns: 0
> > +
> > +* - ``V4L2_MPEG_VIDEO_HEVC_LEVEL_1``
> > +  - Level 1.0
> > +* - ``V4L2_MPEG_VIDEO_HEVC_LEVEL_2``
> > +  - Level 2.0
> > +* - ``V4L2_MPEG_VIDEO_HEVC_LEVEL_2_1``
> > +  - Level 2.1
> > +* - ``V4L2_MPEG_VIDEO_HEVC_LEVEL_3``
> > +  - Level 3.0
> > +* - 

Re: [Patch v4 10/12] [media] v4l2: Add v4l2 control IDs for HEVC encoder

[Smitha T Murthy]

On Thu, 2017-04-06 at 15:14 +0200, Sylwester Nawrocki wrote:
> On 04/06/2017 08:11 AM, Smitha T Murthy wrote:
> > @@ -775,6 +832,47 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:return "VPX 
> > P-Frame QP Value";
> > case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:   return "VPX 
> > Profile";
> >  
> > +   /* HEVC controls */
> [...]
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_LF_SLICE_BOUNDARY:return "HEVC LF 
> > Across Slice Boundary or Not";
> 
> Please make sure the names are no longer than 31 characters to avoid
> truncation during control enumeration in user space.
> Data structures like struct v4l2_queryctrl, struct v4l2_query_ext_ctrl
> have only 32 bytes long array dedicated for the control name.

I will try to make the names less than 31 characters long without losing
the context. But there are many control names in this file which are
longer than 31 characters like
V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP,
V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_REF_PERIOD etc so I assumed it was
alright to have such long names. But I will shorten them as per your
suggestion.

> 
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_QP:  return "HEVC QP 
> > Values";
> 
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_TYPE: return "HEVC 
> > Hierarchical Coding Type";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER:return "HEVC 
> > Hierarchical Coding Layer";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER_QP:return "HEVC 
> > Hierarchical Layer QP";
> 
> How about s/HIERARCHICAL_/HIER_ for the above 3 control IDs?
> 
Ok I will change it.

> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER0_BITRATE:return "HEVC 
> > Hierarchical Lay 0 Bit Rate";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER1_BITRATE:return "HEVC 
> > Hierarchical Lay 1 Bit Rate";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER2_BITRATE:return "HEVC 
> > Hierarchical Lay 2 Bit Rate";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER3_BITRATE:return "HEVC 
> > Hierarchical Lay 3 Bit Rate";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER4_BITRATE:return "HEVC 
> > Hierarchical Lay 4 Bit Rate";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER5_BITRATE:return "HEVC 
> > Hierarchical Lay 5 Bit Rate";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER6_BITRATE:return "HEVC 
> > Hierarchical Lay 6 Bit Rate";
> 
> Using single letter L instead of LAYER would make the control ID shorter
> and more consistent with existing controls, e.g. 
> V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BITRATE.
> 
Ok I will change it.

> > +   case V4L2_CID_MPEG_VIDEO_HEVC_SIGN_DATA_HIDING: return "HEVC 
> > Sign Data Hiding";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_GENERAL_PB:   return "HEVC 
> > General PB";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_TEMPORAL_ID:  return "HEVC 
> > Temporal ID";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_STRONG_SMOOTHING: return "HEVC 
> > Strong Intra Smoothing";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_INTRA_PU_SPLIT:   return "HEVC 
> > Intra PU Split";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_TMV_PREDICTION:   return "HEVC 
> > TMV Prediction";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_MAX_NUM_MERGE_MV_MINUS1:  return "HEVC 
> > Max Number of Candidate MVs";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_WITHOUT_STARTCODE:return "HEVC 
> > ENC Without Startcode";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_REFRESH_PERIOD:   return "HEVC 
> > Num of I Frame b/w 2 IDR";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_LF_BETA_OFFSET_DIV2:  return "HEVC 
> > Loop Filter Beta Offset";
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_LF_TC_OFFSET_DIV2:return "HEVC 
> > Loop Filter tc Offset";
> 
> s/tc/Tc or s/tc/TC ?
> 
I will correct it.
> > +   case V4L2_CID_MPEG_VIDEO_HEVC_SIZE_OF_LENGTH_FIELD: return "HEVC 
> > Size of Length Field";
> 
> --
> Thanks,
> Sylwester
> 
Thank you for the review.

Regards,
Smitha

RE: [PATCH 2/5] media: Add support for CXD2880 SPI I/F

[Takiguchi, Yasunari]

Dear All

Our patches consists of the following items.
  [PATCH 1/5] dt-bindings: media: Add document file for CXD2880 SPI I/F
  [PATCH 2/5] media: Add support for CXD2880 SPI I/F
  [PATCH 3/5] media: Add suppurt for CXD2880
  [PATCH 4/5] media: Add suppurt for CXD2880 DVB-T2/T functions
  [PATCH 5/5] media: Update MAINTAINERS file for CXD2880

It is necessary to apply all patches before compiling kernel with our code.

Could you re-compile after applying above the patches.

Best Regards,
Takiguchci

Re: [PATCH] [media] coda: do not enumerate YUYV if VDOA is not available

[Lucas Stach]

Am Donnerstag, den 06.04.2017, 16:03 +0200 schrieb Philipp Zabel:
> TRY_FMT already disables the YUYV format if the VDOA is not available.
> ENUM_FMT must do the same.
> 
> Fixes: d40e98c13b3e ("[media] coda: support YUYV output if VDOA is used")
> Signed-off-by: Philipp Zabel 

Reviewed-by: Lucas Stach 

> ---
>  drivers/media/platform/coda/coda-common.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/platform/coda/coda-common.c 
> b/drivers/media/platform/coda/coda-common.c
> index eb6548f46cbac..1c155b055ea30 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -386,6 +386,7 @@ static int coda_enum_fmt(struct file *file, void *priv,
>  {
>   struct video_device *vdev = video_devdata(file);
>   const struct coda_video_device *cvd = to_coda_video_device(vdev);
> + struct coda_ctx *ctx = fh_to_ctx(priv);
>   const u32 *formats;
>  
>   if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> @@ -398,6 +399,11 @@ static int coda_enum_fmt(struct file *file, void *priv,
>   if (f->index >= CODA_MAX_FORMATS || formats[f->index] == 0)
>   return -EINVAL;
>  
> + /* Skip YUYV if the vdoa is not available */
> + if (!ctx->vdoa && f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + formats[f->index] == V4L2_PIX_FMT_YUYV)
> + return -EINVAL;
> +
>   f->pixelformat = formats[f->index];
>  
>   return 0;

Re: [PATCH] [media] media-entity: only call dev_dbg_obj if mdev is not NULL

[Sakari Ailus]

Hi Helen,

On Thu, Apr 06, 2017 at 04:32:00PM -0300, Helen Koike wrote:
> Fix kernel Oops NULL pointer deference
> Call dev_dbg_obj only after checking if gobj->mdev is not NULL
> 
> Signed-off-by: Helen Koike 
> ---
>  drivers/media/media-entity.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 5640ca2..bc44193 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -199,12 +199,12 @@ void media_gobj_create(struct media_device *mdev,
>  
>  void media_gobj_destroy(struct media_gobj *gobj)
>  {
> - dev_dbg_obj(__func__, gobj);
> -
>   /* Do nothing if the object is not linked. */
>   if (gobj->mdev == NULL)
>   return;
>  
> + dev_dbg_obj(__func__, gobj);
> +
>   gobj->mdev->topology_version++;
>  
>   /* Remove the object from mdev list */

Where is media_gobj_destroy() called with an object with NULL mdev?

I do not object to the change, but would like to know because I don't think
it's supposed to happen.

There are issues though, until the patches fixing object referencing are
finished and merged. Unfortunately I haven't been able to work on those
recently, will pick them up again soon...

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH 2/5] media: Add support for CXD2880 SPI I/F

[kbuild test robot]

Hi Yasunari,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.11-rc5 next-20170406]
[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/Yasunari-Takiguchi-sony-com/dt-bindings-media-Add-document-file-and-driver/20170407-064503
base:   git://linuxtv.org/media_tree.git master
config: x86_64-acpi-redef (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: the 
linux-review/Yasunari-Takiguchi-sony-com/dt-bindings-media-Add-document-file-and-driver/20170407-064503
 HEAD 9a65bc5ba5fc3ec361e835324009bf7097391121 builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

>> drivers/media//spi/cxd2880-spi.c:32:21: fatal error: cxd2880.h: No such file 
>> or directory
#include "cxd2880.h"
^
   compilation terminated.

vim +32 drivers/media//spi/cxd2880-spi.c

16   * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, 
BUT
17   * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
18   * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND 
ON
19   * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR 
TORT
20   * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE 
USE OF
21   * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
22   *
23   * You should have received a copy of the GNU General Public License 
along
24   * with this program; if not, see <http://www.gnu.org/licenses/>.
25   */
26  
27  #include 
28  
29  #include "dvb_demux.h"
30  #include "dmxdev.h"
31  #include "dvb_frontend.h"
  > 32  #include "cxd2880.h"
33  
34  #define CXD2880_MAX_FILTER_SIZE 32
35  #define BURST_WRITE_MAX 128
36  #define MAX_TRANS_PACKET 300
37  
38  struct cxd2880_ts_buf_info {
39  u8 read_ready;
40  u8 almost_full;

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


.config.gz
Description: application/gzip

[PATCH 3/3] staging: atomisp: move mipi_info assignment to next line in __get_asd_from_port()

[Daeseok Youn]

The line which is initializing mipi_info variable is too long
to read. It would be placed in next line.

Signed-off-by: Daeseok Youn 
---
This series of patches are related to previous patches:
[1] https://lkml.org/lkml/2017/3/27/159
[2] https://lkml.org/lkml/2017/3/30/1068
[3] https://lkml.org/lkml/2017/3/30/1069

 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 4af76b5..2208477 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -532,9 +532,11 @@ static void clear_irq_reg(struct atomisp_device *isp)
/* Check which isp subdev to send eof */
for (i = 0; i < isp->num_of_streams; i++) {
struct atomisp_sub_device *asd = >asd[i];
-   struct camera_mipi_info *mipi_info =
-   atomisp_to_sensor_mipi_info(
-   isp->inputs[asd->input_curr].camera);
+   struct camera_mipi_info *mipi_info;
+
+   mipi_info = atomisp_to_sensor_mipi_info(
+   isp->inputs[asd->input_curr].camera);
+
if (asd->streaming == ATOMISP_DEVICE_STREAMING_ENABLED &&
__get_mipi_port(isp, mipi_info->port) == port) {
return asd;
-- 
1.9.1

[PATCH 2/3] staging: atomisp: replace ">asd[i]" with "asd" in __get_asd_from_port()

[Daeseok Youn]

The address of isp->asd[i] is already assigned to
local "asd" variable. ">asd[i]" would be replaced with
just "asd".

Signed-off-by: Daeseok Youn 
---
This series of patches are related to previous patches:
[1] https://lkml.org/lkml/2017/3/27/159
[2] https://lkml.org/lkml/2017/3/30/1068
[3] https://lkml.org/lkml/2017/3/30/1069

 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index c3d0596..4af76b5 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -535,9 +535,9 @@ static void clear_irq_reg(struct atomisp_device *isp)
struct camera_mipi_info *mipi_info =
atomisp_to_sensor_mipi_info(
isp->inputs[asd->input_curr].camera);
-   if (isp->asd[i].streaming == ATOMISP_DEVICE_STREAMING_ENABLED &&
+   if (asd->streaming == ATOMISP_DEVICE_STREAMING_ENABLED &&
__get_mipi_port(isp, mipi_info->port) == port) {
-   return >asd[i];
+   return asd;
}
}
 
-- 
1.9.1

[PATCH 1/3] staging: atomisp: remove enable_isp_irq function and add disable_isp_irq

[Daeseok Youn]

Enable/Disable ISP irq is switched with "enable" parameter of
enable_isp_irq(). It would be better splited to two such as
enable_isp_irq()/disable_isp_irq().

But the enable_isp_irq() is no use in atomisp_cmd.c file.
So remove the enable_isp_irq() function and add
disable_isp_irq function only.

Signed-off-by: Daeseok Youn 
---
This series of patches are related to previous patches:
[1] https://lkml.org/lkml/2017/3/27/159
[2] https://lkml.org/lkml/2017/3/30/1068
[3] https://lkml.org/lkml/2017/3/30/1069

 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 36 ++
 1 file changed, 9 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 0ba5d8b..c3d0596 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -375,34 +375,16 @@ int atomisp_reset(struct atomisp_device *isp)
 }
 
 /*
- * interrupt enable/disable functions
+ * interrupt disable functions
  */
-static void enable_isp_irq(enum hrt_isp_css_irq irq, bool enable)
-{
-   if (enable) {
-   irq_enable_channel(IRQ0_ID, irq);
-   /*sh_css_hrt_irq_enable(irq, true, false);*/
-   switch (irq) { /*We only have sp interrupt right now*/
-   case hrt_isp_css_irq_sp:
-   /*sh_css_hrt_irq_enable_sp(true);*/
-   cnd_sp_irq_enable(SP0_ID, true);
-   break;
-   default:
-   break;
-   }
+static void disable_isp_irq(enum hrt_isp_css_irq irq)
+{
+   irq_disable_channel(IRQ0_ID, irq);
 
-   } else {
-   /*sh_css_hrt_irq_disable(irq);*/
-   irq_disable_channel(IRQ0_ID, irq);
-   switch (irq) {
-   case hrt_isp_css_irq_sp:
-   /*sh_css_hrt_irq_enable_sp(false);*/
-   cnd_sp_irq_enable(SP0_ID, false);
-   break;
-   default:
-   break;
-   }
-   }
+   if (irq != hrt_isp_css_irq_sp)
+   return;
+
+   cnd_sp_irq_enable(SP0_ID, false);
 }
 
 /*
@@ -1415,7 +1397,7 @@ static void __atomisp_css_recover(struct atomisp_device 
*isp, bool isp_timeout)
}
 
/* clear irq */
-   enable_isp_irq(hrt_isp_css_irq_sp, false);
+   disable_isp_irq(hrt_isp_css_irq_sp);
clear_isp_irq(hrt_isp_css_irq_sp);
 
/* Set the SRSE to 3 before resetting */
-- 
1.9.1