On Mon, 10 Apr 2017 17:20:20 +0300
Claudiu Beznea <[email protected]> wrote:

> Implement suspend and resume power management specific
> function to allow PWM controller to correctly suspend
> and resume.
> 
> Signed-off-by: Claudiu Beznea <[email protected]>
> ---
>  drivers/pwm/pwm-atmel.c | 81 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 530d7dc..75177c6 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -58,6 +58,8 @@
>  #define PWM_MAX_PRD          0xFFFF
>  #define PRD_MAX_PRES         10
>  
> +#define PWM_MAX_CH_NUM               (4)
> +
>  struct atmel_pwm_registers {
>       u8 period;
>       u8 period_upd;
> @@ -65,11 +67,18 @@ struct atmel_pwm_registers {
>       u8 duty_upd;
>  };
>  
> +struct atmel_pwm_pm_ctx {
> +     u32 cmr;
> +     u32 cdty;
> +     u32 cprd;
> +};
> +
>  struct atmel_pwm_chip {
>       struct pwm_chip chip;
>       struct clk *clk;
>       void __iomem *base;
>       const struct atmel_pwm_registers *regs;
> +     struct atmel_pwm_pm_ctx ctx[PWM_MAX_CH_NUM];

Hm, I'm pretty sure you can rely on the current PWM state and call
atmel_pwm_apply() at resume time instead of doing that. See what I did
here [1].

Thierry, maybe it's time to start thinking about a generic solution to
save/restore PWM states.

>  
>       unsigned int updated_pwms;
>       /* ISR is cleared when read, ensure only one thread does that */
> @@ -333,6 +342,77 @@ atmel_pwm_get_driver_data(struct platform_device *pdev)
>       return (struct atmel_pwm_registers *)id->driver_data;
>  }
>  
> +#ifdef CONFIG_PM_SLEEP
> +static int atmel_pwm_suspend(struct device *dev)
> +{
> +     struct atmel_pwm_chip *atmel_pwm = dev_get_drvdata(dev);
> +     struct pwm_device *pwm = atmel_pwm->chip.pwms;
> +     int i;
> +     bool disable_clk = false;
> +
> +     for (i = 0; i < atmel_pwm->chip.npwm; i++, pwm++) {
> +             if (!pwm_is_enabled(pwm))
> +                     continue;
> +
> +             disable_clk = true;
> +             atmel_pwm->ctx[i].cdty =
> +                     atmel_pwm_ch_readl(atmel_pwm, i,
> +                                        atmel_pwm->regs->duty);
> +             atmel_pwm->ctx[i].cprd =
> +                     atmel_pwm_ch_readl(atmel_pwm, i,
> +                                        atmel_pwm->regs->period);
> +             atmel_pwm->ctx[i].cmr =
> +                     atmel_pwm_ch_readl(atmel_pwm, i, PWM_CMR);
> +
> +             atmel_pwm_disable(&atmel_pwm->chip, pwm, false);
> +     }
> +
> +     if (disable_clk)
> +             clk_disable(atmel_pwm->clk);

I'm not so sure we want to disable the PWM and the PWM chip clk when
entering suspend. What if the PWM is driving a critical device (like a
regulator) that has to stay enabled in suspend?
Shouldn't we delegate this responsibility to the PWM user?

[1]http://patchwork.ozlabs.org/patch/734306/

Reply via email to