[PATCH v2 0/4] spi: cadence-qspi: Fix runtime PM and system-wide suspend

[PATCH v2 0/4] spi: cadence-qspi: Fix runtime PM and system-wide suspend

[Théo Lebrun]

Hi,

This V2 got a false start, hoping you got a good laugh out of it. :-)
Sorry for the spam.

This fixes runtime PM and system-wide suspend for the cadence-qspi
driver. Seeing how runtime PM and autosuspend are enabled by default, I
believe this affects all users of the driver.

The V1 has been split into three commits and a fourth commit is added to
add system-wide suspend/resume callbacks as discussed under V1.

The MIPS platform at hand, used for debugging and testing, is currently
not supported by the driver. It is the Mobileye EyeQ5 [0]. No code
changes are required for support, only a new compatible and appropriate
match data + flags. That will come later, with some performance-related
patches.

This series has been tested on both Mobileye EyeQ5 hardware and the TI
J7200 EVM board, under s2idle.

Thanks all,
Théo

[0]: https://lore.kernel.org/lkml/20240118155252.397947-1-gregory.clement@bootlin.com/

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Changes in v2:
- Split the initial change into three separate commits, to make intents
  clearer.
- Mark controller as suspended during the system-wide suspend.
- Link to v1: https://lore.kernel.org/r/20240202-cdns-qspi-pm-fix-v1-1-3c8feb2bfdd8@bootlin.com

---
Théo Lebrun (4):
      spi: cadence-qspi: put runtime in runtime PM hooks names
      spi: cadence-qspi: fix pointer reference in runtime PM hooks
      spi: cadence-qspi: remove system-wide suspend helper calls from runtime PM hooks
      spi: cadence-qspi: add system-wide suspend and resume callbacks

 drivers/spi/spi-cadence-quadspi.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)
---
base-commit: 13acce918af915278e49980a3038df31845dbf39
change-id: 20240202-cdns-qspi-pm-fix-29600cc6d7bf

Best regards,
-- 
Théo Lebrun <theo.lebrun@bootlin.com>

[PATCH v2 1/4] spi: cadence-qspi: put runtime in runtime PM hooks names

[Théo Lebrun]

Follow kernel naming convention with regards to power-management
callback function names.

The convention in the kernel is:
 - prefix_suspend means the system-wide suspend callback;
 - prefix_runtime_suspend means the runtime PM suspend callback.
The same applies to resume callbacks.

Fixes: 0578a6dbfe75 ("spi: spi-cadence-quadspi: add runtime pm support")
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/spi/spi-cadence-quadspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 74647dfcb86c..720b28d2980c 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1927,7 +1927,7 @@ static void cqspi_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 }
 
-static int cqspi_suspend(struct device *dev)
+static int cqspi_runtime_suspend(struct device *dev)
 {
 	struct cqspi_st *cqspi = dev_get_drvdata(dev);
 	struct spi_controller *host = dev_get_drvdata(dev);
@@ -1941,7 +1941,7 @@ static int cqspi_suspend(struct device *dev)
 	return ret;
 }
 
-static int cqspi_resume(struct device *dev)
+static int cqspi_runtime_resume(struct device *dev)
 {
 	struct cqspi_st *cqspi = dev_get_drvdata(dev);
 	struct spi_controller *host = dev_get_drvdata(dev);
@@ -1956,8 +1956,8 @@ static int cqspi_resume(struct device *dev)
 	return spi_controller_resume(host);
 }
 
