[PATCH spi-next 0/3] spi: spi-fsl-lpspi: various cleanup and enhancement patches - part 2

[PATCH spi-next 0/3] spi: spi-fsl-lpspi: various cleanup and enhancement patches - part 2

[Marc Kleine-Budde]

While optimizing the spi-fsl-lpspi driver, I created some cleanup and
enhacement patches.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Marc Kleine-Budde (3):
      spi: spi-fsl-lpspi: pass struct fsl_lpspi_data instead of struct spi_controller if possible
      spi: spi-fsl-lpspi: fsl_lpspi_transfer_one(): directly return ret
      spi: spi-fsl-lpspi: cache content of Interrupt Enable Register (IER)

 drivers/spi/spi-fsl-lpspi.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)
---
base-commit: 85b0e19de168d82cbd879a4c8e13fabad5ef4dd8
change-id: 20260916-spi-fsl-lpspi-more-cleanups-d2c934a372a1

Best regards,
--  
Marc Kleine-Budde <mkl@pengutronix.de>

[PATCH spi-next 1/3] spi: spi-fsl-lpspi: pass struct fsl_lpspi_data instead of struct spi_controller if possible

[Marc Kleine-Budde]

Instead of passing a spi_controller pointer to functions and using
spi_controller_get_devdata() to get the struct fsl_lpspi_data pointer,
directly pass a struct fsl_lpspi_data pointer.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/spi/spi-fsl-lpspi.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
index 5aa0a75b4e57..a3c5bab1acc7 100644
--- a/drivers/spi/spi-fsl-lpspi.c
+++ b/drivers/spi/spi-fsl-lpspi.c
@@ -550,11 +550,8 @@ static int fsl_lpspi_target_abort(struct spi_controller *controller)
 	return 0;
 }
 
-static int fsl_lpspi_wait_for_completion(struct spi_controller *controller)
+static int fsl_lpspi_wait_for_completion(struct fsl_lpspi_data *fsl_lpspi)
 {
-	struct fsl_lpspi_data *fsl_lpspi =
-				spi_controller_get_devdata(controller);
-
 	if (fsl_lpspi->is_target) {
 		if (wait_for_completion_interruptible(&fsl_lpspi->xfer_done) ||
 			fsl_lpspi->target_aborted) {
@@ -758,11 +755,9 @@ static int fsl_lpspi_dma_init(struct device *dev,
 	return ret;
 }
 
-static int fsl_lpspi_pio_transfer(struct spi_controller *controller,
+static int fsl_lpspi_pio_transfer(struct fsl_lpspi_data *fsl_lpspi,
 				  struct spi_transfer *t)
 {
-	struct fsl_lpspi_data *fsl_lpspi =
-				spi_controller_get_devdata(controller);
 	int ret;
 
 	fsl_lpspi->tx_buf = t->tx_buf;
@@ -774,7 +769,7 @@ static int fsl_lpspi_pio_transfer(struct spi_controller *controller,
 
 	fsl_lpspi_write_tx_fifo(fsl_lpspi);
 
-	ret = fsl_lpspi_wait_for_completion(controller);
+	ret = fsl_lpspi_wait_for_completion(fsl_lpspi);
 
 	fsl_lpspi_reset(fsl_lpspi);
 
@@ -803,7 +798,7 @@ static int fsl_lpspi_transfer_one(struct spi_controller *controller,
 	if (fsl_lpspi->usedma)
 		ret = fsl_lpspi_dma_transfer(controller, fsl_lpspi, t);
 	else
-		ret = fsl_lpspi_pio_transfer(controller, t);
+		ret = fsl_lpspi_pio_transfer(fsl_lpspi, t);
 	if (ret < 0)
 		return ret;
 

-- 
2.53.0

[PATCH spi-next 2/3] spi: spi-fsl-lpspi: fsl_lpspi_transfer_one(): directly return ret

[Marc Kleine-Budde]

Both fsl_lpspi_dma_transfer() and fsl_lpspi_pio_transfer() only return
negative error values in case of error or 0 in case of success.

Directly return ret, there is no need to check for positive error values
and return them as 0.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/spi/spi-fsl-lpspi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
index a3c5bab1acc7..b25f94b2ef73 100644
--- a/drivers/spi/spi-fsl-lpspi.c
+++ b/drivers/spi/spi-fsl-lpspi.c
@@ -799,10 +799,8 @@ static int fsl_lpspi_transfer_one(struct spi_controller *controller,
 		ret = fsl_lpspi_dma_transfer(controller, fsl_lpspi, t);
 	else
 		ret = fsl_lpspi_pio_transfer(fsl_lpspi, t);
-	if (ret < 0)
-		return ret;
 
-	return 0;
+	return ret;
 }
 
 static irqreturn_t fsl_lpspi_isr(int irq, void *dev_id)

-- 
2.53.0

[PATCH spi-next 3/3] spi: spi-fsl-lpspi: cache content of Interrupt Enable Register (IER)

[Marc Kleine-Budde]

To improve performance avoid reading the Interrupt Enable Register (IER)
from the hardware, cache in struct fsl_lpspi_data::reg_ier.

Introduce struct fsl_lpspi_data::reg_ier, to cache the current value.
Extend fsl_lpspi_intctrl() to update the cache and replace the open coded
writing of the IRQ register in fsl_lpspi_prepare_message() with
fsl_lpspi_intctrl().

In fsl_lpspi_isr() don't read the IER from the hardware, but use the cached
value from struct fsl_lpspi_data::reg_ier.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/spi/spi-fsl-lpspi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
index b25f94b2ef73..5b158b0cb5af 100644
--- a/drivers/spi/spi-fsl-lpspi.c
+++ b/drivers/spi/spi-fsl-lpspi.c
@@ -122,6 +122,7 @@ struct fsl_lpspi_data {
 	void (*rx)(struct fsl_lpspi_data *fsl_lpspi);
 
 	u32 remain;
+	u32 reg_ier;
 	u8 watermark;
 	u8 txfifosize;
 	u8 rxfifosize;
@@ -199,6 +200,7 @@ LPSPI_BUF_TX(u32)
 static void fsl_lpspi_intctrl(struct fsl_lpspi_data *fsl_lpspi,
 			      unsigned int enable)
 {
+	fsl_lpspi->reg_ier = enable;
 	writel(enable, fsl_lpspi->base + IMX7ULP_IER);
 }
 
@@ -525,7 +527,7 @@ static int fsl_lpspi_prepare_message(struct spi_controller *controller,
 	fsl_lpspi_set_cmd(fsl_lpspi);
 
 	/* No IRQs */
-	writel(0, fsl_lpspi->base + IMX7ULP_IER);
+	fsl_lpspi_intctrl(fsl_lpspi, 0);
 
 	/* Controller disable, clear FIFOs, clear status */
 	writel(CR_RRF | CR_RTF, fsl_lpspi->base + IMX7ULP_CR);
@@ -808,7 +810,7 @@ static irqreturn_t fsl_lpspi_isr(int irq, void *dev_id)
 	u32 temp_SR, temp_IER;
 	struct fsl_lpspi_data *fsl_lpspi = dev_id;
 
-	temp_IER = readl(fsl_lpspi->base + IMX7ULP_IER);
+	temp_IER = fsl_lpspi->reg_ier;
 	fsl_lpspi_intctrl(fsl_lpspi, 0);
 	temp_SR = readl(fsl_lpspi->base + IMX7ULP_SR);
 

-- 
2.53.0

Re: [PATCH spi-next 1/3] spi: spi-fsl-lpspi: pass struct fsl_lpspi_data instead of struct spi_controller if possible

[Frank Li]

On Fri, Oct 09, 2026 at 12:16:35PM +0200, Marc Kleine-Budde wrote:
> Instead of passing a spi_controller pointer to functions and using
> spi_controller_get_devdata() to get the struct fsl_lpspi_data pointer,
> directly pass a struct fsl_lpspi_data pointer.
>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---

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

>  drivers/spi/spi-fsl-lpspi.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
> index 5aa0a75b4e57..a3c5bab1acc7 100644
> --- a/drivers/spi/spi-fsl-lpspi.c
> +++ b/drivers/spi/spi-fsl-lpspi.c
> @@ -550,11 +550,8 @@ static int fsl_lpspi_target_abort(struct spi_controller *controller)
>  	return 0;
>  }
>
> -static int fsl_lpspi_wait_for_completion(struct spi_controller *controller)
> +static int fsl_lpspi_wait_for_completion(struct fsl_lpspi_data *fsl_lpspi)
>  {
> -	struct fsl_lpspi_data *fsl_lpspi =
> -				spi_controller_get_devdata(controller);
> -
>  	if (fsl_lpspi->is_target) {
>  		if (wait_for_completion_interruptible(&fsl_lpspi->xfer_done) ||
>  			fsl_lpspi->target_aborted) {
> @@ -758,11 +755,9 @@ static int fsl_lpspi_dma_init(struct device *dev,
>  	return ret;
>  }
>
> -static int fsl_lpspi_pio_transfer(struct spi_controller *controller,
> +static int fsl_lpspi_pio_transfer(struct fsl_lpspi_data *fsl_lpspi,
>  				  struct spi_transfer *t)
>  {
> -	struct fsl_lpspi_data *fsl_lpspi =
> -				spi_controller_get_devdata(controller);
>  	int ret;
>
>  	fsl_lpspi->tx_buf = t->tx_buf;
> @@ -774,7 +769,7 @@ static int fsl_lpspi_pio_transfer(struct spi_controller *controller,
>
>  	fsl_lpspi_write_tx_fifo(fsl_lpspi);
>
> -	ret = fsl_lpspi_wait_for_completion(controller);
> +	ret = fsl_lpspi_wait_for_completion(fsl_lpspi);
>
>  	fsl_lpspi_reset(fsl_lpspi);
>
> @@ -803,7 +798,7 @@ static int fsl_lpspi_transfer_one(struct spi_controller *controller,
>  	if (fsl_lpspi->usedma)
>  		ret = fsl_lpspi_dma_transfer(controller, fsl_lpspi, t);
>  	else
> -		ret = fsl_lpspi_pio_transfer(controller, t);
> +		ret = fsl_lpspi_pio_transfer(fsl_lpspi, t);
>  	if (ret < 0)
>  		return ret;
>
>
> --
> 2.53.0
>
>

Re: [PATCH spi-next 2/3] spi: spi-fsl-lpspi: fsl_lpspi_transfer_one(): directly return ret

[Frank Li]

On Fri, Oct 09, 2026 at 12:16:36PM +0200, Marc Kleine-Budde wrote:

subject:

spi: spi-fsl-lpspi: directly return ret in fsl_lpspi_transfer_one().

Frank

> Both fsl_lpspi_dma_transfer() and fsl_lpspi_pio_transfer() only return
> negative error values in case of error or 0 in case of success.
>
> Directly return ret, there is no need to check for positive error values
> and return them as 0.
>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/spi/spi-fsl-lpspi.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
> index a3c5bab1acc7..b25f94b2ef73 100644
> --- a/drivers/spi/spi-fsl-lpspi.c
> +++ b/drivers/spi/spi-fsl-lpspi.c
> @@ -799,10 +799,8 @@ static int fsl_lpspi_transfer_one(struct spi_controller *controller,
>  		ret = fsl_lpspi_dma_transfer(controller, fsl_lpspi, t);
>  	else
>  		ret = fsl_lpspi_pio_transfer(fsl_lpspi, t);
> -	if (ret < 0)
> -		return ret;
>
> -	return 0;
> +	return ret;
>  }
>
>  static irqreturn_t fsl_lpspi_isr(int irq, void *dev_id)
>
> --
> 2.53.0
>
>

Re: [PATCH spi-next 3/3] spi: spi-fsl-lpspi: cache content of Interrupt Enable Register (IER)

[Frank Li]

On Fri, Oct 09, 2026 at 12:16:37PM +0200, Marc Kleine-Budde wrote:
> To improve performance avoid reading the Interrupt Enable Register (IER)
> from the hardware, cache in struct fsl_lpspi_data::reg_ier.

Do you have bench mark data? I am not sure how much gain by only avoid
a register read.

Frank

>
> Introduce struct fsl_lpspi_data::reg_ier, to cache the current value.
> Extend fsl_lpspi_intctrl() to update the cache and replace the open coded
> writing of the IRQ register in fsl_lpspi_prepare_message() with
> fsl_lpspi_intctrl().
>
> In fsl_lpspi_isr() don't read the IER from the hardware, but use the cached
> value from struct fsl_lpspi_data::reg_ier.
>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/spi/spi-fsl-lpspi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
> index b25f94b2ef73..5b158b0cb5af 100644
> --- a/drivers/spi/spi-fsl-lpspi.c
> +++ b/drivers/spi/spi-fsl-lpspi.c
> @@ -122,6 +122,7 @@ struct fsl_lpspi_data {
>  	void (*rx)(struct fsl_lpspi_data *fsl_lpspi);
>
>  	u32 remain;
> +	u32 reg_ier;
>  	u8 watermark;
>  	u8 txfifosize;
>  	u8 rxfifosize;
> @@ -199,6 +200,7 @@ LPSPI_BUF_TX(u32)
>  static void fsl_lpspi_intctrl(struct fsl_lpspi_data *fsl_lpspi,
>  			      unsigned int enable)
>  {
> +	fsl_lpspi->reg_ier = enable;
>  	writel(enable, fsl_lpspi->base + IMX7ULP_IER);
>  }
>
> @@ -525,7 +527,7 @@ static int fsl_lpspi_prepare_message(struct spi_controller *controller,
>  	fsl_lpspi_set_cmd(fsl_lpspi);
>
>  	/* No IRQs */
> -	writel(0, fsl_lpspi->base + IMX7ULP_IER);
> +	fsl_lpspi_intctrl(fsl_lpspi, 0);
>
>  	/* Controller disable, clear FIFOs, clear status */
>  	writel(CR_RRF | CR_RTF, fsl_lpspi->base + IMX7ULP_CR);
> @@ -808,7 +810,7 @@ static irqreturn_t fsl_lpspi_isr(int irq, void *dev_id)
>  	u32 temp_SR, temp_IER;
>  	struct fsl_lpspi_data *fsl_lpspi = dev_id;
>
> -	temp_IER = readl(fsl_lpspi->base + IMX7ULP_IER);
> +	temp_IER = fsl_lpspi->reg_ier;
>  	fsl_lpspi_intctrl(fsl_lpspi, 0);
>  	temp_SR = readl(fsl_lpspi->base + IMX7ULP_SR);
>
>
> --
> 2.53.0
>
>