[PATCH 0/10] PNX clock API fixes

[PATCH 0/10] PNX clock API fixes

[Russell King - ARM Linux]

Following on from the previous set of patches...

The PNX4008 clock API usage is extremely poor.  Nothing works as it was
intended to - clk_set_rate() is used to turn clocks on and off, or
set dividers.  I certainly doesn't take the desired clock rate in Hz.
clk_get_rate() doesn't report the clock rate in Hz either - it doesn't
even report any kind of rate.

clk_enable and clk_disable seem to be very weird too.

This patch set fixes the I2C and watchdog drivers so that they can be
used with clk API implementations which do actually conform with the
clk API.

As per the previous set, selective cc's.  Full set will be on
linux-arm-kernel.

[PATCH 06/10] ARM: PNX4008: convert i2c clocks to match by device only

[Russell King - ARM Linux]

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 arch/arm/mach-pnx4008/clock.c |    6 +++---
 arch/arm/mach-pnx4008/i2c.c   |    8 ++------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-pnx4008/clock.c b/arch/arm/mach-pnx4008/clock.c
index 270c254..26659cd 100644
--- a/arch/arm/mach-pnx4008/clock.c
+++ b/arch/arm/mach-pnx4008/clock.c
@@ -797,9 +797,9 @@ static struct clk_lookup onchip_clkreg[] = {
 	{ .clk = &jpeg_ck,	.con_id = "jpeg_ck"	},
 	{ .clk = &ms_ck,	.con_id = "ms_ck"	},
 	{ .clk = &touch_ck,	.con_id = "touch_ck"	},
-	{ .clk = &i2c0_ck,	.con_id = "i2c0_ck"	},
-	{ .clk = &i2c1_ck,	.con_id = "i2c1_ck"	},
-	{ .clk = &i2c2_ck,	.con_id = "i2c2_ck"	},
+	{ .clk = &i2c0_ck,	.dev_id = "pnx-i2c.0"	},
+	{ .clk = &i2c1_ck,	.dev_id = "pnx-i2c.1"	},
+	{ .clk = &i2c2_ck,	.dev_id = "pnx-i2c.2"	},
 	{ .clk = &spi0_ck,	.con_id = "spi0_ck"	},
 	{ .clk = &spi1_ck,	.con_id = "spi1_ck"	},
 	{ .clk = &uart3_ck,	.con_id = "uart3_ck"	},
diff --git a/arch/arm/mach-pnx4008/i2c.c b/arch/arm/mach-pnx4008/i2c.c
index f3fea29..c986b3a 100644
--- a/arch/arm/mach-pnx4008/i2c.c
+++ b/arch/arm/mach-pnx4008/i2c.c
@@ -21,11 +21,9 @@
 static int set_clock_run(struct platform_device *pdev)
 {
 	struct clk *clk;
-	char name[10];
 	int retval = 0;
 
-	snprintf(name, 10, "i2c%d_ck", pdev->id);
-	clk = clk_get(&pdev->dev, name);
+	clk = clk_get(&pdev->dev, NULL);
 	if (!IS_ERR(clk)) {
 		clk_set_rate(clk, 1);
 		clk_put(clk);
@@ -38,11 +36,9 @@ static int set_clock_run(struct platform_device *pdev)
 static int set_clock_stop(struct platform_device *pdev)
 {
 	struct clk *clk;
-	char name[10];
 	int retval = 0;
 
-	snprintf(name, 10, "i2c%d_ck", pdev->id);
-	clk = clk_get(&pdev->dev, name);
+	clk = clk_get(&pdev->dev, NULL);
 	if (!IS_ERR(clk)) {
 		clk_set_rate(clk, 0);
 		clk_put(clk);
-- 
1.6.2.5

[PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks into driver

[Russell King - ARM Linux]

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 arch/arm/mach-pnx4008/i2c.c  |   24 ------------------------
 drivers/i2c/busses/i2c-pnx.c |    9 +++++++--
 include/linux/i2c-pnx.h      |    4 ----
 3 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-pnx4008/i2c.c b/arch/arm/mach-pnx4008/i2c.c
index c986b3a..707d819 100644
--- a/arch/arm/mach-pnx4008/i2c.c
+++ b/arch/arm/mach-pnx4008/i2c.c
@@ -48,24 +48,6 @@ static int set_clock_stop(struct platform_device *pdev)
 	return retval;
 }
 
-static int i2c_pnx_suspend(struct platform_device *pdev, pm_message_t state)
-{
-	int retval = 0;
-#ifdef CONFIG_PM
-	retval = set_clock_run(pdev);
-#endif
-	return retval;
-}
-
-static int i2c_pnx_resume(struct platform_device *pdev)
-{
-	int retval = 0;
-#ifdef CONFIG_PM
-	retval = set_clock_run(pdev);
-#endif
-	return retval;
-}
-
 static u32 calculate_input_freq(struct platform_device *pdev)
 {
 	return HCLK_MHZ;
@@ -102,8 +84,6 @@ static struct i2c_adapter pnx_adapter2 = {
 };
 
 static struct i2c_pnx_data i2c0_data = {
-	.suspend = i2c_pnx_suspend,
-	.resume = i2c_pnx_resume,
 	.calculate_input_freq = calculate_input_freq,
 	.set_clock_run = set_clock_run,
 	.set_clock_stop = set_clock_stop,
@@ -111,8 +91,6 @@ static struct i2c_pnx_data i2c0_data = {
 };
 
 static struct i2c_pnx_data i2c1_data = {
-	.suspend = i2c_pnx_suspend,
-	.resume = i2c_pnx_resume,
 	.calculate_input_freq = calculate_input_freq,
 	.set_clock_run = set_clock_run,
 	.set_clock_stop = set_clock_stop,
@@ -120,8 +98,6 @@ static struct i2c_pnx_data i2c1_data = {
 };
 
 static struct i2c_pnx_data i2c2_data = {
-	.suspend = i2c_pnx_suspend,
-	.resume = i2c_pnx_resume,
 	.calculate_input_freq = calculate_input_freq,
 	.set_clock_run = set_clock_run,
 	.set_clock_stop = set_clock_stop,
diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index 1fca590..8fe7de8 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -545,18 +545,23 @@ static struct i2c_algorithm pnx_algorithm = {
 	.functionality = i2c_pnx_func,
 };
 
+#ifdef CONFIG_PM
 static int i2c_pnx_controller_suspend(struct platform_device *pdev,
 				      pm_message_t state)
 {
 	struct i2c_pnx_data *i2c_pnx = platform_get_drvdata(pdev);
-	return i2c_pnx->suspend(pdev, state);
+	return i2c_pnx->set_clock_run(pdev);
 }
 
 static int i2c_pnx_controller_resume(struct platform_device *pdev)
 {
 	struct i2c_pnx_data *i2c_pnx = platform_get_drvdata(pdev);
-	return i2c_pnx->resume(pdev);
+	return i2c_pnx->set_clock_run(pdev);
 }
+#else
+#define i2c_pnx_controller_suspend	NULL
+#define i2c_pnx_controller_resume	NULL
+#endif
 
 static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 {
diff --git a/include/linux/i2c-pnx.h b/include/linux/i2c-pnx.h
index 9eb07bb..71de7f9 100644
--- a/include/linux/i2c-pnx.h
+++ b/include/linux/i2c-pnx.h
@@ -12,8 +12,6 @@
 #ifndef __I2C_PNX_H__
 #define __I2C_PNX_H__
 
-#include <linux/pm.h>
-
 struct platform_device;
 
 struct i2c_pnx_mif {
@@ -34,8 +32,6 @@ struct i2c_pnx_algo_data {
 };
 
 struct i2c_pnx_data {
-	int (*suspend) (struct platform_device *pdev, pm_message_t state);
-	int (*resume) (struct platform_device *pdev);
 	u32 (*calculate_input_freq) (struct platform_device *pdev);
 	int (*set_clock_run) (struct platform_device *pdev);
 	int (*set_clock_stop) (struct platform_device *pdev);
-- 
1.6.2.5

Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks into driver

[Russell King - ARM Linux]

On Fri, Nov 20, 2009 at 10:50:34AM +0000, Russell King - ARM Linux wrote:
> -static int i2c_pnx_suspend(struct platform_device *pdev, pm_message_t state)
> -{
> -	int retval = 0;
> -#ifdef CONFIG_PM
> -	retval = set_clock_run(pdev);
> -#endif

BTW, a comment from PNX folk (Vitaly/Kevin) would be appreciated.  Why
does PNX enable the clock when going into suspend?  Should I assume that
this should actually be disabling the clock?

Also, if Vitaly doesn't have anything to do with PNX4008 anymore, it
would be a good idea to update the MAINTAINERS file.

Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks into driver

[Vitaly Wool]

[-- Attachment #1.1: Type: text/plain, Size: 1022 bytes --]

On Sat, Nov 21, 2009 at 3:53 PM, Russell King - ARM Linux <
linux@arm.linux.org.uk> wrote:

> On Fri, Nov 20, 2009 at 10:50:34AM +0000, Russell King - ARM Linux wrote:
> > -static int i2c_pnx_suspend(struct platform_device *pdev, pm_message_t
> state)
> > -{
> > -     int retval = 0;
> > -#ifdef CONFIG_PM
> > -     retval = set_clock_run(pdev);
> > -#endif
>
> BTW, a comment from PNX folk (Vitaly/Kevin) would be appreciated.  Why
> does PNX enable the clock when going into suspend?  Should I assume that
> this should actually be disabling the clock?
>

As far as I recall, at some point there was a function that was taking the
second boolean parameter (enable or disable the clock). So now it is, yes,
not making much sense.


> Also, if Vitaly doesn't have anything to do with PNX4008 anymore, it
> would be a good idea to update the MAINTAINERS file.
>

I'd like to keep maintaining this platform, but it's also up to NXP because
I don't have the actual working hardware now so there's not much I can do.

Vitaly

[-- Attachment #1.2: Type: text/html, Size: 1548 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks into driver

[Linus Walleij]

If you have this up for testing anyway, would it be advisable to take this
opportunity to also switch i2c-pnx over to using struct dev_pm_ops?

i2c-pxa, i2c-s3c2410 and i2c-sh_mobile are already using the new
PM primitives.

Yours,
Linus Walleij

Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks into driver

[Russell King - ARM Linux]

On Sat, Nov 21, 2009 at 06:27:54PM +0100, Linus Walleij wrote:
> If you have this up for testing anyway, would it be advisable to take this
> opportunity to also switch i2c-pnx over to using struct dev_pm_ops?

These patches are only compile-tested, and that's partly why there's soo
many of them - each one does one transformation, which makes it easy to
review for correctness.

I'm also hoping that Kevin will test them on later PNX hardware so that
they can be submitted.

RE: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks into driver

[Kevin Wells]

> Subject: Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks
> into driver
> 
> On Sat, Nov 21, 2009 at 06:27:54PM +0100, Linus Walleij wrote:
> > If you have this up for testing anyway, would it be advisable to take
> this
> > opportunity to also switch i2c-pnx over to using struct dev_pm_ops?
> 
> These patches are only compile-tested, and that's partly why there's soo
> many of them - each one does one transformation, which makes it easy to
> review for correctness.
> 
> I'm also hoping that Kevin will test them on later PNX hardware so that
> they can be submitted.

I'll test the changes later today and look at the other identified issues
related to this driver.

RE: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks into driver

[Kevin Wells]

> Subject: Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks
> into driver
> 
> On Sat, Nov 21, 2009 at 06:27:54PM +0100, Linus Walleij wrote:
> > If you have this up for testing anyway, would it be advisable to take
> this
> > opportunity to also switch i2c-pnx over to using struct dev_pm_ops?
> 

With the clock change below, suspend and resume won't be needed anymore.

> These patches are only compile-tested, and that's partly why there's soo
> many of them - each one does one transformation, which makes it easy to
> review for correctness.
> 
> I'm also hoping that Kevin will test them on later PNX hardware so that
> they can be submitted.

All the I2C patches work fine. In regards to the clock being enabled on
suspend, I think that's a bug - it should be disabled. It only uses extra
power keeping the clock gated on when an I2C transaction isn't in progress.
The clock can be gated on prior to an I2C transaction starting and
then gated off at the end of it. This will save a small trickle of power.

I'll send over patch with these changes.

Kevin

Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks into driver

[Russell King - ARM Linux]

On Wed, Nov 25, 2009 at 10:19:26PM +0100, Kevin Wells wrote:
> > Subject: Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks
> > into driver
> > 
> > On Sat, Nov 21, 2009 at 06:27:54PM +0100, Linus Walleij wrote:
> > > If you have this up for testing anyway, would it be advisable to take
> > this
> > > opportunity to also switch i2c-pnx over to using struct dev_pm_ops?
> > 
> 
> With the clock change below, suspend and resume won't be needed anymore.
> 
> > These patches are only compile-tested, and that's partly why there's soo
> > many of them - each one does one transformation, which makes it easy to
> > review for correctness.
> > 
> > I'm also hoping that Kevin will test them on later PNX hardware so that
> > they can be submitted.
> 
> All the I2C patches work fine. In regards to the clock being enabled on
> suspend, I think that's a bug - it should be disabled. It only uses extra
> power keeping the clock gated on when an I2C transaction isn't in progress.
> The clock can be gated on prior to an I2C transaction starting and
> then gated off at the end of it. This will save a small trickle of power.
> 
> I'll send over patch with these changes.

I didn't hear any response, so the PNX4008 patches are still hanging
around unmerged, and now have rejects.

I'm not attached to these patches so I'm not going to be putting any
additional effort into them (which means I'm not going to be putting
any effort into fixing those rejects or trying to get them merged)
especially as it means that they're now going to have to be maintained
by someone for three months...

RE: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks into driver

[Kevin Wells]

> Subject: Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume callbacks
> into driver
> 
> On Wed, Nov 25, 2009 at 10:19:26PM +0100, Kevin Wells wrote:
> > > Subject: Re: [PATCH 07/10] ARM: PNX4008: move i2c suspend/resume
> callbacks
> > > into driver
> > >
> > > On Sat, Nov 21, 2009 at 06:27:54PM +0100, Linus Walleij wrote:
> > > > If you have this up for testing anyway, would it be advisable to
> take
> > > this
> > > > opportunity to also switch i2c-pnx over to using struct dev_pm_ops?
> > >
> >
> > With the clock change below, suspend and resume won't be needed anymore.
> >
> > > These patches are only compile-tested, and that's partly why there's
> soo
> > > many of them - each one does one transformation, which makes it easy
> to
> > > review for correctness.
> > >
> > > I'm also hoping that Kevin will test them on later PNX hardware so
> that
> > > they can be submitted.
> >
> > All the I2C patches work fine. In regards to the clock being enabled on
> > suspend, I think that's a bug - it should be disabled. It only uses
> extra
> > power keeping the clock gated on when an I2C transaction isn't in
> progress.
> > The clock can be gated on prior to an I2C transaction starting and
> > then gated off at the end of it. This will save a small trickle of
> power.
> >
> > I'll send over patch with these changes.
> 
> I didn't hear any response, so the PNX4008 patches are still hanging
> around unmerged, and now have rejects.
> 
I tested the patches and they worked fine. I thought I sent another
message saying that...

> I'm not attached to these patches so I'm not going to be putting any
> additional effort into them (which means I'm not going to be putting
> any effort into fixing those rejects or trying to get them merged)
> especially as it means that they're now going to have to be maintained
> by someone for three months...

We use these drivers on several other devices and many variants, I'll
work on getting the code into mainline and getting everything cleaned up.
I think the changes made the driver better and made our clock support in
the mach- area less complex (and much more generic). I've actually
updated and tested my local 32xx arch files with these changes and would
like to submit those initial files based on those changes, so I do have
a very strong interest to get them included....

[PATCH 08/10] ARM: PNX4008: move i2c clock start/stop into driver

[Russell King - ARM Linux]

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 arch/arm/mach-pnx4008/i2c.c  |   36 ------------------------------------
 drivers/i2c/busses/i2c-pnx.c |   39 ++++++++++++++++++++++++++++++---------
 include/linux/i2c-pnx.h      |    4 ++--
 3 files changed, 32 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-pnx4008/i2c.c b/arch/arm/mach-pnx4008/i2c.c
index 707d819..14b4906 100644
--- a/arch/arm/mach-pnx4008/i2c.c
+++ b/arch/arm/mach-pnx4008/i2c.c
@@ -18,36 +18,6 @@
 #include <mach/irqs.h>
 #include <mach/i2c.h>
 
-static int set_clock_run(struct platform_device *pdev)
-{
-	struct clk *clk;
-	int retval = 0;
-
-	clk = clk_get(&pdev->dev, NULL);
-	if (!IS_ERR(clk)) {
-		clk_set_rate(clk, 1);
-		clk_put(clk);
-	} else
-		retval = -ENOENT;
-
-	return retval;
-}
-
-static int set_clock_stop(struct platform_device *pdev)
-{
-	struct clk *clk;
-	int retval = 0;
-
-	clk = clk_get(&pdev->dev, NULL);
-	if (!IS_ERR(clk)) {
-		clk_set_rate(clk, 0);
-		clk_put(clk);
-	} else
-		retval = -ENOENT;
-
-	return retval;
-}
-
 static u32 calculate_input_freq(struct platform_device *pdev)
 {
 	return HCLK_MHZ;
@@ -85,22 +55,16 @@ static struct i2c_adapter pnx_adapter2 = {
 
 static struct i2c_pnx_data i2c0_data = {
 	.calculate_input_freq = calculate_input_freq,
-	.set_clock_run = set_clock_run,
-	.set_clock_stop = set_clock_stop,
 	.adapter = &pnx_adapter0,
 };
 
 static struct i2c_pnx_data i2c1_data = {
 	.calculate_input_freq = calculate_input_freq,
-	.set_clock_run = set_clock_run,
-	.set_clock_stop = set_clock_stop,
 	.adapter = &pnx_adapter1,
 };
 
 static struct i2c_pnx_data i2c2_data = {
 	.calculate_input_freq = calculate_input_freq,
-	.set_clock_run = set_clock_run,
-	.set_clock_stop = set_clock_stop,
 	.adapter = &pnx_adapter2,
 };
 
diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index 8fe7de8..0a0ee4a 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -20,6 +20,9 @@
 #include <linux/platform_device.h>
 #include <linux/i2c-pnx.h>
 #include <linux/io.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+
 #include <mach/hardware.h>
 #include <mach/i2c.h>
 #include <asm/irq.h>
@@ -550,13 +553,22 @@ static int i2c_pnx_controller_suspend(struct platform_device *pdev,
 				      pm_message_t state)
 {
 	struct i2c_pnx_data *i2c_pnx = platform_get_drvdata(pdev);
-	return i2c_pnx->set_clock_run(pdev);
+	struct i2c_pnx_algo_data *alg_data = i2c_pnx->adapter->algo_data;
+
+	/* FIXME: disable clock? */
+	clk_set_rate(alg_data->clk, 1);
+
+	return 0;
 }
 
 static int i2c_pnx_controller_resume(struct platform_device *pdev)
 {
 	struct i2c_pnx_data *i2c_pnx = platform_get_drvdata(pdev);
-	return i2c_pnx->set_clock_run(pdev);
+	struct i2c_pnx_algo_data *alg_data = i2c_pnx->adapter->algo_data;
+
+	clk_set_rate(alg_data->clk, 1);
+
+	return 0;
 }
 #else
 #define i2c_pnx_controller_suspend	NULL
@@ -580,6 +592,15 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, i2c_pnx);
 
+	i2c_pnx->adapter->algo = &pnx_algorithm;
+	alg_data = i2c_pnx->adapter->algo_data;
+
+	alg_data->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(alg_data->clk)) {
+		ret = PTR_ERR(alg_data->clk);
+		goto out_drvdata;
+	}
+
 	if (i2c_pnx->calculate_input_freq)
 		freq_mhz = i2c_pnx->calculate_input_freq(pdev);
 	else {
@@ -588,9 +609,6 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 		       "%d MHz\n", freq_mhz);
 	}
 
-	i2c_pnx->adapter->algo = &pnx_algorithm;
-
-	alg_data = i2c_pnx->adapter->algo_data;
 	init_timer(&alg_data->mif.timer);
 	alg_data->mif.timer.function = i2c_pnx_timeout;
 	alg_data->mif.timer.data = (unsigned long)i2c_pnx->adapter;
@@ -602,7 +620,7 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 		       "I/O region 0x%08x for I2C already in use.\n",
 		       alg_data->base);
 		ret = -ENODEV;
-		goto out_drvdata;
+		goto out_clkget;
 	}
 
 	if (!(alg_data->ioaddr =
@@ -612,7 +630,7 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 		goto out_release;
 	}
 
-	i2c_pnx->set_clock_run(pdev);
+	clk_set_rate(alg_data->clk, 1);
 
 	/*
 	 * Clock Divisor High This value is the number of system clocks
@@ -657,11 +675,13 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 out_irq:
 	free_irq(alg_data->irq, alg_data);
 out_clock:
-	i2c_pnx->set_clock_stop(pdev);
+	clk_set_rate(alg_data->clk, 0);
 out_unmap:
 	iounmap((void *)alg_data->ioaddr);
 out_release:
 	release_mem_region(alg_data->base, I2C_PNX_REGION_SIZE);
+out_clkget:
+	clk_put(alg_data->clk);
 out_drvdata:
 	platform_set_drvdata(pdev, NULL);
 out:
@@ -676,9 +696,10 @@ static int __devexit i2c_pnx_remove(struct platform_device *pdev)
 
 	free_irq(alg_data->irq, alg_data);
 	i2c_del_adapter(adap);
-	i2c_pnx->set_clock_stop(pdev);
+	clk_set_rate(alg_data->clk, 0);
 	iounmap((void *)alg_data->ioaddr);
 	release_mem_region(alg_data->base, I2C_PNX_REGION_SIZE);
+	clk_put(alg_data->clk);
 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
diff --git a/include/linux/i2c-pnx.h b/include/linux/i2c-pnx.h
index 71de7f9..688e292 100644
--- a/include/linux/i2c-pnx.h
+++ b/include/linux/i2c-pnx.h
@@ -13,6 +13,7 @@
 #define __I2C_PNX_H__
 
 struct platform_device;
+struct clk;
 
 struct i2c_pnx_mif {
 	int			ret;		/* Return value */
@@ -29,12 +30,11 @@ struct i2c_pnx_algo_data {
 	int			irq;
 	struct i2c_pnx_mif	mif;
 	int			last;
+	struct clk		*clk;
 };
 
 struct i2c_pnx_data {
 	u32 (*calculate_input_freq) (struct platform_device *pdev);
-	int (*set_clock_run) (struct platform_device *pdev);
-	int (*set_clock_stop) (struct platform_device *pdev);
 	struct i2c_adapter *adapter;
 };
 
-- 
1.6.2.5

[PATCH 09/10] ARM: PNX4008: convert i2c-pnx to use clk API enable/disable calls

[Russell King - ARM Linux]

clk_set_rate() is not supposed to be used to turn clocks on and off.
That's what clk_enable/clk_disable is for.

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 arch/arm/mach-pnx4008/clock.c |   12 ++++++------
 drivers/i2c/busses/i2c-pnx.c  |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-pnx4008/clock.c b/arch/arm/mach-pnx4008/clock.c
index 26659cd..c60d798 100644
--- a/arch/arm/mach-pnx4008/clock.c
+++ b/arch/arm/mach-pnx4008/clock.c
@@ -639,30 +639,30 @@ static struct clk i2c0_ck = {
 	.name = "i2c0_ck",
 	.parent = &per_ck,
 	.flags = NEEDS_INITIALIZATION,
-	.round_rate = &on_off_round_rate,
-	.set_rate = &on_off_set_rate,
 	.enable_shift = 0,
 	.enable_reg = I2CCLKCTRL_REG,
+	.enable = clk_reg_enable,
+	.disable = clk_reg_disable,
 };
 
 static struct clk i2c1_ck = {
 	.name = "i2c1_ck",
 	.parent = &per_ck,
 	.flags = NEEDS_INITIALIZATION,
-	.round_rate = &on_off_round_rate,
-	.set_rate = &on_off_set_rate,
 	.enable_shift = 1,
 	.enable_reg = I2CCLKCTRL_REG,
+	.enable = clk_reg_enable,
+	.disable = clk_reg_disable,
 };
 
 static struct clk i2c2_ck = {
 	.name = "i2c2_ck",
 	.parent = &per_ck,
 	.flags = NEEDS_INITIALIZATION,
-	.round_rate = &on_off_round_rate,
-	.set_rate = &on_off_set_rate,
 	.enable_shift = 2,
 	.enable_reg = USB_OTG_CLKCTRL_REG,
+	.enable = clk_reg_enable,
+	.disable = clk_reg_disable,
 };
 
 static struct clk spi0_ck = {
diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index 0a0ee4a..34da92d 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -555,8 +555,8 @@ static int i2c_pnx_controller_suspend(struct platform_device *pdev,
 	struct i2c_pnx_data *i2c_pnx = platform_get_drvdata(pdev);
 	struct i2c_pnx_algo_data *alg_data = i2c_pnx->adapter->algo_data;
 
-	/* FIXME: disable clock? */
-	clk_set_rate(alg_data->clk, 1);
+	/* FIXME: shouldn't this be clk_disable? */
+	clk_enable(alg_data->clk);
 
 	return 0;
 }
@@ -566,9 +566,7 @@ static int i2c_pnx_controller_resume(struct platform_device *pdev)
 	struct i2c_pnx_data *i2c_pnx = platform_get_drvdata(pdev);
 	struct i2c_pnx_algo_data *alg_data = i2c_pnx->adapter->algo_data;
 
-	clk_set_rate(alg_data->clk, 1);
-
-	return 0;
+	return clk_enable(alg_data->clk);
 }
 #else
 #define i2c_pnx_controller_suspend	NULL
@@ -630,7 +628,9 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 		goto out_release;
 	}
 
-	clk_set_rate(alg_data->clk, 1);
+	ret = clk_enable(alg_data->clk);
+	if (ret)
+		goto out_unmap;
 
 	/*
 	 * Clock Divisor High This value is the number of system clocks
@@ -650,7 +650,7 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 	iowrite32(mcntrl_reset, I2C_REG_CTL(alg_data));
 	if (wait_reset(I2C_PNX_TIMEOUT, alg_data)) {
 		ret = -ENODEV;
-		goto out_unmap;
+		goto out_clock;
 	}
 	init_completion(&alg_data->mif.complete);
 
@@ -675,7 +675,7 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 out_irq:
 	free_irq(alg_data->irq, alg_data);
 out_clock:
-	clk_set_rate(alg_data->clk, 0);
+	clk_disable(alg_data->clk);
 out_unmap:
 	iounmap((void *)alg_data->ioaddr);
 out_release:
@@ -696,7 +696,7 @@ static int __devexit i2c_pnx_remove(struct platform_device *pdev)
 
 	free_irq(alg_data->irq, alg_data);
 	i2c_del_adapter(adap);
-	clk_set_rate(alg_data->clk, 0);
+	clk_disable(alg_data->clk);
 	iounmap((void *)alg_data->ioaddr);
 	release_mem_region(alg_data->base, I2C_PNX_REGION_SIZE);
 	clk_put(alg_data->clk);
-- 
1.6.2.5

[PATCH 10/10] ARM: PNX4008: get i2c clock rate from clk API

[Russell King - ARM Linux]

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 arch/arm/mach-pnx4008/clock.c |    9 ++++++---
 arch/arm/mach-pnx4008/i2c.c   |    9 ---------
 drivers/i2c/busses/i2c-pnx.c  |   15 ++++-----------
 include/linux/i2c-pnx.h       |    1 -
 4 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-pnx4008/clock.c b/arch/arm/mach-pnx4008/clock.c
index c60d798..a38ef66 100644
--- a/arch/arm/mach-pnx4008/clock.c
+++ b/arch/arm/mach-pnx4008/clock.c
@@ -638,9 +638,10 @@ static struct clk flash_ck = {
 static struct clk i2c0_ck = {
 	.name = "i2c0_ck",
 	.parent = &per_ck,
-	.flags = NEEDS_INITIALIZATION,
+	.flags = NEEDS_INITIALIZATION | FIXED_RATE,
 	.enable_shift = 0,
 	.enable_reg = I2CCLKCTRL_REG,
+	.rate = 13000000,
 	.enable = clk_reg_enable,
 	.disable = clk_reg_disable,
 };
@@ -648,9 +649,10 @@ static struct clk i2c0_ck = {
 static struct clk i2c1_ck = {
 	.name = "i2c1_ck",
 	.parent = &per_ck,
-	.flags = NEEDS_INITIALIZATION,
+	.flags = NEEDS_INITIALIZATION | FIXED_RATE,
 	.enable_shift = 1,
 	.enable_reg = I2CCLKCTRL_REG,
+	.rate = 13000000,
 	.enable = clk_reg_enable,
 	.disable = clk_reg_disable,
 };
@@ -658,9 +660,10 @@ static struct clk i2c1_ck = {
 static struct clk i2c2_ck = {
 	.name = "i2c2_ck",
 	.parent = &per_ck,
-	.flags = NEEDS_INITIALIZATION,
+	.flags = NEEDS_INITIALIZATION | FIXED_RATE,
 	.enable_shift = 2,
 	.enable_reg = USB_OTG_CLKCTRL_REG,
+	.rate = 13000000,
 	.enable = clk_reg_enable,
 	.disable = clk_reg_disable,
 };
diff --git a/arch/arm/mach-pnx4008/i2c.c b/arch/arm/mach-pnx4008/i2c.c
index 14b4906..23ec335 100644
--- a/arch/arm/mach-pnx4008/i2c.c
+++ b/arch/arm/mach-pnx4008/i2c.c
@@ -18,12 +18,6 @@
 #include <mach/irqs.h>
 #include <mach/i2c.h>
 
-static u32 calculate_input_freq(struct platform_device *pdev)
-{
-	return HCLK_MHZ;
-}
-
-
 static struct i2c_pnx_algo_data pnx_algo_data0 = {
 	.base = PNX4008_I2C1_BASE,
 	.irq = I2C_1_INT,
@@ -54,17 +48,14 @@ static struct i2c_adapter pnx_adapter2 = {
 };
 
 static struct i2c_pnx_data i2c0_data = {
-	.calculate_input_freq = calculate_input_freq,
 	.adapter = &pnx_adapter0,
 };
 
 static struct i2c_pnx_data i2c1_data = {
-	.calculate_input_freq = calculate_input_freq,
 	.adapter = &pnx_adapter1,
 };
 
 static struct i2c_pnx_data i2c2_data = {
-	.calculate_input_freq = calculate_input_freq,
 	.adapter = &pnx_adapter2,
 };
 
diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index 34da92d..a390ef3 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -31,7 +31,6 @@
 #define I2C_PNX_TIMEOUT		10 /* msec */
 #define I2C_PNX_SPEED_KHZ	100
 #define I2C_PNX_REGION_SIZE	0x100
-#define PNX_DEFAULT_FREQ	13 /* MHz */
 
 static inline int wait_timeout(long timeout, struct i2c_pnx_algo_data *data)
 {
@@ -578,7 +577,7 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 	unsigned long tmp;
 	int ret = 0;
 	struct i2c_pnx_algo_data *alg_data;
-	int freq_mhz;
+	unsigned long freq;
 	struct i2c_pnx_data *i2c_pnx = pdev->dev.platform_data;
 
 	if (!i2c_pnx || !i2c_pnx->adapter) {
@@ -599,14 +598,6 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 		goto out_drvdata;
 	}
 
-	if (i2c_pnx->calculate_input_freq)
-		freq_mhz = i2c_pnx->calculate_input_freq(pdev);
-	else {
-		freq_mhz = PNX_DEFAULT_FREQ;
-		dev_info(&pdev->dev, "Setting bus frequency to default value: "
-		       "%d MHz\n", freq_mhz);
-	}
-
 	init_timer(&alg_data->mif.timer);
 	alg_data->mif.timer.function = i2c_pnx_timeout;
 	alg_data->mif.timer.data = (unsigned long)i2c_pnx->adapter;
@@ -632,6 +623,8 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 	if (ret)
 		goto out_unmap;
 
+	freq = clk_get_rate(alg_data->clk);
+
 	/*
 	 * Clock Divisor High This value is the number of system clocks
 	 * the serial clock (SCL) will be high.
@@ -643,7 +636,7 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 	 * the deglitching filter length.
 	 */
 
-	tmp = ((freq_mhz * 1000) / I2C_PNX_SPEED_KHZ) / 2 - 2;
+	tmp = ((freq / 1000) / I2C_PNX_SPEED_KHZ) / 2 - 2;
 	iowrite32(tmp, I2C_REG_CKH(alg_data));
 	iowrite32(tmp, I2C_REG_CKL(alg_data));
 
diff --git a/include/linux/i2c-pnx.h b/include/linux/i2c-pnx.h
index 688e292..9035711 100644
--- a/include/linux/i2c-pnx.h
+++ b/include/linux/i2c-pnx.h
@@ -34,7 +34,6 @@ struct i2c_pnx_algo_data {
 };
 
 struct i2c_pnx_data {
-	u32 (*calculate_input_freq) (struct platform_device *pdev);
 	struct i2c_adapter *adapter;
 };
 
-- 
1.6.2.5

[PATCH 10/10] ARM: PNX4008: get i2c clock rate from clk API

[Kevin Wells]

Signed-off-by: Kevin Wells <kevin.wells-3arQi8VN3Tc@public.gmane.org>
---
 arch/arm/mach-pnx4008/clock.c |    9 ++++++---
 arch/arm/mach-pnx4008/i2c.c   |    9 ---------
 drivers/i2c/busses/i2c-pnx.c  |   15 ++++-----------
 include/linux/i2c-pnx.h       |    1 -
 4 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-pnx4008/clock.c b/arch/arm/mach-pnx4008/clock.c
index 3f6b681..4448fb9 100644
--- a/arch/arm/mach-pnx4008/clock.c
+++ b/arch/arm/mach-pnx4008/clock.c
@@ -639,9 +639,10 @@ static struct clk flash_ck = {
 static struct clk i2c0_ck = {
 	.name = "i2c0_ck",
 	.parent = &per_ck,
-	.flags = NEEDS_INITIALIZATION,
+	.flags = NEEDS_INITIALIZATION | FIXED_RATE,
 	.enable_shift = 0,
 	.enable_reg = I2CCLKCTRL_REG,
+	.rate = 13000000,
 	.enable = clk_reg_enable,
 	.disable = clk_reg_disable,
 };
@@ -649,9 +650,10 @@ static struct clk i2c0_ck = {
 static struct clk i2c1_ck = {
 	.name = "i2c1_ck",
 	.parent = &per_ck,
-	.flags = NEEDS_INITIALIZATION,
+	.flags = NEEDS_INITIALIZATION | FIXED_RATE,
 	.enable_shift = 1,
 	.enable_reg = I2CCLKCTRL_REG,
+	.rate = 13000000,
 	.enable = clk_reg_enable,
 	.disable = clk_reg_disable,
 };
@@ -659,9 +661,10 @@ static struct clk i2c1_ck = {
 static struct clk i2c2_ck = {
 	.name = "i2c2_ck",
 	.parent = &per_ck,
-	.flags = NEEDS_INITIALIZATION,
+	.flags = NEEDS_INITIALIZATION | FIXED_RATE,
 	.enable_shift = 2,
 	.enable_reg = USB_OTG_CLKCTRL_REG,
+	.rate = 13000000,
 	.enable = clk_reg_enable,
 	.disable = clk_reg_disable,
 };
diff --git a/arch/arm/mach-pnx4008/i2c.c b/arch/arm/mach-pnx4008/i2c.c
index 14b4906..23ec335 100644
--- a/arch/arm/mach-pnx4008/i2c.c
+++ b/arch/arm/mach-pnx4008/i2c.c
@@ -18,12 +18,6 @@
 #include <mach/irqs.h>
 #include <mach/i2c.h>
 
-static u32 calculate_input_freq(struct platform_device *pdev)
-{
-	return HCLK_MHZ;
-}
-
-
 static struct i2c_pnx_algo_data pnx_algo_data0 = {
 	.base = PNX4008_I2C1_BASE,
 	.irq = I2C_1_INT,
@@ -54,17 +48,14 @@ static struct i2c_adapter pnx_adapter2 = {
 };
 
 static struct i2c_pnx_data i2c0_data = {
-	.calculate_input_freq = calculate_input_freq,
 	.adapter = &pnx_adapter0,
 };
 
 static struct i2c_pnx_data i2c1_data = {
-	.calculate_input_freq = calculate_input_freq,
 	.adapter = &pnx_adapter1,
 };
 
 static struct i2c_pnx_data i2c2_data = {
-	.calculate_input_freq = calculate_input_freq,
 	.adapter = &pnx_adapter2,
 };
 
diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index 7ed425a..531074e 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -31,7 +31,6 @@
 #define I2C_PNX_TIMEOUT		10 /* msec */
 #define I2C_PNX_SPEED_KHZ	100
 #define I2C_PNX_REGION_SIZE	0x100
-#define PNX_DEFAULT_FREQ	13 /* MHz */
 
 static inline int wait_timeout(long timeout, struct i2c_pnx_algo_data *data)
 {
@@ -578,7 +577,7 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 	unsigned long tmp;
 	int ret = 0;
 	struct i2c_pnx_algo_data *alg_data;
-	int freq_mhz;
+	unsigned long freq;
 	struct i2c_pnx_data *i2c_pnx = pdev->dev.platform_data;
 
 	if (!i2c_pnx || !i2c_pnx->adapter) {
@@ -599,14 +598,6 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 		goto out_drvdata;
 	}
 
-	if (i2c_pnx->calculate_input_freq)
-		freq_mhz = i2c_pnx->calculate_input_freq(pdev);
-	else {
-		freq_mhz = PNX_DEFAULT_FREQ;
-		dev_info(&pdev->dev, "Setting bus frequency to default value: "
-		       "%d MHz\n", freq_mhz);
-	}
-
 	init_timer(&alg_data->mif.timer);
 	alg_data->mif.timer.function = i2c_pnx_timeout;
 	alg_data->mif.timer.data = (unsigned long)i2c_pnx->adapter;
@@ -632,6 +623,8 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 	if (ret)
 		goto out_unmap;
 
+	freq = clk_get_rate(alg_data->clk);
+
 	/*
 	 * Clock Divisor High This value is the number of system clocks
 	 * the serial clock (SCL) will be high.
@@ -643,7 +636,7 @@ static int __devinit i2c_pnx_probe(struct platform_device *pdev)
 	 * the deglitching filter length.
 	 */
 
-	tmp = ((freq_mhz * 1000) / I2C_PNX_SPEED_KHZ) / 2 - 2;
+	tmp = ((freq / 1000) / I2C_PNX_SPEED_KHZ) / 2 - 2;
 	iowrite32(tmp, I2C_REG_CKH(alg_data));
 	iowrite32(tmp, I2C_REG_CKL(alg_data));
 
diff --git a/include/linux/i2c-pnx.h b/include/linux/i2c-pnx.h
index 3a69306..d7942c5 100644
--- a/include/linux/i2c-pnx.h
+++ b/include/linux/i2c-pnx.h
@@ -34,7 +34,6 @@ struct i2c_pnx_algo_data {
 };
 
 struct i2c_pnx_data {
-	u32 (*calculate_input_freq) (struct platform_device *pdev);
 	struct i2c_adapter *adapter;
 };
 
-- 
1.6.0.6

Re: [PATCH 10/10] ARM: PNX4008: get i2c clock rate from clk API

[Vitaly Wool]

[-- Attachment #1.1: Type: text/plain, Size: 438 bytes --]

On Tue, Dec 22, 2009 at 2:23 AM, Kevin Wells <kevin.wells@nxp.com> wrote:

>
> Signed-off-by: Kevin Wells <kevin.wells@nxp.com>
>

Acked-by: Vitaly Wool <vitalywool@gmail.com>


> ---
>  arch/arm/mach-pnx4008/clock.c |    9 ++++++---
>  arch/arm/mach-pnx4008/i2c.c   |    9 ---------
>  drivers/i2c/busses/i2c-pnx.c  |   15 ++++-----------
>  include/linux/i2c-pnx.h       |    1 -
>  4 files changed, 10 insertions(+), 24 deletions(-)
>

[-- Attachment #1.2: Type: text/html, Size: 977 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel