[U-Boot] [PATCH 03/12] 83xx, kmeter1: add I2C, dtt, eeprom support

[U-Boot] [PATCH 03/12] 83xx, kmeter1: add I2C, dtt, eeprom support

[Heiko Schocher]

This patch adds I2C support for the Keymile kmeter1 board.
It uses the First I2C Controller from the CPU, for
accessing 4 temperature sensors, an eeprom with IVM data
and the booteeprom over a pca9547 mux.

Signed-off-by: Heiko Schocher <hs@denx.de>
---
 board/keymile/common/common.c   |    2 +-
 board/keymile/kmeter1/kmeter1.c |   32 ++++++++++++++++++++++++++++++++
 drivers/hwmon/lm75.c            |   14 +++++++++++++-
 include/configs/kmeter1.h       |   22 ++++++++++++++++++++++
 4 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
index 1338950..48ce613 100644
--- a/board/keymile/common/common.c
+++ b/board/keymile/common/common.c
@@ -318,7 +318,7 @@ int ivm_read_eeprom (void)
 	if (buf != NULL)
 		dev_addr = simple_strtoul ((char *)buf, NULL, 16);

-	if (eeprom_read (dev_addr, 0, i2c_buffer, CONFIG_SYS_IVM_EEPROM_MAX_LEN) != 0) {
+	if (i2c_read(dev_addr, 0, 1, i2c_buffer, CONFIG_SYS_IVM_EEPROM_MAX_LEN) != 0) {
 		printf ("Error reading EEprom\n");
 		return -2;
 	}
diff --git a/board/keymile/kmeter1/kmeter1.c b/board/keymile/kmeter1/kmeter1.c
index f04a57a..c2df432 100644
--- a/board/keymile/kmeter1/kmeter1.c
+++ b/board/keymile/kmeter1/kmeter1.c
@@ -59,6 +59,23 @@ const qe_iop_conf_t qe_iop_conf_tab[] = {
 	{0,  0, 0, 0, QE_IOP_TAB_END},
 };

+static int board_init_i2c_busses (void)
+{
+	I2C_MUX_DEVICE *dev = NULL;
+	uchar	*buf;
+
+	/* Set up the Bus for the DTTs */
+	buf = (unsigned char *) getenv ("dtt_bus");
+	if (buf != NULL)
+		dev = i2c_mux_ident_muxstring (buf);
+	if (dev == NULL) {
+		printf ("Error couldn't add Bus for DTT\n");
+		printf ("please setup dtt_bus to where your\n");
+		printf ("DTT is found.\n");
+	}
+	return 0;
+}
+
 int board_early_init_r (void)
 {
 	void *reg = (void *)(CONFIG_SYS_IMMR + 0x14a8);
@@ -80,6 +97,12 @@ int board_early_init_r (void)
 	return 0;
 }

+int misc_init_r (void)
+{
+	board_init_i2c_busses ();
+	return 0;
+}
+
 int fixed_sdram(void)
 {
 	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
@@ -156,3 +179,12 @@ void ft_board_setup (void *blob, bd_t *bd)
 	ft_cpu_setup (blob, bd);
 }
 #endif
+
+#if defined(CONFIG_HUSH_INIT_VAR)
+extern int ivm_read_eeprom (void);
+int hush_init_var (void)
+{
+	ivm_read_eeprom ();
+	return 0;
+}
+#endif
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 8119821..ecfc345 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -42,6 +42,8 @@
 #define DTT_TEMP_HYST		0x2
 #define DTT_TEMP_SET		0x3

+DECLARE_GLOBAL_DATA_PTR;
+
 int dtt_read(int sensor, int reg)
 {
 	int dlen;
@@ -157,7 +159,17 @@ int dtt_init (void)

 	/* switch to correct I2C bus */
 	old_bus = I2C_GET_BUS();
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
+	if ((gd->flags & GD_FLG_RELOC) != GD_FLG_RELOC) {
+		uchar *tmp = (uchar *)getenv("dtt_bus");
+		if (tmp == NULL)
+			printf("FAILED: dtt_bus not defined\n");
+		else {
+			if (i2c_mux_ident_muxstring_f(tmp))
+				printf("FAILED: couldnt switch to bus\n");
+		}
+	} else {
+		I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
+	}

 	for (i = 0; i < sizeof(sensors); i++) {
 	if (_dtt_init(sensors[i]) != 0)
diff --git a/include/configs/kmeter1.h b/include/configs/kmeter1.h
index 25b1c17..0980eec 100644
--- a/include/configs/kmeter1.h
+++ b/include/configs/kmeter1.h
@@ -32,6 +32,7 @@
 /* include common defines/options for all Keymile boards */
 #include "keymile-common.h"

+#define CONFIG_MISC_INIT_R	1
 /*
  * System Clock Setup
  */
@@ -300,6 +301,25 @@
 #define CONFIG_ENV_SIZE		0x2000
 #endif /* CFG_RAMBOOT */

+/* I2C */
+#define CONFIG_HARD_I2C		/* I2C with hardware support */
+#undef	CONFIG_SOFT_I2C		/* I2C bit-banged */
+#define CONFIG_FSL_I2C
+#define CONFIG_SYS_I2C_SPEED	200000	/* I2C speed and slave address */
+#define CONFIG_SYS_I2C_SLAVE	0x7F
+#define CONFIG_SYS_I2C_OFFSET	0x3000
+
+/* I2C SYSMON (LM75, AD7414 is almost compatible)			*/
+#define CONFIG_DTT_LM75		1	/* ON Semi's LM75		*/
+#define CONFIG_DTT_SENSORS	{0, 1, 2, 3}	/* Sensor addresses		*/
+#define CONFIG_SYS_DTT_MAX_TEMP	70
+#define CONFIG_SYS_DTT_LOW_TEMP	-30
+#define CONFIG_SYS_DTT_HYSTERESIS	3
+#define CONFIG_SYS_DTT_BUS_NUM		(2)
+#define CONFIG_SYS_DTT_BOARD_INIT	1
+
+#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	2
+
 #if defined(CONFIG_PCI)
 #define CONFIG_CMD_PCI
 #endif
@@ -452,6 +472,8 @@
 	"loadfdt=tftp ${fdt_addr_r} ${fdt_file}\0"			\
 	"loadkernel=tftp ${kernel_addr_r} ${boot_file}\0"		\
 	"unlock=yes\0"							\
+	"EEprom_ivm=pca9547:70:9\0"					\
+	"dtt_bus=pca9547:70:a\0"					\
    ""

 #endif /* __CONFIG_H */
-- 
1.6.0.6

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

[U-Boot] [PATCH 03/12] 83xx, kmeter1: add I2C, dtt, eeprom support

[Kim Phillips]

On Wed, 11 Feb 2009 19:25:49 +0100
Heiko Schocher <hs@denx.de> wrote:

> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 8119821..ecfc345 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -42,6 +42,8 @@
>  #define DTT_TEMP_HYST		0x2
>  #define DTT_TEMP_SET		0x3
> 
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  int dtt_read(int sensor, int reg)
>  {
>  	int dlen;
> @@ -157,7 +159,17 @@ int dtt_init (void)
> 
>  	/* switch to correct I2C bus */
>  	old_bus = I2C_GET_BUS();
> -	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
> +	if ((gd->flags & GD_FLG_RELOC) != GD_FLG_RELOC) {
> +		uchar *tmp = (uchar *)getenv("dtt_bus");

sorry Heiko, I should have asked you before: this is a hardcoded
environment variable in driver code, does the dtt bus value change
between boots?  If not, what's the reason the value isn't specified as
a CONFIG_SYS_DTT_BUS_NUM?

> +		if (tmp == NULL)
> +			printf("FAILED: dtt_bus not defined\n");
> +		else {
> +			if (i2c_mux_ident_muxstring_f(tmp))
> +				printf("FAILED: couldnt switch to bus\n");
> +		}

btw, these braces unnecessary.

Kim

[U-Boot] [PATCH 03/12] 83xx, kmeter1: add I2C, dtt, eeprom support

[Heiko Schocher]

Hello Kim,

Kim Phillips wrote:
> On Wed, 11 Feb 2009 19:25:49 +0100
> Heiko Schocher <hs@denx.de> wrote:
> 
>> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>> index 8119821..ecfc345 100644
>> --- a/drivers/hwmon/lm75.c
>> +++ b/drivers/hwmon/lm75.c
>> @@ -42,6 +42,8 @@
>>  #define DTT_TEMP_HYST		0x2
>>  #define DTT_TEMP_SET		0x3
>>
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>>  int dtt_read(int sensor, int reg)
>>  {
>>  	int dlen;
>> @@ -157,7 +159,17 @@ int dtt_init (void)
>>
>>  	/* switch to correct I2C bus */
>>  	old_bus = I2C_GET_BUS();
>> -	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
>> +	if ((gd->flags & GD_FLG_RELOC) != GD_FLG_RELOC) {
>> +		uchar *tmp = (uchar *)getenv("dtt_bus");
> 
> sorry Heiko, I should have asked you before: this is a hardcoded
> environment variable in driver code, does the dtt bus value change
> between boots?  If not, what's the reason the value isn't specified as
> a CONFIG_SYS_DTT_BUS_NUM?

It changes not between boots, but between different hardware.

>> +		if (tmp == NULL)
>> +			printf("FAILED: dtt_bus not defined\n");
>> +		else {
>> +			if (i2c_mux_ident_muxstring_f(tmp))
>> +				printf("FAILED: couldnt switch to bus\n");
>> +		}
> 
> btw, these braces unnecessary.

Yep, I change this.

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

[U-Boot] [PATCH 03/12] 83xx, kmeter1: add I2C, dtt, eeprom support

[Kim Phillips]

On Tue, 17 Feb 2009 07:03:23 +0100
Heiko Schocher <hs@denx.de> wrote:

> Hello Kim,

Hello Heiko,

> Kim Phillips wrote:
> > On Wed, 11 Feb 2009 19:25:49 +0100
> > Heiko Schocher <hs@denx.de> wrote:
> >>  	/* switch to correct I2C bus */
> >>  	old_bus = I2C_GET_BUS();
> >> -	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
> >> +	if ((gd->flags & GD_FLG_RELOC) != GD_FLG_RELOC) {
> >> +		uchar *tmp = (uchar *)getenv("dtt_bus");
> > 
> > sorry Heiko, I should have asked you before: this is a hardcoded
> > environment variable in driver code, does the dtt bus value change
> > between boots?  If not, what's the reason the value isn't specified as
> > a CONFIG_SYS_DTT_BUS_NUM?
> 
> It changes not between boots, but between different hardware.

so why not make it a CONFIG_SYS_DTT_BUS_NUM instead of permanently
polluting environment namespace.  That's what the rest of the boards
that use this DTT code do; what makes the kmeter1 special in this
regard?  Because CONFIG_SYS_DTT_BUS_NUM is not just a single number?
In that case we can make CONFIG_SYS_DTT_BUS_REF or something (not sure
if that's a good enough name).

Kim

[U-Boot] [PATCH 03/12] 83xx, kmeter1: add I2C, dtt, eeprom support

[Heiko Schocher]

Hello Kim

Kim Phillips wrote:
> On Tue, 17 Feb 2009 07:03:23 +0100
> Heiko Schocher <hs@denx.de> wrote:
> 
>> Hello Kim,
> 
> Hello Heiko,
> 
>> Kim Phillips wrote:
>>> On Wed, 11 Feb 2009 19:25:49 +0100
>>> Heiko Schocher <hs@denx.de> wrote:
>>>>  	/* switch to correct I2C bus */
>>>>  	old_bus = I2C_GET_BUS();
>>>> -	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
>>>> +	if ((gd->flags & GD_FLG_RELOC) != GD_FLG_RELOC) {
>>>> +		uchar *tmp = (uchar *)getenv("dtt_bus");
>>> sorry Heiko, I should have asked you before: this is a hardcoded
>>> environment variable in driver code, does the dtt bus value change
>>> between boots?  If not, what's the reason the value isn't specified as
>>> a CONFIG_SYS_DTT_BUS_NUM?
>> It changes not between boots, but between different hardware.
> 
> so why not make it a CONFIG_SYS_DTT_BUS_NUM instead of permanently
> polluting environment namespace.  That's what the rest of the boards
> that use this DTT code do; what makes the kmeter1 special in this
> regard?  Because CONFIG_SYS_DTT_BUS_NUM is not just a single number?

There are several hardwareversions, which only differ in the way, how
the DTTs are reached (0,1,2 muxes and they use also different channels
...). So we save this "way" in an environment variable. When running
in RAM, it is just a number see else part ...

> In that case we can make CONFIG_SYS_DTT_BUS_REF or something (not sure
> if that's a good enough name).

What exactly do you mean with that?

But I was wondering why the DTTs are initialized so early. Is this
necessary? If not, I don;t need this ...

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

[U-Boot] [PATCH 03/12] 83xx, kmeter1: add I2C, dtt, eeprom support

[Kim Phillips]

On Wed, 18 Feb 2009 09:14:05 +0100
Heiko Schocher <hs@denx.de> wrote:

> >>> between boots?  If not, what's the reason the value isn't specified as
> >>> a CONFIG_SYS_DTT_BUS_NUM?
> >> It changes not between boots, but between different hardware.
> > 
> > so why not make it a CONFIG_SYS_DTT_BUS_NUM instead of permanently
> > polluting environment namespace.  That's what the rest of the boards
> > that use this DTT code do; what makes the kmeter1 special in this
> > regard?  Because CONFIG_SYS_DTT_BUS_NUM is not just a single number?
> 
> There are several hardwareversions, which only differ in the way, how
> the DTTs are reached (0,1,2 muxes and they use also different channels
> ...). So we save this "way" in an environment variable. When running
> in RAM, it is just a number see else part ...
> 
> > In that case we can make CONFIG_SYS_DTT_BUS_REF or something (not sure
> > if that's a good enough name).
> 
> What exactly do you mean with that?

you're assigning the env var dtt_bus "pca9547:70:a" whereas other
boards assign CONFIG_SYS_DTT_BUS_NUM a single integer value.

> But I was wondering why the DTTs are initialized so early. Is this
> necessary? If not, I don;t need this ...

I searched and didn't find out why.  If there's no reason then maybe we
don't need it indeed.

Meanwhile, I've applied the 'v2' versions of your patches and still
have trouble building the kmeter1 board*, probably due to inter-patch
dependencies in this patchseries.  This means at minimum you're breaking
git-bisection abilities for your boards.  Can you resubmit without
breaking bisection?

Also, did you want any of these to go through the mpc83xx
tree, or should WD pick all of them up?  Please make two separate
patchseries if you're targeting two trees.

Kim

* currently giving up at:

Configuring for kmeter1 board...
fsl_i2c.c:45: warning: 'i2c_bus_num_mux' defined but not used
lm75.c: In function 'dtt_init':
lm75.c:167: warning: implicit declaration of function 'i2c_mux_ident_muxstring_f'
../common/common.c: In function 'ivm_check_crc':
../common/common.c:202: error: 'CONFIG_SYS_IVM_EEPROM_PAGE_LEN' undeclared (first use in this function)
../common/common.c:202: error: (Each undeclared identifier is reported only once
../common/common.c:202: error: for each function it appears in.)
../common/common.c: In function 'ivm_analyze_block2':
../common/common.c:215: error: 'CONFIG_SYS_IVM_EEPROM_PAGE_LEN' undeclared (first use in this function)
../common/common.c:215: warning: unused variable 'valbuf'
../common/common.c: In function 'ivm_analyze_eeprom':
../common/common.c:244: error: 'CONFIG_SYS_IVM_EEPROM_PAGE_LEN' undeclared (first use in this function)
../common/common.c:244: warning: unused variable 'valbuf'
../common/common.c: In function 'ivm_read_eeprom':
../common/common.c:298: error: 'I2C_MUX_DEVICE' undeclared (first use in this function)
../common/common.c:298: error: 'dev' undeclared (first use in this function)
../common/common.c:299: error: 'CONFIG_SYS_IVM_EEPROM_MAX_LEN' undeclared (first use in this function)
../common/common.c:301: error: 'CONFIG_SYS_IVM_EEPROM_ADR' undeclared (first use in this function)
../common/common.c:309: warning: implicit declaration of function 'i2c_mux_ident_muxstring'
../common/common.c:299: warning: unused variable 'i2c_buffer'
make[1]: *** [../common/common.o] Error 1
make[1]: *** Waiting for unfinished jobs....
kmeter1.c: In function 'board_init_i2c_busses':
kmeter1.c:67: error: 'I2C_MUX_DEVICE' undeclared (first use in this function)
kmeter1.c:67: error: (Each undeclared identifier is reported only once
kmeter1.c:67: error: for each function it appears in.)
kmeter1.c:67: error: 'dev' undeclared (first use in this function)
kmeter1.c:73: warning: implicit declaration of function 'i2c_mux_ident_muxstring'
make[1]: *** [kmeter1.o] Error 1
make: *** [board/keymile/kmeter1/libkmeter1.a] Error 2
powerpc-linux-gnu-size: './u-boot': No such file

[U-Boot] [PATCH 03/12] 83xx, kmeter1: add I2C, dtt, eeprom support

[Heiko Schocher]

Hello Kim,

Kim Phillips wrote:
> On Wed, 18 Feb 2009 09:14:05 +0100
> Heiko Schocher <hs@denx.de> wrote:
[...]
>> But I was wondering why the DTTs are initialized so early. Is this
>> necessary? If not, I don;t need this ...
> 
> I searched and didn't find out why.  If there's no reason then maybe we
> don't need it indeed.

I think so too.

> Meanwhile, I've applied the 'v2' versions of your patches and still
> have trouble building the kmeter1 board*, probably due to inter-patch
> dependencies in this patchseries.  This means at minimum you're breaking
> git-bisection abilities for your boards.  Can you resubmit without
> breaking bisection?

:-( Sorry for that. I am actually learning that I have to look
for such a point too. I always compiled with all off my patches...

This happend, so I think, because the kmeter shares a lot of common
options with other boards from this manufacturer ... and I did a
rework from that common options in patch 10/12 ...

I try to make a new series, without breaking bisection ...

> Also, did you want any of these to go through the mpc83xx
> tree, or should WD pick all of them up?  Please make two separate
> patchseries if you're targeting two trees.

OK, thanks

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