| Message ID | 1586333531-21641-2-git-send-email-hsin-hsiung.wang@mediatek.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Add Support for MediaTek PMIC MT6358 | expand |
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>
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.
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. >
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 --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;
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(-)