2020-01-04 11:30:52

by Dejin Zheng

Subject: [PATCH] usb: gadget: udc: core: Warn about failed to find udc

If we do not warn here, the user may not know failed to
find udc class driver because it silently fails.
Let's print a warning in that case so developers find
these problems faster.

Signed-off-by: Dejin Zheng <[email protected]>
---
drivers/usb/gadget/udc/core.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 51fa614b4079..9b11046480fe 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1414,6 +1414,8 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
}

mutex_unlock(&udc_lock);
+ if (ret)
+ pr_warn("udc-core: couldn't find an available UDC or it's busy\n");
return ret;
found:
ret = udc_bind_to_driver(udc, driver);
--
2.17.1

2020-01-04 11:47:02

by Greg Kroah-Hartman

Subject: Re: [PATCH] usb: gadget: udc: core: Warn about failed to find udc

On Sat, Jan 04, 2020 at 07:28:36PM +0800, Dejin Zheng wrote:
> If we do not warn here, the user may not know failed to
> find udc class driver because it silently fails.
> Let's print a warning in that case so developers find
> these problems faster.
>
> Signed-off-by: Dejin Zheng <[email protected]>
> ---
> drivers/usb/gadget/udc/core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 51fa614b4079..9b11046480fe 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -1414,6 +1414,8 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
> }
>
> mutex_unlock(&udc_lock);
> + if (ret)
> + pr_warn("udc-core: couldn't find an available UDC or it's busy\n");
> return ret;
> found:
> ret = udc_bind_to_driver(udc, driver);
> --
> 2.17.1
>

Isn't this going to cause a lot more messages than is really needed?

And as you have a device, shouldn't this be dev_warn()?

thanks,

greg k-h

2020-01-04 13:10:44

by Dejin Zheng

Subject: Re: [PATCH] usb: gadget: udc: core: Warn about failed to find udc

On Sat, Jan 04, 2020 at 12:45:30PM +0100, Greg KH wrote:
> On Sat, Jan 04, 2020 at 07:28:36PM +0800, Dejin Zheng wrote:
> > If we do not warn here, the user may not know failed to
> > find udc class driver because it silently fails.
> > Let's print a warning in that case so developers find
> > these problems faster.
> >
> > Signed-off-by: Dejin Zheng <[email protected]>
> > ---
> > drivers/usb/gadget/udc/core.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> > index 51fa614b4079..9b11046480fe 100644
> > --- a/drivers/usb/gadget/udc/core.c
> > +++ b/drivers/usb/gadget/udc/core.c
> > @@ -1414,6 +1414,8 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
> > }
> >
> > mutex_unlock(&udc_lock);
> > + if (ret)
> > + pr_warn("udc-core: couldn't find an available UDC or it's busy\n");
> > return ret;
> > found:
> > ret = udc_bind_to_driver(udc, driver);
> > --
> > 2.17.1
> >
>
> Isn't this going to cause a lot more messages than is really needed?

I think this should not happen. here just for a gadget driver to find
an udc device by the same name. one driver only call this function once.
if it found, the udc device will bind this driver. otherwise, print this
warning.

I also searched for the caller of this function, For example:
gadget_dev_desc_UDC_store() and usb_composite_probe(), They also
failed silently without any warning messages for return error of
this function.

>
> And as you have a device, shouldn't this be dev_warn()?

Sorry, my comments has some mistake. I did not have a device when the
gadget driver failed to find an udc device by the same name. I will
fix it on patch v2.

>
> thanks,
>
> greg k-h