spi: armada-3700: Fix padding when sending not 4-byte aligned data
In 4-byte transfer mode, extra padding/dummy bytes '0xff' would be
sent in write operation if TX data is not 4-byte aligned since the
SPI data register is always shifted out as whole 4 bytes.
Fix this by using the header count feature that allows to transfer 0 to
4 bytes. Use it to actually send the first 1 to 3 bytes of data before
the rest of the buffer that will hence be 4-byte aligned.
Signed-off-by: Zachary Zhang <zhangzg@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
index a28702b..9172cb2 100644
--- a/drivers/spi/spi-armada-3700.c
+++ b/drivers/spi/spi-armada-3700.c
@@ -99,11 +99,6 @@
/* A3700_SPI_IF_TIME_REG */
#define A3700_SPI_CLK_CAPT_EDGE BIT(7)
-/* Flags and macros for struct a3700_spi */
-#define A3700_INSTR_CNT 1
-#define A3700_ADDR_CNT 3
-#define A3700_DUMMY_CNT 1
-
struct a3700_spi {
struct spi_master *master;
void __iomem *base;
@@ -117,9 +112,6 @@ struct a3700_spi {
u8 byte_len;
u32 wait_mask;
struct completion done;
- u32 addr_cnt;
- u32 instr_cnt;
- size_t hdr_cnt;
};
static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
@@ -449,59 +441,43 @@ static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
static void a3700_spi_header_set(struct a3700_spi *a3700_spi)
{
- u32 instr_cnt = 0, addr_cnt = 0, dummy_cnt = 0;
+ unsigned int addr_cnt;
u32 val = 0;
/* Clear the header registers */
spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, 0);
spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, 0);
spireg_write(a3700_spi, A3700_SPI_IF_RMODE_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, 0);
/* Set header counters */
if (a3700_spi->tx_buf) {
- if (a3700_spi->buf_len <= a3700_spi->instr_cnt) {
- instr_cnt = a3700_spi->buf_len;
- } else if (a3700_spi->buf_len <= (a3700_spi->instr_cnt +
- a3700_spi->addr_cnt)) {
- instr_cnt = a3700_spi->instr_cnt;
- addr_cnt = a3700_spi->buf_len - instr_cnt;
- } else if (a3700_spi->buf_len <= a3700_spi->hdr_cnt) {
- instr_cnt = a3700_spi->instr_cnt;
- addr_cnt = a3700_spi->addr_cnt;
- /* Need to handle the normal write case with 1 byte
- * data
- */
- if (!a3700_spi->tx_buf[instr_cnt + addr_cnt])
- dummy_cnt = a3700_spi->buf_len - instr_cnt -
- addr_cnt;
+ /*
+ * when tx data is not 4 bytes aligned, there will be unexpected
+ * bytes out of SPI output register, since it always shifts out
+ * as whole 4 bytes. This might cause incorrect transaction with
+ * some devices. To avoid that, use SPI header count feature to
+ * transfer up to 3 bytes of data first, and then make the rest
+ * of data 4-byte aligned.
+ */
+ addr_cnt = a3700_spi->buf_len % 4;
+ if (addr_cnt) {
+ val = (addr_cnt & A3700_SPI_ADDR_CNT_MASK)
+ << A3700_SPI_ADDR_CNT_BIT;
+ spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, val);
+
+ /* Update the buffer length to be transferred */
+ a3700_spi->buf_len -= addr_cnt;
+
+ /* transfer 1~3 bytes through address count */
+ val = 0;
+ while (addr_cnt--) {
+ val = (val << 8) | a3700_spi->tx_buf[0];
+ a3700_spi->tx_buf++;
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, val);
}
- val |= ((instr_cnt & A3700_SPI_INSTR_CNT_MASK)
- << A3700_SPI_INSTR_CNT_BIT);
- val |= ((addr_cnt & A3700_SPI_ADDR_CNT_MASK)
- << A3700_SPI_ADDR_CNT_BIT);
- val |= ((dummy_cnt & A3700_SPI_DUMMY_CNT_MASK)
- << A3700_SPI_DUMMY_CNT_BIT);
}
- spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, val);
-
- /* Update the buffer length to be transferred */
- a3700_spi->buf_len -= (instr_cnt + addr_cnt + dummy_cnt);
-
- /* Set Instruction */
- val = 0;
- while (instr_cnt--) {
- val = (val << 8) | a3700_spi->tx_buf[0];
- a3700_spi->tx_buf++;
- }
- spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, val);
-
- /* Set Address */
- val = 0;
- while (addr_cnt--) {
- val = (val << 8) | a3700_spi->tx_buf[0];
- a3700_spi->tx_buf++;
- }
- spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, val);
}
static int a3700_is_wfifo_full(struct a3700_spi *a3700_spi)
@@ -515,35 +491,12 @@ static int a3700_is_wfifo_full(struct a3700_spi *a3700_spi)
static int a3700_spi_fifo_write(struct a3700_spi *a3700_spi)
{
u32 val;
- int i = 0;
while (!a3700_is_wfifo_full(a3700_spi) && a3700_spi->buf_len) {
- val = 0;
- if (a3700_spi->buf_len >= 4) {
- val = cpu_to_le32(*(u32 *)a3700_spi->tx_buf);
- spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
-
- a3700_spi->buf_len -= 4;
- a3700_spi->tx_buf += 4;
- } else {
- /*
- * If the remained buffer length is less than 4-bytes,
- * we should pad the write buffer with all ones. So that
- * it avoids overwrite the unexpected bytes following
- * the last one.
- */
- val = GENMASK(31, 0);
- while (a3700_spi->buf_len) {
- val &= ~(0xff << (8 * i));
- val |= *a3700_spi->tx_buf++ << (8 * i);
- i++;
- a3700_spi->buf_len--;
-
- spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG,
- val);
- }
- break;
- }
+ val = cpu_to_le32(*(u32 *)a3700_spi->tx_buf);
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+ a3700_spi->buf_len -= 4;
+ a3700_spi->tx_buf += 4;
}
return 0;
@@ -648,9 +601,6 @@ static int a3700_spi_transfer_one(struct spi_master *master,
a3700_spi->rx_buf = xfer->rx_buf;
a3700_spi->buf_len = xfer->len;
- /* SPI transfer headers */
- a3700_spi_header_set(a3700_spi);
-
if (xfer->tx_buf)
nbits = xfer->tx_nbits;
else if (xfer->rx_buf)
@@ -658,6 +608,12 @@ static int a3700_spi_transfer_one(struct spi_master *master,
a3700_spi_pin_mode_set(a3700_spi, nbits, xfer->rx_buf ? true : false);
+ /* Flush the FIFOs */
+ a3700_spi_fifo_flush(a3700_spi);
+
+ /* Transfer first bytes of data when buffer is not 4-byte aligned */
+ a3700_spi_header_set(a3700_spi);
+
if (xfer->rx_buf) {
/* Set read data length */
spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
@@ -736,16 +692,11 @@ static int a3700_spi_transfer_one(struct spi_master *master,
dev_err(&spi->dev, "wait wfifo empty timed out\n");
return -ETIMEDOUT;
}
- } else {
- /*
- * If the instruction in SPI_INSTR does not require data
- * to be written to the SPI device, wait until SPI_RDY
- * is 1 for the SPI interface to be in idle.
- */
- if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
- dev_err(&spi->dev, "wait xfer ready timed out\n");
- return -ETIMEDOUT;
- }
+ }
+
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait xfer ready timed out\n");
+ return -ETIMEDOUT;
}
val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
@@ -837,10 +788,6 @@ static int a3700_spi_probe(struct platform_device *pdev)
memset(spi, 0, sizeof(struct a3700_spi));
spi->master = master;
- spi->instr_cnt = A3700_INSTR_CNT;
- spi->addr_cnt = A3700_ADDR_CNT;
- spi->hdr_cnt = A3700_INSTR_CNT + A3700_ADDR_CNT +
- A3700_DUMMY_CNT;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
spi->base = devm_ioremap_resource(dev, res);