[PATCH 1/4] ARM: ux500: Provide a means to obtain the SMSC9115 clock when DT is enabled

[PATCH 1/4] ARM: ux500: Provide a means to obtain the SMSC9115 clock when DT is enabled

[Lee Jones]

Device Tree names devices differently to how some frameworks expect
them. Until we can move a platform over to the new way of obtaining
resources, we have to use the OF_DEV_AUXDATA() macros to craft a
device name which can be used in searches during allocation time.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/cpu-db8500.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index db0bb75..dfb68ce 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -284,6 +284,7 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80128000, "nmk-i2c.2", NULL),
 	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80110000, "nmk-i2c.3", NULL),
 	OF_DEV_AUXDATA("st,nomadik-i2c", 0x8012a000, "nmk-i2c.4", NULL),
+	OF_DEV_AUXDATA("smsc,lan9115", 0x50000000, "smsc911x", NULL),
 	/* Requires device name bindings. */
 	OF_DEV_AUXDATA("stericsson,nmk_pinctrl", 0, "pinctrl-db8500", NULL),
 	/* Requires clock name and DMA bindings. */
-- 
1.7.9.5

[PATCH 2/4] clk: ux500: Ensure the FMSC clock is obtainable

[Lee Jones]

The FMSC clock is traditionally used for NAND flash devices when
used on the ux500 series platforms. This patch makes it searchable
during a clock-name search.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/clk/ux500/u8500_clk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index 6b889a0..a601802 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -324,7 +324,7 @@ void u8500_clk_init(void)
 
 	clk = clk_reg_prcc_pclk("p3_pclk0", "per3clk", U8500_CLKRST3_BASE,
 				BIT(0), 0);
-	clk_register_clkdev(clk, NULL, "fsmc");
+	clk_register_clkdev(clk, "fsmc", NULL);
 
 	clk = clk_reg_prcc_pclk("p3_pclk1", "per3clk", U8500_CLKRST3_BASE,
 				BIT(1), 0);
-- 
1.7.9.5

[PATCH 2/4] clk: ux500: Ensure the FMSC clock is obtainable

[Mike Turquette]

Quoting Lee Jones (2012-12-19 09:19:45)
> The FMSC clock is traditionally used for NAND flash devices when
> used on the ux500 series platforms. This patch makes it searchable
> during a clock-name search.
> 
> Cc: Mike Turquette <mturquette@linaro.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Mike Turquette <mturquette@linaro.org>

Who do you want this series to go through?

Regards,
Mike

> ---
>  drivers/clk/ux500/u8500_clk.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> index 6b889a0..a601802 100644
> --- a/drivers/clk/ux500/u8500_clk.c
> +++ b/drivers/clk/ux500/u8500_clk.c
> @@ -324,7 +324,7 @@ void u8500_clk_init(void)
>  
>         clk = clk_reg_prcc_pclk("p3_pclk0", "per3clk", U8500_CLKRST3_BASE,
>                                 BIT(0), 0);
> -       clk_register_clkdev(clk, NULL, "fsmc");
> +       clk_register_clkdev(clk, "fsmc", NULL);
>  
>         clk = clk_reg_prcc_pclk("p3_pclk1", "per3clk", U8500_CLKRST3_BASE,
>                                 BIT(1), 0);
> -- 
> 1.7.9.5

[PATCH 2/4] clk: ux500: Ensure the FMSC clock is obtainable

[Lee Jones]

On Tue, 15 Jan 2013, Mike Turquette wrote:

> Quoting Lee Jones (2012-12-19 09:19:45)
> > The FMSC clock is traditionally used for NAND flash devices when
> > used on the ux500 series platforms. This patch makes it searchable
> > during a clock-name search.
> > 
> > Cc: Mike Turquette <mturquette@linaro.org>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Acked-by: Mike Turquette <mturquette@linaro.org>
> 
> Who do you want this series to go through?

To be honest I don't really mind. I can take them through the
arm-soc tree with "[1/4] ARM ...", but I still need an Ack from
the SMSC911x maintainer for "[4/4] ...", before I can do that.

Still waiting for that. :(

> > ---
> >  drivers/clk/ux500/u8500_clk.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> > index 6b889a0..a601802 100644
> > --- a/drivers/clk/ux500/u8500_clk.c
> > +++ b/drivers/clk/ux500/u8500_clk.c
> > @@ -324,7 +324,7 @@ void u8500_clk_init(void)
> >  
> >         clk = clk_reg_prcc_pclk("p3_pclk0", "per3clk", U8500_CLKRST3_BASE,
> >                                 BIT(0), 0);
> > -       clk_register_clkdev(clk, NULL, "fsmc");
> > +       clk_register_clkdev(clk, "fsmc", NULL);
> >  
> >         clk = clk_reg_prcc_pclk("p3_pclk1", "per3clk", U8500_CLKRST3_BASE,
> >                                 BIT(1), 0);
> > -- 
> > 1.7.9.5

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 3/4] clk: ux500: Provide an alias for the SMSC911x Ethernet chip

[Lee Jones]

In the case of some of the ux500 platforms, an Ethernet chip is
placed on an extended bus which is traditionally used as a NAND
flash chip placeholder. The p3_pclk0 clock is used to control it,
so we are required to provide and easy way to access it from the
SMSC911x driver. We do this using an alias provided by this patch.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/clk/ux500/u8500_clk.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index a601802..9d9add1 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -325,6 +325,7 @@ void u8500_clk_init(void)
 	clk = clk_reg_prcc_pclk("p3_pclk0", "per3clk", U8500_CLKRST3_BASE,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "fsmc", NULL);
+	clk_register_clkdev(clk, NULL, "smsc911x");
 
 	clk = clk_reg_prcc_pclk("p3_pclk1", "per3clk", U8500_CLKRST3_BASE,
 				BIT(1), 0);
-- 
1.7.9.5

[PATCH 3/4] clk: ux500: Provide an alias for the SMSC911x Ethernet chip

[Lee Jones]

On Wed, 19 Dec 2012, Lee Jones wrote:

> In the case of some of the ux500 platforms, an Ethernet chip is
> placed on an extended bus which is traditionally used as a NAND
> flash chip placeholder. The p3_pclk0 clock is used to control it,
> so we are required to provide and easy way to access it from the
> SMSC911x driver. We do this using an alias provided by this patch.
> 
> Cc: Mike Turquette <mturquette@linaro.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/clk/ux500/u8500_clk.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> index a601802..9d9add1 100644
> --- a/drivers/clk/ux500/u8500_clk.c
> +++ b/drivers/clk/ux500/u8500_clk.c
> @@ -325,6 +325,7 @@ void u8500_clk_init(void)
>  	clk = clk_reg_prcc_pclk("p3_pclk0", "per3clk", U8500_CLKRST3_BASE,
>  				BIT(0), 0);
>  	clk_register_clkdev(clk, "fsmc", NULL);
> +	clk_register_clkdev(clk, NULL, "smsc911x");
>  
>  	clk = clk_reg_prcc_pclk("p3_pclk1", "per3clk", U8500_CLKRST3_BASE,
>  				BIT(1), 0);
> -- 
> 1.7.9.5
> 

I still need a maintiner Ack for this before I can push it.

Mike?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 3/4] clk: ux500: Provide an alias for the SMSC911x Ethernet chip

[Ulf Hansson]

On 9 January 2013 09:56, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 19 Dec 2012, Lee Jones wrote:
>
>> In the case of some of the ux500 platforms, an Ethernet chip is
>> placed on an extended bus which is traditionally used as a NAND
>> flash chip placeholder. The p3_pclk0 clock is used to control it,
>> so we are required to provide and easy way to access it from the
>> SMSC911x driver. We do this using an alias provided by this patch.
>>
>> Cc: Mike Turquette <mturquette@linaro.org>
>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> ---
>>  drivers/clk/ux500/u8500_clk.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
>> index a601802..9d9add1 100644
>> --- a/drivers/clk/ux500/u8500_clk.c
>> +++ b/drivers/clk/ux500/u8500_clk.c
>> @@ -325,6 +325,7 @@ void u8500_clk_init(void)
>>       clk = clk_reg_prcc_pclk("p3_pclk0", "per3clk", U8500_CLKRST3_BASE,
>>                               BIT(0), 0);
>>       clk_register_clkdev(clk, "fsmc", NULL);
>> +     clk_register_clkdev(clk, NULL, "smsc911x");
>>
>>       clk = clk_reg_prcc_pclk("p3_pclk1", "per3clk", U8500_CLKRST3_BASE,
>>                               BIT(1), 0);
>> --
>> 1.7.9.5
>>
>
> I still need a maintiner Ack for this before I can push it.
>
> Mike?
>
> --
> Lee Jones
> Linaro ST-Ericsson Landing Team Lead
> Linaro.org ? Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

