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
