diff mbox series

usb: gadget: udc: core: Warn about failed to find udc

Message ID 20200104112836.28036-1-zhengdejin5@gmail.com (mailing list archive)
State Superseded
Headers show
Series usb: gadget: udc: core: Warn about failed to find udc | expand

Commit Message

Dejin Zheng Jan. 4, 2020, 11:28 a.m. UTC
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 <zhengdejin5@gmail.com>
---
 drivers/usb/gadget/udc/core.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Greg Kroah-Hartman Jan. 4, 2020, 11:45 a.m. UTC | #1
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 <zhengdejin5@gmail.com>
> ---
>  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
Dejin Zheng Jan. 4, 2020, 1:09 p.m. UTC | #2
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 <zhengdejin5@gmail.com>
> > ---
> >  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
diff mbox series

Patch

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);