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

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)).
Recent Linux bindings are used, based on example .dts files found in branch for-3.3/dt at:
git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git
(I could not find the actual binding documentation to include here)
Changes in v2: - Adjust definitions to fit new peripheral clock bindings - Change 'speed' to 'clock-frequency' - Remove u-boot,pinmux binding (sadly) - Use DIV_ROUND_UP() instead of a home-grown macro - Tidy comment style - Change i2c array to static - Remove i2c configuring using CONFIG (use fdt instead) - Make i2c/dvc decision come from fdt - Use new fdtdec alias decode function - Simplify code in i2c_addr_ok() - Return an error if an unavailable i2c bus is selected - Add build warning if CONFIG_SYS_I2C_INIT_BOARD is not defined - Disable port 2 as it is not used
Simon Glass (6): tegra: Rename NV_PA_PMC_BASE to TEGRA2_PMC_BASE fdt: Add function to allow aliases to refer to multiple nodes tegra: fdt: Add extra I2C bindings for U-Boot 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/dts/tegra20.dtsi | 10 +- arch/arm/include/asm/arch-tegra2/tegra2.h | 4 +- arch/arm/include/asm/arch-tegra2/tegra2_i2c.h | 160 +++++++ board/nvidia/common/board.c | 7 + board/nvidia/dts/tegra2-seaboard.dts | 10 + drivers/i2c/Makefile | 1 + drivers/i2c/tegra2_i2c.c | 551 +++++++++++++++++++++++++ include/configs/seaboard.h | 8 + include/fdtdec.h | 17 + lib/fdtdec.c | 18 +- 12 files changed, 785 insertions(+), 13 deletions(-) create mode 100644 arch/arm/include/asm/arch-tegra2/tegra2_i2c.h create mode 100644 drivers/i2c/tegra2_i2c.c

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 ---
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 410d0bd..77a627d 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 */

Some devices can deal with multiple compatible properties. The devices need to know which nodes to bind to which features. For example an I2C driver which supports two different controller types will want to know which type it is dealing with in each case.
The new fdtdec_add_aliases_for_id() function deals with this by allowing the driver to search for additional compatible nodes for a different ID. It can then detect the new ones and perform appropriate processing.
Another option considered was to return a tuple (node offset, compat id) and have the function be passed a list of compatible IDs. This is more overhead for the common case though. We may add such a function later if more drivers in U-Boot require it.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/fdtdec.h | 15 +++++++++++++++ lib/fdtdec.c | 16 ++++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h index bd2222c..e6d63f9 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -200,6 +200,21 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name, enum fdt_compat_id id, int *node_list, int maxcount);
/* + * This function is similar to fdtdec_find_aliases_for_id() except that it + * adds to the node_list that is passed in. Any 0 elements are considered + * available for allocation - others are considered already used and are + * skipped. + * + * You can use this by calling fdtdec_find_aliases_for_id() with an + * uninitialised array, then setting the elements that are returned to -1, + * say, then calling this function, perhaps with a different compat id. + * Any elements you get back that are >0 are new nodes added by the call + * to this function. + */ +int fdtdec_add_aliases_for_id(const void *blob, const char *name, + enum fdt_compat_id id, int *node_list, int maxcount); + +/* * Get the name for a compatible ID * * @param id Compatible ID to look for diff --git a/lib/fdtdec.c b/lib/fdtdec.c index daf2c7e..ddd56b9 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -151,10 +151,18 @@ int fdtdec_next_alias(const void *blob, const char *name, return node; }
-/* TODO: Can we tighten this code up a little? */ int fdtdec_find_aliases_for_id(const void *blob, const char *name, enum fdt_compat_id id, int *node_list, int maxcount) { + memset(node_list, '\0', sizeof(*node_list) * maxcount); + + return fdtdec_add_aliases_for_id(blob, name, id, node_list, maxcount); +} + +/* TODO: Can we tighten this code up a little? */ +int fdtdec_add_aliases_for_id(const void *blob, const char *name, + enum fdt_compat_id id, int *node_list, int maxcount) +{ int name_len = strlen(name); int nodes[maxcount]; int num_found = 0; @@ -184,8 +192,6 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name, __func__, name);
/* Now find all the aliases */ - memset(node_list, '\0', sizeof(*node_list) * maxcount); - for (offset = fdt_first_property_offset(blob, alias_node); offset > 0; offset = fdt_next_property_offset(blob, offset)) { @@ -232,11 +238,13 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name, * it as done. */ if (fdtdec_get_is_enabled(blob, node)) { + if (node_list[number]) + continue; node_list[number] = node; if (number >= num_found) num_found = number + 1; } - nodes[j] = 0; + nodes[found] = 0; }
/* Add any nodes not mentioned by an alias */

On 01/12/2012 12:00 PM, Simon Glass wrote:
Some devices can deal with multiple compatible properties. The devices need to know which nodes to bind to which features. For example an I2C driver which supports two different controller types will want to know which type it is dealing with in each case.
The new fdtdec_add_aliases_for_id() function deals with this by allowing the driver to search for additional compatible nodes for a different ID. It can then detect the new ones and perform appropriate processing.
Another option considered was to return a tuple (node offset, compat id) and have the function be passed a list of compatible IDs. This is more overhead for the common case though. We may add such a function later if more drivers in U-Boot require it.
+int fdtdec_add_aliases_for_id(const void *blob, const char *name,
enum fdt_compat_id id, int *node_list, int maxcount)
...
@@ -232,11 +238,13 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name, * it as done. */ if (fdtdec_get_is_enabled(blob, node)) {
if (node_list[number])
}continue; node_list[number] = node; if (number >= num_found) num_found = number + 1;
nodes[j] = 0;
nodes[found] = 0;
}
/* Add any nodes not mentioned by an alias */
Aren't those two changes really bug-fixes to the underlying patch rather than part of this enhancement?
I'm not convinced this patch is correct in all cases. It'll probably work fine for simply device-trees though. Consider the following case:
4 device nodes A: compat=i2c alias for usb0 B: compat=i2c no alias C: compat=i2c alias for usb2 D: compat=i2cx alias for usb1
First, we search for all compat=i2c, the list comes back:
0=A 1=B 2=C
Then we search for all compat=i2cx, the list comes back:
-1 -1 -1 D
So D's alias of 1 isn't honored even though if both IDs were searched for together, it would have been.
Also, what about the scenario where a driver searches for both nvidia,tegra20-i2c then later nvidia,tegra30-i2c, given that all the DT nodes are probably compatible with both?

Hi Stephen,
On Thu, Jan 19, 2012 at 12:49 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, Simon Glass wrote:
Some devices can deal with multiple compatible properties. The devices need to know which nodes to bind to which features. For example an I2C driver which supports two different controller types will want to know which type it is dealing with in each case.
The new fdtdec_add_aliases_for_id() function deals with this by allowing the driver to search for additional compatible nodes for a different ID. It can then detect the new ones and perform appropriate processing.
Another option considered was to return a tuple (node offset, compat id) and have the function be passed a list of compatible IDs. This is more overhead for the common case though. We may add such a function later if more drivers in U-Boot require it.
+int fdtdec_add_aliases_for_id(const void *blob, const char *name,
- enum fdt_compat_id id, int *node_list, int maxcount)
...
@@ -232,11 +238,13 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name, * it as done. */ if (fdtdec_get_is_enabled(blob, node)) {
- if (node_list[number])
- continue;
node_list[number] = node; if (number >= num_found) num_found = number + 1; }
- nodes[j] = 0;
- nodes[found] = 0;
}
/* Add any nodes not mentioned by an alias */
Aren't those two changes really bug-fixes to the underlying patch rather than part of this enhancement?
Clean-ups maybe. The first change can change behaviour when someone has two aliases the same. The second is equivalent (j == found) but nicer I think.
I'm not convinced this patch is correct in all cases. It'll probably work fine for simply device-trees though. Consider the following case:
4 device nodes A: compat=i2c alias for usb0 B: compat=i2c no alias C: compat=i2c alias for usb2 D: compat=i2cx alias for usb1
First, we search for all compat=i2c, the list comes back:
0=A 1=B 2=C
Then we search for all compat=i2cx, the list comes back:
-1 -1 -1 D
So D's alias of 1 isn't honored even though if both IDs were searched for together, it would have been.
Do we really want that behaviour? Overall I think it is a bit odd to have devices with no aliases if you actually care about the ordering. At least in U-Boot ordering is important. The simple fix above is to provide an alias for everything, which is what we do. If we do want the behaviour you suggest then I will need the change to doing it in one pass.
Also, what about the scenario where a driver searches for both nvidia,tegra20-i2c then later nvidia,tegra30-i2c, given that all the DT nodes are probably compatible with both?
If all DT notes are compatible with both, why are you searching for one and then the other? Why not just search for one?
It sounds like a decision here as to whether we want a one-pass algorithm with a list of compatible nodes, or the current additive priorty-based approach.
Regards, Simon
-- nvpublic

On 01/19/2012 04:45 PM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:49 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, Simon Glass wrote:
Some devices can deal with multiple compatible properties. The devices need to know which nodes to bind to which features. For example an I2C driver which supports two different controller types will want to know which type it is dealing with in each case.
The new fdtdec_add_aliases_for_id() function deals with this by allowing the driver to search for additional compatible nodes for a different ID. It can then detect the new ones and perform appropriate processing.
Another option considered was to return a tuple (node offset, compat id) and have the function be passed a list of compatible IDs. This is more overhead for the common case though. We may add such a function later if more drivers in U-Boot require it.
...
I'm not convinced this patch is correct in all cases. It'll probably work fine for simply device-trees though. Consider the following case:
4 device nodes A: compat=i2c alias for usb0 B: compat=i2c no alias C: compat=i2c alias for usb2 D: compat=i2cx alias for usb1
First, we search for all compat=i2c, the list comes back:
0=A 1=B 2=C
Then we search for all compat=i2cx, the list comes back:
-1 -1 -1 D
So D's alias of 1 isn't honored even though if both IDs were searched for together, it would have been.
Do we really want that behaviour? Overall I think it is a bit odd to have devices with no aliases if you actually care about the ordering. At least in U-Boot ordering is important. The simple fix above is to provide an alias for everything, which is what we do. If we do want the behaviour you suggest then I will need the change to doing it in one pass.
I think that behaviour is the intended given that DT content. Despite the fact that content is admittedly a little twisted and perhaps not commonly useful, since this is user-supplied content in general, I don't think we can simply not accept it or not implement the fairly obvious intent.
Also, what about the scenario where a driver searches for both nvidia,tegra20-i2c then later nvidia,tegra30-i2c, given that all the DT nodes are probably compatible with both?
If all DT notes are compatible with both, why are you searching for one and then the other? Why not just search for one?
I suppose one /shouldn't/ need to.
I was thinking of supporting the case where somebody only wrote compatible = "nvidia,tegra30-i2c" and left out the Tegra20 variant. Still, is arguably a bug, and that won't work in the kernel either since we're not going around adding all the new compatible values into the drivers when the devices are compatible with the existing HW anyway.
I guess if we do get some legitimate case that trips this issue, we can just fix it then, since it's entirely isolated to the code and doesn't impact anything externally visible etc.

Hi Stephen,
On Thu, Jan 19, 2012 at 4:17 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/19/2012 04:45 PM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:49 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, Simon Glass wrote:
Some devices can deal with multiple compatible properties. The devices need to know which nodes to bind to which features. For example an I2C driver which supports two different controller types will want to know which type it is dealing with in each case.
The new fdtdec_add_aliases_for_id() function deals with this by allowing the driver to search for additional compatible nodes for a different ID. It can then detect the new ones and perform appropriate processing.
Another option considered was to return a tuple (node offset, compat id) and have the function be passed a list of compatible IDs. This is more overhead for the common case though. We may add such a function later if more drivers in U-Boot require it.
...
I'm not convinced this patch is correct in all cases. It'll probably work fine for simply device-trees though. Consider the following case:
4 device nodes A: compat=i2c alias for usb0 B: compat=i2c no alias C: compat=i2c alias for usb2 D: compat=i2cx alias for usb1
First, we search for all compat=i2c, the list comes back:
0=A 1=B 2=C
Then we search for all compat=i2cx, the list comes back:
-1 -1 -1 D
So D's alias of 1 isn't honored even though if both IDs were searched for together, it would have been.
Do we really want that behaviour? Overall I think it is a bit odd to have devices with no aliases if you actually care about the ordering. At least in U-Boot ordering is important. The simple fix above is to provide an alias for everything, which is what we do. If we do want the behaviour you suggest then I will need the change to doing it in one pass.
I think that behaviour is the intended given that DT content. Despite the fact that content is admittedly a little twisted and perhaps not commonly useful, since this is user-supplied content in general, I don't think we can simply not accept it or not implement the fairly obvious intent.
Also, what about the scenario where a driver searches for both nvidia,tegra20-i2c then later nvidia,tegra30-i2c, given that all the DT nodes are probably compatible with both?
If all DT notes are compatible with both, why are you searching for one and then the other? Why not just search for one?
I suppose one /shouldn't/ need to.
I was thinking of supporting the case where somebody only wrote compatible = "nvidia,tegra30-i2c" and left out the Tegra20 variant. Still, is arguably a bug, and that won't work in the kernel either since we're not going around adding all the new compatible values into the drivers when the devices are compatible with the existing HW anyway.
I guess if we do get some legitimate case that trips this issue, we can just fix it then, since it's entirely isolated to the code and doesn't impact anything externally visible etc.
Hmmm well perhaps I should add a code comment and see how we go. It would be nice if this situation could be spotted. I am concerned about anything that is non-obvious here. Will take a look but otherwise it sounds like we can punt this until later (unless we decide to move to the one-pass option).
Anyway this is in a place by itself with unit tests so we should be able to fiddle if needed.
Regards, Simon
-- nvpublic

