diff mbox

[U-Boot,v3,2/2] hwmon: do not init sensors on startup

Message ID 1307610451-13457-1-git-send-email-hs@denx.de
State Superseded
Headers show

Commit Message

Heiko Schocher June 9, 2011, 9:07 a.m. UTC 
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 <hs@denx.de>
cc: Wolfgang Denk <wd@denx.de>
cc: Holger Brunck <holger.brunck@keymile.com>

---
- 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(-)

Comments

Holger Brunck Aug. 1, 2011, 7:23 a.m. UTC | #1 
Hi,

On 06/09/2011 11:07 AM, Heiko Schocher 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 <hs@denx.de>
> cc: Wolfgang Denk <wd@denx.de>
> cc: Holger Brunck <holger.brunck@keymile.com>
> 
> ---

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 <holger.brunck@keymile.com>

Regards
Holger
Wolfgang Denk Aug. 1, 2011, 10:12 a.m. UTC | #2 
Dear Heiko Schocher,

In message <1307610451-13457-1-git-send-email-hs@denx.de> 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 <hs@denx.de>
> cc: Wolfgang Denk <wd@denx.de>
> cc: Holger Brunck <holger.brunck@keymile.com>

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
Wolfgang Denk Aug. 1, 2011, 10:13 a.m. UTC | #3 
Dear Holger Brunck,

In message <4E365478.4050100@keymile.com> 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
diff mbox

Patch

diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c
index aaa5add..e84cd52 100644
--- a/arch/powerpc/lib/board.c
+++ b/arch/powerpc/lib/board.c
@@ -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:  ");
diff --git a/common/cmd_dtt.c b/common/cmd_dtt.c
index 3388e43..5e60650 100644
--- a/common/cmd_dtt.c
+++ b/common/cmd_dtt.c
@@ -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);
diff --git a/drivers/hwmon/adm1021.c b/drivers/hwmon/adm1021.c
index d753e9a..3ef80af 100644
--- a/drivers/hwmon/adm1021.c
+++ b/drivers/hwmon/adm1021.c
@@ -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)
diff --git a/drivers/hwmon/adt7460.c b/drivers/hwmon/adt7460.c
index caef70a..b7e36fe 100644
--- a/drivers/hwmon/adt7460.c
+++ b/drivers/hwmon/adt7460.c
@@ -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 at I2C address 0x%2x\n", ADT7460_ADDRESS);
 
diff --git a/drivers/hwmon/ds1621.c b/drivers/hwmon/ds1621.c
index 5a2ea62..4e1db6d 100644
--- a/drivers/hwmon/ds1621.c
+++ b/drivers/hwmon/ds1621.c
@@ -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;
diff --git a/drivers/hwmon/ds1775.c b/drivers/hwmon/ds1775.c
index 80fb26f..feabdf2 100644
--- a/drivers/hwmon/ds1775.c
+++ b/drivers/hwmon/ds1775.c
@@ -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);
diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c
index 2f1f3cf..f3adf64 100644
--- a/drivers/hwmon/lm63.c
+++ b/drivers/hwmon/lm63.c
@@ -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;
-}
diff --git a/drivers/hwmon/lm73.c b/drivers/hwmon/lm73.c
index 7b5d893..45cc6db 100644
--- a/drivers/hwmon/lm73.c
+++ b/drivers/hwmon/lm73.c
@@ -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)
 {
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 8119821..29c1a39 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -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)
 {
diff --git a/drivers/hwmon/lm81.c b/drivers/hwmon/lm81.c
index afe5b0d..f1572ba 100644
--- a/drivers/hwmon/lm81.c
+++ b/drivers/hwmon/lm81.c
@@ -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)  \
diff --git a/include/dtt.h b/include/dtt.h
index 399b64a..bbaa033 100644
--- a/include/dtt.h
+++ b/include/dtt.h
@@ -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);