| Message ID | 20231002165857.19512-5-avromanov@salutedevices.com |
|---|---|
| State | Superseded |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | Support USB for Meson A1 | expand |
On 02/10/2023 18:58, Alexey Romanov wrote: > It is better to place clk_enable() in phy_meson_g12a_usb2_init() > and clk_disable() in phy_meson_g12a_usb2_exit(). > > For more detailed information, please see comments in the review of > a similar driver in the Linux Kernel: > > https://lore.kernel.org/all/CAFBinCCEhobbyKHuKDWzTYCQWgNT1-e8=7hMhq1mvT6CuEOjGw@mail.gmail.com/ > > Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> > --- > drivers/phy/meson-g12a-usb2.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/phy/meson-g12a-usb2.c b/drivers/phy/meson-g12a-usb2.c > index 2e366b16ae..7b028784a0 100644 > --- a/drivers/phy/meson-g12a-usb2.c > +++ b/drivers/phy/meson-g12a-usb2.c > @@ -160,6 +160,14 @@ static int phy_meson_g12a_usb2_init(struct phy *phy) > struct phy_meson_g12a_usb2_priv *priv = dev_get_priv(dev); > int ret; > > +#if CONFIG_IS_ENABLED(CLK) > + ret = clk_enable(&priv->clk); > + if (ret && ret != -ENOSYS && ret != -ENOTSUPP) { > + pr_err("failed to enable PHY clock\n"); > + return ret; > + } > +#endif > + > ret = reset_assert(&priv->reset); > udelay(1); > ret |= reset_deassert(&priv->reset); > @@ -252,6 +260,10 @@ static int phy_meson_g12a_usb2_exit(struct phy *phy) > struct phy_meson_g12a_usb2_priv *priv = dev_get_priv(dev); > int ret; > > +#if CONFIG_IS_ENABLED(CLK) > + clk_disable(&priv->clk); > +#endif > + > ret = reset_assert(&priv->reset); > if (ret) > return ret; > @@ -289,13 +301,6 @@ int meson_g12a_usb2_phy_probe(struct udevice *dev) > ret = clk_get_by_index(dev, 0, &priv->clk); > if (ret < 0) > return ret; > - > - ret = clk_enable(&priv->clk); > - if (ret && ret != -ENOSYS && ret != -ENOTSUPP) { > - pr_err("failed to enable PHY clock\n"); > - clk_free(&priv->clk); > - return ret; > - } > #endif > > return 0; Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
diff --git a/drivers/phy/meson-g12a-usb2.c b/drivers/phy/meson-g12a-usb2.c index 2e366b16ae..7b028784a0 100644 --- a/drivers/phy/meson-g12a-usb2.c +++ b/drivers/phy/meson-g12a-usb2.c @@ -160,6 +160,14 @@ static int phy_meson_g12a_usb2_init(struct phy *phy) struct phy_meson_g12a_usb2_priv *priv = dev_get_priv(dev); int ret; +#if CONFIG_IS_ENABLED(CLK) + ret = clk_enable(&priv->clk); + if (ret && ret != -ENOSYS && ret != -ENOTSUPP) { + pr_err("failed to enable PHY clock\n"); + return ret; + } +#endif + ret = reset_assert(&priv->reset); udelay(1); ret |= reset_deassert(&priv->reset); @@ -252,6 +260,10 @@ static int phy_meson_g12a_usb2_exit(struct phy *phy) struct phy_meson_g12a_usb2_priv *priv = dev_get_priv(dev); int ret; +#if CONFIG_IS_ENABLED(CLK) + clk_disable(&priv->clk); +#endif + ret = reset_assert(&priv->reset); if (ret) return ret; @@ -289,13 +301,6 @@ int meson_g12a_usb2_phy_probe(struct udevice *dev) ret = clk_get_by_index(dev, 0, &priv->clk); if (ret < 0) return ret; - - ret = clk_enable(&priv->clk); - if (ret && ret != -ENOSYS && ret != -ENOTSUPP) { - pr_err("failed to enable PHY clock\n"); - clk_free(&priv->clk); - return ret; - } #endif return 0;
It is better to place clk_enable() in phy_meson_g12a_usb2_init() and clk_disable() in phy_meson_g12a_usb2_exit(). For more detailed information, please see comments in the review of a similar driver in the Linux Kernel: https://lore.kernel.org/all/CAFBinCCEhobbyKHuKDWzTYCQWgNT1-e8=7hMhq1mvT6CuEOjGw@mail.gmail.com/ Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> --- drivers/phy/meson-g12a-usb2.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)