[v1] spi: spi-fsl-lpspi: Generic fixes and support for S32G devices

[PATCH 04/13] spi: spi-fsl-lpspi: Clear status register after disabling the module

Posted by James Clark 5 months, 4 weeks ago

From: Larisa Grigore <larisa.grigore@nxp.com>

Clear the error flags after disabling the module to avoid the case when
a flag is set between when the flags were just cleared, and when the
module is disabled.

Although fsl_lpspi_reset() was only introduced in commit a15dc3d657fa
("spi: lpspi: Fix CLK pin becomes low before one transfer"), the
original driver only reset SR in the interrupt handler, making it
vulnerable to the same issue. Therefore the fixes commit is set at the
introduction of the driver.

Fixes: 5314987de5e5 ("spi: imx: add lpspi bus driver")
Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@nxp.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
 drivers/spi/spi-fsl-lpspi.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
index aab92ee7eb94..79b170426bee 100644
--- a/drivers/spi/spi-fsl-lpspi.c
+++ b/drivers/spi/spi-fsl-lpspi.c
@@ -82,6 +82,8 @@
 #define TCR_RXMSK	BIT(19)
 #define TCR_TXMSK	BIT(18)
 
+#define SR_CLEAR_MASK	GENMASK(13, 8)
+
 struct fsl_lpspi_devtype_data {
 	u8 prescale_max;
 };
@@ -536,14 +538,13 @@ static int fsl_lpspi_reset(struct fsl_lpspi_data *fsl_lpspi)
 		fsl_lpspi_intctrl(fsl_lpspi, 0);
 	}
 
-	/* W1C for all flags in SR */
-	temp = 0x3F << 8;
-	writel(temp, fsl_lpspi->base + IMX7ULP_SR);
-
 	/* Clear FIFO and disable module */
 	temp = CR_RRF | CR_RTF;
 	writel(temp, fsl_lpspi->base + IMX7ULP_CR);
 
+	/* W1C for all flags in SR */
+	writel(SR_CLEAR_MASK, fsl_lpspi->base + IMX7ULP_SR);
+
 	return 0;
 }
 

-- 
2.34.1

Re: [PATCH 04/13] spi: spi-fsl-lpspi: Clear status register after disabling the module

Posted by Frank Li 5 months, 4 weeks ago

On Thu, Aug 14, 2025 at 05:06:44PM +0100, James Clark wrote:
> From: Larisa Grigore <larisa.grigore@nxp.com>
>
> Clear the error flags after disabling the module to avoid the case when
> a flag is set between when the flags were just cleared, and when the
> module is disabled.

between flags clear and module disable

And use SR_CLEAR_MASK to replace hardcoded value for improved readability


Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> Although fsl_lpspi_reset() was only introduced in commit a15dc3d657fa
> ("spi: lpspi: Fix CLK pin becomes low before one transfer"), the
> original driver only reset SR in the interrupt handler, making it
> vulnerable to the same issue. Therefore the fixes commit is set at the
> introduction of the driver.
>
> Fixes: 5314987de5e5 ("spi: imx: add lpspi bus driver")
> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@nxp.com>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
>  drivers/spi/spi-fsl-lpspi.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
> index aab92ee7eb94..79b170426bee 100644
> --- a/drivers/spi/spi-fsl-lpspi.c
> +++ b/drivers/spi/spi-fsl-lpspi.c
> @@ -82,6 +82,8 @@
>  #define TCR_RXMSK	BIT(19)
>  #define TCR_TXMSK	BIT(18)
>
> +#define SR_CLEAR_MASK	GENMASK(13, 8)
> +
>  struct fsl_lpspi_devtype_data {
>  	u8 prescale_max;
>  };
> @@ -536,14 +538,13 @@ static int fsl_lpspi_reset(struct fsl_lpspi_data *fsl_lpspi)
>  		fsl_lpspi_intctrl(fsl_lpspi, 0);
>  	}
>
> -	/* W1C for all flags in SR */
> -	temp = 0x3F << 8;
> -	writel(temp, fsl_lpspi->base + IMX7ULP_SR);
> -
>  	/* Clear FIFO and disable module */
>  	temp = CR_RRF | CR_RTF;
>  	writel(temp, fsl_lpspi->base + IMX7ULP_CR);
>
> +	/* W1C for all flags in SR */
> +	writel(SR_CLEAR_MASK, fsl_lpspi->base + IMX7ULP_SR);
> +
>  	return 0;
>  }
>
>
> --
> 2.34.1
>

Re: [PATCH 04/13] spi: spi-fsl-lpspi: Clear status register after disabling the module

Posted by James Clark 5 months, 3 weeks ago


On 14/08/2025 5:58 pm, Frank Li wrote:
> On Thu, Aug 14, 2025 at 05:06:44PM +0100, James Clark wrote:
>> From: Larisa Grigore <larisa.grigore@nxp.com>
>>
>> Clear the error flags after disabling the module to avoid the case when
>> a flag is set between when the flags were just cleared, and when the
>> module is disabled.
> 
> between flags clear and module disable
> 
> And use SR_CLEAR_MASK to replace hardcoded value for improved readability
> 

Ack

> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> 
>>
>> Although fsl_lpspi_reset() was only introduced in commit a15dc3d657fa
>> ("spi: lpspi: Fix CLK pin becomes low before one transfer"), the
>> original driver only reset SR in the interrupt handler, making it
>> vulnerable to the same issue. Therefore the fixes commit is set at the
>> introduction of the driver.
>>
>> Fixes: 5314987de5e5 ("spi: imx: add lpspi bus driver")
>> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@nxp.com>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>>   drivers/spi/spi-fsl-lpspi.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
>> index aab92ee7eb94..79b170426bee 100644
>> --- a/drivers/spi/spi-fsl-lpspi.c
>> +++ b/drivers/spi/spi-fsl-lpspi.c
>> @@ -82,6 +82,8 @@
>>   #define TCR_RXMSK	BIT(19)
>>   #define TCR_TXMSK	BIT(18)
>>
>> +#define SR_CLEAR_MASK	GENMASK(13, 8)
>> +
>>   struct fsl_lpspi_devtype_data {
>>   	u8 prescale_max;
>>   };
>> @@ -536,14 +538,13 @@ static int fsl_lpspi_reset(struct fsl_lpspi_data *fsl_lpspi)
>>   		fsl_lpspi_intctrl(fsl_lpspi, 0);
>>   	}
>>
>> -	/* W1C for all flags in SR */
>> -	temp = 0x3F << 8;
>> -	writel(temp, fsl_lpspi->base + IMX7ULP_SR);
>> -
>>   	/* Clear FIFO and disable module */
>>   	temp = CR_RRF | CR_RTF;
>>   	writel(temp, fsl_lpspi->base + IMX7ULP_CR);
>>
>> +	/* W1C for all flags in SR */
>> +	writel(SR_CLEAR_MASK, fsl_lpspi->base + IMX7ULP_SR);
>> +
>>   	return 0;
>>   }
>>
>>
>> --
>> 2.34.1
>>