| Message ID | 20200515104758.6934-1-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| Headers | show |
| Series | spi: dw: Add generic DW DMA controller support | expand |
On Fri, May 15, 2020 at 01:47:39PM +0300, Serge Semin wrote: > 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") Thanks! I'm going to review it soon. Hint for the next time, please start always a new thread with new version. > 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: Allison Randal <allison@lohutok.net> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Gareth Williams <gareth.williams.jx@renesas.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 > > --- > > 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. > > Serge Semin (19): > dt-bindings: spi: dw: Add Tx/Rx DMA properties > spi: dw: Add Tx/Rx finish wait methods to the MID DMA > spi: dw: Clear DMAC register when done or stopped > spi: dw: Fix native CS being unset > 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: Initialize paddr in DW SPI MMIO private data > spi: dw: Fix Rx-only DMA transfers > 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 | 42 --- > .../bindings/spi/snps,dw-apb-ssi.yaml | 127 +++++++++ > .../devicetree/bindings/spi/spi-dw.txt | 24 -- > drivers/spi/Kconfig | 15 +- > drivers/spi/Makefile | 7 +- > drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} | 257 ++++++++++-------- > drivers/spi/spi-dw-mmio.c | 9 +- > drivers/spi/spi-dw-pci.c | 50 +++- > drivers/spi/spi-dw.c | 98 +++---- > drivers/spi/spi-dw.h | 33 ++- > 10 files changed, 405 insertions(+), 257 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-mid.c => spi-dw-dma.c} (53%) > > -- > 2.25.1 >
On Fri, May 15, 2020 at 01:47:41PM +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. General question, doesn't spi core provides us some helpers like spi_delay_exec()? > 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: Allison Randal <allison@lohutok.net> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Gareth Williams <gareth.williams.jx@renesas.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > > --- > > Changelog v2: > - Use conditional statement instead of the ternary operator in the ref > clock getter. > - Move the patch to the head of the series so one could be picked up to > the stable kernels as a fix. You forgot a Fixes tag. > --- > drivers/spi/spi-dw-mid.c | 50 ++++++++++++++++++++++++++++++++++++++++ > drivers/spi/spi-dw.h | 10 ++++++++ > 2 files changed, 60 insertions(+) > > diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c > index 177e1f5ec62b..7a5ae1506365 100644 > --- a/drivers/spi/spi-dw-mid.c > +++ b/drivers/spi/spi-dw-mid.c > @@ -16,7 +16,9 @@ > #include <linux/irqreturn.h> > #include <linux/pci.h> > #include <linux/platform_data/dma-dw.h> > +#include <linux/delay.h> Keep it in order. > > +#define WAIT_RETRIES 5 > #define RX_BUSY 0 > #define TX_BUSY 1 > > @@ -141,6 +143,28 @@ static enum dma_slave_buswidth convert_dma_width(u32 dma_width) { > return DMA_SLAVE_BUSWIDTH_UNDEFINED; > } > > +static inline bool dw_spi_dma_tx_busy(struct dw_spi *dws) > +{ > + return !(dw_readl(dws, DW_SPI_SR) & SR_TF_EMPT); > +} > + > +static void dw_spi_dma_wait_tx_done(struct dw_spi *dws) > +{ > + int retry = WAIT_RETRIES; > + unsigned long ns; > + > + ns = (NSEC_PER_SEC / spi_get_clk(dws)) * dws->n_bytes * BITS_PER_BYTE; > + ns *= dw_readl(dws, DW_SPI_TXFLR); > + > + while (dw_spi_dma_tx_busy(dws) && retry--) > + ndelay(ns); This misses power management for CPU and do you really need this to be atomic? At the end why not to use readx_poll_timeout() ? > + if (retry < 0) { Usually we do unsigned int retries = NNNN; do { ... } while (--retries); if (!retries) ... But in any case, see above. > + dev_err(&dws->master->dev, "Tx hanged up\n"); > + dws->master->cur_msg->status = -EIO; > + } > +} Same comments to Rx part. > + > /* > * dws->dma_chan_busy is set before the dma transfer starts, callback for tx > * channel will clear a corresponding bit. > @@ -149,6 +173,8 @@ static void dw_spi_dma_tx_done(void *arg) > { > struct dw_spi *dws = arg; > > + dw_spi_dma_wait_tx_done(dws); > + > clear_bit(TX_BUSY, &dws->dma_chan_busy); > if (test_bit(RX_BUSY, &dws->dma_chan_busy)) > return; > @@ -188,6 +214,28 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws, > return txdesc; > } > > +static inline bool dw_spi_dma_rx_busy(struct dw_spi *dws) > +{ > + return !!(dw_readl(dws, DW_SPI_SR) & SR_RF_NOT_EMPT); > +} > + > +static void dw_spi_dma_wait_rx_done(struct dw_spi *dws) > +{ > + int retry = WAIT_RETRIES; > + unsigned long ns; > + > + ns = (NSEC_PER_SEC / spi_get_clk(dws)) * dws->n_bytes * BITS_PER_BYTE; > + ns *= dw_readl(dws, DW_SPI_RXFLR); > + > + while (dw_spi_dma_rx_busy(dws) && retry--) > + ndelay(ns); > + > + if (retry < 0) { > + dev_err(&dws->master->dev, "Rx hanged up\n"); > + dws->master->cur_msg->status = -EIO; > + } > +} > + > /* > * dws->dma_chan_busy is set before the dma transfer starts, callback for rx > * channel will clear a corresponding bit. > @@ -196,6 +244,8 @@ static void dw_spi_dma_rx_done(void *arg) > { > struct dw_spi *dws = arg; > > + dw_spi_dma_wait_rx_done(dws); > + > clear_bit(RX_BUSY, &dws->dma_chan_busy); > if (test_bit(TX_BUSY, &dws->dma_chan_busy)) > return; > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index e92d43b9a9e6..81364f501b7e 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -210,6 +210,16 @@ static inline void spi_set_clk(struct dw_spi *dws, u16 div) > dw_writel(dws, DW_SPI_BAUDR, div); > } > > +static inline u32 spi_get_clk(struct dw_spi *dws) > +{ > + u32 div = dw_readl(dws, DW_SPI_BAUDR); > + > + if (!div) > + return 0; > + > + return dws->max_freq / div; > +} > + > /* Disable IRQ bits */ > static inline void spi_mask_intr(struct dw_spi *dws, u32 mask) > { > -- > 2.25.1 >
On Fri, May 15, 2020 at 01:47:43PM +0300, Serge Semin wrote: > Commit 6e0a32d6f376 ("spi: dw: Fix default polarity of native > chipselect") attempted to fix the problem when GPIO active-high > chip-select is utilized to communicate with some SPI slave. It fixed > the problem, but broke the normal native CS support. At the same time > the reversion commit ada9e3fcc175 ("spi: dw: Correct handling of native > chipselect") didn't solve the problem either, since it just inverted > the set_cs() polarity perception without taking into account that > CS-high might be applicable. Here is what is done to finally fix the > problem. > > DW SPI controller demands any native CS being set in order to proceed > with data transfer. So in order to activate the SPI communications we > must set any bit in the Slave Select DW SPI controller register no > matter whether the platform requests the GPIO- or native CS. Preferably > it should be the bit corresponding to the SPI slave CS number. But > currently the dw_spi_set_cs() method activates the chip-select > only if the second argument is false. Since the second argument of the > set_cs callback is expected to be a boolean with "is-high" semantics > (actual chip-select pin state value), the bit in the DW SPI Slave > Select register will be set only if SPI core requests the driver > to set the CS in the low state. So this will work for active-low > GPIO-based CS case, and won't work for active-high CS setting > the bit when SPI core actually needs to deactivate the CS. > > This commit fixes the problem for all described cases. So no matter > whether an SPI slave needs GPIO- or native-based CS with active-high > or low signal the corresponding bit will be set in SER. Nice catch! Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Fixes: ada9e3fcc175 ("spi: dw: Correct handling of native chipselect") > Fixes: 6e0a32d6f376 ("spi: dw: Fix default polarity of native chipselect") > Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com> > Acked-by: Linus Walleij <linus.walleij@linaro.org> > 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: Allison Randal <allison@lohutok.net> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Gareth Williams <gareth.williams.jx@renesas.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > > --- > > Changelog v2: > - Add a comment about SER register, that it must be set to activate the > SPI communications. > --- > drivers/spi/spi-dw.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c > index 6de196df9c96..450c8218caeb 100644 > --- a/drivers/spi/spi-dw.c > +++ b/drivers/spi/spi-dw.c > @@ -124,8 +124,16 @@ static inline void dw_spi_debugfs_remove(struct dw_spi *dws) > void dw_spi_set_cs(struct spi_device *spi, bool enable) > { > struct dw_spi *dws = spi_controller_get_devdata(spi->controller); > + bool cs_high = !!(spi->mode & SPI_CS_HIGH); > > - if (!enable) > + /* > + * DW SPI controller demands any native CS being set in order to > + * proceed with data transfer. So in order to activate the SPI > + * communications we must set a corresponding bit in the Slave > + * Enable register no matter whether the SPI core is configured to > + * support active-high or active-low CS level. > + */ > + if (cs_high == enable) > dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select)); > else if (dws->cs_override) > dw_writel(dws, DW_SPI_SER, 0); > -- > 2.25.1 >
On Fri, May 15, 2020 at 03:01:11PM +0300, Andy Shevchenko wrote: > General question, doesn't spi core provides us some helpers like > spi_delay_exec()? Well, nobody wrote one. It's also a bit tricky to handle given that often you're checking some controller specific things while a FIFO in the IP drains/fills, though nothing insurmountable.
On Fri, May 15, 2020 at 01:47:45PM +0300, Serge Semin wrote: > Having them declared is redundant since each struct dw_dma_chan has > the same structure embedded and the structure from the passed dma_chan > private pointer will be copied there as a result of the next calls > chain: > dma_request_channel() -> find_candidate() -> dma_chan_get() -> > device_alloc_chan_resources() = dwc_alloc_chan_resources() -> > dw_dma_filter(). > So just remove the static dw_dma_chan structures and use a locally > declared data instance with dst_id/src_id set to the same values as > the static copies used to have. ... > -static struct dw_dma_slave mid_dma_tx = { .dst_id = 1 }; > -static struct dw_dma_slave mid_dma_rx = { .src_id = 0 }; > + struct dw_dma_slave slave = {0}; I really would like to leave them separated and as in the original form, i.e. struct dw_dma_slave tx = { .dst_id = 1 }; struct dw_dma_slave rx = { .src_id = 0 }; those src and dst IDs are put in that form on purpose... > + /* 1. Init rx channel (.src_id = 0, .dst_id = 0) */ ...this comment adds a bit of confusion. (Needs more time to parse and understand what IDs are in use) > + slave.dma_dev = &dma_dev->dev; > + dws->rxchan = dma_request_channel(mask, mid_spi_dma_chan_filter, &slave); > + /* 2. Init tx channel (.src_id = 0, .dst_id = 1) */ Ditto. P.S. Just a recommendation for the future: in all your patches try to be less invasive where it's possible.
On Fri, May 15, 2020 at 01:18:15PM +0100, Mark Brown wrote: > On Fri, May 15, 2020 at 03:01:11PM +0300, Andy Shevchenko wrote: > > > General question, doesn't spi core provides us some helpers like > > spi_delay_exec()? > > Well, nobody wrote one. spi_delay_exec() seems quite good to be used here. Can we use it for delays? > It's also a bit tricky to handle given that > often you're checking some controller specific things while a FIFO in > the IP drains/fills, though nothing insurmountable. Yes, input can be gathered by reading controller specific data.
On Fri, May 15, 2020 at 01:47:46PM +0300, Serge Semin wrote: > 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. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > 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: Allison Randal <allison@lohutok.net> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Gareth Williams <gareth.williams.jx@renesas.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 >
On Fri, May 15, 2020 at 03:37:02PM +0300, Andy Shevchenko wrote: > On Fri, May 15, 2020 at 01:18:15PM +0100, Mark Brown wrote: > > Well, nobody wrote one. > spi_delay_exec() seems quite good to be used here. > Can we use it for delays? I guess we could, though it's really there because for historical reasons we've got a bunch of different ways of specifying delays from client drivers rather than for the executing a delay where you've already got a good idea of the length of the delay.
On Fri, May 15, 2020 at 01:47:47PM +0300, Serge Semin wrote: > This member has exactly the same value as n_bytes of the DW SPI private > data object, it's calculated at the same point of the transfer method, > n_bytes isn't changed during the whole transfer, and they even serve for > the same purpose - keep number of bytes per transfer word, though the > dma_width is used only to calculate the DMA source/destination addresses > width, which n_bytes could be also utilized for. Taking all of these > into account let's replace the dma_width member usage with n_bytes one > and remove the former. I've no strong opinion about this. So, after addressing one issue below, Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > 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: Allison Randal <allison@lohutok.net> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Gareth Williams <gareth.williams.jx@renesas.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-mid.c | 10 +++++----- > drivers/spi/spi-dw.c | 1 - > drivers/spi/spi-dw.h | 1 - > 3 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c > index be886e22a1b1..ca8813a693d8 100644 > --- a/drivers/spi/spi-dw-mid.c > +++ b/drivers/spi/spi-dw-mid.c > @@ -132,10 +132,10 @@ static bool mid_spi_can_dma(struct spi_controller *master, > return xfer->len > dws->fifo_len; > } > > -static enum dma_slave_buswidth convert_dma_width(u32 dma_width) { > - if (dma_width == 1) > +static enum dma_slave_buswidth convert_dma_width(u8 n_bytes) { It seems somebody (maybe even me) at some point messed up between enum definition and function that returns an enum. For what said, { should be on the separate line. > + if (n_bytes == 1) > return DMA_SLAVE_BUSWIDTH_1_BYTE; > - else if (dma_width == 2) > + else if (n_bytes == 2) > return DMA_SLAVE_BUSWIDTH_2_BYTES; > > return DMA_SLAVE_BUSWIDTH_UNDEFINED; > @@ -195,7 +195,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws, > txconf.dst_addr = dws->dma_addr; > txconf.dst_maxburst = 16; > txconf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; > - txconf.dst_addr_width = convert_dma_width(dws->dma_width); > + txconf.dst_addr_width = convert_dma_width(dws->n_bytes); > txconf.device_fc = false; > > dmaengine_slave_config(dws->txchan, &txconf); > @@ -268,7 +268,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_rx(struct dw_spi *dws, > rxconf.src_addr = dws->dma_addr; > rxconf.src_maxburst = 16; > rxconf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; > - rxconf.src_addr_width = convert_dma_width(dws->dma_width); > + rxconf.src_addr_width = convert_dma_width(dws->n_bytes); > rxconf.device_fc = false; > > dmaengine_slave_config(dws->rxchan, &rxconf); > diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c > index 450c8218caeb..1edb8cdd11ee 100644 > --- a/drivers/spi/spi-dw.c > +++ b/drivers/spi/spi-dw.c > @@ -353,7 +353,6 @@ static int dw_spi_transfer_one(struct spi_controller *master, > } > > 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); > > cr0 = dws->update_cr0(master, spi, transfer); > dw_writel(dws, DW_SPI_CTRLR0, cr0); > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index b6ab81e0c747..4902f937c3d7 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -136,7 +136,6 @@ struct dw_spi { > void *rx_end; > int dma_mapped; > u8 n_bytes; /* current is a 1/2 bytes op */ > - u32 dma_width; > irqreturn_t (*transfer_handler)(struct dw_spi *dws); > u32 current_freq; /* frequency in hz */ > > -- > 2.25.1 >
On Fri, May 15, 2020 at 04:05:59PM +0300, Serge Semin wrote: > On Fri, May 15, 2020 at 04:03:05PM +0300, Andy Shevchenko wrote: > > On Fri, May 15, 2020 at 01:47:47PM +0300, Serge Semin wrote: > > > This member has exactly the same value as n_bytes of the DW SPI private > > > data object, it's calculated at the same point of the transfer method, > > > n_bytes isn't changed during the whole transfer, and they even serve for > > > the same purpose - keep number of bytes per transfer word, though the > > > dma_width is used only to calculate the DMA source/destination addresses > > > width, which n_bytes could be also utilized for. Taking all of these > > > into account let's replace the dma_width member usage with n_bytes one > > > and remove the former. > > > > I've no strong opinion about this. > > So, after addressing one issue below, > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> ... > > > -static enum dma_slave_buswidth convert_dma_width(u32 dma_width) { > > > - if (dma_width == 1) > > > > > +static enum dma_slave_buswidth convert_dma_width(u8 n_bytes) { > > > > It seems somebody (maybe even me) at some point messed up between enum > > definition and function that returns an enum. > > > > For what said, { should be on the separate line. > > See the patch 16/19: "spi: dw: Cleanup generic DW DMA code namings" > in this series. Since you are touching that line here, it makes sense to do it here rather than ping-pong to other patch in very same series.
On Fri, May 15, 2020 at 01:47:51PM +0300, Serge Semin wrote: > Tx-only DMA transfers are working perfectly fine since in this case > the code just ignores the Rx FIFO overflow interrupts. But it turns > out the SPI Rx-only transfers are broken since nothing pushing any > data to the shift registers, so the Rx FIFO is left empty and the > SPI core subsystems just returns a timeout error. Since DW DMAC > driver doesn't support something like cyclic write operations of > a single byte to a device register, the only way to support the > Rx-only SPI transfers is to fake it by using a dummy Tx-buffer. > This is what we intend to fix in this commit by setting the > SPI_CONTROLLER_MUST_TX flag for DMA-capable platform. I'm fine with this if Mark considers this right thing to do. So, conditionally Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > 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: Allison Randal <allison@lohutok.net> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Gareth Williams <gareth.williams.jx@renesas.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > --- > drivers/spi/spi-dw.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c > index 1edb8cdd11ee..31607b40147d 100644 > --- a/drivers/spi/spi-dw.c > +++ b/drivers/spi/spi-dw.c > @@ -517,6 +517,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws) > dev_warn(dev, "DMA init failed\n"); > } else { > master->can_dma = dws->dma_ops->can_dma; > + master->flags |= SPI_CONTROLLER_MUST_TX; > } > } > > -- > 2.25.1 >
On Fri, May 15, 2020 at 01:47:52PM +0300, Serge Semin wrote: > This is a preparation patch before adding the DW DMA support into the > DW SPI MMIO driver. We need to unpin the Non-DMA-specific code from the > intended to be generic DW APB SSI DMA code. This isn't that hard, > since the most part of the spi-dw-mid.c driver in fact implements a > generic DMA interface for the DW SPI controller driver. The only Intel > MID specifics concern getting the max frequency from the MRST Clock > Control Unit and fetching the DMA controller channels from > corresponding PCIe DMA controller. Since first one is related with the > SPI interface configuration we moved it' implementation into the > DW PCIe-SPI driver module. After that former spi-dw-mid.c file > can be just renamed to be the DW SPI DMA module optionally compiled in to > the DW APB SSI core driver. Cc list here is huge! I think this patch should go immediately after bunch of fixes. ... > -obj-$(CONFIG_SPI_DESIGNWARE) += spi-dw.o > +obj-$(CONFIG_SPI_DESIGNWARE) += spi-dw-core.o > +spi-dw-core-y := spi-dw.o > +spi-dw-core-$(CONFIG_SPI_DW_DMA) += spi-dw-dma.o We may leave module name the same, right? obj-$(CONFIG_SPI_DESIGNWARE) += spi-dw.o spi-dw-y := spi-dw-core.o spi-dw-$(CONFIG_SPI_DW_DMA) += spi-dw-dma.o > obj-$(CONFIG_SPI_DW_MMIO) += spi-dw-mmio.o > -obj-$(CONFIG_SPI_DW_PCI) += spi-dw-midpci.o > -spi-dw-midpci-objs := spi-dw-pci.o spi-dw-mid.o > +obj-$(CONFIG_SPI_DW_PCI) += spi-dw-pci.o > -/* Some specific info for SPI0 controller on Intel MID */ > - > -/* HW info for MRST Clk Control Unit, 32b reg per controller */ > -#define MRST_SPI_CLK_BASE 100000000 /* 100m */ > -#define MRST_CLK_SPI_REG 0xff11d86c > -#define CLK_SPI_BDIV_OFFSET 0 > -#define CLK_SPI_BDIV_MASK 0x00000007 > -#define CLK_SPI_CDIV_OFFSET 9 > -#define CLK_SPI_CDIV_MASK 0x00000e00 > -#define CLK_SPI_DISABLE_OFFSET 8 > - > -int dw_spi_mid_init_mfld(struct dw_spi *dws) > -{ > - void __iomem *clk_reg; > - u32 clk_cdiv; > - > - clk_reg = ioremap(MRST_CLK_SPI_REG, 16); > - if (!clk_reg) > - return -ENOMEM; > - > - /* Get SPI controller operating freq info */ > - clk_cdiv = readl(clk_reg + dws->bus_num * sizeof(u32)); > - clk_cdiv &= CLK_SPI_CDIV_MASK; > - clk_cdiv >>= CLK_SPI_CDIV_OFFSET; > - dws->max_freq = MRST_SPI_CLK_BASE / (clk_cdiv + 1); > - > - iounmap(clk_reg); > - > - /* Register hook to configure CTRLR0 */ > - dws->update_cr0 = dw_spi_update_cr0; > - > - dw_spi_mid_setup_dma_mfld(dws); > - return 0; > -} > - > -int dw_spi_mid_init_generic(struct dw_spi *dws) > -{ > - /* Register hook to configure CTRLR0 */ > - dws->update_cr0 = dw_spi_update_cr0; > - > - dw_spi_mid_setup_dma_generic(dws); > - return 0; > -} > +EXPORT_SYMBOL_GPL(dw_spi_mid_setup_dma_generic); > diff --git a/drivers/spi/spi-dw-pci.c b/drivers/spi/spi-dw-pci.c > index dde54a918b5d..c13707b8493e 100644 > --- a/drivers/spi/spi-dw-pci.c > +++ b/drivers/spi/spi-dw-pci.c > @@ -15,6 +15,15 @@ > > #define DRIVER_NAME "dw_spi_pci" > > +/* HW info for MRST Clk Control Unit, 32b reg per controller */ > +#define MRST_SPI_CLK_BASE 100000000 /* 100m */ > +#define MRST_CLK_SPI_REG 0xff11d86c > +#define CLK_SPI_BDIV_OFFSET 0 > +#define CLK_SPI_BDIV_MASK 0x00000007 > +#define CLK_SPI_CDIV_OFFSET 9 > +#define CLK_SPI_CDIV_MASK 0x00000e00 > +#define CLK_SPI_DISABLE_OFFSET 8 > + > struct spi_pci_desc { > int (*setup)(struct dw_spi *); > u16 num_cs; > @@ -22,20 +31,55 @@ struct spi_pci_desc { > u32 max_freq; > }; > > +static int spi_mid_init(struct dw_spi *dws) > +{ > + void __iomem *clk_reg; > + u32 clk_cdiv; > + > + clk_reg = ioremap(MRST_CLK_SPI_REG, 16); > + if (!clk_reg) > + return -ENOMEM; > + > + /* Get SPI controller operating freq info */ > + clk_cdiv = readl(clk_reg + dws->bus_num * sizeof(u32)); > + clk_cdiv &= CLK_SPI_CDIV_MASK; > + clk_cdiv >>= CLK_SPI_CDIV_OFFSET; > + dws->max_freq = MRST_SPI_CLK_BASE / (clk_cdiv + 1); > + > + iounmap(clk_reg); > + > + /* Register hook to configure CTRLR0 */ > + dws->update_cr0 = dw_spi_update_cr0; > + > + dw_spi_mid_setup_dma_mfld(dws); > + > + return 0; > +} > + > +static int spi_generic_init(struct dw_spi *dws) > +{ > + /* Register hook to configure CTRLR0 */ > + dws->update_cr0 = dw_spi_update_cr0; > + > + dw_spi_mid_setup_dma_generic(dws); > + > + return 0; > +} > + > static struct spi_pci_desc spi_pci_mid_desc_1 = { > - .setup = dw_spi_mid_init_mfld, > + .setup = spi_mid_init, > .num_cs = 5, > .bus_num = 0, > }; > > static struct spi_pci_desc spi_pci_mid_desc_2 = { > - .setup = dw_spi_mid_init_mfld, > + .setup = spi_mid_init, > .num_cs = 2, > .bus_num = 1, > }; > > static struct spi_pci_desc spi_pci_ehl_desc = { > - .setup = dw_spi_mid_init_generic, > + .setup = spi_generic_init, > .num_cs = 2, > .bus_num = -1, > .max_freq = 100000000, > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index d0c8b7d3a5d2..75fdcc5e7642 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -265,8 +265,16 @@ extern u32 dw_spi_update_cr0_v1_01a(struct spi_controller *master, > struct spi_device *spi, > struct spi_transfer *transfer); > > -/* platform related setup */ > -extern int dw_spi_mid_init_mfld(struct dw_spi *dws); > -extern int dw_spi_mid_init_generic(struct dw_spi *dws); > +#ifdef CONFIG_SPI_DW_DMA > + > +extern void dw_spi_mid_setup_dma_mfld(struct dw_spi *dws); > +extern void dw_spi_mid_setup_dma_generic(struct dw_spi *dws); > + I would drop blank lines and extern keywords. > +#else > + > +static inline void dw_spi_mid_setup_dma_mfld(struct dw_spi *dws) {} > +static inline void dw_spi_mid_setup_dma_generic(struct dw_spi *dws) {} > + Ditto for blank lines. > +#endif /* !CONFIG_SPI_DW_DMA */ > > #endif /* DW_SPI_HEADER_H */ > -- > 2.25.1 >
On Fri, May 15, 2020 at 05:16:27PM +0300, Serge Semin wrote: > On Fri, May 15, 2020 at 04:49:56PM +0300, Andy Shevchenko wrote: > > On Fri, May 15, 2020 at 04:05:59PM +0300, Serge Semin wrote: > > > On Fri, May 15, 2020 at 04:03:05PM +0300, Andy Shevchenko wrote: > > > > On Fri, May 15, 2020 at 01:47:47PM +0300, Serge Semin wrote: > > > > > This member has exactly the same value as n_bytes of the DW SPI private > > > > > data object, it's calculated at the same point of the transfer method, > > > > > n_bytes isn't changed during the whole transfer, and they even serve for > > > > > the same purpose - keep number of bytes per transfer word, though the > > > > > dma_width is used only to calculate the DMA source/destination addresses > > > > > width, which n_bytes could be also utilized for. Taking all of these > > > > > into account let's replace the dma_width member usage with n_bytes one > > > > > and remove the former. > > > > > > > > I've no strong opinion about this. > > > > So, after addressing one issue below, > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > ... > > > > > > > -static enum dma_slave_buswidth convert_dma_width(u32 dma_width) { > > > > > - if (dma_width == 1) > > > > > > > > > +static enum dma_slave_buswidth convert_dma_width(u8 n_bytes) { > > > > > > > > It seems somebody (maybe even me) at some point messed up between enum > > > > definition and function that returns an enum. > > > > > > > > For what said, { should be on the separate line. > > > > > > See the patch 16/19: "spi: dw: Cleanup generic DW DMA code namings" > > > in this series. > > > > Since you are touching that line here, it makes sense to do it here rather than > > ping-pong to other patch in very same series. > > You didn't open the patch I referred to, did you? Patches in the series are going on purpose. I look at them in the sequence of the appearance. But okay, I looked at it and I found what I expected. I think that you may reorder patch 16 to be one right after renaming module. > If you did, you would have > seen that I touched that line there too in the framework of the naming cleanup > procedure. So please, stop wasting my time with trivial stuff. Haven't you missed my tag? It means I spent *my* time on *your* stuff. Please, be respectful to reviewers.
On Fri, May 15, 2020 at 01:47:57PM +0300, Serge Semin wrote: > DebugFS kernel interface provides a dedicated method to create the > registers dump file. Use it instead of creating a generic DebugFS > file with manually written read callback function. With below nit addressed, Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > 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: Allison Randal <allison@lohutok.net> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Gareth Williams <gareth.williams.jx@renesas.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > --- > drivers/spi/spi-dw.c | 86 ++++++++++++++------------------------------ > drivers/spi/spi-dw.h | 2 ++ > 2 files changed, 28 insertions(+), 60 deletions(-) > > diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c > index 31607b40147d..bb470cff40d3 100644 > --- a/drivers/spi/spi-dw.c > +++ b/drivers/spi/spi-dw.c > @@ -29,66 +29,29 @@ struct chip_data { > }; > > #ifdef CONFIG_DEBUG_FS > -#define SPI_REGS_BUFSIZE 1024 > -static ssize_t dw_spi_show_regs(struct file *file, char __user *user_buf, > - size_t count, loff_t *ppos) > -{ > - struct dw_spi *dws = file->private_data; > - char *buf; > - u32 len = 0; > - ssize_t ret; > - > - buf = kzalloc(SPI_REGS_BUFSIZE, GFP_KERNEL); > - if (!buf) > - return 0; > - > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "%s registers:\n", dev_name(&dws->master->dev)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "=================================\n"); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "CTRLR0: \t0x%08x\n", dw_readl(dws, DW_SPI_CTRLR0)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "CTRLR1: \t0x%08x\n", dw_readl(dws, DW_SPI_CTRLR1)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "SSIENR: \t0x%08x\n", dw_readl(dws, DW_SPI_SSIENR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "SER: \t\t0x%08x\n", dw_readl(dws, DW_SPI_SER)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "BAUDR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_BAUDR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "TXFTLR: \t0x%08x\n", dw_readl(dws, DW_SPI_TXFTLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "RXFTLR: \t0x%08x\n", dw_readl(dws, DW_SPI_RXFTLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "TXFLR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_TXFLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "RXFLR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_RXFLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "SR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_SR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "IMR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_IMR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "ISR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_ISR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "DMACR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_DMACR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "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)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "=================================\n"); > - > - ret = simple_read_from_buffer(user_buf, count, ppos, buf, len); > - kfree(buf); > - return ret; > + > +#define DW_SPI_DBGFS_REG(_name, _off) \ > +{ \ > + .name = _name, \ > + .offset = _off \ As previously discussed (did I miss your answer?) the comma at the end leaves better pattern for maintenance prospective. > } > > -static const struct file_operations dw_spi_regs_ops = { > - .owner = THIS_MODULE, > - .open = simple_open, > - .read = dw_spi_show_regs, > - .llseek = default_llseek, > +static const struct debugfs_reg32 dw_spi_dbgfs_regs[] = { > + DW_SPI_DBGFS_REG("CTRLR0", DW_SPI_CTRLR0), > + DW_SPI_DBGFS_REG("CTRLR1", DW_SPI_CTRLR1), > + DW_SPI_DBGFS_REG("SSIENR", DW_SPI_SSIENR), > + DW_SPI_DBGFS_REG("SER", DW_SPI_SER), > + DW_SPI_DBGFS_REG("BAUDR", DW_SPI_BAUDR), > + DW_SPI_DBGFS_REG("TXFTLR", DW_SPI_TXFTLR), > + DW_SPI_DBGFS_REG("RXFTLR", DW_SPI_RXFTLR), > + DW_SPI_DBGFS_REG("TXFLR", DW_SPI_TXFLR), > + DW_SPI_DBGFS_REG("RXFLR", DW_SPI_RXFLR), > + DW_SPI_DBGFS_REG("SR", DW_SPI_SR), > + DW_SPI_DBGFS_REG("IMR", DW_SPI_IMR), > + DW_SPI_DBGFS_REG("ISR", DW_SPI_ISR), > + DW_SPI_DBGFS_REG("DMACR", DW_SPI_DMACR), > + DW_SPI_DBGFS_REG("DMATDLR", DW_SPI_DMATDLR), > + DW_SPI_DBGFS_REG("DMARDLR", DW_SPI_DMARDLR) > }; > > static int dw_spi_debugfs_init(struct dw_spi *dws) > @@ -100,8 +63,11 @@ static int dw_spi_debugfs_init(struct dw_spi *dws) > if (!dws->debugfs) > return -ENOMEM; > > - debugfs_create_file("registers", S_IFREG | S_IRUGO, > - dws->debugfs, (void *)dws, &dw_spi_regs_ops); > + dws->regset.regs = dw_spi_dbgfs_regs; > + dws->regset.nregs = ARRAY_SIZE(dw_spi_dbgfs_regs); > + dws->regset.base = dws->regs; > + debugfs_create_regset32("registers", 0400, dws->debugfs, &dws->regset); > + > return 0; > } > > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index 24462b0c65cb..4adce6da6013 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -2,6 +2,7 @@ > #ifndef DW_SPI_HEADER_H > #define DW_SPI_HEADER_H > > +#include <linux/debugfs.h> > #include <linux/irqreturn.h> > #include <linux/io.h> > #include <linux/scatterlist.h> > @@ -150,6 +151,7 @@ struct dw_spi { > > #ifdef CONFIG_DEBUG_FS > struct dentry *debugfs; > + struct debugfs_regset32 regset; > #endif > }; > > -- > 2.25.1 >
On Fri, May 15, 2020 at 02:49:50PM +0300, Andy Shevchenko wrote: > On Fri, May 15, 2020 at 01:47:39PM +0300, Serge Semin wrote: > > 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") > > Thanks! I'm going to review it soon. > > Hint for the next time, please start always a new thread with new version. Thanks for suggestion. Indeed my knowledge of the submission procedure has been incomplete: "However, for a multi-patch series, it is generally best to avoid using In-Reply-To: to link to older versions of the series. This way multiple versions of the patch don't become an unmanageable forest of references in email clients." [1] [1] Documentation/process/submitting-patches.rst -Sergey > > > 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: Allison Randal <allison@lohutok.net> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Cc: Gareth Williams <gareth.williams.jx@renesas.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 > > > > --- > > > > 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. > > > > Serge Semin (19): > > dt-bindings: spi: dw: Add Tx/Rx DMA properties > > spi: dw: Add Tx/Rx finish wait methods to the MID DMA > > spi: dw: Clear DMAC register when done or stopped > > spi: dw: Fix native CS being unset > > 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: Initialize paddr in DW SPI MMIO private data > > spi: dw: Fix Rx-only DMA transfers > > 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 | 42 --- > > .../bindings/spi/snps,dw-apb-ssi.yaml | 127 +++++++++ > > .../devicetree/bindings/spi/spi-dw.txt | 24 -- > > drivers/spi/Kconfig | 15 +- > > drivers/spi/Makefile | 7 +- > > drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} | 257 ++++++++++-------- > > drivers/spi/spi-dw-mmio.c | 9 +- > > drivers/spi/spi-dw-pci.c | 50 +++- > > drivers/spi/spi-dw.c | 98 +++---- > > drivers/spi/spi-dw.h | 33 ++- > > 10 files changed, 405 insertions(+), 257 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-mid.c => spi-dw-dma.c} (53%) > > > > -- > > 2.25.1 > > > > -- > With Best Regards, > Andy Shevchenko > >
On Fri, May 15, 2020 at 05:40:50PM +0300, Andy Shevchenko wrote: > On Fri, May 15, 2020 at 01:47:51PM +0300, Serge Semin wrote: > > Tx-only DMA transfers are working perfectly fine since in this case > > the code just ignores the Rx FIFO overflow interrupts. But it turns > > out the SPI Rx-only transfers are broken since nothing pushing any > > data to the shift registers, so the Rx FIFO is left empty and the > > SPI core subsystems just returns a timeout error. Since DW DMAC > > driver doesn't support something like cyclic write operations of > > a single byte to a device register, the only way to support the > > Rx-only SPI transfers is to fake it by using a dummy Tx-buffer. > > This is what we intend to fix in this commit by setting the > > SPI_CONTROLLER_MUST_TX flag for DMA-capable platform. > I'm fine with this if Mark considers this right thing to do. > So, conditionally > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Yes, this is good - it's quite a common issue that controllers have and the main reason the flag exists is to provide a standard fix for it.
On Fri, May 15, 2020 at 01:47:39PM +0300, Serge Semin wrote: Appled: > dt-bindings: spi: dw: Add Tx/Rx DMA properties > spi: dw: Clear DMAC register when done or stopped > spi: dw: Fix native CS being unset > spi: dw: Initialize paddr in DW SPI MMIO private data Thanks. No issues from me with the other patches. All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail.
On Fri, May 15, 2020 at 11:02:50PM +0300, Serge Semin wrote: > On Fri, May 15, 2020 at 01:41:31PM +0100, Mark Brown wrote: > > I guess we could, though it's really there because for historical > > reasons we've got a bunch of different ways of specifying delays from > > client drivers rather than for the executing a delay where you've > > already got a good idea of the length of the delay. > A beauty of spi_delay_exec() is that it provides a selective delay. I mean it > checks the delay value and selects an appropriate delay method like ndelay, > udelay and so on. That's the only reason I'd use it here. But It has got a few > drawbacks: Right, usually you'd have a good ideal how long the delay is and therefore just be able to go directly for an appropraite delay function. > - timeout value has type u16. It's too small to keep nanoseconds. That could be increased, though obviously if you have a bigger delay you can specify it in usecs instead. > - semantically the xfer argument isn't optional and we can't fetch it that easy > in the dmaengine completion callbacks. Not sure I follow this. > So if there were an alternative method like _spi_transfer_delay_ns() I'd use it. > Otherwise we'd need to locally implement the selective delay. Unless you know > another alternative, which does it. If you don't and there isn't one then in > order to not over-complicate a simple delay-loop code I'd simply leave the > ndelay() here. Not that I'm aware of.
On Fri, May 15, 2020 at 10:40:58PM +0300, Serge Semin wrote: > On Fri, May 15, 2020 at 03:01:11PM +0300, Andy Shevchenko wrote: > > On Fri, May 15, 2020 at 01:47:41PM +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. ... > > You forgot a Fixes tag. > > If you find a commit this patch fixes I'd be glad to add the tag.) I believe you can do it, but I will help you here, what about 7063c0d942a1 ("spi/dw_spi: add DMA support") or may be more closer to the reality 30c8eb52cc4a ("spi: dw-mid: split rx and tx callbacks when DMA") ? ... > I will if v3 is needed. I guess it will be due to Fixes tag.
On Sat, May 16, 2020 at 05:20:30PM +0300, Serge Semin wrote: > On Fri, May 15, 2020 at 03:34:22PM +0300, Andy Shevchenko wrote: > > On Fri, May 15, 2020 at 01:47:45PM +0300, Serge Semin wrote: > > > Having them declared is redundant since each struct dw_dma_chan has > > > the same structure embedded and the structure from the passed dma_chan > > > private pointer will be copied there as a result of the next calls > > > chain: > > > dma_request_channel() -> find_candidate() -> dma_chan_get() -> > > > device_alloc_chan_resources() = dwc_alloc_chan_resources() -> > > > dw_dma_filter(). > > > So just remove the static dw_dma_chan structures and use a locally > > > declared data instance with dst_id/src_id set to the same values as > > > the static copies used to have. > > > > ... > > > > > -static struct dw_dma_slave mid_dma_tx = { .dst_id = 1 }; > > > -static struct dw_dma_slave mid_dma_rx = { .src_id = 0 }; > > > > > + struct dw_dma_slave slave = {0}; > > > > I really would like to leave them separated and as in the original form, i.e. > > > > struct dw_dma_slave tx = { .dst_id = 1 }; > > struct dw_dma_slave rx = { .src_id = 0 }; > > > > those src and dst IDs are put in that form on purpose... > > As long as you don't tell us what purpose it is, my position won't change. It's not the way when your changes makes this the older (upstreamed) stuff's issue, it's an opposite. But I will help you here... > These structures declared here just hold the static memory and nothing > else. Allocating them on stack is better. I'm not talking about stack, it's fine for me, what I'm talking about is *how* they are being initialized. Read my message again carefully, please.
On Mon, May 18, 2020 at 02:04:53PM +0300, Serge Semin wrote: > On Mon, May 18, 2020 at 11:51:30AM +0100, Mark Brown wrote: > > > - semantically the xfer argument isn't optional and we can't fetch it that easy > > > in the dmaengine completion callbacks. > > Not sure I follow this. > I mean is it Ok to call the spi_delay_exec like this: spi_delay_exec(delay, NULL), > with null passed instead of xfer pointer? Semantically the pointer is required only > if we'd need to calculate the SPI_DELAY_UNIT_SCK delay, but here we'll need > USECS and NSECS delays. So at the first glace there is no problem with passed > NULL instead of xfer. But doing so we'd rely on the semantic peculiarity, which > may seem a bit hackish. Yes, that should be fine if you don't specify a SCK delay. There's no reason to be looking at that if the delay isn't specified in SCKs.
On Sat, May 16, 2020 at 11:46:34PM +0300, Serge Semin wrote: > On Fri, May 15, 2020 at 06:10:56PM +0300, Andy Shevchenko wrote: > > On Fri, May 15, 2020 at 01:47:57PM +0300, Serge Semin wrote: > > > DebugFS kernel interface provides a dedicated method to create the > > > registers dump file. Use it instead of creating a generic DebugFS > > > file with manually written read callback function. > > With below nit addressed, > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > +#define DW_SPI_DBGFS_REG(_name, _off) \ > > > +{ \ > > > + .name = _name, \ > > > > > + .offset = _off \ > > > > As previously discussed (did I miss your answer?) the comma at the end leaves > > better pattern for maintenance prospective. > > Ah, sorry. Missed that. This comma is hardly needed seeing the structure > consists of just two elements. So I'd rather leave it as is. While it's a really small thing, I consider that it's not good to make someone's else problem what can be done here. So, please, consider to add a comma. Look at the other drivers and code in the kernel. This is at least defacto preferred style.
On Mon, May 18, 2020 at 2:01 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Sat, May 16, 2020 at 05:20:30PM +0300, Serge Semin wrote: > > On Fri, May 15, 2020 at 03:34:22PM +0300, Andy Shevchenko wrote: > > > On Fri, May 15, 2020 at 01:47:45PM +0300, Serge Semin wrote: > > > > Having them declared is redundant since each struct dw_dma_chan has > > > > the same structure embedded and the structure from the passed dma_chan > > > > private pointer will be copied there as a result of the next calls > > > > chain: > > > > dma_request_channel() -> find_candidate() -> dma_chan_get() -> > > > > device_alloc_chan_resources() = dwc_alloc_chan_resources() -> > > > > dw_dma_filter(). > > > > So just remove the static dw_dma_chan structures and use a locally > > > > declared data instance with dst_id/src_id set to the same values as > > > > the static copies used to have. > > > > > > ... > > > > > > > -static struct dw_dma_slave mid_dma_tx = { .dst_id = 1 }; > > > > -static struct dw_dma_slave mid_dma_rx = { .src_id = 0 }; > > > > > > > + struct dw_dma_slave slave = {0}; > > > > > > I really would like to leave them separated and as in the original form, i.e. > > > > > > struct dw_dma_slave tx = { .dst_id = 1 }; > > > struct dw_dma_slave rx = { .src_id = 0 }; > > > > > > those src and dst IDs are put in that form on purpose... > > > > As long as you don't tell us what purpose it is, my position won't change. > > It's not the way when your changes makes this the older (upstreamed) stuff's > issue, it's an opposite. But I will help you here... > > > These structures declared here just hold the static memory and nothing > > else. Allocating them on stack is better. > > I'm not talking about stack, it's fine for me, what I'm talking about is *how* > they are being initialized. Read my message again carefully, please. And to avoid additional churn around this, the purpose is to show what Dreq number is in use actually for these transfers (that's why 0 assignment is explicitly there and no counterpart Dreq filled).
On Mon, May 18, 2020 at 03:58:50PM +0300, Serge Semin wrote: > On Sat, May 16, 2020 at 11:17:25PM +0300, Serge Semin wrote: > > Only if we rename the spi-dw.c to spi-dw-core.c. Such modification will break all > > the pending patches merging. Mark, are you ok with this? > Mark, could give me your comment regarding this renaming? Are you ok with this? Either way is fine for me.
On Mon, May 18, 2020 at 3:32 PM Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote: > On Mon, May 18, 2020 at 02:38:13PM +0300, Andy Shevchenko wrote: > > On Mon, May 18, 2020 at 2:01 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Sat, May 16, 2020 at 05:20:30PM +0300, Serge Semin wrote: ... > > > I'm not talking about stack, it's fine for me, what I'm talking about is *how* > > > they are being initialized. Read my message again carefully, please. > > > > And to avoid additional churn around this, the purpose is to show what > > Dreq number is in use actually for these transfers (that's why 0 > > assignment is explicitly there and no counterpart Dreq filled). > > Sorry, but nothing persuasive so far. We still can remove their static definition > and explicitly assign zero or one to the src and dst request IDs on stack. I'll do > this in v3, since I have to send one anyway. Right, I'm completely fine with it, thanks!
On Mon, May 18, 2020 at 05:08:25PM +0300, Serge Semin wrote: > On Mon, May 18, 2020 at 02:18:22PM +0300, Andy Shevchenko wrote: > > On Sat, May 16, 2020 at 11:46:34PM +0300, Serge Semin wrote: > > > On Fri, May 15, 2020 at 06:10:56PM +0300, Andy Shevchenko wrote: > > > > On Fri, May 15, 2020 at 01:47:57PM +0300, Serge Semin wrote: > > > > > DebugFS kernel interface provides a dedicated method to create the > > > > > registers dump file. Use it instead of creating a generic DebugFS > > > > > file with manually written read callback function. > > > > With below nit addressed, > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > > > > +#define DW_SPI_DBGFS_REG(_name, _off) \ > > > > > +{ \ > > > > > + .name = _name, \ > > > > > > > > > + .offset = _off \ > > > > > > > > As previously discussed (did I miss your answer?) the comma at the end leaves > > > > better pattern for maintenance prospective. > > > > > > Ah, sorry. Missed that. This comma is hardly needed seeing the structure > > > consists of just two elements. So I'd rather leave it as is. > > > > While it's a really small thing, I consider that it's not good to make > > someone's else problem what can be done here. So, please, consider to add a > > comma. Look at the other drivers and code in the kernel. This is at least > > defacto preferred style. > > Andy, you never give up, don't you? =) First of all, I really appreciate work you have done so far (I mean it). Now to the point. You see, I always try to have a rationale behind any proposed comment. I agree, that some of them can be considered as a bikeshedding for a certain developer, but on big picture with this scale of project even small change (being made or being rejected to be made) can provoke additional churn with a good magnitude, if we consider all possible cases where somebody, e.g., can take into account existing code to copy'n'paste from). So, I would easy give up on something if there will be a stronger (than mine) argument why the proposed thing is not good to be done (at least as a part of the discussed patch set). I also want to and will learn from the developers as a reviewer. Hope that above will clarify my reviewer's point of view to the code. > Agreed then. I'll add comma to the > initializer and also after the last member here: > DW_SPI_DBGFS_REG("ISR", DW_SPI_ISR), > DW_SPI_DBGFS_REG("DMACR", DW_SPI_DMACR), > DW_SPI_DBGFS_REG("DMATDLR", DW_SPI_DMATDLR), > - DW_SPI_DBGFS_REG("DMARDLR", DW_SPI_DMARDLR) > + DW_SPI_DBGFS_REG("DMARDLR", DW_SPI_DMARDLR), Good catch, thanks for taking it into consideration as well. > }; > > static int dw_spi_debugfs_init(struct dw_spi *dws)
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") 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: Allison Randal <allison@lohutok.net> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Gareth Williams <gareth.williams.jx@renesas.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 --- 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. Serge Semin (19): dt-bindings: spi: dw: Add Tx/Rx DMA properties spi: dw: Add Tx/Rx finish wait methods to the MID DMA spi: dw: Clear DMAC register when done or stopped spi: dw: Fix native CS being unset 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: Initialize paddr in DW SPI MMIO private data spi: dw: Fix Rx-only DMA transfers 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 | 42 --- .../bindings/spi/snps,dw-apb-ssi.yaml | 127 +++++++++ .../devicetree/bindings/spi/spi-dw.txt | 24 -- drivers/spi/Kconfig | 15 +- drivers/spi/Makefile | 7 +- drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} | 257 ++++++++++-------- drivers/spi/spi-dw-mmio.c | 9 +- drivers/spi/spi-dw-pci.c | 50 +++- drivers/spi/spi-dw.c | 98 +++---- drivers/spi/spi-dw.h | 33 ++- 10 files changed, 405 insertions(+), 257 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-mid.c => spi-dw-dma.c} (53%)