Re: [PATCH v2 07/23] v4l: fwnode: Let the caller provide V4L2 fwnode endpoint

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jacopo,

On Wed, Sep 12, 2018 at 04:51:07PM +0200, jacopo mondi wrote:
> Hi Sakari,
> 
> On Mon, Aug 27, 2018 at 12:29:44PM +0300, Sakari Ailus wrote:
> > Instead of allocating the V4L2 fwnode endpoint in
> > v4l2_fwnode_endpoint_alloc_parse, let the caller to do this. This allows
> > setting default parameters for the endpoint which is a very common need
> > for drivers.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/media/i2c/ov2659.c             | 14 +++++-----
> >  drivers/media/i2c/smiapp/smiapp-core.c | 26 +++++++++---------
> >  drivers/media/i2c/tc358743.c           | 26 +++++++++---------
> >  drivers/media/v4l2-core/v4l2-fwnode.c  | 49 +++++++++++++---------------------
> >  include/media/v4l2-fwnode.h            | 10 ++++---
> >  5 files changed, 60 insertions(+), 65 deletions(-)
> >
> 
> [snip]
> 
> > -struct v4l2_fwnode_endpoint *v4l2_fwnode_endpoint_alloc_parse(
> > -	struct fwnode_handle *fwnode)
> > +int v4l2_fwnode_endpoint_alloc_parse(
> > +	struct fwnode_handle *fwnode, struct v4l2_fwnode_endpoint *vep)
> 
> Looking at the resulting implementation of
> "v4l2_fwnode_endpoint_alloc_parse" and "v4l2_fwnode_endpoint_parse" I
> wonder if there's still value in keeping them separate... Now that in
> both cases the caller has to provide an v4l2_fwnode_endpoint, isn't it
> worth making a single function out of them, that behaves like
> "alloc_parse" is doing nowadays (allocates vep->link_frequencies
> conditionally on the presence of the "link-frequencies" property) ?

The problem with that would be that the caller would have to know if there
are variable length properties, i.e. the caller would always have to call
v4l2_fwnode_endpoint_free() once it no longer needs them. For quite a few
drivers this means immediately after calling the function which parsed
them.

I prefer to keep this simple for the drivers that need no such properties.

> 
> Or is the size of the allocated vep relevant in the async subdevice
> matching or registration process? I guess not, but I might be missing
> something...

It's not. The link frequencies are a pointer anyway.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux