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 <[email protected]> > Acked-by: Benoit Parrot <[email protected]> # 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 = &isd->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, &vep); > + ret = v4l2_fwnode_endpoint_parse(fwn, &vfwn); > 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 series, I wouldn't rename the variable. > notifier->subdevs = devm_kcalloc( > dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL); [snip] > @@ -2219,12 +2220,12 @@ static int isp_probe(struct platform_device *pdev) > if (IS_ERR(isp->syscon)) > return PTR_ERR(isp->syscon); > > - ret = of_property_read_u32_index(pdev->dev.of_node, "syscon", 1, > - &isp->syscon_offset); > + ret = of_property_read_u32_index(pdev->dev.of_node, > + "syscon", 1, &isp->syscon_offset); This change doesn't seem to be needed. > if (ret) > return ret; > > - ret = isp_of_parse_nodes(&pdev->dev, &isp->notifier); > + ret = isp_fwnodes_parse(&pdev->dev, &isp->notifier); > if (ret < 0) > return ret; > [snip] > diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c > b/drivers/media/platform/xilinx/xilinx-vipp.c index feb3b2f..6a2721b 100644 > --- a/drivers/media/platform/xilinx/xilinx-vipp.c > +++ b/drivers/media/platform/xilinx/xilinx-vipp.c [snip] > @@ -103,9 +103,10 @@ static int xvip_graph_build_one(struct > xvip_composite_device *xdev, * the link. > */ > if (link.local_port >= local->num_pads) { > - dev_err(xdev->dev, "invalid port number %u on %s\n", > - link.local_port, link.local_node->full_name); > - v4l2_of_put_link(&link); > + dev_err(xdev->dev, "invalid port number %u for %s\n", > + link.local_port, > + to_of_node(link.local_node)->full_name); This makes me believe that we're missing a fwnode_full_name() function. > + v4l2_fwnode_put_link(&link); > ret = -EINVAL; > break; > } [snip] > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h > index a675d8a..bc9cf51 100644 > --- a/include/media/v4l2-fwnode.h > +++ b/include/media/v4l2-fwnode.h > @@ -17,10 +17,10 @@ > #ifndef _V4L2_FWNODE_H > #define _V4L2_FWNODE_H > > +#include <linux/errno.h> > +#include <linux/fwnode.h> > #include <linux/list.h> > #include <linux/types.h> > -#include <linux/errno.h> > -#include <linux/of_graph.h> > > #include <media/v4l2-mediabus.h> This probably belongs to another patch (at least the alphabetical sorting does). -- Regards, Laurent Pinchart
