Re: [PATCH RESEND v2 2/2] usb: udc: Try to clarify an error message

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Marek,

> >>> --- a/drivers/usb/gadget/udc/udc-core.c
> >>> +++ b/drivers/usb/gadget/udc/udc-core.c
> >>> @@ -323,6 +323,7 @@ err1:
> >>>    int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
> >>>    {
> >>>    	struct usb_udc		*udc = NULL;
> >>> +	unsigned int		udc_count = 0;
> >>>    	int			ret;  
> >>>    >   	if (!driver || !driver->bind || !driver->setup)  
> >>> @@ -330,12 +331,22 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)  
> >>>    >   	mutex_lock(&udc_lock);  
> >>>    	list_for_each_entry(udc, &udc_list, list) {
> >>> +		udc_count++;
> >>> +
> >>>    		/* For now we take the first one */
> >>>    		if (!udc->driver)
> >>>    			goto found;
> >>>    	}  
> >>>    > -	printf("couldn't find an available UDC\n");  
> >>> +	if (!udc_count)
> >>> +		printf("No UDC available in the system\n");
> >>> +	else
> >>> +		/* When this happens, users should 'unbind <class> <index>'
> >>> +		 * using the output of 'dm tree' and looking at the line right
> >>> +		 * after the USB peripheral/device controller.
> >>> +		 */
> >>> +		printf("All UDCs in use (%d available), use the unbind command\n",
> >>> +		       udc_count);  
> >>
> >> Users should really use 'bind' command in the first place, to avoid this guesswork to which UDC (on systems with multiple UDCs) a gadget gets bound to, and instead bind the gadget to specific UDC they want to bind it to. This code should then be removed entirely.  
> > 
> > There are two cases when this can happen:
> > * a single UDC (common) but a function already bound, there is no
> >    guessing here
> > * two (or more) UDC and there is some guessing
> > 
> > Before your cleanup, drivers would get automatically unbound from the
> > UDC to let the one being needed to bind.  
> 
> How did that ever work ?

I believe both commands (fastboot vs. network commands like dhcp, tftp,
etc) would automatically bind to the first UDC and unbind once the
command done. But you recently changed that.

> > This is no longer the case,
> > so there is a change in the CLI and I want to help people facing
> > this new problem with a slightly more comprehensive message because
> > people don't fully understand what USB is all about. We cannot
> > ask all U-Boot USB users to be U-Boot experts and USB experts. This
> > is just a little help.  
> 
> In that case, you have to handle the details like CMD_BIND may not be enabled, and CMD_DM may not be enabled.

If I understand correctly you would like this to be described:

CMD_BIND imply CMD_DM
USB_GADGET imply CMD_BIND

Currently there is:

CMD_BIND default y if USB_ETHER

Maybe we should instead have:

CMD_BIND default y if USB_GADGET

And perhaps I would even go further and change the dependency
direction, so:

USB_GADGET imply CMD_BIND
CMD_BIND imply CMD_DM

Would this work for you?

> But then, maybe what we want to do is fix the automatic switching of
> gadgets to make it work again ?

This is a longer term goal I guess.

> > In an ideal world, we will have the possibility to create
> > composite gadgets and all this can go away once it is well
> > integrated, I guess?  
> 
> I believe you would still have to do a disconnect/connect cycle even with a composite driver, you cannot just add functions to existing composite at runtime.

I have in mind an environment variable which could define the "default"
composite gadget that we want at boot time, but then if people want to
change the gadgets exposed they would indeed need some kind of
disconnect/connect machinery, agreed.

Thanks, Miquèl