Re: [PATCH v3 7/7] pwm: rockchip: Enable clock before calling clk_get_rate()

* Re: [PATCH v3 7/7] pwm: rockchip: Enable clock before calling clk_get_rate()
@ 2020-12-25  7:15 Kever Yang
  2021-01-13  9:16 ` Uwe Kleine-König
  0 siblings, 1 reply; 4+ messages in thread
From: Kever Yang @ 2020-12-25  7:15 UTC (permalink / raw)
  To: Simon South
  Cc: tpiepho, thierry.reding, u.kleine-koenig, Robin Murphy,
	lee.jones, Heiko Stuebner, bbrezillon, linux-pwm,
	linux-arm-kernel, linux-rockchip, David Wu, steven.liu

[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]

+ David and Steven,

Hi Steven,
    please help to review this patch set.

Thanks
- Kever

Simon South <simon@simonsouth.net> 于2020年12月24日周四 上午12:01写道：

> The documentation for clk_get_rate() in include/linux/clk.h states the
> function's result is valid only for a clock source that has been
> enabled. However, the Rockchip PWM driver uses this function in two places
> to query the rate of a clock without first ensuring it is enabled.
>
> Fix this by modifying rockchip_pwm_get_state() and rockchip_pwm_apply() so
> they enable a device's PWM clock before querying its rate (in the latter
> case, the querying is actually done in rockchip_pwm_config()) and disable
> the clock again before returning.
>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Simon South <simon@simonsouth.net>
> ---
>  drivers/pwm/pwm-rockchip.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index 02da7370db70..44425eeb4e81 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -72,6 +72,10 @@ static void rockchip_pwm_get_state(struct pwm_chip
> *chip,
>         if (ret)
>                 return;
>
> +       ret = clk_enable(pc->clk);
> +       if (ret)
> +               return;
> +
>         clk_rate = clk_get_rate(pc->clk);
>
>         tmp = readl_relaxed(pc->base + pc->data->regs.period);
> @@ -90,6 +94,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
>         else
>                 state->polarity = PWM_POLARITY_NORMAL;
>
> +       clk_disable(pc->clk);
>         clk_disable(pc->pclk);
>  }
>
> @@ -189,6 +194,10 @@ static int rockchip_pwm_apply(struct pwm_chip *chip,
> struct pwm_device *pwm,
>         if (ret)
>                 return ret;
>
> +       ret = clk_enable(pc->clk);
> +       if (ret)
> +               return ret;
> +
>         pwm_get_state(pwm, &curstate);
>         enabled = curstate.enabled;
>
> @@ -208,6 +217,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip,
> struct pwm_device *pwm,
>         }
>
>  out:
> +       clk_disable(pc->clk);
>         clk_disable(pc->pclk);
>
>         return ret;
> --
> 2.29.2
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>

[-- Attachment #2: Type: text/html, Size: 3476 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread