Re: [PATCH v2 2/2] Revert "usb: dwc3: Don't switch OTG -> peripheral if extcon is present"

[Ferry Toth <fntoth@gmail.com>]

Op 15-10-2022 om 21:54 schreef Andrey Smirnov:
> On Thu, Oct 13, 2022 at 12:35 PM Ferry Toth <fntoth@gmail.com> wrote:
>> <SNIP>
>>> My end goal here is to find a way to test vanilla v6.0 with the two
>>> patches reverted on your end. I thought that during my testing I saw
>>> tusb1210 print those timeout messages during its probe and that
>>> disabling the driver worked to break the loop, but I went back to
>>> double check and it doesn't work so scratch that idea. Configuring
>>> extcon as a built-in breaks host functionality with or without patches
>>> on my end, so I'm not sure it could be a path.
>>>
>>> I won't have time to try things with
>>> 0043b-TODO-driver-core-Break-infinite-loop-when-deferred-p.patch until
>>> the weekend, meanwhile can you give this diff a try with vanilla (no
>>> reverts) v6.0:
>>>
> OK, got a chance to try things with that patch. Both v6.0 and v6.0
> with my patches reverted work the same, my Kingston DataTraveller USB
> stick enumerates and works as expected.
>
Iow you don't need the patch at all to get usb to work. There has got to 
be a difference in our configs.

Did you have a chance to look at mine (here: 
https://drive.google.com/file/d/1aKJWMqiAXnReeLCvxshzjKwGxIWQ7eJk/view?usp=sharing)

Else, send me yours.

>>> modified   drivers/phy/ti/phy-tusb1210.c
>>> @@ -127,6 +127,7 @@ static int tusb1210_set_mode(struct phy *phy, enum
>>> phy_mode mode, int submode)
>>>     u8 reg;
>>>
>>>     ret = tusb1210_ulpi_read(tusb, ULPI_OTG_CTRL, &reg);
>>> + WARN_ON(ret < 0);
>>>     if (ret < 0)
>>>     return ret;
>>>
>>> @@ -152,7 +153,10 @@ static int tusb1210_set_mode(struct phy *phy,
>>> enum phy_mode mode, int submode)
>>>     }
>>>
>>>     tusb->otg_ctrl = reg;
>>> - return tusb1210_ulpi_write(tusb, ULPI_OTG_CTRL, reg);
>>> + ret = tusb1210_ulpi_write(tusb, ULPI_OTG_CTRL, reg);
>>> + WARN_ON(ret < 0);
>>> + return ret;
>>> +
>>>    }
>>>
>>>    #ifdef CONFIG_POWER_SUPPLY
>>>
>>> ? I'm curious to see if there's masked errors on your end since dwc3
>>> driver doesn't check for those.
>> root@yuna:~# dmesg | grep -i -E 'warn|assert|error|tusb|dwc3'
>> 8250_mid: probe of 0000:00:04.0 failed with error -16
>> platform regulatory.0: Direct firmware load for regulatory.db failed
>> with error -2
>> brcmfmac mmc2:0001:1: Direct firmware load for
>> brcm/brcmfmac43340-sdio.Intel Corporation-Merrifield.bin failed with
>> error -2
>> sof-audio-pci-intel-tng 0000:00:0d.0: error: I/O region is too small.
>> sof-audio-pci-intel-tng 0000:00:0d.0: error: failed to probe DSP -19
>>
>>
>>>> This is done through configfs only when the switch is set to device mode.
>>> Sure, but can it be disabled? We are looking for unknown variables, so
>>> excluding this would be a reasonable thing to do.
>> It's not enabled until I flip the switch to device mode.
> OK to cut this back and forth short, I think it'd be easier to just
> ask you to run what I run. Here's vanilla v6.0 bzImage and initrd
> (built with your config + CONFIG_PHY_TUSB1210=y) I tested with

What do you mean by this? My config is with

CONFIG_GENERIC_PHY=y
CONFIG_PHY_TUSB1210=y

> https://drive.google.com/drive/folders/1H28AL1coPPZ2kLTYskDuDdWo-oE7DRPH?usp=sharing
> let's see how it behaves on your setup. There's also the U-Boot binary

Ok, it's getting weirder and weirder. The following is with my U-Boot 
and your kernel/initrd

1) I placed them in /boot which is on my btrfs partition on the emmc (my 
U-Boot has btrfs enabled)

Linux kernel version 6.0.0-edison-acpi-standard 
(andreysm@neptunefw-builder) #8 SMP PREEMPT_DYNAMIC Sat Oct 15 18:47:19 
UTC 2022
Building boot_params at 0x00090000
Loading bzImage at address 100000 (12064480 bytes)
Initial RAM disk at linear address 0x06000000, size 25165824 bytes
Kernel command line: "quiet root=/dev/mmcblk0p8 
rootflags=subvol=@,compress=lzo rootfstype=btrfs console=ttyS2,115200n8 
earlyprintk=ttyS2,115200n8,keep loglevel=4 systemd.unit=multi-user.target"
Kernel loaded at 00100000, setup_base=00090000

Usb drive is not detected regardless booting with stick plugged or 
plugging later on.

# lsusb
Bus 001 Device 001: ID 1d6b:0002
Bus 002 Device 001: ID 1d6b:0003

No TUSB1210 probed

# dmesg | grep dwc3
#

2) I placed them in my vfat rescue partition

Linux kernel version 6.0.0-edison-acpi-standard 
(andreysm@neptunefw-builder) #8 SMP PREEMPT_DYNAMIC Sat Oct 15 18:47:19 
UTC 2022
Building boot_params at 0x00090000
Loading bzImage at address 100000 (12064480 bytes)
Initial RAM disk at linear address 0x06000000, size 25165824 bytes
Kernel command line: "debugshell=0 tty1 console=ttyS2,115200n8 
root=/dev/mmcblk0p7 rootfstype=vfat systemd.unit=multi-user.target"
Kernel loaded at 00100000, setup_base=00090000

Usb drive is detected.

# lsusb
Bus 001 Device 001: ID 1d6b:0002
Bus 001 Device 002: ID 125f:312b
Bus 002 Device 001: ID 1d6b:0003

TUSB1210 probed

# dmesg | grep dwc3
[    8.605845] tusb1210 dwc3.0.auto.ulpi: GPIO lookup for consumer reset
[    8.605876] tusb1210 dwc3.0.auto.ulpi: using ACPI for GPIO lookup
[    8.605927] tusb1210 dwc3.0.auto.ulpi: using lookup tables for GPIO 
lookup
[    8.605941] tusb1210 dwc3.0.auto.ulpi: No GPIO consumer reset found
[    8.605956] tusb1210 dwc3.0.auto.ulpi: GPIO lookup for consumer cs
[    8.605970] tusb1210 dwc3.0.auto.ulpi: using ACPI for GPIO lookup
[    8.606011] tusb1210 dwc3.0.auto.ulpi: using lookup tables for GPIO 
lookup
[    8.606024] tusb1210 dwc3.0.auto.ulpi: No GPIO consumer cs found
[    8.669317] tusb1210 dwc3.0.auto.ulpi: error -110 writing val 0x41 to 
reg 0x80

## note: options debugshell, root and rootfstype are normally handled by 
a script in my initrd, so I guess here noop.

> I use in that folder in case you want to give it a try.
>
> Now on Merrifield dwc3_get_extcon() doesn't do anything but call
> extcon_get_extcon_dev() which doesn't touch any hardware or interact
> with other drivers, so assuming
>
>> So current v6.0 has: dwc3_get_extcon - dwc3_get_dr_mode - ... -
>> dwc3_core_init - .. - dwc3_core_init_mode (not working)
>>
>> I changed to: dwc3_get_dr_mode - dwc3_get_extcon - .. - dwc3_core_init -
>> .. - dwc3_core_init_mode (no change)
>>
>> Then to: dwc3_get_dr_mode - .. - dwc3_core_init - .. - dwc3_get_extcon -
>> dwc3_core_init_mode (works)
> still holds(did you double check that with vanilla v6.0?) the only
I didn't check
> difference that I can see is execution timings. It seems to me it's
> either an extra delay added by execution of  extcon_get_extcon_dev()
> (unlikely) or multiple partial probes that include dwc3_core_init()
> that change things. You can try to check the latter by adding an
> artificial probe deferral point after dwc3_core_init(). Something like
> (didn't test this):
>
> modified   drivers/usb/dwc3/core.c
> @@ -1860,6 +1860,10 @@ static int dwc3_probe(struct platform_device *pdev)
>    goto err3;
>
>    ret = dwc3_core_init(dwc);
> + static int deferral_counter = 0;
> + if (deferral_counter++ < 9) /* I counted 9 deferrals in my testing */
> + ret = -EPROBE_DEFER;
> +
>    if (ret) {
>    dev_err_probe(dev, ret, "failed to initialize core\n");
>    goto err4;

Not sure how you wanted this tested. So I assume on vanilla booting from 
btrfs on eemc. It crashes but maybe the trace is usefull. After crash it 
continues but no USB appears at all.

[    4.185151] kobject_add_internal failed for dwc3.0.auto.ulpi with 
-EEXIST, don't try to register things with the same name in the same 
directory.
[    4.198625] dwc3 dwc3.0.auto: failed to register ULPI interface
[    4.213112] BUG: kernel NULL pointer dereference, address: 
0000000000000001

[    4.220260] #PF: supervisor read access in kernel mode
[    4.225523] #PF: error_code(0x0000) - not-present page
[    4.230785] PGD 0 P4D 0
[    4.233402] Oops: 0000 [#1] PREEMPT SMP PTI
[    4.237696] CPU: 0 PID: 8 Comm: kworker/u4:0 Not tainted 
6.0.0-edison-acpi-standard #1
[    4.245802] Hardware name: Intel Corporation Merrifield/BODEGA BAY, 
BIOS 542 2015.01.21:18.19.48
[    4.254791] Workqueue: events_unbound deferred_probe_work_func
[    4.260793] RIP: 0010:strlen+0x0/0x20
[    4.264559] Code: b6 07 38 d0 74 14 48 83 c7 01 84 c0 74 05 48 39 f7 
75 ec 31 c0 c3 cc cc cc cc 48 89 f8 c3 cc cc cc cc 0f 1f 84 00 00 00 00 
00 <80> 3f 00 74 14 48 89 f8 48 83 c0 01 80 38 00 75 f7 48 29 f8 c3 cc
[    4.283751] RSP: 0018:ffffa9520004bb10 EFLAGS: 00010202
[    4.289113] RAX: 0000000000000018 RBX: ffff9c5807d77c18 RCX: 
0000000000000000
[    4.296417] RDX: ffffa9520004bb88 RSI: 0000000000000cc0 RDI: 
0000000000000001
[    4.303719] RBP: 0000000000000001 R08: 0000000000000000 R09: 
ffffa9520004baf0
[    4.311022] R10: ffffffffffffffff R11: 000000000000000b R12: 
0000000000000cc0
[    4.318323] R13: ffff9c5807d77c18 R14: ffff9c5807d77c18 R15: 
000000000000a710
[    4.325625] FS:  0000000000000000(0000) GS:ffff9c583e200000(0000) 
knlGS:0000000000000000
[    4.333907] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    4.339793] CR2: 0000000000000001 CR3: 000000001540c000 CR4: 
00000000001006f0
[    4.347098] Call Trace:
[    4.349611]  <TASK>
[    4.351771]  kstrdup_const+0x2d/0x70
[    4.355453]  kobject_set_name_vargs+0x1e/0x90
[    4.359939]  dev_set_name+0x4e/0x70
[    4.363534]  device_add+0x5b/0x960
[    4.367036]  ? complete_all+0x1b/0x80
[    4.370808]  ulpi_register_interface+0x213/0x290
[    4.375551]  dwc3_ulpi_init+0x17/0x40
[    4.379314]  dwc3_core_init+0xc29/0x1b70
[    4.383350]  dwc3_probe+0x115a/0x1900
[    4.387122]  platform_probe+0x3a/0xa0
[    4.390892]  really_probe+0xdc/0x390
[    4.394567]  ? pm_runtime_barrier+0x4b/0x80
[    4.398867]  __driver_probe_device+0x73/0x170
[    4.403340]  driver_probe_device+0x19/0x90
[    4.407545]  __device_attach_driver+0x80/0x110
[    4.412105]  ? driver_allows_async_probing+0x60/0x60
[    4.417195]  ? driver_allows_async_probing+0x60/0x60
[    4.422286]  bus_for_each_drv+0x79/0xc0
[    4.426230]  __device_attach+0xb7/0x210
[    4.430169]  bus_probe_device+0x89/0xa0
[    4.434111]  deferred_probe_work_func+0x85/0xe0
[    4.438762]  process_one_work+0x1d7/0x3a0
[    4.440890] EXT4-fs (mmcblk1): recovery complete
[    4.442785]  worker_thread+0x48/0x3c0
[    4.442809]  ? rescuer_thread+0x380/0x380
[    4.442828]  kthread+0xe2/0x110
[    4.447489] EXT4-fs (mmcblk1): mounted filesystem with ordered data 
mode. Quota mode: none.
[    4.451086]  ? kthread_complete_and_exit+0x20/0x20
[    4.451107]  ret_from_fork+0x22/0x30
[    4.475811]  </TASK>
[    4.478052] Modules linked in: mmc_block extcon_intel_mrfld sdhci_pci 
cqhci sdhci led_class mmc_core intel_soc_pmic_mrfld btrfs libcrc32c xor 
zlib_deflate raid6_pq zstd_compress
[    4.494312] CR2: 0000000000000001
[    4.497717] ---[ end trace 0000000000000000 ]---