[PATCH 3/4] clk: ux500: Provide an alias for the SMSC911x Ethernet chip

[Mike Turquette]

Quoting Lee Jones (2013-01-09 00:56:36)
> On Wed, 19 Dec 2012, Lee Jones wrote:
> 
> > In the case of some of the ux500 platforms, an Ethernet chip is
> > placed on an extended bus which is traditionally used as a NAND
> > flash chip placeholder. The p3_pclk0 clock is used to control it,
> > so we are required to provide and easy way to access it from the
> > SMSC911x driver. We do this using an alias provided by this patch.
> > 
> > Cc: Mike Turquette <mturquette@linaro.org>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/clk/ux500/u8500_clk.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> > index a601802..9d9add1 100644
> > --- a/drivers/clk/ux500/u8500_clk.c
> > +++ b/drivers/clk/ux500/u8500_clk.c
> > @@ -325,6 +325,7 @@ void u8500_clk_init(void)
> >       clk = clk_reg_prcc_pclk("p3_pclk0", "per3clk", U8500_CLKRST3_BASE,
> >                               BIT(0), 0);
> >       clk_register_clkdev(clk, "fsmc", NULL);
> > +     clk_register_clkdev(clk, NULL, "smsc911x");
> >  
> >       clk = clk_reg_prcc_pclk("p3_pclk1", "per3clk", U8500_CLKRST3_BASE,
> >                               BIT(1), 0);
> > -- 
> > 1.7.9.5
> > 
> 
> I still need a maintiner Ack for this before I can push it.
> 
> Mike?
> 

Acked-by: Mike Turquette <mturquette@linaro.org>

> -- 
> Lee Jones
> Linaro ST-Ericsson Landing Team Lead
> Linaro.org ? Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

[PATCH 4/4] net/smsc911x: Provide common clock functionality

[Lee Jones]

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 at vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/net/ethernet/smsc/smsc911x.c |   31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 4616bf2..f6196cd 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 (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 (pdata->clk)
+		clk_disable_unprepare(pdata->clk);
+
 	return ret;
 }
 
@@ -421,6 +436,14 @@ 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 %d\n", PTR_ERR(pdata->clk));
+		pdata->clk = NULL;
+	}
+
 	return ret;
 }
 
@@ -436,6 +459,12 @@ static void smsc911x_free_resources(struct platform_device *pdev)
 	/* Free regulators */
 	regulator_bulk_free(ARRAY_SIZE(pdata->supplies),
 			pdata->supplies);
+
+	/* Free clock */
+	if (pdata->clk) {
+		clk_put(pdata->clk);
+		pdata->clk = NULL;
+	}
 }
 
 /* waits for MAC not busy, with timeout.  Only called by smsc911x_mac_read
-- 
1.7.9.5

[PATCH 4/4] net/smsc911x: Provide common clock functionality

[Linus Walleij]

On Wed, Dec 19, 2012 at 6:19 PM, Lee Jones <lee.jones@linaro.org> wrote:

> 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 at vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Seems to me like it'll do the trick.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 4/4] net/smsc911x: Provide common clock functionality

[Russell King - ARM Linux]

On Thu, Dec 20, 2012 at 08:12:08PM +0100, Linus Walleij wrote:
> On Wed, Dec 19, 2012 at 6:19 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > 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 at vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Seems to me like it'll do the trick.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

This looks fairly dangerous.  What about those platforms which use this
driver, but don't provide a clock for it?

It looks like this will result in those platforms losing their ethernet
support.  There's at least a bunch of the ARM evaluation boards which
make use of this driver...

[PATCH 4/4] net/smsc911x: Provide common clock functionality

[Lee Jones]

On Thu, 20 Dec 2012, Russell King - ARM Linux wrote:

> On Thu, Dec 20, 2012 at 08:12:08PM +0100, Linus Walleij wrote:
> > On Wed, Dec 19, 2012 at 6:19 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > 
> > > 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 at vger.kernel.org
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > Seems to me like it'll do the trick.
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> This looks fairly dangerous.  What about those platforms which use this
> driver, but don't provide a clock for it?
> 
> It looks like this will result in those platforms losing their ethernet
> support.  There's at least a bunch of the ARM evaluation boards which
> make use of this driver...

Right, but nothing should regress. If no clock is provided the driver
moves on during the request and will refuse to prepare, enable and
disable there after. 

Unless I've made a mistake somewhere? If so, I'd be happy to fixup.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 4/4] net/smsc911x: Provide common clock functionality

[Russell King - ARM Linux]

On Thu, Dec 20, 2012 at 08:35:14PM +0000, Lee Jones wrote:
> On Thu, 20 Dec 2012, Russell King - ARM Linux wrote:
> 
> > On Thu, Dec 20, 2012 at 08:12:08PM +0100, Linus Walleij wrote:
> > > On Wed, Dec 19, 2012 at 6:19 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > > 
> > > > 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 at vger.kernel.org
> > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > 
> > > Seems to me like it'll do the trick.
> > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > 
> > This looks fairly dangerous.  What about those platforms which use this
> > driver, but don't provide a clock for it?
> > 
> > It looks like this will result in those platforms losing their ethernet
> > support.  There's at least a bunch of the ARM evaluation boards which
> > make use of this driver...
> 
> Right, but nothing should regress. If no clock is provided the driver
> moves on during the request and will refuse to prepare, enable and
> disable there after. 
> 
> Unless I've made a mistake somewhere? If so, I'd be happy to fixup.

No, but... don't use NULL for that.  Use IS_ERR(pdata->clk) instead.

[PATCH 4/4] net/smsc911x: Provide common clock functionality

[Lee Jones]

On Thu, 20 Dec 2012, Russell King - ARM Linux wrote:

> On Thu, Dec 20, 2012 at 08:35:14PM +0000, Lee Jones wrote:
> > On Thu, 20 Dec 2012, Russell King - ARM Linux wrote:
> > 
> > > On Thu, Dec 20, 2012 at 08:12:08PM +0100, Linus Walleij wrote:
> > > > On Wed, Dec 19, 2012 at 6:19 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > > > 
> > > > > 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 at vger.kernel.org
> > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > > 
> > > > Seems to me like it'll do the trick.
> > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > 
> > > This looks fairly dangerous.  What about those platforms which use this
> > > driver, but don't provide a clock for it?
> > > 
> > > It looks like this will result in those platforms losing their ethernet
> > > support.  There's at least a bunch of the ARM evaluation boards which
> > > make use of this driver...
> > 
> > Right, but nothing should regress. If no clock is provided the driver
> > moves on during the request and will refuse to prepare, enable and
> > disable there after. 
> > 
> > Unless I've made a mistake somewhere? If so, I'd be happy to fixup.
> 
> No, but... don't use NULL for that.  Use IS_ERR(pdata->clk) instead.

I'm a bit confused. I do use IS_ERR, then if there was a problem
pdata->clk is set to NULL, then we test for NULL thereafter:

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

Are you saying remove "pdata->clk = NULL;" and test for IS_ERR
every time?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 4/4] net/smsc911x: Provide common clock functionality

[Russell King - ARM Linux]

On Fri, Dec 21, 2012 at 09:13:06AM +0000, Lee Jones wrote:
> Are you saying remove "pdata->clk = NULL;" and test for IS_ERR
> every time?

Exactly.

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

[Lee Jones]

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 at 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(-)

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
-- 
1.7.9.5

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

[Linus Walleij]

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

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

[Lee Jones]

No, you're right, I'm a moron.

Will fix up and resend when I'm back to work.

Sent from my mobile Linux device.
On Dec 26, 2012 12:51 AM, "Linus Walleij" <linus.walleij@linaro.org> wrote:

> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121227/73803030/attachment-0001.html>

[PATCH 4/4 v3] net/smsc911x: Provide common clock functionality

[Lee Jones]

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 at 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(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index e112877..f9b6bb7 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
-- 
1.7.9.5

[PATCH 4/4 v3] net/smsc911x: Provide common clock functionality

[Linus Walleij]

On Thu, Jan 3, 2013 at 12:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> 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 at vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Looks all right to me now:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 4/4 v3] net/smsc911x: Provide common clock functionality

[Lee Jones]

On Thu, 03 Jan 2013, Linus Walleij wrote:

> On Thu, Jan 3, 2013 at 12:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > 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 at vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Looks all right to me now:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Great, thanks Linus.

Who will take this patch now?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 4/4 v3] net/smsc911x: Provide common clock functionality

[Lee Jones]

On Thu, 03 Jan 2013, Linus Walleij wrote:

> On Thu, Jan 3, 2013 at 12:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > 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 at vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Looks all right to me now:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I still need a maintiner Ack for this before I can push it.

Anyone?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 4/4 v3] net/smsc911x: Provide common clock functionality

[Ulf Hansson]

On 9 January 2013 09:55, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 03 Jan 2013, Linus Walleij wrote:
>
>> On Thu, Jan 3, 2013 at 12:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> > 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 at vger.kernel.org
>> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>
>> Looks all right to me now:
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> I still need a maintiner Ack for this before I can push it.
>
> Anyone?
>
> --
> Lee Jones
> Linaro ST-Ericsson Landing Team Lead
> Linaro.org ? Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

[PATCH 4/4 v3] net/smsc911x: Provide common clock functionality

[Linus Walleij]

On Wed, Jan 9, 2013 at 9:55 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 03 Jan 2013, Linus Walleij wrote:
>
>> On Thu, Jan 3, 2013 at 12:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> > 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 at vger.kernel.org
>> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>
>> Looks all right to me now:
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> I still need a maintiner Ack for this before I can push it.
>
> Anyone?

Steve is there sometimes. But I'd ask David Miller to ACK
it if he's OK with the change.

Yours,
Linus Walleij

[PATCH 4/4 v3] net/smsc911x: Provide common clock functionality

[Lee Jones]

> >> > 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 at vger.kernel.org
> >> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >>
> >> Looks all right to me now:
> >> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > I still need a maintiner Ack for this before I can push it.
> >
> > Anyone?
> 
> Steve is there sometimes. But I'd ask David Miller to ACK
> it if he's OK with the change.
> 
> Yours,
> Linus Walleij

Hi David,

Does this [1] patch look okay to you too?

It's the last Ack I require before I can push the set.

[1] https://patchwork.kernel.org/patch/1926971/

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 4/4 v3] net/smsc911x: Provide common clock functionality

[David Miller]

From: Lee Jones <lee.jones@linaro.org>
Date: Thu, 17 Jan 2013 10:47:44 +0000

> https://patchwork.kernel.org/patch/1926971/

I'm fine with this:

Acked-by: David S. Miller <davem@davemloft.net>

[PATCH 4/4] net/smsc911x: Provide common clock functionality

[Lee Jones]

On Wed, 19 Dec 2012, Lee Jones wrote:

> 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 at vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/net/ethernet/smsc/smsc911x.c |   31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index 4616bf2..f6196cd 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 (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 (pdata->clk)
> +		clk_disable_unprepare(pdata->clk);
> +
>  	return ret;
>  }
>  
> @@ -421,6 +436,14 @@ 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 %d\n", PTR_ERR(pdata->clk));
> +		pdata->clk = NULL;
> +	}
> +
>  	return ret;
>  }
>  
> @@ -436,6 +459,12 @@ static void smsc911x_free_resources(struct platform_device *pdev)
>  	/* Free regulators */
>  	regulator_bulk_free(ARRAY_SIZE(pdata->supplies),
>  			pdata->supplies);
> +
> +	/* Free clock */
> +	if (pdata->clk) {
> +		clk_put(pdata->clk);
> +		pdata->clk = NULL;
> +	}
>  }
>  
>  /* waits for MAC not busy, with timeout.  Only called by smsc911x_mac_read
> -- 
> 1.7.9.5
> 

I still need a maintiner Ack for this before I can push it.

Mike?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 1/4] ARM: ux500: Provide a means to obtain the SMSC9115 clock when DT is enabled

[Linus Walleij]

On Wed, Dec 19, 2012 at 6:19 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Device Tree names devices differently to how some frameworks expect
> them. Until we can move a platform over to the new way of obtaining
> resources, we have to use the OF_DEV_AUXDATA() macros to craft a
> device name which can be used in searches during allocation time.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I think that the best way to fix this regression is to gather
ACKs from the involved subsystems and push it throug
ARM SoC directly.

I thing Cc: stable at kernel.org needs to be added to all patches
in this set as well.

Yours,
Linus Walleij