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

Wolfgang Denk wd at denx.de
Tue Mar 25 23:19:33 CET 2014


Dear Tom,

In message <20140325200420.GU16360 at bill-the-cat> you wrote:
> 
> With respect to danger / readability, no, either way is just as
> dangerous (or not dangerous) and it's still fairly dense code either
> way and fixing a problem with an incorrect shift value is the same
> effort.

The key problem which I detected with sr32() was that it was in
several places called with a width of 32 - which looked perfectly fine
when the intention was to clear / set the whole 32 bit variable.  This
went unnoticed becuase it was just a normally looking argument.  If
the shift operation that resulted from that had been visible, the
problem would have been much easier to detect.  Seeing an expression
"value << 32" on u32 data types rings some alarms.

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
If I had to live my life again,  I'd  make  the  same  mistakes, only
sooner.                                          -- Tallulah Bankhead


More information about the U-Boot mailing list