Re: [PATCH v5 3/7] pwm: tegra: Avoid hard-coded max clock frequency

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Potential division by zero during probe due to unvalidated dynamically fetched clock rate.
- [High] Logic error in `dev_pm_opp_set_rate()` usage causes probe failure on devices with OPP tables.
--

commit 74520499a078af31d704c76e29fb2e41b43c4ecc
Author: Yi-Wei Wang <yiweiw@nvidia.com>

pwm: tegra: Avoid hard-coded max clock frequency

This commit changes the Tegra PWM driver to dynamically fetch the max
clock rate from the current parent clock using dev_pm_opp_set_rate()
and removes the hard-coded maximum frequencies from the SoC data.

> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index f8c5495a56a66..303e2720034d2 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
[ ... ]
> @@ -303,7 +300,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	/* Set maximum frequency of the IP */
> -	ret = dev_pm_opp_set_rate(&pdev->dev, pc->soc->max_frequency);
> +	ret = dev_pm_opp_set_rate(&pdev->dev, ULONG_MAX);

[Severity: High]
Does setting the target frequency to ULONG_MAX work safely with OPP tables?

dev_pm_opp_set_rate() will round ULONG_MAX to the clock driver's absolute
maximum frequency. If a platform (such as Tegra30) has an OPP table where
the highest defined OPP is lower than this absolute maximum,
dev_pm_opp_set_rate() will fail to find a matching OPP and return -ERANGE.

Could this failure inadvertently abort the driver probe on platforms with
such OPP tables?

>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "Failed to set max frequency: %d\n", ret);
>  		goto put_pm;
> @@ -318,7 +315,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
>  
>  	/* Set minimum limit of PWM period for the IP */
>  	pc->min_period_ns =
> -	    (NSEC_PER_SEC / (pc->soc->max_frequency >> TEGRA_PWM_DUTY_WIDTH)) + 1;
> +	    (NSEC_PER_SEC / (pc->clk_rate >> TEGRA_PWM_DUTY_WIDTH)) + 1;

[Severity: High]
Is it possible for pc->clk_rate to be less than 256 here?

If clk_get_rate() returns 0 (such as when the clock is unconfigured) or a
value less than 1 << TEGRA_PWM_DUTY_WIDTH, the right shift will evaluate to
0.

Could this lead to a division by zero panic during device probe?

>  
>  	pc->rst = devm_reset_control_get_exclusive(&pdev->dev, "pwm");
>  	if (IS_ERR(pc->rst)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529-t264-pwm-v5-0-7bf9e405a96a@nvidia.com?part=3