Re: [PATCH 2/3] media: v4l2-fwnode: cleanup functions that parse endpoints

[Mauro Carvalho Chehab <mchehab+samsung@kernel.org>]

Em Fri, 5 Oct 2018 11:01:18 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:

> Hi Mauro,
> 
> Feel free to ignore the comments on the first patch regarding the functions
> below. There are other issues there though.
> 
> On Thu, Oct 04, 2018 at 06:13:47PM -0400, Mauro Carvalho Chehab wrote:
> > There is already a typedef for the parse endpoint function.
> > However, instead of using it, it is redefined at the C file
> > (and on one of the function headers).
> > 
> > Replace them by the function typedef, in order to cleanup
> > several related coding style warnings.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > ---
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 64 ++++++++++++---------------
> >  include/media/v4l2-fwnode.h           | 19 ++++----
> >  2 files changed, 37 insertions(+), 46 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index 4e518d5fddd8..a7c2487154a4 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -596,12 +596,10 @@ EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
> >  
> >  static int
> >  v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
> > -		struct v4l2_async_notifier *notifier,
> > -		struct fwnode_handle *endpoint,
> > -		unsigned int asd_struct_size,
> > -		int (*parse_endpoint)(struct device *dev,
> > -				      struct v4l2_fwnode_endpoint *vep,
> > -				      struct v4l2_async_subdev *asd))
> > +					  struct v4l2_async_notifier *notifier,
> > +					  struct fwnode_handle *endpoint,
> > +					  unsigned int asd_struct_size,
> > +					  parse_endpoint_func parse_endpoint)
> >  {
> >  	struct v4l2_fwnode_endpoint vep = { .bus_type = 0 };
> >  	struct v4l2_async_subdev *asd;
> > @@ -657,13 +655,12 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
> >  }
> >  
> >  static int
> > -__v4l2_async_notifier_parse_fwnode_endpoints(struct device *dev,
> > -			struct v4l2_async_notifier *notifier,
> > -			size_t asd_struct_size,
> > -			unsigned int port, bool has_port,
> > -			int (*parse_endpoint)(struct device *dev,
> > -					      struct v4l2_fwnode_endpoint *vep,
> > -					      struct v4l2_async_subdev *asd))
> > +__v4l2_async_notifier_parse_fwnode_ep(struct device *dev,
> > +				      struct v4l2_async_notifier *notifier,
> > +				      size_t asd_struct_size,
> > +				      unsigned int port,
> > +				      bool has_port,
> > +				      parse_endpoint_func parse_endpoint)
> >  {
> >  	struct fwnode_handle *fwnode;
> >  	int ret = 0;
> > @@ -708,31 +705,27 @@ __v4l2_async_notifier_parse_fwnode_endpoints(struct device *dev,
> >  
> >  int
> >  v4l2_async_notifier_parse_fwnode_endpoints(struct device *dev,
> > -		struct v4l2_async_notifier *notifier,
> > -		size_t asd_struct_size,
> > -		int (*parse_endpoint)(struct device *dev,
> > -				      struct v4l2_fwnode_endpoint *vep,
> > -				      struct v4l2_async_subdev *asd))
> > +					   struct v4l2_async_notifier *notifier,
> > +					   size_t asd_struct_size,
> > +					   parse_endpoint_func parse_endpoint)
> >  {
> > -	return __v4l2_async_notifier_parse_fwnode_endpoints(dev, notifier,
> > -							    asd_struct_size, 0,
> > -							    false,
> > -							    parse_endpoint);
> > +	return __v4l2_async_notifier_parse_fwnode_ep(dev, notifier,
> > +						     asd_struct_size, 0,
> > +						     false, parse_endpoint);
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);
> >  
> >  int
> >  v4l2_async_notifier_parse_fwnode_endpoints_by_port(struct device *dev,
> > -			struct v4l2_async_notifier *notifier,
> > -			size_t asd_struct_size, unsigned int port,
> > -			int (*parse_endpoint)(struct device *dev,
> > -					      struct v4l2_fwnode_endpoint *vep,
> > -					      struct v4l2_async_subdev *asd))
> > +						   struct v4l2_async_notifier *notifier,
> > +						   size_t asd_struct_size,
> > +						   unsigned int port,
> > +						   parse_endpoint_func parse_endpoint)  
> 
> This is still over 80 here. I think we could think of abbreviating what's
> in the function name, not limiting to the endpoint. I think I'd prefer to
> leave that for 4.21 as there's not much time anymore.

Yes, I know. Renaming the function is the only way to get rid of
those remaining warnings. If you're ok with renaming, IMHO it is best
do do it right now, as we are already churning a lot of fwnode-related
code, avoiding the need of touching it again for 4.21.

Thanks,
Mauro