From: Xavier Drudis Ferran <xdrudis@tinet.cat>
To: u-boot@lists.denx.de
Cc: Simon Glass <sjg@chromium.org>,
Philipp Tomsich <philipp.tomsich@vrull.eu>,
Kever Yang <kever.yang@rock-chips.com>,
Lukasz Majewski <lukma@denx.de>,
Sean Anderson <seanga2@gmail.com>, Marek Vasut <marex@denx.de>
Subject: Re: [PATCH v4 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2.
Date: Fri, 9 Dec 2022 16:39:07 +0100 [thread overview]
Message-ID: <Y5NWmwOWgyd6n4zw@localhost> (raw)
arch/arm/dts/rk3399.dtsi has a node
usb_host0_ehci: usb@fe380000 {
compatible = "generic-ehci";
with clocks:
clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
<&u2phy0>;
The first 2 refer to nodes with class UCLASS_CLK, but &u2phy0
has class UCLASS_PHY.
u2phy0: usb2phy@e450 {
compatible = "rockchip,rk3399-usb2phy";
Since clk_get_bulk() only looks for devices with UCLASS_CLK,
it fails with -ENODEV and then ehci_usb_probe() aborts.
The consequence is peripherals connected to a USB 2 port (e.g. in a
Rock Pi 4 the white port, nearer the edge) not being detected.
They're detected if CONFIG_USB_OHCI_GENERIC is selected in Kconfig,
because ohci_usb_probe() does not abort when one clk_get_by_index()
fails, but then they work in USB 1 mode,.
rk3399.dtsi comes from linux and the u2phy0 was added[1] to the clock
list in:
commit b5d1c57299734f5b54035ef2e61706b83041f20c
Author: William wu <wulf@rock-chips.com>
Date: Wed Dec 21 18:41:05 2016 +0800
arm64: dts: rockchip: add u2phy clock for ehci and ohci of rk3399
We found that the suspend process was blocked when it run into
ehci/ohci module due to clk-480m of usb2-phy was disabled.
[...]
Suspend concerns don't apply to U-Boot, and the problem with U-Boot
failing to probe EHCI doesn't apply to linux, because in linux
rockchip_usb2phy_clk480m_register makes u2phy0 a proper clock provider
when called by rockchip_usb2phy_probe().
So I can think of a few alternative solutions:
1- Change ehci_usb_probe() to make it more similar to
ohci_usb_probe(), and survive failure to get one clock. Looks a
little harder, and I don't know whether it could break something if
it ignored a clock that was important for something else than
suspend.
2- Change rk3399.dtsi effecttively reverting the linux commit
b5d1c57299734f5b54035ef2e61706b83041f20c. This dealigns the .dtsi
from linux and seems fragile at the next synchronisation.
3- Change the clock list in rk3399-u-boot.dtsi or somewhere else.
This survives .dts* sync but may survive "too much" and miss some
change from linux that we might want.
4- Enable CONFIG_USB_OHCI_GENERIC and use the ports in USB 1 mode.
This would need to be made for all boards using rk3399. In a
simple test reading one file from USB storage it gave 769.5 KiB/s
instead of 20.5 MiB/s with solution 2.
5- Trying to replicate linux and have usb2phy somehow provide a clk,
or have a separate clock device for usb2phy in addition to the phy
device.
This series is a second attempt to implement option 5 as Marek Vasut
requested in December 5th. Options 1 and 3 didn't get through[2,3].
The first patch in the series (identical to v3) just registers usb2phy
as a clock driver (device_bind_driver() didn't work but
device_bind_driver_to_node() did), without any specific operations, so
that ehci-generic.c finds it and is happy. It worked in my tests on a
Rock Pi 4 B+ (rk3399).
Since Marek Vasut objected to an operationless driver[4], the second
patch adds enable and disable operations adapted from linux prepare
and unprepare operations (and round_rate(), which doesn't seem very
useful anyway since it's a fixed clock). Since there're no users of
this clock in u-boot, I can't see any difference in my tests with only
the first patch or both, so I can't be sure it really works if it's
ever needed, but it's hopefully more complete.
Links: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/
[2] https://patchwork.ozlabs.org/project/uboot/patch/20220701185959.GC1700@begut/#2954536
[3] https://patchwork.ozlabs.org/project/uboot/patch/Y44+ayJfUlI08ptM@localhost/#3016099
[4] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/#3018135
Cc: Simon Glass <sjg@chromium.org>
Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
---
Changes:
v4: move v3 to one patch in the series and add a second patch
to add operations to enable disable the usb2phy 480Mhz clock.
Also, honour clock-output-names for what is worth.
v3: implement option 5 (bind usb2phy as a clk driver too) instead
of option 1 (ehci-generic.c tolerates missing clocks).
v2: implement option 1 (ehci-generic.c tolerates missing clocks)
instead of option 3 (change dts node to remove the missing
clock).
next reply other threads:[~2022-12-09 15:39 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-09 15:39 Xavier Drudis Ferran [this message]
2022-12-09 15:44 ` [PATCH v4 1/2] arm: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Xavier Drudis Ferran
2022-12-09 15:47 ` [PATCH v4 2/2] arm: rk3399: usb2phy: phy-rockchip-inno-usb2.c: Implement operations for the 480MHz usb2phy clock in rk3399 Xavier Drudis Ferran
2022-12-11 5:20 ` Marek Vasut
2022-12-11 12:22 ` Xavier Drudis Ferran
2022-12-16 0:43 ` [PATCH v4 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Marek Vasut
2023-02-19 19:48 ` Christoph Fritz
2023-02-27 12:22 ` Xavier Drudis Ferran
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y5NWmwOWgyd6n4zw@localhost \
--to=xdrudis@tinet.cat \
--cc=kever.yang@rock-chips.com \
--cc=lukma@denx.de \
--cc=marex@denx.de \
--cc=philipp.tomsich@vrull.eu \
--cc=seanga2@gmail.com \
--cc=sjg@chromium.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox