| Message ID | 20240102-j7200-pcie-s2r-v2-2-8e4f7d228ec2@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add suspend to ram support for PCIe on J7200 | expand |
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>
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 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
* 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
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,
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);
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(-)