Hi Dietmar,

Thank you for having a look at this.

On 10/26/21 5:51 PM, Dietmar Eggemann wrote:

On 15/10/2021 16:45, Lukasz Luba wrote:

[snip]

quoted

+#define arch_thermal_pressure_update	topology_thermal_pressure_update

s/thermal_pressure_update/update_thermal_pressure ?

I can reorder that naming.

The scheme seems to be {arch|topology}_*foo*_thermal_pressure

But ...

quoted

[snip]

quoted

+void topology_thermal_pressure_update(const struct cpumask *cpus,
+				      unsigned long capped_freq)
+{

... why not just s/unsigned long th_pressure/unsigned long capped_freq
in existing topology_set_thermal_pressure() and move code the
frequency/capacity conversion in there? The patch set will become
considerably smaller.

I've been trying to avoid confusion when changing actually behavior
of the API function. Thus, introducing new would IMO opinion
make sure the old 'set' function was expecting proper pressure
value, while the new 'update' expects frequency.

I agree that the patch set would be smaller in that case, but I'm
not sure if that would not hide some issues. This one would
definitely break compilation of some vendor modules (or drivers
queuing or under review), not silently passing them through (with wrong
argument).

  void topology_set_thermal_pressure(const struct cpumask *cpus,
-                              unsigned long th_pressure)
+                              unsigned long capped_freq)

[snip]

  EXPORT_SYMBOL_GPL(topology_set_thermal_pressure);

And a user like [drivers/thermal/cpufreq_cooling.c] can call
arch_set_thermal_pressure(cpus, frequency).

[...]

I'm not sure if that is a safe way.

Regards,
Lukasz