* [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2
@ 2023-06-05 15:04 Xavier Drudis Ferran
2023-06-05 15:05 ` [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock Xavier Drudis Ferran
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Xavier Drudis Ferran @ 2023-06-05 15:04 UTC (permalink / raw)
To: u-boot
Cc: Simon Glass, Philipp Tomsich, Kever Yang, Lukasz Majewski,
Sean Anderson, Marek Vasut, Christoph Fritz, Jagan Teki
EHCI probing in Rock pi 4 currently fails.
Add a clock driver for usb2phy so that probing EHCI does not fail when
missing one of the clocks in the bundle for usb_host0_ehci, since
usb2phy is UCLASS_PHY but not UCLASS_CLK.
Xavier Drudis Ferran (2):
phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock
phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock
drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 109 +++++++++++++++++-
1 file changed, 106 insertions(+), 3 deletions(-)
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>
Cc: Christoph Fritz <chf.fritz@googlemail.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
---
Changes:
v7: Error handling. Remove unnecessary if.
Adding config data for rk3568 (untested).
v6: just retested over current next branch and some corrections
to message and headers
(no changes to code).
v5: fixes a bug that Christoph Fritz discovered, consisting in the
wrong eror code returned when enabling or disabling the clock
because property_enable() returns an error code in linux but
the modified register value in U-Boot. This caused the clk
disable to abort before freeing the clock.
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).
--
2.20.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock 2023-06-05 15:04 [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Xavier Drudis Ferran @ 2023-06-05 15:05 ` Xavier Drudis Ferran 2023-06-06 0:53 ` Kever Yang 2023-06-06 16:53 ` Jagan Teki 2023-06-05 15:06 ` [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock Xavier Drudis Ferran 2023-06-07 21:42 ` [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Marek Vasut 2 siblings, 2 replies; 13+ messages in thread From: Xavier Drudis Ferran @ 2023-06-05 15:05 UTC (permalink / raw) To: u-boot Cc: Simon Glass, Philipp Tomsich, Kever Yang, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz, Jagan Teki 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 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@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> Cc: Christoph Fritz <chf.fritz@googlemail.com> Cc: Jagan Teki <jagan@amarulasolutions.com> Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> --- 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, -- 2.20.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock 2023-06-05 15:05 ` [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock Xavier Drudis Ferran @ 2023-06-06 0:53 ` Kever Yang 2023-06-06 16:53 ` Jagan Teki 1 sibling, 0 replies; 13+ messages in thread From: Kever Yang @ 2023-06-06 0:53 UTC (permalink / raw) To: Xavier Drudis Ferran, u-boot Cc: Simon Glass, Philipp Tomsich, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz, Jagan Teki On 2023/6/5 23:05, Xavier Drudis Ferran wrote: > 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 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@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> > Cc: Christoph Fritz <chf.fritz@googlemail.com> > Cc: Jagan Teki <jagan@amarulasolutions.com> > > Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> Reviewed-by: Kever Yang <kever.yang@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, ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock 2023-06-05 15:05 ` [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock Xavier Drudis Ferran 2023-06-06 0:53 ` Kever Yang @ 2023-06-06 16:53 ` Jagan Teki 1 sibling, 0 replies; 13+ messages in thread From: Jagan Teki @ 2023-06-06 16:53 UTC (permalink / raw) To: Xavier Drudis Ferran Cc: u-boot, Simon Glass, Philipp Tomsich, Kever Yang, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz On Mon, Jun 5, 2023 at 8:35 PM Xavier Drudis Ferran <xdrudis@tinet.cat> wrote: > > 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 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@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> > Cc: Christoph Fritz <chf.fritz@googlemail.com> > Cc: Jagan Teki <jagan@amarulasolutions.com> > > Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> > --- Thanks for fixing USB from the last couple of releases. Reviewed-by: Jagan Teki <jagan@amarulasolutions.com> Tested-by: Jagan Teki <jagan@amarulasolutions.com> # rk3399, rk3328, rv1126 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock 2023-06-05 15:04 [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Xavier Drudis Ferran 2023-06-05 15:05 ` [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock Xavier Drudis Ferran @ 2023-06-05 15:06 ` Xavier Drudis Ferran 2023-06-06 0:54 ` Kever Yang ` (2 more replies) 2023-06-07 21:42 ` [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Marek Vasut 2 siblings, 3 replies; 13+ messages in thread From: Xavier Drudis Ferran @ 2023-06-05 15:06 UTC (permalink / raw) To: u-boot Cc: Simon Glass, Philipp Tomsich, Kever Yang, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz, Jagan Teki This clock doesn't seem needed but appears in a phandle list used by ehci-generic.c to bulk enable it. The phandle list comes from linux, where it is needed for suspend/resume to work [1]. My tests give the same results with or without this patch, but Marek Vasut found it weird to declare an empty clk_ops [2]. So I adapted the code from linux 6.1-rc8 so that it hopefully works if it ever has some user. For now, without real use, it seems to at least not give any errors when called. Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/ [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/ 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> Cc: Christoph Fritz <chf.fritz@googlemail.com> Cc: Jagan Teki <jagan@amarulasolutions.com> Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> --- v7: add clkout_ctl values for rk3568 (from linux). UNTESTED (I don't have the hardware). v6: just retested over current next branch and some corrections to message and headers (no changes to code). v5: ignores the return value from property_enable() which is not an error code in U-Boot (unlike in linux). This avoid a false failure of rockchip_usb2phy_clk_disable() that interfered with clock disable and appeared to cause hang or reset. --- drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 80 ++++++++++++++++++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c index 732d37201d..be5f79490c 100644 --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c @@ -56,6 +56,7 @@ struct rockchip_usb2phy_port_cfg { struct rockchip_usb2phy_cfg { unsigned int reg; + struct usb2phy_reg clkout_ctl; const struct rockchip_usb2phy_port_cfg port_cfgs[USB2PHY_NUM_PORTS]; }; @@ -77,6 +78,18 @@ static inline int property_enable(void *reg_base, return writel(val, reg_base + reg->offset); } +static inline bool property_enabled(void *reg_base, + const struct usb2phy_reg *reg) +{ + unsigned int tmp, orig; + unsigned int mask = GENMASK(reg->bitend, reg->bitstart); + + orig = readl(reg_base + reg->offset); + + tmp = (orig & mask) >> reg->bitstart; + return tmp != reg->disable; +} + static const struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy) { @@ -169,7 +182,63 @@ static struct phy_ops rockchip_usb2phy_ops = { .of_xlate = rockchip_usb2phy_of_xlate, }; +/** + * round_rate() - Adjust a rate to the exact rate a clock can provide. + * @clk: The clock to manipulate. + * @rate: Desidered clock rate in Hz. + * + * Return: rounded rate in Hz, or -ve error code. + */ +ulong rockchip_usb2phy_clk_round_rate(struct clk *clk, ulong rate) +{ + return 480000000; +} + +/** + * enable() - Enable a clock. + * @clk: The clock to manipulate. + * + * Return: zero on success, or -ve error code. + */ +int rockchip_usb2phy_clk_enable(struct clk *clk) +{ + struct udevice *parent = dev_get_parent(clk->dev); + struct rockchip_usb2phy *priv = dev_get_priv(parent); + const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg; + + /* turn on 480m clk output if it is off */ + if (!property_enabled(priv->reg_base, &phy_cfg->clkout_ctl)) { + property_enable(priv->reg_base, &phy_cfg->clkout_ctl, true); + + /* waiting for the clk become stable */ + usleep_range(1200, 1300); + } + + return 0; +} + +/** + * disable() - Disable a clock. + * @clk: The clock to manipulate. + * + * Return: zero on success, or -ve error code. + */ +int rockchip_usb2phy_clk_disable(struct clk *clk) +{ + struct udevice *parent = dev_get_parent(clk->dev); + struct rockchip_usb2phy *priv = dev_get_priv(parent); + const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg; + + /* turn off 480m clk output */ + property_enable(priv->reg_base, &phy_cfg->clkout_ctl, false); + + return 0; +} + static struct clk_ops rockchip_usb2phy_clk_ops = { + .enable = rockchip_usb2phy_clk_enable, + .disable = rockchip_usb2phy_clk_disable, + .round_rate = rockchip_usb2phy_clk_round_rate }; static int rockchip_usb2phy_probe(struct udevice *dev) @@ -255,8 +324,11 @@ static int rockchip_usb2phy_bind(struct udevice *dev) } node = dev_ofnode(dev); - name = ofnode_get_name(node); - dev_dbg(dev, "clk for node %s\n", name); + name = "clk_usbphy_480m"; + dev_read_string_index(dev, "clock-output-names", 0, &name); + + dev_dbg(dev, "clk %s for node %s\n", name, ofnode_get_name(node)); + ret = device_bind_driver_to_node(dev, "rockchip_usb2phy_clock", name, node, &usb2phy_dev); if (ret) { @@ -276,6 +348,7 @@ bind_fail: static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = { { .reg = 0xe450, + .clkout_ctl = { 0xe450, 4, 4, 1, 0 }, .port_cfgs = { [USB2PHY_PORT_OTG] = { .phy_sus = { 0xe454, 1, 0, 2, 1 }, @@ -297,6 +370,7 @@ static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = { }, { .reg = 0xe460, + .clkout_ctl = { 0xe460, 4, 4, 1, 0 }, .port_cfgs = { [USB2PHY_PORT_OTG] = { .phy_sus = { 0xe464, 1, 0, 2, 1 }, @@ -322,6 +396,7 @@ static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = { static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = { { .reg = 0xfe8a0000, + .clkout_ctl = { 0x0008, 4, 4, 1, 0 }, .port_cfgs = { [USB2PHY_PORT_OTG] = { .phy_sus = { 0x0000, 8, 0, 0x052, 0x1d1 }, @@ -347,6 +422,7 @@ static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = { }, { .reg = 0xfe8b0000, + .clkout_ctl = { 0x0008, 4, 4, 1, 0 }, .port_cfgs = { [USB2PHY_PORT_OTG] = { .phy_sus = { 0x0000, 8, 0, 0x1d2, 0x1d1 }, -- 2.20.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock 2023-06-05 15:06 ` [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock Xavier Drudis Ferran @ 2023-06-06 0:54 ` Kever Yang 2023-06-19 6:34 ` Jagan Teki 2023-06-06 16:54 ` Jagan Teki 2023-06-08 7:36 ` Jagan Teki 2 siblings, 1 reply; 13+ messages in thread From: Kever Yang @ 2023-06-06 0:54 UTC (permalink / raw) To: Xavier Drudis Ferran, u-boot Cc: Simon Glass, Philipp Tomsich, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz, Jagan Teki On 2023/6/5 23:06, Xavier Drudis Ferran wrote: > This clock doesn't seem needed but appears in a phandle list used by > ehci-generic.c to bulk enable it. The phandle list comes from linux, > where it is needed for suspend/resume to work [1]. > > My tests give the same results with or without this patch, but Marek > Vasut found it weird to declare an empty clk_ops [2]. > > So I adapted the code from linux 6.1-rc8 so that it hopefully works > if it ever has some user. For now, without real use, it seems to > at least not give any errors when called. > > Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/ > [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/ > > 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> > Cc: Christoph Fritz <chf.fritz@googlemail.com> > Cc: Jagan Teki <jagan@amarulasolutions.com> > > Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> Thanks, - Kever > --- > > v7: add clkout_ctl values for rk3568 (from linux). > UNTESTED (I don't have the hardware). > > v6: just retested over current next branch and some corrections > to message and headers > (no changes to code). > > v5: ignores the return value from property_enable() which is not > an error code in U-Boot (unlike in linux). This avoid a false > failure of rockchip_usb2phy_clk_disable() that interfered with > clock disable and appeared to cause hang or reset. > --- > drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 80 ++++++++++++++++++- > 1 file changed, 78 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > index 732d37201d..be5f79490c 100644 > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > @@ -56,6 +56,7 @@ struct rockchip_usb2phy_port_cfg { > > struct rockchip_usb2phy_cfg { > unsigned int reg; > + struct usb2phy_reg clkout_ctl; > const struct rockchip_usb2phy_port_cfg port_cfgs[USB2PHY_NUM_PORTS]; > }; > > @@ -77,6 +78,18 @@ static inline int property_enable(void *reg_base, > return writel(val, reg_base + reg->offset); > } > > +static inline bool property_enabled(void *reg_base, > + const struct usb2phy_reg *reg) > +{ > + unsigned int tmp, orig; > + unsigned int mask = GENMASK(reg->bitend, reg->bitstart); > + > + orig = readl(reg_base + reg->offset); > + > + tmp = (orig & mask) >> reg->bitstart; > + return tmp != reg->disable; > +} > + > static const > struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy) > { > @@ -169,7 +182,63 @@ static struct phy_ops rockchip_usb2phy_ops = { > .of_xlate = rockchip_usb2phy_of_xlate, > }; > > +/** > + * round_rate() - Adjust a rate to the exact rate a clock can provide. > + * @clk: The clock to manipulate. > + * @rate: Desidered clock rate in Hz. > + * > + * Return: rounded rate in Hz, or -ve error code. > + */ > +ulong rockchip_usb2phy_clk_round_rate(struct clk *clk, ulong rate) > +{ > + return 480000000; > +} > + > +/** > + * enable() - Enable a clock. > + * @clk: The clock to manipulate. > + * > + * Return: zero on success, or -ve error code. > + */ > +int rockchip_usb2phy_clk_enable(struct clk *clk) > +{ > + struct udevice *parent = dev_get_parent(clk->dev); > + struct rockchip_usb2phy *priv = dev_get_priv(parent); > + const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg; > + > + /* turn on 480m clk output if it is off */ > + if (!property_enabled(priv->reg_base, &phy_cfg->clkout_ctl)) { > + property_enable(priv->reg_base, &phy_cfg->clkout_ctl, true); > + > + /* waiting for the clk become stable */ > + usleep_range(1200, 1300); > + } > + > + return 0; > +} > + > +/** > + * disable() - Disable a clock. > + * @clk: The clock to manipulate. > + * > + * Return: zero on success, or -ve error code. > + */ > +int rockchip_usb2phy_clk_disable(struct clk *clk) > +{ > + struct udevice *parent = dev_get_parent(clk->dev); > + struct rockchip_usb2phy *priv = dev_get_priv(parent); > + const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg; > + > + /* turn off 480m clk output */ > + property_enable(priv->reg_base, &phy_cfg->clkout_ctl, false); > + > + return 0; > +} > + > static struct clk_ops rockchip_usb2phy_clk_ops = { > + .enable = rockchip_usb2phy_clk_enable, > + .disable = rockchip_usb2phy_clk_disable, > + .round_rate = rockchip_usb2phy_clk_round_rate > }; > > static int rockchip_usb2phy_probe(struct udevice *dev) > @@ -255,8 +324,11 @@ static int rockchip_usb2phy_bind(struct udevice *dev) > } > > node = dev_ofnode(dev); > - name = ofnode_get_name(node); > - dev_dbg(dev, "clk for node %s\n", name); > + name = "clk_usbphy_480m"; > + dev_read_string_index(dev, "clock-output-names", 0, &name); > + > + dev_dbg(dev, "clk %s for node %s\n", name, ofnode_get_name(node)); > + > ret = device_bind_driver_to_node(dev, "rockchip_usb2phy_clock", > name, node, &usb2phy_dev); > if (ret) { > @@ -276,6 +348,7 @@ bind_fail: > static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = { > { > .reg = 0xe450, > + .clkout_ctl = { 0xe450, 4, 4, 1, 0 }, > .port_cfgs = { > [USB2PHY_PORT_OTG] = { > .phy_sus = { 0xe454, 1, 0, 2, 1 }, > @@ -297,6 +370,7 @@ static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = { > }, > { > .reg = 0xe460, > + .clkout_ctl = { 0xe460, 4, 4, 1, 0 }, > .port_cfgs = { > [USB2PHY_PORT_OTG] = { > .phy_sus = { 0xe464, 1, 0, 2, 1 }, > @@ -322,6 +396,7 @@ static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = { > static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = { > { > .reg = 0xfe8a0000, > + .clkout_ctl = { 0x0008, 4, 4, 1, 0 }, > .port_cfgs = { > [USB2PHY_PORT_OTG] = { > .phy_sus = { 0x0000, 8, 0, 0x052, 0x1d1 }, > @@ -347,6 +422,7 @@ static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = { > }, > { > .reg = 0xfe8b0000, > + .clkout_ctl = { 0x0008, 4, 4, 1, 0 }, > .port_cfgs = { > [USB2PHY_PORT_OTG] = { > .phy_sus = { 0x0000, 8, 0, 0x1d2, 0x1d1 }, ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock 2023-06-06 0:54 ` Kever Yang @ 2023-06-19 6:34 ` Jagan Teki 2023-06-19 7:08 ` Xavier Drudis Ferran 0 siblings, 1 reply; 13+ messages in thread From: Jagan Teki @ 2023-06-19 6:34 UTC (permalink / raw) To: Kever Yang Cc: Xavier Drudis Ferran, u-boot, Simon Glass, Philipp Tomsich, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz Hi Kever, On Tue, Jun 6, 2023 at 6:24 AM Kever Yang <kever.yang@rock-chips.com> wrote: > > > On 2023/6/5 23:06, Xavier Drudis Ferran wrote: > > This clock doesn't seem needed but appears in a phandle list used by > > ehci-generic.c to bulk enable it. The phandle list comes from linux, > > where it is needed for suspend/resume to work [1]. > > > > My tests give the same results with or without this patch, but Marek > > Vasut found it weird to declare an empty clk_ops [2]. > > > > So I adapted the code from linux 6.1-rc8 so that it hopefully works > > if it ever has some user. For now, without real use, it seems to > > at least not give any errors when called. > > > > Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/ > > [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/ > > > > 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> > > Cc: Christoph Fritz <chf.fritz@googlemail.com> > > Cc: Jagan Teki <jagan@amarulasolutions.com> > > > > Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> > Reviewed-by: Kever Yang <kever.yang@rock-chips.com> Please merge these two asap. Better would these two be part of the coming release? Thanks, Jagan. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock 2023-06-19 6:34 ` Jagan Teki @ 2023-06-19 7:08 ` Xavier Drudis Ferran 2023-06-19 7:12 ` Jagan Teki 0 siblings, 1 reply; 13+ messages in thread From: Xavier Drudis Ferran @ 2023-06-19 7:08 UTC (permalink / raw) To: Jagan Teki Cc: Kever Yang, Xavier Drudis Ferran, u-boot, Simon Glass, Philipp Tomsich, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz El Mon, Jun 19, 2023 at 12:04:51PM +0530, Jagan Teki deia: > > Please merge these two asap. Better would these two be part of the > coming release? > How do you mean ? They're both in master and next now. see commits: e81512ac30c154c320b54036919cd3a6f4cc1516 40359c94405b103d25233d8d727d671748b751b9 Or do you mean I should send fixes to comments and static qualifiers for 3 functions as you pointed out ? https://lists.denx.de/pipermail/u-boot/2023-June/519708.html I wasn't sure if that was a notice for me to do it better next time or it required a clean up patch. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock 2023-06-19 7:08 ` Xavier Drudis Ferran @ 2023-06-19 7:12 ` Jagan Teki 0 siblings, 0 replies; 13+ messages in thread From: Jagan Teki @ 2023-06-19 7:12 UTC (permalink / raw) To: Xavier Drudis Ferran Cc: Kever Yang, u-boot, Simon Glass, Philipp Tomsich, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz On Mon, Jun 19, 2023 at 12:38 PM Xavier Drudis Ferran <xdrudis@tinet.cat> wrote: > > El Mon, Jun 19, 2023 at 12:04:51PM +0530, Jagan Teki deia: > > > > Please merge these two asap. Better would these two be part of the > > coming release? > > > > How do you mean ? > > They're both in master and next now. Ohh. Okay, I didn't see that. > > see commits: > > e81512ac30c154c320b54036919cd3a6f4cc1516 > 40359c94405b103d25233d8d727d671748b751b9 > > Or do you mean I should send fixes to comments and static qualifiers > for 3 functions as you pointed out ? Yes, may be second one. No problem. Thanks, Jagan. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock 2023-06-05 15:06 ` [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock Xavier Drudis Ferran 2023-06-06 0:54 ` Kever Yang @ 2023-06-06 16:54 ` Jagan Teki 2023-06-08 7:36 ` Jagan Teki 2 siblings, 0 replies; 13+ messages in thread From: Jagan Teki @ 2023-06-06 16:54 UTC (permalink / raw) To: Xavier Drudis Ferran Cc: u-boot, Simon Glass, Philipp Tomsich, Kever Yang, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz On Mon, Jun 5, 2023 at 8:37 PM Xavier Drudis Ferran <xdrudis@tinet.cat> wrote: > > This clock doesn't seem needed but appears in a phandle list used by > ehci-generic.c to bulk enable it. The phandle list comes from linux, > where it is needed for suspend/resume to work [1]. > > My tests give the same results with or without this patch, but Marek > Vasut found it weird to declare an empty clk_ops [2]. > > So I adapted the code from linux 6.1-rc8 so that it hopefully works > if it ever has some user. For now, without real use, it seems to > at least not give any errors when called. > > Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/ > [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/ > > 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> > Cc: Christoph Fritz <chf.fritz@googlemail.com> > Cc: Jagan Teki <jagan@amarulasolutions.com> > > Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> > --- Reviewed-by: Jagan Teki <jagan@amarulasolutions.com> Tested-by: Jagan Teki <jagan@amarulasolutions.com> # rk3399, rk3328, rv1126 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock 2023-06-05 15:06 ` [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock Xavier Drudis Ferran 2023-06-06 0:54 ` Kever Yang 2023-06-06 16:54 ` Jagan Teki @ 2023-06-08 7:36 ` Jagan Teki 2 siblings, 0 replies; 13+ messages in thread From: Jagan Teki @ 2023-06-08 7:36 UTC (permalink / raw) To: Xavier Drudis Ferran Cc: u-boot, Simon Glass, Philipp Tomsich, Kever Yang, Lukasz Majewski, Sean Anderson, Marek Vasut, Christoph Fritz On Mon, Jun 5, 2023 at 8:37 PM Xavier Drudis Ferran <xdrudis@tinet.cat> wrote: > > This clock doesn't seem needed but appears in a phandle list used by > ehci-generic.c to bulk enable it. The phandle list comes from linux, > where it is needed for suspend/resume to work [1]. > > My tests give the same results with or without this patch, but Marek > Vasut found it weird to declare an empty clk_ops [2]. > > So I adapted the code from linux 6.1-rc8 so that it hopefully works > if it ever has some user. For now, without real use, it seems to > at least not give any errors when called. > > Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/ > [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/ > > 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> > Cc: Christoph Fritz <chf.fritz@googlemail.com> > Cc: Jagan Teki <jagan@amarulasolutions.com> > > Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> > --- > > v7: add clkout_ctl values for rk3568 (from linux). > UNTESTED (I don't have the hardware). > > v6: just retested over current next branch and some corrections > to message and headers > (no changes to code). > > v5: ignores the return value from property_enable() which is not > an error code in U-Boot (unlike in linux). This avoid a false > failure of rockchip_usb2phy_clk_disable() that interfered with > clock disable and appeared to cause hang or reset. > --- > drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 80 ++++++++++++++++++- > 1 file changed, 78 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > index 732d37201d..be5f79490c 100644 > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > @@ -56,6 +56,7 @@ struct rockchip_usb2phy_port_cfg { > > struct rockchip_usb2phy_cfg { > unsigned int reg; > + struct usb2phy_reg clkout_ctl; > const struct rockchip_usb2phy_port_cfg port_cfgs[USB2PHY_NUM_PORTS]; > }; > > @@ -77,6 +78,18 @@ static inline int property_enable(void *reg_base, > return writel(val, reg_base + reg->offset); > } > > +static inline bool property_enabled(void *reg_base, > + const struct usb2phy_reg *reg) > +{ > + unsigned int tmp, orig; > + unsigned int mask = GENMASK(reg->bitend, reg->bitstart); > + > + orig = readl(reg_base + reg->offset); > + > + tmp = (orig & mask) >> reg->bitstart; > + return tmp != reg->disable; > +} > + > static const > struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy) > { > @@ -169,7 +182,63 @@ static struct phy_ops rockchip_usb2phy_ops = { > .of_xlate = rockchip_usb2phy_of_xlate, > }; > > +/** > + * round_rate() - Adjust a rate to the exact rate a clock can provide. > + * @clk: The clock to manipulate. > + * @rate: Desidered clock rate in Hz. > + * > + * Return: rounded rate in Hz, or -ve error code. > + */ I forgot to comment, this last time. I feel these explicit comments wouldn't be required as clk-uclass clearly documented. > +ulong rockchip_usb2phy_clk_round_rate(struct clk *clk, ulong rate) static > +{ > + return 480000000; > +} > + > +/** > + * enable() - Enable a clock. > + * @clk: The clock to manipulate. > + * > + * Return: zero on success, or -ve error code. > + */ ditto. > +int rockchip_usb2phy_clk_enable(struct clk *clk) ditto. > +{ > + struct udevice *parent = dev_get_parent(clk->dev); > + struct rockchip_usb2phy *priv = dev_get_priv(parent); > + const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg; > + > + /* turn on 480m clk output if it is off */ > + if (!property_enabled(priv->reg_base, &phy_cfg->clkout_ctl)) { > + property_enable(priv->reg_base, &phy_cfg->clkout_ctl, true); > + > + /* waiting for the clk become stable */ > + usleep_range(1200, 1300); > + } > + > + return 0; > +} > + > +/** > + * disable() - Disable a clock. > + * @clk: The clock to manipulate. > + * > + * Return: zero on success, or -ve error code. > + */ ditto. > +int rockchip_usb2phy_clk_disable(struct clk *clk) ditto. Thanks, Jagan. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 2023-06-05 15:04 [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Xavier Drudis Ferran 2023-06-05 15:05 ` [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock Xavier Drudis Ferran 2023-06-05 15:06 ` [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock Xavier Drudis Ferran @ 2023-06-07 21:42 ` Marek Vasut 2023-06-08 6:52 ` Xavier Drudis Ferran 2 siblings, 1 reply; 13+ messages in thread From: Marek Vasut @ 2023-06-07 21:42 UTC (permalink / raw) To: Xavier Drudis Ferran, u-boot Cc: Simon Glass, Philipp Tomsich, Kever Yang, Lukasz Majewski, Sean Anderson, Christoph Fritz, Jagan Teki On 6/5/23 17:04, Xavier Drudis Ferran wrote: > EHCI probing in Rock pi 4 currently fails. > > Add a clock driver for usb2phy so that probing EHCI does not fail when > missing one of the clocks in the bundle for usb_host0_ehci, since > usb2phy is UCLASS_PHY but not UCLASS_CLK. > > Xavier Drudis Ferran (2): > phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock > phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock > > drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 109 +++++++++++++++++- > 1 file changed, 106 insertions(+), 3 deletions(-) Applied both to usb/master . btw the cover letter subject should not have 'arm: dts:' tags, but rather 'phy:' tag , since this does not touch any DTs . ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 2023-06-07 21:42 ` [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Marek Vasut @ 2023-06-08 6:52 ` Xavier Drudis Ferran 0 siblings, 0 replies; 13+ messages in thread From: Xavier Drudis Ferran @ 2023-06-08 6:52 UTC (permalink / raw) To: Marek Vasut Cc: Xavier Drudis Ferran, u-boot, Simon Glass, Philipp Tomsich, Kever Yang, Lukasz Majewski, Sean Anderson, Christoph Fritz, Jagan Teki El Wed, Jun 07, 2023 at 11:42:40PM +0200, Marek Vasut deia: > On 6/5/23 17:04, Xavier Drudis Ferran wrote: > > EHCI probing in Rock pi 4 currently fails. > > > > Add a clock driver for usb2phy so that probing EHCI does not fail when > > missing one of the clocks in the bundle for usb_host0_ehci, since > > usb2phy is UCLASS_PHY but not UCLASS_CLK. > > > > Xavier Drudis Ferran (2): > > phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock > > phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock > > > > drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 109 +++++++++++++++++- > > 1 file changed, 106 insertions(+), 3 deletions(-) > > Applied both to usb/master . > > btw the cover letter subject should not have 'arm: dts:' tags, but rather > 'phy:' tag , since this does not touch any DTs . Yes, sorry. v1 did touch *-u-boot.dts. I hesitated about what was more confusing, changing subject on different versions of a patch with the same intent or keeping the old subject. Thanks for merging. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-06-19 7:13 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-05 15:04 [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Xavier Drudis Ferran 2023-06-05 15:05 ` [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock Xavier Drudis Ferran 2023-06-06 0:53 ` Kever Yang 2023-06-06 16:53 ` Jagan Teki 2023-06-05 15:06 ` [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock Xavier Drudis Ferran 2023-06-06 0:54 ` Kever Yang 2023-06-19 6:34 ` Jagan Teki 2023-06-19 7:08 ` Xavier Drudis Ferran 2023-06-19 7:12 ` Jagan Teki 2023-06-06 16:54 ` Jagan Teki 2023-06-08 7:36 ` Jagan Teki 2023-06-07 21:42 ` [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Marek Vasut 2023-06-08 6:52 ` Xavier Drudis Ferran
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox