Patchwork [4/4,v2] net/smsc911x: Provide common clock functionality

login
register
mail settings
Submitter Lee Jones
Date Dec. 21, 2012, 11:41 a.m.
Message ID <20121221114105.GP2691@gmail.com>
Download mbox | patch
Permalink /patch/207792/
State RFC
Delegated to: David Miller
Headers show

Comments

Lee Jones - Dec. 21, 2012, 11:41 a.m.
Some platforms provide clocks which require enabling before the
SMSC911x chip will power on. This patch uses the new common clk
framework to do just that. If no clock is provided, it will just
be ignored and the driver will continue to assume that no clock
is required for the chip to run successfully.

Cc: Steve Glendinning <steve.glendinning@shawell.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/net/ethernet/smsc/smsc911x.c |   29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)
Linus Walleij - Dec. 26, 2012, 12:51 a.m.
On Fri, Dec 21, 2012 at 12:41 PM, Lee Jones <lee.jones@linaro.org> wrote:

> +       if (IS_ERR(pdata->clk)) {
> +               ret = clk_prepare_enable(pdata->clk);
> +               if (ret < 0)
> +                       netdev_err(ndev, "failed to enable clock %d\n", ret);
> +       }

I think you got all of these backwards now, shouldn't it be if
(!IS_ERR(pdata->clk)) { } ...?

It's late here but enlighten me if I don't get it.

> +       if (IS_ERR(pdata->clk))
> +               clk_disable_unprepare(pdata->clk);

Dito.

> +       /* Request clock */
> +       pdata->clk = clk_get(&pdev->dev, NULL);
> +       if (IS_ERR(pdata->clk))
> +               netdev_warn(ndev, "couldn't get clock %li\n", PTR_ERR(pdata->clk));

This one seems correct though.

> +       /* Free clock */
> +       if (IS_ERR(pdata->clk)) {
> +               clk_put(pdata->clk);
> +               pdata->clk = NULL;
> +       }

Should be !IS_ERR()

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index e112877..afa4d62 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -33,6 +33,7 @@ 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/crc32.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/etherdevice.h>
@@ -144,6 +145,9 @@  struct smsc911x_data {
 
 	/* regulators */
 	struct regulator_bulk_data supplies[SMSC911X_NUM_SUPPLIES];
+
+	/* clock */
+	struct clk *clk;
 };
 
 /* Easy access to information */
@@ -369,7 +373,7 @@  out:
 }
 
 /*
- * enable resources, currently just regulators.
+ * enable regulator and clock resources.
  */
 static int smsc911x_enable_resources(struct platform_device *pdev)
 {
@@ -382,6 +386,13 @@  static int smsc911x_enable_resources(struct platform_device *pdev)
 	if (ret)
 		netdev_err(ndev, "failed to enable regulators %d\n",
 				ret);
+
+	if (IS_ERR(pdata->clk)) {
+		ret = clk_prepare_enable(pdata->clk);
+		if (ret < 0)
+			netdev_err(ndev, "failed to enable clock %d\n", ret);
+	}
+
 	return ret;
 }
 
@@ -396,6 +407,10 @@  static int smsc911x_disable_resources(struct platform_device *pdev)
 
 	ret = regulator_bulk_disable(ARRAY_SIZE(pdata->supplies),
 			pdata->supplies);
+
+	if (IS_ERR(pdata->clk))
+		clk_disable_unprepare(pdata->clk);
+
 	return ret;
 }
 
@@ -421,6 +436,12 @@  static int smsc911x_request_resources(struct platform_device *pdev)
 	if (ret)
 		netdev_err(ndev, "couldn't get regulators %d\n",
 				ret);
+
+	/* Request clock */
+	pdata->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(pdata->clk))
+		netdev_warn(ndev, "couldn't get clock %li\n", PTR_ERR(pdata->clk));
+
 	return ret;
 }
 
@@ -436,6 +457,12 @@  static void smsc911x_free_resources(struct platform_device *pdev)
 	/* Free regulators */
 	regulator_bulk_free(ARRAY_SIZE(pdata->supplies),
 			pdata->supplies);
+
+	/* Free clock */
+	if (IS_ERR(pdata->clk)) {
+		clk_put(pdata->clk);
+		pdata->clk = NULL;
+	}
 }
 
 /* waits for MAC not busy, with timeout.  Only called by smsc911x_mac_read