[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