Re: [PATCH v4] usb: dwc3: Don't switch OTG -> peripheral if extcon is present

[Ferry Toth <fntoth@gmail.com>]

Hi,

Maybe some inspiration below.

Op 24-09-2022 om 03:34 schreef Andrey Smirnov:
> On Fri, Sep 23, 2022 at 2:12 PM Ferry Toth <fntoth@gmail.com> wrote:
>> Hi,
>>
>> Op 23-09-2022 om 18:42 schreef Andy Shevchenko:
>>> On Thu, Sep 22, 2022 at 04:32:55PM -0700, Andrey Smirnov wrote:
>>>> On Thu, Sep 22, 2022 at 3:23 AM Ferry Toth <fntoth@gmail.com> wrote:
>>>>> On 22-09-2022 12:08, Andy Shevchenko wrote:
>>>>> On Sun, Apr 03, 2022 at 09:49:07AM -0700, Andrey Smirnov wrote:
>>> FYI: For now I sent a revert, but if we got a solution quicker we always
>>> can choose the course of actions.
>>>
>>>>> If the extcon device exists, get the mode from the extcon device. If
>>>>> the controller is DRD and the driver is unable to determine the mode,
>>>>> only then default the dr_mode to USB_DR_MODE_PERIPHERAL.
>>>>>
>>>>> According to Ferry (Cc'ed) this broke Intel Merrifield platform. Ferry, can you
>>>>> share bisect log?
>>>>>
>>>>> I can but not right now. But what I did was bisect between 5.18.0 (good) and 5.19.0 (bad) then when I got near the culprit (~20 remaining) based on the commit message I tried 0f01017191384e3962fa31520a9fd9846c3d352f "usb: dwc3: Don't switch OTG -> peripheral if extcon is present" (bad) and commit before that (good).
>>>>>
>>>>> The effect of the patch is that on Merrifield (I tested with Intel Edison Arduino board which has a HW switch to select between host and device mode) device mode works but in host mode USB is completely not working.
>>>>>
>>>>> Currently on host mode - when working - superfluous error messages from tusb1210 appear. When host mode is not working there are no tusb1210 messages in the logs / on the console at all. Seemingly tusb1210 is not probed, which points in the direction of a relation to extcon.
>>>>>
>>>>> Taking into account the late cycle, I would like to revert the change. And
>>>>> Ferry and I would help to test any other (non-regressive) approach).
>>>>>
>>>>> I have not yet tested if a simple revert fixes the problem but will tonight.
>>>>>
>>>>>
>>>>> I would be happy to test other approaches too.
>>>> It's a bit hard for me to suggest an alternative approach without
>>>> knowing how things are breaking in this case. I'd love to order one of
>>>> those boards to repro and fix this on my end, but it looks like this
>>>> HW is EOLed and out of stock in most places. If you guys know how to
>>>> get my hands on those boards I'm all ears.
>>> There are still some second hand Intel Edison boards flying around
>>> (but maybe cost a bit more than expected) and there are also
>>> Dell Venue 7 3740 tablets based on the same platform/SoC. The latter
>>> option though requires more actions in order something to be boot
>>> there.
>>>
>>> In any case, it's probably quicker to ask Ferry or me for testing.
>>> (Although currently I have no access to the board to test OTG, it's
>>>    remote device which I can only power on and off and it has always
>>>    be in host mode.)
>>>
>>>> Barring that, Ferry can you dig more into this failure? E.g. is it this hunk
>>>>
>>>> @@ -85,7 +86,7 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
>>>>                    * mode. If the controller supports DRD but the dr_mode is not
>>>>                    * specified or set to OTG, then set the mode to peripheral.
>>>>                    */
>>>> -               if (mode == USB_DR_MODE_OTG &&
>>>> +               if (mode == USB_DR_MODE_OTG && !dwc->edev &&
>>>>                       (!IS_ENABLED(CONFIG_USB_ROLE_SWITCH) ||
>>>>                        !device_property_read_bool(dwc->dev, "usb-role-switch")) &&
>>>>                       !DWC3_VER_IS_PRIOR(DWC3, 330A))
>>>> @@ -1632,6 +1633,51 @@ static void dwc3_check_params(struct dwc3 *dwc)
>>>>           }
>>>>    }
>>>>
>>>> that's problematic or moving
>>> I think you wanted to revert only this line and test?
>> On v6.0-rc6 and reverting manually only this line
>>
>> - if (mode == USB_DR_MODE_OTG && !dwc->edev &&
>>
>> + if (mode == USB_DR_MODE_OTG &&
>>
>> host mode still does not work (no change visible).
> Cool, thanks for checking that. Don't think I have any more
> experiments off the top of my head to run. I'll have to go read that
> code more. I'll reply in the thread if I have something new to
> try/say.

It seems the problem is not extcon directly. When I switch to device 
mode, usb gadgets are working fire. Also when I

# cat /sys/class/extcon/extcon0/state
USB=0
USB-HOST=1
SDP=0
CDP=0
DCP=0
ACA=0

USB-HOST changes nicely to 0 and back when I flip the switch.

Also, in host mode I normally have (and now with host mode not working 
it the same):

root@yuna:~# dmesg | grep -i usb
ACPI: bus type USB registered
...
usbcore: registered new interface driver usbhid
usbhid: USB HID core driver
xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 1
xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 2
xhci-hcd xhci-hcd.1.auto: Host supports USB 3.0 SuperSpeed
usb usb1: New USB device found, idVendor=1d6b, idProduct=0002, 
bcdDevice= 6.00
usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
usb usb1: Product: xHCI Host Controller
usb usb1: Manufacturer: Linux 6.0.0-rc6-edison-acpi-standard xhci-hcd
usb usb1: SerialNumber: xhci-hcd.1.auto
hub 1-0:1.0: USB hub found
usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
usb usb2: New USB device found, idVendor=1d6b, idProduct=0003, 
bcdDevice= 6.00
usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
usb usb2: Product: xHCI Host Controller
usb usb2: Manufacturer: Linux 6.0.0-rc6-edison-acpi-standard xhci-hcd
usb usb2: SerialNumber: xhci-hcd.1.auto
hub 2-0:1.0: USB hub found

So, extcon works, xhci host controller "works". The problem may be no 
ulpi (tusb1210). Checking on 6.0-rc6 with host mode not working:

root@yuna:~# cat /sys/bus/ulpi/devices/dwc3.0.auto.ulpi/uevent
DEVTYPE=ulpi_device
MODALIAS=ulpi:v0000p0000
root@yuna:~# cat 
/sys/bus/ulpi/devices/dwc3.0.auto.ulpi/waiting_for_supplier
0
And on 5.15 with host mode working:

root@edison:~# cat /sys/bus/ulpi/devices/dwc3.0.auto.ulpi/uevent
DEVTYPE=ulpi_device
DRIVER=tusb1210
MODALIAS=ulpi:v0451p1508
root@edison:~#  cat 
/sys/bus/ulpi/devices/dwc3.0.auto.ulpi/waiting_for_supplier
cat: /sys/bus/ulpi/devices/dwc3.0.auto.ulpi/waiting_for_supplier: No 
such file or directory

Ulpi is there but is waiting for driver.

>>>>    static int dwc3_probe(struct platform_device *pdev)
>>>>    {
>>>>           struct device           *dev = &pdev->dev;
>>>> @@ -1744,6 +1790,13 @@ static int dwc3_probe(struct platform_device *pdev)
>>>>                   goto err2;
>>>>           }
>>>>
>>>> +       dwc->edev = dwc3_get_extcon(dwc);
>>>> +       if (IS_ERR(dwc->edev)) {
>>>> +               ret = PTR_ERR(dwc->edev);
>>>> +               dev_err_probe(dwc->dev, ret, "failed to get extcon\n");
>>>> +               goto err3;
>>>> +       }
>>>> +
>>>>           ret = dwc3_get_dr_mode(dwc);
>>>>           if (ret)
>>>>                   goto err3;
>>>>
>>>> to happen earlier?
>>> It is not always possible to have an extcon driver available, that's why in
>>> some cases the probe of it defers. I dunno how your patch supposed to work
>>> in that case.
>>>
>>>> Does tracing the "mrfld_bcove_pwrsrc" driver (the
>>>> excton provider in this case AFIACT) show anything interesting?
>>> I believe there is nothing interesting.
>>>