My comments are inline below.
- John
On 05/10/2011 03:10 PM, Stephen Warren wrote:
> John Bonesio wrote at Tuesday, May 10, 2011 2:15 PM:
>> This patch initializes i2c controller devices in board-dt.c. The i2c
>> controller
>> is added to tegra250.dtsi so later on-board i2c devices can be found and
>> initialized based on the device tree information.
>
>> diff --git a/arch/arm/boot/dts/tegra250.dtsi b/arch/arm/boot/dts/tegra250.dtsi
>
>> +
>> + i2c@7000C000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "nvidia,tegra250-i2c";
>> + reg = <0x7000C000 0x100>;
>> + interrupts = < 70 >;
>> + };
>
> Could we add 'status = "disabled"' for all of these in the core Tegra SoC
> file. Then, the board-specific files (e.g. tegra-harmony.dts) will set
> 'status = "ok"' for just the particular controllers that are actually used
> by the board. The SDHCI controllers are setup this way now per my recent
> patches.
Good idea.
>
>> diff --git a/arch/arm/mach-tegra/board-dt.c b/arch/arm/mach-tegra/board-dt.c
>
>> +static struct tegra_i2c_platform_data harmony_i2c1_platform_data = {
>> + .bus_clk_rate = 400000,
>> +};
>
> Similar to my comments on the SDHCI patch, shouldn't the platform data
> come only from devicetree?
>
>> +static void __init harmony_i2c_init(void)
>
> It'd be nice not to name stuff in board-dt.c after "harmony", since the
> idea is for it to work on arbitrary boards given the appropriate
> devicetree.
Yep. I'll fix this in the next revision.
>
>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>
> Since the driver and board files go to separate subsystems, shouldn't
> the changes to this file be a separate patch, to aid eventual
> upstreaming?
Maybe. Let's see what others think.
>
> BTW, thanks for working on Tegra!
>
:-)
--
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
