[04/12,media] uvc: enable subscriptions to other events

Message ID 20170616073915.5027-5-gustavo@padovan.org (mailing list archive)
State New
Delegated to: Laurent Pinchart
Headers

Commit Message

Gustavo F. Padovan June 16, 2017, 7:39 a.m. UTC
  From: Gustavo Padovan <gustavo.padovan@collabora.com>

Call v4l2_ctrl_subscribe_event to subscribe to more events supported by
v4l.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
---
 drivers/media/usb/uvc/uvc_v4l2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Shuah Khan July 7, 2017, 2:38 p.m. UTC | #1
On 06/16/2017 01:39 AM, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Call v4l2_ctrl_subscribe_event to subscribe to more events supported by
> v4l.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 3e7e283..dfa0ccd 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -1240,7 +1240,7 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh,
>  	case V4L2_EVENT_CTRL:
>  		return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops);
>  	default:
> -		return -EINVAL;
> +		return v4l2_ctrl_subscribe_event(fh, sub);

This looks incorrect. With this change driver will be subscribing to all
v4l2 events? Is this the intent?

>  	}
>  }
>  
> 

thanks,
-- Shuah
  
Gustavo F. Padovan July 10, 2017, 7:38 p.m. UTC | #2
2017-07-07 Shuah Khan <shuahkh@osg.samsung.com>:

> On 06/16/2017 01:39 AM, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.com>
> > 
> > Call v4l2_ctrl_subscribe_event to subscribe to more events supported by
> > v4l.
> > 
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> > ---
> >  drivers/media/usb/uvc/uvc_v4l2.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> > index 3e7e283..dfa0ccd 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -1240,7 +1240,7 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh,
> >  	case V4L2_EVENT_CTRL:
> >  		return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops);
> >  	default:
> > -		return -EINVAL;
> > +		return v4l2_ctrl_subscribe_event(fh, sub);
> 
> This looks incorrect. With this change driver will be subscribing to all
> v4l2 events? Is this the intent?

The intent was to enable this driver to subscribe to BUF_QUEUED events. 
It is the only one who can't at the moment. I'll review this.

	Gustavo
  
Gustavo F. Padovan July 26, 2017, 12:26 a.m. UTC | #3
2017-07-10 Gustavo Padovan <gustavo@padovan.org>:

> 2017-07-07 Shuah Khan <shuahkh@osg.samsung.com>:
> 
> > On 06/16/2017 01:39 AM, Gustavo Padovan wrote:
> > > From: Gustavo Padovan <gustavo.padovan@collabora.com>
> > > 
> > > Call v4l2_ctrl_subscribe_event to subscribe to more events supported by
> > > v4l.
> > > 
> > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> > > ---
> > >  drivers/media/usb/uvc/uvc_v4l2.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> > > index 3e7e283..dfa0ccd 100644
> > > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > > @@ -1240,7 +1240,7 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh,
> > >  	case V4L2_EVENT_CTRL:
> > >  		return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops);
> > >  	default:
> > > -		return -EINVAL;
> > > +		return v4l2_ctrl_subscribe_event(fh, sub);
> > 
> > This looks incorrect. With this change driver will be subscribing to all
> > v4l2 events? Is this the intent?
> 
> The intent was to enable this driver to subscribe to BUF_QUEUED events. 
> It is the only one who can't at the moment. I'll review this.

This change do not enable all events, the only new event added is the
BUF_QUEUED. v4l2_ctrl_subscribe_event() only add V4L2_EVENT_CTRL and
V4L2_EVENT_BUF_QUEUED, but the V4L2_EVENT_CTRL case can't be accessed
tere is this situation.

	Gustavo
  

Patch

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3e7e283..dfa0ccd 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1240,7 +1240,7 @@  static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh,
 	case V4L2_EVENT_CTRL:
 		return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops);
 	default:
-		return -EINVAL;
+		return v4l2_ctrl_subscribe_event(fh, sub);
 	}
 }