Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-09-01 Thread Sakari Ailus
Hi Laurent,

On Fri, Sep 01, 2017 at 11:55:43AM +0300, Laurent Pinchart wrote:
...
> > >> +static int parse_endpoint(
> > >> +struct device *dev, struct v4l2_async_notifier *notifier,
> > >> +struct fwnode_handle *endpoint, unsigned int asd_struct_size,
> > >> +int (*parse_single)(struct device *dev,
> > >> +struct v4l2_fwnode_endpoint *vep,
> > >> +struct v4l2_async_subdev *asd))
> > >> +{
> > >> +struct v4l2_async_subdev *asd;
> > >> +struct v4l2_fwnode_endpoint *vep;
> > >> +int ret = 0;
> > >> +
> > >> +asd = kzalloc(asd_struct_size, GFP_KERNEL);
> > >> +if (!asd)
> > >> +return -ENOMEM;
> > >> +
> > >> +asd->match.fwnode.fwnode =
> > >> +fwnode_graph_get_remote_port_parent(endpoint);
> > >> +if (!asd->match.fwnode.fwnode) {
> > >> +dev_warn(dev, "bad remote port parent\n");
> > >> +ret = -EINVAL;
> > >> +goto out_err;
> > >> +}
> > >> +
> > >> +/* Ignore endpoints the parsing of which failed. */
> > > 
> > > You don't ignore them anymore, the comment should be updated.
> > 
> > Hmm. I actually intended to do something else about this. :-) As there's a
> > single error code, handling that would need to be done a little bit
> > differently right now.
> > 
> > I'd print a warning and proceed. What do you think?
> > 
> > Even if there's a bad DT endpoint, that shouldn't prevent the rest from
> > working, right?
> 
> I think it should, to make sure we catch DT issues. We both know how many 
> vendors don't care about warnings, so I'd rather fail completely to ensure DT 
> will be fixed (and even ten I wouldn't be surprised if some vendors patched 
> the code to remove the check instead of fixing their DT ;-)).

If they test the DTS, they should find out that the device does not work.

If they do not, some devices will work even if others fail.

Therefore I don't see why everything should fail when a part of faulty.
Extending that a little, you should also halt the system to make sure the
problem will be noticed. :-)

> > >> @@ -121,6 +122,21 @@ int v4l2_async_notifier_register(struct v4l2_device
> > >> *v4l2_dev, void v4l2_async_notifier_unregister(struct
> > >> v4l2_async_notifier *notifier);
> > >> 
> > >>  /**
> > >> + * v4l2_async_notifier_release - release notifier resources
> > >> + * @notifier: pointer to  v4l2_async_notifier
> > > 
> > > That's quite obvious given the type of the argument. It would be much more
> > > useful to tell which notifier pointer this function expects (although in
> > > this case it should be obvious too): "(pointer to )?the notifier whose
> > > resources will be released".
> > 
> > This fully matches to the documentation elsewhere in the same file. :-)
> 
> Feel free to fix the rest of the file :-)

That's out of scope of this patch.

> > > "The function can be called multiple times to populate the same notifier
> > > from endpoints of different @dev devices before registering the notifier.
> > > It can't be called anymore once the notifier has been registered."
> > 
> > I don't think there's really a use case for calling this for more than one
> > device, is there?
> 
> I don't have one in mind, but I was wondering. If there isn't then you don't 
> need notifier_realloc(), which could be moved to the next patch.

I don't think there's even benefit from moving it to the next patch, it
just adds to the reviewable code, nothing else.

-- 
Regards,

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


Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-09-01 Thread Laurent Pinchart
Hi Sakari,

On Tuesday, 29 August 2017 17:31:21 EEST Sakari Ailus wrote:
> On Tue, Aug 29, 2017 at 05:02:54PM +0300, Laurent Pinchart wrote:
> > On Tuesday, 29 August 2017 14:03:12 EEST Sakari Ailus wrote:
> >> The current practice is that drivers iterate over their endpoints and
> >> parse each endpoint separately. This is very similar in a number of
> >> drivers, implement a generic function for the job. Driver specific
> >> matters can be taken into account in the driver specific callback.
> >> 
> >> Convert the omap3isp as an example.
> >> 
> >> Signed-off-by: Sakari Ailus 
> >> ---
> >> 
> >>  drivers/media/platform/omap3isp/isp.c | 115 ++-
> >>  drivers/media/platform/omap3isp/isp.h |   5 +-
> >>  drivers/media/v4l2-core/v4l2-async.c  |  16 +
> >>  drivers/media/v4l2-core/v4l2-fwnode.c | 132 
> >>  include/media/v4l2-async.h| > 20 +-
> >>  include/media/v4l2-fwnode.h   |  39 ++
> >>  6 files changed, 242 insertions(+), 85 deletions(-)

[snip]

> >> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> >> b/drivers/media/v4l2-core/v4l2-fwnode.c index 706f9e7b90f1..39a587c6992a
> >> 100644
> >> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> >> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c

[snip]

> >> +static int parse_endpoint(
> >> +  struct device *dev, struct v4l2_async_notifier *notifier,
> >> +  struct fwnode_handle *endpoint, unsigned int asd_struct_size,
> >> +  int (*parse_single)(struct device *dev,
> >> +  struct v4l2_fwnode_endpoint *vep,
> >> +  struct v4l2_async_subdev *asd))
> >> +{
> >> +  struct v4l2_async_subdev *asd;
> >> +  struct v4l2_fwnode_endpoint *vep;
> >> +  int ret = 0;
> >> +
> >> +  asd = kzalloc(asd_struct_size, GFP_KERNEL);
> >> +  if (!asd)
> >> +  return -ENOMEM;
> >> +
> >> +  asd->match.fwnode.fwnode =
> >> +  fwnode_graph_get_remote_port_parent(endpoint);
> >> +  if (!asd->match.fwnode.fwnode) {
> >> +  dev_warn(dev, "bad remote port parent\n");
> >> +  ret = -EINVAL;
> >> +  goto out_err;
> >> +  }
> >> +
> >> +  /* Ignore endpoints the parsing of which failed. */
> > 
> > You don't ignore them anymore, the comment should be updated.
> 
> Hmm. I actually intended to do something else about this. :-) As there's a
> single error code, handling that would need to be done a little bit
> differently right now.
> 
> I'd print a warning and proceed. What do you think?
> 
> Even if there's a bad DT endpoint, that shouldn't prevent the rest from
> working, right?

I think it should, to make sure we catch DT issues. We both know how many 
vendors don't care about warnings, so I'd rather fail completely to ensure DT 
will be fixed (and even ten I wouldn't be surprised if some vendors patched 
the code to remove the check instead of fixing their DT ;-)).

> >> +  vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
> >> +  if (IS_ERR(vep)) {
> >> +  ret = PTR_ERR(vep);
> >> +  dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
> >> +   ret);
> >> +  goto out_err;
> >> +  }
> >> +
> >> +  ret = parse_single(dev, vep, asd);
> >> +  v4l2_fwnode_endpoint_free(vep);
> >> +  if (ret) {
> >> +  dev_warn(dev, "driver could not parse endpoint (%d)\n", ret);
> >> +  goto out_err;
> >> +  }
> >> +
> >> +  asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> >> +  notifier->subdevs[notifier->num_subdevs] = asd;
> >> +  notifier->num_subdevs++;
> >> +
> >> +  return 0;
> >> +
> >> +out_err:
> >> +  fwnode_handle_put(asd->match.fwnode.fwnode);
> >> +  kfree(asd);
> >> +
> >> +  return ret;
> >> +}

[snip]

> >> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> >> index c69d8c8a66d0..4a44ab47ab04 100644
> >> --- a/include/media/v4l2-async.h
> >> +++ b/include/media/v4l2-async.h


