| Message ID | 00ff6ad6-4554-2ce5-32ba-de47dcfcd81b@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: move getting (R)MII refclock to phylib | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 18 this patch: 18 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | success | Errors and warnings before: 18 this patch: 18 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 18 this patch: 18 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 21 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On 3/24/23 11:03, Heiner Kallweit wrote: > Now that getting the reference clock has been moved to phylib, > we can remove it here. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/phy/smsc.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c > index 730964b85..48654c684 100644 > --- a/drivers/net/phy/smsc.c > +++ b/drivers/net/phy/smsc.c > @@ -278,7 +278,6 @@ int smsc_phy_probe(struct phy_device *phydev) > { > struct device *dev = &phydev->mdio.dev; > struct smsc_phy_priv *priv; > - struct clk *refclk; > > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > if (!priv) > @@ -291,13 +290,7 @@ int smsc_phy_probe(struct phy_device *phydev) > > phydev->priv = priv; > > - /* Make clk optional to keep DTB backward compatibility. */ > - refclk = devm_clk_get_optional_enabled(dev, NULL); > - if (IS_ERR(refclk)) > - return dev_err_probe(dev, PTR_ERR(refclk), > - "Failed to request clock\n"); > - > - return clk_set_rate(refclk, 50 * 1000 * 1000); > + return clk_set_rate(phydev->refclk, 50 * 1000 * 1000); AFAIR one should be calling clk_prepare_enable() before clk_set_rate(), which neither smsc.c nor micrel.c do. If we insist on moving this code to the PHY library which I have no strong objections against, we might provide a PHY_REQUIRES_REFCLK flag that the generic code can key off, or in the same of bcm7xx.c: PHY_LET_ME_MANAGED_MY_CLOCK?
On 3/24/23 11:03, Heiner Kallweit wrote: > Now that getting the reference clock has been moved to phylib, > we can remove it here. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/phy/smsc.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c > index 730964b85..48654c684 100644 > --- a/drivers/net/phy/smsc.c > +++ b/drivers/net/phy/smsc.c > @@ -278,7 +278,6 @@ int smsc_phy_probe(struct phy_device *phydev) > { > struct device *dev = &phydev->mdio.dev; > struct smsc_phy_priv *priv; > - struct clk *refclk; > > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > if (!priv) > @@ -291,13 +290,7 @@ int smsc_phy_probe(struct phy_device *phydev) > > phydev->priv = priv; > > - /* Make clk optional to keep DTB backward compatibility. */ > - refclk = devm_clk_get_optional_enabled(dev, NULL); > - if (IS_ERR(refclk)) > - return dev_err_probe(dev, PTR_ERR(refclk), > - "Failed to request clock\n"); > - > - return clk_set_rate(refclk, 50 * 1000 * 1000); > + return clk_set_rate(phydev->refclk, 50 * 1000 * 1000); AFAIR one should be calling clk_prepare_enable() before clk_set_rate(), which neither smsc.c nor micrel.c do. If we insist on moving this code to the PHY library which I have no strong objections against, we might provide a PHY_REQUIRES_REFCLK flag that the generic code can key off, or in the same of bcm7xx.c: PHY_LET_ME_MANAGED_MY_CLOCK?
On 3/24/23 12:07, Florian Fainelli wrote: > On 3/24/23 11:03, Heiner Kallweit wrote: >> Now that getting the reference clock has been moved to phylib, >> we can remove it here. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/net/phy/smsc.c | 9 +-------- >> 1 file changed, 1 insertion(+), 8 deletions(-) >> >> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c >> index 730964b85..48654c684 100644 >> --- a/drivers/net/phy/smsc.c >> +++ b/drivers/net/phy/smsc.c >> @@ -278,7 +278,6 @@ int smsc_phy_probe(struct phy_device *phydev) >> { >> struct device *dev = &phydev->mdio.dev; >> struct smsc_phy_priv *priv; >> - struct clk *refclk; >> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> if (!priv) >> @@ -291,13 +290,7 @@ int smsc_phy_probe(struct phy_device *phydev) >> phydev->priv = priv; >> - /* Make clk optional to keep DTB backward compatibility. */ >> - refclk = devm_clk_get_optional_enabled(dev, NULL); >> - if (IS_ERR(refclk)) >> - return dev_err_probe(dev, PTR_ERR(refclk), >> - "Failed to request clock\n"); >> - >> - return clk_set_rate(refclk, 50 * 1000 * 1000); >> + return clk_set_rate(phydev->refclk, 50 * 1000 * 1000); > > AFAIR one should be calling clk_prepare_enable() before clk_set_rate(), > which neither smsc.c nor micrel.c do. Which is implied by the devm_clk_get_optional_enabled().
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c index 730964b85..48654c684 100644 --- a/drivers/net/phy/smsc.c +++ b/drivers/net/phy/smsc.c @@ -278,7 +278,6 @@ int smsc_phy_probe(struct phy_device *phydev) { struct device *dev = &phydev->mdio.dev; struct smsc_phy_priv *priv; - struct clk *refclk; priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) @@ -291,13 +290,7 @@ int smsc_phy_probe(struct phy_device *phydev) phydev->priv = priv; - /* Make clk optional to keep DTB backward compatibility. */ - refclk = devm_clk_get_optional_enabled(dev, NULL); - if (IS_ERR(refclk)) - return dev_err_probe(dev, PTR_ERR(refclk), - "Failed to request clock\n"); - - return clk_set_rate(refclk, 50 * 1000 * 1000); + return clk_set_rate(phydev->refclk, 50 * 1000 * 1000); } EXPORT_SYMBOL_GPL(smsc_phy_probe);
Now that getting the reference clock has been moved to phylib, we can remove it here. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/smsc.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)