On Thu, 15 Sep 2011 21:59:30 +0200 Uwe Hermann <uwe@hermann-uwe.de> wrote:
On Thu, Sep 08, 2011 at 12:56:21AM +0200, Stefan Tauner wrote:
+ 0x13 (O_SPIOP): + Send and receive bytes via SPI. + Maximum slen is Q_WRNMAXLEN result after Q_BUSTYPE returns ^ Full stop.
+ only SPI or S_BUSTYPE == SPI is used. Same for rlen and Q_RDNMAXLEN. ^ Capitalize
hm i think urja wanted this to be in one sentence otherwise it does not make sense. i have changed this now to "Maximum slen is Q_WRNMAXLEN in case Q_BUSTYPE returns SPI only or S_BUSTYPE was used to set SPI exclusively before. Same for rlen and Q_RDNMAXLEN."
+ if (sp_docommand(S_CMD_Q_BUSTYPE, 0, NULL, 1, &c)) { + msg_perr("Warning: NAK to query supported buses\n");
perr and "Warning" doesn't sound consistent, but can be fixed in another patch.
yes, but it is used all over the place.
+ if (!sp_docommand(S_CMD_Q_RDNMAXLEN, 0, NULL, 3, rbuf)) { + uint32_t v; + v = ((unsigned int)(rbuf[0]) << 0); + v |= ((unsigned int)(rbuf[1]) << 8); + v |= ((unsigned int)(rbuf[2]) << 16);
Why (unsigned int) and not (uint32_t)? This is done in multiple places.
i have no idea actually, i just moved the code :) i have left it alone for now. this can be fixed later too.
+ if (!sp_check_commandavail(S_CMD_O_DELAY)) { + internal_delay(delay); + msg_pdbg("Note: serprog_delay used, but the programmer doesnt support delay\n");
doesnt -> doesn't
and 80 char limit..
+ for(i = 0; i < len; ) { ^ missing space
the missing third argument is a more WTF imho. especially since the increment is done in the last line of the for body anyway... i have moved it from there into the head of the for instruction.
Otherwise Acked-by: Uwe Hermann <uwe@hermann-uwe.de>
thanks for the review! i hope this is quite useful for some (future) users. committed in r1442. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner