RE: [PATCH v2] spi: add Renesas RPC-IF driver

Chris Brandt
2020-02-10T19:34:55Z

Hello Sergei,

On Feb 10, 2020, Chris Brandt wrote:
> Side note, erase seems OK...but writing data seems to get messed up.
> As you can see below, it's adding 2 bytes of 00 into the write stream.
>
> $ flash_eraseall /dev/mtd3
> Erasing 256 Kibyte @ 1000000 - 100% complete.
> $ hexdump -C -n100 /dev/mtd3
> 00000000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> |................|
> *
> 00000060
> $ hexdump -C -n100 /dev/mtd3
> 00000000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> |................|
> *
> 00000060
> $ echo "hello" > /dev/mtd3
> $ hexdump -C -n100 /dev/mtd3
> 00000000 68 65 6c 6c 00 00 6f 0a ff ff ff ff ff ff ff ff
> |hell..o.........|
> 00000010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> |................|
> *
> 00000060

I think here is the issue of the extra 00s in the write byte stream.

For your code, you have this in function rpcif_io_xfer():

memcpy(data, rpc->buffer + pos, nbytes);
if (nbytes > 4) {
regmap_write(rpc->regmap, RPCIF_SMWDR1,
data[0]);
regmap_write(rpc->regmap, RPCIF_SMWDR0,
data[1]);
} else if (nbytes > 2) {
regmap_write(rpc->regmap, RPCIF_SMWDR0,
data[0]);
} else {
regmap_write(rpc->regmap, RPCIF_SMWDR0,
data[0] << 16);
}

But, you cannot do a 32-bit register write when you have less than 32-bits of data.

If you only have 2 bytes of data left to write, you have to do a 16-bit write to the SMWDR0 register.
If you only have 1 byte of data left to write, you have to do a 8-bit write to the SMWDR0 register.

If you only have 3 bytes of data left to write, you first send 2 bytes, then send the last byte.

Your regmap is only set up to do 32-bit writes, so you'll have to use something like iowrite16 and iowrite8.
This is why I did not use regmap in my SPI-BSC driver.

For example, here is the code from my SPI-BSC driver:

while (len > 0) {
if (len >= 4) {
unit = 4;
smenr = SMENR_SPIDE(SPIDE_32BITS);
} else {
unit = len;
if (unit == 3)
unit = 2;

if (unit >= 2)
smenr = SMENR_SPIDE(SPIDE_16BITS);
else
smenr = SMENR_SPIDE(SPIDE_8BITS);
}

/* set 4bytes data, bit stream */
smwdr0 = *data++;
if (unit >= 2)
smwdr0 |= (u32)(*data++ << 8);
if (unit >= 3)
smwdr0 |= (u32)(*data++ << 16);
if (unit >= 4)
smwdr0 |= (u32)(*data++ << 24);

/* mask unwrite area */
if (unit == 3)
smwdr0 |= 0xFF000000;
else if (unit == 2)
smwdr0 |= 0xFFFF0000;
else if (unit == 1)
smwdr0 |= 0xFFFFFF00;

/* write send data. */
if (unit == 2)
spibsc_write16(sbsc, SMWDR0, (u16)smwdr0);
else if (unit == 1)
spibsc_write8(sbsc, SMWDR0, (u8)smwdr0);
else
spibsc_write(sbsc, SMWDR0, smwdr0);

Chris