[U-Boot] [PATCH 0/7] tegra: Add I2C driver and associated parts

[U-Boot] [PATCH 0/7] tegra: Add I2C driver and associated parts

[Simon Glass]

This series brings in an I2C driver for Tegra which can be
configured by a flat device tree.

It supports 8- and 16-bit addresses and both the normal I2C ports and
the DVC port (for controlling the power management unit (PMU)).


Simon Glass (6):
  tegra: Rename NV_PA_PMC_BASE to TEGRA2_PMC_BASE
  tegra: fdt: Add extra I2C definitions for U-Boot
  tegra: Add I2C support to funcmux
  tegra: Initialise I2C on Nvidia boards
  tegra: Select I2C ordering for Seaboard
  tegra: Enable I2C on Seaboard

Yen Lin (1):
  tegra: Add I2C driver

 arch/arm/cpu/armv7/tegra2/ap20.c              |   10 +-
 arch/arm/cpu/armv7/tegra2/board.c             |    2 +-
 arch/arm/cpu/armv7/tegra2/funcmux.c           |   75 +++-
 arch/arm/dts/tegra20.dtsi                     |   12 +
 arch/arm/include/asm/arch-tegra2/funcmux.h    |    3 +
 arch/arm/include/asm/arch-tegra2/tegra2.h     |    8 +-
 arch/arm/include/asm/arch-tegra2/tegra2_i2c.h |  167 ++++++++
 board/nvidia/common/board.c                   |    4 +
 board/nvidia/dts/tegra2-seaboard.dts          |    5 +
 drivers/i2c/Makefile                          |    1 +
 drivers/i2c/tegra2_i2c.c                      |  533 +++++++++++++++++++++++++
 include/configs/seaboard.h                    |    8 +
 include/fdtdec.h                              |    1 +
 lib/fdtdec.c                                  |    1 +
 14 files changed, 809 insertions(+), 21 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
 create mode 100644 drivers/i2c/tegra2_i2c.c

-- 
1.7.3.1

[U-Boot] [PATCH 1/7] tegra: Rename NV_PA_PMC_BASE to TEGRA2_PMC_BASE

[Simon Glass]

Change this name to fit with the current convention in the Tegra
header file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/ap20.c          |   10 +++++-----
 arch/arm/cpu/armv7/tegra2/board.c         |    2 +-
 arch/arm/include/asm/arch-tegra2/tegra2.h |    4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/ap20.c b/arch/arm/cpu/armv7/tegra2/ap20.c
index 4c44bb3..3ea2e26 100644
--- a/arch/arm/cpu/armv7/tegra2/ap20.c
+++ b/arch/arm/cpu/armv7/tegra2/ap20.c
@@ -105,14 +105,14 @@ static void enable_cpu_clock(int enable)
 
 static int is_cpu_powered(void)
 {
-	struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
+	struct pmc_ctlr *pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
 
 	return (readl(&pmc->pmc_pwrgate_status) & CPU_PWRED) ? 1 : 0;
 }
 
 static void remove_cpu_io_clamps(void)
 {
-	struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
+	struct pmc_ctlr *pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
 	u32 reg;
 
 	/* Remove the clamps on the CPU I/O signals */
@@ -126,7 +126,7 @@ static void remove_cpu_io_clamps(void)
 
 static void powerup_cpu(void)
 {
-	struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
+	struct pmc_ctlr *pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
 	u32 reg;
 	int timeout = IO_STABILIZATION_DELAY;
 
@@ -157,7 +157,7 @@ static void powerup_cpu(void)
 
 static void enable_cpu_power_rail(void)
 {
-	struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
+	struct pmc_ctlr *pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
 	u32 reg;
 
 	reg = readl(&pmc->pmc_cntrl);
@@ -277,7 +277,7 @@ void enable_scu(void)
 
 void init_pmc_scratch(void)
 {
-	struct pmc_ctlr *const pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
+	struct pmc_ctlr *const pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
 	int i;
 
 	/* SCRATCH0 is initialized by the boot ROM and shouldn't be cleared */
diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
index ea06570..2c91e69 100644
--- a/arch/arm/cpu/armv7/tegra2/board.c
+++ b/arch/arm/cpu/armv7/tegra2/board.c
@@ -47,7 +47,7 @@ enum {
 
 unsigned int query_sdram_size(void)
 {
-	struct pmc_ctlr *const pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
+	struct pmc_ctlr *const pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
 	u32 reg;
 
 	reg = readl(&pmc->pmc_scratch20);
diff --git a/arch/arm/include/asm/arch-tegra2/tegra2.h b/arch/arm/include/asm/arch-tegra2/tegra2.h
index baae2eb..ca1881e 100644
--- a/arch/arm/include/asm/arch-tegra2/tegra2.h
+++ b/arch/arm/include/asm/arch-tegra2/tegra2.h
@@ -39,7 +39,7 @@
 #define NV_PA_APB_UARTD_BASE	(NV_PA_APB_MISC_BASE + 0x6300)
 #define NV_PA_APB_UARTE_BASE	(NV_PA_APB_MISC_BASE + 0x6400)
 #define TEGRA2_SPI_BASE		(NV_PA_APB_MISC_BASE + 0xC380)
-#define NV_PA_PMC_BASE		0x7000E400
+#define TEGRA2_PMC_BASE		(NV_PA_APB_MISC_BASE + 0xE400)
 #define NV_PA_CSITE_BASE	0x70040000
 #define TEGRA_USB1_BASE		0xC5000000
 #define TEGRA_USB3_BASE		0xC5008000
@@ -55,7 +55,7 @@ struct timerus {
 	unsigned int cntr_1us;
 };
 #else  /* __ASSEMBLY__ */
-#define PRM_RSTCTRL		NV_PA_PMC_BASE
+#define PRM_RSTCTRL		TEGRA2_PMC_BASE
 #endif
 
 #endif	/* TEGRA2_H */
-- 
1.7.3.1

[U-Boot] [PATCH 1/7] tegra: Rename NV_PA_PMC_BASE to TEGRA2_PMC_BASE

[Marek Vasut]

> Change this name to fit with the current convention in the Tegra
> header file.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/arm/cpu/armv7/tegra2/ap20.c          |   10 +++++-----
>  arch/arm/cpu/armv7/tegra2/board.c         |    2 +-
>  arch/arm/include/asm/arch-tegra2/tegra2.h |    4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/tegra2/ap20.c
> b/arch/arm/cpu/armv7/tegra2/ap20.c index 4c44bb3..3ea2e26 100644
> --- a/arch/arm/cpu/armv7/tegra2/ap20.c
> +++ b/arch/arm/cpu/armv7/tegra2/ap20.c
> @@ -105,14 +105,14 @@ static void enable_cpu_clock(int enable)
> 
>  static int is_cpu_powered(void)
>  {
> -	struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
> +	struct pmc_ctlr *pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
> 
>  	return (readl(&pmc->pmc_pwrgate_status) & CPU_PWRED) ? 1 : 0;
>  }
> 
>  static void remove_cpu_io_clamps(void)
>  {
> -	struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
> +	struct pmc_ctlr *pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
>  	u32 reg;
> 
>  	/* Remove the clamps on the CPU I/O signals */
> @@ -126,7 +126,7 @@ static void remove_cpu_io_clamps(void)
> 
>  static void powerup_cpu(void)
>  {
> -	struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
> +	struct pmc_ctlr *pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
>  	u32 reg;
>  	int timeout = IO_STABILIZATION_DELAY;
> 
> @@ -157,7 +157,7 @@ static void powerup_cpu(void)
> 
>  static void enable_cpu_power_rail(void)
>  {
> -	struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
> +	struct pmc_ctlr *pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
>  	u32 reg;
> 
>  	reg = readl(&pmc->pmc_cntrl);
> @@ -277,7 +277,7 @@ void enable_scu(void)
> 
>  void init_pmc_scratch(void)
>  {
> -	struct pmc_ctlr *const pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
> +	struct pmc_ctlr *const pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
>  	int i;
> 
>  	/* SCRATCH0 is initialized by the boot ROM and shouldn't be cleared */
> diff --git a/arch/arm/cpu/armv7/tegra2/board.c
> b/arch/arm/cpu/armv7/tegra2/board.c index ea06570..2c91e69 100644
> --- a/arch/arm/cpu/armv7/tegra2/board.c
> +++ b/arch/arm/cpu/armv7/tegra2/board.c
> @@ -47,7 +47,7 @@ enum {
> 
>  unsigned int query_sdram_size(void)
>  {
> -	struct pmc_ctlr *const pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
> +	struct pmc_ctlr *const pmc = (struct pmc_ctlr *)TEGRA2_PMC_BASE;
>  	u32 reg;
> 
>  	reg = readl(&pmc->pmc_scratch20);
> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2.h
> b/arch/arm/include/asm/arch-tegra2/tegra2.h index baae2eb..ca1881e 100644
> --- a/arch/arm/include/asm/arch-tegra2/tegra2.h
> +++ b/arch/arm/include/asm/arch-tegra2/tegra2.h
> @@ -39,7 +39,7 @@
>  #define NV_PA_APB_UARTD_BASE	(NV_PA_APB_MISC_BASE + 0x6300)
>  #define NV_PA_APB_UARTE_BASE	(NV_PA_APB_MISC_BASE + 0x6400)
>  #define TEGRA2_SPI_BASE		(NV_PA_APB_MISC_BASE + 0xC380)
> -#define NV_PA_PMC_BASE		0x7000E400
> +#define TEGRA2_PMC_BASE		(NV_PA_APB_MISC_BASE + 0xE400)
>  #define NV_PA_CSITE_BASE	0x70040000
>  #define TEGRA_USB1_BASE		0xC5000000
>  #define TEGRA_USB3_BASE		0xC5008000
> @@ -55,7 +55,7 @@ struct timerus {
>  	unsigned int cntr_1us;
>  };
>  #else  /* __ASSEMBLY__ */
> -#define PRM_RSTCTRL		NV_PA_PMC_BASE
> +#define PRM_RSTCTRL		TEGRA2_PMC_BASE
>  #endif
> 
>  #endif	/* TEGRA2_H */

Looks ok
Acked-by: Marek Vasut <marek.vasut@gmail.com>

[U-Boot] [PATCH 1/7] tegra: Rename NV_PA_PMC_BASE to TEGRA2_PMC_BASE

[Stephen Warren]

On 12/26/2011 11:11 AM, Simon Glass wrote:
> Change this name to fit with the current convention in the Tegra
> header file.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Acked-by: Stephen Warren <swarren@nvidia.com>

-- 
nvpublic

[U-Boot] [PATCH 2/7] tegra: fdt: Add extra I2C definitions for U-Boot

[Simon Glass]

Add U-Boot's peripheral ID and pinmux selection to the Tegra20
device tree file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/dts/tegra20.dtsi |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index 27e3127..41ee0b2 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -18,6 +18,9 @@
 		compatible = "nvidia,tegra20-i2c";
 		reg = <0x7000C000 0x100>;
 		interrupts = < 70 >;
+		u-boot,pinmux = <0>;
+		speed = <100000>;
+		u-boot,periph-id = <12>;	// PERIPH_ID_I2C1
 	};
 
 	i2c at 7000c400 {
@@ -26,6 +29,9 @@
 		compatible = "nvidia,tegra20-i2c";
 		reg = <0x7000C400 0x100>;
 		interrupts = < 116 >;
+		u-boot,pinmux = <1>;
+		speed = <100000>;
+		u-boot,periph-id = <54>;	// PERIPH_ID_I2C2
 	};
 
 	i2c at 7000c500 {
@@ -34,6 +40,9 @@
 		compatible = "nvidia,tegra20-i2c";
 		reg = <0x7000C500 0x100>;
 		interrupts = < 124 >;
+		u-boot,pinmux = <0>;
+		speed = <100000>;
+		u-boot,periph-id = <67>;	// PERIPH_ID_I2C3
 	};
 
 	i2c at 7000d000 {
@@ -42,6 +51,9 @@
 		compatible = "nvidia,tegra20-i2c";
 		reg = <0x7000D000 0x200>;
 		interrupts = < 85 >;
+		u-boot,pinmux = <0>;
+		speed = <100000>;
+		u-boot,periph-id = <47>;	// PERIPH_ID_DVC_I2C
 	};
 
 	i2s at 70002800 {
-- 
1.7.3.1

[U-Boot] [PATCH 2/7] tegra: fdt: Add extra I2C definitions for U-Boot

[Marek Vasut]

> Add U-Boot's peripheral ID and pinmux selection to the Tegra20
> device tree file.

Isn't this supposed to go to Linux kernel ?

M
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/arm/dts/tegra20.dtsi |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
> index 27e3127..41ee0b2 100644
> --- a/arch/arm/dts/tegra20.dtsi
> +++ b/arch/arm/dts/tegra20.dtsi
> @@ -18,6 +18,9 @@
>  		compatible = "nvidia,tegra20-i2c";
>  		reg = <0x7000C000 0x100>;
>  		interrupts = < 70 >;
> +		u-boot,pinmux = <0>;
> +		speed = <100000>;
> +		u-boot,periph-id = <12>;	// PERIPH_ID_I2C1
>  	};
> 
>  	i2c at 7000c400 {
> @@ -26,6 +29,9 @@
>  		compatible = "nvidia,tegra20-i2c";
>  		reg = <0x7000C400 0x100>;
>  		interrupts = < 116 >;
> +		u-boot,pinmux = <1>;
> +		speed = <100000>;
> +		u-boot,periph-id = <54>;	// PERIPH_ID_I2C2
>  	};
> 
>  	i2c at 7000c500 {
> @@ -34,6 +40,9 @@
>  		compatible = "nvidia,tegra20-i2c";
>  		reg = <0x7000C500 0x100>;
>  		interrupts = < 124 >;
> +		u-boot,pinmux = <0>;
> +		speed = <100000>;
> +		u-boot,periph-id = <67>;	// PERIPH_ID_I2C3
>  	};
> 
>  	i2c at 7000d000 {
> @@ -42,6 +51,9 @@
>  		compatible = "nvidia,tegra20-i2c";
>  		reg = <0x7000D000 0x200>;
>  		interrupts = < 85 >;
> +		u-boot,pinmux = <0>;
> +		speed = <100000>;
> +		u-boot,periph-id = <47>;	// PERIPH_ID_DVC_I2C
>  	};
> 
>  	i2s at 70002800 {

[U-Boot] [PATCH 2/7] tegra: fdt: Add extra I2C definitions for U-Boot

[Stephen Warren]

Simon Glass wrote at Monday, December 26, 2011 11:12 AM:
> Add U-Boot's peripheral ID and pinmux selection to the Tegra20
> device tree file.

> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
...
>  		compatible = "nvidia,tegra20-i2c";
>  		reg = <0x7000C000 0x100>;
>  		interrupts = < 70 >;
> +		u-boot,pinmux = <0>;

That isn't acceptable to me for the same reasons I've outline many
times before while discussing USB.

At least periph-id can be argued to be related to the HW, but the pinmux
value is a hack that really has no place in device tree.

> +		speed = <100000>;

That's already defined as "clock-frequency" in the Linux kernel.

> +		u-boot,periph-id = <12>;	// PERIPH_ID_I2C1

-- 
nvpublic

[U-Boot] [PATCH 2/7] tegra: fdt: Add extra I2C definitions for U-Boot

[Simon Glass]

Hi Stephen,

On Mon, Dec 26, 2011 at 8:35 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Monday, December 26, 2011 11:12 AM:
>> Add U-Boot's peripheral ID and pinmux selection to the Tegra20
>> device tree file.
>
>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
> ...
>> ? ? ? ? ? ? ? compatible = "nvidia,tegra20-i2c";
>> ? ? ? ? ? ? ? reg = <0x7000C000 0x100>;
>> ? ? ? ? ? ? ? interrupts = < 70 >;
>> + ? ? ? ? ? ? u-boot,pinmux = <0>;
>
> That isn't acceptable to me for the same reasons I've outline many
> times before while discussing USB.
>
> At least periph-id can be argued to be related to the HW, but the pinmux
> value is a hack that really has no place in device tree.

Are you sure you have this right? The pinmux is set by the hardware,
since it is the board schematic which determines which SOC pins are
connected to the I2C bus, for example.

There are only a few possible combinations for each port, so I believe
it makes sense to represent these by a number, at least until we find
out what Linux will do.

But if not, what do you propose we do instead?

>
>> + ? ? ? ? ? ? speed = <100000>;
>
> That's already defined as "clock-frequency" in the Linux kernel.

OK thanks. I must be chasing a moving target here, as I got this
device tree from the -next branch of the kernel only a few weeks ago.
Can you please point me to the latest definition for i2c?

Regards,
Simon

>
>> + ? ? ? ? ? ? u-boot,periph-id = <12>; ? ? ? ?// PERIPH_ID_I2C1
>
> --
> nvpublic
>

[U-Boot] [PATCH 2/7] tegra: fdt: Add extra I2C definitions for U-Boot

[Stephen Warren]

Simon Glass wrote at Monday, December 26, 2011 10:15 PM:
> On Mon, Dec 26, 2011 at 8:35 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Simon Glass wrote at Monday, December 26, 2011 11:12 AM:
> >> Add U-Boot's peripheral ID and pinmux selection to the Tegra20
> >> device tree file.
> >
> >> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
> > ...
> >> ? ? ? ? ? ? ? compatible = "nvidia,tegra20-i2c";
> >> ? ? ? ? ? ? ? reg = <0x7000C000 0x100>;
> >> ? ? ? ? ? ? ? interrupts = < 70 >;
> >> + ? ? ? ? ? ? u-boot,pinmux = <0>;
> >
> > That isn't acceptable to me for the same reasons I've outline many
> > times before while discussing USB.
> >
> > At least periph-id can be argued to be related to the HW, but the pinmux
> > value is a hack that really has no place in device tree.
> 
> Are you sure you have this right? The pinmux is set by the hardware,
> since it is the board schematic which determines which SOC pins are
> connected to the I2C bus, for example.
> 
> There are only a few possible combinations for each port, so I believe
> it makes sense to represent these by a number, at least until we find
> out what Linux will do.

For some peripherals, the number of options is low.

For others, there are quite a few combinations.

Either way, there is no "select pinmux option N" for anything; those
"option N" values are something purely internal to the U-Boot driver,
and don't even come from tables in the Tegra documentation.

Either way as I've explained before, we can't add a temporary DT binding,
then put a proper one in place later, since that will make old .dts files
incompatible with new bootloaders or kernels.

> But if not, what do you propose we do instead?

Given there's still a lot of flux with DT, I'd suggest adding all the
drivers to U-Boot without any DT support at all. That should avoid all
the contentious issues. DT support can be added later. Preferably, adding
DT support for a given driver would happen at roughly the same time for
both U-Boot and the Linux kernel, and get review from DT experts from
both development teams. We may have to defer some aspects of DT support
quite some time, since areas such as clocks and pinmux may need quite
a bit of discussion to get right.

-- 
nvpublic

[U-Boot] [PATCH 2/7] tegra: fdt: Add extra I2C definitions for U-Boot

[Simon Glass]

Hi Stephen,

On Dec 28, 2011 10:40 PM, "Stephen Warren" <swarren@nvidia.com> wrote:
>
> Simon Glass wrote at Monday, December 26, 2011 10:15 PM:
> > On Mon, Dec 26, 2011 at 8:35 PM, Stephen Warren <swarren@nvidia.com>
wrote:
> > > Simon Glass wrote at Monday, December 26, 2011 11:12 AM:
> > >> Add U-Boot's peripheral ID and pinmux selection to the Tegra20
> > >> device tree file.
> > >
> > >> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
> > > ...
> > >>               compatible = "nvidia,tegra20-i2c";
> > >>               reg = <0x7000C000 0x100>;
> > >>               interrupts = < 70 >;
> > >> +             u-boot,pinmux = <0>;
> > >
> > > That isn't acceptable to me for the same reasons I've outline many
> > > times before while discussing USB.
> > >
> > > At least periph-id can be argued to be related to the HW, but the
pinmux
> > > value is a hack that really has no place in device tree.
> >
> > Are you sure you have this right? The pinmux is set by the hardware,
> > since it is the board schematic which determines which SOC pins are
> > connected to the I2C bus, for example.
> >
> > There are only a few possible combinations for each port, so I believe
> > it makes sense to represent these by a number, at least until we find
> > out what Linux will do.
>
> For some peripherals, the number of options is low.
>
> For others, there are quite a few combinations.
>
> Either way, there is no "select pinmux option N" for anything; those
> "option N" values are something purely internal to the U-Boot driver,
> and don't even come from tables in the Tegra documentation.
>




> Either way as I've explained before, we can't add a temporary DT binding,
> then put a proper one in place later, since that will make old .dts files
> incompatible with new bootloaders or kernels.

Well the fts is in U-Boot so when we change the bindings in U-Boot we
change the code. I don't see the problem.

>
> > But if not, what do you propose we do instead?
>
> Given there's still a lot of flux with DT, I'd suggest adding all the
> drivers to U-Boot without any DT support at all. That should avoid all
> the contentious issues. DT support can be added later. Preferably, adding
> DT support for a given driver would happen at roughly the same time for
> both U-Boot and the Linux kernel, and get review from DT experts from
> both development teams. We may have to defer some aspects of DT support
> quite some time, since areas such as clocks and pinmux may need quite
> a bit of discussion to get right.

I understand the desire to wait, but this seems less than ideal to me. Why
not just make some changes when the kernel makes up its mind?

Look, the fdt is just an agreement between the controller and the
user/driver on the bindings to use. I think we are over-thinking this...

Regards,
Simon

>
> --
> nvpublic
>

[U-Boot] [PATCH 3/7] tegra: Add I2C support to funcmux

[Simon Glass]

Add support to funcmux for selecting I2C functions and programming
the pinmux appropriately.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/funcmux.c        |   75 +++++++++++++++++++++++-----
 arch/arm/include/asm/arch-tegra2/funcmux.h |    3 +
 2 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c b/arch/arm/cpu/armv7/tegra2/funcmux.c
index 0878f51..82d994a 100644
--- a/arch/arm/cpu/armv7/tegra2/funcmux.c
+++ b/arch/arm/cpu/armv7/tegra2/funcmux.c
@@ -26,27 +26,70 @@
 
 int funcmux_select(enum periph_id id, int config)
 {
-	if (config != 0) {
-		debug("%s: invalid config %d for periph_id %d", __func__,
-		      config, id);
-		return -1;
-	}
+	int bad_config = config != 0;
+
 	switch (id) {
 	case PERIPH_ID_UART1:
-		pinmux_set_func(PINGRP_IRRX, PMUX_FUNC_UARTA);
-		pinmux_set_func(PINGRP_IRTX, PMUX_FUNC_UARTA);
-		pinmux_tristate_disable(PINGRP_IRRX);
-		pinmux_tristate_disable(PINGRP_IRTX);
+		if (config == 0) {
+			pinmux_set_func(PINGRP_IRRX, PMUX_FUNC_UARTA);
+			pinmux_set_func(PINGRP_IRTX, PMUX_FUNC_UARTA);
+			pinmux_tristate_disable(PINGRP_IRRX);
+			pinmux_tristate_disable(PINGRP_IRTX);
+		}
 		break;
 
 	case PERIPH_ID_UART2:
-		pinmux_set_func(PINGRP_UAD, PMUX_FUNC_IRDA);
-		pinmux_tristate_disable(PINGRP_UAD);
+		if (config == 0) {
+			pinmux_set_func(PINGRP_UAD, PMUX_FUNC_IRDA);
+			pinmux_tristate_disable(PINGRP_UAD);
+		}
 		break;
 
 	case PERIPH_ID_UART4:
-		pinmux_set_func(PINGRP_GMC, PMUX_FUNC_UARTD);
-		pinmux_tristate_disable(PINGRP_GMC);
+		if (config == 0) {
+			pinmux_set_func(PINGRP_GMC, PMUX_FUNC_UARTD);
+			pinmux_tristate_disable(PINGRP_GMC);
+		}
+		break;
+
+	case PERIPH_ID_DVC_I2C:
+		/* there is only one selection, pinmux_config is ignored */
+		if (config == 0) {
+			pinmux_set_func(PINGRP_I2CP, PMUX_FUNC_I2C);
+			pinmux_tristate_disable(PINGRP_I2CP);
+		}
+		break;
+
+	case PERIPH_ID_I2C1:
+		/* support pinmux_config of 0 for now, */
+		if (config == 0) {
+			pinmux_set_func(PINGRP_RM, PMUX_FUNC_I2C);
+			pinmux_tristate_disable(PINGRP_RM);
+		}
+		break;
+	case PERIPH_ID_I2C2: /* I2C2 */
+		switch (config) {
+		case 0:	/* DDC pin group, select I2C2 */
+			pinmux_set_func(PINGRP_DDC, PMUX_FUNC_I2C2);
+			/* PTA to HDMI */
+			pinmux_set_func(PINGRP_PTA, PMUX_FUNC_HDMI);
+			pinmux_tristate_disable(PINGRP_DDC);
+			break;
+		case 1:	/* PTA pin group, select I2C2 */
+			pinmux_set_func(PINGRP_PTA, PMUX_FUNC_I2C2);
+			/* set DDC_SEL to RSVDx (RSVD2 works for now) */
+			pinmux_set_func(PINGRP_DDC, PMUX_FUNC_RSVD2);
+			pinmux_tristate_disable(PINGRP_PTA);
+			bad_config = 0;
+			break;
+		}
+		break;
+	case PERIPH_ID_I2C3: /* I2C3 */
+		/* support pinmux_config of 0 for now */
+		if (config == 0) {
+			pinmux_set_func(PINGRP_DTF, PMUX_FUNC_I2C3);
+			pinmux_tristate_disable(PINGRP_DTF);
+		}
 		break;
 
 	default:
@@ -54,5 +97,11 @@ int funcmux_select(enum periph_id id, int config)
 		return -1;
 	}
 
+	if (bad_config) {
+		debug("%s: invalid config %d for periph_id %d", __func__,
+		      config, id);
+		return -1;
+	}
+
 	return 0;
 }
diff --git a/arch/arm/include/asm/arch-tegra2/funcmux.h b/arch/arm/include/asm/arch-tegra2/funcmux.h
index 2d724a2..d4f9cfb 100644
--- a/arch/arm/include/asm/arch-tegra2/funcmux.h
+++ b/arch/arm/include/asm/arch-tegra2/funcmux.h
@@ -32,6 +32,9 @@
  * The basic config is 0, and higher numbers indicate different
  * pinmux settings to bring the peripheral out on other pins,
  *
+ * This function also disables tristate for the function's pins,
+ * so that they operate in normal mode.
+ *
  * @param id		Peripheral id
  * @param config	Configuration to use (generally 0)
  * @return 0 if ok, -1 on error (e.g. incorrect id or config)
-- 
1.7.3.1

[U-Boot] [PATCH 3/7] tegra: Add I2C support to funcmux

[Stephen Warren]

On 12/26/2011 11:11 AM, Simon Glass wrote:
> Add support to funcmux for selecting I2C functions and programming
> the pinmux appropriately.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Conceptually the changes here look fine. I think that adding the new I2C
cases and the tests for valid config values to the existing cases are
probably worthing of two separate patches though.

-- 
nvpublic

[U-Boot] [PATCH 3/7] tegra: Add I2C support to funcmux

[Simon Glass]

Hi Stephen,

On Mon, Jan 9, 2012 at 1:36 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 12/26/2011 11:11 AM, Simon Glass wrote:
>> Add support to funcmux for selecting I2C functions and programming
>> the pinmux appropriately.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Conceptually the changes here look fine. I think that adding the new I2C
> cases and the tests for valid config values to the existing cases are
> probably worthing of two separate patches though.

OK, I might do this as part of the mmc side.

Regards,
Simon

>
> --
> nvpublic

[U-Boot] [PATCH 3/7] tegra: Add I2C support to funcmux

[Simon Glass]

Hi Stephe,

On Mon, Jan 9, 2012 at 1:40 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Stephen,
>
> On Mon, Jan 9, 2012 at 1:36 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> On 12/26/2011 11:11 AM, Simon Glass wrote:
>>> Add support to funcmux for selecting I2C functions and programming
>>> the pinmux appropriately.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>
>> Conceptually the changes here look fine. I think that adding the new I2C
>> cases and the tests for valid config values to the existing cases are
>> probably worthing of two separate patches though.
>
> OK, I might do this as part of the mmc side.

Since there are a few things there now I have created a separate
funcmux series and moved this I2C patch into that. I will drop it from
here when I send the next version.

Regards,
Simon

>
> Regards,
> Simon
>
>>
>> --
>> nvpublic

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Simon Glass]

From: Yen Lin <yelin@nvidia.com>

Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
The driver supports building both with and without CONFIG_OF_CONTROL.

Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
in the board config header file:

I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
	(typically this will be 0 to bring the port out the common
		pins)

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/include/asm/arch-tegra2/tegra2.h     |    4 +
 arch/arm/include/asm/arch-tegra2/tegra2_i2c.h |  167 ++++++++
 drivers/i2c/Makefile                          |    1 +
 drivers/i2c/tegra2_i2c.c                      |  533 +++++++++++++++++++++++++
 include/fdtdec.h                              |    1 +
 lib/fdtdec.c                                  |    1 +
 6 files changed, 707 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
 create mode 100644 drivers/i2c/tegra2_i2c.c

diff --git a/arch/arm/include/asm/arch-tegra2/tegra2.h b/arch/arm/include/asm/arch-tegra2/tegra2.h
index ca1881e..d1a70da 100644
--- a/arch/arm/include/asm/arch-tegra2/tegra2.h
+++ b/arch/arm/include/asm/arch-tegra2/tegra2.h
@@ -40,6 +40,10 @@
 #define NV_PA_APB_UARTE_BASE	(NV_PA_APB_MISC_BASE + 0x6400)
 #define TEGRA2_SPI_BASE		(NV_PA_APB_MISC_BASE + 0xC380)
 #define TEGRA2_PMC_BASE		(NV_PA_APB_MISC_BASE + 0xE400)
+#define TEGRA2_I2C1_BASE	(NV_PA_APB_MISC_BASE + 0xC000)
+#define TEGRA2_I2C2_BASE	(NV_PA_APB_MISC_BASE + 0xC400)
+#define TEGRA2_I2C3_BASE	(NV_PA_APB_MISC_BASE + 0xC500)
+#define TEGRA2_DVC_BASE		(NV_PA_APB_MISC_BASE + 0xD000)
 #define NV_PA_CSITE_BASE	0x70040000
 #define TEGRA_USB1_BASE		0xC5000000
 #define TEGRA_USB3_BASE		0xC5008000
diff --git a/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
new file mode 100644
index 0000000..4920d47
--- /dev/null
+++ b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
@@ -0,0 +1,167 @@
+/*
+ * NVIDIA Tegra2 I2C controller
+ *
+ * Copyright 2010-2011 NVIDIA Corporation
+ *
+ * This software may be used and distributed according to the
+ * terms of the GNU Public License, Version 2, incorporated
+ * herein by reference.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * Version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _TEGRA2_I2C_H_
+#define _TEGRA2_I2C_H_
+
+#include <asm/types.h>
+
+/* Convert the number of bytes to word. */
+#define BYTES_TO_WORDS(size)		(((size) + 3) >> 2)
+
+/* Convert i2c slave address to be put on bus  */
+#define I2C_ADDR_ON_BUS(chip)		(chip << 1)
+
+#ifndef CONFIG_OF_CONTROL
+enum {
+	I2CSPEED_KHZ = 100,		/* in KHz */
+};
+#endif
+
+enum {
+	I2C_TIMEOUT_USEC = 10000,	/* Wait time for completion */
+	I2C_FIFO_DEPTH = 8,		/* I2C fifo depth */
+};
+
+enum i2c_transaction_flags {
+	I2C_IS_WRITE = 0x1,		/* for I2C write operation */
+	I2C_IS_10_BIT_ADDRESS = 0x2,	/* for 10-bit I2C slave address */
+	I2C_USE_REPEATED_START = 0x4,	/* for repeat start */
+	I2C_NO_ACK = 0x8,		/* for slave that won't generate ACK */
+	I2C_SOFTWARE_CONTROLLER	= 0x10,	/* for I2C transfer using GPIO */
+	I2C_NO_STOP = 0x20,
+};
+
+/* Contians the I2C transaction details */
+struct i2c_trans_info {
+	/* flags to indicate the transaction details */
+	enum i2c_transaction_flags flags;
+	u32 address;	/* I2C slave device address */
+	u32 num_bytes;	/* number of bytes to be transferred */
+	/* Send/receive buffer. For I2C send operation this buffer should be
+	 * filled with the data to be sent to the slave device. For I2C receive
+	 * operation this buffer is filled with the data received from the
+	 * slave device. */
+	u8 *buf;
+	int is_10bit_address;
+};
+
+struct i2c_control {
+	u32 tx_fifo;
+	u32 rx_fifo;
+	u32 packet_status;
+	u32 fifo_control;
+	u32 fifo_status;
+	u32 int_mask;
+	u32 int_status;
+};
+
+struct dvc_ctlr {
+	u32 ctrl1;			/* 00: DVC_CTRL_REG1 */
+	u32 ctrl2;			/* 04: DVC_CTRL_REG2 */
+	u32 ctrl3;			/* 08: DVC_CTRL_REG3 */
+	u32 status;			/* 0C: DVC_STATUS_REG */
+	u32 ctrl;			/* 10: DVC_I2C_CTRL_REG */
+	u32 addr_data;			/* 14: DVC_I2C_ADDR_DATA_REG */
+	u32 reserved_0[2];		/* 18: */
+	u32 req;			/* 20: DVC_REQ_REGISTER */
+	u32 addr_data3;			/* 24: DVC_I2C_ADDR_DATA_REG_3 */
+	u32 reserved_1[6];		/* 28: */
+	u32 cnfg;			/* 40: DVC_I2C_CNFG */
+	u32 cmd_addr0;			/* 44: DVC_I2C_CMD_ADDR0 */
+	u32 cmd_addr1;			/* 48: DVC_I2C_CMD_ADDR1 */
+	u32 cmd_data1;			/* 4C: DVC_I2C_CMD_DATA1 */
+	u32 cmd_data2;			/* 50: DVC_I2C_CMD_DATA2 */
+	u32 reserved_2[2];		/* 54: */
+	u32 i2c_status;			/* 5C: DVC_I2C_STATUS */
+	struct i2c_control control;	/* 60 ~ 78 */
+};
+
+struct i2c_ctlr {
+	u32 cnfg;			/* 00: I2C_I2C_CNFG */
+	u32 cmd_addr0;			/* 04: I2C_I2C_CMD_ADDR0 */
+	u32 cmd_addr1;			/* 08: I2C_I2C_CMD_DATA1 */
+	u32 cmd_data1;			/* 0C: I2C_I2C_CMD_DATA2 */
+	u32 cmd_data2;			/* 10: DVC_I2C_CMD_DATA2 */
+	u32 reserved_0[2];		/* 14: */
+	u32 status;			/* 1C: I2C_I2C_STATUS */
+	u32 sl_cnfg;			/* 20: I2C_I2C_SL_CNFG */
+	u32 sl_rcvd;			/* 24: I2C_I2C_SL_RCVD */
+	u32 sl_status;			/* 28: I2C_I2C_SL_STATUS */
+	u32 sl_addr1;			/* 2C: I2C_I2C_SL_ADDR1 */
+	u32 sl_addr2;			/* 30: I2C_I2C_SL_ADDR2 */
+	u32 reserved_1[2];		/* 34: */
+	u32 sl_delay_count;		/* 3C: I2C_I2C_SL_DELAY_COUNT */
+	u32 reserved_2[4];		/* 40: */
+	struct i2c_control control;	/* 50 ~ 68 */
+};
+
+/* bit fields definitions for IO Packet Header 1 format */
+#define PKT_HDR1_PROTOCOL_SHIFT		4
+#define PKT_HDR1_PROTOCOL_MASK		(0xf << PKT_HDR1_PROTOCOL_SHIFT)
+#define PKT_HDR1_CTLR_ID_SHIFT		12
+#define PKT_HDR1_CTLR_ID_MASK		(0xf << PKT_HDR1_CTLR_ID_SHIFT)
+#define PKT_HDR1_PKT_ID_SHIFT		16
+#define PKT_HDR1_PKT_ID_MASK		(0xff << PKT_HDR1_PKT_ID_SHIFT)
+#define PROTOCOL_TYPE_I2C		1
+
+/* bit fields definitions for IO Packet Header 2 format */
+#define PKT_HDR2_PAYLOAD_SIZE_SHIFT	0
+#define PKT_HDR2_PAYLOAD_SIZE_MASK	(0xfff << PKT_HDR2_PAYLOAD_SIZE_SHIFT)
+
+/* bit fields definitions for IO Packet Header 3 format */
+#define PKT_HDR3_READ_MODE_SHIFT	19
+#define PKT_HDR3_READ_MODE_MASK		(1 << PKT_HDR3_READ_MODE_SHIFT)
+#define PKT_HDR3_SLAVE_ADDR_SHIFT	0
+#define PKT_HDR3_SLAVE_ADDR_MASK	(0x3ff << PKT_HDR3_SLAVE_ADDR_SHIFT)
+
+#define DVC_CTRL_REG3_I2C_HW_SW_PROG_SHIFT	26
+#define DVC_CTRL_REG3_I2C_HW_SW_PROG_MASK	\
+				(1 << DVC_CTRL_REG3_I2C_HW_SW_PROG_SHIFT)
+
+/* I2C_CNFG */
+#define I2C_CNFG_NEW_MASTER_FSM_SHIFT	11
+#define I2C_CNFG_NEW_MASTER_FSM_MASK	(1 << I2C_CNFG_NEW_MASTER_FSM_SHIFT)
+#define I2C_CNFG_PACKET_MODE_SHIFT	10
+#define I2C_CNFG_PACKET_MODE_MASK	(1 << I2C_CNFG_PACKET_MODE_SHIFT)
+
+/* I2C_SL_CNFG */
+#define I2C_SL_CNFG_NEWSL_SHIFT		2
+#define I2C_SL_CNFG_NEWSL_MASK		(1 << I2C_SL_CNFG_NEWSL_SHIFT)
+
+/* I2C_FIFO_STATUS */
+#define TX_FIFO_FULL_CNT_SHIFT		0
+#define TX_FIFO_FULL_CNT_MASK		(0xf << TX_FIFO_FULL_CNT_SHIFT)
+#define TX_FIFO_EMPTY_CNT_SHIFT		4
+#define TX_FIFO_EMPTY_CNT_MASK		(0xf << TX_FIFO_EMPTY_CNT_SHIFT)
+
+/* I2C_INTERRUPT_STATUS */
+#define I2C_INT_XFER_COMPLETE_SHIFT	7
+#define I2C_INT_XFER_COMPLETE_MASK	(1 << I2C_INT_XFER_COMPLETE_SHIFT)
+#define I2C_INT_NO_ACK_SHIFT		3
+#define I2C_INT_NO_ACK_MASK		(1 << I2C_INT_NO_ACK_SHIFT)
+#define I2C_INT_ARBITRATION_LOST_SHIFT	2
+#define I2C_INT_ARBITRATION_LOST_MASK	(1 << I2C_INT_ARBITRATION_LOST_SHIFT)
+
+#endif
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 504db03..c123c72 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -41,6 +41,7 @@ COBJS-$(CONFIG_DRIVER_S3C24X0_I2C) += s3c24x0_i2c.o
 COBJS-$(CONFIG_S3C44B0_I2C) += s3c44b0_i2c.o
 COBJS-$(CONFIG_SOFT_I2C) += soft_i2c.o
 COBJS-$(CONFIG_SPEAR_I2C) += spr_i2c.o
+COBJS-$(CONFIG_TEGRA2_I2C) += tegra2_i2c.o
 COBJS-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
 COBJS-$(CONFIG_U8500_I2C) += u8500_i2c.o
 COBJS-$(CONFIG_SH_I2C) += sh_i2c.o
diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
new file mode 100644
index 0000000..ae43531
--- /dev/null
+++ b/drivers/i2c/tegra2_i2c.c
@@ -0,0 +1,533 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+ * Copyright (c) 2010-2011 NVIDIA Corporation
+ *  NVIDIA Corporation <www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/clk_rst.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/funcmux.h>
+#include <asm/arch/gpio.h>
+#include <asm/arch/pinmux.h>
+#include <asm/arch/tegra2_i2c.h>
+#include <fdtdec.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static unsigned int i2c_bus_num;
+
+/* Information about i2c controller */
+struct i2c_bus {
+	int			id;
+	enum periph_id		periph_id;
+	int			speed;
+	int			pinmux_config;
+	struct i2c_control	*control;
+	struct i2c_ctlr		*regs;
+};
+
+struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
+
+static void set_packet_mode(struct i2c_bus *i2c_bus)
+{
+	u32 config;
+
+	config = I2C_CNFG_NEW_MASTER_FSM_MASK | I2C_CNFG_PACKET_MODE_MASK;
+
+	if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C) {
+		struct dvc_ctlr *dvc = (struct dvc_ctlr *)i2c_bus->regs;
+
+		writel(config, &dvc->cnfg);
+	} else {
+		writel(config, &i2c_bus->regs->cnfg);
+		/*
+		 * program I2C_SL_CNFG.NEWSL to ENABLE. This fixes probe
+		 * issues, i.e., some slaves may be wrongly detected.
+		 */
+		setbits_le32(&i2c_bus->regs->sl_cnfg, I2C_SL_CNFG_NEWSL_MASK);
+	}
+}
+
+static void i2c_reset_controller(struct i2c_bus *i2c_bus)
+{
+	/* Reset I2C controller. */
+	reset_periph(i2c_bus->periph_id, 1);
+
+	/* re-program config register to packet mode */
+	set_packet_mode(i2c_bus);
+}
+
+static void i2c_init_controller(struct i2c_bus *i2c_bus)
+{
+	/* TODO: Fix bug which makes us need to do this */
+	clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_OSC,
+			       i2c_bus->speed * (8 * 2 - 1));
+
+	/* Reset I2C controller. */
+	i2c_reset_controller(i2c_bus);
+
+	/* Configure I2C controller. */
+	if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C) {	/* only for DVC I2C */
+		struct dvc_ctlr *dvc = (struct dvc_ctlr *)i2c_bus->regs;
+
+		setbits_le32(&dvc->ctrl3, DVC_CTRL_REG3_I2C_HW_SW_PROG_MASK);
+	}
+
+	funcmux_select(i2c_bus->periph_id, i2c_bus->pinmux_config);
+}
+
+static void send_packet_headers(
+	struct i2c_bus *i2c_bus,
+	struct i2c_trans_info *trans,
+	u32 packet_id)
+{
+	u32 data;
+
+	/* prepare header1: Header size = 0 Protocol = I2C, pktType = 0 */
+	data = PROTOCOL_TYPE_I2C << PKT_HDR1_PROTOCOL_SHIFT;
+	data |= packet_id << PKT_HDR1_PKT_ID_SHIFT;
+	data |= i2c_bus->id << PKT_HDR1_CTLR_ID_SHIFT;
+	writel(data, &i2c_bus->control->tx_fifo);
+	debug("pkt header 1 sent (0x%x)\n", data);
+
+	/* prepare header2 */
+	data = (trans->num_bytes - 1) << PKT_HDR2_PAYLOAD_SIZE_SHIFT;
+	writel(data, &i2c_bus->control->tx_fifo);
+	debug("pkt header 2 sent (0x%x)\n", data);
+
+	/* prepare IO specific header: configure the slave address */
+	data = trans->address << PKT_HDR3_SLAVE_ADDR_SHIFT;
+
+	/* Enable Read if it is not a write transaction */
+	if (!(trans->flags & I2C_IS_WRITE))
+		data |= PKT_HDR3_READ_MODE_MASK;
+
+	/* Write I2C specific header */
+	writel(data, &i2c_bus->control->tx_fifo);
+	debug("pkt header 3 sent (0x%x)\n", data);
+}
+
+static int wait_for_tx_fifo_empty(struct i2c_control *control)
+{
+	u32 count;
+	int timeout_us = I2C_TIMEOUT_USEC;
+
+	while (timeout_us >= 0) {
+		count = (readl(&control->fifo_status) & TX_FIFO_EMPTY_CNT_MASK)
+				>> TX_FIFO_EMPTY_CNT_SHIFT;
+		if (count == I2C_FIFO_DEPTH)
+			return 1;
+		udelay(10);
+		timeout_us -= 10;
+	};
+
+	return 0;
+}
+
+static int wait_for_rx_fifo_notempty(struct i2c_control *control)
+{
+	u32 count;
+	int timeout_us = I2C_TIMEOUT_USEC;
+
+	while (timeout_us >= 0) {
+		count = (readl(&control->fifo_status) & TX_FIFO_FULL_CNT_MASK)
+				>> TX_FIFO_FULL_CNT_SHIFT;
+		if (count)
+			return 1;
+		udelay(10);
+		timeout_us -= 10;
+	};
+
+	return 0;
+}
+
+static int wait_for_transfer_complete(struct i2c_control *control)
+{
+	int int_status;
+	int timeout_us = I2C_TIMEOUT_USEC;
+
+	while (timeout_us >= 0) {
+		int_status = readl(&control->int_status);
+		if (int_status & I2C_INT_NO_ACK_MASK)
+			return -int_status;
+		if (int_status & I2C_INT_ARBITRATION_LOST_MASK)
+			return -int_status;
+		if (int_status & I2C_INT_XFER_COMPLETE_MASK)
+			return 0;
+
+		udelay(10);
+		timeout_us -= 10;
+	};
+
+	return -1;
+}
+
+static int send_recv_packets(
+	struct i2c_bus *i2c_bus,
+	struct i2c_trans_info *trans)
+{
+	struct i2c_control *control = i2c_bus->control;
+	u32 int_status;
+	u32 words;
+	u8 *dptr;
+	u32 local;
+	uchar last_bytes;
+	int error = 0;
+	int is_write = trans->flags & I2C_IS_WRITE;
+
+	/* clear status from previous transaction, XFER_COMPLETE, NOACK, etc. */
+	int_status = readl(&control->int_status);
+	writel(int_status, &control->int_status);
+
+	send_packet_headers(i2c_bus, trans, 1);
+
+	words = BYTES_TO_WORDS(trans->num_bytes);
+	last_bytes = trans->num_bytes & 3;
+	dptr = trans->buf;
+
+	while (words) {
+		if (is_write) {
+			/* deal with word alignment */
+			if ((unsigned)dptr & 3) {
+				memcpy(&local, dptr, sizeof(u32));
+				writel(local, &control->tx_fifo);
+				debug("pkt data sent (0x%x)\n", local);
+			} else {
+				writel(*(u32 *)dptr, &control->tx_fifo);
+				debug("pkt data sent (0x%x)\n", *(u32 *)dptr);
+			}
+			if (!wait_for_tx_fifo_empty(control)) {
+				error = -1;
+				goto exit;
+			}
+		} else {
+			if (!wait_for_rx_fifo_notempty(control)) {
+				error = -1;
+				goto exit;
+			}
+			/*
+			 * for the last word, we read into our local buffer,
+			 * in case that caller did not provide enough buffer.
+			 */
+			local = readl(&control->rx_fifo);
+			if ((words == 1) && last_bytes)
+				memcpy(dptr, (char *)&local, last_bytes);
+			else if ((unsigned)dptr & 3)
+				memcpy(dptr, &local, sizeof(u32));
+			else
+				*(u32 *)dptr = local;
+			debug("pkt data received (0x%x)\n", local);
+		}
+		words--;
+		dptr += sizeof(u32);
+	}
+
+	if (wait_for_transfer_complete(control)) {
+		error = -1;
+		goto exit;
+	}
+	return 0;
+exit:
+	/* error, reset the controller. */
+	i2c_reset_controller(i2c_bus);
+
+	return error;
+}
+
+static int tegra2_i2c_write_data(u32 addr, u8 *data, u32 len)
+{
+	int error;
+	struct i2c_trans_info trans_info;
+
+	trans_info.address = addr;
+	trans_info.buf = data;
+	trans_info.flags = I2C_IS_WRITE;
+	trans_info.num_bytes = len;
+	trans_info.is_10bit_address = 0;
+
+	error = send_recv_packets(&i2c_controllers[i2c_bus_num], &trans_info);
+	if (error)
+		debug("tegra2_i2c_write_data: Error (%d) !!!\n", error);
+
+	return error;
+}
+
+static int tegra2_i2c_read_data(u32 addr, u8 *data, u32 len)
+{
+	int error;
+	struct i2c_trans_info trans_info;
+
+	trans_info.address = addr | 1;
+	trans_info.buf = data;
+	trans_info.flags = 0;
+	trans_info.num_bytes = len;
+	trans_info.is_10bit_address = 0;
+
+	error = send_recv_packets(&i2c_controllers[i2c_bus_num], &trans_info);
+	if (error)
+		debug("tegra2_i2c_read_data: Error (%d) !!!\n", error);
+
+	return error;
+}
+
+#ifndef CONFIG_OF_CONTROL
+static const enum periph_id i2c_periph_ids[CONFIG_SYS_MAX_I2C_BUS] = {
+	PERIPH_ID_DVC_I2C,
+	PERIPH_ID_I2C1,
+	PERIPH_ID_I2C2,
+	PERIPH_ID_I2C3
+};
+
+static const u32 *i2c_bus_base[CONFIG_SYS_MAX_I2C_BUS] = {
+	(u32 *)TEGRA2_DVC_BASE,
+	(u32 *)TEGRA2_I2C1_BASE,
+	(u32 *)TEGRA2_I2C2_BASE,
+	(u32 *)TEGRA2_I2C3_BASE
+};
+
+/* pinmux_configs based on the pinmux configuration */
+static const int pinmux_configs[CONFIG_SYS_MAX_I2C_BUS] = {
+	CONFIG_I2CP_PIN_MUX,	/* for I2CP (DVC I2C) */
+	CONFIG_I2C1_PIN_MUX,	/* for I2C1 */
+	CONFIG_I2C2_PIN_MUX,	/* for I2C2 */
+	CONFIG_I2C3_PIN_MUX	/* for I2C3 */
+};
+
+static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
+{
+	int i = *index;
+
+	if (i >= CONFIG_SYS_MAX_I2C_BUS)
+		return -1;
+
+	i2c_bus->periph_id = i2c_periph_ids[i];
+	i2c_bus->pinmux_config = pinmux_configs[i];
+	i2c_bus->regs = (struct i2c_ctlr *)i2c_bus_base[i];
+	i2c_bus->speed = I2CSPEED_KHZ * 1000;
+
+	*index = i + 1;
+
+	return 0;
+}
+#else
+static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
+{
+	const void *blob = gd->fdt_blob;
+	int node;
+
+	node = fdtdec_next_alias(blob, "i2c", COMPAT_NVIDIA_TEGRA20_I2C,
+				 index);
+	if (node < 0)
+		return -1;
+
+	i2c_bus->regs = (struct i2c_ctlr *)fdtdec_get_addr(blob, node, "reg");
+	i2c_bus->pinmux_config = fdtdec_get_int(blob, node, "u-boot,pinmux", 0);
+	i2c_bus->speed = fdtdec_get_int(blob, node, "speed", 0);
+	i2c_bus->periph_id = fdtdec_get_int(blob, node, "u-boot,periph-id", -1);
+
+	if (i2c_bus->periph_id == -1)
+		return -FDT_ERR_NOTFOUND;
+
+	return 0;
+}
+#endif
+
+int i2c_init_board(void)
+{
+	struct i2c_bus *i2c_bus;
+	int index = 0;
+	int i;
+
+	/* build the i2c_controllers[] for each controller */
+	for (i = 0; i < CONFIG_SYS_MAX_I2C_BUS; ++i) {
+		i2c_bus = &i2c_controllers[i];
+		i2c_bus->id = i;
+
+		if (i2c_get_config(&index, i2c_bus)) {
+			printf("i2c_init_board: failed to find bus %d\n", i);
+			return -1;
+		}
+
+		if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C)
+			i2c_bus->control =
+				&((struct dvc_ctlr *)i2c_bus->regs)->control;
+		else
+			i2c_bus->control = &i2c_bus->regs->control;
+
+		i2c_init_controller(i2c_bus);
+	}
+
+	return 0;
+}
+
+void i2c_init(int speed, int slaveaddr)
+{
+	debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
+}
+
+/* i2c write version without the register address */
+int i2c_write_data(uchar chip, uchar *buffer, int len)
+{
+	int rc;
+
+	debug("i2c_write_data: chip=0x%x, len=0x%x\n", chip, len);
+	debug("write_data: ");
+	/* use rc for counter */
+	for (rc = 0; rc < len; ++rc)
+		debug(" 0x%02x", buffer[rc]);
+	debug("\n");
+
+	rc = tegra2_i2c_write_data(I2C_ADDR_ON_BUS(chip), buffer, len);
+	if (rc)
+		debug("i2c_write_data(): rc=%d\n", rc);
+
+	return rc;
+}
+
+/* i2c read version without the register address */
+int i2c_read_data(uchar chip, uchar *buffer, int len)
+{
+	int rc;
+
+	debug("inside i2c_read_data():\n");
+	rc = tegra2_i2c_read_data(I2C_ADDR_ON_BUS(chip), buffer, len);
+	if (rc) {
+		debug("i2c_read_data(): rc=%d\n", rc);
+		return rc;
+	}
+
+	debug("i2c_read_data: ");
+	/* reuse rc for counter*/
+	for (rc = 0; rc < len; ++rc)
+		debug(" 0x%02x", buffer[rc]);
+	debug("\n");
+
+	return 0;
+}
+
+/* Probe to see if a chip is present. */
+int i2c_probe(uchar chip)
+{
+	int rc;
+	uchar reg;
+
+	debug("i2c_probe: addr=0x%x\n", chip);
+	reg = 0;
+	rc = i2c_write_data(chip, &reg, 1);
+	if (rc) {
+		debug("Error probing 0x%x.\n", chip);
+		return 1;
+	}
+	return 0;
+}
+
+static int i2c_addr_ok(const uint addr, const int alen)
+{
+	if (alen < 0 || alen > sizeof(addr))
+		return 0;
+	if (alen != sizeof(addr)) {
+		uint max_addr = (1 << (8 * alen)) - 1;
+		if (addr > max_addr)
+			return 0;
+	}
+	return 1;
+}
+
+/* Read bytes */
+int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
+{
+	uint offset;
+	int i;
+
+	debug("i2c_read: chip=0x%x, addr=0x%x, len=0x%x\n",
+				chip, addr, len);
+	if (!i2c_addr_ok(addr, alen)) {
+		debug("i2c_read: Bad address %x.%d.\n", addr, alen);
+		return 1;
+	}
+	for (offset = 0; offset < len; offset++) {
+		if (alen) {
+			uchar data[alen];
+			for (i = 0; i < alen; i++) {
+				data[alen - i - 1] =
+					(addr + offset) >> (8 * i);
+			}
+			if (i2c_write_data(chip, data, alen)) {
+				debug("i2c_read: error sending (0x%x)\n",
+					addr);
+				return 1;
+			}
+		}
+		if (i2c_read_data(chip, buffer + offset, 1)) {
+			debug("i2c_read: error reading (0x%x)\n", addr);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+/* Write bytes */
+int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
+{
+	uint offset;
+	int i;
+
+	debug("i2c_write: chip=0x%x, addr=0x%x, len=0x%x\n",
+				chip, addr, len);
+	if (!i2c_addr_ok(addr, alen)) {
+		debug("i2c_write: Bad address %x.%d.\n", addr, alen);
+		return 1;
+	}
+	for (offset = 0; offset < len; offset++) {
+		uchar data[alen + 1];
+		for (i = 0; i < alen; i++)
+			data[alen - i - 1] = (addr + offset) >> (8 * i);
+		data[alen] = buffer[offset];
+		if (i2c_write_data(chip, data, alen + 1)) {
+			debug("i2c_write: error sending (0x%x)\n", addr);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+#if defined(CONFIG_I2C_MULTI_BUS)
+/*
+ * Functions for multiple I2C bus handling
+ */
+unsigned int i2c_get_bus_num(void)
+{
+	return i2c_bus_num;
+}
+
+int i2c_set_bus_num(unsigned int bus)
+{
+	if (bus >= CONFIG_SYS_MAX_I2C_BUS)
+		return -1;
+	i2c_bus_num = bus;
+
+	return 0;
+}
+#endif
diff --git a/include/fdtdec.h b/include/fdtdec.h
index a8911b5..5547676 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -58,6 +58,7 @@ struct fdt_memory {
 enum fdt_compat_id {
 	COMPAT_UNKNOWN,
 	COMPAT_NVIDIA_TEGRA20_USB,	/* Tegra2 USB port */
+	COMPAT_NVIDIA_TEGRA20_I2C,	/* Tegra2 i2c */
 
 	COMPAT_COUNT,
 };
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 931b4ce..fb3d79d 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -38,6 +38,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_TEGRA20_I2C, "nvidia,tegra20-i2c"),
 };
 
 /**
-- 
1.7.3.1

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Marek Vasut]

> From: Yen Lin <yelin@nvidia.com>
> 
> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
> The driver supports building both with and without CONFIG_OF_CONTROL.
> 
> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
> in the board config header file:
> 
> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
> 	(typically this will be 0 to bring the port out the common
> 		pins)
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/arm/include/asm/arch-tegra2/tegra2.h     |    4 +
>  arch/arm/include/asm/arch-tegra2/tegra2_i2c.h |  167 ++++++++
>  drivers/i2c/Makefile                          |    1 +
>  drivers/i2c/tegra2_i2c.c                      |  533
> +++++++++++++++++++++++++ include/fdtdec.h                              | 
>   1 +
>  lib/fdtdec.c                                  |    1 +
>  6 files changed, 707 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
>  create mode 100644 drivers/i2c/tegra2_i2c.c
> 
> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2.h
> b/arch/arm/include/asm/arch-tegra2/tegra2.h index ca1881e..d1a70da 100644
> --- a/arch/arm/include/asm/arch-tegra2/tegra2.h
> +++ b/arch/arm/include/asm/arch-tegra2/tegra2.h
> @@ -40,6 +40,10 @@
>  #define NV_PA_APB_UARTE_BASE	(NV_PA_APB_MISC_BASE + 0x6400)
>  #define TEGRA2_SPI_BASE		(NV_PA_APB_MISC_BASE + 0xC380)
>  #define TEGRA2_PMC_BASE		(NV_PA_APB_MISC_BASE + 0xE400)
> +#define TEGRA2_I2C1_BASE	(NV_PA_APB_MISC_BASE + 0xC000)
> +#define TEGRA2_I2C2_BASE	(NV_PA_APB_MISC_BASE + 0xC400)
> +#define TEGRA2_I2C3_BASE	(NV_PA_APB_MISC_BASE + 0xC500)
> +#define TEGRA2_DVC_BASE		(NV_PA_APB_MISC_BASE + 0xD000)
>  #define NV_PA_CSITE_BASE	0x70040000
>  #define TEGRA_USB1_BASE		0xC5000000
>  #define TEGRA_USB3_BASE		0xC5008000
> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
> b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h new file mode 100644
> index 0000000..4920d47
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
> @@ -0,0 +1,167 @@
> +/*
> + * NVIDIA Tegra2 I2C controller
> + *
> + * Copyright 2010-2011 NVIDIA Corporation
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * Version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef _TEGRA2_I2C_H_
> +#define _TEGRA2_I2C_H_
> +
> +#include <asm/types.h>
> +
> +/* Convert the number of bytes to word. */
> +#define BYTES_TO_WORDS(size)		(((size) + 3) >> 2)

Isn't this stuff in include/common.h ? div_round_up() or something ?
> +
> +/* Convert i2c slave address to be put on bus  */
> +#define I2C_ADDR_ON_BUS(chip)		(chip << 1)
> +
> +#ifndef CONFIG_OF_CONTROL
> +enum {
> +	I2CSPEED_KHZ = 100,		/* in KHz */
> +};
> +#endif
> +
> +enum {
> +	I2C_TIMEOUT_USEC = 10000,	/* Wait time for completion */
> +	I2C_FIFO_DEPTH = 8,		/* I2C fifo depth */
> +};
> +
> +enum i2c_transaction_flags {
> +	I2C_IS_WRITE = 0x1,		/* for I2C write operation */
> +	I2C_IS_10_BIT_ADDRESS = 0x2,	/* for 10-bit I2C slave address */
> +	I2C_USE_REPEATED_START = 0x4,	/* for repeat start */
> +	I2C_NO_ACK = 0x8,		/* for slave that won't generate ACK */
> +	I2C_SOFTWARE_CONTROLLER	= 0x10,	/* for I2C transfer using GPIO */
> +	I2C_NO_STOP = 0x20,
> +};
> +
> +/* Contians the I2C transaction details */
> +struct i2c_trans_info {
> +	/* flags to indicate the transaction details */
> +	enum i2c_transaction_flags flags;
> +	u32 address;	/* I2C slave device address */
> +	u32 num_bytes;	/* number of bytes to be transferred */
> +	/* Send/receive buffer. For I2C send operation this buffer should be

Wrong comment, do you run this stuff through checkpatch ?

> +	 * filled with the data to be sent to the slave device. For I2C receive
> +	 * operation this buffer is filled with the data received from the
> +	 * slave device. */
> +	u8 *buf;
> +	int is_10bit_address;
> +};
> +
> +struct i2c_control {
> +	u32 tx_fifo;
> +	u32 rx_fifo;
> +	u32 packet_status;
> +	u32 fifo_control;
> +	u32 fifo_status;
> +	u32 int_mask;
> +	u32 int_status;
> +};
> +
> +struct dvc_ctlr {
> +	u32 ctrl1;			/* 00: DVC_CTRL_REG1 */
> +	u32 ctrl2;			/* 04: DVC_CTRL_REG2 */
> +	u32 ctrl3;			/* 08: DVC_CTRL_REG3 */
> +	u32 status;			/* 0C: DVC_STATUS_REG */
> +	u32 ctrl;			/* 10: DVC_I2C_CTRL_REG */
> +	u32 addr_data;			/* 14: DVC_I2C_ADDR_DATA_REG */
> +	u32 reserved_0[2];		/* 18: */
> +	u32 req;			/* 20: DVC_REQ_REGISTER */
> +	u32 addr_data3;			/* 24: DVC_I2C_ADDR_DATA_REG_3 */
> +	u32 reserved_1[6];		/* 28: */
> +	u32 cnfg;			/* 40: DVC_I2C_CNFG */
> +	u32 cmd_addr0;			/* 44: DVC_I2C_CMD_ADDR0 */
> +	u32 cmd_addr1;			/* 48: DVC_I2C_CMD_ADDR1 */
> +	u32 cmd_data1;			/* 4C: DVC_I2C_CMD_DATA1 */
> +	u32 cmd_data2;			/* 50: DVC_I2C_CMD_DATA2 */
> +	u32 reserved_2[2];		/* 54: */
> +	u32 i2c_status;			/* 5C: DVC_I2C_STATUS */
> +	struct i2c_control control;	/* 60 ~ 78 */
> +};
> +
> +struct i2c_ctlr {
> +	u32 cnfg;			/* 00: I2C_I2C_CNFG */
> +	u32 cmd_addr0;			/* 04: I2C_I2C_CMD_ADDR0 */
> +	u32 cmd_addr1;			/* 08: I2C_I2C_CMD_DATA1 */
> +	u32 cmd_data1;			/* 0C: I2C_I2C_CMD_DATA2 */
> +	u32 cmd_data2;			/* 10: DVC_I2C_CMD_DATA2 */
> +	u32 reserved_0[2];		/* 14: */
> +	u32 status;			/* 1C: I2C_I2C_STATUS */
> +	u32 sl_cnfg;			/* 20: I2C_I2C_SL_CNFG */
> +	u32 sl_rcvd;			/* 24: I2C_I2C_SL_RCVD */
> +	u32 sl_status;			/* 28: I2C_I2C_SL_STATUS */
> +	u32 sl_addr1;			/* 2C: I2C_I2C_SL_ADDR1 */
> +	u32 sl_addr2;			/* 30: I2C_I2C_SL_ADDR2 */
> +	u32 reserved_1[2];		/* 34: */
> +	u32 sl_delay_count;		/* 3C: I2C_I2C_SL_DELAY_COUNT */
> +	u32 reserved_2[4];		/* 40: */
> +	struct i2c_control control;	/* 50 ~ 68 */
> +};
> +
> +/* bit fields definitions for IO Packet Header 1 format */
> +#define PKT_HDR1_PROTOCOL_SHIFT		4
> +#define PKT_HDR1_PROTOCOL_MASK		(0xf << PKT_HDR1_PROTOCOL_SHIFT)
> +#define PKT_HDR1_CTLR_ID_SHIFT		12
> +#define PKT_HDR1_CTLR_ID_MASK		(0xf << PKT_HDR1_CTLR_ID_SHIFT)
> +#define PKT_HDR1_PKT_ID_SHIFT		16
> +#define PKT_HDR1_PKT_ID_MASK		(0xff << PKT_HDR1_PKT_ID_SHIFT)
> +#define PROTOCOL_TYPE_I2C		1
> +
> +/* bit fields definitions for IO Packet Header 2 format */
> +#define PKT_HDR2_PAYLOAD_SIZE_SHIFT	0
> +#define PKT_HDR2_PAYLOAD_SIZE_MASK	(0xfff << PKT_HDR2_PAYLOAD_SIZE_SHIFT)
> +
> +/* bit fields definitions for IO Packet Header 3 format */
> +#define PKT_HDR3_READ_MODE_SHIFT	19
> +#define PKT_HDR3_READ_MODE_MASK		(1 << PKT_HDR3_READ_MODE_SHIFT)
> +#define PKT_HDR3_SLAVE_ADDR_SHIFT	0
> +#define PKT_HDR3_SLAVE_ADDR_MASK	(0x3ff << PKT_HDR3_SLAVE_ADDR_SHIFT)
> +
> +#define DVC_CTRL_REG3_I2C_HW_SW_PROG_SHIFT	26
> +#define DVC_CTRL_REG3_I2C_HW_SW_PROG_MASK	\
> +				(1 << DVC_CTRL_REG3_I2C_HW_SW_PROG_SHIFT)
> +
> +/* I2C_CNFG */
> +#define I2C_CNFG_NEW_MASTER_FSM_SHIFT	11
> +#define I2C_CNFG_NEW_MASTER_FSM_MASK	(1 << I2C_CNFG_NEW_MASTER_FSM_SHIFT)
> +#define I2C_CNFG_PACKET_MODE_SHIFT	10
> +#define I2C_CNFG_PACKET_MODE_MASK	(1 << I2C_CNFG_PACKET_MODE_SHIFT)
> +
> +/* I2C_SL_CNFG */
> +#define I2C_SL_CNFG_NEWSL_SHIFT		2
> +#define I2C_SL_CNFG_NEWSL_MASK		(1 << I2C_SL_CNFG_NEWSL_SHIFT)
> +
> +/* I2C_FIFO_STATUS */
> +#define TX_FIFO_FULL_CNT_SHIFT		0
> +#define TX_FIFO_FULL_CNT_MASK		(0xf << TX_FIFO_FULL_CNT_SHIFT)
> +#define TX_FIFO_EMPTY_CNT_SHIFT		4
> +#define TX_FIFO_EMPTY_CNT_MASK		(0xf << TX_FIFO_EMPTY_CNT_SHIFT)
> +
> +/* I2C_INTERRUPT_STATUS */
> +#define I2C_INT_XFER_COMPLETE_SHIFT	7
> +#define I2C_INT_XFER_COMPLETE_MASK	(1 << I2C_INT_XFER_COMPLETE_SHIFT)
> +#define I2C_INT_NO_ACK_SHIFT		3
> +#define I2C_INT_NO_ACK_MASK		(1 << I2C_INT_NO_ACK_SHIFT)
> +#define I2C_INT_ARBITRATION_LOST_SHIFT	2
> +#define I2C_INT_ARBITRATION_LOST_MASK	(1 <<
> I2C_INT_ARBITRATION_LOST_SHIFT) +
> +#endif
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index 504db03..c123c72 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -41,6 +41,7 @@ COBJS-$(CONFIG_DRIVER_S3C24X0_I2C) += s3c24x0_i2c.o
>  COBJS-$(CONFIG_S3C44B0_I2C) += s3c44b0_i2c.o
>  COBJS-$(CONFIG_SOFT_I2C) += soft_i2c.o
>  COBJS-$(CONFIG_SPEAR_I2C) += spr_i2c.o
> +COBJS-$(CONFIG_TEGRA2_I2C) += tegra2_i2c.o
>  COBJS-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
>  COBJS-$(CONFIG_U8500_I2C) += u8500_i2c.o
>  COBJS-$(CONFIG_SH_I2C) += sh_i2c.o
> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
> new file mode 100644
> index 0000000..ae43531
> --- /dev/null
> +++ b/drivers/i2c/tegra2_i2c.c
> @@ -0,0 +1,533 @@
> +/*
> + * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
> + * Copyright (c) 2010-2011 NVIDIA Corporation
> + *  NVIDIA Corporation <www.nvidia.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/clk_rst.h>
> +#include <asm/arch/clock.h>
> +#include <asm/arch/funcmux.h>
> +#include <asm/arch/gpio.h>
> +#include <asm/arch/pinmux.h>
> +#include <asm/arch/tegra2_i2c.h>
> +#include <fdtdec.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static unsigned int i2c_bus_num;
> +
> +/* Information about i2c controller */
> +struct i2c_bus {
> +	int			id;
> +	enum periph_id		periph_id;
> +	int			speed;
> +	int			pinmux_config;
> +	struct i2c_control	*control;
> +	struct i2c_ctlr		*regs;
> +};
> +
> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];

static

> +
> +static void set_packet_mode(struct i2c_bus *i2c_bus)
> +{
> +	u32 config;
> +
> +	config = I2C_CNFG_NEW_MASTER_FSM_MASK | I2C_CNFG_PACKET_MODE_MASK;
> +
> +	if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C) {
> +		struct dvc_ctlr *dvc = (struct dvc_ctlr *)i2c_bus->regs;
> +
> +		writel(config, &dvc->cnfg);
> +	} else {
> +		writel(config, &i2c_bus->regs->cnfg);
> +		/*
> +		 * program I2C_SL_CNFG.NEWSL to ENABLE. This fixes probe
> +		 * issues, i.e., some slaves may be wrongly detected.
> +		 */
> +		setbits_le32(&i2c_bus->regs->sl_cnfg, I2C_SL_CNFG_NEWSL_MASK);
> +	}
> +}
> +
> +static void i2c_reset_controller(struct i2c_bus *i2c_bus)
> +{
> +	/* Reset I2C controller. */
> +	reset_periph(i2c_bus->periph_id, 1);
> +
> +	/* re-program config register to packet mode */
> +	set_packet_mode(i2c_bus);
> +}
> +
> +static void i2c_init_controller(struct i2c_bus *i2c_bus)
> +{
> +	/* TODO: Fix bug which makes us need to do this */
> +	clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_OSC,
> +			       i2c_bus->speed * (8 * 2 - 1));
> +
> +	/* Reset I2C controller. */
> +	i2c_reset_controller(i2c_bus);
> +
> +	/* Configure I2C controller. */
> +	if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C) {	/* only for DVC I2C */
> +		struct dvc_ctlr *dvc = (struct dvc_ctlr *)i2c_bus->regs;
> +
> +		setbits_le32(&dvc->ctrl3, DVC_CTRL_REG3_I2C_HW_SW_PROG_MASK);
> +	}
> +
> +	funcmux_select(i2c_bus->periph_id, i2c_bus->pinmux_config);
> +}
> +
> +static void send_packet_headers(
> +	struct i2c_bus *i2c_bus,
> +	struct i2c_trans_info *trans,
> +	u32 packet_id)
> +{
> +	u32 data;
> +
> +	/* prepare header1: Header size = 0 Protocol = I2C, pktType = 0 */
> +	data = PROTOCOL_TYPE_I2C << PKT_HDR1_PROTOCOL_SHIFT;
> +	data |= packet_id << PKT_HDR1_PKT_ID_SHIFT;
> +	data |= i2c_bus->id << PKT_HDR1_CTLR_ID_SHIFT;
> +	writel(data, &i2c_bus->control->tx_fifo);
> +	debug("pkt header 1 sent (0x%x)\n", data);
> +
> +	/* prepare header2 */
> +	data = (trans->num_bytes - 1) << PKT_HDR2_PAYLOAD_SIZE_SHIFT;
> +	writel(data, &i2c_bus->control->tx_fifo);
> +	debug("pkt header 2 sent (0x%x)\n", data);
> +
> +	/* prepare IO specific header: configure the slave address */
> +	data = trans->address << PKT_HDR3_SLAVE_ADDR_SHIFT;
> +
> +	/* Enable Read if it is not a write transaction */
> +	if (!(trans->flags & I2C_IS_WRITE))
> +		data |= PKT_HDR3_READ_MODE_MASK;
> +
> +	/* Write I2C specific header */
> +	writel(data, &i2c_bus->control->tx_fifo);
> +	debug("pkt header 3 sent (0x%x)\n", data);
> +}
> +
> +static int wait_for_tx_fifo_empty(struct i2c_control *control)
> +{
> +	u32 count;
> +	int timeout_us = I2C_TIMEOUT_USEC;
> +
> +	while (timeout_us >= 0) {
> +		count = (readl(&control->fifo_status) & TX_FIFO_EMPTY_CNT_MASK)
> +				>> TX_FIFO_EMPTY_CNT_SHIFT;
> +		if (count == I2C_FIFO_DEPTH)
> +			return 1;
> +		udelay(10);
> +		timeout_us -= 10;
> +	};
> +
> +	return 0;
> +}
> +
> +static int wait_for_rx_fifo_notempty(struct i2c_control *control)
> +{
> +	u32 count;
> +	int timeout_us = I2C_TIMEOUT_USEC;
> +
> +	while (timeout_us >= 0) {
> +		count = (readl(&control->fifo_status) & TX_FIFO_FULL_CNT_MASK)
> +				>> TX_FIFO_FULL_CNT_SHIFT;
> +		if (count)
> +			return 1;
> +		udelay(10);
> +		timeout_us -= 10;
> +	};
> +
> +	return 0;
> +}
> +
> +static int wait_for_transfer_complete(struct i2c_control *control)
> +{
> +	int int_status;
> +	int timeout_us = I2C_TIMEOUT_USEC;
> +
> +	while (timeout_us >= 0) {
> +		int_status = readl(&control->int_status);
> +		if (int_status & I2C_INT_NO_ACK_MASK)
> +			return -int_status;
> +		if (int_status & I2C_INT_ARBITRATION_LOST_MASK)
> +			return -int_status;
> +		if (int_status & I2C_INT_XFER_COMPLETE_MASK)
> +			return 0;
> +
> +		udelay(10);
> +		timeout_us -= 10;
> +	};
> +
> +	return -1;
> +}
> +
> +static int send_recv_packets(
> +	struct i2c_bus *i2c_bus,
> +	struct i2c_trans_info *trans)
> +{
> +	struct i2c_control *control = i2c_bus->control;
> +	u32 int_status;
> +	u32 words;
> +	u8 *dptr;
> +	u32 local;
> +	uchar last_bytes;
> +	int error = 0;
> +	int is_write = trans->flags & I2C_IS_WRITE;
> +
> +	/* clear status from previous transaction, XFER_COMPLETE, NOACK, etc. */
> +	int_status = readl(&control->int_status);
> +	writel(int_status, &control->int_status);
> +
> +	send_packet_headers(i2c_bus, trans, 1);
> +
> +	words = BYTES_TO_WORDS(trans->num_bytes);
> +	last_bytes = trans->num_bytes & 3;
> +	dptr = trans->buf;
> +
> +	while (words) {
> +		if (is_write) {
> +			/* deal with word alignment */
> +			if ((unsigned)dptr & 3) {
> +				memcpy(&local, dptr, sizeof(u32));
> +				writel(local, &control->tx_fifo);
> +				debug("pkt data sent (0x%x)\n", local);
> +			} else {
> +				writel(*(u32 *)dptr, &control->tx_fifo);
> +				debug("pkt data sent (0x%x)\n", *(u32 *)dptr);
> +			}
> +			if (!wait_for_tx_fifo_empty(control)) {
> +				error = -1;
> +				goto exit;
> +			}
> +		} else {
> +			if (!wait_for_rx_fifo_notempty(control)) {
> +				error = -1;
> +				goto exit;
> +			}
> +			/*
> +			 * for the last word, we read into our local buffer,
> +			 * in case that caller did not provide enough buffer.
> +			 */
> +			local = readl(&control->rx_fifo);
> +			if ((words == 1) && last_bytes)
> +				memcpy(dptr, (char *)&local, last_bytes);
> +			else if ((unsigned)dptr & 3)
> +				memcpy(dptr, &local, sizeof(u32));
> +			else
> +				*(u32 *)dptr = local;
> +			debug("pkt data received (0x%x)\n", local);
> +		}
> +		words--;
> +		dptr += sizeof(u32);
> +	}
> +
> +	if (wait_for_transfer_complete(control)) {
> +		error = -1;
> +		goto exit;
> +	}
> +	return 0;
> +exit:
> +	/* error, reset the controller. */
> +	i2c_reset_controller(i2c_bus);
> +
> +	return error;
> +}
> +
> +static int tegra2_i2c_write_data(u32 addr, u8 *data, u32 len)
> +{
> +	int error;
> +	struct i2c_trans_info trans_info;
> +
> +	trans_info.address = addr;
> +	trans_info.buf = data;
> +	trans_info.flags = I2C_IS_WRITE;
> +	trans_info.num_bytes = len;
> +	trans_info.is_10bit_address = 0;
> +
> +	error = send_recv_packets(&i2c_controllers[i2c_bus_num], &trans_info);
> +	if (error)
> +		debug("tegra2_i2c_write_data: Error (%d) !!!\n", error);
> +
> +	return error;
> +}
> +
> +static int tegra2_i2c_read_data(u32 addr, u8 *data, u32 len)
> +{
> +	int error;
> +	struct i2c_trans_info trans_info;
> +
> +	trans_info.address = addr | 1;
> +	trans_info.buf = data;
> +	trans_info.flags = 0;
> +	trans_info.num_bytes = len;
> +	trans_info.is_10bit_address = 0;
> +
> +	error = send_recv_packets(&i2c_controllers[i2c_bus_num], &trans_info);
> +	if (error)
> +		debug("tegra2_i2c_read_data: Error (%d) !!!\n", error);
> +
> +	return error;
> +}
> +
> +#ifndef CONFIG_OF_CONTROL
> +static const enum periph_id i2c_periph_ids[CONFIG_SYS_MAX_I2C_BUS] = {
> +	PERIPH_ID_DVC_I2C,
> +	PERIPH_ID_I2C1,
> +	PERIPH_ID_I2C2,
> +	PERIPH_ID_I2C3
> +};
> +
> +static const u32 *i2c_bus_base[CONFIG_SYS_MAX_I2C_BUS] = {
> +	(u32 *)TEGRA2_DVC_BASE,
> +	(u32 *)TEGRA2_I2C1_BASE,
> +	(u32 *)TEGRA2_I2C2_BASE,
> +	(u32 *)TEGRA2_I2C3_BASE
> +};
> +
> +/* pinmux_configs based on the pinmux configuration */
> +static const int pinmux_configs[CONFIG_SYS_MAX_I2C_BUS] = {
> +	CONFIG_I2CP_PIN_MUX,	/* for I2CP (DVC I2C) */
> +	CONFIG_I2C1_PIN_MUX,	/* for I2C1 */
> +	CONFIG_I2C2_PIN_MUX,	/* for I2C2 */
> +	CONFIG_I2C3_PIN_MUX	/* for I2C3 */
> +};
> +
> +static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
> +{
> +	int i = *index;
> +
> +	if (i >= CONFIG_SYS_MAX_I2C_BUS)
> +		return -1;
> +
> +	i2c_bus->periph_id = i2c_periph_ids[i];
> +	i2c_bus->pinmux_config = pinmux_configs[i];
> +	i2c_bus->regs = (struct i2c_ctlr *)i2c_bus_base[i];
> +	i2c_bus->speed = I2CSPEED_KHZ * 1000;
> +
> +	*index = i + 1;
> +
> +	return 0;
> +}
> +#else
> +static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
> +{
> +	const void *blob = gd->fdt_blob;
> +	int node;
> +
> +	node = fdtdec_next_alias(blob, "i2c", COMPAT_NVIDIA_TEGRA20_I2C,
> +				 index);
> +	if (node < 0)
> +		return -1;
> +
> +	i2c_bus->regs = (struct i2c_ctlr *)fdtdec_get_addr(blob, node, "reg");
> +	i2c_bus->pinmux_config = fdtdec_get_int(blob, node, "u-boot,pinmux", 0);
> +	i2c_bus->speed = fdtdec_get_int(blob, node, "speed", 0);
> +	i2c_bus->periph_id = fdtdec_get_int(blob, node, "u-boot,periph-id", -1);
> +
> +	if (i2c_bus->periph_id == -1)
> +		return -FDT_ERR_NOTFOUND;
> +
> +	return 0;
> +}
> +#endif
> +
> +int i2c_init_board(void)
> +{
> +	struct i2c_bus *i2c_bus;
> +	int index = 0;
> +	int i;
> +
> +	/* build the i2c_controllers[] for each controller */
> +	for (i = 0; i < CONFIG_SYS_MAX_I2C_BUS; ++i) {
> +		i2c_bus = &i2c_controllers[i];
> +		i2c_bus->id = i;
> +
> +		if (i2c_get_config(&index, i2c_bus)) {
> +			printf("i2c_init_board: failed to find bus %d\n", i);
> +			return -1;
> +		}
> +
> +		if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C)
> +			i2c_bus->control =
> +				&((struct dvc_ctlr *)i2c_bus->regs)->control;
> +		else
> +			i2c_bus->control = &i2c_bus->regs->control;
> +
> +		i2c_init_controller(i2c_bus);
> +	}
> +
> +	return 0;
> +}
> +
> +void i2c_init(int speed, int slaveaddr)
> +{
> +	debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
> +}
> +
> +/* i2c write version without the register address */
> +int i2c_write_data(uchar chip, uchar *buffer, int len)
> +{
> +	int rc;
> +
> +	debug("i2c_write_data: chip=0x%x, len=0x%x\n", chip, len);
> +	debug("write_data: ");
> +	/* use rc for counter */
> +	for (rc = 0; rc < len; ++rc)
> +		debug(" 0x%02x", buffer[rc]);
> +	debug("\n");
> +
> +	rc = tegra2_i2c_write_data(I2C_ADDR_ON_BUS(chip), buffer, len);
> +	if (rc)
> +		debug("i2c_write_data(): rc=%d\n", rc);
> +
> +	return rc;
> +}
> +
> +/* i2c read version without the register address */
> +int i2c_read_data(uchar chip, uchar *buffer, int len)
> +{
> +	int rc;
> +
> +	debug("inside i2c_read_data():\n");
> +	rc = tegra2_i2c_read_data(I2C_ADDR_ON_BUS(chip), buffer, len);
> +	if (rc) {
> +		debug("i2c_read_data(): rc=%d\n", rc);
> +		return rc;
> +	}
> +
> +	debug("i2c_read_data: ");
> +	/* reuse rc for counter*/
> +	for (rc = 0; rc < len; ++rc)
> +		debug(" 0x%02x", buffer[rc]);
> +	debug("\n");
> +
> +	return 0;
> +}
> +
> +/* Probe to see if a chip is present. */
> +int i2c_probe(uchar chip)
> +{
> +	int rc;
> +	uchar reg;
> +
> +	debug("i2c_probe: addr=0x%x\n", chip);
> +	reg = 0;
> +	rc = i2c_write_data(chip, &reg, 1);
> +	if (rc) {
> +		debug("Error probing 0x%x.\n", chip);
> +		return 1;
> +	}
> +	return 0;
> +}
> +
> +static int i2c_addr_ok(const uint addr, const int alen)
> +{
> +	if (alen < 0 || alen > sizeof(addr))
> +		return 0;
> +	if (alen != sizeof(addr)) {
> +		uint max_addr = (1 << (8 * alen)) - 1;
> +		if (addr > max_addr)
> +			return 0;
> +	}
> +	return 1;
> +}
> +
> +/* Read bytes */
> +int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
> +{
> +	uint offset;
> +	int i;
> +
> +	debug("i2c_read: chip=0x%x, addr=0x%x, len=0x%x\n",
> +				chip, addr, len);
> +	if (!i2c_addr_ok(addr, alen)) {
> +		debug("i2c_read: Bad address %x.%d.\n", addr, alen);
> +		return 1;
> +	}
> +	for (offset = 0; offset < len; offset++) {
> +		if (alen) {
> +			uchar data[alen];
> +			for (i = 0; i < alen; i++) {
> +				data[alen - i - 1] =
> +					(addr + offset) >> (8 * i);
> +			}
> +			if (i2c_write_data(chip, data, alen)) {
> +				debug("i2c_read: error sending (0x%x)\n",
> +					addr);
> +				return 1;
> +			}
> +		}
> +		if (i2c_read_data(chip, buffer + offset, 1)) {
> +			debug("i2c_read: error reading (0x%x)\n", addr);
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/* Write bytes */
> +int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
> +{
> +	uint offset;
> +	int i;
> +
> +	debug("i2c_write: chip=0x%x, addr=0x%x, len=0x%x\n",
> +				chip, addr, len);
> +	if (!i2c_addr_ok(addr, alen)) {
> +		debug("i2c_write: Bad address %x.%d.\n", addr, alen);
> +		return 1;
> +	}
> +	for (offset = 0; offset < len; offset++) {
> +		uchar data[alen + 1];
> +		for (i = 0; i < alen; i++)
> +			data[alen - i - 1] = (addr + offset) >> (8 * i);
> +		data[alen] = buffer[offset];
> +		if (i2c_write_data(chip, data, alen + 1)) {
> +			debug("i2c_write: error sending (0x%x)\n", addr);
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +#if defined(CONFIG_I2C_MULTI_BUS)
> +/*
> + * Functions for multiple I2C bus handling
> + */
> +unsigned int i2c_get_bus_num(void)
> +{
> +	return i2c_bus_num;
> +}
> +
> +int i2c_set_bus_num(unsigned int bus)
> +{
> +	if (bus >= CONFIG_SYS_MAX_I2C_BUS)
> +		return -1;
> +	i2c_bus_num = bus;
> +
> +	return 0;
> +}
> +#endif

get/set_speed missing ?

> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index a8911b5..5547676 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -58,6 +58,7 @@ struct fdt_memory {
>  enum fdt_compat_id {
>  	COMPAT_UNKNOWN,
>  	COMPAT_NVIDIA_TEGRA20_USB,	/* Tegra2 USB port */
> +	COMPAT_NVIDIA_TEGRA20_I2C,	/* Tegra2 i2c */
> 
>  	COMPAT_COUNT,
>  };
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 931b4ce..fb3d79d 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -38,6 +38,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_TEGRA20_I2C, "nvidia,tegra20-i2c"),
>  };
> 
>  /**
M

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Simon Glass]

Hi Marek,

On Mon, Dec 26, 2011 at 11:15 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> From: Yen Lin <yelin@nvidia.com>
>>
>> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
>> The driver supports building both with and without CONFIG_OF_CONTROL.
>>
>> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
>> in the board config header file:
>>
>> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
>> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
>> ? ? ? (typically this will be 0 to bring the port out the common
>> ? ? ? ? ? ? ? pins)
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> ?arch/arm/include/asm/arch-tegra2/tegra2.h ? ? | ? ?4 +
>> ?arch/arm/include/asm/arch-tegra2/tegra2_i2c.h | ?167 ++++++++
>> ?drivers/i2c/Makefile ? ? ? ? ? ? ? ? ? ? ? ? ?| ? ?1 +
>> ?drivers/i2c/tegra2_i2c.c ? ? ? ? ? ? ? ? ? ? ?| ?533
>> +++++++++++++++++++++++++ include/fdtdec.h ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?|
>> ? 1 +
>> ?lib/fdtdec.c ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? ?1 +
>> ?6 files changed, 707 insertions(+), 0 deletions(-)
>> ?create mode 100644 arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
>> ?create mode 100644 drivers/i2c/tegra2_i2c.c
>>
>> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2.h
>> b/arch/arm/include/asm/arch-tegra2/tegra2.h index ca1881e..d1a70da 100644
>> --- a/arch/arm/include/asm/arch-tegra2/tegra2.h
>> +++ b/arch/arm/include/asm/arch-tegra2/tegra2.h
>> @@ -40,6 +40,10 @@
>> ?#define NV_PA_APB_UARTE_BASE (NV_PA_APB_MISC_BASE + 0x6400)
>> ?#define TEGRA2_SPI_BASE ? ? ? ? ? ? ?(NV_PA_APB_MISC_BASE + 0xC380)
>> ?#define TEGRA2_PMC_BASE ? ? ? ? ? ? ?(NV_PA_APB_MISC_BASE + 0xE400)
>> +#define TEGRA2_I2C1_BASE ? ? (NV_PA_APB_MISC_BASE + 0xC000)
>> +#define TEGRA2_I2C2_BASE ? ? (NV_PA_APB_MISC_BASE + 0xC400)
>> +#define TEGRA2_I2C3_BASE ? ? (NV_PA_APB_MISC_BASE + 0xC500)
>> +#define TEGRA2_DVC_BASE ? ? ? ? ? ? ?(NV_PA_APB_MISC_BASE + 0xD000)
>> ?#define NV_PA_CSITE_BASE ? ? 0x70040000
>> ?#define TEGRA_USB1_BASE ? ? ? ? ? ? ?0xC5000000
>> ?#define TEGRA_USB3_BASE ? ? ? ? ? ? ?0xC5008000
>> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
>> b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h new file mode 100644
>> index 0000000..4920d47
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
>> @@ -0,0 +1,167 @@
>> +/*
>> + * NVIDIA Tegra2 I2C controller
>> + *
>> + * Copyright 2010-2011 NVIDIA Corporation
>> + *
>> + * This software may be used and distributed according to the
>> + * terms of the GNU Public License, Version 2, incorporated
>> + * herein by reference.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * Version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +#ifndef _TEGRA2_I2C_H_
>> +#define _TEGRA2_I2C_H_
>> +
>> +#include <asm/types.h>
>> +
>> +/* Convert the number of bytes to word. */
>> +#define BYTES_TO_WORDS(size) ? ? ? ? (((size) + 3) >> 2)
>
> Isn't this stuff in include/common.h ? div_round_up() or something ?
>> +
>> +/* Convert i2c slave address to be put on bus ?*/
>> +#define I2C_ADDR_ON_BUS(chip) ? ? ? ? ? ? ? ?(chip << 1)
>> +
>> +#ifndef CONFIG_OF_CONTROL
>> +enum {
>> + ? ? I2CSPEED_KHZ = 100, ? ? ? ? ? ? /* in KHz */
>> +};
>> +#endif
>> +
>> +enum {
>> + ? ? I2C_TIMEOUT_USEC = 10000, ? ? ? /* Wait time for completion */
>> + ? ? I2C_FIFO_DEPTH = 8, ? ? ? ? ? ? /* I2C fifo depth */
>> +};
>> +
>> +enum i2c_transaction_flags {
>> + ? ? I2C_IS_WRITE = 0x1, ? ? ? ? ? ? /* for I2C write operation */
>> + ? ? I2C_IS_10_BIT_ADDRESS = 0x2, ? ?/* for 10-bit I2C slave address */
>> + ? ? I2C_USE_REPEATED_START = 0x4, ? /* for repeat start */
>> + ? ? I2C_NO_ACK = 0x8, ? ? ? ? ? ? ? /* for slave that won't generate ACK */
>> + ? ? I2C_SOFTWARE_CONTROLLER = 0x10, /* for I2C transfer using GPIO */
>> + ? ? I2C_NO_STOP = 0x20,
>> +};
>> +
>> +/* Contians the I2C transaction details */
>> +struct i2c_trans_info {
>> + ? ? /* flags to indicate the transaction details */
>> + ? ? enum i2c_transaction_flags flags;
>> + ? ? u32 address; ? ?/* I2C slave device address */
>> + ? ? u32 num_bytes; ?/* number of bytes to be transferred */
>> + ? ? /* Send/receive buffer. For I2C send operation this buffer should be
>
> Wrong comment, do you run this stuff through checkpatch ?

Fixed style and adjusted comment. Yes I do, but checkpatch isn't perfect.

>
>> + ? ? ?* filled with the data to be sent to the slave device. For I2C receive
>> + ? ? ?* operation this buffer is filled with the data received from the
>> + ? ? ?* slave device. */
>> + ? ? u8 *buf;
>> + ? ? int is_10bit_address;
>> +};
>> +
>> +struct i2c_control {
>> + ? ? u32 tx_fifo;
>> + ? ? u32 rx_fifo;
>> + ? ? u32 packet_status;
>> + ? ? u32 fifo_control;
>> + ? ? u32 fifo_status;
>> + ? ? u32 int_mask;
>> + ? ? u32 int_status;
>> +};
>> +
>> +struct dvc_ctlr {
>> + ? ? u32 ctrl1; ? ? ? ? ? ? ? ? ? ? ?/* 00: DVC_CTRL_REG1 */
>> + ? ? u32 ctrl2; ? ? ? ? ? ? ? ? ? ? ?/* 04: DVC_CTRL_REG2 */
>> + ? ? u32 ctrl3; ? ? ? ? ? ? ? ? ? ? ?/* 08: DVC_CTRL_REG3 */
>> + ? ? u32 status; ? ? ? ? ? ? ? ? ? ? /* 0C: DVC_STATUS_REG */
>> + ? ? u32 ctrl; ? ? ? ? ? ? ? ? ? ? ? /* 10: DVC_I2C_CTRL_REG */
>> + ? ? u32 addr_data; ? ? ? ? ? ? ? ? ?/* 14: DVC_I2C_ADDR_DATA_REG */
>> + ? ? u32 reserved_0[2]; ? ? ? ? ? ? ?/* 18: */
>> + ? ? u32 req; ? ? ? ? ? ? ? ? ? ? ? ?/* 20: DVC_REQ_REGISTER */
>> + ? ? u32 addr_data3; ? ? ? ? ? ? ? ? /* 24: DVC_I2C_ADDR_DATA_REG_3 */
>> + ? ? u32 reserved_1[6]; ? ? ? ? ? ? ?/* 28: */
>> + ? ? u32 cnfg; ? ? ? ? ? ? ? ? ? ? ? /* 40: DVC_I2C_CNFG */
>> + ? ? u32 cmd_addr0; ? ? ? ? ? ? ? ? ?/* 44: DVC_I2C_CMD_ADDR0 */
>> + ? ? u32 cmd_addr1; ? ? ? ? ? ? ? ? ?/* 48: DVC_I2C_CMD_ADDR1 */
>> + ? ? u32 cmd_data1; ? ? ? ? ? ? ? ? ?/* 4C: DVC_I2C_CMD_DATA1 */
>> + ? ? u32 cmd_data2; ? ? ? ? ? ? ? ? ?/* 50: DVC_I2C_CMD_DATA2 */
>> + ? ? u32 reserved_2[2]; ? ? ? ? ? ? ?/* 54: */
>> + ? ? u32 i2c_status; ? ? ? ? ? ? ? ? /* 5C: DVC_I2C_STATUS */
>> + ? ? struct i2c_control control; ? ? /* 60 ~ 78 */
>> +};
>> +
>> +struct i2c_ctlr {
>> + ? ? u32 cnfg; ? ? ? ? ? ? ? ? ? ? ? /* 00: I2C_I2C_CNFG */
>> + ? ? u32 cmd_addr0; ? ? ? ? ? ? ? ? ?/* 04: I2C_I2C_CMD_ADDR0 */
>> + ? ? u32 cmd_addr1; ? ? ? ? ? ? ? ? ?/* 08: I2C_I2C_CMD_DATA1 */
>> + ? ? u32 cmd_data1; ? ? ? ? ? ? ? ? ?/* 0C: I2C_I2C_CMD_DATA2 */
>> + ? ? u32 cmd_data2; ? ? ? ? ? ? ? ? ?/* 10: DVC_I2C_CMD_DATA2 */
>> + ? ? u32 reserved_0[2]; ? ? ? ? ? ? ?/* 14: */
>> + ? ? u32 status; ? ? ? ? ? ? ? ? ? ? /* 1C: I2C_I2C_STATUS */
>> + ? ? u32 sl_cnfg; ? ? ? ? ? ? ? ? ? ?/* 20: I2C_I2C_SL_CNFG */
>> + ? ? u32 sl_rcvd; ? ? ? ? ? ? ? ? ? ?/* 24: I2C_I2C_SL_RCVD */
>> + ? ? u32 sl_status; ? ? ? ? ? ? ? ? ?/* 28: I2C_I2C_SL_STATUS */
>> + ? ? u32 sl_addr1; ? ? ? ? ? ? ? ? ? /* 2C: I2C_I2C_SL_ADDR1 */
>> + ? ? u32 sl_addr2; ? ? ? ? ? ? ? ? ? /* 30: I2C_I2C_SL_ADDR2 */
>> + ? ? u32 reserved_1[2]; ? ? ? ? ? ? ?/* 34: */
>> + ? ? u32 sl_delay_count; ? ? ? ? ? ? /* 3C: I2C_I2C_SL_DELAY_COUNT */
>> + ? ? u32 reserved_2[4]; ? ? ? ? ? ? ?/* 40: */
>> + ? ? struct i2c_control control; ? ? /* 50 ~ 68 */
>> +};
>> +
>> +/* bit fields definitions for IO Packet Header 1 format */
>> +#define PKT_HDR1_PROTOCOL_SHIFT ? ? ? ? ? ? ?4
>> +#define PKT_HDR1_PROTOCOL_MASK ? ? ? ? ? ? ? (0xf << PKT_HDR1_PROTOCOL_SHIFT)
>> +#define PKT_HDR1_CTLR_ID_SHIFT ? ? ? ? ? ? ? 12
>> +#define PKT_HDR1_CTLR_ID_MASK ? ? ? ? ? ? ? ?(0xf << PKT_HDR1_CTLR_ID_SHIFT)
>> +#define PKT_HDR1_PKT_ID_SHIFT ? ? ? ? ? ? ? ?16
>> +#define PKT_HDR1_PKT_ID_MASK ? ? ? ? (0xff << PKT_HDR1_PKT_ID_SHIFT)
>> +#define PROTOCOL_TYPE_I2C ? ? ? ? ? ?1
>> +
>> +/* bit fields definitions for IO Packet Header 2 format */
>> +#define PKT_HDR2_PAYLOAD_SIZE_SHIFT ?0
>> +#define PKT_HDR2_PAYLOAD_SIZE_MASK ? (0xfff << PKT_HDR2_PAYLOAD_SIZE_SHIFT)
>> +
>> +/* bit fields definitions for IO Packet Header 3 format */
>> +#define PKT_HDR3_READ_MODE_SHIFT ? ? 19
>> +#define PKT_HDR3_READ_MODE_MASK ? ? ? ? ? ? ?(1 << PKT_HDR3_READ_MODE_SHIFT)
>> +#define PKT_HDR3_SLAVE_ADDR_SHIFT ? ?0
>> +#define PKT_HDR3_SLAVE_ADDR_MASK ? ? (0x3ff << PKT_HDR3_SLAVE_ADDR_SHIFT)
>> +
>> +#define DVC_CTRL_REG3_I2C_HW_SW_PROG_SHIFT ? 26
>> +#define DVC_CTRL_REG3_I2C_HW_SW_PROG_MASK ? ?\
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? (1 << DVC_CTRL_REG3_I2C_HW_SW_PROG_SHIFT)
>> +
>> +/* I2C_CNFG */
>> +#define I2C_CNFG_NEW_MASTER_FSM_SHIFT ? ? ? ?11
>> +#define I2C_CNFG_NEW_MASTER_FSM_MASK (1 << I2C_CNFG_NEW_MASTER_FSM_SHIFT)
>> +#define I2C_CNFG_PACKET_MODE_SHIFT ? 10
>> +#define I2C_CNFG_PACKET_MODE_MASK ? ?(1 << I2C_CNFG_PACKET_MODE_SHIFT)
>> +
>> +/* I2C_SL_CNFG */
>> +#define I2C_SL_CNFG_NEWSL_SHIFT ? ? ? ? ? ? ?2
>> +#define I2C_SL_CNFG_NEWSL_MASK ? ? ? ? ? ? ? (1 << I2C_SL_CNFG_NEWSL_SHIFT)
>> +
>> +/* I2C_FIFO_STATUS */
>> +#define TX_FIFO_FULL_CNT_SHIFT ? ? ? ? ? ? ? 0
>> +#define TX_FIFO_FULL_CNT_MASK ? ? ? ? ? ? ? ?(0xf << TX_FIFO_FULL_CNT_SHIFT)
>> +#define TX_FIFO_EMPTY_CNT_SHIFT ? ? ? ? ? ? ?4
>> +#define TX_FIFO_EMPTY_CNT_MASK ? ? ? ? ? ? ? (0xf << TX_FIFO_EMPTY_CNT_SHIFT)
>> +
>> +/* I2C_INTERRUPT_STATUS */
>> +#define I2C_INT_XFER_COMPLETE_SHIFT ?7
>> +#define I2C_INT_XFER_COMPLETE_MASK ? (1 << I2C_INT_XFER_COMPLETE_SHIFT)
>> +#define I2C_INT_NO_ACK_SHIFT ? ? ? ? 3
>> +#define I2C_INT_NO_ACK_MASK ? ? ? ? ?(1 << I2C_INT_NO_ACK_SHIFT)
>> +#define I2C_INT_ARBITRATION_LOST_SHIFT ? ? ? 2
>> +#define I2C_INT_ARBITRATION_LOST_MASK ? ? ? ?(1 <<
>> I2C_INT_ARBITRATION_LOST_SHIFT) +
>> +#endif
>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>> index 504db03..c123c72 100644
>> --- a/drivers/i2c/Makefile
>> +++ b/drivers/i2c/Makefile
>> @@ -41,6 +41,7 @@ COBJS-$(CONFIG_DRIVER_S3C24X0_I2C) += s3c24x0_i2c.o
>> ?COBJS-$(CONFIG_S3C44B0_I2C) += s3c44b0_i2c.o
>> ?COBJS-$(CONFIG_SOFT_I2C) += soft_i2c.o
>> ?COBJS-$(CONFIG_SPEAR_I2C) += spr_i2c.o
>> +COBJS-$(CONFIG_TEGRA2_I2C) += tegra2_i2c.o
>> ?COBJS-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
>> ?COBJS-$(CONFIG_U8500_I2C) += u8500_i2c.o
>> ?COBJS-$(CONFIG_SH_I2C) += sh_i2c.o
>> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
>> new file mode 100644
>> index 0000000..ae43531
>> --- /dev/null
>> +++ b/drivers/i2c/tegra2_i2c.c
>> @@ -0,0 +1,533 @@
>> +/*
>> + * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
>> + * Copyright (c) 2010-2011 NVIDIA Corporation
>> + * ?NVIDIA Corporation <www.nvidia.com>
>> + *
>> + * See file CREDITS for list of people who contributed to this
>> + * project.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/clk_rst.h>
>> +#include <asm/arch/clock.h>
>> +#include <asm/arch/funcmux.h>
>> +#include <asm/arch/gpio.h>
>> +#include <asm/arch/pinmux.h>
>> +#include <asm/arch/tegra2_i2c.h>
>> +#include <fdtdec.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static unsigned int i2c_bus_num;
>> +
>> +/* Information about i2c controller */
>> +struct i2c_bus {
>> + ? ? int ? ? ? ? ? ? ? ? ? ? id;
>> + ? ? enum periph_id ? ? ? ? ?periph_id;
>> + ? ? int ? ? ? ? ? ? ? ? ? ? speed;
>> + ? ? int ? ? ? ? ? ? ? ? ? ? pinmux_config;
>> + ? ? struct i2c_control ? ? ?*control;
>> + ? ? struct i2c_ctlr ? ? ? ? *regs;
>> +};
>> +
>> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
>
> static

done

>
>> +
>> +static void set_packet_mode(struct i2c_bus *i2c_bus)
>> +{
>> + ? ? u32 config;
>> +
>> + ? ? config = I2C_CNFG_NEW_MASTER_FSM_MASK | I2C_CNFG_PACKET_MODE_MASK;
>> +
>> + ? ? if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C) {
>> + ? ? ? ? ? ? struct dvc_ctlr *dvc = (struct dvc_ctlr *)i2c_bus->regs;
>> +
>> + ? ? ? ? ? ? writel(config, &dvc->cnfg);
>> + ? ? } else {
>> + ? ? ? ? ? ? writel(config, &i2c_bus->regs->cnfg);
>> + ? ? ? ? ? ? /*
>> + ? ? ? ? ? ? ?* program I2C_SL_CNFG.NEWSL to ENABLE. This fixes probe
>> + ? ? ? ? ? ? ?* issues, i.e., some slaves may be wrongly detected.
>> + ? ? ? ? ? ? ?*/
>> + ? ? ? ? ? ? setbits_le32(&i2c_bus->regs->sl_cnfg, I2C_SL_CNFG_NEWSL_MASK);
>> + ? ? }
>> +}
>> +
>> +static void i2c_reset_controller(struct i2c_bus *i2c_bus)
>> +{
>> + ? ? /* Reset I2C controller. */
>> + ? ? reset_periph(i2c_bus->periph_id, 1);
>> +
>> + ? ? /* re-program config register to packet mode */
>> + ? ? set_packet_mode(i2c_bus);
>> +}
>> +
>> +static void i2c_init_controller(struct i2c_bus *i2c_bus)
>> +{
>> + ? ? /* TODO: Fix bug which makes us need to do this */
>> + ? ? clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_OSC,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?i2c_bus->speed * (8 * 2 - 1));
>> +
>> + ? ? /* Reset I2C controller. */
>> + ? ? i2c_reset_controller(i2c_bus);
>> +
>> + ? ? /* Configure I2C controller. */
>> + ? ? if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C) { ?/* only for DVC I2C */
>> + ? ? ? ? ? ? struct dvc_ctlr *dvc = (struct dvc_ctlr *)i2c_bus->regs;
>> +
>> + ? ? ? ? ? ? setbits_le32(&dvc->ctrl3, DVC_CTRL_REG3_I2C_HW_SW_PROG_MASK);
>> + ? ? }
>> +
>> + ? ? funcmux_select(i2c_bus->periph_id, i2c_bus->pinmux_config);
>> +}
>> +
>> +static void send_packet_headers(
>> + ? ? struct i2c_bus *i2c_bus,
>> + ? ? struct i2c_trans_info *trans,
>> + ? ? u32 packet_id)
>> +{
>> + ? ? u32 data;
>> +
>> + ? ? /* prepare header1: Header size = 0 Protocol = I2C, pktType = 0 */
>> + ? ? data = PROTOCOL_TYPE_I2C << PKT_HDR1_PROTOCOL_SHIFT;
>> + ? ? data |= packet_id << PKT_HDR1_PKT_ID_SHIFT;
>> + ? ? data |= i2c_bus->id << PKT_HDR1_CTLR_ID_SHIFT;
>> + ? ? writel(data, &i2c_bus->control->tx_fifo);
>> + ? ? debug("pkt header 1 sent (0x%x)\n", data);
>> +
>> + ? ? /* prepare header2 */
>> + ? ? data = (trans->num_bytes - 1) << PKT_HDR2_PAYLOAD_SIZE_SHIFT;
>> + ? ? writel(data, &i2c_bus->control->tx_fifo);
>> + ? ? debug("pkt header 2 sent (0x%x)\n", data);
>> +
>> + ? ? /* prepare IO specific header: configure the slave address */
>> + ? ? data = trans->address << PKT_HDR3_SLAVE_ADDR_SHIFT;
>> +
>> + ? ? /* Enable Read if it is not a write transaction */
>> + ? ? if (!(trans->flags & I2C_IS_WRITE))
>> + ? ? ? ? ? ? data |= PKT_HDR3_READ_MODE_MASK;
>> +
>> + ? ? /* Write I2C specific header */
>> + ? ? writel(data, &i2c_bus->control->tx_fifo);
>> + ? ? debug("pkt header 3 sent (0x%x)\n", data);
>> +}
>> +
>> +static int wait_for_tx_fifo_empty(struct i2c_control *control)
>> +{
>> + ? ? u32 count;
>> + ? ? int timeout_us = I2C_TIMEOUT_USEC;
>> +
>> + ? ? while (timeout_us >= 0) {
>> + ? ? ? ? ? ? count = (readl(&control->fifo_status) & TX_FIFO_EMPTY_CNT_MASK)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? >> TX_FIFO_EMPTY_CNT_SHIFT;
>> + ? ? ? ? ? ? if (count == I2C_FIFO_DEPTH)
>> + ? ? ? ? ? ? ? ? ? ? return 1;
>> + ? ? ? ? ? ? udelay(10);
>> + ? ? ? ? ? ? timeout_us -= 10;
>> + ? ? };
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static int wait_for_rx_fifo_notempty(struct i2c_control *control)
>> +{
>> + ? ? u32 count;
>> + ? ? int timeout_us = I2C_TIMEOUT_USEC;
>> +
>> + ? ? while (timeout_us >= 0) {
>> + ? ? ? ? ? ? count = (readl(&control->fifo_status) & TX_FIFO_FULL_CNT_MASK)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? >> TX_FIFO_FULL_CNT_SHIFT;
>> + ? ? ? ? ? ? if (count)
>> + ? ? ? ? ? ? ? ? ? ? return 1;
>> + ? ? ? ? ? ? udelay(10);
>> + ? ? ? ? ? ? timeout_us -= 10;
>> + ? ? };
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static int wait_for_transfer_complete(struct i2c_control *control)
>> +{
>> + ? ? int int_status;
>> + ? ? int timeout_us = I2C_TIMEOUT_USEC;
>> +
>> + ? ? while (timeout_us >= 0) {
>> + ? ? ? ? ? ? int_status = readl(&control->int_status);
>> + ? ? ? ? ? ? if (int_status & I2C_INT_NO_ACK_MASK)
>> + ? ? ? ? ? ? ? ? ? ? return -int_status;
>> + ? ? ? ? ? ? if (int_status & I2C_INT_ARBITRATION_LOST_MASK)
>> + ? ? ? ? ? ? ? ? ? ? return -int_status;
>> + ? ? ? ? ? ? if (int_status & I2C_INT_XFER_COMPLETE_MASK)
>> + ? ? ? ? ? ? ? ? ? ? return 0;
>> +
>> + ? ? ? ? ? ? udelay(10);
>> + ? ? ? ? ? ? timeout_us -= 10;
>> + ? ? };
>> +
>> + ? ? return -1;
>> +}
>> +
>> +static int send_recv_packets(
>> + ? ? struct i2c_bus *i2c_bus,
>> + ? ? struct i2c_trans_info *trans)
>> +{
>> + ? ? struct i2c_control *control = i2c_bus->control;
>> + ? ? u32 int_status;
>> + ? ? u32 words;
>> + ? ? u8 *dptr;
>> + ? ? u32 local;
>> + ? ? uchar last_bytes;
>> + ? ? int error = 0;
>> + ? ? int is_write = trans->flags & I2C_IS_WRITE;
>> +
>> + ? ? /* clear status from previous transaction, XFER_COMPLETE, NOACK, etc. */
>> + ? ? int_status = readl(&control->int_status);
>> + ? ? writel(int_status, &control->int_status);
>> +
>> + ? ? send_packet_headers(i2c_bus, trans, 1);
>> +
>> + ? ? words = BYTES_TO_WORDS(trans->num_bytes);
>> + ? ? last_bytes = trans->num_bytes & 3;
>> + ? ? dptr = trans->buf;
>> +
>> + ? ? while (words) {
>> + ? ? ? ? ? ? if (is_write) {
>> + ? ? ? ? ? ? ? ? ? ? /* deal with word alignment */
>> + ? ? ? ? ? ? ? ? ? ? if ((unsigned)dptr & 3) {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? memcpy(&local, dptr, sizeof(u32));
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? writel(local, &control->tx_fifo);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? debug("pkt data sent (0x%x)\n", local);
>> + ? ? ? ? ? ? ? ? ? ? } else {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? writel(*(u32 *)dptr, &control->tx_fifo);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? debug("pkt data sent (0x%x)\n", *(u32 *)dptr);
>> + ? ? ? ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? ? ? ? ? if (!wait_for_tx_fifo_empty(control)) {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? error = -1;
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto exit;
>> + ? ? ? ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? } else {
>> + ? ? ? ? ? ? ? ? ? ? if (!wait_for_rx_fifo_notempty(control)) {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? error = -1;
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto exit;
>> + ? ? ? ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? ? ? ? ? /*
>> + ? ? ? ? ? ? ? ? ? ? ?* for the last word, we read into our local buffer,
>> + ? ? ? ? ? ? ? ? ? ? ?* in case that caller did not provide enough buffer.
>> + ? ? ? ? ? ? ? ? ? ? ?*/
>> + ? ? ? ? ? ? ? ? ? ? local = readl(&control->rx_fifo);
>> + ? ? ? ? ? ? ? ? ? ? if ((words == 1) && last_bytes)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? memcpy(dptr, (char *)&local, last_bytes);
>> + ? ? ? ? ? ? ? ? ? ? else if ((unsigned)dptr & 3)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? memcpy(dptr, &local, sizeof(u32));
>> + ? ? ? ? ? ? ? ? ? ? else
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? *(u32 *)dptr = local;
>> + ? ? ? ? ? ? ? ? ? ? debug("pkt data received (0x%x)\n", local);
>> + ? ? ? ? ? ? }
>> + ? ? ? ? ? ? words--;
>> + ? ? ? ? ? ? dptr += sizeof(u32);
>> + ? ? }
>> +
>> + ? ? if (wait_for_transfer_complete(control)) {
>> + ? ? ? ? ? ? error = -1;
>> + ? ? ? ? ? ? goto exit;
>> + ? ? }
>> + ? ? return 0;
>> +exit:
>> + ? ? /* error, reset the controller. */
>> + ? ? i2c_reset_controller(i2c_bus);
>> +
>> + ? ? return error;
>> +}
>> +
>> +static int tegra2_i2c_write_data(u32 addr, u8 *data, u32 len)
>> +{
>> + ? ? int error;
>> + ? ? struct i2c_trans_info trans_info;
>> +
>> + ? ? trans_info.address = addr;
>> + ? ? trans_info.buf = data;
>> + ? ? trans_info.flags = I2C_IS_WRITE;
>> + ? ? trans_info.num_bytes = len;
>> + ? ? trans_info.is_10bit_address = 0;
>> +
>> + ? ? error = send_recv_packets(&i2c_controllers[i2c_bus_num], &trans_info);
>> + ? ? if (error)
>> + ? ? ? ? ? ? debug("tegra2_i2c_write_data: Error (%d) !!!\n", error);
>> +
>> + ? ? return error;
>> +}
>> +
>> +static int tegra2_i2c_read_data(u32 addr, u8 *data, u32 len)
>> +{
>> + ? ? int error;
>> + ? ? struct i2c_trans_info trans_info;
>> +
>> + ? ? trans_info.address = addr | 1;
>> + ? ? trans_info.buf = data;
>> + ? ? trans_info.flags = 0;
>> + ? ? trans_info.num_bytes = len;
>> + ? ? trans_info.is_10bit_address = 0;
>> +
>> + ? ? error = send_recv_packets(&i2c_controllers[i2c_bus_num], &trans_info);
>> + ? ? if (error)
>> + ? ? ? ? ? ? debug("tegra2_i2c_read_data: Error (%d) !!!\n", error);
>> +
>> + ? ? return error;
>> +}
>> +
>> +#ifndef CONFIG_OF_CONTROL
>> +static const enum periph_id i2c_periph_ids[CONFIG_SYS_MAX_I2C_BUS] = {
>> + ? ? PERIPH_ID_DVC_I2C,
>> + ? ? PERIPH_ID_I2C1,
>> + ? ? PERIPH_ID_I2C2,
>> + ? ? PERIPH_ID_I2C3
>> +};
>> +
>> +static const u32 *i2c_bus_base[CONFIG_SYS_MAX_I2C_BUS] = {
>> + ? ? (u32 *)TEGRA2_DVC_BASE,
>> + ? ? (u32 *)TEGRA2_I2C1_BASE,
>> + ? ? (u32 *)TEGRA2_I2C2_BASE,
>> + ? ? (u32 *)TEGRA2_I2C3_BASE
>> +};
>> +
>> +/* pinmux_configs based on the pinmux configuration */
>> +static const int pinmux_configs[CONFIG_SYS_MAX_I2C_BUS] = {
>> + ? ? CONFIG_I2CP_PIN_MUX, ? ?/* for I2CP (DVC I2C) */
>> + ? ? CONFIG_I2C1_PIN_MUX, ? ?/* for I2C1 */
>> + ? ? CONFIG_I2C2_PIN_MUX, ? ?/* for I2C2 */
>> + ? ? CONFIG_I2C3_PIN_MUX ? ? /* for I2C3 */
>> +};
>> +
>> +static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
>> +{
>> + ? ? int i = *index;
>> +
>> + ? ? if (i >= CONFIG_SYS_MAX_I2C_BUS)
>> + ? ? ? ? ? ? return -1;
>> +
>> + ? ? i2c_bus->periph_id = i2c_periph_ids[i];
>> + ? ? i2c_bus->pinmux_config = pinmux_configs[i];
>> + ? ? i2c_bus->regs = (struct i2c_ctlr *)i2c_bus_base[i];
>> + ? ? i2c_bus->speed = I2CSPEED_KHZ * 1000;
>> +
>> + ? ? *index = i + 1;
>> +
>> + ? ? return 0;
>> +}
>> +#else
>> +static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
>> +{
>> + ? ? const void *blob = gd->fdt_blob;
>> + ? ? int node;
>> +
>> + ? ? node = fdtdec_next_alias(blob, "i2c", COMPAT_NVIDIA_TEGRA20_I2C,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?index);
>> + ? ? if (node < 0)
>> + ? ? ? ? ? ? return -1;
>> +
>> + ? ? i2c_bus->regs = (struct i2c_ctlr *)fdtdec_get_addr(blob, node, "reg");
>> + ? ? i2c_bus->pinmux_config = fdtdec_get_int(blob, node, "u-boot,pinmux", 0);
>> + ? ? i2c_bus->speed = fdtdec_get_int(blob, node, "speed", 0);
>> + ? ? i2c_bus->periph_id = fdtdec_get_int(blob, node, "u-boot,periph-id", -1);
>> +
>> + ? ? if (i2c_bus->periph_id == -1)
>> + ? ? ? ? ? ? return -FDT_ERR_NOTFOUND;
>> +
>> + ? ? return 0;
>> +}
>> +#endif
>> +
>> +int i2c_init_board(void)
>> +{
>> + ? ? struct i2c_bus *i2c_bus;
>> + ? ? int index = 0;
>> + ? ? int i;
>> +
>> + ? ? /* build the i2c_controllers[] for each controller */
>> + ? ? for (i = 0; i < CONFIG_SYS_MAX_I2C_BUS; ++i) {
>> + ? ? ? ? ? ? i2c_bus = &i2c_controllers[i];
>> + ? ? ? ? ? ? i2c_bus->id = i;
>> +
>> + ? ? ? ? ? ? if (i2c_get_config(&index, i2c_bus)) {
>> + ? ? ? ? ? ? ? ? ? ? printf("i2c_init_board: failed to find bus %d\n", i);
>> + ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? }
>> +
>> + ? ? ? ? ? ? if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C)
>> + ? ? ? ? ? ? ? ? ? ? i2c_bus->control =
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? &((struct dvc_ctlr *)i2c_bus->regs)->control;
>> + ? ? ? ? ? ? else
>> + ? ? ? ? ? ? ? ? ? ? i2c_bus->control = &i2c_bus->regs->control;
>> +
>> + ? ? ? ? ? ? i2c_init_controller(i2c_bus);
>> + ? ? }
>> +
>> + ? ? return 0;
>> +}
>> +
>> +void i2c_init(int speed, int slaveaddr)
>> +{
>> + ? ? debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
>> +}
>> +
>> +/* i2c write version without the register address */
>> +int i2c_write_data(uchar chip, uchar *buffer, int len)
>> +{
>> + ? ? int rc;
>> +
>> + ? ? debug("i2c_write_data: chip=0x%x, len=0x%x\n", chip, len);
>> + ? ? debug("write_data: ");
>> + ? ? /* use rc for counter */
>> + ? ? for (rc = 0; rc < len; ++rc)
>> + ? ? ? ? ? ? debug(" 0x%02x", buffer[rc]);
>> + ? ? debug("\n");
>> +
>> + ? ? rc = tegra2_i2c_write_data(I2C_ADDR_ON_BUS(chip), buffer, len);
>> + ? ? if (rc)
>> + ? ? ? ? ? ? debug("i2c_write_data(): rc=%d\n", rc);
>> +
>> + ? ? return rc;
>> +}
>> +
>> +/* i2c read version without the register address */
>> +int i2c_read_data(uchar chip, uchar *buffer, int len)
>> +{
>> + ? ? int rc;
>> +
>> + ? ? debug("inside i2c_read_data():\n");
>> + ? ? rc = tegra2_i2c_read_data(I2C_ADDR_ON_BUS(chip), buffer, len);
>> + ? ? if (rc) {
>> + ? ? ? ? ? ? debug("i2c_read_data(): rc=%d\n", rc);
>> + ? ? ? ? ? ? return rc;
>> + ? ? }
>> +
>> + ? ? debug("i2c_read_data: ");
>> + ? ? /* reuse rc for counter*/
>> + ? ? for (rc = 0; rc < len; ++rc)
>> + ? ? ? ? ? ? debug(" 0x%02x", buffer[rc]);
>> + ? ? debug("\n");
>> +
>> + ? ? return 0;
>> +}
>> +
>> +/* Probe to see if a chip is present. */
>> +int i2c_probe(uchar chip)
>> +{
>> + ? ? int rc;
>> + ? ? uchar reg;
>> +
>> + ? ? debug("i2c_probe: addr=0x%x\n", chip);
>> + ? ? reg = 0;
>> + ? ? rc = i2c_write_data(chip, &reg, 1);
>> + ? ? if (rc) {
>> + ? ? ? ? ? ? debug("Error probing 0x%x.\n", chip);
>> + ? ? ? ? ? ? return 1;
>> + ? ? }
>> + ? ? return 0;
>> +}
>> +
>> +static int i2c_addr_ok(const uint addr, const int alen)
>> +{
>> + ? ? if (alen < 0 || alen > sizeof(addr))
>> + ? ? ? ? ? ? return 0;
>> + ? ? if (alen != sizeof(addr)) {
>> + ? ? ? ? ? ? uint max_addr = (1 << (8 * alen)) - 1;
>> + ? ? ? ? ? ? if (addr > max_addr)
>> + ? ? ? ? ? ? ? ? ? ? return 0;
>> + ? ? }
>> + ? ? return 1;
>> +}
>> +
>> +/* Read bytes */
>> +int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
>> +{
>> + ? ? uint offset;
>> + ? ? int i;
>> +
>> + ? ? debug("i2c_read: chip=0x%x, addr=0x%x, len=0x%x\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? chip, addr, len);
>> + ? ? if (!i2c_addr_ok(addr, alen)) {
>> + ? ? ? ? ? ? debug("i2c_read: Bad address %x.%d.\n", addr, alen);
>> + ? ? ? ? ? ? return 1;
>> + ? ? }
>> + ? ? for (offset = 0; offset < len; offset++) {
>> + ? ? ? ? ? ? if (alen) {
>> + ? ? ? ? ? ? ? ? ? ? uchar data[alen];
>> + ? ? ? ? ? ? ? ? ? ? for (i = 0; i < alen; i++) {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? data[alen - i - 1] =
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (addr + offset) >> (8 * i);
>> + ? ? ? ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? ? ? ? ? if (i2c_write_data(chip, data, alen)) {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? debug("i2c_read: error sending (0x%x)\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? addr);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? return 1;
>> + ? ? ? ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? }
>> + ? ? ? ? ? ? if (i2c_read_data(chip, buffer + offset, 1)) {
>> + ? ? ? ? ? ? ? ? ? ? debug("i2c_read: error reading (0x%x)\n", addr);
>> + ? ? ? ? ? ? ? ? ? ? return 1;
>> + ? ? ? ? ? ? }
>> + ? ? }
>> +
>> + ? ? return 0;
>> +}
>> +
>> +/* Write bytes */
>> +int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
>> +{
>> + ? ? uint offset;
>> + ? ? int i;
>> +
>> + ? ? debug("i2c_write: chip=0x%x, addr=0x%x, len=0x%x\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? chip, addr, len);
>> + ? ? if (!i2c_addr_ok(addr, alen)) {
>> + ? ? ? ? ? ? debug("i2c_write: Bad address %x.%d.\n", addr, alen);
>> + ? ? ? ? ? ? return 1;
>> + ? ? }
>> + ? ? for (offset = 0; offset < len; offset++) {
>> + ? ? ? ? ? ? uchar data[alen + 1];
>> + ? ? ? ? ? ? for (i = 0; i < alen; i++)
>> + ? ? ? ? ? ? ? ? ? ? data[alen - i - 1] = (addr + offset) >> (8 * i);
>> + ? ? ? ? ? ? data[alen] = buffer[offset];
>> + ? ? ? ? ? ? if (i2c_write_data(chip, data, alen + 1)) {
>> + ? ? ? ? ? ? ? ? ? ? debug("i2c_write: error sending (0x%x)\n", addr);
>> + ? ? ? ? ? ? ? ? ? ? return 1;
>> + ? ? ? ? ? ? }
>> + ? ? }
>> +
>> + ? ? return 0;
>> +}
>> +
>> +#if defined(CONFIG_I2C_MULTI_BUS)
>> +/*
>> + * Functions for multiple I2C bus handling
>> + */
>> +unsigned int i2c_get_bus_num(void)
>> +{
>> + ? ? return i2c_bus_num;
>> +}
>> +
>> +int i2c_set_bus_num(unsigned int bus)
>> +{
>> + ? ? if (bus >= CONFIG_SYS_MAX_I2C_BUS)
>> + ? ? ? ? ? ? return -1;
>> + ? ? i2c_bus_num = bus;
>> +
>> + ? ? return 0;
>> +}
>> +#endif
>
> get/set_speed missing ?

This was done during init, so hoping we don't have to do it every time?

>
>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>> index a8911b5..5547676 100644
>> --- a/include/fdtdec.h
>> +++ b/include/fdtdec.h
>> @@ -58,6 +58,7 @@ struct fdt_memory {
>> ?enum fdt_compat_id {
>> ? ? ? COMPAT_UNKNOWN,
>> ? ? ? COMPAT_NVIDIA_TEGRA20_USB, ? ? ?/* Tegra2 USB port */
>> + ? ? COMPAT_NVIDIA_TEGRA20_I2C, ? ? ?/* Tegra2 i2c */
>>
>> ? ? ? COMPAT_COUNT,
>> ?};
>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> index 931b4ce..fb3d79d 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -38,6 +38,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_TEGRA20_I2C, "nvidia,tegra20-i2c"),
>> ?};
>>
>> ?/**
> M

Regards,
Simon

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Marek Vasut]

> Hi Marek,
> 
> On Mon, Dec 26, 2011 at 11:15 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> From: Yen Lin <yelin@nvidia.com>
> >> 
> >> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
> >> The driver supports building both with and without CONFIG_OF_CONTROL.
> >> 
> >> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
> >> in the board config header file:
> >> 
> >> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
> >> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
> >>       (typically this will be 0 to bring the port out the common
> >>               pins)
> >> 
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> ---
> >>  arch/arm/include/asm/arch-tegra2/tegra2.h     |    4 +
> >>  arch/arm/include/asm/arch-tegra2/tegra2_i2c.h |  167 ++++++++
> >>  drivers/i2c/Makefile                          |    1 +
> >>  drivers/i2c/tegra2_i2c.c                      |  533
> >> +++++++++++++++++++++++++ include/fdtdec.h                            
> >>  | 1 +
> >>  lib/fdtdec.c                                  |    1 +
> >>  6 files changed, 707 insertions(+), 0 deletions(-)
> >>  create mode 100644 arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
> >>  create mode 100644 drivers/i2c/tegra2_i2c.c
> >> 
> >> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2.h
> >> b/arch/arm/include/asm/arch-tegra2/tegra2.h index ca1881e..d1a70da
> >> 100644 --- a/arch/arm/include/asm/arch-tegra2/tegra2.h
> >> +++ b/arch/arm/include/asm/arch-tegra2/tegra2.h
> >> @@ -40,6 +40,10 @@
> >>  #define NV_PA_APB_UARTE_BASE (NV_PA_APB_MISC_BASE + 0x6400)
> >>  #define TEGRA2_SPI_BASE              (NV_PA_APB_MISC_BASE + 0xC380)
> >>  #define TEGRA2_PMC_BASE              (NV_PA_APB_MISC_BASE + 0xE400)
> >> +#define TEGRA2_I2C1_BASE     (NV_PA_APB_MISC_BASE + 0xC000)
> >> +#define TEGRA2_I2C2_BASE     (NV_PA_APB_MISC_BASE + 0xC400)
> >> +#define TEGRA2_I2C3_BASE     (NV_PA_APB_MISC_BASE + 0xC500)
> >> +#define TEGRA2_DVC_BASE              (NV_PA_APB_MISC_BASE + 0xD000)
> >>  #define NV_PA_CSITE_BASE     0x70040000
> >>  #define TEGRA_USB1_BASE              0xC5000000
> >>  #define TEGRA_USB3_BASE              0xC5008000
> >> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
> >> b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h new file mode 100644
> >> index 0000000..4920d47
> >> --- /dev/null
> >> +++ b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
> >> @@ -0,0 +1,167 @@
> >> +/*
> >> + * NVIDIA Tegra2 I2C controller
> >> + *
> >> + * Copyright 2010-2011 NVIDIA Corporation
> >> + *
> >> + * This software may be used and distributed according to the
> >> + * terms of the GNU Public License, Version 2, incorporated
> >> + * herein by reference.
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License
> >> + * Version 2 as published by the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> >> + * MA 02111-1307 USA
> >> + */
> >> +
> >> +#ifndef _TEGRA2_I2C_H_
> >> +#define _TEGRA2_I2C_H_
> >> +
> >> +#include <asm/types.h>
> >> +
> >> +/* Convert the number of bytes to word. */
> >> +#define BYTES_TO_WORDS(size)         (((size) + 3) >> 2)
> > 
> > Isn't this stuff in include/common.h ? div_round_up() or something ?
> > 
> >> +
> >> +/* Convert i2c slave address to be put on bus  */
> >> +#define I2C_ADDR_ON_BUS(chip)                (chip << 1)
> >> +
> >> +#ifndef CONFIG_OF_CONTROL
> >> +enum {
> >> +     I2CSPEED_KHZ = 100,             /* in KHz */
> >> +};
> >> +#endif
> >> +
> >> +enum {
> >> +     I2C_TIMEOUT_USEC = 10000,       /* Wait time for completion */
> >> +     I2C_FIFO_DEPTH = 8,             /* I2C fifo depth */
> >> +};
> >> +
> >> +enum i2c_transaction_flags {
> >> +     I2C_IS_WRITE = 0x1,             /* for I2C write operation */
> >> +     I2C_IS_10_BIT_ADDRESS = 0x2,    /* for 10-bit I2C slave address */
> >> +     I2C_USE_REPEATED_START = 0x4,   /* for repeat start */
> >> +     I2C_NO_ACK = 0x8,               /* for slave that won't generate
> >> ACK */ +     I2C_SOFTWARE_CONTROLLER = 0x10, /* for I2C transfer using
> >> GPIO */ +     I2C_NO_STOP = 0x20,
> >> +};
> >> +
> >> +/* Contians the I2C transaction details */
> >> +struct i2c_trans_info {
> >> +     /* flags to indicate the transaction details */
> >> +     enum i2c_transaction_flags flags;
> >> +     u32 address;    /* I2C slave device address */
> >> +     u32 num_bytes;  /* number of bytes to be transferred */
> >> +     /* Send/receive buffer. For I2C send operation this buffer should
> >> be
> > 
> > Wrong comment, do you run this stuff through checkpatch ?
> 
> Fixed style and adjusted comment. Yes I do, but checkpatch isn't perfect.
> 
> >> +      * filled with the data to be sent to the slave device. For I2C
> >> receive +      * operation this buffer is filled with the data received
> >> from the +      * slave device. */
> >> +     u8 *buf;
> >> +     int is_10bit_address;
> >> +};
> >> +
> >> +struct i2c_control {
> >> +     u32 tx_fifo;
> >> +     u32 rx_fifo;
> >> +     u32 packet_status;
> >> +     u32 fifo_control;
> >> +     u32 fifo_status;
> >> +     u32 int_mask;
> >> +     u32 int_status;
> >> +};
> >> +
> >> +struct dvc_ctlr {
> >> +     u32 ctrl1;                      /* 00: DVC_CTRL_REG1 */
> >> +     u32 ctrl2;                      /* 04: DVC_CTRL_REG2 */
> >> +     u32 ctrl3;                      /* 08: DVC_CTRL_REG3 */
> >> +     u32 status;                     /* 0C: DVC_STATUS_REG */
> >> +     u32 ctrl;                       /* 10: DVC_I2C_CTRL_REG */
> >> +     u32 addr_data;                  /* 14: DVC_I2C_ADDR_DATA_REG */
> >> +     u32 reserved_0[2];              /* 18: */
> >> +     u32 req;                        /* 20: DVC_REQ_REGISTER */
> >> +     u32 addr_data3;                 /* 24: DVC_I2C_ADDR_DATA_REG_3 */
> >> +     u32 reserved_1[6];              /* 28: */
> >> +     u32 cnfg;                       /* 40: DVC_I2C_CNFG */
> >> +     u32 cmd_addr0;                  /* 44: DVC_I2C_CMD_ADDR0 */
> >> +     u32 cmd_addr1;                  /* 48: DVC_I2C_CMD_ADDR1 */
> >> +     u32 cmd_data1;                  /* 4C: DVC_I2C_CMD_DATA1 */
> >> +     u32 cmd_data2;                  /* 50: DVC_I2C_CMD_DATA2 */
> >> +     u32 reserved_2[2];              /* 54: */
> >> +     u32 i2c_status;                 /* 5C: DVC_I2C_STATUS */
> >> +     struct i2c_control control;     /* 60 ~ 78 */
> >> +};
> >> +
> >> +struct i2c_ctlr {
> >> +     u32 cnfg;                       /* 00: I2C_I2C_CNFG */
> >> +     u32 cmd_addr0;                  /* 04: I2C_I2C_CMD_ADDR0 */
> >> +     u32 cmd_addr1;                  /* 08: I2C_I2C_CMD_DATA1 */
> >> +     u32 cmd_data1;                  /* 0C: I2C_I2C_CMD_DATA2 */
> >> +     u32 cmd_data2;                  /* 10: DVC_I2C_CMD_DATA2 */
> >> +     u32 reserved_0[2];              /* 14: */
> >> +     u32 status;                     /* 1C: I2C_I2C_STATUS */
> >> +     u32 sl_cnfg;                    /* 20: I2C_I2C_SL_CNFG */
> >> +     u32 sl_rcvd;                    /* 24: I2C_I2C_SL_RCVD */
> >> +     u32 sl_status;                  /* 28: I2C_I2C_SL_STATUS */
> >> +     u32 sl_addr1;                   /* 2C: I2C_I2C_SL_ADDR1 */
> >> +     u32 sl_addr2;                   /* 30: I2C_I2C_SL_ADDR2 */
> >> +     u32 reserved_1[2];              /* 34: */
> >> +     u32 sl_delay_count;             /* 3C: I2C_I2C_SL_DELAY_COUNT */
> >> +     u32 reserved_2[4];              /* 40: */
> >> +     struct i2c_control control;     /* 50 ~ 68 */
> >> +};
> >> +
> >> +/* bit fields definitions for IO Packet Header 1 format */
> >> +#define PKT_HDR1_PROTOCOL_SHIFT              4
> >> +#define PKT_HDR1_PROTOCOL_MASK               (0xf <<
> >> PKT_HDR1_PROTOCOL_SHIFT) +#define PKT_HDR1_CTLR_ID_SHIFT              
> >> 12
> >> +#define PKT_HDR1_CTLR_ID_MASK                (0xf <<
> >> PKT_HDR1_CTLR_ID_SHIFT) +#define PKT_HDR1_PKT_ID_SHIFT              
> >>  16
> >> +#define PKT_HDR1_PKT_ID_MASK         (0xff << PKT_HDR1_PKT_ID_SHIFT)
> >> +#define PROTOCOL_TYPE_I2C            1
> >> +
> >> +/* bit fields definitions for IO Packet Header 2 format */
> >> +#define PKT_HDR2_PAYLOAD_SIZE_SHIFT  0
> >> +#define PKT_HDR2_PAYLOAD_SIZE_MASK   (0xfff <<
> >> PKT_HDR2_PAYLOAD_SIZE_SHIFT) +
> >> +/* bit fields definitions for IO Packet Header 3 format */
> >> +#define PKT_HDR3_READ_MODE_SHIFT     19
> >> +#define PKT_HDR3_READ_MODE_MASK              (1 <<
> >> PKT_HDR3_READ_MODE_SHIFT) +#define PKT_HDR3_SLAVE_ADDR_SHIFT    0
> >> +#define PKT_HDR3_SLAVE_ADDR_MASK     (0x3ff <<
> >> PKT_HDR3_SLAVE_ADDR_SHIFT) +
> >> +#define DVC_CTRL_REG3_I2C_HW_SW_PROG_SHIFT   26
> >> +#define DVC_CTRL_REG3_I2C_HW_SW_PROG_MASK    \
> >> +                             (1 << DVC_CTRL_REG3_I2C_HW_SW_PROG_SHIFT)
> >> +
> >> +/* I2C_CNFG */
> >> +#define I2C_CNFG_NEW_MASTER_FSM_SHIFT        11
> >> +#define I2C_CNFG_NEW_MASTER_FSM_MASK (1 <<
> >> I2C_CNFG_NEW_MASTER_FSM_SHIFT) +#define I2C_CNFG_PACKET_MODE_SHIFT   10
> >> +#define I2C_CNFG_PACKET_MODE_MASK    (1 << I2C_CNFG_PACKET_MODE_SHIFT)
> >> +
> >> +/* I2C_SL_CNFG */
> >> +#define I2C_SL_CNFG_NEWSL_SHIFT              2
> >> +#define I2C_SL_CNFG_NEWSL_MASK               (1 <<
> >> I2C_SL_CNFG_NEWSL_SHIFT) +
> >> +/* I2C_FIFO_STATUS */
> >> +#define TX_FIFO_FULL_CNT_SHIFT               0
> >> +#define TX_FIFO_FULL_CNT_MASK                (0xf <<
> >> TX_FIFO_FULL_CNT_SHIFT) +#define TX_FIFO_EMPTY_CNT_SHIFT              4
> >> +#define TX_FIFO_EMPTY_CNT_MASK               (0xf <<
> >> TX_FIFO_EMPTY_CNT_SHIFT) +
> >> +/* I2C_INTERRUPT_STATUS */
> >> +#define I2C_INT_XFER_COMPLETE_SHIFT  7
> >> +#define I2C_INT_XFER_COMPLETE_MASK   (1 << I2C_INT_XFER_COMPLETE_SHIFT)
> >> +#define I2C_INT_NO_ACK_SHIFT         3
> >> +#define I2C_INT_NO_ACK_MASK          (1 << I2C_INT_NO_ACK_SHIFT)
> >> +#define I2C_INT_ARBITRATION_LOST_SHIFT       2
> >> +#define I2C_INT_ARBITRATION_LOST_MASK        (1 <<
> >> I2C_INT_ARBITRATION_LOST_SHIFT) +
> >> +#endif
> >> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> >> index 504db03..c123c72 100644
> >> --- a/drivers/i2c/Makefile
> >> +++ b/drivers/i2c/Makefile
> >> @@ -41,6 +41,7 @@ COBJS-$(CONFIG_DRIVER_S3C24X0_I2C) += s3c24x0_i2c.o
> >>  COBJS-$(CONFIG_S3C44B0_I2C) += s3c44b0_i2c.o
> >>  COBJS-$(CONFIG_SOFT_I2C) += soft_i2c.o
> >>  COBJS-$(CONFIG_SPEAR_I2C) += spr_i2c.o
> >> +COBJS-$(CONFIG_TEGRA2_I2C) += tegra2_i2c.o
> >>  COBJS-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
> >>  COBJS-$(CONFIG_U8500_I2C) += u8500_i2c.o
> >>  COBJS-$(CONFIG_SH_I2C) += sh_i2c.o
> >> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
> >> new file mode 100644
> >> index 0000000..ae43531
> >> --- /dev/null
> >> +++ b/drivers/i2c/tegra2_i2c.c
> >> @@ -0,0 +1,533 @@
> >> +/*
> >> + * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
> >> + * Copyright (c) 2010-2011 NVIDIA Corporation
> >> + *  NVIDIA Corporation <www.nvidia.com>
> >> + *
> >> + * See file CREDITS for list of people who contributed to this
> >> + * project.
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License as
> >> + * published by the Free Software Foundation; either version 2 of
> >> + * the License, or (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> >> + * MA 02111-1307 USA
> >> + */
> >> +
> >> +#include <common.h>
> >> +#include <asm/io.h>
> >> +#include <asm/arch/clk_rst.h>
> >> +#include <asm/arch/clock.h>
> >> +#include <asm/arch/funcmux.h>
> >> +#include <asm/arch/gpio.h>
> >> +#include <asm/arch/pinmux.h>
> >> +#include <asm/arch/tegra2_i2c.h>
> >> +#include <fdtdec.h>
> >> +
> >> +DECLARE_GLOBAL_DATA_PTR;
> >> +
> >> +static unsigned int i2c_bus_num;
> >> +
> >> +/* Information about i2c controller */
> >> +struct i2c_bus {
> >> +     int                     id;
> >> +     enum periph_id          periph_id;
> >> +     int                     speed;
> >> +     int                     pinmux_config;
> >> +     struct i2c_control      *control;
> >> +     struct i2c_ctlr         *regs;
> >> +};
> >> +
> >> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
> > 
> > static
> 
> done
> 
> >> +
> >> +static void set_packet_mode(struct i2c_bus *i2c_bus)
> >> +{
> >> +     u32 config;
> >> +
> >> +     config = I2C_CNFG_NEW_MASTER_FSM_MASK | I2C_CNFG_PACKET_MODE_MASK;
> >> +
> >> +     if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C) {
> >> +             struct dvc_ctlr *dvc = (struct dvc_ctlr *)i2c_bus->regs;
> >> +
> >> +             writel(config, &dvc->cnfg);
> >> +     } else {
> >> +             writel(config, &i2c_bus->regs->cnfg);
> >> +             /*
> >> +              * program I2C_SL_CNFG.NEWSL to ENABLE. This fixes probe
> >> +              * issues, i.e., some slaves may be wrongly detected.
> >> +              */
> >> +             setbits_le32(&i2c_bus->regs->sl_cnfg,
> >> I2C_SL_CNFG_NEWSL_MASK); +     }
> >> +}
> >> +
> >> +static void i2c_reset_controller(struct i2c_bus *i2c_bus)
> >> +{
> >> +     /* Reset I2C controller. */
> >> +     reset_periph(i2c_bus->periph_id, 1);
> >> +
> >> +     /* re-program config register to packet mode */
> >> +     set_packet_mode(i2c_bus);
> >> +}
> >> +
> >> +static void i2c_init_controller(struct i2c_bus *i2c_bus)
> >> +{
> >> +     /* TODO: Fix bug which makes us need to do this */
> >> +     clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_OSC,
> >> +                            i2c_bus->speed * (8 * 2 - 1));
> >> +
> >> +     /* Reset I2C controller. */
> >> +     i2c_reset_controller(i2c_bus);
> >> +
> >> +     /* Configure I2C controller. */
> >> +     if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C) {  /* only for DVC
> >> I2C */ +             struct dvc_ctlr *dvc = (struct dvc_ctlr
> >> *)i2c_bus->regs; +
> >> +             setbits_le32(&dvc->ctrl3,
> >> DVC_CTRL_REG3_I2C_HW_SW_PROG_MASK); +     }
> >> +
> >> +     funcmux_select(i2c_bus->periph_id, i2c_bus->pinmux_config);
> >> +}
> >> +
> >> +static void send_packet_headers(
> >> +     struct i2c_bus *i2c_bus,
> >> +     struct i2c_trans_info *trans,
> >> +     u32 packet_id)
> >> +{
> >> +     u32 data;
> >> +
> >> +     /* prepare header1: Header size = 0 Protocol = I2C, pktType = 0 */
> >> +     data = PROTOCOL_TYPE_I2C << PKT_HDR1_PROTOCOL_SHIFT;
> >> +     data |= packet_id << PKT_HDR1_PKT_ID_SHIFT;
> >> +     data |= i2c_bus->id << PKT_HDR1_CTLR_ID_SHIFT;
> >> +     writel(data, &i2c_bus->control->tx_fifo);
> >> +     debug("pkt header 1 sent (0x%x)\n", data);
> >> +
> >> +     /* prepare header2 */
> >> +     data = (trans->num_bytes - 1) << PKT_HDR2_PAYLOAD_SIZE_SHIFT;
> >> +     writel(data, &i2c_bus->control->tx_fifo);
> >> +     debug("pkt header 2 sent (0x%x)\n", data);
> >> +
> >> +     /* prepare IO specific header: configure the slave address */
> >> +     data = trans->address << PKT_HDR3_SLAVE_ADDR_SHIFT;
> >> +
> >> +     /* Enable Read if it is not a write transaction */
> >> +     if (!(trans->flags & I2C_IS_WRITE))
> >> +             data |= PKT_HDR3_READ_MODE_MASK;
> >> +
> >> +     /* Write I2C specific header */
> >> +     writel(data, &i2c_bus->control->tx_fifo);
> >> +     debug("pkt header 3 sent (0x%x)\n", data);
> >> +}
> >> +
> >> +static int wait_for_tx_fifo_empty(struct i2c_control *control)
> >> +{
> >> +     u32 count;
> >> +     int timeout_us = I2C_TIMEOUT_USEC;
> >> +
> >> +     while (timeout_us >= 0) {
> >> +             count = (readl(&control->fifo_status) &
> >> TX_FIFO_EMPTY_CNT_MASK) +                             >>
> >> TX_FIFO_EMPTY_CNT_SHIFT;
> >> +             if (count == I2C_FIFO_DEPTH)
> >> +                     return 1;
> >> +             udelay(10);
> >> +             timeout_us -= 10;
> >> +     };
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static int wait_for_rx_fifo_notempty(struct i2c_control *control)
> >> +{
> >> +     u32 count;
> >> +     int timeout_us = I2C_TIMEOUT_USEC;
> >> +
> >> +     while (timeout_us >= 0) {
> >> +             count = (readl(&control->fifo_status) &
> >> TX_FIFO_FULL_CNT_MASK) +                             >>
> >> TX_FIFO_FULL_CNT_SHIFT;
> >> +             if (count)
> >> +                     return 1;
> >> +             udelay(10);
> >> +             timeout_us -= 10;
> >> +     };
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static int wait_for_transfer_complete(struct i2c_control *control)
> >> +{
> >> +     int int_status;
> >> +     int timeout_us = I2C_TIMEOUT_USEC;
> >> +
> >> +     while (timeout_us >= 0) {
> >> +             int_status = readl(&control->int_status);
> >> +             if (int_status & I2C_INT_NO_ACK_MASK)
> >> +                     return -int_status;
> >> +             if (int_status & I2C_INT_ARBITRATION_LOST_MASK)
> >> +                     return -int_status;
> >> +             if (int_status & I2C_INT_XFER_COMPLETE_MASK)
> >> +                     return 0;
> >> +
> >> +             udelay(10);
> >> +             timeout_us -= 10;
> >> +     };
> >> +
> >> +     return -1;
> >> +}
> >> +
> >> +static int send_recv_packets(
> >> +     struct i2c_bus *i2c_bus,
> >> +     struct i2c_trans_info *trans)
> >> +{
> >> +     struct i2c_control *control = i2c_bus->control;
> >> +     u32 int_status;
> >> +     u32 words;
> >> +     u8 *dptr;
> >> +     u32 local;
> >> +     uchar last_bytes;
> >> +     int error = 0;
> >> +     int is_write = trans->flags & I2C_IS_WRITE;
> >> +
> >> +     /* clear status from previous transaction, XFER_COMPLETE, NOACK,
> >> etc. */ +     int_status = readl(&control->int_status);
> >> +     writel(int_status, &control->int_status);
> >> +
> >> +     send_packet_headers(i2c_bus, trans, 1);
> >> +
> >> +     words = BYTES_TO_WORDS(trans->num_bytes);
> >> +     last_bytes = trans->num_bytes & 3;
> >> +     dptr = trans->buf;
> >> +
> >> +     while (words) {
> >> +             if (is_write) {
> >> +                     /* deal with word alignment */
> >> +                     if ((unsigned)dptr & 3) {
> >> +                             memcpy(&local, dptr, sizeof(u32));
> >> +                             writel(local, &control->tx_fifo);
> >> +                             debug("pkt data sent (0x%x)\n", local);
> >> +                     } else {
> >> +                             writel(*(u32 *)dptr, &control->tx_fifo);
> >> +                             debug("pkt data sent (0x%x)\n", *(u32
> >> *)dptr); +                     }
> >> +                     if (!wait_for_tx_fifo_empty(control)) {
> >> +                             error = -1;
> >> +                             goto exit;
> >> +                     }
> >> +             } else {
> >> +                     if (!wait_for_rx_fifo_notempty(control)) {
> >> +                             error = -1;
> >> +                             goto exit;
> >> +                     }
> >> +                     /*
> >> +                      * for the last word, we read into our local
> >> buffer, +                      * in case that caller did not provide
> >> enough buffer. +                      */
> >> +                     local = readl(&control->rx_fifo);
> >> +                     if ((words == 1) && last_bytes)
> >> +                             memcpy(dptr, (char *)&local, last_bytes);
> >> +                     else if ((unsigned)dptr & 3)
> >> +                             memcpy(dptr, &local, sizeof(u32));
> >> +                     else
> >> +                             *(u32 *)dptr = local;
> >> +                     debug("pkt data received (0x%x)\n", local);
> >> +             }
> >> +             words--;
> >> +             dptr += sizeof(u32);
> >> +     }
> >> +
> >> +     if (wait_for_transfer_complete(control)) {
> >> +             error = -1;
> >> +             goto exit;
> >> +     }
> >> +     return 0;
> >> +exit:
> >> +     /* error, reset the controller. */
> >> +     i2c_reset_controller(i2c_bus);
> >> +
> >> +     return error;
> >> +}
> >> +
> >> +static int tegra2_i2c_write_data(u32 addr, u8 *data, u32 len)
> >> +{
> >> +     int error;
> >> +     struct i2c_trans_info trans_info;
> >> +
> >> +     trans_info.address = addr;
> >> +     trans_info.buf = data;
> >> +     trans_info.flags = I2C_IS_WRITE;
> >> +     trans_info.num_bytes = len;
> >> +     trans_info.is_10bit_address = 0;
> >> +
> >> +     error = send_recv_packets(&i2c_controllers[i2c_bus_num],
> >> &trans_info); +     if (error)
> >> +             debug("tegra2_i2c_write_data: Error (%d) !!!\n", error);
> >> +
> >> +     return error;
> >> +}
> >> +
> >> +static int tegra2_i2c_read_data(u32 addr, u8 *data, u32 len)
> >> +{
> >> +     int error;
> >> +     struct i2c_trans_info trans_info;
> >> +
> >> +     trans_info.address = addr | 1;
> >> +     trans_info.buf = data;
> >> +     trans_info.flags = 0;
> >> +     trans_info.num_bytes = len;
> >> +     trans_info.is_10bit_address = 0;
> >> +
> >> +     error = send_recv_packets(&i2c_controllers[i2c_bus_num],
> >> &trans_info); +     if (error)
> >> +             debug("tegra2_i2c_read_data: Error (%d) !!!\n", error);
> >> +
> >> +     return error;
> >> +}
> >> +
> >> +#ifndef CONFIG_OF_CONTROL
> >> +static const enum periph_id i2c_periph_ids[CONFIG_SYS_MAX_I2C_BUS] = {
> >> +     PERIPH_ID_DVC_I2C,
> >> +     PERIPH_ID_I2C1,
> >> +     PERIPH_ID_I2C2,
> >> +     PERIPH_ID_I2C3
> >> +};
> >> +
> >> +static const u32 *i2c_bus_base[CONFIG_SYS_MAX_I2C_BUS] = {
> >> +     (u32 *)TEGRA2_DVC_BASE,
> >> +     (u32 *)TEGRA2_I2C1_BASE,
> >> +     (u32 *)TEGRA2_I2C2_BASE,
> >> +     (u32 *)TEGRA2_I2C3_BASE
> >> +};
> >> +
> >> +/* pinmux_configs based on the pinmux configuration */
> >> +static const int pinmux_configs[CONFIG_SYS_MAX_I2C_BUS] = {
> >> +     CONFIG_I2CP_PIN_MUX,    /* for I2CP (DVC I2C) */
> >> +     CONFIG_I2C1_PIN_MUX,    /* for I2C1 */
> >> +     CONFIG_I2C2_PIN_MUX,    /* for I2C2 */
> >> +     CONFIG_I2C3_PIN_MUX     /* for I2C3 */
> >> +};
> >> +
> >> +static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
> >> +{
> >> +     int i = *index;
> >> +
> >> +     if (i >= CONFIG_SYS_MAX_I2C_BUS)
> >> +             return -1;
> >> +
> >> +     i2c_bus->periph_id = i2c_periph_ids[i];
> >> +     i2c_bus->pinmux_config = pinmux_configs[i];
> >> +     i2c_bus->regs = (struct i2c_ctlr *)i2c_bus_base[i];
> >> +     i2c_bus->speed = I2CSPEED_KHZ * 1000;
> >> +
> >> +     *index = i + 1;
> >> +
> >> +     return 0;
> >> +}
> >> +#else
> >> +static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
> >> +{
> >> +     const void *blob = gd->fdt_blob;
> >> +     int node;
> >> +
> >> +     node = fdtdec_next_alias(blob, "i2c", COMPAT_NVIDIA_TEGRA20_I2C,
> >> +                              index);
> >> +     if (node < 0)
> >> +             return -1;
> >> +
> >> +     i2c_bus->regs = (struct i2c_ctlr *)fdtdec_get_addr(blob, node,
> >> "reg"); +     i2c_bus->pinmux_config = fdtdec_get_int(blob, node,
> >> "u-boot,pinmux", 0); +     i2c_bus->speed = fdtdec_get_int(blob, node,
> >> "speed", 0);
> >> +     i2c_bus->periph_id = fdtdec_get_int(blob, node,
> >> "u-boot,periph-id", -1); +
> >> +     if (i2c_bus->periph_id == -1)
> >> +             return -FDT_ERR_NOTFOUND;
> >> +
> >> +     return 0;
> >> +}
> >> +#endif
> >> +
> >> +int i2c_init_board(void)
> >> +{
> >> +     struct i2c_bus *i2c_bus;
> >> +     int index = 0;
> >> +     int i;
> >> +
> >> +     /* build the i2c_controllers[] for each controller */
> >> +     for (i = 0; i < CONFIG_SYS_MAX_I2C_BUS; ++i) {
> >> +             i2c_bus = &i2c_controllers[i];
> >> +             i2c_bus->id = i;
> >> +
> >> +             if (i2c_get_config(&index, i2c_bus)) {
> >> +                     printf("i2c_init_board: failed to find bus %d\n",
> >> i); +                     return -1;
> >> +             }
> >> +
> >> +             if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C)
> >> +                     i2c_bus->control =
> >> +                             &((struct dvc_ctlr
> >> *)i2c_bus->regs)->control; +             else
> >> +                     i2c_bus->control = &i2c_bus->regs->control;
> >> +
> >> +             i2c_init_controller(i2c_bus);
> >> +     }
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +void i2c_init(int speed, int slaveaddr)
> >> +{
> >> +     debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
> >> +}
> >> +
> >> +/* i2c write version without the register address */
> >> +int i2c_write_data(uchar chip, uchar *buffer, int len)
> >> +{
> >> +     int rc;
> >> +
> >> +     debug("i2c_write_data: chip=0x%x, len=0x%x\n", chip, len);
> >> +     debug("write_data: ");
> >> +     /* use rc for counter */
> >> +     for (rc = 0; rc < len; ++rc)
> >> +             debug(" 0x%02x", buffer[rc]);
> >> +     debug("\n");
> >> +
> >> +     rc = tegra2_i2c_write_data(I2C_ADDR_ON_BUS(chip), buffer, len);
> >> +     if (rc)
> >> +             debug("i2c_write_data(): rc=%d\n", rc);
> >> +
> >> +     return rc;
> >> +}
> >> +
> >> +/* i2c read version without the register address */
> >> +int i2c_read_data(uchar chip, uchar *buffer, int len)
> >> +{
> >> +     int rc;
> >> +
> >> +     debug("inside i2c_read_data():\n");
> >> +     rc = tegra2_i2c_read_data(I2C_ADDR_ON_BUS(chip), buffer, len);
> >> +     if (rc) {
> >> +             debug("i2c_read_data(): rc=%d\n", rc);
> >> +             return rc;
> >> +     }
> >> +
> >> +     debug("i2c_read_data: ");
> >> +     /* reuse rc for counter*/
> >> +     for (rc = 0; rc < len; ++rc)
> >> +             debug(" 0x%02x", buffer[rc]);
> >> +     debug("\n");
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +/* Probe to see if a chip is present. */
> >> +int i2c_probe(uchar chip)
> >> +{
> >> +     int rc;
> >> +     uchar reg;
> >> +
> >> +     debug("i2c_probe: addr=0x%x\n", chip);
> >> +     reg = 0;
> >> +     rc = i2c_write_data(chip, &reg, 1);
> >> +     if (rc) {
> >> +             debug("Error probing 0x%x.\n", chip);
> >> +             return 1;
> >> +     }
> >> +     return 0;
> >> +}
> >> +
> >> +static int i2c_addr_ok(const uint addr, const int alen)
> >> +{
> >> +     if (alen < 0 || alen > sizeof(addr))
> >> +             return 0;
> >> +     if (alen != sizeof(addr)) {
> >> +             uint max_addr = (1 << (8 * alen)) - 1;
> >> +             if (addr > max_addr)
> >> +                     return 0;
> >> +     }
> >> +     return 1;
> >> +}
> >> +
> >> +/* Read bytes */
> >> +int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
> >> +{
> >> +     uint offset;
> >> +     int i;
> >> +
> >> +     debug("i2c_read: chip=0x%x, addr=0x%x, len=0x%x\n",
> >> +                             chip, addr, len);
> >> +     if (!i2c_addr_ok(addr, alen)) {
> >> +             debug("i2c_read: Bad address %x.%d.\n", addr, alen);
> >> +             return 1;
> >> +     }
> >> +     for (offset = 0; offset < len; offset++) {
> >> +             if (alen) {
> >> +                     uchar data[alen];
> >> +                     for (i = 0; i < alen; i++) {
> >> +                             data[alen - i - 1] =
> >> +                                     (addr + offset) >> (8 * i);
> >> +                     }
> >> +                     if (i2c_write_data(chip, data, alen)) {
> >> +                             debug("i2c_read: error sending (0x%x)\n",
> >> +                                     addr);
> >> +                             return 1;
> >> +                     }
> >> +             }
> >> +             if (i2c_read_data(chip, buffer + offset, 1)) {
> >> +                     debug("i2c_read: error reading (0x%x)\n", addr);
> >> +                     return 1;
> >> +             }
> >> +     }
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +/* Write bytes */
> >> +int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
> >> +{
> >> +     uint offset;
> >> +     int i;
> >> +
> >> +     debug("i2c_write: chip=0x%x, addr=0x%x, len=0x%x\n",
> >> +                             chip, addr, len);
> >> +     if (!i2c_addr_ok(addr, alen)) {
> >> +             debug("i2c_write: Bad address %x.%d.\n", addr, alen);
> >> +             return 1;
> >> +     }
> >> +     for (offset = 0; offset < len; offset++) {
> >> +             uchar data[alen + 1];
> >> +             for (i = 0; i < alen; i++)
> >> +                     data[alen - i - 1] = (addr + offset) >> (8 * i);
> >> +             data[alen] = buffer[offset];
> >> +             if (i2c_write_data(chip, data, alen + 1)) {
> >> +                     debug("i2c_write: error sending (0x%x)\n", addr);
> >> +                     return 1;
> >> +             }
> >> +     }
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +#if defined(CONFIG_I2C_MULTI_BUS)
> >> +/*
> >> + * Functions for multiple I2C bus handling
> >> + */
> >> +unsigned int i2c_get_bus_num(void)
> >> +{
> >> +     return i2c_bus_num;
> >> +}
> >> +
> >> +int i2c_set_bus_num(unsigned int bus)
> >> +{
> >> +     if (bus >= CONFIG_SYS_MAX_I2C_BUS)
> >> +             return -1;
> >> +     i2c_bus_num = bus;
> >> +
> >> +     return 0;
> >> +}
> >> +#endif
> > 
> > get/set_speed missing ?
> 
> This was done during init, so hoping we don't have to do it every time?

No, but you can use those functions to adjust the speed on the fly (there's i2c 
speed ... command in uboot)
> 
> >> diff --git a/include/fdtdec.h b/include/fdtdec.h
> >> index a8911b5..5547676 100644
> >> --- a/include/fdtdec.h
> >> +++ b/include/fdtdec.h
> >> @@ -58,6 +58,7 @@ struct fdt_memory {
> >>  enum fdt_compat_id {
> >>       COMPAT_UNKNOWN,
> >>       COMPAT_NVIDIA_TEGRA20_USB,      /* Tegra2 USB port */
> >> +     COMPAT_NVIDIA_TEGRA20_I2C,      /* Tegra2 i2c */
> >> 
> >>       COMPAT_COUNT,
> >>  };
> >> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> >> index 931b4ce..fb3d79d 100644
> >> --- a/lib/fdtdec.c
> >> +++ b/lib/fdtdec.c
> >> @@ -38,6 +38,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_TEGRA20_I2C, "nvidia,tegra20-i2c"),
> >>  };
> >> 
> >>  /**
> > 
> > M
> 
> Regards,
> Simon

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Simon Glass]

Hi Marek,

On Sun, Jan 8, 2012 at 9:06 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> Hi Marek,
[snip]
>> >> +
>> >> +int i2c_set_bus_num(unsigned int bus)
>> >> +{
>> >> + ? ? if (bus >= CONFIG_SYS_MAX_I2C_BUS)
>> >> + ? ? ? ? ? ? return -1;
>> >> + ? ? i2c_bus_num = bus;
>> >> +
>> >> + ? ? return 0;
>> >> +}
>> >> +#endif
>> >
>> > get/set_speed missing ?
>>
>> This was done during init, so hoping we don't have to do it every time?
>
> No, but you can use those functions to adjust the speed on the fly (there's i2c
> speed ... command in uboot)

OK I see. I will take a look at implementing i2c_set_bus_speed() and
i2c_get_bus_speed().

Regards,
Simon

>>
>> >> diff --git a/include/fdtdec.h b/include/fdtdec.h
>> >> index a8911b5..5547676 100644
>> >> --- a/include/fdtdec.h
>> >> +++ b/include/fdtdec.h
>> >> @@ -58,6 +58,7 @@ struct fdt_memory {
>> >> ?enum fdt_compat_id {
>> >> ? ? ? COMPAT_UNKNOWN,
>> >> ? ? ? COMPAT_NVIDIA_TEGRA20_USB, ? ? ?/* Tegra2 USB port */
>> >> + ? ? COMPAT_NVIDIA_TEGRA20_I2C, ? ? ?/* Tegra2 i2c */
>> >>
>> >> ? ? ? COMPAT_COUNT,
>> >> ?};
>> >> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> >> index 931b4ce..fb3d79d 100644
>> >> --- a/lib/fdtdec.c
>> >> +++ b/lib/fdtdec.c
>> >> @@ -38,6 +38,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_TEGRA20_I2C, "nvidia,tegra20-i2c"),
>> >> ?};
>> >>
>> >> ?/**
>> >
>> > M
>>
>> Regards,
>> Simon

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Mike Frysinger]

On Monday 26 December 2011 13:11:48 Simon Glass wrote:
> --- /dev/null
> +++ b/drivers/i2c/tegra2_i2c.c
>
> +static int wait_for_tx_fifo_empty(struct i2c_control *control)
> ...
> +	while (timeout_us >= 0) {
> ...
> +	};

no semicolon here

> +static int wait_for_rx_fifo_notempty(struct i2c_control *control)
> ...
> +	while (timeout_us >= 0) {
> ...
> +	};

no semicolon here

> +static int wait_for_transfer_complete(struct i2c_control *control)
> ...
> +	while (timeout_us >= 0) {
> ...
> +	};

no semicolon here
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120108/93a22cf9/attachment.pgp>

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Simon Glass]

Hi Mike,

On Sat, Jan 7, 2012 at 9:57 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday 26 December 2011 13:11:48 Simon Glass wrote:
>> --- /dev/null
>> +++ b/drivers/i2c/tegra2_i2c.c
>>
>> +static int wait_for_tx_fifo_empty(struct i2c_control *control)
>> ...
>> + ? ? while (timeout_us >= 0) {
>> ...
>> + ? ? };
>
> no semicolon here
>
>> +static int wait_for_rx_fifo_notempty(struct i2c_control *control)
>> ...
>> + ? ? while (timeout_us >= 0) {
>> ...
>> + ? ? };
>
> no semicolon here
>
>> +static int wait_for_transfer_complete(struct i2c_control *control)
>> ...
>> + ? ? while (timeout_us >= 0) {
>> ...
>> + ? ? };
>
> no semicolon here
> -mike

done - thanks.

Regards,
Simon

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Stephen Warren]

On 12/26/2011 11:11 AM, Simon Glass wrote:
> From: Yen Lin <yelin@nvidia.com>
> 
> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
> The driver supports building both with and without CONFIG_OF_CONTROL.
> 
> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
> in the board config header file:
> 
> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
>         (typically this will be 0 to bring the port out the common
>                 pins)
...
> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
...
> +#ifndef CONFIG_OF_CONTROL
> +enum {
> +       I2CSPEED_KHZ = 100,             /* in KHz */
> +};
> +#endif

The patch description says the board needs to define I2CSPEED_KHZ, yet
that seems to allow the board to override it, otherwise a default is
used. The patch description probably needs to be fixed.

The way the optional override is implemented is a little confusing. I'd
rather see:

#ifndef CONFIG_OF_CONTROL
#ifndef I2CSPEED_KHZ
#define I2CSPEED_KHZ 100
#endif
#endif

That makes it a lot more obvious it's a default. And actually, why not
put that within some other ifndef CONFIG_OF_CONTROL where the
enum/define is used, since it's presumably only used very locally in the
non-OF initialization code.

...
> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
...
> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];

What if there are I2C bus extenders/muxes/... such that there are more
I2C buses in the system than Tegra I2C controllers? I'd rather see an
explicit TEGRA_I2C_NUM_CONTROLLERS define used throughout this patch.

I didn't really review the I2C driver code itself, since I'm not
terribly familiar with the I2C HW details.

> +static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
> +{
> +       const void *blob = gd->fdt_blob;
> +       int node;
> +
> +       node = fdtdec_next_alias(blob, "i2c", COMPAT_NVIDIA_TEGRA20_I2C,
> +                                index);
> +       if (node < 0)
> +               return -1;

I assume the I2C patches will also be reworked to enumerate based on
nodes with a specific compatible flag, and then optionally using aliases
to number them, just like you said you're rework USB?

> +int i2c_init_board(void)
> +{
> +       struct i2c_bus *i2c_bus;
> +       int index = 0;
> +       int i;
> +
> +       /* build the i2c_controllers[] for each controller */
> +       for (i = 0; i < CONFIG_SYS_MAX_I2C_BUS; ++i) {
> +               i2c_bus = &i2c_controllers[i];
> +               i2c_bus->id = i;
> +
> +               if (i2c_get_config(&index, i2c_bus)) {
> +                       printf("i2c_init_board: failed to find bus %d\n", i);
> +                       return -1;
> +               }
> +
> +               if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C)
> +                       i2c_bus->control =
> +                               &((struct dvc_ctlr *)i2c_bus->regs)->control;
> +               else
> +                       i2c_bus->control = &i2c_bus->regs->control;

When instantiating controllers from device tree (as opposed to the
static !CONFIG_OF_CONTROL case), that conditional should be driven by
device tree properties. The kernel already represents this using two
separate compatible values for the different HW: nvidia,tegra20-i2c and
nvidia,tegra20-i2c-dvc.

> +
> +               i2c_init_controller(i2c_bus);
> +       }
> +
> +       return 0;
> +}

> +void i2c_init(int speed, int slaveaddr)
> +{
> +       debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
> +}

Surely that function needs to actually do something, at least set up the
clocks so that the (user?) requested rate is honored, or print an error
message if you're only allowed to use the hard-coded bus rate.

> +/* Probe to see if a chip is present. */
> +int i2c_probe(uchar chip)
> +{
> +       int rc;
> +       uchar reg;
> +
> +       debug("i2c_probe: addr=0x%x\n", chip);
> +       reg = 0;
> +       rc = i2c_write_data(chip, &reg, 1);
> +       if (rc) {
> +               debug("Error probing 0x%x.\n", chip);
> +               return 1;
> +       }
> +       return 0;
> +}
> +

> +static int i2c_addr_ok(const uint addr, const int alen)
> +{
> +       if (alen < 0 || alen > sizeof(addr))
> +               return 0;
> +       if (alen != sizeof(addr)) {
> +               uint max_addr = (1 << (8 * alen)) - 1;
> +               if (addr > max_addr)
> +                       return 0;
> +       }
> +       return 1;
> +}

That seems rather roundabout. Only two address lengths are valid; 7 and
10-bit, so wouldn't it be better to only accept those specific values?

-- 
nvpublic

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Simon Glass]

Hi Stephem,

On Mon, Jan 9, 2012 at 2:07 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 12/26/2011 11:11 AM, Simon Glass wrote:
>> From: Yen Lin <yelin@nvidia.com>
>>
>> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
>> The driver supports building both with and without CONFIG_OF_CONTROL.
>>
>> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
>> in the board config header file:
>>
>> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
>> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
>> ? ? ? ? (typically this will be 0 to bring the port out the common
>> ? ? ? ? ? ? ? ? pins)
> ...
>> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h
> ...
>> +#ifndef CONFIG_OF_CONTROL
>> +enum {
>> + ? ? ? I2CSPEED_KHZ = 100, ? ? ? ? ? ? /* in KHz */
>> +};
>> +#endif
>
> The patch description says the board needs to define I2CSPEED_KHZ, yet
> that seems to allow the board to override it, otherwise a default is
> used. The patch description probably needs to be fixed.
>
> The way the optional override is implemented is a little confusing. I'd
> rather see:
>
> #ifndef CONFIG_OF_CONTROL
> #ifndef I2CSPEED_KHZ
> #define I2CSPEED_KHZ 100
> #endif
> #endif
>
> That makes it a lot more obvious it's a default. And actually, why not
> put that within some other ifndef CONFIG_OF_CONTROL where the
> enum/define is used, since it's presumably only used very locally in the
> non-OF initialization code.

I'm removing the non-OF code since we don't use it.

>
> ...
>> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
> ...
>> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
>
> What if there are I2C bus extenders/muxes/... such that there are more
> I2C buses in the system than Tegra I2C controllers? I'd rather see an
> explicit TEGRA_I2C_NUM_CONTROLLERS define used throughout this patch.

We don't actually support CONFIG_I2C_MUX, so I can't see how that
could happen. Can you please explain a bit more?

>
> I didn't really review the I2C driver code itself, since I'm not
> terribly familiar with the I2C HW details.

It had quite a bit of review before it went into our tree, and it
certainly has had a lot of testing.

>
>> +static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
>> +{
>> + ? ? ? const void *blob = gd->fdt_blob;
>> + ? ? ? int node;
>> +
>> + ? ? ? node = fdtdec_next_alias(blob, "i2c", COMPAT_NVIDIA_TEGRA20_I2C,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?index);
>> + ? ? ? if (node < 0)
>> + ? ? ? ? ? ? ? return -1;
>
> I assume the I2C patches will also be reworked to enumerate based on
> nodes with a specific compatible flag, and then optionally using aliases
> to number them, just like you said you're rework USB?

Yes, will do. The hard bit of that was creating an fdtdec function to handle it.

>
>> +int i2c_init_board(void)
>> +{
>> + ? ? ? struct i2c_bus *i2c_bus;
>> + ? ? ? int index = 0;
>> + ? ? ? int i;
>> +
>> + ? ? ? /* build the i2c_controllers[] for each controller */
>> + ? ? ? for (i = 0; i < CONFIG_SYS_MAX_I2C_BUS; ++i) {
>> + ? ? ? ? ? ? ? i2c_bus = &i2c_controllers[i];
>> + ? ? ? ? ? ? ? i2c_bus->id = i;
>> +
>> + ? ? ? ? ? ? ? if (i2c_get_config(&index, i2c_bus)) {
>> + ? ? ? ? ? ? ? ? ? ? ? printf("i2c_init_board: failed to find bus %d\n", i);
>> + ? ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? ? }
>> +
>> + ? ? ? ? ? ? ? if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C)
>> + ? ? ? ? ? ? ? ? ? ? ? i2c_bus->control =
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &((struct dvc_ctlr *)i2c_bus->regs)->control;
>> + ? ? ? ? ? ? ? else
>> + ? ? ? ? ? ? ? ? ? ? ? i2c_bus->control = &i2c_bus->regs->control;
>
> When instantiating controllers from device tree (as opposed to the
> static !CONFIG_OF_CONTROL case), that conditional should be driven by
> device tree properties. The kernel already represents this using two
> separate compatible values for the different HW: nvidia,tegra20-i2c and
> nvidia,tegra20-i2c-dvc.

Not in the device tree file I got from the kernel...has it changed?

I will make this change. By dropping non-fdt use it will make this
work almost for free.

>
>> +
>> + ? ? ? ? ? ? ? i2c_init_controller(i2c_bus);
>> + ? ? ? }
>> +
>> + ? ? ? return 0;
>> +}
>
>> +void i2c_init(int speed, int slaveaddr)
>> +{
>> + ? ? ? debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
>> +}
>
> Surely that function needs to actually do something, at least set up the
> clocks so that the (user?) requested rate is honored, or print an error
> message if you're only allowed to use the hard-coded bus rate.

See my other message. I suppose we could reinit, but we really don't
want to honour the speed, since the fdt speed setting is then lost and
irrecoverable. For now it feels like we should ignore it.

If fdt use grows then perhaps U-Boot might accept patches to
understand the concept of initing i2c as a unit, and once at the start
(or first use), rather than the current setup.

>
>> +/* Probe to see if a chip is present. */
>> +int i2c_probe(uchar chip)
>> +{
>> + ? ? ? int rc;
>> + ? ? ? uchar reg;
>> +
>> + ? ? ? debug("i2c_probe: addr=0x%x\n", chip);
>> + ? ? ? reg = 0;
>> + ? ? ? rc = i2c_write_data(chip, &reg, 1);
>> + ? ? ? if (rc) {
>> + ? ? ? ? ? ? ? debug("Error probing 0x%x.\n", chip);
>> + ? ? ? ? ? ? ? return 1;
>> + ? ? ? }
>> + ? ? ? return 0;
>> +}
>> +
>
>> +static int i2c_addr_ok(const uint addr, const int alen)
>> +{
>> + ? ? ? if (alen < 0 || alen > sizeof(addr))
>> + ? ? ? ? ? ? ? return 0;
>> + ? ? ? if (alen != sizeof(addr)) {
>> + ? ? ? ? ? ? ? uint max_addr = (1 << (8 * alen)) - 1;
>> + ? ? ? ? ? ? ? if (addr > max_addr)
>> + ? ? ? ? ? ? ? ? ? ? ? return 0;
>> + ? ? ? }
>> + ? ? ? return 1;
>> +}
>
> That seems rather roundabout. Only two address lengths are valid; 7 and
> 10-bit, so wouldn't it be better to only accept those specific values?

Yes I agree - I will change it to just check that one or two address
bytes are provided.

Regards,
Simon

>
> --
> nvpublic

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Stephen Warren]

Simon Glass wrote ednesday, January 11, 2012 9:18 PM:
> On Mon, Jan 9, 2012 at 2:07 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > On 12/26/2011 11:11 AM, Simon Glass wrote:
> >> From: Yen Lin <yelin@nvidia.com>
> >>
> >> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
> >> The driver supports building both with and without CONFIG_OF_CONTROL.
> >>
> >> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
> >> in the board config header file:
> >>
> >> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
> >> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
> >> ? ? ? ? (typically this will be 0 to bring the port out the common
> >> ? ? ? ? ? ? ? ? pins)
...

> > ...
> >> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
> > ...
> >> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
> >
> > What if there are I2C bus extenders/muxes/... such that there are more
> > I2C buses in the system than Tegra I2C controllers? I'd rather see an
> > explicit TEGRA_I2C_NUM_CONTROLLERS define used throughout this patch.
> 
> We don't actually support CONFIG_I2C_MUX, so I can't see how that
> could happen. Can you please explain a bit more?

We may not support it now, but I see no reason we won't in the future.
If we confuse the two defines now, it'll make it harder to allow muxes
in the future. The fix is simply using the correct define name within
the I2C driver isn't it; pretty simple.

> >> +int i2c_init_board(void)
> >> +{
> >> + ? ? ? struct i2c_bus *i2c_bus;
> >> + ? ? ? int index = 0;
> >> + ? ? ? int i;
> >> +
> >> + ? ? ? /* build the i2c_controllers[] for each controller */
> >> + ? ? ? for (i = 0; i < CONFIG_SYS_MAX_I2C_BUS; ++i) {
> >> + ? ? ? ? ? ? ? i2c_bus = &i2c_controllers[i];
> >> + ? ? ? ? ? ? ? i2c_bus->id = i;
> >> +
> >> + ? ? ? ? ? ? ? if (i2c_get_config(&index, i2c_bus)) {
> >> + ? ? ? ? ? ? ? ? ? ? ? printf("i2c_init_board: failed to find bus %d\n", i);
> >> + ? ? ? ? ? ? ? ? ? ? ? return -1;
> >> + ? ? ? ? ? ? ? }
> >> +
> >> + ? ? ? ? ? ? ? if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C)
> >> + ? ? ? ? ? ? ? ? ? ? ? i2c_bus->control =
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &((struct dvc_ctlr *)i2c_bus->regs)->control;
> >> + ? ? ? ? ? ? ? else
> >> + ? ? ? ? ? ? ? ? ? ? ? i2c_bus->control = &i2c_bus->regs->control;
> >
> > When instantiating controllers from device tree (as opposed to the
> > static !CONFIG_OF_CONTROL case), that conditional should be driven by
> > device tree properties. The kernel already represents this using two
> > separate compatible values for the different HW: nvidia,tegra20-i2c and
> > nvidia,tegra20-i2c-dvc.
> 
> Not in the device tree file I got from the kernel...has it changed?

Yes, the latest is:

        i2c at 7000c000 {
                compatible = "nvidia,tegra20-i2c";
        i2c at 7000c400 {
                compatible = "nvidia,tegra20-i2c";
        i2c at 7000c500 {
                compatible = "nvidia,tegra20-i2c";
        i2c at 7000d000 {
                compatible = "nvidia,tegra20-i2c-dvc";


> >> +
> >> + ? ? ? ? ? ? ? i2c_init_controller(i2c_bus);
> >> + ? ? ? }
> >> +
> >> + ? ? ? return 0;
> >> +}
> >
> >> +void i2c_init(int speed, int slaveaddr)
> >> +{
> >> + ? ? ? debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
> >> +}
> >
> > Surely that function needs to actually do something, at least set up the
> > clocks so that the (user?) requested rate is honored, or print an error
> > message if you're only allowed to use the hard-coded bus rate.
> 
> See my other message. I suppose we could reinit, but we really don't
> want to honour the speed, since the fdt speed setting is then lost and
> irrecoverable. For now it feels like we should ignore it.

Hmm. I suspect the answer here is roughly to override the following in
cmd_i2c.c:

/* TODO: Implement architecture-specific get/set functions */
unsigned int __def_i2c_get_bus_speed(void)
{
        return CONFIG_SYS_I2C_SPEED;
}
unsigned int i2c_get_bus_speed(void)
        __attribute__((weak, alias("__def_i2c_get_bus_speed")));

int __def_i2c_set_bus_speed(unsigned int speed)
{
        if (speed != CONFIG_SYS_I2C_SPEED)
                return -1;

        return 0;
}
int i2c_set_bus_speed(unsigned int)
        __attribute__((weak, alias("__def_i2c_set_bus_speed")));

To actually read/write the rate in use by the driver.

Then, fix do_i2c_reset() to use i2c_get_bus_speed() so it interacts
correctly with those functions.

There may be other places that need to be updates to use those function
instead of hard-coding CONFIG_SYS_I2C_SPEED too. Perhaps we could even
get away without defining CONFIG_SYS_I2C_SPEED for Tegra since it's
meaningless. Instead, ifdef those default function definitions above
based on whether CONFIG_SYS_I2C_SPEED is defined or not.

-- 
nvpublic

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Heiko Schocher]

Hello Stephen,

Stephen Warren wrote:
> Simon Glass wrote ednesday, January 11, 2012 9:18 PM:
>> On Mon, Jan 9, 2012 at 2:07 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>> On 12/26/2011 11:11 AM, Simon Glass wrote:
>>>> From: Yen Lin <yelin@nvidia.com>
>>>>
>>>> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
>>>> The driver supports building both with and without CONFIG_OF_CONTROL.
>>>>
>>>> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
>>>> in the board config header file:
>>>>
>>>> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
>>>> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
>>>>         (typically this will be 0 to bring the port out the common
>>>>                 pins)
> ...
> 
>>> ...
>>>> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
>>> ...
>>>> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
>>> What if there are I2C bus extenders/muxes/... such that there are more
>>> I2C buses in the system than Tegra I2C controllers? I'd rather see an
>>> explicit TEGRA_I2C_NUM_CONTROLLERS define used throughout this patch.
>> We don't actually support CONFIG_I2C_MUX, so I can't see how that
>> could happen. Can you please explain a bit more?
> 
> We may not support it now, but I see no reason we won't in the future.
> If we confuse the two defines now, it'll make it harder to allow muxes
> in the future. The fix is simply using the correct define name within
> the I2C driver isn't it; pretty simple.

If we really have this case, we *must* get the multibus/multiadapter
branch from here:

http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2_20111112

to uboot mainline! With that approach we could handle this case
in a clean way ... This branch is not in sync with current TOT, and
it has to be tested again, as I didn't found time for this in the
last year :-( Also, as its base is >2 years old, there are a lot
of checkpatch errors in this patchserie, which have to be cleaned
up ... hope I get some time for it ... help is welcome.

>>>> +int i2c_init_board(void)
>>>> +{
[...]
>>> Surely that function needs to actually do something, at least set up the
>>> clocks so that the (user?) requested rate is honored, or print an error
>>> message if you're only allowed to use the hard-coded bus rate.
>> See my other message. I suppose we could reinit, but we really don't
>> want to honour the speed, since the fdt speed setting is then lost and
>> irrecoverable. For now it feels like we should ignore it.
> 
> Hmm. I suspect the answer here is roughly to override the following in
> cmd_i2c.c:
> 
> /* TODO: Implement architecture-specific get/set functions */
> unsigned int __def_i2c_get_bus_speed(void)
> {
>         return CONFIG_SYS_I2C_SPEED;
> }
> unsigned int i2c_get_bus_speed(void)
>         __attribute__((weak, alias("__def_i2c_get_bus_speed")));
> 
> int __def_i2c_set_bus_speed(unsigned int speed)
> {
>         if (speed != CONFIG_SYS_I2C_SPEED)
>                 return -1;
> 
>         return 0;
> }
> int i2c_set_bus_speed(unsigned int)
>         __attribute__((weak, alias("__def_i2c_set_bus_speed")));
> 
> To actually read/write the rate in use by the driver.

Yep, for this reason this functions are weak, and you can define
a driver specific function which returns the current used settings
in the driver.

Code should compile, if tegra2 not define CONFIG_SYS_I2C_SPEED,
as a "default" value for CONFIG_SYS_I2C_SPEED is defined in
include/i2c.h ...

> Then, fix do_i2c_reset() to use i2c_get_bus_speed() so it interacts
> correctly with those functions.

Yep, I think, you are right here, that should be fixed. As the
default function for i2c_get_bus_speed returns CONFIG_SYS_I2C_SPEED,
that should be OK.

> There may be other places that need to be updates to use those function
> instead of hard-coding CONFIG_SYS_I2C_SPEED too. Perhaps we could even
> get away without defining CONFIG_SYS_I2C_SPEED for Tegra since it's
> meaningless. Instead, ifdef those default function definitions above
> based on whether CONFIG_SYS_I2C_SPEED is defined or not.
> 

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Simon Glass]

Hi Heiko,

On Thu, Jan 12, 2012 at 11:12 PM, Heiko Schocher <hs@denx.de> wrote:
> Hello Stephen,
>
> Stephen Warren wrote:
>> Simon Glass wrote ednesday, January 11, 2012 9:18 PM:
>>> On Mon, Jan 9, 2012 at 2:07 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>>> On 12/26/2011 11:11 AM, Simon Glass wrote:
>>>>> From: Yen Lin <yelin@nvidia.com>
>>>>>
>>>>> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
>>>>> The driver supports building both with and without CONFIG_OF_CONTROL.
>>>>>
>>>>> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
>>>>> in the board config header file:
>>>>>
>>>>> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
>>>>> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
>>>>> ? ? ? ? (typically this will be 0 to bring the port out the common
>>>>> ? ? ? ? ? ? ? ? pins)
>> ...
>>
>>>> ...
>>>>> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
>>>> ...
>>>>> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
>>>> What if there are I2C bus extenders/muxes/... such that there are more
>>>> I2C buses in the system than Tegra I2C controllers? I'd rather see an
>>>> explicit TEGRA_I2C_NUM_CONTROLLERS define used throughout this patch.
>>> We don't actually support CONFIG_I2C_MUX, so I can't see how that
>>> could happen. Can you please explain a bit more?
>>
>> We may not support it now, but I see no reason we won't in the future.
>> If we confuse the two defines now, it'll make it harder to allow muxes
>> in the future. The fix is simply using the correct define name within
>> the I2C driver isn't it; pretty simple.
>
> If we really have this case, we *must* get the multibus/multiadapter
> branch from here:
>
> http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2_20111112
>
> to uboot mainline! With that approach we could handle this case
> in a clean way ... This branch is not in sync with current TOT, and
> it has to be tested again, as I didn't found time for this in the
> last year :-( Also, as its base is >2 years old, there are a lot
> of checkpatch errors in this patchserie, which have to be cleaned
> up ... hope I get some time for it ... help is welcome.
>

Hmm quite a few patches. I see this email from 2.5 years ago
indicating that it was ready but needed more testing:

http://lists.denx.de/pipermail/u-boot/2009-July/056726.html

2a083b7 i2c: get rid of HARD_i2C
ccb680c i2c: add dynamic bus allocation
b68480a i2c, u8500: added new multibus/multiadapter support
692f037 i2c, marvell: added new multibus/multiadapter support
3b0258c i2c, spear: added new multibus/multiadapter support
a06deff i2c, at91rm9200: added new multibus/multiadapter support
0758f6b i2c, kirkwood: added new multibus/multiadapter support
7919863 i2c, mv: added new multibus/multiadapter support
b164243 i2c, mpc824x: added new multibus/multiadapter support
a5f8bd1 i2c, tsi108: added new multibus/multiadapter support
9db40d8 i2c, s3c44b0: added new multibus/multiadapter support
67b95ef i2c, s3c24x0: added new multibus/multiadapter support
6b2c3f5 i2c, omap24xx: added new multibus/multiadapter support
1f8ffed i2c, omap1510: added new multibus/multiadapter support
c41757c i2c, mxc: added new multibus/multiadapter support
5c22159 i2c, davinci: added new multibus/multiadapter support
3dd28ab i2c, bfin: added new multibus/multiadapter support
3d1c119 i2c, mpc8220: added new multibus/multiadapter support
8f4d062 i2c, mpc512x: added new multibus/multiadapter support
1bd40d4 i2c, mpc5xxx: added new multibus/multiadapter support
6225ccb i2c, 8xx: added new multibus/multiadapter support
d94a632 i2c, ppc4xx_i2c: added new multibus/multiadapter support
c34a705 i2c, mpc8260_i2c: added new multibus/multiadapter support
832b409 i2c, fsl_i2c: switch to new multibus/multiadapter support
06de378 i2c, soft-i2c: switch to new multibus/multiadapter support
e47115f i2c, common: common changes for multibus/multiadapter support
d7a70d7 i2c: add i2c_core and prepare for new multibus support

 489 files changed, 8512 insertions(+), 7063 deletions(-)

checkpatch.pl found 295 error(s), 653 warning(s)

I have had a look through the patches. I agree that i2c needs a clean
up and this seems to fit the bill. How can we get this committed?

The first two commits aren't too hard to bring over but it doesn't
actually build. I could perhaps have a look at getting it to build
correctly with and without CONFIG_SYS_I2C, and see if I can get the
latter working on Tegra.

Then perhaps we could add the new support as just another option so we
have CONFIG_HARD_I2C, CONFIG_SOFT_I2C and CONFIG_SYS_I2C. That might
be a start, and drivers can move over to the new system. What do you
think?

>>>>> +int i2c_init_board(void)
>>>>> +{
> [...]
>>>> Surely that function needs to actually do something, at least set up the
>>>> clocks so that the (user?) requested rate is honored, or print an error
>>>> message if you're only allowed to use the hard-coded bus rate.
>>> See my other message. I suppose we could reinit, but we really don't
>>> want to honour the speed, since the fdt speed setting is then lost and
>>> irrecoverable. For now it feels like we should ignore it.
>>
>> Hmm. I suspect the answer here is roughly to override the following in
>> cmd_i2c.c:
>>
>> /* TODO: Implement architecture-specific get/set functions */
>> unsigned int __def_i2c_get_bus_speed(void)
>> {
>> ? ? ? ? return CONFIG_SYS_I2C_SPEED;
>> }
>> unsigned int i2c_get_bus_speed(void)
>> ? ? ? ? __attribute__((weak, alias("__def_i2c_get_bus_speed")));
>>
>> int __def_i2c_set_bus_speed(unsigned int speed)
>> {
>> ? ? ? ? if (speed != CONFIG_SYS_I2C_SPEED)
>> ? ? ? ? ? ? ? ? return -1;
>>
>> ? ? ? ? return 0;
>> }
>> int i2c_set_bus_speed(unsigned int)
>> ? ? ? ? __attribute__((weak, alias("__def_i2c_set_bus_speed")));
>>
>> To actually read/write the rate in use by the driver.
>
> Yep, for this reason this functions are weak, and you can define
> a driver specific function which returns the current used settings
> in the driver.
>
> Code should compile, if tegra2 not define CONFIG_SYS_I2C_SPEED,
> as a "default" value for CONFIG_SYS_I2C_SPEED is defined in
> include/i2c.h ...
>
>> Then, fix do_i2c_reset() to use i2c_get_bus_speed() so it interacts
>> correctly with those functions.
>
> Yep, I think, you are right here, that should be fixed. As the
> default function for i2c_get_bus_speed returns CONFIG_SYS_I2C_SPEED,
> that should be OK.

Yes ok I will take a look.

Regards,
Simon

>
>> There may be other places that need to be updates to use those function
>> instead of hard-coding CONFIG_SYS_I2C_SPEED too. Perhaps we could even
>> get away without defining CONFIG_SYS_I2C_SPEED for Tegra since it's
>> meaningless. Instead, ifdef those default function definitions above
>> based on whether CONFIG_SYS_I2C_SPEED is defined or not.
>>
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Heiko Schocher]

Hello Simon,

Simon Glass wrote:
> Hi Heiko,
> 
> On Thu, Jan 12, 2012 at 11:12 PM, Heiko Schocher <hs@denx.de> wrote:
>> Hello Stephen,
>>
>> Stephen Warren wrote:
>>> Simon Glass wrote ednesday, January 11, 2012 9:18 PM:
>>>> On Mon, Jan 9, 2012 at 2:07 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>>>> On 12/26/2011 11:11 AM, Simon Glass wrote:
>>>>>> From: Yen Lin <yelin@nvidia.com>
>>>>>>
>>>>>> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
>>>>>> The driver supports building both with and without CONFIG_OF_CONTROL.
>>>>>>
>>>>>> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
>>>>>> in the board config header file:
>>>>>>
>>>>>> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
>>>>>> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
>>>>>>         (typically this will be 0 to bring the port out the common
>>>>>>                 pins)
>>> ...
>>>
>>>>> ...
>>>>>> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
>>>>> ...
>>>>>> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
>>>>> What if there are I2C bus extenders/muxes/... such that there are more
>>>>> I2C buses in the system than Tegra I2C controllers? I'd rather see an
>>>>> explicit TEGRA_I2C_NUM_CONTROLLERS define used throughout this patch.
>>>> We don't actually support CONFIG_I2C_MUX, so I can't see how that
>>>> could happen. Can you please explain a bit more?
>>> We may not support it now, but I see no reason we won't in the future.
>>> If we confuse the two defines now, it'll make it harder to allow muxes
>>> in the future. The fix is simply using the correct define name within
>>> the I2C driver isn't it; pretty simple.
>> If we really have this case, we *must* get the multibus/multiadapter
>> branch from here:
>>
>> http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2_20111112
>>
>> to uboot mainline! With that approach we could handle this case
>> in a clean way ... This branch is not in sync with current TOT, and
>> it has to be tested again, as I didn't found time for this in the
>> last year :-( Also, as its base is >2 years old, there are a lot
>> of checkpatch errors in this patchserie, which have to be cleaned
>> up ... hope I get some time for it ... help is welcome.
>>
> 
> Hmm quite a few patches. I see this email from 2.5 years ago

Yep, there was some work on this ... but the last year, I just found
time for rebasing this branch, add new boards/i2c drivers and just
try compile clean ... so there are maybe some errors, which you can
solve only with a debugger ... Sorry for that ...

> indicating that it was ready but needed more testing:
> 
> http://lists.denx.de/pipermail/u-boot/2009-July/056726.html
> 
> 2a083b7 i2c: get rid of HARD_i2C
> ccb680c i2c: add dynamic bus allocation
> b68480a i2c, u8500: added new multibus/multiadapter support
> 692f037 i2c, marvell: added new multibus/multiadapter support
> 3b0258c i2c, spear: added new multibus/multiadapter support
> a06deff i2c, at91rm9200: added new multibus/multiadapter support
> 0758f6b i2c, kirkwood: added new multibus/multiadapter support
> 7919863 i2c, mv: added new multibus/multiadapter support
> b164243 i2c, mpc824x: added new multibus/multiadapter support
> a5f8bd1 i2c, tsi108: added new multibus/multiadapter support
> 9db40d8 i2c, s3c44b0: added new multibus/multiadapter support
> 67b95ef i2c, s3c24x0: added new multibus/multiadapter support
> 6b2c3f5 i2c, omap24xx: added new multibus/multiadapter support
> 1f8ffed i2c, omap1510: added new multibus/multiadapter support
> c41757c i2c, mxc: added new multibus/multiadapter support
> 5c22159 i2c, davinci: added new multibus/multiadapter support
> 3dd28ab i2c, bfin: added new multibus/multiadapter support
> 3d1c119 i2c, mpc8220: added new multibus/multiadapter support
> 8f4d062 i2c, mpc512x: added new multibus/multiadapter support
> 1bd40d4 i2c, mpc5xxx: added new multibus/multiadapter support
> 6225ccb i2c, 8xx: added new multibus/multiadapter support
> d94a632 i2c, ppc4xx_i2c: added new multibus/multiadapter support
> c34a705 i2c, mpc8260_i2c: added new multibus/multiadapter support
> 832b409 i2c, fsl_i2c: switch to new multibus/multiadapter support
> 06de378 i2c, soft-i2c: switch to new multibus/multiadapter support
> e47115f i2c, common: common changes for multibus/multiadapter support
> d7a70d7 i2c: add i2c_core and prepare for new multibus support
> 
>  489 files changed, 8512 insertions(+), 7063 deletions(-)
> 
> checkpatch.pl found 295 error(s), 653 warning(s)

Yep, I know :-(

> I have had a look through the patches. I agree that i2c needs a clean
> up and this seems to fit the bill. How can we get this committed?

We should try to get rid of the checkpatch errors. After that we
should test it on some platforms (arm, powerpc) and if this step
is done, we can bring it to mainline in the next upcoming merge
window.

If we break i2c support on some boards, I hope board maintainers
will speak up then, and send bugfix patches ;-)

> The first two commits aren't too hard to bring over but it doesn't
> actually build. I could perhaps have a look at getting it to build

Hups, I run a "MAKEALL -a arm" without errors ... will look
into this next week.

> correctly with and without CONFIG_SYS_I2C, and see if I can get the
> latter working on Tegra.
> 
> Then perhaps we could add the new support as just another option so we
> have CONFIG_HARD_I2C, CONFIG_SOFT_I2C and CONFIG_SYS_I2C. That might
> be a start, and drivers can move over to the new system. What do you
> think?

I would prefer to get the hole branch to mainline, so we can get
rid off the CONFIG_HARD_I2C define ... but maybe it is worth to
try this in more smaller steps ...

>>>>>> +int i2c_init_board(void)
>>>>>> +{
>> [...]
>>>>> Surely that function needs to actually do something, at least set up the
>>>>> clocks so that the (user?) requested rate is honored, or print an error
>>>>> message if you're only allowed to use the hard-coded bus rate.
>>>> See my other message. I suppose we could reinit, but we really don't
>>>> want to honour the speed, since the fdt speed setting is then lost and
>>>> irrecoverable. For now it feels like we should ignore it.
>>> Hmm. I suspect the answer here is roughly to override the following in
>>> cmd_i2c.c:
>>>
>>> /* TODO: Implement architecture-specific get/set functions */
>>> unsigned int __def_i2c_get_bus_speed(void)
>>> {
>>>         return CONFIG_SYS_I2C_SPEED;
>>> }
>>> unsigned int i2c_get_bus_speed(void)
>>>         __attribute__((weak, alias("__def_i2c_get_bus_speed")));
>>>
>>> int __def_i2c_set_bus_speed(unsigned int speed)
>>> {
>>>         if (speed != CONFIG_SYS_I2C_SPEED)
>>>                 return -1;
>>>
>>>         return 0;
>>> }
>>> int i2c_set_bus_speed(unsigned int)
>>>         __attribute__((weak, alias("__def_i2c_set_bus_speed")));
>>>
>>> To actually read/write the rate in use by the driver.
>> Yep, for this reason this functions are weak, and you can define
>> a driver specific function which returns the current used settings
>> in the driver.
>>
>> Code should compile, if tegra2 not define CONFIG_SYS_I2C_SPEED,
>> as a "default" value for CONFIG_SYS_I2C_SPEED is defined in
>> include/i2c.h ...
>>
>>> Then, fix do_i2c_reset() to use i2c_get_bus_speed() so it interacts
>>> correctly with those functions.
>> Yep, I think, you are right here, that should be fixed. As the
>> default function for i2c_get_bus_speed returns CONFIG_SYS_I2C_SPEED,
>> that should be OK.
> 
> Yes ok I will take a look.

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 4/7] tegra: Add I2C driver

[Simon Glass]

Hi Stephen,

On Thu, Jan 12, 2012 at 11:14 AM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote ednesday, January 11, 2012 9:18 PM:
>> On Mon, Jan 9, 2012 at 2:07 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> > On 12/26/2011 11:11 AM, Simon Glass wrote:
>> >> From: Yen Lin <yelin@nvidia.com>
>> >>
>> >> Add basic i2c driver for Tegra2 with 8- and 16-bit address support.
>> >> The driver supports building both with and without CONFIG_OF_CONTROL.
>> >>
>> >> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied
>> >> in the board config header file:
>> >>
>> >> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000)
>> >> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3)
>> >> ? ? ? ? (typically this will be 0 to bring the port out the common
>> >> ? ? ? ? ? ? ? ? pins)
> ...
>
>> > ...
>> >> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
>> > ...
>> >> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
>> >
>> > What if there are I2C bus extenders/muxes/... such that there are more
>> > I2C buses in the system than Tegra I2C controllers? I'd rather see an
>> > explicit TEGRA_I2C_NUM_CONTROLLERS define used throughout this patch.
>>
>> We don't actually support CONFIG_I2C_MUX, so I can't see how that
>> could happen. Can you please explain a bit more?
>
> We may not support it now, but I see no reason we won't in the future.
> If we confuse the two defines now, it'll make it harder to allow muxes
> in the future. The fix is simply using the correct define name within
> the I2C driver isn't it; pretty simple.

OK I have added the new define.

>
>> >> +int i2c_init_board(void)
>> >> +{
>> >> + ? ? ? struct i2c_bus *i2c_bus;
>> >> + ? ? ? int index = 0;
>> >> + ? ? ? int i;
>> >> +
>> >> + ? ? ? /* build the i2c_controllers[] for each controller */
>> >> + ? ? ? for (i = 0; i < CONFIG_SYS_MAX_I2C_BUS; ++i) {
>> >> + ? ? ? ? ? ? ? i2c_bus = &i2c_controllers[i];
>> >> + ? ? ? ? ? ? ? i2c_bus->id = i;
>> >> +
>> >> + ? ? ? ? ? ? ? if (i2c_get_config(&index, i2c_bus)) {
>> >> + ? ? ? ? ? ? ? ? ? ? ? printf("i2c_init_board: failed to find bus %d\n", i);
>> >> + ? ? ? ? ? ? ? ? ? ? ? return -1;
>> >> + ? ? ? ? ? ? ? }
>> >> +
>> >> + ? ? ? ? ? ? ? if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C)
>> >> + ? ? ? ? ? ? ? ? ? ? ? i2c_bus->control =
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &((struct dvc_ctlr *)i2c_bus->regs)->control;
>> >> + ? ? ? ? ? ? ? else
>> >> + ? ? ? ? ? ? ? ? ? ? ? i2c_bus->control = &i2c_bus->regs->control;
>> >
>> > When instantiating controllers from device tree (as opposed to the
>> > static !CONFIG_OF_CONTROL case), that conditional should be driven by
>> > device tree properties. The kernel already represents this using two
>> > separate compatible values for the different HW: nvidia,tegra20-i2c and
>> > nvidia,tegra20-i2c-dvc.
>>
>> Not in the device tree file I got from the kernel...has it changed?
>
> Yes, the latest is:
>
> ? ? ? ?i2c at 7000c000 {
> ? ? ? ? ? ? ? ?compatible = "nvidia,tegra20-i2c";
> ? ? ? ?i2c at 7000c400 {
> ? ? ? ? ? ? ? ?compatible = "nvidia,tegra20-i2c";
> ? ? ? ?i2c at 7000c500 {
> ? ? ? ? ? ? ? ?compatible = "nvidia,tegra20-i2c";
> ? ? ? ?i2c at 7000d000 {
> ? ? ? ? ? ? ? ?compatible = "nvidia,tegra20-i2c-dvc";

OK I think I have this now.

>
>
>> >> +
>> >> + ? ? ? ? ? ? ? i2c_init_controller(i2c_bus);
>> >> + ? ? ? }
>> >> +
>> >> + ? ? ? return 0;
>> >> +}
>> >
>> >> +void i2c_init(int speed, int slaveaddr)
>> >> +{
>> >> + ? ? ? debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
>> >> +}
>> >
>> > Surely that function needs to actually do something, at least set up the
>> > clocks so that the (user?) requested rate is honored, or print an error
>> > message if you're only allowed to use the hard-coded bus rate.
>>
>> See my other message. I suppose we could reinit, but we really don't
>> want to honour the speed, since the fdt speed setting is then lost and
>> irrecoverable. For now it feels like we should ignore it.
>
> Hmm. I suspect the answer here is roughly to override the following in
> cmd_i2c.c:
>
> /* TODO: Implement architecture-specific get/set functions */
> unsigned int __def_i2c_get_bus_speed(void)
> {
> ? ? ? ?return CONFIG_SYS_I2C_SPEED;
> }
> unsigned int i2c_get_bus_speed(void)
> ? ? ? ?__attribute__((weak, alias("__def_i2c_get_bus_speed")));
>
> int __def_i2c_set_bus_speed(unsigned int speed)
> {
> ? ? ? ?if (speed != CONFIG_SYS_I2C_SPEED)
> ? ? ? ? ? ? ? ?return -1;
>
> ? ? ? ?return 0;
> }
> int i2c_set_bus_speed(unsigned int)
> ? ? ? ?__attribute__((weak, alias("__def_i2c_set_bus_speed")));
>
> To actually read/write the rate in use by the driver.
>
> Then, fix do_i2c_reset() to use i2c_get_bus_speed() so it interacts
> correctly with those functions.

The reset will override the fdt speed, but there's no easy way around
that. U-Boot expects a single speed for all I2C at present.

>
> There may be other places that need to be updates to use those function
> instead of hard-coding CONFIG_SYS_I2C_SPEED too. Perhaps we could even
> get away without defining CONFIG_SYS_I2C_SPEED for Tegra since it's
> meaningless. Instead, ifdef those default function definitions above
> based on whether CONFIG_SYS_I2C_SPEED is defined or not.

I think we should leave this until the I2C refactor is in. In fact I
would go further and say that U-Boot should probably support more
flexibility here - but the API is what it is at the moment and I feel
uncomfortable changing it just for Tegra. As you probably saw I sent
up Heiko's patches and one or other of us may get to this.

Regards,
Simon

>
> --
> nvpublic
>

[U-Boot] [PATCH 5/7] tegra: Initialise I2C on Nvidia boards

[Simon Glass]

This enables I2C on all Nvidia boards including Seaboard and
Harmony.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 board/nvidia/common/board.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
index a7c566d..63edfdb 100644
--- a/board/nvidia/common/board.c
+++ b/board/nvidia/common/board.c
@@ -34,6 +34,7 @@
 #include <asm/arch/uart.h>
 #include <spi.h>
 #include <asm/arch/usb.h>
+#include <i2c.h>
 #include "board.h"
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -69,6 +70,9 @@ int board_init(void)
 #endif
 	/* boot param addr */
 	gd->bd->bi_boot_params = (NV_PA_SDRAM_BASE + 0x100);
+#ifdef CONFIG_TEGRA2_I2C
+	i2c_init_board();
+#endif
 
 #ifdef CONFIG_USB_EHCI_TEGRA
 	/* For USB GPIO PD0. for now, since we have no pinmux in fdt */
-- 
1.7.3.1

[U-Boot] [PATCH 6/7] tegra: Select I2C ordering for Seaboard

[Simon Glass]

Select the port ordering for I2C on Seaboard.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 board/nvidia/dts/tegra2-seaboard.dts |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
index 839e761..187dec0 100644
--- a/board/nvidia/dts/tegra2-seaboard.dts
+++ b/board/nvidia/dts/tegra2-seaboard.dts
@@ -15,6 +15,11 @@
 		/* This defines the order of our USB ports */
 		usb0 = "/usb at c5008000";
 		usb1 = "/usb at c5000000";
+
+		i2c0 = "/i2c at 7000d000";
+		i2c1 = "/i2c at 7000c000";
+		i2c2 = "/i2c at 7000c400";
+		i2c3 = "/i2c at 7000c500";
 	};
 
 	memory {
-- 
1.7.3.1

[U-Boot] [PATCH 6/7] tegra: Select I2C ordering for Seaboard

[Stephen Warren]

On 12/26/2011 11:11 AM, Simon Glass wrote:
> Select the port ordering for I2C on Seaboard.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Acked-by: Stephen Warren <swarren@nvidia.com>

-- 
nvpublic

[U-Boot] [PATCH 7/7] tegra: Enable I2C on Seaboard

[Simon Glass]

This enables I2C on Seaboard.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 include/configs/seaboard.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
index 1dc775a..6e04278 100644
--- a/include/configs/seaboard.h
+++ b/include/configs/seaboard.h
@@ -59,6 +59,14 @@
 #define CONFIG_CMD_SF
 #define CONFIG_SPI_FLASH_SIZE		(4 << 20)
 
+/* I2C */
+#define CONFIG_TEGRA2_I2C
+#define CONFIG_SYS_I2C_INIT_BOARD
+#define CONFIG_I2C_MULTI_BUS
+#define CONFIG_SYS_MAX_I2C_BUS		4
+#define CONFIG_SYS_I2C_SPEED		100000
+#define CONFIG_CMD_I2C
+
 /* SD/MMC */
 #define CONFIG_MMC
 #define CONFIG_GENERIC_MMC
-- 
1.7.3.1

[U-Boot] [PATCH 7/7] tegra: Enable I2C on Seaboard

[Stephen Warren]

On 12/26/2011 11:11 AM, Simon Glass wrote:
> This enables I2C on Seaboard.
...
> diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
...
> +#define CONFIG_SYS_I2C_INIT_BOARD

I don't think that option is correct for Seaboard; the description in
the README indicates this causes a function named i2c_init_board() to be
called from boards/xxx/board.c, which is supposed to use GPIOs to unhang
the I2C bus. However, this raises a couple of issues:

1) Patch 5 in this series calls i2c_init_board() ifdef CONFIG_TEGRA2_I2C
rather than depending on this CONFIG option.

2) Tegra's i2c_init_board() doesn't appear to be anything to do with bus
unhanging, but is instead regular I2C initialization. Perhaps the
function should be renamed?

-- 
nvpublic

[U-Boot] [PATCH 7/7] tegra: Enable I2C on Seaboard

[Simon Glass]

+U-Boot, which I seemed to have dropped by mistake

Hi Stephen,

On Thu, Jan 12, 2012 at 10:59 AM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Wednesday, January 11, 2012 8:00 PM:
>> On Mon, Jan 9, 2012 at 1:45 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> > On 12/26/2011 11:11 AM, Simon Glass wrote:
>> >> This enables I2C on Seaboard.
>> > ...
>> >> diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
>> > ...
>> >> +#define CONFIG_SYS_I2C_INIT_BOARD
>> >
>> > I don't think that option is correct for Seaboard; the description in
>> > the README indicates this causes a function named i2c_init_board() to be
>> > called from boards/xxx/board.c, which is supposed to use GPIOs to unhang
>> > the I2C bus. However, this raises a couple of issues:
>> >
>> > 1) Patch 5 in this series calls i2c_init_board() ifdef CONFIG_TEGRA2_I2C
>> > rather than depending on this CONFIG option.
>> >
>> > 2) Tegra's i2c_init_board() doesn't appear to be anything to do with bus
>> > unhanging, but is instead regular I2C initialization. Perhaps the
>> > function should be renamed?
>>
>> I have had a bit of a look at this. From what I can see the problem is
>> that i2c_init() is passed a bus speed, but this is just
>> CONFIG_SYS_I2C_SPEED. We want to control the speed individually for
>> each port. Yes would could use i2c_init() and ignore the speed, but
>> that seems odd also. At least with the way it is we don't ignore the
>> setting.
>>
>> We also don't define CONFIG_HARD_I2C which again seems odd, but I
>> suppose is for the same reason (we don't want to call i2c_init()).
>
> Hmm. It sounds like we should replace CONFIG_TEGRA2_I2C with
> CONFIG_HARD_I2C given the README description of the latter?

Well we could, but we would need to ignore the speed argument. Do you
think that is better?

>
>> Also U-Boot tends to call i2c_init() before every use of i2c, whereas
>> we just want to init once and be done with it.
>>
>> I think the function name is correct, but perhaps I should change the
>> #ifdef you mention in 1 above to CONFIG_SYS_I2C_INIT_BOARD. But for
>> i2c to function on Tegra boards, this must be defined, so in fact this
>> might be counterproductive. So perhaps a check that it is defined is
>> best?
>
> But README explicitly says that i2c_init_board() is for bus unhanging
> which isn't what the Tegra implementation does. How can the function
> name be correct given that?
>
Well we should rename the function IMO. It seems to me that with a a
name like that it is for initing i2c on the board.

> Don't we just want to make i2c_init() use a global/static variable to
> determine whether the device has already been initialized, and defer all
> the init until first usage or something like that? That seems in line
> with U-Boot's desire not to initialize drivers until they're actually
> used.

Actually that might work - if we keep i2c_init() a no-op, and wait
until we get a request for a bus before we look up the fdt and init
that port. But I suspect we might need to init port 0 immediately
since there is no explicit call to i2c_set_bus_num() for that. It's a
little wonky whatever we do. What do you think?

(have just sent the series again with fdt changes, but can update once
we sort this out).

Regards,
Simon

[U-Boot] [PATCH 7/7] tegra: Enable I2C on Seaboard

[Stephen Warren]

Simon Glass wrote at Thursday, January 12, 2012 12:10 PM:
> On Thu, Jan 12, 2012 at 10:59 AM, Stephen Warren <swarren@nvidia.com> wrote:
> > Simon Glass wrote at Wednesday, January 11, 2012 8:00 PM:
...
> >> Also U-Boot tends to call i2c_init() before every use of i2c, whereas
> >> we just want to init once and be done with it.
> >>
> >> I think the function name is correct, but perhaps I should change the
> >> #ifdef you mention in 1 above to CONFIG_SYS_I2C_INIT_BOARD. But for
> >> i2c to function on Tegra boards, this must be defined, so in fact this
> >> might be counterproductive. So perhaps a check that it is defined is
> >> best?
> >
> > But README explicitly says that i2c_init_board() is for bus unhanging
> > which isn't what the Tegra implementation does. How can the function
> > name be correct given that?
>
> Well we should rename the function IMO. It seems to me that with a a
> name like that it is for initing i2c on the board.

You mean change the function that's activated by CONFIG_SYS_I2C_INIT_BOARD
to be something more like i2c_unhang_board()? That makes sense to me. I
have no idea how much of a hassle it'd be to update any existing boards.

> > Don't we just want to make i2c_init() use a global/static variable to
> > determine whether the device has already been initialized, and defer all
> > the init until first usage or something like that? That seems in line
> > with U-Boot's desire not to initialize drivers until they're actually
> > used.
> 
> Actually that might work - if we keep i2c_init() a no-op, and wait
> until we get a request for a bus before we look up the fdt and init
> that port. But I suspect we might need to init port 0 immediately
> since there is no explicit call to i2c_set_bus_num() for that. It's a
> little wonky whatever we do. What do you think?

It does make sense to me to at least parse out all the DT stuff early
on, so there's a single place to do all the alias processing etc. Given
what I just wrote in the other email about fixing the I2C bus speed
stuff, would making those changes here help this at all?

-- 
nvpublic

[U-Boot] [PATCH 7/7] tegra: Enable I2C on Seaboard

[Simon Glass]

Hi Stephen,

On Thu, Jan 12, 2012 at 11:21 AM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Thursday, January 12, 2012 12:10 PM:
>> On Thu, Jan 12, 2012 at 10:59 AM, Stephen Warren <swarren@nvidia.com> wrote:
>> > Simon Glass wrote at Wednesday, January 11, 2012 8:00 PM:
> ...
>> >> Also U-Boot tends to call i2c_init() before every use of i2c, whereas
>> >> we just want to init once and be done with it.
>> >>
>> >> I think the function name is correct, but perhaps I should change the
>> >> #ifdef you mention in 1 above to CONFIG_SYS_I2C_INIT_BOARD. But for
>> >> i2c to function on Tegra boards, this must be defined, so in fact this
>> >> might be counterproductive. So perhaps a check that it is defined is
>> >> best?
>> >
>> > But README explicitly says that i2c_init_board() is for bus unhanging
>> > which isn't what the Tegra implementation does. How can the function
>> > name be correct given that?
>>
>> Well we should rename the function IMO. It seems to me that with a a
>> name like that it is for initing i2c on the board.
>
> You mean change the function that's activated by CONFIG_SYS_I2C_INIT_BOARD
> to be something more like i2c_unhang_board()? That makes sense to me. I
> have no idea how much of a hassle it'd be to update any existing boards.

Well we should change the README or change the function name. I'm not
sure which is correct.

>
>> > Don't we just want to make i2c_init() use a global/static variable to
>> > determine whether the device has already been initialized, and defer all
>> > the init until first usage or something like that? That seems in line
>> > with U-Boot's desire not to initialize drivers until they're actually
>> > used.
>>
>> Actually that might work - if we keep i2c_init() a no-op, and wait
>> until we get a request for a bus before we look up the fdt and init
>> that port. But I suspect we might need to init port 0 immediately
>> since there is no explicit call to i2c_set_bus_num() for that. It's a
>> little wonky whatever we do. What do you think?
>
> It does make sense to me to at least parse out all the DT stuff early
> on, so there's a single place to do all the alias processing etc. Given
> what I just wrote in the other email about fixing the I2C bus speed
> stuff, would making those changes here help this at all?

I think the device tree processing should be done later because
i2c_init() is called very early, before the dcache is enabled. It is
very  s l o w  to touch device tree before then. To be honest, that's
another reason for leaving it as it is, with i2c_init() being a no-op.
But we might be able to init the bus on first use with enough if()
statements :-)

I will take a look. I am not going to refactor U-Boot I2C here,
although I think it could use a few tweaked...

Regards,
Simon
>
> --
> nvpublic
>

[U-Boot] [PATCH 7/7] tegra: Enable I2C on Seaboard

[Heiko Schocher]

Hello Simon,

Simon Glass wrote:
> +U-Boot, which I seemed to have dropped by mistake
> 
> Hi Stephen,
> 
> On Thu, Jan 12, 2012 at 10:59 AM, Stephen Warren <swarren@nvidia.com> wrote:
>> Simon Glass wrote at Wednesday, January 11, 2012 8:00 PM:
>>> On Mon, Jan 9, 2012 at 1:45 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>>> On 12/26/2011 11:11 AM, Simon Glass wrote:
>>>>> This enables I2C on Seaboard.
>>>> ...
>>>>> diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
>>>> ...
>>>>> +#define CONFIG_SYS_I2C_INIT_BOARD
>>>> I don't think that option is correct for Seaboard; the description in
>>>> the README indicates this causes a function named i2c_init_board() to be
>>>> called from boards/xxx/board.c, which is supposed to use GPIOs to unhang
>>>> the I2C bus. However, this raises a couple of issues:
>>>>
>>>> 1) Patch 5 in this series calls i2c_init_board() ifdef CONFIG_TEGRA2_I2C
>>>> rather than depending on this CONFIG option.
>>>>
>>>> 2) Tegra's i2c_init_board() doesn't appear to be anything to do with bus
>>>> unhanging, but is instead regular I2C initialization. Perhaps the
>>>> function should be renamed?
>>> I have had a bit of a look at this. From what I can see the problem is
>>> that i2c_init() is passed a bus speed, but this is just
>>> CONFIG_SYS_I2C_SPEED. We want to control the speed individually for
>>> each port. Yes would could use i2c_init() and ignore the speed, but
>>> that seems odd also. At least with the way it is we don't ignore the
>>> setting.
>>>
>>> We also don't define CONFIG_HARD_I2C which again seems odd, but I
>>> suppose is for the same reason (we don't want to call i2c_init()).
>> Hmm. It sounds like we should replace CONFIG_TEGRA2_I2C with
>> CONFIG_HARD_I2C given the README description of the latter?
> 
> Well we could, but we would need to ignore the speed argument. Do you
> think that is better?

Without defining CONFIG_HARD_i2C, i2c_init() never gets called ...
naming CONFIG_TEGRA2_I2C is absolutely OK for enabling the tegra i2c
driver.

>>> Also U-Boot tends to call i2c_init() before every use of i2c, whereas
>>> we just want to init once and be done with it.
>>>
>>> I think the function name is correct, but perhaps I should change the
>>> #ifdef you mention in 1 above to CONFIG_SYS_I2C_INIT_BOARD. But for
>>> i2c to function on Tegra boards, this must be defined, so in fact this
>>> might be counterproductive. So perhaps a check that it is defined is
>>> best?
>> But README explicitly says that i2c_init_board() is for bus unhanging
>> which isn't what the Tegra implementation does. How can the function
>> name be correct given that?
>>
> Well we should rename the function IMO. It seems to me that with a a
> name like that it is for initing i2c on the board.

Yes, the name is missleading. On the other hand is a deblocking also
some sort of "init" ... but I see no problem in using the i2c_init_board()
for doing board specific i2c inits too ... maybe the README should be
adapted ... but this problem results, that you don't want to use
i2c_init() ...

>> Don't we just want to make i2c_init() use a global/static variable to
>> determine whether the device has already been initialized, and defer all
>> the init until first usage or something like that? That seems in line
>> with U-Boot's desire not to initialize drivers until they're actually
>> used.

Yep, that would be inline with that ... but how would you use "i2c reset"
command? That only calls i2c_init() ... so, you never can reset/reinit
your i2c bus controller.

> Actually that might work - if we keep i2c_init() a no-op, and wait
> until we get a request for a bus before we look up the fdt and init
> that port. But I suspect we might need to init port 0 immediately
> since there is no explicit call to i2c_set_bus_num() for that. It's a
> little wonky whatever we do. What do you think?

Yep, the clean way would be to use the multibus/multiadapter branch:

http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2_20111112

but as I said in a previous EMail ... there is some work to do,
before it is mainline acceptable ...

[...]

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany