[PATCH 1/3] pxa: Hookup MAX7312 thats connected to I2C.

[Eric Miao]

On Wed, Aug 25, 2010 at 8:59 PM, Marek Vasut <marek.vasut at gmail.com> wrote:
> Dne St 25. srpna 2010 11:18:16 Sachin Sanap napsal(a):
>> Signed-off-by: Sachin Sanap <ssanap at 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);
>