[PATCH] spi: dw-mmio: support suspend/resume

[PATCH] spi: dw-mmio: support suspend/resume

[Jisheng Zhang]

Add system wide suspend and resume support, the implementation is
straightforward, just call spi_controller_suspend() then assert the
reset and disable clks for suspend, enable clks and deassert reset
then call spi_controller_resume() for resume.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/spi/spi-dw-mmio.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 33239b4778cb..b8123db4ad55 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -392,6 +392,38 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static int dw_spi_mmio_suspend(struct device *dev)
+{
+	struct dw_spi_mmio *dwsmmio = dev_get_drvdata(dev);
+	int ret;
+
+	ret = dw_spi_suspend_controller(&dwsmmio->dws);
+	if (ret)
+		return ret;
+
+	reset_control_assert(dwsmmio->rstc);
+
+	clk_disable_unprepare(dwsmmio->pclk);
+	clk_disable_unprepare(dwsmmio->clk);
+
+	return 0;
+}
+
+static int dw_spi_mmio_resume(struct device *dev)
+{
+	struct dw_spi_mmio *dwsmmio = dev_get_drvdata(dev);
+
+	clk_prepare_enable(dwsmmio->clk);
+	clk_prepare_enable(dwsmmio->pclk);
+
+	reset_control_deassert(dwsmmio->rstc);
+
+	return dw_spi_resume_controller(&dwsmmio->dws);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(dw_spi_mmio_pm_ops,
+				dw_spi_mmio_suspend, dw_spi_mmio_resume);
+
 static void dw_spi_mmio_remove(struct platform_device *pdev)
 {
 	struct dw_spi_mmio *dwsmmio = platform_get_drvdata(pdev);
@@ -435,6 +467,7 @@ static struct platform_driver dw_spi_mmio_driver = {
 		.name	= DRIVER_NAME,
 		.of_match_table = dw_spi_mmio_of_match,
 		.acpi_match_table = ACPI_PTR(dw_spi_mmio_acpi_match),
+		.pm	= pm_sleep_ptr(&dw_spi_mmio_pm_ops),
 	},
 };
 module_platform_driver(dw_spi_mmio_driver);
-- 
2.51.0

Re: [PATCH] spi: dw-mmio: support suspend/resume

[Mark Brown]

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

On Thu, Jan 22, 2026 at 11:50:46PM +0800, Jisheng Zhang wrote:

> +static int dw_spi_mmio_suspend(struct device *dev)
> +{

> +	clk_disable_unprepare(dwsmmio->pclk);
> +	clk_disable_unprepare(dwsmmio->clk);

These were allocated using devm_clk_get_prepare_enable() so we shouldn't
really be fiddling with the state at runtime.  In practice this should
always be fine I think but it's not really something we're supposed to
be doing, in theory we could fail to resume and then end up doing a
double disable on removal.  Probably the open coded version would have
the same issue though so perhaps this is pedantic...

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

Re: [PATCH] spi: dw-mmio: support suspend/resume

[Andy Shevchenko]

On Thu, Jan 22, 2026 at 05:57:42PM +0000, Mark Brown wrote:
> On Thu, Jan 22, 2026 at 11:50:46PM +0800, Jisheng Zhang wrote:

...

> > +	clk_disable_unprepare(dwsmmio->pclk);
> > +	clk_disable_unprepare(dwsmmio->clk);
> 
> These were allocated using devm_clk_get_prepare_enable() so we shouldn't
> really be fiddling with the state at runtime.  In practice this should
> always be fine I think but it's not really something we're supposed to
> be doing, in theory we could fail to resume and then end up doing a
> double disable on removal.  Probably the open coded version would have
> the same issue though so perhaps this is pedantic...

We clearly can call clk_disable(), but I'm not sure unprepare is the stage that
has no side-effects here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] spi: dw-mmio: support suspend/resume

[Mark Brown]

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

On Tue, Jan 27, 2026 at 05:01:54PM +0100, Andy Shevchenko wrote:
> On Thu, Jan 22, 2026 at 05:57:42PM +0000, Mark Brown wrote:

> > These were allocated using devm_clk_get_prepare_enable() so we shouldn't
> > really be fiddling with the state at runtime.  In practice this should
> > always be fine I think but it's not really something we're supposed to
> > be doing, in theory we could fail to resume and then end up doing a
> > double disable on removal.  Probably the open coded version would have
> > the same issue though so perhaps this is pedantic...

> We clearly can call clk_disable(), but I'm not sure unprepare is the stage that
> has no side-effects here.

What makes you say that disable is OK?

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

Re: [PATCH] spi: dw-mmio: support suspend/resume

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 04:07:11PM +0000, Mark Brown wrote:
> On Tue, Jan 27, 2026 at 05:01:54PM +0100, Andy Shevchenko wrote:
> > On Thu, Jan 22, 2026 at 05:57:42PM +0000, Mark Brown wrote:
> 
> > > These were allocated using devm_clk_get_prepare_enable() so we shouldn't
> > > really be fiddling with the state at runtime.  In practice this should
> > > always be fine I think but it's not really something we're supposed to
> > > be doing, in theory we could fail to resume and then end up doing a
> > > double disable on removal.  Probably the open coded version would have
> > > the same issue though so perhaps this is pedantic...
> 
> > We clearly can call clk_disable(), but I'm not sure unprepare is the stage that
> > has no side-effects here.
> 
> What makes you say that disable is OK?

It's (assumed to be) paired with clk_enable() in the resume.
I was talking from CLK usage perspective. However, after looking
at the resume implementation in drivers/base/power/main.c I think
the failed resume of one device doesn't prevent it's removal or
anything. It just collects statistics and records an error, but
no other actions are taken. Which means anything that needs to
be paired between suspend/resume and not checked at remove or
shutdown is prone to the same problem.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] spi: dw-mmio: support suspend/resume

