hwmon: do not init sensors on startup

PATCH New Heiko Schocher 3 participants 21 messages Dec 9, 2010 – Aug 4, 2011

Subsystems (3)

PowerPC Hardware monitoring LM73 hardware monitor driver

lore

Share & tools

lore

Heiko Schocher

author

2010-12-09 14:00 UTC #0 lore raw

PATCH 1/1 hwmon: do not init sensors on startup

The U-Boot Design Principles[1] clearly say:

Initialize devices only when they are needed within U-Boot, i.e. don't initialize the Ethernet interface(s) unless U-Boot performs a download over Ethernet; don't initialize any IDE or USB devices unless U-Boot actually tries to load files from these, etc. (and don't forget to shut down these devices after using them - otherwise nasty things may happen when you try to boot your OS).

So, do not initialize and read the sensors on startup.

Signed-off-by Heiko Schocher <[email protected]>

---
 arch/powerpc/lib/board.c |    3 ---
 common/cmd_dtt.c         |   14 ++++++++++++--
 drivers/hwmon/adm1021.c  |   27 +++------------------------
 drivers/hwmon/adt7460.c  |    2 +-
 drivers/hwmon/ds1621.c   |   19 +------------------
 drivers/hwmon/ds1775.c   |   19 +------------------
 drivers/hwmon/lm63.c     |   19 +------------------
 drivers/hwmon/lm73.c     |   20 ++------------------
 drivers/hwmon/lm75.c     |   29 ++---------------------------
 drivers/hwmon/lm81.c     |   21 ++-------------------
 include/dtt.h            |    2 +-
 11 files changed, 26 insertions(+), 149 deletions(-)

arch/powerpc/lib/board.c +0 -3

index b21c1d6..f9412a9 100644
@@ -917,9 +917,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 
 	WATCHDOG_RESET ();
 
-#if defined(CONFIG_DTT)		/* Digital Thermometers and Thermostats */
-	dtt_init ();
-#endif
 #if defined(CONFIG_CMD_SCSI)
 	WATCHDOG_RESET ();
 	puts ("SCSI:  ");

common/cmd_dtt.c +12 -2

index 3388e43..37ce4df 100644
@@ -28,6 +28,8 @@
 #include <dtt.h>
 #include <i2c.h>
 
+static unsigned long sensors_init_done = 0;
+
 int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int i;

@@ -42,8 +44,16 @@ int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	 * Loop through sensors, read
 	 * temperature, and output it.
 	 */
-	for (i = 0; i < sizeof (sensors); i++)
-		printf ("DTT%d: %i C\n", i + 1, dtt_get_temp (sensors[i]));
+	for (i = 0; i < sizeof (sensors); i++) {
+		if ((sensors_init_done & (1 << i)) != (1 << i)) {
+			if (dtt_init_one(sensors[i]) == 0)
+				sensors_init_done |= (1 << i);
+			else
+				printf("DTT%d: Failed init!\n", i);
+		}
+		if ((sensors_init_done & (1 << i)) == (1 << i))
+			printf ("DTT%d: %i C\n", i + 1, dtt_get_temp(sensors[i]));
+	}
 
 	/* switch back to original I2C bus */
 	I2C_SET_BUS(old_bus);

drivers/hwmon/adm1021.c +3 ~~-24~~

index d753e9a..3ef80af 100644
@@ -109,8 +109,8 @@ dtt_write (int sensor, int reg, int val)
 	return 0;
 } /* dtt_write() */
 
-static int
-_dtt_init (int sensor)
+int
+dtt_init_one (int sensor)
 {
 	dtt_cfg_t *dcp = &dttcfg[sensor >> 1];
 	int reg, val;

@@ -164,28 +164,7 @@ _dtt_init (int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-int
-dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	/* switch to correct I2C bus */
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf ("%s%d FAILED INIT\n", header, i+1);
-		else
-			printf ("%s%d is %i C\n", header, i+1,
-				dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int
 dtt_get_temp (int sensor)

drivers/hwmon/adt7460.c +1 -1

index caef70a..b7e36fe 100644
@@ -50,7 +50,7 @@ int dtt_write(int sensor, int reg, int val)
 	return 0;
 }
 
-int dtt_init(void)
+int dtt_init_one(int sensor)
 {
 	printf("ADT7460@I2C address 0x%2x\n", ADT7460_ADDRESS);

drivers/hwmon/ds1621.c +1 ~~-18~~

index 5a2ea62..4e1db6d 100644
@@ -126,7 +126,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -155,23 +155,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i + 1);
-		else
-			printf("DTT%d:  %i C\n", i + 1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	int i;

drivers/hwmon/ds1775.c +1 ~~-18~~

index 80fb26f..feabdf2 100644
@@ -98,7 +98,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -133,23 +133,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i+1);
-		else
-			printf("DTT%d:  %i C\n", i+1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	return (dtt_read(sensor, DTT_READ_TEMP) / 256);

drivers/hwmon/lm63.c +1 ~~-18~~

index 03616e1..2e49258 100644
@@ -93,7 +93,7 @@ int dtt_write(int sensor, int reg, int val)
 	return 0;
 }				/* dtt_write() */
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int i;
 	int val;

@@ -155,20 +155,3 @@ int dtt_get_temp(int sensor)
 	/* Ignore LSB for now, U-Boot only prints natural numbers */
 	return temp >> 8;
 }
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-
-	return 0;
-}

drivers/hwmon/lm73.c +2 ~~-18~~

index 7b5d893..45cc6db 100644
@@ -112,7 +112,7 @@ int dtt_write(int const sensor, int const reg, int const val)
 			      dlen);
 } /* dtt_write() */
 
-static int _dtt_init(int const sensor)
+int dtt_init_one(int const sensor)
 {
 	int val;

@@ -148,23 +148,7 @@ static int _dtt_init(int const sensor)
 
 	dtt_read(sensor, DTT_CONTROL);	/* clear temperature flags */
 	return 0;
-} /* _dtt_init() */
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (0 != _dtt_init(sensors[i]))
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-	return 0;
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int const sensor)
 {

drivers/hwmon/lm75.c +2 ~~-27~~

index 8119821..29c1a39 100644
@@ -119,7 +119,7 @@ int dtt_write(int sensor, int reg, int val)
 } /* dtt_write() */
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -145,32 +145,7 @@ static int _dtt_init(int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-	int old_bus;
-
-	/* switch to correct I2C bus */
-	old_bus = I2C_GET_BUS();
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-		printf("%s%d FAILED INIT\n", header, i+1);
-	else
-		printf("%s%d is %i C\n", header, i+1,
-		dtt_get_temp(sensors[i]));
-	}
-	/* switch back to original I2C bus */
-	I2C_SET_BUS(old_bus);
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int sensor)
 {

drivers/hwmon/lm81.c +2 ~~-19~~

index afe5b0d..f1572ba 100644
@@ -89,7 +89,7 @@ int dtt_write(int sensor, int reg, int val)
 #define DTT_CONFIG	0x40
 #define DTT_ADR		0x48
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int	man;
 	int	adr;

@@ -111,26 +111,9 @@ static int _dtt_init(int sensor)
 
 	debug ("DTT:   Found LM81@%x Rev: %d\n", adr, rev);
 	return 0;
-} /* _dtt_init() */
+} /* dtt_init_one() */
 
 
-int dtt_init (void)
-{
-    int i;
-    unsigned char sensors[] = CONFIG_DTT_SENSORS;
-    const char *const header = "DTT:   ";
-
-    for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-	    printf("%s%d FAILED INIT\n", header, i+1);
-	else
-	    printf("%s%d is %i C\n", header, i+1,
-		   dtt_get_temp(sensors[i]));
-    }
-
-    return (0);
-} /* dtt_init() */
-
 #define TEMP_FROM_REG(temp) \
    ((temp)<256?((((temp)&0x1fe) >> 1) * 10)	 + ((temp) & 1) * 5:  \
 	       ((((temp)&0x1fe) >> 1) -255) * 10 - ((temp) & 1) * 5)  \

include/dtt.h +1 -1

