[PATCH v1 0/2] Fix an irq issue and add system pm ops for StarFive JH7110 pinctrl drivers

[PATCH v1 0/2] Fix an irq issue and add system pm ops for StarFive JH7110 pinctrl drivers

[Hal Feng]

This patchset fixes some issues arising when CONFIG_PM is enabled
or suspending to disk.

The first patch fixes failure to set irq after CONFIG_PM is enabled.

The second patch adds system pm ops to save and restore context. So it can
ensure that the pins configuration keep consistent with the one before
suspending.

Hal Feng (2):
  pinctrl: starfive: jh7110: Fix failure to set irq after CONFIG_PM is
    enabled
  pinctrl: starfive: jh7110: Add system pm ops to save and restore
    context

 MAINTAINERS                                   |  1 +
 .../starfive/pinctrl-starfive-jh7110-aon.c    |  4 ++
 .../starfive/pinctrl-starfive-jh7110-sys.c    |  4 ++
 .../starfive/pinctrl-starfive-jh7110.c        | 42 ++++++++++++++++++-
 .../starfive/pinctrl-starfive-jh7110.h        |  4 ++
 5 files changed, 53 insertions(+), 2 deletions(-)


base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
-- 
2.38.1

[PATCH v1 1/2] pinctrl: starfive: jh7110: Fix failure to set irq after CONFIG_PM is enabled

[Hal Feng]

The issue was found when we enabled CONFIG_PM and tested edge events using
libgpiod.

> # gpiomon -r gpiochip0 55
> gpiomon: error waiting for events: Permission denied

`gpiomon` will call irq_chip_pm_get() and then call pm_runtime_resume_and_get()
if (IS_ENABLED(CONFIG_PM) && sfp->gc.irq.domain->pm_dev).
pm_runtime_resume_and_get() will fail if the runtime pm of pinctrl device
is disabled.

As we expect the pinctrl driver can be always working and never suspend
during runtime, unset sfp->gc.irq.domain->pm_dev to make sure
pm_runtime_resume_and_get() won't be called when setting irq.

Fixes: 447976ab62c5 ("pinctrl: starfive: Add StarFive JH7110 sys controller driver")
Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
index 5fe729b4a03d..72747ad497b5 100644
--- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
+++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
@@ -968,8 +968,6 @@ int jh7110_pinctrl_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "could not register gpiochip\n");
 
-	irq_domain_set_pm_device(sfp->gc.irq.domain, dev);
-
 	dev_info(dev, "StarFive GPIO chip registered %d GPIOs\n", sfp->gc.ngpio);
 
 	return pinctrl_enable(sfp->pctl);
-- 
2.38.1

[PATCH v1 2/2] pinctrl: starfive: jh7110: Add system pm ops to save and restore context

[Hal Feng]

Add system pm ops to save and restore pinctrl registers
when suspending and resuming the driver, respectively.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 MAINTAINERS                                   |  1 +
 .../starfive/pinctrl-starfive-jh7110-aon.c    |  4 ++
 .../starfive/pinctrl-starfive-jh7110-sys.c    |  4 ++
 .../starfive/pinctrl-starfive-jh7110.c        | 40 +++++++++++++++++++
 .../starfive/pinctrl-starfive-jh7110.h        |  4 ++
 5 files changed, 53 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4cc6bf79fdd8..6c7eabeef28f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20302,6 +20302,7 @@ F:	include/dt-bindings/clock/starfive?jh71*.h
 STARFIVE JH71X0 PINCTRL DRIVERS
 M:	Emil Renner Berthing <kernel@esmil.dk>
 M:	Jianlong Huang <jianlong.huang@starfivetech.com>
+M:	Hal Feng <hal.feng@starfivetech.com>
 L:	linux-gpio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/pinctrl/starfive,jh71*.yaml
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110-aon.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110-aon.c
index 8cf28aaed254..8691fd55594a 100644
--- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110-aon.c
+++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110-aon.c
@@ -34,6 +34,8 @@
 #define JH7110_AON_NGPIO		4
 #define JH7110_AON_GC_BASE		64
 
+#define JH7110_AON_REGS_NUM		37
+
 /* registers */
 #define JH7110_AON_DOEN			0x0
 #define JH7110_AON_DOUT			0x4
@@ -148,6 +150,7 @@ static const struct jh7110_pinctrl_soc_info jh7110_aon_pinctrl_info = {
 	.gpi_mask	= GENMASK(3, 0),
 	.gpioin_reg_base	   = JH7110_AON_GPIOIN,
 	.irq_reg		   = &jh7110_aon_irq_reg,
+	.nsaved_regs		   = JH7110_AON_REGS_NUM,
 	.jh7110_set_one_pin_mux  = jh7110_aon_set_one_pin_mux,
 	.jh7110_get_padcfg_base  = jh7110_aon_get_padcfg_base,
 	.jh7110_gpio_irq_handler = jh7110_aon_irq_handler,
@@ -168,6 +171,7 @@ static struct platform_driver jh7110_aon_pinctrl_driver = {
 	.driver = {
 		.name = "starfive-jh7110-aon-pinctrl",
 		.of_match_table = jh7110_aon_pinctrl_of_match,
+		.pm = pm_sleep_ptr(&jh7110_pinctrl_pm_ops),
 	},
 };
 module_platform_driver(jh7110_aon_pinctrl_driver);
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110-sys.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110-sys.c
index bc279a39613f..0654731d5c27 100644
--- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110-sys.c
+++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110-sys.c
@@ -33,6 +33,8 @@
 #define JH7110_SYS_NGPIO		64
 #define JH7110_SYS_GC_BASE		0
 
+#define JH7110_SYS_REGS_NUM		174
+
 /* registers */
 #define JH7110_SYS_DOEN			0x000
 #define JH7110_SYS_DOUT			0x040
@@ -419,6 +421,7 @@ static const struct jh7110_pinctrl_soc_info jh7110_sys_pinctrl_info = {
 	.gpi_mask	= GENMASK(6, 0),
 	.gpioin_reg_base	   = JH7110_SYS_GPIOIN,
 	.irq_reg		   = &jh7110_sys_irq_reg,
+	.nsaved_regs		   = JH7110_SYS_REGS_NUM,
 	.jh7110_set_one_pin_mux  = jh7110_sys_set_one_pin_mux,
 	.jh7110_get_padcfg_base  = jh7110_sys_get_padcfg_base,
 	.jh7110_gpio_irq_handler = jh7110_sys_irq_handler,
@@ -439,6 +442,7 @@ static struct platform_driver jh7110_sys_pinctrl_driver = {
 	.driver = {
 		.name = "starfive-jh7110-sys-pinctrl",
 		.of_match_table = jh7110_sys_pinctrl_of_match,
+		.pm = pm_sleep_ptr(&jh7110_pinctrl_pm_ops),
 	},
 };
 module_platform_driver(jh7110_sys_pinctrl_driver);
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
index 72747ad497b5..813b5c2f11b6 100644
--- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
+++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
@@ -873,6 +873,13 @@ int jh7110_pinctrl_probe(struct platform_device *pdev)
 	if (!sfp)
 		return -ENOMEM;
 
+#if IS_ENABLED(CONFIG_PM_SLEEP)
+	sfp->saved_regs = devm_kcalloc(dev, info->nsaved_regs,
+				       sizeof(*sfp->saved_regs), GFP_KERNEL);
+	if (!sfp->saved_regs)
+		return -ENOMEM;
+#endif
+
 	sfp->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(sfp->base))
 		return PTR_ERR(sfp->base);
@@ -974,6 +981,39 @@ int jh7110_pinctrl_probe(struct platform_device *pdev)
 }
 EXPORT_SYMBOL_GPL(jh7110_pinctrl_probe);
 
+static int jh7110_pinctrl_suspend(struct device *dev)
+{
+	struct jh7110_pinctrl *sfp = dev_get_drvdata(dev);
+	unsigned long flags;
+	unsigned int i;
+
+	raw_spin_lock_irqsave(&sfp->lock, flags);
+	for (i = 0 ; i < sfp->info->nsaved_regs ; i++)
+		sfp->saved_regs[i] = readl_relaxed(sfp->base + 4 * i);
+
+	raw_spin_unlock_irqrestore(&sfp->lock, flags);
+	return 0;
+}
+
+static int jh7110_pinctrl_resume(struct device *dev)
+{
+	struct jh7110_pinctrl *sfp = dev_get_drvdata(dev);
+	unsigned long flags;
+	unsigned int i;
+
+	raw_spin_lock_irqsave(&sfp->lock, flags);
+	for (i = 0 ; i < sfp->info->nsaved_regs ; i++)
+		writel_relaxed(sfp->saved_regs[i], sfp->base + 4 * i);
+
+	raw_spin_unlock_irqrestore(&sfp->lock, flags);
+	return 0;
+}
+
+const struct dev_pm_ops jh7110_pinctrl_pm_ops = {
+	LATE_SYSTEM_SLEEP_PM_OPS(jh7110_pinctrl_suspend, jh7110_pinctrl_resume)
+};
+EXPORT_SYMBOL_GPL(jh7110_pinctrl_pm_ops);
+
 MODULE_DESCRIPTION("Pinctrl driver for the StarFive JH7110 SoC");
 MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
 MODULE_AUTHOR("Jianlong Huang <jianlong.huang@starfivetech.com>");
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.h b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.h
index 3f20b7ff96dd..a33d0d4e1382 100644
--- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.h
+++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.h
@@ -21,6 +21,7 @@ struct jh7110_pinctrl {
 	/* register read/write mutex */
 	struct mutex mutex;
 	const struct jh7110_pinctrl_soc_info *info;
+	u32 *saved_regs;
 };
 
 struct jh7110_gpio_irq_reg {
@@ -50,6 +51,8 @@ struct jh7110_pinctrl_soc_info {
 
 	const struct jh7110_gpio_irq_reg *irq_reg;
 
+	unsigned int nsaved_regs;
+
 	/* generic pinmux */
 	int (*jh7110_set_one_pin_mux)(struct jh7110_pinctrl *sfp,
 				      unsigned int pin,
@@ -66,5 +69,6 @@ void jh7110_set_gpiomux(struct jh7110_pinctrl *sfp, unsigned int pin,
 			unsigned int din, u32 dout, u32 doen);
 int jh7110_pinctrl_probe(struct platform_device *pdev);
 struct jh7110_pinctrl *jh7110_from_irq_desc(struct irq_desc *desc);
+extern const struct dev_pm_ops jh7110_pinctrl_pm_ops;
 
 #endif /* __PINCTRL_STARFIVE_JH7110_H__ */
-- 
2.38.1

Re: [PATCH v1 0/2] Fix an irq issue and add system pm ops for StarFive JH7110 pinctrl drivers

[Linus Walleij]

On Tue, Sep 5, 2023 at 2:22 PM Hal Feng <hal.feng@starfivetech.com> wrote:

> This patchset fixes some issues arising when CONFIG_PM is enabled
> or suspending to disk.
>
> The first patch fixes failure to set irq after CONFIG_PM is enabled.
>
> The second patch adds system pm ops to save and restore context. So it can
> ensure that the pins configuration keep consistent with the one before
> suspending.

Patches applied for fixes!

Yours,
Linus Walleij