net: phy: micrel: support !CONFIG_HAVE_CLK

Since NULL is a valid clock, we shouldn't use
IS_ERR_OR_NULL.

Implemented as Russell King suggested in:

http://lkml.kernel.org/r/[email protected]

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/net/phy/micrel.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/net/phy/micrel.c +1 -0

index 1190fd8..d958d13 100644
@@ -522,6 +522,7 @@ static int kszphy_probe(struct phy_device *phydev)
 	struct device_node *np = phydev->dev.of_node;
 	struct kszphy_priv *priv;
 	struct clk *clk;
+	unsigned long rate = 0;
 	int ret;
 
 	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);

Quoted 25 lines · Niklas Cassel

Since NULL is a valid clock, we shouldn't use
IS_ERR_OR_NULL.

Implemented as Russell King suggested in:

http://lkml.kernel.org/r/[email protected]

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/net/phy/micrel.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/net/phy/micrel.c +1 -0

index 1190fd8..d958d13 100644
@@ -522,6 +522,7 @@ static int kszphy_probe(struct phy_device *phydev)
 	struct device_node *np = phydev->dev.of_node;
 	struct kszphy_priv *priv;
 	struct clk *clk;
+	unsigned long rate = 0;
 	int ret;
 
 	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);

drivers/net/phy/micrel.c static int kszphy_probe(struct phy_device *phydev) · line 555

Quoted 9 lines

drivers/net/phy/micrel.c +4 -2

@@ -548,8 +549,10 @@ static int kszphy_probe(struct phy_device *phydev)
 	}
 
 	clk = devm_clk_get(&phydev->dev, "rmii-ref");
-	if (!IS_ERR(clk)) {
-		unsigned long rate = clk_get_rate(clk);
+	if (!IS_ERR(clk))
+		rate = clk_get_rate(clk);
+
+	if (rate) {

And how do you expect this to work as clk_get_rate returns -EINVAL when clk is NULL?

Have you not tested your patch?

Quoted 39 lines · Niklas Cassel

Quoted 25 lines

Since NULL is a valid clock, we shouldn't use
IS_ERR_OR_NULL.

Implemented as Russell King suggested in:

http://lkml.kernel.org/r/[email protected]

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/net/phy/micrel.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/net/phy/micrel.c +1 -0

index 1190fd8..d958d13 100644
@@ -522,6 +522,7 @@ static int kszphy_probe(struct phy_device *phydev)
 	struct device_node *np = phydev->dev.of_node;
 	struct kszphy_priv *priv;
 	struct clk *clk;
+	unsigned long rate = 0;
 	int ret;
 
 	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);

drivers/net/phy/micrel.c static int kszphy_probe(struct phy_device *phydev) · line 555

Quoted 9 lines

drivers/net/phy/micrel.c +4 -2

@@ -548,8 +549,10 @@ static int kszphy_probe(struct phy_device *phydev)
 	}
 
 	clk = devm_clk_get(&phydev->dev, "rmii-ref");
-	if (!IS_ERR(clk)) {
-		unsigned long rate = clk_get_rate(clk);
+	if (!IS_ERR(clk))
+		rate = clk_get_rate(clk);
+
+	if (rate) {

And how do you expect this to work as clk_get_rate returns -EINVAL when clk is NULL?

Quoted 1 line

Have you not tested your patch?

Nevermind, it is 0 if !CONFIG_HAVE_CLK, but still does not look like the right fix.

Since NULL is a valid clock, we shouldn't use
IS_ERR_OR_NULL.

        clk = devm_clk_get(&phydev->dev, "rmii-ref");

Quoted 2 lines · Niklas Cassel

Since NULL is a valid clock, we shouldn't use
IS_ERR_OR_NULL.

Yes, but this code is not using IS_ERR_OR_NULL.

It seems that you are not describing the problem you are trying to solve.

What is the exact issue you are seeing?

Quoted 1 line

        clk = devm_clk_get(&phydev->dev, "rmii-ref");

You need to provide the 'rmii-ref' in your board file or dts.

Are you doing this?

Quoted 9 lines · Fabio Estevam

Quoted 2 lines · Niklas Cassel

Since NULL is a valid clock, we shouldn't use
IS_ERR_OR_NULL.

Yes, but this code is not using IS_ERR_OR_NULL.

It seems that you are not describing the problem you are trying to solve.

Quoted 8 lines

What is the exact issue you are seeing?

Quoted 1 line

        clk = devm_clk_get(&phydev->dev, "rmii-ref");

You need to provide the 'rmii-ref' in your board file or dts.

Are you doing this?

I'm sorry, my commit message didn't really describe the problem properly.

When you compile your kernel without the clk.h API (CONFIG_HAVE_CLK), devm_clk_get returns NULL (which also happens to be a valid clock in the clk.h API). So it's not a matter of DT or board-files. This is on a mips platform without DT support.

I simply want the drivers/net/phy/micrel.c driver to also work without CONFIG_HAVE_CLK, like it did before commit

63f44b2bfccdd98193bbd602747f780c0fae0f02
net: phy: micrel: add generic clock-mode-select support

Now I get an error every time I boot with "Clock rate out of range: 0", since clk_get_rate returns 0 when compiling with !CONFIG_HAVE_CLK.

It is confusing that devm_clk_get returns a valid clock in the clk.h API (NULL), when compiling with !CONFIG_HAVE_CLK, but according to Russell King in:

http://lkml.kernel.org/r/[email protected]

this was intentional. (explained in the lkml mail above.)

In the same mail Russell King also suggests how a driver can support both !CONFIG_HAVE_CLK and CONFIG_HAVE_CLK.

This is the suggestion I followed when writing my patch.