[U-Boot] [PATCH] board/BuR/kwb: fix usage of 'i2c_set_bus_speed'

[Gerhard Sittig]

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 at 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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de