[PATCH 0/2] Fix PM Hooks in the Cadence Quad SPI Driver

[PATCH 0/2] Fix PM Hooks in the Cadence Quad SPI Driver

[Dhruva Gole]

This series adds fixes to the Suspend and Resume Hooks of the CQSPI Driver.
Earlier, the ltp-ddt test "OSPI_S_FUNC_DD_RW_ERASESIZE_UBIFS" failed
post system suspend resume. It is now passing as shown in [0].

Tested on top of linux-next (next-20230414) on an AM625-SK-EVM. For logs,
please refer [0].

[0] https://gist.github.com/DhruvaG2000/29b15f9c8a1e632c6819a986e5f28ab2

Dhruva Gole (2):
  spi: cadence-quadspi: use macro SIMPLE_DEV_PM_OPS
  spi: cadence-quadspi: fix suspend-resume implementations

 drivers/spi/spi-cadence-quadspi.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH 1/2] spi: cadence-quadspi: use macro SIMPLE_DEV_PM_OPS

[Dhruva Gole]

Using this macro makes the code more readable.
It also inits the members of dev_pm_ops in the following manner
without us explicitly needing to:

.suspend = cqspi_suspend, \
.resume = cqspi_resume, \
.freeze = suspend_fn, \
.thaw = resume_fn, \
.poweroff = suspend_fn, \
.restore = resume_fn,

Fixes: 140623410536 ("mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller")
Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 79ab7e309644..d47e954fe809 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1829,10 +1829,7 @@ static int cqspi_resume(struct device *dev)
 	return 0;
 }
 
-static const struct dev_pm_ops cqspi__dev_pm_ops = {
-	.suspend = cqspi_suspend,
-	.resume = cqspi_resume,
-};
+static SIMPLE_DEV_PM_OPS(cqspi__dev_pm_ops, cqspi_suspend, cqspi_resume);
 
 #define CQSPI_DEV_PM_OPS	(&cqspi__dev_pm_ops)
 #else
-- 
2.25.1

Re: [PATCH 1/2] spi: cadence-quadspi: use macro SIMPLE_DEV_PM_OPS

[Mark Brown]

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

On Mon, Apr 17, 2023 at 02:40:26PM +0530, Dhruva Gole wrote:

> -static const struct dev_pm_ops cqspi__dev_pm_ops = {
> -	.suspend = cqspi_suspend,
> -	.resume = cqspi_resume,
> -};
> +static SIMPLE_DEV_PM_OPS(cqspi__dev_pm_ops, cqspi_suspend, cqspi_resume);
>  
>  #define CQSPI_DEV_PM_OPS	(&cqspi__dev_pm_ops)
>  #else

These days you should use DEFINE_SIMPLE_DEV_PM_OPS() instead.

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

Re: [PATCH 1/2] spi: cadence-quadspi: use macro SIMPLE_DEV_PM_OPS

[Gole, Dhruva]

Hi Mark, Thanks for going through my patch.

On 4/18/2023 6:22 PM, Mark Brown wrote:
> On Mon, Apr 17, 2023 at 02:40:26PM +0530, Dhruva Gole wrote:
>
>> -static const struct dev_pm_ops cqspi__dev_pm_ops = {
>> -	.suspend = cqspi_suspend,
>> -	.resume = cqspi_resume,
>> -};
>> +static SIMPLE_DEV_PM_OPS(cqspi__dev_pm_ops, cqspi_suspend, cqspi_resume);
>>  
>>  #define CQSPI_DEV_PM_OPS	(&cqspi__dev_pm_ops)
>>  #else
> These days you should use DEFINE_SIMPLE_DEV_PM_OPS() instead.


Sure, I can respin this series with this macro instead,

https://elixir.bootlin.com/linux/v6.1/source/include/linux/pm.h#L399

So essentially it will look like,

static DEFINE_SIMPLE_DEV_PM_OPS(cqspi__dev_pm_ops, cqspi_suspend, cqspi_resume);


Are there any other improvements you'd like me to make in my next revision of this series?

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

Re: [PATCH 1/2] spi: cadence-quadspi: use macro SIMPLE_DEV_PM_OPS

[Mark Brown]

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

On Tue, Apr 18, 2023 at 07:47:57PM +0530, Gole, Dhruva wrote:

> Are there any other improvements you'd like me to make in my next revision of this series?

That was it.  I think I queued patch 2 already.

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

[PATCH 2/2] spi: cadence-quadspi: fix suspend-resume implementations

[Dhruva Gole]

The cadence QSPI driver misbehaves after performing a full system suspend
resume:
...
spi-nor spi0.0: resume() failed
...
This results in a flash connected via OSPI interface after system suspend-
resume to be unusable.
fix these suspend and resume functions.

Fixes: 140623410536 ("mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller")
Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index d47e954fe809..91908e8cf92e 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1816,17 +1816,30 @@ static void cqspi_remove(struct platform_device *pdev)
 static int cqspi_suspend(struct device *dev)
 {
 	struct cqspi_st *cqspi = dev_get_drvdata(dev);
+	struct spi_master *master = dev_get_drvdata(dev);
+	int ret;
 
+	ret = spi_master_suspend(master);
 	cqspi_controller_enable(cqspi, 0);
-	return 0;
+
+	clk_disable_unprepare(cqspi->clk);
+
+	return ret;
 }
 
 static int cqspi_resume(struct device *dev)
 {
 	struct cqspi_st *cqspi = dev_get_drvdata(dev);
+	struct spi_master *master = dev_get_drvdata(dev);
 
-	cqspi_controller_enable(cqspi, 1);
-	return 0;
+	clk_prepare_enable(cqspi->clk);
+	cqspi_wait_idle(cqspi);
+	cqspi_controller_init(cqspi);
+
+	cqspi->current_cs = -1;
+	cqspi->sclk = 0;
+
+	return spi_master_resume(master);
 }
 
 static SIMPLE_DEV_PM_OPS(cqspi__dev_pm_ops, cqspi_suspend, cqspi_resume);
-- 
2.25.1

Re: (subset) [PATCH 0/2] Fix PM Hooks in the Cadence Quad SPI Driver

[Mark Brown]

On Mon, 17 Apr 2023 14:40:25 +0530, Dhruva Gole wrote:
> This series adds fixes to the Suspend and Resume Hooks of the CQSPI Driver.
> Earlier, the ltp-ddt test "OSPI_S_FUNC_DD_RW_ERASESIZE_UBIFS" failed
> post system suspend resume. It is now passing as shown in [0].
> 
> Tested on top of linux-next (next-20230414) on an AM625-SK-EVM. For logs,
> please refer [0].
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[2/2] spi: cadence-quadspi: fix suspend-resume implementations
      commit: 2087e85bb66ee3652dafe732bb9b9b896229eafc

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark