Re: [PATCH 1/5] pwm: tegra: Avoid hard-coded max clock frequency

[Uwe Kleine-König]

On Mon, Mar 23, 2026 at 11:36:37AM +0900, Mikko Perttunen wrote:
> From: Yi-Wei Wang <yiweiw@xxxxxxxxxx>
> 
> The clock driving the Tegra PWM IP can be sourced from different parent
> clocks. Hence, let dev_pm_opp_set_rate() set the max clock rate based
> upon the current parent clock that can be specified via device-tree.
> 
> After this, the Tegra194 SoC data becomes redundant, so get rid of it.
> 
> Signed-off-by: Yi-Wei Wang <yiweiw@xxxxxxxxxx>
> Co-developed-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
> Signed-off-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
> ---
>  drivers/pwm/pwm-tegra.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 172063b51d44..759b98b97b6e 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -59,9 +59,6 @@
>  
>  struct tegra_pwm_soc {
>  	unsigned int num_channels;
> -
> -	/* Maximum IP frequency for given SoCs */
> -	unsigned long max_frequency;
>  };
>  
>  struct tegra_pwm_chip {
> @@ -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, S64_MAX);

The documentation comment for dev_pm_opp_set_rate() reads:

	Device wanting to run at fmax provided by the opp, should have
	already rounded to the target OPP's frequency.

I think using S64_MAX is technically fine (assuming there are no issues
with big numbers in that function), but still it feels wrong to use
something simpler than the comment suggests. Am I missing something?

>  	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 >> PWM_DUTY_WIDTH)) + 1;
> +	    (NSEC_PER_SEC / (pc->clk_rate >> PWM_DUTY_WIDTH)) + 1;

Orthogonal to this patch: Should this be

	DIV_ROUND_UP(NSEC_PER_SEC, pc->clk_rate >> PWM_DUTY_WIDTH)

? Or even

	DIV_ROUND_UP(NSEC_PER_SEC < PWM_DUTY_WIDTH, pc->clk_rate);

? (Note, the latter doesn't work as is, as the first parameter has an
overflow, I guess you're still getting my question.)

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature