[oops, missed reply-all] Hi, On Mon, Mar 7, 2011 at 9:24 AM, Stephen Warren <swarren@xxxxxxxxxx> 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@xxxxxxxxx> >> --- >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html