[U-Boot] [PATCH V3 09/13] ARM: tegra: use clrsetbits_le32 in pinmux driver

Wolfgang Denk wd at denx.de
Tue Mar 25 18:06:07 CET 2014


Dear Stephen Warren,

In message <5331B55B.7080209 at wwwdotorg.org> you wrote:
>
> > No, please do not do that.  Please use plain clrsetbits_le32() as is.
> > All these hidden shifts are (a) mostly unreadable and (b) sometimes
> > dangerous.
> 
> Seriously, are you joking now?????

No, I am not.

> If I was to write out the clrsetbits_le32() at each call site, I'd be
> writing out this supposedly dangerous shift N times instead of once. If

N = 2, to be precise.  And you have to type it only once, but to
maintain that code for a long, long time.

> the shift is somehow dangerous (BTW, it isn't!) then surely isolating it
> in one place, so that mistakes aren't made when writing the duplicate
> copies, is the right thing to do.

Well, I've just fixed a number of places were such code _was_
dangerous, but well hidden under a nice wrapper function like yours.

Best regards,

Wolfgang Denk

-- 
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
Sometimes, too long is too long.                          - Joe Crowe


More information about the U-Boot mailing list