[PATCH] ft2232_spi.c: use ftdi.h named constants instead of magic numbers
Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> --- ft2232_spi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ft2232_spi.c b/ft2232_spi.c index 7e7e2b1..4921caa 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -350,7 +350,7 @@ int ft2232_spi_init(void) if (clock_5x) { msg_pdbg("Disable divide-by-5 front stage\n"); - buf[0] = 0x8a; /* Disable divide-by-5. */ + buf[0] = DIS_DIV_5; if (send_buf(ftdic, buf, 1)) { ret = -5; goto ftdi_err; @@ -361,7 +361,7 @@ int ft2232_spi_init(void) } msg_pdbg("Set clock divisor\n"); - buf[0] = 0x86; /* command "set divisor" */ + buf[0] = TCK_DIVISOR; buf[1] = (divisor / 2 - 1) & 0xff; buf[2] = ((divisor / 2 - 1) >> 8) & 0xff; if (send_buf(ftdic, buf, 3)) { @@ -374,7 +374,7 @@ int ft2232_spi_init(void) /* Disconnect TDI/DO to TDO/DI for loopback. */ msg_pdbg("No loopback of TDI/DO TDO/DI\n"); - buf[0] = 0x85; + buf[0] = LOOPBACK_END; if (send_buf(ftdic, buf, 1)) { ret = -7; goto ftdi_err; @@ -453,7 +453,7 @@ static int ft2232_spi_send_command(struct flashctx *flash, * read command, then do the fetch of the results. */ if (readcnt) { - buf[i++] = 0x20; + buf[i++] = MPSSE_DO_READ; buf[i++] = (readcnt - 1) & 0xff; buf[i++] = ((readcnt - 1) >> 8) & 0xff; ret = send_buf(ftdic, buf, i); -- 2.1.4
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 - 0x11 to enable writes I guess those can be obtained from the header as well? -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
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. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
On Sat, 24 Jan 2015 16:24:36 +0100 Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at> wrote:
- 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.
Yes, this is right. I've just checked FTDI AN114 document about this. The MPSSE_DO_WRITE | MPSSE_WRITE_NEG is correct for the 0x11 number. Also, today I've added ft2232_spi_send_multicommand function for ft2232 programmer to group multi-commands into one ftdi request and encreased ftdi write buffer. Now I have the only 3-7 ms delay for each 256b write instead of 2 delays before: 1st for WREN and 2nd for PROGRAM command. The result I've got is 18 min for write + verify of 32 MB flash. Boris
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
On Sun, 25 Jan 2015 02:56:40 +0400 Antony Pavlov <antonynpavlov@gmail.com> wrote:
So I suppose that you are right and the 0x11 seems to be equivalent of "MPSSE_DO_WRITE | MPSSE_WRITE_NEG".
Ok, thanks (to Boris too)! I have committed the respective change in r1872. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
participants (3)
-
Antony Pavlov -
Boris Baykov -
Stefan Tauner