[PATCH 00/18] spi: stm32: various driver enhancements

[PATCH 00/18] spi: stm32: various driver enhancements

[Alain Volmat]

This serie provides spi-stm32 driver enhancements in various area such as:
  - code simplification
  - race condition fix
  - fixes in case of usage of SPI with DMA
  - suspend/resume fixes
  - issues triggered by spi-loopback-test

Alain Volmat (4):
  spi: stm32-spi: defer probe for reset
  spi: stm32: always perform registers configuration prior to transfer
  spi: stm32: properly handle 0 byte transfer
  spi: stm32h7: ensure message are smaller than max size

Amelie Delaunay (10):
  spi: stm32: use bitfield macros
  spi: stm32h7: replace private SPI_1HZ_NS with NSEC_PER_SEC
  spi: stm32h7: fix irq handler
  spi: stm32h7: rework rx fifo read function
  spi: stm32h7: fix dbg/warn/err conditions in irq handler
  spi: stm32: wait for completion in transfer_one()
  spi: stm32: fix fifo threshold level in case of short transfer
  spi: stm32h7: fix handling of dma transfer completed
  spi: stm32: improve suspend/resume management
  spi: stm32: fix stm32_spi_prepare_mbr in case of odd clk_rate

Antonio Borneo (3):
  spi: stm32h7: remove unused mode fault MODF event handling
  spi: stm32h7: fix race condition at end of transfer
  spi: stm32: move spi disable out of irq handler

Etienne Carriere (1):
  spi: stm32-spi: driver uses reset controller only at init

 drivers/spi/spi-stm32.c | 425 ++++++++++++++++++++++++++++--------------------
 1 file changed, 247 insertions(+), 178 deletions(-)

[PATCH 01/18] spi: stm32-spi: driver uses reset controller only at init

[Alain Volmat]

From: Etienne Carriere <etienne.carriere@st.com>

Remove reset controller device reference from the device private
structure since it is used only at probe time and can be discarded
once used to reset the SPI device.

