On Wed, Dec 21, 2022 at 11:07:50AM +0100, Ferry Toth wrote:
> Hi,
>
> On 20-12-2022 20:43, Guenter Roeck wrote:
> > On Mon, Dec 05, 2022 at 09:15:26PM +0100, Ferry Toth wrote:
> > > Since commit 0f0101719138 ("usb: dwc3: Don't switch OTG -> peripheral
> > > if extcon is present") Dual Role support on Intel Merrifield platform
> > > broke due to rearranging the call to dwc3_get_extcon().
> > >
> > > It appears to be caused by ulpi_read_id() on the first test write failing
> > > with -ETIMEDOUT. Currently ulpi_read_id() expects to discover the phy via
> > > DT when the test write fails and returns 0 in that case, even if DT does not
> > > provide the phy. As a result usb probe completes without phy.
> > >
> > > Make ulpi_read_id() return -ETIMEDOUT to its user if the first test write
> > > fails. The user should then handle it appropriately. A follow up patch
> > > will make dwc3_core_init() set -EPROBE_DEFER in this case and bail out.
> > >
> > > Fixes: ef6a7bcfb01c ("usb: ulpi: Support device discovery via DT")
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Ferry Toth <ftoth@xxxxxxxxxxxxxx>
> > Hi,
> >
> > this patch results in some qemu test failures, specifically xilinx-zynq-a9
> > machine and zynq-zc702 as well as zynq-zed devicetree files, when trying
> > to boot from USB drive. The log shows
>
> I'm not familiar with that platform. Does it use dt to discover the ulpi
> device?
>
The dt usb description includes
usb_phy0: phy0 {
compatible = "usb-nop-xceiv";
#phy-cells = <0>;
};
...
&usb0 {
status = "okay";
dr_mode = "host";
usb-phy = <&usb_phy0>;
};
...
usb0: usb@e0002000 {
compatible = "xlnx,zynq-usb-2.20a", "chipidea,usb2";
status = "disabled";
clocks = <&clkc 28>;
interrupt-parent = <&intc>;
interrupts = <0 21 4>;
reg = <0xe0002000 0x1000>;
phy_type = "ulpi";
};
The chipidea core initialization code includes
if (!platdata->phy_mode)
platdata->phy_mode = of_usb_get_phy_mode(dev->of_node);
Does that mean that every chipidea based usb implementation specifying
phy_type = "ulpi";
in their devicetree description will now fail, plus maybe others
who determine the phy mode from devicetree ?
> I'm guessing that the problem is actually caused by "usb: ulpi: defer
> ulpi_register on ulpi_read_id timeout".
>
Confused. Isn't that this patch ?
> ulpi_read_id() now returns ETIMEDOUT due to the test write ulpi_write(ulpi,
> ULPI_SCRATCH, 0xaa) failing.
>
> Maybe we can create a fix by skipping the test write in case dt discovery
> is available and calling of_device_request_module() directly, instead of
> masking the timed out test write as it was before?
>
I have no idea. All I can see is that it appears that there was a reason
for not returning an error if that test write failed.
Thanks,
Guenter
> > ci_hdrc ci_hdrc.0: failed to register ULPI interface
> > ci_hdrc: probe of ci_hdrc.0 failed with error -110
> >
> > and the USB interface does not instantiate. Reverting this patch fixes
> > the problem. Bisect log is attached.
> >
> > A detailed log is available at
> > https://kerneltests.org/builders/qemu-arm-v7-master/builds/484/steps/qemubuildcommand/logs/stdio
> >
> > Guenter
> >
> > ---
> > # bad: [35f79d0e2c98ff6ecb9b5fc33113158dc7f7353c] Merge tag 'parisc-for-6.2-1' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux
> > # good: [830b3c68c1fb1e9176028d02ef86f3cf76aa2476] Linux 6.1
> > git bisect start 'HEAD' 'v6.1'
> > # good: [90b12f423d3c8a89424c7bdde18e1923dfd0941e] Merge tag 'for-linus-6.2-1' of https://github.com/cminyard/linux-ipmi
> > git bisect good 90b12f423d3c8a89424c7bdde18e1923dfd0941e
> > # good: [c7020e1b346d5840e93b58cc4f2c67fc645d8df9] Merge tag 'pci-v6.2-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci
> > git bisect good c7020e1b346d5840e93b58cc4f2c67fc645d8df9
> > # bad: [b83a7080d30032cf70832bc2bb04cc342e203b88] Merge tag 'staging-6.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
> > git bisect bad b83a7080d30032cf70832bc2bb04cc342e203b88
> > # good: [057b40f43ce429a02e793adf3cfbf2446a19a38e] Merge tag 'acpi-6.2-rc1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
> > git bisect good 057b40f43ce429a02e793adf3cfbf2446a19a38e
> > # good: [851f657a86421dded42b6175c6ea0f4f5e86af97] Merge tag '6.2-rc-smb3-client-fixes-part1' of git://git.samba.org/sfrench/cifs-2.6
> > git bisect good 851f657a86421dded42b6175c6ea0f4f5e86af97
> > # good: [fa205589d5e9fc2d1b2f8d31f665152da04160bc] staging: r8188eu: stop beacon processing if kmalloc fails
> > git bisect good fa205589d5e9fc2d1b2f8d31f665152da04160bc
> > # good: [4051a1c96e4883f3445cc8f239c214be622f4c6c] Merge tag 'thunderbolt-for-v6.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt into usb-next
> > git bisect good 4051a1c96e4883f3445cc8f239c214be622f4c6c
> > # good: [84e57d292203a45c96dbcb2e6be9dd80961d981a] Merge tag 'exfat-for-6.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/exfat
> > git bisect good 84e57d292203a45c96dbcb2e6be9dd80961d981a
> > # good: [6f1f0ad910f73f5533b65e1748448d334e0ec697] usb: gadget: udc: drop obsolete dependencies on COMPILE_TEST
> > git bisect good 6f1f0ad910f73f5533b65e1748448d334e0ec697
> > # good: [c7912f27dedd874d49eadf78b5b6fbfdec52c7c3] staging: rtl8192e: Fix spelling mistake "ContryIE" -> "CountryIE"
> > git bisect good c7912f27dedd874d49eadf78b5b6fbfdec52c7c3
> > # bad: [63130462c919ece0ad0d9bb5a1f795ef8d79687e] usb: dwc3: core: defer probe on ulpi_read_id timeout
> > git bisect bad 63130462c919ece0ad0d9bb5a1f795ef8d79687e
> > # good: [38cea8e31e9ef143187135d714aed4d7bd18463c] dt-bindings: vendor-prefixes: add Genesys Logic
> > git bisect good 38cea8e31e9ef143187135d714aed4d7bd18463c
> > # good: [9bae996ffa28ac03b6d95382a2a082eb219e745a] usb: misc: onboard_usb_hub: add Genesys Logic GL850G hub support
> > git bisect good 9bae996ffa28ac03b6d95382a2a082eb219e745a
> > # bad: [8a7b31d545d3a15f0e6f5984ae16f0ca4fd76aac] usb: ulpi: defer ulpi_register on ulpi_read_id timeout
> > git bisect bad 8a7b31d545d3a15f0e6f5984ae16f0ca4fd76aac
> > # first bad commit: [8a7b31d545d3a15f0e6f5984ae16f0ca4fd76aac] usb: ulpi: defer ulpi_register on ulpi_read_id timeout
