[PATCH v3 00/13] sunxi spi fixes

[PATCH v3 00/13] sunxi spi fixes

[Michal Suchanek]

Hello,

This is update of the sunxi spi patches that should give full-featured SPI
driver.

First three patches fix issues with the current driver and can be of use for
stable kernels so adding cc for those.

I merged the sun4i and sun6i driver because there several issues that need to
be fixed in both separately and they are even out of sync wrt some fixes.
I guess some of the merge patches can be squashed.

I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
tell if that side was broken by this patchset - especially the last patch that
adds DMA was afaik never tested on sun6i.


Emilio López (1):
  spi: sun4i: add DMA support

Michal Suchanek (12):
  spi: sunxi: set maximum and minimum speed of SPI master
  spi: sunxi: fix transfer timeout
  spi: sun4i: fix FIFO limit
  spi: sunxi: expose maximum transfer size limit
  spi: sun6i: update CS handling from spi-sun4i
  spi: sunxi: rename sun4i,sun6i -> sunxi
  spi: sunxi: rename constants to match between sun4i and sun6i
  spi: sunxi: synchronize whitespace, comments, struct
  spi: sunxi: use register map
  spi: sunxi: merge sun4i and sun6i SPI driver
  dt: spi: sun4i: merge sun4i and sun6i binding doc
  spi: sunxi: remove CONFIG_SPI_SUN6I

 .../devicetree/bindings/spi/spi-sun4i.txt          |  21 +-
 .../devicetree/bindings/spi/spi-sun6i.txt          |  24 -
 arch/arm/configs/multi_v7_defconfig                |   1 -
 arch/arm/configs/sunxi_defconfig                   |   1 -
 drivers/spi/Kconfig                                |  10 +-
 drivers/spi/Makefile                               |   1 -
 drivers/spi/spi-sun4i.c                            | 728 +++++++++++++++++----
 drivers/spi/spi-sun6i.c                            | 482 --------------
 8 files changed, 600 insertions(+), 668 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
 delete mode 100644 drivers/spi/spi-sun6i.c

-- 
2.8.1


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

[PATCH v3 01/13] spi: sunxi: set maximum and minimum speed of SPI master

[Michal Suchanek]

The speed limits are unset in the sun4i and sun6i SPI drivers.

The maximum speed of SPI master is used when maximum speed of SPI slave
is not specified. Also the __spi_validate function should check that
transfer speeds do not exceed the master limits.

The user manual for A10 and A31 specifies maximum
speed of the SPI clock as 100MHz and minimum as 3kHz.

Setting the SPI clock to out-of-spec values can lock up the SoC.

Signed-off-by: Michal Suchanek <hramrach@gmail.com>
--
v2:
new patch
v3:
fix constant style
---
 drivers/spi/spi-sun4i.c | 2 ++
 drivers/spi/spi-sun6i.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 1ddd9e2..4213508 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -387,6 +387,8 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 	}
 
 	sspi->master = master;
+	master->max_speed_hz = 100 * 1000 * 1000;
+	master->min_speed_hz =          3 * 1000;
 	master->set_cs = sun4i_spi_set_cs;
 	master->transfer_one = sun4i_spi_transfer_one;
 	master->num_chipselect = 4;
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 42e2c4b..fe70695 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -386,6 +386,8 @@ static int sun6i_spi_probe(struct platform_device *pdev)
 	}
 
 	sspi->master = master;
+	master->max_speed_hz = 100 * 1000 * 1000;
+	master->min_speed_hz =          3 * 1000;
 	master->set_cs = sun6i_spi_set_cs;
 	master->transfer_one = sun6i_spi_transfer_one;
 	master->num_chipselect = 4;
-- 
2.8.1

Re: [PATCH v3 01/13] spi: sunxi: set maximum and minimum speed of SPI master

[Maxime Ripard]

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

On Mon, Jun 13, 2016 at 05:46:49PM -0000, Michal Suchanek wrote:
> The speed limits are unset in the sun4i and sun6i SPI drivers.
> 
> The maximum speed of SPI master is used when maximum speed of SPI slave
> is not specified. Also the __spi_validate function should check that
> transfer speeds do not exceed the master limits.
> 
> The user manual for A10 and A31 specifies maximum
> speed of the SPI clock as 100MHz and minimum as 3kHz.
> 
> Setting the SPI clock to out-of-spec values can lock up the SoC.
> 
> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> --
> v2:
> new patch
> v3:
> fix constant style
> ---
>  drivers/spi/spi-sun4i.c | 2 ++
>  drivers/spi/spi-sun6i.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 1ddd9e2..4213508 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -387,6 +387,8 @@ static int sun4i_spi_probe(struct platform_device *pdev)
>  	}
>  
>  	sspi->master = master;
> +	master->max_speed_hz = 100 * 1000 * 1000;
> +	master->min_speed_hz =          3 * 1000;
>  	master->set_cs = sun4i_spi_set_cs;
>  	master->transfer_one = sun4i_spi_transfer_one;
>  	master->num_chipselect = 4;
> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
> index 42e2c4b..fe70695 100644
> --- a/drivers/spi/spi-sun6i.c
> +++ b/drivers/spi/spi-sun6i.c
> @@ -386,6 +386,8 @@ static int sun6i_spi_probe(struct platform_device *pdev)
>  	}
>  
>  	sspi->master = master;
> +	master->max_speed_hz = 100 * 1000 * 1000;
> +	master->min_speed_hz =          3 * 1000;
>  	master->set_cs = sun6i_spi_set_cs;
>  	master->transfer_one = sun6i_spi_transfer_one;
>  	master->num_chipselect = 4;

I really don't get why you want to do that kind of padding, when no
one does in the rest of the driver, or the rest of the kernel.

Once properly changed,
Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[PATCH v3 04/13] spi: sunxi: expose maximum transfer size limit

[Michal Suchanek]

The sun4i spi hardware can trasfer at most 63 bytes of data without DMA
support so report the limitation. Same for sun6i.

Signed-off-by: Michal Suchanek <hramrach@gmail.com>
---
 drivers/spi/spi-sun4i.c | 6 ++++++
 drivers/spi/spi-sun6i.c | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index f2848b7..ba2b5c8 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -167,6 +167,11 @@ static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)
 	sun4i_spi_write(sspi, SUN4I_CTL_REG, reg);
 }
 
+static size_t sun4i_spi_max_transfer_size(struct spi_device *spi)
+{
+	return SUN4I_FIFO_DEPTH - 1;
+}
+
 static int sun4i_spi_transfer_one(struct spi_master *master,
 				  struct spi_device *spi,
 				  struct spi_transfer *tfr)
@@ -411,6 +416,7 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
 	master->dev.of_node = pdev->dev.of_node;
 	master->auto_runtime_pm = true;
+	master->max_transfer_size = sun4i_spi_max_transfer_size;
 
 	sspi->hclk = devm_clk_get(&pdev->dev, "ahb");
 	if (IS_ERR(sspi->hclk)) {
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 8f6be86..bc29a98 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -153,6 +153,10 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
 	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
 }
 
+static size_t sun6i_spi_max_transfer_size(struct spi_device *spi)
+{
+	return SUN6I_FIFO_DEPTH - 1;
+}
 
 static int sun6i_spi_transfer_one(struct spi_master *master,
 				  struct spi_device *spi,
@@ -403,6 +407,7 @@ static int sun6i_spi_probe(struct platform_device *pdev)
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
 	master->dev.of_node = pdev->dev.of_node;
 	master->auto_runtime_pm = true;
+	master->max_transfer_size = sun6i_spi_max_transfer_size;
 
 	sspi->hclk = devm_clk_get(&pdev->dev, "ahb");
 	if (IS_ERR(sspi->hclk)) {
-- 
2.8.1

Re: [PATCH v3 04/13] spi: sunxi: expose maximum transfer size limit

[Maxime Ripard]

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

On Mon, Jun 13, 2016 at 05:46:50PM -0000, Michal Suchanek wrote:
> The sun4i spi hardware can trasfer at most 63 bytes of data without DMA
> support so report the limitation. Same for sun6i.
> 
> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v3 03/13] spi: sun4i: fix FIFO limit

[Michal Suchanek]

When testing SPI without DMA I noticed that filling the FIFO on the
spi controller causes timeout.

Always leave room for one byte in the FIFO.

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

---
v2:
use EMSGSIZE instead of EINVAL
v3:
fix comment style
---
 drivers/spi/spi-sun4i.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 0c2216a..f2848b7 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -180,7 +180,10 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 
 	/* We don't support transfer larger than the FIFO */
 	if (tfr->len > SUN4I_FIFO_DEPTH)
-		return -EINVAL;
+		return -EMSGSIZE;
+
+	if (tfr->tx_buf && tfr->len >= SUN4I_FIFO_DEPTH)
+		return -EMSGSIZE;
 
 	reinit_completion(&sspi->done);
 	sspi->tx_buf = tfr->tx_buf;
@@ -270,8 +273,12 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	sun4i_spi_write(sspi, SUN4I_BURST_CNT_REG, SUN4I_BURST_CNT(tfr->len));
 	sun4i_spi_write(sspi, SUN4I_XMIT_CNT_REG, SUN4I_XMIT_CNT(tx_len));
 
-	/* Fill the TX FIFO */
-	sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
+	/*
+	 * Fill the TX FIFO
+	 * Filling the FIFO fully causes timeout for some reason
+	 * at least on spi2 on A10s
+	 */
+	sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1);
 
 	/* Enable the interrupts */
 	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC);
-- 
2.8.1

Re: [PATCH v3 03/13] spi: sun4i: fix FIFO limit

[Maxime Ripard]

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

On Mon, Jun 13, 2016 at 05:46:49PM -0000, Michal Suchanek wrote:
> When testing SPI without DMA I noticed that filling the FIFO on the
> spi controller causes timeout.
> 
> Always leave room for one byte in the FIFO.
> 
> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v3 02/13] spi: sunxi: fix transfer timeout

[Michal Suchanek]

The trasfer timeout is fixed at 1000 ms. Reading a 4Mbyte flash over
1MHz SPI bus takes way longer than that. Calculate the timeout from the
actual time the transfer is supposed to take and multiply by 2 for good
measure.

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

v2:
- fix build error
- use unsigned instead of int in max_t
- use tfr->speed_hz instead of sspi->max_speed_hz
  - commit 47284e3e0f3c427c93f8583549b6c938e8a18015 changes the driver
    to use tfr->speed_hz
v3:
- avoid max_t by using unsigned constant
---
 drivers/spi/spi-sun4i.c | 10 +++++++++-
 drivers/spi/spi-sun6i.c | 10 +++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 4213508..0c2216a 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -173,6 +173,7 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 {
 	struct sun4i_spi *sspi = spi_master_get_devdata(master);
 	unsigned int mclk_rate, div, timeout;
+	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
 	int ret = 0;
 	u32 reg;
@@ -279,9 +280,16 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
 	sun4i_spi_write(sspi, SUN4I_CTL_REG, reg | SUN4I_CTL_XCH);
 
+	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
+	start = jiffies;
 	timeout = wait_for_completion_timeout(&sspi->done,
-					      msecs_to_jiffies(1000));
+					      msecs_to_jiffies(tx_time));
+	end = jiffies;
 	if (!timeout) {
+		dev_warn(&master->dev,
+			 "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
+			 dev_name(&spi->dev), tfr->len, tfr->speed_hz,
+			 jiffies_to_msecs(end - start), tx_time);
 		ret = -ETIMEDOUT;
 		goto out;
 	}
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index fe70695..8f6be86 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -160,6 +160,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 {
 	struct sun6i_spi *sspi = spi_master_get_devdata(master);
 	unsigned int mclk_rate, div, timeout;
+	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
 	int ret = 0;
 	u32 reg;
@@ -269,9 +270,16 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 	reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
 	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg | SUN6I_TFR_CTL_XCH);
 
+	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
+	start = jiffies;
 	timeout = wait_for_completion_timeout(&sspi->done,
-					      msecs_to_jiffies(1000));
+					      msecs_to_jiffies(tx_time));
+	end = jiffies;
 	if (!timeout) {
+		dev_warn(&master->dev,
+			 "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
+			 dev_name(&spi->dev), tfr->len, tfr->speed_hz,
+			 jiffies_to_msecs(end - start), tx_time);
 		ret = -ETIMEDOUT;
 		goto out;
 	}
-- 
2.8.1

Re: [PATCH v3 02/13] spi: sunxi: fix transfer timeout

[Maxime Ripard]

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

On Mon, Jun 13, 2016 at 05:46:49PM -0000, Michal Suchanek wrote:
> The trasfer timeout is fixed at 1000 ms. Reading a 4Mbyte flash over
> 1MHz SPI bus takes way longer than that. Calculate the timeout from the
> actual time the transfer is supposed to take and multiply by 2 for good
> measure.
> 
> Signed-off-by: Michal Suchanek <hramrach@gmail.com>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[PATCH v3 06/13] spi: sunxi: rename sun4i,sun6i -> sunxi

[Michal Suchanek]

Prepare to merge drivers.

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-sun4i.c | 274 +++++++++++++++++++++++-----------------------
 drivers/spi/spi-sun6i.c | 284 ++++++++++++++++++++++++------------------------
 2 files changed, 279 insertions(+), 279 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index ba2b5c8..155d720 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -22,58 +22,58 @@
 
 #include <linux/spi/spi.h>
 
-#define SUN4I_FIFO_DEPTH		64
-
-#define SUN4I_RXDATA_REG		0x00
-
-#define SUN4I_TXDATA_REG		0x04
-
-#define SUN4I_CTL_REG			0x08
-#define SUN4I_CTL_ENABLE			BIT(0)
-#define SUN4I_CTL_MASTER			BIT(1)
-#define SUN4I_CTL_CPHA				BIT(2)
-#define SUN4I_CTL_CPOL				BIT(3)
-#define SUN4I_CTL_CS_ACTIVE_LOW			BIT(4)
-#define SUN4I_CTL_LMTF				BIT(6)
-#define SUN4I_CTL_TF_RST			BIT(8)
-#define SUN4I_CTL_RF_RST			BIT(9)
-#define SUN4I_CTL_XCH				BIT(10)
-#define SUN4I_CTL_CS_MASK			0x3000
-#define SUN4I_CTL_CS(cs)			(((cs) << 12) & SUN4I_CTL_CS_MASK)
-#define SUN4I_CTL_DHB				BIT(15)
-#define SUN4I_CTL_CS_MANUAL			BIT(16)
-#define SUN4I_CTL_CS_LEVEL			BIT(17)
-#define SUN4I_CTL_TP				BIT(18)
-
-#define SUN4I_INT_CTL_REG		0x0c
-#define SUN4I_INT_CTL_TC			BIT(16)
-
-#define SUN4I_INT_STA_REG		0x10
-
-#define SUN4I_DMA_CTL_REG		0x14
-
-#define SUN4I_WAIT_REG			0x18
-
-#define SUN4I_CLK_CTL_REG		0x1c
-#define SUN4I_CLK_CTL_CDR2_MASK			0xff
-#define SUN4I_CLK_CTL_CDR2(div)			((div) & SUN4I_CLK_CTL_CDR2_MASK)
-#define SUN4I_CLK_CTL_CDR1_MASK			0xf
-#define SUN4I_CLK_CTL_CDR1(div)			(((div) & SUN4I_CLK_CTL_CDR1_MASK) << 8)
-#define SUN4I_CLK_CTL_DRS			BIT(12)
-
-#define SUN4I_BURST_CNT_REG		0x20
-#define SUN4I_BURST_CNT(cnt)			((cnt) & 0xffffff)
-
-#define SUN4I_XMIT_CNT_REG		0x24
-#define SUN4I_XMIT_CNT(cnt)			((cnt) & 0xffffff)
-
-#define SUN4I_FIFO_STA_REG		0x28
-#define SUN4I_FIFO_STA_RF_CNT_MASK		0x7f
-#define SUN4I_FIFO_STA_RF_CNT_BITS		0
-#define SUN4I_FIFO_STA_TF_CNT_MASK		0x7f
-#define SUN4I_FIFO_STA_TF_CNT_BITS		16
-
-struct sun4i_spi {
+#define SUNXI_FIFO_DEPTH		64
+
+#define SUNXI_RXDATA_REG		0x00
+
+#define SUNXI_TXDATA_REG		0x04
+
+#define SUNXI_CTL_REG			0x08
+#define SUNXI_CTL_ENABLE		BIT(0)
+#define SUNXI_CTL_MASTER		BIT(1)
+#define SUNXI_CTL_CPHA			BIT(2)
+#define SUNXI_CTL_CPOL			BIT(3)
+#define SUNXI_CTL_CS_ACTIVE_LOW		BIT(4)
+#define SUNXI_CTL_LMTF			BIT(6)
+#define SUNXI_CTL_TF_RST		BIT(8)
+#define SUNXI_CTL_RF_RST		BIT(9)
+#define SUNXI_CTL_XCH			BIT(10)
+#define SUNXI_CTL_CS_MASK		0x3000
+#define SUNXI_CTL_CS(cs)		(((cs) << 12) & SUNXI_CTL_CS_MASK)
+#define SUNXI_CTL_DHB			BIT(15)
+#define SUNXI_CTL_CS_MANUAL		BIT(16)
+#define SUNXI_CTL_CS_LEVEL		BIT(17)
+#define SUNXI_CTL_TP			BIT(18)
+
+#define SUNXI_INT_CTL_REG		0x0c
+#define SUNXI_INT_CTL_TC		BIT(16)
+
+#define SUNXI_INT_STA_REG		0x10
+
+#define SUNXI_DMA_CTL_REG		0x14
+
+#define SUNXI_WAIT_REG			0x18
+
+#define SUNXI_CLK_CTL_REG		0x1c
+#define SUNXI_CLK_CTL_CDR2_MASK		0xff
+#define SUNXI_CLK_CTL_CDR2(div)		((div) & SUNXI_CLK_CTL_CDR2_MASK)
+#define SUNXI_CLK_CTL_CDR1_MASK		0xf
+#define SUNXI_CLK_CTL_CDR1(div)		(((div) & SUNXI_CLK_CTL_CDR1_MASK) << 8)
+#define SUNXI_CLK_CTL_DRS		BIT(12)
+
+#define SUNXI_BURST_CNT_REG		0x20
+#define SUNXI_BURST_CNT(cnt)		((cnt) & 0xffffff)
+
+#define SUNXI_XMIT_CNT_REG		0x24
+#define SUNXI_XMIT_CNT(cnt)		((cnt) & 0xffffff)
+
+#define SUNXI_FIFO_STA_REG		0x28
+#define SUNXI_FIFO_STA_RF_CNT_MASK	0x7f
+#define SUNXI_FIFO_STA_RF_CNT_BITS	0
+#define SUNXI_FIFO_STA_TF_CNT_MASK	0x7f
+#define SUNXI_FIFO_STA_TF_CNT_BITS	16
+
+struct sunxi_spi {
 	struct spi_master	*master;
 	void __iomem		*base_addr;
 	struct clk		*hclk;
@@ -86,37 +86,37 @@ struct sun4i_spi {
 	int			len;
 };
 
-static inline u32 sun4i_spi_read(struct sun4i_spi *sspi, u32 reg)
+static inline u32 sunxi_spi_read(struct sunxi_spi *sspi, u32 reg)
 {
 	return readl(sspi->base_addr + reg);
 }
 
-static inline void sun4i_spi_write(struct sun4i_spi *sspi, u32 reg, u32 value)
+static inline void sunxi_spi_write(struct sunxi_spi *sspi, u32 reg, u32 value)
 {
 	writel(value, sspi->base_addr + reg);
 }
 
-static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len)
+static inline void sunxi_spi_drain_fifo(struct sunxi_spi *sspi, int len)
 {
 	u32 reg, cnt;
 	u8 byte;
 
 	/* See how much data is available */
-	reg = sun4i_spi_read(sspi, SUN4I_FIFO_STA_REG);
-	reg &= SUN4I_FIFO_STA_RF_CNT_MASK;
-	cnt = reg >> SUN4I_FIFO_STA_RF_CNT_BITS;
+	reg = sunxi_spi_read(sspi, SUNXI_FIFO_STA_REG);
+	reg &= SUNXI_FIFO_STA_RF_CNT_MASK;
+	cnt = reg >> SUNXI_FIFO_STA_RF_CNT_BITS;
 
 	if (len > cnt)
 		len = cnt;
 
 	while (len--) {
-		byte = readb(sspi->base_addr + SUN4I_RXDATA_REG);
+		byte = readb(sspi->base_addr + SUNXI_RXDATA_REG);
 		if (sspi->rx_buf)
 			*sspi->rx_buf++ = byte;
 	}
 }
 
-static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len)
+static inline void sunxi_spi_fill_fifo(struct sunxi_spi *sspi, int len)
 {
 	u8 byte;
 
@@ -125,28 +125,28 @@ static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len)
 
 	while (len--) {
 		byte = sspi->tx_buf ? *sspi->tx_buf++ : 0;
-		writeb(byte, sspi->base_addr + SUN4I_TXDATA_REG);
+		writeb(byte, sspi->base_addr + SUNXI_TXDATA_REG);
 		sspi->len--;
 	}
 }
 
-static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)
+static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 {
-	struct sun4i_spi *sspi = spi_master_get_devdata(spi->master);
+	struct sunxi_spi *sspi = spi_master_get_devdata(spi->master);
 	u32 reg;
 
-	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
+	reg = sunxi_spi_read(sspi, SUNXI_CTL_REG);
 
-	reg &= ~SUN4I_CTL_CS_MASK;
-	reg |= SUN4I_CTL_CS(spi->chip_select);
+	reg &= ~SUNXI_CTL_CS_MASK;
+	reg |= SUNXI_CTL_CS(spi->chip_select);
 
 	/* We want to control the chip select manually */
-	reg |= SUN4I_CTL_CS_MANUAL;
+	reg |= SUNXI_CTL_CS_MANUAL;
 
 	if (enable)
-		reg |= SUN4I_CTL_CS_LEVEL;
+		reg |= SUNXI_CTL_CS_LEVEL;
 	else
-		reg &= ~SUN4I_CTL_CS_LEVEL;
+		reg &= ~SUNXI_CTL_CS_LEVEL;
 
 	/*
 	 * Even though this looks irrelevant since we are supposed to
@@ -160,23 +160,23 @@ static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)
 	 * low.
 	 */
 	if (spi->mode & SPI_CS_HIGH)
-		reg &= ~SUN4I_CTL_CS_ACTIVE_LOW;
+		reg &= ~SUNXI_CTL_CS_ACTIVE_LOW;
 	else
-		reg |= SUN4I_CTL_CS_ACTIVE_LOW;
+		reg |= SUNXI_CTL_CS_ACTIVE_LOW;
 
-	sun4i_spi_write(sspi, SUN4I_CTL_REG, reg);
+	sunxi_spi_write(sspi, SUNXI_CTL_REG, reg);
 }
 
-static size_t sun4i_spi_max_transfer_size(struct spi_device *spi)
+static size_t sunxi_spi_max_transfer_size(struct spi_device *spi)
 {
-	return SUN4I_FIFO_DEPTH - 1;
+	return SUNXI_FIFO_DEPTH - 1;
 }
 
-static int sun4i_spi_transfer_one(struct spi_master *master,
+static int sunxi_spi_transfer_one(struct spi_master *master,
 				  struct spi_device *spi,
 				  struct spi_transfer *tfr)
 {
-	struct sun4i_spi *sspi = spi_master_get_devdata(master);
+	struct sunxi_spi *sspi = spi_master_get_devdata(master);
 	unsigned int mclk_rate, div, timeout;
 	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
@@ -184,10 +184,10 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	u32 reg;
 
 	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > SUN4I_FIFO_DEPTH)
+	if (tfr->len > SUNXI_FIFO_DEPTH)
 		return -EMSGSIZE;
 
-	if (tfr->tx_buf && tfr->len >= SUN4I_FIFO_DEPTH)
+	if (tfr->tx_buf && tfr->len >= SUNXI_FIFO_DEPTH)
 		return -EMSGSIZE;
 
 	reinit_completion(&sspi->done);
@@ -196,33 +196,33 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	sspi->len = tfr->len;
 
 	/* Clear pending interrupts */
-	sun4i_spi_write(sspi, SUN4I_INT_STA_REG, ~0);
+	sunxi_spi_write(sspi, SUNXI_INT_STA_REG, ~0);
 
 
-	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
+	reg = sunxi_spi_read(sspi, SUNXI_CTL_REG);
 
 	/* Reset FIFOs */
-	sun4i_spi_write(sspi, SUN4I_CTL_REG,
-			reg | SUN4I_CTL_RF_RST | SUN4I_CTL_TF_RST);
+	sunxi_spi_write(sspi, SUNXI_CTL_REG,
+			reg | SUNXI_CTL_RF_RST | SUNXI_CTL_TF_RST);
 
 	/*
 	 * Setup the transfer control register: Chip Select,
 	 * polarities, etc.
 	 */
 	if (spi->mode & SPI_CPOL)
-		reg |= SUN4I_CTL_CPOL;
+		reg |= SUNXI_CTL_CPOL;
 	else
-		reg &= ~SUN4I_CTL_CPOL;
+		reg &= ~SUNXI_CTL_CPOL;
 
 	if (spi->mode & SPI_CPHA)
-		reg |= SUN4I_CTL_CPHA;
+		reg |= SUNXI_CTL_CPHA;
 	else
-		reg &= ~SUN4I_CTL_CPHA;
+		reg &= ~SUNXI_CTL_CPHA;
 
 	if (spi->mode & SPI_LSB_FIRST)
-		reg |= SUN4I_CTL_LMTF;
+		reg |= SUNXI_CTL_LMTF;
 	else
-		reg &= ~SUN4I_CTL_LMTF;
+		reg &= ~SUNXI_CTL_LMTF;
 
 
 	/*
@@ -230,11 +230,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	 * FIFO with bogus data
 	 */
 	if (sspi->rx_buf)
-		reg &= ~SUN4I_CTL_DHB;
+		reg &= ~SUNXI_CTL_DHB;
 	else
-		reg |= SUN4I_CTL_DHB;
+		reg |= SUNXI_CTL_DHB;
 
-	sun4i_spi_write(sspi, SUN4I_CTL_REG, reg);
+	sunxi_spi_write(sspi, SUNXI_CTL_REG, reg);
 
 	/* Ensure that we have a parent clock fast enough */
 	mclk_rate = clk_get_rate(sspi->mclk);
@@ -258,39 +258,39 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	 * frequency, fall back to CDR1.
 	 */
 	div = mclk_rate / (2 * tfr->speed_hz);
-	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
+	if (div <= (SUNXI_CLK_CTL_CDR2_MASK + 1)) {
 		if (div > 0)
 			div--;
 
-		reg = SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
+		reg = SUNXI_CLK_CTL_CDR2(div) | SUNXI_CLK_CTL_DRS;
 	} else {
 		div = ilog2(mclk_rate) - ilog2(tfr->speed_hz);
-		reg = SUN4I_CLK_CTL_CDR1(div);
+		reg = SUNXI_CLK_CTL_CDR1(div);
 	}
 
-	sun4i_spi_write(sspi, SUN4I_CLK_CTL_REG, reg);
+	sunxi_spi_write(sspi, SUNXI_CLK_CTL_REG, reg);
 
 	/* Setup the transfer now... */
 	if (sspi->tx_buf)
 		tx_len = tfr->len;
 
 	/* Setup the counters */
-	sun4i_spi_write(sspi, SUN4I_BURST_CNT_REG, SUN4I_BURST_CNT(tfr->len));
-	sun4i_spi_write(sspi, SUN4I_XMIT_CNT_REG, SUN4I_XMIT_CNT(tx_len));
+	sunxi_spi_write(sspi, SUNXI_BURST_CNT_REG, SUNXI_BURST_CNT(tfr->len));
+	sunxi_spi_write(sspi, SUNXI_XMIT_CNT_REG, SUNXI_XMIT_CNT(tx_len));
 
 	/*
 	 * Fill the TX FIFO
 	 * Filling the FIFO fully causes timeout for some reason
 	 * at least on spi2 on A10s
 	 */
-	sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1);
+	sunxi_spi_fill_fifo(sspi, SUNXI_FIFO_DEPTH - 1);
 
 	/* Enable the interrupts */
-	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC);
+	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, SUNXI_INT_CTL_TC);
 
 	/* Start the transfer */
-	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
-	sun4i_spi_write(sspi, SUN4I_CTL_REG, reg | SUN4I_CTL_XCH);
+	reg = sunxi_spi_read(sspi, SUNXI_CTL_REG);
+	sunxi_spi_write(sspi, SUNXI_CTL_REG, reg | SUNXI_CTL_XCH);
 
 	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
 	start = jiffies;
@@ -306,22 +306,22 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
-	sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
+	sunxi_spi_drain_fifo(sspi, SUNXI_FIFO_DEPTH);
 
 out:
-	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
+	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, 0);
 
 	return ret;
 }
 
-static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)
+static irqreturn_t sunxi_spi_handler(int irq, void *dev_id)
 {
-	struct sun4i_spi *sspi = dev_id;
-	u32 status = sun4i_spi_read(sspi, SUN4I_INT_STA_REG);
+	struct sunxi_spi *sspi = dev_id;
+	u32 status = sunxi_spi_read(sspi, SUNXI_INT_STA_REG);
 
 	/* Transfer complete */
-	if (status & SUN4I_INT_CTL_TC) {
-		sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TC);
+	if (status & SUNXI_INT_CTL_TC) {
+		sunxi_spi_write(sspi, SUNXI_INT_STA_REG, SUNXI_INT_CTL_TC);
 		complete(&sspi->done);
 		return IRQ_HANDLED;
 	}
@@ -329,10 +329,10 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-static int sun4i_spi_runtime_resume(struct device *dev)
+static int sunxi_spi_runtime_resume(struct device *dev)
 {
 	struct spi_master *master = dev_get_drvdata(dev);
-	struct sun4i_spi *sspi = spi_master_get_devdata(master);
+	struct sunxi_spi *sspi = spi_master_get_devdata(master);
 	int ret;
 
 	ret = clk_prepare_enable(sspi->hclk);
@@ -347,8 +347,8 @@ static int sun4i_spi_runtime_resume(struct device *dev)
 		goto err;
 	}
 
-	sun4i_spi_write(sspi, SUN4I_CTL_REG,
-			SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER | SUN4I_CTL_TP);
+	sunxi_spi_write(sspi, SUNXI_CTL_REG,
+			SUNXI_CTL_ENABLE | SUNXI_CTL_MASTER | SUNXI_CTL_TP);
 
 	return 0;
 
@@ -358,10 +358,10 @@ out:
 	return ret;
 }
 
