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

[Viresh Kumar]

On 25-03-26, 07:12, Uwe Kleine-König wrote:
> On Wed, Mar 25, 2026 at 09:34:55AM +0900, Mikko Perttunen wrote:
> > On Wednesday, March 25, 2026 1:45 AM Uwe Kleine-König wrote:
> > > On Mon, Mar 23, 2026 at 11:36:37AM +0900, Mikko Perttunen wrote:
> > > > @@ -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.

And that is correct, right ? This comment is talking about the max freq possible
with each OPP and not the highest freq possible with the device. If a device
supports 5 OPPs (0-4) and if we want to run at the freq mentioned in the OPP3
entry in DT, then the caller must send a frequency such that clk_round_rate()
returns the frequency of the OPP3.

In the above case though, we will end up running at the highest freq returned
by clk_round_rate() and an OPP corresponding to that.

> > > 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?

I think S64_MAX will work as well, unless clk_round_rate() returns a frequency
higher than what the OPP table mentions. It may still work, but the values may
be confusing and inconsistent.

> > Looking at the history of the function, the comment was added in the commit 
> > below. It seems like it used to be that the opp framework always used the fmax 
> > of each OPP even if a lower rate was specified, but after the change, the 
> > caller has to specify the fmax rate if they want that rate specifically. As 
> > such I don't think it should be an issue in our case, as we're just using the 
> > rate to find an OPP and don't have a specific one in mind.

Right.

> So the comment describing dev_pm_opp_set_rate() needs an update, right?

Maybe, not sure. But as I mentioned earlier, it is written with the context of
each OPP's highest freq.

-- 
viresh