[PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock

Kever Yang kever.yang at rock-chips.com
Tue Jun 6 02:53:33 CEST 2023


On 2023/6/5 23:05, Xavier Drudis Ferran wrote:
> arch/arm/dts/rk3399.dtsi has a node
>
>    usb_host0_ehci: usb at 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 at 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 at 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 effectively 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 patch tries to implement option 5 as Marek Vasut requested in
> December 5th.  Options 1 and 3 didn't get through [2][3].
>
> It 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).
>
> Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/
>        [2] https://patchwork.ozlabs.org/project/uboot/patch/20220701185959.GC1700@begut/
>        [3] https://patchwork.ozlabs.org/project/uboot/patch/Y44+ayJfUlI08ptM@localhost/
>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Philipp Tomsich <philipp.tomsich at vrull.eu>
> Cc: Kever Yang <kever.yang at rock-chips.com>
> Cc: Lukasz Majewski <lukma at denx.de>
> Cc: Sean Anderson <seanga2 at gmail.com>
> Cc: Marek Vasut <marex at denx.de>
> Cc: Christoph Fritz <chf.fritz at googlemail.com>
> Cc: Jagan Teki <jagan at amarulasolutions.com>
>
> Signed-off-by: Xavier Drudis Ferran <xdrudis at tinet.cat>
Reviewed-by: Kever Yang <kever.yang at rock-chips.com>

Thanks,
- Kever
> ---
>
>   V7: improve error handling. Call device_chld_unbind() on error.
>       Remove unnecessary if.
>
>   v6: just retested over current next branch and some corrections
>       to message and headers
>       (no changes to code).
> ---
>   drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 33 +++++++++++++++++--
>   1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> index 55e1dbcfef..732d37201d 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> @@ -7,10 +7,11 @@
>    */
>   
>   #include <common.h>
> -#include <clk.h>
> +#include <clk-uclass.h>
>   #include <dm.h>
>   #include <asm/global_data.h>
>   #include <dm/device_compat.h>
> +#include <dm/device-internal.h>
>   #include <dm/lists.h>
>   #include <generic-phy.h>
>   #include <reset.h>
> @@ -168,6 +169,9 @@ static struct phy_ops rockchip_usb2phy_ops = {
>   	.of_xlate = rockchip_usb2phy_of_xlate,
>   };
>   
> +static struct clk_ops rockchip_usb2phy_clk_ops = {
> +};
> +
>   static int rockchip_usb2phy_probe(struct udevice *dev)
>   {
>   	struct rockchip_usb2phy *priv = dev_get_priv(dev);
> @@ -234,7 +238,8 @@ static int rockchip_usb2phy_bind(struct udevice *dev)
>   	dev_for_each_subnode(node, dev) {
>   		if (!ofnode_valid(node)) {
>   			dev_info(dev, "subnode %s not found\n", dev->name);
> -			return -ENXIO;
> +			ret = -ENXIO;
> +			goto bind_fail;
>   		}
>   
>   		name = ofnode_get_name(node);
> @@ -245,10 +250,26 @@ static int rockchip_usb2phy_bind(struct udevice *dev)
>   		if (ret) {
>   			dev_err(dev,
>   				"'%s' cannot bind 'rockchip_usb2phy_port'\n", name);
> -			return ret;
> +			goto bind_fail;
>   		}
>   	}
>   
> +	node = dev_ofnode(dev);
> +	name = ofnode_get_name(node);
> +	dev_dbg(dev, "clk for node %s\n", name);
> +	ret = device_bind_driver_to_node(dev, "rockchip_usb2phy_clock",
> +					 name, node, &usb2phy_dev);
> +	if (ret) {
> +		dev_err(dev,
> +			"'%s' cannot bind 'rockchip_usb2phy_clock'\n", name);
> +		goto bind_fail;
> +	}
> +
> +	return 0;
> +
> +bind_fail:
> +	device_chld_unbind(dev, NULL);
> +
>   	return ret;
>   }
>   
> @@ -366,6 +387,12 @@ U_BOOT_DRIVER(rockchip_usb2phy_port) = {
>   	.ops		= &rockchip_usb2phy_ops,
>   };
>   
> +U_BOOT_DRIVER(rockchip_usb2phy_clock) = {
> +	.name		= "rockchip_usb2phy_clock",
> +	.id		= UCLASS_CLK,
> +	.ops		= &rockchip_usb2phy_clk_ops,
> +};
> +
>   U_BOOT_DRIVER(rockchip_usb2phy) = {
>   	.name	= "rockchip_usb2phy",
>   	.id	= UCLASS_PHY,


More information about the U-Boot mailing list