On 2023-07-13 12:08, Marek Vasut wrote:
> On 7/13/23 11:56, Jonas Karlman wrote:
>> Hi Marek,
>>
>> Sorry for late reply.
>> On 2023-06-05 12:14, Marek Vasut wrote:
>>> On 5/30/23 12:26, Jonas Karlman wrote:
>>>> When dr_mode is peripheral or otg and U-Boot has not been built with
>>>> DM_USB_GADGET support, booting such device may end up with:
>>>>
>>>> dwc3_glue_bind_common: subnode name: usb@fcc00000
>>>> Error binding driver 'dwc3-generic-wrapper': -6
>>>> Some drivers failed to bind
>>>> initcall sequence 00000000effbca08 failed at call 0000000000a217c8
>>>> (err=-6)
>>>> ### ERROR ### Please RESET the board ###
>>>>
>>>> Instead fail gracfully with ENODEV to allow board continue booting.
>>>>
>>>> dwc3_glue_bind_common: subnode name: usb@fcc00000
>>>> dwc3_glue_bind_common: unsupported dr_mode
>>>>
>>>> Signed-off-by: Jonas Karlman <[email protected]>
>>>> ---
>>>> drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c
>>>> b/drivers/usb/dwc3/dwc3-generic.c
>>>> index c28ad47bddd8..f7859a530280 100644
>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>> @@ -422,13 +422,13 @@ static int dwc3_glue_bind_common(struct udevice
>>>> *parent, ofnode node)
>>>> dr_mode = usb_get_dr_mode(node);
>>>>
>>>> switch (dr_mode) {
>>>> +#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>> case USB_DR_MODE_PERIPHERAL:
>>>> case USB_DR_MODE_OTG:
>>>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>
>>> Why not just do
>>>
>>> #else
>>> return -ENODEV
>>> #endif
>>>
>>> here ?
>>
>> The code was changed to closer match how host mode is already handled,
>> and the default switch case already return -ENODEV.
>
> Let's try the above and see if that makes the code more readable.
>
> Also, you might want to try
>
> if (!CONFIG_IS_ENABLED(DM_USB_GADGET))
> return -ENODEV;
>
> To improve build coverage.
In my opinion this (after this patch as-is) is more readable and has the
benefit of the debug message when DM_USB_GADGET is disabled.
switch (dr_mode) {
#if CONFIG_IS_ENABLED(DM_USB_GADGET)
case USB_DR_MODE_PERIPHERAL:
case USB_DR_MODE_OTG:
debug("%s: dr_mode: OTG or Peripheral\n", __func__);
driver = "dwc3-generic-peripheral";
break;
#endif
#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD)
case USB_DR_MODE_HOST:
debug("%s: dr_mode: HOST\n", __func__);
driver = "dwc3-generic-host";
break;
#endif
default:
debug("%s: unsupported dr_mode\n", __func__);
return -ENODEV;
};
Compared to the following:
switch (dr_mode) {
case USB_DR_MODE_PERIPHERAL:
case USB_DR_MODE_OTG:
if (!CONFIG_IS_ENABLED(DM_USB_GADGET))
return -ENODEV;
debug("%s: dr_mode: OTG or Peripheral\n", __func__);
driver = "dwc3-generic-peripheral";
break;
#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD)
case USB_DR_MODE_HOST:
debug("%s: dr_mode: HOST\n", __func__);
driver = "dwc3-generic-host";
break;
#endif
default:
debug("%s: unsupported dr_mode\n", __func__);
return -ENODEV;
};
Any suggestion on how this could otherwise be refactored?
The original intent with the patch is to fix a non-booting
config to a booting config with minimal code changes :-)
Regards,
Jonas
