diff mbox

[v2] cpupower: Fix no-rounding MHz frequency output

Message ID 20171025135132.16324-1-prarit@redhat.com (mailing list archive)
State Accepted
Delegated to: Shuah Khan
Headers show

Commit Message

Prarit Bhargava Oct. 25, 2017, 1:51 p.m. UTC
'cpupower frequency-info -ln' returns kHz values on systems with MHz range
minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
the command returns

hardware limits: 800000 MHz - 4.200000 GHz

The code that causes this error can be removed.  The next else if clause
will handle the output correctly such that

hardware limits: 800.000 MHz - 4.200000 GHz

is displayed correctly.

[v2]: Remove two lines instead of fixing broken code.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Thomas Renninger <trenn@suse.com>
Cc: Stafford Horne <shorne@gmail.com>
Cc: Shuah Khan <shuah@kernel.org>
---
 tools/power/cpupower/utils/cpufreq-info.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Stafford Horne Oct. 25, 2017, 10:01 p.m. UTC | #1
On Wed, Oct 25, 2017 at 09:51:32AM -0400, Prarit Bhargava wrote:
> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
> the command returns
> 
> hardware limits: 800000 MHz - 4.200000 GHz
> 
> The code that causes this error can be removed.  The next else if clause
> will handle the output correctly such that
> 
> hardware limits: 800.000 MHz - 4.200000 GHz
> 
> is displayed correctly.
> 
> [v2]: Remove two lines instead of fixing broken code.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Thomas Renninger <trenn@suse.com>
> Cc: Stafford Horne <shorne@gmail.com>
> Cc: Shuah Khan <shuah@kernel.org>

Reviewed-by: Stafford Horne <shorne@gmail.com>

> ---
>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
> index 3e701f0e9c14..df43cd45d810 100644
> --- a/tools/power/cpupower/utils/cpufreq-info.c
> +++ b/tools/power/cpupower/utils/cpufreq-info.c
> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>  		if (speed > 1000000)
>  			printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>  				((unsigned int) speed%1000000));
> -		else if (speed > 100000)
> -			printf("%u MHz", (unsigned int) speed);
>  		else if (speed > 1000)
>  			printf("%u.%03u MHz", ((unsigned int) speed/1000),
>  				(unsigned int) (speed%1000));
> -- 
> 2.15.0.rc0.39.g2f0e14e64
>
shuah Nov. 1, 2017, 9 p.m. UTC | #2
On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
> the command returns
> 
> hardware limits: 800000 MHz - 4.200000 GHz
> 
> The code that causes this error can be removed.  The next else if clause
> will handle the output correctly such that
> 
> hardware limits: 800.000 MHz - 4.200000 GHz
> 
> is displayed correctly.
> 
> [v2]: Remove two lines instead of fixing broken code.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Thomas Renninger <trenn@suse.com>
> Cc: Stafford Horne <shorne@gmail.com>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
> index 3e701f0e9c14..df43cd45d810 100644
> --- a/tools/power/cpupower/utils/cpufreq-info.c
> +++ b/tools/power/cpupower/utils/cpufreq-info.c
> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>  		if (speed > 1000000)
>  			printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>  				((unsigned int) speed%1000000));
> -		else if (speed > 100000)
> -			printf("%u MHz", (unsigned int) speed);
>  		else if (speed > 1000)
>  			printf("%u.%03u MHz", ((unsigned int) speed/1000),
>  				(unsigned int) (speed%1000));
> 

Thanks.  I will queue this up for 4.15-rc1.

-- Shuah
shuah Nov. 1, 2017, 9:01 p.m. UTC | #3
On 10/25/2017 04:01 PM, Stafford Horne wrote:
> On Wed, Oct 25, 2017 at 09:51:32AM -0400, Prarit Bhargava wrote:
>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>> the command returns
>>
>> hardware limits: 800000 MHz - 4.200000 GHz
>>
>> The code that causes this error can be removed.  The next else if clause
>> will handle the output correctly such that
>>
>> hardware limits: 800.000 MHz - 4.200000 GHz
>>
>> is displayed correctly.
>>
>> [v2]: Remove two lines instead of fixing broken code.
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: Thomas Renninger <trenn@suse.com>
>> Cc: Stafford Horne <shorne@gmail.com>
>> Cc: Shuah Khan <shuah@kernel.org>
> 
> Reviewed-by: Stafford Horne <shorne@gmail.com>

Thanks for the review.

-- Shuah