-static DEFINE_RUNTIME_DEV_PM_OPS(cqspi_dev_pm_ops, cqspi_suspend,
-				 cqspi_resume, NULL);
+static DEFINE_RUNTIME_DEV_PM_OPS(cqspi_dev_pm_ops, cqspi_runtime_suspend,
+				 cqspi_runtime_resume, NULL);
 
 static const struct cqspi_driver_platdata cdns_qspi = {
 	.quirks = CQSPI_DISABLE_DAC_MODE,

-- 
2.43.0

Re: [PATCH v2 1/4] spi: cadence-qspi: put runtime in runtime PM hooks names

[Dhruva Gole]

On Feb 05, 2024 at 15:57:29 +0100, Théo Lebrun wrote:
> Follow kernel naming convention with regards to power-management
> callback function names.
> 
> The convention in the kernel is:
>  - prefix_suspend means the system-wide suspend callback;
>  - prefix_runtime_suspend means the runtime PM suspend callback.
> The same applies to resume callbacks.
> 
> Fixes: 0578a6dbfe75 ("spi: spi-cadence-quadspi: add runtime pm support")

Not sure if it's a bug as such since there's no functional change other
than renaming.

> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/spi/spi-cadence-quadspi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 74647dfcb86c..720b28d2980c 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1927,7 +1927,7 @@ static void cqspi_remove(struct platform_device *pdev)
>  	pm_runtime_disable(&pdev->dev);
>  }
>  
> -static int cqspi_suspend(struct device *dev)
> +static int cqspi_runtime_suspend(struct device *dev)
>  {
>  	struct cqspi_st *cqspi = dev_get_drvdata(dev);
>  	struct spi_controller *host = dev_get_drvdata(dev);
> @@ -1941,7 +1941,7 @@ static int cqspi_suspend(struct device *dev)
>  	return ret;
>  }
>  
> -static int cqspi_resume(struct device *dev)
> +static int cqspi_runtime_resume(struct device *dev)
>  {
>  	struct cqspi_st *cqspi = dev_get_drvdata(dev);
>  	struct spi_controller *host = dev_get_drvdata(dev);
> @@ -1956,8 +1956,8 @@ static int cqspi_resume(struct device *dev)
>  	return spi_controller_resume(host);
>  }
>  
> -static DEFINE_RUNTIME_DEV_PM_OPS(cqspi_dev_pm_ops, cqspi_suspend,
> -				 cqspi_resume, NULL);
> +static DEFINE_RUNTIME_DEV_PM_OPS(cqspi_dev_pm_ops, cqspi_runtime_suspend,
> +				 cqspi_runtime_resume, NULL);
>  


No objections as such,
Reviewed-by: Dhruva Gole <d-gole@ti.com>

-- 
Best regards,
Dhruva Gole <d-gole@ti.com>

Re: [PATCH v2 1/4] spi: cadence-qspi: put runtime in runtime PM hooks names

[Théo Lebrun]

Hello,

On Wed Feb 7, 2024 at 9:33 AM CET, Dhruva Gole wrote:
> On Feb 05, 2024 at 15:57:29 +0100, Théo Lebrun wrote:
> > Follow kernel naming convention with regards to power-management
> > callback function names.
> > 
> > The convention in the kernel is:
> >  - prefix_suspend means the system-wide suspend callback;
> >  - prefix_runtime_suspend means the runtime PM suspend callback.
> > The same applies to resume callbacks.
> > 
> > Fixes: 0578a6dbfe75 ("spi: spi-cadence-quadspi: add runtime pm support")
>
> Not sure if it's a bug as such since there's no functional change other
> than renaming.

I see where you come from. I'll fix it when/if there is a second revision.

[...]

> No objections as such,
> Reviewed-by: Dhruva Gole <d-gole@ti.com>

Thanks!

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 2/4] spi: cadence-qspi: fix pointer reference in runtime PM hooks

[Théo Lebrun]

dev_get_drvdata() gets used to acquire the pointer to cqspi and the SPI
controller. Neither embed the other; this lead to memory corruption.

On a given platform (Mobileye EyeQ5) the memory corruption is hidden
inside cqspi->f_pdata. Also, this uninitialised memory is used as a
mutex (ctlr->bus_lock_mutex) by spi_controller_suspend().

