[PATCH] usb: dwc3: respect role-switch-default-mode

[Marek Vasut]

On 6/24/24 1:41 PM, Caleb Connolly wrote:
> Hi Marek,
> 
> On 24/06/2024 02:33, Marek Vasut wrote:
>> On 6/21/24 4:11 AM, Caleb Connolly wrote:
>>> * Factor out the common pattern of checking the dr_mode property on
>>>    the usb node and it's parent
>>> * Respect the usb-role-switch property, rather than requiring dr_mode be
>>>    set
>>> * Override OTG mode with the value in role-switch-default-mode
>>>
>>> The devicetree bindings don't dictate that dr_mode is a required
>>> property, but the dwc3 driver would refuse to probe if it was unset
>>> (this breaks the Qualcomm RB2 board which doesn't set it).
>>>
>>> Here, we update the driver to respect the other properties that can be
>>> used to describe the controller, and more gracefully handle some of the
>>> edge cases.
>>>
>>> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
>>> ---
>>> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>>> Cc: Neil Armstrong <neil.armstrong at linaro.org>
>>> ---
>>>   drivers/usb/dwc3/dwc3-generic.c | 56 +++++++++++++++++++++++++--------
>>>   1 file changed, 43 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c 
>>> b/drivers/usb/dwc3/dwc3-generic.c
>>> index cbb5d61dfb0b..1cf8b90e8bbf 100644
>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>> @@ -49,8 +49,46 @@ struct dwc3_generic_host_priv {
>>>       struct dwc3_generic_priv gen_priv;
>>>       struct udevice *vbus_supply;
>>>   };
>>> +/**
>>> + * dwc3_get_preferred_dr_mode() - Get the preferred DR mode for the 
>>> USB node
>>> + *
>>> + * Since we don't support dynamic role switching yet in dwc3, this 
>>> is a slightly
>>> + * opinionated wrapper around usb_get_dr_mode() which will respect the
>>> + * role-switch-default-mode property if it is present and the 
>>> dr_mode is OTG.
>>> + *
>>> + * @child: Node to get the DR mode for
>>> + */
>>> +static enum usb_dr_mode dwc3_get_preferred_dr_mode(ofnode node)
>>> +{
>>> +    enum usb_dr_mode mode;
>>> +    bool is_role_switch = ofnode_has_property(node, "usb-role-switch");
>>
>> The usb-role-switch DT property is not DWC3 specific, even in U-Boot 
>> it is used by e.g. STM32MP15xx which is DWC2. This should be added to 
>> core code.
> 
> I couldn't see an obvious way to add this desired behaviour without 
> risking breaking some existing boards. Do you mean keep this as a new 
> function but just move it to core code?

Yes, make this available to other drivers by placing it into core code.

> Or update the existing 
> get_dr_mode() function?

Better keep the get_dr_mode() as-is , I think the 
get_preferred_dr_mode() is a superset, right ?

You could prepare a conversion series and switch other drivers over to 
it, people can then test and selectively revert if something breaks.

>> Also, it would be really good to sync the DWC3 driver with more up to 
>> date Linux state, but that's out of the scope of this patch obviously.
> 
> Yes, I'm definitely feeling this. Last time I tried updating dwc3 as you 
> suggested (reverting all patches since the Linux import, updating, 
> re-applying)... But there have just been too many changes on the Linux 
> side for this to work anymore, and too many arch specific dwc3 glue 
> drivers which call into the core that might break.

I am hoping these arch/ drivers should have been cleaned up by now, what 
is there that is still open ?

> I would love it if we could figure out some path forward here, even if 
> that's just someone spending more than half a day on the porting effort :D

I'd love to find a way to update too.