diff mbox series

usb: gadget: uvc: mark the ctrl request for the streaming interface

Message ID 20221009222000.1790385-1-m.grzeschik@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series usb: gadget: uvc: mark the ctrl request for the streaming interface | expand

Commit Message

Michael Grzeschik Oct. 9, 2022, 10:20 p.m. UTC
For the userspace it is needed to distinguish between reqeuests for the
control or streaming interace. The userspace would have to parse the
configfs to know which interface index it has to compare the ctrl
requests against, since the interface numbers are not fixed, e.g. for
composite gadgets.

The kernel has this information when handing over the ctrl request to
the userspace. This patch adds a variable to indicate if the ctrl
request was meant for the streaming interface.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/usb/gadget/function/f_uvc.c | 6 ++++++
 include/uapi/linux/usb/g_uvc.h      | 2 ++
 2 files changed, 8 insertions(+)

Comments

Greg KH Oct. 10, 2022, 6:30 a.m. UTC | #1
On Mon, Oct 10, 2022 at 12:20:00AM +0200, Michael Grzeschik wrote:
> For the userspace it is needed to distinguish between reqeuests for the
> control or streaming interace. The userspace would have to parse the
> configfs to know which interface index it has to compare the ctrl
> requests against, since the interface numbers are not fixed, e.g. for
> composite gadgets.
> 
> The kernel has this information when handing over the ctrl request to
> the userspace. This patch adds a variable to indicate if the ctrl
> request was meant for the streaming interface.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/usb/gadget/function/f_uvc.c | 6 ++++++
>  include/uapi/linux/usb/g_uvc.h      | 2 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> index 6e196e06181ecf..132d47798c0f13 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -228,6 +228,7 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
>  	struct uvc_device *uvc = to_uvc(f);
>  	struct v4l2_event v4l2_event;
>  	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
> +	unsigned int interface = le16_to_cpu(ctrl->wIndex) & 0xff;
>  
>  	if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS) {
>  		uvcg_info(f, "invalid request type\n");
> @@ -246,6 +247,11 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
>  	uvc->event_length = le16_to_cpu(ctrl->wLength);
>  
>  	memset(&v4l2_event, 0, sizeof(v4l2_event));
> +
> +	/* check for the interface number, so the userspace doesn't have to */
> +	if (interface == uvc->streaming_intf)
> +		uvc_event->ctrlreq_streaming = 1;
> +
>  	v4l2_event.type = UVC_EVENT_SETUP;
>  	memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req));
>  	v4l2_event_queue(&uvc->vdev, &v4l2_event);
> diff --git a/include/uapi/linux/usb/g_uvc.h b/include/uapi/linux/usb/g_uvc.h
> index 652f169a019e7d..8711d706e5bfb0 100644
> --- a/include/uapi/linux/usb/g_uvc.h
> +++ b/include/uapi/linux/usb/g_uvc.h
> @@ -27,6 +27,8 @@ struct uvc_request_data {
>  };
>  
>  struct uvc_event {
> +	/* indicate if the ctrl request is for the streaming interface */
> +	__u8 ctrlreq_streaming;

How can you change a public api structure like this without breaking all
existing userspace code?

thanks,

greg k-h
Michael Grzeschik Oct. 10, 2022, 2:47 p.m. UTC | #2
On Mon, Oct 10, 2022 at 08:30:25AM +0200, Greg KH wrote:
>On Mon, Oct 10, 2022 at 12:20:00AM +0200, Michael Grzeschik wrote:
>> For the userspace it is needed to distinguish between reqeuests for the
>> control or streaming interace. The userspace would have to parse the
>> configfs to know which interface index it has to compare the ctrl
>> requests against, since the interface numbers are not fixed, e.g. for
>> composite gadgets.
>>
>> The kernel has this information when handing over the ctrl request to
>> the userspace. This patch adds a variable to indicate if the ctrl
>> request was meant for the streaming interface.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> ---
>>  drivers/usb/gadget/function/f_uvc.c | 6 ++++++
>>  include/uapi/linux/usb/g_uvc.h      | 2 ++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
>> index 6e196e06181ecf..132d47798c0f13 100644
>> --- a/drivers/usb/gadget/function/f_uvc.c
>> +++ b/drivers/usb/gadget/function/f_uvc.c
>> @@ -228,6 +228,7 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
>>  	struct uvc_device *uvc = to_uvc(f);
>>  	struct v4l2_event v4l2_event;
>>  	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
>> +	unsigned int interface = le16_to_cpu(ctrl->wIndex) & 0xff;
>>
>>  	if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS) {
>>  		uvcg_info(f, "invalid request type\n");
>> @@ -246,6 +247,11 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
>>  	uvc->event_length = le16_to_cpu(ctrl->wLength);
>>
>>  	memset(&v4l2_event, 0, sizeof(v4l2_event));
>> +
>> +	/* check for the interface number, so the userspace doesn't have to */
>> +	if (interface == uvc->streaming_intf)
>> +		uvc_event->ctrlreq_streaming = 1;
>> +
>>  	v4l2_event.type = UVC_EVENT_SETUP;
>>  	memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req));
>>  	v4l2_event_queue(&uvc->vdev, &v4l2_event);
>> diff --git a/include/uapi/linux/usb/g_uvc.h b/include/uapi/linux/usb/g_uvc.h
>> index 652f169a019e7d..8711d706e5bfb0 100644
>> --- a/include/uapi/linux/usb/g_uvc.h
>> +++ b/include/uapi/linux/usb/g_uvc.h
>> @@ -27,6 +27,8 @@ struct uvc_request_data {
>>  };
>>
>>  struct uvc_event {
>> +	/* indicate if the ctrl request is for the streaming interface */
>> +	__u8 ctrlreq_streaming;
>
>How can you change a public api structure like this without breaking all
>existing userspace code?

Absolutely right, just skip that patch.

Thanks,
Michael
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 6e196e06181ecf..132d47798c0f13 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -228,6 +228,7 @@  uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 	struct uvc_device *uvc = to_uvc(f);
 	struct v4l2_event v4l2_event;
 	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	unsigned int interface = le16_to_cpu(ctrl->wIndex) & 0xff;
 
 	if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS) {
 		uvcg_info(f, "invalid request type\n");
@@ -246,6 +247,11 @@  uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 	uvc->event_length = le16_to_cpu(ctrl->wLength);
 
 	memset(&v4l2_event, 0, sizeof(v4l2_event));
+
+	/* check for the interface number, so the userspace doesn't have to */
+	if (interface == uvc->streaming_intf)
+		uvc_event->ctrlreq_streaming = 1;
+
 	v4l2_event.type = UVC_EVENT_SETUP;
 	memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req));
 	v4l2_event_queue(&uvc->vdev, &v4l2_event);
diff --git a/include/uapi/linux/usb/g_uvc.h b/include/uapi/linux/usb/g_uvc.h
index 652f169a019e7d..8711d706e5bfb0 100644
--- a/include/uapi/linux/usb/g_uvc.h
+++ b/include/uapi/linux/usb/g_uvc.h
@@ -27,6 +27,8 @@  struct uvc_request_data {
 };
 
 struct uvc_event {
+	/* indicate if the ctrl request is for the streaming interface */
+	__u8 ctrlreq_streaming;
 	union {
 		enum usb_device_speed speed;
 		struct usb_ctrlrequest req;