index 399b64a..bbaa033 100644
@@ -52,7 +52,7 @@
 #endif
 #endif /* CONFIG_DTT_ADM1021 */
 
-extern int dtt_init (void);
+extern int dtt_init_one (int);
 extern int dtt_read(int sensor, int reg);
 extern int dtt_write(int sensor, int reg, int val);
 extern int dtt_get_temp(int sensor);

Signature · 1 line

1.7.2.3

Wolfgang Denk

2010-12-09 14:26 UTC #1 lore raw

PATCH 1/1 hwmon: do not init sensors on startup

Dear Heiko Schocher,

common/cmd_dtt.c line 31

Quoted 8 lines · In message <[email protected]> you wrote:

common/cmd_dtt.c +1 -0

@@ -28,6 +28,8 @@
 #include <dtt.h>
 #include <i2c.h>
 
+static unsigned long sensors_init_done = 0;

What if there are more then 32 sensors?

Quoted 9 lines

common/cmd_dtt.c +0 -2

 int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int i;

@@ -42,8 +44,16 @@ int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	 * Loop through sensors, read
 	 * temperature, and output it.
 	 */
-	for (i = 0; i < sizeof (sensors); i++)
-		printf ("DTT%d: %i C\n", i + 1, dtt_get_temp (sensors[i]));

common/cmd_dtt.c int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 56

Quoted 10 lines

common/cmd_dtt.c +10 -0

+	for (i = 0; i < sizeof (sensors); i++) {
+		if ((sensors_init_done & (1 << i)) != (1 << i)) {
+			if (dtt_init_one(sensors[i]) == 0)
+				sensors_init_done |= (1 << i);
+			else
+				printf("DTT%d: Failed init!\n", i);
+		}
+		if ((sensors_init_done & (1 << i)) == (1 << i))
+			printf ("DTT%d: %i C\n", i + 1, dtt_get_temp(sensors[i]));
+	}

This is overly complicated, it seems. Why not:

	for (i = 0; i < sizeof(sensors); i++) {
		if ((sensors_init_done & (1 << i)) == 0) {
			if (dtt_init_one(sensors[i]) != 0) {
				printf("DTT%d: init failed\n", i);
				continue;
			}
			sensors_init_done |= (1 << i);
		}

		printf("DTT%d: %i C\n", i + 1, dtt_get_temp(sensors[i]));
	}

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected]
"We don't have to protect the environment -- the Second Coming is  at
hand."                                                   - James Watt

Heiko Schocher

author

2010-12-09 15:45 UTC #2 lore raw

PATCH 1/1 hwmon: do not init sensors on startup

Hello Wolfgang,

Quoted 10 lines · Wolfgang Denk

common/cmd_dtt.c line 31
Quoted 7 lines · In message <[email protected]> you wrote:
common/cmd_dtt.c +1 -0
@@ -28,6 +28,8 @@
 #include <dtt.h>
 #include <i2c.h>
 
+static unsigned long sensors_init_done = 0;
What if there are more then 32 sensors?

Ok, that would not work ... Hmm.. I can get the number of DTTs on a board with ARRAY_SIZE(CONFIG_DTT_SENSORS) ... should I use this info to allocate an array, which stores the info, if the DTTs are initialized?

Quoted 22 lines

Quoted 9 lines

common/cmd_dtt.c +0 -2

 int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int i;

@@ -42,8 +44,16 @@ int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	 * Loop through sensors, read
 	 * temperature, and output it.
 	 */
-	for (i = 0; i < sizeof (sensors); i++)
-		printf ("DTT%d: %i C\n", i + 1, dtt_get_temp (sensors[i]));

common/cmd_dtt.c int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 56

Quoted 10 lines

common/cmd_dtt.c +10 -0

+	for (i = 0; i < sizeof (sensors); i++) {
+		if ((sensors_init_done & (1 << i)) != (1 << i)) {
+			if (dtt_init_one(sensors[i]) == 0)
+				sensors_init_done |= (1 << i);
+			else
+				printf("DTT%d: Failed init!\n", i);
+		}
+		if ((sensors_init_done & (1 << i)) == (1 << i))
+			printf ("DTT%d: %i C\n", i + 1, dtt_get_temp(sensors[i]));
+	}

This is overly complicated, it seems. Why not:

Quoted 11 lines

	for (i = 0; i < sizeof(sensors); i++) {
		if ((sensors_init_done & (1 << i)) == 0) {
			if (dtt_init_one(sensors[i]) != 0) {
				printf("DTT%d: init failed\n", i);
				continue;
			}
			sensors_init_done |= (1 << i);
		}

		printf("DTT%d: %i C\n", i + 1, dtt_get_temp(sensors[i]));
	}

Yep, that looks better, change this in v2.

bye,
Heiko

Signature · 2 lines

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

Wolfgang Denk

2010-12-09 18:50 UTC #3 lore raw

PATCH 1/1 hwmon: do not init sensors on startup

Dear Heiko Schocher,

Quoted 8 lines · In message <[email protected]> you wrote:

Quoted 4 lines
Quoted 2 lines
+static unsigned long sensors_init_done = 0;
What if there are more then 32 sensors?
Ok, that would not work ... Hmm.. I can get the number of DTTs on a board with ARRAY_SIZE(CONFIG_DTT_SENSORS) ... should I use this info to allocate an array, which stores the info, if the DTTs are initialized?

No - it's only a theoretical case. But I think you should add a compile-time test to catch such errors.

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
... bacteriological warfare ... hard to believe we were once foolish
enough to play around with that.
	-- McCoy, "The Omega Glory", stardate unknown

Heiko Schocher

author

2010-12-13 09:26 UTC #4 lore raw

PATCH 1/2 common: introduce a genutils.h file

In reply to Heiko Schocher

as discussed in thread
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/76654

and as a result of this discussion Michael Zaidman proposed to create a genutils.h, see:

http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/78692

So, introduce this genutils.h, as BUILD_BUG_ON is also needed in sensor support.

Signed-off-by Heiko Schocher <[email protected]>

cc Michael Zaidman <[email protected]>

---
maybe I should include genutils.h in common.h ?

 include/genutils.h  |   30 ++++++++++++++++++++++++++++++
 include/ubi_uboot.h |    4 +---
 2 files changed, 31 insertions(+), 3 deletions(-)
 create mode 100644 include/genutils.h

include/genutils.h new file +30 -0

new file mode 100644
index 0000000..102f048
@@ -0,0 +1,30 @@
+/*
+ * (C) Copyright 2010
+ * Heiko Schocher, DENX Software Engineering, hs at denx.de.
+ *
+ * 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
+ */
+
+#ifndef __GENUTILS_H_
+#define __GENUTILS_H_	1
+
+/* Force a compilation error if condition is true */
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+
+#endif	/* __GENUTILS_H_ */

include/ubi_uboot.h +1 -3

index d2c90a6..ad15c8e 100644
@@ -16,6 +16,7 @@
 
 #include <common.h>
 #include <compiler.h>
+#include <genutils.h>
 #include <malloc.h>
 #include <div64.h>
 #include <linux/crc32.h>

@@ -191,9 +192,6 @@ static inline long IS_ERR(const void *ptr)
 	return IS_ERR_VALUE((unsigned long)ptr);
 }
 
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
-
 /* module */
 #define THIS_MODULE		0
 #define try_module_get(...)	1

Signature · 1 line

1.7.2.3

Wolfgang Denk

2011-01-09 16:46 UTC #5 lore raw

PATCH 1/2 common: introduce a genutils.h file

Dear Heiko Schocher,

Quoted 11 lines · In message <[email protected]> you wrote:

as discussed in thread
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/76654
and as a result of this discussion Michael Zaidman proposed to create a genutils.h, see:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/78692
So, introduce this genutils.h, as BUILD_BUG_ON is also needed in sensor support.

Quoted 6 lines

Signed-off-by Heiko Schocher <[email protected]>
cc Michael Zaidman <[email protected]>
---
maybe I should include genutils.h in common.h ?
 include/genutils.h  |   30 ++++++++++++++++++++++++++++++

