* [U-Boot] [PATCH v2 0/3] Add I2C driver for T114 Dalmore
@ 2013-02-06 23:26 Tom Warren
2013-02-06 23:26 ` [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver Tom Warren
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Tom Warren @ 2013-02-06 23:26 UTC (permalink / raw)
To: u-boot
Update DT tables and enable I2C driver support for the
Tegra114 Dalmore board. This uses the standard Tegra I2C driver.
5 controllers are supported, although all may not have devices
behind them on every board.
Changes in V2:
- use new calculation for T114 I2C clocks
- incorporate review comments from StephenW for the dtsi file
Tom Warren (3):
Tegra: I2C: Add T114 clock support to tegra_i2c driver
Tegra114: fdt: Update DT files with I2C info for T114/Dalmore
Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board
arch/arm/dts/tegra114.dtsi | 56 +++++++++++++++++++++++++++
arch/arm/include/asm/arch-tegra/tegra_i2c.h | 6 +++
board/nvidia/dts/tegra114-dalmore.dts | 33 ++++++++++++++++
drivers/i2c/tegra_i2c.c | 22 ++++++++++-
include/configs/dalmore.h | 9 ++++
include/configs/tegra114-common.h | 3 +
6 files changed, 128 insertions(+), 1 deletions(-)
^ permalink raw reply [flat|nested] 27+ messages in thread* [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver 2013-02-06 23:26 [U-Boot] [PATCH v2 0/3] Add I2C driver for T114 Dalmore Tom Warren @ 2013-02-06 23:26 ` Tom Warren 2013-02-06 23:50 ` Stephen Warren ` (2 more replies) 2013-02-06 23:26 ` [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore Tom Warren 2013-02-06 23:26 ` [U-Boot] [PATCH v2 3/3] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board Tom Warren 2 siblings, 3 replies; 27+ messages in thread From: Tom Warren @ 2013-02-06 23:26 UTC (permalink / raw) To: u-boot T114 has a slightly different I2C clock, with a new divisor for standard/fast mode and HS mode. Tested on my Dalmore, and the I2C clock is 100KHz +/- 3% on my Saleae Logic analyzer. Signed-off-by: Tom Warren <twarren@nvidia.com> --- v2: new arch/arm/include/asm/arch-tegra/tegra_i2c.h | 6 ++++++ drivers/i2c/tegra_i2c.c | 22 +++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletions(-) diff --git a/arch/arm/include/asm/arch-tegra/tegra_i2c.h b/arch/arm/include/asm/arch-tegra/tegra_i2c.h index 2650744..853e59b 100644 --- a/arch/arm/include/asm/arch-tegra/tegra_i2c.h +++ b/arch/arm/include/asm/arch-tegra/tegra_i2c.h @@ -105,6 +105,7 @@ struct i2c_ctlr { u32 sl_delay_count; /* 3C: I2C_I2C_SL_DELAY_COUNT */ u32 reserved_2[4]; /* 40: */ struct i2c_control control; /* 50 ~ 68 */ + u32 clk_div; /* 6C: I2C_I2C_CLOCK_DIVISOR */ }; /* bit fields definitions for IO Packet Header 1 format */ @@ -154,6 +155,11 @@ struct i2c_ctlr { #define I2C_INT_ARBITRATION_LOST_SHIFT 2 #define I2C_INT_ARBITRATION_LOST_MASK (1 << I2C_INT_ARBITRATION_LOST_SHIFT) +/* I2C_CLK_DIVISOR_REGISTER */ +#define CLK_DIV_STD_FAST_MODE 0x19 +#define CLK_DIV_HS_MODE 1 +#define CLK_MULT_STD_FAST_MODE 8 + /** * Returns the bus number of the DVC controller * diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c index efc77fa..0558648 100644 --- a/drivers/i2c/tegra_i2c.c +++ b/drivers/i2c/tegra_i2c.c @@ -88,7 +88,27 @@ static void i2c_init_controller(struct i2c_bus *i2c_bus) * 16 to get the right frequency. */ clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_PERIPH, - i2c_bus->speed * 2 * 8); + i2c_bus->speed * 2 * 8); +#if defined(CONFIG_TEGRA114) + /* + * T114 I2C went to a single clock source for standard/fast and + * HS clock speeds. The new clock rate setting calculation is: + * SCL = CLK_SOURCE.I2C / + * (CLK_MULT_STD_FAST_MODE * (I2C_CLK_DIV_STD_FAST_MODE+1) * + * I2C FREQUENCY DIVISOR) as per the T114 TRM (sec 30.3.1). + * + * NOTE: We do this here, after the initial clock/pll start, + * because if we read the clk_div reg before the controller + * is running, we hang, and we need it for the new calc. + */ + int clk_div_std_fast_mode = readl(&i2c_bus->regs->clk_div) >> 16; + debug("%s: CLK_DIV_STD_FAST_MODE setting = %d\n", __func__, + clk_div_std_fast_mode); + + clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_PERIPH, + CLK_MULT_STD_FAST_MODE * (clk_div_std_fast_mode+1) * + i2c_bus->speed * 2); +#endif /* T114 */ /* Reset I2C controller. */ i2c_reset_controller(i2c_bus); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver 2013-02-06 23:26 ` [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver Tom Warren @ 2013-02-06 23:50 ` Stephen Warren 2013-02-07 14:52 ` Laxman Dewangan 2013-02-08 9:15 ` Laxman Dewangan 2 siblings, 0 replies; 27+ messages in thread From: Stephen Warren @ 2013-02-06 23:50 UTC (permalink / raw) To: u-boot On 02/06/2013 04:26 PM, Tom Warren wrote: > T114 has a slightly different I2C clock, with a new divisor for > standard/fast mode and HS mode. Tested on my Dalmore, and the I2C > clock is 100KHz +/- 3% on my Saleae Logic analyzer. This looks plausible to me, but best to have Laxman review it since he's the I2C expert. I'll forward the whole original message to him for this purpose. At least for me though, Reviewed-by: Stephen Warren <swarren@nvidia.com> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver 2013-02-06 23:26 ` [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver Tom Warren 2013-02-06 23:50 ` Stephen Warren @ 2013-02-07 14:52 ` Laxman Dewangan 2013-02-08 9:15 ` Laxman Dewangan 2 siblings, 0 replies; 27+ messages in thread From: Laxman Dewangan @ 2013-02-07 14:52 UTC (permalink / raw) To: u-boot On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: > T114 has a slightly different I2C clock, with a new divisor for > standard/fast mode and HS mode. Tested on my Dalmore, and the I2C > clock is 100KHz +/- 3% on my Saleae Logic analyzer. > > Signed-off-by: Tom Warren <twarren@nvidia.com> > --- > Changes looks good. Acked-by: Laxman Dewangan<ldewangan@nvidia.com> ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver 2013-02-06 23:26 ` [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver Tom Warren 2013-02-06 23:50 ` Stephen Warren 2013-02-07 14:52 ` Laxman Dewangan @ 2013-02-08 9:15 ` Laxman Dewangan 2013-02-08 16:39 ` Tom Warren 2 siblings, 1 reply; 27+ messages in thread From: Laxman Dewangan @ 2013-02-08 9:15 UTC (permalink / raw) To: u-boot On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: > T114 has a slightly different I2C clock, with a new divisor for > standard/fast mode and HS mode. Tested on my Dalmore, and the I2C > clock is 100KHz +/- 3% on my Saleae Logic analyzer. > > Signed-off-by: Tom Warren <twarren@nvidia.com> > --- > v2: new > */ > clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_PERIPH, > - i2c_bus->speed * 2 * 8); > + i2c_bus->speed * 2 * 8); I think you do not need to multipled by 2 again here. *2 can be remove. I2C clock divder is U16 type. > > + > + clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_PERIPH, > + CLK_MULT_STD_FAST_MODE * (clk_div_std_fast_mode+1) * > + i2c_bus->speed * 2); Same as above, *2 is not required. ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver 2013-02-08 9:15 ` Laxman Dewangan @ 2013-02-08 16:39 ` Tom Warren 0 siblings, 0 replies; 27+ messages in thread From: Tom Warren @ 2013-02-08 16:39 UTC (permalink / raw) To: u-boot Laxman, On Fri, Feb 8, 2013 at 2:15 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote: > On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: >> >> T114 has a slightly different I2C clock, with a new divisor for >> standard/fast mode and HS mode. Tested on my Dalmore, and the I2C >> clock is 100KHz +/- 3% on my Saleae Logic analyzer. >> >> Signed-off-by: Tom Warren <twarren@nvidia.com> >> --- >> v2: new > > >> */ >> clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_PERIPH, >> - i2c_bus->speed * 2 * 8); >> + i2c_bus->speed * 2 * 8); > > > I think you do not need to multipled by 2 again here. *2 can be remove. > I2C clock divder is U16 type. > > > >> > >> + >> + clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_PERIPH, >> + CLK_MULT_STD_FAST_MODE * (clk_div_std_fast_mode+1) * >> + i2c_bus->speed * 2); > > > Same as above, *2 is not required. I measured the I2C clock on J51 (GEN1_I2C) on my Dalmore, and the clock is indeed 100KHz (+/- 3 Hz). Tom > > > > ----------------------------------------------------------------------------------- > This email message is for the sole use of the intended recipient(s) and may > contain > confidential information. Any unauthorized review, use, disclosure or > distribution > is prohibited. If you are not the intended recipient, please contact the > sender by > reply email and destroy all copies of the original message. > ----------------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-06 23:26 [U-Boot] [PATCH v2 0/3] Add I2C driver for T114 Dalmore Tom Warren 2013-02-06 23:26 ` [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver Tom Warren @ 2013-02-06 23:26 ` Tom Warren 2013-02-07 0:00 ` Stephen Warren 2013-02-07 14:57 ` Laxman Dewangan 2013-02-06 23:26 ` [U-Boot] [PATCH v2 3/3] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board Tom Warren 2 siblings, 2 replies; 27+ messages in thread From: Tom Warren @ 2013-02-06 23:26 UTC (permalink / raw) To: u-boot Note that T114 does not have a separate/different DVC (power I2C) controller like T20 - all 5 I2C controllers are identical, but I2C5 is used to designate the controller intended for power control (PWR_I2C in the schematics). Signed-off-by: Tom Warren <twarren@nvidia.com> --- v2: Match dts files with kernel files, remove unused apdma node arch/arm/dts/tegra114.dtsi | 56 +++++++++++++++++++++++++++++++++ board/nvidia/dts/tegra114-dalmore.dts | 33 +++++++++++++++++++ 2 files changed, 89 insertions(+), 0 deletions(-) diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi index d06cd12..7b0c835 100644 --- a/arch/arm/dts/tegra114.dtsi +++ b/arch/arm/dts/tegra114.dtsi @@ -2,4 +2,60 @@ / { compatible = "nvidia,tegra114"; + + tegra_car: clock at 60006000 { + compatible = "nvidia,tegra114-car, nvidia,tegra30-car"; + reg = <0x60006000 0x1000>; + #clock-cells = <1>; + }; + + i2c at 7000c000 { + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; + reg = <0x7000c000 0x100>; + interrupts = <0 38 0x04>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&tegra_car 12>; + status = "disabled"; + }; + + i2c at 7000c400 { + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; + reg = <0x7000c400 0x100>; + interrupts = <0 84 0x04>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&tegra_car 54>; + status = "disabled"; + }; + + i2c at 7000c500 { + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; + reg = <0x7000c500 0x100>; + interrupts = <0 92 0x04>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&tegra_car 67>; + status = "disabled"; + }; + + i2c at 7000c700 { + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; + reg = <0x7000c700 0x100>; + interrupts = <0 120 0x04>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&tegra_car 103>; + status = "disabled"; + }; + + i2c at 7000d000 { + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; + reg = <0x7000d000 0x100>; + interrupts = <0 53 0x04>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&tegra_car 47>; + status = "disabled"; + }; }; diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts index 7315577..13b07f3 100644 --- a/board/nvidia/dts/tegra114-dalmore.dts +++ b/board/nvidia/dts/tegra114-dalmore.dts @@ -6,8 +6,41 @@ model = "NVIDIA Dalmore"; compatible = "nvidia,dalmore", "nvidia,tegra114"; + aliases { + i2c0 = "/i2c at 7000d000"; + i2c1 = "/i2c at 7000c000"; + i2c2 = "/i2c at 7000c400"; + i2c3 = "/i2c at 7000c500"; + i2c4 = "/i2c at 7000c700"; + }; + memory { device_type = "memory"; reg = <0x80000000 0x80000000>; }; + + i2c at 7000c000 { + status = "okay"; + clock-frequency = <100000>; + }; + + i2c at 7000c400 { + status = "okay"; + clock-frequency = <100000>; + }; + + i2c at 7000c500 { + status = "okay"; + clock-frequency = <100000>; + }; + + i2c at 7000c700 { + status = "okay"; + clock-frequency = <100000>; + }; + + i2c at 7000d000 { + status = "okay"; + clock-frequency = <100000>; + }; }; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-06 23:26 ` [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore Tom Warren @ 2013-02-07 0:00 ` Stephen Warren 2013-02-07 16:14 ` Tom Warren 2013-02-07 14:57 ` Laxman Dewangan 1 sibling, 1 reply; 27+ messages in thread From: Stephen Warren @ 2013-02-07 0:00 UTC (permalink / raw) To: u-boot On 02/06/2013 04:26 PM, Tom Warren wrote: > Note that T114 does not have a separate/different DVC (power I2C) > controller like T20 - all 5 I2C controllers are identical, but > I2C5 is used to designate the controller intended for power > control (PWR_I2C in the schematics). > diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi > + tegra_car: clock at 60006000 { > + compatible = "nvidia,tegra114-car, nvidia,tegra30-car"; Only the Tegra114 value should be listed here; the presence of the Tegra30 value in the upstream kernel is a mistake that will be fixed as part of the Tegra114 clock driver patches. > + i2c at 7000c000 { > + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; Same here; only include the Tegra114 value since the HW isn't 100% compatible with the Tegra20 HW. > diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts > + i2c at 7000d000 { > + status = "okay"; > + clock-frequency = <100000>; > + }; According to our downstream Linux kernel, that I2C controller can run up to 400KHz on this board. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 0:00 ` Stephen Warren @ 2013-02-07 16:14 ` Tom Warren 2013-02-07 18:17 ` Stephen Warren 0 siblings, 1 reply; 27+ messages in thread From: Tom Warren @ 2013-02-07 16:14 UTC (permalink / raw) To: u-boot Stephen, On Wed, Feb 6, 2013 at 5:00 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 02/06/2013 04:26 PM, Tom Warren wrote: >> Note that T114 does not have a separate/different DVC (power I2C) >> controller like T20 - all 5 I2C controllers are identical, but >> I2C5 is used to designate the controller intended for power >> control (PWR_I2C in the schematics). > >> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi > >> + tegra_car: clock at 60006000 { >> + compatible = "nvidia,tegra114-car, nvidia,tegra30-car"; > > Only the Tegra114 value should be listed here; the presence of the > Tegra30 value in the upstream kernel is a mistake that will be fixed as > part of the Tegra114 clock driver patches. OK. But this is why I think hewing to the Linux DT files, while laudable, is a time sink. Though since T114 is so new & still settling in, I guess some churn is expected. > >> + i2c at 7000c000 { >> + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; > > Same here; only include the Tegra114 value since the HW isn't 100% > compatible with the Tegra20 HW. What does the 'compatible' designater mean, exactly? Does it require 100% identical HW? Compatible, in a SW/HW sense, to me means that newer SW will work with older/similar (but not exactly the same) HW, etc. Tegra U-Boot uses the "tegra20-i2c" name to find and load the I2C driver. I could add a new entry in the compat-names table for tegra114-i2c, but I still don't think there's enough difference between T20/T30 and T114 I2C to justify a whole new I2C driver - so far, it's just one register with a new clk divider that has to be taken in consideration when programming the clock source divider. I could do as the driver does for T20, where there is a separate DVC controller, plus 3 normal I2C controllers. I could 'find_aliases' for the tegrat114-i2c controller first, process its nodes, and return. If no tegrat114-i2c exists, the driver would continue on to look for tegra20-i2c/tegra20-dvc controller info as it does now. It'd still be one tegra_i2c.c driver, with a flag in the i2c_bus struct telling me if I found T114 I2C HW (i2c_bus->single_clk, etc.) and I could then do the new clock divider dance based on that flag. How does that sound? > >> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts > >> + i2c at 7000d000 { >> + status = "okay"; >> + clock-frequency = <100000>; >> + }; > > According to our downstream Linux kernel, that I2C controller can run up > to 400KHz on this board. > But it also runs @ 100KHz just fine, too. Do we need to run at the fastest clock? That's the DVC (PWR_I2C) controller, which U-Boot does little to nothing with right now. I can set it to 400KHz, but what are the advantages/justifications? Is anything wrong with leaving it at 100KHz? Tom ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 16:14 ` Tom Warren @ 2013-02-07 18:17 ` Stephen Warren 2013-02-12 18:13 ` Simon Glass 0 siblings, 1 reply; 27+ messages in thread From: Stephen Warren @ 2013-02-07 18:17 UTC (permalink / raw) To: u-boot On 02/07/2013 09:14 AM, Tom Warren wrote: > Stephen, > > On Wed, Feb 6, 2013 at 5:00 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 02/06/2013 04:26 PM, Tom Warren wrote: >>> Note that T114 does not have a separate/different DVC (power I2C) >>> controller like T20 - all 5 I2C controllers are identical, but >>> I2C5 is used to designate the controller intended for power >>> control (PWR_I2C in the schematics). >> >>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi >> >>> + tegra_car: clock at 60006000 { >>> + compatible = "nvidia,tegra114-car, nvidia,tegra30-car"; >> >> Only the Tegra114 value should be listed here; the presence of the >> Tegra30 value in the upstream kernel is a mistake that will be fixed as >> part of the Tegra114 clock driver patches. > > OK. But this is why I think hewing to the Linux DT files, while > laudable, is a time sink. Though since T114 is so new & still settling > in, I guess some churn is expected. The issue here is not about making U-Boot fall in line with the kernel. The issue is making the device tree in U-Boot be correct. Right now, the kernel happens to have the most correct device tree, so it's a good reference for U-Boot. >>> + i2c at 7000c000 { >>> + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; >> >> Same here; only include the Tegra114 value since the HW isn't 100% >> compatible with the Tegra20 HW. > > What does the 'compatible' designater mean, exactly? Does it require > 100% identical HW? Compatible, in a SW/HW sense, to me means that > newer SW will work with older/similar (but not exactly the same) HW, > etc. The idea here is that the first entry in compatible always defines the most specific implementation identification possible. So, compatible will at least contain: Tegra20: nvidia,tegra20-i2c Tegra30: nvidia,tegra30-i2c Tegra114: nvidia,tegra114-i2c Now, if a piece of newer hardware can be operated 100% correctly (ignoring issues due to new features not being exposed, or operating at degraded performance) by a driver that only knows about the older hardware, then the compatible property can additionally contain other values indicating what HW it is compatible with. So, we actually end up with: Tegra20: nvidia,tegra20-i2c Tegra30: nvidia,tegra30-i2c nvidia,tegra20-i2c Tegra114: nvidia,tegra114-i2c Tegra114 I2C HW isn't marked as compatible with either Tegra20 or Tegra30, because the clock divider programming must be different. > Tegra U-Boot uses the "tegra20-i2c" name to find and load the I2C > driver. I could add a new entry in the compat-names table for > tegra114-i2c, Yes, that is the way to go. The driver should include a list of all the different compatible values that it supports. > but I still don't think there's enough difference > between T20/T30 and T114 I2C to justify a whole new I2C driver - so > far, it's just one register with a new clk divider that has to be > taken in consideration when programming the clock source divider. But that's required to operate the hardware correctly. It doesn't matter how trivial the difference is; it could just be a single bit that needs to be set/cleared on new HW - there would still be a difference that would otherwise make the HW operate incorrectly. > I could do as the driver does for T20, where there is a separate DVC > controller, plus 3 normal I2C controllers. I could 'find_aliases' for > the tegrat114-i2c controller first, process its nodes, and return. If > no tegrat114-i2c exists, the driver would continue on to look for > tegra20-i2c/tegra20-dvc controller info as it does now. It'd still be > one tegra_i2c.c driver, with a flag in the i2c_bus struct telling me > if I found T114 I2C HW (i2c_bus->single_clk, etc.) and I could then do > the new clock divider dance based on that flag. How does that sound? The U-Boot function that returns the list of devices supported by the driver should be enhanced so that it can search for nodes compatible with any 1 (or more) of n compatible values at a time, rather than just a single compatible value. Then, all you'd have to do with this change is add a new entry into the table, not add extra code that calls that function separately for each compatible value. >>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts >> >>> + i2c at 7000d000 { >>> + status = "okay"; >>> + clock-frequency = <100000>; >>> + }; >> >> According to our downstream Linux kernel, that I2C controller can run up >> to 400KHz on this board. > > But it also runs @ 100KHz just fine, too. Do we need to run at the > fastest clock? That's the DVC (PWR_I2C) controller, which U-Boot does > little to nothing with right now. > > I can set it to 400KHz, but what are the advantages/justifications? Is > anything wrong with leaving it at 100KHz? The device tree is about describing the hardware. The hardware can run at 400KHz, so the device tree should accurately describe this. It's simply a matter of correctness, rather than randomly filling in something that happens to work. One practical advantage here is increased boot speed due to I2C accesses taking less time. The advantage might be small here since we probably don't configure too many regulators in U-Boot, but I bet Simon Glass is counting every uS. And again, if/when we can ever use the same DT for U-Boot and the kernel, this needs to be corrected so the kernel isn't forced to run at a reduced speed for some reason. The kernel sends many many more commands to the PMIC, and many are time-sensitive (e.g. CPU voltage bus adjustments for DVFS). ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 18:17 ` Stephen Warren @ 2013-02-12 18:13 ` Simon Glass 2013-02-12 19:13 ` Tom Warren 0 siblings, 1 reply; 27+ messages in thread From: Simon Glass @ 2013-02-12 18:13 UTC (permalink / raw) To: u-boot Hi Stephen, On Thu, Feb 7, 2013 at 10:17 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 02/07/2013 09:14 AM, Tom Warren wrote: >> Stephen, >> >> On Wed, Feb 6, 2013 at 5:00 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 02/06/2013 04:26 PM, Tom Warren wrote: >>>> Note that T114 does not have a separate/different DVC (power I2C) >>>> controller like T20 - all 5 I2C controllers are identical, but >>>> I2C5 is used to designate the controller intended for power >>>> control (PWR_I2C in the schematics). >>> >>>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi >>> >>>> + tegra_car: clock at 60006000 { >>>> + compatible = "nvidia,tegra114-car, nvidia,tegra30-car"; >>> >>> Only the Tegra114 value should be listed here; the presence of the >>> Tegra30 value in the upstream kernel is a mistake that will be fixed as >>> part of the Tegra114 clock driver patches. >> >> OK. But this is why I think hewing to the Linux DT files, while >> laudable, is a time sink. Though since T114 is so new & still settling >> in, I guess some churn is expected. > > The issue here is not about making U-Boot fall in line with the kernel. > The issue is making the device tree in U-Boot be correct. Right now, the > kernel happens to have the most correct device tree, so it's a good > reference for U-Boot. > >>>> + i2c at 7000c000 { >>>> + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; >>> >>> Same here; only include the Tegra114 value since the HW isn't 100% >>> compatible with the Tegra20 HW. >> >> What does the 'compatible' designater mean, exactly? Does it require >> 100% identical HW? Compatible, in a SW/HW sense, to me means that >> newer SW will work with older/similar (but not exactly the same) HW, >> etc. > > The idea here is that the first entry in compatible always defines the > most specific implementation identification possible. So, compatible > will at least contain: > > Tegra20: nvidia,tegra20-i2c > Tegra30: nvidia,tegra30-i2c > Tegra114: nvidia,tegra114-i2c > > Now, if a piece of newer hardware can be operated 100% correctly > (ignoring issues due to new features not being exposed, or operating at > degraded performance) by a driver that only knows about the older > hardware, then the compatible property can additionally contain other > values indicating what HW it is compatible with. So, we actually end up > with: > > Tegra20: nvidia,tegra20-i2c > Tegra30: nvidia,tegra30-i2c nvidia,tegra20-i2c > Tegra114: nvidia,tegra114-i2c > > Tegra114 I2C HW isn't marked as compatible with either Tegra20 or > Tegra30, because the clock divider programming must be different. > >> Tegra U-Boot uses the "tegra20-i2c" name to find and load the I2C >> driver. I could add a new entry in the compat-names table for >> tegra114-i2c, > > Yes, that is the way to go. The driver should include a list of all the > different compatible values that it supports. > >> but I still don't think there's enough difference >> between T20/T30 and T114 I2C to justify a whole new I2C driver - so >> far, it's just one register with a new clk divider that has to be >> taken in consideration when programming the clock source divider. > > But that's required to operate the hardware correctly. It doesn't matter > how trivial the difference is; it could just be a single bit that needs > to be set/cleared on new HW - there would still be a difference that > would otherwise make the HW operate incorrectly. > >> I could do as the driver does for T20, where there is a separate DVC >> controller, plus 3 normal I2C controllers. I could 'find_aliases' for >> the tegrat114-i2c controller first, process its nodes, and return. If >> no tegrat114-i2c exists, the driver would continue on to look for >> tegra20-i2c/tegra20-dvc controller info as it does now. It'd still be >> one tegra_i2c.c driver, with a flag in the i2c_bus struct telling me >> if I found T114 I2C HW (i2c_bus->single_clk, etc.) and I could then do >> the new clock divider dance based on that flag. How does that sound? > > The U-Boot function that returns the list of devices supported by the > driver should be enhanced so that it can search for nodes compatible > with any 1 (or more) of n compatible values at a time, rather than just > a single compatible value. Then, all you'd have to do with this change > is add a new entry into the table, not add extra code that calls that > function separately for each compatible value. Yes, that needs to be done, and while we are at it I think the function should return a list containing struct {int offset; enum fdt_compat_id; }; I could probably do this by end of week if no one else can do it faster. Please let me know. Tests need to updated also. > >>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts >>> >>>> + i2c at 7000d000 { >>>> + status = "okay"; >>>> + clock-frequency = <100000>; >>>> + }; >>> >>> According to our downstream Linux kernel, that I2C controller can run up >>> to 400KHz on this board. >> >> But it also runs @ 100KHz just fine, too. Do we need to run at the >> fastest clock? That's the DVC (PWR_I2C) controller, which U-Boot does >> little to nothing with right now. >> >> I can set it to 400KHz, but what are the advantages/justifications? Is >> anything wrong with leaving it at 100KHz? > > The device tree is about describing the hardware. The hardware can run > at 400KHz, so the device tree should accurately describe this. It's > simply a matter of correctness, rather than randomly filling in > something that happens to work. > > One practical advantage here is increased boot speed due to I2C accesses > taking less time. The advantage might be small here since we probably > don't configure too many regulators in U-Boot, but I bet Simon Glass is > counting every uS. Well we count uS, but only refactor code for mS :-) > > And again, if/when we can ever use the same DT for U-Boot and the > kernel, this needs to be corrected so the kernel isn't forced to run at > a reduced speed for some reason. The kernel sends many many more > commands to the PMIC, and many are time-sensitive (e.g. CPU voltage bus > adjustments for DVFS). Yes we should use the kernel FDT where possible. > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot Regards, Simon ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-12 18:13 ` Simon Glass @ 2013-02-12 19:13 ` Tom Warren 2013-02-12 19:17 ` Simon Glass 0 siblings, 1 reply; 27+ messages in thread From: Tom Warren @ 2013-02-12 19:13 UTC (permalink / raw) To: u-boot Simon, On Tue, Feb 12, 2013 at 11:13 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Stephen, > > On Thu, Feb 7, 2013 at 10:17 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 02/07/2013 09:14 AM, Tom Warren wrote: >>> Stephen, >>> >>> On Wed, Feb 6, 2013 at 5:00 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>> On 02/06/2013 04:26 PM, Tom Warren wrote: >>>>> Note that T114 does not have a separate/different DVC (power I2C) >>>>> controller like T20 - all 5 I2C controllers are identical, but >>>>> I2C5 is used to designate the controller intended for power >>>>> control (PWR_I2C in the schematics). >>>> >>>>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi >>>> >>>>> + tegra_car: clock at 60006000 { >>>>> + compatible = "nvidia,tegra114-car, nvidia,tegra30-car"; >>>> >>>> Only the Tegra114 value should be listed here; the presence of the >>>> Tegra30 value in the upstream kernel is a mistake that will be fixed as >>>> part of the Tegra114 clock driver patches. >>> >>> OK. But this is why I think hewing to the Linux DT files, while >>> laudable, is a time sink. Though since T114 is so new & still settling >>> in, I guess some churn is expected. >> >> The issue here is not about making U-Boot fall in line with the kernel. >> The issue is making the device tree in U-Boot be correct. Right now, the >> kernel happens to have the most correct device tree, so it's a good >> reference for U-Boot. >> >>>>> + i2c at 7000c000 { >>>>> + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; >>>> >>>> Same here; only include the Tegra114 value since the HW isn't 100% >>>> compatible with the Tegra20 HW. >>> >>> What does the 'compatible' designater mean, exactly? Does it require >>> 100% identical HW? Compatible, in a SW/HW sense, to me means that >>> newer SW will work with older/similar (but not exactly the same) HW, >>> etc. >> >> The idea here is that the first entry in compatible always defines the >> most specific implementation identification possible. So, compatible >> will at least contain: >> >> Tegra20: nvidia,tegra20-i2c >> Tegra30: nvidia,tegra30-i2c >> Tegra114: nvidia,tegra114-i2c >> >> Now, if a piece of newer hardware can be operated 100% correctly >> (ignoring issues due to new features not being exposed, or operating at >> degraded performance) by a driver that only knows about the older >> hardware, then the compatible property can additionally contain other >> values indicating what HW it is compatible with. So, we actually end up >> with: >> >> Tegra20: nvidia,tegra20-i2c >> Tegra30: nvidia,tegra30-i2c nvidia,tegra20-i2c >> Tegra114: nvidia,tegra114-i2c >> >> Tegra114 I2C HW isn't marked as compatible with either Tegra20 or >> Tegra30, because the clock divider programming must be different. >> >>> Tegra U-Boot uses the "tegra20-i2c" name to find and load the I2C >>> driver. I could add a new entry in the compat-names table for >>> tegra114-i2c, >> >> Yes, that is the way to go. The driver should include a list of all the >> different compatible values that it supports. >> >>> but I still don't think there's enough difference >>> between T20/T30 and T114 I2C to justify a whole new I2C driver - so >>> far, it's just one register with a new clk divider that has to be >>> taken in consideration when programming the clock source divider. >> >> But that's required to operate the hardware correctly. It doesn't matter >> how trivial the difference is; it could just be a single bit that needs >> to be set/cleared on new HW - there would still be a difference that >> would otherwise make the HW operate incorrectly. >> >>> I could do as the driver does for T20, where there is a separate DVC >>> controller, plus 3 normal I2C controllers. I could 'find_aliases' for >>> the tegrat114-i2c controller first, process its nodes, and return. If >>> no tegrat114-i2c exists, the driver would continue on to look for >>> tegra20-i2c/tegra20-dvc controller info as it does now. It'd still be >>> one tegra_i2c.c driver, with a flag in the i2c_bus struct telling me >>> if I found T114 I2C HW (i2c_bus->single_clk, etc.) and I could then do >>> the new clock divider dance based on that flag. How does that sound? >> >> The U-Boot function that returns the list of devices supported by the >> driver should be enhanced so that it can search for nodes compatible >> with any 1 (or more) of n compatible values at a time, rather than just >> a single compatible value. Then, all you'd have to do with this change >> is add a new entry into the table, not add extra code that calls that >> function separately for each compatible value. > > Yes, that needs to be done, and while we are at it I think the > function should return a list containing struct {int offset; enum > fdt_compat_id; }; > > I could probably do this by end of week if no one else can do it > faster. Please let me know. Tests need to updated also. I won't have time for this this week (on vacation Friday thru Tuesday). If you want to tackle this, go ahead. The current T114 I2C patchset is good-to-go AFAICT, with the exception of the clock divisor fix Stephen just pointed out in another thread. So you can use that as your basis for rewrite, if you wish. > >> >>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts >>>> >>>>> + i2c at 7000d000 { >>>>> + status = "okay"; >>>>> + clock-frequency = <100000>; >>>>> + }; >>>> >>>> According to our downstream Linux kernel, that I2C controller can run up >>>> to 400KHz on this board. >>> >>> But it also runs @ 100KHz just fine, too. Do we need to run at the >>> fastest clock? That's the DVC (PWR_I2C) controller, which U-Boot does >>> little to nothing with right now. >>> >>> I can set it to 400KHz, but what are the advantages/justifications? Is >>> anything wrong with leaving it at 100KHz? >> >> The device tree is about describing the hardware. The hardware can run >> at 400KHz, so the device tree should accurately describe this. It's >> simply a matter of correctness, rather than randomly filling in >> something that happens to work. >> >> One practical advantage here is increased boot speed due to I2C accesses >> taking less time. The advantage might be small here since we probably >> don't configure too many regulators in U-Boot, but I bet Simon Glass is >> counting every uS. > > Well we count uS, but only refactor code for mS :-) > >> >> And again, if/when we can ever use the same DT for U-Boot and the >> kernel, this needs to be corrected so the kernel isn't forced to run at >> a reduced speed for some reason. The kernel sends many many more >> commands to the PMIC, and many are time-sensitive (e.g. CPU voltage bus >> adjustments for DVFS). > > Yes we should use the kernel FDT where possible. That's the current POR for Tegra DT use in upstream U-Boot, assuming I can find an up-to-date kernel with the latest DTS files (I'll use Stephen's swarren/linux-tegra.git/for-next until told otherwise). Tom > >> _______________________________________________ >> U-Boot mailing list >> U-Boot at lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot > > Regards, > Simon ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-12 19:13 ` Tom Warren @ 2013-02-12 19:17 ` Simon Glass 2013-02-12 19:26 ` Stephen Warren 0 siblings, 1 reply; 27+ messages in thread From: Simon Glass @ 2013-02-12 19:17 UTC (permalink / raw) To: u-boot Hi Tom, On Tue, Feb 12, 2013 at 11:13 AM, Tom Warren <twarren.nvidia@gmail.com> wrote: > Simon, > > On Tue, Feb 12, 2013 at 11:13 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Stephen, >> >> On Thu, Feb 7, 2013 at 10:17 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 02/07/2013 09:14 AM, Tom Warren wrote: >>>> Stephen, >>>> >>>> On Wed, Feb 6, 2013 at 5:00 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>>> On 02/06/2013 04:26 PM, Tom Warren wrote: >>>>>> Note that T114 does not have a separate/different DVC (power I2C) >>>>>> controller like T20 - all 5 I2C controllers are identical, but >>>>>> I2C5 is used to designate the controller intended for power >>>>>> control (PWR_I2C in the schematics). >>>>> >>>>>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi >>>>> >>>>>> + tegra_car: clock at 60006000 { >>>>>> + compatible = "nvidia,tegra114-car, nvidia,tegra30-car"; >>>>> >>>>> Only the Tegra114 value should be listed here; the presence of the >>>>> Tegra30 value in the upstream kernel is a mistake that will be fixed as >>>>> part of the Tegra114 clock driver patches. >>>> >>>> OK. But this is why I think hewing to the Linux DT files, while >>>> laudable, is a time sink. Though since T114 is so new & still settling >>>> in, I guess some churn is expected. >>> >>> The issue here is not about making U-Boot fall in line with the kernel. >>> The issue is making the device tree in U-Boot be correct. Right now, the >>> kernel happens to have the most correct device tree, so it's a good >>> reference for U-Boot. >>> >>>>>> + i2c at 7000c000 { >>>>>> + compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c"; >>>>> >>>>> Same here; only include the Tegra114 value since the HW isn't 100% >>>>> compatible with the Tegra20 HW. >>>> >>>> What does the 'compatible' designater mean, exactly? Does it require >>>> 100% identical HW? Compatible, in a SW/HW sense, to me means that >>>> newer SW will work with older/similar (but not exactly the same) HW, >>>> etc. >>> >>> The idea here is that the first entry in compatible always defines the >>> most specific implementation identification possible. So, compatible >>> will at least contain: >>> >>> Tegra20: nvidia,tegra20-i2c >>> Tegra30: nvidia,tegra30-i2c >>> Tegra114: nvidia,tegra114-i2c >>> >>> Now, if a piece of newer hardware can be operated 100% correctly >>> (ignoring issues due to new features not being exposed, or operating at >>> degraded performance) by a driver that only knows about the older >>> hardware, then the compatible property can additionally contain other >>> values indicating what HW it is compatible with. So, we actually end up >>> with: >>> >>> Tegra20: nvidia,tegra20-i2c >>> Tegra30: nvidia,tegra30-i2c nvidia,tegra20-i2c >>> Tegra114: nvidia,tegra114-i2c >>> >>> Tegra114 I2C HW isn't marked as compatible with either Tegra20 or >>> Tegra30, because the clock divider programming must be different. >>> >>>> Tegra U-Boot uses the "tegra20-i2c" name to find and load the I2C >>>> driver. I could add a new entry in the compat-names table for >>>> tegra114-i2c, >>> >>> Yes, that is the way to go. The driver should include a list of all the >>> different compatible values that it supports. >>> >>>> but I still don't think there's enough difference >>>> between T20/T30 and T114 I2C to justify a whole new I2C driver - so >>>> far, it's just one register with a new clk divider that has to be >>>> taken in consideration when programming the clock source divider. >>> >>> But that's required to operate the hardware correctly. It doesn't matter >>> how trivial the difference is; it could just be a single bit that needs >>> to be set/cleared on new HW - there would still be a difference that >>> would otherwise make the HW operate incorrectly. >>> >>>> I could do as the driver does for T20, where there is a separate DVC >>>> controller, plus 3 normal I2C controllers. I could 'find_aliases' for >>>> the tegrat114-i2c controller first, process its nodes, and return. If >>>> no tegrat114-i2c exists, the driver would continue on to look for >>>> tegra20-i2c/tegra20-dvc controller info as it does now. It'd still be >>>> one tegra_i2c.c driver, with a flag in the i2c_bus struct telling me >>>> if I found T114 I2C HW (i2c_bus->single_clk, etc.) and I could then do >>>> the new clock divider dance based on that flag. How does that sound? >>> >>> The U-Boot function that returns the list of devices supported by the >>> driver should be enhanced so that it can search for nodes compatible >>> with any 1 (or more) of n compatible values at a time, rather than just >>> a single compatible value. Then, all you'd have to do with this change >>> is add a new entry into the table, not add extra code that calls that >>> function separately for each compatible value. >> >> Yes, that needs to be done, and while we are at it I think the >> function should return a list containing struct {int offset; enum >> fdt_compat_id; }; >> >> I could probably do this by end of week if no one else can do it >> faster. Please let me know. Tests need to updated also. > > I won't have time for this this week (on vacation Friday thru > Tuesday). If you want to tackle this, go ahead. The current T114 I2C > patchset is good-to-go AFAICT, with the exception of the clock divisor > fix Stephen just pointed out in another thread. So you can use that as > your basis for rewrite, if you wish. OK, well it is independent of these patches, but seem find as they are anyway. It's something we should do but doesn' t need to hold up proceedings. > >> >>> >>>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts >>>>> >>>>>> + i2c at 7000d000 { >>>>>> + status = "okay"; >>>>>> + clock-frequency = <100000>; >>>>>> + }; >>>>> >>>>> According to our downstream Linux kernel, that I2C controller can run up >>>>> to 400KHz on this board. >>>> >>>> But it also runs @ 100KHz just fine, too. Do we need to run at the >>>> fastest clock? That's the DVC (PWR_I2C) controller, which U-Boot does >>>> little to nothing with right now. >>>> >>>> I can set it to 400KHz, but what are the advantages/justifications? Is >>>> anything wrong with leaving it at 100KHz? >>> >>> The device tree is about describing the hardware. The hardware can run >>> at 400KHz, so the device tree should accurately describe this. It's >>> simply a matter of correctness, rather than randomly filling in >>> something that happens to work. >>> >>> One practical advantage here is increased boot speed due to I2C accesses >>> taking less time. The advantage might be small here since we probably >>> don't configure too many regulators in U-Boot, but I bet Simon Glass is >>> counting every uS. >> >> Well we count uS, but only refactor code for mS :-) >> >>> >>> And again, if/when we can ever use the same DT for U-Boot and the >>> kernel, this needs to be corrected so the kernel isn't forced to run at >>> a reduced speed for some reason. The kernel sends many many more >>> commands to the PMIC, and many are time-sensitive (e.g. CPU voltage bus >>> adjustments for DVFS). >> >> Yes we should use the kernel FDT where possible. > That's the current POR for Tegra DT use in upstream U-Boot, assuming I > can find an up-to-date kernel with the latest DTS files (I'll use > Stephen's swarren/linux-tegra.git/for-next until told otherwise). Yes that was always my problem - finding what the kernel actually used, might use, etc. Regards, Simon > > Tom >> >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot at lists.denx.de >>> http://lists.denx.de/mailman/listinfo/u-boot >> >> Regards, >> Simon ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-12 19:17 ` Simon Glass @ 2013-02-12 19:26 ` Stephen Warren 2013-02-12 19:32 ` Simon Glass 2013-02-12 19:33 ` Tom Warren 0 siblings, 2 replies; 27+ messages in thread From: Stephen Warren @ 2013-02-12 19:26 UTC (permalink / raw) To: u-boot On 02/12/2013 12:17 PM, Simon Glass wrote: > Hi Tom, > > On Tue, Feb 12, 2013 at 11:13 AM, Tom Warren <twarren.nvidia@gmail.com> wrote: ... >> That's the current POR for Tegra DT use in upstream U-Boot, assuming I >> can find an up-to-date kernel with the latest DTS files (I'll use >> Stephen's swarren/linux-tegra.git/for-next until told otherwise). > > Yes that was always my problem - finding what the kernel actually > used, might use, etc. I must say I find this quite puzzling; the kernel repos aren't exactly hidden. Tegra's kernel for-next is likely the best place right now as any DT changes typically go through that tree. However, on the off-chance any other maintainer picks up any changes, linux-next.git would have the latest bindings. That's all been true for a year or more. That said, there is a move to either move the binding definitions and .dts files out of the kernel tree and/or stop taking changes to them through individual SoC trees, but perhaps through the device tree repo. If that does happen, I'll let you know. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-12 19:26 ` Stephen Warren @ 2013-02-12 19:32 ` Simon Glass 2013-02-12 19:33 ` Tom Warren 1 sibling, 0 replies; 27+ messages in thread From: Simon Glass @ 2013-02-12 19:32 UTC (permalink / raw) To: u-boot Hi Stephen, On Tue, Feb 12, 2013 at 11:26 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 02/12/2013 12:17 PM, Simon Glass wrote: >> Hi Tom, >> >> On Tue, Feb 12, 2013 at 11:13 AM, Tom Warren <twarren.nvidia@gmail.com> wrote: > ... >>> That's the current POR for Tegra DT use in upstream U-Boot, assuming I >>> can find an up-to-date kernel with the latest DTS files (I'll use >>> Stephen's swarren/linux-tegra.git/for-next until told otherwise). >> >> Yes that was always my problem - finding what the kernel actually >> used, might use, etc. > > I must say I find this quite puzzling; the kernel repos aren't exactly > hidden. Well if a change has made it to a repo then things are easier, particularly if it is next/. It was chasing down patches on mailing lists that I found hard. > > Tegra's kernel for-next is likely the best place right now as any DT > changes typically go through that tree. However, on the off-chance any > other maintainer picks up any changes, linux-next.git would have the > latest bindings. That's all been true for a year or more. > > That said, there is a move to either move the binding definitions and > .dts files out of the kernel tree and/or stop taking changes to them > through individual SoC trees, but perhaps through the device tree repo. > If that does happen, I'll let you know. Sounds good. Regards, Simon ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-12 19:26 ` Stephen Warren 2013-02-12 19:32 ` Simon Glass @ 2013-02-12 19:33 ` Tom Warren 1 sibling, 0 replies; 27+ messages in thread From: Tom Warren @ 2013-02-12 19:33 UTC (permalink / raw) To: u-boot On Tue, Feb 12, 2013 at 12:26 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 02/12/2013 12:17 PM, Simon Glass wrote: >> Hi Tom, >> >> On Tue, Feb 12, 2013 at 11:13 AM, Tom Warren <twarren.nvidia@gmail.com> wrote: > ... >>> That's the current POR for Tegra DT use in upstream U-Boot, assuming I >>> can find an up-to-date kernel with the latest DTS files (I'll use >>> Stephen's swarren/linux-tegra.git/for-next until told otherwise). >> >> Yes that was always my problem - finding what the kernel actually >> used, might use, etc. > > I must say I find this quite puzzling; the kernel repos aren't exactly > hidden. Not hidden, but numerous. Perhaps I should have said 'can find _the correct_ kernel ...'. I've been pointed to 2 or 3 different repos over the course of these DT patches. I'm not a kernel guru, and I don't have the bandwidth to monitor kernel reflectors, etc. When I need something kernel-related, I resort to Google or asking you. > > Tegra's kernel for-next is likely the best place right now as any DT > changes typically go through that tree. However, on the off-chance any > other maintainer picks up any changes, linux-next.git would have the > latest bindings. That's all been true for a year or more. > > That said, there is a move to either move the binding definitions and > .dts files out of the kernel tree and/or stop taking changes to them > through individual SoC trees, but perhaps through the device tree repo. > If that does happen, I'll let you know. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-06 23:26 ` [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore Tom Warren 2013-02-07 0:00 ` Stephen Warren @ 2013-02-07 14:57 ` Laxman Dewangan 2013-02-07 16:17 ` Tom Warren 1 sibling, 1 reply; 27+ messages in thread From: Laxman Dewangan @ 2013-02-07 14:57 UTC (permalink / raw) To: u-boot On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: > Note that T114 does not have a separate/different DVC (power I2C) > controller like T20 - all 5 I2C controllers are identical, but > I2C5 is used to designate the controller intended for power > control (PWR_I2C in the schematics). > > Signed-off-by: Tom Warren <twarren@nvidia.com> > --- > diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts > index 7315577..13b07f3 100644 > --- a/board/nvidia/dts/tegra114-dalmore.dts > +++ b/board/nvidia/dts/tegra114-dalmore.dts > @@ -6,8 +6,41 @@ > model =NVIDIA Dalmore"; > compatible =nvidia,dalmore", "nvidia,tegra114"; > > + aliases { > + i2c0 =/i2c at 7000d000"; > + i2c1 =/i2c at 7000c000"; > + i2c2 =/i2c at 7000c400"; > + i2c3 =/i2c at 7000c500"; > + i2c4 =/i2c at 7000c700"; > + }; Can we move this to tegar114.dtsi file. otherwise it looks good. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 14:57 ` Laxman Dewangan @ 2013-02-07 16:17 ` Tom Warren 2013-02-07 18:07 ` Stephen Warren 0 siblings, 1 reply; 27+ messages in thread From: Tom Warren @ 2013-02-07 16:17 UTC (permalink / raw) To: u-boot Laxman, On Thu, Feb 7, 2013 at 7:57 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote: > On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: >> >> Note that T114 does not have a separate/different DVC (power I2C) >> controller like T20 - all 5 I2C controllers are identical, but >> I2C5 is used to designate the controller intended for power >> control (PWR_I2C in the schematics). >> >> Signed-off-by: Tom Warren <twarren@nvidia.com> >> --- > > > >> diff --git a/board/nvidia/dts/tegra114-dalmore.dts >> b/board/nvidia/dts/tegra114-dalmore.dts >> index 7315577..13b07f3 100644 >> --- a/board/nvidia/dts/tegra114-dalmore.dts >> +++ b/board/nvidia/dts/tegra114-dalmore.dts >> @@ -6,8 +6,41 @@ >> model =NVIDIA Dalmore"; >> compatible =nvidia,dalmore", "nvidia,tegra114"; >> >> + aliases { >> + i2c0 =/i2c at 7000d000"; >> + i2c1 =/i2c at 7000c000"; >> + i2c2 =/i2c at 7000c400"; >> + i2c3 =/i2c at 7000c500"; >> + i2c4 =/i2c at 7000c700"; >> + }; > > > Can we move this to tegar114.dtsi file. I could, but why? Most, if not all, of the U-Boot boards that use DT are putting their aliases in the .dts file in the board directory. > > otherwise it looks good. > Acked-by: Laxman Dewangan <ldewangan@nvidia.com> Thanks, Laxman. Appreciate your taking a look. Tom > > > ----------------------------------------------------------------------------------- > This email message is for the sole use of the intended recipient(s) and may > contain > confidential information. Any unauthorized review, use, disclosure or > distribution > is prohibited. If you are not the intended recipient, please contact the > sender by > reply email and destroy all copies of the original message. > ----------------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 16:17 ` Tom Warren @ 2013-02-07 18:07 ` Stephen Warren 2013-02-07 18:14 ` Tom Warren 0 siblings, 1 reply; 27+ messages in thread From: Stephen Warren @ 2013-02-07 18:07 UTC (permalink / raw) To: u-boot On 02/07/2013 09:17 AM, Tom Warren wrote: > Laxman, > > On Thu, Feb 7, 2013 at 7:57 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote: >> On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: >>> >>> Note that T114 does not have a separate/different DVC (power I2C) >>> controller like T20 - all 5 I2C controllers are identical, but >>> I2C5 is used to designate the controller intended for power >>> control (PWR_I2C in the schematics). >>> >>> Signed-off-by: Tom Warren <twarren@nvidia.com> >>> --- >> >> >> >>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts >>> b/board/nvidia/dts/tegra114-dalmore.dts >>> index 7315577..13b07f3 100644 >>> --- a/board/nvidia/dts/tegra114-dalmore.dts >>> +++ b/board/nvidia/dts/tegra114-dalmore.dts >>> @@ -6,8 +6,41 @@ >>> model =NVIDIA Dalmore"; >>> compatible =nvidia,dalmore", "nvidia,tegra114"; >>> >>> + aliases { >>> + i2c0 =/i2c at 7000d000"; >>> + i2c1 =/i2c at 7000c000"; >>> + i2c2 =/i2c at 7000c400"; >>> + i2c3 =/i2c at 7000c500"; >>> + i2c4 =/i2c at 7000c700"; >>> + }; >> >> >> Can we move this to tegar114.dtsi file. > > I could, but why? Most, if not all, of the U-Boot boards that use DT > are putting their aliases in the .dts file in the board directory. Laxman, the issue here is that right now in U-Boot, I believe, if a particular board only uses I2C adapters 0, 2, and 4, then only U-Boot device IDs 0, 1, and 2 are defined, rather than IDs 0, 2, and 4. That's why the aliases are in the per-board file for now, because the actual set of I2C adapters enabled is board-specific. Tom, as background for Laxman's request, for other devices (e.g. serial ports), customer engineers have pushed back on that naming scheme, and always want a specific HW device to end up with a static name, irrespective of which other devices of the same type are used, if any. For that reason, in the kernel, we have aliases for the serial ports in tegra*.dtsi rather than in per-board files, and the names are static. So I wonder if in U-Boot we really have to have IDs 0..n rather than e.g. IDs 0, 2, 4 for the I2C ports (when some aren't used). Then, we could just put the aliases in tegra*.dtsi, which makes life simpler when creating board .dts files... ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 18:07 ` Stephen Warren @ 2013-02-07 18:14 ` Tom Warren 2013-02-07 18:20 ` Stephen Warren 0 siblings, 1 reply; 27+ messages in thread From: Tom Warren @ 2013-02-07 18:14 UTC (permalink / raw) To: u-boot Stephen, On Thu, Feb 7, 2013 at 11:07 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 02/07/2013 09:17 AM, Tom Warren wrote: >> Laxman, >> >> On Thu, Feb 7, 2013 at 7:57 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote: >>> On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: >>>> >>>> Note that T114 does not have a separate/different DVC (power I2C) >>>> controller like T20 - all 5 I2C controllers are identical, but >>>> I2C5 is used to designate the controller intended for power >>>> control (PWR_I2C in the schematics). >>>> >>>> Signed-off-by: Tom Warren <twarren@nvidia.com> >>>> --- >>> >>> >>> >>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts >>>> b/board/nvidia/dts/tegra114-dalmore.dts >>>> index 7315577..13b07f3 100644 >>>> --- a/board/nvidia/dts/tegra114-dalmore.dts >>>> +++ b/board/nvidia/dts/tegra114-dalmore.dts >>>> @@ -6,8 +6,41 @@ >>>> model =NVIDIA Dalmore"; >>>> compatible =nvidia,dalmore", "nvidia,tegra114"; >>>> >>>> + aliases { >>>> + i2c0 =/i2c at 7000d000"; >>>> + i2c1 =/i2c at 7000c000"; >>>> + i2c2 =/i2c at 7000c400"; >>>> + i2c3 =/i2c at 7000c500"; >>>> + i2c4 =/i2c at 7000c700"; >>>> + }; >>> >>> >>> Can we move this to tegar114.dtsi file. >> >> I could, but why? Most, if not all, of the U-Boot boards that use DT >> are putting their aliases in the .dts file in the board directory. > > Laxman, the issue here is that right now in U-Boot, I believe, if a > particular board only uses I2C adapters 0, 2, and 4, then only U-Boot > device IDs 0, 1, and 2 are defined, rather than IDs 0, 2, and 4. That's > why the aliases are in the per-board file for now, because the actual > set of I2C adapters enabled is board-specific. > > Tom, as background for Laxman's request, for other devices (e.g. serial > ports), customer engineers have pushed back on that naming scheme, and > always want a specific HW device to end up with a static name, > irrespective of which other devices of the same type are used, if any. > For that reason, in the kernel, we have aliases for the serial ports in > tegra*.dtsi rather than in per-board files, and the names are static. > > So I wonder if in U-Boot we really have to have IDs 0..n rather than > e.g. IDs 0, 2, 4 for the I2C ports (when some aren't used). Then, we > could just put the aliases in tegra*.dtsi, which makes life simpler when > creating board .dts files... Thanks for the background info. I'm focusing on getting T114 I2C in right now, so I can move on to MMC and USB, and what I have seems reasonable/conforms to what's already done in U-Boot. I don't want to get into reworking everyone's dts/dtsi files right now to move aliases around, or find out which boards really have unused I2C ports (Dalmore uses 'em all, BTW, according to our I2C spreadsheet). If you want to send a set of cleanup/re-org patches for everybody's DTS files, or even just the Tegra boards, feel free. I'll be glad to review it/apply it to u-boot-tegra later. Thanks, Tom ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 18:14 ` Tom Warren @ 2013-02-07 18:20 ` Stephen Warren 2013-02-07 19:05 ` Tom Warren 0 siblings, 1 reply; 27+ messages in thread From: Stephen Warren @ 2013-02-07 18:20 UTC (permalink / raw) To: u-boot On 02/07/2013 11:14 AM, Tom Warren wrote: > Stephen, > > On Thu, Feb 7, 2013 at 11:07 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 02/07/2013 09:17 AM, Tom Warren wrote: >>> Laxman, >>> >>> On Thu, Feb 7, 2013 at 7:57 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote: >>>> On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: >>>>> >>>>> Note that T114 does not have a separate/different DVC (power I2C) >>>>> controller like T20 - all 5 I2C controllers are identical, but >>>>> I2C5 is used to designate the controller intended for power >>>>> control (PWR_I2C in the schematics). >>>>> >>>>> Signed-off-by: Tom Warren <twarren@nvidia.com> >>>>> --- >>>> >>>> >>>> >>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts >>>>> b/board/nvidia/dts/tegra114-dalmore.dts >>>>> index 7315577..13b07f3 100644 >>>>> --- a/board/nvidia/dts/tegra114-dalmore.dts >>>>> +++ b/board/nvidia/dts/tegra114-dalmore.dts >>>>> @@ -6,8 +6,41 @@ >>>>> model =NVIDIA Dalmore"; >>>>> compatible =nvidia,dalmore", "nvidia,tegra114"; >>>>> >>>>> + aliases { >>>>> + i2c0 =/i2c at 7000d000"; >>>>> + i2c1 =/i2c at 7000c000"; >>>>> + i2c2 =/i2c at 7000c400"; >>>>> + i2c3 =/i2c at 7000c500"; >>>>> + i2c4 =/i2c at 7000c700"; >>>>> + }; >>>> >>>> >>>> Can we move this to tegar114.dtsi file. >>> >>> I could, but why? Most, if not all, of the U-Boot boards that use DT >>> are putting their aliases in the .dts file in the board directory. >> >> Laxman, the issue here is that right now in U-Boot, I believe, if a >> particular board only uses I2C adapters 0, 2, and 4, then only U-Boot >> device IDs 0, 1, and 2 are defined, rather than IDs 0, 2, and 4. That's >> why the aliases are in the per-board file for now, because the actual >> set of I2C adapters enabled is board-specific. >> >> Tom, as background for Laxman's request, for other devices (e.g. serial >> ports), customer engineers have pushed back on that naming scheme, and >> always want a specific HW device to end up with a static name, >> irrespective of which other devices of the same type are used, if any. >> For that reason, in the kernel, we have aliases for the serial ports in >> tegra*.dtsi rather than in per-board files, and the names are static. >> >> So I wonder if in U-Boot we really have to have IDs 0..n rather than >> e.g. IDs 0, 2, 4 for the I2C ports (when some aren't used). Then, we >> could just put the aliases in tegra*.dtsi, which makes life simpler when >> creating board .dts files... > > Thanks for the background info. I'm focusing on getting T114 I2C in > right now, so I can move on to MMC and USB, and what I have seems > reasonable/conforms to what's already done in U-Boot. That doesn't necessarily make it correct. > I don't want to get into reworking everyone's dts/dtsi files right now > to move aliases around, or find out which boards really have unused > I2C ports (Dalmore uses 'em all, BTW, according to our I2C > spreadsheet). If you want to send a set of cleanup/re-org patches for > everybody's DTS files, or even just the Tegra boards, feel free. I'll > be glad to review it/apply it to u-boot-tegra later. Sorry, but that's simply part of writing the .dts file for a board; there is no way around that. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 18:20 ` Stephen Warren @ 2013-02-07 19:05 ` Tom Warren 2013-02-07 19:08 ` Stephen Warren 0 siblings, 1 reply; 27+ messages in thread From: Tom Warren @ 2013-02-07 19:05 UTC (permalink / raw) To: u-boot Stephen, On Thu, Feb 7, 2013 at 11:20 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 02/07/2013 11:14 AM, Tom Warren wrote: >> Stephen, >> >> On Thu, Feb 7, 2013 at 11:07 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 02/07/2013 09:17 AM, Tom Warren wrote: >>>> Laxman, >>>> >>>> On Thu, Feb 7, 2013 at 7:57 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote: >>>>> On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: >>>>>> >>>>>> Note that T114 does not have a separate/different DVC (power I2C) >>>>>> controller like T20 - all 5 I2C controllers are identical, but >>>>>> I2C5 is used to designate the controller intended for power >>>>>> control (PWR_I2C in the schematics). >>>>>> >>>>>> Signed-off-by: Tom Warren <twarren@nvidia.com> >>>>>> --- >>>>> >>>>> >>>>> >>>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts >>>>>> b/board/nvidia/dts/tegra114-dalmore.dts >>>>>> index 7315577..13b07f3 100644 >>>>>> --- a/board/nvidia/dts/tegra114-dalmore.dts >>>>>> +++ b/board/nvidia/dts/tegra114-dalmore.dts >>>>>> @@ -6,8 +6,41 @@ >>>>>> model =NVIDIA Dalmore"; >>>>>> compatible =nvidia,dalmore", "nvidia,tegra114"; >>>>>> >>>>>> + aliases { >>>>>> + i2c0 =/i2c at 7000d000"; >>>>>> + i2c1 =/i2c at 7000c000"; >>>>>> + i2c2 =/i2c at 7000c400"; >>>>>> + i2c3 =/i2c at 7000c500"; >>>>>> + i2c4 =/i2c at 7000c700"; >>>>>> + }; >>>>> >>>>> >>>>> Can we move this to tegar114.dtsi file. >>>> >>>> I could, but why? Most, if not all, of the U-Boot boards that use DT >>>> are putting their aliases in the .dts file in the board directory. >>> >>> Laxman, the issue here is that right now in U-Boot, I believe, if a >>> particular board only uses I2C adapters 0, 2, and 4, then only U-Boot >>> device IDs 0, 1, and 2 are defined, rather than IDs 0, 2, and 4. That's >>> why the aliases are in the per-board file for now, because the actual >>> set of I2C adapters enabled is board-specific. >>> >>> Tom, as background for Laxman's request, for other devices (e.g. serial >>> ports), customer engineers have pushed back on that naming scheme, and >>> always want a specific HW device to end up with a static name, >>> irrespective of which other devices of the same type are used, if any. >>> For that reason, in the kernel, we have aliases for the serial ports in >>> tegra*.dtsi rather than in per-board files, and the names are static. >>> >>> So I wonder if in U-Boot we really have to have IDs 0..n rather than >>> e.g. IDs 0, 2, 4 for the I2C ports (when some aren't used). Then, we >>> could just put the aliases in tegra*.dtsi, which makes life simpler when >>> creating board .dts files... >> >> Thanks for the background info. I'm focusing on getting T114 I2C in >> right now, so I can move on to MMC and USB, and what I have seems >> reasonable/conforms to what's already done in U-Boot. > > That doesn't necessarily make it correct. > >> I don't want to get into reworking everyone's dts/dtsi files right now >> to move aliases around, or find out which boards really have unused >> I2C ports (Dalmore uses 'em all, BTW, according to our I2C >> spreadsheet). If you want to send a set of cleanup/re-org patches for >> everybody's DTS files, or even just the Tegra boards, feel free. I'll >> be glad to review it/apply it to u-boot-tegra later. > > Sorry, but that's simply part of writing the .dts file for a board; > there is no way around that. OK, how about this. For T114, I'll move the aliases to tegra114.dtsi, and use 400KHz for all I2C nodes except I2C1 (saw some internal code saying that it may not run well at that speed on the E1611 board). Since all 5 controllers have devices behind them, I think it's OK to leave all 5 in the aliases. What do you think of that approach? Adding Yen Lin to CC Tom ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 19:05 ` Tom Warren @ 2013-02-07 19:08 ` Stephen Warren 2013-02-07 19:59 ` Tom Warren 0 siblings, 1 reply; 27+ messages in thread From: Stephen Warren @ 2013-02-07 19:08 UTC (permalink / raw) To: u-boot On 02/07/2013 12:05 PM, Tom Warren wrote: > Stephen, > > On Thu, Feb 7, 2013 at 11:20 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 02/07/2013 11:14 AM, Tom Warren wrote: >>> Stephen, >>> >>> On Thu, Feb 7, 2013 at 11:07 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>> On 02/07/2013 09:17 AM, Tom Warren wrote: >>>>> Laxman, >>>>> >>>>> On Thu, Feb 7, 2013 at 7:57 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote: >>>>>> On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: >>>>>>> >>>>>>> Note that T114 does not have a separate/different DVC (power I2C) >>>>>>> controller like T20 - all 5 I2C controllers are identical, but >>>>>>> I2C5 is used to designate the controller intended for power >>>>>>> control (PWR_I2C in the schematics). >>>>>>> >>>>>>> Signed-off-by: Tom Warren <twarren@nvidia.com> >>>>>>> --- >>>>>> >>>>>> >>>>>> >>>>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts >>>>>>> b/board/nvidia/dts/tegra114-dalmore.dts >>>>>>> index 7315577..13b07f3 100644 >>>>>>> --- a/board/nvidia/dts/tegra114-dalmore.dts >>>>>>> +++ b/board/nvidia/dts/tegra114-dalmore.dts >>>>>>> @@ -6,8 +6,41 @@ >>>>>>> model =NVIDIA Dalmore"; >>>>>>> compatible =nvidia,dalmore", "nvidia,tegra114"; >>>>>>> >>>>>>> + aliases { >>>>>>> + i2c0 =/i2c at 7000d000"; >>>>>>> + i2c1 =/i2c at 7000c000"; >>>>>>> + i2c2 =/i2c at 7000c400"; >>>>>>> + i2c3 =/i2c at 7000c500"; >>>>>>> + i2c4 =/i2c at 7000c700"; >>>>>>> + }; >>>>>> >>>>>> >>>>>> Can we move this to tegar114.dtsi file. >>>>> >>>>> I could, but why? Most, if not all, of the U-Boot boards that use DT >>>>> are putting their aliases in the .dts file in the board directory. >>>> >>>> Laxman, the issue here is that right now in U-Boot, I believe, if a >>>> particular board only uses I2C adapters 0, 2, and 4, then only U-Boot >>>> device IDs 0, 1, and 2 are defined, rather than IDs 0, 2, and 4. That's >>>> why the aliases are in the per-board file for now, because the actual >>>> set of I2C adapters enabled is board-specific. >>>> >>>> Tom, as background for Laxman's request, for other devices (e.g. serial >>>> ports), customer engineers have pushed back on that naming scheme, and >>>> always want a specific HW device to end up with a static name, >>>> irrespective of which other devices of the same type are used, if any. >>>> For that reason, in the kernel, we have aliases for the serial ports in >>>> tegra*.dtsi rather than in per-board files, and the names are static. >>>> >>>> So I wonder if in U-Boot we really have to have IDs 0..n rather than >>>> e.g. IDs 0, 2, 4 for the I2C ports (when some aren't used). Then, we >>>> could just put the aliases in tegra*.dtsi, which makes life simpler when >>>> creating board .dts files... >>> >>> Thanks for the background info. I'm focusing on getting T114 I2C in >>> right now, so I can move on to MMC and USB, and what I have seems >>> reasonable/conforms to what's already done in U-Boot. >> >> That doesn't necessarily make it correct. >> >>> I don't want to get into reworking everyone's dts/dtsi files right now >>> to move aliases around, or find out which boards really have unused >>> I2C ports (Dalmore uses 'em all, BTW, according to our I2C >>> spreadsheet). If you want to send a set of cleanup/re-org patches for >>> everybody's DTS files, or even just the Tegra boards, feel free. I'll >>> be glad to review it/apply it to u-boot-tegra later. >> >> Sorry, but that's simply part of writing the .dts file for a board; >> there is no way around that. > > OK, how about this. For T114, I'll move the aliases to tegra114.dtsi, I believe that makes sense. But please do check that if you do disable one of the controllers, that the other 4 I2C ports do all get registered and still work OK, so we've tested that code-path. I'm not sure if we have tested the case where the I2C channels don't have contiguous IDs yet, so I have no idea if it works. > and use 400KHz for all I2C nodes except I2C1 (saw some internal code > saying that it may not run well at that speed on the E1611 board). > Since all 5 controllers have devices behind them, I think it's OK to > leave all 5 in the aliases. The downstream kernel only has I2C5 (IIRC) set to 400KHz - the other 4 are limited to 100KHz. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore 2013-02-07 19:08 ` Stephen Warren @ 2013-02-07 19:59 ` Tom Warren 0 siblings, 0 replies; 27+ messages in thread From: Tom Warren @ 2013-02-07 19:59 UTC (permalink / raw) To: u-boot Stephen, On Thu, Feb 7, 2013 at 12:08 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 02/07/2013 12:05 PM, Tom Warren wrote: >> Stephen, >> >> On Thu, Feb 7, 2013 at 11:20 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 02/07/2013 11:14 AM, Tom Warren wrote: >>>> Stephen, >>>> >>>> On Thu, Feb 7, 2013 at 11:07 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>>> On 02/07/2013 09:17 AM, Tom Warren wrote: >>>>>> Laxman, >>>>>> >>>>>> On Thu, Feb 7, 2013 at 7:57 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote: >>>>>>> On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: >>>>>>>> >>>>>>>> Note that T114 does not have a separate/different DVC (power I2C) >>>>>>>> controller like T20 - all 5 I2C controllers are identical, but >>>>>>>> I2C5 is used to designate the controller intended for power >>>>>>>> control (PWR_I2C in the schematics). >>>>>>>> >>>>>>>> Signed-off-by: Tom Warren <twarren@nvidia.com> >>>>>>>> --- >>>>>>> >>>>>>> >>>>>>> >>>>>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts >>>>>>>> b/board/nvidia/dts/tegra114-dalmore.dts >>>>>>>> index 7315577..13b07f3 100644 >>>>>>>> --- a/board/nvidia/dts/tegra114-dalmore.dts >>>>>>>> +++ b/board/nvidia/dts/tegra114-dalmore.dts >>>>>>>> @@ -6,8 +6,41 @@ >>>>>>>> model =NVIDIA Dalmore"; >>>>>>>> compatible =nvidia,dalmore", "nvidia,tegra114"; >>>>>>>> >>>>>>>> + aliases { >>>>>>>> + i2c0 =/i2c at 7000d000"; >>>>>>>> + i2c1 =/i2c at 7000c000"; >>>>>>>> + i2c2 =/i2c at 7000c400"; >>>>>>>> + i2c3 =/i2c at 7000c500"; >>>>>>>> + i2c4 =/i2c at 7000c700"; >>>>>>>> + }; >>>>>>> >>>>>>> >>>>>>> Can we move this to tegar114.dtsi file. >>>>>> >>>>>> I could, but why? Most, if not all, of the U-Boot boards that use DT >>>>>> are putting their aliases in the .dts file in the board directory. >>>>> >>>>> Laxman, the issue here is that right now in U-Boot, I believe, if a >>>>> particular board only uses I2C adapters 0, 2, and 4, then only U-Boot >>>>> device IDs 0, 1, and 2 are defined, rather than IDs 0, 2, and 4. That's >>>>> why the aliases are in the per-board file for now, because the actual >>>>> set of I2C adapters enabled is board-specific. >>>>> >>>>> Tom, as background for Laxman's request, for other devices (e.g. serial >>>>> ports), customer engineers have pushed back on that naming scheme, and >>>>> always want a specific HW device to end up with a static name, >>>>> irrespective of which other devices of the same type are used, if any. >>>>> For that reason, in the kernel, we have aliases for the serial ports in >>>>> tegra*.dtsi rather than in per-board files, and the names are static. >>>>> >>>>> So I wonder if in U-Boot we really have to have IDs 0..n rather than >>>>> e.g. IDs 0, 2, 4 for the I2C ports (when some aren't used). Then, we >>>>> could just put the aliases in tegra*.dtsi, which makes life simpler when >>>>> creating board .dts files... >>>> >>>> Thanks for the background info. I'm focusing on getting T114 I2C in >>>> right now, so I can move on to MMC and USB, and what I have seems >>>> reasonable/conforms to what's already done in U-Boot. >>> >>> That doesn't necessarily make it correct. >>> >>>> I don't want to get into reworking everyone's dts/dtsi files right now >>>> to move aliases around, or find out which boards really have unused >>>> I2C ports (Dalmore uses 'em all, BTW, according to our I2C >>>> spreadsheet). If you want to send a set of cleanup/re-org patches for >>>> everybody's DTS files, or even just the Tegra boards, feel free. I'll >>>> be glad to review it/apply it to u-boot-tegra later. >>> >>> Sorry, but that's simply part of writing the .dts file for a board; >>> there is no way around that. >> >> OK, how about this. For T114, I'll move the aliases to tegra114.dtsi, > > I believe that makes sense. > > But please do check that if you do disable one of the controllers, that > the other 4 I2C ports do all get registered and still work OK, so we've > tested that code-path. I'm not sure if we have tested the case where the > I2C channels don't have contiguous IDs yet, so I have no idea if it works. Yep, if I remove the 'status = okay' from one of the ports (meaning that the .dtsi 'status=disabled' takes over), it can't be selected in the 'i2c' command @ the cmd prompt. Even if I take out dev 2, for instance, I can still select/probe devs 0, 1, 3, and 4. > >> and use 400KHz for all I2C nodes except I2C1 (saw some internal code >> saying that it may not run well at that speed on the E1611 board). >> Since all 5 controllers have devices behind them, I think it's OK to >> leave all 5 in the aliases. > > The downstream kernel only has I2C5 (IIRC) set to 400KHz - the other 4 > are limited to 100KHz. OK, I'll update just I2C5/dev 0/PWR_I2C to 400KHz. Thanks ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 3/3] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board 2013-02-06 23:26 [U-Boot] [PATCH v2 0/3] Add I2C driver for T114 Dalmore Tom Warren 2013-02-06 23:26 ` [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver Tom Warren 2013-02-06 23:26 ` [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore Tom Warren @ 2013-02-06 23:26 ` Tom Warren 2013-02-07 0:01 ` Stephen Warren 2013-02-07 14:58 ` Laxman Dewangan 2 siblings, 2 replies; 27+ messages in thread From: Tom Warren @ 2013-02-06 23:26 UTC (permalink / raw) To: u-boot Tested all 5 'buses', i2c probe enumerates device addresses on bus 1 and 2. Signed-off-by: Tom Warren <twarren@nvidia.com> --- v2: No change include/configs/dalmore.h | 9 +++++++++ include/configs/tegra114-common.h | 3 +++ 2 files changed, 12 insertions(+), 0 deletions(-) diff --git a/include/configs/dalmore.h b/include/configs/dalmore.h index ce32c80..b1a6e34 100644 --- a/include/configs/dalmore.h +++ b/include/configs/dalmore.h @@ -41,6 +41,15 @@ #define CONFIG_MACH_TYPE MACH_TYPE_DALMORE #define CONFIG_BOARD_EARLY_INIT_F + +/* I2C */ +#define CONFIG_TEGRA_I2C +#define CONFIG_SYS_I2C_INIT_BOARD +#define CONFIG_I2C_MULTI_BUS +#define CONFIG_SYS_MAX_I2C_BUS TEGRA_I2C_NUM_CONTROLLERS +#define CONFIG_SYS_I2C_SPEED 100000 +#define CONFIG_CMD_I2C + #define CONFIG_ENV_IS_NOWHERE #define MACH_TYPE_DALMORE 4304 /* not yet in mach-types.h */ diff --git a/include/configs/tegra114-common.h b/include/configs/tegra114-common.h index 0033530..c2986d8 100644 --- a/include/configs/tegra114-common.h +++ b/include/configs/tegra114-common.h @@ -76,4 +76,7 @@ #define CONFIG_SPL_LDSCRIPT "$(CPUDIR)/tegra114/u-boot-spl.lds" +/* Total I2C ports on Tegra114 */ +#define TEGRA_I2C_NUM_CONTROLLERS 5 + #endif /* _TEGRA114_COMMON_H_ */ -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 3/3] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board 2013-02-06 23:26 ` [U-Boot] [PATCH v2 3/3] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board Tom Warren @ 2013-02-07 0:01 ` Stephen Warren 2013-02-07 14:58 ` Laxman Dewangan 1 sibling, 0 replies; 27+ messages in thread From: Stephen Warren @ 2013-02-07 0:01 UTC (permalink / raw) To: u-boot On 02/06/2013 04:26 PM, Tom Warren wrote: > Tested all 5 'buses', i2c probe enumerates device addresses on bus > 1 and 2. This patch, Reviewed-by: Stephen Warren <swarren@nvidia.com> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH v2 3/3] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board 2013-02-06 23:26 ` [U-Boot] [PATCH v2 3/3] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board Tom Warren 2013-02-07 0:01 ` Stephen Warren @ 2013-02-07 14:58 ` Laxman Dewangan 1 sibling, 0 replies; 27+ messages in thread From: Laxman Dewangan @ 2013-02-07 14:58 UTC (permalink / raw) To: u-boot On Thursday 07 February 2013 04:56 AM, Tom Warren wrote: > Tested all 5 'buses', i2c probe enumerates device addresses on bus > 1 and 2. > > Signed-off-by: Tom Warren <twarren@nvidia.com> > --- > v2: No change > Looks good. Acked-by: Laxman Dewangan<ldewangan@nvidia.com> ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2013-02-12 19:33 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-02-06 23:26 [U-Boot] [PATCH v2 0/3] Add I2C driver for T114 Dalmore Tom Warren 2013-02-06 23:26 ` [U-Boot] [PATCH v2 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver Tom Warren 2013-02-06 23:50 ` Stephen Warren 2013-02-07 14:52 ` Laxman Dewangan 2013-02-08 9:15 ` Laxman Dewangan 2013-02-08 16:39 ` Tom Warren 2013-02-06 23:26 ` [U-Boot] [PATCH v2 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore Tom Warren 2013-02-07 0:00 ` Stephen Warren 2013-02-07 16:14 ` Tom Warren 2013-02-07 18:17 ` Stephen Warren 2013-02-12 18:13 ` Simon Glass 2013-02-12 19:13 ` Tom Warren 2013-02-12 19:17 ` Simon Glass 2013-02-12 19:26 ` Stephen Warren 2013-02-12 19:32 ` Simon Glass 2013-02-12 19:33 ` Tom Warren 2013-02-07 14:57 ` Laxman Dewangan 2013-02-07 16:17 ` Tom Warren 2013-02-07 18:07 ` Stephen Warren 2013-02-07 18:14 ` Tom Warren 2013-02-07 18:20 ` Stephen Warren 2013-02-07 19:05 ` Tom Warren 2013-02-07 19:08 ` Stephen Warren 2013-02-07 19:59 ` Tom Warren 2013-02-06 23:26 ` [U-Boot] [PATCH v2 3/3] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board Tom Warren 2013-02-07 0:01 ` Stephen Warren 2013-02-07 14:58 ` Laxman Dewangan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.