[PATCH v4 00/16] spi: dw: Add generic DW DMA controller support

[PATCH v4 00/16] spi: dw: Add generic DW DMA controller support

[Serge Semin]

Baikal-T1 SoC provides a DW DMA controller to perform low-speed peripherals
Mem-to-Dev and Dev-to-Mem transaction. This is also applicable to the DW
APB SSI devices embedded into the SoC. Currently the DMA-based transfers
are supported by the DW APB SPI driver only as a middle layer code for
Intel MID/Elkhart PCI devices. Seeing the same code can be used for normal
platform DMAC device we introduced a set of patches to fix it within this
series.

First of all we need to add the Tx and Rx DMA channels support into the DW
APB SSI binding. Then there are several fixes and cleanups provided as a
initial preparation for the Generic DMA support integration: add Tx/Rx
finish wait methods, clear DMAC register when done or stopped, Fix native
CS being unset, enable interrupts in accordance with DMA xfer mode,
discard static DW DMA slave structures, discard unused void priv pointer
and dma_width member of the dw_spi structure, provide the DMA Tx/Rx burst
length parametrisation and make sure it's optionally set in accordance
with the DMA max-burst capability.

In order to have the DW APB SSI MMIO driver working with DMA we need to
initialize the paddr field with the physical base address of the DW APB SSI
registers space. Then we unpin the Intel MID specific code from the
generic DMA one and placed it into the spi-dw-pci.c driver, which is a
better place for it anyway. After that the naming cleanups are performed
since the code is going to be used for a generic DMAC device. Finally the
Generic DMA initialization can be added to the generic version of the
DW APB SSI IP.

Last but not least we traditionally convert the legacy plain text-based
dt-binding file with yaml-based one and as a cherry on a cake replace
the manually written DebugFS registers read method with a ready-to-use
for the same purpose regset32 DebugFS interface usage.

This patchset is rebased and tested on the spi/for-next (5.7-rc5):
base-commit: fe9fce6b2cf3 ("Merge remote-tracking branch 'spi/for-5.8' into spi-next")

Link: https://lore.kernel.org/linux-spi/20200508132943.9826-1-Sergey.Semin@baikalelectronics.ru/
Changelog v2:
- Rebase on top of the spi repository for-next branch.
- Move bindings conversion patch to the tail of the series.
- Move fixes to the head of the series.
- Apply as many changes as possible to be applied the Generic DMA
  functionality support is added and the spi-dw-mid is moved to the
  spi-dw-dma driver.
- Discard patch "spi: dw: Fix dma_slave_config used partly uninitialized"
  since the problem has already been fixed.
- Add new patch "spi: dw: Discard unused void priv pointer".
- Add new patch "spi: dw: Discard dma_width member of the dw_spi structure".
  n_bytes member of the DW SPI data can be used instead.
- Build the DMA functionality into the DW APB SSI core if required instead
  of creating a separate kernel module.
- Use conditional statement instead of the ternary operator in the ref
  clock getter.

Link: https://lore.kernel.org/linux-spi/20200515104758.6934-1-Sergey.Semin@baikalelectronics.ru/
Changelog v3:
- Use spi_delay_exec() method to wait for the DMA operation completion.
- Explicitly initialize the dw_dma_slave members on stack.
- Discard the dws->fifo_len utilization in the Tx FIFO DMA threshold
  setting from the patch where we just add the default burst length
  constants.
- Use min() method to calculate the optimal burst values.
- Add new patch which moves the spi-dw.c source file to spi-dw-core.c in
  order to preserve the DW APB SSI core driver name.
- Add commas in the debugfs_reg32 structure initializer and after the last
  entry of the dw_spi_dbgfs_regs array.

Link: https://lore.kernel.org/linux-spi/20200521012206.14472-1-Sergey.Semin@baikalelectronics.ru
Changelog v4:
- Get back ndelay() method to wait for an SPI transfer completion.
  spi_delay_exec() isn't suitable for the atomic context.

Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-spi@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Serge Semin (16):
  spi: dw: Add Tx/Rx finish wait methods to the MID DMA
  spi: dw: Enable interrupts in accordance with DMA xfer mode
  spi: dw: Discard static DW DMA slave structures
  spi: dw: Discard unused void priv pointer
  spi: dw: Discard dma_width member of the dw_spi structure
  spi: dw: Parameterize the DMA Rx/Tx burst length
  spi: dw: Use DMA max burst to set the request thresholds
  spi: dw: Fix Rx-only DMA transfers
  spi: dw: Add core suffix to the DW APB SSI core source file
  spi: dw: Move Non-DMA code to the DW PCIe-SPI driver
  spi: dw: Remove DW DMA code dependency from DW_DMAC_PCI
  spi: dw: Add DW SPI DMA/PCI/MMIO dependency on the DW SPI core
  spi: dw: Cleanup generic DW DMA code namings
  spi: dw: Add DMA support to the DW SPI MMIO driver
  spi: dw: Use regset32 DebugFS method to create regdump file
  dt-bindings: spi: Convert DW SPI binding to DT schema

 .../bindings/spi/snps,dw-apb-ssi.txt          |  44 ---
 .../bindings/spi/snps,dw-apb-ssi.yaml         | 127 +++++++++
 .../devicetree/bindings/spi/spi-dw.txt        |  24 --
 drivers/spi/Kconfig                           |  15 +-
 drivers/spi/Makefile                          |   5 +-
 drivers/spi/{spi-dw.c => spi-dw-core.c}       |  88 ++----
 drivers/spi/{spi-dw-mid.c => spi-dw-dma.c}    | 261 ++++++++++--------
 drivers/spi/spi-dw-mmio.c                     |   4 +
 drivers/spi/spi-dw-pci.c                      |  50 +++-
 drivers/spi/spi-dw.h                          |  33 ++-
 10 files changed, 392 insertions(+), 259 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
 create mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-dw.txt
 rename drivers/spi/{spi-dw.c => spi-dw-core.c} (82%)
 rename drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} (55%)

-- 
2.25.1

[PATCH v4 02/16] spi: dw: Enable interrupts in accordance with DMA xfer mode

[Serge Semin]

It's pointless to track the Tx overrun interrupts if Rx-only SPI
transfer is issued. Similarly there is no need in handling the Rx
overrun/underrun interrupts if Tx-only SPI transfer is executed.
So lets unmask the interrupts only if corresponding SPI
transactions are implied.

Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 drivers/spi/spi-dw-mid.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
index 2152c0cb3953..f24fcd925b48 100644
--- a/drivers/spi/spi-dw-mid.c
+++ b/drivers/spi/spi-dw-mid.c
@@ -297,19 +297,23 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_rx(struct dw_spi *dws,
 
 static int mid_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
 {
-	u16 dma_ctrl = 0;
+	u16 imr = 0, dma_ctrl = 0;
 
 	dw_writel(dws, DW_SPI_DMARDLR, 0xf);
 	dw_writel(dws, DW_SPI_DMATDLR, 0x10);
 
-	if (xfer->tx_buf)
+	if (xfer->tx_buf) {
 		dma_ctrl |= SPI_DMA_TDMAE;
-	if (xfer->rx_buf)
+		imr |= SPI_INT_TXOI;
+	}
+	if (xfer->rx_buf) {
 		dma_ctrl |= SPI_DMA_RDMAE;
+		imr |= SPI_INT_RXUI | SPI_INT_RXOI;
+	}
 	dw_writel(dws, DW_SPI_DMACR, dma_ctrl);
 
 	/* Set the interrupt mask */
-	spi_umask_intr(dws, SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI);
+	spi_umask_intr(dws, imr);
 
 	dws->transfer_handler = dma_transfer;
 
-- 
2.25.1

[PATCH v4 04/16] spi: dw: Discard unused void priv pointer

[Serge Semin]

Seeing the "void *priv" member of the dw_spi data structure is unused
let's remove it. The glue-layers can embed the DW APB SSI controller
descriptor into their private data object. MMIO driver for instance
already utilizes that design pattern.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v2:
- It's a new patch created as a result of more thorough driver study.
---
 drivers/spi/spi-dw.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 60e9e430ce7b..b6ab81e0c747 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -147,8 +147,6 @@ struct dw_spi {
 	dma_addr_t		dma_addr; /* phy address of the Data register */
 	const struct dw_spi_dma_ops *dma_ops;
 
-	/* Bus interface info */
-	void			*priv;
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs;
 #endif
-- 
2.25.1

[PATCH v4 06/16] spi: dw: Parameterize the DMA Rx/Tx burst length

[Serge Semin]

It isn't good to have numeric literals in the code especially if there
are multiple of them and they are related. Let's replace the Tx and Rx
burst level literals with the corresponding constants.

Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v3:
- Discard the dws->fifo_len utilization in the Tx FIFO DMA threshold
  setting.
---
 drivers/spi/spi-dw-mid.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
index c39bc8758339..1598c36c905f 100644
--- a/drivers/spi/spi-dw-mid.c
+++ b/drivers/spi/spi-dw-mid.c
@@ -20,7 +20,9 @@
 
 #define WAIT_RETRIES	5
 #define RX_BUSY		0
+#define RX_BURST_LEVEL	16
 #define TX_BUSY		1
+#define TX_BURST_LEVEL	16
 
 static bool mid_spi_dma_chan_filter(struct dma_chan *chan, void *param)
 {
@@ -198,7 +200,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws,
 	memset(&txconf, 0, sizeof(txconf));
 	txconf.direction = DMA_MEM_TO_DEV;
 	txconf.dst_addr = dws->dma_addr;
-	txconf.dst_maxburst = 16;
+	txconf.dst_maxburst = TX_BURST_LEVEL;
 	txconf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	txconf.dst_addr_width = convert_dma_width(dws->n_bytes);
 	txconf.device_fc = false;
@@ -273,7 +275,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_rx(struct dw_spi *dws,
 	memset(&rxconf, 0, sizeof(rxconf));
 	rxconf.direction = DMA_DEV_TO_MEM;
 	rxconf.src_addr = dws->dma_addr;
-	rxconf.src_maxburst = 16;
+	rxconf.src_maxburst = RX_BURST_LEVEL;
 	rxconf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	rxconf.src_addr_width = convert_dma_width(dws->n_bytes);
 	rxconf.device_fc = false;
@@ -298,8 +300,8 @@ static int mid_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
 {
 	u16 imr = 0, dma_ctrl = 0;
 
-	dw_writel(dws, DW_SPI_DMARDLR, 0xf);
-	dw_writel(dws, DW_SPI_DMATDLR, 0x10);
+	dw_writel(dws, DW_SPI_DMARDLR, RX_BURST_LEVEL - 1);
+	dw_writel(dws, DW_SPI_DMATDLR, TX_BURST_LEVEL);
 
 	if (xfer->tx_buf) {
 		dma_ctrl |= SPI_DMA_TDMAE;
-- 
2.25.1

[PATCH v4 07/16] spi: dw: Use DMA max burst to set the request thresholds

[Serge Semin]

Each channel of DMA controller may have a limited length of burst
transaction (number of IO operations performed at ones in a single
DMA client request). This parameter can be used to setup the most
optimal DMA Tx/Rx data level values. In order to avoid the Tx buffer
overrun we can set the DMA Tx level to be of FIFO depth minus the
maximum burst transactions length. To prevent the Rx buffer underflow
the DMA Rx level should be set to the maximum burst transactions length.
This commit setups the DMA channels and the DW SPI DMA Tx/Rx levels
in accordance with these rules.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v3:
- Use min() method to calculate the optimal burst values.
---
 drivers/spi/spi-dw-mid.c | 37 +++++++++++++++++++++++++++++++++----
 drivers/spi/spi-dw.h     |  2 ++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
index 1598c36c905f..ac96b99ef226 100644
--- a/drivers/spi/spi-dw-mid.c
+++ b/drivers/spi/spi-dw-mid.c
@@ -35,6 +35,31 @@ static bool mid_spi_dma_chan_filter(struct dma_chan *chan, void *param)
 	return true;
 }
 
+static void mid_spi_maxburst_init(struct dw_spi *dws)
+{
+	struct dma_slave_caps caps;
+	u32 max_burst, def_burst;
+	int ret;
+
+	def_burst = dws->fifo_len / 2;
+
+	ret = dma_get_slave_caps(dws->rxchan, &caps);
+	if (!ret && caps.max_burst)
+		max_burst = caps.max_burst;
+	else
+		max_burst = RX_BURST_LEVEL;
+
+	dws->rxburst = min(max_burst, def_burst);
+
+	ret = dma_get_slave_caps(dws->txchan, &caps);
+	if (!ret && caps.max_burst)
+		max_burst = caps.max_burst;
+	else
+		max_burst = TX_BURST_LEVEL;
+
+	dws->txburst = min(max_burst, def_burst);
+}
+
 static int mid_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
 {
 	struct dw_dma_slave slave = {
@@ -70,6 +95,8 @@ static int mid_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
 	dws->master->dma_rx = dws->rxchan;
 	dws->master->dma_tx = dws->txchan;
 
+	mid_spi_maxburst_init(dws);
+
 	return 0;
 
 free_rxchan:
@@ -95,6 +122,8 @@ static int mid_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
 	dws->master->dma_rx = dws->rxchan;
 	dws->master->dma_tx = dws->txchan;
 
+	mid_spi_maxburst_init(dws);
+
 	return 0;
 }
 
@@ -200,7 +229,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws,
 	memset(&txconf, 0, sizeof(txconf));
 	txconf.direction = DMA_MEM_TO_DEV;
 	txconf.dst_addr = dws->dma_addr;
-	txconf.dst_maxburst = TX_BURST_LEVEL;
+	txconf.dst_maxburst = dws->txburst;
 	txconf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	txconf.dst_addr_width = convert_dma_width(dws->n_bytes);
 	txconf.device_fc = false;
@@ -275,7 +304,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_rx(struct dw_spi *dws,
 	memset(&rxconf, 0, sizeof(rxconf));
 	rxconf.direction = DMA_DEV_TO_MEM;
 	rxconf.src_addr = dws->dma_addr;
-	rxconf.src_maxburst = RX_BURST_LEVEL;
+	rxconf.src_maxburst = dws->rxburst;
 	rxconf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	rxconf.src_addr_width = convert_dma_width(dws->n_bytes);
 	rxconf.device_fc = false;
@@ -300,8 +329,8 @@ static int mid_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
 {
 	u16 imr = 0, dma_ctrl = 0;
 
-	dw_writel(dws, DW_SPI_DMARDLR, RX_BURST_LEVEL - 1);
-	dw_writel(dws, DW_SPI_DMATDLR, TX_BURST_LEVEL);
+	dw_writel(dws, DW_SPI_DMARDLR, dws->rxburst - 1);
+	dw_writel(dws, DW_SPI_DMATDLR, dws->fifo_len - dws->txburst);
 
 	if (xfer->tx_buf) {
 		dma_ctrl |= SPI_DMA_TDMAE;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 4902f937c3d7..d0c8b7d3a5d2 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -141,7 +141,9 @@ struct dw_spi {
 
 	/* DMA info */
 	struct dma_chan		*txchan;
+	u32			txburst;
 	struct dma_chan		*rxchan;
+	u32			rxburst;
 	unsigned long		dma_chan_busy;
 	dma_addr_t		dma_addr; /* phy address of the Data register */
 	const struct dw_spi_dma_ops *dma_ops;
-- 
2.25.1

[PATCH v4 11/16] spi: dw: Remove DW DMA code dependency from DW_DMAC_PCI

[Serge Semin]

Since there is a generic method available to initialize the DW SPI DMA
interface on any DT and ACPI-based platforms, which in general can be
designed with not only DW DMAC but with any DMA engine on board, we can
freely remove the CONFIG_DW_DMAC_PCI config from dependency list of
CONFIG_SPI_DW_DMA. Especially seeing that we don't use anything DW DMAC
specific in the new driver.

Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 drivers/spi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 03b061975f70..6a84f3dad35c 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -228,7 +228,7 @@ config SPI_DESIGNWARE
 
 config SPI_DW_DMA
 	bool "DMA support for DW SPI controller"
-	depends on SPI_DESIGNWARE && DW_DMAC_PCI
+	depends on SPI_DESIGNWARE
 
 config SPI_DW_PCI
 	tristate "PCI interface driver for DW SPI core"
-- 
2.25.1

[PATCH v4 12/16] spi: dw: Add DW SPI DMA/PCI/MMIO dependency on the DW SPI core

[Serge Semin]

Seeing all of the DW SPI driver components like DW SPI DMA/PCI/MMIO
depend on the DW SPI core code it's better to use the if-endif
conditional kernel config statement to signify that common dependency.

Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 drivers/spi/Kconfig | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 6a84f3dad35c..3cdf8310d185 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -226,17 +226,20 @@ config SPI_DESIGNWARE
 	help
 	  general driver for SPI controller core from DesignWare
 
+if SPI_DESIGNWARE
+
 config SPI_DW_DMA
 	bool "DMA support for DW SPI controller"
-	depends on SPI_DESIGNWARE
 
 config SPI_DW_PCI
 	tristate "PCI interface driver for DW SPI core"
-	depends on SPI_DESIGNWARE && PCI
+	depends on PCI
 
 config SPI_DW_MMIO
 	tristate "Memory-mapped io interface driver for DW SPI core"
-	depends on SPI_DESIGNWARE
+	depends on HAS_IOMEM
+
+endif
 
 config SPI_DLN2
        tristate "Diolan DLN-2 USB SPI adapter"
-- 
2.25.1

[PATCH v4 14/16] spi: dw: Add DMA support to the DW SPI MMIO driver

[Serge Semin]

Since the common code in the spi-dw-dma.c driver is ready to be used
by the MMIO driver and now provides a method to generically (on any
DT or ACPI-based platforms) retrieve the Tx/Rx DMA channel handlers,
we can use it and a set of the common DW SPI DMA callbacks to enable
DMA at least for generic "snps,dw-apb-ssi" and "snps,dwc-ssi-1.01a"
devices.

Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 drivers/spi/spi-dw-mmio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 0894b4c09496..e23d0c53a664 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -149,6 +149,8 @@ static int dw_spi_dw_apb_init(struct platform_device *pdev,
 	/* Register hook to configure CTRLR0 */
 	dwsmmio->dws.update_cr0 = dw_spi_update_cr0;
 
+	dw_spi_dma_setup_generic(&dwsmmio->dws);
+
 	return 0;
 }
 
@@ -158,6 +160,8 @@ static int dw_spi_dwc_ssi_init(struct platform_device *pdev,
 	/* Register hook to configure CTRLR0 */
 	dwsmmio->dws.update_cr0 = dw_spi_update_cr0_v1_01a;
 
+	dw_spi_dma_setup_generic(&dwsmmio->dws);
+
 	return 0;
 }
 
-- 
2.25.1

[PATCH v4 16/16] dt-bindings: spi: Convert DW SPI binding to DT schema

[Serge Semin]

Modern device tree bindings are supposed to be created as YAML-files
in accordance with dt-schema. This commit replaces two DW SPI legacy
bare text bindings with YAML file. As before the bindings file states
that the corresponding dts node is supposed to be compatible either
with generic DW APB SSI controller or with Microsemi/Amazon/Renesas/Intel
vendors-specific controllers, to have registers, interrupts and clocks
properties. Though in case of Microsemi version of the controller
there must be two registers resources specified. Properties like
clock-names, reg-io-width, cs-gpio, num-cs, DMA and slave device
sub-nodes are optional.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-mips@vger.kernel.org
---
 .../bindings/spi/snps,dw-apb-ssi.txt          |  44 ------
 .../bindings/spi/snps,dw-apb-ssi.yaml         | 127 ++++++++++++++++++
 .../devicetree/bindings/spi/spi-dw.txt        |  24 ----
 3 files changed, 127 insertions(+), 68 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
 create mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-dw.txt

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
deleted file mode 100644
index 020e3168ee41..000000000000
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ /dev/null
@@ -1,44 +0,0 @@
-Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
-
-Required properties:
-- compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi", where soc is "ocelot" or
-  "jaguar2", or "amazon,alpine-dw-apb-ssi", or "snps,dwc-ssi-1.01a" or
-  "intel,keembay-ssi"
-- reg : The register base for the controller. For "mscc,<soc>-spi", a second
-  register set is required (named ICPU_CFG:SPI_MST)
-- interrupts : One interrupt, used by the controller.
-- #address-cells : <1>, as required by generic SPI binding.
-- #size-cells : <0>, also as required by generic SPI binding.
-- clocks : phandles for the clocks, see the description of clock-names below.
-   The phandle for the "ssi_clk" is required. The phandle for the "pclk" clock
-   is optional. If a single clock is specified but no clock-name, it is the
-   "ssi_clk" clock. If both clocks are listed, the "ssi_clk" must be first.
-
-Optional properties:
-- clock-names : Contains the names of the clocks:
-    "ssi_clk", for the core clock used to generate the external SPI clock.
-    "pclk", the interface clock, required for register access. If a clock domain
-     used to enable this clock then it should be named "pclk_clkdomain".
-- cs-gpios : Specifies the gpio pins to be used for chipselects.
-- num-cs : The number of chipselects. If omitted, this will default to 4.
-- reg-io-width : The I/O register width (in bytes) implemented by this
-  device.  Supported values are 2 or 4 (the default).
-- dmas : Phandle + identifiers of Tx and Rx DMA channels.
-- dma-names : Contains the names of the DMA channels. Must be "tx" and "rx".
-
-Child nodes as per the generic SPI binding.
-
-Example:
-
-	spi@fff00000 {
-		compatible = "snps,dw-apb-ssi";
-		reg = <0xfff00000 0x1000>;
-		interrupts = <0 154 4>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-		clocks = <&spi_m_clk>;
-		num-cs = <2>;
-		cs-gpios = <&gpio0 13 0>,
-			   <&gpio0 14 0>;
-	};
-
diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
new file mode 100644
index 000000000000..1fcab6415136
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/snps,dw-apb-ssi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface
+
+maintainers:
+  - Mark Brown <broonie@kernel.org>
+
+allOf:
+  - $ref: "spi-controller.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mscc,ocelot-spi
+              - mscc,jaguar2-spi
+    then:
+      properties:
+        reg:
+          minItems: 2
+
+properties:
+  compatible:
+    oneOf:
+      - description: Generic DW SPI Controller
+        enum:
+          - snps,dw-apb-ssi
+          - snps,dwc-ssi-1.01a
+      - description: Microsemi Ocelot/Jaguar2 SoC SPI Controller
+        items:
+          - enum:
+              - mscc,ocelot-spi
+              - mscc,jaguar2-spi
+          - const: snps,dw-apb-ssi
+      - description: Amazon Alpine SPI Controller
+        const: amazon,alpine-dw-apb-ssi
+      - description: Renesas RZ/N1 SPI Controller
+        items:
+          - const: renesas,rzn1-spi
+          - const: snps,dw-apb-ssi
+      - description: Intel Keem Bay SPI Controller
+        const: intel,keembay-ssi
+
+  reg:
+    minItems: 1
+    items:
+      - description: DW APB SSI controller memory mapped registers
+      - description: SPI MST region map
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    items:
+      - description: SPI Controller reference clock source
+      - description: APB interface clock source
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: ssi_clk
+      - const: pclk
+
+  reg-io-width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: I/O register width (in bytes) implemented by this device
+    default: 4
+    enum: [ 2, 4 ]
+
+  num-cs:
+    default: 4
+    minimum: 1
+    maximum: 4
+
+  dmas:
+    items:
+      - description: TX DMA Channel
+      - description: RX DMA Channel
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+patternProperties:
+  "^.*@[0-9a-f]+$":
+    type: object
+    properties:
+      reg:
+        minimum: 0
+        maximum: 3
+
+      spi-rx-bus-width:
+        const: 1
+
+      spi-tx-bus-width:
+        const: 1
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - interrupts
+  - clocks
+
+examples:
+  - |
+    spi@fff00000 {
+      compatible = "snps,dw-apb-ssi";
+      reg = <0xfff00000 0x1000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      interrupts = <0 154 4>;
+      clocks = <&spi_m_clk>;
+      num-cs = <2>;
+      cs-gpios = <&gpio0 13 0>,
+                 <&gpio0 14 0>;
+    };
+...
diff --git a/Documentation/devicetree/bindings/spi/spi-dw.txt b/Documentation/devicetree/bindings/spi/spi-dw.txt
deleted file mode 100644
index 7b63ed601990..000000000000
--- a/Documentation/devicetree/bindings/spi/spi-dw.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-Synopsys DesignWare SPI master
-
-Required properties:
-- compatible: should be "snps,designware-spi"
-- #address-cells: see spi-bus.txt
-- #size-cells: see spi-bus.txt
-- reg: address and length of the spi master registers
-- interrupts: should contain one interrupt
-- clocks: spi clock phandle
-- num-cs: see spi-bus.txt
-
-Optional properties:
-- cs-gpios: see spi-bus.txt
-
-Example:
-
-spi: spi@4020a000 {
-	compatible = "snps,designware-spi";
-	interrupts = <11 1>;
-	reg = <0x4020a000 0x1000>;
-	clocks = <&pclk>;
-	num-cs = <2>;
-	cs-gpios = <&banka 0 0>;
-};
-- 
2.25.1

Re: [PATCH v4 06/16] spi: dw: Parameterize the DMA Rx/Tx burst length

[Andy Shevchenko]

On Fri, May 22, 2020 at 03:07:55AM +0300, Serge Semin wrote:
> It isn't good to have numeric literals in the code especially if there
> are multiple of them and they are related. Let's replace the Tx and Rx
> burst level literals with the corresponding constants.


You missed my tag.

> Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> 
> ---
> 
> Changelog v3:
> - Discard the dws->fifo_len utilization in the Tx FIFO DMA threshold
>   setting.
> ---
>  drivers/spi/spi-dw-mid.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
> index c39bc8758339..1598c36c905f 100644
> --- a/drivers/spi/spi-dw-mid.c
> +++ b/drivers/spi/spi-dw-mid.c
> @@ -20,7 +20,9 @@
>  
>  #define WAIT_RETRIES	5
>  #define RX_BUSY		0
> +#define RX_BURST_LEVEL	16
>  #define TX_BUSY		1
> +#define TX_BURST_LEVEL	16
>  
>  static bool mid_spi_dma_chan_filter(struct dma_chan *chan, void *param)
>  {
> @@ -198,7 +200,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws,
>  	memset(&txconf, 0, sizeof(txconf));
>  	txconf.direction = DMA_MEM_TO_DEV;
>  	txconf.dst_addr = dws->dma_addr;
> -	txconf.dst_maxburst = 16;
> +	txconf.dst_maxburst = TX_BURST_LEVEL;
>  	txconf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>  	txconf.dst_addr_width = convert_dma_width(dws->n_bytes);
>  	txconf.device_fc = false;
> @@ -273,7 +275,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_rx(struct dw_spi *dws,
>  	memset(&rxconf, 0, sizeof(rxconf));
>  	rxconf.direction = DMA_DEV_TO_MEM;
>  	rxconf.src_addr = dws->dma_addr;
> -	rxconf.src_maxburst = 16;
> +	rxconf.src_maxburst = RX_BURST_LEVEL;
>  	rxconf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>  	rxconf.src_addr_width = convert_dma_width(dws->n_bytes);
>  	rxconf.device_fc = false;
> @@ -298,8 +300,8 @@ static int mid_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
>  {
>  	u16 imr = 0, dma_ctrl = 0;
>  
> -	dw_writel(dws, DW_SPI_DMARDLR, 0xf);
> -	dw_writel(dws, DW_SPI_DMATDLR, 0x10);
> +	dw_writel(dws, DW_SPI_DMARDLR, RX_BURST_LEVEL - 1);
> +	dw_writel(dws, DW_SPI_DMATDLR, TX_BURST_LEVEL);
>  
>  	if (xfer->tx_buf) {
>  		dma_ctrl |= SPI_DMA_TDMAE;
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 12/16] spi: dw: Add DW SPI DMA/PCI/MMIO dependency on the DW SPI core

[Andy Shevchenko]

On Fri, May 22, 2020 at 03:08:01AM +0300, Serge Semin wrote:
> Seeing all of the DW SPI driver components like DW SPI DMA/PCI/MMIO
> depend on the DW SPI core code it's better to use the if-endif
> conditional kernel config statement to signify that common dependency.
> 
> Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>

> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Here and for the future, when you add somebody's tag, drop their appearance in
Cc. git-send-email automatically converts known tags to Cc.

> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 12/16] spi: dw: Add DW SPI DMA/PCI/MMIO dependency on the DW SPI core

[Andy Shevchenko]

On Fri, May 22, 2020 at 02:07:38PM +0300, Andy Shevchenko wrote:
> On Fri, May 22, 2020 at 03:08:01AM +0300, Serge Semin wrote:
> > Seeing all of the DW SPI driver components like DW SPI DMA/PCI/MMIO
> > depend on the DW SPI core code it's better to use the if-endif
> > conditional kernel config statement to signify that common dependency.
> > 
> > Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> > Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> > Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> 
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Here and for the future, when you add somebody's tag, drop their appearance in
> Cc. git-send-email automatically converts known tags to Cc.

*) here == in this entire series.

> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: devicetree@vger.kernel.org

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Andy Shevchenko]

On Fri, May 22, 2020 at 03:07:50AM +0300, Serge Semin wrote:
> Since DMA transfers are performed asynchronously with actual SPI
> transaction, then even if DMA transfers are finished it doesn't mean
> all data is actually pushed to the SPI bus. Some data might still be
> in the controller FIFO. This is specifically true for Tx-only
> transfers. In this case if the next SPI transfer is recharged while
> a tail of the previous one is still in FIFO, we'll loose that tail
> data. In order to fix this lets add the wait procedure of the Tx/Rx
> SPI transfers completion after the corresponding DMA transactions
> are finished.

...

> Fixes: 7063c0d942a1 ("spi/dw_spi: add DMA support")

Usually we put this before any other tags.

> Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Cc: Rob Herring <robh+dt@kernel.org>

Are you sure Rob needs this to see?
You really need to shrink Cc lists of the patches to send them on common sense basis.

> Cc: linux-mips@vger.kernel.org

> Cc: devicetree@vger.kernel.org

Ditto.

...

> Changelog v4:
> - Get back ndelay() method to wait for an SPI transfer completion.
>   spi_delay_exec() isn't suitable for the atomic context.

OTOH we may teach spi_delay_exec() to perform atomic sleeps.

...

> +	while (dw_spi_dma_tx_busy(dws) && retry--)
> +		ndelay(ns);

I might be mistaken, but I think I told that this one misses to keep power
management in mind.

Have you read Documentation/process/volatile-considered-harmful.rst ?

...

> +	while (dw_spi_dma_rx_busy(dws) && retry--)
> +		ndelay(ns);

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]

On Fri, May 22, 2020 at 02:52:35PM +0300, Serge Semin wrote:
> On Fri, May 22, 2020 at 02:13:40PM +0300, Andy Shevchenko wrote:

> > > Changelog v4:
> > > - Get back ndelay() method to wait for an SPI transfer completion.
> > >   spi_delay_exec() isn't suitable for the atomic context.

> > OTOH we may teach spi_delay_exec() to perform atomic sleeps.

> Please, see it's implementation. It does atomic delay when the delay value
> is less than 10us. But selectively gets to the usleep_range() if value is
> greater than that.

Yes, I hadn't realised this was in atomic context - _delay_exec() is
just not safe to use there, it'll swich to a sleeping delay if the time
is long enough.

> > > +	while (dw_spi_dma_tx_busy(dws) && retry--)
> > > +		ndelay(ns);

> > I might be mistaken, but I think I told that this one misses to keep power
> > management in mind.

> Here we already in nearly atomic context due to the callback executed in the
> tasklet. What power management could be during a tasklet execution? Again we
> can't call sleeping methods in here. What do you suggest in substitution?

You'd typically have a cpu_relax() in there as well as the ndelay().

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Andy Shevchenko]

On Fri, May 22, 2020 at 02:52:35PM +0300, Serge Semin wrote:
> On Fri, May 22, 2020 at 02:13:40PM +0300, Andy Shevchenko wrote:
> > On Fri, May 22, 2020 at 03:07:50AM +0300, Serge Semin wrote:
> > > Since DMA transfers are performed asynchronously with actual SPI
> > > transaction, then even if DMA transfers are finished it doesn't mean
> > > all data is actually pushed to the SPI bus. Some data might still be
> > > in the controller FIFO. This is specifically true for Tx-only
> > > transfers. In this case if the next SPI transfer is recharged while
> > > a tail of the previous one is still in FIFO, we'll loose that tail
> > > data. In order to fix this lets add the wait procedure of the Tx/Rx
> > > SPI transfers completion after the corresponding DMA transactions
> > > are finished.

...

> > > Changelog v4:
> > > - Get back ndelay() method to wait for an SPI transfer completion.
> > >   spi_delay_exec() isn't suitable for the atomic context.
> > 
> > OTOH we may teach spi_delay_exec() to perform atomic sleeps.
> 
> Please, see it's implementation. It does atomic delay when the delay value
> is less than 10us. But selectively gets to the usleep_range() if value is
> greater than that.

Oh, than it means we may do a very long busy loop here which is not good at
all. If we have 10Hz clock, it might take seconds of doing nothing!

...

> > > +	while (dw_spi_dma_tx_busy(dws) && retry--)
> > > +		ndelay(ns);
> > 
> > I might be mistaken, but I think I told that this one misses to keep power
> > management in mind.
> 
> Here we already in nearly atomic context due to the callback executed in the
> tasklet. What power management could be during a tasklet execution? Again we
> can't call sleeping methods in here. What do you suggest in substitution?
> 
> > Have you read Documentation/process/volatile-considered-harmful.rst ?
> 
> That's mentoring tone is redundant. Please, stop it.

I simple gave you pointers to where you may read about power management in busy
loops. Yes, I admit that documentation title and the relation to busy loops is
not obvious.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 770 bytes --]

On Fri, May 22, 2020 at 03:12:21PM +0300, Andy Shevchenko wrote:
> On Fri, May 22, 2020 at 02:52:35PM +0300, Serge Semin wrote:

> > Please, see it's implementation. It does atomic delay when the delay value
> > is less than 10us. But selectively gets to the usleep_range() if value is
> > greater than that.

> Oh, than it means we may do a very long busy loop here which is not good at
> all. If we have 10Hz clock, it might take seconds of doing nothing!

Realistically it seems unlikely that the clock will be even as slow as
double digit kHz though, and if we do I'd not be surprised to see other
problems kicking in.  It's definitely good to handle such things if we
can but so long as everything is OK for realistic use cases I'm not sure
it should be a blocker.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Andy Shevchenko]

On Fri, May 22, 2020 at 01:18:20PM +0100, Mark Brown wrote:
> On Fri, May 22, 2020 at 03:12:21PM +0300, Andy Shevchenko wrote:
> > On Fri, May 22, 2020 at 02:52:35PM +0300, Serge Semin wrote:
> 
> > > Please, see it's implementation. It does atomic delay when the delay value
> > > is less than 10us. But selectively gets to the usleep_range() if value is
> > > greater than that.
> 
> > Oh, than it means we may do a very long busy loop here which is not good at
> > all. If we have 10Hz clock, it might take seconds of doing nothing!
> 
> Realistically it seems unlikely that the clock will be even as slow as
> double digit kHz though, and if we do I'd not be surprised to see other
> problems kicking in.  It's definitely good to handle such things if we
> can but so long as everything is OK for realistic use cases I'm not sure
> it should be a blocker.

Perhaps some kind of warning? Funny that using spi_delay_exec() will issue such
a warning as a side effect of its implementation.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Andy Shevchenko]

On Fri, May 22, 2020 at 05:00:25PM +0300, Serge Semin wrote:
> On Fri, May 22, 2020 at 04:27:43PM +0300, Serge Semin wrote:
> > On Fri, May 22, 2020 at 02:10:13PM +0100, Mark Brown wrote:
> > > On Fri, May 22, 2020 at 03:44:06PM +0300, Serge Semin wrote:
> > > > On Fri, May 22, 2020 at 03:34:27PM +0300, Andy Shevchenko wrote:

...

> > > > > > Realistically it seems unlikely that the clock will be even as slow as
> > > > > > double digit kHz though, and if we do I'd not be surprised to see other
> > > > > > problems kicking in.  It's definitely good to handle such things if we
> > > > > > can but so long as everything is OK for realistic use cases I'm not sure
> > > > > > it should be a blocker.
> > > 
> > > > As I see it the only way to fix the problem for any use-case is to move the
> > > > busy-wait loop out from the tasklet's callback, add a completion variable to the
> > > > DW SPI data and wait for all the DMA transfers completion in the
> > > > dw_spi_dma_transfer() method. Then execute both busy-wait loops (there we can
> > > > use spi_delay_exec() since it's a work-thread) and call
> > > > spi_finalize_current_transfer() after it. What do you think?
> > > 
> > > I'm concerned that this will add latency for the common case to handle a
> > > potential issue for unrealistically slow buses but yeah, if it's an
> > > issue kicking up to task context is how you'd handle it.
> > 
> > I am not that worried about the latency (most likely it'll be the same as
> > before), but I am mostly concerned regarding a most likely need to re-implement
> > a local version spi_transfer_wait(). We can't afford wait for the completion
> > indefinitely here, so the wait_for_completion_timeout() should be used, for which
> > I would have to calculate a decent timeout based on the transfer capabilities,
> > etc. So basically it would mean to partly copy the spi_transfer_wait() to this
> > module.(
> 
> I'd also wait for Andy's suggestion regarding this, since he's been worried
> about the delay length in the first place. So he may come up with a better
> solution in this regard.

The completion approach sounds quite heavy to me.

Since we haven't got any report for such an issue, I prefer as simplest as
possible approach.

If we add might_sleep() wouldn't it be basically reimplementation of the
spi_delay_exec() again?

And second question, do you experience this warning on your system?

My point is: let's warn and see if anybody comes with a bug report. We will
solve an issue when it appears.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]

On Fri, May 22, 2020 at 05:45:42PM +0300, Serge Semin wrote:
> On Fri, May 22, 2020 at 05:36:39PM +0300, Andy Shevchenko wrote:

> > My point is: let's warn and see if anybody comes with a bug report. We will
> > solve an issue when it appears.

> In my environment the stack trace happened (strictly speaking it has been a
> BUG() invoked due to the sleep_range() called within the tasklet) when SPI bus
> had been enabled to work with !8MHz! clock. It's quite normal bus speed.
> So we'll get the bug report pretty soon.)

Right, that definitely needs to be fixed then - 8MHz is indeed a totally
normal clock rate for SPI so people will hit it.  I guess if there's a
noticable performance hit to defer to thread then we could implement
both and look at how long the delay is going to be to decide which to
use, that's annoyingly complicated though so if the overhead is small
enough we could just not bother.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Serge Semin]

On Fri, May 22, 2020 at 04:22:41PM +0100, Mark Brown wrote:
> On Fri, May 22, 2020 at 05:45:42PM +0300, Serge Semin wrote:
> > On Fri, May 22, 2020 at 05:36:39PM +0300, Andy Shevchenko wrote:
> 
> > > My point is: let's warn and see if anybody comes with a bug report. We will
> > > solve an issue when it appears.
> 
> > In my environment the stack trace happened (strictly speaking it has been a
> > BUG() invoked due to the sleep_range() called within the tasklet) when SPI bus
> > had been enabled to work with !8MHz! clock. It's quite normal bus speed.
> > So we'll get the bug report pretty soon.)
> 
> Right, that definitely needs to be fixed then - 8MHz is indeed a totally
> normal clock rate for SPI so people will hit it.  I guess if there's a
> noticable performance hit to defer to thread then we could implement
> both and look at how long the delay is going to be to decide which to
> use, that's annoyingly complicated though so if the overhead is small
> enough we could just not bother.

As I suggested before we can implement a solution without performance drop.
Just wait for the DMA completion locally in the dw_spi_dma_transfer() method and
return 0 instead of 1 from the transfer_one() callback. In that function we'll
wait while DMA finishes its business, after that we can check the Tx/Rx FIFO
emptiness and wait for the data to be completely transferred with delays or
sleeps or whatever.

There are several drawback of the solution:
1) We need to alter the dw_spi_transfer_one() method in a way one would return
0 instead of 1 (for DMA) so the generic spi_transfer_one_message() method would
be aware that the transfer has been finished and it doesn't need to wait by
calling the spi_transfer_wait() method.
2) Locally in the dw_spi_dma_transfer() I have to implement a method similar
to the spi_transfer_wait(). It won't be that similar though. We can predict a
completion timeout better in here due to using a more exact SPI bus frequency.
Anyway in the rest of aspects the functions will be nearly the same. 
3) Not using spi_transfer_wait() means we also have to locally add the SPI
timeout statistics incremental.

So to speak the local wait method will be like this:

+static int dw_spi_dma_wait(struct dw_spi *dws, struct spi_transfer *xfer)
+{
+ 	struct spi_statistics *statm = &dws->master->statistics;
+	struct spi_statistics *stats = &dws->master->cur_msg->spi->statistics;
+	unsigned long ms = 1;
+
+	ms = xfer->len * MSEC_PER_SEC * BITS_PER_BYTE;
+	ms /= xfer->effective_speed_hz;
+	ms += ms + 200;
+
+	ms = wait_for_completion_timeout(&dws->xfer_completion,
+					msecs_to_jiffies(ms));
+
+	if (ms == 0) {
+		SPI_STATISTICS_INCREMENT_FIELD(statm, timedout);
+		SPI_STATISTICS_INCREMENT_FIELD(stats, timedout);
+		dev_err(&dws->master->cur_msg->spi->dev,
+			"SPI transfer timed out\n");
+			return -ETIMEDOUT;
+	}
+}

NOTE Currently the DW APB SSI driver doesn't set xfer->effective_speed_hz, though as
far as I can see that field exists there to be initialized by the SPI controller
driver, right? If so, strange it isn't done in any SPI drivers...

Then we can use that method to wait for the DMA transfers completion:

+static int dw_spi_dma_transfer(struct dw_spi *dws, struct spi_transfer *xfer)
+{
+	...
+	/* DMA channels/buffers preparation and the transfers execution */
+	...
+
+	ret = dw_spi_dma_wait(dws, xfer);
+	if (ret)
+		return ret;
+
+	ret = dw_spi_dma_wait_tx_done(dws);
+	if (ret)
+		return ret;
+
+	ret = dw_spi_dma_wait_rx_done(dws);
+	if (ret)
+		return ret;
+
+	return 0;
+}

What do think about this?

If you don't mind I'll send this fixup separately from the patchset we discuss
here, since it's going to be a series of patches. What would be better for you:
implement it based on the current DW APB SSI driver, or on top of this
patchset "spi: dw: Add generic DW DMA controller support" (it's being under
review in this email thread) ? Anyway, if the fixup is getting to be that
complicated, will it have to be backported to another stable kernels?

-Sergey

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1784 bytes --]

On Sat, May 23, 2020 at 11:34:10AM +0300, Serge Semin wrote:
> On Fri, May 22, 2020 at 04:22:41PM +0100, Mark Brown wrote:

> > Right, that definitely needs to be fixed then - 8MHz is indeed a totally
> > normal clock rate for SPI so people will hit it.  I guess if there's a
> > noticable performance hit to defer to thread then we could implement
> > both and look at how long the delay is going to be to decide which to
> > use, that's annoyingly complicated though so if the overhead is small
> > enough we could just not bother.

> As I suggested before we can implement a solution without performance drop.
> Just wait for the DMA completion locally in the dw_spi_dma_transfer() method and
> return 0 instead of 1 from the transfer_one() callback. In that function we'll
> wait while DMA finishes its business, after that we can check the Tx/Rx FIFO
> emptiness and wait for the data to be completely transferred with delays or
> sleeps or whatever.

No extra context switches there at least, that's the main issue.

> NOTE Currently the DW APB SSI driver doesn't set xfer->effective_speed_hz, though as
> far as I can see that field exists there to be initialized by the SPI controller
> driver, right? If so, strange it isn't done in any SPI drivers...

Yes.  Not that many people are concerned about the exact timing it turns
out, the work that was being used for never fully made it upstream.

> What do think about this?

Sure.

> patchset "spi: dw: Add generic DW DMA controller support" (it's being under
> review in this email thread) ? Anyway, if the fixup is getting to be that
> complicated, will it have to be backported to another stable kernels?

No, if it's too invasive it shouldn't be (though the stable people might
decide they want it anyway these days :/ ).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

[Serge Semin]

On Mon, May 25, 2020 at 12:41:32PM +0100, Mark Brown wrote:
> On Sat, May 23, 2020 at 11:34:10AM +0300, Serge Semin wrote:
> > On Fri, May 22, 2020 at 04:22:41PM +0100, Mark Brown wrote:
> 
> > > Right, that definitely needs to be fixed then - 8MHz is indeed a totally
> > > normal clock rate for SPI so people will hit it.  I guess if there's a
> > > noticable performance hit to defer to thread then we could implement
> > > both and look at how long the delay is going to be to decide which to
> > > use, that's annoyingly complicated though so if the overhead is small
> > > enough we could just not bother.
> 
> > As I suggested before we can implement a solution without performance drop.
> > Just wait for the DMA completion locally in the dw_spi_dma_transfer() method and
> > return 0 instead of 1 from the transfer_one() callback. In that function we'll
> > wait while DMA finishes its business, after that we can check the Tx/Rx FIFO
> > emptiness and wait for the data to be completely transferred with delays or
> > sleeps or whatever.
> 
> No extra context switches there at least, that's the main issue.

Right. There won't be extra context switch.

> 
> > NOTE Currently the DW APB SSI driver doesn't set xfer->effective_speed_hz, though as
> > far as I can see that field exists there to be initialized by the SPI controller
> > driver, right? If so, strange it isn't done in any SPI drivers...
> 
> Yes.  Not that many people are concerned about the exact timing it turns
> out, the work that was being used for never fully made it upstream.
> 
> > What do think about this?
> 
> Sure.

Great. I'll send a new patchset soon. It'll fix the Tx/Rx non-empty issue in
accordance with the proposed design.

-Sergey

> 
> > patchset "spi: dw: Add generic DW DMA controller support" (it's being under
> > review in this email thread) ? Anyway, if the fixup is getting to be that
> > complicated, will it have to be backported to another stable kernels?
> 
> No, if it's too invasive it shouldn't be (though the stable people might
> decide they want it anyway these days :/ ).

Re: [PATCH v4 16/16] dt-bindings: spi: Convert DW SPI binding to DT schema

[Rob Herring]

On Fri, 22 May 2020 03:08:05 +0300, Serge Semin wrote:
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces two DW SPI legacy
> bare text bindings with YAML file. As before the bindings file states
> that the corresponding dts node is supposed to be compatible either
> with generic DW APB SSI controller or with Microsemi/Amazon/Renesas/Intel
> vendors-specific controllers, to have registers, interrupts and clocks
> properties. Though in case of Microsemi version of the controller
> there must be two registers resources specified. Properties like
> clock-names, reg-io-width, cs-gpio, num-cs, DMA and slave device
> sub-nodes are optional.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: linux-mips@vger.kernel.org
> ---
>  .../bindings/spi/snps,dw-apb-ssi.txt          |  44 ------
>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 127 ++++++++++++++++++
>  .../devicetree/bindings/spi/spi-dw.txt        |  24 ----
>  3 files changed, 127 insertions(+), 68 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
>  create mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-dw.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>