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

Stephen Warren swarren at wwwdotorg.org
Tue Mar 25 17:56:59 CET 2014


On 03/25/2014 10:54 AM, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message <1395764855-23377-1-git-send-email-swarren at wwwdotorg.org> you wrote:
>>
>> +static inline void update_reg_mask_shift_val(u32 *reg, u32 mask, u32 shift,
>> +					     u32 val)
>> +{
>> +	clrsetbits_le32(reg, mask << shift, val << shift);
>> +}
> 
> 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?????

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
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.


More information about the U-Boot mailing list