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(-)

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);
 
-- 
2.36.0

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

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?

-- 
Thank you,
Dmitry