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

[oops, missed reply-all]

Hi,

On Mon, Mar 7, 2011 at 9:24 AM, Stephen Warren [off-list ref] wrote:

Olof Johansson wrote at Monday, March 07, 2011 1:27 AM:

quoted

Register the base i2c devices on seaboard. A few more are pending,
but it's a start.

Signed-off-by: Olof Johansson <redacted>
---
?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).
[...]

[...]

quoted

+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.

quoted

+ ? ? 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.

quoted

+ ? ? 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.

quoted

+
+ ? ? 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