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