Add U-Boot's peripheral clock information to the Tegra20 device tree file.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v2: - Adjust definitions to fit new peripheral clock bindings - Change 'speed' to 'clock-frequency' - Remove u-boot,pinmux binding (sadly)
arch/arm/dts/tegra20.dtsi | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi index ca7b523..963cf27 100644 --- a/arch/arm/dts/tegra20.dtsi +++ b/arch/arm/dts/tegra20.dtsi @@ -45,6 +45,8 @@ compatible = "nvidia,tegra20-i2c"; reg = <0x7000C000 0x100>; interrupts = < 70 >; + clock-frequency = <100000>; + clocks = <&periph_clk 12>; // PERIPH_ID_I2C1 };
i2c@7000c400 { @@ -53,6 +55,8 @@ compatible = "nvidia,tegra20-i2c"; reg = <0x7000C400 0x100>; interrupts = < 116 >; + clock-frequency = <100000>; + clocks = <&periph_clk 54>; // PERIPH_ID_I2C2 };
i2c@7000c500 { @@ -61,14 +65,18 @@ compatible = "nvidia,tegra20-i2c"; reg = <0x7000C500 0x100>; interrupts = < 124 >; + clock-frequency = <100000>; + clocks = <&periph_clk 67>; // PERIPH_ID_I2C3 };
i2c@7000d000 { #address-cells = <1>; #size-cells = <0>; - compatible = "nvidia,tegra20-i2c"; + compatible = "nvidia,tegra20-i2c-dvc"; reg = <0x7000D000 0x200>; interrupts = < 85 >; + clock-frequency = <100000>; + clocks = <&periph_clk 47>; // PERIPH_ID_DVC_I2C };
i2s@70002800 {

Hello Simon,
Simon Glass wrote:
Add U-Boot's peripheral clock information to the Tegra20 device tree file.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Adjust definitions to fit new peripheral clock bindings
- Change 'speed' to 'clock-frequency'
- Remove u-boot,pinmux binding (sadly)
arch/arm/dts/tegra20.dtsi | 10 +++++++++-
Hmm.. we have no such a directory in the u-boot source tree ... ? I think, this patch is for the linux tree?
[...]
bye, Heiko

Hi Heiko,
On Thu, Jan 12, 2012 at 10:31 PM, Heiko Schocher hs@denx.de wrote:
Hello Simon,
Simon Glass wrote:
Add U-Boot's peripheral clock information to the Tegra20 device tree file.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Adjust definitions to fit new peripheral clock bindings
- Change 'speed' to 'clock-frequency'
- Remove u-boot,pinmux binding (sadly)
arch/arm/dts/tegra20.dtsi | 10 +++++++++-
Hmm.. we have no such a directory in the u-boot source tree ... ? I think, this patch is for the linux tree?
No, it's for a future U-Boot :-) It builds on USB series (which brings in the basic device tree file).
Regards, Simon
[...]
bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

On 01/12/2012 12:00 PM, Simon Glass wrote:
Add U-Boot's peripheral clock information to the Tegra20 device tree file.
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi index ca7b523..963cf27 100644 --- a/arch/arm/dts/tegra20.dtsi +++ b/arch/arm/dts/tegra20.dtsi @@ -45,6 +45,8 @@ compatible = "nvidia,tegra20-i2c"; reg = <0x7000C000 0x100>; interrupts = < 70 >;
clock-frequency = <100000>;
That's board-specific information. I'd rather not include it in the SoC .dtsi file even as a default; that way, it forces people to think about the correct value and put it in their board file, whereas including a default means people probably won't think about it.
clocks = <&periph_clk 12>; // PERIPH_ID_I2C1
Of course, that'll need updating based on the resolution of the clock binding thread I started.

Hi Stephen,
On Thu, Jan 19, 2012 at 12:51 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, Simon Glass wrote:
Add U-Boot's peripheral clock information to the Tegra20 device tree file.
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi index ca7b523..963cf27 100644 --- a/arch/arm/dts/tegra20.dtsi +++ b/arch/arm/dts/tegra20.dtsi @@ -45,6 +45,8 @@ compatible = "nvidia,tegra20-i2c"; reg = <0x7000C000 0x100>; interrupts = < 70 >;
- clock-frequency = <100000>;
That's board-specific information. I'd rather not include it in the SoC .dtsi file even as a default; that way, it forces people to think about the correct value and put it in their board file, whereas including a default means people probably won't think about it.
OK I moved this into the board file - sadly this means everyone will need to specify it here. Or should we have a default?
- clocks = <&periph_clk 12>; // PERIPH_ID_I2C1
Of course, that'll need updating based on the resolution of the clock binding thread I started.
Yes we can come back to this when resolved.
Regards, Simon
-- nvpublic

On 01/22/2012 10:41 AM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:51 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, Simon Glass wrote:
Add U-Boot's peripheral clock information to the Tegra20 device tree file.
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi index ca7b523..963cf27 100644 --- a/arch/arm/dts/tegra20.dtsi +++ b/arch/arm/dts/tegra20.dtsi @@ -45,6 +45,8 @@ compatible = "nvidia,tegra20-i2c"; reg = <0x7000C000 0x100>; interrupts = < 70 >;
clock-frequency = <100000>;
That's board-specific information. I'd rather not include it in the SoC .dtsi file even as a default; that way, it forces people to think about the correct value and put it in their board file, whereas including a default means people probably won't think about it.
OK I moved this into the board file - sadly this means everyone will need to specify it here. Or should we have a default?
I think I'd rather not have the default. That way, people need to think about the correct value to use in their board file, rather than having it magically work without having validated the value.
If people disagree, we'd need to update the kernel's tegra[23]0.dtsi, since they assume ${board}.dts will set this property.

Hi Stephen,
On Mon, Jan 23, 2012 at 10:25 AM, Stephen Warren swarren@nvidia.com wrote:
On 01/22/2012 10:41 AM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:51 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, Simon Glass wrote:
Add U-Boot's peripheral clock information to the Tegra20 device tree file.
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi index ca7b523..963cf27 100644 --- a/arch/arm/dts/tegra20.dtsi +++ b/arch/arm/dts/tegra20.dtsi @@ -45,6 +45,8 @@ compatible = "nvidia,tegra20-i2c"; reg = <0x7000C000 0x100>; interrupts = < 70 >;
- clock-frequency = <100000>;
That's board-specific information. I'd rather not include it in the SoC .dtsi file even as a default; that way, it forces people to think about the correct value and put it in their board file, whereas including a default means people probably won't think about it.
OK I moved this into the board file - sadly this means everyone will need to specify it here. Or should we have a default?
I think I'd rather not have the default. That way, people need to think about the correct value to use in their board file, rather than having it magically work without having validated the value.
If people disagree, we'd need to update the kernel's tegra[23]0.dtsi, since they assume ${board}.dts will set this property.
OK that's fine with me.
Regards, Simon
-- nvpublic

From: Yen Lin yelin@nvidia.com
Add basic i2c driver for Tegra2 with 8- and 16-bit address support. The driver requires CONFIG_OF_CONTROL to obtain its configuration from the device tree.
(Simon Glass: sjg@chromium.org modified for upstream)
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v2: - Use DIV_ROUND_UP() instead of a home-grown macro - Tidy comment style - Change i2c array to static - Adjust definitions to fit new peripheral clock bindings - Remove i2c configuring using CONFIG (use fdt instead) - Make i2c/dvc decision come from fdt - Use new fdtdec alias decode function - Simplify code in i2c_addr_ok() - Return an error if an unavailable i2c bus is selected
arch/arm/include/asm/arch-tegra2/tegra2_i2c.h | 160 +++++++ drivers/i2c/Makefile | 1 + drivers/i2c/tegra2_i2c.c | 551 +++++++++++++++++++++++++ include/fdtdec.h | 2 + lib/fdtdec.c | 2 + 5 files changed, 716 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_i2c.h b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h new file mode 100644 index 0000000..86f6a01 --- /dev/null +++ b/arch/arm/include/asm/arch-tegra2/tegra2_i2c.h @@ -0,0 +1,160 @@ +/* + * 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 i2c slave address to be put on bus */ +#define I2C_ADDR_ON_BUS(chip) (chip << 1) + +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 the I2C send operation this buffer should + * be filled with the data to be sent to the slave device. For the 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..a7db714 --- /dev/null +++ b/drivers/i2c/tegra2_i2c.c @@ -0,0 +1,551 @@ +/* + * 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; + int is_dvc; /* DVC type, rather than I2C */ + int inited; /* bus is inited */ +}; + +static 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->is_dvc) { + 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->is_dvc) { /* 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 = DIV_ROUND_UP(trans->num_bytes, 4); + 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 +#error "Please enable device tree support to use this driver" +#endif + +unsigned int i2c_get_bus_speed(void) +{ + return i2c_controllers[i2c_bus_num].speed; +} + +int i2c_set_bus_speed(unsigned int speed) +{ + struct i2c_bus *i2c_bus; + + i2c_bus = &i2c_controllers[i2c_bus_num]; + i2c_bus->speed = speed; + i2c_init_controller(i2c_bus); + + return 0; +} + +static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus) +{ + i2c_bus->regs = (struct i2c_ctlr *)fdtdec_get_addr(blob, node, "reg"); + + /* + * We don't have a binding for pinmux yet. Leave it out for now. So + * far no one needs anything other than the default. + */ + i2c_bus->pinmux_config = FUNCMUX_DEFAULT; + i2c_bus->speed = fdtdec_get_int(blob, node, "clock-frequency", 0); + i2c_bus->periph_id = fdtdec_decode_periph_id(blob, node); + + if (i2c_bus->periph_id == -1) + return -FDT_ERR_NOTFOUND; + + return 0; +} + +/* + * Process a list of nodes, adding them to our list of I2C ports. + * + * @param blob fdt blob + * @param node_list list of nodes to process (any <=0 are ignored) + * @param count number of nodes to process + * @param is_dvc 1 if these are DVC ports, 0 if standard I2C + * @return 0 if ok, -1 on error + */ +static int process_nodes(const void *blob, int node_list[], int count, + int is_dvc) +{ + struct i2c_bus *i2c_bus; + int i; + + /* build the i2c_controllers[] for each controller */ + for (i = 0; i < count; i++) { + int node = node_list[i]; + + if (node <= 0) + continue; + + i2c_bus = &i2c_controllers[i]; + i2c_bus->id = i; + + if (i2c_get_config(blob, node, i2c_bus)) { + printf("i2c_init_board: failed to decode bus %d\n", i); + return -1; + } + + i2c_bus->is_dvc = is_dvc; + if (is_dvc) { + i2c_bus->control = + &((struct dvc_ctlr *)i2c_bus->regs)->control; + } else { + i2c_bus->control = &i2c_bus->regs->control; + } + debug("%s: controller bus %d at %p, periph_id %d, speed %d: ", + is_dvc ? "dvc" : "i2c", i, i2c_bus->regs, + i2c_bus->periph_id, i2c_bus->speed); + i2c_init_controller(i2c_bus); + debug("ok\n"); + i2c_bus->inited = 1; + + /* Mark position as used */ + node_list[i] = -1; + } + + return 0; +} + +int i2c_init_board(void) +{ + int node_list[CONFIG_SYS_MAX_I2C_BUS]; + const void *blob = gd->fdt_blob; + int count; + + /* First get the normal i2c ports */ + count = fdtdec_find_aliases_for_id(blob, "i2c", + COMPAT_NVIDIA_TEGRA20_I2C, node_list, + CONFIG_SYS_MAX_I2C_BUS); + if (process_nodes(blob, node_list, count, 0)) + return -1; + + /* Now look for dvc ports */ + count = fdtdec_add_aliases_for_id(blob, "i2c", + COMPAT_NVIDIA_TEGRA20_DVC, node_list, + CONFIG_SYS_MAX_I2C_BUS); + if (process_nodes(blob, node_list, count, 1)) + return -1; + + 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, ®, 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) +{ + /* We support 7 or 10 bit addresses, so one or two bytes each */ + return addr == 1 || addr == 2; +} + +/* 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 || !i2c_controllers[bus].inited) + return -1; + i2c_bus_num = bus; + + return 0; +} +#endif diff --git a/include/fdtdec.h b/include/fdtdec.h index e6d63f9..82264d4 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -58,6 +58,8 @@ struct fdt_memory { enum fdt_compat_id { COMPAT_UNKNOWN, COMPAT_NVIDIA_TEGRA20_USB, /* Tegra2 USB port */ + COMPAT_NVIDIA_TEGRA20_I2C, /* Tegra2 i2c */ + COMPAT_NVIDIA_TEGRA20_DVC, /* Tegra2 dvc (really just i2c) */
COMPAT_COUNT, }; diff --git a/lib/fdtdec.c b/lib/fdtdec.c index ddd56b9..3330edf 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -38,6 +38,8 @@ 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"), + COMPAT(NVIDIA_TEGRA20_DVC, "nvidia,tegra20-i2c-dvc"), };
const char *fdtdec_get_compatible(enum fdt_compat_id id)

Hello Simon,
Simon Glass wrote:
From: Yen Lin yelin@nvidia.com
Add basic i2c driver for Tegra2 with 8- and 16-bit address support. The driver requires CONFIG_OF_CONTROL to obtain its configuration from the device tree.
(Simon Glass: sjg@chromium.org modified for upstream)
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Use DIV_ROUND_UP() instead of a home-grown macro
- Tidy comment style
- Change i2c array to static
- Adjust definitions to fit new peripheral clock bindings
- Remove i2c configuring using CONFIG (use fdt instead)
Why? Ah found it ... Hmm.. why we don't need the non OF case?
- Make i2c/dvc decision come from fdt
- Use new fdtdec alias decode function
- Simplify code in i2c_addr_ok()
- Return an error if an unavailable i2c bus is selected
arch/arm/include/asm/arch-tegra2/tegra2_i2c.h | 160 +++++++ drivers/i2c/Makefile | 1 + drivers/i2c/tegra2_i2c.c | 551 +++++++++++++++++++++++++ include/fdtdec.h | 2 + lib/fdtdec.c | 2 + 5 files changed, 716 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/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c new file mode 100644 index 0000000..a7db714 --- /dev/null +++ b/drivers/i2c/tegra2_i2c.c @@ -0,0 +1,551 @@
[...]
+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));
Can you use here some defines? What is (8 * 2 - 1) ?
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
Hmm.. see above question. Ok, if somebody need want to use this driver without CONFIG_OF_CONTROL it must be added ...
+/*
- Process a list of nodes, adding them to our list of I2C ports.
- @param blob fdt blob
- @param node_list list of nodes to process (any <=0 are ignored)
- @param count number of nodes to process
- @param is_dvc 1 if these are DVC ports, 0 if standard I2C
- @return 0 if ok, -1 on error
- */
+static int process_nodes(const void *blob, int node_list[], int count,
int is_dvc)
+{
- struct i2c_bus *i2c_bus;
- int i;
- /* build the i2c_controllers[] for each controller */
- for (i = 0; i < count; i++) {
int node = node_list[i];
if (node <= 0)
continue;
i2c_bus = &i2c_controllers[i];
i2c_bus->id = i;
if (i2c_get_config(blob, node, i2c_bus)) {
printf("i2c_init_board: failed to decode bus %d\n", i);
return -1;
}
i2c_bus->is_dvc = is_dvc;
if (is_dvc) {
i2c_bus->control =
&((struct dvc_ctlr *)i2c_bus->regs)->control;
} else {
i2c_bus->control = &i2c_bus->regs->control;
}
debug("%s: controller bus %d at %p, periph_id %d, speed %d: ",
is_dvc ? "dvc" : "i2c", i, i2c_bus->regs,
i2c_bus->periph_id, i2c_bus->speed);
i2c_init_controller(i2c_bus);
debug("ok\n");
i2c_bus->inited = 1;
/* Mark position as used */
node_list[i] = -1;
- }
- return 0;
+}
+int i2c_init_board(void) +{
- int node_list[CONFIG_SYS_MAX_I2C_BUS];
- const void *blob = gd->fdt_blob;
- int count;
- /* First get the normal i2c ports */
- count = fdtdec_find_aliases_for_id(blob, "i2c",
COMPAT_NVIDIA_TEGRA20_I2C, node_list,
CONFIG_SYS_MAX_I2C_BUS);
- if (process_nodes(blob, node_list, count, 0))
return -1;
- /* Now look for dvc ports */
- count = fdtdec_add_aliases_for_id(blob, "i2c",
COMPAT_NVIDIA_TEGRA20_DVC, node_list,
CONFIG_SYS_MAX_I2C_BUS);
- if (process_nodes(blob, node_list, count, 1))
return -1;
- return 0;
+}
+void i2c_init(int speed, int slaveaddr) +{
- debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
+}
Hmm... i2c_init is called to init the i2c subsystem ... you do nothing here ... and use i2c_init_board for init the i2c bus, right?
But i2c_init_board is not called from the driver ... ah, you do this in board code ... Ok ...
I think, you do this, because i2c_init is called very early, and so processing fdt is slow?
[...]
bye, Heiko

Hi Heiko,
On Thu, Jan 12, 2012 at 11:25 PM, Heiko Schocher hs@denx.de wrote:
Hello Simon,
Simon Glass wrote:
From: Yen Lin yelin@nvidia.com
Add basic i2c driver for Tegra2 with 8- and 16-bit address support. The driver requires CONFIG_OF_CONTROL to obtain its configuration from the device tree.
(Simon Glass: sjg@chromium.org modified for upstream)
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Use DIV_ROUND_UP() instead of a home-grown macro
- Tidy comment style
- Change i2c array to static
- Adjust definitions to fit new peripheral clock bindings
- Remove i2c configuring using CONFIG (use fdt instead)
Why? Ah found it ... Hmm.. why we don't need the non OF case?
The intent is that Tegra boards will run with fdt turned on. It means that we should be able to build U-Boot for a Tegra platform using the Linux fdt file and not lots of manual config. Of course this works better for some peripherals than others, but I2C works well enough.
- Make i2c/dvc decision come from fdt
- Use new fdtdec alias decode function
- Simplify code in i2c_addr_ok()
- Return an error if an unavailable i2c bus is selected
arch/arm/include/asm/arch-tegra2/tegra2_i2c.h | 160 +++++++ drivers/i2c/Makefile | 1 + drivers/i2c/tegra2_i2c.c | 551 +++++++++++++++++++++++++ include/fdtdec.h | 2 + lib/fdtdec.c | 2 + 5 files changed, 716 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/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c new file mode 100644 index 0000000..a7db714 --- /dev/null +++ b/drivers/i2c/tegra2_i2c.c @@ -0,0 +1,551 @@
[...]
+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));
Can you use here some defines? What is (8 * 2 - 1) ?
I will look up the bug and find out.
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
Hmm.. see above question. Ok, if somebody need want to use this driver without CONFIG_OF_CONTROL it must be added ...
Yes and they need a way of getting the configuration in there. The fdt is nicer...
+/*
- Process a list of nodes, adding them to our list of I2C ports.
- @param blob fdt blob
- @param node_list list of nodes to process (any <=0 are ignored)
- @param count number of nodes to process
- @param is_dvc 1 if these are DVC ports, 0 if standard I2C
- @return 0 if ok, -1 on error
- */
+static int process_nodes(const void *blob, int node_list[], int count,
- int is_dvc)
+{
- struct i2c_bus *i2c_bus;
- int i;
- /* build the i2c_controllers[] for each controller */
- for (i = 0; i < count; i++) {
- int node = node_list[i];
- if (node <= 0)
- continue;
- i2c_bus = &i2c_controllers[i];
- i2c_bus->id = i;
- if (i2c_get_config(blob, node, i2c_bus)) {
- printf("i2c_init_board: failed to decode bus %d\n", i);
- return -1;
- }
- i2c_bus->is_dvc = is_dvc;
- if (is_dvc) {
- i2c_bus->control =
- &((struct dvc_ctlr *)i2c_bus->regs)->control;
- } else {
- i2c_bus->control = &i2c_bus->regs->control;
- }
- debug("%s: controller bus %d at %p, periph_id %d, speed %d: ",
- is_dvc ? "dvc" : "i2c", i, i2c_bus->regs,
- i2c_bus->periph_id, i2c_bus->speed);
- i2c_init_controller(i2c_bus);
- debug("ok\n");
- i2c_bus->inited = 1;
- /* Mark position as used */
- node_list[i] = -1;
- }
- return 0;
+}
+int i2c_init_board(void) +{
- int node_list[CONFIG_SYS_MAX_I2C_BUS];
- const void *blob = gd->fdt_blob;
- int count;
- /* First get the normal i2c ports */
- count = fdtdec_find_aliases_for_id(blob, "i2c",
- COMPAT_NVIDIA_TEGRA20_I2C, node_list,
- CONFIG_SYS_MAX_I2C_BUS);
- if (process_nodes(blob, node_list, count, 0))
- return -1;
- /* Now look for dvc ports */
- count = fdtdec_add_aliases_for_id(blob, "i2c",
- COMPAT_NVIDIA_TEGRA20_DVC, node_list,
- CONFIG_SYS_MAX_I2C_BUS);
- if (process_nodes(blob, node_list, count, 1))
- return -1;
- return 0;
+}
+void i2c_init(int speed, int slaveaddr) +{
- debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
+}
Hmm... i2c_init is called to init the i2c subsystem ... you do nothing here ... and use i2c_init_board for init the i2c bus, right?
But i2c_init_board is not called from the driver ... ah, you do this in board code ... Ok ...
I think, you do this, because i2c_init is called very early, and so processing fdt is slow?
That's not the main reason but it is true. We have no need of early I2C on the platform. Also we don't want to set the speed as this is defined individually per port in the fdt.
Regards, Simon
[...]
bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Hello Simon,
Simon Glass wrote:
Hi Heiko,
On Thu, Jan 12, 2012 at 11:25 PM, Heiko Schocher hs@denx.de wrote:
Hello Simon,
Simon Glass wrote:
From: Yen Lin yelin@nvidia.com
Add basic i2c driver for Tegra2 with 8- and 16-bit address support. The driver requires CONFIG_OF_CONTROL to obtain its configuration from the device tree.
(Simon Glass: sjg@chromium.org modified for upstream)
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Use DIV_ROUND_UP() instead of a home-grown macro
- Tidy comment style
- Change i2c array to static
- Adjust definitions to fit new peripheral clock bindings
- Remove i2c configuring using CONFIG (use fdt instead)
Why? Ah found it ... Hmm.. why we don't need the non OF case?
The intent is that Tegra boards will run with fdt turned on. It means that we should be able to build U-Boot for a Tegra platform using the Linux fdt file and not lots of manual config. Of course this works better for some peripherals than others, but I2C works well enough.
Ah, Ok, nice!
- Make i2c/dvc decision come from fdt
- Use new fdtdec alias decode function
- Simplify code in i2c_addr_ok()
- Return an error if an unavailable i2c bus is selected
arch/arm/include/asm/arch-tegra2/tegra2_i2c.h | 160 +++++++ drivers/i2c/Makefile | 1 + drivers/i2c/tegra2_i2c.c | 551 +++++++++++++++++++++++++ include/fdtdec.h | 2 + lib/fdtdec.c | 2 + 5 files changed, 716 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/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c new file mode 100644 index 0000000..a7db714 --- /dev/null +++ b/drivers/i2c/tegra2_i2c.c @@ -0,0 +1,551 @@
[...]
+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));
Can you use here some defines?
What is (8 * 2 - 1) ?
I will look up the bug and find out.
Not a bug, just I did not know what this values are for ...
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
Hmm.. see above question. Ok, if somebody need want to use this driver without CONFIG_OF_CONTROL it must be added ...
Yes and they need a way of getting the configuration in there. The fdt is nicer...
;-)
[...]
+void i2c_init(int speed, int slaveaddr) +{
debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
+}
Hmm... i2c_init is called to init the i2c subsystem ... you do nothing here ... and use i2c_init_board for init the i2c bus, right?
But i2c_init_board is not called from the driver ... ah, you do this in board code ... Ok ...
I think, you do this, because i2c_init is called very early, and so processing fdt is slow?
That's not the main reason but it is true. We have no need of early I2C on the platform. Also we don't want to set the speed as this is defined individually per port in the fdt.
Ok.
bye, Heiko