> >> @@ -121,6 +122,21 @@ int v4l2_async_notifier_register(struct v4l2_device
> >> *v4l2_dev, void v4l2_async_notifier_unregister(struct
> >> v4l2_async_notifier *notifier);
> >> 
> >>  /**
> >> + * v4l2_async_notifier_release - release notifier resources
> >> + * @notifier: pointer to  v4l2_async_notifier
> > 
> > That's quite obvious given the type of the argument. It would be much more
> > useful to tell which notifier pointer this function expects (although in
> > this case it should be obvious too): "(pointer to )?the notifier whose
> > resources will be released".
> 
> This fully matches to the documentation elsewhere in the same file. :-)

Feel free to fix the rest of the file :-)

> I'll replace the text with yours.
> 
> >> + *
> >> + * Release memory resources related to a notifier, including the async
> >> + * sub-devices allocated for the purposes of the notifier. The user is
> >> + * responsible for releasing the notifier's resources after calling
> >> + * @v4l2_async_notifier_parse_fwnode_endpoints.
> >> + *
> >> + * There is no harm 

Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-08-29 Thread Sakari Ailus
Hi Laurent,

On Tue, Aug 29, 2017 at 05:02:54PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.

Thanks for the review!

> 
> On Tuesday, 29 August 2017 14:03:12 EEST Sakari Ailus wrote:
> > The current practice is that drivers iterate over their endpoints and
> > parse each endpoint separately. This is very similar in a number of
> > drivers, implement a generic function for the job. Driver specific matters
> > can be taken into account in the driver specific callback.
> > 
> > Convert the omap3isp as an example.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/platform/omap3isp/isp.c | 115 ++---
> >  drivers/media/platform/omap3isp/isp.h |   5 +-
> >  drivers/media/v4l2-core/v4l2-async.c  |  16 +
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 132
> > ++ include/media/v4l2-async.h| 
> > 20 +-
> >  include/media/v4l2-fwnode.h   |  39 ++
> >  6 files changed, 242 insertions(+), 85 deletions(-)
> > 
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c index 1a428fe9f070..a546cf774d40
> > 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -2001,6 +2001,7 @@ static int isp_remove(struct platform_device *pdev)
> > __omap3isp_put(isp, false);
> > 
> > media_entity_enum_cleanup(>crashed);
> > +   v4l2_async_notifier_release(>notifier);
> > 
> > return 0;
> >  }
> > @@ -2011,44 +2012,41 @@ enum isp_of_phy {
> > ISP_OF_PHY_CSIPHY2,
> >  };
> > 
> > -static int isp_fwnode_parse(struct device *dev, struct fwnode_handle
> > *fwnode, -  struct isp_async_subdev *isd)
> > +static int isp_fwnode_parse(struct device *dev,
> > +   struct v4l2_fwnode_endpoint *vep,
> > +   struct v4l2_async_subdev *asd)
> >  {
> > +   struct isp_async_subdev *isd =
> > +   container_of(asd, struct isp_async_subdev, asd);
> > struct isp_bus_cfg *buscfg = >bus;
> > -   struct v4l2_fwnode_endpoint vep;
> > -   unsigned int i;
> > -   int ret;
> > bool csi1 = false;
> > -
> > -   ret = v4l2_fwnode_endpoint_parse(fwnode, );
> > -   if (ret)
> > -   return ret;
> > +   unsigned int i;
> > 
> > dev_dbg(dev, "parsing endpoint %pOF, interface %u\n",
> > -   to_of_node(fwnode), vep.base.port);
> > +   to_of_node(vep->base.local_fwnode), vep->base.port);
> > 
> > -   switch (vep.base.port) {
> > +   switch (vep->base.port) {
> > case ISP_OF_PHY_PARALLEL:
> > buscfg->interface = ISP_INTERFACE_PARALLEL;
> > buscfg->bus.parallel.data_lane_shift =
> > -   vep.bus.parallel.data_shift;
> > +   vep->bus.parallel.data_shift;
> > buscfg->bus.parallel.clk_pol =
> > -   !!(vep.bus.parallel.flags
> > +   !!(vep->bus.parallel.flags
> >& V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > buscfg->bus.parallel.hs_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > +   !!(vep->bus.parallel.flags & 
> > V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > buscfg->bus.parallel.vs_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > +   !!(vep->bus.parallel.flags & 
> > V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > buscfg->bus.parallel.fld_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> > +   !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> > buscfg->bus.parallel.data_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> > -   buscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;
> > +   !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> > +   buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
> > break;
> > 
> > case ISP_OF_PHY_CSIPHY1:
> > case ISP_OF_PHY_CSIPHY2:
> > -   switch (vep.bus_type) {
> > +   switch (vep->bus_type) {
> > case V4L2_MBUS_CCP2:
> > case V4L2_MBUS_CSI1:
> > dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
> > @@ -2060,11 +2058,11 @@ static int isp_fwnode_parse(struct device *dev,
> > struct fwnode_handle *fwnode, break;
> > default:
> > dev_err(dev, "unsupported bus type %u\n",
> > -   vep.bus_type);
> > +   vep->bus_type);
> > return -EINVAL;
> > }
> > 
> > -   switch (vep.base.port) {
> > +   switch (vep->base.port) {
> > case ISP_OF_PHY_CSIPHY1:
> > if (csi1)
> > buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
> > @@ 

Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-08-29 Thread Laurent Pinchart
Hi Sakari,

Thank you for the patch.

On Tuesday, 29 August 2017 14:03:12 EEST Sakari Ailus wrote:
> The current practice is that drivers iterate over their endpoints and
> parse each endpoint separately. This is very similar in a number of
> drivers, implement a generic function for the job. Driver specific matters
> can be taken into account in the driver specific callback.
> 
> Convert the omap3isp as an example.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/platform/omap3isp/isp.c | 115 ++---
>  drivers/media/platform/omap3isp/isp.h |   5 +-
>  drivers/media/v4l2-core/v4l2-async.c  |  16 +
>  drivers/media/v4l2-core/v4l2-fwnode.c | 132
> ++ include/media/v4l2-async.h| 
> 20 +-
>  include/media/v4l2-fwnode.h   |  39 ++
>  6 files changed, 242 insertions(+), 85 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 1a428fe9f070..a546cf774d40
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2001,6 +2001,7 @@ static int isp_remove(struct platform_device *pdev)
>   __omap3isp_put(isp, false);
> 
>   media_entity_enum_cleanup(>crashed);
> + v4l2_async_notifier_release(>notifier);
> 
>   return 0;
>  }
> @@ -2011,44 +2012,41 @@ enum isp_of_phy {
>   ISP_OF_PHY_CSIPHY2,
>  };
> 
> -static int isp_fwnode_parse(struct device *dev, struct fwnode_handle
> *fwnode, -struct isp_async_subdev *isd)
> +static int isp_fwnode_parse(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd)
>  {
> + struct isp_async_subdev *isd =
> + container_of(asd, struct isp_async_subdev, asd);
>   struct isp_bus_cfg *buscfg = >bus;
> - struct v4l2_fwnode_endpoint vep;
> - unsigned int i;
> - int ret;
>   bool csi1 = false;
> -
> - ret = v4l2_fwnode_endpoint_parse(fwnode, );
> - if (ret)
> - return ret;
> + unsigned int i;
> 
>   dev_dbg(dev, "parsing endpoint %pOF, interface %u\n",
> - to_of_node(fwnode), vep.base.port);
> + to_of_node(vep->base.local_fwnode), vep->base.port);
> 
> - switch (vep.base.port) {
> + switch (vep->base.port) {
>   case ISP_OF_PHY_PARALLEL:
>   buscfg->interface = ISP_INTERFACE_PARALLEL;
>   buscfg->bus.parallel.data_lane_shift =
> - vep.bus.parallel.data_shift;
> + vep->bus.parallel.data_shift;
>   buscfg->bus.parallel.clk_pol =
> - !!(vep.bus.parallel.flags
> + !!(vep->bus.parallel.flags
>  & V4L2_MBUS_PCLK_SAMPLE_FALLING);
>   buscfg->bus.parallel.hs_pol =
> - !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> + !!(vep->bus.parallel.flags & 
> V4L2_MBUS_VSYNC_ACTIVE_LOW);
>   buscfg->bus.parallel.vs_pol =
> - !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> + !!(vep->bus.parallel.flags & 
> V4L2_MBUS_HSYNC_ACTIVE_LOW);
>   buscfg->bus.parallel.fld_pol =
> - !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> + !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
>   buscfg->bus.parallel.data_pol =
> - !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> - buscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;
> + !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> + buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
>   break;
> 
>   case ISP_OF_PHY_CSIPHY1:
>   case ISP_OF_PHY_CSIPHY2:
> - switch (vep.bus_type) {
> + switch (vep->bus_type) {
>   case V4L2_MBUS_CCP2:
>   case V4L2_MBUS_CSI1:
>   dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
> @@ -2060,11 +2058,11 @@ static int isp_fwnode_parse(struct device *dev,
> struct fwnode_handle *fwnode, break;
>   default:
>   dev_err(dev, "unsupported bus type %u\n",
> - vep.bus_type);
> + vep->bus_type);
>   return -EINVAL;
>   }
> 
> - switch (vep.base.port) {
> + switch (vep->base.port) {
>   case ISP_OF_PHY_CSIPHY1:
>   if (csi1)
>   buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
> @@ -2080,47 +2078,47 @@ static int isp_fwnode_parse(struct device *dev,
> struct fwnode_handle *fwnode, }
>   if (csi1) {
>   buscfg->bus.ccp2.lanecfg.clk.pos =
> - 

[PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-08-29 Thread Sakari Ailus
The current practice is that drivers iterate over their endpoints and
parse each endpoint separately. This is very similar in a number of
drivers, implement a generic function for the job. Driver specific matters
can be taken into account in the driver specific callback.

Convert the omap3isp as an example.

Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/omap3isp/isp.c | 115 ++---
 drivers/media/platform/omap3isp/isp.h |   5 +-
 drivers/media/v4l2-core/v4l2-async.c  |  16 +
 drivers/media/v4l2-core/v4l2-fwnode.c | 132 ++
 include/media/v4l2-async.h|  20 +-
 include/media/v4l2-fwnode.h   |  39 ++
 6 files changed, 242 insertions(+), 85 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 1a428fe9f070..a546cf774d40 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2001,6 +2001,7 @@ static int isp_remove(struct platform_device *pdev)
__omap3isp_put(isp, false);
 
media_entity_enum_cleanup(>crashed);
+   v4l2_async_notifier_release(>notifier);
 
return 0;
 }
@@ -2011,44 +2012,41 @@ enum isp_of_phy {
ISP_OF_PHY_CSIPHY2,
 };
 
-static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
-   struct isp_async_subdev *isd)
+static int isp_fwnode_parse(struct device *dev,
+   struct v4l2_fwnode_endpoint *vep,
+   struct v4l2_async_subdev *asd)
 {
+   struct isp_async_subdev *isd =
+   container_of(asd, struct isp_async_subdev, asd);
struct isp_bus_cfg *buscfg = >bus;
-   struct v4l2_fwnode_endpoint vep;
-   unsigned int i;
-   int ret;
bool csi1 = false;
-
-   ret = v4l2_fwnode_endpoint_parse(fwnode, );
-   if (ret)
-   return ret;
+   unsigned int i;
 
dev_dbg(dev, "parsing endpoint %pOF, interface %u\n",
-   to_of_node(fwnode), vep.base.port);
+   to_of_node(vep->base.local_fwnode), vep->base.port);
 
-   switch (vep.base.port) {
+   switch (vep->base.port) {
case ISP_OF_PHY_PARALLEL:
buscfg->interface = ISP_INTERFACE_PARALLEL;
buscfg->bus.parallel.data_lane_shift =
-   vep.bus.parallel.data_shift;
+   vep->bus.parallel.data_shift;
buscfg->bus.parallel.clk_pol =
-   !!(vep.bus.parallel.flags
+   !!(vep->bus.parallel.flags
   & V4L2_MBUS_PCLK_SAMPLE_FALLING);
buscfg->bus.parallel.hs_pol =
-   !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+   !!(vep->bus.parallel.flags & 
V4L2_MBUS_VSYNC_ACTIVE_LOW);
buscfg->bus.parallel.vs_pol =
-   !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+   !!(vep->bus.parallel.flags & 
V4L2_MBUS_HSYNC_ACTIVE_LOW);
buscfg->bus.parallel.fld_pol =
-   !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+   !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
buscfg->bus.parallel.data_pol =
-   !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
-   buscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;
+   !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+   buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
break;
 
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
-   switch (vep.bus_type) {
+   switch (vep->bus_type) {
case V4L2_MBUS_CCP2:
case V4L2_MBUS_CSI1:
dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
@@ -2060,11 +2058,11 @@ static int isp_fwnode_parse(struct device *dev, struct 
fwnode_handle *fwnode,
break;
default:
dev_err(dev, "unsupported bus type %u\n",
-   vep.bus_type);
+   vep->bus_type);
return -EINVAL;
}
 
-   switch (vep.base.port) {
+   switch (vep->base.port) {
case ISP_OF_PHY_CSIPHY1:
if (csi1)
buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
@@ -2080,47 +2078,47 @@ static int isp_fwnode_parse(struct device *dev, struct 
fwnode_handle *fwnode,
}
if (csi1) {
buscfg->bus.ccp2.lanecfg.clk.pos =
-   vep.bus.mipi_csi1.clock_lane;
+   vep->bus.mipi_csi1.clock_lane;