| Message ID | 1580899678-26393-1-git-send-email-fabrice.gasnier@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | counter: stm32-timer-cnt: add power management support | expand |
On Wed, Feb 05, 2020 at 11:47:58AM +0100, Fabrice Gasnier wrote: > Add suspend/resume PM sleep ops. When going to low power, enforce the > counter isn't active. > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> These changes are purely for the PM ops and don't involve the Counter API, so I don't see any see any issues with this patch from my end. Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com> > --- > drivers/counter/stm32-timer-cnt.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c > index 3eafcce..3b84503 100644 > --- a/drivers/counter/stm32-timer-cnt.c > +++ b/drivers/counter/stm32-timer-cnt.c > @@ -12,6 +12,7 @@ > #include <linux/iio/types.h> > #include <linux/mfd/stm32-timers.h> > #include <linux/module.h> > +#include <linux/pinctrl/consumer.h> > #include <linux/platform_device.h> > > #define TIM_CCMR_CCXS (BIT(8) | BIT(0)) > @@ -358,10 +359,33 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev) > priv->counter.num_signals = ARRAY_SIZE(stm32_signals); > priv->counter.priv = priv; > > + platform_set_drvdata(pdev, priv); > + > /* Register Counter device */ > return devm_counter_register(dev, &priv->counter); > } > > +static int __maybe_unused stm32_timer_cnt_suspend(struct device *dev) > +{ > + struct stm32_timer_cnt *priv = dev_get_drvdata(dev); > + u32 cr1; > + > + /* Check for active counter */ > + regmap_read(priv->regmap, TIM_CR1, &cr1); > + if (cr1 & TIM_CR1_CEN) > + return -EBUSY; > + > + return pinctrl_pm_select_sleep_state(dev); > +} > + > +static int __maybe_unused stm32_timer_cnt_resume(struct device *dev) > +{ > + return pinctrl_pm_select_default_state(dev); > +} > + > +static SIMPLE_DEV_PM_OPS(stm32_timer_cnt_pm_ops, stm32_timer_cnt_suspend, > + stm32_timer_cnt_resume); > + > static const struct of_device_id stm32_timer_cnt_of_match[] = { > { .compatible = "st,stm32-timer-counter", }, > {}, > @@ -373,6 +397,7 @@ static struct platform_driver stm32_timer_cnt_driver = { > .driver = { > .name = "stm32-timer-counter", > .of_match_table = stm32_timer_cnt_of_match, > + .pm = &stm32_timer_cnt_pm_ops, > }, > }; > module_platform_driver(stm32_timer_cnt_driver); > -- > 2.7.4 >
On Wed, 5 Feb 2020 11:47:58 +0100 Fabrice Gasnier <fabrice.gasnier@st.com> wrote: > Add suspend/resume PM sleep ops. When going to low power, enforce the > counter isn't active. > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > --- > drivers/counter/stm32-timer-cnt.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c > index 3eafcce..3b84503 100644 > --- a/drivers/counter/stm32-timer-cnt.c > +++ b/drivers/counter/stm32-timer-cnt.c > @@ -12,6 +12,7 @@ > #include <linux/iio/types.h> > #include <linux/mfd/stm32-timers.h> > #include <linux/module.h> > +#include <linux/pinctrl/consumer.h> > #include <linux/platform_device.h> > > #define TIM_CCMR_CCXS (BIT(8) | BIT(0)) > @@ -358,10 +359,33 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev) > priv->counter.num_signals = ARRAY_SIZE(stm32_signals); > priv->counter.priv = priv; > > + platform_set_drvdata(pdev, priv); > + > /* Register Counter device */ > return devm_counter_register(dev, &priv->counter); > } > > +static int __maybe_unused stm32_timer_cnt_suspend(struct device *dev) > +{ > + struct stm32_timer_cnt *priv = dev_get_drvdata(dev); > + u32 cr1; > + > + /* Check for active counter */ > + regmap_read(priv->regmap, TIM_CR1, &cr1); > + if (cr1 & TIM_CR1_CEN) > + return -EBUSY; Runtime suspend is allowed to refuse to happen (and that is explicitly documented) Is that allowed for normal suspend? I didn't think it was. This will stop a suspend, but it's doing it by signalling a failure. I'm not sure for example that the system will resume everything that was suspended before this call. Normally you are in the tough luck category if a system decides to suspend underneath you. There are ways to prevent it from userspace (wakelocks etc). > + > + return pinctrl_pm_select_sleep_state(dev); > +} > + > +static int __maybe_unused stm32_timer_cnt_resume(struct device *dev) > +{ > + return pinctrl_pm_select_default_state(dev); > +} > + > +static SIMPLE_DEV_PM_OPS(stm32_timer_cnt_pm_ops, stm32_timer_cnt_suspend, > + stm32_timer_cnt_resume); > + > static const struct of_device_id stm32_timer_cnt_of_match[] = { > { .compatible = "st,stm32-timer-counter", }, > {}, > @@ -373,6 +397,7 @@ static struct platform_driver stm32_timer_cnt_driver = { > .driver = { > .name = "stm32-timer-counter", > .of_match_table = stm32_timer_cnt_of_match, > + .pm = &stm32_timer_cnt_pm_ops, > }, > }; > module_platform_driver(stm32_timer_cnt_driver);
On 2/8/20 4:35 PM, Jonathan Cameron wrote: > On Wed, 5 Feb 2020 11:47:58 +0100 > Fabrice Gasnier <fabrice.gasnier@st.com> wrote: > >> Add suspend/resume PM sleep ops. When going to low power, enforce the >> counter isn't active. >> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> >> --- >> drivers/counter/stm32-timer-cnt.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c >> index 3eafcce..3b84503 100644 >> --- a/drivers/counter/stm32-timer-cnt.c >> +++ b/drivers/counter/stm32-timer-cnt.c >> @@ -12,6 +12,7 @@ >> #include <linux/iio/types.h> >> #include <linux/mfd/stm32-timers.h> >> #include <linux/module.h> >> +#include <linux/pinctrl/consumer.h> >> #include <linux/platform_device.h> >> >> #define TIM_CCMR_CCXS (BIT(8) | BIT(0)) >> @@ -358,10 +359,33 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev) >> priv->counter.num_signals = ARRAY_SIZE(stm32_signals); >> priv->counter.priv = priv; >> >> + platform_set_drvdata(pdev, priv); >> + >> /* Register Counter device */ >> return devm_counter_register(dev, &priv->counter); >> } >> >> +static int __maybe_unused stm32_timer_cnt_suspend(struct device *dev) >> +{ >> + struct stm32_timer_cnt *priv = dev_get_drvdata(dev); >> + u32 cr1; >> + >> + /* Check for active counter */ >> + regmap_read(priv->regmap, TIM_CR1, &cr1); >> + if (cr1 & TIM_CR1_CEN) >> + return -EBUSY; > > Runtime suspend is allowed to refuse to happen (and that is > explicitly documented) > Is that allowed for normal suspend? I didn't think it was. > This will stop a suspend, but it's doing it by signalling a failure. Hi Jonathan, I'll rework this patch, to avoid stopping the suspend. Another approach is to properly save/backup the registers here, and to disable the counter in low power mode. I'll update this in V2. Thanks for reviewing, Fabrice > I'm not sure for example that the system will resume everything that > was suspended before this call. > > Normally you are in the tough luck category if a system decides > to suspend underneath you. There are ways to prevent it > from userspace (wakelocks etc). > >> + >> + return pinctrl_pm_select_sleep_state(dev); >> +} >> + >> +static int __maybe_unused stm32_timer_cnt_resume(struct device *dev) >> +{ >> + return pinctrl_pm_select_default_state(dev); >> +} >> + >> +static SIMPLE_DEV_PM_OPS(stm32_timer_cnt_pm_ops, stm32_timer_cnt_suspend, >> + stm32_timer_cnt_resume); >> + >> static const struct of_device_id stm32_timer_cnt_of_match[] = { >> { .compatible = "st,stm32-timer-counter", }, >> {}, >> @@ -373,6 +397,7 @@ static struct platform_driver stm32_timer_cnt_driver = { >> .driver = { >> .name = "stm32-timer-counter", >> .of_match_table = stm32_timer_cnt_of_match, >> + .pm = &stm32_timer_cnt_pm_ops, >> }, >> }; >> module_platform_driver(stm32_timer_cnt_driver); >
diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c index 3eafcce..3b84503 100644 --- a/drivers/counter/stm32-timer-cnt.c +++ b/drivers/counter/stm32-timer-cnt.c @@ -12,6 +12,7 @@ #include <linux/iio/types.h> #include <linux/mfd/stm32-timers.h> #include <linux/module.h> +#include <linux/pinctrl/consumer.h> #include <linux/platform_device.h> #define TIM_CCMR_CCXS (BIT(8) | BIT(0)) @@ -358,10 +359,33 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev) priv->counter.num_signals = ARRAY_SIZE(stm32_signals); priv->counter.priv = priv; + platform_set_drvdata(pdev, priv); + /* Register Counter device */ return devm_counter_register(dev, &priv->counter); } +static int __maybe_unused stm32_timer_cnt_suspend(struct device *dev) +{ + struct stm32_timer_cnt *priv = dev_get_drvdata(dev); + u32 cr1; + + /* Check for active counter */ + regmap_read(priv->regmap, TIM_CR1, &cr1); + if (cr1 & TIM_CR1_CEN) + return -EBUSY; + + return pinctrl_pm_select_sleep_state(dev); +} + +static int __maybe_unused stm32_timer_cnt_resume(struct device *dev) +{ + return pinctrl_pm_select_default_state(dev); +} + +static SIMPLE_DEV_PM_OPS(stm32_timer_cnt_pm_ops, stm32_timer_cnt_suspend, + stm32_timer_cnt_resume); + static const struct of_device_id stm32_timer_cnt_of_match[] = { { .compatible = "st,stm32-timer-counter", }, {}, @@ -373,6 +397,7 @@ static struct platform_driver stm32_timer_cnt_driver = { .driver = { .name = "stm32-timer-counter", .of_match_table = stm32_timer_cnt_of_match, + .pm = &stm32_timer_cnt_pm_ops, }, }; module_platform_driver(stm32_timer_cnt_driver);
Add suspend/resume PM sleep ops. When going to low power, enforce the counter isn't active. Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> --- drivers/counter/stm32-timer-cnt.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)