Re: [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs

[Samuel Holland]

On 9/29/21 11:19 PM, Chanwoo Choi wrote:
> Hi Samuel,
> 
> 
> On 9/29/21 1:42 PM, Samuel Holland wrote:
>> Since commit ea572f816032 ("PM / devfreq: Change return type of
>> devfreq_set_freq_table()"), all devfreq devices are required to have a
>> valid freq_table. If freq_table is not provided by the driver, it will
>> be filled in by set_freq_table() from the OPPs; if that fails,
>> devfreq_add_device() will return an error.
>>
>> However, since commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when
>> adding the devfreq device"), devfreq devices are _also_ required to have
>> an OPP table, even if they provide freq_table. devfreq_add_device()
>> requires dev_pm_opp_find_freq_ceil() and dev_pm_opp_find_freq_floor() to
>> return successfully, specifically to initialize scaling_min/max_freq.
>>
>> Not all drivers need an OPP table. For example, a driver where all
>> frequencies are determined dynamically could work by filling out only
>> freq_table. But with the current code it must call dev_pm_opp_add() on
>> every freq_table entry to probe successfully.
> 
> As you commented, if device has no opp table, it should call dev_pm_opp_add().
> The devfreq have to use OPP for controlling the frequency/regulator.
> 
> Actually, I want that all devfreq driver uses the OPP as default way.

The current code/documentation implies that an OPP table is intended to
be optional. For example:

 * struct devfreq - Device devfreq structure
...
 * @opp_table:  Reference to OPP table of dev.parent, if one exists.

So this should be updated if an OPP table is no longer optional.

> Are there any reason why don't use the OPP table?

dev_pm_opp_add() takes a voltage, and assumes the existence of some
voltage regulator, but there is none involved here. The only way to have
an OPP table without regulators is to use a static table in the
devicetree. But that also doesn't make much sense, because the OPPs
aren't actually customizable; they are integer dividers from a fixed
base clock. And adding a fixed OPP table to each board would be a lot of
work to replace a trivial loop in the driver. So it seems to be the
wrong abstraction.

Using an OPP table adds extra complexity (memory allocations, error
cases), just to duplicate the list of frequencies that already has to
exist in freq_table. And the driver works fine without any of that.

Regards,
Samuel