Re: [PATCH v13 11/25] v4l: async: Introduce helpers for calling async ops callbacks

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

 




Hi Laurent,

On Tue, Sep 19, 2017 at 03:43:48PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday, 19 September 2017 15:13:11 EEST Sakari Ailus wrote:
> > On Tue, Sep 19, 2017 at 03:01:14PM +0300, Laurent Pinchart wrote:
> > > On Friday, 15 September 2017 17:17:10 EEST Sakari Ailus wrote:
> > >> Add three helper functions to call async operations callbacks. Besides
> > >> simplifying callbacks, this allows async notifiers to have no ops set,
> > >> i.e. it can be left NULL.
> > >> 
> > >> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > >> Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> > >> ---
> > >> 
> > >>  drivers/media/v4l2-core/v4l2-async.c | 49 +++++++++++++++++++++--------
> > >>  include/media/v4l2-async.h           |  1 +
> > >>  2 files changed, 37 insertions(+), 13 deletions(-)
> > >> 
> > >> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> > >> b/drivers/media/v4l2-core/v4l2-async.c index 7b2125b3d62f..c35d04b9122f
> > >> 100644
> > >> --- a/drivers/media/v4l2-core/v4l2-async.c
> > >> +++ b/drivers/media/v4l2-core/v4l2-async.c
> > >> @@ -25,6 +25,34 @@
> > >> 
> > >>  #include <media/v4l2-fwnode.h>
> > >>  #include <media/v4l2-subdev.h>
> > >> 
> > >> +static int v4l2_async_notifier_call_bound(struct v4l2_async_notifier
> > >> *n,
> > >> +					  struct v4l2_subdev *subdev,
> > >> +					  struct v4l2_async_subdev *asd)
> > >> +{
> > >> +	if (!n->ops || !n->ops->bound)
> > >> +		return 0;
> > >> +
> > >> +	return n->ops->bound(n, subdev, asd);
> > >> +}
> > >> +
> > >> +static void v4l2_async_notifier_call_unbind(struct v4l2_async_notifier
> > >> *n,
> > >> +					    struct v4l2_subdev *subdev,
> > >> +					    struct v4l2_async_subdev *asd)
> > >> +{
> > >> +	if (!n->ops || !n->ops->unbind)
> > >> +		return;
> > >> +
> > >> +	n->ops->unbind(n, subdev, asd);
> > >> +}
> > >> +
> > >> +static int v4l2_async_notifier_call_complete(struct v4l2_async_notifier
> > >> *n)
> > >> +{
> > >> +	if (!n->ops || !n->ops->complete)
> > >> +		return 0;
> > >> +
> > >> +	return n->ops->complete(n);
> > >> +}
> > >> +
> > > 
> > > Wouldn't it be enough to add a single v4l2_async_notifier_call() macro ?
> > > 
> > > #define v4l2_async_notifier_call(n, op, args...) \
> > > 
> > > 	((n)->ops && (n)->ops->op ? (n)->ops->op(n, ##args) : 0)
> > 
> > I actually had that in an earlier version but I changed it based on review
> > comments from Hans. A single macro isn't enough: some functions have int
> > return type. I think the way it is now is nicer.
> 
> What bothers me there is the overhead of a function call.

Overhead... of a function call?

Do you really mean what you're saying? :-) The functions will be called a
relatively small number of times during module loading / device probing.

> 
> By the way, what's the use case for ops being NULL ?

If a driver has no need for any of the callbacks, there's no benefit from
having to set ops struct either. This applies to devices that are
associated to the sensor, for instance.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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