package: add powertop 2.7
PATCH 1/1 package: add powertop 2.7
---
v2:
- Propagate dependencies from libnl and pciutils.
- Add description for powertop-autotune.patch.
- Fix hash origin data.
- Remove defaults-satisfied POWERTOP_SOURCE value.
package/Config.in | 1 + package/powertop/Config.in | 13 +++++++++++++ package/powertop/powertop-autotune.patch | 15 +++++++++++++++ package/powertop/powertop.hash | 2 ++ package/powertop/powertop.mk | 13 +++++++++++++ 5 files changed, 44 insertions(+) create mode 100644 package/powertop/Config.in create mode 100644 package/powertop/powertop-autotune.patch create mode 100644 package/powertop/powertop.hash create mode 100644 package/powertop/powertop.mk
package/Config.in +1 -0
@@ -365,6 +365,7 @@ endif source "package/pciutils/Config.in" source "package/picocom/Config.in" source "package/pifmrds/Config.in" + source "package/powertop/Config.in" source "package/pps-tools/Config.in" source "package/read-edid/Config.in" source "package/rng-tools/Config.in"
package/powertop/Config.in new file +13 -0
@@ -0,0 +1,13 @@ +config BR2_PACKAGE_POWERTOP + bool "powertop" + # pciutils dependency + depends on !BR2_bfin + # libnl dependency + depends on BR2_TOOLCHAIN_HAS_THREADS + select BR2_PACKAGE_NCURSES + select BR2_PACKAGE_PCIUTILS + select BR2_PACKAGE_LIBNL + help + A tool to diagnose issues with power consumption and power management + + https://01.org/powertop/
package/powertop/powertop-autotune.patch new file +15 -0
@@ -0,0 +1,15 @@ +Patch pulled from https://projects.archlinux.org/svntogit/community.git/tree/trunk?h=packages/powertop&id=37469c47b885c50365f57044e4ad72e0e3512b91 + +Fixes a use-after-close bug in create_all_devfreq_devices(). + +Signed-off-by: Steven Noonan <[email protected]> + +--- a/src/devices/devfreq.cpp ++++ b/src/devices/devfreq.cpp +@@ -247,6 +247,7 @@ void create_all_devfreq_devices(void) + fprintf(stderr, "Devfreq not enabled\n"); + is_enabled = false; + closedir(dir); ++ dir = NULL; + return; + }
package/powertop/powertop.hash new file +2 -0
@@ -0,0 +1,2 @@ +# Locally-generated hash +sha256 8d4b1490e2baad4467c0ded3c423db4472dcbf7b2dd8f8f2a928f54047c678ca powertop-2.7.tar.gz
package/powertop/powertop.mk new file +13 -0
@@ -0,0 +1,13 @@ +################################################################################ +# +# powertop +# +################################################################################ + +POWERTOP_VERSION = 2.7 +POWERTOP_SITE = https://01.org/sites/default/files/downloads/powertop/ +POWERTOP_DEPENDENCIES = pciutils ncurses libnl +POWERTOP_LICENSE = GPLv2 +POWERTOP_LICENSE_FILES = COPYING + +$(eval $(autotools-package))
Signature · 1 line
2.3.3
PATCH 1/1 package: add powertop 2.7
Dear Steven Noonan,
Quoted 1 line · Steven Noonan
Signed-off-by Steven Noonan <[email protected]>
I've applied, but to be honest, I found that they were too many trivial issues that should have been discovered before the patch was submitted. Here is the list of things that I had to change:
[Thomas:
- fix commit title
- powertop wants libintl unconditionally, so make sure BR2_PACKAGE_GETTEXT is selected when BR2_NEEDS_GETTEXT is set, and add gettext to the dependencies.
- add missing comment about thread dependency.
- add missing dependency on host-pkgconf, without which powertop cannot find libnl.
- patch src/Makefile.am to not pass -fstack-protector, which fails to build if the toolchain does not have SSP support.
- rename patch powertop-autotune.patch to confirm to the patch naming convention.]
Commit title: we normally expect "<foo>: new package" for package additions. I agree, this is pure nitpicking :-)
The libintl issue was trivially found by using a uClibc toolchain, which is the default in Buildroot. You can for example use
http://autobuild.buildroot.org/toolchains/configs/br-arm-full.config as a base configuration file to quickly test things against uClibc.
The missing thread comment was trivial, it's documented in the section "17.2.2. Dependencies on target and toolchain options" in the Buildroot manual.
The missing dependency on host-pkgconf is seen when you do a build with just powertop. Whenever you submit a new package, make sure you do a build from scratch with just this package enabled, so that you have some minimal verification that the dependencies are correct.
The -fstack-protector issue was also seen because I'm using a standard uClibc toolchain, which do not have SSP enabled by default.
The patch naming convention is also documented in the Buildroot manual.
In the light of those comments, could you have a second look at your other "new package" submissions and retest them with a uClibc toolchain, and with a Buildroot build having just each new package enabled, in order to discover missing dependencies?
Thanks a lot!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/1 package: add powertop 2.7
Quoted 9 lines · Thomas Petazzoni
Dear Steven Noonan,
Quoted 1 line · Steven Noonan
Signed-off-by Steven Noonan <[email protected]>I've applied, but to be honest, I found that they were too many trivial issues that should have been discovered before the patch was submitted. Here is the list of things that I had to change:
Quoted 12 lines
[Thomas:
- fix commit title
- powertop wants libintl unconditionally, so make sureBR2_PACKAGE_GETTEXTis selected whenBR2_NEEDS_GETTEXTis set, and add gettext to the dependencies.
- add missing comment about thread dependency.
- add missing dependency on host-pkgconf, without which powertop cannot find libnl.
- patch src/Makefile.am to not pass -fstack-protector, which fails to build if the toolchain does not have SSP support.
- rename patch powertop-autotune.patch to confirm to the patch naming convention.]
Yes, I agree, that number of issues is ridiculous.
Quoted 20 lines
Commit title: we normally expect "<foo>: new package" for package additions. I agree, this is pure nitpicking :-)
The libintl issue was trivially found by using a uClibc toolchain, which is the default in Buildroot. You can for example use
http://autobuild.buildroot.org/toolchains/configs/br-arm-full.config as a base configuration file to quickly test things against uClibc.The missing thread comment was trivial, it's documented in the section "17.2.2. Dependencies on target and toolchain options" in the Buildroot manual.
The missing dependency on host-pkgconf is seen when you do a build with just powertop. Whenever you submit a new package, make sure you do a build from scratch with just this package enabled, so that you have some minimal verification that the dependencies are correct.
The -fstack-protector issue was also seen because I'm using a standard uClibc toolchain, which do not have SSP enabled by default.
Quoted 8 lines
The patch naming convention is also documented in the Buildroot manual.
In the light of those comments, could you have a second look at your other "new package" submissions and retest them with a uClibc toolchain, and with a Buildroot build having just each new package enabled, in order to discover missing dependencies?
Thanks a lot!
I appreciate the feedback. I had only been testing with x86_64 and x32 glibc builds, and hadn't thought of uClibc issues. I'll be sure to try and test that in the future.
PATCH 1/1 package: add powertop 2.7
Dear Steven Noonan,
Quoted 13 lines · Steven Noonan
Quoted 4 lines
[Thomas:
- fix commit title
- powertop wants libintl unconditionally, so make sureBR2_PACKAGE_GETTEXTis selected whenBR2_NEEDS_GETTEXTis set,Quoted 8 lines
and add gettext to the dependencies.
- add missing comment about thread dependency.
- add missing dependency on host-pkgconf, without which powertop cannot find libnl.
- patch src/Makefile.am to not pass -fstack-protector, which fails to build if the toolchain does not have SSP support.
- rename patch powertop-autotune.patch to confirm to the patch naming convention.]
Quoted 1 line
Yes, I agree, that number of issues is ridiculous.
Well, I wouldn't use "ridiculous" : those are your first Buildroot contributions, so it's kind of expected that you don't necessarily know all the combinations we typically test before applying patches. I'm very happy to see new contributors, and to see you and Mike Williams taking care of the systemd stuff. So clearly I don't want to shoot down your contributions :-)
Quoted 3 lines
I appreciate the feedback. I had only been testing with
x86_64and x32 glibc builds, and hadn't thought of uClibc issues. I'll be sure to try and test that in the future.
Yes, even though glibc is widely used, uClibc is still our default C library, and so we continue to check that things build fine with it. You have some configuration fragments that use pre-built uClibc toolchains at http://autobuild.buildroot.org/toolchains/configs/, which make it easy to test various situations without having to rebuild a full toolchain each time.
Thanks,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com