On Tue, Mar 25, 2014 at 05:54:10PM +0100, Wolfgang Denk wrote:
> Dear Stephen Warren,
>
> In message <1395764855-23377-1-git-send-email-swar...@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.No, this is why the lack of comments hurts things. This isn't sr32 from OMAP-land (which was on my todo list somewhere, thanks). sr32 was an incorrect generic function. This is a specific-use function that should say something like: /* * Set the correct pinmux value for a given part. We need to clear out * M bits worth of the field and then set possibly less than M bits * worth of value. */ 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. -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
