diff mbox series

[v1,1/5] phy: amlogic: during USB PHY clkin obtaining, enable it

Message ID 20230414152423.19842-2-ddrokosov@sberdevices.ru
State New
Headers
Series arm64: meson: support Amlogic A1 USB OTG controller |

Commit Message

Dmitry Rokosov April 14, 2023, 3:24 p.m. UTC 
  Previously, all Amlogic boards used the XTAL clock as the USB PHY input
clock, and it did not need to be enabled as it was the default board
clock. However, in new Amlogic SoCs such as the A1 family, USB PHY uses
a gated clock, so it is necessary to enable this gated clock during
probing.

Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 drivers/phy/amlogic/phy-meson-g12a-usb2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Martin Blumenstingl April 16, 2023, 8:54 p.m. UTC | #1 
Hi Dmitry,

On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
[...]
> -       priv->clk = devm_clk_get(dev, "xtal");
> +       priv->clk = devm_clk_get_enabled(dev, "xtal");
Generally this works fine but I wouldn't recommend this approach if:
- there's some required wait time after the clock has been enabled
(see phy_meson_g12a_usb2_init - there's already some required wait
time after triggering the reset)
- clock gating (for power saving) is needed when the dwc3 driver is
unloaded by the PHY driver is not

In this case: just manually manage the clock in phy_meson_g12a_usb2_{init,exit}


Best regards,
Martin
Dmitry Rokosov April 17, 2023, 11:57 a.m. UTC | #2 
On Sun, Apr 16, 2023 at 10:54:17PM +0200, Martin Blumenstingl wrote:
> Hi Dmitry,
> 
> On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
> [...]
> > -       priv->clk = devm_clk_get(dev, "xtal");
> > +       priv->clk = devm_clk_get_enabled(dev, "xtal");
> Generally this works fine but I wouldn't recommend this approach if:
> - there's some required wait time after the clock has been enabled
> (see phy_meson_g12a_usb2_init - there's already some required wait
> time after triggering the reset)
> - clock gating (for power saving) is needed when the dwc3 driver is
> unloaded by the PHY driver is not
> 
> In this case: just manually manage the clock in phy_meson_g12a_usb2_{init,exit}

I'm sorry, but I'm not fully understanding your point. Currently, no
sleeps are required for this clock and we don't have any logic for
power saving (g12a phy_ops doesn't have power_on()/power_off()
implementation).
However, I believe all of your arguments make sense for the future
development of the phy_meson_g12a_usb2 driver. Is that correct?
diff mbox series

Patch

diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
index 9d1efa0d9394..26b99fbe1026 100644
--- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
+++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
@@ -315,7 +315,7 @@  static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	priv->clk = devm_clk_get(dev, "xtal");
+	priv->clk = devm_clk_get_enabled(dev, "xtal");
 	if (IS_ERR(priv->clk))
 		return PTR_ERR(priv->clk);