diff mbox series

答复: [PATCH v2] usb: gadget: uvc: Fixes the abnormal enumeration problem of mobile phone as UVC camera.

Message ID TYUPR06MB62175899DECC7A9B0DA0DF01D2B12@TYUPR06MB6217.apcprd06.prod.outlook.com (mailing list archive)
State Superseded
Headers show
Series 答复: [PATCH v2] usb: gadget: uvc: Fixes the abnormal enumeration problem of mobile phone as UVC camera. | expand

Commit Message

胡连勤 July 31, 2024, 11:49 a.m. UTC
When the phone is connected to the computer to use the webcam function, 
the phone needs to be enumerated as a uvc camera function.

Because uvc->func.bind_deactivated is configured as true in the f_uvc 
driver uvc_alloc function, the usb_gadget_deactivate function is called 
during the execution of the configfs_composite_bind function to 
set gadget->deactivated to true, which in turn causes the 
usb_gadget_connect_locked function to fail to call the corresponding 
controller pullup operation (such as: dwc3_gadget_pullup, 
mtu3_gadget_pullup), and the USB cannot be enumerated 
normally under the uvc function combination.

After applying this patch, we measured that under the uvc function, 
the dwc3 controller and the mtu3 controller can be enumerated normally, 
and the webcam function is normal.

Fixes this by removing the setting of uvc->func.bind_deactivated to true.

Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
---
v2:
  - Add "Fixes:" tag on the commit
  - Modify description information.
---
 drivers/usb/gadget/function/f_uvc.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Greg Kroah-Hartman July 31, 2024, 11:54 a.m. UTC | #1
On Wed, Jul 31, 2024 at 11:49:11AM +0000, 胡连勤 wrote:
> When the phone is connected to the computer to use the webcam function, 
> the phone needs to be enumerated as a uvc camera function.
> 
> Because uvc->func.bind_deactivated is configured as true in the f_uvc 
> driver uvc_alloc function, the usb_gadget_deactivate function is called 
> during the execution of the configfs_composite_bind function to 
> set gadget->deactivated to true, which in turn causes the 
> usb_gadget_connect_locked function to fail to call the corresponding 
> controller pullup operation (such as: dwc3_gadget_pullup, 
> mtu3_gadget_pullup), and the USB cannot be enumerated 
> normally under the uvc function combination.
> 
> After applying this patch, we measured that under the uvc function, 
> the dwc3 controller and the mtu3 controller can be enumerated normally, 
> and the webcam function is normal.
> 
> Fixes this by removing the setting of uvc->func.bind_deactivated to true.
> 
> Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
> ---
> v2:
>   - Add "Fixes:" tag on the commit

There  is no fixes tag here :(

Also you have a lot of trailing whitespace on the commit changelog, is
that intentional?

thanks,

greg k-h
胡连勤 July 31, 2024, 12:03 p.m. UTC | #2
Hello linux community expert:

Q: There  is no fixes tag here :(
A: Is it necessary to add this fixes tag? I currently don’t know how to add this tag.

Q: Also you have a lot of trailing whitespace on the commit changelog, is that intentional?
A: There was something wrong with the text editor I was using.

Note: This is my first time submitting a patch to the Linux community. I am still familiar with the detailed rules and feel very nervous.

Thanks

-----邮件原件-----
发件人: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org> 
发送时间: 2024年7月31日 19:54
收件人: 胡连勤 <hulianqin@vivo.com>
抄送: laurent.pinchart@ideasonboard.com; dan.scally@ideasonboard.com; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; akpm@linux-foundation.org; opensource.kernel <opensource.kernel@vivo.com>
主题: Re: 答复: [PATCH v2] usb: gadget: uvc: Fixes the abnormal enumeration problem of mobile phone as UVC camera.

On Wed, Jul 31, 2024 at 11:49:11AM +0000, 胡连勤 wrote:
> When the phone is connected to the computer to use the webcam 
> function, the phone needs to be enumerated as a uvc camera function.
> 
> Because uvc->func.bind_deactivated is configured as true in the f_uvc 
> driver uvc_alloc function, the usb_gadget_deactivate function is 
> called during the execution of the configfs_composite_bind function to 
> set gadget->deactivated to true, which in turn causes the 
> usb_gadget_connect_locked function to fail to call the corresponding 
> controller pullup operation (such as: dwc3_gadget_pullup, 
> mtu3_gadget_pullup), and the USB cannot be enumerated normally under 
> the uvc function combination.
> 
> After applying this patch, we measured that under the uvc function, 
> the dwc3 controller and the mtu3 controller can be enumerated 
> normally, and the webcam function is normal.
> 
> Fixes this by removing the setting of uvc->func.bind_deactivated to true.
> 
> Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
> ---
> v2:
>   - Add "Fixes:" tag on the commit

There  is no fixes tag here :(

Also you have a lot of trailing whitespace on the commit changelog, is that intentional?

thanks,

greg k-h
Laurent Pinchart July 31, 2024, 12:23 p.m. UTC | #3
On Wed, Jul 31, 2024 at 11:49:11AM +0000, 胡连勤 wrote:
> When the phone is connected to the computer to use the webcam function, 
> the phone needs to be enumerated as a uvc camera function.
> 
> Because uvc->func.bind_deactivated is configured as true in the f_uvc 
> driver uvc_alloc function, the usb_gadget_deactivate function is called 
> during the execution of the configfs_composite_bind function to 
> set gadget->deactivated to true, which in turn causes the 
> usb_gadget_connect_locked function to fail to call the corresponding 
> controller pullup operation (such as: dwc3_gadget_pullup, 
> mtu3_gadget_pullup), and the USB cannot be enumerated 
> normally under the uvc function combination.
> 
> After applying this patch, we measured that under the uvc function, 
> the dwc3 controller and the mtu3 controller can be enumerated normally, 
> and the webcam function is normal.
> 
> Fixes this by removing the setting of uvc->func.bind_deactivated to true.
> 
> Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
> ---
> v2:
>   - Add "Fixes:" tag on the commit
>   - Modify description information.
> ---
>  drivers/usb/gadget/function/f_uvc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> index 40187b7112e7..6d63bea14211 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -1107,7 +1107,6 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
>  	uvc->func.disable = uvc_function_disable;
>  	uvc->func.setup = uvc_function_setup;
>  	uvc->func.free_func = uvc_free;
> -	uvc->func.bind_deactivated = true;

This is done on purpose. The UVC function requires a userspace control
application, so the function is deactivated by default at bind time, and
gets activated when the application is ready.

This patch isn't right.

>  
>  	return &uvc->func;
>  
> -- 
> 2.39.0
> 
> 
> -----邮件原件-----
> 发件人: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org> 
> 发送时间: 2024年7月31日 19:13
> 收件人: 胡连勤 <hulianqin@vivo.com>
> 抄送: akpm@linux-foundation.org; linux-kernel@vger.kernel.org; opensource.kernel <opensource.kernel@vivo.com>
> 主题: Re: [PATCH v1] usb: gadget: uvc: Fixed the abnormal enumeration problem of mobile phone as UVC camera.
> 
> On Wed, Jul 31, 2024 at 08:45:31AM +0000, 胡连勤 wrote:
> > When the phone is connected to the computer to use the webcam 
> > function, the phone needs to be enumerated as a uvc camera function.
> 
> <snip>
> 
> Note, why aren't you using scripts/get_maintainer.pl to find the proper mailing list and developers for this change?  Please do so.
> 
> thanks,
> 
> greg k-h
Greg Kroah-Hartman July 31, 2024, 12:24 p.m. UTC | #4
A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Wed, Jul 31, 2024 at 12:03:50PM +0000, 胡连勤 wrote:
> Hello linux community expert:
> 
> Q: There  is no fixes tag here :(
> A: Is it necessary to add this fixes tag? I currently don’t know how to add this tag.

Please read Documentation/process/process/submitting-patches.rst for how
to do this.

> Q: Also you have a lot of trailing whitespace on the commit changelog, is that intentional?
> A: There was something wrong with the text editor I was using.
> 
> Note: This is my first time submitting a patch to the Linux community. I am still familiar with the detailed rules and feel very nervous.

Please slow down and read the in-kernel documentation pointed at above.
That should help out a lot here.

thanks,

greg k-h
Laurent Pinchart July 31, 2024, 12:54 p.m. UTC | #5
Hello 胡,

When replying, please don't drop the CC list unless you need to discuss
confidential topics. Your question, and the answers you will get, can
help other people facing the same problem, so it's important to keep
them on public mailing lists.

On Wed, Jul 31, 2024 at 12:46:27PM +0000, 胡连勤 wrote:
> Hello linux community expert:
> 
> Which kernel interface does the user space program call to modify bind_deactivated to false?
> I traced the kernel code and have not found where to modify this value.

In this very specific case, the function is activated by a call to
usb_function_activate() in uvc_function_connect(), itself called from
uvc_v4l2_subscribe_event() in response to the application calling the
VIDIOC_SUBSCRIBE_EVENT ioctl on the video device to subscribe to the
UVC_EVENT_SETUP event.

You can find a sample userspace application meant to work with this
driver at https://gitlab.freedesktop.org/camera/uvc-gadget. Note that I
haven't used it personally for a few years, so I may not be able to
easily provide detailed technical support.

> -----邮件原件-----
> 发件人: Laurent Pinchart <laurent.pinchart@ideasonboard.com> 
> 发送时间: 2024年7月31日 20:24
> 收件人: 胡连勤 <hulianqin@vivo.com>
> 抄送: gregkh@linuxfoundation.org; dan.scally@ideasonboard.com; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; akpm@linux-foundation.org; opensource.kernel <opensource.kernel@vivo.com>
> 主题: Re: 答复: [PATCH v2] usb: gadget: uvc: Fixes the abnormal enumeration problem of mobile phone as UVC camera.
> 
> On Wed, Jul 31, 2024 at 11:49:11AM +0000, 胡连勤 wrote:
> > When the phone is connected to the computer to use the webcam 
> > function, the phone needs to be enumerated as a uvc camera function.
> > 
> > Because uvc->func.bind_deactivated is configured as true in the f_uvc 
> > driver uvc_alloc function, the usb_gadget_deactivate function is 
> > called during the execution of the configfs_composite_bind function to 
> > set gadget->deactivated to true, which in turn causes the 
> > usb_gadget_connect_locked function to fail to call the corresponding 
> > controller pullup operation (such as: dwc3_gadget_pullup, 
> > mtu3_gadget_pullup), and the USB cannot be enumerated normally under 
> > the uvc function combination.
> > 
> > After applying this patch, we measured that under the uvc function, 
> > the dwc3 controller and the mtu3 controller can be enumerated 
> > normally, and the webcam function is normal.
> > 
> > Fixes this by removing the setting of uvc->func.bind_deactivated to true.
> > 
> > Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
> > ---
> > v2:
> >   - Add "Fixes:" tag on the commit
> >   - Modify description information.
> > ---
> >  drivers/usb/gadget/function/f_uvc.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/usb/gadget/function/f_uvc.c 
> > b/drivers/usb/gadget/function/f_uvc.c
> > index 40187b7112e7..6d63bea14211 100644
> > --- a/drivers/usb/gadget/function/f_uvc.c
> > +++ b/drivers/usb/gadget/function/f_uvc.c
> > @@ -1107,7 +1107,6 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
> >  	uvc->func.disable = uvc_function_disable;
> >  	uvc->func.setup = uvc_function_setup;
> >  	uvc->func.free_func = uvc_free;
> > -	uvc->func.bind_deactivated = true;
> 
> This is done on purpose. The UVC function requires a userspace control
> application, so the function is deactivated by default at bind time,
> and gets activated when the application is ready.
> 
> This patch isn't right.
> 
> >  	return &uvc->func;
> >  
> > -----邮件原件-----
> > 发件人: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
> > 发送时间: 2024年7月31日 19:13
> > 收件人: 胡连勤 <hulianqin@vivo.com>
> > 抄送: akpm@linux-foundation.org; linux-kernel@vger.kernel.org; 
> > opensource.kernel <opensource.kernel@vivo.com>
> > 主题: Re: [PATCH v1] usb: gadget: uvc: Fixed the abnormal enumeration problem of mobile phone as UVC camera.
> > 
> > On Wed, Jul 31, 2024 at 08:45:31AM +0000, 胡连勤 wrote:
> > > When the phone is connected to the computer to use the webcam 
> > > function, the phone needs to be enumerated as a uvc camera function.
> > 
> > <snip>
> > 
> > Note, why aren't you using scripts/get_maintainer.pl to find the
> > proper mailing list and developers for this change?  Please do so.
> > 
> > thanks,
> > 
> > greg k-h
胡连勤 July 31, 2024, 1:10 p.m. UTC | #6
Hello linux community expert:

Thank you very much for the Linux community experts answering my questions.

Thanks

-----邮件原件-----
发件人: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
发送时间: 2024年7月31日 20:55
收件人: 胡连勤 <hulianqin@vivo.com>
抄送: gregkh@linuxfoundation.org; dan.scally@ideasonboard.com; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; akpm@linux-foundation.org; opensource.kernel <opensource.kernel@vivo.com>
主题: Re: [PATCH v2] usb: gadget: uvc: Fixes the abnormal enumeration problem of mobile phone as UVC camera.

Hello 胡,

When replying, please don't drop the CC list unless you need to discuss confidential topics. Your question, and the answers you will get, can help other people facing the same problem, so it's important to keep them on public mailing lists.

On Wed, Jul 31, 2024 at 12:46:27PM +0000, 胡连勤 wrote:
> Hello linux community expert:
>
> Which kernel interface does the user space program call to modify bind_deactivated to false?
> I traced the kernel code and have not found where to modify this value.

In this very specific case, the function is activated by a call to
usb_function_activate() in uvc_function_connect(), itself called from
uvc_v4l2_subscribe_event() in response to the application calling the VIDIOC_SUBSCRIBE_EVENT ioctl on the video device to subscribe to the UVC_EVENT_SETUP event.

You can find a sample userspace application meant to work with this driver at https://gitlab.freedesktop.org/camera/uvc-gadget. Note that I haven't used it personally for a few years, so I may not be able to easily provide detailed technical support.

> -----邮件原件-----
> 发件人: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 发送时间: 2024年7月31日 20:24
> 收件人: 胡连勤 <hulianqin@vivo.com>
> 抄送: gregkh@linuxfoundation.org; dan.scally@ideasonboard.com;
> linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
> akpm@linux-foundation.org; opensource.kernel
> <opensource.kernel@vivo.com>
> 主题: Re: 答复: [PATCH v2] usb: gadget: uvc: Fixes the abnormal enumeration problem of mobile phone as UVC camera.
>
> On Wed, Jul 31, 2024 at 11:49:11AM +0000, 胡连勤 wrote:
> > When the phone is connected to the computer to use the webcam
> > function, the phone needs to be enumerated as a uvc camera function.
> >
> > Because uvc->func.bind_deactivated is configured as true in the
> > f_uvc driver uvc_alloc function, the usb_gadget_deactivate function
> > is called during the execution of the configfs_composite_bind
> > function to set gadget->deactivated to true, which in turn causes
> > the usb_gadget_connect_locked function to fail to call the
> > corresponding controller pullup operation (such as:
> > dwc3_gadget_pullup, mtu3_gadget_pullup), and the USB cannot be
> > enumerated normally under the uvc function combination.
> >
> > After applying this patch, we measured that under the uvc function,
> > the dwc3 controller and the mtu3 controller can be enumerated
> > normally, and the webcam function is normal.
> >
> > Fixes this by removing the setting of uvc->func.bind_deactivated to true.
> >
> > Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
> > ---
> > v2:
> >   - Add "Fixes:" tag on the commit
> >   - Modify description information.
> > ---
> >  drivers/usb/gadget/function/f_uvc.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_uvc.c
> > b/drivers/usb/gadget/function/f_uvc.c
> > index 40187b7112e7..6d63bea14211 100644
> > --- a/drivers/usb/gadget/function/f_uvc.c
> > +++ b/drivers/usb/gadget/function/f_uvc.c
> > @@ -1107,7 +1107,6 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
> >     uvc->func.disable = uvc_function_disable;
> >     uvc->func.setup = uvc_function_setup;
> >     uvc->func.free_func = uvc_free;
> > -   uvc->func.bind_deactivated = true;
>
> This is done on purpose. The UVC function requires a userspace control
> application, so the function is deactivated by default at bind time,
> and gets activated when the application is ready.
>
> This patch isn't right.
>
> >     return &uvc->func;
> >
> > -----邮件原件-----
> > 发件人: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
> > 发送时间: 2024年7月31日 19:13
> > 收件人: 胡连勤 <hulianqin@vivo.com>
> > 抄送: akpm@linux-foundation.org; linux-kernel@vger.kernel.org;
> > opensource.kernel <opensource.kernel@vivo.com>
> > 主题: Re: [PATCH v1] usb: gadget: uvc: Fixed the abnormal enumeration problem of mobile phone as UVC camera.
> >
> > On Wed, Jul 31, 2024 at 08:45:31AM +0000, 胡连勤 wrote:
> > > When the phone is connected to the computer to use the webcam
> > > function, the phone needs to be enumerated as a uvc camera function.
> >
> > <snip>
> >
> > Note, why aren't you using scripts/get_maintainer.pl to find the
> > proper mailing list and developers for this change?  Please do so.
> >
> > thanks,
> >
> > greg k-h

--
Regards,

Laurent Pinchart
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 40187b7112e7..6d63bea14211 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -1107,7 +1107,6 @@  static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
 	uvc->func.disable = uvc_function_disable;
 	uvc->func.setup = uvc_function_setup;
 	uvc->func.free_func = uvc_free;
-	uvc->func.bind_deactivated = true;
 
 	return &uvc->func;