[PATCH 1/3] ARM: tegra: seaboard: register i2c devices

[Olof Johansson]

[oops, missed reply-all]

Hi,

On Mon, Mar 7, 2011 at 9:24 AM, Stephen Warren <swarren at nvidia.com> wrote:
> Olof Johansson wrote at Monday, March 07, 2011 1:27 AM:
>> Register the base i2c devices on seaboard. A few more are pending,
>> but it's a start.
>>
>> Signed-off-by: Olof Johansson <olof at lixom.net>
>> ---
>>  arch/arm/mach-tegra/board-seaboard-pinmux.c |    1 +
>>  arch/arm/mach-tegra/board-seaboard.c        |   83
>> +++++++++++++++++++++++++++
>>  2 files changed, 84 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-tegra/board-seaboard-pinmux.c b/arch/arm/mach-
>> tegra/board-seaboard-pinmux.c
>> index 7e96d49..d84d1dd 100644
>> --- a/arch/arm/mach-tegra/board-seaboard-pinmux.c
>> +++ b/arch/arm/mach-tegra/board-seaboard-pinmux.c
>> @@ -166,6 +166,7 @@ static struct tegra_gpio_table gpio_table[] = {
>>       { .gpio = TEGRA_GPIO_SD2_POWER,         .enable = true },
>>       { .gpio = TEGRA_GPIO_LIDSWITCH,         .enable = true },
>>       { .gpio = TEGRA_GPIO_POWERKEY,          .enable = true },
>> +     { .gpio = TEGRA_GPIO_ISL29018_IRQ,      .enable = true },
>
> The indentation between fields looks different there.

No, shouldn't be? I just checked the code, it uses the same whitespace
(tabs/spaces).
[...]

[...]

>> +static void __init seaboard_i2c_init(void)
>> +{
>> +     gpio_request(TEGRA_GPIO_ISL29018_IRQ, "isl29018");
>> +     gpio_direction_input(TEGRA_GPIO_ISL29018_IRQ);
>
> Hmm. For some reason I thought drivers did this themselves, or IRQ
> registration did this for them. However, I looked and that's not true. I
> think I was remembering snd_soc_jack_add_gpios instead.
>
> So, this code looks fine, but I guess equivalent calls are missing for the
> WM8903 IRQ in my patches?

Yeah, probably.

>> +     i2c_register_board_info(0, &isl29018_device, 1);
>> +
>> +     i2c_register_board_info(4, &adt7461_device, 1);
>> +
>> +     common_i2c_init();
>> +}
>> +
>> +static void __init kaen_i2c_init(void)
>> +{
>
> {seaboard,kaen,wario}_i2c_init seem identical. Should this be a single shared
> function, and only the board-specific bits in the non-common functions? Also,
> see below.

They are now -- they weren't when all i2c devices were registered. I
can re-join them and split them when devices are re-introduced later.



>
>> +     gpio_request(TEGRA_GPIO_ISL29018_IRQ, "isl29018");
>> +     gpio_direction_input(TEGRA_GPIO_ISL29018_IRQ);
>> +
>> +     i2c_register_board_info(0, &isl29018_device, 1);
>> +
>> +     i2c_register_board_info(4, &adt7461_device, 1);
>> +
>> +     common_i2c_init();
>> +}
>> +
>> +static void __init wario_i2c_init(void)
>> +{
>> +     gpio_request(TEGRA_GPIO_ISL29018_IRQ, "isl29018");
>> +     gpio_direction_input(TEGRA_GPIO_ISL29018_IRQ);
>> +
>> +     i2c_register_board_info(0, &isl29018_device, 1);
>> +
>> +     i2c_register_board_info(4, &adt7461_device, 1);
>> +
>> +     common_i2c_init();
>> +}
>> +
>>  static void __init __tegra_seaboard_init(void)
>>  {
>>       seaboard_pinmux_init();
>> @@ -145,6 +222,8 @@ static void __init tegra_seaboard_init(void)
>>       debug_uart_platform_data[0].irq = INT_UARTD;
>>
>>       __tegra_seaboard_init();
>> +
>> +     seaboard_i2c_init();
>>  }
>>
>>  static void __init tegra_kaen_init(void)
>> @@ -155,6 +234,8 @@ static void __init tegra_kaen_init(void)
>>       debug_uart_platform_data[0].irq = INT_UARTB;
>>
>>       __tegra_seaboard_init();
>
> __tegra_seaboard_init calls seaboard_i2c_init.

It shouldn't be (note __tegra* vs tegra*). Maybe I should rename
__tegra_seaboard_init to something less alike.

>
>> +
>> +     kaen_i2c_init();
>
> kaen_i2c_init is the same as seaboard_i2c_init.
>
> So, all the registration happens twice?

Shouldn't be. The code I am looking at doesn't, so please check your side again?


-Olof