> 
>> ---
>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>> index 3e701f0e9c14..df43cd45d810 100644
>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>  		if (speed > 1000000)
>>  			printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>  				((unsigned int) speed%1000000));
>> -		else if (speed > 100000)
>> -			printf("%u MHz", (unsigned int) speed);
>>  		else if (speed > 1000)
>>  			printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>  				(unsigned int) (speed%1000));
>> -- 
>> 2.15.0.rc0.39.g2f0e14e64
>>
> 
>
Rafael J. Wysocki Nov. 1, 2017, 9:33 p.m. UTC | #4
On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>> the command returns
>>
>> hardware limits: 800000 MHz - 4.200000 GHz
>>
>> The code that causes this error can be removed.  The next else if clause
>> will handle the output correctly such that
>>
>> hardware limits: 800.000 MHz - 4.200000 GHz
>>
>> is displayed correctly.
>>
>> [v2]: Remove two lines instead of fixing broken code.
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: Thomas Renninger <trenn@suse.com>
>> Cc: Stafford Horne <shorne@gmail.com>
>> Cc: Shuah Khan <shuah@kernel.org>
>> ---
>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>> index 3e701f0e9c14..df43cd45d810 100644
>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>               if (speed > 1000000)
>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>                               ((unsigned int) speed%1000000));
>> -             else if (speed > 100000)
>> -                     printf("%u MHz", (unsigned int) speed);
>>               else if (speed > 1000)
>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>                               (unsigned int) (speed%1000));
>>
>
> Thanks.  I will queue this up for 4.15-rc1.

OK

So are you going to maintain this utility going forward?

Thanks,
Rafael
Shuah Khan Nov. 1, 2017, 9:38 p.m. UTC | #5
On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>> the command returns
>>>
>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>
>>> The code that causes this error can be removed.  The next else if clause
>>> will handle the output correctly such that
>>>
>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>
>>> is displayed correctly.
>>>
>>> [v2]: Remove two lines instead of fixing broken code.
>>>
>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>> Cc: Thomas Renninger <trenn@suse.com>
>>> Cc: Stafford Horne <shorne@gmail.com>
>>> Cc: Shuah Khan <shuah@kernel.org>
>>> ---
>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>> index 3e701f0e9c14..df43cd45d810 100644
>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>               if (speed > 1000000)
>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>                               ((unsigned int) speed%1000000));
>>> -             else if (speed > 100000)
>>> -                     printf("%u MHz", (unsigned int) speed);
>>>               else if (speed > 1000)
>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>                               (unsigned int) (speed%1000));
>>>
>>
>> Thanks.  I will queue this up for 4.15-rc1.
> 
> OK
> 
> So are you going to maintain this utility going forward?
> 

oops. I was on auto-pilot responding to patches sitting in my Inbox.
Wrong email response. Sorry about that.

Please ignore. Mu bad.

thanks,
-- Shuah
Rafael J. Wysocki Nov. 1, 2017, 9:46 p.m. UTC | #6
On Wed, Nov 1, 2017 at 10:38 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
>> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>>> the command returns
>>>>
>>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>>
>>>> The code that causes this error can be removed.  The next else if clause
>>>> will handle the output correctly such that
>>>>
>>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>>
>>>> is displayed correctly.
>>>>
>>>> [v2]: Remove two lines instead of fixing broken code.
>>>>
>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>> Cc: Thomas Renninger <trenn@suse.com>
>>>> Cc: Stafford Horne <shorne@gmail.com>
>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>> ---
>>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>>  1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>>> index 3e701f0e9c14..df43cd45d810 100644
>>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>>               if (speed > 1000000)
>>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>>                               ((unsigned int) speed%1000000));
>>>> -             else if (speed > 100000)
>>>> -                     printf("%u MHz", (unsigned int) speed);
>>>>               else if (speed > 1000)
>>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>>                               (unsigned int) (speed%1000));
>>>>
>>>
>>> Thanks.  I will queue this up for 4.15-rc1.
>>
>> OK
>>
>> So are you going to maintain this utility going forward?
>>
>
> oops. I was on auto-pilot responding to patches sitting in my Inbox.
> Wrong email response. Sorry about that.
>
> Please ignore. Mu bad.

OK :-)

But that said, from my perspective, cpupower is basically not maintained.

Thomas, who sort of maintained it, but then basically became a patch
reviewer for it, does not respond to patches any more and I am not
sufficiently familiar with the code to be able to effectively review
the patches myself, nor I have the time to get more familiar with it.

For this reason, I'm inclined to drop this code from the kernel source
tree unless somebody steps in to fill the gap.

Thanks,
Rafael
Shuah Khan Nov. 1, 2017, 9:49 p.m. UTC | #7
On 11/01/2017 03:46 PM, Rafael J. Wysocki wrote:
> On Wed, Nov 1, 2017 at 10:38 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
>>> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>>>> the command returns
>>>>>
>>>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>>>
>>>>> The code that causes this error can be removed.  The next else if clause
>>>>> will handle the output correctly such that
>>>>>
>>>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>>>
>>>>> is displayed correctly.
>>>>>
>>>>> [v2]: Remove two lines instead of fixing broken code.
>>>>>
>>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>>> Cc: Thomas Renninger <trenn@suse.com>
>>>>> Cc: Stafford Horne <shorne@gmail.com>
>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>> ---
>>>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>>>  1 file changed, 2 deletions(-)
>>>>>
>>>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>>>> index 3e701f0e9c14..df43cd45d810 100644
>>>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>>>               if (speed > 1000000)
>>>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>>>                               ((unsigned int) speed%1000000));
>>>>> -             else if (speed > 100000)
>>>>> -                     printf("%u MHz", (unsigned int) speed);
>>>>>               else if (speed > 1000)
>>>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>>>                               (unsigned int) (speed%1000));
>>>>>
>>>>
>>>> Thanks.  I will queue this up for 4.15-rc1.
>>>
>>> OK
>>>
>>> So are you going to maintain this utility going forward?
>>>
>>
>> oops. I was on auto-pilot responding to patches sitting in my Inbox.
>> Wrong email response. Sorry about that.
>>
>> Please ignore. Mu bad.
> 
> OK :-)
> 
> But that said, from my perspective, cpupower is basically not maintained.
> 
> Thomas, who sort of maintained it, but then basically became a patch
> reviewer for it, does not respond to patches any more and I am not
> sufficiently familiar with the code to be able to effectively review
> the patches myself, nor I have the time to get more familiar with it.
> 
> For this reason, I'm inclined to drop this code from the kernel source
> tree unless somebody steps in to fill the gap.
> 

Please don't drop this from kernel sources.. I think this is useful. If you
are looking for a maintainer, I will be happy to step up to maintain it.


thanks,
-- Shuah
Rafael J. Wysocki Nov. 1, 2017, 10:07 p.m. UTC | #8
On Wed, Nov 1, 2017 at 10:49 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 11/01/2017 03:46 PM, Rafael J. Wysocki wrote:
>> On Wed, Nov 1, 2017 at 10:38 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
>>>> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>>>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>>>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>>>>> the command returns
>>>>>>
>>>>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>>>>
>>>>>> The code that causes this error can be removed.  The next else if clause
>>>>>> will handle the output correctly such that
>>>>>>
>>>>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>>>>
>>>>>> is displayed correctly.
>>>>>>
>>>>>> [v2]: Remove two lines instead of fixing broken code.
>>>>>>
>>>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>>>> Cc: Thomas Renninger <trenn@suse.com>
>>>>>> Cc: Stafford Horne <shorne@gmail.com>
>>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>>> ---
>>>>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>>>>  1 file changed, 2 deletions(-)
>>>>>>
>>>>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>> index 3e701f0e9c14..df43cd45d810 100644
>>>>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>>>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>>>>               if (speed > 1000000)
>>>>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>>>>                               ((unsigned int) speed%1000000));
>>>>>> -             else if (speed > 100000)
>>>>>> -                     printf("%u MHz", (unsigned int) speed);
>>>>>>               else if (speed > 1000)
>>>>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>>>>                               (unsigned int) (speed%1000));
>>>>>>
>>>>>
>>>>> Thanks.  I will queue this up for 4.15-rc1.
>>>>
>>>> OK
>>>>
>>>> So are you going to maintain this utility going forward?
>>>>
>>>
>>> oops. I was on auto-pilot responding to patches sitting in my Inbox.
>>> Wrong email response. Sorry about that.
>>>
>>> Please ignore. Mu bad.
>>
>> OK :-)
>>
>> But that said, from my perspective, cpupower is basically not maintained.
>>
>> Thomas, who sort of maintained it, but then basically became a patch
>> reviewer for it, does not respond to patches any more and I am not
>> sufficiently familiar with the code to be able to effectively review
>> the patches myself, nor I have the time to get more familiar with it.
>>
>> For this reason, I'm inclined to drop this code from the kernel source
>> tree unless somebody steps in to fill the gap.
>>
>
> Please don't drop this from kernel sources.. I think this is useful. If you
> are looking for a maintainer, I will be happy to step up to maintain it.

Cool, please do that then. :-)

I will be happy to take pull requests with cpupower changes so that
they go in along with the other PM material.

Thanks,
Rafael
Shuah Khan Nov. 1, 2017, 10:30 p.m. UTC | #9
On 11/01/2017 04:07 PM, Rafael J. Wysocki wrote:
> On Wed, Nov 1, 2017 at 10:49 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> On 11/01/2017 03:46 PM, Rafael J. Wysocki wrote:
>>> On Wed, Nov 1, 2017 at 10:38 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>>> On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
>>>>> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>>>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>>>>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>>>>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>>>>>> the command returns
>>>>>>>
>>>>>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>>>>>
>>>>>>> The code that causes this error can be removed.  The next else if clause
>>>>>>> will handle the output correctly such that
>>>>>>>
>>>>>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>>>>>
>>>>>>> is displayed correctly.
>>>>>>>
>>>>>>> [v2]: Remove two lines instead of fixing broken code.
>>>>>>>
>>>>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>>>>> Cc: Thomas Renninger <trenn@suse.com>
>>>>>>> Cc: Stafford Horne <shorne@gmail.com>
>>>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>>>> ---
>>>>>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>>>>>  1 file changed, 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>> index 3e701f0e9c14..df43cd45d810 100644
>>>>>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>>>>>               if (speed > 1000000)
>>>>>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>>>>>                               ((unsigned int) speed%1000000));
>>>>>>> -             else if (speed > 100000)
>>>>>>> -                     printf("%u MHz", (unsigned int) speed);
>>>>>>>               else if (speed > 1000)
>>>>>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>>>>>                               (unsigned int) (speed%1000));
>>>>>>>
>>>>>>
>>>>>> Thanks.  I will queue this up for 4.15-rc1.
>>>>>
>>>>> OK
>>>>>
>>>>> So are you going to maintain this utility going forward?
>>>>>
>>>>
>>>> oops. I was on auto-pilot responding to patches sitting in my Inbox.
>>>> Wrong email response. Sorry about that.
>>>>
>>>> Please ignore. Mu bad.
>>>
>>> OK :-)
>>>
>>> But that said, from my perspective, cpupower is basically not maintained.
>>>
>>> Thomas, who sort of maintained it, but then basically became a patch
>>> reviewer for it, does not respond to patches any more and I am not
>>> sufficiently familiar with the code to be able to effectively review
>>> the patches myself, nor I have the time to get more familiar with it.
>>>
>>> For this reason, I'm inclined to drop this code from the kernel source
>>> tree unless somebody steps in to fill the gap.
>>>
>>
>> Please don't drop this from kernel sources.. I think this is useful. If you
>> are looking for a maintainer, I will be happy to step up to maintain it.
> 
> Cool, please do that then. :-)
> 
> I will be happy to take pull requests with cpupower changes so that
> they go in along with the other PM material.
> 

Sounds like a plan. I can do that. I will work on getting git setup and
send pull requests. We can get that going for 4.15 unless you think it is
late for you to get pull requests.

thanks,
-- Shuah
Rafael J. Wysocki Nov. 1, 2017, 10:34 p.m. UTC | #10
On Wed, Nov 1, 2017 at 11:30 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 11/01/2017 04:07 PM, Rafael J. Wysocki wrote:
>> On Wed, Nov 1, 2017 at 10:49 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> On 11/01/2017 03:46 PM, Rafael J. Wysocki wrote:
>>>> On Wed, Nov 1, 2017 at 10:38 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>>>> On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
>>>>>> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>>>>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>>>>>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>>>>>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>>>>>>> the command returns
>>>>>>>>
>>>>>>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>>>>>>
>>>>>>>> The code that causes this error can be removed.  The next else if clause
>>>>>>>> will handle the output correctly such that
>>>>>>>>
>>>>>>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>>>>>>
>>>>>>>> is displayed correctly.
>>>>>>>>
>>>>>>>> [v2]: Remove two lines instead of fixing broken code.
>>>>>>>>
>>>>>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>>>>>> Cc: Thomas Renninger <trenn@suse.com>
>>>>>>>> Cc: Stafford Horne <shorne@gmail.com>
>>>>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>>>>> ---
>>>>>>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>>>>>>  1 file changed, 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>> index 3e701f0e9c14..df43cd45d810 100644
>>>>>>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>>>>>>               if (speed > 1000000)
>>>>>>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>>>>>>                               ((unsigned int) speed%1000000));
>>>>>>>> -             else if (speed > 100000)
>>>>>>>> -                     printf("%u MHz", (unsigned int) speed);
>>>>>>>>               else if (speed > 1000)
>>>>>>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>>>>>>                               (unsigned int) (speed%1000));
>>>>>>>>
>>>>>>>
>>>>>>> Thanks.  I will queue this up for 4.15-rc1.
>>>>>>
>>>>>> OK
>>>>>>
>>>>>> So are you going to maintain this utility going forward?
>>>>>>
>>>>>
>>>>> oops. I was on auto-pilot responding to patches sitting in my Inbox.
>>>>> Wrong email response. Sorry about that.
>>>>>
>>>>> Please ignore. Mu bad.
>>>>
>>>> OK :-)
>>>>
>>>> But that said, from my perspective, cpupower is basically not maintained.
>>>>
>>>> Thomas, who sort of maintained it, but then basically became a patch
>>>> reviewer for it, does not respond to patches any more and I am not
>>>> sufficiently familiar with the code to be able to effectively review
>>>> the patches myself, nor I have the time to get more familiar with it.
>>>>
>>>> For this reason, I'm inclined to drop this code from the kernel source
>>>> tree unless somebody steps in to fill the gap.
>>>>
>>>
>>> Please don't drop this from kernel sources.. I think this is useful. If you
>>> are looking for a maintainer, I will be happy to step up to maintain it.
>>
>> Cool, please do that then. :-)
>>
>> I will be happy to take pull requests with cpupower changes so that
>> they go in along with the other PM material.
>>
>
> Sounds like a plan. I can do that. I will work on getting git setup and
> send pull requests. We can get that going for 4.15 unless you think it is
> late for you to get pull requests.

That should be fine.  There still is a couple of days at least before
the merge window. :-)
Prarit Bhargava Nov. 2, 2017, 12:42 a.m. UTC | #11
On 11/01/2017 06:30 PM, Shuah Khan wrote:
> On 11/01/2017 04:07 PM, Rafael J. Wysocki wrote:
>> On Wed, Nov 1, 2017 at 10:49 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> On 11/01/2017 03:46 PM, Rafael J. Wysocki wrote:
>>>> On Wed, Nov 1, 2017 at 10:38 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>>>> On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
>>>>>> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>>>>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>>>>>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>>>>>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>>>>>>> the command returns
>>>>>>>>
>>>>>>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>>>>>>
>>>>>>>> The code that causes this error can be removed.  The next else if clause
>>>>>>>> will handle the output correctly such that
>>>>>>>>
>>>>>>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>>>>>>
>>>>>>>> is displayed correctly.
>>>>>>>>
>>>>>>>> [v2]: Remove two lines instead of fixing broken code.
>>>>>>>>
>>>>>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>>>>>> Cc: Thomas Renninger <trenn@suse.com>
>>>>>>>> Cc: Stafford Horne <shorne@gmail.com>
>>>>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>>>>> ---
>>>>>>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>>>>>>  1 file changed, 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>> index 3e701f0e9c14..df43cd45d810 100644
>>>>>>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>>>>>>               if (speed > 1000000)
>>>>>>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>>>>>>                               ((unsigned int) speed%1000000));
>>>>>>>> -             else if (speed > 100000)
>>>>>>>> -                     printf("%u MHz", (unsigned int) speed);
>>>>>>>>               else if (speed > 1000)
>>>>>>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>>>>>>                               (unsigned int) (speed%1000));
>>>>>>>>
>>>>>>>
>>>>>>> Thanks.  I will queue this up for 4.15-rc1.
>>>>>>
>>>>>> OK
>>>>>>
>>>>>> So are you going to maintain this utility going forward?
>>>>>>
>>>>>
>>>>> oops. I was on auto-pilot responding to patches sitting in my Inbox.
>>>>> Wrong email response. Sorry about that.
>>>>>
>>>>> Please ignore. Mu bad.
>>>>
>>>> OK :-)
>>>>
>>>> But that said, from my perspective, cpupower is basically not maintained.
>>>>
>>>> Thomas, who sort of maintained it, but then basically became a patch
>>>> reviewer for it, does not respond to patches any more and I am not
>>>> sufficiently familiar with the code to be able to effectively review
>>>> the patches myself, nor I have the time to get more familiar with it.
>>>>
>>>> For this reason, I'm inclined to drop this code from the kernel source
>>>> tree unless somebody steps in to fill the gap.
>>>>
>>>
>>> Please don't drop this from kernel sources.. I think this is useful. If you
>>> are looking for a maintainer, I will be happy to step up to maintain it.
>>
>> Cool, please do that then. :-)
>>
>> I will be happy to take pull requests with cpupower changes so that
>> they go in along with the other PM material.
>>
> 
> Sounds like a plan. I can do that. I will work on getting git setup and
> send pull requests. We can get that going for 4.15 unless you think it is
> late for you to get pull requests.

Shuah, I have two other cleanup patches that should be applied to cpupower.  I
will post them shortly.

P.

> 
> thanks,
> -- Shuah
>
Shuah Khan Nov. 2, 2017, 6:59 p.m. UTC | #12
On 11/01/2017 06:42 PM, Prarit Bhargava wrote:
> 
> 
> On 11/01/2017 06:30 PM, Shuah Khan wrote:
>> On 11/01/2017 04:07 PM, Rafael J. Wysocki wrote:
>>> On Wed, Nov 1, 2017 at 10:49 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>>> On 11/01/2017 03:46 PM, Rafael J. Wysocki wrote:
>>>>> On Wed, Nov 1, 2017 at 10:38 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>>>>> On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
>>>>>>> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>>>>>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>>>>>>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>>>>>>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>>>>>>>> the command returns
>>>>>>>>>
>>>>>>>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>>>>>>>
>>>>>>>>> The code that causes this error can be removed.  The next else if clause
>>>>>>>>> will handle the output correctly such that
>>>>>>>>>
>>>>>>>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>>>>>>>
>>>>>>>>> is displayed correctly.
>>>>>>>>>
>>>>>>>>> [v2]: Remove two lines instead of fixing broken code.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>>>>>>> Cc: Thomas Renninger <trenn@suse.com>
>>>>>>>>> Cc: Stafford Horne <shorne@gmail.com>
>>>>>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>>>>>> ---
>>>>>>>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>>>>>>>  1 file changed, 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>>> index 3e701f0e9c14..df43cd45d810 100644
>>>>>>>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>>>>>>>               if (speed > 1000000)
>>>>>>>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>>>>>>>                               ((unsigned int) speed%1000000));
>>>>>>>>> -             else if (speed > 100000)
>>>>>>>>> -                     printf("%u MHz", (unsigned int) speed);
>>>>>>>>>               else if (speed > 1000)
>>>>>>>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>>>>>>>                               (unsigned int) (speed%1000));
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks.  I will queue this up for 4.15-rc1.
>>>>>>>
>>>>>>> OK
>>>>>>>
>>>>>>> So are you going to maintain this utility going forward?
>>>>>>>
>>>>>>
>>>>>> oops. I was on auto-pilot responding to patches sitting in my Inbox.
>>>>>> Wrong email response. Sorry about that.
>>>>>>
>>>>>> Please ignore. Mu bad.
>>>>>
>>>>> OK :-)
>>>>>
>>>>> But that said, from my perspective, cpupower is basically not maintained.
>>>>>
>>>>> Thomas, who sort of maintained it, but then basically became a patch
>>>>> reviewer for it, does not respond to patches any more and I am not
>>>>> sufficiently familiar with the code to be able to effectively review
>>>>> the patches myself, nor I have the time to get more familiar with it.
>>>>>
>>>>> For this reason, I'm inclined to drop this code from the kernel source
>>>>> tree unless somebody steps in to fill the gap.
>>>>>
>>>>
>>>> Please don't drop this from kernel sources.. I think this is useful. If you
>>>> are looking for a maintainer, I will be happy to step up to maintain it.
>>>
>>> Cool, please do that then. :-)
>>>
>>> I will be happy to take pull requests with cpupower changes so that
>>> they go in along with the other PM material.
>>>
>>
>> Sounds like a plan. I can do that. I will work on getting git setup and
>> send pull requests. We can get that going for 4.15 unless you think it is
>> late for you to get pull requests.
> 
> Shuah, I have two other cleanup patches that should be applied to cpupower.  I
> will post them shortly.
> 

Hi Prarit,

Does this tool build for you? I am seeing:

utils/helpers/amd.c:7:21: fatal error: pci/pci.h: No such file or directory
 #include <pci/pci.h>
                     ^
compilation terminated.
Makefile:221: recipe for target 'utils/helpers/amd.o' failed
make: *** [utils/helpers/amd.o] Error 1

thanks,
-- Shuah
Prarit Bhargava Nov. 2, 2017, 7:43 p.m. UTC | #13
On 11/02/2017 02:59 PM, Shuah Khan wrote:
> On 11/01/2017 06:42 PM, Prarit Bhargava wrote:
>>
>>
>> On 11/01/2017 06:30 PM, Shuah Khan wrote:
>>> On 11/01/2017 04:07 PM, Rafael J. Wysocki wrote:
>>>> On Wed, Nov 1, 2017 at 10:49 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>>>> On 11/01/2017 03:46 PM, Rafael J. Wysocki wrote:
>>>>>> On Wed, Nov 1, 2017 at 10:38 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>>>>>> On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
>>>>>>>> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>>>>>>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>>>>>>>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>>>>>>>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>>>>>>>>> the command returns
>>>>>>>>>>
>>>>>>>>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>>>>>>>>
>>>>>>>>>> The code that causes this error can be removed.  The next else if clause
>>>>>>>>>> will handle the output correctly such that
>>>>>>>>>>
>>>>>>>>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>>>>>>>>
>>>>>>>>>> is displayed correctly.
>>>>>>>>>>
>>>>>>>>>> [v2]: Remove two lines instead of fixing broken code.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>>>>>>>> Cc: Thomas Renninger <trenn@suse.com>
>>>>>>>>>> Cc: Stafford Horne <shorne@gmail.com>
>>>>>>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>>>>>>> ---
>>>>>>>>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>>>>>>>>  1 file changed, 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>>>> index 3e701f0e9c14..df43cd45d810 100644
>>>>>>>>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>>>>>>>>               if (speed > 1000000)
>>>>>>>>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>>>>>>>>                               ((unsigned int) speed%1000000));
>>>>>>>>>> -             else if (speed > 100000)
>>>>>>>>>> -                     printf("%u MHz", (unsigned int) speed);
>>>>>>>>>>               else if (speed > 1000)
>>>>>>>>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>>>>>>>>                               (unsigned int) (speed%1000));
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks.  I will queue this up for 4.15-rc1.
>>>>>>>>
>>>>>>>> OK
>>>>>>>>
>>>>>>>> So are you going to maintain this utility going forward?
>>>>>>>>
>>>>>>>
>>>>>>> oops. I was on auto-pilot responding to patches sitting in my Inbox.
>>>>>>> Wrong email response. Sorry about that.
>>>>>>>
>>>>>>> Please ignore. Mu bad.
>>>>>>
>>>>>> OK :-)
>>>>>>
>>>>>> But that said, from my perspective, cpupower is basically not maintained.
>>>>>>
>>>>>> Thomas, who sort of maintained it, but then basically became a patch
>>>>>> reviewer for it, does not respond to patches any more and I am not
>>>>>> sufficiently familiar with the code to be able to effectively review
>>>>>> the patches myself, nor I have the time to get more familiar with it.
>>>>>>
>>>>>> For this reason, I'm inclined to drop this code from the kernel source
>>>>>> tree unless somebody steps in to fill the gap.
>>>>>>
>>>>>
>>>>> Please don't drop this from kernel sources.. I think this is useful. If you
>>>>> are looking for a maintainer, I will be happy to step up to maintain it.
>>>>
>>>> Cool, please do that then. :-)
>>>>
>>>> I will be happy to take pull requests with cpupower changes so that
>>>> they go in along with the other PM material.
>>>>
>>>
>>> Sounds like a plan. I can do that. I will work on getting git setup and
>>> send pull requests. We can get that going for 4.15 unless you think it is
>>> late for you to get pull requests.
>>
>> Shuah, I have two other cleanup patches that should be applied to cpupower.  I
>> will post them shortly.
>>
> 
> Hi Prarit,
> 
> Does this tool build for you? I am seeing:
> 
> utils/helpers/amd.c:7:21: fatal error: pci/pci.h: No such file or directory
>  #include <pci/pci.h>
>                      ^
> compilation terminated.
> Makefile:221: recipe for target 'utils/helpers/amd.o' failed
> make: *** [utils/helpers/amd.o] Error 1
> 

It does not build on latest due to some other unrelated change.  I'm looking at
it now.

P.

> thanks,
> -- Shuah
>
Prarit Bhargava Nov. 2, 2017, 8:05 p.m. UTC | #14
On 11/02/2017 02:59 PM, Shuah Khan wrote:
> On 11/01/2017 06:42 PM, Prarit Bhargava wrote:
>>
>>
>> On 11/01/2017 06:30 PM, Shuah Khan wrote:
>>> On 11/01/2017 04:07 PM, Rafael J. Wysocki wrote:
>>>> On Wed, Nov 1, 2017 at 10:49 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>>>> On 11/01/2017 03:46 PM, Rafael J. Wysocki wrote:
>>>>>> On Wed, Nov 1, 2017 at 10:38 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>>>>>> On 11/01/2017 03:33 PM, Rafael J. Wysocki wrote:
>>>>>>>> On Wed, Nov 1, 2017 at 10:00 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>>>>>>> On 10/25/2017 07:51 AM, Prarit Bhargava wrote:
>>>>>>>>>> 'cpupower frequency-info -ln' returns kHz values on systems with MHz range
>>>>>>>>>> minimum CPU frequency range.  For example, on a 800MHz to 4.20GHz system
>>>>>>>>>> the command returns
>>>>>>>>>>
>>>>>>>>>> hardware limits: 800000 MHz - 4.200000 GHz
>>>>>>>>>>
>>>>>>>>>> The code that causes this error can be removed.  The next else if clause
>>>>>>>>>> will handle the output correctly such that
>>>>>>>>>>
>>>>>>>>>> hardware limits: 800.000 MHz - 4.200000 GHz
>>>>>>>>>>
>>>>>>>>>> is displayed correctly.
>>>>>>>>>>
>>>>>>>>>> [v2]: Remove two lines instead of fixing broken code.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>>>>>>>> Cc: Thomas Renninger <trenn@suse.com>
>>>>>>>>>> Cc: Stafford Horne <shorne@gmail.com>
>>>>>>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>>>>>>> ---
>>>>>>>>>>  tools/power/cpupower/utils/cpufreq-info.c | 2 --
>>>>>>>>>>  1 file changed, 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>>>> index 3e701f0e9c14..df43cd45d810 100644
>>>>>>>>>> --- a/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>>>> +++ b/tools/power/cpupower/utils/cpufreq-info.c
>>>>>>>>>> @@ -93,8 +93,6 @@ static void print_speed(unsigned long speed)
>>>>>>>>>>               if (speed > 1000000)
>>>>>>>>>>                       printf("%u.%06u GHz", ((unsigned int) speed/1000000),
>>>>>>>>>>                               ((unsigned int) speed%1000000));
>>>>>>>>>> -             else if (speed > 100000)
>>>>>>>>>> -                     printf("%u MHz", (unsigned int) speed);
>>>>>>>>>>               else if (speed > 1000)
>>>>>>>>>>                       printf("%u.%03u MHz", ((unsigned int) speed/1000),
>>>>>>>>>>                               (unsigned int) (speed%1000));
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks.  I will queue this up for 4.15-rc1.
>>>>>>>>
>>>>>>>> OK
>>>>>>>>
>>>>>>>> So are you going to maintain this utility going forward?
>>>>>>>>
>>>>>>>
>>>>>>> oops. I was on auto-pilot responding to patches sitting in my Inbox.
>>>>>>> Wrong email response. Sorry about that.
>>>>>>>
>>>>>>> Please ignore. Mu bad.
>>>>>>
>>>>>> OK :-)
>>>>>>
>>>>>> But that said, from my perspective, cpupower is basically not maintained.
>>>>>>
>>>>>> Thomas, who sort of maintained it, but then basically became a patch
>>>>>> reviewer for it, does not respond to patches any more and I am not
>>>>>> sufficiently familiar with the code to be able to effectively review
>>>>>> the patches myself, nor I have the time to get more familiar with it.
>>>>>>
>>>>>> For this reason, I'm inclined to drop this code from the kernel source
>>>>>> tree unless somebody steps in to fill the gap.
>>>>>>
>>>>>
>>>>> Please don't drop this from kernel sources.. I think this is useful. If you
>>>>> are looking for a maintainer, I will be happy to step up to maintain it.
>>>>
>>>> Cool, please do that then. :-)
>>>>
>>>> I will be happy to take pull requests with cpupower changes so that
>>>> they go in along with the other PM material.
>>>>
>>>
>>> Sounds like a plan. I can do that. I will work on getting git setup and
>>> send pull requests. We can get that going for 4.15 unless you think it is
>>> late for you to get pull requests.
>>
>> Shuah, I have two other cleanup patches that should be applied to cpupower.  I
>> will post them shortly.
>>
> 
> Hi Prarit,
> 
> Does this tool build for you? I am seeing:
> 
> utils/helpers/amd.c:7:21: fatal error: pci/pci.h: No such file or directory
>  #include <pci/pci.h>
>                      ^
> compilation terminated.
> Makefile:221: recipe for target 'utils/helpers/amd.o' failed
> make: *** [utils/helpers/amd.o] Error 1

This is strange.  I saw it, tried a git-bisect and now I can't reproduce it.
I'll check out a new tree and see if I can make this reproduce.

P.

> 
> thanks,
> -- Shuah
>
Prarit Bhargava Nov. 2, 2017, 8:18 p.m. UTC | #15
On 11/02/2017 02:59 PM, Shuah Khan wrote:
> On 11/01/2017 06:42 PM, Prarit Bhargava wrote:
> 
> Hi Prarit,
> 
> Does this tool build for you? I am seeing:
> 
> utils/helpers/amd.c:7:21: fatal error: pci/pci.h: No such file or directory
>  #include <pci/pci.h>
>                      ^
> compilation terminated.
> Makefile:221: recipe for target 'utils/helpers/amd.o' failed
> make: *** [utils/helpers/amd.o] Error 1
> 

Got it ... you have to have the pciutils-devel package installed which
installs /usr/include/pci/pci.h.

That file contains:

struct pci_access {
  /* Options you can change: */
  unsigned int method;                  /* Access method */
  int writeable;                        /* Open in read/write mode */
  int buscentric;                       /* Bus-centric view of the world */
<snip>

P.

> thanks,
> -- Shuah
>
Shuah Khan Nov. 2, 2017, 8:20 p.m. UTC | #16
On 11/02/2017 02:18 PM, Prarit Bhargava wrote:
> 
> 
> On 11/02/2017 02:59 PM, Shuah Khan wrote:
>> On 11/01/2017 06:42 PM, Prarit Bhargava wrote:
>>
>> Hi Prarit,
>>
>> Does this tool build for you? I am seeing:
>>
>> utils/helpers/amd.c:7:21: fatal error: pci/pci.h: No such file or directory
>>  #include <pci/pci.h>
>>                      ^
>> compilation terminated.
>> Makefile:221: recipe for target 'utils/helpers/amd.o' failed
>> make: *** [utils/helpers/amd.o] Error 1
>>
> 
> Got it ... you have to have the pciutils-devel package installed which
> installs /usr/include/pci/pci.h.
> 
> That file contains:
> 
> struct pci_access {
>   /* Options you can change: */
>   unsigned int method;                  /* Access method */
>   int writeable;                        /* Open in read/write mode */
>   int buscentric;                       /* Bus-centric view of the world */
> <snip>
> 

Thanks for looking into this.

-- Shuah
diff mbox

Patch

diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
index 3e701f0e9c14..df43cd45d810 100644
--- a/tools/power/cpupower/utils/cpufreq-info.c
+++ b/tools/power/cpupower/utils/cpufreq-info.c
@@ -93,8 +93,6 @@  static void print_speed(unsigned long speed)
 		if (speed > 1000000)
 			printf("%u.%06u GHz", ((unsigned int) speed/1000000),
 				((unsigned int) speed%1000000));
-		else if (speed > 100000)
-			printf("%u MHz", (unsigned int) speed);
 		else if (speed > 1000)
 			printf("%u.%03u MHz", ((unsigned int) speed/1000),
 				(unsigned int) (speed%1000));