Serge Semin writes:

On Wed, May 13, 2020 at 04:00:23PM +0200, Lars Povlsen wrote:

quoted

This add support for the RX_SAMPLE_DLY register. If enabled in the
Designware IP, it allows tuning of the rx data signal by means of an
internal rx sample fifo.

The register is located at offset 0xf0, and if the option is not
enabled in the IP, changing the register will have no effect.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <redacted>
---
 drivers/spi/spi-dw.c | 7 +++++++
 drivers/spi/spi-dw.h | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index e572eb34a3c1a..32997f28fa5bb 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c

@@ -81,6 +81,9 @@ static ssize_t dw_spi_show_regs(struct file *file, char __user *user_buf,
                      "DMATDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMATDLR));
      len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
                      "DMARDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMARDLR));

quoted

+     len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
+                      "RX_SAMPLE_DLY: \t0x%08x\n",
+                      dw_readl(dws, DW_SPI_RX_SAMPLE_DLY));

debugfs_reg32 interface is now utilized in the driver to dump the registers
state. So this will have to be converted to just a new entry in the
dw_spi_dbgfs_regs array.

Ok, I'll have a look at this.

quoted

      len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
                      "=================================\n");

@@ -315,6 +318,10 @@ static int dw_spi_transfer_one(struct spi_controller *master,
              spi_set_clk(dws, chip->clk_div);
      }

quoted

+     /* Apply RX sample delay, iff requested (nonzero) */

s/iff/if

quoted

+     if (dws->rx_sample_dly)
+             dw_writel(dws, DW_SPI_RX_SAMPLE_DLY, dws->rx_sample_dly);
+
      dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
      dws->dma_width = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);

diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 1bf5713e047d3..ed6e47b3f50da 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h

@@ -31,6 +31,7 @@
 #define DW_SPI_IDR                   0x58
 #define DW_SPI_VERSION                       0x5c
 #define DW_SPI_DR                    0x60
+#define DW_SPI_RX_SAMPLE_DLY         0xf0
 #define DW_SPI_CS_OVERRIDE           0xf4

 /* Bit fields in CTRLR0 */

@@ -111,6 +112,7 @@ struct dw_spi {

      int                     cs_override;
      u32                     reg_io_width;   /* DR I/O width in bytes */

quoted

+     u8                      rx_sample_dly;  /* RX fifo tuning (option) */

This doesn't seem like a good place for this parameter. The sample delay is
SPI-slave specific. So as I see it, the parameter should be moved to the
chip_data.

Yes, I got that in other comments, and you are right I guess.

I will apply that approach of having rx_sample_dly per SPI slave.

---Lars

-Sergey

quoted

      u16                     bus_num;
      u16                     num_cs;         /* supported slave numbers */
      void (*set_cs)(struct spi_device *spi, bool enable);
--
2.26.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lars Povlsen,
Microchip