[U-Boot] [PATCH] board/BuR/kwb: fix usage of 'i2c_set_bus_speed'
- fix: return-value of 'i2c_set_bus_speed' was interpreted wrong
Signed-off-by: Hannes Petermaier oe5hpm@oevsv.at --- board/BuR/kwb/board.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/BuR/kwb/board.c b/board/BuR/kwb/board.c index 8aa16bc..8fb5e68 100644 --- a/board/BuR/kwb/board.c +++ b/board/BuR/kwb/board.c @@ -120,7 +120,7 @@ void am33xx_spl_board_init(void)
/* power-ON 3V3 via Resetcontroller */ oldspeed = i2c_get_bus_speed(); - if (0 != i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) { + if (0 <= i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) { buf = RSTCTRL_FORCE_PWR_NEN; i2c_write(RSTCTRL_ADDR, RSTCTRL_CTRLREG, 1, (uint8_t *)&buf, sizeof(buf)); @@ -221,7 +221,7 @@ int board_late_init(void) TPS65217_WLEDCTRL1, 0x09, 0xFF); /* write bootinfo into scratchregister of resetcontroller */ oldspeed = i2c_get_bus_speed(); - if (0 != i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) { + if (0 <= i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) { i2c_write(RSTCTRL_ADDR, RSTCTRL_SCRATCHREG, 1, (uint8_t *)&buf, sizeof(buf)); i2c_set_bus_speed(oldspeed);
On Fri, Mar 07, 2014 at 18:56 +0100, Hannes Petermaier wrote:
- fix: return-value of 'i2c_set_bus_speed' was interpreted wrong
Signed-off-by: Hannes Petermaier oe5hpm@oevsv.at
board/BuR/kwb/board.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/BuR/kwb/board.c b/board/BuR/kwb/board.c index 8aa16bc..8fb5e68 100644 --- a/board/BuR/kwb/board.c +++ b/board/BuR/kwb/board.c @@ -120,7 +120,7 @@ void am33xx_spl_board_init(void)
/* power-ON 3V3 via Resetcontroller */ oldspeed = i2c_get_bus_speed();
- if (0 != i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) {
- if (0 <= i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) { buf = RSTCTRL_FORCE_PWR_NEN; i2c_write(RSTCTRL_ADDR, RSTCTRL_CTRLREG, 1, (uint8_t *)&buf, sizeof(buf));
While you are at it, can you fixup this Yoda programming style and use the regular idiom instead? It hurts the brain to have to stop and read code "backwards" before seeing what's going on.
virtually yours Gerhard Sittig
Hi Gerhard,
On 2014-03-08 18:38, Gerhard Sittig wrote:
On Fri, Mar 07, 2014 at 18:56 +0100, Hannes Petermaier wrote:
- fix: return-value of 'i2c_set_bus_speed' was interpreted wrong
Signed-off-by: Hannes Petermaier oe5hpm@oevsv.at
board/BuR/kwb/board.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/BuR/kwb/board.c b/board/BuR/kwb/board.c index 8aa16bc..8fb5e68 100644 --- a/board/BuR/kwb/board.c +++ b/board/BuR/kwb/board.c @@ -120,7 +120,7 @@ void am33xx_spl_board_init(void)
/* power-ON 3V3 via Resetcontroller */ oldspeed = i2c_get_bus_speed();
- if (0 != i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) {
- if (0 <= i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) { buf = RSTCTRL_FORCE_PWR_NEN; i2c_write(RSTCTRL_ADDR, RSTCTRL_CTRLREG, 1, (uint8_t *)&buf, sizeof(buf));
While you are at it, can you fixup this Yoda programming style and use the regular idiom instead? It hurts the brain to have to stop and read code "backwards" before seeing what's going on.
OK. I'll send v2. Maybe i can change my personal idiom. I've learned to do this so to avoid a unintended assignment to a variable. For exmaple: if (var = 10) compiler will do assignment to variable. if (10 = var) compiler will generate error.
But in these days "checkpatch.pl" will warn in such case.
virtually yours Gerhard Sittig
best regards, Hannes
On Sat, Mar 08, 2014 at 19:14 +0100, Hannes Petermaier wrote:
Hi Gerhard,
On 2014-03-08 18:38, Gerhard Sittig wrote:
On Fri, Mar 07, 2014 at 18:56 +0100, Hannes Petermaier wrote:
- fix: return-value of 'i2c_set_bus_speed' was interpreted wrong
Signed-off-by: Hannes Petermaier oe5hpm@oevsv.at
board/BuR/kwb/board.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/BuR/kwb/board.c b/board/BuR/kwb/board.c index 8aa16bc..8fb5e68 100644 --- a/board/BuR/kwb/board.c +++ b/board/BuR/kwb/board.c @@ -120,7 +120,7 @@ void am33xx_spl_board_init(void) /* power-ON 3V3 via Resetcontroller */ oldspeed = i2c_get_bus_speed();
- if (0 != i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) {
- if (0 <= i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) { buf = RSTCTRL_FORCE_PWR_NEN; i2c_write(RSTCTRL_ADDR, RSTCTRL_CTRLREG, 1, (uint8_t *)&buf, sizeof(buf));
While you are at it, can you fixup this Yoda programming style and use the regular idiom instead? It hurts the brain to have to stop and read code "backwards" before seeing what's going on.
OK. I'll send v2. Maybe i can change my personal idiom. I've learned to do this so to avoid a unintended assignment to a variable. For exmaple: if (var = 10) compiler will do assignment to variable. if (10 = var) compiler will generate error.
(This is not personally pointing at you, but is meant for the archives and for others to see why I feel this way about it, and why it's not just cosmetics.)
Years ago I was on a similar trip, but there was one reply which made me realize how weird it is: "They can remember to put the constant in front, but cannot remember to use the correct operator." I felt embarassed, and woke up. :)
Some two months ago I learned about the "Yoda programming" name, which I feel a very good match for this style. It's really sick to have to read such code. I don't care whether a constant is in some relation to something else, I care about whether the result of calling a function means success or error. Putting things backwards only "works" for those who had intense exposure to and are trained for this style, to everyone else it feels totally unnatural and obfuscates what's happening, and unnecessarily requires mental resources to parse what happens and whether it's correct. Code should not be written for compilers, but for humans to read and verify and maintain the source.
BTW have C compilers been warning about "if (a = b)" for some decades by now. There really is no point in this reverse idiom.
virtually yours Gerhard Sittig
- fix: return-value of 'i2c_set_bus_speed' was interpreted wrong
Signed-off-by: Hannes Petermaier oe5hpm@oevsv.at
--- Changes for v2: - fixed up Yoda programming style at testing return value of i2c_set_bus_speed :-) --- board/BuR/kwb/board.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/board/BuR/kwb/board.c b/board/BuR/kwb/board.c index 8aa16bc..804765a 100644 --- a/board/BuR/kwb/board.c +++ b/board/BuR/kwb/board.c @@ -120,7 +120,7 @@ void am33xx_spl_board_init(void)
/* power-ON 3V3 via Resetcontroller */ oldspeed = i2c_get_bus_speed(); - if (0 != i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) { + if (i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC) >= 0) { buf = RSTCTRL_FORCE_PWR_NEN; i2c_write(RSTCTRL_ADDR, RSTCTRL_CTRLREG, 1, (uint8_t *)&buf, sizeof(buf)); @@ -221,7 +221,7 @@ int board_late_init(void) TPS65217_WLEDCTRL1, 0x09, 0xFF); /* write bootinfo into scratchregister of resetcontroller */ oldspeed = i2c_get_bus_speed(); - if (0 != i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC)) { + if (i2c_set_bus_speed(CONFIG_SYS_OMAP24_I2C_SPEED_PSOC) >= 0) { i2c_write(RSTCTRL_ADDR, RSTCTRL_SCRATCHREG, 1, (uint8_t *)&buf, sizeof(buf)); i2c_set_bus_speed(oldspeed);
On Sat, Mar 08, 2014 at 07:09:32PM +0100, Hannes Petermaier wrote:
- fix: return-value of 'i2c_set_bus_speed' was interpreted wrong
Signed-off-by: Hannes Petermaier oe5hpm@oevsv.at
Applied to u-boot-ti/master, thanks!
On Fri, Mar 07, 2014 at 06:56:55PM +0100, Hannes Petermaier wrote:
- fix: return-value of 'i2c_set_bus_speed' was interpreted wrong
Signed-off-by: Hannes Petermaier oe5hpm@oevsv.at
Applied to u-boot-ti/master, thanks!
participants (4)
-
Gerhard Sittig -
Hannes Petermaier -
Hannes Petermaier -
Tom Rini