[PATCH] usb: gadget: uvc: mark the ctrl request for the streaming interface

[PATCH] usb: gadget: uvc: mark the ctrl request for the streaming interface

[Michael Grzeschik]

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;
 	union {
 		enum usb_device_speed speed;
 		struct usb_ctrlrequest req;
-- 
2.30.2

Re: [PATCH] usb: gadget: uvc: mark the ctrl request for the streaming interface

[Greg KH]

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

Re: [PATCH] usb: gadget: uvc: mark the ctrl request for the streaming interface

[Michael Grzeschik]

[-- Attachment #1: Type: text/plain, Size: 2935 bytes --]

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

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]