Signed-off-by: Etienne Carriere <etienne.carriere@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 4c643dfc7fbb..838d3ce3ebae 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -267,7 +267,6 @@ struct stm32_spi_cfg {
  * @base: virtual memory area
  * @clk: hw kernel clock feeding the SPI clock generator
  * @clk_rate: rate of the hw kernel clock feeding the SPI clock generator
- * @rst: SPI controller reset line
  * @lock: prevent I/O concurrent access
  * @irq: SPI controller interrupt line
  * @fifo_size: size of the embedded fifo in bytes
@@ -293,7 +292,6 @@ struct stm32_spi {
 	void __iomem *base;
 	struct clk *clk;
 	u32 clk_rate;
-	struct reset_control *rst;
 	spinlock_t lock; /* prevent I/O concurrent access */
 	int irq;
 	unsigned int fifo_size;
@@ -1824,6 +1822,7 @@ static int stm32_spi_probe(struct platform_device *pdev)
 	struct spi_master *master;
 	struct stm32_spi *spi;
 	struct resource *res;
+	struct reset_control *rst;
 	int ret;
 
 	master = spi_alloc_master(&pdev->dev, sizeof(struct stm32_spi));
@@ -1887,11 +1886,11 @@ static int stm32_spi_probe(struct platform_device *pdev)
 		goto err_clk_disable;
 	}
 
-	spi->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
-	if (!IS_ERR(spi->rst)) {
-		reset_control_assert(spi->rst);
+	rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (!IS_ERR(rst)) {
+		reset_control_assert(rst);
 		udelay(2);
-		reset_control_deassert(spi->rst);
+		reset_control_deassert(rst);
 	}
 
 	if (spi->cfg->has_fifo)
-- 
2.7.4

[PATCH 02/18] spi: stm32-spi: defer probe for reset

[Alain Volmat]

Defer the probe operation when a reset controller device is expected
but have not yet been probed.

This change replaces use of devm_reset_control_get_exclusive() with
devm_reset_control_get_optional_exclusive() as reset controller is
optional which is now explicitly stated.

Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 838d3ce3ebae..eaa416c551c9 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -1886,8 +1886,16 @@ static int stm32_spi_probe(struct platform_device *pdev)
 		goto err_clk_disable;
 	}
 
-	rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
-	if (!IS_ERR(rst)) {
+	rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
+	if (rst) {
+		if (IS_ERR(rst)) {
+			ret = PTR_ERR(rst);
+			if (ret != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "reset get failed: %d\n",
+					ret);
+			goto err_clk_disable;
+		}
+
 		reset_control_assert(rst);
 		udelay(2);
 		reset_control_deassert(rst);
-- 
2.7.4

Re: [PATCH 02/18] spi: stm32-spi: defer probe for reset

[Mark Brown]

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

On Wed, Aug 05, 2020 at 09:01:57AM +0200, Alain Volmat wrote:

> -	rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> -	if (!IS_ERR(rst)) {
> +	rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> +	if (rst) {
> +		if (IS_ERR(rst)) {
> +			ret = PTR_ERR(rst);
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(&pdev->dev, "reset get failed: %d\n",
> +					ret);
> +			goto err_clk_disable;
> +		}

This will not provide any diagnostics when deferring which isn't very
helpful if there's issues.

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

Re: [PATCH 02/18] spi: stm32-spi: defer probe for reset

[Alain Volmat]

On Wed, Aug 05, 2020 at 11:49:06AM +0100, Mark Brown wrote:
> On Wed, Aug 05, 2020 at 09:01:57AM +0200, Alain Volmat wrote:
> 
> > -	rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> > -	if (!IS_ERR(rst)) {
> > +	rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> > +	if (rst) {
> > +		if (IS_ERR(rst)) {
> > +			ret = PTR_ERR(rst);
> > +			if (ret != -EPROBE_DEFER)
> > +				dev_err(&pdev->dev, "reset get failed: %d\n",
> > +					ret);
> > +			goto err_clk_disable;
> > +		}
> 
> This will not provide any diagnostics when deferring which isn't very
> helpful if there's issues.

Do you mean that a message when deferring would be needed ?

I am worrying that this would lead to having too much noise during boot
since probe deferring is kinda common. Of course it can also be due to a bad
configuration of the kernel as well but having looked around I think that
usually driver are rather silent in case of deferring.

Re: [PATCH 02/18] spi: stm32-spi: defer probe for reset

[Mark Brown]

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

On Fri, Aug 07, 2020 at 03:42:54PM +0200, Alain Volmat wrote:
> On Wed, Aug 05, 2020 at 11:49:06AM +0100, Mark Brown wrote:
> > On Wed, Aug 05, 2020 at 09:01:57AM +0200, Alain Volmat wrote:

> > > -	rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> > > -	if (!IS_ERR(rst)) {
> > > +	rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
> > > +	if (rst) {
> > > +		if (IS_ERR(rst)) {
> > > +			ret = PTR_ERR(rst);
> > > +			if (ret != -EPROBE_DEFER)
> > > +				dev_err(&pdev->dev, "reset get failed: %d\n",
> > > +					ret);
> > > +			goto err_clk_disable;
> > > +		}

> > This will not provide any diagnostics when deferring which isn't very
> > helpful if there's issues.

> Do you mean that a message when deferring would be needed ?

Yes, if for examaple the reset driver isn't being built then the driver
will defer for ever waiting for it to instantiate and the user will have
to have some method for figuring out what it's waiting for.

> I am worrying that this would lead to having too much noise during boot
> since probe deferring is kinda common. Of course it can also be due to a bad
> configuration of the kernel as well but having looked around I think that
> usually driver are rather silent in case of deferring.

This is not something that should be open coded in your driver.

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

[PATCH 03/18] spi: stm32h7: remove unused mode fault MODF event handling

[Alain Volmat]

From: Antonio Borneo <antonio.borneo@st.com>

Accordingly to STM32H7 document RM0433, "mode fault" MODF is
a special mode to handle a spi bus with multiple masters, in
which each master has to "detect" if another master enables
its CS to take control of the bus. Once this is detected,
all other masters has to temporarily switch to "slave" mode.

Such multi-master mode is not supported in Linux and this
driver properly disables the mode by setting the bits
SPI_CR1_SSI and SPI_CFG2_SSM, thus forcing a master only
operating mode.

In this condition, we will never receive an interrupt due to
MODF event and we do not need to handle it.

Remove all the unused code around handling MODF events.

Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index eaa416c551c9..df22dea784d9 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -129,7 +129,6 @@
 #define STM32H7_SPI_IER_EOTIE		BIT(3)
 #define STM32H7_SPI_IER_TXTFIE		BIT(4)
 #define STM32H7_SPI_IER_OVRIE		BIT(6)
-#define STM32H7_SPI_IER_MODFIE		BIT(9)
 #define STM32H7_SPI_IER_ALL		GENMASK(10, 0)
 
 /* STM32H7_SPI_SR bit fields */
@@ -137,7 +136,6 @@
 #define STM32H7_SPI_SR_TXP		BIT(1)
 #define STM32H7_SPI_SR_EOT		BIT(3)
 #define STM32H7_SPI_SR_OVR		BIT(6)
-#define STM32H7_SPI_SR_MODF		BIT(9)
 #define STM32H7_SPI_SR_SUSP		BIT(11)
 #define STM32H7_SPI_SR_RXPLVL_SHIFT	13
 #define STM32H7_SPI_SR_RXPLVL		GENMASK(14, 13)
@@ -933,11 +931,6 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 			end = true;
 	}
 
-	if (sr & STM32H7_SPI_SR_MODF) {
-		dev_warn(spi->dev, "Mode fault: transfer aborted\n");
-		end = true;
-	}
-
 	if (sr & STM32H7_SPI_SR_OVR) {
 		dev_warn(spi->dev, "Overrun: received value discarded\n");
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
@@ -1201,7 +1194,7 @@ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi)
 
 	/* Enable the interrupts relative to the end of transfer */
 	ier |= STM32H7_SPI_IER_EOTIE | STM32H7_SPI_IER_TXTFIE |
-	       STM32H7_SPI_IER_OVRIE | STM32H7_SPI_IER_MODFIE;
+	       STM32H7_SPI_IER_OVRIE;
 
 	spin_lock_irqsave(&spi->lock, flags);
 
@@ -1251,8 +1244,7 @@ static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)
 	/* Enable the interrupts relative to the end of transfer */
 	stm32_spi_set_bits(spi, STM32H7_SPI_IER, STM32H7_SPI_IER_EOTIE |
 						 STM32H7_SPI_IER_TXTFIE |
-						 STM32H7_SPI_IER_OVRIE |
-						 STM32H7_SPI_IER_MODFIE);
+						 STM32H7_SPI_IER_OVRIE);
 
 	stm32_spi_enable(spi);
 
-- 
2.7.4

Re: [PATCH 03/18] spi: stm32h7: remove unused mode fault MODF event handling

[Mark Brown]

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

On Wed, Aug 05, 2020 at 09:01:58AM +0200, Alain Volmat wrote:
> From: Antonio Borneo <antonio.borneo@st.com>

> Accordingly to STM32H7 document RM0433, "mode fault" MODF is
> a special mode to handle a spi bus with multiple masters, in
> which each master has to "detect" if another master enables
> its CS to take control of the bus. Once this is detected,
> all other masters has to temporarily switch to "slave" mode.

> Such multi-master mode is not supported in Linux and this
> driver properly disables the mode by setting the bits
> SPI_CR1_SSI and SPI_CFG2_SSM, thus forcing a master only
> operating mode.

> In this condition, we will never receive an interrupt due to
> MODF event and we do not need to handle it.

> Remove all the unused code around handling MODF events.

What does having this cost us?  Hopefully it's not doing much but if
something goes wrong with the hardware and this does get flagged up for
some reason it'd be good to know about it.  It doesn't seem to be having
a big impact on the code or anything.

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

[PATCH 04/18] spi: stm32: use bitfield macros

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

To avoid defining shift and mask separately and hand-coding the bit
manipulation, use the bitfield macros.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 55 ++++++++++++++++---------------------------------
 1 file changed, 18 insertions(+), 37 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index df22dea784d9..a5b926a5c4d9 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -5,6 +5,7 @@
 // Copyright (C) 2017, STMicroelectronics - All Rights Reserved
 // Author(s): Amelie Delaunay <amelie.delaunay@st.com> for STMicroelectronics.
 
+#include <linux/bitfield.h>
 #include <linux/debugfs.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -30,8 +31,8 @@
 #define STM32F4_SPI_CR1_CPHA		BIT(0)
 #define STM32F4_SPI_CR1_CPOL		BIT(1)
 #define STM32F4_SPI_CR1_MSTR		BIT(2)
-#define STM32F4_SPI_CR1_BR_SHIFT	3
 #define STM32F4_SPI_CR1_BR		GENMASK(5, 3)
+#define STM32F4_SPI_CR1_BR_SHIFT	3
 #define STM32F4_SPI_CR1_SPE		BIT(6)
 #define STM32F4_SPI_CR1_LSBFRST		BIT(7)
 #define STM32F4_SPI_CR1_SSI		BIT(8)
@@ -93,27 +94,22 @@
 #define STM32H7_SPI_CR1_SSI		BIT(12)
 
 /* STM32H7_SPI_CR2 bit fields */
-#define STM32H7_SPI_CR2_TSIZE_SHIFT	0
 #define STM32H7_SPI_CR2_TSIZE		GENMASK(15, 0)
+#define STM32H7_SPI_TSIZE_MAX		GENMASK(15, 0)
 
 /* STM32H7_SPI_CFG1 bit fields */
-#define STM32H7_SPI_CFG1_DSIZE_SHIFT	0
 #define STM32H7_SPI_CFG1_DSIZE		GENMASK(4, 0)
-#define STM32H7_SPI_CFG1_FTHLV_SHIFT	5
 #define STM32H7_SPI_CFG1_FTHLV		GENMASK(8, 5)
 #define STM32H7_SPI_CFG1_RXDMAEN	BIT(14)
 #define STM32H7_SPI_CFG1_TXDMAEN	BIT(15)
-#define STM32H7_SPI_CFG1_MBR_SHIFT	28
 #define STM32H7_SPI_CFG1_MBR		GENMASK(30, 28)
+#define STM32H7_SPI_CFG1_MBR_SHIFT	28
 #define STM32H7_SPI_CFG1_MBR_MIN	0
 #define STM32H7_SPI_CFG1_MBR_MAX	(GENMASK(30, 28) >> 28)
 
 /* STM32H7_SPI_CFG2 bit fields */
-#define STM32H7_SPI_CFG2_MIDI_SHIFT	4
 #define STM32H7_SPI_CFG2_MIDI		GENMASK(7, 4)
-#define STM32H7_SPI_CFG2_COMM_SHIFT	17
 #define STM32H7_SPI_CFG2_COMM		GENMASK(18, 17)
-#define STM32H7_SPI_CFG2_SP_SHIFT	19
 #define STM32H7_SPI_CFG2_SP		GENMASK(21, 19)
 #define STM32H7_SPI_CFG2_MASTER		BIT(22)
 #define STM32H7_SPI_CFG2_LSBFRST	BIT(23)
@@ -137,7 +133,6 @@
 #define STM32H7_SPI_SR_EOT		BIT(3)
 #define STM32H7_SPI_SR_OVR		BIT(6)
 #define STM32H7_SPI_SR_SUSP		BIT(11)
-#define STM32H7_SPI_SR_RXPLVL_SHIFT	13
 #define STM32H7_SPI_SR_RXPLVL		GENMASK(14, 13)
 #define STM32H7_SPI_SR_RXWNE		BIT(15)
 
@@ -412,9 +407,7 @@ static int stm32h7_spi_get_bpw_mask(struct stm32_spi *spi)
 	stm32_spi_set_bits(spi, STM32H7_SPI_CFG1, STM32H7_SPI_CFG1_DSIZE);
 
 	cfg1 = readl_relaxed(spi->base + STM32H7_SPI_CFG1);
-	max_bpw = (cfg1 & STM32H7_SPI_CFG1_DSIZE) >>
-		  STM32H7_SPI_CFG1_DSIZE_SHIFT;
-	max_bpw += 1;
+	max_bpw = FIELD_GET(STM32H7_SPI_CFG1_DSIZE, cfg1) + 1;
 
 	spin_unlock_irqrestore(&spi->lock, flags);
 
@@ -591,8 +584,7 @@ static void stm32f4_spi_read_rx(struct stm32_spi *spi)
 static void stm32h7_spi_read_rxfifo(struct stm32_spi *spi, bool flush)
 {
 	u32 sr = readl_relaxed(spi->base + STM32H7_SPI_SR);
-	u32 rxplvl = (sr & STM32H7_SPI_SR_RXPLVL) >>
-		     STM32H7_SPI_SR_RXPLVL_SHIFT;
+	u32 rxplvl = FIELD_GET(STM32H7_SPI_SR_RXPLVL, sr);
 
 	while ((spi->rx_len > 0) &&
 	       ((sr & STM32H7_SPI_SR_RXP) ||
@@ -619,8 +611,7 @@ static void stm32h7_spi_read_rxfifo(struct stm32_spi *spi, bool flush)
 		}
 
 		sr = readl_relaxed(spi->base + STM32H7_SPI_SR);
-		rxplvl = (sr & STM32H7_SPI_SR_RXPLVL) >>
-			 STM32H7_SPI_SR_RXPLVL_SHIFT;
+		rxplvl = FIELD_GET(STM32H7_SPI_SR_RXPLVL, sr);
 	}
 
 	dev_dbg(spi->dev, "%s%s: %d bytes left\n", __func__,
@@ -1380,15 +1371,13 @@ static void stm32h7_spi_set_bpw(struct stm32_spi *spi)
 	bpw = spi->cur_bpw - 1;
 
 	cfg1_clrb |= STM32H7_SPI_CFG1_DSIZE;
-	cfg1_setb |= (bpw << STM32H7_SPI_CFG1_DSIZE_SHIFT) &
-		     STM32H7_SPI_CFG1_DSIZE;
+	cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_DSIZE, bpw);
 
 	spi->cur_fthlv = stm32h7_spi_prepare_fthlv(spi);
 	fthlv = spi->cur_fthlv - 1;
 
 	cfg1_clrb |= STM32H7_SPI_CFG1_FTHLV;
-	cfg1_setb |= (fthlv << STM32H7_SPI_CFG1_FTHLV_SHIFT) &
-		     STM32H7_SPI_CFG1_FTHLV;
+	cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_FTHLV, fthlv);
 
 	writel_relaxed(
 		(readl_relaxed(spi->base + STM32H7_SPI_CFG1) &
@@ -1406,8 +1395,7 @@ static void stm32_spi_set_mbr(struct stm32_spi *spi, u32 mbrdiv)
 	u32 clrb = 0, setb = 0;
 
 	clrb |= spi->cfg->regs->br.mask;
-	setb |= ((u32)mbrdiv << spi->cfg->regs->br.shift) &
-		spi->cfg->regs->br.mask;
+	setb |= (mbrdiv << spi->cfg->regs->br.shift) & spi->cfg->regs->br.mask;
 
 	writel_relaxed((readl_relaxed(spi->base + spi->cfg->regs->br.reg) &
 			~clrb) | setb,
@@ -1498,8 +1486,7 @@ static int stm32h7_spi_set_mode(struct stm32_spi *spi, unsigned int comm_type)
 	}
 
 	cfg2_clrb |= STM32H7_SPI_CFG2_COMM;
-	cfg2_setb |= (mode << STM32H7_SPI_CFG2_COMM_SHIFT) &
-		     STM32H7_SPI_CFG2_COMM;
+	cfg2_setb |= FIELD_PREP(STM32H7_SPI_CFG2_COMM, mode);
 
 	writel_relaxed(
 		(readl_relaxed(spi->base + STM32H7_SPI_CFG2) &
@@ -1522,14 +1509,14 @@ static void stm32h7_spi_data_idleness(struct stm32_spi *spi, u32 len)
 	cfg2_clrb |= STM32H7_SPI_CFG2_MIDI;
 	if ((len > 1) && (spi->cur_midi > 0)) {
 		u32 sck_period_ns = DIV_ROUND_UP(SPI_1HZ_NS, spi->cur_speed);
-		u32 midi = min((u32)DIV_ROUND_UP(spi->cur_midi, sck_period_ns),
-			       (u32)STM32H7_SPI_CFG2_MIDI >>
-			       STM32H7_SPI_CFG2_MIDI_SHIFT);
+		u32 midi = min_t(u32,
+				 DIV_ROUND_UP(spi->cur_midi, sck_period_ns),
+				 FIELD_GET(STM32H7_SPI_CFG2_MIDI,
+					   STM32H7_SPI_CFG2_MIDI));
 
 		dev_dbg(spi->dev, "period=%dns, midi=%d(=%dns)\n",
 			sck_period_ns, midi, midi * sck_period_ns);
-		cfg2_setb |= (midi << STM32H7_SPI_CFG2_MIDI_SHIFT) &
-			     STM32H7_SPI_CFG2_MIDI;
+		cfg2_setb |= FIELD_PREP(STM32H7_SPI_CFG2_MIDI, midi);
 	}
 
 	writel_relaxed((readl_relaxed(spi->base + STM32H7_SPI_CFG2) &
@@ -1544,14 +1531,8 @@ static void stm32h7_spi_data_idleness(struct stm32_spi *spi, u32 len)
  */
 static int stm32h7_spi_number_of_data(struct stm32_spi *spi, u32 nb_words)
 {
-	u32 cr2_clrb = 0, cr2_setb = 0;
-
-	if (nb_words <= (STM32H7_SPI_CR2_TSIZE >>
-			 STM32H7_SPI_CR2_TSIZE_SHIFT)) {
-		cr2_clrb |= STM32H7_SPI_CR2_TSIZE;
-		cr2_setb = nb_words << STM32H7_SPI_CR2_TSIZE_SHIFT;
-		writel_relaxed((readl_relaxed(spi->base + STM32H7_SPI_CR2) &
-				~cr2_clrb) | cr2_setb,
+	if (nb_words <= STM32H7_SPI_TSIZE_MAX) {
+		writel_relaxed(FIELD_PREP(STM32H7_SPI_CR2_TSIZE, nb_words),
 			       spi->base + STM32H7_SPI_CR2);
 	} else {
 		return -EMSGSIZE;
-- 
2.7.4

[PATCH 05/18] spi: stm32h7: replace private SPI_1HZ_NS with NSEC_PER_SEC

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

Replace SPI_1HZ_NS private constant with NSEC_PER_SEC, which is easier
to read and understand.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index a5b926a5c4d9..b90367d522f2 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -159,8 +159,6 @@
 #define SPI_3WIRE_TX		3
 #define SPI_3WIRE_RX		4
 
-#define SPI_1HZ_NS		1000000000
-
 /*
  * use PIO for small transfers, avoiding DMA setup/teardown overhead for drivers
  * without fifo buffers.
@@ -1508,7 +1506,7 @@ static void stm32h7_spi_data_idleness(struct stm32_spi *spi, u32 len)
 
 	cfg2_clrb |= STM32H7_SPI_CFG2_MIDI;
 	if ((len > 1) && (spi->cur_midi > 0)) {
-		u32 sck_period_ns = DIV_ROUND_UP(SPI_1HZ_NS, spi->cur_speed);
+		u32 sck_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, spi->cur_speed);
 		u32 midi = min_t(u32,
 				 DIV_ROUND_UP(spi->cur_midi, sck_period_ns),
 				 FIELD_GET(STM32H7_SPI_CFG2_MIDI,
-- 
2.7.4

[PATCH 06/18] spi: stm32h7: fix irq handler

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

Check the expected flags in irq handler instead of all flags set in
status register and clear only flags that are not automatically cleared
by hardware.
In case of Full-Duplex mode, DXP flag is set when RXP and TXP flags are
set. But to avoid 2 different handlings, just add TXP and RXP flag in
the mask instead of DXP, and then keep the initial handling of TXP and
RXP events.
Also rephrase comment about EOTIE which is one of the interrupt enable
bits. It is not triggered by any event.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 40 ++++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index b90367d522f2..bbda73937668 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -131,6 +131,7 @@
 #define STM32H7_SPI_SR_RXP		BIT(0)
 #define STM32H7_SPI_SR_TXP		BIT(1)
 #define STM32H7_SPI_SR_EOT		BIT(3)
+#define STM32H7_SPI_SR_TXTF		BIT(4)
 #define STM32H7_SPI_SR_OVR		BIT(6)
 #define STM32H7_SPI_SR_SUSP		BIT(11)
 #define STM32H7_SPI_SR_RXPLVL		GENMASK(14, 13)
@@ -881,7 +882,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 {
 	struct spi_master *master = dev_id;
 	struct stm32_spi *spi = spi_master_get_devdata(master);
-	u32 sr, ier, mask;
+	u32 sr, ier, mask, ifcr;
 	unsigned long flags;
 	bool end = false;
 
@@ -889,26 +890,31 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 
 	sr = readl_relaxed(spi->base + STM32H7_SPI_SR);
 	ier = readl_relaxed(spi->base + STM32H7_SPI_IER);
+	ifcr = 0;
 
 	mask = ier;
-	/* EOTIE is triggered on EOT, SUSP and TXC events. */
+	/*
+	 * EOTIE enables irq from EOT, SUSP and TXC events. We need to set
+	 * SUSP to acknowledge it later. TXC is automatically cleared
+	 */
 	mask |= STM32H7_SPI_SR_SUSP;
 	/*
-	 * When TXTF is set, DXPIE and TXPIE are cleared. So in case of
-	 * Full-Duplex, need to poll RXP event to know if there are remaining
-	 * data, before disabling SPI.
+	 * DXPIE is set in Full-Duplex, one IT will be raised if TXP and RXP
+	 * are set. So in case of Full-Duplex, need to poll TXP and RXP event.
 	 */
-	if (spi->rx_buf && !spi->cur_usedma)
-		mask |= STM32H7_SPI_SR_RXP;
+	if ((spi->cur_comm == SPI_FULL_DUPLEX) && (!spi->cur_usedma))
+		mask |= STM32H7_SPI_SR_TXP | STM32H7_SPI_SR_RXP;
 
-	if (!(sr & mask)) {
+	mask &= sr;
+
+	if (!mask) {
 		dev_dbg(spi->dev, "spurious IT (sr=0x%08x, ier=0x%08x)\n",
 			sr, ier);
 		spin_unlock_irqrestore(&spi->lock, flags);
 		return IRQ_NONE;
 	}
 
-	if (sr & STM32H7_SPI_SR_SUSP) {
+	if (mask & STM32H7_SPI_SR_SUSP) {
 		dev_warn(spi->dev, "Communication suspended\n");
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
 			stm32h7_spi_read_rxfifo(spi, false);
@@ -918,9 +924,10 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 		 */
 		if (spi->cur_usedma)
 			end = true;
+		ifcr |= STM32H7_SPI_SR_SUSP;
 	}
 
-	if (sr & STM32H7_SPI_SR_OVR) {
+	if (mask & STM32H7_SPI_SR_OVR) {
 		dev_warn(spi->dev, "Overrun: received value discarded\n");
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
 			stm32h7_spi_read_rxfifo(spi, false);
@@ -930,23 +937,28 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 		 */
 		if (spi->cur_usedma)
 			end = true;
+		ifcr |= STM32H7_SPI_SR_OVR;
 	}
 
-	if (sr & STM32H7_SPI_SR_EOT) {
+	if (mask & STM32H7_SPI_SR_TXTF)
+		ifcr |= STM32H7_SPI_SR_TXTF;
+
+	if (mask & STM32H7_SPI_SR_EOT) {
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
 			stm32h7_spi_read_rxfifo(spi, true);
 		end = true;
+		ifcr |= STM32H7_SPI_SR_EOT;
 	}
 
-	if (sr & STM32H7_SPI_SR_TXP)
+	if (mask & STM32H7_SPI_SR_TXP)
 		if (!spi->cur_usedma && (spi->tx_buf && (spi->tx_len > 0)))
 			stm32h7_spi_write_txfifo(spi);
 
-	if (sr & STM32H7_SPI_SR_RXP)
+	if (mask & STM32H7_SPI_SR_RXP)
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
 			stm32h7_spi_read_rxfifo(spi, false);
 
-	writel_relaxed(mask, spi->base + STM32H7_SPI_IFCR);
+	writel_relaxed(ifcr, spi->base + STM32H7_SPI_IFCR);
 
 	spin_unlock_irqrestore(&spi->lock, flags);
 
-- 
2.7.4

[PATCH 07/18] spi: stm32h7: rework rx fifo read function

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

Remove flush parameter and check RXWNE or RXPLVL when end of transfer
flag is set.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index bbda73937668..06478643855a 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -575,29 +575,30 @@ static void stm32f4_spi_read_rx(struct stm32_spi *spi)
 /**
  * stm32h7_spi_read_rxfifo - Read bytes in Receive Data Register
  * @spi: pointer to the spi controller data structure
- * @flush: boolean indicating that FIFO should be flushed
  *
  * Write in rx_buf depends on remaining bytes to avoid to write beyond
  * rx_buf end.
  */
-static void stm32h7_spi_read_rxfifo(struct stm32_spi *spi, bool flush)
+static void stm32h7_spi_read_rxfifo(struct stm32_spi *spi)
 {
 	u32 sr = readl_relaxed(spi->base + STM32H7_SPI_SR);
 	u32 rxplvl = FIELD_GET(STM32H7_SPI_SR_RXPLVL, sr);
 
 	while ((spi->rx_len > 0) &&
 	       ((sr & STM32H7_SPI_SR_RXP) ||
-		(flush && ((sr & STM32H7_SPI_SR_RXWNE) || (rxplvl > 0))))) {
+		((sr & STM32H7_SPI_SR_EOT) &&
+		 ((sr & STM32H7_SPI_SR_RXWNE) || (rxplvl > 0))))) {
 		u32 offs = spi->cur_xferlen - spi->rx_len;
 
 		if ((spi->rx_len >= sizeof(u32)) ||
-		    (flush && (sr & STM32H7_SPI_SR_RXWNE))) {
+		    (sr & STM32H7_SPI_SR_RXWNE)) {
 			u32 *rx_buf32 = (u32 *)(spi->rx_buf + offs);
 
 			*rx_buf32 = readl_relaxed(spi->base + STM32H7_SPI_RXDR);
 			spi->rx_len -= sizeof(u32);
 		} else if ((spi->rx_len >= sizeof(u16)) ||
-			   (flush && (rxplvl >= 2 || spi->cur_bpw > 8))) {
+			   (!(sr & STM32H7_SPI_SR_RXWNE) &&
+			    (rxplvl >= 2 || spi->cur_bpw > 8))) {
 			u16 *rx_buf16 = (u16 *)(spi->rx_buf + offs);
 
 			*rx_buf16 = readw_relaxed(spi->base + STM32H7_SPI_RXDR);
@@ -613,8 +614,8 @@ static void stm32h7_spi_read_rxfifo(struct stm32_spi *spi, bool flush)
 		rxplvl = FIELD_GET(STM32H7_SPI_SR_RXPLVL, sr);
 	}
 
-	dev_dbg(spi->dev, "%s%s: %d bytes left\n", __func__,
-		flush ? "(flush)" : "", spi->rx_len);
+	dev_dbg(spi->dev, "%s: %d bytes left (sr=%08x)\n",
+		__func__, spi->rx_len, sr);
 }
 
 /**
@@ -682,12 +683,7 @@ static void stm32f4_spi_disable(struct stm32_spi *spi)
  * @spi: pointer to the spi controller data structure
  *
  * RX-Fifo is flushed when SPI controller is disabled. To prevent any data
- * loss, use stm32h7_spi_read_rxfifo(flush) to read the remaining bytes in
- * RX-Fifo.
- * Normally, if TSIZE has been configured, we should relax the hardware at the
- * reception of the EOT interrupt. But in case of error, EOT will not be
- * raised. So the subsystem unprepare_message call allows us to properly
- * complete the transfer from an hardware point of view.
+ * loss, use stm32_spi_read_rxfifo to read the remaining bytes in RX-Fifo.
  */
 static void stm32h7_spi_disable(struct stm32_spi *spi)
 {
@@ -722,7 +718,7 @@ static void stm32h7_spi_disable(struct stm32_spi *spi)
 	}
 
 	if (!spi->cur_usedma && spi->rx_buf && (spi->rx_len > 0))
-		stm32h7_spi_read_rxfifo(spi, true);
+		stm32h7_spi_read_rxfifo(spi);
 
 	if (spi->cur_usedma && spi->dma_tx)
 		dmaengine_terminate_all(spi->dma_tx);
@@ -917,7 +913,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 	if (mask & STM32H7_SPI_SR_SUSP) {
 		dev_warn(spi->dev, "Communication suspended\n");
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
-			stm32h7_spi_read_rxfifo(spi, false);
+			stm32h7_spi_read_rxfifo(spi);
 		/*
 		 * If communication is suspended while using DMA, it means
 		 * that something went wrong, so stop the current transfer
@@ -930,7 +926,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 	if (mask & STM32H7_SPI_SR_OVR) {
 		dev_warn(spi->dev, "Overrun: received value discarded\n");
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
-			stm32h7_spi_read_rxfifo(spi, false);
+			stm32h7_spi_read_rxfifo(spi);
 		/*
 		 * If overrun is detected while using DMA, it means that
 		 * something went wrong, so stop the current transfer
@@ -945,7 +941,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 
 	if (mask & STM32H7_SPI_SR_EOT) {
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
-			stm32h7_spi_read_rxfifo(spi, true);
+			stm32h7_spi_read_rxfifo(spi);
 		end = true;
 		ifcr |= STM32H7_SPI_SR_EOT;
 	}
@@ -956,7 +952,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 
 	if (mask & STM32H7_SPI_SR_RXP)
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
-			stm32h7_spi_read_rxfifo(spi, false);
+			stm32h7_spi_read_rxfifo(spi);
 
 	writel_relaxed(ifcr, spi->base + STM32H7_SPI_IFCR);
 
-- 
2.7.4

[PATCH 08/18] spi: stm32h7: fix dbg/warn/err conditions in irq handler

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

Make spurious interrupts visible. We do not expect to receive
them, so rise at least a warning if it happens.

Don't bother repeating the suspended RX messages; to avoid RX
overrun we have set SPI_CR1_MASRX that enables the automatic
suspended RX so, mainly in irq mode, it's normal that we will
receive SUSP interrupts every time the CPU is too much loaded
or the spi speed too high and we fail to remove on-time the
data from the RX queue. Moreover, when the CPU is overloaded
there is a delay while serving the interrupt. This forces
inactivity on the SPI bus between bytes. So the warning message
"System too slow, spi speed not guaranteed" is inaccurate; the
term "spi speed" is currently used in kernel for the toggling
frequency of the spi CLK pin, which is driven by HW and is not
impacted by CPU overload. The correct term should be
"data throughput".

RX overrun is an error condition that signals a corrupted RX
stream both in dma and in irq modes. Report the error and
abort the transfer in either cases.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 06478643855a..6731e3ff0e50 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -904,14 +904,16 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 	mask &= sr;
 
 	if (!mask) {
-		dev_dbg(spi->dev, "spurious IT (sr=0x%08x, ier=0x%08x)\n",
-			sr, ier);
+		dev_warn(spi->dev, "spurious IT (sr=0x%08x, ier=0x%08x)\n",
+			 sr, ier);
 		spin_unlock_irqrestore(&spi->lock, flags);
 		return IRQ_NONE;
 	}
 
 	if (mask & STM32H7_SPI_SR_SUSP) {
-		dev_warn(spi->dev, "Communication suspended\n");
+		dev_warn_once(spi->dev,
+			      "System too slow is limiting data throughput\n");
+
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
 			stm32h7_spi_read_rxfifo(spi);
 		/*
@@ -924,15 +926,8 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 	}
 
 	if (mask & STM32H7_SPI_SR_OVR) {
-		dev_warn(spi->dev, "Overrun: received value discarded\n");
-		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
-			stm32h7_spi_read_rxfifo(spi);
-		/*
-		 * If overrun is detected while using DMA, it means that
-		 * something went wrong, so stop the current transfer
-		 */
-		if (spi->cur_usedma)
-			end = true;
+		dev_err(spi->dev, "Overrun: RX data lost\n");
+		end = true;
 		ifcr |= STM32H7_SPI_SR_OVR;
 	}
 
-- 
2.7.4

[PATCH 09/18] spi: stm32h7: fix race condition at end of transfer

[Alain Volmat]

From: Antonio Borneo <antonio.borneo@st.com>

The caller of stm32_spi_transfer_one(), spi_transfer_one_message(),
is waiting for us to call spi_finalize_current_transfer() and will
eventually schedule a new transfer, if available.
We should guarantee that the spi controller is really available
before calling spi_finalize_current_transfer().

Move the call to spi_finalize_current_transfer() _after_ the call
to stm32_spi_disable().

Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 6731e3ff0e50..a931c821c280 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -954,8 +954,8 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 	spin_unlock_irqrestore(&spi->lock, flags);
 
 	if (end) {
-		spi_finalize_current_transfer(master);
 		stm32h7_spi_disable(spi);
+		spi_finalize_current_transfer(master);
 	}
 
 	return IRQ_HANDLED;
-- 
2.7.4

Re: [PATCH 09/18] spi: stm32h7: fix race condition at end of transfer

[Mark Brown]

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

On Wed, Aug 05, 2020 at 09:02:04AM +0200, Alain Volmat wrote:
> From: Antonio Borneo <antonio.borneo@st.com>
> 
> The caller of stm32_spi_transfer_one(), spi_transfer_one_message(),
> is waiting for us to call spi_finalize_current_transfer() and will
> eventually schedule a new transfer, if available.
> We should guarantee that the spi controller is really available
> before calling spi_finalize_current_transfer().
> 
> Move the call to spi_finalize_current_transfer() _after_ the call
> to stm32_spi_disable().

This seems like a bug fix and should therefore be at the start of the
series so it can be sent to mainline without the new development work.

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

[PATCH 10/18] spi: stm32: wait for completion in transfer_one()

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

In irq mode, when cpu is under heavy load and spi speed is set too
high, the irq handler is not fast enough to feed the spi FIFOs.
This does not compromises the data tranferred; the spi clock is
temporarily stopped, the transfer takes longer time and the real
speed is much lower than expected.
The caller of transfer_one(), spi_transfer_one_message(), waits for
the end of current transfer. It sets a timeouts with quite a generous
tolerance of +100% + 100ms, but this can still expire.

We could make transfer_one() blocking till the end of the transfer
and bypass the wait/timeout mechanism in spi_transfer_one_message().
But if for some reason, we never get either an error (OVR, SUSP) event
or end of transfer (EOT) event, xfer_completion will never "complete".
That's why a timeout is useful here to avoid a hang. Timeout delay is
deducted from the transfer length, the real speed and the optional delay
we can add between each data frames. Timeout delay is doubled compared to
the theorical transfer duration.

While doing it to address irq mode only, take benefit of the new
code structure and wait also in dma mode so an eventual error can be
returned to the framework.

Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 87 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 62 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index a931c821c280..7e3894455331 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -240,7 +240,7 @@ struct stm32_spi_cfg {
 	int (*set_mode)(struct stm32_spi *spi, unsigned int comm_type);
 	void (*set_data_idleness)(struct stm32_spi *spi, u32 length);
 	int (*set_number_of_data)(struct stm32_spi *spi, u32 length);
-	void (*transfer_one_dma_start)(struct stm32_spi *spi);
+	int (*transfer_one_dma_start)(struct stm32_spi *spi);
 	void (*dma_rx_cb)(void *data);
 	void (*dma_tx_cb)(void *data);
 	int (*transfer_one_irq)(struct stm32_spi *spi);
@@ -276,6 +276,8 @@ struct stm32_spi_cfg {
  * @dma_tx: dma channel for TX transfer
  * @dma_rx: dma channel for RX transfer
  * @phys_addr: SPI registers physical base address
+ * @xfer_completion: completion to wait for end of transfer
+ * @xfer_status: current transfer status
  */
 struct stm32_spi {
 	struct device *dev;
@@ -303,6 +305,8 @@ struct stm32_spi {
 	struct dma_chan *dma_tx;
 	struct dma_chan *dma_rx;
 	dma_addr_t phys_addr;
+	struct completion xfer_completion;
+	int xfer_status;
 };
 
 static const struct stm32_spi_regspec stm32f4_spi_regspec = {
@@ -863,8 +867,8 @@ static irqreturn_t stm32f4_spi_irq_thread(int irq, void *dev_id)
 	struct spi_master *master = dev_id;
 	struct stm32_spi *spi = spi_master_get_devdata(master);
 
-	spi_finalize_current_transfer(master);
 	stm32f4_spi_disable(spi);
+	complete(&spi->xfer_completion);
 
 	return IRQ_HANDLED;
 }
@@ -920,13 +924,16 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 		 * If communication is suspended while using DMA, it means
 		 * that something went wrong, so stop the current transfer
 		 */
-		if (spi->cur_usedma)
+		if (spi->cur_usedma) {
+			spi->xfer_status = -EIO;
 			end = true;
+		}
 		ifcr |= STM32H7_SPI_SR_SUSP;
 	}
 
 	if (mask & STM32H7_SPI_SR_OVR) {
 		dev_err(spi->dev, "Overrun: RX data lost\n");
+		spi->xfer_status = -EIO;
 		end = true;
 		ifcr |= STM32H7_SPI_SR_OVR;
 	}
@@ -955,7 +962,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 
 	if (end) {
 		stm32h7_spi_disable(spi);
-		spi_finalize_current_transfer(master);
+		complete(&spi->xfer_completion);
 	}
 
 	return IRQ_HANDLED;
@@ -1026,8 +1033,8 @@ static void stm32f4_spi_dma_tx_cb(void *data)
 	struct stm32_spi *spi = data;
 
 	if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX) {
-		spi_finalize_current_transfer(spi->master);
 		stm32f4_spi_disable(spi);
+		complete(&spi->xfer_completion);
 	}
 }
 
@@ -1041,8 +1048,8 @@ static void stm32f4_spi_dma_rx_cb(void *data)
 {
 	struct stm32_spi *spi = data;
 
-	spi_finalize_current_transfer(spi->master);
 	stm32f4_spi_disable(spi);
+	complete(&spi->xfer_completion);
 }
 
 /**
@@ -1124,9 +1131,6 @@ static void stm32_spi_dma_config(struct stm32_spi *spi,
  * stm32f4_spi_transfer_one_irq - transfer a single spi_transfer using
  *				  interrupts
  * @spi: pointer to the spi controller data structure
- *
- * It must returns 0 if the transfer is finished or 1 if the transfer is still
- * in progress.
  */
 static int stm32f4_spi_transfer_one_irq(struct stm32_spi *spi)
 {
@@ -1160,16 +1164,13 @@ static int stm32f4_spi_transfer_one_irq(struct stm32_spi *spi)
 
 	spin_unlock_irqrestore(&spi->lock, flags);
 
-	return 1;
+	return 0;
 }
 
 /**
  * stm32h7_spi_transfer_one_irq - transfer a single spi_transfer using
  *				  interrupts
  * @spi: pointer to the spi controller data structure
- *
- * It must returns 0 if the transfer is finished or 1 if the transfer is still
- * in progress.
  */
 static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi)
 {
@@ -1202,7 +1203,7 @@ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi)
 
 	spin_unlock_irqrestore(&spi->lock, flags);
 
-	return 1;
+	return 0;
 }
 
 /**
@@ -1210,8 +1211,12 @@ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi)
  *					transfer using DMA
  * @spi: pointer to the spi controller data structure
  */
-static void stm32f4_spi_transfer_one_dma_start(struct stm32_spi *spi)
+static int stm32f4_spi_transfer_one_dma_start(struct stm32_spi *spi)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&spi->lock, flags);
+
 	/* In DMA mode end of transfer is handled by DMA TX or RX callback. */
 	if (spi->cur_comm == SPI_SIMPLEX_RX || spi->cur_comm == SPI_3WIRE_RX ||
 	    spi->cur_comm == SPI_FULL_DUPLEX) {
@@ -1224,6 +1229,10 @@ static void stm32f4_spi_transfer_one_dma_start(struct stm32_spi *spi)
 	}
 
 	stm32_spi_enable(spi);
+
+	spin_unlock_irqrestore(&spi->lock, flags);
+
+	return 0;
 }
 
 /**
@@ -1231,8 +1240,12 @@ static void stm32f4_spi_transfer_one_dma_start(struct stm32_spi *spi)
  *					transfer using DMA
  * @spi: pointer to the spi controller data structure
  */
-static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)
+static int stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&spi->lock, flags);
+
 	/* Enable the interrupts relative to the end of transfer */
 	stm32_spi_set_bits(spi, STM32H7_SPI_IER, STM32H7_SPI_IER_EOTIE |
 						 STM32H7_SPI_IER_TXTFIE |
@@ -1241,15 +1254,16 @@ static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)
 	stm32_spi_enable(spi);
 
 	stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART);
+
+	spin_unlock_irqrestore(&spi->lock, flags);
+
+	return 0;
 }
 
 /**
  * stm32_spi_transfer_one_dma - transfer a single spi_transfer using DMA
  * @spi: pointer to the spi controller data structure
  * @xfer: pointer to the spi_transfer structure
- *
- * It must returns 0 if the transfer is finished or 1 if the transfer is still
- * in progress.
  */
 static int stm32_spi_transfer_one_dma(struct stm32_spi *spi,
 				      struct spi_transfer *xfer)
@@ -1325,12 +1339,9 @@ static int stm32_spi_transfer_one_dma(struct stm32_spi *spi,
 		stm32_spi_set_bits(spi, spi->cfg->regs->dma_tx_en.reg,
 				   spi->cfg->regs->dma_tx_en.mask);
 	}
-
-	spi->cfg->transfer_one_dma_start(spi);
-
 	spin_unlock_irqrestore(&spi->lock, flags);
 
-	return 1;
+	return spi->cfg->transfer_one_dma_start(spi);
 
 dma_submit_error:
 	if (spi->dma_rx)
@@ -1640,6 +1651,7 @@ static int stm32_spi_transfer_one(struct spi_master *master,
 				  struct spi_transfer *transfer)
 {
 	struct stm32_spi *spi = spi_master_get_devdata(master);
+	u32 xfer_time, midi_delay_ns;
 	int ret;
 
 	spi->tx_buf = transfer->tx_buf;
@@ -1656,10 +1668,34 @@ static int stm32_spi_transfer_one(struct spi_master *master,
 		return ret;
 	}
 
+	reinit_completion(&spi->xfer_completion);
+	spi->xfer_status = 0;
+
 	if (spi->cur_usedma)
-		return stm32_spi_transfer_one_dma(spi, transfer);
+		ret = stm32_spi_transfer_one_dma(spi, transfer);
 	else
-		return spi->cfg->transfer_one_irq(spi);
+		ret = spi->cfg->transfer_one_irq(spi);
+
+	if (ret)
+		return ret;
+
+	/* Wait for transfer to complete */
+	xfer_time = spi->cur_xferlen * 8 * MSEC_PER_SEC / spi->cur_speed;
+	midi_delay_ns = spi->cur_xferlen * 8 / spi->cur_bpw * spi->cur_midi;
+	xfer_time += DIV_ROUND_UP(midi_delay_ns, NSEC_PER_MSEC);
+	xfer_time = max(2 * xfer_time, 100U);
+
+	ret = wait_for_completion_timeout(&spi->xfer_completion,
+					  (msecs_to_jiffies(xfer_time)));
+	if (!ret) {
+		dev_err(spi->dev, "SPI transfer timeout (%u ms)\n", xfer_time);
+		spi->xfer_status = -ETIMEDOUT;
+		spi->cfg->disable(spi);
+	}
+
+	spi_finalize_current_transfer(master);
+
+	return spi->xfer_status;
 }
 
 /**
@@ -1810,6 +1846,7 @@ static int stm32_spi_probe(struct platform_device *pdev)
 	spi->dev = &pdev->dev;
 	spi->master = master;
 	spin_lock_init(&spi->lock);
+	init_completion(&spi->xfer_completion);
 
 	spi->cfg = (const struct stm32_spi_cfg *)
 		of_match_device(pdev->dev.driver->of_match_table,
-- 
2.7.4

Re: [PATCH 10/18] spi: stm32: wait for completion in transfer_one()

[Mark Brown]

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

On Wed, Aug 05, 2020 at 09:02:05AM +0200, Alain Volmat wrote:

> We could make transfer_one() blocking till the end of the transfer
> and bypass the wait/timeout mechanism in spi_transfer_one_message().
> But if for some reason, we never get either an error (OVR, SUSP) event
> or end of transfer (EOT) event, xfer_completion will never "complete".
> That's why a timeout is useful here to avoid a hang. Timeout delay is
> deducted from the transfer length, the real speed and the optional delay
> we can add between each data frames. Timeout delay is doubled compared to
> the theorical transfer duration.

> While doing it to address irq mode only, take benefit of the new
> code structure and wait also in dma mode so an eventual error can be
> returned to the framework.

I can't tell from this changelog what this change is intended to do
which makes it very difficult to review.  If the timeout is too short
for some systems under extreme load it would be better to provide a
generic way of configuring it rather than open coding timeouts, or
otherwise improve the generic code.

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

[PATCH 11/18] spi: stm32: fix fifo threshold level in case of short transfer

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

When transfer is shorter than half of the fifo, set the data packet size
up to transfer size instead of up to half of the fifo.
Check also that threshold is set at least to 1 data frame.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 7e3894455331..0eda9903e11e 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -459,20 +459,24 @@ static int stm32_spi_prepare_mbr(struct stm32_spi *spi, u32 speed_hz,
 /**
  * stm32h7_spi_prepare_fthlv - Determine FIFO threshold level
  * @spi: pointer to the spi controller data structure
+ * @xfer_len: length of the message to be transferred
  */
-static u32 stm32h7_spi_prepare_fthlv(struct stm32_spi *spi)
+static u32 stm32h7_spi_prepare_fthlv(struct stm32_spi *spi, u32 xfer_len)
 {
-	u32 fthlv, half_fifo;
+	u32 fthlv, half_fifo, packet;
 
 	/* data packet should not exceed 1/2 of fifo space */
 	half_fifo = (spi->fifo_size / 2);
 
+	/* data_packet should not exceed transfer length */
+	packet = (half_fifo > xfer_len) ? xfer_len : half_fifo;
+
 	if (spi->cur_bpw <= 8)
-		fthlv = half_fifo;
+		fthlv = packet;
 	else if (spi->cur_bpw <= 16)
-		fthlv = half_fifo / 2;
+		fthlv = packet / 2;
 	else
-		fthlv = half_fifo / 4;
+		fthlv = packet / 4;
 
 	/* align packet size with data registers access */
 	if (spi->cur_bpw > 8)
@@ -480,6 +484,9 @@ static u32 stm32h7_spi_prepare_fthlv(struct stm32_spi *spi)
 	else
 		fthlv -= (fthlv % 4); /* multiple of 4 */
 
+	if (!fthlv)
+		fthlv = 1;
+
 	return fthlv;
 }
 
@@ -1385,7 +1392,7 @@ static void stm32h7_spi_set_bpw(struct stm32_spi *spi)
 	cfg1_clrb |= STM32H7_SPI_CFG1_DSIZE;
 	cfg1_setb |= FIELD_PREP(STM32H7_SPI_CFG1_DSIZE, bpw);
 
-	spi->cur_fthlv = stm32h7_spi_prepare_fthlv(spi);
+	spi->cur_fthlv = stm32h7_spi_prepare_fthlv(spi, spi->cur_xferlen);
 	fthlv = spi->cur_fthlv - 1;
 
 	cfg1_clrb |= STM32H7_SPI_CFG1_FTHLV;
@@ -1571,6 +1578,8 @@ static int stm32_spi_transfer_one_setup(struct stm32_spi *spi,
 
 	spin_lock_irqsave(&spi->lock, flags);
 
+	spi->cur_xferlen = transfer->len;
+
 	if (spi->cur_bpw != transfer->bits_per_word) {
 		spi->cur_bpw = transfer->bits_per_word;
 		spi->cfg->set_bpw(spi);
@@ -1618,8 +1627,6 @@ static int stm32_spi_transfer_one_setup(struct stm32_spi *spi,
 			goto out;
 	}
 
-	spi->cur_xferlen = transfer->len;
-
 	dev_dbg(spi->dev, "transfer communication mode set to %d\n",
 		spi->cur_comm);
 	dev_dbg(spi->dev,
-- 
2.7.4

Re: [PATCH 11/18] spi: stm32: fix fifo threshold level in case of short transfer

[Mark Brown]

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

On Wed, Aug 05, 2020 at 09:02:06AM +0200, Alain Volmat wrote:
> From: Amelie Delaunay <amelie.delaunay@st.com>
> 
> When transfer is shorter than half of the fifo, set the data packet size
> up to transfer size instead of up to half of the fifo.
> Check also that threshold is set at least to 1 data frame.

This looks like another fix which should be before any new development.

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

[PATCH 12/18] spi: stm32: move spi disable out of irq handler

[Alain Volmat]

From: Antonio Borneo <antonio.borneo@st.com>

The spi disable could potentially require some time to finish.
It has to be executed at the end of a transfer, but there is
no reason to call it in the irq handler.

Simplify the irq handler by moving out the spi disable. The
synchronization through xfer_completion is used to defer the
execution of spi disable.

Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 0eda9903e11e..1a703c4a65db 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -874,7 +874,6 @@ static irqreturn_t stm32f4_spi_irq_thread(int irq, void *dev_id)
 	struct spi_master *master = dev_id;
 	struct stm32_spi *spi = spi_master_get_devdata(master);
 
-	stm32f4_spi_disable(spi);
 	complete(&spi->xfer_completion);
 
 	return IRQ_HANDLED;
@@ -963,15 +962,18 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
 		if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
 			stm32h7_spi_read_rxfifo(spi);
 
-	writel_relaxed(ifcr, spi->base + STM32H7_SPI_IFCR);
-
-	spin_unlock_irqrestore(&spi->lock, flags);
-
 	if (end) {
-		stm32h7_spi_disable(spi);
+		/* Disable interrupts and clear status flags */
+		writel_relaxed(0, spi->base + STM32H7_SPI_IER);
+		writel_relaxed(STM32H7_SPI_IFCR_ALL,
+			       spi->base + STM32H7_SPI_IFCR);
+
 		complete(&spi->xfer_completion);
+	} else {
+		writel_relaxed(ifcr, spi->base + STM32H7_SPI_IFCR);
 	}
 
+	spin_unlock_irqrestore(&spi->lock, flags);
 	return IRQ_HANDLED;
 }
 
@@ -1039,10 +1041,8 @@ static void stm32f4_spi_dma_tx_cb(void *data)
 {
 	struct stm32_spi *spi = data;
 
-	if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX) {
-		stm32f4_spi_disable(spi);
+	if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX)
 		complete(&spi->xfer_completion);
-	}
 }
 
 /**
@@ -1055,7 +1055,6 @@ static void stm32f4_spi_dma_rx_cb(void *data)
 {
 	struct stm32_spi *spi = data;
 
-	stm32f4_spi_disable(spi);
 	complete(&spi->xfer_completion);
 }
 
@@ -1697,9 +1696,10 @@ static int stm32_spi_transfer_one(struct spi_master *master,
 	if (!ret) {
 		dev_err(spi->dev, "SPI transfer timeout (%u ms)\n", xfer_time);
 		spi->xfer_status = -ETIMEDOUT;
-		spi->cfg->disable(spi);
 	}
 
+	spi->cfg->disable(spi);
+
 	spi_finalize_current_transfer(master);
 
 	return spi->xfer_status;
-- 
2.7.4

Re: [PATCH 12/18] spi: stm32: move spi disable out of irq handler

[Mark Brown]

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

On Wed, Aug 05, 2020 at 09:02:07AM +0200, Alain Volmat wrote:
> From: Antonio Borneo <antonio.borneo@st.com>
> 
> The spi disable could potentially require some time to finish.
> It has to be executed at the end of a transfer, but there is
> no reason to call it in the irq handler.
> 
> Simplify the irq handler by moving out the spi disable. The
> synchronization through xfer_completion is used to defer the
> execution of spi disable.

Should this be an unprepare_transfer_hardware() operation?  That would
allow the framework to take care of scheduling this in an appropriate
context with the added advantage of not needing to do it when there's
another message scheduled immediately.

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

[PATCH 13/18] spi: stm32h7: fix handling of dma transfer completed

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

The rx dma is completed "after" the last data is received
from spi. Thus, to avoid loss of rx data, it's mandatory to
wait for the dma callback before tearing down the rx dma in
stm32_spi_disable().

The tx dma is of course already completed when last data is
sent from spi. But both spi and dma use threaded interrupts,
thus there is no guarantee that the dma irq handler is already
executed when the spi irq handler triggers stm32_spi_disable().
Waiting for dma callback will allow a clean termination of the
dma.

Remove the unused code in the current dma callback, signal the
end of dma through completion, then delay spi disable until
the dma callback has been executed.

Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 1a703c4a65db..b0a9642392e9 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -275,6 +275,7 @@ struct stm32_spi_cfg {
  * @rx_len: number of data to be read in bytes
  * @dma_tx: dma channel for TX transfer
  * @dma_rx: dma channel for RX transfer
+ * @dma_completion: completion to wait for end of DMA transfer
  * @phys_addr: SPI registers physical base address
  * @xfer_completion: completion to wait for end of transfer
  * @xfer_status: current transfer status
@@ -304,6 +305,7 @@ struct stm32_spi {
 	int rx_len;
 	struct dma_chan *dma_tx;
 	struct dma_chan *dma_rx;
+	struct completion dma_completion;
 	dma_addr_t phys_addr;
 	struct completion xfer_completion;
 	int xfer_status;
@@ -1062,25 +1064,18 @@ static void stm32f4_spi_dma_rx_cb(void *data)
  * stm32h7_spi_dma_cb - dma callback
  * @data: pointer to the spi controller data structure
  *
- * DMA callback is called when the transfer is complete or when an error
- * occurs. If the transfer is complete, EOT flag is raised.
+ * DMA callback is called when the transfer is complete.
  */
 static void stm32h7_spi_dma_cb(void *data)
 {
 	struct stm32_spi *spi = data;
 	unsigned long flags;
-	u32 sr;
 
 	spin_lock_irqsave(&spi->lock, flags);
 
-	sr = readl_relaxed(spi->base + STM32H7_SPI_SR);
+	complete(&spi->dma_completion);
 
 	spin_unlock_irqrestore(&spi->lock, flags);
-
-	if (!(sr & STM32H7_SPI_SR_EOT))
-		dev_warn(spi->dev, "DMA error (sr=0x%08x)\n", sr);
-
-	/* Now wait for EOT, or SUSP or OVR in case of error */
 }
 
 /**
@@ -1274,12 +1269,20 @@ static int stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)
 static int stm32_spi_transfer_one_dma(struct stm32_spi *spi,
 				      struct spi_transfer *xfer)
 {
+	dma_async_tx_callback rx_done = NULL, tx_done = NULL;
 	struct dma_slave_config tx_dma_conf, rx_dma_conf;
 	struct dma_async_tx_descriptor *tx_dma_desc, *rx_dma_desc;
 	unsigned long flags;
 
 	spin_lock_irqsave(&spi->lock, flags);
 
+	if (spi->rx_buf)
+		rx_done = spi->cfg->dma_rx_cb;
+	else if (spi->tx_buf)
+		tx_done = spi->cfg->dma_tx_cb;
+
+	reinit_completion(&spi->dma_completion);
+
 	rx_dma_desc = NULL;
 	if (spi->rx_buf && spi->dma_rx) {
 		stm32_spi_dma_config(spi, &rx_dma_conf, DMA_DEV_TO_MEM);
@@ -1316,7 +1319,7 @@ static int stm32_spi_transfer_one_dma(struct stm32_spi *spi,
 		goto dma_desc_error;
 
 	if (rx_dma_desc) {
-		rx_dma_desc->callback = spi->cfg->dma_rx_cb;
+		rx_dma_desc->callback = rx_done;
 		rx_dma_desc->callback_param = spi;
 
 		if (dma_submit_error(dmaengine_submit(rx_dma_desc))) {
@@ -1330,7 +1333,7 @@ static int stm32_spi_transfer_one_dma(struct stm32_spi *spi,
 	if (tx_dma_desc) {
 		if (spi->cur_comm == SPI_SIMPLEX_TX ||
 		    spi->cur_comm == SPI_3WIRE_TX) {
-			tx_dma_desc->callback = spi->cfg->dma_tx_cb;
+			tx_dma_desc->callback = tx_done;
 			tx_dma_desc->callback_param = spi;
 		}
 
@@ -1658,6 +1661,7 @@ static int stm32_spi_transfer_one(struct spi_master *master,
 {
 	struct stm32_spi *spi = spi_master_get_devdata(master);
 	u32 xfer_time, midi_delay_ns;
+	unsigned long timeout;
 	int ret;
 
 	spi->tx_buf = transfer->tx_buf;
@@ -1690,10 +1694,14 @@ static int stm32_spi_transfer_one(struct spi_master *master,
 	midi_delay_ns = spi->cur_xferlen * 8 / spi->cur_bpw * spi->cur_midi;
 	xfer_time += DIV_ROUND_UP(midi_delay_ns, NSEC_PER_MSEC);
 	xfer_time = max(2 * xfer_time, 100U);
+	timeout = msecs_to_jiffies(xfer_time);
+
+	timeout = wait_for_completion_timeout(&spi->xfer_completion, timeout);
+	if (timeout && spi->cur_usedma)
+		timeout = wait_for_completion_timeout(&spi->dma_completion,
+						      timeout);
 
-	ret = wait_for_completion_timeout(&spi->xfer_completion,
-					  (msecs_to_jiffies(xfer_time)));
-	if (!ret) {
+	if (!timeout) {
 		dev_err(spi->dev, "SPI transfer timeout (%u ms)\n", xfer_time);
 		spi->xfer_status = -ETIMEDOUT;
 	}
@@ -1854,6 +1862,7 @@ static int stm32_spi_probe(struct platform_device *pdev)
 	spi->master = master;
 	spin_lock_init(&spi->lock);
 	init_completion(&spi->xfer_completion);
+	init_completion(&spi->dma_completion);
 
 	spi->cfg = (const struct stm32_spi_cfg *)
 		of_match_device(pdev->dev.driver->of_match_table,
-- 
2.7.4

Re: [PATCH 13/18] spi: stm32h7: fix handling of dma transfer completed

[Mark Brown]

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

On Wed, Aug 05, 2020 at 09:02:08AM +0200, Alain Volmat wrote:
> From: Amelie Delaunay <amelie.delaunay@st.com>
> 
> The rx dma is completed "after" the last data is received
> from spi. Thus, to avoid loss of rx data, it's mandatory to
> wait for the dma callback before tearing down the rx dma in
> stm32_spi_disable().

Again, please put fixes at the start of the series before new
development.

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

[PATCH 14/18] spi: stm32: improve suspend/resume management

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

This patch adds pinctrl power management, and reconfigure spi controller
in case of resume.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index b0a9642392e9..0aec32538093 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -14,6 +14,7 @@
 #include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include <linux/spi/spi.h>
@@ -2030,6 +2031,8 @@ static int stm32_spi_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 
+	pinctrl_pm_select_sleep_state(&pdev->dev);
+
 	return 0;
 }
 
@@ -2041,13 +2044,18 @@ static int stm32_spi_runtime_suspend(struct device *dev)
 
 	clk_disable_unprepare(spi->clk);
 
-	return 0;
+	return pinctrl_pm_select_sleep_state(dev);
 }
 
 static int stm32_spi_runtime_resume(struct device *dev)
 {
 	struct spi_master *master = dev_get_drvdata(dev);
 	struct stm32_spi *spi = spi_master_get_devdata(master);
+	int ret;
+
+	ret = pinctrl_pm_select_default_state(dev);
+	if (ret)
+		return ret;
 
 	return clk_prepare_enable(spi->clk);
 }
@@ -2077,10 +2085,23 @@ static int stm32_spi_resume(struct device *dev)
 		return ret;
 
 	ret = spi_master_resume(master);
-	if (ret)
+	if (ret) {
 		clk_disable_unprepare(spi->clk);
+		return ret;
+	}
 
-	return ret;
+	ret = pm_runtime_get_sync(dev);
+	if (ret) {
+		dev_err(dev, "Unable to power device:%d\n", ret);
+		return ret;
+	}
+
+	spi->cfg->config(spi);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
 }
 #endif
 
-- 
2.7.4

[PATCH 15/18] spi: stm32: fix stm32_spi_prepare_mbr in case of odd clk_rate

[Alain Volmat]

From: Amelie Delaunay <amelie.delaunay@st.com>

Fix spi->clk_rate when it is odd to the nearest lowest even value because
minimum SPI divider is 2.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 0aec32538093..2665d8d7f318 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -436,7 +436,8 @@ static int stm32_spi_prepare_mbr(struct stm32_spi *spi, u32 speed_hz,
 {
 	u32 div, mbrdiv;
 
-	div = DIV_ROUND_UP(spi->clk_rate, speed_hz);
+	/* Ensure spi->clk_rate is even */
+	div = DIV_ROUND_UP(spi->clk_rate & ~0x1, speed_hz);
 
 	/*
 	 * SPI framework set xfer->speed_hz to master->max_speed_hz if
-- 
2.7.4

Re: [PATCH 15/18] spi: stm32: fix stm32_spi_prepare_mbr in case of odd clk_rate

[Mark Brown]

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

On Wed, Aug 05, 2020 at 09:02:10AM +0200, Alain Volmat wrote:
> From: Amelie Delaunay <amelie.delaunay@st.com>
> 
> Fix spi->clk_rate when it is odd to the nearest lowest even value because
> minimum SPI divider is 2.

This is a fix too.

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

[PATCH 16/18] spi: stm32: always perform registers configuration prior to transfer

[Alain Volmat]

SPI registers content may have been lost upon suspend/resume sequence.
So, always compute and apply the necessary configuration in
stm32_spi_transfer_one_setup routine.

Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 42 +++++++++++++++++-------------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 2665d8d7f318..177f82700de0 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -1579,41 +1579,33 @@ static int stm32_spi_transfer_one_setup(struct stm32_spi *spi,
 	unsigned long flags;
 	unsigned int comm_type;
 	int nb_words, ret = 0;
+	int mbr;
 
 	spin_lock_irqsave(&spi->lock, flags);
 
 	spi->cur_xferlen = transfer->len;
 
-	if (spi->cur_bpw != transfer->bits_per_word) {
-		spi->cur_bpw = transfer->bits_per_word;
-		spi->cfg->set_bpw(spi);
-	}
-
-	if (spi->cur_speed != transfer->speed_hz) {
-		int mbr;
-
-		/* Update spi->cur_speed with real clock speed */
-		mbr = stm32_spi_prepare_mbr(spi, transfer->speed_hz,
-					    spi->cfg->baud_rate_div_min,
-					    spi->cfg->baud_rate_div_max);
-		if (mbr < 0) {
-			ret = mbr;
-			goto out;
-		}
+	spi->cur_bpw = transfer->bits_per_word;
+	spi->cfg->set_bpw(spi);
 
-		transfer->speed_hz = spi->cur_speed;
-		stm32_spi_set_mbr(spi, mbr);
+	/* Update spi->cur_speed with real clock speed */
+	mbr = stm32_spi_prepare_mbr(spi, transfer->speed_hz,
+				    spi->cfg->baud_rate_div_min,
+				    spi->cfg->baud_rate_div_max);
+	if (mbr < 0) {
+		ret = mbr;
+		goto out;
 	}
 
-	comm_type = stm32_spi_communication_type(spi_dev, transfer);
-	if (spi->cur_comm != comm_type) {
-		ret = spi->cfg->set_mode(spi, comm_type);
+	transfer->speed_hz = spi->cur_speed;
+	stm32_spi_set_mbr(spi, mbr);
 
-		if (ret < 0)
-			goto out;
+	comm_type = stm32_spi_communication_type(spi_dev, transfer);
+	ret = spi->cfg->set_mode(spi, comm_type);
+	if (ret < 0)
+		goto out;
 
-		spi->cur_comm = comm_type;
-	}
+	spi->cur_comm = comm_type;
 
 	if (spi->cfg->set_data_idleness)
 		spi->cfg->set_data_idleness(spi, transfer->len);
-- 
2.7.4

Re: [PATCH 16/18] spi: stm32: always perform registers configuration prior to transfer

[Mark Brown]

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

On Wed, Aug 05, 2020 at 09:02:11AM +0200, Alain Volmat wrote:
> SPI registers content may have been lost upon suspend/resume sequence.
> So, always compute and apply the necessary configuration in
> stm32_spi_transfer_one_setup routine.

This also.

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

[PATCH 17/18] spi: stm32: properly handle 0 byte transfer

[Alain Volmat]

On 0 byte transfer request, return straight from the
xfer function after finalizing the transfer.

Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 177f82700de0..b909afd9e99b 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -1658,6 +1658,12 @@ static int stm32_spi_transfer_one(struct spi_master *master,
 	unsigned long timeout;
 	int ret;
 
+	/* Don't do anything on 0 bytes transfers */
+	if (transfer->len == 0) {
+		spi->xfer_status = 0;
+		goto finalize;
+	}
+
 	spi->tx_buf = transfer->tx_buf;
 	spi->rx_buf = transfer->rx_buf;
 	spi->tx_len = spi->tx_buf ? transfer->len : 0;
@@ -1702,6 +1708,7 @@ static int stm32_spi_transfer_one(struct spi_master *master,
 
 	spi->cfg->disable(spi);
 
+finalize:
 	spi_finalize_current_transfer(master);
 
 	return spi->xfer_status;
-- 
2.7.4

[PATCH 18/18] spi: stm32h7: ensure message are smaller than max size

[Alain Volmat]

Ensure that messages given to transfer_one handler can actually be
handled by it. For that purpose rely on the SPI framework
spi_split_transfers_maxsize function to split messages whenever necessary.

Signed-off-by: Alain Volmat <alain.volmat@st.com>
---
 drivers/spi/spi-stm32.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index b909afd9e99b..bd21b698af84 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -1021,6 +1021,20 @@ static int stm32_spi_prepare_msg(struct spi_master *master,
 		spi_dev->mode & SPI_LSB_FIRST,
 		spi_dev->mode & SPI_CS_HIGH);
 
+	/* On STM32H7, messages should not exceed a maximum size setted
+	 * afterward via the set_number_of_data function. In order to
+	 * ensure that, split large messages into several messages
+	 */
+	if (spi->cfg->set_number_of_data) {
+		int ret;
+
+		ret = spi_split_transfers_maxsize(master, msg,
+						  STM32H7_SPI_TSIZE_MAX,
+						  GFP_KERNEL | GFP_DMA);
+		if (ret)
+			return ret;
+	}
+
 	spin_lock_irqsave(&spi->lock, flags);
 
 	/* CPOL, CPHA and LSB FIRST bits have common register */
-- 
2.7.4