[flashrom] [PATCH] ft2232_spi.c: use ftdi.h named constants instead of magic numbers

[Antony Pavlov]

On Sat, 24 Jan 2015 20:03:23 +0100
Stefan Tauner <stefan.tauner at alumni.tuwien.ac.at> wrote:

> On Sat, 24 Jan 2015 16:24:36 +0100
> Stefan Tauner <stefan.tauner at 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