Re: [PATCH v3 5/9] spi: spi-mem: Allow masters to transfer dummy cycles directly by hardware

From: Sowjanya Komatineni
Date: Sat Dec 12 2020 - 12:29:59 EST



On 12/12/20 2:57 AM, Boris Brezillon wrote:
On Fri, 11 Dec 2020 13:15:59 -0800
Sowjanya Komatineni <skomatineni@xxxxxxxxxx> wrote:

This patch adds a flag SPI_MASTER_USES_HW_DUMMY_CYCLES for the controllers
that support transfer of dummy cycles by the hardware directly.
Hm, not sure this is a good idea. I mean, if we expect regular SPI
devices to use this feature, then why not, but if it's just for
spi-mem, I'd recommend implementing a driver-specific exec_op() instead
of using the default one.

dummy cycles programming is SPI device specific.

Transfer of dummy bytes by SW or HW controller can be depending on features supported by controller.

Adding controller driver specific exec_op() Just for skipping dummy bytes transfer will have so much of redundant code pretty much what all spi_mem_exec_op does.

So in v1, I handled this in controller driver by skipping SW transfer of dummy bytes during dummy phase and programming dummy cycles in controller register to allow HW to transfer.

Based on v1 feedback discussion, added this flag SPI_MASTER_USES_HW_DUMMY_CYCLES which can be used by controllers supporting HW dummy bytes transfer and updated spi_mem_exec_op to skip SW dummy bytes.

This helps other controllers supporting HW transfer of dummy bytes as well just to set the flag and use dummy cycles directly.

If we go for those core changes, we should at least add a
ctrl->max_dummy_cycles field so the core can fallback to regular writes
when the number of dummy cycles in the spi_mem_op exceeds what the
controller can do.
Yes makes sense. Will add this once we decide on keeping this flag to identify controllers supporting HW transfer of dummy bytes Vs SW transfer.
For controller with this flag set, spi-mem driver will skip dummy bytes
transfer in the spi message.

Controller drivers can get the number of dummy cycles from spi_message.

Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
---
drivers/spi/spi-mem.c | 18 +++++++++++-------
include/linux/spi/spi.h | 8 ++++++++
2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index f3a3f19..38a523b 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -350,13 +350,17 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
}
if (op->dummy.nbytes) {
- memset(tmpbuf + op->addr.nbytes + 1, 0xff, op->dummy.nbytes);
- xfers[xferpos].tx_buf = tmpbuf + op->addr.nbytes + 1;
- xfers[xferpos].len = op->dummy.nbytes;
- xfers[xferpos].tx_nbits = op->dummy.buswidth;
- spi_message_add_tail(&xfers[xferpos], &msg);
- xferpos++;
- totalxferlen += op->dummy.nbytes;
+ if (ctlr->flags & SPI_MASTER_USES_HW_DUMMY_CYCLES) {
+ msg.dummy_cycles = (op->dummy.nbytes * 8) / op->dummy.buswidth;
+ } else {
+ memset(tmpbuf + op->addr.nbytes + 1, 0xff, op->dummy.nbytes);
+ xfers[xferpos].tx_buf = tmpbuf + op->addr.nbytes + 1;
+ xfers[xferpos].len = op->dummy.nbytes;
+ xfers[xferpos].tx_nbits = op->dummy.buswidth;
+ spi_message_add_tail(&xfers[xferpos], &msg);
+ xferpos++;
+ totalxferlen += op->dummy.nbytes;
+ }
}
if (op->data.nbytes) {
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index aa09fdc..2024149 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -512,6 +512,8 @@ struct spi_controller {
#define SPI_MASTER_GPIO_SS BIT(5) /* GPIO CS must select slave */
+#define SPI_MASTER_USES_HW_DUMMY_CYCLES BIT(6) /* HW dummy bytes transfer */
+
/* flag indicating this is an SPI slave controller */
bool slave;
@@ -1022,6 +1024,12 @@ struct spi_message {
unsigned actual_length;
int status;
+ /*
+ * dummy cycles in the message transfer. This is used by the controller
+ * drivers supports transfer of dummy cycles directly by the hardware.
+ */
+ u8 dummy_cycles;
+
/* for optional use by whatever driver currently owns the
* spi_message ... between calls to spi_async and then later
* complete(), that's the spi_controller controller driver.