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

[Chris Brandt <Chris.Brandt@xxxxxxxxxxx>]

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