On 01/12/2012 12:00 PM, Simon Glass wrote:
From: Yen Lin yelin@nvidia.com
Add basic i2c driver for Tegra2 with 8- and 16-bit address support. The driver requires CONFIG_OF_CONTROL to obtain its configuration from the device tree.
(Simon Glass: sjg@chromium.org modified for upstream)
Signed-off-by: Simon Glass sjg@chromium.org
Why isn't Yen's S-o-b line here, since he's listed as the patch author in git?
diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
+static struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
I still don't like using the wrong define here, but since I see you pushing the multi-bus stuff upstream, so I assume you'll come back and fix this when it makes a difference, so I won't object too much about this.
+void i2c_init(int speed, int slaveaddr) +{
debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
+}
What was the resolution on making this do something correct instead of ignoring the request?

Hi Stephen,
On Thu, Jan 19, 2012 at 1:08 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, Simon Glass wrote:
From: Yen Lin yelin@nvidia.com
Add basic i2c driver for Tegra2 with 8- and 16-bit address support. The driver requires CONFIG_OF_CONTROL to obtain its configuration from the device tree.
(Simon Glass: sjg@chromium.org modified for upstream)
Signed-off-by: Simon Glass sjg@chromium.org
Why isn't Yen's S-o-b line here, since he's listed as the patch author in git?
It will work if he replies to this thread or the new patch that I sent out. I cannot add it for him.
diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
+static struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS];
I still don't like using the wrong define here, but since I see you pushing the multi-bus stuff upstream, so I assume you'll come back and fix this when it makes a difference, so I won't object too much about this.
I have changed this over.
+void i2c_init(int speed, int slaveaddr) +{
- debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
+}
What was the resolution on making this do something correct instead of ignoring the request?
I am just going to change the speed, thus forgetting the speed that the fdt specified. There is no correct answer here - only the least wrong. I will flip it the other way and see what people think.
Regards, Simon
-- nvpublic

Hi Simon,
On Fri, 2012-02-03 at 15:26 -0800, Simon Glass wrote:
Why isn't Yen's S-o-b line here, since he's listed as the patch author in git?
It will work if he replies to this thread or the new patch that I sent out. I cannot add it for him.
Don't know how to add a S-o-b line to the patch. Please add "Signed-off-by: Yen Lin yelin@nvidia.com"
Thanks, Yen
----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------

Hi Yen,
On Mon, Feb 6, 2012 at 1:41 PM, Yen Lin yelin@nvidia.com wrote:
Hi Simon,
On Fri, 2012-02-03 at 15:26 -0800, Simon Glass wrote:
Why isn't Yen's S-o-b line here, since he's listed as the patch author in git?
It will work if he replies to this thread or the new patch that I sent out. I cannot add it for him.
Don't know how to add a S-o-b line to the patch. Please add "Signed-off-by: Yen Lin yelin@nvidia.com"
You can just put that starting at column 1 without quotes:
Signed-off-by: Yen Lin yelin@nvidia.com
Then it should turn up here:
http://patchwork.ozlabs.org/patch/135679/
Regards, Simon
Thanks, Yen
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.

This enables I2C on all Nvidia boards including Seaboard and Harmony.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v2: - Add build warning if CONFIG_SYS_I2C_INIT_BOARD is not defined
board/nvidia/common/board.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c index a7c566d..ddcfcf4 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,12 @@ int board_init(void) #endif /* boot param addr */ gd->bd->bi_boot_params = (NV_PA_SDRAM_BASE + 0x100); +#ifdef CONFIG_TEGRA2_I2C +#ifndef CONFIG_SYS_I2C_INIT_BOARD +#error "You must define CONFIG_SYS_I2C_INIT_BOARD to use i2c on Nvidia boards" +#endif + i2c_init_board(); +#endif
#ifdef CONFIG_USB_EHCI_TEGRA /* For USB GPIO PD0. for now, since we have no pinmux in fdt */

Select the port ordering for I2C on Seaboard.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Stephen Warren swarren@nvidia.com --- Changes in v2: - Disable port 2 as it is not used
board/nvidia/dts/tegra2-seaboard.dts | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts index 7a4ecae..d2cc428 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@c5008000"; usb1 = "/usb@c5000000"; + + i2c0 = "/i2c@7000d000"; + i2c1 = "/i2c@7000c000"; + i2c2 = "/i2c@7000c400"; + i2c3 = "/i2c@7000c500"; };
memory { @@ -44,4 +49,9 @@ usb@c5004000 { status = "disabled"; }; + + i2c@7000c400 { + status = "disabled"; + }; + };

On 01/12/2012 12:00 PM, 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
This isn't the patch that I ack'd.
diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not tegra2-seaboard.dts to match the naming in the kernel?
@@ -44,4 +49,9 @@ usb@c5004000 { status = "disabled"; };
- i2c@7000c400 {
status = "disabled";
- };
};
That chunk wasn't in the original patch, and doesn't match the kernel's .dts file (and I believe that I2C controller really is in use, so shouldn't be disabled).

Hi Stephen,
On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, 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
This isn't the patch that I ack'd.
Sorry, I added the disable.
diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not tegra2-seaboard.dts to match the naming in the kernel?
@@ -44,4 +49,9 @@ usb@c5004000 { status = "disabled"; };
- i2c@7000c400 {
- status = "disabled";
- };
};
That chunk wasn't in the original patch, and doesn't match the kernel's .dts file (and I believe that I2C controller really is in use, so shouldn't be disabled).
It cannot be used - remember the discussion about pinmux? We elected to disable I2C1 at present since you didn't like my nvidia,pinmux binding for selecting which value to pass to funcmux. The fix is to pass 1 instead of 0 for that port, but we have no clean way to specify this.
Rather disable it than leave it enabled and not working.
Regards, Simon
-- nvpublic