-static int sun4i_spi_runtime_suspend(struct device *dev)
+static int sunxi_spi_runtime_suspend(struct device *dev)
 {
 	struct spi_master *master = dev_get_drvdata(dev);
-	struct sun4i_spi *sspi = spi_master_get_devdata(master);
+	struct sunxi_spi *sspi = spi_master_get_devdata(master);
 
 	clk_disable_unprepare(sspi->mclk);
 	clk_disable_unprepare(sspi->hclk);
@@ -369,14 +369,14 @@ static int sun4i_spi_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int sun4i_spi_probe(struct platform_device *pdev)
+static int sunxi_spi_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
-	struct sun4i_spi *sspi;
+	struct sunxi_spi *sspi;
 	struct resource	*res;
 	int ret = 0, irq;
 
-	master = spi_alloc_master(&pdev->dev, sizeof(struct sun4i_spi));
+	master = spi_alloc_master(&pdev->dev, sizeof(struct sunxi_spi));
 	if (!master) {
 		dev_err(&pdev->dev, "Unable to allocate SPI Master\n");
 		return -ENOMEM;
@@ -399,8 +399,8 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 		goto err_free_master;
 	}
 
-	ret = devm_request_irq(&pdev->dev, irq, sun4i_spi_handler,
-			       0, "sun4i-spi", sspi);
+	ret = devm_request_irq(&pdev->dev, irq, sunxi_spi_handler,
+			       0, "sunxi-spi", sspi);
 	if (ret) {
 		dev_err(&pdev->dev, "Cannot request IRQ\n");
 		goto err_free_master;
@@ -409,14 +409,14 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 	sspi->master = master;
 	master->max_speed_hz = 100 * 1000 * 1000;
 	master->min_speed_hz =          3 * 1000;
-	master->set_cs = sun4i_spi_set_cs;
-	master->transfer_one = sun4i_spi_transfer_one;
+	master->set_cs = sunxi_spi_set_cs;
+	master->transfer_one = sunxi_spi_transfer_one;
 	master->num_chipselect = 4;
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
 	master->dev.of_node = pdev->dev.of_node;
 	master->auto_runtime_pm = true;
-	master->max_transfer_size = sun4i_spi_max_transfer_size;
+	master->max_transfer_size = sunxi_spi_max_transfer_size;
 
 	sspi->hclk = devm_clk_get(&pdev->dev, "ahb");
 	if (IS_ERR(sspi->hclk)) {
@@ -438,7 +438,7 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 	 * This wake-up/shutdown pattern is to be able to have the
 	 * device woken up, even if runtime_pm is disabled
 	 */
-	ret = sun4i_spi_runtime_resume(&pdev->dev);
+	ret = sunxi_spi_runtime_resume(&pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Couldn't resume the device\n");
 		goto err_free_master;
@@ -458,40 +458,40 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 
 err_pm_disable:
 	pm_runtime_disable(&pdev->dev);
-	sun4i_spi_runtime_suspend(&pdev->dev);
+	sunxi_spi_runtime_suspend(&pdev->dev);
 err_free_master:
 	spi_master_put(master);
 	return ret;
 }
 
-static int sun4i_spi_remove(struct platform_device *pdev)
+static int sunxi_spi_remove(struct platform_device *pdev)
 {
 	pm_runtime_disable(&pdev->dev);
 
 	return 0;
 }
 
-static const struct of_device_id sun4i_spi_match[] = {
+static const struct of_device_id sunxi_spi_match[] = {
 	{ .compatible = "allwinner,sun4i-a10-spi", },
 	{}
 };
-MODULE_DEVICE_TABLE(of, sun4i_spi_match);
+MODULE_DEVICE_TABLE(of, sunxi_spi_match);
 
-static const struct dev_pm_ops sun4i_spi_pm_ops = {
-	.runtime_resume		= sun4i_spi_runtime_resume,
-	.runtime_suspend	= sun4i_spi_runtime_suspend,
+static const struct dev_pm_ops sunxi_spi_pm_ops = {
+	.runtime_resume		= sunxi_spi_runtime_resume,
+	.runtime_suspend	= sunxi_spi_runtime_suspend,
 };
 
-static struct platform_driver sun4i_spi_driver = {
-	.probe	= sun4i_spi_probe,
-	.remove	= sun4i_spi_remove,
+static struct platform_driver sunxi_spi_driver = {
+	.probe	= sunxi_spi_probe,
+	.remove	= sunxi_spi_remove,
 	.driver	= {
-		.name		= "sun4i-spi",
-		.of_match_table	= sun4i_spi_match,
-		.pm		= &sun4i_spi_pm_ops,
+		.name		= "sunxi-spi",
+		.of_match_table	= sunxi_spi_match,
+		.pm		= &sunxi_spi_pm_ops,
 	},
 };
-module_platform_driver(sun4i_spi_driver);
+module_platform_driver(sunxi_spi_driver);
 
 MODULE_AUTHOR("Pan Nan <pannan-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org>");
 MODULE_AUTHOR("Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 06759d0..a27bf8f 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -23,62 +23,62 @@
 
 #include <linux/spi/spi.h>
 
-#define SUN6I_FIFO_DEPTH		128
-
-#define SUN6I_GBL_CTL_REG		0x04
-#define SUN6I_GBL_CTL_BUS_ENABLE		BIT(0)
-#define SUN6I_GBL_CTL_MASTER			BIT(1)
-#define SUN6I_GBL_CTL_TP			BIT(7)
-#define SUN6I_GBL_CTL_RST			BIT(31)
-
-#define SUN6I_TFR_CTL_REG		0x08
-#define SUN6I_TFR_CTL_CPHA			BIT(0)
-#define SUN6I_TFR_CTL_CPOL			BIT(1)
-#define SUN6I_TFR_CTL_SPOL			BIT(2)
-#define SUN6I_TFR_CTL_CS_MASK			0x30
-#define SUN6I_TFR_CTL_CS(cs)			(((cs) << 4) & SUN6I_TFR_CTL_CS_MASK)
-#define SUN6I_TFR_CTL_CS_MANUAL			BIT(6)
-#define SUN6I_TFR_CTL_CS_LEVEL			BIT(7)
-#define SUN6I_TFR_CTL_DHB			BIT(8)
-#define SUN6I_TFR_CTL_FBS			BIT(12)
-#define SUN6I_TFR_CTL_XCH			BIT(31)
-
-#define SUN6I_INT_CTL_REG		0x10
-#define SUN6I_INT_CTL_RF_OVF			BIT(8)
-#define SUN6I_INT_CTL_TC			BIT(12)
-
-#define SUN6I_INT_STA_REG		0x14
-
-#define SUN6I_FIFO_CTL_REG		0x18
-#define SUN6I_FIFO_CTL_RF_RST			BIT(15)
-#define SUN6I_FIFO_CTL_TF_RST			BIT(31)
-
-#define SUN6I_FIFO_STA_REG		0x1c
-#define SUN6I_FIFO_STA_RF_CNT_MASK		0x7f
-#define SUN6I_FIFO_STA_RF_CNT_BITS		0
-#define SUN6I_FIFO_STA_TF_CNT_MASK		0x7f
-#define SUN6I_FIFO_STA_TF_CNT_BITS		16
-
-#define SUN6I_CLK_CTL_REG		0x24
-#define SUN6I_CLK_CTL_CDR2_MASK			0xff
-#define SUN6I_CLK_CTL_CDR2(div)			(((div) & SUN6I_CLK_CTL_CDR2_MASK) << 0)
-#define SUN6I_CLK_CTL_CDR1_MASK			0xf
-#define SUN6I_CLK_CTL_CDR1(div)			(((div) & SUN6I_CLK_CTL_CDR1_MASK) << 8)
-#define SUN6I_CLK_CTL_DRS			BIT(12)
-
-#define SUN6I_BURST_CNT_REG		0x30
-#define SUN6I_BURST_CNT(cnt)			((cnt) & 0xffffff)
-
-#define SUN6I_XMIT_CNT_REG		0x34
-#define SUN6I_XMIT_CNT(cnt)			((cnt) & 0xffffff)
-
-#define SUN6I_BURST_CTL_CNT_REG		0x38
-#define SUN6I_BURST_CTL_CNT_STC(cnt)		((cnt) & 0xffffff)
-
-#define SUN6I_TXDATA_REG		0x200
-#define SUN6I_RXDATA_REG		0x300
-
-struct sun6i_spi {
+#define SUNXI_FIFO_DEPTH		128
+
+#define SUNXI_GBL_CTL_REG		0x04
+#define SUNXI_GBL_CTL_BUS_ENABLE	BIT(0)
+#define SUNXI_GBL_CTL_MASTER		BIT(1)
+#define SUNXI_GBL_CTL_TP		BIT(7)
+#define SUNXI_GBL_CTL_RST		BIT(31)
+
+#define SUNXI_TFR_CTL_REG		0x08
+#define SUNXI_TFR_CTL_CPHA		BIT(0)
+#define SUNXI_TFR_CTL_CPOL		BIT(1)
+#define SUNXI_TFR_CTL_SPOL		BIT(2)
+#define SUNXI_TFR_CTL_CS_MASK		0x30
+#define SUNXI_TFR_CTL_CS(cs)		(((cs) << 4) & SUNXI_TFR_CTL_CS_MASK)
+#define SUNXI_TFR_CTL_CS_MANUAL		BIT(6)
+#define SUNXI_TFR_CTL_CS_LEVEL		BIT(7)
+#define SUNXI_TFR_CTL_DHB		BIT(8)
+#define SUNXI_TFR_CTL_FBS		BIT(12)
+#define SUNXI_TFR_CTL_XCH		BIT(31)
+
+#define SUNXI_INT_CTL_REG		0x10
+#define SUNXI_INT_CTL_RF_OVF		BIT(8)
+#define SUNXI_INT_CTL_TC		BIT(12)
+
+#define SUNXI_INT_STA_REG		0x14
+
+#define SUNXI_FIFO_CTL_REG		0x18
+#define SUNXI_FIFO_CTL_RF_RST		BIT(15)
+#define SUNXI_FIFO_CTL_TF_RST		BIT(31)
+
+#define SUNXI_FIFO_STA_REG		0x1c
+#define SUNXI_FIFO_STA_RF_CNT_MASK	0x7f
+#define SUNXI_FIFO_STA_RF_CNT_BITS	0
+#define SUNXI_FIFO_STA_TF_CNT_MASK	0x7f
+#define SUNXI_FIFO_STA_TF_CNT_BITS	16
+
+#define SUNXI_CLK_CTL_REG		0x24
+#define SUNXI_CLK_CTL_CDR2_MASK		0xff
+#define SUNXI_CLK_CTL_CDR2(div)		(((div) & SUNXI_CLK_CTL_CDR2_MASK) << 0)
+#define SUNXI_CLK_CTL_CDR1_MASK		0xf
+#define SUNXI_CLK_CTL_CDR1(div)		(((div) & SUNXI_CLK_CTL_CDR1_MASK) << 8)
+#define SUNXI_CLK_CTL_DRS		BIT(12)
+
+#define SUNXI_BURST_CNT_REG		0x30
+#define SUNXI_BURST_CNT(cnt)		((cnt) & 0xffffff)
+
+#define SUNXI_XMIT_CNT_REG		0x34
+#define SUNXI_XMIT_CNT(cnt)		((cnt) & 0xffffff)
+
+#define SUNXI_BURST_CTL_CNT_REG		0x38
+#define SUNXI_BURST_CTL_CNT_STC(cnt)	((cnt) & 0xffffff)
+
+#define SUNXI_TXDATA_REG		0x200
+#define SUNXI_RXDATA_REG		0x300
+
+struct sunxi_spi {
 	struct spi_master	*master;
 	void __iomem		*base_addr;
 	struct clk		*hclk;
@@ -92,37 +92,37 @@ struct sun6i_spi {
 	int			len;
 };
 
-static inline u32 sun6i_spi_read(struct sun6i_spi *sspi, u32 reg)
+static inline u32 sunxi_spi_read(struct sunxi_spi *sspi, u32 reg)
 {
 	return readl(sspi->base_addr + reg);
 }
 
-static inline void sun6i_spi_write(struct sun6i_spi *sspi, u32 reg, u32 value)
+static inline void sunxi_spi_write(struct sunxi_spi *sspi, u32 reg, u32 value)
 {
 	writel(value, sspi->base_addr + reg);
 }
 
-static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi, int len)
+static inline void sunxi_spi_drain_fifo(struct sunxi_spi *sspi, int len)
 {
 	u32 reg, cnt;
 	u8 byte;
 
 	/* See how much data is available */
-	reg = sun6i_spi_read(sspi, SUN6I_FIFO_STA_REG);
-	reg &= SUN6I_FIFO_STA_RF_CNT_MASK;
-	cnt = reg >> SUN6I_FIFO_STA_RF_CNT_BITS;
+	reg = sunxi_spi_read(sspi, SUNXI_FIFO_STA_REG);
+	reg &= SUNXI_FIFO_STA_RF_CNT_MASK;
+	cnt = reg >> SUNXI_FIFO_STA_RF_CNT_BITS;
 
 	if (len > cnt)
 		len = cnt;
 
 	while (len--) {
-		byte = readb(sspi->base_addr + SUN6I_RXDATA_REG);
+		byte = readb(sspi->base_addr + SUNXI_RXDATA_REG);
 		if (sspi->rx_buf)
 			*sspi->rx_buf++ = byte;
 	}
 }
 
-static inline void sun6i_spi_fill_fifo(struct sun6i_spi *sspi, int len)
+static inline void sunxi_spi_fill_fifo(struct sunxi_spi *sspi, int len)
 {
 	u8 byte;
 
@@ -131,27 +131,27 @@ static inline void sun6i_spi_fill_fifo(struct sun6i_spi *sspi, int len)
 
 	while (len--) {
 		byte = sspi->tx_buf ? *sspi->tx_buf++ : 0;
-		writeb(byte, sspi->base_addr + SUN6I_TXDATA_REG);
+		writeb(byte, sspi->base_addr + SUNXI_TXDATA_REG);
 		sspi->len--;
 	}
 }
 
-static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
+static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 {
-	struct sun6i_spi *sspi = spi_master_get_devdata(spi->master);
+	struct sunxi_spi *sspi = spi_master_get_devdata(spi->master);
 	u32 reg;
 
-	reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
-	reg &= ~SUN6I_TFR_CTL_CS_MASK;
-	reg |= SUN6I_TFR_CTL_CS(spi->chip_select);
+	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
+	reg &= ~SUNXI_TFR_CTL_CS_MASK;
+	reg |= SUNXI_TFR_CTL_CS(spi->chip_select);
 
 	/* We want to control the chip select manually */
-	reg |= SUN6I_TFR_CTL_CS_MANUAL;
+	reg |= SUNXI_TFR_CTL_CS_MANUAL;
 
 	if (enable)
-		reg |= SUN6I_TFR_CTL_CS_LEVEL;
+		reg |= SUNXI_TFR_CTL_CS_LEVEL;
 	else
-		reg &= ~SUN6I_TFR_CTL_CS_LEVEL;
+		reg &= ~SUNXI_TFR_CTL_CS_LEVEL;
 
 	/*
 	 * Even though this looks irrelevant since we are supposed to
@@ -165,23 +165,23 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
 	 * low.
 	 */
 	if (spi->mode & SPI_CS_HIGH)
-		reg &= ~SUN6I_TFR_CTL_SPOL;
+		reg &= ~SUNXI_TFR_CTL_SPOL;
 	else
-		reg |= SUN6I_TFR_CTL_SPOL;
+		reg |= SUNXI_TFR_CTL_SPOL;
 
-	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
 }
 
-static size_t sun6i_spi_max_transfer_size(struct spi_device *spi)
+static size_t sunxi_spi_max_transfer_size(struct spi_device *spi)
 {
-	return SUN6I_FIFO_DEPTH - 1;
+	return SUNXI_FIFO_DEPTH - 1;
 }
 
-static int sun6i_spi_transfer_one(struct spi_master *master,
+static int sunxi_spi_transfer_one(struct spi_master *master,
 				  struct spi_device *spi,
 				  struct spi_transfer *tfr)
 {
-	struct sun6i_spi *sspi = spi_master_get_devdata(master);
+	struct sunxi_spi *sspi = spi_master_get_devdata(master);
 	unsigned int mclk_rate, div, timeout;
 	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
@@ -189,7 +189,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 	u32 reg;
 
 	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > SUN6I_FIFO_DEPTH)
+	if (tfr->len > SUNXI_FIFO_DEPTH)
 		return -EINVAL;
 
 	reinit_completion(&sspi->done);
@@ -198,43 +198,43 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 	sspi->len = tfr->len;
 
 	/* Clear pending interrupts */
-	sun6i_spi_write(sspi, SUN6I_INT_STA_REG, ~0);
+	sunxi_spi_write(sspi, SUNXI_INT_STA_REG, ~0);
 
 	/* Reset FIFO */
-	sun6i_spi_write(sspi, SUN6I_FIFO_CTL_REG,
-			SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
+	sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
+			SUNXI_FIFO_CTL_RF_RST | SUNXI_FIFO_CTL_TF_RST);
 
 	/*
 	 * Setup the transfer control register: Chip Select,
 	 * polarities, etc.
 	 */
-	reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
+	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
 
 	if (spi->mode & SPI_CPOL)
-		reg |= SUN6I_TFR_CTL_CPOL;
+		reg |= SUNXI_TFR_CTL_CPOL;
 	else
-		reg &= ~SUN6I_TFR_CTL_CPOL;
+		reg &= ~SUNXI_TFR_CTL_CPOL;
 
 	if (spi->mode & SPI_CPHA)
-		reg |= SUN6I_TFR_CTL_CPHA;
+		reg |= SUNXI_TFR_CTL_CPHA;
 	else
-		reg &= ~SUN6I_TFR_CTL_CPHA;
+		reg &= ~SUNXI_TFR_CTL_CPHA;
 
 	if (spi->mode & SPI_LSB_FIRST)
-		reg |= SUN6I_TFR_CTL_FBS;
+		reg |= SUNXI_TFR_CTL_FBS;
 	else
-		reg &= ~SUN6I_TFR_CTL_FBS;
+		reg &= ~SUNXI_TFR_CTL_FBS;
 
 	/*
 	 * If it's a TX only transfer, we don't want to fill the RX
 	 * FIFO with bogus data
 	 */
 	if (sspi->rx_buf)
-		reg &= ~SUN6I_TFR_CTL_DHB;
+		reg &= ~SUNXI_TFR_CTL_DHB;
 	else
-		reg |= SUN6I_TFR_CTL_DHB;
+		reg |= SUNXI_TFR_CTL_DHB;
 
-	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
 
 	/* Ensure that we have a parent clock fast enough */
 	mclk_rate = clk_get_rate(sspi->mclk);
@@ -258,37 +258,37 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 	 * frequency, fall back to CDR1.
 	 */
 	div = mclk_rate / (2 * tfr->speed_hz);
-	if (div <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) {
+	if (div <= (SUNXI_CLK_CTL_CDR2_MASK + 1)) {
 		if (div > 0)
 			div--;
 
-		reg = SUN6I_CLK_CTL_CDR2(div) | SUN6I_CLK_CTL_DRS;
+		reg = SUNXI_CLK_CTL_CDR2(div) | SUNXI_CLK_CTL_DRS;
 	} else {
 		div = ilog2(mclk_rate) - ilog2(tfr->speed_hz);
-		reg = SUN6I_CLK_CTL_CDR1(div);
+		reg = SUNXI_CLK_CTL_CDR1(div);
 	}
 
-	sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg);
+	sunxi_spi_write(sspi, SUNXI_CLK_CTL_REG, reg);
 
 	/* Setup the transfer now... */
 	if (sspi->tx_buf)
 		tx_len = tfr->len;
 
 	/* Setup the counters */
-	sun6i_spi_write(sspi, SUN6I_BURST_CNT_REG, SUN6I_BURST_CNT(tfr->len));
-	sun6i_spi_write(sspi, SUN6I_XMIT_CNT_REG, SUN6I_XMIT_CNT(tx_len));
-	sun6i_spi_write(sspi, SUN6I_BURST_CTL_CNT_REG,
-			SUN6I_BURST_CTL_CNT_STC(tx_len));
+	sunxi_spi_write(sspi, SUNXI_BURST_CNT_REG, SUNXI_BURST_CNT(tfr->len));
+	sunxi_spi_write(sspi, SUNXI_XMIT_CNT_REG, SUNXI_XMIT_CNT(tx_len));
+	sunxi_spi_write(sspi, SUNXI_BURST_CTL_CNT_REG,
+			SUNXI_BURST_CTL_CNT_STC(tx_len));
 
 	/* Fill the TX FIFO */
-	sun6i_spi_fill_fifo(sspi, SUN6I_FIFO_DEPTH);
+	sunxi_spi_fill_fifo(sspi, SUNXI_FIFO_DEPTH);
 
 	/* Enable the interrupts */
-	sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, SUN6I_INT_CTL_TC);
+	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, SUNXI_INT_CTL_TC);
 
 	/* Start the transfer */
-	reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
-	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg | SUN6I_TFR_CTL_XCH);
+	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg | SUNXI_TFR_CTL_XCH);
 
 	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
 	start = jiffies;
@@ -304,22 +304,22 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
-	sun6i_spi_drain_fifo(sspi, SUN6I_FIFO_DEPTH);
+	sunxi_spi_drain_fifo(sspi, SUNXI_FIFO_DEPTH);
 
 out:
-	sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, 0);
+	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, 0);
 
 	return ret;
 }
 
-static irqreturn_t sun6i_spi_handler(int irq, void *dev_id)
+static irqreturn_t sunxi_spi_handler(int irq, void *dev_id)
 {
-	struct sun6i_spi *sspi = dev_id;
-	u32 status = sun6i_spi_read(sspi, SUN6I_INT_STA_REG);
+	struct sunxi_spi *sspi = dev_id;
+	u32 status = sunxi_spi_read(sspi, SUNXI_INT_STA_REG);
 
 	/* Transfer complete */
-	if (status & SUN6I_INT_CTL_TC) {
-		sun6i_spi_write(sspi, SUN6I_INT_STA_REG, SUN6I_INT_CTL_TC);
+	if (status & SUNXI_INT_CTL_TC) {
+		sunxi_spi_write(sspi, SUNXI_INT_STA_REG, SUNXI_INT_CTL_TC);
 		complete(&sspi->done);
 		return IRQ_HANDLED;
 	}
@@ -327,10 +327,10 @@ static irqreturn_t sun6i_spi_handler(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-static int sun6i_spi_runtime_resume(struct device *dev)
+static int sunxi_spi_runtime_resume(struct device *dev)
 {
 	struct spi_master *master = dev_get_drvdata(dev);
-	struct sun6i_spi *sspi = spi_master_get_devdata(master);
+	struct sunxi_spi *sspi = spi_master_get_devdata(master);
 	int ret;
 
 	ret = clk_prepare_enable(sspi->hclk);
@@ -351,8 +351,8 @@ static int sun6i_spi_runtime_resume(struct device *dev)
 		goto err2;
 	}
 
-	sun6i_spi_write(sspi, SUN6I_GBL_CTL_REG,
-			SUN6I_GBL_CTL_BUS_ENABLE | SUN6I_GBL_CTL_MASTER | SUN6I_GBL_CTL_TP);
+	sunxi_spi_write(sspi, SUNXI_GBL_CTL_REG,
+			SUNXI_GBL_CTL_BUS_ENABLE | SUNXI_GBL_CTL_MASTER | SUNXI_GBL_CTL_TP);
 
 	return 0;
 
@@ -364,10 +364,10 @@ out:
 	return ret;
 }
 
-static int sun6i_spi_runtime_suspend(struct device *dev)
+static int sunxi_spi_runtime_suspend(struct device *dev)
 {
 	struct spi_master *master = dev_get_drvdata(dev);
-	struct sun6i_spi *sspi = spi_master_get_devdata(master);
+	struct sunxi_spi *sspi = spi_master_get_devdata(master);
 
 	reset_control_assert(sspi->rstc);
 	clk_disable_unprepare(sspi->mclk);
@@ -376,14 +376,14 @@ static int sun6i_spi_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int sun6i_spi_probe(struct platform_device *pdev)
+static int sunxi_spi_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
-	struct sun6i_spi *sspi;
+	struct sunxi_spi *sspi;
 	struct resource	*res;
 	int ret = 0, irq;
 
-	master = spi_alloc_master(&pdev->dev, sizeof(struct sun6i_spi));
+	master = spi_alloc_master(&pdev->dev, sizeof(struct sunxi_spi));
 	if (!master) {
 		dev_err(&pdev->dev, "Unable to allocate SPI Master\n");
 		return -ENOMEM;
@@ -406,8 +406,8 @@ static int sun6i_spi_probe(struct platform_device *pdev)
 		goto err_free_master;
 	}
 
-	ret = devm_request_irq(&pdev->dev, irq, sun6i_spi_handler,
-			       0, "sun6i-spi", sspi);
+	ret = devm_request_irq(&pdev->dev, irq, sunxi_spi_handler,
+			       0, "sunxi-spi", sspi);
 	if (ret) {
 		dev_err(&pdev->dev, "Cannot request IRQ\n");
 		goto err_free_master;
@@ -416,14 +416,14 @@ static int sun6i_spi_probe(struct platform_device *pdev)
 	sspi->master = master;
 	master->max_speed_hz = 100 * 1000 * 1000;
 	master->min_speed_hz =          3 * 1000;
-	master->set_cs = sun6i_spi_set_cs;
-	master->transfer_one = sun6i_spi_transfer_one;
+	master->set_cs = sunxi_spi_set_cs;
+	master->transfer_one = sunxi_spi_transfer_one;
 	master->num_chipselect = 4;
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
 	master->dev.of_node = pdev->dev.of_node;
 	master->auto_runtime_pm = true;
-	master->max_transfer_size = sun6i_spi_max_transfer_size;
+	master->max_transfer_size = sunxi_spi_max_transfer_size;
 
 	sspi->hclk = devm_clk_get(&pdev->dev, "ahb");
 	if (IS_ERR(sspi->hclk)) {
@@ -452,7 +452,7 @@ static int sun6i_spi_probe(struct platform_device *pdev)
 	 * This wake-up/shutdown pattern is to be able to have the
 	 * device woken up, even if runtime_pm is disabled
 	 */
-	ret = sun6i_spi_runtime_resume(&pdev->dev);
+	ret = sunxi_spi_runtime_resume(&pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Couldn't resume the device\n");
 		goto err_free_master;
@@ -472,40 +472,40 @@ static int sun6i_spi_probe(struct platform_device *pdev)
 
 err_pm_disable:
 	pm_runtime_disable(&pdev->dev);
-	sun6i_spi_runtime_suspend(&pdev->dev);
+	sunxi_spi_runtime_suspend(&pdev->dev);
 err_free_master:
 	spi_master_put(master);
 	return ret;
 }
 
-static int sun6i_spi_remove(struct platform_device *pdev)
+static int sunxi_spi_remove(struct platform_device *pdev)
 {
 	pm_runtime_disable(&pdev->dev);
 
 	return 0;
 }
 
-static const struct of_device_id sun6i_spi_match[] = {
+static const struct of_device_id sunxi_spi_match[] = {
 	{ .compatible = "allwinner,sun6i-a31-spi", },
 	{}
 };
-MODULE_DEVICE_TABLE(of, sun6i_spi_match);
+MODULE_DEVICE_TABLE(of, sunxi_spi_match);
 
-static const struct dev_pm_ops sun6i_spi_pm_ops = {
-	.runtime_resume		= sun6i_spi_runtime_resume,
-	.runtime_suspend	= sun6i_spi_runtime_suspend,
+static const struct dev_pm_ops sunxi_spi_pm_ops = {
+	.runtime_resume		= sunxi_spi_runtime_resume,
+	.runtime_suspend	= sunxi_spi_runtime_suspend,
 };
 
-static struct platform_driver sun6i_spi_driver = {
-	.probe	= sun6i_spi_probe,
-	.remove	= sun6i_spi_remove,
+static struct platform_driver sunxi_spi_driver = {
+	.probe	= sunxi_spi_probe,
+	.remove	= sunxi_spi_remove,
 	.driver	= {
-		.name		= "sun6i-spi",
-		.of_match_table	= sun6i_spi_match,
-		.pm		= &sun6i_spi_pm_ops,
+		.name		= "sunxi-spi",
+		.of_match_table	= sunxi_spi_match,
+		.pm		= &sunxi_spi_pm_ops,
 	},
 };
-module_platform_driver(sun6i_spi_driver);
+module_platform_driver(sunxi_spi_driver);
 
 MODULE_AUTHOR("Pan Nan <pannan-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org>");
 MODULE_AUTHOR("Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
-- 
2.8.1

[PATCH v3 05/13] spi: sun6i: update CS handling from spi-sun4i

[Michal Suchanek]

While trying to merge sun4i and sun6i spi drivers I noticed
the sun4i driver seems to have more reasonable CS handling.

Update sun6i to same.

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-sun6i.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index bc29a98..06759d0 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -145,11 +145,30 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
 	reg &= ~SUN6I_TFR_CTL_CS_MASK;
 	reg |= SUN6I_TFR_CTL_CS(spi->chip_select);
 
+	/* We want to control the chip select manually */
+	reg |= SUN6I_TFR_CTL_CS_MANUAL;
+
 	if (enable)
 		reg |= SUN6I_TFR_CTL_CS_LEVEL;
 	else
 		reg &= ~SUN6I_TFR_CTL_CS_LEVEL;
 
+	/*
+	 * Even though this looks irrelevant since we are supposed to
+	 * be controlling the chip select manually, this bit also
+	 * controls the levels of the chip select for inactive
+	 * devices.
+	 *
+	 * If we don't set it, the chip select level will go low by
+	 * default when the device is idle, which is not really
+	 * expected in the common case where the chip select is active
+	 * low.
+	 */
+	if (spi->mode & SPI_CS_HIGH)
+		reg &= ~SUN6I_TFR_CTL_SPOL;
+	else
+		reg |= SUN6I_TFR_CTL_SPOL;
+
 	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
 }
 
@@ -215,9 +234,6 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 	else
 		reg |= SUN6I_TFR_CTL_DHB;
 
-	/* We want to control the chip select manually */
-	reg |= SUN6I_TFR_CTL_CS_MANUAL;
-
 	sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
 
 	/* Ensure that we have a parent clock fast enough */
-- 
2.8.1

[PATCH v3 08/13] spi: sunxi: synchronize whitespace, comments, struct

[Michal Suchanek]

Minimize differences between sun4i and sun6i driver without code
changes.

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-sun4i.c | 11 ++++++-----
 drivers/spi/spi-sun6i.c |  8 +++-----
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index b7f8de1..bbb0996 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 #include <linux/spi/spi.h>
 
@@ -56,7 +57,7 @@
 
 #define SUNXI_CLK_CTL_REG		0x1c
 #define SUNXI_CLK_CTL_CDR2_MASK		0xff
-#define SUNXI_CLK_CTL_CDR2(div)		((div) & SUNXI_CLK_CTL_CDR2_MASK)
+#define SUNXI_CLK_CTL_CDR2(div)		(((div) & SUNXI_CLK_CTL_CDR2_MASK) << 0)
 #define SUNXI_CLK_CTL_CDR1_MASK		0xf
 #define SUNXI_CLK_CTL_CDR1(div)		(((div) & SUNXI_CLK_CTL_CDR1_MASK) << 8)
 #define SUNXI_CLK_CTL_DRS		BIT(12)
@@ -78,6 +79,7 @@ struct sunxi_spi {
 	void __iomem		*base_addr;
 	struct clk		*hclk;
 	struct clk		*mclk;
+	struct reset_control	*rstc;
 
 	struct completion	done;
 
@@ -136,7 +138,6 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 	u32 reg;
 
 	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-
 	reg &= ~SUNXI_TFR_CTL_CS_MASK;
 	reg |= SUNXI_TFR_CTL_CS(spi->chip_select);
 
@@ -198,7 +199,6 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	/* Clear pending interrupts */
 	sunxi_spi_write(sspi, SUNXI_INT_STA_REG, ~0);
 
-
 	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
 
 	/* Reset FIFOs */
@@ -224,7 +224,6 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	else
 		reg &= ~SUNXI_TFR_CTL_FBS;
 
-
 	/*
 	 * If it's a TX only transfer, we don't want to fill the RX
 	 * FIFO with bogus data
@@ -248,7 +247,7 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	 *
 	 * We have two choices there. Either we can use the clock
 	 * divide rate 1, which is calculated thanks to this formula:
-	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
+	 * SPI_CLK = MOD_CLK / (2 ^ cdr)
 	 * Or we can use CDR2, which is calculated with the formula:
 	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
 	 * Wether we use the former or the latter is set through the
@@ -352,6 +351,8 @@ static int sunxi_spi_runtime_resume(struct device *dev)
 
 	return 0;
 
+err2:
+	clk_disable_unprepare(sspi->mclk);
 err:
 	clk_disable_unprepare(sspi->hclk);
 out:
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index f26b52a..d14a953 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -29,7 +29,6 @@
 #define SUNXI_CTL_ENABLE		BIT(0)
 #define SUNXI_CTL_MASTER		BIT(1)
 #define SUNXI_CTL_TP			BIT(7)
-#define SUNXI_GBL_CTL_RST		BIT(31)
 
 #define SUNXI_TFR_CTL_REG		0x08
 #define SUNXI_TFR_CTL_CPHA		BIT(0)
@@ -44,7 +43,6 @@
 #define SUNXI_TFR_CTL_XCH		BIT(31)
 
 #define SUNXI_INT_CTL_REG		0x10
-#define SUNXI_INT_CTL_RF_OVF		BIT(8)
 #define SUNXI_INT_CTL_TC		BIT(12)
 
 #define SUNXI_INT_STA_REG		0x14
@@ -200,7 +198,9 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	/* Clear pending interrupts */
 	sunxi_spi_write(sspi, SUNXI_INT_STA_REG, ~0);
 
-	/* Reset FIFO */
+	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
+
+	/* Reset FIFOs */
 	sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
 			SUNXI_CTL_RF_RST | SUNXI_CTL_TF_RST);
 
@@ -208,8 +208,6 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	 * Setup the transfer control register: Chip Select,
 	 * polarities, etc.
 	 */
-	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-
 	if (spi->mode & SPI_CPOL)
 		reg |= SUNXI_TFR_CTL_CPOL;
 	else
-- 
2.8.1

[PATCH v3 09/13] spi: sunxi: use register map

[Michal Suchanek]

The driver code uses the fact that several bits are set in one register
to set them at once.

Between hardware revisions these bits might migrate to different
position or different register altogether but the logical bit groups
that are used together end up in the same register.

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-sun4i.c | 227 ++++++++++++++++++++++++++++++--------------
 drivers/spi/spi-sun6i.c | 243 ++++++++++++++++++++++++++++++++----------------
 2 files changed, 322 insertions(+), 148 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index bbb0996..0b8e6c6 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -23,63 +23,118 @@
 
 #include <linux/spi/spi.h>
 
-#define SUNXI_FIFO_DEPTH		64
+#define SUN4I_FIFO_DEPTH		64
+#define SUN6I_FIFO_DEPTH		128
 
-#define SUNXI_RXDATA_REG		0x00
+#define SUN4I_COMPATIBLE "allwinner,sun4i-a10-spi"
+#define SUN6I_COMPATIBLE "allwinner,sun6i-a31-spi"
 
-#define SUNXI_TXDATA_REG		0x04
+#define SUNXI_TFR_CTL_CS(bitmap, cs)	(((cs) << \
+					  (bitmap)[SUNXI_TFR_CTL_CS_SHIFT]) \
+					 & (bitmap)[SUNXI_TFR_CTL_CS_MASK])
 
-#define SUNXI_TFR_CTL_REG		0x08
-#define SUNXI_CTL_ENABLE		BIT(0)
-#define SUNXI_CTL_MASTER		BIT(1)
-#define SUNXI_TFR_CTL_CPHA		BIT(2)
-#define SUNXI_TFR_CTL_CPOL		BIT(3)
-#define SUNXI_TFR_CTL_SPOL		BIT(4)
-#define SUNXI_TFR_CTL_FBS		BIT(6)
-#define SUNXI_CTL_TF_RST		BIT(8)
-#define SUNXI_CTL_RF_RST		BIT(9)
-#define SUNXI_TFR_CTL_XCH		BIT(10)
-#define SUNXI_TFR_CTL_CS_MASK		0x3000
-#define SUNXI_TFR_CTL_CS(cs)		(((cs) << 12) & SUNXI_TFR_CTL_CS_MASK)
-#define SUNXI_TFR_CTL_DHB		BIT(15)
-#define SUNXI_TFR_CTL_CS_MANUAL		BIT(16)
-#define SUNXI_TFR_CTL_CS_LEVEL		BIT(17)
-#define SUNXI_CTL_TP			BIT(18)
+#define SUNXI_CNT_MASK			0xffffff
+#define SUNXI_XMIT_CNT(cnt)		((cnt) & SUNXI_CNT_MASK)
+#define SUNXI_BURST_CNT(cnt)		((cnt) & SUNXI_CNT_MASK)
+#define SUNXI_BURST_CTL_CNT_STC(cnt)	((cnt) & SUNXI_CNT_MASK)
 
-#define SUNXI_INT_CTL_REG		0x0c
-#define SUNXI_INT_CTL_TC		BIT(16)
-
-#define SUNXI_INT_STA_REG		0x10
-
-#define SUNXI_DMA_CTL_REG		0x14
-
-#define SUNXI_WAIT_REG			0x18
-
-#define SUNXI_CLK_CTL_REG		0x1c
+#define SUNXI_CLK_CTL_DRS		BIT(12)
 #define SUNXI_CLK_CTL_CDR2_MASK		0xff
 #define SUNXI_CLK_CTL_CDR2(div)		(((div) & SUNXI_CLK_CTL_CDR2_MASK) << 0)
 #define SUNXI_CLK_CTL_CDR1_MASK		0xf
 #define SUNXI_CLK_CTL_CDR1(div)		(((div) & SUNXI_CLK_CTL_CDR1_MASK) << 8)
-#define SUNXI_CLK_CTL_DRS		BIT(12)
-
-#define SUNXI_BURST_CNT_REG		0x20
-#define SUNXI_BURST_CNT(cnt)		((cnt) & 0xffffff)
 
-#define SUNXI_XMIT_CNT_REG		0x24
-#define SUNXI_XMIT_CNT(cnt)		((cnt) & 0xffffff)
-
-#define SUNXI_FIFO_STA_REG		0x28
 #define SUNXI_FIFO_STA_RF_CNT_MASK	0x7f
 #define SUNXI_FIFO_STA_RF_CNT_BITS	0
 #define SUNXI_FIFO_STA_TF_CNT_MASK	0x7f
 #define SUNXI_FIFO_STA_TF_CNT_BITS	16
 
+enum SPI_SUNXI_TYPE {
+	SPI_SUN4I = 1,
+	SPI_SUN6I,
+};
+
+enum SUNXI_REG_ENUM {
+	SUNXI_RXDATA_REG,
+	SUNXI_TXDATA_REG,
+	SUNXI_TFR_CTL_REG,
+	SUNXI_INT_CTL_REG,
+	SUNXI_INT_STA_REG,
+	SUNXI_WAIT_REG,
+	SUNXI_CLK_CTL_REG,
+	SUNXI_BURST_CNT_REG,
+	SUNXI_XMIT_CNT_REG,
+	SUNXI_FIFO_STA_REG,
+	SUNXI_VERSION_REG,
+	SUNXI_GBL_CTL_REG,
+	SUNXI_FIFO_CTL_REG,
+	SUNXI_BURST_CTL_CNT_REG,
+	SUNXI_NUM_REGS
+};
+
+static int sun4i_regmap[SUNXI_NUM_REGS] = {
+/* SUNXI_RXDATA_REG */			0x00,
+/* SUNXI_TXDATA_REG */			0x04,
+/* SUNXI_TFR_CTL_REG */			0x08,
+/* SUNXI_INT_CTL_REG */			0x0c,
+/* SUNXI_INT_STA_REG */			0x10,
+/* SUNXI_WAIT_REG */			0x18,
+/* SUNXI_CLK_CTL_REG */			0x1c,
+/* SUNXI_BURST_CNT_REG */		0x20,
+/* SUNXI_XMIT_CNT_REG */		0x24,
+/* SUNXI_FIFO_STA_REG */		0x28,
+-1, -1, -1, -1
+};
+
+enum SUNXI_BITMAP_ENUM {
+	SUNXI_CTL_ENABLE,
+	SUNXI_CTL_MASTER,
+	SUNXI_TFR_CTL_CPHA,
+	SUNXI_TFR_CTL_CPOL,
+	SUNXI_TFR_CTL_CS_ACTIVE_LOW,
+	SUNXI_TFR_CTL_FBS,
+	SUNXI_CTL_TF_RST,
+	SUNXI_CTL_RF_RST,
+	SUNXI_TFR_CTL_XCH,
+	SUNXI_TFR_CTL_CS_MASK,
+	SUNXI_TFR_CTL_CS_SHIFT,
+	SUNXI_TFR_CTL_DHB,
+	SUNXI_TFR_CTL_CS_MANUAL,
+	SUNXI_TFR_CTL_CS_LEVEL,
+	SUNXI_CTL_TP,
+	SUNXI_INT_CTL_TC,
+	SUNXI_BITMAP_SIZE
+};
+
+static int sun4i_bitmap[SUNXI_BITMAP_SIZE] = {
+/* SUNXI_CTL_ENABLE */			BIT(0),
+/* SUNXI_CTL_MASTER */			BIT(1),
+/* SUNXI_TFR_CTL_CPHA */		BIT(2),
+/* SUNXI_TFR_CTL_CPOL */		BIT(3),
+/* SUNXI_TFR_CTL_CS_ACTIVE_LOW */	BIT(4),
+/* SUNXI_TFR_CTL_FBS */			BIT(6),
+/* SUNXI_CTL_TF_RST */			BIT(8),
+/* SUNXI_CTL_RF_RST */			BIT(9),
+/* SUNXI_TFR_CTL_XCH */			BIT(10),
+/* SUNXI_TFR_CTL_CS_MASK */		0x3000,
+/* SUNXI_TFR_CTL_CS_SHIFT */		12,
+/* SUNXI_TFR_CTL_DHB */			BIT(15),
+/* SUNXI_TFR_CTL_CS_MANUAL */		BIT(16),
+/* SUNXI_TFR_CTL_CS_LEVEL */		BIT(17),
+/* SUNXI_CTL_TP */			BIT(18),
+/* SUNXI_INT_CTL_TC */			BIT(16),
+};
+
 struct sunxi_spi {
 	struct spi_master	*master;
 	void __iomem		*base_addr;
 	struct clk		*hclk;
 	struct clk		*mclk;
 	struct reset_control	*rstc;
+	int			(*regmap)[SUNXI_NUM_REGS];
+	int			(*bitmap)[SUNXI_BITMAP_SIZE];
+	int			fifo_depth;
+	int			type;
 
 	struct completion	done;
 
@@ -88,14 +143,31 @@ struct sunxi_spi {
 	int			len;
 };
 
-static inline u32 sunxi_spi_read(struct sunxi_spi *sspi, u32 reg)
+static inline u32 sspi_reg(struct sunxi_spi *sspi, enum SUNXI_REG_ENUM name)
+{
+	BUG_ON((name >= SUNXI_NUM_REGS) || (name < 0) ||
+	       (*sspi->regmap)[name] < 0);
+	return (*sspi->regmap)[name];
+}
+
+static inline u32 sunxi_spi_read(struct sunxi_spi *sspi,
+				 enum SUNXI_REG_ENUM name)
 {
-	return readl(sspi->base_addr + reg);
+	return readl(sspi->base_addr + sspi_reg(sspi, name));
 }
 
-static inline void sunxi_spi_write(struct sunxi_spi *sspi, u32 reg, u32 value)
+static inline void sunxi_spi_write(struct sunxi_spi *sspi,
+				   enum SUNXI_REG_ENUM name, u32 value)
 {
-	writel(value, sspi->base_addr + reg);
+	writel(value, sspi->base_addr + sspi_reg(sspi, name));
+}
+
+static inline u32 sspi_bits(struct sunxi_spi *sspi,
+			    enum SUNXI_BITMAP_ENUM name)
+{
+	BUG_ON((name >= SUNXI_BITMAP_SIZE) || (name < 0) ||
+	       (*sspi->bitmap)[name] <= 0);
+	return (*sspi->bitmap)[name];
 }
 
 static inline void sunxi_spi_drain_fifo(struct sunxi_spi *sspi, int len)
@@ -112,7 +184,8 @@ static inline void sunxi_spi_drain_fifo(struct sunxi_spi *sspi, int len)
 		len = cnt;
 
 	while (len--) {
-		byte = readb(sspi->base_addr + SUNXI_RXDATA_REG);
+		byte = readb(sspi->base_addr +
+			     sspi_reg(sspi, SUNXI_RXDATA_REG));
 		if (sspi->rx_buf)
 			*sspi->rx_buf++ = byte;
 	}
@@ -127,7 +200,8 @@ static inline void sunxi_spi_fill_fifo(struct sunxi_spi *sspi, int len)
 
 	while (len--) {
 		byte = sspi->tx_buf ? *sspi->tx_buf++ : 0;
-		writeb(byte, sspi->base_addr + SUNXI_TXDATA_REG);
+		writeb(byte, sspi->base_addr +
+		       sspi_reg(sspi, SUNXI_TXDATA_REG));
 		sspi->len--;
 	}
 }
@@ -138,16 +212,16 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 	u32 reg;
 
 	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-	reg &= ~SUNXI_TFR_CTL_CS_MASK;
-	reg |= SUNXI_TFR_CTL_CS(spi->chip_select);
+	reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CS_MASK);
+	reg |= SUNXI_TFR_CTL_CS(*sspi->bitmap, spi->chip_select);
 
 	/* We want to control the chip select manually */
-	reg |= SUNXI_TFR_CTL_CS_MANUAL;
+	reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CS_MANUAL);
 
 	if (enable)
-		reg |= SUNXI_TFR_CTL_CS_LEVEL;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CS_LEVEL);
 	else
-		reg &= ~SUNXI_TFR_CTL_CS_LEVEL;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CS_LEVEL);
 
 	/*
 	 * Even though this looks irrelevant since we are supposed to
@@ -161,16 +235,19 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 	 * low.
 	 */
 	if (spi->mode & SPI_CS_HIGH)
-		reg &= ~SUNXI_TFR_CTL_SPOL;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CS_ACTIVE_LOW);
 	else
-		reg |= SUNXI_TFR_CTL_SPOL;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CS_ACTIVE_LOW);
 
 	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
 }
 
 static size_t sunxi_spi_max_transfer_size(struct spi_device *spi)
 {
-	return SUNXI_FIFO_DEPTH - 1;
+	struct spi_master *master = spi->master;
+	struct sunxi_spi *sspi = spi_master_get_devdata(master);
+
+	return sspi->fifo_depth - 1;
 }
 
 static int sunxi_spi_transfer_one(struct spi_master *master,
@@ -185,10 +262,10 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	u32 reg;
 
 	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > SUNXI_FIFO_DEPTH)
+	if (tfr->len > sspi->fifo_depth)
 		return -EMSGSIZE;
 
-	if (tfr->tx_buf && tfr->len >= SUNXI_FIFO_DEPTH)
+	if (tfr->tx_buf && tfr->len >= sspi->fifo_depth)
 		return -EMSGSIZE;
 
 	reinit_completion(&sspi->done);
@@ -203,35 +280,36 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 
 	/* Reset FIFOs */
 	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
-			reg | SUNXI_CTL_RF_RST | SUNXI_CTL_TF_RST);
+			reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
+			sspi_bits(sspi, SUNXI_CTL_TF_RST));
 
 	/*
 	 * Setup the transfer control register: Chip Select,
 	 * polarities, etc.
 	 */
 	if (spi->mode & SPI_CPOL)
-		reg |= SUNXI_TFR_CTL_CPOL;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CPOL);
 	else
-		reg &= ~SUNXI_TFR_CTL_CPOL;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CPOL);
 
 	if (spi->mode & SPI_CPHA)
-		reg |= SUNXI_TFR_CTL_CPHA;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CPHA);
 	else
-		reg &= ~SUNXI_TFR_CTL_CPHA;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CPHA);
 
 	if (spi->mode & SPI_LSB_FIRST)
-		reg |= SUNXI_TFR_CTL_FBS;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_FBS);
 	else
-		reg &= ~SUNXI_TFR_CTL_FBS;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_FBS);
 
 	/*
 	 * If it's a TX only transfer, we don't want to fill the RX
 	 * FIFO with bogus data
 	 */
 	if (sspi->rx_buf)
-		reg &= ~SUNXI_TFR_CTL_DHB;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_DHB);
 	else
-		reg |= SUNXI_TFR_CTL_DHB;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_DHB);
 
 	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
 
@@ -282,14 +360,16 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	 * Filling the FIFO fully causes timeout for some reason
 	 * at least on spi2 on A10s
 	 */
-	sunxi_spi_fill_fifo(sspi, SUNXI_FIFO_DEPTH - 1);
+	sunxi_spi_fill_fifo(sspi, sspi->fifo_depth - 1);
 
 	/* Enable the interrupts */
-	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, SUNXI_INT_CTL_TC);
+	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG,
+			sspi_bits(sspi, SUNXI_INT_CTL_TC));
 
 	/* Start the transfer */
 	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg | SUNXI_TFR_CTL_XCH);
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
+			reg | sspi_bits(sspi, SUNXI_TFR_CTL_XCH));
 
 	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
 	start = jiffies;
@@ -305,7 +385,7 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
-	sunxi_spi_drain_fifo(sspi, SUNXI_FIFO_DEPTH);
+	sunxi_spi_drain_fifo(sspi, sspi->fifo_depth);
 
 out:
 	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, 0);
@@ -319,8 +399,9 @@ static irqreturn_t sunxi_spi_handler(int irq, void *dev_id)
 	u32 status = sunxi_spi_read(sspi, SUNXI_INT_STA_REG);
 
 	/* Transfer complete */
-	if (status & SUNXI_INT_CTL_TC) {
-		sunxi_spi_write(sspi, SUNXI_INT_STA_REG, SUNXI_INT_CTL_TC);
+	if (status & sspi_bits(sspi, SUNXI_INT_CTL_TC)) {
+		sunxi_spi_write(sspi, SUNXI_INT_STA_REG,
+				sspi_bits(sspi, SUNXI_INT_CTL_TC));
 		complete(&sspi->done);
 		return IRQ_HANDLED;
 	}
@@ -347,7 +428,9 @@ static int sunxi_spi_runtime_resume(struct device *dev)
 	}
 
 	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
-			SUNXI_CTL_ENABLE | SUNXI_CTL_MASTER | SUNXI_CTL_TP);
+			sspi_bits(sspi, SUNXI_CTL_ENABLE) |
+			sspi_bits(sspi, SUNXI_CTL_MASTER) |
+			sspi_bits(sspi, SUNXI_CTL_TP));
 
 	return 0;
 
@@ -408,6 +491,10 @@ static int sunxi_spi_probe(struct platform_device *pdev)
 	}
 
 	sspi->master = master;
+	sspi->fifo_depth = SUN4I_FIFO_DEPTH;
+	sspi->type = SPI_SUN4I;
+	sspi->regmap = &sun4i_regmap;
+	sspi->bitmap = &sun4i_bitmap;
 	master->max_speed_hz = 100 * 1000 * 1000;
 	master->min_speed_hz =          3 * 1000;
 	master->set_cs = sunxi_spi_set_cs;
@@ -473,7 +560,7 @@ static int sunxi_spi_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id sunxi_spi_match[] = {
-	{ .compatible = "allwinner,sun4i-a10-spi", },
+	{ .compatible = SUN4I_COMPATIBLE, },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sunxi_spi_match);
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index d14a953..4215c3e 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -23,58 +23,110 @@
 
 #include <linux/spi/spi.h>
 
-#define SUNXI_FIFO_DEPTH		128
-
-#define SUNXI_GBL_CTL_REG		0x04
-#define SUNXI_CTL_ENABLE		BIT(0)
-#define SUNXI_CTL_MASTER		BIT(1)
-#define SUNXI_CTL_TP			BIT(7)
-
-#define SUNXI_TFR_CTL_REG		0x08
-#define SUNXI_TFR_CTL_CPHA		BIT(0)
-#define SUNXI_TFR_CTL_CPOL		BIT(1)
-#define SUNXI_TFR_CTL_SPOL		BIT(2)
-#define SUNXI_TFR_CTL_CS_MASK		0x30
-#define SUNXI_TFR_CTL_CS(cs)		(((cs) << 4) & SUNXI_TFR_CTL_CS_MASK)
-#define SUNXI_TFR_CTL_CS_MANUAL		BIT(6)
-#define SUNXI_TFR_CTL_CS_LEVEL		BIT(7)
-#define SUNXI_TFR_CTL_DHB		BIT(8)
-#define SUNXI_TFR_CTL_FBS		BIT(12)
-#define SUNXI_TFR_CTL_XCH		BIT(31)
-
-#define SUNXI_INT_CTL_REG		0x10
-#define SUNXI_INT_CTL_TC		BIT(12)
-
-#define SUNXI_INT_STA_REG		0x14
-
-#define SUNXI_FIFO_CTL_REG		0x18
-#define SUNXI_CTL_RF_RST		BIT(15)
-#define SUNXI_CTL_TF_RST		BIT(31)
-
-#define SUNXI_FIFO_STA_REG		0x1c
-#define SUNXI_FIFO_STA_RF_CNT_MASK	0x7f
-#define SUNXI_FIFO_STA_RF_CNT_BITS	0
-#define SUNXI_FIFO_STA_TF_CNT_MASK	0x7f
-#define SUNXI_FIFO_STA_TF_CNT_BITS	16
+#define SUN4I_FIFO_DEPTH		64
+#define SUN6I_FIFO_DEPTH		128
+
+#define SUN4I_COMPATIBLE "allwinner,sun4i-a10-spi"
+#define SUN6I_COMPATIBLE "allwinner,sun6i-a31-spi"
+
+#define SUNXI_TFR_CTL_CS(bitmap, cs)	(((cs) << \
+					  (bitmap)[SUNXI_TFR_CTL_CS_SHIFT]) \
+					 & (bitmap)[SUNXI_TFR_CTL_CS_MASK])
+
+#define SUNXI_CNT_MASK			0xffffff
+#define SUNXI_XMIT_CNT(cnt)		((cnt) & SUNXI_CNT_MASK)
+#define SUNXI_BURST_CNT(cnt)		((cnt) & SUNXI_CNT_MASK)
+#define SUNXI_BURST_CTL_CNT_STC(cnt)	((cnt) & SUNXI_CNT_MASK)
 
-#define SUNXI_CLK_CTL_REG		0x24
+#define SUNXI_CLK_CTL_DRS		BIT(12)
 #define SUNXI_CLK_CTL_CDR2_MASK		0xff
 #define SUNXI_CLK_CTL_CDR2(div)		(((div) & SUNXI_CLK_CTL_CDR2_MASK) << 0)
 #define SUNXI_CLK_CTL_CDR1_MASK		0xf
 #define SUNXI_CLK_CTL_CDR1(div)		(((div) & SUNXI_CLK_CTL_CDR1_MASK) << 8)
-#define SUNXI_CLK_CTL_DRS		BIT(12)
 
-#define SUNXI_BURST_CNT_REG		0x30
-#define SUNXI_BURST_CNT(cnt)		((cnt) & 0xffffff)
+#define SUNXI_FIFO_STA_RF_CNT_MASK	0x7f
+#define SUNXI_FIFO_STA_RF_CNT_BITS	0
+#define SUNXI_FIFO_STA_TF_CNT_MASK	0x7f
+#define SUNXI_FIFO_STA_TF_CNT_BITS	16
+
+enum SPI_SUNXI_TYPE {
+	SPI_SUN4I = 1,
+	SPI_SUN6I,
+};
 
-#define SUNXI_XMIT_CNT_REG		0x34
-#define SUNXI_XMIT_CNT(cnt)		((cnt) & 0xffffff)
+enum SUNXI_REG_ENUM {
+	SUNXI_RXDATA_REG,
+	SUNXI_TXDATA_REG,
+	SUNXI_TFR_CTL_REG,
+	SUNXI_INT_CTL_REG,
+	SUNXI_INT_STA_REG,
+	SUNXI_WAIT_REG,
+	SUNXI_CLK_CTL_REG,
+	SUNXI_BURST_CNT_REG,
+	SUNXI_XMIT_CNT_REG,
+	SUNXI_FIFO_STA_REG,
+	SUNXI_VERSION_REG,
+	SUNXI_GBL_CTL_REG,
+	SUNXI_FIFO_CTL_REG,
+	SUNXI_BURST_CTL_CNT_REG,
+	SUNXI_NUM_REGS
+};
 
-#define SUNXI_BURST_CTL_CNT_REG		0x38
-#define SUNXI_BURST_CTL_CNT_STC(cnt)	((cnt) & 0xffffff)
+static int sun6i_regmap[SUNXI_NUM_REGS] = {
+/* SUNXI_RXDATA_REG */			0x300,
+/* SUNXI_TXDATA_REG */			0x200,
+/* SUNXI_TFR_CTL_REG */			0x08,
+/* SUNXI_INT_CTL_REG */			0x10,
+/* SUNXI_INT_STA_REG */			0x14,
+/* SUNXI_WAIT_REG */			0x20,
+/* SUNXI_CLK_CTL_REG */			0x24,
+/* SUNXI_BURST_CNT_REG */		0x30,
+/* SUNXI_XMIT_CNT_REG */		0x34,
+/* SUNXI_FIFO_STA_REG */		0x1c,
+/* SUNXI_VERSION_REG */			0x00,
+/* SUNXI_GBL_CTL_REG */			0x04,
+/* SUNXI_FIFO_CTL_REG */		0x18,
+/* SUNXI_BURST_CTL_CNT_REG */		0x38,
+};
 
-#define SUNXI_TXDATA_REG		0x200
-#define SUNXI_RXDATA_REG		0x300
+enum SUNXI_BITMAP_ENUM {
+	SUNXI_CTL_ENABLE,
+	SUNXI_CTL_MASTER,
+	SUNXI_TFR_CTL_CPHA,
+	SUNXI_TFR_CTL_CPOL,
+	SUNXI_TFR_CTL_CS_ACTIVE_LOW,
+	SUNXI_TFR_CTL_FBS,
+	SUNXI_CTL_TF_RST,
+	SUNXI_CTL_RF_RST,
+	SUNXI_TFR_CTL_XCH,
+	SUNXI_TFR_CTL_CS_MASK,
+	SUNXI_TFR_CTL_CS_SHIFT,
+	SUNXI_TFR_CTL_DHB,
+	SUNXI_TFR_CTL_CS_MANUAL,
+	SUNXI_TFR_CTL_CS_LEVEL,
+	SUNXI_CTL_TP,
+	SUNXI_INT_CTL_TC,
+	SUNXI_BITMAP_SIZE
+};
+
+static int sun6i_bitmap[SUNXI_BITMAP_SIZE] = {
+/* SUNXI_CTL_ENABLE */			BIT(0),
+/* SUNXI_CTL_MASTER */			BIT(1),
+/* SUNXI_TFR_CTL_CPHA */		BIT(0),
+/* SUNXI_TFR_CTL_CPOL */		BIT(1),
+/* SUNXI_TFR_CTL_CS_ACTIVE_LOW */	BIT(2),
+/* SUNXI_TFR_CTL_FBS */			BIT(12),
+/* SUNXI_CTL_TF_RST */			BIT(31),
+/* SUNXI_CTL_RF_RST */			BIT(15),
+/* SUNXI_TFR_CTL_XCH */			BIT(31),
+/* SUNXI_TFR_CTL_CS_MASK */		0x30,
+/* SUNXI_TFR_CTL_CS_SHIFT */		4,
+/* SUNXI_TFR_CTL_DHB */			BIT(8),
+/* SUNXI_TFR_CTL_CS_MANUAL */		BIT(6),
+/* SUNXI_TFR_CTL_CS_LEVEL */		BIT(7),
+/* SUNXI_CTL_TP */			BIT(7),
+/* SUNXI_INT_CTL_TC */			BIT(12),
+};
 
 struct sunxi_spi {
 	struct spi_master	*master;
@@ -82,6 +134,10 @@ struct sunxi_spi {
 	struct clk		*hclk;
 	struct clk		*mclk;
 	struct reset_control	*rstc;
+	int			(*regmap)[SUNXI_NUM_REGS];
+	int			(*bitmap)[SUNXI_BITMAP_SIZE];
+	int			fifo_depth;
+	int			type;
 
 	struct completion	done;
 
@@ -90,14 +146,31 @@ struct sunxi_spi {
 	int			len;
 };
 
-static inline u32 sunxi_spi_read(struct sunxi_spi *sspi, u32 reg)
+static inline u32 sspi_reg(struct sunxi_spi *sspi, enum SUNXI_REG_ENUM name)
+{
+	BUG_ON((name >= SUNXI_NUM_REGS) || (name < 0) ||
+	       (*sspi->regmap)[name] < 0);
+	return (*sspi->regmap)[name];
+}
+
+static inline u32 sunxi_spi_read(struct sunxi_spi *sspi,
+				 enum SUNXI_REG_ENUM name)
 {
-	return readl(sspi->base_addr + reg);
+	return readl(sspi->base_addr + sspi_reg(sspi, name));
 }
 
-static inline void sunxi_spi_write(struct sunxi_spi *sspi, u32 reg, u32 value)
+static inline void sunxi_spi_write(struct sunxi_spi *sspi,
+				   enum SUNXI_REG_ENUM name, u32 value)
 {
-	writel(value, sspi->base_addr + reg);
+	writel(value, sspi->base_addr + sspi_reg(sspi, name));
+}
+
+static inline u32 sspi_bits(struct sunxi_spi *sspi,
+			    enum SUNXI_BITMAP_ENUM name)
+{
+	BUG_ON((name >= SUNXI_BITMAP_SIZE) || (name < 0) ||
+	       (*sspi->bitmap)[name] <= 0);
+	return (*sspi->bitmap)[name];
 }
 
 static inline void sunxi_spi_drain_fifo(struct sunxi_spi *sspi, int len)
@@ -114,7 +187,8 @@ static inline void sunxi_spi_drain_fifo(struct sunxi_spi *sspi, int len)
 		len = cnt;
 
 	while (len--) {
-		byte = readb(sspi->base_addr + SUNXI_RXDATA_REG);
+		byte = readb(sspi->base_addr +
+			     sspi_reg(sspi, SUNXI_RXDATA_REG));
 		if (sspi->rx_buf)
 			*sspi->rx_buf++ = byte;
 	}
@@ -129,7 +203,8 @@ static inline void sunxi_spi_fill_fifo(struct sunxi_spi *sspi, int len)
 
 	while (len--) {
 		byte = sspi->tx_buf ? *sspi->tx_buf++ : 0;
-		writeb(byte, sspi->base_addr + SUNXI_TXDATA_REG);
+		writeb(byte, sspi->base_addr +
+		       sspi_reg(sspi, SUNXI_TXDATA_REG));
 		sspi->len--;
 	}
 }
@@ -140,16 +215,16 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 	u32 reg;
 
 	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-	reg &= ~SUNXI_TFR_CTL_CS_MASK;
-	reg |= SUNXI_TFR_CTL_CS(spi->chip_select);
+	reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CS_MASK);
+	reg |= SUNXI_TFR_CTL_CS(*sspi->bitmap, spi->chip_select);
 
 	/* We want to control the chip select manually */
-	reg |= SUNXI_TFR_CTL_CS_MANUAL;
+	reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CS_MANUAL);
 
 	if (enable)
-		reg |= SUNXI_TFR_CTL_CS_LEVEL;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CS_LEVEL);
 	else
-		reg &= ~SUNXI_TFR_CTL_CS_LEVEL;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CS_LEVEL);
 
 	/*
 	 * Even though this looks irrelevant since we are supposed to
@@ -163,16 +238,19 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 	 * low.
 	 */
 	if (spi->mode & SPI_CS_HIGH)
-		reg &= ~SUNXI_TFR_CTL_SPOL;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CS_ACTIVE_LOW);
 	else
-		reg |= SUNXI_TFR_CTL_SPOL;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CS_ACTIVE_LOW);
 
 	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
 }
 
 static size_t sunxi_spi_max_transfer_size(struct spi_device *spi)
 {
-	return SUNXI_FIFO_DEPTH - 1;
+	struct spi_master *master = spi->master;
+	struct sunxi_spi *sspi = spi_master_get_devdata(master);
+
+	return sspi->fifo_depth - 1;
 }
 
 static int sunxi_spi_transfer_one(struct spi_master *master,
@@ -187,8 +265,8 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	u32 reg;
 
 	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > SUNXI_FIFO_DEPTH)
-		return -EINVAL;
+	if (tfr->len > sspi->fifo_depth)
+		return -EMSGSIZE;
 
 	reinit_completion(&sspi->done);
 	sspi->tx_buf = tfr->tx_buf;
@@ -202,35 +280,36 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 
 	/* Reset FIFOs */
 	sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
-			SUNXI_CTL_RF_RST | SUNXI_CTL_TF_RST);
+			sspi_bits(sspi, SUNXI_CTL_RF_RST) |
+			sspi_bits(sspi, SUNXI_CTL_TF_RST));
 
 	/*
 	 * Setup the transfer control register: Chip Select,
 	 * polarities, etc.
 	 */
 	if (spi->mode & SPI_CPOL)
-		reg |= SUNXI_TFR_CTL_CPOL;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CPOL);
 	else
-		reg &= ~SUNXI_TFR_CTL_CPOL;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CPOL);
 
 	if (spi->mode & SPI_CPHA)
-		reg |= SUNXI_TFR_CTL_CPHA;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CPHA);
 	else
-		reg &= ~SUNXI_TFR_CTL_CPHA;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CPHA);
 
 	if (spi->mode & SPI_LSB_FIRST)
-		reg |= SUNXI_TFR_CTL_FBS;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_FBS);
 	else
-		reg &= ~SUNXI_TFR_CTL_FBS;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_FBS);
 
 	/*
 	 * If it's a TX only transfer, we don't want to fill the RX
 	 * FIFO with bogus data
 	 */
 	if (sspi->rx_buf)
-		reg &= ~SUNXI_TFR_CTL_DHB;
+		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_DHB);
 	else
-		reg |= SUNXI_TFR_CTL_DHB;
+		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_DHB);
 
 	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
 
@@ -275,18 +354,19 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	/* Setup the counters */
 	sunxi_spi_write(sspi, SUNXI_BURST_CNT_REG, SUNXI_BURST_CNT(tfr->len));
 	sunxi_spi_write(sspi, SUNXI_XMIT_CNT_REG, SUNXI_XMIT_CNT(tx_len));
-	sunxi_spi_write(sspi, SUNXI_BURST_CTL_CNT_REG,
-			SUNXI_BURST_CTL_CNT_STC(tx_len));
+	sunxi_spi_write(sspi, SUNXI_BURST_CTL_CNT_REG, SUNXI_BURST_CTL_CNT_STC(tx_len));
 
 	/* Fill the TX FIFO */
-	sunxi_spi_fill_fifo(sspi, SUNXI_FIFO_DEPTH);
+	sunxi_spi_fill_fifo(sspi, sspi->fifo_depth);
 
 	/* Enable the interrupts */
-	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, SUNXI_INT_CTL_TC);
+	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG,
+			sspi_bits(sspi, SUNXI_INT_CTL_TC));
 
 	/* Start the transfer */
 	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg | SUNXI_TFR_CTL_XCH);
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
+			reg | sspi_bits(sspi, SUNXI_TFR_CTL_XCH));
 
 	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
 	start = jiffies;
@@ -302,7 +382,7 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
-	sunxi_spi_drain_fifo(sspi, SUNXI_FIFO_DEPTH);
+	sunxi_spi_drain_fifo(sspi, sspi->fifo_depth);
 
 out:
 	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, 0);
@@ -316,8 +396,9 @@ static irqreturn_t sunxi_spi_handler(int irq, void *dev_id)
 	u32 status = sunxi_spi_read(sspi, SUNXI_INT_STA_REG);
 
 	/* Transfer complete */
-	if (status & SUNXI_INT_CTL_TC) {
-		sunxi_spi_write(sspi, SUNXI_INT_STA_REG, SUNXI_INT_CTL_TC);
+	if (status & sspi_bits(sspi, SUNXI_INT_CTL_TC)) {
+		sunxi_spi_write(sspi, SUNXI_INT_STA_REG,
+				sspi_bits(sspi, SUNXI_INT_CTL_TC));
 		complete(&sspi->done);
 		return IRQ_HANDLED;
 	}
@@ -350,7 +431,9 @@ static int sunxi_spi_runtime_resume(struct device *dev)
 	}
 
 	sunxi_spi_write(sspi, SUNXI_GBL_CTL_REG,
-			SUNXI_CTL_ENABLE | SUNXI_CTL_MASTER | SUNXI_CTL_TP);
+			sspi_bits(sspi, SUNXI_CTL_ENABLE) |
+			sspi_bits(sspi, SUNXI_CTL_MASTER) |
+			sspi_bits(sspi, SUNXI_CTL_TP));
 
 	return 0;
 
@@ -412,6 +495,10 @@ static int sunxi_spi_probe(struct platform_device *pdev)
 	}
 
 	sspi->master = master;
+	sspi->fifo_depth = SUN6I_FIFO_DEPTH;
+	sspi->type = SPI_SUN6I;
+	sspi->regmap = &sun6i_regmap;
+	sspi->bitmap = &sun6i_bitmap;
 	master->max_speed_hz = 100 * 1000 * 1000;
 	master->min_speed_hz =          3 * 1000;
 	master->set_cs = sunxi_spi_set_cs;
@@ -484,7 +571,7 @@ static int sunxi_spi_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id sunxi_spi_match[] = {
-	{ .compatible = "allwinner,sun6i-a31-spi", },
+	{ .compatible = SUN6I_COMPATIBLE, },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sunxi_spi_match);
-- 
2.8.1

[PATCH v3 07/13] spi: sunxi: rename constants to match between sun4i and sun6i

[Michal Suchanek]

SUNXI_CTL_ -> SUNXI_TFR_CTL_
SUNXI_TFR_CTL_LMTF -> SUNXI_TFR_CTL_FBS
SUNXI_TFR_CTL_CS_ACTIVE_LOW -> SUNXI_TFR_CTL_SPOL
and some SUNXI_???_CTL_ -> SUNXI_CTL_
for constants migrated to different registers between sun4i and sun6i

No functional change.

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-sun4i.c | 68 ++++++++++++++++++++++++-------------------------
 drivers/spi/spi-sun6i.c | 14 +++++-----
 2 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 155d720..b7f8de1 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -28,21 +28,21 @@
 
 #define SUNXI_TXDATA_REG		0x04
 
-#define SUNXI_CTL_REG			0x08
+#define SUNXI_TFR_CTL_REG		0x08
 #define SUNXI_CTL_ENABLE		BIT(0)
 #define SUNXI_CTL_MASTER		BIT(1)
-#define SUNXI_CTL_CPHA			BIT(2)
-#define SUNXI_CTL_CPOL			BIT(3)
-#define SUNXI_CTL_CS_ACTIVE_LOW		BIT(4)
-#define SUNXI_CTL_LMTF			BIT(6)
+#define SUNXI_TFR_CTL_CPHA		BIT(2)
+#define SUNXI_TFR_CTL_CPOL		BIT(3)
+#define SUNXI_TFR_CTL_SPOL		BIT(4)
+#define SUNXI_TFR_CTL_FBS		BIT(6)
 #define SUNXI_CTL_TF_RST		BIT(8)
 #define SUNXI_CTL_RF_RST		BIT(9)
-#define SUNXI_CTL_XCH			BIT(10)
-#define SUNXI_CTL_CS_MASK		0x3000
-#define SUNXI_CTL_CS(cs)		(((cs) << 12) & SUNXI_CTL_CS_MASK)
-#define SUNXI_CTL_DHB			BIT(15)
-#define SUNXI_CTL_CS_MANUAL		BIT(16)
-#define SUNXI_CTL_CS_LEVEL		BIT(17)
+#define SUNXI_TFR_CTL_XCH		BIT(10)
+#define SUNXI_TFR_CTL_CS_MASK		0x3000
+#define SUNXI_TFR_CTL_CS(cs)		(((cs) << 12) & SUNXI_TFR_CTL_CS_MASK)
+#define SUNXI_TFR_CTL_DHB		BIT(15)
+#define SUNXI_TFR_CTL_CS_MANUAL		BIT(16)
+#define SUNXI_TFR_CTL_CS_LEVEL		BIT(17)
 #define SUNXI_CTL_TP			BIT(18)
 
 #define SUNXI_INT_CTL_REG		0x0c
@@ -135,18 +135,18 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 	struct sunxi_spi *sspi = spi_master_get_devdata(spi->master);
 	u32 reg;
 
-	reg = sunxi_spi_read(sspi, SUNXI_CTL_REG);
+	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
 
-	reg &= ~SUNXI_CTL_CS_MASK;
-	reg |= SUNXI_CTL_CS(spi->chip_select);
+	reg &= ~SUNXI_TFR_CTL_CS_MASK;
+	reg |= SUNXI_TFR_CTL_CS(spi->chip_select);
 
 	/* We want to control the chip select manually */
-	reg |= SUNXI_CTL_CS_MANUAL;
+	reg |= SUNXI_TFR_CTL_CS_MANUAL;
 
 	if (enable)
-		reg |= SUNXI_CTL_CS_LEVEL;
+		reg |= SUNXI_TFR_CTL_CS_LEVEL;
 	else
-		reg &= ~SUNXI_CTL_CS_LEVEL;
+		reg &= ~SUNXI_TFR_CTL_CS_LEVEL;
 
 	/*
 	 * Even though this looks irrelevant since we are supposed to
@@ -160,11 +160,11 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 	 * low.
 	 */
 	if (spi->mode & SPI_CS_HIGH)
-		reg &= ~SUNXI_CTL_CS_ACTIVE_LOW;
+		reg &= ~SUNXI_TFR_CTL_SPOL;
 	else
-		reg |= SUNXI_CTL_CS_ACTIVE_LOW;
+		reg |= SUNXI_TFR_CTL_SPOL;
 
-	sunxi_spi_write(sspi, SUNXI_CTL_REG, reg);
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
 }
 
 static size_t sunxi_spi_max_transfer_size(struct spi_device *spi)
@@ -199,10 +199,10 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	sunxi_spi_write(sspi, SUNXI_INT_STA_REG, ~0);
 
 
-	reg = sunxi_spi_read(sspi, SUNXI_CTL_REG);
+	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
 
 	/* Reset FIFOs */
-	sunxi_spi_write(sspi, SUNXI_CTL_REG,
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
 			reg | SUNXI_CTL_RF_RST | SUNXI_CTL_TF_RST);
 
 	/*
@@ -210,19 +210,19 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	 * polarities, etc.
 	 */
 	if (spi->mode & SPI_CPOL)
-		reg |= SUNXI_CTL_CPOL;
+		reg |= SUNXI_TFR_CTL_CPOL;
 	else
-		reg &= ~SUNXI_CTL_CPOL;
+		reg &= ~SUNXI_TFR_CTL_CPOL;
 
 	if (spi->mode & SPI_CPHA)
-		reg |= SUNXI_CTL_CPHA;
+		reg |= SUNXI_TFR_CTL_CPHA;
 	else
-		reg &= ~SUNXI_CTL_CPHA;
+		reg &= ~SUNXI_TFR_CTL_CPHA;
 
 	if (spi->mode & SPI_LSB_FIRST)
-		reg |= SUNXI_CTL_LMTF;
+		reg |= SUNXI_TFR_CTL_FBS;
 	else
-		reg &= ~SUNXI_CTL_LMTF;
+		reg &= ~SUNXI_TFR_CTL_FBS;
 
 
 	/*
@@ -230,11 +230,11 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	 * FIFO with bogus data
 	 */
 	if (sspi->rx_buf)
-		reg &= ~SUNXI_CTL_DHB;
+		reg &= ~SUNXI_TFR_CTL_DHB;
 	else
-		reg |= SUNXI_CTL_DHB;
+		reg |= SUNXI_TFR_CTL_DHB;
 
-	sunxi_spi_write(sspi, SUNXI_CTL_REG, reg);
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
 
 	/* Ensure that we have a parent clock fast enough */
 	mclk_rate = clk_get_rate(sspi->mclk);
@@ -289,8 +289,8 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, SUNXI_INT_CTL_TC);
 
 	/* Start the transfer */
-	reg = sunxi_spi_read(sspi, SUNXI_CTL_REG);
-	sunxi_spi_write(sspi, SUNXI_CTL_REG, reg | SUNXI_CTL_XCH);
+	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg | SUNXI_TFR_CTL_XCH);
 
 	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
 	start = jiffies;
@@ -347,7 +347,7 @@ static int sunxi_spi_runtime_resume(struct device *dev)
 		goto err;
 	}
 
-	sunxi_spi_write(sspi, SUNXI_CTL_REG,
+	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
 			SUNXI_CTL_ENABLE | SUNXI_CTL_MASTER | SUNXI_CTL_TP);
 
 	return 0;
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index a27bf8f..f26b52a 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -26,9 +26,9 @@
 #define SUNXI_FIFO_DEPTH		128
 
 #define SUNXI_GBL_CTL_REG		0x04
-#define SUNXI_GBL_CTL_BUS_ENABLE	BIT(0)
-#define SUNXI_GBL_CTL_MASTER		BIT(1)
-#define SUNXI_GBL_CTL_TP		BIT(7)
+#define SUNXI_CTL_ENABLE		BIT(0)
+#define SUNXI_CTL_MASTER		BIT(1)
+#define SUNXI_CTL_TP			BIT(7)
 #define SUNXI_GBL_CTL_RST		BIT(31)
 
 #define SUNXI_TFR_CTL_REG		0x08
@@ -50,8 +50,8 @@
 #define SUNXI_INT_STA_REG		0x14
 
 #define SUNXI_FIFO_CTL_REG		0x18
-#define SUNXI_FIFO_CTL_RF_RST		BIT(15)
-#define SUNXI_FIFO_CTL_TF_RST		BIT(31)
+#define SUNXI_CTL_RF_RST		BIT(15)
+#define SUNXI_CTL_TF_RST		BIT(31)
 
 #define SUNXI_FIFO_STA_REG		0x1c
 #define SUNXI_FIFO_STA_RF_CNT_MASK	0x7f
@@ -202,7 +202,7 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 
 	/* Reset FIFO */
 	sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
-			SUNXI_FIFO_CTL_RF_RST | SUNXI_FIFO_CTL_TF_RST);
+			SUNXI_CTL_RF_RST | SUNXI_CTL_TF_RST);
 
 	/*
 	 * Setup the transfer control register: Chip Select,
@@ -352,7 +352,7 @@ static int sunxi_spi_runtime_resume(struct device *dev)
 	}
 
 	sunxi_spi_write(sspi, SUNXI_GBL_CTL_REG,
-			SUNXI_GBL_CTL_BUS_ENABLE | SUNXI_GBL_CTL_MASTER | SUNXI_GBL_CTL_TP);
+			SUNXI_CTL_ENABLE | SUNXI_CTL_MASTER | SUNXI_CTL_TP);
 
 	return 0;
 
-- 
2.8.1

Re: [PATCH v3 07/13] spi: sunxi: rename constants to match between sun4i and sun6i

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> SUNXI_CTL_ -> SUNXI_TFR_CTL_
> SUNXI_TFR_CTL_LMTF -> SUNXI_TFR_CTL_FBS

I don't know these abbreviations, are they both referring to the same thing?

> SUNXI_TFR_CTL_CS_ACTIVE_LOW -> SUNXI_TFR_CTL_SPOL

It looks like you're making the constant name less descriptive here.
Is the old version (CS_ACTIVE_LOW) incorrect?

> and some SUNXI_???_CTL_ -> SUNXI_CTL_
> for constants migrated to different registers between sun4i and sun6i
>
> No functional change.
>
> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/spi/spi-sun4i.c | 68 ++++++++++++++++++++++++-------------------------
>  drivers/spi/spi-sun6i.c | 14 +++++-----
>  2 files changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 155d720..b7f8de1 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -28,21 +28,21 @@
>
>  #define SUNXI_TXDATA_REG               0x04
>
> -#define SUNXI_CTL_REG                  0x08
> +#define SUNXI_TFR_CTL_REG              0x08
>  #define SUNXI_CTL_ENABLE               BIT(0)
>  #define SUNXI_CTL_MASTER               BIT(1)
> -#define SUNXI_CTL_CPHA                 BIT(2)
> -#define SUNXI_CTL_CPOL                 BIT(3)
> -#define SUNXI_CTL_CS_ACTIVE_LOW                BIT(4)
> -#define SUNXI_CTL_LMTF                 BIT(6)
> +#define SUNXI_TFR_CTL_CPHA             BIT(2)
> +#define SUNXI_TFR_CTL_CPOL             BIT(3)
> +#define SUNXI_TFR_CTL_SPOL             BIT(4)
> +#define SUNXI_TFR_CTL_FBS              BIT(6)
>  #define SUNXI_CTL_TF_RST               BIT(8)
>  #define SUNXI_CTL_RF_RST               BIT(9)
> -#define SUNXI_CTL_XCH                  BIT(10)
> -#define SUNXI_CTL_CS_MASK              0x3000
> -#define SUNXI_CTL_CS(cs)               (((cs) << 12) & SUNXI_CTL_CS_MASK)
> -#define SUNXI_CTL_DHB                  BIT(15)
> -#define SUNXI_CTL_CS_MANUAL            BIT(16)
> -#define SUNXI_CTL_CS_LEVEL             BIT(17)
> +#define SUNXI_TFR_CTL_XCH              BIT(10)
> +#define SUNXI_TFR_CTL_CS_MASK          0x3000
> +#define SUNXI_TFR_CTL_CS(cs)           (((cs) << 12) & SUNXI_TFR_CTL_CS_MASK)
> +#define SUNXI_TFR_CTL_DHB              BIT(15)
> +#define SUNXI_TFR_CTL_CS_MANUAL                BIT(16)
> +#define SUNXI_TFR_CTL_CS_LEVEL         BIT(17)
>  #define SUNXI_CTL_TP                   BIT(18)
>
>  #define SUNXI_INT_CTL_REG              0x0c
> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
> index a27bf8f..f26b52a 100644
> --- a/drivers/spi/spi-sun6i.c
> +++ b/drivers/spi/spi-sun6i.c
> @@ -26,9 +26,9 @@
>  #define SUNXI_FIFO_DEPTH               128
>
>  #define SUNXI_GBL_CTL_REG              0x04
> -#define SUNXI_GBL_CTL_BUS_ENABLE       BIT(0)
> -#define SUNXI_GBL_CTL_MASTER           BIT(1)
> -#define SUNXI_GBL_CTL_TP               BIT(7)
> +#define SUNXI_CTL_ENABLE               BIT(0)
> +#define SUNXI_CTL_MASTER               BIT(1)
> +#define SUNXI_CTL_TP                   BIT(7)

If these are bit definitions for the GBL register, why throw that
information away?

>  #define SUNXI_GBL_CTL_RST              BIT(31)
>
>  #define SUNXI_TFR_CTL_REG              0x08
> @@ -50,8 +50,8 @@
>  #define SUNXI_INT_STA_REG              0x14
>
>  #define SUNXI_FIFO_CTL_REG             0x18
> -#define SUNXI_FIFO_CTL_RF_RST          BIT(15)
> -#define SUNXI_FIFO_CTL_TF_RST          BIT(31)
> +#define SUNXI_CTL_RF_RST               BIT(15)
> +#define SUNXI_CTL_TF_RST               BIT(31)

Same here with FIFO.

>
>  #define SUNXI_FIFO_STA_REG             0x1c
>  #define SUNXI_FIFO_STA_RF_CNT_MASK     0x7f

My gut feeling on this is that we have a lot of cases of a definition
of a register offset, then definitions of the bits in that register
with that register encoded into the constant's name. You appear to be
throwing a lot of that information away which makes me worry.

Thanks,

-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH v3 07/13] spi: sunxi: rename constants to match between sun4i and sun6i

[Michal Suchanek]

Hello,

On 14 June 2016 at 01:31, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> SUNXI_CTL_ -> SUNXI_TFR_CTL_
>> SUNXI_TFR_CTL_LMTF -> SUNXI_TFR_CTL_FBS
>
> I don't know these abbreviations, are they both referring to the same thing?
>
>> SUNXI_TFR_CTL_CS_ACTIVE_LOW -> SUNXI_TFR_CTL_SPOL
>
> It looks like you're making the constant name less descriptive here.
> Is the old version (CS_ACTIVE_LOW) incorrect?
>
>> and some SUNXI_???_CTL_ -> SUNXI_CTL_
>> for constants migrated to different registers between sun4i and sun6i
>>
>> No functional change.
>>
>>  #define SUNXI_INT_CTL_REG              0x0c
>> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
>> index a27bf8f..f26b52a 100644
>> --- a/drivers/spi/spi-sun6i.c
>> +++ b/drivers/spi/spi-sun6i.c
>> @@ -26,9 +26,9 @@
>>  #define SUNXI_FIFO_DEPTH               128
>>
>>  #define SUNXI_GBL_CTL_REG              0x04
>> -#define SUNXI_GBL_CTL_BUS_ENABLE       BIT(0)
>> -#define SUNXI_GBL_CTL_MASTER           BIT(1)
>> -#define SUNXI_GBL_CTL_TP               BIT(7)
>> +#define SUNXI_CTL_ENABLE               BIT(0)
>> +#define SUNXI_CTL_MASTER               BIT(1)
>> +#define SUNXI_CTL_TP                   BIT(7)
>
> If these are bit definitions for the GBL register, why throw that
> information away?

Those bits are on the TFR register in the earlier IP so it makes
perfect sense to me this way.

Thanks

Michal

[PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc

[Michal Suchanek]

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/spi/spi-sun4i.txt          | 21 ++++++++++---------
 .../devicetree/bindings/spi/spi-sun6i.txt          | 24 ----------------------
 2 files changed, 11 insertions(+), 34 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-sun4i.txt b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
index de827f5..329e543 100644
--- a/Documentation/devicetree/bindings/spi/spi-sun4i.txt
+++ b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
@@ -1,7 +1,8 @@
-Allwinner A10 SPI controller
+Allwinner A10/A31 SPI controller
 
 Required properties:
-- compatible: Should be "allwinner,sun4-a10-spi".
+- compatible: Should be one of	"allwinner,sun4i-a10-spi" and
+				"allwinner,sun6i-a31-spi"
 - reg: Should contain register location and length.
 - interrupts: Should contain interrupt.
 - clocks: phandle to the clocks feeding the SPI controller. Two are
@@ -9,16 +10,16 @@ Required properties:
   - "ahb": the gated AHB parent clock
   - "mod": the parent module clock
 - clock-names: Must contain the clock names described just above
+- resets: (sun6i only) phandle to the reset controller asserting
+	  this device in reset
 
 Example:
 
-spi1: spi@01c06000 {
-	compatible = "allwinner,sun4i-a10-spi";
-	reg = <0x01c06000 0x1000>;
-	interrupts = <11>;
-	clocks = <&ahb_gates 21>, <&spi1_clk>;
+spi1: spi@01c69000 {
+	compatible = "allwinner,sun6i-a31-spi";
+	reg = <0x01c69000 0x1000>;
+	interrupts = <0 66 4>;
+	clocks = <&ahb1_gates 21>, <&spi1_clk>;
 	clock-names = "ahb", "mod";
-	status = "disabled";
-	#address-cells = <1>;
-	#size-cells = <0>;
+	resets = <&ahb1_rst 21>;
 };
diff --git a/Documentation/devicetree/bindings/spi/spi-sun6i.txt b/Documentation/devicetree/bindings/spi/spi-sun6i.txt
deleted file mode 100644
index 21de73d..0000000
--- a/Documentation/devicetree/bindings/spi/spi-sun6i.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-Allwinner A31 SPI controller
-
-Required properties:
-- compatible: Should be "allwinner,sun6i-a31-spi".
-- reg: Should contain register location and length.
-- interrupts: Should contain interrupt.
-- clocks: phandle to the clocks feeding the SPI controller. Two are
-          needed:
-  - "ahb": the gated AHB parent clock
-  - "mod": the parent module clock
-- clock-names: Must contain the clock names described just above
-- resets: phandle to the reset controller asserting this device in
-          reset
-
-Example:
-
-spi1: spi@01c69000 {
-	compatible = "allwinner,sun6i-a31-spi";
-	reg = <0x01c69000 0x1000>;
-	interrupts = <0 66 4>;
-	clocks = <&ahb1_gates 21>, <&spi1_clk>;
-	clock-names = "ahb", "mod";
-	resets = <&ahb1_rst 21>;
-};
-- 
2.8.1

Re: [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/spi/spi-sun4i.txt          | 21 ++++++++++---------
>  .../devicetree/bindings/spi/spi-sun6i.txt          | 24 ----------------------
>  2 files changed, 11 insertions(+), 34 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-sun4i.txt b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
> index de827f5..329e543 100644
> --- a/Documentation/devicetree/bindings/spi/spi-sun4i.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
> @@ -1,7 +1,8 @@
> -Allwinner A10 SPI controller
> +Allwinner A10/A31 SPI controller
>
>  Required properties:
> -- compatible: Should be "allwinner,sun4-a10-spi".
> +- compatible: Should be one of "allwinner,sun4i-a10-spi" and
> +                               "allwinner,sun6i-a31-spi"
>  - reg: Should contain register location and length.
>  - interrupts: Should contain interrupt.
>  - clocks: phandle to the clocks feeding the SPI controller. Two are
> @@ -9,16 +10,16 @@ Required properties:
>    - "ahb": the gated AHB parent clock
>    - "mod": the parent module clock
>  - clock-names: Must contain the clock names described just above
> +- resets: (sun6i only) phandle to the reset controller asserting
> +         this device in reset
>
>  Example:
>
> -spi1: spi@01c06000 {
> -       compatible = "allwinner,sun4i-a10-spi";
> -       reg = <0x01c06000 0x1000>;
> -       interrupts = <11>;
> -       clocks = <&ahb_gates 21>, <&spi1_clk>;
> +spi1: spi@01c69000 {
> +       compatible = "allwinner,sun6i-a31-spi";
> +       reg = <0x01c69000 0x1000>;
> +       interrupts = <0 66 4>;
> +       clocks = <&ahb1_gates 21>, <&spi1_clk>;
>         clock-names = "ahb", "mod";
> -       status = "disabled";
> -       #address-cells = <1>;
> -       #size-cells = <0>;
> +       resets = <&ahb1_rst 21>;

Why not have an example of each type?

>  };

Thanks,

-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc

[Michal Suchanek]

On 14 June 2016 at 01:45, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  .../devicetree/bindings/spi/spi-sun4i.txt          | 21 ++++++++++---------
>>  .../devicetree/bindings/spi/spi-sun6i.txt          | 24 ----------------------
>>  2 files changed, 11 insertions(+), 34 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-sun4i.txt b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>> index de827f5..329e543 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>> +++ b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>> @@ -1,7 +1,8 @@
>> -Allwinner A10 SPI controller
>> +Allwinner A10/A31 SPI controller
>>
>>  Required properties:
>> -- compatible: Should be "allwinner,sun4-a10-spi".
>> +- compatible: Should be one of "allwinner,sun4i-a10-spi" and
>> +                               "allwinner,sun6i-a31-spi"
>>  - reg: Should contain register location and length.
>>  - interrupts: Should contain interrupt.
>>  - clocks: phandle to the clocks feeding the SPI controller. Two are
>> @@ -9,16 +10,16 @@ Required properties:
>>    - "ahb": the gated AHB parent clock
>>    - "mod": the parent module clock
>>  - clock-names: Must contain the clock names described just above
>> +- resets: (sun6i only) phandle to the reset controller asserting
>> +         this device in reset
>>
>>  Example:
>>
>> -spi1: spi@01c06000 {
>> -       compatible = "allwinner,sun4i-a10-spi";
>> -       reg = <0x01c06000 0x1000>;
>> -       interrupts = <11>;
>> -       clocks = <&ahb_gates 21>, <&spi1_clk>;
>> +spi1: spi@01c69000 {
>> +       compatible = "allwinner,sun6i-a31-spi";
>> +       reg = <0x01c69000 0x1000>;
>> +       interrupts = <0 66 4>;
>> +       clocks = <&ahb1_gates 21>, <&spi1_clk>;
>>         clock-names = "ahb", "mod";
>> -       status = "disabled";
>> -       #address-cells = <1>;
>> -       #size-cells = <0>;
>> +       resets = <&ahb1_rst 21>;
>
> Why not have an example of each type?

How many binding docs have examples of all types?

There are actual DTs using these so you can look at those as well.

This driver covers 3 types of bindings which look different in the DT:

sun4i IP with some Chinese interrupt controller, sun4i IP with GIC,
and sun6i IP with GIC.

Thanks

Michal

Re: [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 2:40 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 14 June 2016 at 01:45, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/spi/spi-sun4i.txt          | 21 ++++++++++---------
>>>  .../devicetree/bindings/spi/spi-sun6i.txt          | 24 ----------------------
>>>  2 files changed, 11 insertions(+), 34 deletions(-)
>>>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-sun4i.txt b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>>> index de827f5..329e543 100644
>>> --- a/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>>> +++ b/Documentation/devicetree/bindings/spi/spi-sun4i.txt
>>> @@ -1,7 +1,8 @@
>>> -Allwinner A10 SPI controller
>>> +Allwinner A10/A31 SPI controller
>>>
>>>  Required properties:
>>> -- compatible: Should be "allwinner,sun4-a10-spi".
>>> +- compatible: Should be one of "allwinner,sun4i-a10-spi" and
>>> +                               "allwinner,sun6i-a31-spi"
>>>  - reg: Should contain register location and length.
>>>  - interrupts: Should contain interrupt.
>>>  - clocks: phandle to the clocks feeding the SPI controller. Two are
>>> @@ -9,16 +10,16 @@ Required properties:
>>>    - "ahb": the gated AHB parent clock
>>>    - "mod": the parent module clock
>>>  - clock-names: Must contain the clock names described just above
>>> +- resets: (sun6i only) phandle to the reset controller asserting
>>> +         this device in reset
>>>
>>>  Example:
>>>
>>> -spi1: spi@01c06000 {
>>> -       compatible = "allwinner,sun4i-a10-spi";
>>> -       reg = <0x01c06000 0x1000>;
>>> -       interrupts = <11>;
>>> -       clocks = <&ahb_gates 21>, <&spi1_clk>;
>>> +spi1: spi@01c69000 {
>>> +       compatible = "allwinner,sun6i-a31-spi";
>>> +       reg = <0x01c69000 0x1000>;
>>> +       interrupts = <0 66 4>;
>>> +       clocks = <&ahb1_gates 21>, <&spi1_clk>;
>>>         clock-names = "ahb", "mod";
>>> -       status = "disabled";
>>> -       #address-cells = <1>;
>>> -       #size-cells = <0>;
>>> +       resets = <&ahb1_rst 21>;
>>
>> Why not have an example of each type?
>
> How many binding docs have examples of all types?

I'm pretty sure that there's a few. This was only a suggestion, so if
it's not to your taste, ignore it.

> There are actual DTs using these so you can look at those as well.
>
> This driver covers 3 types of bindings which look different in the DT:
>
> sun4i IP with some Chinese interrupt controller, sun4i IP with GIC,
> and sun6i IP with GIC.

Fair point.

Thanks,

-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/

[PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Michal Suchanek]

The drivers are very similar and share multiple flaws which needed
separate fixes for both drivers.

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/Kconfig     |   8 +-
 drivers/spi/Makefile    |   1 -
 drivers/spi/spi-sun4i.c | 156 +++++++++++--
 drivers/spi/spi-sun6i.c | 598 ------------------------------------------------
 4 files changed, 143 insertions(+), 620 deletions(-)
 delete mode 100644 drivers/spi/spi-sun6i.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 9d8c84b..1b5045b 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -575,17 +575,17 @@ config SPI_ST_SSC4
 	  this option, support will be included for the SSC driven SPI.
 
 config SPI_SUN4I
-	tristate "Allwinner A10 SoCs SPI controller"
+	tristate "Allwinner A1X/A2X/A31 SoCs SPI controller"
 	depends on ARCH_SUNXI || COMPILE_TEST
+	depends on RESET_CONTROLLER
 	help
 	  SPI driver for Allwinner sun4i, sun5i and sun7i SoCs
 
 config SPI_SUN6I
-	tristate "Allwinner A31 SPI controller"
+	tristate "Allwinner A31 SPI controller (deprecated compatibility option)"
 	depends on ARCH_SUNXI || COMPILE_TEST
 	depends on RESET_CONTROLLER
-	help
-	  This enables using the SPI controller on the Allwinner A31 SoCs.
+	select SPI_SUN4I
 
 config SPI_MXS
 	tristate "Freescale MXS SPI controller"
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index fbb255c..b0387e8c 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -83,7 +83,6 @@ obj-$(CONFIG_SPI_SH_SCI)		+= spi-sh-sci.o
 obj-$(CONFIG_SPI_SIRF)		+= spi-sirf.o
 obj-$(CONFIG_SPI_ST_SSC4)		+= spi-st-ssc4.o
 obj-$(CONFIG_SPI_SUN4I)			+= spi-sun4i.o
-obj-$(CONFIG_SPI_SUN6I)			+= spi-sun6i.o
 obj-$(CONFIG_SPI_TEGRA114)		+= spi-tegra114.o
 obj-$(CONFIG_SPI_TEGRA20_SFLASH)	+= spi-tegra20-sflash.o
 obj-$(CONFIG_SPI_TEGRA20_SLINK)		+= spi-tegra20-slink.o
diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 0b8e6c6..c76f8e4 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -20,6 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
+#include <linux/of.h>
 
 #include <linux/spi/spi.h>
 
@@ -86,6 +87,23 @@ static int sun4i_regmap[SUNXI_NUM_REGS] = {
 -1, -1, -1, -1
 };
 
+static int sun6i_regmap[SUNXI_NUM_REGS] = {
+/* SUNXI_RXDATA_REG */			0x300,
+/* SUNXI_TXDATA_REG */			0x200,
+/* SUNXI_TFR_CTL_REG */			0x08,
+/* SUNXI_INT_CTL_REG */			0x10,
+/* SUNXI_INT_STA_REG */			0x14,
+/* SUNXI_WAIT_REG */			0x20,
+/* SUNXI_CLK_CTL_REG */			0x24,
+/* SUNXI_BURST_CNT_REG */		0x30,
+/* SUNXI_XMIT_CNT_REG */		0x34,
+/* SUNXI_FIFO_STA_REG */		0x1c,
+/* SUNXI_VERSION_REG */			0x00,
+/* SUNXI_GBL_CTL_REG */			0x04,
+/* SUNXI_FIFO_CTL_REG */		0x18,
+/* SUNXI_BURST_CTL_CNT_REG */		0x38,
+};
+
 enum SUNXI_BITMAP_ENUM {
 	SUNXI_CTL_ENABLE,
 	SUNXI_CTL_MASTER,
@@ -125,6 +143,25 @@ static int sun4i_bitmap[SUNXI_BITMAP_SIZE] = {
 /* SUNXI_INT_CTL_TC */			BIT(16),
 };
 
+static int sun6i_bitmap[SUNXI_BITMAP_SIZE] = {
+/* SUNXI_CTL_ENABLE */			BIT(0),
+/* SUNXI_CTL_MASTER */			BIT(1),
+/* SUNXI_TFR_CTL_CPHA */		BIT(0),
+/* SUNXI_TFR_CTL_CPOL */		BIT(1),
+/* SUNXI_TFR_CTL_CS_ACTIVE_LOW */	BIT(2),
+/* SUNXI_TFR_CTL_FBS */			BIT(12),
+/* SUNXI_CTL_TF_RST */			BIT(31),
+/* SUNXI_CTL_RF_RST */			BIT(15),
+/* SUNXI_TFR_CTL_XCH */			BIT(31),
+/* SUNXI_TFR_CTL_CS_MASK */		0x30,
+/* SUNXI_TFR_CTL_CS_SHIFT */		4,
+/* SUNXI_TFR_CTL_DHB */			BIT(8),
+/* SUNXI_TFR_CTL_CS_MANUAL */		BIT(6),
+/* SUNXI_TFR_CTL_CS_LEVEL */		BIT(7),
+/* SUNXI_CTL_TP */			BIT(7),
+/* SUNXI_INT_CTL_TC */			BIT(12),
+};
+
 struct sunxi_spi {
 	struct spi_master	*master;
 	void __iomem		*base_addr;
@@ -265,7 +302,12 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	if (tfr->len > sspi->fifo_depth)
 		return -EMSGSIZE;
 
-	if (tfr->tx_buf && tfr->len >= sspi->fifo_depth)
+	/*
+	 * Filling the FIFO fully causes timeout for some reason
+	 * at least on spi2 on A10s
+	 */
+	if ((sspi->type == SPI_SUN4I) &&
+	    tfr->tx_buf && tfr->len >= sspi->fifo_depth)
 		return -EMSGSIZE;
 
 	reinit_completion(&sspi->done);
@@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
 
 	/* Reset FIFOs */
-	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
-			reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
-			sspi_bits(sspi, SUNXI_CTL_TF_RST));
+	if (sspi->type == SPI_SUN4I)
+		sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
+				reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
+				sspi_bits(sspi, SUNXI_CTL_TF_RST));
+	else
+		sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
+				sspi_bits(sspi, SUNXI_CTL_RF_RST) |
+				sspi_bits(sspi, SUNXI_CTL_TF_RST));
 
 	/*
 	 * Setup the transfer control register: Chip Select,
@@ -354,13 +401,12 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 	/* Setup the counters */
 	sunxi_spi_write(sspi, SUNXI_BURST_CNT_REG, SUNXI_BURST_CNT(tfr->len));
 	sunxi_spi_write(sspi, SUNXI_XMIT_CNT_REG, SUNXI_XMIT_CNT(tx_len));
+	if (sspi->type == SPI_SUN6I)
+		sunxi_spi_write(sspi, SUNXI_BURST_CTL_CNT_REG,
+				SUNXI_BURST_CTL_CNT_STC(tx_len));
 
-	/*
-	 * Fill the TX FIFO
-	 * Filling the FIFO fully causes timeout for some reason
-	 * at least on spi2 on A10s
-	 */
-	sunxi_spi_fill_fifo(sspi, sspi->fifo_depth - 1);
+	/* Fill the TX FIFO */
+	sunxi_spi_fill_fifo(sspi, sspi->fifo_depth);
 
 	/* Enable the interrupts */
 	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG,
@@ -413,7 +459,7 @@ static int sunxi_spi_runtime_resume(struct device *dev)
 {
 	struct spi_master *master = dev_get_drvdata(dev);
 	struct sunxi_spi *sspi = spi_master_get_devdata(master);
-	int ret;
+	int ret, reg;
 
 	ret = clk_prepare_enable(sspi->hclk);
 	if (ret) {
@@ -427,7 +473,19 @@ static int sunxi_spi_runtime_resume(struct device *dev)
 		goto err;
 	}
 
-	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
+	if (sspi->rstc) {
+		ret = reset_control_deassert(sspi->rstc);
+		if (ret) {
+			dev_err(dev, "Couldn't deassert the device from reset\n");
+			goto err2;
+		}
+	}
+
+	if (sspi->type == SPI_SUN4I)
+		reg = SUNXI_TFR_CTL_REG;
+	else
+		reg = SUNXI_GBL_CTL_REG;
+	sunxi_spi_write(sspi, reg,
 			sspi_bits(sspi, SUNXI_CTL_ENABLE) |
 			sspi_bits(sspi, SUNXI_CTL_MASTER) |
 			sspi_bits(sspi, SUNXI_CTL_TP));
@@ -447,6 +505,8 @@ static int sunxi_spi_runtime_suspend(struct device *dev)
 	struct spi_master *master = dev_get_drvdata(dev);
 	struct sunxi_spi *sspi = spi_master_get_devdata(master);
 
+	if (sspi->rstc)
+		reset_control_assert(sspi->rstc);
 	clk_disable_unprepare(sspi->mclk);
 	clk_disable_unprepare(sspi->hclk);
 
@@ -459,6 +519,13 @@ static int sunxi_spi_probe(struct platform_device *pdev)
 	struct sunxi_spi *sspi;
 	struct resource	*res;
 	int ret = 0, irq;
+	const char *desc = NULL;
+	u32 version = 0;
+
+	if (!pdev->dev.of_node) {
+		dev_err(&pdev->dev, "No devicetree data.");
+		return -EINVAL;
+	}
 
 	master = spi_alloc_master(&pdev->dev, sizeof(struct sunxi_spi));
 	if (!master) {
@@ -491,10 +558,37 @@ static int sunxi_spi_probe(struct platform_device *pdev)
 	}
 
 	sspi->master = master;
-	sspi->fifo_depth = SUN4I_FIFO_DEPTH;
-	sspi->type = SPI_SUN4I;
-	sspi->regmap = &sun4i_regmap;
-	sspi->bitmap = &sun4i_bitmap;
+	if (of_device_is_compatible(pdev->dev.of_node, SUN4I_COMPATIBLE)) {
+		sspi->fifo_depth = SUN4I_FIFO_DEPTH;
+		sspi->type = SPI_SUN4I;
+		sspi->regmap = &sun4i_regmap;
+		sspi->bitmap = &sun4i_bitmap;
+	} else if (of_device_is_compatible(pdev->dev.of_node,
+					   SUN6I_COMPATIBLE)) {
+		sspi->fifo_depth = SUN6I_FIFO_DEPTH;
+		sspi->type = SPI_SUN6I;
+		sspi->regmap = &sun6i_regmap;
+		sspi->bitmap = &sun6i_bitmap;
+	} else {
+		const char *str = NULL;
+		int i = 1;
+
+		of_property_read_string(pdev->dev.of_node, "compatible", &str);
+		dev_err(&pdev->dev, "Unknown device compatible %s", str);
+		/* is there no sane way to print a string array property ? */
+		if (of_property_count_strings(pdev->dev.of_node, "compatible")
+		    > 1) {
+			while (!of_property_read_string_index(pdev->dev.of_node,
+							      "compatible", i,
+							      &str)) {
+				pr_err(", %s", str);
+				i++;
+			}
+		}
+		ret = -EINVAL;
+		goto err_free_master;
+	}
+
 	master->max_speed_hz = 100 * 1000 * 1000;
 	master->min_speed_hz =          3 * 1000;
 	master->set_cs = sunxi_spi_set_cs;
@@ -522,6 +616,15 @@ static int sunxi_spi_probe(struct platform_device *pdev)
 
 	init_completion(&sspi->done);
 
+	if (sspi->type == SPI_SUN6I) {
+		sspi->rstc = devm_reset_control_get(&pdev->dev, NULL);
+		if (IS_ERR(sspi->rstc)) {
+			dev_err(&pdev->dev, "Couldn't get reset controller\n");
+			ret = PTR_ERR(sspi->rstc);
+			goto err_free_master;
+		}
+	}
+
 	/*
 	 * This wake-up/shutdown pattern is to be able to have the
 	 * device woken up, even if runtime_pm is disabled
@@ -542,6 +645,24 @@ static int sunxi_spi_probe(struct platform_device *pdev)
 		goto err_pm_disable;
 	}
 
+	switch (sspi->type) {
+	case SPI_SUN4I:
+		desc = "sun4i";
+		break;
+	case SPI_SUN6I:
+		desc = "sun6i";
+		break;
+	}
+	dev_notice(&pdev->dev,
+		   "%s SPI controller at 0x%08x, IRQ %i, %i bytes FIFO",
+		   desc, res->start, irq, sspi->fifo_depth);
+	if (sspi->type != SPI_SUN4I) {
+		version = sunxi_spi_read(sspi, SUNXI_VERSION_REG);
+		dev_notice(&pdev->dev, "HW revision %x.%x",
+			   version >> 16,
+			   version && 0xff);
+	}
+
 	return 0;
 
 err_pm_disable:
@@ -561,6 +682,7 @@ static int sunxi_spi_remove(struct platform_device *pdev)
 
 static const struct of_device_id sunxi_spi_match[] = {
 	{ .compatible = SUN4I_COMPATIBLE, },
+	{ .compatible = SUN6I_COMPATIBLE, },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sunxi_spi_match);
@@ -583,5 +705,5 @@ module_platform_driver(sunxi_spi_driver);
 
 MODULE_AUTHOR("Pan Nan <pannan-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org>");
 MODULE_AUTHOR("Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
-MODULE_DESCRIPTION("Allwinner A1X/A20 SPI controller driver");
+MODULE_DESCRIPTION("Allwinner A1X/A2X/A31 SPI controller driver");
 MODULE_LICENSE("GPL");
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
deleted file mode 100644
index 4215c3e..0000000
--- a/drivers/spi/spi-sun6i.c
+++ /dev/null
@@ -1,598 +0,0 @@
-/*
- * Copyright (C) 2012 - 2014 Allwinner Tech
- * Pan Nan <pannan-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org>
- *
- * Copyright (C) 2014 Maxime Ripard
- * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- */
-
-#include <linux/clk.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/interrupt.h>
-#include <linux/io.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
-#include <linux/reset.h>
-
-#include <linux/spi/spi.h>
-
-#define SUN4I_FIFO_DEPTH		64
-#define SUN6I_FIFO_DEPTH		128
-
-#define SUN4I_COMPATIBLE "allwinner,sun4i-a10-spi"
-#define SUN6I_COMPATIBLE "allwinner,sun6i-a31-spi"
-
-#define SUNXI_TFR_CTL_CS(bitmap, cs)	(((cs) << \
-					  (bitmap)[SUNXI_TFR_CTL_CS_SHIFT]) \
-					 & (bitmap)[SUNXI_TFR_CTL_CS_MASK])
-
-#define SUNXI_CNT_MASK			0xffffff
-#define SUNXI_XMIT_CNT(cnt)		((cnt) & SUNXI_CNT_MASK)
-#define SUNXI_BURST_CNT(cnt)		((cnt) & SUNXI_CNT_MASK)
-#define SUNXI_BURST_CTL_CNT_STC(cnt)	((cnt) & SUNXI_CNT_MASK)
-
-#define SUNXI_CLK_CTL_DRS		BIT(12)
-#define SUNXI_CLK_CTL_CDR2_MASK		0xff
-#define SUNXI_CLK_CTL_CDR2(div)		(((div) & SUNXI_CLK_CTL_CDR2_MASK) << 0)
-#define SUNXI_CLK_CTL_CDR1_MASK		0xf
-#define SUNXI_CLK_CTL_CDR1(div)		(((div) & SUNXI_CLK_CTL_CDR1_MASK) << 8)
-
-#define SUNXI_FIFO_STA_RF_CNT_MASK	0x7f
-#define SUNXI_FIFO_STA_RF_CNT_BITS	0
-#define SUNXI_FIFO_STA_TF_CNT_MASK	0x7f
-#define SUNXI_FIFO_STA_TF_CNT_BITS	16
-
-enum SPI_SUNXI_TYPE {
-	SPI_SUN4I = 1,
-	SPI_SUN6I,
-};
-
-enum SUNXI_REG_ENUM {
-	SUNXI_RXDATA_REG,
-	SUNXI_TXDATA_REG,
-	SUNXI_TFR_CTL_REG,
-	SUNXI_INT_CTL_REG,
-	SUNXI_INT_STA_REG,
-	SUNXI_WAIT_REG,
-	SUNXI_CLK_CTL_REG,
-	SUNXI_BURST_CNT_REG,
-	SUNXI_XMIT_CNT_REG,
-	SUNXI_FIFO_STA_REG,
-	SUNXI_VERSION_REG,
-	SUNXI_GBL_CTL_REG,
-	SUNXI_FIFO_CTL_REG,
-	SUNXI_BURST_CTL_CNT_REG,
-	SUNXI_NUM_REGS
-};
-
-static int sun6i_regmap[SUNXI_NUM_REGS] = {
-/* SUNXI_RXDATA_REG */			0x300,
-/* SUNXI_TXDATA_REG */			0x200,
-/* SUNXI_TFR_CTL_REG */			0x08,
-/* SUNXI_INT_CTL_REG */			0x10,
-/* SUNXI_INT_STA_REG */			0x14,
-/* SUNXI_WAIT_REG */			0x20,
-/* SUNXI_CLK_CTL_REG */			0x24,
-/* SUNXI_BURST_CNT_REG */		0x30,
-/* SUNXI_XMIT_CNT_REG */		0x34,
-/* SUNXI_FIFO_STA_REG */		0x1c,
-/* SUNXI_VERSION_REG */			0x00,
-/* SUNXI_GBL_CTL_REG */			0x04,
-/* SUNXI_FIFO_CTL_REG */		0x18,
-/* SUNXI_BURST_CTL_CNT_REG */		0x38,
-};
-
-enum SUNXI_BITMAP_ENUM {
-	SUNXI_CTL_ENABLE,
-	SUNXI_CTL_MASTER,
-	SUNXI_TFR_CTL_CPHA,
-	SUNXI_TFR_CTL_CPOL,
-	SUNXI_TFR_CTL_CS_ACTIVE_LOW,
-	SUNXI_TFR_CTL_FBS,
-	SUNXI_CTL_TF_RST,
-	SUNXI_CTL_RF_RST,
-	SUNXI_TFR_CTL_XCH,
-	SUNXI_TFR_CTL_CS_MASK,
-	SUNXI_TFR_CTL_CS_SHIFT,
-	SUNXI_TFR_CTL_DHB,
-	SUNXI_TFR_CTL_CS_MANUAL,
-	SUNXI_TFR_CTL_CS_LEVEL,
-	SUNXI_CTL_TP,
-	SUNXI_INT_CTL_TC,
-	SUNXI_BITMAP_SIZE
-};
-
-static int sun6i_bitmap[SUNXI_BITMAP_SIZE] = {
-/* SUNXI_CTL_ENABLE */			BIT(0),
-/* SUNXI_CTL_MASTER */			BIT(1),
-/* SUNXI_TFR_CTL_CPHA */		BIT(0),
-/* SUNXI_TFR_CTL_CPOL */		BIT(1),
-/* SUNXI_TFR_CTL_CS_ACTIVE_LOW */	BIT(2),
-/* SUNXI_TFR_CTL_FBS */			BIT(12),
-/* SUNXI_CTL_TF_RST */			BIT(31),
-/* SUNXI_CTL_RF_RST */			BIT(15),
-/* SUNXI_TFR_CTL_XCH */			BIT(31),
-/* SUNXI_TFR_CTL_CS_MASK */		0x30,
-/* SUNXI_TFR_CTL_CS_SHIFT */		4,
-/* SUNXI_TFR_CTL_DHB */			BIT(8),
-/* SUNXI_TFR_CTL_CS_MANUAL */		BIT(6),
-/* SUNXI_TFR_CTL_CS_LEVEL */		BIT(7),
-/* SUNXI_CTL_TP */			BIT(7),
-/* SUNXI_INT_CTL_TC */			BIT(12),
-};
-
-struct sunxi_spi {
-	struct spi_master	*master;
-	void __iomem		*base_addr;
-	struct clk		*hclk;
-	struct clk		*mclk;
-	struct reset_control	*rstc;
-	int			(*regmap)[SUNXI_NUM_REGS];
-	int			(*bitmap)[SUNXI_BITMAP_SIZE];
-	int			fifo_depth;
-	int			type;
-
-	struct completion	done;
-
-	const u8		*tx_buf;
-	u8			*rx_buf;
-	int			len;
-};
-
-static inline u32 sspi_reg(struct sunxi_spi *sspi, enum SUNXI_REG_ENUM name)
-{
-	BUG_ON((name >= SUNXI_NUM_REGS) || (name < 0) ||
-	       (*sspi->regmap)[name] < 0);
-	return (*sspi->regmap)[name];
-}
-
-static inline u32 sunxi_spi_read(struct sunxi_spi *sspi,
-				 enum SUNXI_REG_ENUM name)
-{
-	return readl(sspi->base_addr + sspi_reg(sspi, name));
-}
-
-static inline void sunxi_spi_write(struct sunxi_spi *sspi,
-				   enum SUNXI_REG_ENUM name, u32 value)
-{
-	writel(value, sspi->base_addr + sspi_reg(sspi, name));
-}
-
-static inline u32 sspi_bits(struct sunxi_spi *sspi,
-			    enum SUNXI_BITMAP_ENUM name)
-{
-	BUG_ON((name >= SUNXI_BITMAP_SIZE) || (name < 0) ||
-	       (*sspi->bitmap)[name] <= 0);
-	return (*sspi->bitmap)[name];
-}
-
-static inline void sunxi_spi_drain_fifo(struct sunxi_spi *sspi, int len)
-{
-	u32 reg, cnt;
-	u8 byte;
-
-	/* See how much data is available */
-	reg = sunxi_spi_read(sspi, SUNXI_FIFO_STA_REG);
-	reg &= SUNXI_FIFO_STA_RF_CNT_MASK;
-	cnt = reg >> SUNXI_FIFO_STA_RF_CNT_BITS;
-
-	if (len > cnt)
-		len = cnt;
-
-	while (len--) {
-		byte = readb(sspi->base_addr +
-			     sspi_reg(sspi, SUNXI_RXDATA_REG));
-		if (sspi->rx_buf)
-			*sspi->rx_buf++ = byte;
-	}
-}
-
-static inline void sunxi_spi_fill_fifo(struct sunxi_spi *sspi, int len)
-{
-	u8 byte;
-
-	if (len > sspi->len)
-		len = sspi->len;
-
-	while (len--) {
-		byte = sspi->tx_buf ? *sspi->tx_buf++ : 0;
-		writeb(byte, sspi->base_addr +
-		       sspi_reg(sspi, SUNXI_TXDATA_REG));
-		sspi->len--;
-	}
-}
-
-static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
-{
-	struct sunxi_spi *sspi = spi_master_get_devdata(spi->master);
-	u32 reg;
-
-	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-	reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CS_MASK);
-	reg |= SUNXI_TFR_CTL_CS(*sspi->bitmap, spi->chip_select);
-
-	/* We want to control the chip select manually */
-	reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CS_MANUAL);
-
-	if (enable)
-		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CS_LEVEL);
-	else
-		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CS_LEVEL);
-
-	/*
-	 * Even though this looks irrelevant since we are supposed to
-	 * be controlling the chip select manually, this bit also
-	 * controls the levels of the chip select for inactive
-	 * devices.
-	 *
-	 * If we don't set it, the chip select level will go low by
-	 * default when the device is idle, which is not really
-	 * expected in the common case where the chip select is active
-	 * low.
-	 */
-	if (spi->mode & SPI_CS_HIGH)
-		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CS_ACTIVE_LOW);
-	else
-		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CS_ACTIVE_LOW);
-
-	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
-}
-
-static size_t sunxi_spi_max_transfer_size(struct spi_device *spi)
-{
-	struct spi_master *master = spi->master;
-	struct sunxi_spi *sspi = spi_master_get_devdata(master);
-
-	return sspi->fifo_depth - 1;
-}
-
-static int sunxi_spi_transfer_one(struct spi_master *master,
-				  struct spi_device *spi,
-				  struct spi_transfer *tfr)
-{
-	struct sunxi_spi *sspi = spi_master_get_devdata(master);
-	unsigned int mclk_rate, div, timeout;
-	unsigned int start, end, tx_time;
-	unsigned int tx_len = 0;
-	int ret = 0;
-	u32 reg;
-
-	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > sspi->fifo_depth)
-		return -EMSGSIZE;
-
-	reinit_completion(&sspi->done);
-	sspi->tx_buf = tfr->tx_buf;
-	sspi->rx_buf = tfr->rx_buf;
-	sspi->len = tfr->len;
-
-	/* Clear pending interrupts */
-	sunxi_spi_write(sspi, SUNXI_INT_STA_REG, ~0);
-
-	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-
-	/* Reset FIFOs */
-	sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
-			sspi_bits(sspi, SUNXI_CTL_RF_RST) |
-			sspi_bits(sspi, SUNXI_CTL_TF_RST));
-
-	/*
-	 * Setup the transfer control register: Chip Select,
-	 * polarities, etc.
-	 */
-	if (spi->mode & SPI_CPOL)
-		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CPOL);
-	else
-		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CPOL);
-
-	if (spi->mode & SPI_CPHA)
-		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_CPHA);
-	else
-		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_CPHA);
-
-	if (spi->mode & SPI_LSB_FIRST)
-		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_FBS);
-	else
-		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_FBS);
-
-	/*
-	 * If it's a TX only transfer, we don't want to fill the RX
-	 * FIFO with bogus data
-	 */
-	if (sspi->rx_buf)
-		reg &= ~sspi_bits(sspi, SUNXI_TFR_CTL_DHB);
-	else
-		reg |= sspi_bits(sspi, SUNXI_TFR_CTL_DHB);
-
-	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg);
-
-	/* Ensure that we have a parent clock fast enough */
-	mclk_rate = clk_get_rate(sspi->mclk);
-	if (mclk_rate < (2 * tfr->speed_hz)) {
-		clk_set_rate(sspi->mclk, 2 * tfr->speed_hz);
-		mclk_rate = clk_get_rate(sspi->mclk);
-	}
-
-	/*
-	 * Setup clock divider.
-	 *
-	 * We have two choices there. Either we can use the clock
-	 * divide rate 1, which is calculated thanks to this formula:
-	 * SPI_CLK = MOD_CLK / (2 ^ cdr)
-	 * Or we can use CDR2, which is calculated with the formula:
-	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
-	 * Wether we use the former or the latter is set through the
-	 * DRS bit.
-	 *
-	 * First try CDR2, and if we can't reach the expected
-	 * frequency, fall back to CDR1.
-	 */
-	div = mclk_rate / (2 * tfr->speed_hz);
-	if (div <= (SUNXI_CLK_CTL_CDR2_MASK + 1)) {
-		if (div > 0)
-			div--;
-
-		reg = SUNXI_CLK_CTL_CDR2(div) | SUNXI_CLK_CTL_DRS;
-	} else {
-		div = ilog2(mclk_rate) - ilog2(tfr->speed_hz);
-		reg = SUNXI_CLK_CTL_CDR1(div);
-	}
-
-	sunxi_spi_write(sspi, SUNXI_CLK_CTL_REG, reg);
-
-	/* Setup the transfer now... */
-	if (sspi->tx_buf)
-		tx_len = tfr->len;
-
-	/* Setup the counters */
-	sunxi_spi_write(sspi, SUNXI_BURST_CNT_REG, SUNXI_BURST_CNT(tfr->len));
-	sunxi_spi_write(sspi, SUNXI_XMIT_CNT_REG, SUNXI_XMIT_CNT(tx_len));
-	sunxi_spi_write(sspi, SUNXI_BURST_CTL_CNT_REG, SUNXI_BURST_CTL_CNT_STC(tx_len));
-
-	/* Fill the TX FIFO */
-	sunxi_spi_fill_fifo(sspi, sspi->fifo_depth);
-
-	/* Enable the interrupts */
-	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG,
-			sspi_bits(sspi, SUNXI_INT_CTL_TC));
-
-	/* Start the transfer */
-	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
-			reg | sspi_bits(sspi, SUNXI_TFR_CTL_XCH));
-
-	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
-	start = jiffies;
-	timeout = wait_for_completion_timeout(&sspi->done,
-					      msecs_to_jiffies(tx_time));
-	end = jiffies;
-	if (!timeout) {
-		dev_warn(&master->dev,
-			 "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
-			 dev_name(&spi->dev), tfr->len, tfr->speed_hz,
-			 jiffies_to_msecs(end - start), tx_time);
-		ret = -ETIMEDOUT;
-		goto out;
-	}
-
-	sunxi_spi_drain_fifo(sspi, sspi->fifo_depth);
-
-out:
-	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, 0);
-
-	return ret;
-}
-
-static irqreturn_t sunxi_spi_handler(int irq, void *dev_id)
-{
-	struct sunxi_spi *sspi = dev_id;
-	u32 status = sunxi_spi_read(sspi, SUNXI_INT_STA_REG);
-
-	/* Transfer complete */
-	if (status & sspi_bits(sspi, SUNXI_INT_CTL_TC)) {
-		sunxi_spi_write(sspi, SUNXI_INT_STA_REG,
-				sspi_bits(sspi, SUNXI_INT_CTL_TC));
-		complete(&sspi->done);
-		return IRQ_HANDLED;
-	}
-
-	return IRQ_NONE;
-}
-
-static int sunxi_spi_runtime_resume(struct device *dev)
-{
-	struct spi_master *master = dev_get_drvdata(dev);
-	struct sunxi_spi *sspi = spi_master_get_devdata(master);
-	int ret;
-
-	ret = clk_prepare_enable(sspi->hclk);
-	if (ret) {
-		dev_err(dev, "Couldn't enable AHB clock\n");
-		goto out;
-	}
-
-	ret = clk_prepare_enable(sspi->mclk);
-	if (ret) {
-		dev_err(dev, "Couldn't enable module clock\n");
-		goto err;
-	}
-
-	ret = reset_control_deassert(sspi->rstc);
-	if (ret) {
-		dev_err(dev, "Couldn't deassert the device from reset\n");
-		goto err2;
-	}
-
-	sunxi_spi_write(sspi, SUNXI_GBL_CTL_REG,
-			sspi_bits(sspi, SUNXI_CTL_ENABLE) |
-			sspi_bits(sspi, SUNXI_CTL_MASTER) |
-			sspi_bits(sspi, SUNXI_CTL_TP));
-
-	return 0;
-
-err2:
-	clk_disable_unprepare(sspi->mclk);
-err:
-	clk_disable_unprepare(sspi->hclk);
-out:
-	return ret;
-}
-
-static int sunxi_spi_runtime_suspend(struct device *dev)
-{
-	struct spi_master *master = dev_get_drvdata(dev);
-	struct sunxi_spi *sspi = spi_master_get_devdata(master);
-
-	reset_control_assert(sspi->rstc);
-	clk_disable_unprepare(sspi->mclk);
-	clk_disable_unprepare(sspi->hclk);
-
-	return 0;
-}
-
-static int sunxi_spi_probe(struct platform_device *pdev)
-{
-	struct spi_master *master;
-	struct sunxi_spi *sspi;
-	struct resource	*res;
-	int ret = 0, irq;
-
-	master = spi_alloc_master(&pdev->dev, sizeof(struct sunxi_spi));
-	if (!master) {
-		dev_err(&pdev->dev, "Unable to allocate SPI Master\n");
-		return -ENOMEM;
-	}
-
-	platform_set_drvdata(pdev, master);
-	sspi = spi_master_get_devdata(master);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	sspi->base_addr = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(sspi->base_addr)) {
-		ret = PTR_ERR(sspi->base_addr);
-		goto err_free_master;
-	}
-
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		dev_err(&pdev->dev, "No spi IRQ specified\n");
-		ret = -ENXIO;
-		goto err_free_master;
-	}
-
-	ret = devm_request_irq(&pdev->dev, irq, sunxi_spi_handler,
-			       0, "sunxi-spi", sspi);
-	if (ret) {
-		dev_err(&pdev->dev, "Cannot request IRQ\n");
-		goto err_free_master;
-	}
-
-	sspi->master = master;
-	sspi->fifo_depth = SUN6I_FIFO_DEPTH;
-	sspi->type = SPI_SUN6I;
-	sspi->regmap = &sun6i_regmap;
-	sspi->bitmap = &sun6i_bitmap;
-	master->max_speed_hz = 100 * 1000 * 1000;
-	master->min_speed_hz =          3 * 1000;
-	master->set_cs = sunxi_spi_set_cs;
-	master->transfer_one = sunxi_spi_transfer_one;
-	master->num_chipselect = 4;
-	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
-	master->bits_per_word_mask = SPI_BPW_MASK(8);
-	master->dev.of_node = pdev->dev.of_node;
-	master->auto_runtime_pm = true;
-	master->max_transfer_size = sunxi_spi_max_transfer_size;
-
-	sspi->hclk = devm_clk_get(&pdev->dev, "ahb");
-	if (IS_ERR(sspi->hclk)) {
-		dev_err(&pdev->dev, "Unable to acquire AHB clock\n");
-		ret = PTR_ERR(sspi->hclk);
-		goto err_free_master;
-	}
-
-	sspi->mclk = devm_clk_get(&pdev->dev, "mod");
-	if (IS_ERR(sspi->mclk)) {
-		dev_err(&pdev->dev, "Unable to acquire module clock\n");
-		ret = PTR_ERR(sspi->mclk);
-		goto err_free_master;
-	}
-
-	init_completion(&sspi->done);
-
-	sspi->rstc = devm_reset_control_get(&pdev->dev, NULL);
-	if (IS_ERR(sspi->rstc)) {
-		dev_err(&pdev->dev, "Couldn't get reset controller\n");
-		ret = PTR_ERR(sspi->rstc);
-		goto err_free_master;
-	}
-
-	/*
-	 * This wake-up/shutdown pattern is to be able to have the
-	 * device woken up, even if runtime_pm is disabled
-	 */
-	ret = sunxi_spi_runtime_resume(&pdev->dev);
-	if (ret) {
-		dev_err(&pdev->dev, "Couldn't resume the device\n");
-		goto err_free_master;
-	}
-
-	pm_runtime_set_active(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
-	pm_runtime_idle(&pdev->dev);
-
-	ret = devm_spi_register_master(&pdev->dev, master);
-	if (ret) {
-		dev_err(&pdev->dev, "cannot register SPI master\n");
-		goto err_pm_disable;
-	}
-
-	return 0;
-
-err_pm_disable:
-	pm_runtime_disable(&pdev->dev);
-	sunxi_spi_runtime_suspend(&pdev->dev);
-err_free_master:
-	spi_master_put(master);
-	return ret;
-}
-
-static int sunxi_spi_remove(struct platform_device *pdev)
-{
-	pm_runtime_disable(&pdev->dev);
-
-	return 0;
-}
-
-static const struct of_device_id sunxi_spi_match[] = {
-	{ .compatible = SUN6I_COMPATIBLE, },
-	{}
-};
-MODULE_DEVICE_TABLE(of, sunxi_spi_match);
-
-static const struct dev_pm_ops sunxi_spi_pm_ops = {
-	.runtime_resume		= sunxi_spi_runtime_resume,
-	.runtime_suspend	= sunxi_spi_runtime_suspend,
-};
-
-static struct platform_driver sunxi_spi_driver = {
-	.probe	= sunxi_spi_probe,
-	.remove	= sunxi_spi_remove,
-	.driver	= {
-		.name		= "sunxi-spi",
-		.of_match_table	= sunxi_spi_match,
-		.pm		= &sunxi_spi_pm_ops,
-	},
-};
-module_platform_driver(sunxi_spi_driver);
-
-MODULE_AUTHOR("Pan Nan <pannan-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org>");
-MODULE_AUTHOR("Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
-MODULE_DESCRIPTION("Allwinner A31 SPI controller driver");
-MODULE_LICENSE("GPL");
-- 
2.8.1

Re: [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The drivers are very similar and share multiple flaws which needed
> separate fixes for both drivers.
>
> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/spi/Kconfig     |   8 +-
>  drivers/spi/Makefile    |   1 -
>  drivers/spi/spi-sun4i.c | 156 +++++++++++--
>  drivers/spi/spi-sun6i.c | 598 ------------------------------------------------
>  4 files changed, 143 insertions(+), 620 deletions(-)
>  delete mode 100644 drivers/spi/spi-sun6i.c
>
> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 0b8e6c6..c76f8e4 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
>         reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>
>         /* Reset FIFOs */
> -       sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> -                       reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> -                       sspi_bits(sspi, SUNXI_CTL_TF_RST));
> +       if (sspi->type == SPI_SUN4I)
> +               sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> +                               reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
> +       else
> +               sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
> +                               sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));

If we're already doing different stuff for each generation of the IP,
why not just use the register offsets and bit definitions directly?

>
>         /*
>          * Setup the transfer control register: Chip Select,
> @@ -427,7 +473,19 @@ static int sunxi_spi_runtime_resume(struct device *dev)
>                 goto err;
>         }
>
> -       sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> +       if (sspi->rstc) {
> +               ret = reset_control_deassert(sspi->rstc);
> +               if (ret) {
> +                       dev_err(dev, "Couldn't deassert the device from reset\n");
> +                       goto err2;
> +               }
> +       }
> +
> +       if (sspi->type == SPI_SUN4I)
> +               reg = SUNXI_TFR_CTL_REG;
> +       else
> +               reg = SUNXI_GBL_CTL_REG;
> +       sunxi_spi_write(sspi, reg,
>                         sspi_bits(sspi, SUNXI_CTL_ENABLE) |
>                         sspi_bits(sspi, SUNXI_CTL_MASTER) |
>                         sspi_bits(sspi, SUNXI_CTL_TP));

Same here.

> @@ -491,10 +558,37 @@ static int sunxi_spi_probe(struct platform_device *pdev)
>         }
>
>         sspi->master = master;
> -       sspi->fifo_depth = SUN4I_FIFO_DEPTH;
> -       sspi->type = SPI_SUN4I;
> -       sspi->regmap = &sun4i_regmap;
> -       sspi->bitmap = &sun4i_bitmap;
> +       if (of_device_is_compatible(pdev->dev.of_node, SUN4I_COMPATIBLE)) {
> +               sspi->fifo_depth = SUN4I_FIFO_DEPTH;
> +               sspi->type = SPI_SUN4I;
> +               sspi->regmap = &sun4i_regmap;
> +               sspi->bitmap = &sun4i_bitmap;
> +       } else if (of_device_is_compatible(pdev->dev.of_node,
> +                                          SUN6I_COMPATIBLE)) {
> +               sspi->fifo_depth = SUN6I_FIFO_DEPTH;
> +               sspi->type = SPI_SUN6I;
> +               sspi->regmap = &sun6i_regmap;
> +               sspi->bitmap = &sun6i_bitmap;

Can you store data in the match table instead of doing this?

> +       } else {
> +               const char *str = NULL;
> +               int i = 1;
> +
> +               of_property_read_string(pdev->dev.of_node, "compatible", &str);
> +               dev_err(&pdev->dev, "Unknown device compatible %s", str);
> +               /* is there no sane way to print a string array property ? */
> +               if (of_property_count_strings(pdev->dev.of_node, "compatible")
> +                   > 1) {
> +                       while (!of_property_read_string_index(pdev->dev.of_node,
> +                                                             "compatible", i,
> +                                                             &str)) {
> +                               pr_err(", %s", str);
> +                               i++;
> +                       }
> +               }
> +               ret = -EINVAL;
> +               goto err_free_master;
> +       }
> +
>         master->max_speed_hz = 100 * 1000 * 1000;
>         master->min_speed_hz =          3 * 1000;
>         master->set_cs = sunxi_spi_set_cs;

Thanks,

-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Michal Suchanek]

Hello,

On 14 June 2016 at 01:43, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> The drivers are very similar and share multiple flaws which needed
>> separate fixes for both drivers.
>>
>> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/spi/Kconfig     |   8 +-
>>  drivers/spi/Makefile    |   1 -
>>  drivers/spi/spi-sun4i.c | 156 +++++++++++--
>>  drivers/spi/spi-sun6i.c | 598 ------------------------------------------------
>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>
>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>> index 0b8e6c6..c76f8e4 100644
>> --- a/drivers/spi/spi-sun4i.c
>> +++ b/drivers/spi/spi-sun4i.c
>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
>>         reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>
>>         /* Reset FIFOs */
>> -       sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>> -                       reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>> -                       sspi_bits(sspi, SUNXI_CTL_TF_RST));
>> +       if (sspi->type == SPI_SUN4I)
>> +               sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>> +                               reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>> +       else
>> +               sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>> +                               sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>
> If we're already doing different stuff for each generation of the IP,
> why not just use the register offsets and bit definitions directly?

Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
makes my eyes bleed and you cannot use the check that you are
accessing a register that actually exists.

>> @@ -491,10 +558,37 @@ static int sunxi_spi_probe(struct platform_device *pdev)
>>         }
>>
>>         sspi->master = master;
>> -       sspi->fifo_depth = SUN4I_FIFO_DEPTH;
>> -       sspi->type = SPI_SUN4I;
>> -       sspi->regmap = &sun4i_regmap;
>> -       sspi->bitmap = &sun4i_bitmap;
>> +       if (of_device_is_compatible(pdev->dev.of_node, SUN4I_COMPATIBLE)) {
>> +               sspi->fifo_depth = SUN4I_FIFO_DEPTH;
>> +               sspi->type = SPI_SUN4I;
>> +               sspi->regmap = &sun4i_regmap;
>> +               sspi->bitmap = &sun4i_bitmap;
>> +       } else if (of_device_is_compatible(pdev->dev.of_node,
>> +                                          SUN6I_COMPATIBLE)) {
>> +               sspi->fifo_depth = SUN6I_FIFO_DEPTH;
>> +               sspi->type = SPI_SUN6I;
>> +               sspi->regmap = &sun6i_regmap;
>> +               sspi->bitmap = &sun6i_bitmap;
>
> Can you store data in the match table instead of doing this?

That might be nicer. Will look into this.

Thanks

Michal

Re: [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hello,
>
> On 14 June 2016 at 01:43, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> The drivers are very similar and share multiple flaws which needed
>>> separate fixes for both drivers.
>>>
>>> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  drivers/spi/Kconfig     |   8 +-
>>>  drivers/spi/Makefile    |   1 -
>>>  drivers/spi/spi-sun4i.c | 156 +++++++++++--
>>>  drivers/spi/spi-sun6i.c | 598 ------------------------------------------------
>>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>>
>>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>>> index 0b8e6c6..c76f8e4 100644
>>> --- a/drivers/spi/spi-sun4i.c
>>> +++ b/drivers/spi/spi-sun4i.c
>>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
>>>         reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>>
>>>         /* Reset FIFOs */
>>> -       sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>> -                       reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>> -                       sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>> +       if (sspi->type == SPI_SUN4I)
>>> +               sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>> +                               reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>> +       else
>>> +               sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>>> +                               sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>
>> If we're already doing different stuff for each generation of the IP,
>> why not just use the register offsets and bit definitions directly?
>
> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
> makes my eyes bleed and you cannot use the check that you are
> accessing a register that actually exists.

I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
indirection you added and using them directly, i.e.

#define SUN4I_TFR_CTL_RF_RST BIT(x)
#define SUN4I_TFR_CTL_TF_RST BIT(x)
#define SUN6I_FIFO_CTL_RF_RST BIT(x)
#define SUN6I_FIFO_CTL_TF_RST BIT(x)

then

if (sspi->type == SPI_SUN4I)
    sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
else
    sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);

I.e. the bits that need setting are in different registers, so you
have to have an if statement and separate calls. Therefore there's no
real benefit from the indirection you've introduced here, unless
you're expecting the SUN8I variant to use different bits in one of
those two registers.

Thanks,

-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Michal Suchanek]

On 14 June 2016 at 06:47, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hello,
>>
>> On 14 June 2016 at 01:43, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Hi Michal,
>>>
>>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> The drivers are very similar and share multiple flaws which needed
>>>> separate fixes for both drivers.
>>>>
>>>> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>  drivers/spi/Kconfig     |   8 +-
>>>>  drivers/spi/Makefile    |   1 -
>>>>  drivers/spi/spi-sun4i.c | 156 +++++++++++--
>>>>  drivers/spi/spi-sun6i.c | 598 ------------------------------------------------
>>>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>>>
>>>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>>>> index 0b8e6c6..c76f8e4 100644
>>>> --- a/drivers/spi/spi-sun4i.c
>>>> +++ b/drivers/spi/spi-sun4i.c
>>>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
>>>>         reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>>>
>>>>         /* Reset FIFOs */
>>>> -       sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>>> -                       reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>> -                       sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>> +       if (sspi->type == SPI_SUN4I)
>>>> +               sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>>> +                               reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>> +       else
>>>> +               sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>>>> +                               sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>
>>> If we're already doing different stuff for each generation of the IP,
>>> why not just use the register offsets and bit definitions directly?
>>
>> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
>> makes my eyes bleed and you cannot use the check that you are
>> accessing a register that actually exists.
>
> I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
> indirection you added and using them directly, i.e.
>
> #define SUN4I_TFR_CTL_RF_RST BIT(x)
> #define SUN4I_TFR_CTL_TF_RST BIT(x)
> #define SUN6I_FIFO_CTL_RF_RST BIT(x)
> #define SUN6I_FIFO_CTL_TF_RST BIT(x)
>
> then
>
> if (sspi->type == SPI_SUN4I)
>     sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
> SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
> else
>     sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
> SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
>
> I.e. the bits that need setting are in different registers, so you
> have to have an if statement and separate calls. Therefore there's no
> real benefit from the indirection you've introduced here, unless
> you're expecting the SUN8I variant to use different bits in one of
> those two registers.
>

That looks nice for this particular case.

Still you will have to remember that these bits are specified directly
while other bits on the register are mapped which is not so nice.

Thanks

Michal

Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 3:28 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 14 June 2016 at 06:47, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Hello,
>>>
>>> On 14 June 2016 at 01:43, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> Hi Michal,
>>>>
>>>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> The drivers are very similar and share multiple flaws which needed
>>>>> separate fixes for both drivers.
>>>>>
>>>>> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>>  drivers/spi/Kconfig     |   8 +-
>>>>>  drivers/spi/Makefile    |   1 -
>>>>>  drivers/spi/spi-sun4i.c | 156 +++++++++++--
>>>>>  drivers/spi/spi-sun6i.c | 598 ------------------------------------------------
>>>>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>>>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>>>>
>>>>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>>>>> index 0b8e6c6..c76f8e4 100644
>>>>> --- a/drivers/spi/spi-sun4i.c
>>>>> +++ b/drivers/spi/spi-sun4i.c
>>>>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
>>>>>         reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>>>>
>>>>>         /* Reset FIFOs */
>>>>> -       sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>>>> -                       reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>>> -                       sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>>> +       if (sspi->type == SPI_SUN4I)
>>>>> +               sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>>>> +                               reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>>> +       else
>>>>> +               sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>>>>> +                               sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>>
>>>> If we're already doing different stuff for each generation of the IP,
>>>> why not just use the register offsets and bit definitions directly?
>>>
>>> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
>>> makes my eyes bleed and you cannot use the check that you are
>>> accessing a register that actually exists.
>>
>> I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
>> indirection you added and using them directly, i.e.
>>
>> #define SUN4I_TFR_CTL_RF_RST BIT(x)
>> #define SUN4I_TFR_CTL_TF_RST BIT(x)
>> #define SUN6I_FIFO_CTL_RF_RST BIT(x)
>> #define SUN6I_FIFO_CTL_TF_RST BIT(x)
>>
>> then
>>
>> if (sspi->type == SPI_SUN4I)
>>     sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
>> SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
>> else
>>     sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
>> SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
>>
>> I.e. the bits that need setting are in different registers, so you
>> have to have an if statement and separate calls. Therefore there's no
>> real benefit from the indirection you've introduced here, unless
>> you're expecting the SUN8I variant to use different bits in one of
>> those two registers.
>>
>
> That looks nice for this particular case.

There was another case I pointed out in this driver that could
potentially benefit from this.

> Still you will have to remember that these bits are specified directly
> while other bits on the register are mapped which is not so nice.

True. I'm not sure which path is best in this case. To my eye, your
indirection scheme seems like the "heavy" solution, however I have no
idea what form a "lighter" solution would take.

Other drivers I've seen which have tackled similar problems have used
a large struct to hold all the variant-specific stuff, whether that's
function pointers, register offsets, constants, etc. (so this code
could theoretically be re-written as sunxi_spi_write(sspi,
sspi->fifo_reg, reg | sspi->fifo_reset_arg) or sspi->reset_fifo())
however the driver I saw doing this (rtl8xxxu) was handling much more
significant differences between device variants than you are.

Thanks,

-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Michal Suchanek]

Hello,

On 14 June 2016 at 07:45, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Michal,
>
> On Tue, Jun 14, 2016 at 3:28 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 14 June 2016 at 06:47, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Hi Michal,
>>>
>>> On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> Hello,
>>>>
>>>> On 14 June 2016 at 01:43, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> Hi Michal,
>>>>>
>>>>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>> The drivers are very similar and share multiple flaws which needed
>>>>>> separate fixes for both drivers.
>>>>>>
>>>>>> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> ---
>>>>>>  drivers/spi/Kconfig     |   8 +-
>>>>>>  drivers/spi/Makefile    |   1 -
>>>>>>  drivers/spi/spi-sun4i.c | 156 +++++++++++--
>>>>>>  drivers/spi/spi-sun6i.c | 598 ------------------------------------------------
>>>>>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>>>>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>>>>>
>>>>>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>>>>>> index 0b8e6c6..c76f8e4 100644
>>>>>> --- a/drivers/spi/spi-sun4i.c
>>>>>> +++ b/drivers/spi/spi-sun4i.c
>>>>>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
>>>>>>         reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>>>>>
>>>>>>         /* Reset FIFOs */
>>>>>> -       sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>>>>> -                       reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>>>> -                       sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>>>> +       if (sspi->type == SPI_SUN4I)
>>>>>> +               sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>>>>> +                               reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>>>> +       else
>>>>>> +               sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>>>>>> +                               sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>>>
>>>>> If we're already doing different stuff for each generation of the IP,
>>>>> why not just use the register offsets and bit definitions directly?
>>>>
>>>> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
>>>> makes my eyes bleed and you cannot use the check that you are
>>>> accessing a register that actually exists.
>>>
>>> I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
>>> indirection you added and using them directly, i.e.
>>>
>>> #define SUN4I_TFR_CTL_RF_RST BIT(x)
>>> #define SUN4I_TFR_CTL_TF_RST BIT(x)
>>> #define SUN6I_FIFO_CTL_RF_RST BIT(x)
>>> #define SUN6I_FIFO_CTL_TF_RST BIT(x)
>>>
>>> then
>>>
>>> if (sspi->type == SPI_SUN4I)
>>>     sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
>>> SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
>>> else
>>>     sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
>>> SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
>>>
>>> I.e. the bits that need setting are in different registers, so you
>>> have to have an if statement and separate calls. Therefore there's no
>>> real benefit from the indirection you've introduced here, unless
>>> you're expecting the SUN8I variant to use different bits in one of
>>> those two registers.
>>>
>>
>> That looks nice for this particular case.
>
> There was another case I pointed out in this driver that could
> potentially benefit from this.
>
>> Still you will have to remember that these bits are specified directly
>> while other bits on the register are mapped which is not so nice.
>
> True. I'm not sure which path is best in this case. To my eye, your
> indirection scheme seems like the "heavy" solution, however I have no
> idea what form a "lighter" solution would take.
>
> Other drivers I've seen which have tackled similar problems have used
> a large struct to hold all the variant-specific stuff, whether that's
> function pointers, register offsets, constants, etc. (so this code
> could theoretically be re-written as sunxi_spi_write(sspi,
> sspi->fifo_reg, reg | sspi->fifo_reset_arg)

This won't do. There is fifo_reg and tfr_reg on both IPs but some bits
from one migrated to the other.

> or sspi->reset_fifo())

This would work but would require more thorough rewrite and heavier
adaptation layer.

As it is where different register is used in different IP revision it
is specified explicitly in the code while register names to numbers
are mapped in read() and write(). Value bits are mapped by explicitly
calling a function on the name.

This gives nice 1:1 mapping to the old code and allows checking that
both the register and the bit value in question exist on the IP.

Matching the value to register relies on the driver code, however.

Thanks

Michal

Re: [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver

[Julian Calaby]

Hi Michal,

On Tue, Jun 14, 2016 at 4:35 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hello,
>
> On 14 June 2016 at 07:45, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hi Michal,
>>
>> On Tue, Jun 14, 2016 at 3:28 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On 14 June 2016 at 06:47, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> Hi Michal,
>>>>
>>>> On Tue, Jun 14, 2016 at 2:34 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> Hello,
>>>>>
>>>>> On 14 June 2016 at 01:43, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>> Hi Michal,
>>>>>>
>>>>>> On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>>> The drivers are very similar and share multiple flaws which needed
>>>>>>> separate fixes for both drivers.
>>>>>>>
>>>>>>> Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>>> ---
>>>>>>>  drivers/spi/Kconfig     |   8 +-
>>>>>>>  drivers/spi/Makefile    |   1 -
>>>>>>>  drivers/spi/spi-sun4i.c | 156 +++++++++++--
>>>>>>>  drivers/spi/spi-sun6i.c | 598 ------------------------------------------------
>>>>>>>  4 files changed, 143 insertions(+), 620 deletions(-)
>>>>>>>  delete mode 100644 drivers/spi/spi-sun6i.c
>>>>>>>
>>>>>>> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
>>>>>>> index 0b8e6c6..c76f8e4 100644
>>>>>>> --- a/drivers/spi/spi-sun4i.c
>>>>>>> +++ b/drivers/spi/spi-sun4i.c
>>>>>>> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
>>>>>>>         reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>>>>>>>
>>>>>>>         /* Reset FIFOs */
>>>>>>> -       sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>>>>>> -                       reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>>>>> -                       sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>>>>> +       if (sspi->type == SPI_SUN4I)
>>>>>>> +               sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
>>>>>>> +                               reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>>>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>>>>> +       else
>>>>>>> +               sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
>>>>>>> +                               sspi_bits(sspi, SUNXI_CTL_RF_RST) |
>>>>>>> +                               sspi_bits(sspi, SUNXI_CTL_TF_RST));
>>>>>>
>>>>>> If we're already doing different stuff for each generation of the IP,
>>>>>> why not just use the register offsets and bit definitions directly?
>>>>>
>>>>> Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place
>>>>> makes my eyes bleed and you cannot use the check that you are
>>>>> accessing a register that actually exists.
>>>>
>>>> I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the
>>>> indirection you added and using them directly, i.e.
>>>>
>>>> #define SUN4I_TFR_CTL_RF_RST BIT(x)
>>>> #define SUN4I_TFR_CTL_TF_RST BIT(x)
>>>> #define SUN6I_FIFO_CTL_RF_RST BIT(x)
>>>> #define SUN6I_FIFO_CTL_TF_RST BIT(x)
>>>>
>>>> then
>>>>
>>>> if (sspi->type == SPI_SUN4I)
>>>>     sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |
>>>> SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST);
>>>> else
>>>>     sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |
>>>> SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
>>>>
>>>> I.e. the bits that need setting are in different registers, so you
>>>> have to have an if statement and separate calls. Therefore there's no
>>>> real benefit from the indirection you've introduced here, unless
>>>> you're expecting the SUN8I variant to use different bits in one of
>>>> those two registers.
>>>>
>>>
>>> That looks nice for this particular case.
>>
>> There was another case I pointed out in this driver that could
>> potentially benefit from this.
>>
>>> Still you will have to remember that these bits are specified directly
>>> while other bits on the register are mapped which is not so nice.
>>
>> True. I'm not sure which path is best in this case. To my eye, your
>> indirection scheme seems like the "heavy" solution, however I have no
>> idea what form a "lighter" solution would take.
>>
>> Other drivers I've seen which have tackled similar problems have used
>> a large struct to hold all the variant-specific stuff, whether that's
>> function pointers, register offsets, constants, etc. (so this code
>> could theoretically be re-written as sunxi_spi_write(sspi,
>> sspi->fifo_reg, reg | sspi->fifo_reset_arg)
>
> This won't do. There is fifo_reg and tfr_reg on both IPs but some bits
> from one migrated to the other.
>
>> or sspi->reset_fifo())
>
> This would work but would require more thorough rewrite and heavier
> adaptation layer.
>
> As it is where different register is used in different IP revision it
> is specified explicitly in the code while register names to numbers
> are mapped in read() and write(). Value bits are mapped by explicitly
> calling a function on the name.
>
> This gives nice 1:1 mapping to the old code and allows checking that
> both the register and the bit value in question exist on the IP.

And your method is almost beautiful from that perspective, however I
still feel that it's too "heavy". That said, neither of my suggestions
are much better. I provided them in the hope that they might be
illuminating.

> Matching the value to register relies on the driver code, however.

Of course.

I'm not sure what the state-of-the-art for dealing with variants of
devices where the manufacturer keeps scrambling the registers is,
however I feel we can do better, though I'm not sure how.

Thanks,

-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/

[PATCH v3 13/13] spi: sun4i: add DMA support

[Michal]

From: Emilio López <emilio-0Z03zUJReD5OxF6Tv1QG9Q@public.gmane.org>

This patch adds support for 64 byte or bigger transfers on the
sun4i SPI controller. Said transfers will be performed via DMA.

Signed-off-by: Emilio López <emilio-0Z03zUJReD5OxF6Tv1QG9Q@public.gmane.org>
Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

---

v2:
 - fallback to previous behaviour when DMA initialization fails
v3:
 - adjust to merged driver
 - add bit set/unset helpers
 - add wait_for_dma (default=1) so driver does not randomly load without dma
 - use SUNXI_CNT_MASK as transfer size limit
---
 drivers/spi/spi-sun4i.c | 247 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 230 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index c76f8e4..fd6b1a8 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -14,6 +14,8 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -50,6 +52,12 @@
 #define SUNXI_FIFO_STA_TF_CNT_MASK	0x7f
 #define SUNXI_FIFO_STA_TF_CNT_BITS	16
 
+static int wait_for_dma = 1;
+module_param(wait_for_dma, int, 0644);
+MODULE_PARM_DESC(wait_for_dma,
+		 "When acquiring a DMA channel returns EDEFER return and let kernel defer spi master probe.\n"
+		 "Non-DMA operation is used otherwise (defaults to wait for DMA driver to load).");
+
 enum SPI_SUNXI_TYPE {
 	SPI_SUN4I = 1,
 	SPI_SUN6I,
@@ -61,6 +69,7 @@ enum SUNXI_REG_ENUM {
 	SUNXI_TFR_CTL_REG,
 	SUNXI_INT_CTL_REG,
 	SUNXI_INT_STA_REG,
+	SUNXI_DMA_CTL_REG,
 	SUNXI_WAIT_REG,
 	SUNXI_CLK_CTL_REG,
 	SUNXI_BURST_CNT_REG,
@@ -79,6 +88,7 @@ static int sun4i_regmap[SUNXI_NUM_REGS] = {
 /* SUNXI_TFR_CTL_REG */			0x08,
 /* SUNXI_INT_CTL_REG */			0x0c,
 /* SUNXI_INT_STA_REG */			0x10,
+/* SUNXI_DMA_CTL_REG */			0x14,
 /* SUNXI_WAIT_REG */			0x18,
 /* SUNXI_CLK_CTL_REG */			0x1c,
 /* SUNXI_BURST_CNT_REG */		0x20,
@@ -93,6 +103,7 @@ static int sun6i_regmap[SUNXI_NUM_REGS] = {
 /* SUNXI_TFR_CTL_REG */			0x08,
 /* SUNXI_INT_CTL_REG */			0x10,
 /* SUNXI_INT_STA_REG */			0x14,
+/* SUNXI_DMA_CTL_REG */			-1,
 /* SUNXI_WAIT_REG */			0x20,
 /* SUNXI_CLK_CTL_REG */			0x24,
 /* SUNXI_BURST_CNT_REG */		0x30,
@@ -110,6 +121,7 @@ enum SUNXI_BITMAP_ENUM {
 	SUNXI_TFR_CTL_CPHA,
 	SUNXI_TFR_CTL_CPOL,
 	SUNXI_TFR_CTL_CS_ACTIVE_LOW,
+	SUNXI_CTL_DMA_DEDICATED,
 	SUNXI_TFR_CTL_FBS,
 	SUNXI_CTL_TF_RST,
 	SUNXI_CTL_RF_RST,
@@ -121,6 +133,9 @@ enum SUNXI_BITMAP_ENUM {
 	SUNXI_TFR_CTL_CS_LEVEL,
 	SUNXI_CTL_TP,
 	SUNXI_INT_CTL_TC,
+	SUNXI_CTL_DMA_RF_READY,
+	SUNXI_CTL_DMA_TF_NOT_FULL,
+	SUNXI_CTL_DMA_TF_HALF,
 	SUNXI_BITMAP_SIZE
 };
 
@@ -130,6 +145,7 @@ static int sun4i_bitmap[SUNXI_BITMAP_SIZE] = {
 /* SUNXI_TFR_CTL_CPHA */		BIT(2),
 /* SUNXI_TFR_CTL_CPOL */		BIT(3),
 /* SUNXI_TFR_CTL_CS_ACTIVE_LOW */	BIT(4),
+/* SUNXI_CTL_DMA_DEDICATED */		BIT(5),
 /* SUNXI_TFR_CTL_FBS */			BIT(6),
 /* SUNXI_CTL_TF_RST */			BIT(8),
 /* SUNXI_CTL_RF_RST */			BIT(9),
@@ -141,6 +157,9 @@ static int sun4i_bitmap[SUNXI_BITMAP_SIZE] = {
 /* SUNXI_TFR_CTL_CS_LEVEL */		BIT(17),
 /* SUNXI_CTL_TP */			BIT(18),
 /* SUNXI_INT_CTL_TC */			BIT(16),
+/* SUNXI_CTL_DMA_RF_READY */		BIT(0),
+/* SUNXI_CTL_DMA_TF_NOT_FULL */		BIT(10),
+/* SUNXI_CTL_DMA_TF_HALF */		BIT(9),
 };
 
 static int sun6i_bitmap[SUNXI_BITMAP_SIZE] = {
@@ -149,6 +168,12 @@ static int sun6i_bitmap[SUNXI_BITMAP_SIZE] = {
 /* SUNXI_TFR_CTL_CPHA */		BIT(0),
 /* SUNXI_TFR_CTL_CPOL */		BIT(1),
 /* SUNXI_TFR_CTL_CS_ACTIVE_LOW */	BIT(2),
+/*
+ * Bit 9 is listed as dedicated dma control for rx.
+ * There is no dedicated dma control bit listed for tx and bit 25
+ * on the logical position is listed as unused.
+ */
+/* SUNXI_CTL_DMA_DEDICATED */		BIT(9)|BIT(25),
 /* SUNXI_TFR_CTL_FBS */			BIT(12),
 /* SUNXI_CTL_TF_RST */			BIT(31),
 /* SUNXI_CTL_RF_RST */			BIT(15),
@@ -160,6 +185,15 @@ static int sun6i_bitmap[SUNXI_BITMAP_SIZE] = {
 /* SUNXI_TFR_CTL_CS_LEVEL */		BIT(7),
 /* SUNXI_CTL_TP */			BIT(7),
 /* SUNXI_INT_CTL_TC */			BIT(12),
+/*
+ * On sun4i there are separate bits enabling request on different fifo levels.
+ * On sun6i there is a level field and enable bit which enables request on that
+ * FIFO level. Only one level is ever used so just pack the relevant bits as
+ * one constant.
+ */
+/* SUNXI_CTL_DMA_RF_READY */		BIT(0)|BIT(8),
+/* SUNXI_CTL_DMA_TF_NOT_FULL */		(0x7f << 16)|BIT(24),
+/* SUNXI_CTL_DMA_TF_HALF */		BIT(23)|BIT(24),
 };
 
 struct sunxi_spi {
@@ -207,6 +241,20 @@ static inline u32 sspi_bits(struct sunxi_spi *sspi,
 	return (*sspi->bitmap)[name];
 }
 
+static inline void sunxi_spi_set(struct sunxi_spi *sspi, u32 reg, u32 value)
+{
+	u32 orig = sunxi_spi_read(sspi, reg);
+
+	sunxi_spi_write(sspi, reg, orig | value);
+}
+
+static inline void sunxi_spi_unset(struct sunxi_spi *sspi, u32 reg, u32 value)
+{
+	u32 orig = sunxi_spi_read(sspi, reg);
+
+	sunxi_spi_write(sspi, reg, orig & ~value);
+}
+
 static inline void sunxi_spi_drain_fifo(struct sunxi_spi *sspi, int len)
 {
 	u32 reg, cnt;
@@ -243,6 +291,15 @@ static inline void sunxi_spi_fill_fifo(struct sunxi_spi *sspi, int len)
 	}
 }
 
+static bool sunxi_spi_can_dma(struct spi_master *master,
+			      struct spi_device *spi,
+			      struct spi_transfer *tfr)
+{
+	struct sunxi_spi *sspi = spi_master_get_devdata(master);
+
+	return tfr->len >= sspi->fifo_depth;
+}
+
 static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)
 {
 	struct sunxi_spi *sspi = spi_master_get_devdata(spi->master);
@@ -284,6 +341,8 @@ static size_t sunxi_spi_max_transfer_size(struct spi_device *spi)
 	struct spi_master *master = spi->master;
 	struct sunxi_spi *sspi = spi_master_get_devdata(master);
 
+	if (master->can_dma)
+		return SUNXI_CNT_MASK;
 	return sspi->fifo_depth - 1;
 }
 
@@ -292,22 +351,27 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 				  struct spi_transfer *tfr)
 {
 	struct sunxi_spi *sspi = spi_master_get_devdata(master);
+	struct dma_async_tx_descriptor *desc_tx = NULL, *desc_rx = NULL;
 	unsigned int mclk_rate, div, timeout;
 	unsigned int start, end, tx_time;
 	unsigned int tx_len = 0;
 	int ret = 0;
-	u32 reg;
+	u32 reg, trigger = 0;
+
+	if (!master->can_dma) {
+		/* We don't support transfer larger than the FIFO */
+		if (tfr->len > sspi->fifo_depth)
+			return -EMSGSIZE;
+		/*
+		 * Filling the FIFO fully causes timeout for some reason
+		 * at least on spi2 on A10s
+		 */
+		if ((sspi->type == SPI_SUN4I) &&
+		    tfr->tx_buf && tfr->len >= sspi->fifo_depth)
+			return -EMSGSIZE;
+	}
 
-	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > sspi->fifo_depth)
-		return -EMSGSIZE;
-
-	/*
-	 * Filling the FIFO fully causes timeout for some reason
-	 * at least on spi2 on A10s
-	 */
-	if ((sspi->type == SPI_SUN4I) &&
-	    tfr->tx_buf && tfr->len >= sspi->fifo_depth)
+	if (tfr->len > SUNXI_CNT_MASK)
 		return -EMSGSIZE;
 
 	reinit_completion(&sspi->done);
@@ -405,17 +469,81 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 		sunxi_spi_write(sspi, SUNXI_BURST_CTL_CNT_REG,
 				SUNXI_BURST_CTL_CNT_STC(tx_len));
 
-	/* Fill the TX FIFO */
-	sunxi_spi_fill_fifo(sspi, sspi->fifo_depth);
+	/* Setup transfer buffers */
+	if (sunxi_spi_can_dma(master, spi, tfr)) {
+		dev_dbg(&sspi->master->dev, "Using DMA mode for transfer\n");
+
+		if (sspi->tx_buf) {
+			desc_tx = dmaengine_prep_slave_sg(master->dma_tx,
+					tfr->tx_sg.sgl, tfr->tx_sg.nents,
+					DMA_TO_DEVICE,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+			if (!desc_tx) {
+				dev_err(&sspi->master->dev,
+					"Couldn't prepare dma slave\n");
+				ret = -EIO;
+				goto out;
+			}
+
+			if (sspi->type == SPI_SUN4I)
+				trigger |= sspi_bits(sspi, SUNXI_CTL_DMA_TF_NOT_FULL);
+			else
+				trigger |= sspi_bits(sspi, SUNXI_CTL_DMA_TF_HALF);
+
+			dmaengine_submit(desc_tx);
+			dma_async_issue_pending(master->dma_tx);
+		}
+
+		if (sspi->rx_buf) {
+			desc_rx = dmaengine_prep_slave_sg(master->dma_rx,
+					tfr->rx_sg.sgl, tfr->rx_sg.nents,
+					DMA_FROM_DEVICE,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+			if (!desc_rx) {
+				dev_err(&sspi->master->dev,
+					"Couldn't prepare dma slave\n");
+				ret = -EIO;
+				goto out;
+			}
+
+			trigger |= sspi_bits(sspi, SUNXI_CTL_DMA_RF_READY);
+
+			dmaengine_submit(desc_rx);
+			dma_async_issue_pending(master->dma_rx);
+		}
+
+		/* Enable Dedicated DMA requests */
+		if (sspi->type == SPI_SUN4I) {
+			sunxi_spi_set(sspi, SUNXI_TFR_CTL_REG,
+				      sspi_bits(sspi, SUNXI_CTL_DMA_DEDICATED));
+			sunxi_spi_write(sspi, SUNXI_DMA_CTL_REG, trigger);
+		} else {
+			trigger |= sspi_bits(sspi, SUNXI_CTL_DMA_DEDICATED);
+			sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, trigger);
+		}
+	} else {
+		dev_dbg(&sspi->master->dev, "Using PIO mode for transfer\n");
+
+		/* Disable DMA requests */
+		if (sspi->type == SPI_SUN4I) {
+			sunxi_spi_unset(sspi, SUNXI_TFR_CTL_REG,
+					sspi_bits(sspi, SUNXI_CTL_DMA_DEDICATED));
+			sunxi_spi_write(sspi, SUNXI_DMA_CTL_REG, 0);
+		} else {
+			sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, 0);
+		}
+
+		/* Fill the TX FIFO */
+		sunxi_spi_fill_fifo(sspi, sspi->fifo_depth);
+	}
 
 	/* Enable the interrupts */
 	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG,
 			sspi_bits(sspi, SUNXI_INT_CTL_TC));
 
 	/* Start the transfer */
-	reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
-	sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
-			reg | sspi_bits(sspi, SUNXI_TFR_CTL_XCH));
+	sunxi_spi_set(sspi, SUNXI_TFR_CTL_REG,
+			    sspi_bits(sspi, SUNXI_TFR_CTL_XCH));
 
 	tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
 	start = jiffies;
@@ -431,9 +559,23 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
+out:
+	if (ret < 0 && sunxi_spi_can_dma(master, spi, tfr)) {
+		dev_dbg(&master->dev, "DMA channel teardown");
+		if (sspi->tx_buf)
+			dmaengine_terminate_sync(master->dma_tx);
+		if (sspi->rx_buf)
+			dmaengine_terminate_sync(master->dma_rx);
+	}
+
+	/*
+	 * By this time either the transfer has ended and we have data in the
+	 * FIFO buffer from a PIO RX transfer or the buffer is empty
+	 * or something has failed.
+	 * Empty the buffer either way to avoid leaving garbage around.
+	 */
 	sunxi_spi_drain_fifo(sspi, sspi->fifo_depth);
 
-out:
 	sunxi_spi_write(sspi, SUNXI_INT_CTL_REG, 0);
 
 	return ret;
@@ -515,6 +657,7 @@ static int sunxi_spi_runtime_suspend(struct device *dev)
 
 static int sunxi_spi_probe(struct platform_device *pdev)
 {
+	struct dma_slave_config dma_sconfig;
 	struct spi_master *master;
 	struct sunxi_spi *sspi;
 	struct resource	*res;
@@ -625,6 +768,54 @@ static int sunxi_spi_probe(struct platform_device *pdev)
 		}
 	}
 
+	master->dma_tx = dma_request_slave_channel_reason(&pdev->dev, "tx");
+	if (IS_ERR(master->dma_tx)) {
+		dev_err(&pdev->dev, "Unable to acquire DMA channel TX\n");
+		ret = PTR_ERR(master->dma_tx);
+		goto err_dma_chan;
+	}
+
+	dma_sconfig.direction = DMA_MEM_TO_DEV;
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_addr = res->start + sspi_reg(sspi, SUNXI_TXDATA_REG);
+	dma_sconfig.src_maxburst = 1;
+	dma_sconfig.dst_maxburst = 1;
+
+	ret = dmaengine_slave_config(master->dma_tx, &dma_sconfig);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to configure TX DMA slave\n");
+		goto err_tx_dma_release;
+	}
+
+	master->dma_rx = dma_request_slave_channel_reason(&pdev->dev, "rx");
+	if (IS_ERR(master->dma_rx)) {
+		dev_err(&pdev->dev, "Unable to acquire DMA channel RX\n");
+		ret = PTR_ERR(master->dma_rx);
+		goto err_tx_dma_release;
+	}
+
+	dma_sconfig.direction = DMA_DEV_TO_MEM;
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.src_addr = res->start + sspi_reg(sspi, SUNXI_RXDATA_REG);
+	dma_sconfig.src_maxburst = 1;
+	dma_sconfig.dst_maxburst = 1;
+
+	ret = dmaengine_slave_config(master->dma_rx, &dma_sconfig);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to configure RX DMA slave\n");
+		goto err_rx_dma_release;
+	}
+
+	/*
+	 * This is a bit dodgy. If you set can_dma then map_msg in spi.c
+	 * apparently dereferences your dma channels if non-NULL even if your
+	 * can_dma never returns true (and crashes if the channel is an error
+	 * pointer). So just don't set can_dma unless both channels are valid.
+	 */
+	master->can_dma = sunxi_spi_can_dma;
+wakeup:
 	/*
 	 * This wake-up/shutdown pattern is to be able to have the
 	 * device woken up, even if runtime_pm is disabled
@@ -665,18 +856,40 @@ static int sunxi_spi_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_rx_dma_release:
+	dma_release_channel(master->dma_rx);
+err_tx_dma_release:
+	dma_release_channel(master->dma_tx);
+err_dma_chan:
+	master->dma_tx = NULL;
+	master->dma_rx = NULL;
+	if ((ret == -EPROBE_DEFER) && wait_for_dma)
+		goto err_free_master;
+	goto wakeup;
+
 err_pm_disable:
 	pm_runtime_disable(&pdev->dev);
 	sunxi_spi_runtime_suspend(&pdev->dev);
 err_free_master:
+	if (master->can_dma) {
+		dma_release_channel(master->dma_rx);
+		dma_release_channel(master->dma_tx);
+	}
 	spi_master_put(master);
 	return ret;
 }
 
 static int sunxi_spi_remove(struct platform_device *pdev)
 {
+	struct spi_master *master = platform_get_drvdata(pdev);
+
 	pm_runtime_disable(&pdev->dev);
 
+	if (master->can_dma) {
+		dma_release_channel(master->dma_rx);
+		dma_release_channel(master->dma_tx);
+	}
+
 	return 0;
 }
 
-- 
2.8.1

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 12/13] spi: sunxi: remove CONFIG_SPI_SUN6I

[Michal Suchanek]

This is no longer used.

Signed-off-by: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/configs/multi_v7_defconfig | 1 -
 arch/arm/configs/sunxi_defconfig    | 1 -
 drivers/spi/Kconfig                 | 6 ------
 3 files changed, 8 deletions(-)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 2823490..3196f04 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -362,7 +362,6 @@ CONFIG_SPI_SH_MSIOF=m
 CONFIG_SPI_SH_HSPI=y
 CONFIG_SPI_SIRF=y
 CONFIG_SPI_SUN4I=y
-CONFIG_SPI_SUN6I=y
 CONFIG_SPI_TEGRA114=y
 CONFIG_SPI_TEGRA20_SFLASH=y
 CONFIG_SPI_TEGRA20_SLINK=y
diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 81a1b92..6d0c650 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -78,7 +78,6 @@ CONFIG_I2C_MV64XXX=y
 CONFIG_I2C_SUN6I_P2WI=y
 CONFIG_SPI=y
 CONFIG_SPI_SUN4I=y
-CONFIG_SPI_SUN6I=y
 CONFIG_GPIO_SYSFS=y
 CONFIG_POWER_SUPPLY=y
 CONFIG_AXP20X_POWER=y
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 1b5045b..52e588f 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -581,12 +581,6 @@ config SPI_SUN4I
 	help
 	  SPI driver for Allwinner sun4i, sun5i and sun7i SoCs
 
-config SPI_SUN6I
-	tristate "Allwinner A31 SPI controller (deprecated compatibility option)"
-	depends on ARCH_SUNXI || COMPILE_TEST
-	depends on RESET_CONTROLLER
-	select SPI_SUN4I
-
 config SPI_MXS
 	tristate "Freescale MXS SPI controller"
 	depends on ARCH_MXS
-- 
2.8.1

Re: [PATCH v3 00/13] sunxi spi fixes

[Maxime Ripard]

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

On Mon, Jun 13, 2016 at 05:46:48PM -0000, Michal Suchanek wrote:
> Hello,
> 
> This is update of the sunxi spi patches that should give full-featured SPI
> driver.
> 
> First three patches fix issues with the current driver and can be of use for
> stable kernels so adding cc for those.
> 
> I merged the sun4i and sun6i driver because there several issues that need to
> be fixed in both separately and they are even out of sync wrt some fixes.
> I guess some of the merge patches can be squashed.
> 
> I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
> tell if that side was broken by this patchset - especially the last patch that
> adds DMA was afaik never tested on sun6i.

So, you didn't run that code through checkpatch and you rewrite the
whole thing entirely without even testing it... Awesome.

For the record, I'm still very much opposed to such a merge.

The first fixes are very welcome though, can and should go in.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

Re: [PATCH v3 00/13] sunxi spi fixes

[Michal Suchanek]

Hello,

On 13 June 2016 at 21:57, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Mon, Jun 13, 2016 at 05:46:48PM -0000, Michal Suchanek wrote:
>> Hello,
>>
>> This is update of the sunxi spi patches that should give full-featured SPI
>> driver.
>>
>> First three patches fix issues with the current driver and can be of use for
>> stable kernels so adding cc for those.
>>
>> I merged the sun4i and sun6i driver because there several issues that need to
>> be fixed in both separately and they are even out of sync wrt some fixes.
>> I guess some of the merge patches can be squashed.
>>
>> I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
>> tell if that side was broken by this patchset - especially the last patch that
>> adds DMA was afaik never tested on sun6i.
>
> So, you didn't run that code through checkpatch and you rewrite the
> whole thing entirely without even testing it... Awesome.

Aside from the DMA part this is not a rewrite.

And I did run the code through checkpatch. It still gives some
warnings about BUG_ON and overly long lines, sure. II don't think
those are that serious or that fixing them would improve the code.

Thanks

Michal

Re: [PATCH v3 00/13] sunxi spi fixes

[Michal Suchanek]

Hello,

On 13 June 2016 at 21:57, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Mon, Jun 13, 2016 at 05:46:48PM -0000, Michal Suchanek wrote:
>> Hello,
>>
>> This is update of the sunxi spi patches that should give full-featured SPI
>> driver.
>>
>> First three patches fix issues with the current driver and can be of use for
>> stable kernels so adding cc for those.
>>
>> I merged the sun4i and sun6i driver because there several issues that need to
>> be fixed in both separately and they are even out of sync wrt some fixes.
>> I guess some of the merge patches can be squashed.
>>
>> I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
>> tell if that side was broken by this patchset - especially the last patch that
>> adds DMA was afaik never tested on sun6i.
>

>
> For the record, I'm still very much opposed to such a merge.
>

What is the reason against the merge? I did not find the original discussion.

I tried to rename everything in the drivers from sun4i and sun6i to
sunxi to look at a clean diff and found about 5 differences 2 of which
look like a bug.

Thanks

Michal

Re: [PATCH v3 00/13] sunxi spi fixes

[Maxime Ripard]

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

On Fri, Jun 17, 2016 at 12:34:44PM +0200, Michal Suchanek wrote:
> Hello,
> 
> On 13 June 2016 at 21:57, Maxime Ripard
> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > On Mon, Jun 13, 2016 at 05:46:48PM -0000, Michal Suchanek wrote:
> >> Hello,
> >>
> >> This is update of the sunxi spi patches that should give full-featured SPI
> >> driver.
> >>
> >> First three patches fix issues with the current driver and can be of use for
> >> stable kernels so adding cc for those.
> >>
> >> I merged the sun4i and sun6i driver because there several issues that need to
> >> be fixed in both separately and they are even out of sync wrt some fixes.
> >> I guess some of the merge patches can be squashed.
> >>
> >> I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
> >> tell if that side was broken by this patchset - especially the last patch that
> >> adds DMA was afaik never tested on sun6i.
> >
> >
> > For the record, I'm still very much opposed to such a merge.
> 
> What is the reason against the merge? I did not find the original
> discussion.

I really prefer some code that is concise and clear but a little
duplicated over some code that shares every possible lines of code but
is a giant mess impossible to understand.

I just came across the reg_field stuff in regmap that would allow to
partially address that problem though, there's still the bit indices
issue to overcome though.

> I tried to rename everything in the drivers from sun4i and sun6i to
> sunxi to look at a clean diff and found about 5 differences 2 of which
> look like a bug.

It's hard to tell without testing.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v3 00/13] sunxi spi fixes

[Michal Suchanek]

Hello,

On 25 July 2016 at 09:32, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Fri, Jun 17, 2016 at 12:34:44PM +0200, Michal Suchanek wrote:
>> Hello,
>>
>> On 13 June 2016 at 21:57, Maxime Ripard
>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> > On Mon, Jun 13, 2016 at 05:46:48PM -0000, Michal Suchanek wrote:
>> >> Hello,
>> >>
>> >> This is update of the sunxi spi patches that should give full-featured SPI
>> >> driver.
>> >>
>> >> First three patches fix issues with the current driver and can be of use for
>> >> stable kernels so adding cc for those.
>> >>
>> >> I merged the sun4i and sun6i driver because there several issues that need to
>> >> be fixed in both separately and they are even out of sync wrt some fixes.
>> >> I guess some of the merge patches can be squashed.
>> >>
>> >> I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
>> >> tell if that side was broken by this patchset - especially the last patch that
>> >> adds DMA was afaik never tested on sun6i.
>> >
>> >
>> > For the record, I'm still very much opposed to such a merge.
>>
>> What is the reason against the merge? I did not find the original
>> discussion.
>
> I really prefer some code that is concise and clear but a little
> duplicated over some code that shares every possible lines of code but
> is a giant mess impossible to understand.

Yes, it's been tried. In the case of this driver there is more duplication
than differences. Also bitrot due to different variants receiving different
updates and fixes crept in already.

Adding the remap layer certainly does not make the driver easier to
understand but it's not becoming giant mess either.

>
> I just came across the reg_field stuff in regmap that would allow to
> partially address that problem though, there's still the bit indices
> issue to overcome though.
>
>> I tried to rename everything in the drivers from sun4i and sun6i to
>> sunxi to look at a clean diff and found about 5 differences 2 of which
>> look like a bug.
>
> It's hard to tell without testing.

I tested the merged (and separate) driver on a H3 board and found a
type bug in the remap layer.

I also found I can transfer at most 68 bytes without DMA although
the manuals claim 128 byte FIFO. DMA transfers weren't very reliable
which may or may not be a wiring issue. I have seen similar issues
intermittently with SPI on A10s.

Thanks

Michal

Re: [PATCH v3 00/13] sunxi spi fixes

[Maxime Ripard]

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

On Mon, Jul 25, 2016 at 10:03:14AM +0200, Michal Suchanek wrote:
> Hello,
> 
> On 25 July 2016 at 09:32, Maxime Ripard
> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > On Fri, Jun 17, 2016 at 12:34:44PM +0200, Michal Suchanek wrote:
> >> Hello,
> >>
> >> On 13 June 2016 at 21:57, Maxime Ripard
> >> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> >> > On Mon, Jun 13, 2016 at 05:46:48PM -0000, Michal Suchanek wrote:
> >> >> Hello,
> >> >>
> >> >> This is update of the sunxi spi patches that should give full-featured SPI
> >> >> driver.
> >> >>
> >> >> First three patches fix issues with the current driver and can be of use for
> >> >> stable kernels so adding cc for those.
> >> >>
> >> >> I merged the sun4i and sun6i driver because there several issues that need to
> >> >> be fixed in both separately and they are even out of sync wrt some fixes.
> >> >> I guess some of the merge patches can be squashed.
> >> >>
> >> >> I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
> >> >> tell if that side was broken by this patchset - especially the last patch that
> >> >> adds DMA was afaik never tested on sun6i.
> >> >
> >> >
> >> > For the record, I'm still very much opposed to such a merge.
> >>
> >> What is the reason against the merge? I did not find the original
> >> discussion.
> >
> > I really prefer some code that is concise and clear but a little
> > duplicated over some code that shares every possible lines of code but
> > is a giant mess impossible to understand.
> 
> Yes, it's been tried. In the case of this driver there is more duplication
> than differences. Also bitrot due to different variants receiving different
> updates and fixes crept in already.
> 
> Adding the remap layer certainly does not make the driver easier to
> understand but it's not becoming giant mess either.

Well, when you say that you're not quite fond of using your structure
all over the place because "it makes your eyes bleed", I wouldn't call
that a good sales pitch.

But again, reg_field seems like a good solution for that.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH v3 00/13] sunxi spi fixes

[Michal Suchanek]

On 29 July 2016 at 22:22, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Mon, Jul 25, 2016 at 10:03:14AM +0200, Michal Suchanek wrote:
>> Hello,
>>
>> On 25 July 2016 at 09:32, Maxime Ripard
>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> > On Fri, Jun 17, 2016 at 12:34:44PM +0200, Michal Suchanek wrote:
>> >> Hello,
>> >>
>> >> On 13 June 2016 at 21:57, Maxime Ripard
>> >> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> >> > On Mon, Jun 13, 2016 at 05:46:48PM -0000, Michal Suchanek wrote:
>> >> >> Hello,
>> >> >>
>> >> >> This is update of the sunxi spi patches that should give full-featured SPI
>> >> >> driver.
>> >> >>
>> >> >> First three patches fix issues with the current driver and can be of use for
>> >> >> stable kernels so adding cc for those.
>> >> >>
>> >> >> I merged the sun4i and sun6i driver because there several issues that need to
>> >> >> be fixed in both separately and they are even out of sync wrt some fixes.
>> >> >> I guess some of the merge patches can be squashed.
>> >> >>
>> >> >> I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot
>> >> >> tell if that side was broken by this patchset - especially the last patch that
>> >> >> adds DMA was afaik never tested on sun6i.
>> >> >
>> >> >
>> >> > For the record, I'm still very much opposed to such a merge.
>> >>
>> >> What is the reason against the merge? I did not find the original
>> >> discussion.
>> >
>> > I really prefer some code that is concise and clear but a little
>> > duplicated over some code that shares every possible lines of code but
>> > is a giant mess impossible to understand.
>>
>> Yes, it's been tried. In the case of this driver there is more duplication
>> than differences. Also bitrot due to different variants receiving different
>> updates and fixes crept in already.
>>
>> Adding the remap layer certainly does not make the driver easier to
>> understand but it's not becoming giant mess either.
>
> Well, when you say that you're not quite fond of using your structure
> all over the place because "it makes your eyes bleed", I wouldn't call
> that a good sales pitch.

That's why put dereferencing it inside the read function. It reduces the noise
throughout the code and make it easier to add checks or change the remap
implementation.

>
> But again, reg_field seems like a good solution for that.

I will look at reg_field.

I saw some drivers using remap layers that are way more heavyweight than
looking up an integer in a table so I wanted to avoid that.

Thanks

Michal