Re: [PATCH] USB: gadget: Fix the function name error in sourcesink/loopback.

From: 周城东
Date: Tue Aug 01 2023 - 02:16:07 EST


Hi, Greg KH

> I do not understand this text at all, sorry.
> What exactly is broken and what is changed here to resolve the issue?

The reason for the problem is that the value of struct
usb_function.name is "loopback", while struct usb_function_driver.name
is "Loopback". The same issue exists for sourcesink. When using USB
Config FS, it won't be possible to enable these two functions.

> And please use your full name for patches.

I'm sorry, this is my first time sending kernel patch. How should I
modify my name for the patch that has already been sent? Or should I
resend a new patch?

> You just changed a user-visable api, right? Where did you document this
> and what will it affect?

Yes, I removed lb_modexit and lb_modinit and used a simpler method for
function initialization. This does not affect any other
functionalities. In the old way, the loopback function was called by
sslb_modinit in sourcesink. I believe this is not a good approach as
the loopback and sourcesink should be independent functionalities.
Their purpose is to provide simple examples for USB beginners like myself.


Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 于2023年8月1日周二 13:06写道:
>
> On Mon, Jul 31, 2023 at 09:54:49PM -0700, zhouscd wrote:
> > Inconsistent function names can cause the USB config FS not work.
>
> I do not understand this text at all, sorry.
>
> What exactly is broken and what is changed here to resolve the issue?
>
> >
> > Signed-off-by: zhouscd <zhouscd@xxxxxxxxx>
>
> What commit caused the problem?
>
> And please use your full name for patches.
>
> > ---
> > drivers/usb/gadget/function/f_loopback.c | 13 +----------
> > drivers/usb/gadget/function/f_sourcesink.c | 25 ++--------------------
> > drivers/usb/gadget/function/g_zero.h | 3 ---
> > 3 files changed, 3 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_loopback.c b/drivers/usb/gadget/function/f_loopback.c
> > index ae41f556eb75..45f542b5ff55 100644
> > --- a/drivers/usb/gadget/function/f_loopback.c
> > +++ b/drivers/usb/gadget/function/f_loopback.c
> > @@ -583,16 +583,5 @@ static struct usb_function_instance *loopback_alloc_instance(void)
> >
> > return &lb_opts->func_inst;
> > }
> > -DECLARE_USB_FUNCTION(Loopback, loopback_alloc_instance, loopback_alloc);
> > -
> > -int __init lb_modinit(void)
> > -{
> > - return usb_function_register(&Loopbackusb_func);
> > -}
> > -
> > -void __exit lb_modexit(void)
> > -{
> > - usb_function_unregister(&Loopbackusb_func);
> > -}
> > -
> > +DECLARE_USB_FUNCTION_INIT(loopback, loopback_alloc_instance, loopback_alloc);
> > MODULE_LICENSE("GPL");
> > diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
> > index 6803cd60cc6d..f6d1c095aa2c 100644
> > --- a/drivers/usb/gadget/function/f_sourcesink.c
> > +++ b/drivers/usb/gadget/function/f_sourcesink.c
> > @@ -858,7 +858,7 @@ static struct usb_function *source_sink_alloc_func(
> > ss->bulk_qlen = ss_opts->bulk_qlen;
> > ss->iso_qlen = ss_opts->iso_qlen;
> >
> > - ss->function.name = "source/sink";
> > + ss->function.name = "sourcesink";
>
> You just changed a user-visable api, right? Where did you document this
> and what will it affect?
>
> thanks,
>
> greg k-h