Re: [PATCH 03/17] spi: dw: Split up the generic DMA code and Intel MID driver

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Fri, May 08, 2020 at 04:29:28PM +0300, Serge Semin wrote:
> This is an initial preparation patch before adding the DW DMA support
> into the DW SPI MMIO driver. We need to unpin the DMA-specific code
> from the code intended to be used for Intel MID. This isn't that hard,
> since most part of the spi-dw-mid.c driver in fact implements a generic
> DW 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 object. While seeing there is no other than Medfield
> board with DW DMA controller currently supported we left the DMA
> channels search procedure in the DW SPI DMA module. After being
> cleaned up of the Intel MID specifics former spi-dw-mid.c module
> can be just renamed to be the DW SPI DMA driver.

And I guess this already been done in spi/for-next in less invasive way.

> 
> Co-developed-by: Georgy Vlasov <Georgy.Vlasov@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Georgy Vlasov <Georgy.Vlasov@xxxxxxxxxxxxxxxxxxxx>
> Co-developed-by: Ramil Zaripov <Ramil.Zaripov@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Ramil Zaripov <Ramil.Zaripov@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>
> Cc: Alexey Malahov <Alexey.Malahov@xxxxxxxxxxxxxxxxxxxx>
> Cc: Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>
> Cc: Paul Burton <paulburton@xxxxxxxxxx>
> Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Allison Randal <allison@xxxxxxxxxxx>
> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: Gareth Williams <gareth.williams.jx@xxxxxxxxxxx>
> Cc: linux-mips@xxxxxxxxxxxxxxx
> Cc: devicetree@xxxxxxxxxxxxxxx
> ---
>  drivers/spi/Kconfig                        |  8 ++---
>  drivers/spi/Makefile                       |  4 +--
>  drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} | 36 ++------------------
>  drivers/spi/spi-dw-pci.c                   | 38 ++++++++++++++++++++--
>  drivers/spi/spi-dw.h                       | 12 +++++--
>  5 files changed, 55 insertions(+), 43 deletions(-)
>  rename drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} (88%)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 741b9140992a..9653c7f271e9 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -226,14 +226,14 @@ config SPI_DESIGNWARE
>  	help
>  	  general driver for SPI controller core from DesignWare
>  
> +config SPI_DW_DMA
> +	tristate "DMA support for DW SPI controller"
> +	depends on SPI_DESIGNWARE && DW_DMAC_PCI
> +
>  config SPI_DW_PCI
>  	tristate "PCI interface driver for DW SPI core"
>  	depends on SPI_DESIGNWARE && PCI
>  
> -config SPI_DW_MID_DMA
> -	bool "DMA support for DW SPI controller on Intel MID platform"
> -	depends on SPI_DW_PCI && DW_DMAC_PCI
> -
>  config SPI_DW_MMIO
>  	tristate "Memory-mapped io interface driver for DW SPI core"
>  	depends on SPI_DESIGNWARE
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 28f601327f8c..15eb760412a9 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -36,9 +36,9 @@ obj-$(CONFIG_SPI_COLDFIRE_QSPI)		+= spi-coldfire-qspi.o
>  obj-$(CONFIG_SPI_DAVINCI)		+= spi-davinci.o
>  obj-$(CONFIG_SPI_DLN2)			+= spi-dln2.o
>  obj-$(CONFIG_SPI_DESIGNWARE)		+= spi-dw.o
> +obj-$(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
>  obj-$(CONFIG_SPI_EFM32)			+= spi-efm32.o
>  obj-$(CONFIG_SPI_EP93XX)		+= spi-ep93xx.o
>  obj-$(CONFIG_SPI_FALCON)		+= spi-falcon.o
> diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-dma.c
> similarity index 88%
> rename from drivers/spi/spi-dw-mid.c
> rename to drivers/spi/spi-dw-dma.c
> index 0d86c37e0aeb..0230b4252611 100644
> --- a/drivers/spi/spi-dw-mid.c
> +++ b/drivers/spi/spi-dw-dma.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Special handling for DW core on Intel MID platform
> + * Special handling for DW DMA core
>   *
>   * Copyright (c) 2009, 2014 Intel Corporation.
>   */
> @@ -14,7 +14,6 @@
>  
>  #include "spi-dw.h"
>  
> -#ifdef CONFIG_SPI_DW_MID_DMA
>  #include <linux/pci.h>
>  #include <linux/platform_data/dma-dw.h>
>  
> @@ -283,40 +282,11 @@ static const struct dw_spi_dma_ops mid_dma_ops = {
>  	.dma_transfer	= mid_spi_dma_transfer,
>  	.dma_stop	= mid_spi_dma_stop,
>  };
> -#endif
>  
> -/* 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(struct dw_spi *dws)
> +void dw_spi_pci_dma_setup(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);
> -
> -#ifdef CONFIG_SPI_DW_MID_DMA
>  	dws->dma_tx = &mid_dma_tx;
>  	dws->dma_rx = &mid_dma_rx;
>  	dws->dma_ops = &mid_dma_ops;
> -#endif
> -	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dw_spi_pci_dma_setup);
> diff --git a/drivers/spi/spi-dw-pci.c b/drivers/spi/spi-dw-pci.c
> index 12c131b5fb4e..068f6897b903 100644
> --- a/drivers/spi/spi-dw-pci.c
> +++ b/drivers/spi/spi-dw-pci.c
> @@ -16,6 +16,17 @@
>  
>  #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
> +
> +static int spi_mid_init(struct dw_spi *dws);
> +
>  struct spi_pci_desc {
>  	int	(*setup)(struct dw_spi *);
>  	u16	num_cs;
> @@ -24,13 +35,13 @@ struct spi_pci_desc {
>  };
>  
>  static struct spi_pci_desc spi_pci_mid_desc_1 = {
> -	.setup = dw_spi_mid_init,
> +	.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,
> +	.setup = spi_mid_init,
>  	.num_cs = 2,
>  	.bus_num = 1,
>  };
> @@ -41,6 +52,29 @@ static struct spi_pci_desc spi_pci_ehl_desc = {
>  	.max_freq = 100000000,
>  };
>  
> +/* Some specific info for SPI0 controller on Intel MID */
> +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);
> +
> +	dw_spi_pci_dma_setup(dws);
> +
> +	return 0;
> +}
> +
>  static int spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>  	struct dw_spi *dws;
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 1bf5713e047d..0a4e0890ef85 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -253,6 +253,14 @@ extern void dw_spi_remove_host(struct dw_spi *dws);
>  extern int dw_spi_suspend_host(struct dw_spi *dws);
>  extern int dw_spi_resume_host(struct dw_spi *dws);
>  
> -/* platform related setup */
> -extern int dw_spi_mid_init(struct dw_spi *dws); /* Intel MID platforms */
> +#ifdef CONFIG_SPI_DW_DMA
> +
> +extern void dw_spi_pci_dma_setup(struct dw_spi *dws);
> +
> +#else /* !CONFIG_SPI_DW_DMA */
> +
> +static inline void dw_spi_pci_dma_setup(struct dw_spi *dws) {}
> +
> +#endif /* !CONFIG_SPI_DW_DMA */
> +
>  #endif /* DW_SPI_HEADER_H */
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko