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.ai...@linux.intel.com>
> > >> Acked-by: Hans Verkuil <hans.verk...@cisco.com>
> > >> ---
> > >> 
> > >>  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.ai...@iki.fi

Reply via email to