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 ;-)
>
>> + if ((o_cnt < len) && ((buf_reg_val & SPIBUF_TXFULL_MASK) == 0))
>> {
>> + /* write the data */
>> + data1_reg_val &= ~0xFFFF;
>> + if(txp) {
>> + data1_reg_val |= *txp;
>> + txp++;
>> + }
>
> Same here:
>
> if (txp)
> data1_reg_val |= *txp++;
See my comment above.
> Please fix all other occurrences of "if(" into "if (" as well.
OK
>
>> + /* write to DAT1 is required to keep the serial
>> transfer going */
>> + /* we just terminate when we reach the end */
will fix
>> + writel(data1_reg_val &
>> + ~(1 << SPIDAT1_CSHOLD_SHIFT),
>> &ds->regs->dat1);
>
> Line too long.
ditto
Thanks for the feedback
--
Delio
>
>
>
>
> 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: w...@denx.de
> The software required `Windows 95 or better', so I installed Linux.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
