On Tue, Dec 28, 2021 at 12:55 PM Tom Rini trini@konsulko.com wrote:
On Tue, Dec 28, 2021 at 01:34:18AM -0700, Simon Glass wrote:
Hi Alper,
On Fri, 24 Dec 2021 at 06:06, Alper Nebi Yasak alpernebiyasak@gmail.com wrote:
On boards using the RK3399 SoC, the USB OHCI and EHCI controllers share the same PHY device instance. While these controllers are being stopped they both attempt to power-off and deinitialize it, but trying to power-off the deinitialized PHY device results in a hang. This usually happens just before booting an OS, and can be explicitly triggered by running "usb start; usb stop" in the U-Boot shell.
Implement a uclass-wide counting mechanism for PHY initialization and power state change requests, so that we don't power-off/deinitialize a PHY instance until all of its users want it done. The Allwinner A10 USB PHY driver does this counting in-driver, remove those parts in favour of this in-uclass implementation.
The sandbox PHY operations test needs some changes since the uclass will no longer call into the drivers for actions matching its tracked state (e.g. powering-off a powered-off PHY). Update that test, and add a new one which simulates multiple users of a single PHY.
The major complication here is that PHY handles aren't deduplicated per instance, so the obvious idea of putting the counts in the PHY handles don't immediately work. It seems possible to bind a child udevice per PHY instance to the PHY provider and deduplicate the handles in each child's uclass-private areas, like in the CLK framework. An alternative approach could be to use those bound child udevices themselves as the PHY handles. Instead, to avoid the architectural changes those would require, this patch solves things by dynamically allocating a list of structs (one per instance) in the provider's uclass-private area.
Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com
Changes in v2:
- Rename {phy_,}id_priv -> {phy_,}counts
- Split phy_get_uclass_priv -> phy_{alloc,get}_counts
- Allocate counts (or return error) in generic_phy_get_by_*()
- Remove now-unnecessary null checks for counts of valid phy handles
v1: https://patchwork.ozlabs.org/project/uboot/patch/20211210200124.19226-1-alpe...
drivers/phy/allwinner/phy-sun4i-usb.c | 9 -- drivers/phy/phy-uclass.c | 120 ++++++++++++++++++++++++++ test/dm/phy.c | 83 ++++++++++++++++-- 3 files changed, 198 insertions(+), 14 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Should add comments for the struct
Also I wonder if a simple fixed-length array might be possible instead of the linked list?
Thanks for the review Simon. Since I think this should unblock some common hardware, does everyone think this should be safe enough to pull in now, or no, I shouldn't bend the rules, and take this for v2021.04 instead?
We've had a number of queries each release around this problem so it maybe worthwhile to include, I think that most distros that pre-build things for users are aware of it enough that they've had local patches for some time so personally for me I'm OK either way.
Peter