[U-Boot] [PATCH v3 0/3] Add I2C driver for T114 Dalmore

[U-Boot] [PATCH v3 0/3] Add I2C driver for T114 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.

Changes in V2:
- use new calculation for T114 I2C clocks
- incorporate review comments from StephenW for the dtsi file
Changes in V3:
- Added tegra114-i2c name to compat_names/compat_id table(s)
- Add search for T114 I2C node in i2c_init_board
- Added is_scs flag to i2c_bus struct (single clock source HW)
- Use is_scs flag to trigger new clock source rate calculation
- Moved aliases to dtsi file as per StephenW
- Removed tegra30-car & tegra20-i2c compatible strings
- Changed I2C5 (PWR_I2C) clock to 400MHz

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                  |   64 +++++++++++++++++++++++++++
 arch/arm/include/asm/arch-tegra/tegra_i2c.h |    6 +++
 board/nvidia/dts/tegra114-dalmore.dts       |   28 ++++++++++++
 drivers/i2c/tegra_i2c.c                     |   42 +++++++++++++++--
 include/configs/dalmore.h                   |    9 ++++
 include/configs/tegra114-common.h           |    3 +
 include/fdtdec.h                            |    1 +
 lib/fdtdec.c                                |    1 +
 8 files changed, 149 insertions(+), 5 deletions(-)

[U-Boot] [PATCH v3 1/3] Tegra: I2C: Add T114 clock support to tegra_i2c driver

[Tom Warren]

T114 has a slightly different I2C clock, with a new (extra) divisor
in standard/fast mode and HS mode. Tested on my Dalmore, and the I2C
clock is 100KHz +/- 3Hz on my Saleae Logic analyzer.

Added a new entry in compat_names for T114 I2C since it differs
from the previous Tegra SoCs. A flag is set when T114 I2C HW is
found so new features like the extra clock divisor can be used.

Signed-off-by: Tom Warren <twarren@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
---
v2: new
V3:
- Added tegra114-i2c name to compat_names/compat_id table(s)
- Add search for T114 I2C node in i2c_init_board
- Added is_scs flag to i2c_bus struct (single clock source HW)
- Use is_scs flag to trigger new clock source rate calculation
    
 arch/arm/include/asm/arch-tegra/tegra_i2c.h |    6 ++++
 drivers/i2c/tegra_i2c.c                     |   42 +++++++++++++++++++++++---
 include/fdtdec.h                            |    1 +
 lib/fdtdec.c                                |    1 +
 4 files changed, 45 insertions(+), 5 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..ca71cd3 100644
--- a/drivers/i2c/tegra_i2c.c
+++ b/drivers/i2c/tegra_i2c.c
@@ -46,6 +46,7 @@ struct i2c_bus {
 	struct i2c_control	*control;
 	struct i2c_ctlr		*regs;
 	int			is_dvc;	/* DVC type, rather than I2C */
+	int			is_scs;	/* single clock source (T114+) */
 	int			inited;	/* bus is inited */
 };
 
@@ -88,7 +89,28 @@ 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 (i2c_bus->is_scs) {
+		/*
+		 * 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_stdfst_mode = readl(&i2c_bus->regs->clk_div) >> 16;
+		debug("%s: CLK_DIV_STD_FAST_MODE setting = %d\n", __func__,
+			clk_div_stdfst_mode);
+
+		clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_PERIPH,
+			CLK_MULT_STD_FAST_MODE * (clk_div_stdfst_mode + 1) *
+			i2c_bus->speed * 2);
+	}
 
 	/* Reset I2C controller. */
 	i2c_reset_controller(i2c_bus);
@@ -352,10 +374,11 @@ static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus)
  * @param node_list	list of nodes to process (any <=0 are ignored)
  * @param count		number of nodes to process
  * @param is_dvc	1 if these are DVC ports, 0 if standard I2C
+ * @param is_scs	1 if this HW uses a single clock source (T114+)
  * @return 0 if ok, -1 on error
  */
 static int process_nodes(const void *blob, int node_list[], int count,
-			 int is_dvc)
+			 int is_dvc, int is_scs)
 {
 	struct i2c_bus *i2c_bus;
 	int i;
@@ -375,6 +398,8 @@ static int process_nodes(const void *blob, int node_list[], int count,
 			return -1;
 		}
 
+		i2c_bus->is_scs = is_scs;
+
 		i2c_bus->is_dvc = is_dvc;
 		if (is_dvc) {
 			i2c_bus->control =
@@ -403,18 +428,25 @@ void i2c_init_board(void)
 	const void *blob = gd->fdt_blob;
 	int count;
 
-	/* First get the normal i2c ports */
+	/* First check for newer (T114+) I2C ports */
+	count = fdtdec_find_aliases_for_id(blob, "i2c",
+			COMPAT_NVIDIA_TEGRA114_I2C, node_list,
+			TEGRA_I2C_NUM_CONTROLLERS);
+	if (process_nodes(blob, node_list, count, 0, 1))
+		return;
+
+	/* Now get the older (T20/T30) normal I2C ports */
 	count = fdtdec_find_aliases_for_id(blob, "i2c",
 			COMPAT_NVIDIA_TEGRA20_I2C, node_list,
 			TEGRA_I2C_NUM_CONTROLLERS);
-	if (process_nodes(blob, node_list, count, 0))
+	if (process_nodes(blob, node_list, count, 0, 0))
 		return;
 
 	/* Now look for dvc ports */
 	count = fdtdec_add_aliases_for_id(blob, "i2c",
 			COMPAT_NVIDIA_TEGRA20_DVC, node_list,
 			TEGRA_I2C_NUM_CONTROLLERS);
-	if (process_nodes(blob, node_list, count, 1))
+	if (process_nodes(blob, node_list, count, 1, 0))
 		return;
 }
 
diff --git a/include/fdtdec.h b/include/fdtdec.h
index f77d195..5abf9e7 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -62,6 +62,7 @@ struct fdt_memory {
 enum fdt_compat_id {
 	COMPAT_UNKNOWN,
 	COMPAT_NVIDIA_TEGRA20_USB,	/* Tegra20 USB port */
+	COMPAT_NVIDIA_TEGRA114_I2C,	/* Tegra114 I2C w/single clock source */
 	COMPAT_NVIDIA_TEGRA20_I2C,	/* Tegra20 i2c */
 	COMPAT_NVIDIA_TEGRA20_DVC,	/* Tegra20 dvc (really just i2c) */
 	COMPAT_NVIDIA_TEGRA20_EMC,	/* Tegra20 memory controller */
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 16921e1..2681d74 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -37,6 +37,7 @@ DECLARE_GLOBAL_DATA_PTR;
 static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(UNKNOWN, "<none>"),
 	COMPAT(NVIDIA_TEGRA20_USB, "nvidia,tegra20-ehci"),
+	COMPAT(NVIDIA_TEGRA114_I2C, "nvidia,tegra114-i2c"),
 	COMPAT(NVIDIA_TEGRA20_I2C, "nvidia,tegra20-i2c"),
 	COMPAT(NVIDIA_TEGRA20_DVC, "nvidia,tegra20-i2c-dvc"),
 	COMPAT(NVIDIA_TEGRA20_EMC, "nvidia,tegra20-emc"),
-- 
1.7.0.4

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

[Tom Warren]

T114, like T30, 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). PWR_I2C is set to 400KHz.

Signed-off-by: Tom Warren <twarren@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
---
v2: Match dts files with kernel files, remove unused apdma node
v3:
- moved aliases to dtsi file as per StephenW
- removed tegra30-car & tegra20-i2c compatible strings
- changed I2C5 (PWR_I2C) clock to 400MHz

 arch/arm/dts/tegra114.dtsi            |   64 +++++++++++++++++++++++++++++++++
 board/nvidia/dts/tegra114-dalmore.dts |   28 ++++++++++++++
 2 files changed, 92 insertions(+), 0 deletions(-)

diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
index d06cd12..0655b4d 100644
--- a/arch/arm/dts/tegra114.dtsi
+++ b/arch/arm/dts/tegra114.dtsi
@@ -2,4 +2,68 @@
 
 / {
 	compatible = "nvidia,tegra114";
+
+	aliases {
+		i2c0 = "/i2c at 7000d000";
+		i2c1 = "/i2c at 7000c000";
+		i2c2 = "/i2c at 7000c400";
+		i2c3 = "/i2c at 7000c500";
+		i2c4 = "/i2c at 7000c700";
+	};
+
+	tegra_car: clock at 60006000 {
+		compatible = "nvidia,tegra114-car";
+		reg = <0x60006000 0x1000>;
+		#clock-cells = <1>;
+	};
+
+	i2c at 7000c000 {
+		compatible = "nvidia,tegra114-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";
+		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";
+		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";
+		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";
+		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..3f3cd81 100644
--- a/board/nvidia/dts/tegra114-dalmore.dts
+++ b/board/nvidia/dts/tegra114-dalmore.dts
@@ -6,8 +6,36 @@
 	model = "NVIDIA Dalmore";
 	compatible = "nvidia,dalmore", "nvidia,tegra114";
 
+	aliases {
+	};
+
 	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 = <400000>;
+	};
 };
-- 
1.7.0.4

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

[Stephen Warren]

On 02/08/2013 10:25 AM, Tom Warren wrote:
> T114, like T30, 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). PWR_I2C is set to 400KHz.

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

> +	aliases {
> +	};
> +

There's no point adding an empty aliases node here. Feel free to fix
that up when you apply it rather than reposting if you want.

I'd like too see Laxman sign-off on the "*2" question he had earlier
before actually checking this in.

Aside from that, the series,
Reviewed-by: Stephen Warren <swarren@nvidia.com>

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

[Laxman Dewangan]

On Friday 08 February 2013 11:34 PM, Stephen Warren wrote:
> On 02/08/2013 10:25 AM, Tom Warren wrote:
>> T114, like T30, 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). PWR_I2C is set to 400KHz.
>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts b/board/nvidia/dts/tegra114-dalmore.dts
>> +	aliases {
>> +	};
>> +
> There's no point adding an empty aliases node here. Feel free to fix
> that up when you apply it rather than reposting if you want.
>
> I'd like too see Laxman sign-off on the "*2" question he had earlier
> before actually checking this in.
>
We do not require *2 as the i2c clock divider is DIVU16 type. There was 
bug in early code on kernel also which we fixed in dowstream long back. 
Possibly uboot have not fixed this yet.

-----------------------------------------------------------------------------------
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 v3 2/3] Tegra114: fdt: Update DT files with I2C info for T114/Dalmore

[Tom Warren]

Laxman,

On Tue, Feb 12, 2013 at 5:02 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> On Friday 08 February 2013 11:34 PM, Stephen Warren wrote:
>>
>> On 02/08/2013 10:25 AM, Tom Warren wrote:
>>>
>>> T114, like T30, 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). PWR_I2C is set to 400KHz.
>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts
>>> b/board/nvidia/dts/tegra114-dalmore.dts
>>> +       aliases {
>>> +       };
>>> +
>>
>> There's no point adding an empty aliases node here. Feel free to fix
>> that up when you apply it rather than reposting if you want.
>>
>> I'd like too see Laxman sign-off on the "*2" question he had earlier
>> before actually checking this in.
>>
> We do not require *2 as the i2c clock divider is DIVU16 type. There was bug
> in early code on kernel also which we fixed in dowstream long back. Possibly
> uboot have not fixed this yet.
Yeah, the Tegra I2C driver in U-Boot has always doubled the rate
before calling the clock set routine. But the important thing is that
the actual I2C clock is 100KHz (or 400KHz for I2C5) as measured at the
test points on the board.

I'll look into the Tegra U-Boot clock routine idiosyncrasies later
when I get some more bandwidth.

Thanks,

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

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

[Stephen Warren]

On 02/12/2013 10:40 AM, Tom Warren wrote:
> Laxman,
> 
> On Tue, Feb 12, 2013 at 5:02 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> On Friday 08 February 2013 11:34 PM, Stephen Warren wrote:
>>>
>>> On 02/08/2013 10:25 AM, Tom Warren wrote:
>>>>
>>>> T114, like T30, 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). PWR_I2C is set to 400KHz.
>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts
>>>> b/board/nvidia/dts/tegra114-dalmore.dts
>>>> +       aliases {
>>>> +       };
>>>> +
>>>
>>> There's no point adding an empty aliases node here. Feel free to fix
>>> that up when you apply it rather than reposting if you want.
>>>
>>> I'd like too see Laxman sign-off on the "*2" question he had earlier
>>> before actually checking this in.
>>>
>> We do not require *2 as the i2c clock divider is DIVU16 type. There was bug
>> in early code on kernel also which we fixed in dowstream long back. Possibly
>> uboot have not fixed this yet.
>
> Yeah, the Tegra I2C driver in U-Boot has always doubled the rate
> before calling the clock set routine.

Perhaps it's related to some dividers being U7.1 (i.e. the LSB of the
divider represents 1/2 not 1)? However, as Laxman points out, this isn't
the case for I2C clocks; they have a U16 divider, so the LSB represents
1 not 1/2.

> But the important thing is that
> the actual I2C clock is 100KHz (or 400KHz for I2C5) as measured at the
> test points on the board.
> 
> I'll look into the Tegra U-Boot clock routine idiosyncrasies later
> when I get some more bandwidth.

Just a few minutes of investigation points at clk_get_divider() being buggy.

It assumes that all dividers have a built-in *2 clock multiplier on the
front of them:

        u64 divider = parent_rate * 2;

(the name for that variable is wrong; it should be something more like
parent_rate or divider_input_rate)

This (presence of a *2 pre-multiplier) is true for U7.1 dividers, since
this is required for the LSB of the divider to represent 1/2 not 1.
Hence, I assume that e.g. the SPI driver doesn't do this "* 2" on the
clock rate; it actually has a U7.1 divider.

However, this is not true for U16 dividers, since the HW doesn't need to
multiply up the rate before dividing, since the LSB is 1 not 1/2.

The solution here is to fix clk_get_divider() to return both a field
width and a flag (or width) indicating whether a fractional part of the
divider is present, and then pass that on to adjust_periph_pll() so that
it can only optionally perform this initial "* 2".

So at least that explains it.

I would strongly recommend just fixing this now. However, if you don't
please do file a bug so that we don't forget about this.

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

[Tom Warren]

Stephen,

