Re: [PATCH v3 09/12] usb: cdns3: Handle otg mode as peripheral

[Roger Quadros <rogerq@kernel.org>]

+Peter & Pawel.

On 19/08/2023 02:47, Marek Vasut wrote:
> On 8/18/23 21:26, Roger Quadros wrote:
>>
>>
>> On 18/08/2023 21:48, Tom Rini wrote:
>>> On Thu, Aug 17, 2023 at 11:15:17AM +0300, Roger Quadros wrote:
>>>> Hi Tom,
>>>>
>>>> On 14/08/2023 20:17, Tom Rini wrote:
>>>>> On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote:
>>>>>
>>>>>> Override 'otg' to 'peripheral' mode, since 'otg' mode
>>>>>> is not yet supported by u-boot.
>>>>>>
>>>>>> Signed-off-by: Julien Panis <jpanis@baylibre.com>
>>>>>> Suggested-by: Roger Quadros <rogerq@kernel.org>
>>>>>> ---
>>>>>>   drivers/usb/cdns3/core.c | 4 ++++
>>>>>>   1 file changed, 4 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>>>>> index 644a9791b9c9..bd763fc593e1 100644
>>>>>> --- a/drivers/usb/cdns3/core.c
>>>>>> +++ b/drivers/usb/cdns3/core.c
>>>>>> @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
>>>>>>         dr_mode = best_dr_mode;
>>>>>>   +    /* u-boot doesn't yet support OTG so limit to PERIPHERAL */
>>>>>> +    if (dr_mode == USB_DR_MODE_OTG)
>>>>>> +        dr_mode = USB_DR_MODE_PERIPHERAL;
>>>>>> +
>>>>>>   #if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD)
>>>>>>       if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
>>>>>>           ret = cdns3_host_init(cdns);
>>>>>
>>>>> Julien, why don't we support otg mode here?
>>>>>
>>>>
>>>> dr_mode will never be OTG at this point as the previous if condition
>>>> would have forced it to PERIPHERAL.
>>>>
>>>> My understanding was that u-boot USB framework doesn't support OTG mode
>>>> so we force it to PERIPHERAL.
>>>
>>> Well, the first part of this series is "make unknown state be OTG" for
>>> DWC3, so we do in general (and Marek told me off-list he's used it on
>>> DWC2 as well), so it sounds like these (CDNS3, MUSB) drivers need fixing
>>> / updating.
>>>
>>
>> CDNS3 does not have an internal OTG state machine. We mostly really care
>> about role switching and not the full OTG stack. There needs to be
>> some kind of SW framework to do that in u-boot.
>>
>> Marek could you please advise what can be done here?
> 
> As far as I can tell, MX8MM (CI HDRC), STM32MP1 (DWC2) all can do the host/peripheral switching. Why is that a problem with CDNS3 ? I am not familiar with this controller btw.

So for MX8MM and STM32MP1, do  we support on the fly host/peripheral switching based on USB plug type?
As we don't have interrupts, do you keep polling for ID status change?

CDNS3 does support host/peripheral switching. i.e. cdsn3_hw_role_state_machine() and drd.c.
What we are missing is the interrupt logic to map ID pin status change to role change.

-- 
cheers,
-roger