I do not like the idea of adding yet another nonstandard header file with just a few definitions in it.

Linux defines this is include/linux/kernel.h, i. e. in a pretty central place, and we should porobably do the same.

How about adding it to include/common.h instead?

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The only way to get rid of a temptation is to yield to it.
                                                        - Oscar Wilde

Heiko Schocher

author

2011-01-10 07:09 UTC #6 lore raw

PATCH 1/2 common: introduce a genutils.h file

Hello Wolfgang,

Quoted 19 lines · Wolfgang Denk

Quoted 11 lines · In message <[email protected]> you wrote:
as discussed in thread
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/76654
and as a result of this discussion Michael Zaidman proposed to create a genutils.h, see:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/78692
So, introduce this genutils.h, as BUILD_BUG_ON is also needed in sensor support.
Quoted 6 lines
Signed-off-by Heiko Schocher <[email protected]>
cc Michael Zaidman <[email protected]>
---
maybe I should include genutils.h in common.h ?
 include/genutils.h  |   30 ++++++++++++++++++++++++++++++

Quoted 7 lines

I do not like the idea of adding yet another nonstandard header file with just a few definitions in it.
Linux defines this is include/linux/kernel.h, i. e. in a pretty central place, and we should porobably do the same.
How about adding it to include/common.h instead?

Hmm.. I thought exactly this should not go in common.h as a result from this thread:

http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/78692

bye,
Heiko

Signature · 2 lines

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

Wolfgang Denk

2011-01-10 10:19 UTC #7 lore raw

PATCH 1/2 common: introduce a genutils.h file

Dear Heiko Schocher,

Quoted 10 lines · In message <[email protected]> you wrote:

Quoted 5 lines
Linux defines this is include/linux/kernel.h, i. e. in a pretty central place, and we should porobably do the same.
How about adding it to include/common.h instead?
Hmm.. I thought exactly this should not go in common.h as a result from this thread:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/78692

Well, OK.

Then please let's find a better name than "genutils". Eventually we should create u-boot.h (which then probably would / should include asm/u-boot.h).

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Overdrawn?  But I still have checks left!

Heiko Schocher

author

2011-06-09 09:07 UTC #8 lore raw

PATCH v3 1/2 common: move BUILD_BUG_ON define to common.h

In reply to Heiko Schocher

see discussion also here:
http://patchwork.ozlabs.org/patch/75309/

Signed-off-by Heiko Schocher <[email protected]>

cc Wolfgang Denk <[email protected]>

cc Holger Brunck <[email protected]>

---
- changes for v3
moved this define only to common.h

If we find a good name for a headerfile which is suggested in thread:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/78692

that would be a better place for it. Maybe:
include/gp_helpers.h (general purpose helpers ...)?

 include/common.h    |    3 +++
 include/ubi_uboot.h |    3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

include/common.h +3 -0

index 1e4a6a5..5937bf0 100644
@@ -137,6 +137,9 @@ typedef volatile unsigned char	vu_char;
 #define BUG_ON(condition) do { if (unlikely((condition)!=0)) BUG(); } while(0)
 #endif /* BUG */
 
+/* Force a compilation error if condition is true */
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+
 typedef void (interrupt_handler_t)(void *);
 
 #include <asm/u-boot.h> /* boot information for Linux kernel */

include/ubi_uboot.h +0 -3

index d2c90a6..69006e2 100644
@@ -191,9 +191,6 @@ static inline long IS_ERR(const void *ptr)
 	return IS_ERR_VALUE((unsigned long)ptr);
 }
 
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
-
 /* module */
 #define THIS_MODULE		0
 #define try_module_get(...)	1

Signature · 1 line

1.7.4.4

Wolfgang Denk

Applied

2011-07-27 21:46 UTC #9 lore raw

PATCH v3 1/2 common: move BUILD_BUG_ON define to common.h

Dear Heiko Schocher,

Quoted 15 lines · In message <[email protected]> you wrote:

see discussion also here:
http://patchwork.ozlabs.org/patch/75309/
Signed-off-by Heiko Schocher <[email protected]>
cc Wolfgang Denk <[email protected]>
cc Holger Brunck <[email protected]>
---
- changes for v3
moved this define only to common.h
If we find a good name for a headerfile which is suggested in thread:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/78692

Quoted 7 lines

that would be a better place for it. Maybe:
include/gp_helpers.h (general purpose helpers ...)?
 include/common.h    |    3 +++
 include/ubi_uboot.h |    3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"We don't have to protect the environment -- the Second Coming is  at
hand."                                                   - James Watt

Heiko Schocher

author

2010-12-13 09:26 UTC #10 lore raw

PATCH v2 2/2 hwmon: do not init sensors on startup

In reply to Heiko Schocher

The U-Boot Design Principles[1] clearly say:

So, do not initialize and read the sensors on startup.

Signed-off-by Heiko Schocher <[email protected]>

cc Wolfgang Denk <[email protected]>

---
- changes since v1
add comments from Wolfgang Denk
use BUILD_BUG_ON to create a compileerror if there are defined more than 32 sensors.

 arch/powerpc/lib/board.c |    3 ---
 common/cmd_dtt.c         |   17 +++++++++++++++--
 drivers/hwmon/adm1021.c  |   27 +++------------------------
 drivers/hwmon/adt7460.c  |    2 +-
 drivers/hwmon/ds1621.c   |   19 +------------------
 drivers/hwmon/ds1775.c   |   19 +------------------
 drivers/hwmon/lm63.c     |   19 +------------------
 drivers/hwmon/lm73.c     |   20 ++------------------
 drivers/hwmon/lm75.c     |   29 ++---------------------------
 drivers/hwmon/lm81.c     |   21 ++-------------------
 include/dtt.h            |    2 +-
 11 files changed, 29 insertions(+), 149 deletions(-)

arch/powerpc/lib/board.c +0 -3

index b21c1d6..f9412a9 100644
@@ -917,9 +917,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 
 	WATCHDOG_RESET ();
 
-#if defined(CONFIG_DTT)		/* Digital Thermometers and Thermostats */
-	dtt_init ();
-#endif
 #if defined(CONFIG_CMD_SCSI)
 	WATCHDOG_RESET ();
 	puts ("SCSI:  ");

common/cmd_dtt.c +15 -2

index 3388e43..d86cec5 100644
@@ -24,16 +24,21 @@
 #include <common.h>
 #include <config.h>
 #include <command.h>
+#include <genutils.h>
 
 #include <dtt.h>
 #include <i2c.h>
 
+static unsigned long sensor_initialized = 0;
+
 int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int i;
 	unsigned char sensors[] = CONFIG_DTT_SENSORS;
 	int old_bus;
 
+	/* Force a compilation error, if there are more then 32 sensors */
+	BUILD_BUG_ON(sizeof(sensors) > 32);
 	/* switch to correct I2C bus */
 	old_bus = I2C_GET_BUS();
 	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);

@@ -42,8 +47,16 @@ int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	 * Loop through sensors, read
 	 * temperature, and output it.
 	 */
-	for (i = 0; i < sizeof (sensors); i++)
-		printf ("DTT%d: %i C\n", i + 1, dtt_get_temp (sensors[i]));
+	for (i = 0; i < sizeof (sensors); i++) {
+		if ((sensor_initialized & (1 << i)) == 0) {
+			if (dtt_init_one(sensors[i]) != 0) {
+				printf("DTT%d: Failed init!\n", i);
+				continue;
+			}
+			sensor_initialized |= 1 << i;
+		}
+		printf ("DTT%d: %i C\n", i + 1, dtt_get_temp(sensors[i]));
+	}
 
 	/* switch back to original I2C bus */
 	I2C_SET_BUS(old_bus);

drivers/hwmon/adm1021.c +3 ~~-24~~

index d753e9a..3ef80af 100644
@@ -109,8 +109,8 @@ dtt_write (int sensor, int reg, int val)
 	return 0;
 } /* dtt_write() */
 
-static int
-_dtt_init (int sensor)
+int
+dtt_init_one (int sensor)
 {
 	dtt_cfg_t *dcp = &dttcfg[sensor >> 1];
 	int reg, val;

@@ -164,28 +164,7 @@ _dtt_init (int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-int
-dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	/* switch to correct I2C bus */
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf ("%s%d FAILED INIT\n", header, i+1);
-		else
-			printf ("%s%d is %i C\n", header, i+1,
-				dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int
 dtt_get_temp (int sensor)

drivers/hwmon/adt7460.c +1 -1

index caef70a..b7e36fe 100644
@@ -50,7 +50,7 @@ int dtt_write(int sensor, int reg, int val)
 	return 0;
 }
 
-int dtt_init(void)
+int dtt_init_one(int sensor)
 {
 	printf("ADT7460@I2C address 0x%2x\n", ADT7460_ADDRESS);

drivers/hwmon/ds1621.c +1 ~~-18~~

index 5a2ea62..4e1db6d 100644
@@ -126,7 +126,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -155,23 +155,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i + 1);
-		else
-			printf("DTT%d:  %i C\n", i + 1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	int i;

drivers/hwmon/ds1775.c +1 ~~-18~~

index 80fb26f..feabdf2 100644
@@ -98,7 +98,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -133,23 +133,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i+1);
-		else
-			printf("DTT%d:  %i C\n", i+1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	return (dtt_read(sensor, DTT_READ_TEMP) / 256);

drivers/hwmon/lm63.c +1 ~~-18~~

index 03616e1..2e49258 100644
@@ -93,7 +93,7 @@ int dtt_write(int sensor, int reg, int val)
 	return 0;
 }				/* dtt_write() */
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int i;
 	int val;

@@ -155,20 +155,3 @@ int dtt_get_temp(int sensor)
 	/* Ignore LSB for now, U-Boot only prints natural numbers */
 	return temp >> 8;
 }
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-
-	return 0;
-}

drivers/hwmon/lm73.c +2 ~~-18~~

index 7b5d893..45cc6db 100644
@@ -112,7 +112,7 @@ int dtt_write(int const sensor, int const reg, int const val)
 			      dlen);
 } /* dtt_write() */
 
-static int _dtt_init(int const sensor)
+int dtt_init_one(int const sensor)
 {
 	int val;

@@ -148,23 +148,7 @@ static int _dtt_init(int const sensor)
 
 	dtt_read(sensor, DTT_CONTROL);	/* clear temperature flags */
 	return 0;
-} /* _dtt_init() */
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (0 != _dtt_init(sensors[i]))
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-	return 0;
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int const sensor)
 {

drivers/hwmon/lm75.c +2 ~~-27~~

index 8119821..29c1a39 100644
@@ -119,7 +119,7 @@ int dtt_write(int sensor, int reg, int val)
 } /* dtt_write() */
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -145,32 +145,7 @@ static int _dtt_init(int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-	int old_bus;
-
-	/* switch to correct I2C bus */
-	old_bus = I2C_GET_BUS();
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-		printf("%s%d FAILED INIT\n", header, i+1);
-	else
-		printf("%s%d is %i C\n", header, i+1,
-		dtt_get_temp(sensors[i]));
-	}
-	/* switch back to original I2C bus */
-	I2C_SET_BUS(old_bus);
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int sensor)
 {

drivers/hwmon/lm81.c +2 ~~-19~~

index afe5b0d..f1572ba 100644
@@ -89,7 +89,7 @@ int dtt_write(int sensor, int reg, int val)
 #define DTT_CONFIG	0x40
 #define DTT_ADR		0x48
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int	man;
 	int	adr;

@@ -111,26 +111,9 @@ static int _dtt_init(int sensor)
 
 	debug ("DTT:   Found LM81@%x Rev: %d\n", adr, rev);
 	return 0;
-} /* _dtt_init() */
+} /* dtt_init_one() */
 
 
-int dtt_init (void)
-{
-    int i;
-    unsigned char sensors[] = CONFIG_DTT_SENSORS;
-    const char *const header = "DTT:   ";
-
-    for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-	    printf("%s%d FAILED INIT\n", header, i+1);
-	else
-	    printf("%s%d is %i C\n", header, i+1,
-		   dtt_get_temp(sensors[i]));
-    }
-
-    return (0);
-} /* dtt_init() */
-
 #define TEMP_FROM_REG(temp) \
    ((temp)<256?((((temp)&0x1fe) >> 1) * 10)	 + ((temp) & 1) * 5:  \
 	       ((((temp)&0x1fe) >> 1) -255) * 10 - ((temp) & 1) * 5)  \

include/dtt.h +1 -1

index 399b64a..bbaa033 100644
@@ -52,7 +52,7 @@
 #endif
 #endif /* CONFIG_DTT_ADM1021 */
 
-extern int dtt_init (void);
+extern int dtt_init_one (int);
 extern int dtt_read(int sensor, int reg);
 extern int dtt_write(int sensor, int reg, int val);
 extern int dtt_get_temp(int sensor);

Signature · 1 line

1.7.2.3

Wolfgang Denk

2011-01-09 16:49 UTC #11 lore raw

PATCH v2 2/2 hwmon: do not init sensors on startup

Dear Heiko Schocher,

Quoted 11 lines · In message <[email protected]> you wrote:

The U-Boot Design Principles[1] clearly say:
Initialize devices only when they are needed within U-Boot, i.e. don't initialize the Ethernet interface(s) unless U-Boot performs a download over Ethernet; don't initialize any IDE or USB devices unless U-Boot actually tries to load files from these, etc. (and don't forget to shut down these devices after using them - otherwise nasty things may happen when you try to boot your OS).
So, do not initialize and read the sensors on startup.

Quoted 8 lines

Signed-off-by Heiko Schocher <[email protected]>
cc Wolfgang Denk <[email protected]>
---
- changes since v1
add comments from Wolfgang Denk
use BUILD_BUG_ON to create a compileerror if there are defined more than 32 sensors.

Please resubmit with the BUILD_BUG_ON patch. Thanks.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I paid too much for it, but its worth it.

Heiko Schocher

author

2011-06-09 09:07 UTC #12 lore raw

PATCH v3 2/2 hwmon: do not init sensors on startup

In reply to Heiko Schocher

The U-Boot Design Principles[1] clearly say:

So, do not initialize and read the sensors on startup.

Signed-off-by Heiko Schocher <[email protected]>

cc Wolfgang Denk <[email protected]>

cc Holger Brunck <[email protected]>

---
- changes since v1
add comments from Wolfgang Denk
use BUILD_BUG_ON to create a compileerror if there are defined more than 32 sensors.

- changes since v2
don;t include genutils.h, as macro is now in common.h

 arch/powerpc/lib/board.c |    3 ---
 common/cmd_dtt.c         |   16 ++++++++++++++--
 drivers/hwmon/adm1021.c  |   27 +++------------------------
 drivers/hwmon/adt7460.c  |    2 +-
 drivers/hwmon/ds1621.c   |   19 +------------------
 drivers/hwmon/ds1775.c   |   19 +------------------
 drivers/hwmon/lm63.c     |   19 +------------------
 drivers/hwmon/lm73.c     |   20 ++------------------
 drivers/hwmon/lm75.c     |   29 ++---------------------------
 drivers/hwmon/lm81.c     |   21 ++-------------------
 include/dtt.h            |    2 +-
 11 files changed, 28 insertions(+), 149 deletions(-)

arch/powerpc/lib/board.c +0 -3

index aaa5add..e84cd52 100644
@@ -946,9 +946,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 
 	WATCHDOG_RESET ();
 
-#if defined(CONFIG_DTT)		/* Digital Thermometers and Thermostats */
-	dtt_init ();
-#endif
 #if defined(CONFIG_CMD_SCSI)
 	WATCHDOG_RESET ();
 	puts ("SCSI:  ");

common/cmd_dtt.c +14 -2

index 3388e43..5e60650 100644
@@ -28,12 +28,16 @@
 #include <dtt.h>
 #include <i2c.h>
 
+static unsigned long sensor_initialized = 0;
+
 int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int i;
 	unsigned char sensors[] = CONFIG_DTT_SENSORS;
 	int old_bus;
 
+	/* Force a compilation error, if there are more then 32 sensors */
+	BUILD_BUG_ON(sizeof(sensors) > 32);
 	/* switch to correct I2C bus */
 	old_bus = I2C_GET_BUS();
 	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);

@@ -42,8 +46,16 @@ int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	 * Loop through sensors, read
 	 * temperature, and output it.
 	 */
-	for (i = 0; i < sizeof (sensors); i++)
-		printf ("DTT%d: %i C\n", i + 1, dtt_get_temp (sensors[i]));
+	for (i = 0; i < sizeof(sensors); i++) {
+		if ((sensor_initialized & (1 << i)) == 0) {
+			if (dtt_init_one(sensors[i]) != 0) {
+				printf("DTT%d: Failed init!\n", i);
+				continue;
+			}
+			sensor_initialized |= 1 << i;
+		}
+		printf("DTT%d: %i C\n", i + 1, dtt_get_temp(sensors[i]));
+	}
 
 	/* switch back to original I2C bus */
 	I2C_SET_BUS(old_bus);

drivers/hwmon/adm1021.c +3 ~~-24~~

index d753e9a..3ef80af 100644
@@ -109,8 +109,8 @@ dtt_write (int sensor, int reg, int val)
 	return 0;
 } /* dtt_write() */
 
-static int
-_dtt_init (int sensor)
+int
+dtt_init_one(int sensor)
 {
 	dtt_cfg_t *dcp = &dttcfg[sensor >> 1];
 	int reg, val;

@@ -164,28 +164,7 @@ _dtt_init (int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-int
-dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	/* switch to correct I2C bus */
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf ("%s%d FAILED INIT\n", header, i+1);
-		else
-			printf ("%s%d is %i C\n", header, i+1,
-				dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int
 dtt_get_temp (int sensor)

drivers/hwmon/adt7460.c +1 -1

index caef70a..b7e36fe 100644
@@ -50,7 +50,7 @@ int dtt_write(int sensor, int reg, int val)
 	return 0;
 }
 
-int dtt_init(void)
+int dtt_init_one(int sensor)
 {
 	printf("ADT7460@I2C address 0x%2x\n", ADT7460_ADDRESS);

drivers/hwmon/ds1621.c +1 ~~-18~~

index 5a2ea62..4e1db6d 100644
@@ -126,7 +126,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -155,23 +155,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i + 1);
-		else
-			printf("DTT%d:  %i C\n", i + 1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	int i;

drivers/hwmon/ds1775.c +1 ~~-18~~

index 80fb26f..feabdf2 100644
@@ -98,7 +98,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -133,23 +133,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i+1);
-		else
-			printf("DTT%d:  %i C\n", i+1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	return (dtt_read(sensor, DTT_READ_TEMP) / 256);

drivers/hwmon/lm63.c +1 ~~-18~~

index 2f1f3cf..f3adf64 100644
@@ -101,7 +101,7 @@ static int is_lm64(int sensor)
 	return sensor && (sensor != DTT_I2C_LM63_ADDR);
 }
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int i;
 	int val;

@@ -175,20 +175,3 @@ int dtt_get_temp(int sensor)
 	/* Ignore LSB for now, U-Boot only prints natural numbers */
 	return temp >> 8;
 }
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-
-	return 0;
-}

drivers/hwmon/lm73.c +2 ~~-18~~

index 7b5d893..45cc6db 100644
@@ -112,7 +112,7 @@ int dtt_write(int const sensor, int const reg, int const val)
 			      dlen);
 } /* dtt_write() */
 
-static int _dtt_init(int const sensor)
+int dtt_init_one(int const sensor)
 {
 	int val;

@@ -148,23 +148,7 @@ static int _dtt_init(int const sensor)
 
 	dtt_read(sensor, DTT_CONTROL);	/* clear temperature flags */
 	return 0;
-} /* _dtt_init() */
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (0 != _dtt_init(sensors[i]))
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-	return 0;
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int const sensor)
 {

drivers/hwmon/lm75.c +2 ~~-27~~

index 8119821..29c1a39 100644
@@ -119,7 +119,7 @@ int dtt_write(int sensor, int reg, int val)
 } /* dtt_write() */
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -145,32 +145,7 @@ static int _dtt_init(int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-	int old_bus;
-
-	/* switch to correct I2C bus */
-	old_bus = I2C_GET_BUS();
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-		printf("%s%d FAILED INIT\n", header, i+1);
-	else
-		printf("%s%d is %i C\n", header, i+1,
-		dtt_get_temp(sensors[i]));
-	}
-	/* switch back to original I2C bus */
-	I2C_SET_BUS(old_bus);
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int sensor)
 {

drivers/hwmon/lm81.c +2 ~~-19~~

index afe5b0d..f1572ba 100644
@@ -89,7 +89,7 @@ int dtt_write(int sensor, int reg, int val)
 #define DTT_CONFIG	0x40
 #define DTT_ADR		0x48
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int	man;
 	int	adr;

@@ -111,26 +111,9 @@ static int _dtt_init(int sensor)
 
 	debug ("DTT:   Found LM81@%x Rev: %d\n", adr, rev);
 	return 0;
-} /* _dtt_init() */
+} /* dtt_init_one() */
 
 
-int dtt_init (void)
-{
-    int i;
-    unsigned char sensors[] = CONFIG_DTT_SENSORS;
-    const char *const header = "DTT:   ";
-
-    for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-	    printf("%s%d FAILED INIT\n", header, i+1);
-	else
-	    printf("%s%d is %i C\n", header, i+1,
-		   dtt_get_temp(sensors[i]));
-    }
-
-    return (0);
-} /* dtt_init() */
-
 #define TEMP_FROM_REG(temp) \
    ((temp)<256?((((temp)&0x1fe) >> 1) * 10)	 + ((temp) & 1) * 5:  \
 	       ((((temp)&0x1fe) >> 1) -255) * 10 - ((temp) & 1) * 5)  \

include/dtt.h +1 -1

index 399b64a..bbaa033 100644
@@ -52,7 +52,7 @@
 #endif
 #endif /* CONFIG_DTT_ADM1021 */
 
-extern int dtt_init (void);
+extern int dtt_init_one(int);
 extern int dtt_read(int sensor, int reg);
 extern int dtt_write(int sensor, int reg, int val);
 extern int dtt_get_temp(int sensor);

Signature · 1 line

1.7.4.4

Holger Brunck

2011-08-01 07:23 UTC #13 lore raw

PATCH v3 2/2 hwmon: do not init sensors on startup

Hi,

Quoted 9 lines · Heiko Schocher

The U-Boot Design Principles[1] clearly say:
Initialize devices only when they are needed within U-Boot, i.e. don't initialize the Ethernet interface(s) unless U-Boot performs a download over Ethernet; don't initialize any IDE or USB devices unless U-Boot actually tries to load files from these, etc. (and don't forget to shut down these devices after using them - otherwise nasty things may happen when you try to boot your OS).

Quoted 7 lines

So, do not initialize and read the sensors on startup.
Signed-off-by Heiko Schocher <[email protected]>
cc Wolfgang Denk <[email protected]>
cc Holger Brunck <[email protected]>
---

what's the status of this patch? Is it intended to go in for rc1? I have seen that 1/2 was committed but not this one. This patch runs in our local branch for some month so:

Tested-by Holger Brunck <[email protected]>

Regards
Holger

Wolfgang Denk

2011-08-01 10:13 UTC #14 lore raw

PATCH v3 2/2 hwmon: do not init sensors on startup

Dear Holger Brunck,

Quoted 4 lines · In message <[email protected]> you wrote:

what's the status of this patch? Is it intended to go in for rc1? I have seen that 1/2 was committed but not this one. This patch runs in our local branch for some month so:

I lost it from my queue. But it needs to be fixed (not checkpatch clean).

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Wish not to seem, but to be, the best."                  - Aeschylus

Wolfgang Denk

2011-08-01 10:12 UTC #15 lore raw

PATCH v3 2/2 hwmon: do not init sensors on startup

In reply to Heiko Schocher

Dear Heiko Schocher,

Quoted 9 lines · In message <[email protected]> you wrote:

The U-Boot Design Principles[1] clearly say:
Initialize devices only when they are needed within U-Boot, i.e. don't initialize the Ethernet interface(s) unless U-Boot performs a download over Ethernet; don't initialize any IDE or USB devices unless U-Boot actually tries to load files from these, etc. (and don't forget to shut down these devices after using them - otherwise nasty things may happen when you try to boot your OS).

Quoted 5 lines

So, do not initialize and read the sensors on startup.
Signed-off-by Heiko Schocher <[email protected]>
cc Wolfgang Denk <[email protected]>
cc Holger Brunck <[email protected]>

Checkpatch says:

ERROR: do not initialise statics to 0 or NULL
#111: FILE: common/cmd_dtt.c:31:
+static unsigned long sensor_initialized = 0;

Please fix and resubmit.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A door is what a dog is perpetually on the wrong side of.
                                                        - Ogden Nash

Heiko Schocher

author

2011-08-01 11:08 UTC #16 lore raw

PATCH v4 2/2 hwmon: do not init sensors on startup

In reply to Heiko Schocher

The U-Boot Design Principles[1] clearly say:

So, do not initialize and read the sensors on startup.

Signed-off-by Heiko Schocher <[email protected]>

cc Wolfgang Denk <[email protected]>

cc Holger Brunck <[email protected]>

---
- changes since v1
add comments from Wolfgang Denk
use BUILD_BUG_ON to create a compileerror if there are defined more than 32 sensors.

- changes since v2
don;t include genutils.h, as macro is now in common.h

- changes since v3
do not initialize static sensor_initialized with 0

 arch/powerpc/lib/board.c |    3 ---
 common/cmd_dtt.c         |   16 ++++++++++++++--
 drivers/hwmon/adm1021.c  |   27 +++------------------------
 drivers/hwmon/adt7460.c  |    2 +-
 drivers/hwmon/ds1621.c   |   19 +------------------
 drivers/hwmon/ds1775.c   |   19 +------------------
 drivers/hwmon/lm63.c     |   19 +------------------
 drivers/hwmon/lm73.c     |   20 ++------------------
 drivers/hwmon/lm75.c     |   29 ++---------------------------
 drivers/hwmon/lm81.c     |   21 ++-------------------
 include/dtt.h            |    2 +-
 11 files changed, 28 insertions(+), 149 deletions(-)

arch/powerpc/lib/board.c +0 -3

index aaa5add..e84cd52 100644
@@ -946,9 +946,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 
 	WATCHDOG_RESET ();
 
-#if defined(CONFIG_DTT)		/* Digital Thermometers and Thermostats */
-	dtt_init ();
-#endif
 #if defined(CONFIG_CMD_SCSI)
 	WATCHDOG_RESET ();
 	puts ("SCSI:  ");

common/cmd_dtt.c +14 -2

index 3388e43..31aa420 100644
@@ -28,12 +28,16 @@
 #include <dtt.h>
 #include <i2c.h>
 
+static unsigned long sensor_initialized = 0xffffffff;
+
 int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int i;
 	unsigned char sensors[] = CONFIG_DTT_SENSORS;
 	int old_bus;
 
+	/* Force a compilation error, if there are more then 32 sensors */
+	BUILD_BUG_ON(sizeof(sensors) > 32);
 	/* switch to correct I2C bus */
 	old_bus = I2C_GET_BUS();
 	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);

@@ -42,8 +46,16 @@ int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	 * Loop through sensors, read
 	 * temperature, and output it.
 	 */
-	for (i = 0; i < sizeof (sensors); i++)
-		printf ("DTT%d: %i C\n", i + 1, dtt_get_temp (sensors[i]));
+	for (i = 0; i < sizeof(sensors); i++) {
+		if ((sensor_initialized & (1 << i)) != 0) {
+			if (dtt_init_one(sensors[i]) != 0) {
+				printf("DTT%d: Failed init!\n", i);
+				continue;
+			}
+			sensor_initialized &= ~(1 << i);
+		}
+		printf("DTT%d: %i C\n", i + 1, dtt_get_temp(sensors[i]));
+	}
 
 	/* switch back to original I2C bus */
 	I2C_SET_BUS(old_bus);

drivers/hwmon/adm1021.c +3 ~~-24~~

index d753e9a..d074cb7 100644
@@ -109,8 +109,8 @@ dtt_write (int sensor, int reg, int val)
 	return 0;
 } /* dtt_write() */
 
-static int
-_dtt_init (int sensor)
+int
+dtt_init_one(int sensor)
 {
 	dtt_cfg_t *dcp = &dttcfg[sensor >> 1];
 	int reg, val;

@@ -164,28 +164,7 @@ _dtt_init (int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-int
-dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	/* switch to correct I2C bus */
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf ("%s%d FAILED INIT\n", header, i+1);
-		else
-			printf ("%s%d is %i C\n", header, i+1,
-				dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int
 dtt_get_temp (int sensor)

drivers/hwmon/adt7460.c +1 -1

index caef70a..b7e36fe 100644
@@ -50,7 +50,7 @@ int dtt_write(int sensor, int reg, int val)
 	return 0;
 }
 
-int dtt_init(void)
+int dtt_init_one(int sensor)
 {
 	printf("ADT7460@I2C address 0x%2x\n", ADT7460_ADDRESS);

drivers/hwmon/ds1621.c +1 ~~-18~~

index 5a2ea62..4e1db6d 100644
@@ -126,7 +126,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -155,23 +155,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i + 1);
-		else
-			printf("DTT%d:  %i C\n", i + 1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	int i;

drivers/hwmon/ds1775.c +1 ~~-18~~

index 80fb26f..feabdf2 100644
@@ -98,7 +98,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -133,23 +133,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i+1);
-		else
-			printf("DTT%d:  %i C\n", i+1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	return (dtt_read(sensor, DTT_READ_TEMP) / 256);

drivers/hwmon/lm63.c +1 ~~-18~~

index 2f1f3cf..f3adf64 100644
@@ -101,7 +101,7 @@ static int is_lm64(int sensor)
 	return sensor && (sensor != DTT_I2C_LM63_ADDR);
 }
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int i;
 	int val;

@@ -175,20 +175,3 @@ int dtt_get_temp(int sensor)
 	/* Ignore LSB for now, U-Boot only prints natural numbers */
 	return temp >> 8;
 }
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-
-	return 0;
-}

drivers/hwmon/lm73.c +2 ~~-18~~

index 7b5d893..45cc6db 100644
@@ -112,7 +112,7 @@ int dtt_write(int const sensor, int const reg, int const val)
 			      dlen);
 } /* dtt_write() */
 
-static int _dtt_init(int const sensor)
+int dtt_init_one(int const sensor)
 {
 	int val;

@@ -148,23 +148,7 @@ static int _dtt_init(int const sensor)
 
 	dtt_read(sensor, DTT_CONTROL);	/* clear temperature flags */
 	return 0;
-} /* _dtt_init() */
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (0 != _dtt_init(sensors[i]))
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-	return 0;
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int const sensor)
 {

drivers/hwmon/lm75.c +2 ~~-27~~

index 8119821..29c1a39 100644
@@ -119,7 +119,7 @@ int dtt_write(int sensor, int reg, int val)
 } /* dtt_write() */
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -145,32 +145,7 @@ static int _dtt_init(int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-	int old_bus;
-
-	/* switch to correct I2C bus */
-	old_bus = I2C_GET_BUS();
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-		printf("%s%d FAILED INIT\n", header, i+1);
-	else
-		printf("%s%d is %i C\n", header, i+1,
-		dtt_get_temp(sensors[i]));
-	}
-	/* switch back to original I2C bus */
-	I2C_SET_BUS(old_bus);
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int sensor)
 {

drivers/hwmon/lm81.c +2 ~~-19~~

index afe5b0d..f1572ba 100644
@@ -89,7 +89,7 @@ int dtt_write(int sensor, int reg, int val)
 #define DTT_CONFIG	0x40
 #define DTT_ADR		0x48
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int	man;
 	int	adr;

@@ -111,26 +111,9 @@ static int _dtt_init(int sensor)
 
 	debug ("DTT:   Found LM81@%x Rev: %d\n", adr, rev);
 	return 0;
-} /* _dtt_init() */
+} /* dtt_init_one() */
 
 
-int dtt_init (void)
-{
-    int i;
-    unsigned char sensors[] = CONFIG_DTT_SENSORS;
-    const char *const header = "DTT:   ";
-
-    for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-	    printf("%s%d FAILED INIT\n", header, i+1);
-	else
-	    printf("%s%d is %i C\n", header, i+1,
-		   dtt_get_temp(sensors[i]));
-    }
-
-    return (0);
-} /* dtt_init() */
-
 #define TEMP_FROM_REG(temp) \
    ((temp)<256?((((temp)&0x1fe) >> 1) * 10)	 + ((temp) & 1) * 5:  \
 	       ((((temp)&0x1fe) >> 1) -255) * 10 - ((temp) & 1) * 5)  \