On Tue, Feb 12, 2013 at 11:10 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 02/12/2013 10:40 AM, Tom Warren wrote:
>> Laxman,
>>
>> On Tue, Feb 12, 2013 at 5:02 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>>> On Friday 08 February 2013 11:34 PM, Stephen Warren wrote:
>>>>
>>>> On 02/08/2013 10:25 AM, Tom Warren wrote:
>>>>>
>>>>> T114, like T30, 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). PWR_I2C is set to 400KHz.
>>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts
>>>>> b/board/nvidia/dts/tegra114-dalmore.dts
>>>>> +       aliases {
>>>>> +       };
>>>>> +
>>>>
>>>> There's no point adding an empty aliases node here. Feel free to fix
>>>> that up when you apply it rather than reposting if you want.
>>>>
>>>> I'd like too see Laxman sign-off on the "*2" question he had earlier
>>>> before actually checking this in.
>>>>
>>> We do not require *2 as the i2c clock divider is DIVU16 type. There was bug
>>> in early code on kernel also which we fixed in dowstream long back. Possibly
>>> uboot have not fixed this yet.
>>
>> Yeah, the Tegra I2C driver in U-Boot has always doubled the rate
>> before calling the clock set routine.
>
> Perhaps it's related to some dividers being U7.1 (i.e. the LSB of the
> divider represents 1/2 not 1)? However, as Laxman points out, this isn't
> the case for I2C clocks; they have a U16 divider, so the LSB represents
> 1 not 1/2.
>
>> But the important thing is that
>> the actual I2C clock is 100KHz (or 400KHz for I2C5) as measured at the
>> test points on the board.
>>
>> I'll look into the Tegra U-Boot clock routine idiosyncrasies later
>> when I get some more bandwidth.
>
> Just a few minutes of investigation points at clk_get_divider() being buggy.
>
> It assumes that all dividers have a built-in *2 clock multiplier on the
> front of them:
>
>         u64 divider = parent_rate * 2;
>
> (the name for that variable is wrong; it should be something more like
> parent_rate or divider_input_rate)
>
> This (presence of a *2 pre-multiplier) is true for U7.1 dividers, since
> this is required for the LSB of the divider to represent 1/2 not 1.
> Hence, I assume that e.g. the SPI driver doesn't do this "* 2" on the
> clock rate; it actually has a U7.1 divider.
>
> However, this is not true for U16 dividers, since the HW doesn't need to
> multiply up the rate before dividing, since the LSB is 1 not 1/2.
>
> The solution here is to fix clk_get_divider() to return both a field
> width and a flag (or width) indicating whether a fractional part of the
> divider is present, and then pass that on to adjust_periph_pll() so that
> it can only optionally perform this initial "* 2".
>
> So at least that explains it.
Yeah, I just started looking at it before lunch, and saw the same thing.

I'll see about fixing it now so I can put the T114 I2C patches to bed.

Thanks
>
> I would strongly recommend just fixing this now. However, if you don't
> please do file a bug so that we don't forget about this.

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

[Tom Warren]

Stephen/Laxman,

On Tue, Feb 12, 2013 at 12:07 PM, Tom Warren <twarren.nvidia@gmail.com> wrote:
> Stephen,
>
> On Tue, Feb 12, 2013 at 11:10 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 02/12/2013 10:40 AM, Tom Warren wrote:
>>> Laxman,
>>>
>>> On Tue, Feb 12, 2013 at 5:02 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>>>> On Friday 08 February 2013 11:34 PM, Stephen Warren wrote:
>>>>>
>>>>> On 02/08/2013 10:25 AM, Tom Warren wrote:
>>>>>>
>>>>>> T114, like T30, 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). PWR_I2C is set to 400KHz.
>>>>>> diff --git a/board/nvidia/dts/tegra114-dalmore.dts
>>>>>> b/board/nvidia/dts/tegra114-dalmore.dts
>>>>>> +       aliases {
>>>>>> +       };
>>>>>> +
>>>>>
>>>>> There's no point adding an empty aliases node here. Feel free to fix
>>>>> that up when you apply it rather than reposting if you want.
>>>>>
>>>>> I'd like too see Laxman sign-off on the "*2" question he had earlier
>>>>> before actually checking this in.
>>>>>
>>>> We do not require *2 as the i2c clock divider is DIVU16 type. There was bug
>>>> in early code on kernel also which we fixed in dowstream long back. Possibly
>>>> uboot have not fixed this yet.
>>>
>>> Yeah, the Tegra I2C driver in U-Boot has always doubled the rate
>>> before calling the clock set routine.
>>
>> Perhaps it's related to some dividers being U7.1 (i.e. the LSB of the
>> divider represents 1/2 not 1)? However, as Laxman points out, this isn't
>> the case for I2C clocks; they have a U16 divider, so the LSB represents
>> 1 not 1/2.
>>
>>> But the important thing is that
>>> the actual I2C clock is 100KHz (or 400KHz for I2C5) as measured at the
>>> test points on the board.
>>>
>>> I'll look into the Tegra U-Boot clock routine idiosyncrasies later
>>> when I get some more bandwidth.
>>
>> Just a few minutes of investigation points at clk_get_divider() being buggy.
>>
>> It assumes that all dividers have a built-in *2 clock multiplier on the
>> front of them:
>>
>>         u64 divider = parent_rate * 2;
>>
>> (the name for that variable is wrong; it should be something more like
>> parent_rate or divider_input_rate)
>>
>> This (presence of a *2 pre-multiplier) is true for U7.1 dividers, since
>> this is required for the LSB of the divider to represent 1/2 not 1.
>> Hence, I assume that e.g. the SPI driver doesn't do this "* 2" on the
>> clock rate; it actually has a U7.1 divider.
>>
>> However, this is not true for U16 dividers, since the HW doesn't need to
>> multiply up the rate before dividing, since the LSB is 1 not 1/2.
>>
>> The solution here is to fix clk_get_divider() to return both a field
>> width and a flag (or width) indicating whether a fractional part of the
>> divider is present, and then pass that on to adjust_periph_pll() so that
>> it can only optionally perform this initial "* 2".
>>
>> So at least that explains it.
> Yeah, I just started looking at it before lunch, and saw the same thing.
>
> I'll see about fixing it now so I can put the T114 I2C patches to bed.
>
> Thanks
>>
>> I would strongly recommend just fixing this now. However, if you don't
>> please do file a bug so that we don't forget about this.

I tried a quick fix for the 16-bit clock divider (only I2C HW on
current Tegra HW uses it), but it didn't pass internal review.

I don' t have time to rewrite this before I leave for a few days, so
I'll look at it when I get back, if someone else hasn't already fixed
it by then. But the current T114 I2C patches, which use a WAR of
asking for a 2X rate, work OK for now (I2C1 clock, 100KHz expected
rate, measured as 97-103KHz, so occasionally out-of-spec. Note that
T20 has 'always' had this rate doubling request.

As an aside, the new/extra clock divisor in T114 I2C HW
(CLK_DIVISOR_STD_FAST_MODE) needs to change dynamically to come up
with a better/closer actual rate - it's fixed at 0x19 (25) right now,
and makes getting an exact clock_source divisor difficult. It'd be
better (for 100KHz) if it were 0x32 (50), which would give a divisor
of 9, and 408MHz/8/50+1/9+1 = 100KHz exactly.  I'll look into changing
it for each target I2C bus freq, too, when I get back.

I'll file an internal bug so we can track the 16-bit divisor problem
w/Tegra U-Boot.

Tom

[U-Boot] [PATCH v3 3/3] 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.

Signed-off-by: Tom Warren <twarren@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
---
v2: No change
v3: 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

[U-Boot] [PATCH v3 0/3] Add I2C driver for T114 Dalmore

[Simon Glass]

Hi,

On Fri, Feb 8, 2013 at 9:25 AM, Tom Warren <twarren.nvidia@gmail.com> wrote:
> 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
> Changes in V3:
> - Added tegra114-i2c name to compat_names/compat_id table(s)
> - Add search for T114 I2C node in i2c_init_board
> - Added is_scs flag to i2c_bus struct (single clock source HW)
> - Use is_scs flag to trigger new clock source rate calculation
> - Moved aliases to dtsi file as per StephenW
> - Removed tegra30-car & tegra20-i2c compatible strings
> - Changed I2C5 (PWR_I2C) clock to 400MHz
>
> 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                  |   64 +++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-tegra/tegra_i2c.h |    6 +++
>  board/nvidia/dts/tegra114-dalmore.dts       |   28 ++++++++++++
>  drivers/i2c/tegra_i2c.c                     |   42 +++++++++++++++--
>  include/configs/dalmore.h                   |    9 ++++
>  include/configs/tegra114-common.h           |    3 +
>  include/fdtdec.h                            |    1 +
>  lib/fdtdec.c                                |    1 +
>  8 files changed, 149 insertions(+), 5 deletions(-)
>

Series LGTM except for the comment about the need to enhance the alias function.

Regards,
Simon