On Sat, 24 Jan 2015 20:03:23 +0100 Stefan Tauner stefan.tauner@alumni.tuwien.ac.at wrote:
On Sat, 24 Jan 2015 16:24:36 +0100 Stefan Tauner stefan.tauner@alumni.tuwien.ac.at wrote:
Hello Antony,
thanks for your patch. It looks good in general, but there are still some magic constants in the code:
- the initial values for cs_bits and pindir
No, because they are actually simple bit masks and there are no defines for the meaning of the bits. But we could add our own defines for the bit offsets, e.g. "#define TMS_CS_OFF (3)" and or them together in other #defines for the few combinations we actually need for the initial values... I don't think that's worth it though. But I have added various comments explaining this.
- 0x11 to enable writes
The 0x11 seems to be equivalent of "MPSSE_DO_WRITE | MPSSE_WRITE_NEG", right? If anyone can confirm this, I'll commit that resulting patch.
I have just examined libmpsse.
Please see int SetMode() in libmpsse/src/mpsse.c (see https://code.google.com/p/libmpsse/source/browse/trunk/src/mpsse.c#298), for SPI mode 0 and SPI mode 3:
mpsse->tx |= MPSSE_WRITE_NEG; ... mpsse->txrx |= MPSSE_WRITE_NEG;
next, the mpsse->tx* value is passed to build_block_buffer() https://code.google.com/p/libmpsse/source/browse/trunk/src/mpsse.c#762 e.g.
buf = build_block_buffer(mpsse, mpsse->tx, (unsigned char *) (data + n), txsize, &buf_size);
Here is part of build_block_buffer(struct mpsse_context *mpsse, uint8_t cmd, ... (see https://code.google.com/p/libmpsse/source/browse/trunk/src/support.c#154)
/* Copy in the command for this block */ buf[i++] = cmd; buf[i++] = (rsize & 0xFF); if(!(cmd & MPSSE_BITMODE)) { buf[i++] = ((rsize >> 8) & 0xFF); }
/* On a write, copy the data to transmit after the command */ if(cmd == mpsse->tx || cmd == mpsse->txrx) { memcpy(buf+i, data+k, dsize);
/* i == offset into buf */ i += dsize; /* k == offset into data */ k += dsize; }
So I suppose that you are right and the 0x11 seems to be equivalent of "MPSSE_DO_WRITE | MPSSE_WRITE_NEG".
-- Best regards, Antony Pavlov