* [PATCH v2 0/3] Add PM support to STM32 Timer PWM @ 2019-10-04 12:53 Fabrice Gasnier 2019-10-04 12:53 ` [PATCH v2 1/3] dt-bindings: pwm-stm32: document pinctrl sleep state Fabrice Gasnier ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Fabrice Gasnier @ 2019-10-04 12:53 UTC (permalink / raw) To: thierry.reding, robh+dt, u.kleine-koenig Cc: alexandre.torgue, mark.rutland, mcoquelin.stm32, fabrice.gasnier, devicetree, linux-arm-kernel, linux-kernel, linux-pwm, benjamin.gaignard, linux-stm32 This patch series adds power management support for STM32 Timer PWM: - Document the pinctrl sleep state for STM32 Timer PWM - STM32 Timer PWM driver --- Changes in v2: Follow Uwe suggestions/remarks: - Add a precursor patch to ease reviewing - Use registers read instead of pwm_get_state - Add a comment to mention registers content may be lost in low power mode Fabrice Gasnier (3): dt-bindings: pwm-stm32: document pinctrl sleep state pwm: stm32: split breakinput apply routine to ease PM support pwm: stm32: add power management support .../devicetree/bindings/pwm/pwm-stm32.txt | 8 +- drivers/pwm/pwm-stm32.c | 86 +++++++++++++++++----- 2 files changed, 71 insertions(+), 23 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] dt-bindings: pwm-stm32: document pinctrl sleep state 2019-10-04 12:53 [PATCH v2 0/3] Add PM support to STM32 Timer PWM Fabrice Gasnier @ 2019-10-04 12:53 ` Fabrice Gasnier 2019-10-11 15:10 ` Rob Herring 2019-10-16 6:59 ` Thierry Reding 2019-10-04 12:53 ` [PATCH v2 2/3] pwm: stm32: split breakinput apply routine to ease PM support Fabrice Gasnier 2019-10-04 12:53 ` [PATCH v2 3/3] pwm: stm32: add power management support Fabrice Gasnier 2 siblings, 2 replies; 9+ messages in thread From: Fabrice Gasnier @ 2019-10-04 12:53 UTC (permalink / raw) To: thierry.reding, robh+dt, u.kleine-koenig Cc: alexandre.torgue, mark.rutland, mcoquelin.stm32, fabrice.gasnier, devicetree, linux-arm-kernel, linux-kernel, linux-pwm, benjamin.gaignard, linux-stm32 Add documentation for pinctrl sleep state that can be used by STM32 timers PWM. Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> --- Documentation/devicetree/bindings/pwm/pwm-stm32.txt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt index a8690bf..f1620c1 100644 --- a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt @@ -5,8 +5,9 @@ See ../mfd/stm32-timers.txt for details about the parent node. Required parameters: - compatible: Must be "st,stm32-pwm". -- pinctrl-names: Set to "default". -- pinctrl-0: List of phandles pointing to pin configuration nodes for PWM module. +- pinctrl-names: Set to "default". An additional "sleep" state can be + defined to set pins in sleep state when in low power. +- pinctrl-n: List of phandles pointing to pin configuration nodes for PWM module. For Pinctrl properties see ../pinctrl/pinctrl-bindings.txt - #pwm-cells: Should be set to 3. This PWM chip uses the default 3 cells bindings defined in pwm.txt. @@ -32,7 +33,8 @@ Example: compatible = "st,stm32-pwm"; #pwm-cells = <3>; pinctrl-0 = <&pwm1_pins>; - pinctrl-names = "default"; + pinctrl-1 = <&pwm1_sleep_pins>; + pinctrl-names = "default", "sleep"; st,breakinput = <0 1 5>; }; }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: pwm-stm32: document pinctrl sleep state 2019-10-04 12:53 ` [PATCH v2 1/3] dt-bindings: pwm-stm32: document pinctrl sleep state Fabrice Gasnier @ 2019-10-11 15:10 ` Rob Herring 2019-10-16 6:59 ` Thierry Reding 1 sibling, 0 replies; 9+ messages in thread From: Rob Herring @ 2019-10-11 15:10 UTC (permalink / raw) To: Fabrice Gasnier Cc: thierry.reding, robh+dt, u.kleine-koenig, alexandre.torgue, mark.rutland, mcoquelin.stm32, fabrice.gasnier, devicetree, linux-arm-kernel, linux-kernel, linux-pwm, benjamin.gaignard, linux-stm32 On Fri, 4 Oct 2019 14:53:51 +0200, Fabrice Gasnier wrote: > Add documentation for pinctrl sleep state that can be used by > STM32 timers PWM. > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > --- > Documentation/devicetree/bindings/pwm/pwm-stm32.txt | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: pwm-stm32: document pinctrl sleep state 2019-10-04 12:53 ` [PATCH v2 1/3] dt-bindings: pwm-stm32: document pinctrl sleep state Fabrice Gasnier 2019-10-11 15:10 ` Rob Herring @ 2019-10-16 6:59 ` Thierry Reding 1 sibling, 0 replies; 9+ messages in thread From: Thierry Reding @ 2019-10-16 6:59 UTC (permalink / raw) To: Fabrice Gasnier Cc: robh+dt, u.kleine-koenig, alexandre.torgue, mark.rutland, mcoquelin.stm32, devicetree, linux-arm-kernel, linux-kernel, linux-pwm, benjamin.gaignard, linux-stm32 [-- Attachment #1: Type: text/plain, Size: 374 bytes --] On Fri, Oct 04, 2019 at 02:53:51PM +0200, Fabrice Gasnier wrote: > Add documentation for pinctrl sleep state that can be used by > STM32 timers PWM. > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > --- > Documentation/devicetree/bindings/pwm/pwm-stm32.txt | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) Applied, thanks. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] pwm: stm32: split breakinput apply routine to ease PM support 2019-10-04 12:53 [PATCH v2 0/3] Add PM support to STM32 Timer PWM Fabrice Gasnier 2019-10-04 12:53 ` [PATCH v2 1/3] dt-bindings: pwm-stm32: document pinctrl sleep state Fabrice Gasnier @ 2019-10-04 12:53 ` Fabrice Gasnier 2019-10-16 7:03 ` Thierry Reding 2019-10-04 12:53 ` [PATCH v2 3/3] pwm: stm32: add power management support Fabrice Gasnier 2 siblings, 1 reply; 9+ messages in thread From: Fabrice Gasnier @ 2019-10-04 12:53 UTC (permalink / raw) To: thierry.reding, robh+dt, u.kleine-koenig Cc: alexandre.torgue, mark.rutland, mcoquelin.stm32, fabrice.gasnier, devicetree, linux-arm-kernel, linux-kernel, linux-pwm, benjamin.gaignard, linux-stm32 Split breakinput routine that configures STM32 timers 'break' safety feature upon probe, into two routines: - stm32_pwm_apply_breakinputs() sets all the break inputs into registers. - stm32_pwm_probe_breakinputs() probes the device tree break input settings before calling stm32_pwm_apply_breakinputs() This is a precursor patch to ease PM support. Registers content may get lost during low power. So, break input settings applied upon probe need to be restored upon resume (e.g. by calling stm32_pwm_apply_breakinputs()). Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> --- drivers/pwm/pwm-stm32.c | 48 ++++++++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index 359b085..cf8658c 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -19,6 +19,12 @@ #define CCMR_CHANNEL_MASK 0xFF #define MAX_BREAKINPUT 2 +struct stm32_breakinput { + u32 index; + u32 level; + u32 filter; +}; + struct stm32_pwm { struct pwm_chip chip; struct mutex lock; /* protect pwm config/enable */ @@ -26,15 +32,11 @@ struct stm32_pwm { struct regmap *regmap; u32 max_arr; bool have_complementary_output; + struct stm32_breakinput breakinput[MAX_BREAKINPUT]; + unsigned int nbreakinput; u32 capture[4] ____cacheline_aligned; /* DMA'able buffer */ }; -struct stm32_breakinput { - u32 index; - u32 level; - u32 filter; -}; - static inline struct stm32_pwm *to_stm32_pwm_dev(struct pwm_chip *chip) { return container_of(chip, struct stm32_pwm, chip); @@ -512,15 +514,27 @@ static int stm32_pwm_set_breakinput(struct stm32_pwm *priv, return (bdtr & bke) ? 0 : -EINVAL; } -static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv, +static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv) +{ + int i, ret = 0; + + for (i = 0; i < priv->nbreakinput && !ret; i++) { + ret = stm32_pwm_set_breakinput(priv, + priv->breakinput[i].index, + priv->breakinput[i].level, + priv->breakinput[i].filter); + } + + return ret; +} + +static int stm32_pwm_probe_breakinputs(struct stm32_pwm *priv, struct device_node *np) { - struct stm32_breakinput breakinput[MAX_BREAKINPUT]; - int nb, ret, i, array_size; + int nb, ret, array_size; nb = of_property_count_elems_of_size(np, "st,breakinput", sizeof(struct stm32_breakinput)); - /* * Because "st,breakinput" parameter is optional do not make probe * failed if it doesn't exist. @@ -531,20 +545,14 @@ static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv, if (nb > MAX_BREAKINPUT) return -EINVAL; + priv->nbreakinput = nb; array_size = nb * sizeof(struct stm32_breakinput) / sizeof(u32); ret = of_property_read_u32_array(np, "st,breakinput", - (u32 *)breakinput, array_size); + (u32 *)priv->breakinput, array_size); if (ret) return ret; - for (i = 0; i < nb && !ret; i++) { - ret = stm32_pwm_set_breakinput(priv, - breakinput[i].index, - breakinput[i].level, - breakinput[i].filter); - } - - return ret; + return stm32_pwm_apply_breakinputs(priv); } static void stm32_pwm_detect_complementary(struct stm32_pwm *priv) @@ -614,7 +622,7 @@ static int stm32_pwm_probe(struct platform_device *pdev) if (!priv->regmap || !priv->clk) return -EINVAL; - ret = stm32_pwm_apply_breakinputs(priv, np); + ret = stm32_pwm_probe_breakinputs(priv, np); if (ret) return ret; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] pwm: stm32: split breakinput apply routine to ease PM support 2019-10-04 12:53 ` [PATCH v2 2/3] pwm: stm32: split breakinput apply routine to ease PM support Fabrice Gasnier @ 2019-10-16 7:03 ` Thierry Reding 0 siblings, 0 replies; 9+ messages in thread From: Thierry Reding @ 2019-10-16 7:03 UTC (permalink / raw) To: Fabrice Gasnier Cc: robh+dt, u.kleine-koenig, alexandre.torgue, mark.rutland, mcoquelin.stm32, devicetree, linux-arm-kernel, linux-kernel, linux-pwm, benjamin.gaignard, linux-stm32 [-- Attachment #1: Type: text/plain, Size: 4424 bytes --] On Fri, Oct 04, 2019 at 02:53:52PM +0200, Fabrice Gasnier wrote: > Split breakinput routine that configures STM32 timers 'break' safety > feature upon probe, into two routines: > - stm32_pwm_apply_breakinputs() sets all the break inputs into registers. > - stm32_pwm_probe_breakinputs() probes the device tree break input settings > before calling stm32_pwm_apply_breakinputs() > > This is a precursor patch to ease PM support. Registers content may get > lost during low power. So, break input settings applied upon probe need > to be restored upon resume (e.g. by calling stm32_pwm_apply_breakinputs()). > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > --- > drivers/pwm/pwm-stm32.c | 48 ++++++++++++++++++++++++++++-------------------- > 1 file changed, 28 insertions(+), 20 deletions(-) Applied, thanks. I've made some minor changes, mostly for consistency with other drivers and the PWM core. See below. > diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c > index 359b085..cf8658c 100644 > --- a/drivers/pwm/pwm-stm32.c > +++ b/drivers/pwm/pwm-stm32.c > @@ -19,6 +19,12 @@ > #define CCMR_CHANNEL_MASK 0xFF > #define MAX_BREAKINPUT 2 > > +struct stm32_breakinput { > + u32 index; > + u32 level; > + u32 filter; > +}; > + > struct stm32_pwm { > struct pwm_chip chip; > struct mutex lock; /* protect pwm config/enable */ > @@ -26,15 +32,11 @@ struct stm32_pwm { > struct regmap *regmap; > u32 max_arr; > bool have_complementary_output; > + struct stm32_breakinput breakinput[MAX_BREAKINPUT]; > + unsigned int nbreakinput; I changed these to breakinputs and num_breakinputs since they are slightly more consistent with the naming elsewhere in PWM. > u32 capture[4] ____cacheline_aligned; /* DMA'able buffer */ > }; > > -struct stm32_breakinput { > - u32 index; > - u32 level; > - u32 filter; > -}; > - > static inline struct stm32_pwm *to_stm32_pwm_dev(struct pwm_chip *chip) > { > return container_of(chip, struct stm32_pwm, chip); > @@ -512,15 +514,27 @@ static int stm32_pwm_set_breakinput(struct stm32_pwm *priv, > return (bdtr & bke) ? 0 : -EINVAL; > } > > -static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv, > +static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv) > +{ > + int i, ret = 0; Made i unsigned int. > + > + for (i = 0; i < priv->nbreakinput && !ret; i++) { > + ret = stm32_pwm_set_breakinput(priv, > + priv->breakinput[i].index, > + priv->breakinput[i].level, > + priv->breakinput[i].filter); > + } I thought this was a little odd, so I changed it to explicitly check the value of ret and return on error. > + > + return ret; And then this became "return 0;" > +} > + > +static int stm32_pwm_probe_breakinputs(struct stm32_pwm *priv, > struct device_node *np) > { > - struct stm32_breakinput breakinput[MAX_BREAKINPUT]; > - int nb, ret, i, array_size; > + int nb, ret, array_size; > > nb = of_property_count_elems_of_size(np, "st,breakinput", > sizeof(struct stm32_breakinput)); > - Dropped this since it made the code look cluttered. Thierry > /* > * Because "st,breakinput" parameter is optional do not make probe > * failed if it doesn't exist. > @@ -531,20 +545,14 @@ static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv, > if (nb > MAX_BREAKINPUT) > return -EINVAL; > > + priv->nbreakinput = nb; > array_size = nb * sizeof(struct stm32_breakinput) / sizeof(u32); > ret = of_property_read_u32_array(np, "st,breakinput", > - (u32 *)breakinput, array_size); > + (u32 *)priv->breakinput, array_size); > if (ret) > return ret; > > - for (i = 0; i < nb && !ret; i++) { > - ret = stm32_pwm_set_breakinput(priv, > - breakinput[i].index, > - breakinput[i].level, > - breakinput[i].filter); > - } > - > - return ret; > + return stm32_pwm_apply_breakinputs(priv); > } > > static void stm32_pwm_detect_complementary(struct stm32_pwm *priv) > @@ -614,7 +622,7 @@ static int stm32_pwm_probe(struct platform_device *pdev) > if (!priv->regmap || !priv->clk) > return -EINVAL; > > - ret = stm32_pwm_apply_breakinputs(priv, np); > + ret = stm32_pwm_probe_breakinputs(priv, np); > if (ret) > return ret; > > -- > 2.7.4 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] pwm: stm32: add power management support 2019-10-04 12:53 [PATCH v2 0/3] Add PM support to STM32 Timer PWM Fabrice Gasnier 2019-10-04 12:53 ` [PATCH v2 1/3] dt-bindings: pwm-stm32: document pinctrl sleep state Fabrice Gasnier 2019-10-04 12:53 ` [PATCH v2 2/3] pwm: stm32: split breakinput apply routine to ease PM support Fabrice Gasnier @ 2019-10-04 12:53 ` Fabrice Gasnier 2019-10-16 7:06 ` Thierry Reding 2 siblings, 1 reply; 9+ messages in thread From: Fabrice Gasnier @ 2019-10-04 12:53 UTC (permalink / raw) To: thierry.reding, robh+dt, u.kleine-koenig Cc: alexandre.torgue, mark.rutland, mcoquelin.stm32, fabrice.gasnier, devicetree, linux-arm-kernel, linux-kernel, linux-pwm, benjamin.gaignard, linux-stm32 Add suspend/resume PM sleep ops. When going to low power, enforce the PWM channel isn't active. Let the PWM consumers disable it during their own suspend sequence, see [1]. So, perform a check here, and handle the pinctrl states. Also restore the break inputs upon resume, as registers content may be lost when going to low power mode. [1] https://lkml.org/lkml/2019/2/5/770 Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> --- Changes in v2: Follow Uwe suggestions/remarks: - Add a precursor patch to ease reviewing - Use registers read instead of pwm_get_state - Add a comment to mention registers content may be lost in low power mode --- drivers/pwm/pwm-stm32.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index cf8658c..546b661 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -12,6 +12,7 @@ #include <linux/mfd/stm32-timers.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/pinctrl/consumer.h> #include <linux/platform_device.h> #include <linux/pwm.h> @@ -655,6 +656,42 @@ static int stm32_pwm_remove(struct platform_device *pdev) return 0; } +static int __maybe_unused stm32_pwm_suspend(struct device *dev) +{ + struct stm32_pwm *priv = dev_get_drvdata(dev); + unsigned int ch; + u32 ccer, mask; + + /* Look for active channels */ + ccer = active_channels(priv); + + for (ch = 0; ch < priv->chip.npwm; ch++) { + mask = TIM_CCER_CC1E << (ch * 4); + if (ccer & mask) { + dev_err(dev, "The consumer didn't stop us (%s)\n", + priv->chip.pwms[ch].label); + return -EBUSY; + } + } + + return pinctrl_pm_select_sleep_state(dev); +} + +static int __maybe_unused stm32_pwm_resume(struct device *dev) +{ + struct stm32_pwm *priv = dev_get_drvdata(dev); + int ret; + + ret = pinctrl_pm_select_default_state(dev); + if (ret) + return ret; + + /* restore breakinput registers that may have been lost in low power */ + return stm32_pwm_apply_breakinputs(priv); +} + +static SIMPLE_DEV_PM_OPS(stm32_pwm_pm_ops, stm32_pwm_suspend, stm32_pwm_resume); + static const struct of_device_id stm32_pwm_of_match[] = { { .compatible = "st,stm32-pwm", }, { /* end node */ }, @@ -667,6 +704,7 @@ static struct platform_driver stm32_pwm_driver = { .driver = { .name = "stm32-pwm", .of_match_table = stm32_pwm_of_match, + .pm = &stm32_pwm_pm_ops, }, }; module_platform_driver(stm32_pwm_driver); -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] pwm: stm32: add power management support 2019-10-04 12:53 ` [PATCH v2 3/3] pwm: stm32: add power management support Fabrice Gasnier @ 2019-10-16 7:06 ` Thierry Reding 2019-10-16 10:08 ` Fabrice Gasnier 0 siblings, 1 reply; 9+ messages in thread From: Thierry Reding @ 2019-10-16 7:06 UTC (permalink / raw) To: Fabrice Gasnier Cc: robh+dt, u.kleine-koenig, alexandre.torgue, mark.rutland, mcoquelin.stm32, devicetree, linux-arm-kernel, linux-kernel, linux-pwm, benjamin.gaignard, linux-stm32 [-- Attachment #1: Type: text/plain, Size: 3230 bytes --] On Fri, Oct 04, 2019 at 02:53:53PM +0200, Fabrice Gasnier wrote: > Add suspend/resume PM sleep ops. When going to low power, enforce the PWM > channel isn't active. Let the PWM consumers disable it during their own > suspend sequence, see [1]. So, perform a check here, and handle the > pinctrl states. Also restore the break inputs upon resume, as registers > content may be lost when going to low power mode. > > [1] https://lkml.org/lkml/2019/2/5/770 > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > --- > Changes in v2: > Follow Uwe suggestions/remarks: > - Add a precursor patch to ease reviewing > - Use registers read instead of pwm_get_state > - Add a comment to mention registers content may be lost in low power mode > --- > drivers/pwm/pwm-stm32.c | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) Applied, thanks. I made two minor changes, though, see below. > > diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c > index cf8658c..546b661 100644 > --- a/drivers/pwm/pwm-stm32.c > +++ b/drivers/pwm/pwm-stm32.c > @@ -12,6 +12,7 @@ > #include <linux/mfd/stm32-timers.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/pinctrl/consumer.h> > #include <linux/platform_device.h> > #include <linux/pwm.h> > > @@ -655,6 +656,42 @@ static int stm32_pwm_remove(struct platform_device *pdev) > return 0; > } > > +static int __maybe_unused stm32_pwm_suspend(struct device *dev) > +{ > + struct stm32_pwm *priv = dev_get_drvdata(dev); > + unsigned int ch; I renamed this to just "i", which is more idiomatic for loop variables. The function is small enough not to need to differentiate between loop variables. > + u32 ccer, mask; > + > + /* Look for active channels */ > + ccer = active_channels(priv); > + > + for (ch = 0; ch < priv->chip.npwm; ch++) { > + mask = TIM_CCER_CC1E << (ch * 4); > + if (ccer & mask) { > + dev_err(dev, "The consumer didn't stop us (%s)\n", > + priv->chip.pwms[ch].label); Changed this to: "PWM %u still in use by consumer %s\n", i, priv->chip.pwms[i].label I think that might help clarify which PWM is still enabled in case the consumers don't set a label. Thierry > + return -EBUSY; > + } > + } > + > + return pinctrl_pm_select_sleep_state(dev); > +} > + > +static int __maybe_unused stm32_pwm_resume(struct device *dev) > +{ > + struct stm32_pwm *priv = dev_get_drvdata(dev); > + int ret; > + > + ret = pinctrl_pm_select_default_state(dev); > + if (ret) > + return ret; > + > + /* restore breakinput registers that may have been lost in low power */ > + return stm32_pwm_apply_breakinputs(priv); > +} > + > +static SIMPLE_DEV_PM_OPS(stm32_pwm_pm_ops, stm32_pwm_suspend, stm32_pwm_resume); > + > static const struct of_device_id stm32_pwm_of_match[] = { > { .compatible = "st,stm32-pwm", }, > { /* end node */ }, > @@ -667,6 +704,7 @@ static struct platform_driver stm32_pwm_driver = { > .driver = { > .name = "stm32-pwm", > .of_match_table = stm32_pwm_of_match, > + .pm = &stm32_pwm_pm_ops, > }, > }; > module_platform_driver(stm32_pwm_driver); > -- > 2.7.4 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] pwm: stm32: add power management support 2019-10-16 7:06 ` Thierry Reding @ 2019-10-16 10:08 ` Fabrice Gasnier 0 siblings, 0 replies; 9+ messages in thread From: Fabrice Gasnier @ 2019-10-16 10:08 UTC (permalink / raw) To: Thierry Reding Cc: robh+dt, u.kleine-koenig, alexandre.torgue, mark.rutland, mcoquelin.stm32, devicetree, linux-arm-kernel, linux-kernel, linux-pwm, benjamin.gaignard, linux-stm32 On 10/16/19 9:06 AM, Thierry Reding wrote: > On Fri, Oct 04, 2019 at 02:53:53PM +0200, Fabrice Gasnier wrote: >> Add suspend/resume PM sleep ops. When going to low power, enforce the PWM >> channel isn't active. Let the PWM consumers disable it during their own >> suspend sequence, see [1]. So, perform a check here, and handle the >> pinctrl states. Also restore the break inputs upon resume, as registers >> content may be lost when going to low power mode. >> >> [1] https://lkml.org/lkml/2019/2/5/770 >> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> >> --- >> Changes in v2: >> Follow Uwe suggestions/remarks: >> - Add a precursor patch to ease reviewing >> - Use registers read instead of pwm_get_state >> - Add a comment to mention registers content may be lost in low power mode >> --- >> drivers/pwm/pwm-stm32.c | 38 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) > > Applied, thanks. I made two minor changes, though, see below. > >> >> diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c >> index cf8658c..546b661 100644 >> --- a/drivers/pwm/pwm-stm32.c >> +++ b/drivers/pwm/pwm-stm32.c >> @@ -12,6 +12,7 @@ >> #include <linux/mfd/stm32-timers.h> >> #include <linux/module.h> >> #include <linux/of.h> >> +#include <linux/pinctrl/consumer.h> >> #include <linux/platform_device.h> >> #include <linux/pwm.h> >> >> @@ -655,6 +656,42 @@ static int stm32_pwm_remove(struct platform_device *pdev) >> return 0; >> } >> >> +static int __maybe_unused stm32_pwm_suspend(struct device *dev) >> +{ >> + struct stm32_pwm *priv = dev_get_drvdata(dev); >> + unsigned int ch; > > I renamed this to just "i", which is more idiomatic for loop variables. > The function is small enough not to need to differentiate between loop > variables. > >> + u32 ccer, mask; >> + >> + /* Look for active channels */ >> + ccer = active_channels(priv); >> + >> + for (ch = 0; ch < priv->chip.npwm; ch++) { >> + mask = TIM_CCER_CC1E << (ch * 4); >> + if (ccer & mask) { >> + dev_err(dev, "The consumer didn't stop us (%s)\n", >> + priv->chip.pwms[ch].label); > > Changed this to: > > "PWM %u still in use by consumer %s\n", i, priv->chip.pwms[i].label > > I think that might help clarify which PWM is still enabled in case the > consumers don't set a label. Hi Thierry, Many thanks for all the improvements on this series! Best Regards, Fabrice > > Thierry > >> + return -EBUSY; >> + } >> + } >> + >> + return pinctrl_pm_select_sleep_state(dev); >> +} >> + >> +static int __maybe_unused stm32_pwm_resume(struct device *dev) >> +{ >> + struct stm32_pwm *priv = dev_get_drvdata(dev); >> + int ret; >> + >> + ret = pinctrl_pm_select_default_state(dev); >> + if (ret) >> + return ret; >> + >> + /* restore breakinput registers that may have been lost in low power */ >> + return stm32_pwm_apply_breakinputs(priv); >> +} >> + >> +static SIMPLE_DEV_PM_OPS(stm32_pwm_pm_ops, stm32_pwm_suspend, stm32_pwm_resume); >> + >> static const struct of_device_id stm32_pwm_of_match[] = { >> { .compatible = "st,stm32-pwm", }, >> { /* end node */ }, >> @@ -667,6 +704,7 @@ static struct platform_driver stm32_pwm_driver = { >> .driver = { >> .name = "stm32-pwm", >> .of_match_table = stm32_pwm_of_match, >> + .pm = &stm32_pwm_pm_ops, >> }, >> }; >> module_platform_driver(stm32_pwm_driver); >> -- >> 2.7.4 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-10-16 10:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-04 12:53 [PATCH v2 0/3] Add PM support to STM32 Timer PWM Fabrice Gasnier 2019-10-04 12:53 ` [PATCH v2 1/3] dt-bindings: pwm-stm32: document pinctrl sleep state Fabrice Gasnier 2019-10-11 15:10 ` Rob Herring 2019-10-16 6:59 ` Thierry Reding 2019-10-04 12:53 ` [PATCH v2 2/3] pwm: stm32: split breakinput apply routine to ease PM support Fabrice Gasnier 2019-10-16 7:03 ` Thierry Reding 2019-10-04 12:53 ` [PATCH v2 3/3] pwm: stm32: add power management support Fabrice Gasnier 2019-10-16 7:06 ` Thierry Reding 2019-10-16 10:08 ` Fabrice Gasnier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).