Re: [PATCH 2/2] pwm: tegra: Fix required rate when clock is lower than needed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 26, 2022 at 11:13:05AM +0100, Jon Hunter wrote:
> If the 'required_clk_rate' is greater than the clock rate that can be
> provided, then when mul_u64_u64_div_u64() is called to determine the
> 'rate' for the PWM divider, 0 will be returned. If 'rate' is 0, then we
> will return -EINVAL and fail to configure the PWM. Fix this by adding 1
> to the PWM_DUTY_WIDTH when calculating the 'required_clk_rate' to ensure
> that 'rate' is greater or equal to 1. This fixes an issue on Tegra234
> where configuring the PWM fan fails.
> 
> Fixes: 8c193f4714df ("pwm: tegra: Optimize period calculation")
> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> ---
>  drivers/pwm/pwm-tegra.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 8a33c500f93b..973e2c1533ab 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -148,6 +148,19 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  		required_clk_rate = DIV_ROUND_UP_ULL((NSEC_PER_SEC << PWM_DUTY_WIDTH),
>  						     period_ns);
>  
> +		/*
> +		 * If the 'required_clk_rate' is greater than the clock rate
> +		 * that can be provided, then when mul_u64_u64_div_u64() is
> +		 * called to determine the 'rate' for the PWM divider, 0 will
> +		 * be returned. If 'rate' is 0, then we will return -EINVAL and
> +		 * fail to configure the PWM. If this case, add 1 to the
> +		 * PWM_DUTY_WIDTH when calculating the 'required_clk_rate' to
> +		 * ensure that 'rate' is greater or equal to 1.
> +		 */
> +		if (required_clk_rate > clk_round_rate(pc->clk, required_clk_rate))
> +			required_clk_rate = DIV_ROUND_UP_ULL((NSEC_PER_SEC << (PWM_DUTY_WIDTH + 1)),
> +							     period_ns);
> +

It's implicit knowledge that (roughly) doubling the clk rate is the
right value (i.e the minimal value to get a 
clk_rate >= (NSEC_PER_SEC << PWM_DUTY_WIDTH) / period_ns?

>  		err = dev_pm_opp_set_rate(pc->dev, required_clk_rate);
>  		if (err < 0)
>  			return -EINVAL;

Is it obvious that dev_pm_opp_set_rate(pc->dev, ...) and
clk_round_rate() correlate enough that the latter tells anything about
the former? Would it make sense to use clk_set_rate instead of
dev_pm_opp_set_rate?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux