linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] pxa: Hookup MAX7312 thats connected to I2C.
@ 2010-08-25  9:18 Sachin Sanap
  2010-08-25 11:21 ` Haojian Zhuang
  2010-08-25 12:59 ` Marek Vasut
  0 siblings, 2 replies; 5+ messages in thread
From: Sachin Sanap @ 2010-08-25  9:18 UTC (permalink / raw)
  To: linux-arm-kernel


Signed-off-by: Sachin Sanap <ssanap@marvell.com>
---
 arch/arm/mach-mmp/aspenite.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mmp/aspenite.c b/arch/arm/mach-mmp/aspenite.c
index 244655d..d19c26c 100644
--- a/arch/arm/mach-mmp/aspenite.c
+++ b/arch/arm/mach-mmp/aspenite.c
@@ -23,9 +23,12 @@
 #include <mach/mfp-pxa168.h>
 #include <mach/pxa168.h>
 #include <mach/gpio.h>
+#include <linux/i2c/pca953x.h>
 
 #include "common.h"
 
+#define GPIO_EXT1(x)	(128 + 16 + (x))
+
 static unsigned long common_pin_config[] __initdata = {
 	/* Data Flash Interface */
 	GPIO0_DFI_D15,
@@ -66,6 +69,11 @@ static unsigned long common_pin_config[] __initdata = {
 	GPIO115_I2S_BCLK,
 	GPIO116_I2S_RXD,
 	GPIO117_I2S_TXD,
+
+	/* i2c bus */
+	GPIO105_CI2C_SDA,
+	GPIO106_CI2C_SCL,
+
 };
 
 static struct smc91x_platdata smc91x_info = {
@@ -130,17 +138,36 @@ static struct pxa3xx_nand_platform_data aspenite_nand_info = {
 	.nr_parts	= ARRAY_SIZE(aspenite_nand_partitions),
 };
 
-static struct i2c_board_info aspenite_i2c_info[] __initdata = {
+#if defined(CONFIG_GPIO_PCA953X)
+static struct pca953x_platform_data max7312_data[] = {
+	[0] = {
+		.gpio_base      = GPIO_EXT1(0),
+	},
+};
+#endif
+
+static struct i2c_board_info aspenite_pwri2c_info[] __initdata = {
 	{ I2C_BOARD_INFO("wm8753", 0x1b), },
 };
 
+static struct i2c_board_info aspenite_i2c_info[] __initdata = {
+#if defined(CONFIG_GPIO_PCA953X)
+	{
+		I2C_BOARD_INFO("max7312", 0x20),
+		.irq = IRQ_GPIO(120),
+		.platform_data  = &max7312_data[0],
+	},
+#endif
+};
+
 static void __init common_init(void)
 {
 	mfp_config(ARRAY_AND_SIZE(common_pin_config));
 
 	/* on-chip devices */
 	pxa168_add_uart(1);
-	pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
+	pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
+	pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));
 	pxa168_add_ssp(1);
 	pxa168_add_nand(&aspenite_nand_info);
 
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 1/3] pxa: Hookup MAX7312 thats connected to I2C.
  2010-08-25  9:18 [PATCH 1/3] pxa: Hookup MAX7312 thats connected to I2C Sachin Sanap
@ 2010-08-25 11:21 ` Haojian Zhuang
  2010-08-25 12:59 ` Marek Vasut
  1 sibling, 0 replies; 5+ messages in thread
From: Haojian Zhuang @ 2010-08-25 11:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Aug 25, 2010 at 5:18 PM, Sachin Sanap <ssanap@marvell.com> wrote:
>
> Signed-off-by: Sachin Sanap <ssanap@marvell.com>
> ---
> ?arch/arm/mach-mmp/aspenite.c | ? 31 +++++++++++++++++++++++++++++--
> ?1 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-mmp/aspenite.c b/arch/arm/mach-mmp/aspenite.c
> index 244655d..d19c26c 100644
> --- a/arch/arm/mach-mmp/aspenite.c
> +++ b/arch/arm/mach-mmp/aspenite.c
> @@ -23,9 +23,12 @@
> ?#include <mach/mfp-pxa168.h>
> ?#include <mach/pxa168.h>
> ?#include <mach/gpio.h>
> +#include <linux/i2c/pca953x.h>
>
> ?#include "common.h"
>
> +#define GPIO_EXT1(x) ? (128 + 16 + (x))
> +
You should use macro at here. For example NR_BUILTIN_GPIO.

> ?static unsigned long common_pin_config[] __initdata = {
> ? ? ? ?/* Data Flash Interface */
> ? ? ? ?GPIO0_DFI_D15,
> @@ -66,6 +69,11 @@ static unsigned long common_pin_config[] __initdata = {
> ? ? ? ?GPIO115_I2S_BCLK,
> ? ? ? ?GPIO116_I2S_RXD,
> ? ? ? ?GPIO117_I2S_TXD,
> +
> + ? ? ? /* i2c bus */
> + ? ? ? GPIO105_CI2C_SDA,
> + ? ? ? GPIO106_CI2C_SCL,
> +
> ?};
>
> ?static struct smc91x_platdata smc91x_info = {
> @@ -130,17 +138,36 @@ static struct pxa3xx_nand_platform_data aspenite_nand_info = {
> ? ? ? ?.nr_parts ? ? ? = ARRAY_SIZE(aspenite_nand_partitions),
> ?};
>
> -static struct i2c_board_info aspenite_i2c_info[] __initdata = {
> +#if defined(CONFIG_GPIO_PCA953X)
> +static struct pca953x_platform_data max7312_data[] = {
> + ? ? ? [0] = {
> + ? ? ? ? ? ? ? .gpio_base ? ? ?= GPIO_EXT1(0),
> + ? ? ? },
> +};
> +#endif
> +
> +static struct i2c_board_info aspenite_pwri2c_info[] __initdata = {
> ? ? ? ?{ I2C_BOARD_INFO("wm8753", 0x1b), },
> ?};
>
> +static struct i2c_board_info aspenite_i2c_info[] __initdata = {
> +#if defined(CONFIG_GPIO_PCA953X)
> + ? ? ? {
> + ? ? ? ? ? ? ? I2C_BOARD_INFO("max7312", 0x20),
> + ? ? ? ? ? ? ? .irq = IRQ_GPIO(120),
> + ? ? ? ? ? ? ? .platform_data ?= &max7312_data[0],
> + ? ? ? },
> +#endif
> +};
> +
> ?static void __init common_init(void)
> ?{
> ? ? ? ?mfp_config(ARRAY_AND_SIZE(common_pin_config));
>
> ? ? ? ?/* on-chip devices */
> ? ? ? ?pxa168_add_uart(1);
> - ? ? ? pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
> + ? ? ? pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
> + ? ? ? pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));
> ? ? ? ?pxa168_add_ssp(1);
> ? ? ? ?pxa168_add_nand(&aspenite_nand_info);
>
> --
> 1.6.2.5
>
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] pxa: Hookup MAX7312 thats connected to I2C.
  2010-08-25  9:18 [PATCH 1/3] pxa: Hookup MAX7312 thats connected to I2C Sachin Sanap
  2010-08-25 11:21 ` Haojian Zhuang
@ 2010-08-25 12:59 ` Marek Vasut
  2010-08-25 13:14   ` Eric Miao
  1 sibling, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2010-08-25 12:59 UTC (permalink / raw)
  To: linux-arm-kernel

Dne St 25. srpna 2010 11:18:16 Sachin Sanap napsal(a):
> Signed-off-by: Sachin Sanap <ssanap@marvell.com>
> ---
>  arch/arm/mach-mmp/aspenite.c |   31 +++++++++++++++++++++++++++++--
>  1 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-mmp/aspenite.c b/arch/arm/mach-mmp/aspenite.c
> index 244655d..d19c26c 100644
> --- a/arch/arm/mach-mmp/aspenite.c
> +++ b/arch/arm/mach-mmp/aspenite.c
> @@ -23,9 +23,12 @@
>  #include <mach/mfp-pxa168.h>
>  #include <mach/pxa168.h>
>  #include <mach/gpio.h>
> +#include <linux/i2c/pca953x.h>
> 
>  #include "common.h"
> 
> +#define GPIO_EXT1(x)	(128 + 16 + (x))
> +
>  static unsigned long common_pin_config[] __initdata = {
>  	/* Data Flash Interface */
>  	GPIO0_DFI_D15,
> @@ -66,6 +69,11 @@ static unsigned long common_pin_config[] __initdata = {
>  	GPIO115_I2S_BCLK,
>  	GPIO116_I2S_RXD,
>  	GPIO117_I2S_TXD,
> +
> +	/* i2c bus */
> +	GPIO105_CI2C_SDA,
> +	GPIO106_CI2C_SCL,
> +
>  };
> 
>  static struct smc91x_platdata smc91x_info = {
> @@ -130,17 +138,36 @@ static struct pxa3xx_nand_platform_data
> aspenite_nand_info = { .nr_parts	= ARRAY_SIZE(aspenite_nand_partitions),
>  };
> 
> -static struct i2c_board_info aspenite_i2c_info[] __initdata = {
> +#if defined(CONFIG_GPIO_PCA953X)
> +static struct pca953x_platform_data max7312_data[] = {
> +	[0] = {
> +		.gpio_base      = GPIO_EXT1(0),
> +	},
> +};
> +#endif
> +
> +static struct i2c_board_info aspenite_pwri2c_info[] __initdata = {
>  	{ I2C_BOARD_INFO("wm8753", 0x1b), },
>  };
> 
> +static struct i2c_board_info aspenite_i2c_info[] __initdata = {
> +#if defined(CONFIG_GPIO_PCA953X)
> +	{
> +		I2C_BOARD_INFO("max7312", 0x20),
> +		.irq = IRQ_GPIO(120),
> +		.platform_data  = &max7312_data[0],
> +	},
> +#endif
> +};
> +
I see what you're trying to achieve here, but this seems quite clumsy actually. 
Try this approach (see mach-pxa/colibri_pxa320.c for example):

#if defined(...aspenite_pi2c_chip is selected) || defined(...dtto..._MODULE)
static <pi2c chips pdata> aspenite_pwri2c_info = {

};

statid void __init aspenite_pi2c_init(void)
{
	pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));
}
#else
static inline void aspenite_pi2c_init(void) {}
#endif

static void __init common_init()
{
...
  	pxa168_add_uart(1);
	aspenite_pi2c_init();
	aspenite_i2c_init();
...
}

>  static void __init common_init(void)
>  {
>  	mfp_config(ARRAY_AND_SIZE(common_pin_config));
> 
>  	/* on-chip devices */
>  	pxa168_add_uart(1);
> -	pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
> +	pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
> +	pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));

Why not just add:
pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));

But anyway, this can be dumped with the code I outlined above. Cheers
?
>  	pxa168_add_ssp(1);
>  	pxa168_add_nand(&aspenite_nand_info);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] pxa: Hookup MAX7312 thats connected to I2C.
  2010-08-25 12:59 ` Marek Vasut
@ 2010-08-25 13:14   ` Eric Miao
  2010-08-25 13:18     ` Marek Vasut
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Miao @ 2010-08-25 13:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Aug 25, 2010 at 8:59 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dne St 25. srpna 2010 11:18:16 Sachin Sanap napsal(a):
>> Signed-off-by: Sachin Sanap <ssanap@marvell.com>
>> ---
>> ?arch/arm/mach-mmp/aspenite.c | ? 31 +++++++++++++++++++++++++++++--
>> ?1 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-mmp/aspenite.c b/arch/arm/mach-mmp/aspenite.c
>> index 244655d..d19c26c 100644
>> --- a/arch/arm/mach-mmp/aspenite.c
>> +++ b/arch/arm/mach-mmp/aspenite.c
>> @@ -23,9 +23,12 @@
>> ?#include <mach/mfp-pxa168.h>
>> ?#include <mach/pxa168.h>
>> ?#include <mach/gpio.h>
>> +#include <linux/i2c/pca953x.h>
>>
>> ?#include "common.h"
>>
>> +#define GPIO_EXT1(x) (128 + 16 + (x))
>> +
>> ?static unsigned long common_pin_config[] __initdata = {
>> ? ? ? /* Data Flash Interface */
>> ? ? ? GPIO0_DFI_D15,
>> @@ -66,6 +69,11 @@ static unsigned long common_pin_config[] __initdata = {
>> ? ? ? GPIO115_I2S_BCLK,
>> ? ? ? GPIO116_I2S_RXD,
>> ? ? ? GPIO117_I2S_TXD,
>> +
>> + ? ? /* i2c bus */
>> + ? ? GPIO105_CI2C_SDA,
>> + ? ? GPIO106_CI2C_SCL,
>> +
>> ?};
>>
>> ?static struct smc91x_platdata smc91x_info = {
>> @@ -130,17 +138,36 @@ static struct pxa3xx_nand_platform_data
>> aspenite_nand_info = { .nr_parts ? ? ?= ARRAY_SIZE(aspenite_nand_partitions),
>> ?};
>>
>> -static struct i2c_board_info aspenite_i2c_info[] __initdata = {
>> +#if defined(CONFIG_GPIO_PCA953X)
>> +static struct pca953x_platform_data max7312_data[] = {
>> + ? ? [0] = {
>> + ? ? ? ? ? ? .gpio_base ? ? ?= GPIO_EXT1(0),
>> + ? ? },
>> +};
>> +#endif
>> +
>> +static struct i2c_board_info aspenite_pwri2c_info[] __initdata = {
>> ? ? ? { I2C_BOARD_INFO("wm8753", 0x1b), },
>> ?};
>>
>> +static struct i2c_board_info aspenite_i2c_info[] __initdata = {
>> +#if defined(CONFIG_GPIO_PCA953X)
>> + ? ? {
>> + ? ? ? ? ? ? I2C_BOARD_INFO("max7312", 0x20),
>> + ? ? ? ? ? ? .irq = IRQ_GPIO(120),
>> + ? ? ? ? ? ? .platform_data ?= &max7312_data[0],
>> + ? ? },
>> +#endif
>> +};
>> +
> I see what you're trying to achieve here, but this seems quite clumsy actually.
> Try this approach (see mach-pxa/colibri_pxa320.c for example):
>
> #if defined(...aspenite_pi2c_chip is selected) || defined(...dtto..._MODULE)
> static <pi2c chips pdata> aspenite_pwri2c_info = {
>
> };
>
> statid void __init aspenite_pi2c_init(void)
> {
> ? ? ? ?pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));
> }
> #else
> static inline void aspenite_pi2c_init(void) {}
> #endif
>

Or to leave those #ifdef .. #endif out completely. I don't actually care
that much about the benefit of saving some additional bytes compared
with the mess sometimes with all those #ifdef .. floating around.

> static void __init common_init()
> {
> ...
> ? ? ? ?pxa168_add_uart(1);
> ? ? ? ?aspenite_pi2c_init();
> ? ? ? ?aspenite_i2c_init();
> ...
> }
>
>> ?static void __init common_init(void)
>> ?{
>> ? ? ? mfp_config(ARRAY_AND_SIZE(common_pin_config));
>>
>> ? ? ? /* on-chip devices */
>> ? ? ? pxa168_add_uart(1);
>> - ? ? pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
>> + ? ? pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
>> + ? ? pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));
>
> Why not just add:
> pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));
>
> But anyway, this can be dumped with the code I outlined above. Cheers
> ?
>> ? ? ? pxa168_add_ssp(1);
>> ? ? ? pxa168_add_nand(&aspenite_nand_info);
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] pxa: Hookup MAX7312 thats connected to I2C.
  2010-08-25 13:14   ` Eric Miao
@ 2010-08-25 13:18     ` Marek Vasut
  0 siblings, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2010-08-25 13:18 UTC (permalink / raw)
  To: linux-arm-kernel

Dne St 25. srpna 2010 15:14:35 Eric Miao napsal(a):
> On Wed, Aug 25, 2010 at 8:59 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dne St 25. srpna 2010 11:18:16 Sachin Sanap napsal(a):
> >> Signed-off-by: Sachin Sanap <ssanap@marvell.com>
> >> ---
> >>  arch/arm/mach-mmp/aspenite.c |   31 +++++++++++++++++++++++++++++--
> >>  1 files changed, 29 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/arch/arm/mach-mmp/aspenite.c b/arch/arm/mach-mmp/aspenite.c
> >> index 244655d..d19c26c 100644
> >> --- a/arch/arm/mach-mmp/aspenite.c
> >> +++ b/arch/arm/mach-mmp/aspenite.c
> >> @@ -23,9 +23,12 @@
> >>  #include <mach/mfp-pxa168.h>
> >>  #include <mach/pxa168.h>
> >>  #include <mach/gpio.h>
> >> +#include <linux/i2c/pca953x.h>
> >> 
> >>  #include "common.h"
> >> 
> >> +#define GPIO_EXT1(x) (128 + 16 + (x))
> >> +
> >>  static unsigned long common_pin_config[] __initdata = {
> >>       /* Data Flash Interface */
> >>       GPIO0_DFI_D15,
> >> @@ -66,6 +69,11 @@ static unsigned long common_pin_config[] __initdata =
> >> { GPIO115_I2S_BCLK,
> >>       GPIO116_I2S_RXD,
> >>       GPIO117_I2S_TXD,
> >> +
> >> +     /* i2c bus */
> >> +     GPIO105_CI2C_SDA,
> >> +     GPIO106_CI2C_SCL,
> >> +
> >>  };
> >> 
> >>  static struct smc91x_platdata smc91x_info = {
> >> @@ -130,17 +138,36 @@ static struct pxa3xx_nand_platform_data
> >> aspenite_nand_info = { .nr_parts      =
> >> ARRAY_SIZE(aspenite_nand_partitions), };
> >> 
> >> -static struct i2c_board_info aspenite_i2c_info[] __initdata = {
> >> +#if defined(CONFIG_GPIO_PCA953X)
> >> +static struct pca953x_platform_data max7312_data[] = {
> >> +     [0] = {
> >> +             .gpio_base      = GPIO_EXT1(0),
> >> +     },
> >> +};
> >> +#endif
> >> +
> >> +static struct i2c_board_info aspenite_pwri2c_info[] __initdata = {
> >>       { I2C_BOARD_INFO("wm8753", 0x1b), },
> >>  };
> >> 
> >> +static struct i2c_board_info aspenite_i2c_info[] __initdata = {
> >> +#if defined(CONFIG_GPIO_PCA953X)
> >> +     {
> >> +             I2C_BOARD_INFO("max7312", 0x20),
> >> +             .irq = IRQ_GPIO(120),
> >> +             .platform_data  = &max7312_data[0],
> >> +     },
> >> +#endif
> >> +};
> >> +
> > 
> > I see what you're trying to achieve here, but this seems quite clumsy
> > actually. Try this approach (see mach-pxa/colibri_pxa320.c for example):
> > 
> > #if defined(...aspenite_pi2c_chip is selected) ||
> > defined(...dtto..._MODULE) static <pi2c chips pdata>
> > aspenite_pwri2c_info = {
> > 
> > };
> > 
> > statid void __init aspenite_pi2c_init(void)
> > {
> >        pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));
> > }
> > #else
> > static inline void aspenite_pi2c_init(void) {}
> > #endif
> 
> Or to leave those #ifdef .. #endif out completely. I don't actually care
> that much about the benefit of saving some additional bytes compared
> with the mess sometimes with all those #ifdef .. floating around.

Well if you use nice formating like I did with very descriptive comments (like 
palm has, balloon3 has, vpac has etc, where even blind dude has to notice what 
is in the code below), this much ifdefs is actually ok I think.

> 
> > static void __init common_init()
> > {
> > ...
> >        pxa168_add_uart(1);
> >        aspenite_pi2c_init();
> >        aspenite_i2c_init();
> > ...
> > }
> > 
> >>  static void __init common_init(void)
> >>  {
> >>       mfp_config(ARRAY_AND_SIZE(common_pin_config));
> >> 
> >>       /* on-chip devices */
> >>       pxa168_add_uart(1);
> >> -     pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
> >> +     pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_i2c_info));
> >> +     pxa168_add_twsi(1, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));
> > 
> > Why not just add:
> > pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(aspenite_pwri2c_info));
> > 
> > But anyway, this can be dumped with the code I outlined above. Cheers
> > ?
> > 
> >>       pxa168_add_ssp(1);
> >>       pxa168_add_nand(&aspenite_nand_info);

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-08-25 13:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-25  9:18 [PATCH 1/3] pxa: Hookup MAX7312 thats connected to I2C Sachin Sanap
2010-08-25 11:21 ` Haojian Zhuang
2010-08-25 12:59 ` Marek Vasut
2010-08-25 13:14   ` Eric Miao
2010-08-25 13:18     ` Marek Vasut

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).