On 9/3/2020 11:15 PM, Marco Felsch wrote:
Hi Florian,
On 20-09-02 21:39, Florian Fainelli wrote:
The internal Gigabit PHY on Broadcom STB chips has a digital clock which
drives its MDIO interface among other things, the driver now requests
and manage that clock during .probe() and .remove() accordingly.
Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
---
drivers/net/phy/bcm7xxx.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 692048d86ab1..f0ffcdcaef03 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -11,6 +11,7 @@
#include "bcm-phy-lib.h"
#include <linux/bitops.h>
#include <linux/brcmphy.h>
+#include <linux/clk.h>
#include <linux/mdio.h>
/* Broadcom BCM7xxx internal PHY registers */
@@ -39,6 +40,7 @@
struct bcm7xxx_phy_priv {
u64 *stats;
+ struct clk *clk;
};
static int bcm7xxx_28nm_d0_afe_config_init(struct phy_device *phydev)
@@ -534,7 +536,19 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
if (!priv->stats)
return -ENOMEM;
- return 0;
+ priv->clk = devm_clk_get_optional(&phydev->mdio.dev, NULL);
Since the clock is binded to the mdio-dev here..
+ if (IS_ERR(priv->clk))
+ return PTR_ERR(priv->clk);
+
+ return clk_prepare_enable(priv->clk);
clould we use devm_add_action_or_reset() here so we don't have to
register the .remove() hook?
Maybe, more on that below.
+}
+
+static void bcm7xxx_28nm_remove(struct phy_device *phydev)
+{
+ struct bcm7xxx_phy_priv *priv = phydev->priv;
+
+ clk_disable_unprepare(priv->clk);
+ devm_clk_put(&phydev->mdio.dev, priv->clk);
Is this really necessary? The devm_clk_get_optional() function already
registers the devm_clk_release() hook.
Yes, because you can unbind the PHY driver from sysfs, and if you want
to bind that driver again, which will call .probe() again, you must undo
strictly everything that .probe() did. The embedded mdio_device does not
go away, so there will be no automatic freeing of resources. Using
devm_* may be confusing, so using just the plain clk_get() and clk_put()
may be clearer here.
--
Florian
