[RFC] PXA3xx: Add support for power i2c bus

[RFC] PXA3xx: Add support for power i2c bus

[Mike Rapoport]

This patch adds support for power i2c bus on PXA3xx processor.

Signed-off-by: Mike Rapoport <mike@compulab.co.il>

 arch/arm/mach-pxa/devices.c          |    9 ++++++
 arch/arm/mach-pxa/generic.h          |    7 ++++-
 arch/arm/mach-pxa/include/mach/i2c.h |    2 +-
 arch/arm/mach-pxa/pxa27x.c           |    2 +-
 arch/arm/mach-pxa/pxa3xx.c           |   25 ++++++++++++++++++
 drivers/i2c/busses/i2c-pxa.c         |   46 +++++++++++++++++++++++----------
 6 files changed, 74 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
index 35736fc..9c72098 100644
--- a/arch/arm/mach-pxa/devices.c
+++ b/arch/arm/mach-pxa/devices.c
@@ -712,6 +712,15 @@ void __init pxa_set_camera_info(struct pxacamera_platform_data *info)
 {
 	pxa_register_device(&pxa27x_device_camera, info);
 }
+
+void __init pxa_set_i2c_power_info(struct i2c_pxa_platform_data *info)
+{
+	if (cpu_is_pxa27x())
+		pxa27x_set_i2c_power_info(info);
+	else if (cpu_is_pxa3xx())
+		pxa3xx_set_i2c_power_info(info);
+
+}
 #endif /* CONFIG_PXA27x || CONFIG_PXA3xx */

 #ifdef CONFIG_PXA3xx
diff --git a/arch/arm/mach-pxa/generic.h b/arch/arm/mach-pxa/generic.h
index 041c048..13a786d 100644
--- a/arch/arm/mach-pxa/generic.h
+++ b/arch/arm/mach-pxa/generic.h
@@ -9,9 +9,10 @@
  * published by the Free Software Foundation.
  */

-typedef int (*set_wake_t)(unsigned int, unsigned int);
+typedef int (*set_wake_t) (unsigned int, unsigned int);

 struct sys_timer;
+struct i2c_pxa_platform_data;

 extern struct sys_timer pxa_timer;
 extern void __init pxa_init_irq(int irq_nr, set_wake_t fn);
@@ -42,9 +43,11 @@ extern unsigned pxa25x_get_memclk_frequency_10khz(void);
 #ifdef CONFIG_PXA27x
 extern unsigned pxa27x_get_clk_frequency_khz(int);
 extern unsigned pxa27x_get_memclk_frequency_10khz(void);
+extern void pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info);
 #else
 #define pxa27x_get_clk_frequency_khz(x)		(0)
 #define pxa27x_get_memclk_frequency_10khz()	(0)
+static inline void pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info) {}
 #endif

 #if defined(CONFIG_PXA25x) || defined(CONFIG_PXA27x)
@@ -57,10 +60,12 @@ static inline void pxa2xx_clear_reset_status(unsigned int mask) {}
 extern unsigned pxa3xx_get_clk_frequency_khz(int);
 extern unsigned pxa3xx_get_memclk_frequency_10khz(void);
 extern void pxa3xx_clear_reset_status(unsigned int);
+extern void pxa3xx_set_i2c_power_info(struct i2c_pxa_platform_data *info);
 #else
 #define pxa3xx_get_clk_frequency_khz(x)		(0)
 #define pxa3xx_get_memclk_frequency_10khz()	(0)
 static inline void pxa3xx_clear_reset_status(unsigned int mask) {}
+static inline void pxa3xx_set_i2c_power_info(struct i2c_pxa_platform_data *info) {}
 #endif

 extern struct sysdev_class pxa_irq_sysclass;
diff --git a/arch/arm/mach-pxa/include/mach/i2c.h b/arch/arm/mach-pxa/include/mach/i2c.h
index 80596b0..c1ea8d8 100644
--- a/arch/arm/mach-pxa/include/mach/i2c.h
+++ b/arch/arm/mach-pxa/include/mach/i2c.h
@@ -70,7 +70,7 @@ struct i2c_pxa_platform_data {

 extern void pxa_set_i2c_info(struct i2c_pxa_platform_data *info);

-#ifdef CONFIG_PXA27x
+#if defined(CONFIG_PXA27x) || defined(CONFIG_PXA3xx)
 extern void pxa_set_i2c_power_info(struct i2c_pxa_platform_data *info);
 #endif

diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index f9f6a9c..c33cf6a 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -349,7 +349,7 @@ struct platform_device pxa27x_device_i2c_power = {
 	.num_resources	= ARRAY_SIZE(i2c_power_resources),
 };

-void __init pxa_set_i2c_power_info(struct i2c_pxa_platform_data *info)
+void __init pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info)
 {
 	local_irq_disable();
 	PCFR |= PCFR_PI2CEN;
diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c
index 03cbc38..d5ba95a 100644
--- a/arch/arm/mach-pxa/pxa3xx.c
+++ b/arch/arm/mach-pxa/pxa3xx.c
@@ -509,6 +509,30 @@ void __init pxa3xx_init_irq(void)
  * device registration specific to PXA3xx.
  */

+static struct resource i2c_power_resources[] = {
+	{
+		.start  = 0x40f500c0,
+		.end    = 0x40f500d3,
+		.flags	= IORESOURCE_MEM,
+	}, {
+		.start	= IRQ_PWRI2C,
+		.end	= IRQ_PWRI2C,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+struct platform_device pxa3xx_device_i2c_power = {
+	.name		= "pxa2xx-i2c",
+	.id		= 1,
+	.resource	= i2c_power_resources,
+	.num_resources	= ARRAY_SIZE(i2c_power_resources),
+};
+
+void __init pxa3xx_set_i2c_power_info(struct i2c_pxa_platform_data *info)
+{
+	pxa3xx_device_i2c_power.dev.platform_data = info;
+}
+
 static struct platform_device *devices[] __initdata = {
 /*	&pxa_device_udc,	The UDC driver is PXA25x only */
 	&pxa_device_ffuart,
@@ -522,6 +546,7 @@ static struct platform_device *devices[] __initdata = {
 	&pxa3xx_device_ssp4,
 	&pxa27x_device_pwm0,
 	&pxa27x_device_pwm1,
+	&pxa3xx_device_i2c_power,
 };

 static struct sys_device pxa3xx_sysdev[] = {
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 44d8384..c2a7c94 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -68,11 +68,21 @@ struct pxa_i2c {
 	int			use_pio;
 };

-#define _IBMR(i2c)	((i2c)->reg_base + 0)
-#define _IDBR(i2c)	((i2c)->reg_base + 8)
-#define _ICR(i2c)	((i2c)->reg_base + 0x10)
-#define _ISR(i2c)	((i2c)->reg_base + 0x18)
-#define _ISAR(i2c)	((i2c)->reg_base + 0x20)
+#define _IBMR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x0 : 0x0))
+#define _IDBR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x4 : 0x8))
+#define _ICR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x8 : 0x10))
+#define _ISR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0xc : 0x18))
+#define _ISAR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x10 : 0x20))

 /*
  * I2C Slave mode address
@@ -982,10 +992,13 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
 		 i2c->adap.nr);

-	i2c->clk = clk_get(&dev->dev, "I2CCLK");
-	if (IS_ERR(i2c->clk)) {
-		ret = PTR_ERR(i2c->clk);
-		goto eclk;
+	/* PXA3xx has power I2C clock always on */
+	if (!(cpu_is_pxa3xx() && i2c->adap.nr == 1)) {
+		i2c->clk = clk_get(&dev->dev, "I2CCLK");
+		if (IS_ERR(i2c->clk)) {
+			ret = PTR_ERR(i2c->clk);
+			goto eclk;
+		}
 	}

 	i2c->reg_base = ioremap(res->start, res_len(res));
@@ -1008,7 +1021,8 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	}
 #endif

-	clk_enable(i2c->clk);
+	if (i2c->clk)
+		clk_enable(i2c->clk);

 	if (plat) {
 		i2c->adap.class = plat->class;
@@ -1051,10 +1065,12 @@ eadapt:
 	if (!i2c->use_pio)
 		free_irq(irq, i2c);
 ereqirq:
-	clk_disable(i2c->clk);
+	if (i2c->clk)
+		clk_disable(i2c->clk);
 	iounmap(i2c->reg_base);
 eremap:
-	clk_put(i2c->clk);
+	if (i2c->clk)
+		clk_put(i2c->clk);
 eclk:
 	kfree(i2c);
 emalloc:
@@ -1072,8 +1088,10 @@ static int __exit i2c_pxa_remove(struct platform_device *dev)
 	if (!i2c->use_pio)
 		free_irq(i2c->irq, i2c);

-	clk_disable(i2c->clk);
-	clk_put(i2c->clk);
+	if (i2c->clk) {
+		clk_disable(i2c->clk);
+		clk_put(i2c->clk);
+	}

 	iounmap(i2c->reg_base);
 	release_mem_region(i2c->iobase, i2c->iosize);



-- 
Sincerely yours,
Mike.



-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [RFC] PXA3xx: Add support for power i2c bus

[Eric Miao]

On Thu, Aug 14, 2008 at 4:55 PM, Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org> wrote:
> This patch adds support for power i2c bus on PXA3xx processor.
>
> Signed-off-by: Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>
>

I'd like the i2c driver to be converted to ioremap() and {read,write}l()
first before this can happen.

pxa27x_set_i2c_power_info()

and

pxa3xx_set_i2c_power_info()

are OK to co-exist, no need for a unified version. IOW, I'd prefer
the previous invocations of pxa_set_i2c_power_info() to be
renamed "pxa27x_set_i2c_power_info()"

The caller is supposed to know which to call.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [RFC] PXA3xx: Add support for power i2c bus

[Mike Rapoport]

Eric Miao wrote:
> On Thu, Aug 14, 2008 at 4:55 PM, Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org> wrote:
>> This patch adds support for power i2c bus on PXA3xx processor.
>>
>> Signed-off-by: Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>
>>
> 
> I'd like the i2c driver to be converted to ioremap() and {read,write}l()
> first before this can happen.

It is already using {read,write}l() unless I'm realy missing something...


> pxa27x_set_i2c_power_info()
> 
> and
> 
> pxa3xx_set_i2c_power_info()
> 
> are OK to co-exist, no need for a unified version. IOW, I'd prefer
> the previous invocations of pxa_set_i2c_power_info() to be
> renamed "pxa27x_set_i2c_power_info()"

Ok.

> The caller is supposed to know which to call.
> 

-- 
Sincerely yours,
Mike.


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [RFC] PXA3xx: Add support for power i2c bus

[Russell King - ARM Linux]

On Thu, Aug 14, 2008 at 11:55:07AM +0300, Mike Rapoport wrote:
> diff --git a/arch/arm/mach-pxa/generic.h b/arch/arm/mach-pxa/generic.h
> index 041c048..13a786d 100644
> --- a/arch/arm/mach-pxa/generic.h
> +++ b/arch/arm/mach-pxa/generic.h
> @@ -9,9 +9,10 @@
>   * published by the Free Software Foundation.
>   */
> 
> -typedef int (*set_wake_t)(unsigned int, unsigned int);
> +typedef int (*set_wake_t) (unsigned int, unsigned int);

Undoes perfectly good formatting.

> @@ -982,10 +992,13 @@ static int i2c_pxa_probe(struct platform_device *dev)
>  	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
>  		 i2c->adap.nr);
> 
> -	i2c->clk = clk_get(&dev->dev, "I2CCLK");
> -	if (IS_ERR(i2c->clk)) {
> -		ret = PTR_ERR(i2c->clk);
> -		goto eclk;
> +	/* PXA3xx has power I2C clock always on */
> +	if (!(cpu_is_pxa3xx() && i2c->adap.nr == 1)) {
> +		i2c->clk = clk_get(&dev->dev, "I2CCLK");
> +		if (IS_ERR(i2c->clk)) {
> +			ret = PTR_ERR(i2c->clk);
> +			goto eclk;
> +		}

Umm, no.  It would be far better to provide a dummy clock rather than
mess drivers up with this and lots of other conditional stuff.

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [RFC] PXA3xx: Add support for power i2c bus

[Mike Rapoport]

Russell King - ARM Linux wrote:
> On Thu, Aug 14, 2008 at 11:55:07AM +0300, Mike Rapoport wrote:
>> diff --git a/arch/arm/mach-pxa/generic.h b/arch/arm/mach-pxa/generic.h
>> index 041c048..13a786d 100644
>> --- a/arch/arm/mach-pxa/generic.h
>> +++ b/arch/arm/mach-pxa/generic.h
>> @@ -9,9 +9,10 @@
>>   * published by the Free Software Foundation.
>>   */
>>
>> -typedef int (*set_wake_t)(unsigned int, unsigned int);
>> +typedef int (*set_wake_t) (unsigned int, unsigned int);
> 
> Undoes perfectly good formatting.
> 
>> @@ -982,10 +992,13 @@ static int i2c_pxa_probe(struct platform_device *dev)
>>  	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
>>  		 i2c->adap.nr);
>>
>> -	i2c->clk = clk_get(&dev->dev, "I2CCLK");
>> -	if (IS_ERR(i2c->clk)) {
>> -		ret = PTR_ERR(i2c->clk);
>> -		goto eclk;
>> +	/* PXA3xx has power I2C clock always on */
>> +	if (!(cpu_is_pxa3xx() && i2c->adap.nr == 1)) {
>> +		i2c->clk = clk_get(&dev->dev, "I2CCLK");
>> +		if (IS_ERR(i2c->clk)) {
>> +			ret = PTR_ERR(i2c->clk);
>> +			goto eclk;
>> +		}
> 
> Umm, no.  It would be far better to provide a dummy clock rather than
> mess drivers up with this and lots of other conditional stuff.

Would that be better?

Signed-off-by: Mike Rapoport <mike@compulab.co.il>

 arch/arm/mach-pxa/devices.h          |    1 +
 arch/arm/mach-pxa/include/mach/i2c.h |    6 ++++-
 arch/arm/mach-pxa/pxa27x.c           |    2 +-
 arch/arm/mach-pxa/pxa3xx.c           |   45 ++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-pxa.c         |   20 +++++++++++----
 5 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-pxa/devices.h b/arch/arm/mach-pxa/devices.h
index 887c738..bb04af4 100644
--- a/arch/arm/mach-pxa/devices.h
+++ b/arch/arm/mach-pxa/devices.h
@@ -32,5 +32,6 @@ extern struct platform_device pxa27x_device_pwm0;
 extern struct platform_device pxa27x_device_pwm1;

 extern struct platform_device pxa3xx_device_nand;
+extern struct platform_device pxa3xx_device_i2c_power;

 void __init pxa_register_device(struct platform_device *dev, void *data);
diff --git a/arch/arm/mach-pxa/include/mach/i2c.h b/arch/arm/mach-pxa/include/mach/i2c.h
index 80596b0..897e717 100644
--- a/arch/arm/mach-pxa/include/mach/i2c.h
+++ b/arch/arm/mach-pxa/include/mach/i2c.h
@@ -71,7 +71,11 @@ struct i2c_pxa_platform_data {
 extern void pxa_set_i2c_info(struct i2c_pxa_platform_data *info);

 #ifdef CONFIG_PXA27x
-extern void pxa_set_i2c_power_info(struct i2c_pxa_platform_data *info);
+extern void pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info);
+#endif
+
+#ifdef CONFIG_PXA3xx
+extern void pxa3xx_set_i2c_power_info(struct i2c_pxa_platform_data *info);
 #endif

 #endif
diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index f9f6a9c..c33cf6a 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -349,7 +349,7 @@ struct platform_device pxa27x_device_i2c_power = {
 	.num_resources	= ARRAY_SIZE(i2c_power_resources),
 };

-void __init pxa_set_i2c_power_info(struct i2c_pxa_platform_data *info)
+void __init pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info)
 {
 	local_irq_disable();
 	PCFR |= PCFR_PI2CEN;
diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c
index 03cbc38..b3cd5d0 100644
--- a/arch/arm/mach-pxa/pxa3xx.c
+++ b/arch/arm/mach-pxa/pxa3xx.c
@@ -203,6 +203,19 @@ static const struct clkops clk_pout_ops = {
 	.disable	= clk_pout_disable,
 };

+static void clk_dummy_enable(struct clk *clk)
+{
+}
+
+static void clk_dummy_disable(struct clk *clk)
+{
+}
+
+static const struct clkops clk_dummy_ops = {
+	.enable		= clk_dummy_enable,
+	.disable	= clk_dummy_disable,
+};
+
 static struct clk pxa3xx_clks[] = {
 	{
 		.name           = "CLK_POUT",
@@ -211,6 +224,13 @@ static struct clk pxa3xx_clks[] = {
 		.delay          = 70,
 	},

+	/* Power I2C clock is always on */
+	{
+		.name		= "I2CCLK",
+		.ops		= &clk_dummy_ops,
+		.dev		= &pxa3xx_device_i2c_power.dev,
+	},
+
 	PXA3xx_CK("LCDCLK",  LCD,    &clk_pxa3xx_hsio_ops, &pxa_device_fb.dev),
 	PXA3xx_CK("CAMCLK",  CAMERA, &clk_pxa3xx_hsio_ops, NULL),
 	PXA3xx_CK("AC97CLK", AC97,   &clk_pxa3xx_ac97_ops, NULL),
@@ -509,6 +529,30 @@ void __init pxa3xx_init_irq(void)
  * device registration specific to PXA3xx.
  */

+static struct resource i2c_power_resources[] = {
+	{
+		.start  = 0x40f500c0,
+		.end    = 0x40f500d3,
+		.flags	= IORESOURCE_MEM,
+	}, {
+		.start	= IRQ_PWRI2C,
+		.end	= IRQ_PWRI2C,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+struct platform_device pxa3xx_device_i2c_power = {
+	.name		= "pxa2xx-i2c",
+	.id		= 1,
+	.resource	= i2c_power_resources,
+	.num_resources	= ARRAY_SIZE(i2c_power_resources),
+};
+
+void __init pxa3xx_set_i2c_power_info(struct i2c_pxa_platform_data *info)
+{
+	pxa3xx_device_i2c_power.dev.platform_data = info;
+}
+
 static struct platform_device *devices[] __initdata = {
 /*	&pxa_device_udc,	The UDC driver is PXA25x only */
 	&pxa_device_ffuart,
@@ -522,6 +566,7 @@ static struct platform_device *devices[] __initdata = {
 	&pxa3xx_device_ssp4,
 	&pxa27x_device_pwm0,
 	&pxa27x_device_pwm1,
+	&pxa3xx_device_i2c_power,
 };

 static struct sys_device pxa3xx_sysdev[] = {
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 44d8384..981bb07 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -68,11 +68,21 @@ struct pxa_i2c {
 	int			use_pio;
 };

-#define _IBMR(i2c)	((i2c)->reg_base + 0)
-#define _IDBR(i2c)	((i2c)->reg_base + 8)
-#define _ICR(i2c)	((i2c)->reg_base + 0x10)
-#define _ISR(i2c)	((i2c)->reg_base + 0x18)
-#define _ISAR(i2c)	((i2c)->reg_base + 0x20)
+#define _IBMR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x0 : 0x0))
+#define _IDBR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x4 : 0x8))
+#define _ICR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x8 : 0x10))
+#define _ISR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0xc : 0x18))
+#define _ISAR(i2c)	\
+	((i2c)->reg_base + \
+	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x10 : 0x20))

 /*
  * I2C Slave mode address



-- 
Sincerely yours,
Mike.


-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [i2c] [RFC] PXA3xx: Add support for power i2c bus

[Ben Dooks]

On Thu, Aug 14, 2008 at 02:38:09PM +0300, Mike Rapoport wrote:
> Russell King - ARM Linux wrote:
> > On Thu, Aug 14, 2008 at 11:55:07AM +0300, Mike Rapoport wrote:
> >> diff --git a/arch/arm/mach-pxa/generic.h b/arch/arm/mach-pxa/generic.h
> >> index 041c048..13a786d 100644
> >> --- a/arch/arm/mach-pxa/generic.h
> >> +++ b/arch/arm/mach-pxa/generic.h
> >> @@ -9,9 +9,10 @@
> >>   * published by the Free Software Foundation.
> >>   */
> >>
> >> -typedef int (*set_wake_t)(unsigned int, unsigned int);
> >> +typedef int (*set_wake_t) (unsigned int, unsigned int);
> > 
> > Undoes perfectly good formatting.
> > 
> >> @@ -982,10 +992,13 @@ static int i2c_pxa_probe(struct platform_device *dev)
> >>  	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
> >>  		 i2c->adap.nr);
> >>
> >> -	i2c->clk = clk_get(&dev->dev, "I2CCLK");
> >> -	if (IS_ERR(i2c->clk)) {
> >> -		ret = PTR_ERR(i2c->clk);
> >> -		goto eclk;
> >> +	/* PXA3xx has power I2C clock always on */
> >> +	if (!(cpu_is_pxa3xx() && i2c->adap.nr == 1)) {
> >> +		i2c->clk = clk_get(&dev->dev, "I2CCLK");
> >> +		if (IS_ERR(i2c->clk)) {
> >> +			ret = PTR_ERR(i2c->clk);
> >> +			goto eclk;
> >> +		}
> > 
> > Umm, no.  It would be far better to provide a dummy clock rather than
> > mess drivers up with this and lots of other conditional stuff.
> 
> Would that be better?
> 
> Signed-off-by: Mike Rapoport <mike@compulab.co.il>
> 
>  arch/arm/mach-pxa/devices.h          |    1 +
>  arch/arm/mach-pxa/include/mach/i2c.h |    6 ++++-
>  arch/arm/mach-pxa/pxa27x.c           |    2 +-
>  arch/arm/mach-pxa/pxa3xx.c           |   45 ++++++++++++++++++++++++++++++++++
>  drivers/i2c/busses/i2c-pxa.c         |   20 +++++++++++----
>  5 files changed, 67 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-pxa/devices.h b/arch/arm/mach-pxa/devices.h
> index 887c738..bb04af4 100644
> --- a/arch/arm/mach-pxa/devices.h
> +++ b/arch/arm/mach-pxa/devices.h
> @@ -32,5 +32,6 @@ extern struct platform_device pxa27x_device_pwm0;
>  extern struct platform_device pxa27x_device_pwm1;
> 
>  extern struct platform_device pxa3xx_device_nand;
> +extern struct platform_device pxa3xx_device_i2c_power;
> 
>  void __init pxa_register_device(struct platform_device *dev, void *data);
> diff --git a/arch/arm/mach-pxa/include/mach/i2c.h b/arch/arm/mach-pxa/include/mach/i2c.h
> index 80596b0..897e717 100644
> --- a/arch/arm/mach-pxa/include/mach/i2c.h
> +++ b/arch/arm/mach-pxa/include/mach/i2c.h
> @@ -71,7 +71,11 @@ struct i2c_pxa_platform_data {
>  extern void pxa_set_i2c_info(struct i2c_pxa_platform_data *info);
> 
>  #ifdef CONFIG_PXA27x
> -extern void pxa_set_i2c_power_info(struct i2c_pxa_platform_data *info);
> +extern void pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info);
> +#endif
> +
> +#ifdef CONFIG_PXA3xx
> +extern void pxa3xx_set_i2c_power_info(struct i2c_pxa_platform_data *info);
>  #endif
> 
>  #endif
> diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
> index f9f6a9c..c33cf6a 100644
> --- a/arch/arm/mach-pxa/pxa27x.c
> +++ b/arch/arm/mach-pxa/pxa27x.c
> @@ -349,7 +349,7 @@ struct platform_device pxa27x_device_i2c_power = {
>  	.num_resources	= ARRAY_SIZE(i2c_power_resources),
>  };
> 
> -void __init pxa_set_i2c_power_info(struct i2c_pxa_platform_data *info)
> +void __init pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info)
>  {
>  	local_irq_disable();
>  	PCFR |= PCFR_PI2CEN;
> diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c
> index 03cbc38..b3cd5d0 100644
> --- a/arch/arm/mach-pxa/pxa3xx.c
> +++ b/arch/arm/mach-pxa/pxa3xx.c
> @@ -203,6 +203,19 @@ static const struct clkops clk_pout_ops = {
>  	.disable	= clk_pout_disable,
>  };
> 
> +static void clk_dummy_enable(struct clk *clk)
> +{
> +}
> +
> +static void clk_dummy_disable(struct clk *clk)
> +{
> +}
> +
> +static const struct clkops clk_dummy_ops = {
> +	.enable		= clk_dummy_enable,
> +	.disable	= clk_dummy_disable,
> +};
> +
>  static struct clk pxa3xx_clks[] = {
>  	{
>  		.name           = "CLK_POUT",
> @@ -211,6 +224,13 @@ static struct clk pxa3xx_clks[] = {
>  		.delay          = 70,
>  	},
> 
> +	/* Power I2C clock is always on */
> +	{
> +		.name		= "I2CCLK",
> +		.ops		= &clk_dummy_ops,
> +		.dev		= &pxa3xx_device_i2c_power.dev,
> +	},
> +
>  	PXA3xx_CK("LCDCLK",  LCD,    &clk_pxa3xx_hsio_ops, &pxa_device_fb.dev),
>  	PXA3xx_CK("CAMCLK",  CAMERA, &clk_pxa3xx_hsio_ops, NULL),
>  	PXA3xx_CK("AC97CLK", AC97,   &clk_pxa3xx_ac97_ops, NULL),
> @@ -509,6 +529,30 @@ void __init pxa3xx_init_irq(void)
>   * device registration specific to PXA3xx.
>   */
> 
> +static struct resource i2c_power_resources[] = {
> +	{
> +		.start  = 0x40f500c0,
> +		.end    = 0x40f500d3,
> +		.flags	= IORESOURCE_MEM,
> +	}, {
> +		.start	= IRQ_PWRI2C,
> +		.end	= IRQ_PWRI2C,
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
> +struct platform_device pxa3xx_device_i2c_power = {
> +	.name		= "pxa2xx-i2c",
> +	.id		= 1,
> +	.resource	= i2c_power_resources,
> +	.num_resources	= ARRAY_SIZE(i2c_power_resources),
> +};
> +
> +void __init pxa3xx_set_i2c_power_info(struct i2c_pxa_platform_data *info)
> +{
> +	pxa3xx_device_i2c_power.dev.platform_data = info;
> +}
> +
>  static struct platform_device *devices[] __initdata = {
>  /*	&pxa_device_udc,	The UDC driver is PXA25x only */
>  	&pxa_device_ffuart,
> @@ -522,6 +566,7 @@ static struct platform_device *devices[] __initdata = {
>  	&pxa3xx_device_ssp4,
>  	&pxa27x_device_pwm0,
>  	&pxa27x_device_pwm1,
> +	&pxa3xx_device_i2c_power,
>  };
> 
>  static struct sys_device pxa3xx_sysdev[] = {
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index 44d8384..981bb07 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -68,11 +68,21 @@ struct pxa_i2c {
>  	int			use_pio;
>  };
> 
> -#define _IBMR(i2c)	((i2c)->reg_base + 0)
> -#define _IDBR(i2c)	((i2c)->reg_base + 8)
> -#define _ICR(i2c)	((i2c)->reg_base + 0x10)
> -#define _ISR(i2c)	((i2c)->reg_base + 0x18)
> -#define _ISAR(i2c)	((i2c)->reg_base + 0x20)
> +#define _IBMR(i2c)	\
> +	((i2c)->reg_base + \
> +	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x0 : 0x0))
> +#define _IDBR(i2c)	\
> +	((i2c)->reg_base + \
> +	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x4 : 0x8))
> +#define _ICR(i2c)	\
> +	((i2c)->reg_base + \
> +	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x8 : 0x10))
> +#define _ISR(i2c)	\
> +	((i2c)->reg_base + \
> +	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0xc : 0x18))
> +#define _ISAR(i2c)	\
> +	((i2c)->reg_base + \
> +	 ((cpu_is_pxa3xx() && ((i2c)->adap.nr == 1)) ? 0x10 : 0x20))

Not very nice, how about making the i2c structure have pointers
to each of the registers which can be setup at probe time. Or even
just have a multiplier in the structure, as these all look to be reg/2
in the case of pxa3xx and adap.nr == 1... 

Note, adap.nr shouldn't really be used to differentiate, use the
platform device number

+#define _IDBR(i2c)	((i2c)->reg_base + (0x4 << (i2c)->reg_shift))
+#define _ICR(i2c)	((i2c)->reg_base + (0x8 << (i2c)->reg_shift))
etc.

then do in the probe:

	i2c->reg_shift = (cpu_is_pxa3xx() && <is_power_i2c>) ? 0 : 1;

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [i2c] [RFC] PXA3xx: Add support for power i2c bus

[Mike Rapoport]

Ben Dooks wrote:
> On Thu, Aug 14, 2008 at 02:38:09PM +0300, Mike Rapoport wrote:
> 
> Not very nice, how about making the i2c structure have pointers
> to each of the registers which can be setup at probe time. Or even
> just have a multiplier in the structure, as these all look to be reg/2
> in the case of pxa3xx and adap.nr == 1... 
> 
> Note, adap.nr shouldn't really be used to differentiate, use the
> platform device number
> 
> +#define _IDBR(i2c)	((i2c)->reg_base + (0x4 << (i2c)->reg_shift))
> +#define _ICR(i2c)	((i2c)->reg_base + (0x8 << (i2c)->reg_shift))
> etc.
> 
> then do in the probe:
> 
> 	i2c->reg_shift = (cpu_is_pxa3xx() && <is_power_i2c>) ? 0 : 1;
> 

This is really much nicer. The fixed patch follows:

Signed-off-by: Mike Rapoport <mike@compulab.co.il>

 arch/arm/mach-pxa/devices.h          |    1 +
 arch/arm/mach-pxa/include/mach/i2c.h |    6 ++++-
 arch/arm/mach-pxa/pxa27x.c           |    2 +-
 arch/arm/mach-pxa/pxa3xx.c           |   45 ++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-pxa.c         |   12 +++++----
 5 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-pxa/devices.h b/arch/arm/mach-pxa/devices.h
index 887c738..bb04af4 100644
--- a/arch/arm/mach-pxa/devices.h
+++ b/arch/arm/mach-pxa/devices.h
@@ -32,5 +32,6 @@ extern struct platform_device pxa27x_device_pwm0;
 extern struct platform_device pxa27x_device_pwm1;

 extern struct platform_device pxa3xx_device_nand;
+extern struct platform_device pxa3xx_device_i2c_power;

 void __init pxa_register_device(struct platform_device *dev, void *data);
diff --git a/arch/arm/mach-pxa/include/mach/i2c.h b/arch/arm/mach-pxa/include/mach/i2c.h
index 80596b0..897e717 100644
--- a/arch/arm/mach-pxa/include/mach/i2c.h
+++ b/arch/arm/mach-pxa/include/mach/i2c.h
@@ -71,7 +71,11 @@ struct i2c_pxa_platform_data {
 extern void pxa_set_i2c_info(struct i2c_pxa_platform_data *info);

 #ifdef CONFIG_PXA27x
-extern void pxa_set_i2c_power_info(struct i2c_pxa_platform_data *info);
+extern void pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info);
+#endif
+
+#ifdef CONFIG_PXA3xx
+extern void pxa3xx_set_i2c_power_info(struct i2c_pxa_platform_data *info);
 #endif

 #endif
diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index f9f6a9c..c33cf6a 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -349,7 +349,7 @@ struct platform_device pxa27x_device_i2c_power = {
 	.num_resources	= ARRAY_SIZE(i2c_power_resources),
 };

-void __init pxa_set_i2c_power_info(struct i2c_pxa_platform_data *info)
+void __init pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info)
 {
 	local_irq_disable();
 	PCFR |= PCFR_PI2CEN;
diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c
index 03cbc38..b3cd5d0 100644
--- a/arch/arm/mach-pxa/pxa3xx.c
+++ b/arch/arm/mach-pxa/pxa3xx.c
@@ -203,6 +203,19 @@ static const struct clkops clk_pout_ops = {
 	.disable	= clk_pout_disable,
 };

+static void clk_dummy_enable(struct clk *clk)
+{
+}
+
+static void clk_dummy_disable(struct clk *clk)
+{
+}
+
+static const struct clkops clk_dummy_ops = {
+	.enable		= clk_dummy_enable,
+	.disable	= clk_dummy_disable,
+};
+
 static struct clk pxa3xx_clks[] = {
 	{
 		.name           = "CLK_POUT",
@@ -211,6 +224,13 @@ static struct clk pxa3xx_clks[] = {
 		.delay          = 70,
 	},

+	/* Power I2C clock is always on */
+	{
+		.name		= "I2CCLK",
+		.ops		= &clk_dummy_ops,
+		.dev		= &pxa3xx_device_i2c_power.dev,
+	},
+
 	PXA3xx_CK("LCDCLK",  LCD,    &clk_pxa3xx_hsio_ops, &pxa_device_fb.dev),
 	PXA3xx_CK("CAMCLK",  CAMERA, &clk_pxa3xx_hsio_ops, NULL),
 	PXA3xx_CK("AC97CLK", AC97,   &clk_pxa3xx_ac97_ops, NULL),
@@ -509,6 +529,30 @@ void __init pxa3xx_init_irq(void)
  * device registration specific to PXA3xx.
  */

+static struct resource i2c_power_resources[] = {
+	{
+		.start  = 0x40f500c0,
+		.end    = 0x40f500d3,
+		.flags	= IORESOURCE_MEM,
+	}, {
+		.start	= IRQ_PWRI2C,
+		.end	= IRQ_PWRI2C,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+struct platform_device pxa3xx_device_i2c_power = {
+	.name		= "pxa2xx-i2c",
+	.id		= 1,
+	.resource	= i2c_power_resources,
+	.num_resources	= ARRAY_SIZE(i2c_power_resources),
+};
+
+void __init pxa3xx_set_i2c_power_info(struct i2c_pxa_platform_data *info)
+{
+	pxa3xx_device_i2c_power.dev.platform_data = info;
+}
+
 static struct platform_device *devices[] __initdata = {
 /*	&pxa_device_udc,	The UDC driver is PXA25x only */
 	&pxa_device_ffuart,
@@ -522,6 +566,7 @@ static struct platform_device *devices[] __initdata = {
 	&pxa3xx_device_ssp4,
 	&pxa27x_device_pwm0,
 	&pxa27x_device_pwm1,
+	&pxa3xx_device_i2c_power,
 };

 static struct sys_device pxa3xx_sysdev[] = {
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 44d8384..5d5fbc5 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -60,6 +60,7 @@ struct pxa_i2c {
 	u32			icrlog[32];

 	void __iomem		*reg_base;
+	unsigned int		reg_shift;

 	unsigned long		iobase;
 	unsigned long		iosize;
@@ -68,11 +69,11 @@ struct pxa_i2c {
 	int			use_pio;
 };

-#define _IBMR(i2c)	((i2c)->reg_base + 0)
-#define _IDBR(i2c)	((i2c)->reg_base + 8)
-#define _ICR(i2c)	((i2c)->reg_base + 0x10)
-#define _ISR(i2c)	((i2c)->reg_base + 0x18)
-#define _ISAR(i2c)	((i2c)->reg_base + 0x20)
+#define _IBMR(i2c)	((i2c)->reg_base + (0x0 << (i2c)->reg_shift))
+#define _IDBR(i2c)	((i2c)->reg_base + (0x4 << (i2c)->reg_shift))
+#define _ICR(i2c)	((i2c)->reg_base + (0x8 << (i2c)->reg_shift))
+#define _ISR(i2c)	((i2c)->reg_base + (0xc << (i2c)->reg_shift))
+#define _ISAR(i2c)	((i2c)->reg_base + (0x10 << (i2c)->reg_shift))

 /*
  * I2C Slave mode address
@@ -993,6 +994,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 		ret = -EIO;
 		goto eremap;
 	}
+	i2c->reg_shift = (cpu_is_pxa3xx() && (dev->id == 1)) ? 0 : 1;

 	i2c->iobase = res->start;
 	i2c->iosize = res_len(res);


-- 
Sincerely yours,
Mike.


-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [RFC] PXA3xx: Add support for power i2c bus

[Eric Miao]

On Thu, Aug 14, 2008 at 9:43 PM, Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org> wrote:
>
> Ben Dooks wrote:
>> On Thu, Aug 14, 2008 at 02:38:09PM +0300, Mike Rapoport wrote:
>>
>> Not very nice, how about making the i2c structure have pointers
>> to each of the registers which can be setup at probe time. Or even
>> just have a multiplier in the structure, as these all look to be reg/2
>> in the case of pxa3xx and adap.nr == 1...
>>
>> Note, adap.nr shouldn't really be used to differentiate, use the
>> platform device number
>>
>> +#define _IDBR(i2c)   ((i2c)->reg_base + (0x4 << (i2c)->reg_shift))
>> +#define _ICR(i2c)    ((i2c)->reg_base + (0x8 << (i2c)->reg_shift))
>> etc.
>>
>> then do in the probe:
>>
>>       i2c->reg_shift = (cpu_is_pxa3xx() && <is_power_i2c>) ? 0 : 1;
>>
>
> This is really much nicer. The fixed patch follows:
>
> Signed-off-by: Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>

Looks much better now.

You may use INIT_CLK() for the dummy I2C clk for consistency with
other CLK definitions.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [RFC] PXA3xx: Add support for power i2c bus

[Russell King - ARM Linux]

On Thu, Aug 14, 2008 at 04:43:13PM +0300, Mike Rapoport wrote:
> This is really much nicer. The fixed patch follows:
> 
> Signed-off-by: Mike Rapoport <mike@compulab.co.il>
> 
>  arch/arm/mach-pxa/devices.h          |    1 +
>  arch/arm/mach-pxa/include/mach/i2c.h |    6 ++++-
>  arch/arm/mach-pxa/pxa27x.c           |    2 +-
>  arch/arm/mach-pxa/pxa3xx.c           |   45 ++++++++++++++++++++++++++++++++++
>  drivers/i2c/busses/i2c-pxa.c         |   12 +++++----
>  5 files changed, 59 insertions(+), 7 deletions(-)

Ok.

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php