On 02/03/2012 04:24 PM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, 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
This isn't the patch that I ack'd.
Sorry, I added the disable.
diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not tegra2-seaboard.dts to match the naming in the kernel?
@@ -44,4 +49,9 @@ usb@c5004000 { status = "disabled"; };
i2c@7000c400 {
status = "disabled";
};
};
That chunk wasn't in the original patch, and doesn't match the kernel's .dts file (and I believe that I2C controller really is in use, so shouldn't be disabled).
It cannot be used - remember the discussion about pinmux? We elected to disable I2C1 at present since you didn't like my nvidia,pinmux binding for selecting which value to pass to funcmux. The fix is to pass 1 instead of 0 for that port, but we have no clean way to specify this.
Rather disable it than leave it enabled and not working.
Rather than having the .dts file not correctly describe the HW, wouldn't it be better to limit U-Boot's to only initializing the 1 I2C controller that it knows the valid pinmux setting for?

Hi Stephen,
On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren swarren@nvidia.com wrote:
On 02/03/2012 04:24 PM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, 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
This isn't the patch that I ack'd.
Sorry, I added the disable.
diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not tegra2-seaboard.dts to match the naming in the kernel?
@@ -44,4 +49,9 @@ usb@c5004000 { status = "disabled"; };
- i2c@7000c400 {
- status = "disabled";
- };
};
That chunk wasn't in the original patch, and doesn't match the kernel's .dts file (and I believe that I2C controller really is in use, so shouldn't be disabled).
It cannot be used - remember the discussion about pinmux? We elected to disable I2C1 at present since you didn't like my nvidia,pinmux binding for selecting which value to pass to funcmux. The fix is to pass 1 instead of 0 for that port, but we have no clean way to specify this.
Rather disable it than leave it enabled and not working.
Rather than having the .dts file not correctly describe the HW, wouldn't it be better to limit U-Boot's to only initializing the 1 I2C controller that it knows the valid pinmux setting for?
Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not used or connected to anything, so it is a reasonable description of the hardware. The other 3 pinmux settings are valid.
Regards, Simon
-- nvpublic

On 02/03/2012 05:19 PM, Simon Glass wrote:
Hi Stephen,
On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren swarren@nvidia.com wrote:
On 02/03/2012 04:24 PM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, 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
This isn't the patch that I ack'd.
Sorry, I added the disable.
diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not tegra2-seaboard.dts to match the naming in the kernel?
@@ -44,4 +49,9 @@ usb@c5004000 { status = "disabled"; };
i2c@7000c400 {
status = "disabled";
};
};
That chunk wasn't in the original patch, and doesn't match the kernel's .dts file (and I believe that I2C controller really is in use, so shouldn't be disabled).
It cannot be used - remember the discussion about pinmux? We elected to disable I2C1 at present since you didn't like my nvidia,pinmux binding for selecting which value to pass to funcmux. The fix is to pass 1 instead of 0 for that port, but we have no clean way to specify this.
Rather disable it than leave it enabled and not working.
Rather than having the .dts file not correctly describe the HW, wouldn't it be better to limit U-Boot's to only initializing the 1 I2C controller that it knows the valid pinmux setting for?
Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not used or connected to anything, so it is a reasonable description of the hardware. The other 3 pinmux settings are valid.
As best I can tell from my schematics, all 4 I2C ports are used on both Seaboard and Springbank.

Hi Stephen,
On Fri, Feb 3, 2012 at 4:25 PM, Stephen Warren swarren@nvidia.com wrote:
On 02/03/2012 05:19 PM, Simon Glass wrote:
Hi Stephen,
On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren swarren@nvidia.com wrote:
On 02/03/2012 04:24 PM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, 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
This isn't the patch that I ack'd.
Sorry, I added the disable.
diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not tegra2-seaboard.dts to match the naming in the kernel?
@@ -44,4 +49,9 @@ usb@c5004000 { status = "disabled"; };
- i2c@7000c400 {
- status = "disabled";
- };
};
That chunk wasn't in the original patch, and doesn't match the kernel's .dts file (and I believe that I2C controller really is in use, so shouldn't be disabled).
It cannot be used - remember the discussion about pinmux? We elected to disable I2C1 at present since you didn't like my nvidia,pinmux binding for selecting which value to pass to funcmux. The fix is to pass 1 instead of 0 for that port, but we have no clean way to specify this.
Rather disable it than leave it enabled and not working.
Rather than having the .dts file not correctly describe the HW, wouldn't it be better to limit U-Boot's to only initializing the 1 I2C controller that it knows the valid pinmux setting for?
Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not used or connected to anything, so it is a reasonable description of the hardware. The other 3 pinmux settings are valid.
As best I can tell from my schematics, all 4 I2C ports are used on both Seaboard and Springbank.
I don't know about Springbank, but this patch is for Seaboard.
There is nothing on the bus and I can't see anything on the schematic. Can you please tell me what it is used for and which pins you are referring to?
Regards, Simon
-- nvpublic

On 02/03/2012 05:36 PM, Simon Glass wrote:
Hi Stephen,
On Fri, Feb 3, 2012 at 4:25 PM, Stephen Warren swarren@nvidia.com wrote:
On 02/03/2012 05:19 PM, Simon Glass wrote:
Hi Stephen,
On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren swarren@nvidia.com wrote:
On 02/03/2012 04:24 PM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren swarren@nvidia.com wrote:
On 01/12/2012 12:00 PM, 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
This isn't the patch that I ack'd.
Sorry, I added the disable.
> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not tegra2-seaboard.dts to match the naming in the kernel?
> @@ -44,4 +49,9 @@ > usb@c5004000 { > status = "disabled"; > }; > + > + i2c@7000c400 { > + status = "disabled"; > + }; > + > };
That chunk wasn't in the original patch, and doesn't match the kernel's .dts file (and I believe that I2C controller really is in use, so shouldn't be disabled).
It cannot be used - remember the discussion about pinmux? We elected to disable I2C1 at present since you didn't like my nvidia,pinmux binding for selecting which value to pass to funcmux. The fix is to pass 1 instead of 0 for that port, but we have no clean way to specify this.
Rather disable it than leave it enabled and not working.
Rather than having the .dts file not correctly describe the HW, wouldn't it be better to limit U-Boot's to only initializing the 1 I2C controller that it knows the valid pinmux setting for?
Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not used or connected to anything, so it is a reasonable description of the hardware. The other 3 pinmux settings are valid.
As best I can tell from my schematics, all 4 I2C ports are used on both Seaboard and Springbank.
I don't know about Springbank, but this patch is for Seaboard.
There is nothing on the bus and I can't see anything on the schematic. Can you please tell me what it is used for and which pins you are referring to?
GEN2_I2C_SCL/SDA are connected to:
J4 (battery or charger) TPM J6 (touchscreen connector) J26 (mini PCIe) J18 (satellite board)
I assume you have access to the schematics?

Hi Stephen,
On Fri, Feb 3, 2012 at 4:41 PM, Stephen Warren swarren@nvidia.com wrote:
On 02/03/2012 05:36 PM, Simon Glass wrote:
Hi Stephen,
On Fri, Feb 3, 2012 at 4:25 PM, Stephen Warren swarren@nvidia.com wrote:
On 02/03/2012 05:19 PM, Simon Glass wrote:
Hi Stephen,
On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren swarren@nvidia.com wrote:
On 02/03/2012 04:24 PM, Simon Glass wrote:
Hi Stephen,
On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren swarren@nvidia.com wrote: > On 01/12/2012 12:00 PM, 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 > > This isn't the patch that I ack'd.
Sorry, I added the disable.
> >> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts > > Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not > tegra2-seaboard.dts to match the naming in the kernel? > >> @@ -44,4 +49,9 @@ >> usb@c5004000 { >> status = "disabled"; >> }; >> + >> + i2c@7000c400 { >> + status = "disabled"; >> + }; >> + >> }; > > That chunk wasn't in the original patch, and doesn't match the kernel's > .dts file (and I believe that I2C controller really is in use, so > shouldn't be disabled).
It cannot be used - remember the discussion about pinmux? We elected to disable I2C1 at present since you didn't like my nvidia,pinmux binding for selecting which value to pass to funcmux. The fix is to pass 1 instead of 0 for that port, but we have no clean way to specify this.
Rather disable it than leave it enabled and not working.
Rather than having the .dts file not correctly describe the HW, wouldn't it be better to limit U-Boot's to only initializing the 1 I2C controller that it knows the valid pinmux setting for?
Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not used or connected to anything, so it is a reasonable description of the hardware. The other 3 pinmux settings are valid.
As best I can tell from my schematics, all 4 I2C ports are used on both Seaboard and Springbank.
I don't know about Springbank, but this patch is for Seaboard.
There is nothing on the bus and I can't see anything on the schematic. Can you please tell me what it is used for and which pins you are referring to?
GEN2_I2C_SCL/SDA are connected to:
J4 (battery or charger) TPM J6 (touchscreen connector) J26 (mini PCIe) J18 (satellite board)
I assume you have access to the schematics?
OK I see - in U-Boot the TPM will be used even if the others aren't. But the point is that I cannot set the correct pinmux value for that port, since you don't want 'nvidia,pinmux = <1>' in the device tree file. So I think I need to disable it for Seaboard.
Regards, Simon
-- nvpublic

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 6937bcc..1494fc0 100644 --- a/include/configs/seaboard.h +++ b/include/configs/seaboard.h @@ -64,6 +64,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

Hello Simon,
Simon Glass wrote:
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)).
Recent Linux bindings are used, based on example .dts files found in branch for-3.3/dt at:
git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git
(I could not find the actual binding documentation to include here)
Changes in v2:
Hups, seems I missed the v1 ... please add the i2c custodian (me ;-) to cc in further EMails, thanks!
bye, Heiko

Hi Heiko,
On Thu, Jan 12, 2012 at 10:28 PM, Heiko Schocher hs@denx.de wrote:
Hello Simon,
Simon Glass wrote:
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)).
Recent Linux bindings are used, based on example .dts files found in branch for-3.3/dt at:
git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git
(I could not find the actual binding documentation to include here)
Changes in v2:
Hups, seems I missed the v1 ... please add the i2c custodian (me ;-) to cc in further EMails, thanks!
Yes I missed off the i2c tag, will add it next time.
Regards, Simon
bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
participants (4)
-
Heiko Schocher
-
Simon Glass
-
Stephen Warren
-
Yen Lin