[Powertop] Suspecting incorrect if check during powertop init

[Powertop] Suspecting incorrect if check during powertop init

[Rajagopal Venkat]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

Hi,

Recently a patch[1] is applied to remove boardname support from powertop.
This commit adds few lines of code to powertop_init(), to initialize two
global variables. I suspect, if condition is not correct and is suppose
to be as follows. Is that intentional?

        if (access("/var/cache/powertop/saved_parameters.powertop", R_OK)
== 0 ||
            access("/data/local/powertop/saved_parameters.powertop", R_OK)
== 0) {
                global_fixed_parameters = 1;
                global_power_override = 1;
        }


[1] commit 1c7b4b999cb172b59833f829ab7b95fab11d7a00<https://github.com/fenrus75/powertop/commit/1c7b4b999cb172b59833f829ab7b95fab11d7a00>
-- 
Regards,
Rajagopal

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 937 bytes --]

Re: [Powertop] Suspecting incorrect if check during powertop init

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

On 8/7/2012 3:54 AM, Rajagopal Venkat wrote:
> Hi,
> 
> Recently a patch[1] is applied to remove boardname support from powertop.
> This commit adds few lines of code to powertop_init(), to initialize two
> global variables. I suspect, if condition is not correct and is suppose
> to be as follows. Is that intentional?
>  
>         if (access("/var/cache/powertop/saved_parameters.powertop", R_OK) == 0 ||
>             access("/data/local/powertop/saved_parameters.powertop", R_OK) == 0) {
>                 global_fixed_parameters = 1;
>                 global_power_override = 1;
>         }
>  

both are wrong
the whole snippet that assigns these two globals needs to go.

the logic used to be that you could do a boardname override, which would lock system parameters in place
and stop the learning logic
(say, if provided by a manufacturer)

with the boardname stuff gone, those parameters shouldn't be set like this

Re: [Powertop] Suspecting incorrect if check during powertop init

[Chris Ferron]

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

On 08/07/2012 06:28 AM, Arjan van de Ven wrote:
> On 8/7/2012 3:54 AM, Rajagopal Venkat wrote:
>> Hi,
>>
>> Recently a patch[1] is applied to remove boardname support from powertop.
>> This commit adds few lines of code to powertop_init(), to initialize two
>> global variables. I suspect, if condition is not correct and is suppose
>> to be as follows. Is that intentional?
>>   
>>          if (access("/var/cache/powertop/saved_parameters.powertop", R_OK) == 0 ||
>>              access("/data/local/powertop/saved_parameters.powertop", R_OK) == 0) {
>>                  global_fixed_parameters = 1;
>>                  global_power_override = 1;
>>          }
>>   
> both are wrong
> the whole snippet that assigns these two globals needs to go.
>
> the logic used to be that you could do a boardname override, which would lock system parameters in place
> and stop the learning logic
> (say, if provided by a manufacturer)
>
> with the boardname stuff gone, those parameters shouldn't be set like this
>
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
Thanks for the catch. I have pushed the fix.
-C