[U-Boot] [PATCH] Improve DaVinci SPI speed

Nori, Sekhar nsekhar at ti.com
Tue Jun 1 12:18:32 CEST 2010


Hi Delio,

On Sun, May 23, 2010 at 17:11:56, Delio Brignoli wrote:
> Hello Wolfgang,
>
> On 21/05/2010, at 15:13, Wolfgang Denk wrote:
> >> +                          *rxp = buf_reg_val & 0xFF;
> >> +                          rxp++;
> >> +                  }
> >
> > Please change into:
> >
> >                     if (rxp)
> >                             *rxp++ = buf_reg_val & 0xFF;
>
> Are you sure? Is folding that 3 line block into a one liner really worth the loss of readability? I know what that means, so do you and many others on this ML, but that's bound to raise a few eyebrows. A quick google search reveals quite a few articles devoted to clearing up the confusion about the semantics of that pointer dereference and post-increment. Just a few years ago I would have preferred your suggestion, but now I find that readability trumps cleverness, at least in a case like this. In the end it's your decision: if you like to be kinky, it's your prerogative ;-)
>

Looking for the pointer de-reference and increment expression
in Linux kernel, I had ~4500 hits and ~600 hits in U-Boot.
So, may be this is not such an uncommon expression after all
(at least in the kernel/U-Boot world).

Considering this, can you please accept the change Wolfgang
is asking for for so this useful patch can move forward?

After all, code readability is also about pattern matching
in the mind and if developers are already used to seeing this
expression not many would find it odd.

Thanks,
Sekhar


More information about the U-Boot mailing list