Re: [PATCH v9 13/24] v4l: async: Allow async notifier register call succeed with no subdevs

2017-09-11 Thread Sakari Ailus
On Mon, Sep 11, 2017 at 10:05:40AM +0200, Hans Verkuil wrote:
> On 09/08/2017 03:18 PM, Sakari Ailus wrote:
> > The information on how many async sub-devices would be bindable to a
> > notifier is typically dependent on information from platform firmware and
> > it's not driver's business to be aware of that.
> > 
> > Many V4L2 main drivers are perfectly usable (and useful) without async
> > sub-devices and so if there aren't any around, just proceed call the
> > notifier's complete callback immediately without registering the notifier
> > itself.
> > 
> > If a driver needs to check whether there are async sub-devices available,
> > it can be done by inspecting the notifier's num_subdevs field which tells
> > the number of async sub-devices.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/v4l2-async.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> > b/drivers/media/v4l2-core/v4l2-async.c
> > index 7b396ff4302b..9ebc2e079d03 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -170,10 +170,12 @@ int v4l2_async_notifier_register(struct v4l2_device 
> > *v4l2_dev,
> > struct v4l2_async_subdev *asd;
> > int i;
> >  
> > -   if (!v4l2_dev || !notifier->num_subdevs ||
> > -   notifier->num_subdevs > V4L2_MAX_SUBDEVS)
> > +   if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
> > return -EINVAL;
> >  
> > +   if (!notifier->num_subdevs)
> > +   return v4l2_async_notifier_call_complete(notifier);
> > +
> 
> I would move this 'if' down to after the next three lines...
> 
> > notifier->v4l2_dev = v4l2_dev;
> > INIT_LIST_HEAD(¬ifier->waiting);
> > INIT_LIST_HEAD(¬ifier->done);
> > 
> 
> ...that way the notifier struct is properly initialized. Just in case anyone
> ever looks at these three fields.

Makes sense. Fixed.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi


Re: [PATCH v9 13/24] v4l: async: Allow async notifier register call succeed with no subdevs

2017-09-11 Thread Hans Verkuil
On 09/08/2017 03:18 PM, Sakari Ailus wrote:
> The information on how many async sub-devices would be bindable to a
> notifier is typically dependent on information from platform firmware and
> it's not driver's business to be aware of that.
> 
> Many V4L2 main drivers are perfectly usable (and useful) without async
> sub-devices and so if there aren't any around, just proceed call the
> notifier's complete callback immediately without registering the notifier
> itself.
> 
> If a driver needs to check whether there are async sub-devices available,
> it can be done by inspecting the notifier's num_subdevs field which tells
> the number of async sub-devices.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 7b396ff4302b..9ebc2e079d03 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -170,10 +170,12 @@ int v4l2_async_notifier_register(struct v4l2_device 
> *v4l2_dev,
>   struct v4l2_async_subdev *asd;
>   int i;
>  
> - if (!v4l2_dev || !notifier->num_subdevs ||
> - notifier->num_subdevs > V4L2_MAX_SUBDEVS)
> + if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
>   return -EINVAL;
>  
> + if (!notifier->num_subdevs)
> + return v4l2_async_notifier_call_complete(notifier);
> +

I would move this 'if' down to after the next three lines...

>   notifier->v4l2_dev = v4l2_dev;
>   INIT_LIST_HEAD(¬ifier->waiting);
>   INIT_LIST_HEAD(¬ifier->done);
> 

...that way the notifier struct is properly initialized. Just in case anyone
ever looks at these three fields.

Regards,

Hans


[PATCH v9 13/24] v4l: async: Allow async notifier register call succeed with no subdevs

2017-09-08 Thread Sakari Ailus
The information on how many async sub-devices would be bindable to a
notifier is typically dependent on information from platform firmware and
it's not driver's business to be aware of that.

Many V4L2 main drivers are perfectly usable (and useful) without async
sub-devices and so if there aren't any around, just proceed call the
notifier's complete callback immediately without registering the notifier
itself.

If a driver needs to check whether there are async sub-devices available,
it can be done by inspecting the notifier's num_subdevs field which tells
the number of async sub-devices.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/v4l2-async.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 7b396ff4302b..9ebc2e079d03 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -170,10 +170,12 @@ int v4l2_async_notifier_register(struct v4l2_device 
*v4l2_dev,
struct v4l2_async_subdev *asd;
int i;
 
-   if (!v4l2_dev || !notifier->num_subdevs ||
-   notifier->num_subdevs > V4L2_MAX_SUBDEVS)
+   if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
return -EINVAL;
 
+   if (!notifier->num_subdevs)
+   return v4l2_async_notifier_call_complete(notifier);
+
notifier->v4l2_dev = v4l2_dev;
INIT_LIST_HEAD(¬ifier->waiting);
INIT_LIST_HEAD(¬ifier->done);
-- 
2.11.0