[U-Boot] [PATCH 0/2] Add I2C driver for Tegra114 Dalmore

[U-Boot] [PATCH 0/2] Add I2C driver for Tegra114 Dalmore

[Tom Warren]

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.

Tom Warren (2):
  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            |   93 +++++++++++++++++++++++++++++++++
 board/nvidia/dts/tegra114-dalmore.dts |   33 ++++++++++++
 include/configs/dalmore.h             |    9 +++
 include/configs/tegra114-common.h     |    3 +
 4 files changed, 138 insertions(+), 0 deletions(-)

[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Tom Warren]

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>
---
 arch/arm/dts/tegra114.dtsi            |   93 +++++++++++++++++++++++++++++++++
 board/nvidia/dts/tegra114-dalmore.dts |   33 ++++++++++++
 2 files changed, 126 insertions(+), 0 deletions(-)

diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
index d06cd12..76fc83b 100644
--- a/arch/arm/dts/tegra114.dtsi
+++ b/arch/arm/dts/tegra114.dtsi
@@ -2,4 +2,97 @@
 
 / {
 	compatible = "nvidia,tegra114";
+
+	tegra_car: clock at 60006000 {
+		compatible = "nvidia,tegra114-car", "nvidia,tegra20-car";
+		reg = <0x60006000 0x1000>;
+		#clock-cells = <1>;
+	};
+
+	apbdma: dma {
+		compatible = "nvidia,tegra114-apbdma", "nvidia,tegra20-apbdma";
+		reg = <0x6000a000 0x1400>;
+		interrupts = <0 104 0x04
+			      0 105 0x04
+			      0 106 0x04
+			      0 107 0x04
+			      0 108 0x04
+			      0 109 0x04
+			      0 110 0x04
+			      0 111 0x04
+			      0 112 0x04
+			      0 113 0x04
+			      0 114 0x04
+			      0 115 0x04
+			      0 116 0x04
+			      0 117 0x04
+			      0 118 0x04
+			      0 119 0x04
+			      0 128 0x04
+			      0 129 0x04
+			      0 130 0x04
+			      0 131 0x04
+			      0 132 0x04
+			      0 133 0x04
+			      0 134 0x04
+			      0 135 0x04
+			      0 136 0x04
+			      0 137 0x04
+			      0 138 0x04
+			      0 139 0x04
+			      0 140 0x04
+			      0 141 0x04
+			      0 142 0x04
+			      0 143 0x04>;
+	};
+
+	i2c at 7000c000 {
+		compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
+		reg = <0x7000c000 0x100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+		/* PERIPH_ID_I2C1 */
+		clocks = <&tegra_car 12>;
+	};
+
+	i2c at 7000c400 {
+		compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
+		reg = <0x7000c400 0x100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+		/* PERIPH_ID_I2C2 */
+		clocks = <&tegra_car 54>;
+	};
+
+	i2c at 7000c500 {
+		compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
+		reg = <0x7000c500 0x100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+		/* PERIPH_ID_I2C3 */
+		clocks = <&tegra_car 67>;
+	};
+
+	i2c at 7000c700 {
+		compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
+		reg = <0x7000c700 0x100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+		/* PERIPH_ID_I2C4 */
+		clocks = <&tegra_car 103>;
+	};
+
+	i2c at 7000d000 {
+		compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
+		reg = <0x7000d000 0x100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+		/* PERIPH_ID_I2C5 */
+		clocks = <&tegra_car 47>;
+	};
 };
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

[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Stephen Warren]

On 01/29/2013 12:19 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).
> 
> Signed-off-by: Tom Warren <twarren@nvidia.com>
> ---
>  arch/arm/dts/tegra114.dtsi            |   93 +++++++++++++++++++++++++++++++++
>  board/nvidia/dts/tegra114-dalmore.dts |   33 ++++++++++++
>  2 files changed, 126 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
> index d06cd12..76fc83b 100644
> --- a/arch/arm/dts/tegra114.dtsi
> +++ b/arch/arm/dts/tegra114.dtsi
> @@ -2,4 +2,97 @@
>  
>  / {
>  	compatible = "nvidia,tegra114";
> +
> +	tegra_car: clock at 60006000 {
> +		compatible = "nvidia,tegra114-car", "nvidia,tegra20-car";

I don't believe the Tegra114 CAR is backwards-compatible enough with
Tegra20 to include the Tegra20 entry in compatible. In other words, just:

	compatible = "nvidia,tegra114-car";

> +		reg = <0x60006000 0x1000>;
> +		#clock-cells = <1>;
> +	};

> +	apbdma: dma {
> +		compatible = "nvidia,tegra114-apbdma", "nvidia,tegra20-apbdma";

Same here; it isn't fully backwards-compatible according to Laxman's
kernel patches.

I don't think you need the APB DMA node for I2C though; perhaps just
drop it.

If you do keep this node, it needs the clocks property filled in.

> +	i2c at 7000c000 {
> +		compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";

The I2C nodes also aren't backwards-compatible.

> +		reg = <0x7000c000 0x100>;

The interrupts property is missing here.

> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts

Board changes would usually be a separate patch to an SoC change, but I
guess not a big deal.

All 5 I2C ports are used on the board and only support 100KHz? That
seems unusual, but I suppose it's possible.

[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Tom Warren]

Stephen,

On Tue, Jan 29, 2013 at 1:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 01/29/2013 12:19 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).
>>
>> Signed-off-by: Tom Warren <twarren@nvidia.com>
>> ---
>>  arch/arm/dts/tegra114.dtsi            |   93 +++++++++++++++++++++++++++++++++
>>  board/nvidia/dts/tegra114-dalmore.dts |   33 ++++++++++++
>>  2 files changed, 126 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
>> index d06cd12..76fc83b 100644
>> --- a/arch/arm/dts/tegra114.dtsi
>> +++ b/arch/arm/dts/tegra114.dtsi
>> @@ -2,4 +2,97 @@
>>
>>  / {
>>       compatible = "nvidia,tegra114";
>> +
>> +     tegra_car: clock at 60006000 {
>> +             compatible = "nvidia,tegra114-car", "nvidia,tegra20-car";
>
> I don't believe the Tegra114 CAR is backwards-compatible enough with
> Tegra20 to include the Tegra20 entry in compatible. In other words, just:
>
>         compatible = "nvidia,tegra114-car";

Will do.

>
>> +             reg = <0x60006000 0x1000>;
>> +             #clock-cells = <1>;
>> +     };
>
>> +     apbdma: dma {
>> +             compatible = "nvidia,tegra114-apbdma", "nvidia,tegra20-apbdma";
>
> Same here; it isn't fully backwards-compatible according to Laxman's
> kernel patches.

OK.

>
> I don't think you need the APB DMA node for I2C though; perhaps just
> drop it.

It exists in T20 and T30 (via Allen's apdma patch), so I brought it
forward. I can drop it if you feel strongly enough about it. Whoever
needs it (SPI?) will have to add it later.

>
> If you do keep this node, it needs the clocks property filled in.
>
>> +     i2c at 7000c000 {
>> +             compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
>
> The I2C nodes also aren't backwards-compatible.

They were on T30 (same compatible string there, except of course
s/114/30/). And the tegra20-i2c is needed for fdtdec.c to find it
(compat_names table).

>
>> +             reg = <0x7000c000 0x100>;
>
> The interrupts property is missing here.

I didn't have an interrupts property for I2C on the T30 dts files,
either. What is it used for in U-Boot? If it's really needed, can I
just crib it from the kernel dts file(s)?

>
>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts
>
> Board changes would usually be a separate patch to an SoC change, but I
> guess not a big deal.

Actually, this is the way I've always done DT files (in a separate
patch), and then the config files in a separate patch the enable the
feature.

>
> All 5 I2C ports are used on the board and only support 100KHz? That
> seems unusual, but I suppose it's possible.

That's the way I have it on T30, and looking at
arch/arm/boot/dts/tegra30.dtsi in the kernel, all 5 ports are set to
100KHz.

Tom

[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Stephen Warren]

On 01/29/2013 01:40 PM, Tom Warren wrote:
> Stephen,
> 
> On Tue, Jan 29, 2013 at 1:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 01/29/2013 12:19 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).

>> If you do keep this node, it needs the clocks property filled in.
>>
>>> +     i2c at 7000c000 {
>>> +             compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
>>
>> The I2C nodes also aren't backwards-compatible.
> 
> They were on T30 (same compatible string there, except of course
> s/114/30/). And the tegra20-i2c is needed for fdtdec.c to find it
> (compat_names table).

Yes, I believe the HW changes between Tegra20 and Tegra30 were such that
the Tegra20 driver would run unmodified on Tegra30 without issue. I
don't believe that's the case for Tegra114 though. The solution would be
to add the Tegra114 compatible value to the I2C driver so that it can
search for both.

>>> +             reg = <0x7000c000 0x100>;
>>
>> The interrupts property is missing here.
> 
> I didn't have an interrupts property for I2C on the T30 dts files,
> either.

That sounds like a bug in the DT.

> What is it used for in U-Boot?

U-Boot doesn't use interrupts at least on Tegra, so it's not used.
However, it'd be best to keep the U-Boot DT in line with the kernel DT
so we can easily spot differences. There has been talk of trying to use
the same DT for both too, although we're a long long way off from that.

> If it's really needed, can I
> just crib it from the kernel dts file(s)?

Yes, but the Tegra114 patches aren't all checked in yet. See the
following proposed patch for what you need:

https://lkml.org/lkml/2013/1/29/235

(note that patch is missing the clocks properties since the clock
binding isn't finalized and checked in yet)

>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts
>>
>> Board changes would usually be a separate patch to an SoC change, but I
>> guess not a big deal.
> 
> Actually, this is the way I've always done DT files (in a separate
> patch), and then the config files in a separate patch the enable the
> feature.

I meant that tegra114.dtsi and tegra114-dalmore.dts would usually be
separate patches.

>> All 5 I2C ports are used on the board and only support 100KHz? That
>> seems unusual, but I suppose it's possible.
> 
> That's the way I have it on T30, and looking at
> arch/arm/boot/dts/tegra30.dtsi in the kernel, all 5 ports are set to
> 100KHz.

But this is board-specific; it depends on which of the I2C controllers
are actually pinmux'd out onto the board (-> which to enable), and which
devices are attached to those buses (-> max I2C clock rate). I don't see
how Tegra30 nor tegra30.dtsi are relevant here.

[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Tom Warren]

Stephen,

On Tue, Jan 29, 2013 at 2:08 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 01/29/2013 01:40 PM, Tom Warren wrote:
>> Stephen,
>>
>> On Tue, Jan 29, 2013 at 1:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 01/29/2013 12:19 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).
>
>>> If you do keep this node, it needs the clocks property filled in.
>>>
>>>> +     i2c at 7000c000 {
>>>> +             compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
>>>
>>> The I2C nodes also aren't backwards-compatible.
>>
>> They were on T30 (same compatible string there, except of course
>> s/114/30/). And the tegra20-i2c is needed for fdtdec.c to find it
>> (compat_names table).
>
> Yes, I believe the HW changes between Tegra20 and Tegra30 were such that
> the Tegra20 driver would run unmodified on Tegra30 without issue. I
> don't believe that's the case for Tegra114 though. The solution would be
> to add the Tegra114 compatible value to the I2C driver so that it can
> search for both.

Looking at the TRMs for both T30 and T114, I2C looks nearly identical,
except for some additional registers tacked onto the end for bus clear
support. 99% of the bits appear exactly the same, too (with the
addition of a bus clear int bit on T114 in an used bit position).  I
know next to nothing about I2C, but it appears to me that the current
tegra_I2C.c driver should work fine on T114 (and does, since I can
probe addresses with it).  I don't see a need for a different driver
or compat value here.

>
>>>> +             reg = <0x7000c000 0x100>;
>>>
>>> The interrupts property is missing here.
>>
>> I didn't have an interrupts property for I2C on the T30 dts files,
>> either.
>
> That sounds like a bug in the DT.
>
>> What is it used for in U-Boot?
>
> U-Boot doesn't use interrupts at least on Tegra, so it's not used.
> However, it'd be best to keep the U-Boot DT in line with the kernel DT
> so we can easily spot differences. There has been talk of trying to use
> the same DT for both too, although we're a long long way off from that.

I'll add it, but it seems silly to add unused values, especially when
your past review critiques of DT files have asked for minimum
functionality, which I agree with.

>
>> If it's really needed, can I
>> just crib it from the kernel dts file(s)?
>
> Yes, but the Tegra114 patches aren't all checked in yet. See the
> following proposed patch for what you need:
>
> https://lkml.org/lkml/2013/1/29/235

The only diff between Laxman's dtsi file in the proposed patch and
mine is the interrupts property. I added it (it's exactly the same as
the T30 values, BTW, so I'll add interrupts to the T30 dtsi file in a
separate patch).

>
> (note that patch is missing the clocks properties since the clock
> binding isn't finalized and checked in yet)
>
>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts
>>>
>>> Board changes would usually be a separate patch to an SoC change, but I
>>> guess not a big deal.
>>
>> Actually, this is the way I've always done DT files (in a separate
>> patch), and then the config files in a separate patch the enable the
>> feature.
>
> I meant that tegra114.dtsi and tegra114-dalmore.dts would usually be
> separate patches.
>
>>> All 5 I2C ports are used on the board and only support 100KHz? That
>>> seems unusual, but I suppose it's possible.
>>
>> That's the way I have it on T30, and looking at
>> arch/arm/boot/dts/tegra30.dtsi in the kernel, all 5 ports are set to
>> 100KHz.
>
> But this is board-specific; it depends on which of the I2C controllers
> are actually pinmux'd out onto the board (-> which to enable), and which
> devices are attached to those buses (-> max I2C clock rate). I don't see
> how Tegra30 nor tegra30.dtsi are relevant here.

I see your point, but I don't know exactly how to find out the proper
clock & disable settings for E1611 Dalmore. I'll talk to Yen & peruse
the docs.

Thanks,

Tom

[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Stephen Warren]

On 01/29/2013 02:57 PM, Tom Warren wrote:
> Stephen,
> 
> On Tue, Jan 29, 2013 at 2:08 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 01/29/2013 01:40 PM, Tom Warren wrote:
>>> Stephen,
>>>
>>> On Tue, Jan 29, 2013 at 1:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 01/29/2013 12:19 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).
>>
>>>> If you do keep this node, it needs the clocks property filled in.
>>>>
>>>>> +     i2c at 7000c000 {
>>>>> +             compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
>>>>
>>>> The I2C nodes also aren't backwards-compatible.
>>>
>>> They were on T30 (same compatible string there, except of course
>>> s/114/30/). And the tegra20-i2c is needed for fdtdec.c to find it
>>> (compat_names table).
>>
>> Yes, I believe the HW changes between Tegra20 and Tegra30 were such that
>> the Tegra20 driver would run unmodified on Tegra30 without issue. I
>> don't believe that's the case for Tegra114 though. The solution would be
>> to add the Tegra114 compatible value to the I2C driver so that it can
>> search for both.
> 
> Looking at the TRMs for both T30 and T114, I2C looks nearly identical,
> except for some additional registers tacked onto the end for bus clear
> support. 99% of the bits appear exactly the same, too (with the
> addition of a bus clear int bit on T114 in an used bit position).  I
> know next to nothing about I2C, but it appears to me that the current
> tegra_I2C.c driver should work fine on T114 (and does, since I can
> probe addresses with it).  I don't see a need for a different driver
> or compat value here.

I Cc'd in Laxman here, since he wrote the Tegra114 I2C support for the
kernel. He made the decision that Tegra114 I2C wasn't compatible with
Tegra20/30 I2C, and can explain that. Certainly looking at the kernel
patch I pointed at, it seems some of the clock divisors must be
programmed differently, which certainly seems enough to declare the HW
blocks incompatible, even if everything else is identical.

>>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts

>>> That's the way I have it on T30, and looking at
>>> arch/arm/boot/dts/tegra30.dtsi in the kernel, all 5 ports are set to
>>> 100KHz.
>>
>> But this is board-specific; it depends on which of the I2C controllers
>> are actually pinmux'd out onto the board (-> which to enable), and which
>> devices are attached to those buses (-> max I2C clock rate). I don't see
>> how Tegra30 nor tegra30.dtsi are relevant here.
> 
> I see your point, but I don't know exactly how to find out the proper
> clock & disable settings for E1611 Dalmore. I'll talk to Yen & peruse
> the docs.

The board schematics should show which I2C buses are hooked up.

[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Laxman Dewangan]

On Wednesday 30 January 2013 03:43 AM, Stephen Warren wrote:
> On 01/29/2013 02:57 PM, Tom Warren wrote:
>> Stephen,
>>
>> On Tue, Jan 29, 2013 at 2:08 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 01/29/2013 01:40 PM, Tom Warren wrote:
>>>> Stephen,
>>>>
>>>> On Tue, Jan 29, 2013 at 1:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> On 01/29/2013 12:19 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).
>>>>> If you do keep this node, it needs the clocks property filled in.
>>>>>
>>>>>> +     i2c at 7000c000 {
>>>>>> +             compatible = "nvidia,tegra114-i2c", "nvidia,tegra20-i2c";
>>>>> The I2C nodes also aren't backwards-compatible.
>>>> They were on T30 (same compatible string there, except of course
>>>> s/114/30/). And the tegra20-i2c is needed for fdtdec.c to find it
>>>> (compat_names table).
>>> Yes, I believe the HW changes between Tegra20 and Tegra30 were such that
>>> the Tegra20 driver would run unmodified on Tegra30 without issue. I
>>> don't believe that's the case for Tegra114 though. The solution would be
>>> to add the Tegra114 compatible value to the I2C driver so that it can
>>> search for both.
>> Looking at the TRMs for both T30 and T114, I2C looks nearly identical,
>> except for some additional registers tacked onto the end for bus clear
>> support. 99% of the bits appear exactly the same, too (with the
>> addition of a bus clear int bit on T114 in an used bit position).  I
>> know next to nothing about I2C, but it appears to me that the current
>> tegra_I2C.c driver should work fine on T114 (and does, since I can
>> probe addresses with it).  I don't see a need for a different driver
>> or compat value here.
> I Cc'd in Laxman here, since he wrote the Tegra114 I2C support for the
> kernel. He made the decision that Tegra114 I2C wasn't compatible with
> Tegra20/30 I2C, and can explain that. Certainly looking at the kernel
> patch I pointed at, it seems some of the clock divisors must be
> programmed differently, which certainly seems enough to declare the HW
> blocks incompatible, even if everything else is identical.
>

Tegra30 i2c driver will not work as straight. There is los of change 
which need to be taken care.

I posted the change for kernel driver.
Changes are:
- Enable/disable control for per packet transfer complete interrupt.
   Earlier SoCs could not disable this.
- Single clock source for standard/fast and HS mode clock speed.
   The clock divisor for fast/standard mode is added into the i2c
   controller to meet the HS and standard/fast mode of clock speed
   from single source.


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

[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Tom Warren]

Laxman,

On Wed, Jan 30, 2013 at 12:01 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>
> Tegra30 i2c driver will not work as straight. There is los of change which
> need to be taken care.
>
> I posted the change for kernel driver.
> Changes are:
> - Enable/disable control for per packet transfer complete interrupt.
>   Earlier SoCs could not disable this.
> - Single clock source for standard/fast and HS mode clock speed.
>   The clock divisor for fast/standard mode is added into the i2c
>   controller to meet the HS and standard/fast mode of clock speed
>   from single source.
>
>
The Linux driver changes in that patch Stephen pointed to don't look
_that_ extensive - not what I would call 'lots of changes'.  I don't
know that the transfer-complete interrupt disable is needed in U-Boot,
since interrupts aren't used at all. I'll modify the clock source
divisor calculation, though, and see what difference that makes, if
any.


Thanks,

Tom

[U-Boot] [PATCH 1/2] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Stephen Warren]

On 01/29/2013 02:57 PM, Tom Warren wrote:
> Stephen,
> 
> On Tue, Jan 29, 2013 at 2:08 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 01/29/2013 01:40 PM, Tom Warren wrote:
>>> Stephen,
>>>
>>> On Tue, Jan 29, 2013 at 1:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 01/29/2013 12:19 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).

>>>>> +             reg = <0x7000c000 0x100>;
>>>>
>>>> The interrupts property is missing here.
>>>
>>> I didn't have an interrupts property for I2C on the T30 dts files,
>>> either.
>>
>> That sounds like a bug in the DT.
>>
>>> What is it used for in U-Boot?
>>
>> U-Boot doesn't use interrupts at least on Tegra, so it's not used.
>> However, it'd be best to keep the U-Boot DT in line with the kernel DT
>> so we can easily spot differences. There has been talk of trying to use
>> the same DT for both too, although we're a long long way off from that.
> 
> I'll add it, but it seems silly to add unused values, especially when
> your past review critiques of DT files have asked for minimum
> functionality, which I agree with.

I meant to mention here that my previous comments were about unused
nodes; it doesn't seem useful to add a bunch of nodes that we aren't
using, until the drivers are ready and we actually end up using those
nodes. However, when a node is added, we should fill it in fully, so the
DT conforms to the binding for that node.

[U-Boot] [PATCH 2/2] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board

[Tom Warren]

Tested all 5 'buses', i2c probe enumerates device addresses on bus
1 and 2.

Note that this uses the tegra_i2c.c driver w/o modification.

Signed-off-by: Tom Warren <twarren@nvidia.com>
---
 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

[U-Boot] [PATCH 2/2] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board

[Stephen Warren]

On 01/29/2013 12:19 PM, Tom Warren wrote:
> Tested all 5 'buses', i2c probe enumerates device addresses on bus
> 1 and 2.

Why don't the other 3 buses work?

> Note that this uses the tegra_i2c.c driver w/o modification.

The kernel patches imply some mandatory differences in clock divider
programming; see:

https://patchwork.kernel.org/patch/1936371/

[U-Boot] [PATCH 2/2] Tegra114: I2C: Enable I2C driver on Dalmore E1611 eval board

[Tom Warren]

Stephen,

On Tue, Jan 29, 2013 at 1:11 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 01/29/2013 12:19 PM, Tom Warren wrote:
>> Tested all 5 'buses', i2c probe enumerates device addresses on bus
>> 1 and 2.
>
> Why don't the other 3 buses work?

No idea, but I assume because my Dalmore doesn't have sub-boards w/I2C
devices connected (gyro, accel, camera, etc.)

>
>> Note that this uses the tegra_i2c.c driver w/o modification.
>
> The kernel patches imply some mandatory differences in clock divider
> programming; see:
>
> https://patchwork.kernel.org/patch/1936371/

I'll take a look. Thanks.

Tom