[Mark Brown]

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

On Tue, Jan 27, 2026 at 10:51:46PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 27, 2026 at 04:07:11PM +0000, Mark Brown wrote:
> > On Tue, Jan 27, 2026 at 05:01:54PM +0100, Andy Shevchenko wrote:

> > > We clearly can call clk_disable(), but I'm not sure unprepare is the stage that
> > > has no side-effects here.

> > What makes you say that disable is OK?

> It's (assumed to be) paired with clk_enable() in the resume.

I'm concerned that we might manage to get a remove() on a device in
suspend through some means.

> I was talking from CLK usage perspective. However, after looking
> at the resume implementation in drivers/base/power/main.c I think
> the failed resume of one device doesn't prevent it's removal or
> anything. It just collects statistics and records an error, but
> no other actions are taken. Which means anything that needs to
> be paired between suspend/resume and not checked at remove or
> shutdown is prone to the same problem.

Yes, exactly.  How likely that actually is to happen in practice is a
separate question of course.

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

Re: [PATCH] spi: dw-mmio: support suspend/resume

[Andy Shevchenko]

On Thu, Jan 22, 2026 at 11:50:46PM +0800, Jisheng Zhang wrote:
> Add system wide suspend and resume support, the implementation is
> straightforward, just call spi_controller_suspend() then assert the
> reset and disable clks for suspend, enable clks and deassert reset
> then call spi_controller_resume() for resume.

...

> +static int dw_spi_mmio_resume(struct device *dev)
> +{
> +	struct dw_spi_mmio *dwsmmio = dev_get_drvdata(dev);

> +	clk_prepare_enable(dwsmmio->clk);
> +	clk_prepare_enable(dwsmmio->pclk);

You forgot to check for an error code from the above.

> +	reset_control_deassert(dwsmmio->rstc);
> +
> +	return dw_spi_resume_controller(&dwsmmio->dws);
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] spi: dw-mmio: support suspend/resume

[Mark Brown]

On Thu, 22 Jan 2026 23:50:46 +0800, Jisheng Zhang wrote:
> Add system wide suspend and resume support, the implementation is
> straightforward, just call spi_controller_suspend() then assert the
> reset and disable clks for suspend, enable clks and deassert reset
> then call spi_controller_resume() for resume.
> 
> 

Applied to

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

Thanks!

[1/1] spi: dw-mmio: support suspend/resume
      commit: a8b6e3738c872d35f1cf7b3cd3ce67d86d38e7cf

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