include/dtt.h +1 -1

index 399b64a..9e6c08c 100644
@@ -52,7 +52,7 @@
 #endif
 #endif /* CONFIG_DTT_ADM1021 */
 
-extern int dtt_init (void);
+extern int dtt_init_one(int);
 extern int dtt_read(int sensor, int reg);
 extern int dtt_write(int sensor, int reg, int val);
 extern int dtt_get_temp(int sensor);

Signature · 1 line

1.7.6

Wolfgang Denk

2011-08-01 13:28 UTC #17 lore raw

PATCH v4 2/2 hwmon: do not init sensors on startup

Dear Heiko Schocher,

Quoted 10 lines · In message <[email protected]> you wrote:

The U-Boot Design Principles[1] clearly say:
Initialize devices only when they are needed within U-Boot, i.e. don't initialize the Ethernet interface(s) unless U-Boot performs a download over Ethernet; don't initialize any IDE or USB devices unless U-Boot actually tries to load files from these, etc. (and don't forget to shut down these devices after using them - otherwise nasty things may happen when you try to boot your OS).
So, do not initialize and read the sensors on startup.

...

Quoted 2 lines

- changes since v3
do not initialize static sensor_initialized with 0

...

Quoted 1 line

+static unsigned long sensor_initialized = 0xffffffff;

Ummm.... no. You failed to understand what the checkpatch error means.

There is no need to explicitly static variables with 0, because this is their default value if you leave them unitialized - unintialized data is allocated in the BSS segment, and thus get automatically initialized as 0.

The code above enforces allocation ion the data segment, i. e. it increses the (flash) memory footprint.

Please undo this. Just omit the "= 0" par tin the declaration all together.

Thanks.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The more sins you confess, the more books you will sell.

Heiko Schocher

author

2011-08-01 13:54 UTC #18 lore raw

PATCH v4 2/2 hwmon: do not init sensors on startup

Hello Wolfgang,

Quoted 19 lines · Wolfgang Denk

Dear Heiko Schocher,
Quoted 10 lines · In message <[email protected]> you wrote:
The U-Boot Design Principles[1] clearly say:
Initialize devices only when they are needed within U-Boot, i.e. don't initialize the Ethernet interface(s) unless U-Boot performs a download over Ethernet; don't initialize any IDE or USB devices unless U-Boot actually tries to load files from these, etc. (and don't forget to shut down these devices after using them - otherwise nasty things may happen when you try to boot your OS).
So, do not initialize and read the sensors on startup.
...
Quoted 2 lines
- changes since v3
do not initialize static sensor_initialized with 0
...
Quoted 1 line
+static unsigned long sensor_initialized = 0xffffffff;

Quoted 7 lines

Ummm.... no. You failed to understand what the checkpatch error means.
There is no need to explicitly static variables with 0, because this is their default value if you leave them unitialized - unintialized data is allocated in the BSS segment, and thus get automatically initialized as 0.

Ah! Ok, I undo this change.

Quoted 5 lines

The code above enforces allocation ion the data segment, i. e. it increses the (flash) memory footprint.
Please undo this. Just omit the "= 0" par tin the declaration all together.

bye,
Heiko

Signature · 2 lines

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

Heiko Schocher

author

2011-08-01 14:01 UTC #19 lore raw

PATCH v5 2/2 hwmon: do not init sensors on startup

In reply to Heiko Schocher

The U-Boot Design Principles[1] clearly say:

So, do not initialize and read the sensors on startup.

Signed-off-by Heiko Schocher <[email protected]>

cc Wolfgang Denk <[email protected]>

cc Holger Brunck <[email protected]>

---
- changes since v1
add comments from Wolfgang Denk
use BUILD_BUG_ON to create a compileerror if there are defined more than 32 sensors.

- changes since v2
don;t include genutils.h, as macro is now in common.h

- changes since v3
do not initialize static sensor_initialized with 0

- changes since v4
revert change for v4. Just drop the initialization of sensor_initialized with 0.

 arch/powerpc/lib/board.c |    3 ---
 common/cmd_dtt.c         |   16 ++++++++++++++--
 drivers/hwmon/adm1021.c  |   27 +++------------------------
 drivers/hwmon/adt7460.c  |    2 +-
 drivers/hwmon/ds1621.c   |   19 +------------------
 drivers/hwmon/ds1775.c   |   19 +------------------
 drivers/hwmon/lm63.c     |   19 +------------------
 drivers/hwmon/lm73.c     |   20 ++------------------
 drivers/hwmon/lm75.c     |   29 ++---------------------------
 drivers/hwmon/lm81.c     |   21 ++-------------------
 include/dtt.h            |    2 +-
 11 files changed, 28 insertions(+), 149 deletions(-)

arch/powerpc/lib/board.c +0 -3

index aaa5add..e84cd52 100644
@@ -946,9 +946,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 
 	WATCHDOG_RESET ();
 
-#if defined(CONFIG_DTT)		/* Digital Thermometers and Thermostats */
-	dtt_init ();
-#endif
 #if defined(CONFIG_CMD_SCSI)
 	WATCHDOG_RESET ();
 	puts ("SCSI:  ");

common/cmd_dtt.c +14 -2

index 3388e43..5bba12d 100644
@@ -28,12 +28,16 @@
 #include <dtt.h>
 #include <i2c.h>
 
+static unsigned long sensor_initialized;
+
 int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int i;
 	unsigned char sensors[] = CONFIG_DTT_SENSORS;
 	int old_bus;
 
+	/* Force a compilation error, if there are more then 32 sensors */
+	BUILD_BUG_ON(sizeof(sensors) > 32);
 	/* switch to correct I2C bus */
 	old_bus = I2C_GET_BUS();
 	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);

@@ -42,8 +46,16 @@ int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	 * Loop through sensors, read
 	 * temperature, and output it.
 	 */
-	for (i = 0; i < sizeof (sensors); i++)
-		printf ("DTT%d: %i C\n", i + 1, dtt_get_temp (sensors[i]));
+	for (i = 0; i < sizeof(sensors); i++) {
+		if ((sensor_initialized & (1 << i)) == 0) {
+			if (dtt_init_one(sensors[i]) != 0) {
+				printf("DTT%d: Failed init!\n", i);
+				continue;
+			}
+			sensor_initialized |= (1 << i);
+		}
+		printf("DTT%d: %i C\n", i + 1, dtt_get_temp(sensors[i]));
+	}
 
 	/* switch back to original I2C bus */
 	I2C_SET_BUS(old_bus);

drivers/hwmon/adm1021.c +3 ~~-24~~

index d753e9a..d074cb7 100644
@@ -109,8 +109,8 @@ dtt_write (int sensor, int reg, int val)
 	return 0;
 } /* dtt_write() */
 
-static int
-_dtt_init (int sensor)
+int
+dtt_init_one(int sensor)
 {
 	dtt_cfg_t *dcp = &dttcfg[sensor >> 1];
 	int reg, val;

@@ -164,28 +164,7 @@ _dtt_init (int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-int
-dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	/* switch to correct I2C bus */
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf ("%s%d FAILED INIT\n", header, i+1);
-		else
-			printf ("%s%d is %i C\n", header, i+1,
-				dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int
 dtt_get_temp (int sensor)

drivers/hwmon/adt7460.c +1 -1

index caef70a..b7e36fe 100644
@@ -50,7 +50,7 @@ int dtt_write(int sensor, int reg, int val)
 	return 0;
 }
 
-int dtt_init(void)
+int dtt_init_one(int sensor)
 {
 	printf("ADT7460@I2C address 0x%2x\n", ADT7460_ADDRESS);

drivers/hwmon/ds1621.c +1 ~~-18~~

index 5a2ea62..4e1db6d 100644
@@ -126,7 +126,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -155,23 +155,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i + 1);
-		else
-			printf("DTT%d:  %i C\n", i + 1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	int i;

drivers/hwmon/ds1775.c +1 ~~-18~~

index 80fb26f..feabdf2 100644
@@ -98,7 +98,7 @@ int dtt_write(int sensor, int reg, int val)
 }
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -133,23 +133,6 @@ static int _dtt_init(int sensor)
 	return 0;
 }
 
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("DTT%d:  FAILED\n", i+1);
-		else
-			printf("DTT%d:  %i C\n", i+1, dtt_get_temp(sensors[i]));
-	}
-
-	return (0);
-}
-
-
 int dtt_get_temp(int sensor)
 {
 	return (dtt_read(sensor, DTT_READ_TEMP) / 256);

drivers/hwmon/lm63.c +1 ~~-18~~

index 2f1f3cf..f3adf64 100644
@@ -101,7 +101,7 @@ static int is_lm64(int sensor)
 	return sensor && (sensor != DTT_I2C_LM63_ADDR);
 }
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int i;
 	int val;

@@ -175,20 +175,3 @@ int dtt_get_temp(int sensor)
 	/* Ignore LSB for now, U-Boot only prints natural numbers */
 	return temp >> 8;
 }
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (_dtt_init(sensors[i]) != 0)
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-
-	return 0;
-}

drivers/hwmon/lm73.c +2 ~~-18~~

index 7b5d893..45cc6db 100644
@@ -112,7 +112,7 @@ int dtt_write(int const sensor, int const reg, int const val)
 			      dlen);
 } /* dtt_write() */
 
-static int _dtt_init(int const sensor)
+int dtt_init_one(int const sensor)
 {
 	int val;

@@ -148,23 +148,7 @@ static int _dtt_init(int const sensor)
 
 	dtt_read(sensor, DTT_CONTROL);	/* clear temperature flags */
 	return 0;
-} /* _dtt_init() */
-
-int dtt_init(void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-
-	for (i = 0; i < sizeof(sensors); i++) {
-		if (0 != _dtt_init(sensors[i]))
-			printf("%s%d FAILED INIT\n", header, i + 1);
-		else
-			printf("%s%d is %i C\n", header, i + 1,
-			       dtt_get_temp(sensors[i]));
-	}
-	return 0;
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int const sensor)
 {

drivers/hwmon/lm75.c +2 ~~-27~~

index 8119821..29c1a39 100644
@@ -119,7 +119,7 @@ int dtt_write(int sensor, int reg, int val)
 } /* dtt_write() */
 
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int val;

@@ -145,32 +145,7 @@ static int _dtt_init(int sensor)
 		return 1;
 
 	return 0;
-} /* _dtt_init() */
-
-
-int dtt_init (void)
-{
-	int i;
-	unsigned char sensors[] = CONFIG_DTT_SENSORS;
-	const char *const header = "DTT:   ";
-	int old_bus;
-
-	/* switch to correct I2C bus */
-	old_bus = I2C_GET_BUS();
-	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
-
-	for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-		printf("%s%d FAILED INIT\n", header, i+1);
-	else
-		printf("%s%d is %i C\n", header, i+1,
-		dtt_get_temp(sensors[i]));
-	}
-	/* switch back to original I2C bus */
-	I2C_SET_BUS(old_bus);
-
-	return (0);
-} /* dtt_init() */
+} /* dtt_init_one() */
 
 int dtt_get_temp(int sensor)
 {

drivers/hwmon/lm81.c +2 ~~-19~~

index afe5b0d..f1572ba 100644
@@ -89,7 +89,7 @@ int dtt_write(int sensor, int reg, int val)
 #define DTT_CONFIG	0x40
 #define DTT_ADR		0x48
 
-static int _dtt_init(int sensor)
+int dtt_init_one(int sensor)
 {
 	int	man;
 	int	adr;

@@ -111,26 +111,9 @@ static int _dtt_init(int sensor)
 
 	debug ("DTT:   Found LM81@%x Rev: %d\n", adr, rev);
 	return 0;
-} /* _dtt_init() */
+} /* dtt_init_one() */
 
 
-int dtt_init (void)
-{
-    int i;
-    unsigned char sensors[] = CONFIG_DTT_SENSORS;
-    const char *const header = "DTT:   ";
-
-    for (i = 0; i < sizeof(sensors); i++) {
-	if (_dtt_init(sensors[i]) != 0)
-	    printf("%s%d FAILED INIT\n", header, i+1);
-	else
-	    printf("%s%d is %i C\n", header, i+1,
-		   dtt_get_temp(sensors[i]));
-    }
-
-    return (0);
-} /* dtt_init() */
-
 #define TEMP_FROM_REG(temp) \
    ((temp)<256?((((temp)&0x1fe) >> 1) * 10)	 + ((temp) & 1) * 5:  \
 	       ((((temp)&0x1fe) >> 1) -255) * 10 - ((temp) & 1) * 5)  \

include/dtt.h +1 -1

index 399b64a..9e6c08c 100644
@@ -52,7 +52,7 @@
 #endif
 #endif /* CONFIG_DTT_ADM1021 */
 
-extern int dtt_init (void);
+extern int dtt_init_one(int);
 extern int dtt_read(int sensor, int reg);
 extern int dtt_write(int sensor, int reg, int val);
 extern int dtt_get_temp(int sensor);

Signature · 1 line

1.7.6

Wolfgang Denk

Applied

2011-08-04 21:31 UTC #20 lore raw

PATCH v5 2/2 hwmon: do not init sensors on startup

Dear Heiko Schocher,

Quoted 31 lines · In message <[email protected]> you wrote:

The U-Boot Design Principles[1] clearly say:
Initialize devices only when they are needed within U-Boot, i.e. don't initialize the Ethernet interface(s) unless U-Boot performs a download over Ethernet; don't initialize any IDE or USB devices unless U-Boot actually tries to load files from these, etc. (and don't forget to shut down these devices after using them - otherwise nasty things may happen when you try to boot your OS).
So, do not initialize and read the sensors on startup.
Signed-off-by Heiko Schocher <[email protected]>
cc Wolfgang Denk <[email protected]>
cc Holger Brunck <[email protected]>
---
- changes since v1
add comments from Wolfgang Denk
use BUILD_BUG_ON to create a compileerror if there are defined more than 32 sensors.
- changes since v2
don;t include genutils.h, as macro is now in common.h
- changes since v3
do not initialize static sensor_initialized with 0
- changes since v4
revert change for v4. Just drop the initialization of sensor_initialized with 0.

Quoted 12 lines

 arch/powerpc/lib/board.c |    3 ---
 common/cmd_dtt.c         |   16 ++++++++++++++--
 drivers/hwmon/adm1021.c  |   27 +++------------------------
 drivers/hwmon/adt7460.c  |    2 +-
 drivers/hwmon/ds1621.c   |   19 +------------------
 drivers/hwmon/ds1775.c   |   19 +------------------
 drivers/hwmon/lm63.c     |   19 +------------------
 drivers/hwmon/lm73.c     |   20 ++------------------
 drivers/hwmon/lm75.c     |   29 ++---------------------------
 drivers/hwmon/lm81.c     |   21 ++-------------------
 include/dtt.h            |    2 +-
 11 files changed, 28 insertions(+), 149 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The most difficult thing in the world is to know how to  do  a  thing
and to watch someone else doing it wrong, without commenting.
                                                        -- T.H. White