[PATCH V2] mfd: sprd: Add wakeup capability for PMIC irq

[PATCH V2] mfd: sprd: Add wakeup capability for PMIC irq

[Chunyan Zhang]

From: Baolin Wang <baolin.wang7@gmail.com>

When changing to use suspend-to-idle to save power, the PMIC irq can not
wakeup the system due to lack of wakeup capability, which will cause
the sub-irqs (such as power key) of the PMIC can not wake up the system.
Thus we can add the wakeup capability for PMIC irq to solve this issue,
as well as removing the IRQF_NO_SUSPEND flag to allow PMIC irq to be
a wakeup source.

Reported-by: Chunyan Zhang <zhang.lyra@gmail.com>
Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
Tested-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
---
Changes from v1:
* addressed comments from Lee;
* added tested-by from Chunyan.
(This patch is rebased on branch for-mfd-next)
---
 drivers/mfd/sprd-sc27xx-spi.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
index 33336cde4724..adb4a1b13332 100644
--- a/drivers/mfd/sprd-sc27xx-spi.c
+++ b/drivers/mfd/sprd-sc27xx-spi.c
@@ -256,7 +256,7 @@ static int sprd_pmic_probe(struct spi_device *spi)
 	}
 
 	ret = devm_regmap_add_irq_chip(&spi->dev, ddata->regmap, ddata->irq,
-				       IRQF_ONESHOT | IRQF_NO_SUSPEND, 0,
+				       IRQF_ONESHOT, 0,
 				       &ddata->irq_chip, &ddata->irq_data);
 	if (ret) {
 		dev_err(&spi->dev, "Failed to add PMIC irq chip %d\n", ret);
@@ -272,9 +272,34 @@ static int sprd_pmic_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	device_init_wakeup(&spi->dev, true);
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int sprd_pmic_suspend(struct device *dev)
+{
+	struct sprd_pmic *ddata = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(ddata->irq);
+
+	return 0;
+}
+
+static int sprd_pmic_resume(struct device *dev)
+{
+	struct sprd_pmic *ddata = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(ddata->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(sprd_pmic_pm_ops, sprd_pmic_suspend, sprd_pmic_resume);
+
 static const struct of_device_id sprd_pmic_match[] = {
 	{ .compatible = "sprd,sc2731", .data = &sc2731_data },
 	{},
@@ -285,6 +310,7 @@ static struct spi_driver sprd_pmic_driver = {
 	.driver = {
 		.name = "sc27xx-pmic",
 		.of_match_table = sprd_pmic_match,
+		.pm = &sprd_pmic_pm_ops,
 	},
 	.probe = sprd_pmic_probe,
 };
-- 
2.20.1

Re: [PATCH V2] mfd: sprd: Add wakeup capability for PMIC irq

[Chunyan Zhang]

Hi Lee,

On Wed, 27 May 2020 at 14:21, Chunyan Zhang <zhang.lyra@gmail.com> wrote:
>
> From: Baolin Wang <baolin.wang7@gmail.com>
>
> When changing to use suspend-to-idle to save power, the PMIC irq can not
> wakeup the system due to lack of wakeup capability, which will cause
> the sub-irqs (such as power key) of the PMIC can not wake up the system.
> Thus we can add the wakeup capability for PMIC irq to solve this issue,
> as well as removing the IRQF_NO_SUSPEND flag to allow PMIC irq to be
> a wakeup source.
>
> Reported-by: Chunyan Zhang <zhang.lyra@gmail.com>
> Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
> Tested-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> ---
> Changes from v1:
> * addressed comments from Lee;
> * added tested-by from Chunyan.
> (This patch is rebased on branch for-mfd-next)

Could you please pick up this patch if there's no more comments :)

Thanks,
Chunyan

> ---
>  drivers/mfd/sprd-sc27xx-spi.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
> index 33336cde4724..adb4a1b13332 100644
> --- a/drivers/mfd/sprd-sc27xx-spi.c
> +++ b/drivers/mfd/sprd-sc27xx-spi.c
> @@ -256,7 +256,7 @@ static int sprd_pmic_probe(struct spi_device *spi)
>         }
>
>         ret = devm_regmap_add_irq_chip(&spi->dev, ddata->regmap, ddata->irq,
> -                                      IRQF_ONESHOT | IRQF_NO_SUSPEND, 0,
> +                                      IRQF_ONESHOT, 0,
>                                        &ddata->irq_chip, &ddata->irq_data);
>         if (ret) {
>                 dev_err(&spi->dev, "Failed to add PMIC irq chip %d\n", ret);
> @@ -272,9 +272,34 @@ static int sprd_pmic_probe(struct spi_device *spi)
>                 return ret;
>         }
>
> +       device_init_wakeup(&spi->dev, true);
>         return 0;
>  }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int sprd_pmic_suspend(struct device *dev)
> +{
> +       struct sprd_pmic *ddata = dev_get_drvdata(dev);
> +
> +       if (device_may_wakeup(dev))
> +               enable_irq_wake(ddata->irq);
> +
> +       return 0;
> +}
> +
> +static int sprd_pmic_resume(struct device *dev)
> +{
> +       struct sprd_pmic *ddata = dev_get_drvdata(dev);
> +
> +       if (device_may_wakeup(dev))
> +               disable_irq_wake(ddata->irq);
> +
> +       return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(sprd_pmic_pm_ops, sprd_pmic_suspend, sprd_pmic_resume);
> +
>  static const struct of_device_id sprd_pmic_match[] = {
>         { .compatible = "sprd,sc2731", .data = &sc2731_data },
>         {},
> @@ -285,6 +310,7 @@ static struct spi_driver sprd_pmic_driver = {
>         .driver = {
>                 .name = "sc27xx-pmic",
>                 .of_match_table = sprd_pmic_match,
> +               .pm = &sprd_pmic_pm_ops,
>         },
>         .probe = sprd_pmic_probe,
>  };
> --
> 2.20.1
>

Re: [PATCH V2] mfd: sprd: Add wakeup capability for PMIC irq

[Lee Jones]

On Thu, 04 Jun 2020, Chunyan Zhang wrote:

> Hi Lee,
> 
> On Wed, 27 May 2020 at 14:21, Chunyan Zhang <zhang.lyra@gmail.com> wrote:
> >
> > From: Baolin Wang <baolin.wang7@gmail.com>
> >
> > When changing to use suspend-to-idle to save power, the PMIC irq can not
> > wakeup the system due to lack of wakeup capability, which will cause
> > the sub-irqs (such as power key) of the PMIC can not wake up the system.
> > Thus we can add the wakeup capability for PMIC irq to solve this issue,
> > as well as removing the IRQF_NO_SUSPEND flag to allow PMIC irq to be
> > a wakeup source.
> >
> > Reported-by: Chunyan Zhang <zhang.lyra@gmail.com>
> > Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
> > Tested-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > ---
> > Changes from v1:
> > * addressed comments from Lee;
> > * added tested-by from Chunyan.
> > (This patch is rebased on branch for-mfd-next)
> 
> Could you please pick up this patch if there's no more comments :)

Please don't send contentless nags.  Your patch is in my queue.

The merge-window is currently open, which is a) a busy time for
Maintainers and b) a time where some of us take a little breather
before the next release.  Please wait until -rc1 has been released
before sending out your [RESENDS].

> > ---
> >  drivers/mfd/sprd-sc27xx-spi.c | 28 +++++++++++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
> > index 33336cde4724..adb4a1b13332 100644
> > --- a/drivers/mfd/sprd-sc27xx-spi.c
> > +++ b/drivers/mfd/sprd-sc27xx-spi.c
> > @@ -256,7 +256,7 @@ static int sprd_pmic_probe(struct spi_device *spi)
> >         }
> >
> >         ret = devm_regmap_add_irq_chip(&spi->dev, ddata->regmap, ddata->irq,
> > -                                      IRQF_ONESHOT | IRQF_NO_SUSPEND, 0,
> > +                                      IRQF_ONESHOT, 0,
> >                                        &ddata->irq_chip, &ddata->irq_data);
> >         if (ret) {
> >                 dev_err(&spi->dev, "Failed to add PMIC irq chip %d\n", ret);
> > @@ -272,9 +272,34 @@ static int sprd_pmic_probe(struct spi_device *spi)
> >                 return ret;
> >         }
> >
> > +       device_init_wakeup(&spi->dev, true);
> >         return 0;
> >  }
> >
> > +#ifdef CONFIG_PM_SLEEP
> > +static int sprd_pmic_suspend(struct device *dev)
> > +{
> > +       struct sprd_pmic *ddata = dev_get_drvdata(dev);
> > +
> > +       if (device_may_wakeup(dev))
> > +               enable_irq_wake(ddata->irq);
> > +
> > +       return 0;
> > +}
> > +
> > +static int sprd_pmic_resume(struct device *dev)
> > +{
> > +       struct sprd_pmic *ddata = dev_get_drvdata(dev);
> > +
> > +       if (device_may_wakeup(dev))
> > +               disable_irq_wake(ddata->irq);
> > +
> > +       return 0;
> > +}
> > +#endif
> > +
> > +static SIMPLE_DEV_PM_OPS(sprd_pmic_pm_ops, sprd_pmic_suspend, sprd_pmic_resume);
> > +
> >  static const struct of_device_id sprd_pmic_match[] = {
> >         { .compatible = "sprd,sc2731", .data = &sc2731_data },
> >         {},
> > @@ -285,6 +310,7 @@ static struct spi_driver sprd_pmic_driver = {
> >         .driver = {
> >                 .name = "sc27xx-pmic",
> >                 .of_match_table = sprd_pmic_match,
> > +               .pm = &sprd_pmic_pm_ops,
> >         },
> >         .probe = sprd_pmic_probe,
> >  };
> >

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH V2] mfd: sprd: Add wakeup capability for PMIC irq

[Chunyan Zhang]

On Thu, 4 Jun 2020 at 19:16, Lee Jones <lee.jones@linaro.org> wrote:
>
> On Thu, 04 Jun 2020, Chunyan Zhang wrote:
>
> > Hi Lee,
> >
> > On Wed, 27 May 2020 at 14:21, Chunyan Zhang <zhang.lyra@gmail.com> wrote:
> > >
> > > From: Baolin Wang <baolin.wang7@gmail.com>
> > >
> > > When changing to use suspend-to-idle to save power, the PMIC irq can not
> > > wakeup the system due to lack of wakeup capability, which will cause
> > > the sub-irqs (such as power key) of the PMIC can not wake up the system.
> > > Thus we can add the wakeup capability for PMIC irq to solve this issue,
> > > as well as removing the IRQF_NO_SUSPEND flag to allow PMIC irq to be
> > > a wakeup source.
> > >
> > > Reported-by: Chunyan Zhang <zhang.lyra@gmail.com>
> > > Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
> > > Tested-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > > ---
> > > Changes from v1:
> > > * addressed comments from Lee;
> > > * added tested-by from Chunyan.
> > > (This patch is rebased on branch for-mfd-next)
> >
> > Could you please pick up this patch if there's no more comments :)
>
> Please don't send contentless nags.  Your patch is in my queue.
>
> The merge-window is currently open, which is a) a busy time for

I understand that very much, and also appreciate all maintainer's
works for the great opensource project.
And just because of that I know you're busy during merge-window, I was
worried that this patch was left out, since I didn't see it at
mfd-next.
I apologize if the last email made you unhappay. I will not send
reminder email to you anymore.

Best regards,
Chunyan

> Maintainers and b) a time where some of us take a little breather
> before the next release.  Please wait until -rc1 has been released
> before sending out your [RESENDS].
>
> > > ---
> > >  drivers/mfd/sprd-sc27xx-spi.c | 28 +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
> > > index 33336cde4724..adb4a1b13332 100644
> > > --- a/drivers/mfd/sprd-sc27xx-spi.c
> > > +++ b/drivers/mfd/sprd-sc27xx-spi.c
> > > @@ -256,7 +256,7 @@ static int sprd_pmic_probe(struct spi_device *spi)
> > >         }
> > >
> > >         ret = devm_regmap_add_irq_chip(&spi->dev, ddata->regmap, ddata->irq,
> > > -                                      IRQF_ONESHOT | IRQF_NO_SUSPEND, 0,
> > > +                                      IRQF_ONESHOT, 0,
> > >                                        &ddata->irq_chip, &ddata->irq_data);
> > >         if (ret) {
> > >                 dev_err(&spi->dev, "Failed to add PMIC irq chip %d\n", ret);
> > > @@ -272,9 +272,34 @@ static int sprd_pmic_probe(struct spi_device *spi)
> > >                 return ret;
> > >         }
> > >
> > > +       device_init_wakeup(&spi->dev, true);
> > >         return 0;
> > >  }
> > >
> > > +#ifdef CONFIG_PM_SLEEP
> > > +static int sprd_pmic_suspend(struct device *dev)
> > > +{
> > > +       struct sprd_pmic *ddata = dev_get_drvdata(dev);
> > > +
> > > +       if (device_may_wakeup(dev))
> > > +               enable_irq_wake(ddata->irq);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int sprd_pmic_resume(struct device *dev)
> > > +{
> > > +       struct sprd_pmic *ddata = dev_get_drvdata(dev);
> > > +
> > > +       if (device_may_wakeup(dev))
> > > +               disable_irq_wake(ddata->irq);
> > > +
> > > +       return 0;
> > > +}
> > > +#endif
> > > +
> > > +static SIMPLE_DEV_PM_OPS(sprd_pmic_pm_ops, sprd_pmic_suspend, sprd_pmic_resume);
> > > +
> > >  static const struct of_device_id sprd_pmic_match[] = {
> > >         { .compatible = "sprd,sc2731", .data = &sc2731_data },
> > >         {},
> > > @@ -285,6 +310,7 @@ static struct spi_driver sprd_pmic_driver = {
> > >         .driver = {
> > >                 .name = "sc27xx-pmic",
> > >                 .of_match_table = sprd_pmic_match,
> > > +               .pm = &sprd_pmic_pm_ops,
> > >         },
> > >         .probe = sprd_pmic_probe,
> > >  };
> > >
>
> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH V2] mfd: sprd: Add wakeup capability for PMIC irq

[Lee Jones]

On Fri, 05 Jun 2020, Chunyan Zhang wrote:

> On Thu, 4 Jun 2020 at 19:16, Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Thu, 04 Jun 2020, Chunyan Zhang wrote:
> >
> > > Hi Lee,
> > >
> > > On Wed, 27 May 2020 at 14:21, Chunyan Zhang <zhang.lyra@gmail.com> wrote:
> > > >
> > > > From: Baolin Wang <baolin.wang7@gmail.com>
> > > >
> > > > When changing to use suspend-to-idle to save power, the PMIC irq can not
> > > > wakeup the system due to lack of wakeup capability, which will cause
> > > > the sub-irqs (such as power key) of the PMIC can not wake up the system.
> > > > Thus we can add the wakeup capability for PMIC irq to solve this issue,
> > > > as well as removing the IRQF_NO_SUSPEND flag to allow PMIC irq to be
> > > > a wakeup source.
> > > >
> > > > Reported-by: Chunyan Zhang <zhang.lyra@gmail.com>
> > > > Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
> > > > Tested-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > > > ---
> > > > Changes from v1:
> > > > * addressed comments from Lee;
> > > > * added tested-by from Chunyan.
> > > > (This patch is rebased on branch for-mfd-next)
> > >
> > > Could you please pick up this patch if there's no more comments :)
> >
> > Please don't send contentless nags.  Your patch is in my queue.
> >
> > The merge-window is currently open, which is a) a busy time for
> 
> I understand that very much, and also appreciate all maintainer's
> works for the great opensource project.
> And just because of that I know you're busy during merge-window, I was
> worried that this patch was left out, since I didn't see it at
> mfd-next.

It was left out, as it was sent during -rc7.  Very few patches sent to
late in the release cycle are applied during this time, since it
leaves too little time to soak in -next.

The patch should make it into the next kernel release (v5.9) without
issue.

> I apologize if the last email made you unhappay. I will not send
> reminder email to you anymore.

If more than 2 weeks have passed and you genuinely believe your patch
has been forgotten.  The correct means to solve the issue is to submit
a [RESEND].

> > Maintainers and b) a time where some of us take a little breather
> > before the next release.  Please wait until -rc1 has been released
> > before sending out your [RESENDS].

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog