diff mbox series

[v12,1/6] mfd: mt6397: Modify suspend/resume behavior

Message ID 1586333531-21641-2-git-send-email-hsin-hsiung.wang@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Add Support for MediaTek PMIC MT6358 | expand

Commit Message

Hsin-Hsiung Wang April 8, 2020, 8:12 a.m. UTC
Some pmics don't need backup interrupt settings, so we change to use
pm notifier for the pmics which are necessary to store settings.

Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
---
 drivers/mfd/mt6397-core.c       | 30 ------------------------------
 drivers/mfd/mt6397-irq.c        | 35 ++++++++++++++++++++++++++++++++++-
 include/linux/mfd/mt6397/core.h |  2 ++
 3 files changed, 36 insertions(+), 31 deletions(-)

Comments

Lee Jones April 16, 2020, 8:49 a.m. UTC | #1
On Wed, 08 Apr 2020, Hsin-Hsiung Wang wrote:

> Some pmics don't need backup interrupt settings, so we change to use
> pm notifier for the pmics which are necessary to store settings.
> 
> Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> ---
>  drivers/mfd/mt6397-core.c       | 30 ------------------------------
>  drivers/mfd/mt6397-irq.c        | 35 ++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/mt6397/core.h |  2 ++
>  3 files changed, 36 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index 0437c85..d2e70d8 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -100,35 +100,6 @@ static const struct mfd_cell mt6397_devs[] = {
>  	}
>  };
>  
> -#ifdef CONFIG_PM_SLEEP
> -static int mt6397_irq_suspend(struct device *dev)
> -{
> -	struct mt6397_chip *chip = dev_get_drvdata(dev);
> -
> -	regmap_write(chip->regmap, chip->int_con[0], chip->wake_mask[0]);
> -	regmap_write(chip->regmap, chip->int_con[1], chip->wake_mask[1]);
> -
> -	enable_irq_wake(chip->irq);
> -
> -	return 0;
> -}
> -
> -static int mt6397_irq_resume(struct device *dev)
> -{
> -	struct mt6397_chip *chip = dev_get_drvdata(dev);
> -
> -	regmap_write(chip->regmap, chip->int_con[0], chip->irq_masks_cur[0]);
> -	regmap_write(chip->regmap, chip->int_con[1], chip->irq_masks_cur[1]);
> -
> -	disable_irq_wake(chip->irq);
> -
> -	return 0;
> -}
> -#endif
> -
> -static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_irq_suspend,
> -			mt6397_irq_resume);
> -
>  struct chip_data {
>  	u32 cid_addr;
>  	u32 cid_shift;
> @@ -238,7 +209,6 @@ static struct platform_driver mt6397_driver = {
>  	.driver = {
>  		.name = "mt6397",
>  		.of_match_table = of_match_ptr(mt6397_of_match),
> -		.pm = &mt6397_pm_ops,
>  	},
>  	.id_table = mt6397_id,
>  };
> diff --git a/drivers/mfd/mt6397-irq.c b/drivers/mfd/mt6397-irq.c
> index b2d3ce1..2924919 100644
> --- a/drivers/mfd/mt6397-irq.c
> +++ b/drivers/mfd/mt6397-irq.c
> @@ -9,6 +9,7 @@
>  #include <linux/of_irq.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> +#include <linux/suspend.h>
>  #include <linux/mfd/mt6323/core.h>
>  #include <linux/mfd/mt6323/registers.h>
>  #include <linux/mfd/mt6397/core.h>
> @@ -81,7 +82,7 @@ static struct irq_chip mt6397_irq_chip = {
>  static void mt6397_irq_handle_reg(struct mt6397_chip *mt6397, int reg,
>  				  int irqbase)
>  {
> -	unsigned int status;
> +	unsigned int status = 0;

This looks like an unrelated change, no?

>  	int i, irq, ret;
>  
>  	ret = regmap_read(mt6397->regmap, reg, &status);
> @@ -128,6 +129,36 @@ static const struct irq_domain_ops mt6397_irq_domain_ops = {
>  	.map = mt6397_irq_domain_map,
>  };

Other than that.

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Hsin-Hsiung Wang April 20, 2020, 10:52 a.m. UTC | #2
Hi,

On Thu, 2020-04-16 at 09:49 +0100, Lee Jones wrote:
> On Wed, 08 Apr 2020, Hsin-Hsiung Wang wrote:
> 
> > Some pmics don't need backup interrupt settings, so we change to use
> > pm notifier for the pmics which are necessary to store settings.
> > 
> > Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> > ---
> >  drivers/mfd/mt6397-core.c       | 30 ------------------------------
> >  drivers/mfd/mt6397-irq.c        | 35 ++++++++++++++++++++++++++++++++++-
> >  include/linux/mfd/mt6397/core.h |  2 ++
> >  3 files changed, 36 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> > index 0437c85..d2e70d8 100644
> > --- a/drivers/mfd/mt6397-core.c
> > +++ b/drivers/mfd/mt6397-core.c
> > @@ -100,35 +100,6 @@ static const struct mfd_cell mt6397_devs[] = {
> >  	}
> >  };
> >  
> > -#ifdef CONFIG_PM_SLEEP
> > -static int mt6397_irq_suspend(struct device *dev)
> > -{
> > -	struct mt6397_chip *chip = dev_get_drvdata(dev);
> > -
> > -	regmap_write(chip->regmap, chip->int_con[0], chip->wake_mask[0]);
> > -	regmap_write(chip->regmap, chip->int_con[1], chip->wake_mask[1]);
> > -
> > -	enable_irq_wake(chip->irq);
> > -
> > -	return 0;
> > -}
> > -
> > -static int mt6397_irq_resume(struct device *dev)
> > -{
> > -	struct mt6397_chip *chip = dev_get_drvdata(dev);
> > -
> > -	regmap_write(chip->regmap, chip->int_con[0], chip->irq_masks_cur[0]);
> > -	regmap_write(chip->regmap, chip->int_con[1], chip->irq_masks_cur[1]);
> > -
> > -	disable_irq_wake(chip->irq);
> > -
> > -	return 0;
> > -}
> > -#endif
> > -
> > -static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_irq_suspend,
> > -			mt6397_irq_resume);
> > -
> >  struct chip_data {
> >  	u32 cid_addr;
> >  	u32 cid_shift;
> > @@ -238,7 +209,6 @@ static struct platform_driver mt6397_driver = {
> >  	.driver = {
> >  		.name = "mt6397",
> >  		.of_match_table = of_match_ptr(mt6397_of_match),
> > -		.pm = &mt6397_pm_ops,
> >  	},
> >  	.id_table = mt6397_id,
> >  };
> > diff --git a/drivers/mfd/mt6397-irq.c b/drivers/mfd/mt6397-irq.c
> > index b2d3ce1..2924919 100644
> > --- a/drivers/mfd/mt6397-irq.c
> > +++ b/drivers/mfd/mt6397-irq.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/of_irq.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/regmap.h>
> > +#include <linux/suspend.h>
> >  #include <linux/mfd/mt6323/core.h>
> >  #include <linux/mfd/mt6323/registers.h>
> >  #include <linux/mfd/mt6397/core.h>
> > @@ -81,7 +82,7 @@ static struct irq_chip mt6397_irq_chip = {
> >  static void mt6397_irq_handle_reg(struct mt6397_chip *mt6397, int reg,
> >  				  int irqbase)
> >  {
> > -	unsigned int status;
> > +	unsigned int status = 0;
> 
> This looks like an unrelated change, no?
> 

It is to fix the coverity defect.

> >  	int i, irq, ret;
> >  
> >  	ret = regmap_read(mt6397->regmap, reg, &status);
> > @@ -128,6 +129,36 @@ static const struct irq_domain_ops mt6397_irq_domain_ops = {
> >  	.map = mt6397_irq_domain_map,
> >  };
> 
> Other than that.
> 
> For my own reference:
>   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> 
Thanks for your review. I will add it in the next version.
Lee Jones April 20, 2020, 11:15 a.m. UTC | #3
On Mon, 20 Apr 2020, Hsin-hsiung Wang wrote:

> Hi,
> 
> On Thu, 2020-04-16 at 09:49 +0100, Lee Jones wrote:
> > On Wed, 08 Apr 2020, Hsin-Hsiung Wang wrote:
> > 
> > > Some pmics don't need backup interrupt settings, so we change to use
> > > pm notifier for the pmics which are necessary to store settings.
> > > 
> > > Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> > > ---
> > >  drivers/mfd/mt6397-core.c       | 30 ------------------------------
> > >  drivers/mfd/mt6397-irq.c        | 35 ++++++++++++++++++++++++++++++++++-
> > >  include/linux/mfd/mt6397/core.h |  2 ++
> > >  3 files changed, 36 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> > > index 0437c85..d2e70d8 100644
> > > --- a/drivers/mfd/mt6397-core.c
> > > +++ b/drivers/mfd/mt6397-core.c
> > > @@ -100,35 +100,6 @@ static const struct mfd_cell mt6397_devs[] = {
> > >  	}
> > >  };
> > >  
> > > -#ifdef CONFIG_PM_SLEEP
> > > -static int mt6397_irq_suspend(struct device *dev)
> > > -{
> > > -	struct mt6397_chip *chip = dev_get_drvdata(dev);
> > > -
> > > -	regmap_write(chip->regmap, chip->int_con[0], chip->wake_mask[0]);
> > > -	regmap_write(chip->regmap, chip->int_con[1], chip->wake_mask[1]);
> > > -
> > > -	enable_irq_wake(chip->irq);
> > > -
> > > -	return 0;
> > > -}
> > > -
> > > -static int mt6397_irq_resume(struct device *dev)
> > > -{
> > > -	struct mt6397_chip *chip = dev_get_drvdata(dev);
> > > -
> > > -	regmap_write(chip->regmap, chip->int_con[0], chip->irq_masks_cur[0]);
> > > -	regmap_write(chip->regmap, chip->int_con[1], chip->irq_masks_cur[1]);
> > > -
> > > -	disable_irq_wake(chip->irq);
> > > -
> > > -	return 0;
> > > -}
> > > -#endif
> > > -
> > > -static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_irq_suspend,
> > > -			mt6397_irq_resume);
> > > -
> > >  struct chip_data {
> > >  	u32 cid_addr;
> > >  	u32 cid_shift;
> > > @@ -238,7 +209,6 @@ static struct platform_driver mt6397_driver = {
> > >  	.driver = {
> > >  		.name = "mt6397",
> > >  		.of_match_table = of_match_ptr(mt6397_of_match),
> > > -		.pm = &mt6397_pm_ops,
> > >  	},
> > >  	.id_table = mt6397_id,
> > >  };
> > > diff --git a/drivers/mfd/mt6397-irq.c b/drivers/mfd/mt6397-irq.c
> > > index b2d3ce1..2924919 100644
> > > --- a/drivers/mfd/mt6397-irq.c
> > > +++ b/drivers/mfd/mt6397-irq.c
> > > @@ -9,6 +9,7 @@
> > >  #include <linux/of_irq.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/regmap.h>
> > > +#include <linux/suspend.h>
> > >  #include <linux/mfd/mt6323/core.h>
> > >  #include <linux/mfd/mt6323/registers.h>
> > >  #include <linux/mfd/mt6397/core.h>
> > > @@ -81,7 +82,7 @@ static struct irq_chip mt6397_irq_chip = {
> > >  static void mt6397_irq_handle_reg(struct mt6397_chip *mt6397, int reg,
> > >  				  int irqbase)
> > >  {
> > > -	unsigned int status;
> > > +	unsigned int status = 0;
> > 
> > This looks like an unrelated change, no?
> > 
> 
> It is to fix the coverity defect.

Which isn't mentioned in the commit log and doesn't have anything to
do with this patch.  Thus it should be in a separate patch, but I'm
not going to lose any sleep over it.

> > >  	int i, irq, ret;
> > >  
> > >  	ret = regmap_read(mt6397->regmap, reg, &status);
> > > @@ -128,6 +129,36 @@ static const struct irq_domain_ops mt6397_irq_domain_ops = {
> > >  	.map = mt6397_irq_domain_map,
> > >  };
> > 
> > Other than that.
> > 
> > For my own reference:
> >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> > 
> Thanks for your review. I will add it in the next version.
>
Hsin-Hsiung Wang April 20, 2020, 11:55 a.m. UTC | #4
Hi,

On Mon, 2020-04-20 at 12:15 +0100, Lee Jones wrote:
> On Mon, 20 Apr 2020, Hsin-hsiung Wang wrote:
> 
> > Hi,
> > 
> > On Thu, 2020-04-16 at 09:49 +0100, Lee Jones wrote:
> > > On Wed, 08 Apr 2020, Hsin-Hsiung Wang wrote:
> > > 
> > > > Some pmics don't need backup interrupt settings, so we change to use
> > > > pm notifier for the pmics which are necessary to store settings.
> > > > 
> > > > Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> > > > ---
> > > >  drivers/mfd/mt6397-core.c       | 30 ------------------------------
> > > >  drivers/mfd/mt6397-irq.c        | 35 ++++++++++++++++++++++++++++++++++-
> > > >  include/linux/mfd/mt6397/core.h |  2 ++
> > > >  3 files changed, 36 insertions(+), 31 deletions(-)
> > > > 
> > > > diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> > > > index 0437c85..d2e70d8 100644
> > > > --- a/drivers/mfd/mt6397-core.c
> > > > +++ b/drivers/mfd/mt6397-core.c
> > > > @@ -100,35 +100,6 @@ static const struct mfd_cell mt6397_devs[] = {
> > > >  	}
> > > >  };
> > > >  
> > > > -#ifdef CONFIG_PM_SLEEP
> > > > -static int mt6397_irq_suspend(struct device *dev)
> > > > -{
> > > > -	struct mt6397_chip *chip = dev_get_drvdata(dev);
> > > > -
> > > > -	regmap_write(chip->regmap, chip->int_con[0], chip->wake_mask[0]);
> > > > -	regmap_write(chip->regmap, chip->int_con[1], chip->wake_mask[1]);
> > > > -
> > > > -	enable_irq_wake(chip->irq);
> > > > -
> > > > -	return 0;
> > > > -}
> > > > -
> > > > -static int mt6397_irq_resume(struct device *dev)
> > > > -{
> > > > -	struct mt6397_chip *chip = dev_get_drvdata(dev);
> > > > -
> > > > -	regmap_write(chip->regmap, chip->int_con[0], chip->irq_masks_cur[0]);
> > > > -	regmap_write(chip->regmap, chip->int_con[1], chip->irq_masks_cur[1]);
> > > > -
> > > > -	disable_irq_wake(chip->irq);
> > > > -
> > > > -	return 0;
> > > > -}
> > > > -#endif
> > > > -
> > > > -static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_irq_suspend,
> > > > -			mt6397_irq_resume);
> > > > -
> > > >  struct chip_data {
> > > >  	u32 cid_addr;
> > > >  	u32 cid_shift;
> > > > @@ -238,7 +209,6 @@ static struct platform_driver mt6397_driver = {
> > > >  	.driver = {
> > > >  		.name = "mt6397",
> > > >  		.of_match_table = of_match_ptr(mt6397_of_match),
> > > > -		.pm = &mt6397_pm_ops,
> > > >  	},
> > > >  	.id_table = mt6397_id,
> > > >  };
> > > > diff --git a/drivers/mfd/mt6397-irq.c b/drivers/mfd/mt6397-irq.c
> > > > index b2d3ce1..2924919 100644
> > > > --- a/drivers/mfd/mt6397-irq.c
> > > > +++ b/drivers/mfd/mt6397-irq.c
> > > > @@ -9,6 +9,7 @@
> > > >  #include <linux/of_irq.h>
> > > >  #include <linux/platform_device.h>
> > > >  #include <linux/regmap.h>
> > > > +#include <linux/suspend.h>
> > > >  #include <linux/mfd/mt6323/core.h>
> > > >  #include <linux/mfd/mt6323/registers.h>
> > > >  #include <linux/mfd/mt6397/core.h>
> > > > @@ -81,7 +82,7 @@ static struct irq_chip mt6397_irq_chip = {
> > > >  static void mt6397_irq_handle_reg(struct mt6397_chip *mt6397, int reg,
> > > >  				  int irqbase)
> > > >  {
> > > > -	unsigned int status;
> > > > +	unsigned int status = 0;
> > > 
> > > This looks like an unrelated change, no?
> > > 
> > 
> > It is to fix the coverity defect.
> 
> Which isn't mentioned in the commit log and doesn't have anything to
> do with this patch.  Thus it should be in a separate patch, but I'm
> not going to lose any sleep over it.
> 

Could I just add it to the commit message?
Thanks.

> > > >  	int i, irq, ret;
> > > >  
> > > >  	ret = regmap_read(mt6397->regmap, reg, &status);
> > > > @@ -128,6 +129,36 @@ static const struct irq_domain_ops mt6397_irq_domain_ops = {
> > > >  	.map = mt6397_irq_domain_map,
> > > >  };
> > > 
> > > Other than that.
> > > 
> > > For my own reference:
> > >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> > > 
> > Thanks for your review. I will add it in the next version.
> > 
>
diff mbox series

Patch

diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index 0437c85..d2e70d8 100644
--- a/drivers/mfd/mt6397-core.c
+++ b/drivers/mfd/mt6397-core.c
@@ -100,35 +100,6 @@  static const struct mfd_cell mt6397_devs[] = {
 	}
 };
 
-#ifdef CONFIG_PM_SLEEP
-static int mt6397_irq_suspend(struct device *dev)
-{
-	struct mt6397_chip *chip = dev_get_drvdata(dev);
-
-	regmap_write(chip->regmap, chip->int_con[0], chip->wake_mask[0]);
-	regmap_write(chip->regmap, chip->int_con[1], chip->wake_mask[1]);
-
-	enable_irq_wake(chip->irq);
-
-	return 0;
-}
-
-static int mt6397_irq_resume(struct device *dev)
-{
-	struct mt6397_chip *chip = dev_get_drvdata(dev);
-
-	regmap_write(chip->regmap, chip->int_con[0], chip->irq_masks_cur[0]);
-	regmap_write(chip->regmap, chip->int_con[1], chip->irq_masks_cur[1]);
-
-	disable_irq_wake(chip->irq);
-
-	return 0;
-}
-#endif
-
-static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_irq_suspend,
-			mt6397_irq_resume);
-
 struct chip_data {
 	u32 cid_addr;
 	u32 cid_shift;
@@ -238,7 +209,6 @@  static struct platform_driver mt6397_driver = {
 	.driver = {
 		.name = "mt6397",
 		.of_match_table = of_match_ptr(mt6397_of_match),
-		.pm = &mt6397_pm_ops,
 	},
 	.id_table = mt6397_id,
 };
diff --git a/drivers/mfd/mt6397-irq.c b/drivers/mfd/mt6397-irq.c
index b2d3ce1..2924919 100644
--- a/drivers/mfd/mt6397-irq.c
+++ b/drivers/mfd/mt6397-irq.c
@@ -9,6 +9,7 @@ 
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/suspend.h>
 #include <linux/mfd/mt6323/core.h>
 #include <linux/mfd/mt6323/registers.h>
 #include <linux/mfd/mt6397/core.h>
@@ -81,7 +82,7 @@  static struct irq_chip mt6397_irq_chip = {
 static void mt6397_irq_handle_reg(struct mt6397_chip *mt6397, int reg,
 				  int irqbase)
 {
-	unsigned int status;
+	unsigned int status = 0;
 	int i, irq, ret;
 
 	ret = regmap_read(mt6397->regmap, reg, &status);
@@ -128,6 +129,36 @@  static const struct irq_domain_ops mt6397_irq_domain_ops = {
 	.map = mt6397_irq_domain_map,
 };
 
+static int mt6397_irq_pm_notifier(struct notifier_block *notifier,
+				  unsigned long pm_event, void *unused)
+{
+	struct mt6397_chip *chip =
+		container_of(notifier, struct mt6397_chip, pm_nb);
+
+	switch (pm_event) {
+	case PM_SUSPEND_PREPARE:
+		regmap_write(chip->regmap,
+			     chip->int_con[0], chip->wake_mask[0]);
+		regmap_write(chip->regmap,
+			     chip->int_con[1], chip->wake_mask[1]);
+		enable_irq_wake(chip->irq);
+		break;
+
+	case PM_POST_SUSPEND:
+		regmap_write(chip->regmap,
+			     chip->int_con[0], chip->irq_masks_cur[0]);
+		regmap_write(chip->regmap,
+			     chip->int_con[1], chip->irq_masks_cur[1]);
+		disable_irq_wake(chip->irq);
+		break;
+
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
 int mt6397_irq_init(struct mt6397_chip *chip)
 {
 	int ret;
@@ -159,6 +190,7 @@  int mt6397_irq_init(struct mt6397_chip *chip)
 	regmap_write(chip->regmap, chip->int_con[0], 0x0);
 	regmap_write(chip->regmap, chip->int_con[1], 0x0);
 
+	chip->pm_nb.notifier_call = mt6397_irq_pm_notifier;
 	chip->irq_domain = irq_domain_add_linear(chip->dev->of_node,
 						 MT6397_IRQ_NR,
 						 &mt6397_irq_domain_ops,
@@ -177,5 +209,6 @@  int mt6397_irq_init(struct mt6397_chip *chip)
 		return ret;
 	}
 
+	register_pm_notifier(&chip->pm_nb);
 	return 0;
 }
diff --git a/include/linux/mfd/mt6397/core.h b/include/linux/mfd/mt6397/core.h
index fc88d31..b81d333 100644
--- a/include/linux/mfd/mt6397/core.h
+++ b/include/linux/mfd/mt6397/core.h
@@ -8,6 +8,7 @@ 
 #define __MFD_MT6397_CORE_H__
 
 #include <linux/mutex.h>
+#include <linux/notifier.h>
 
 enum chip_id {
 	MT6323_CHIP_ID = 0x23,
@@ -54,6 +55,7 @@  enum mt6397_irq_numbers {
 struct mt6397_chip {
 	struct device *dev;
 	struct regmap *regmap;
+	struct notifier_block pm_nb;
 	int irq;
 	struct irq_domain *irq_domain;
 	struct mutex irqlock;