On 16-05-20, 15:52, Serge Semin wrote:
On Fri, May 15, 2020 at 05:58:47PM +0200, Rafael J. Wysocki wrote:
@@ -2554,7 +2554,7 @@ static int cpufreq_boost_set_sw(int state) break; }
- return ret;
- return ret < 0 ? ret : 0; } int cpufreq_boost_trigger_state(int state)
IMO it is better to update the caller of this function to handle the positive value possibly returned by it correctly.
Could you elaborate why? Viresh seems to be ok with this solution.
And it is absolutely fine for Rafael to not agree with it :)
As I see it the caller doesn't expect the positive value returned by the original freq_qos_update_request(). It just doesn't need to know whether the effective policy has been updated or not, it only needs to make sure the operations has been successful. Moreover the positive value is related only to the !last! active policy, which doesn't give the caller a full picture of the policy change anyway. So taking all of these into account I'd leave the fix as is.
Rafael: This function is called via a function pointer, which can call this or a platform dependent routine (like in acpi-cpufreq.c), and it would be reasonable IMO for the return of that callback to only look for 0 or negative values, as is generally done in the kernel. And so this solution looked okay to me as the positive return is coming from the implementation detail here.