Re: [PATCH v4] usb: dwc3: Don't switch OTG -> peripheral if extcon is present
From: Ferry Toth
Date: Sat Sep 24 2022 - 12:06:13 EST
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@xxxxxxxxx> 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@xxxxxxxxx> 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.