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

[Wolfgang Denk]

Dear Stephen Warren,

In message <5331B4E4.5090101 at wwwdotorg.org> you wrote:
>
> > Please do not invent new bit manipulation functions.  Just use the
> > standard I/O accessors.  And whenever possible, please remove pre-
> > existing functions.
> > 
> > I've just recently sent patches to get rid of such "inventions" that
> > resulted in undefined code.
> 
> That's not what this code is doing. The existing IO accessors are used;
> it's just removing duplication from the parameters passed to the
> existing functions (the shift needs to be written out twice).

Yes, I've seen that.  You can save a little typ[ing this way.  But you
have to type it only once, and then maintain it forever.  And the
helper function obscures what is happening, which is bad.  It makes
the code more difficult to read and understand and maintain,
especially when the shift count is not easily recognizable - at least I
cannot really read what PULL_SHIFT(pin) decodes to.

Please make the code simple to read, use existing standard functions.



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
An armed society is a polite society.