Fixes: 2087e85bb66e ("spi: cadence-quadspi: fix suspend-resume implementations")
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/spi/spi-cadence-quadspi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 720b28d2980c..1a27987638f0 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1930,10 +1930,9 @@ static void cqspi_remove(struct platform_device *pdev)
 static int cqspi_runtime_suspend(struct device *dev)
 {
 	struct cqspi_st *cqspi = dev_get_drvdata(dev);
-	struct spi_controller *host = dev_get_drvdata(dev);
 	int ret;
 
-	ret = spi_controller_suspend(host);
+	ret = spi_controller_suspend(cqspi->host);
 	cqspi_controller_enable(cqspi, 0);
 
 	clk_disable_unprepare(cqspi->clk);
@@ -1944,7 +1943,6 @@ static int cqspi_runtime_suspend(struct device *dev)
 static int cqspi_runtime_resume(struct device *dev)
 {
 	struct cqspi_st *cqspi = dev_get_drvdata(dev);
-	struct spi_controller *host = dev_get_drvdata(dev);
 
 	clk_prepare_enable(cqspi->clk);
 	cqspi_wait_idle(cqspi);
@@ -1953,7 +1951,7 @@ static int cqspi_runtime_resume(struct device *dev)
 	cqspi->current_cs = -1;
 	cqspi->sclk = 0;
 
-	return spi_controller_resume(host);
+	return spi_controller_resume(cqspi->host);
 }
 
 static DEFINE_RUNTIME_DEV_PM_OPS(cqspi_dev_pm_ops, cqspi_runtime_suspend,

-- 
2.43.0

Re: [PATCH v2 2/4] spi: cadence-qspi: fix pointer reference in runtime PM hooks

[Mark Brown]

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

On Mon, Feb 05, 2024 at 03:57:30PM +0100, Théo Lebrun wrote:
> dev_get_drvdata() gets used to acquire the pointer to cqspi and the SPI
> controller. Neither embed the other; this lead to memory corruption.
> 
> On a given platform (Mobileye EyeQ5) the memory corruption is hidden
> inside cqspi->f_pdata. Also, this uninitialised memory is used as a
> mutex (ctlr->bus_lock_mutex) by spi_controller_suspend().

Please place fixes at the start of serieses so that they don't end up
with spurious dependencies on other changes and can more easily be
applied as fixes.

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

Re: [PATCH v2 2/4] spi: cadence-qspi: fix pointer reference in runtime PM hooks

[Dhruva Gole]

Hi Mark,

On Feb 05, 2024 at 15:12:10 +0000, Mark Brown wrote:
> On Mon, Feb 05, 2024 at 03:57:30PM +0100, Théo Lebrun wrote:
> > dev_get_drvdata() gets used to acquire the pointer to cqspi and the SPI
> > controller. Neither embed the other; this lead to memory corruption.
> > 
> > On a given platform (Mobileye EyeQ5) the memory corruption is hidden
> > inside cqspi->f_pdata. Also, this uninitialised memory is used as a
> > mutex (ctlr->bus_lock_mutex) by spi_controller_suspend().
> 
> Please place fixes at the start of serieses so that they don't end up
> with spurious dependencies on other changes and can more easily be
> applied as fixes.

Didn't really understand the comment here, aren't the 1,2 and 3 patches
fixes and the last one the non-fix? Thus fixes are indeed placed at
start of this series right?

Can you help understand with some example series?

-- 
Best regards,
Dhruva Gole <d-gole@ti.com>

Re: [PATCH v2 2/4] spi: cadence-qspi: fix pointer reference in runtime PM hooks

[Mark Brown]

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

On Wed, Feb 07, 2024 at 02:09:02PM +0530, Dhruva Gole wrote:
> On Feb 05, 2024 at 15:12:10 +0000, Mark Brown wrote:

> > Please place fixes at the start of serieses so that they don't end up
> > with spurious dependencies on other changes and can more easily be
> > applied as fixes.

> Didn't really understand the comment here, aren't the 1,2 and 3 patches
> fixes and the last one the non-fix? Thus fixes are indeed placed at
> start of this series right?

Patch 1 is a rename, this is obviously cosmetic and not a bug fix.

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

Re: [PATCH v2 2/4] spi: cadence-qspi: fix pointer reference in runtime PM hooks

[Dhruva Gole]

Hey,

On Feb 07, 2024 at 09:50:16 +0000, Mark Brown wrote:
> On Wed, Feb 07, 2024 at 02:09:02PM +0530, Dhruva Gole wrote:
> > On Feb 05, 2024 at 15:12:10 +0000, Mark Brown wrote:
> 
> > > Please place fixes at the start of serieses so that they don't end up
> > > with spurious dependencies on other changes and can more easily be
> > > applied as fixes.
> 
> > Didn't really understand the comment here, aren't the 1,2 and 3 patches
> > fixes and the last one the non-fix? Thus fixes are indeed placed at
> > start of this series right?
> 
> Patch 1 is a rename, this is obviously cosmetic and not a bug fix.


Well, Theo, seems like you better fix the first patch, then reorder and
send a v3 :)


-- 
Best regards,
Dhruva Gole <d-gole@ti.com>

Re: [PATCH v2 2/4] spi: cadence-qspi: fix pointer reference in runtime PM hooks

[Dhruva Gole]

On Feb 05, 2024 at 15:57:30 +0100, Théo Lebrun wrote:
> dev_get_drvdata() gets used to acquire the pointer to cqspi and the SPI
> controller. Neither embed the other; this lead to memory corruption.
> 
> On a given platform (Mobileye EyeQ5) the memory corruption is hidden
> inside cqspi->f_pdata. Also, this uninitialised memory is used as a
> mutex (ctlr->bus_lock_mutex) by spi_controller_suspend().
> 
> Fixes: 2087e85bb66e ("spi: cadence-quadspi: fix suspend-resume implementations")
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/spi/spi-cadence-quadspi.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 720b28d2980c..1a27987638f0 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1930,10 +1930,9 @@ static void cqspi_remove(struct platform_device *pdev)
>  static int cqspi_runtime_suspend(struct device *dev)
>  {
>  	struct cqspi_st *cqspi = dev_get_drvdata(dev);
> -	struct spi_controller *host = dev_get_drvdata(dev);

Or you could do:
+	struct spi_controller *host = cqspi->host;

>  	int ret;
>  
> -	ret = spi_controller_suspend(host);
> +	ret = spi_controller_suspend(cqspi->host);

And avoid changing these?

>  	cqspi_controller_enable(cqspi, 0);
>  
>  	clk_disable_unprepare(cqspi->clk);
> @@ -1944,7 +1943,6 @@ static int cqspi_runtime_suspend(struct device *dev)
>  static int cqspi_runtime_resume(struct device *dev)
>  {
>  	struct cqspi_st *cqspi = dev_get_drvdata(dev);
> -	struct spi_controller *host = dev_get_drvdata(dev);
>  
>  	clk_prepare_enable(cqspi->clk);
>  	cqspi_wait_idle(cqspi);
> @@ -1953,7 +1951,7 @@ static int cqspi_runtime_resume(struct device *dev)
>  	cqspi->current_cs = -1;
>  	cqspi->sclk = 0;
>  
> -	return spi_controller_resume(host);
> +	return spi_controller_resume(cqspi->host);

ditto.

Thanks,
Dhruva Gole <d-gole@ti.com>

>  }
>  
>  static DEFINE_RUNTIME_DEV_PM_OPS(cqspi_dev_pm_ops, cqspi_runtime_suspend,
> 
> -- 
> 2.43.0
> 
> 

Re: [PATCH v2 2/4] spi: cadence-qspi: fix pointer reference in runtime PM hooks

[Théo Lebrun]

Hello,

On Wed Feb 7, 2024 at 9:42 AM CET, Dhruva Gole wrote:
> On Feb 05, 2024 at 15:57:30 +0100, Théo Lebrun wrote:
> > dev_get_drvdata() gets used to acquire the pointer to cqspi and the SPI
> > controller. Neither embed the other; this lead to memory corruption.
> > 
> > On a given platform (Mobileye EyeQ5) the memory corruption is hidden
> > inside cqspi->f_pdata. Also, this uninitialised memory is used as a
> > mutex (ctlr->bus_lock_mutex) by spi_controller_suspend().
> > 
> > Fixes: 2087e85bb66e ("spi: cadence-quadspi: fix suspend-resume implementations")
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  drivers/spi/spi-cadence-quadspi.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> > index 720b28d2980c..1a27987638f0 100644
> > --- a/drivers/spi/spi-cadence-quadspi.c
> > +++ b/drivers/spi/spi-cadence-quadspi.c
> > @@ -1930,10 +1930,9 @@ static void cqspi_remove(struct platform_device *pdev)
> >  static int cqspi_runtime_suspend(struct device *dev)
> >  {
> >  	struct cqspi_st *cqspi = dev_get_drvdata(dev);
> > -	struct spi_controller *host = dev_get_drvdata(dev);
>
> Or you could do:
> +	struct spi_controller *host = cqspi->host;

Indeed. I preferred minimizing line count as I didn't see a benefit to
introducing a new variable. It goes away new patch anyway. If you
prefer it this way tell me and I'll fix it for next revision.

Thanks Dhruva,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 2/4] spi: cadence-qspi: fix pointer reference in runtime PM hooks

[Dhruva Gole]

On Feb 07, 2024 at 10:28:59 +0100, Théo Lebrun wrote:
> Hello,
> 
> On Wed Feb 7, 2024 at 9:42 AM CET, Dhruva Gole wrote:
> > On Feb 05, 2024 at 15:57:30 +0100, Théo Lebrun wrote:
> > > dev_get_drvdata() gets used to acquire the pointer to cqspi and the SPI
> > > controller. Neither embed the other; this lead to memory corruption.
> > > 
> > > On a given platform (Mobileye EyeQ5) the memory corruption is hidden
> > > inside cqspi->f_pdata. Also, this uninitialised memory is used as a
> > > mutex (ctlr->bus_lock_mutex) by spi_controller_suspend().
> > > 
> > > Fixes: 2087e85bb66e ("spi: cadence-quadspi: fix suspend-resume implementations")
> > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > > ---
> > >  drivers/spi/spi-cadence-quadspi.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> > > index 720b28d2980c..1a27987638f0 100644
> > > --- a/drivers/spi/spi-cadence-quadspi.c
> > > +++ b/drivers/spi/spi-cadence-quadspi.c
> > > @@ -1930,10 +1930,9 @@ static void cqspi_remove(struct platform_device *pdev)
> > >  static int cqspi_runtime_suspend(struct device *dev)
> > >  {
> > >  	struct cqspi_st *cqspi = dev_get_drvdata(dev);
> > > -	struct spi_controller *host = dev_get_drvdata(dev);
> >
> > Or you could do:
> > +	struct spi_controller *host = cqspi->host;
> 
> Indeed. I preferred minimizing line count as I didn't see a benefit to
> introducing a new variable. It goes away new patch anyway. If you
> prefer it this way tell me and I'll fix it for next revision.

I mean since you're going to have to respin then do make this change, it
will further minimise the number of lines of change right?

It goes away in last patch but if atall in some older kernel only
suspend resume support is there then only this will get picked so it's
still not useless code.


-- 
Best regards,
Dhruva Gole <d-gole@ti.com>

[PATCH v2 3/4] spi: cadence-qspi: remove system-wide suspend helper calls from runtime PM hooks

[Théo Lebrun]

The ->runtime_suspend() and ->runtime_resume() callbacks are not
expected to call spi_controller_suspend() and spi_controller_resume().
Remove calls to those in the cadence-qspi driver.

Those helpers have two roles currently:
 - They stop/start the queue, including dealing with the kworker.
 - They toggle the SPI controller SPI_CONTROLLER_SUSPENDED flag. It
   requires acquiring ctlr->bus_lock_mutex.

Step one is irrelevant because cadence-qspi is not queued. Step two
however has two implications:
 - A deadlock occurs, because ->runtime_resume() is called in a context
   where the lock is already taken (in the ->exec_op() callback, where
   the usage count is incremented).
 - It would disallow all operations once the device is auto-suspended.

Here is a brief call tree highlighting the mutex deadlock:

spi_mem_exec_op()
        ...
        spi_mem_access_start()
                mutex_lock(&ctlr->bus_lock_mutex)

        cqspi_exec_mem_op()
                pm_runtime_resume_and_get()
                        cqspi_resume()
                                spi_controller_resume()
                                        mutex_lock(&ctlr->bus_lock_mutex)
                ...

        spi_mem_access_end()
                mutex_unlock(&ctlr->bus_lock_mutex)
        ...

Fixes: 0578a6dbfe75 ("spi: spi-cadence-quadspi: add runtime pm support")
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/spi/spi-cadence-quadspi.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 1a27987638f0..ee14965142ba 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1930,14 +1930,10 @@ static void cqspi_remove(struct platform_device *pdev)
 static int cqspi_runtime_suspend(struct device *dev)
 {
 	struct cqspi_st *cqspi = dev_get_drvdata(dev);
-	int ret;
 
-	ret = spi_controller_suspend(cqspi->host);
 	cqspi_controller_enable(cqspi, 0);
-
 	clk_disable_unprepare(cqspi->clk);
-
-	return ret;
+	return 0;
 }
 
 static int cqspi_runtime_resume(struct device *dev)
@@ -1950,8 +1946,7 @@ static int cqspi_runtime_resume(struct device *dev)
 
 	cqspi->current_cs = -1;
 	cqspi->sclk = 0;
-
-	return spi_controller_resume(cqspi->host);
+	return 0;
 }
 
 static DEFINE_RUNTIME_DEV_PM_OPS(cqspi_dev_pm_ops, cqspi_runtime_suspend,

-- 
2.43.0

[PATCH v2 4/4] spi: cadence-qspi: add system-wide suspend and resume callbacks

[Théo Lebrun]

Each SPI controller is expected to call the spi_controller_suspend() and
spi_controller_resume() callbacks at system-wide suspend and resume.

It (1) handles the kthread worker for queued controllers and (2) marks
the controller as suspended to have spi_sync() fail while the
controller is unavailable.

Those two operations do not require the controller to be active, we do
not need to increment the runtime PM usage counter.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/spi/spi-cadence-quadspi.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index ee14965142ba..f976681187b0 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1949,8 +1949,24 @@ static int cqspi_runtime_resume(struct device *dev)
 	return 0;
 }
 
-static DEFINE_RUNTIME_DEV_PM_OPS(cqspi_dev_pm_ops, cqspi_runtime_suspend,
-				 cqspi_runtime_resume, NULL);
+static int cqspi_suspend(struct device *dev)
+{
+	struct cqspi_st *cqspi = dev_get_drvdata(dev);
+
+	return spi_controller_suspend(cqspi->host);
+}
+
+static int cqspi_resume(struct device *dev)
+{
+	struct cqspi_st *cqspi = dev_get_drvdata(dev);
+
+	return spi_controller_resume(cqspi->host);
+}
+
+static const struct dev_pm_ops cqspi_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(cqspi_runtime_suspend, cqspi_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(cqspi_suspend, cqspi_resume)
+};
 
 static const struct cqspi_driver_platdata cdns_qspi = {
 	.quirks = CQSPI_DISABLE_DAC_MODE,

-- 
2.43.0