The goal is to extend the active period of pinctrl.
Some devices may need active pinctrl after suspend() and/or before
resume().
So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
have active pinctrl until suspend_noirq() (included), and from
resume_noirq() (included).

The deprecated API has been removed to use the new one (dev_pm_ops struct).

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---
 drivers/pinctrl/pinctrl-single.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 19cc0db771a5..0dd4b0e11adf 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1625,7 +1625,6 @@ static int pcs_irq_init_chained_handler(struct pcs_device *pcs,
 	return 0;
 }
 
-#ifdef CONFIG_PM
 static int pcs_save_context(struct pcs_device *pcs)
 {
 	int i, mux_bytes;
@@ -1690,14 +1689,9 @@ static void pcs_restore_context(struct pcs_device *pcs)
 	}
 }
 
-static int pinctrl_single_suspend(struct platform_device *pdev,
-					pm_message_t state)
+static int pinctrl_single_suspend_noirq(struct device *dev)
 {
-	struct pcs_device *pcs;
-
-	pcs = platform_get_drvdata(pdev);
-	if (!pcs)
-		return -EINVAL;
+	struct pcs_device *pcs = dev_get_drvdata(dev);
 
 	if (pcs->flags & PCS_CONTEXT_LOSS_OFF) {
 		int ret;
@@ -1710,20 +1704,19 @@ static int pinctrl_single_suspend(struct platform_device *pdev,
 	return pinctrl_force_sleep(pcs->pctl);
 }
 
-static int pinctrl_single_resume(struct platform_device *pdev)
+static int pinctrl_single_resume_noirq(struct device *dev)
 {
-	struct pcs_device *pcs;
-
-	pcs = platform_get_drvdata(pdev);
-	if (!pcs)
-		return -EINVAL;
+	struct pcs_device *pcs = dev_get_drvdata(dev);
 
 	if (pcs->flags & PCS_CONTEXT_LOSS_OFF)
 		pcs_restore_context(pcs);
 
 	return pinctrl_force_default(pcs->pctl);
 }
-#endif
+
+static DEFINE_NOIRQ_DEV_PM_OPS(pinctrl_single_pm_ops,
+			       pinctrl_single_suspend_noirq,
+			       pinctrl_single_resume_noirq);
 
 /**
  * pcs_quirk_missing_pinctrl_cells - handle legacy binding
@@ -1986,11 +1979,8 @@ static struct platform_driver pcs_driver = {
 	.driver = {
 		.name		= DRIVER_NAME,
 		.of_match_table	= pcs_of_match,
+		.pm = pm_sleep_ptr(&pinctrl_single_pm_ops),
 	},
-#ifdef CONFIG_PM
-	.suspend = pinctrl_single_suspend,
-	.resume = pinctrl_single_resume,
-#endif
 };
 
 module_platform_driver(pcs_driver);

-- 
2.39.2

On Fri, Jan 26, 2024 at 3:37 PM Thomas Richard
<thomas.richard@bootlin.com> wrote:

> The goal is to extend the active period of pinctrl.
> Some devices may need active pinctrl after suspend() and/or before
> resume().
> So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
> have active pinctrl until suspend_noirq() (included), and from
> resume_noirq() (included).
>
> The deprecated API has been removed to use the new one (dev_pm_ops struct).
>
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Do you want to merge this as a series or is this something I
should just apply?

Yours,
Linus Walleij

Hi Linus,

On Sat, Jan 27, 2024 at 11:31:11PM +0100, Linus Walleij wrote:
> On Fri, Jan 26, 2024 at 3:37 PM Thomas Richard
> <thomas.richard@bootlin.com> wrote:
> 
> > The goal is to extend the active period of pinctrl.
> > Some devices may need active pinctrl after suspend() and/or before
> > resume().
> > So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
> > have active pinctrl until suspend_noirq() (included), and from
> > resume_noirq() (included).
> >
> > The deprecated API has been removed to use the new one (dev_pm_ops struct).
> >
> > Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Do you want to merge this as a series or is this something I
> should just apply?

there is still a comment from me pending.

Thanks,
Andi

> Yours,
> Linus Walleij

On 1/29/24 23:49, Andi Shyti wrote:
> Hi Linus,
> 
> On Sat, Jan 27, 2024 at 11:31:11PM +0100, Linus Walleij wrote:
>> On Fri, Jan 26, 2024 at 3:37 PM Thomas Richard
>> <thomas.richard@bootlin.com> wrote:
>>
>>> The goal is to extend the active period of pinctrl.
>>> Some devices may need active pinctrl after suspend() and/or before
>>> resume().
>>> So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
>>> have active pinctrl until suspend_noirq() (included), and from
>>> resume_noirq() (included).
>>>
>>> The deprecated API has been removed to use the new one (dev_pm_ops struct).
>>>
>>> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Do you want to merge this as a series or is this something I
>> should just apply?
> 
> there is still a comment from me pending.

Hi Andi,

Based on your comment, for the next iteration, I will move the cleanup
in a dedicated patch.

@Linus, you can apply pinctrl patches once everything is ok for you.

Regards,

-- 
Thomas Richard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

* Andi Shyti <andi.shyti@kernel.org> [240129 22:49]:
> On Sat, Jan 27, 2024 at 11:31:11PM +0100, Linus Walleij wrote:
> > On Fri, Jan 26, 2024 at 3:37 PM Thomas Richard
> > <thomas.richard@bootlin.com> wrote:
> > 
> > > The goal is to extend the active period of pinctrl.
> > > Some devices may need active pinctrl after suspend() and/or before
> > > resume().
> > > So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
> > > have active pinctrl until suspend_noirq() (included), and from
> > > resume_noirq() (included).
> > >
> > > The deprecated API has been removed to use the new one (dev_pm_ops struct).
> > >
> > > Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> > 
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > 
> > Do you want to merge this as a series or is this something I
> > should just apply?
> 
> there is still a comment from me pending.

FYI I gave this a brief test and things seem to work fine for me. Sounds
like there will be another revision though so I'll test again then.

Regards,

Tony

Hi Thomas,

...

> -	struct pcs_device *pcs;
> -
> -	pcs = platform_get_drvdata(pdev);
> -	if (!pcs)
> -		return -EINVAL;
> +	struct pcs_device *pcs = dev_get_drvdata(dev);

I think this cleanup can be placed in a different patch. Besides,
it's not mentioned in the commit log.

Otherwise looks good.

Andi

On Fri, Jan 26, 2024 at 4:37 PM Thomas Richard
<thomas.richard@bootlin.com> wrote:
>
> The goal is to extend the active period of pinctrl.
> Some devices may need active pinctrl after suspend() and/or before
> resume().
> So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
> have active pinctrl until suspend_noirq() (included), and from
> resume_noirq() (included).
>
> The deprecated API has been removed to use the new one (dev_pm_ops struct).

LGTM, FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


-- 
With Best Regards,
Andy Shevchenko