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

2017-04-10 Thread Sakari Ailus
Moi,

On 04/10/17 13:11, Mika Westerberg wrote:
> On Mon, Apr 10, 2017 at 12:59:36PM +0300, Sakari Ailus wrote:
>> Hi Mika and Laurent,
>>
>> On 04/10/17 12:21, Mika Westerberg wrote:
>>> On Sat, Apr 08, 2017 at 01:55:15AM +0300, Sakari Ailus wrote:
> My ACPI knowledge is limited, but don't ACPI nodes have 4 character names 
> that 
> can be combined in a string to create a full path ?

 There is something, yes, but the ACPI framework currently has no such
 functionality. I believe it could be implemented though. Cc Mika.
>>>
>>> All ACPI node names are 32-bit integers and those are combined to form a
>>> path, like \_SB.PCI0.I2C0 and so on. A single ACPI node name cannot be
>>> larger than 4 chars, though.
>>
>> On OF, each node has a full_node string attached to it. You could
>> produce a similar string on ACPI, it is not currently done. Adding such
>> a string to each fwnode would require some extra memory as well. I
>> wonder if that could be a Kconfig option.
>>
>> It would help debugging though.
>>
>> Providing this information to the user space has been proposed as well:
>> Devicetree spec defines the syntax for such strings. The user can use
>> that information for recognising a particular device in the system.
>>
>> The ACPI spec does, too, but it is limited to ACPI nodes and does not
>> address hierarchical data extensions. We'd define the syntax for those
>> ourselves.
>>
>> Mika: what do you think?
> 
> There is a function acpi_get_name() which you can use to extract the
> full name of the node. Why not investigate how to use that instead of
> duplicating the name in an ACPI node.
> 

acpi_get_name() would obviously be needed to produce such a string in
the first place.

acpi_get_name() puts the string to an existing buffer so it cannot be
used as such to return a pointer to a string (e.g. to be used for
snprintf()). Also, it only contains the device path of the device. The
data extension path matters here, too.

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com


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

2017-04-10 Thread Mika Westerberg
On Mon, Apr 10, 2017 at 12:59:36PM +0300, Sakari Ailus wrote:
> Hi Mika and Laurent,
> 
> On 04/10/17 12:21, Mika Westerberg wrote:
> > On Sat, Apr 08, 2017 at 01:55:15AM +0300, Sakari Ailus wrote:
> >>> My ACPI knowledge is limited, but don't ACPI nodes have 4 character names 
> >>> that 
> >>> can be combined in a string to create a full path ?
> >>
> >> There is something, yes, but the ACPI framework currently has no such
> >> functionality. I believe it could be implemented though. Cc Mika.
> > 
> > All ACPI node names are 32-bit integers and those are combined to form a
> > path, like \_SB.PCI0.I2C0 and so on. A single ACPI node name cannot be
> > larger than 4 chars, though.
> 
> On OF, each node has a full_node string attached to it. You could
> produce a similar string on ACPI, it is not currently done. Adding such
> a string to each fwnode would require some extra memory as well. I
> wonder if that could be a Kconfig option.
> 
> It would help debugging though.
> 
> Providing this information to the user space has been proposed as well:
> Devicetree spec defines the syntax for such strings. The user can use
> that information for recognising a particular device in the system.
> 
> The ACPI spec does, too, but it is limited to ACPI nodes and does not
> address hierarchical data extensions. We'd define the syntax for those
> ourselves.
> 
> Mika: what do you think?

There is a function acpi_get_name() which you can use to extract the
full name of the node. Why not investigate how to use that instead of
duplicating the name in an ACPI node.


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

2017-04-10 Thread Sakari Ailus
Hi Mika and Laurent,

On 04/10/17 12:21, Mika Westerberg wrote:
> On Sat, Apr 08, 2017 at 01:55:15AM +0300, Sakari Ailus wrote:
>>> My ACPI knowledge is limited, but don't ACPI nodes have 4 character names 
>>> that 
>>> can be combined in a string to create a full path ?
>>
>> There is something, yes, but the ACPI framework currently has no such
>> functionality. I believe it could be implemented though. Cc Mika.
> 
> All ACPI node names are 32-bit integers and those are combined to form a
> path, like \_SB.PCI0.I2C0 and so on. A single ACPI node name cannot be
> larger than 4 chars, though.

On OF, each node has a full_node string attached to it. You could
produce a similar string on ACPI, it is not currently done. Adding such
a string to each fwnode would require some extra memory as well. I
wonder if that could be a Kconfig option.

It would help debugging though.

Providing this information to the user space has been proposed as well:
Devicetree spec defines the syntax for such strings. The user can use
that information for recognising a particular device in the system.

The ACPI spec does, too, but it is limited to ACPI nodes and does not
address hierarchical data extensions. We'd define the syntax for those
ourselves.

Mika: what do you think?

On omap3isp and other drivers --- I think the solution for now should be
to assume OF instead. These drivers will be used on OF platforms only
anyway.

-- 
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com


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

2017-04-10 Thread Mika Westerberg
On Sat, Apr 08, 2017 at 01:55:15AM +0300, Sakari Ailus wrote:
> > My ACPI knowledge is limited, but don't ACPI nodes have 4 character names 
> > that 
> > can be combined in a string to create a full path ?
> 
> There is something, yes, but the ACPI framework currently has no such
> functionality. I believe it could be implemented though. Cc Mika.

All ACPI node names are 32-bit integers and those are combined to form a
path, like \_SB.PCI0.I2C0 and so on. A single ACPI node name cannot be
larger than 4 chars, though.


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

2017-04-07 Thread 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 

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

2017-04-07 Thread 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

2017-04-07 Thread 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 5/8] v4l: Switch from V4L2 OF not V4L2 fwnode API

2017-04-07 Thread 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 

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

2017-04-06 Thread Sakari Ailus
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
---help---
  Support for the Analog Devices ADV7604 video decoder.
 
@@ -324,6 +325,7 @@ config VIDEO_TC358743
tristate "Toshiba TC358743 decoder"
depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
select HDMI
+   select V4L2_FWNODE
---help---
  Support for the Toshiba TC358743 HDMI to MIPI CSI-2 bridge.
 
@@ -333,6 +335,7 @@ config VIDEO_TC358743
 config VIDEO_TVP514X
tristate "Texas Instruments TVP514x video decoder"
depends on VIDEO_V4L2 && I2C
+   select V4L2_FWNODE
---help---
  This is a Video4Linux2 sensor-level driver for the TI TVP5146/47
  decoder. It is currently working with the TI OMAP3 camera
@@ -344,6 +347,7 @@ config VIDEO_TVP514X
 config VIDEO_TVP5150
tristate "Texas Instruments TVP5150 video decoder"
depends on VIDEO_V4L2 && I2C
+   select V4L2_FWNODE
---help---
  Support for the Texas Instruments TVP5150 video decoder.
 
@@ -353,6 +357,7 @@ config VIDEO_TVP5150
 config VIDEO_TVP7002
tristate "Texas Instruments TVP7002 video decoder"
depends on VIDEO_V4L2 && I2C
+   select V4L2_FWNODE
---help---
  Support for the Texas Instruments TVP7002 video decoder.
 
@@ -524,6 +529,7 @@ config VIDEO_OV2659
tristate "OmniVision OV2659 sensor support"
depends on VIDEO_V4L2 && I2C
depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
  OV2659 camera.
@@ -616,6 +622,7 @@ config VIDEO_MT9V032
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
select REGMAP_I2C
+   select V4L2_FWNODE
---help---
  This is a Video4Linux2 sensor-level driver for the Micron
  MT9V032 752x480 CMOS sensor.
@@ -663,6 +670,7 @@ config VIDEO_S5K4ECGX
 config VIDEO_S5K5BAF
tristate "Samsung S5K5BAF sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   select V4L2_FWNODE
---help---
  This is a V4L2 sensor-level driver for Samsung S5K5BAF 2M
  camera sensor with an embedded SoC image signal processor.
@@ -673,6 +681,7 @@ source "drivers/media/i2c/et8ek8/Kconfig"
 config VIDEO_S5C73M3
tristate "Samsung S5C73M3 sensor support"
depends on I2C && SPI && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+