[U-Boot] [PATCH][NEXT] 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 hs@denx.de --- 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(-)
diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index b21c1d6..f9412a9 100644 --- a/arch/powerpc/lib/board.c +++ b/arch/powerpc/lib/board.c @@ -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: "); diff --git a/common/cmd_dtt.c b/common/cmd_dtt.c index 3388e43..37ce4df 100644 --- a/common/cmd_dtt.c +++ b/common/cmd_dtt.c @@ -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); 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 03616e1..2e49258 100644 --- a/drivers/hwmon/lm63.c +++ b/drivers/hwmon/lm63.c @@ -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; -} 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);
Dear Heiko Schocher,
In message 1291903238-29071-1-git-send-email-hs@denx.de you wrote:
--- a/common/cmd_dtt.c +++ b/common/cmd_dtt.c @@ -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?
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);elseprintf("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
Hello Wolfgang,
Wolfgang Denk wrote:
In message 1291903238-29071-1-git-send-email-hs@denx.de you wrote:
--- a/common/cmd_dtt.c +++ b/common/cmd_dtt.c @@ -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?
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);elseprintf("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]));}
Yep, that looks better, change this in v2.
bye, Heiko
Dear Heiko Schocher,
In message 4D00F98B.1010507@denx.de you wrote:
+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
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 hs@denx.de cc: Michael Zaidman michael.zaidman@gmail.com --- 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
diff --git a/include/genutils.h b/include/genutils.h new file mode 100644 index 0000000..102f048 --- /dev/null +++ b/include/genutils.h @@ -0,0 +1,30 @@ +/* + * (C) Copyright 2010 + * Heiko Schocher, DENX Software Engineering, hs@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_ */ diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h index d2c90a6..ad15c8e 100644 --- a/include/ubi_uboot.h +++ b/include/ubi_uboot.h @@ -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
Dear Heiko Schocher,
In message 1292232393-26481-1-git-send-email-hs@denx.de 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.
Signed-off-by: Heiko Schocher hs@denx.de cc: Michael Zaidman michael.zaidman@gmail.com
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
Hello Wolfgang,
Wolfgang Denk wrote:
In message 1292232393-26481-1-git-send-email-hs@denx.de 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.
Signed-off-by: Heiko Schocher hs@denx.de cc: Michael Zaidman michael.zaidman@gmail.com
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?
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
Dear Heiko Schocher,
In message 4D2AB091.9030405@denx.de you wrote:
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
see discussion also here: http://patchwork.ozlabs.org/patch/75309/
Signed-off-by: Heiko Schocher hs@denx.de cc: Wolfgang Denk wd@denx.de cc: Holger Brunck holger.brunck@keymile.com
--- - 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(-)
diff --git a/include/common.h b/include/common.h index 1e4a6a5..5937bf0 100644 --- a/include/common.h +++ b/include/common.h @@ -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 */ diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h index d2c90a6..69006e2 100644 --- a/include/ubi_uboot.h +++ b/include/ubi_uboot.h @@ -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
Dear Heiko Schocher,
In message 1307610425-10481-1-git-send-email-hs@denx.de you wrote:
see discussion also here: http://patchwork.ozlabs.org/patch/75309/
Signed-off-by: Heiko Schocher hs@denx.de cc: Wolfgang Denk wd@denx.de cc: Holger Brunck holger.brunck@keymile.com
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(-)
Applied, thanks.
Best regards,
Wolfgang Denk
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
--- - 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(-)
diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index b21c1d6..f9412a9 100644 --- a/arch/powerpc/lib/board.c +++ b/arch/powerpc/lib/board.c @@ -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: "); diff --git a/common/cmd_dtt.c b/common/cmd_dtt.c index 3388e43..d86cec5 100644 --- a/common/cmd_dtt.c +++ b/common/cmd_dtt.c @@ -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); 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 03616e1..2e49258 100644 --- a/drivers/hwmon/lm63.c +++ b/drivers/hwmon/lm63.c @@ -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; -} 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);
Dear Heiko Schocher,
In message 1292232401-26523-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
- 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
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(-)
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);
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
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
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
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
- 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(-)
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..31aa420 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 = 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); diff --git a/drivers/hwmon/adm1021.c b/drivers/hwmon/adm1021.c index d753e9a..d074cb7 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..9e6c08c 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);
Dear Heiko Schocher,
In message 1312196898-20228-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.
...
- changes since v3 do not initialize static sensor_initialized with 0
...
+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
Hello Wolfgang,
Wolfgang Denk wrote:
Dear Heiko Schocher,
In message 1312196898-20228-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.
...
- changes since v3 do not initialize static sensor_initialized with 0
...
+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.
Ah! Ok, I undo this change.
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
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
- 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(-)
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..5bba12d 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; + 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..d074cb7 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..9e6c08c 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);
Dear Heiko Schocher,
In message 1312207303-25153-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
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(-)
Applied, thanks.
Best regards,
Wolfgang Denk
participants (3)
-
Heiko Schocher -
Holger Brunck -
Wolfgang Denk