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.
> 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.
> 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?
BTW, thanks for working on Tegra!
--
nvpublic
ÿô.nlj·Ÿ®‰†+%ŠË±é¥Šwÿº{.nlj·¥Š{±þׯâ^n‡r¡öë¨è&£ûz¹Þúzf£¢·hšˆ§~††Ûÿÿïÿ‘ê_èæ+v‰¨þ)ßø
