2:09 a.m.
From: Nanley Chery <nanley.g.chery(a)intel.com>
This patch series changes PowerTOP's method of accessing sysfs (for
tunables mainly) from explicitly accessing files to using the standard
udev device management library and API.
The current codebase collects data on devices by probing sysfs files
directly with the usual low-level file handling API. This method is
not recommended by the Linux kernel maintainers because the sysfs
organization is not guaranteed to be stable over time. This can lead
to dependent applications breaking with newer kernel releases. Instead
using an abstraction layer, like udev, is suggested
(see https://www.kernel.org/doc/Documentation/sysfs-rules.txt).
The benefits of switching to udev include:
* The removal of libpci and a reduction in repetitive code.
* Improved readability of the tunable code.
* Faster reading of device state values due to caching provided by udev.
I've tried to make this transition as simple as possible by including
the bare minimum of changes required for this feature. There's at least
one more tunable that can be converted (i2c), but I would like to get
feedback on the current implementation first.
Nanley Chery (9):
build with udev library
add udev helper functions
usb_tunable: use udev
syfs_tunable: use udev
runtime_tunable: use udev
replace pciutils usage with udevhwdb
runtime_pm: use udev
remove libpci and most dead code
calibrate: use udev
Android.mk | 1 -
README | 1 -
configure.ac | 20 ++-----
src/Makefile.am | 6 +-
src/calibrate/calibrate.cpp | 44 ++++----------
src/devices/runtime_pm.cpp | 105 +++++++++++---------------------
src/devices/runtime_pm.h | 1 +
src/lib.cpp | 100 +++++++++++++++++++++----------
src/lib.h | 7 ++-
src/main.cpp | 4 +-
src/tuning/runtime.cpp | 139 +++++++++++++------------------------------
src/tuning/runtime.h | 6 +-
src/tuning/tuningsysfs.cpp | 12 +++-
src/tuning/tuningsysfs.h | 1 +
src/tuning/tuningusb.cpp | 142 ++++++++++++++------------------------------
src/tuning/tuningusb.h | 5 +-
16 files changed, 231 insertions(+), 363 deletions(-)
--
2.4.1
2:09 a.m.
New subject: [Powertop] [RFC 1/9] build with udev library
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
---
configure.ac | 5 +++++
src/Makefile.am | 3 +++
2 files changed, 8 insertions(+)
diff --git a/configure.ac b/configure.ac
index 79ea055..9af9604 100644
--- a/configure.ac
+++ b/configure.ac
@@ -54,6 +54,7 @@ AC_CHECK_HEADERS([ \
locale.h \
malloc.h \
ncurses.h \
+ libudev.h \
stdint.h \
stdlib.h \
string.h \
@@ -106,6 +107,10 @@ AC_CHECK_FUNCS([ \
strtoull \
])
+PKG_CHECK_MODULES([UDEV], [libudev], [LIBS="$LIBS $udev_LIBS"], [
+ AC_MSG_ERROR([libudev is required but was not found])
+])
+
PKG_CHECK_MODULES([NCURSES], [ncursesw ncurses], [LIBS="$LIBS $ncurses_LIBS"],
[
AC_SEARCH_LIBS([delwin], [ncursesw ncurses], [], [
AC_MSG_ERROR([ncurses is required but was not found])
diff --git a/src/Makefile.am b/src/Makefile.am
index d7132c0..ac2259c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -133,6 +133,7 @@ powertop_CXXFLAGS = \
$(GLIB2_CFLAGS) \
$(LIBNL_CFLAGS) \
$(NCURSES_CFLAGS) \
+ $(UDEV_CFLAGS) \
$(PCIUTILS_CFLAGS)
@@ -142,6 +143,7 @@ powertop_CPPFLAGS = \
$(LIBNL_CFLAGS) \
$(LIBZ_CFLAGS) \
$(NCURSES_CFLAGS) \
+ $(UDEV_CFLAGS) \
$(PCIUTILS_CFLAGS)
powertop_LDADD = \
@@ -152,6 +154,7 @@ AM_LDFLAGS = \
$(LIBS) \
$(LIBZ_LIBS) \
$(NCURSES_LIBS) \
+ $(UDEV_LIBS) \
$(PCIUTILS_LIBS) \
$(PTHREAD_LIBS) \
$(RESOLV_LIBS)
--
2.4.1
2:09 a.m.
New subject: [Powertop] [RFC 2/9] add udev helper functions
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
---
src/lib.cpp | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/lib.h | 4 ++++
src/main.cpp | 1 +
3 files changed, 59 insertions(+)
diff --git a/src/lib.cpp b/src/lib.cpp
index 437803b..5a9c6be 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -291,6 +291,29 @@ void format_watts(double W, char *buffer, unsigned int len)
align_string(buffer, len, len);
}
+static struct udev *udev_cxt = NULL;
+
+struct udev * get_udev()
+{
+ if (udev_cxt == NULL) {
+ udev_cxt = udev_new();
+ if (udev_cxt == NULL) {
+ printf("Couldn't create udev\n");
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ return udev_cxt;
+}
+
+void cleanup_udev()
+{
+ if (udev_cxt != NULL) {
+ udev_unref(udev_cxt);
+ udev_cxt = NULL;
+ }
+}
+
#ifndef HAVE_NO_PCI
static struct pci_access *pci_access;
@@ -437,6 +460,37 @@ int equals(double a, double b)
return fabs(a - b) <= std::numeric_limits<double>::epsilon();
}
+void process_subsystem(const char *subsystem, callback fn, int num_attr, ...)
+{
+ /* Find devices in subsystem with given attributes */
+ struct udev_enumerate *enumerate = udev_enumerate_new(get_udev());
+ udev_enumerate_add_match_subsystem(enumerate, subsystem);
+ va_list args;
+ va_start (args, num_attr);
+ while (num_attr--) {
+ const char *attr = va_arg(args, const char*);
+ const char *attr_val = va_arg(args, const char*);
+ if (*attr == '+')
+ udev_enumerate_add_match_sysattr(enumerate, &attr[1], attr_val);
+ else if (*attr == '-')
+ udev_enumerate_add_nomatch_sysattr(enumerate, &attr[1], attr_val);
+ else {
+ fprintf (stderr, "Bad format of attribute\n");
+ exit(1);
+ }
+ }
+ va_end (args);
+ udev_enumerate_scan_devices(enumerate);
+
+ /* Run callback on each device */
+ struct udev_list_entry *dev_entry;
+ udev_list_entry_foreach(dev_entry, udev_enumerate_get_list_entry(enumerate))
+ fn(udev_list_entry_get_name(dev_entry));
+
+ /* Cleanup */
+ udev_enumerate_unref(enumerate);
+}
+
void process_directory(const char *d_name, callback fn)
{
struct dirent *entry;
diff --git a/src/lib.h b/src/lib.h
index 5c94271..ae0421f 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -28,6 +28,7 @@
#include <libintl.h>
#include <stdint.h>
#include <stdlib.h>
+#include <libudev.h>
/* Include only for Automake builds */
#ifdef HAVE_CONFIG_H
@@ -70,6 +71,8 @@ extern void format_watts(double W, char *buffer, unsigned int len);
extern char *pci_id_to_name(uint16_t vendor, uint16_t device, char *buffer, int len);
extern void end_pci_access(void);
+extern struct udev * get_udev();
+extern void cleanup_udev();
extern char *fmt_prefix(double n, char *buf);
extern char *pretty_print(const char *str, char *buf, int len);
@@ -77,6 +80,7 @@ extern int equals(double a, double b);
typedef void (*callback)(const char*);
extern void process_directory(const char *d_name, callback fn);
+extern void process_subsystem(const char *subsystem, callback fn, int num_attr, ...);
extern int utf_ok;
extern int get_user_input(char *buf, unsigned sz);
extern int read_msr(int cpu, uint64_t offset, uint64_t *value);
diff --git a/src/main.cpp b/src/main.cpp
index dc69fca..65e54ac 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -379,6 +379,7 @@ void clean_shutdown()
clear_all_devices();
clear_all_devfreq();
clear_all_cpus();
+ cleanup_udev();
return;
}
--
2.4.1
2:09 a.m.
New subject: [Powertop] [RFC 3/9] usb_tunable: use udev
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
---
src/tuning/tuningusb.cpp | 142 +++++++++++++++--------------------------------
src/tuning/tuningusb.h | 5 +-
2 files changed, 47 insertions(+), 100 deletions(-)
diff --git a/src/tuning/tuningusb.cpp b/src/tuning/tuningusb.cpp
index d2a0c11..ea2d26b 100644
--- a/src/tuning/tuningusb.cpp
+++ b/src/tuning/tuningusb.cpp
@@ -25,132 +25,78 @@
#include "tuning.h"
#include "tunable.h"
-#include "unistd.h"
#include "tuningusb.h"
#include <string.h>
-#include <dirent.h>
-#include <utility>
-#include <iostream>
-#include <fstream>
-#include "../lib.h"
-
-usb_tunable::usb_tunable(const char *path, const char *name) : tunable("", 0.9,
_("Good"), _("Bad"), _("Unknown"))
+usb_tunable::usb_tunable(struct udev_device *usb_dev) : tunable("", 0.9,
_("Good"), _("Bad"), _("Unknown"))
{
- ifstream file;
- char filename[4096];
- char vendor[2048];
- char product[2048];
- string str1, str2;
- sprintf(usb_path, "%s/power/control", path);
-
- vendor[0] = 0;
- product[0] = 0;
-
- str1 = read_sysfs_string("%s/idVendor", path);
- str2 = read_sysfs_string("%s/idProduct", path);
+ tun_dev = usb_dev;
+ udev_device_ref(tun_dev);
- sprintf(desc, _("Autosuspend for unknown USB device %s (%s:%s)"), name,
str1.c_str(), str2.c_str());
+ const char * name = udev_device_get_sysname(usb_dev);
+ const char * vendor = udev_device_get_sysattr_value(usb_dev, "manufacturer");
+ const char * product = udev_device_get_sysattr_value(usb_dev, "product");
- sprintf(filename, "%s/manufacturer", path);
- file.open(filename, ios::in);
- if (file) {
- file.getline(vendor, 2047);
- if (strstr(vendor, "Linux "))
- vendor[0] = 0;
- file.close();
- };
- sprintf(filename, "%s/product", path);
- file.open(filename, ios::in);
- if (file) {
- file.getline(product, 2040);
- file.close();
- };
- if (strlen(vendor) && strlen(product))
+ if (vendor && product && strstr(vendor, "Linux "))
sprintf(desc, _("Autosuspend for USB device %s [%s]"), product, vendor);
- else if (strlen(product))
+ else if (product)
sprintf(desc, _("Autosuspend for USB device %s [%s]"), product, name);
- else if (strlen(vendor))
+ else if (vendor && strstr(vendor, "Linux "))
sprintf(desc, _("Autosuspend for USB device %s [%s]"), vendor, name);
+ else {
+ const char * str1 = udev_device_get_sysattr_value(usb_dev, "idVendor");
+ const char * str2 = udev_device_get_sysattr_value(usb_dev, "idProduct");
+ sprintf(desc, _("Autosuspend for unknown USB device %s (%s:%s)"), name, str1,
str2);
+ }
- sprintf(toggle_good, "echo 'auto' > '%s';", usb_path);
- sprintf(toggle_bad, "echo 'on' > '%s';", usb_path);
+ const char * path = udev_device_get_syspath(usb_dev);
+ sprintf(toggle_good, "echo 'auto' > '%s/power/control';",
path);
+ sprintf(toggle_bad, "echo 'on' > '%s/power/control';",
path);
}
-int usb_tunable::good_bad(void)
+usb_tunable::~usb_tunable()
{
- string content;
-
- content = read_sysfs_string(usb_path);
-
- if (strcmp(content.c_str(), "auto") == 0)
- return TUNE_GOOD;
+ udev_device_unref(tun_dev);
+}
- return TUNE_BAD;
+int usb_tunable::good_bad(void)
+{
+ const char * str1 = udev_device_get_sysattr_value(tun_dev, "power/control");
+ return strcmp(str1, "auto") == 0 ? TUNE_GOOD : TUNE_BAD;
}
void usb_tunable::toggle(void)
{
- int good;
- good = good_bad();
-
- if (good == TUNE_GOOD) {
- write_sysfs(usb_path, "on");
- return;
- }
-
- write_sysfs(usb_path, "auto");
+ std::string str = good_bad() == TUNE_GOOD ? "on" : "auto";
+ udev_device_set_sysattr_value(tun_dev, "power/control", &str[0]);
}
const char *usb_tunable::toggle_script(void)
{
- int good;
- good = good_bad();
-
- if (good == TUNE_GOOD) {
- return toggle_bad;
- }
-
- return toggle_good;
+ return good_bad() == TUNE_GOOD ? toggle_bad : toggle_good;
}
static void add_usb_callback(const char *d_name)
{
- class usb_tunable *usb;
- char filename[4096];
- DIR *dir;
-
- sprintf(filename, "/sys/bus/usb/devices/%s/power/control", d_name);
- if (access(filename, R_OK) != 0)
- return;
-
- sprintf(filename, "/sys/bus/usb/devices/%s/power/active_duration", d_name);
- if (access(filename, R_OK)!=0)
- return;
-
- /* every interface of this device should support autosuspend */
- sprintf(filename, "/sys/bus/usb/devices/%s", d_name);
- if ((dir = opendir(filename))) {
- struct dirent *entry;
- while ((entry = readdir(dir))) {
- /* dirname: <busnum>-<devnum>...:<config num>-<interface num>
*/
- if (!isdigit(entry->d_name[0]))
- continue;
- sprintf(filename, "/sys/bus/usb/devices/%s/%s/supports_autosuspend", d_name,
entry->d_name);
- if (access(filename, R_OK) == 0 && read_sysfs(filename) == 0)
- break;
- }
- closedir(dir);
- if (entry)
- return;
- }
-
- sprintf(filename, "/sys/bus/usb/devices/%s", d_name);
- usb = new class usb_tunable(filename, d_name);
- all_tunables.push_back(usb);
+ struct udev *udev = get_udev();
+ struct udev_device *dev = udev_device_new_from_syspath(udev, d_name);
+ struct udev_enumerate *dev_enumerate = udev_enumerate_new(udev);
+ udev_enumerate_add_match_parent(dev_enumerate, dev);
+ udev_enumerate_add_match_sysattr(dev_enumerate, "supports_autosuspend",
"0");
+ udev_enumerate_scan_devices(dev_enumerate);
+
+ /* If no devices in tree lack autosuspend support, add to tunables */
+ if (udev_enumerate_get_list_entry(dev_enumerate) == NULL)
+ all_tunables.push_back(new usb_tunable(dev));
+
+ /* Cleanup */
+ udev_device_unref(dev);
+ udev_enumerate_unref(dev_enumerate);
}
void add_usb_tunables(void)
{
- process_directory("/sys/bus/usb/devices/", add_usb_callback);
+ process_subsystem("usb", add_usb_callback, 2,
+ "+power/control", NULL,
+ "+power/active_duration", NULL);
}
diff --git a/src/tuning/tuningusb.h b/src/tuning/tuningusb.h
index a257904..3554b18 100644
--- a/src/tuning/tuningusb.h
+++ b/src/tuning/tuningusb.h
@@ -32,9 +32,10 @@
using namespace std;
class usb_tunable : public tunable {
- char usb_path[4096];
+ struct udev_device *tun_dev;
public:
- usb_tunable(const char *usb_path, const char *path);
+ usb_tunable(struct udev_device *usb_dev);
+ ~usb_tunable();
virtual int good_bad(void);
--
2.4.1
2:09 a.m.
New subject: [Powertop] [RFC 4/9] syfs_tunable: use udev
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
---
src/tuning/tuningsysfs.cpp | 12 +++++++++---
src/tuning/tuningsysfs.h | 1 +
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/tuning/tuningsysfs.cpp b/src/tuning/tuningsysfs.cpp
index 0859a0c..fb486bd 100644
--- a/src/tuning/tuningsysfs.cpp
+++ b/src/tuning/tuningsysfs.cpp
@@ -115,15 +115,21 @@ void add_sysfs_tunable(const char *str, const char *_sysfs_path,
const char *_ta
static void add_sata_tunables_callback(const char *d_name)
{
+ struct udev_device *dev = udev_device_new_from_syspath(get_udev(), d_name);
+ const char *dev_name = udev_device_get_sysname(dev);
char filename[4096];
char msg[4096];
- sprintf(filename, "/sys/class/scsi_host/%s/link_power_management_policy",
d_name);
- sprintf(msg, _("Enable SATA link power management for %s"), d_name);
+ sprintf(filename, "%s/link_power_management_policy", d_name);
+ sprintf(msg, _("Enable SATA link power management for %s"), dev_name);
add_sysfs_tunable(msg, filename,"min_power");
+
+ /* Cleanup */
+ udev_device_unref(dev);
}
void add_sata_tunables(void)
{
- process_directory("/sys/class/scsi_host", add_sata_tunables_callback);
+ process_subsystem("scsi_host", add_sata_tunables_callback, 1,
+ "+link_power_management_policy", NULL);
}
diff --git a/src/tuning/tuningsysfs.h b/src/tuning/tuningsysfs.h
index ad89717..71586ab 100644
--- a/src/tuning/tuningsysfs.h
+++ b/src/tuning/tuningsysfs.h
@@ -32,6 +32,7 @@
using namespace std;
class sysfs_tunable : public tunable {
+ struct udev_device *tun_dev;
char sysfs_path[4096];
char target_value[4096];
char bad_value[4096];
--
2.4.1
2:09 a.m.
New subject: [Powertop] [RFC 5/9] runtime_tunable: use udev
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
---
src/devices/runtime_pm.cpp | 7 +++
src/devices/runtime_pm.h | 1 +
src/tuning/runtime.cpp | 140 ++++++++++++++-------------------------------
src/tuning/runtime.h | 6 +-
4 files changed, 56 insertions(+), 98 deletions(-)
diff --git a/src/devices/runtime_pm.cpp b/src/devices/runtime_pm.cpp
index eede027..3dfebf4 100644
--- a/src/devices/runtime_pm.cpp
+++ b/src/devices/runtime_pm.cpp
@@ -143,6 +143,13 @@ void runtime_pmdevice::set_human_name(char *_name)
strcpy(humanname, _name);
}
+int udevice_has_runtime_pm(struct udev_device* udevice)
+{
+ const char *sus_t = udev_device_get_sysattr_value(udevice,
"power/runtime_suspended_time");
+ const char *act_t = udev_device_get_sysattr_value(udevice,
"power/runtime_active_time");
+ return (sus_t != NULL &&
+ (atoi(sus_t) || (act_t != NULL && atoi(act_t))));
+}
int device_has_runtime_pm(const char *sysfs_path)
{
diff --git a/src/devices/runtime_pm.h b/src/devices/runtime_pm.h
index ea09dac..0676ca3 100644
--- a/src/devices/runtime_pm.h
+++ b/src/devices/runtime_pm.h
@@ -60,6 +60,7 @@ public:
extern void create_all_runtime_pm_devices(void);
extern int device_has_runtime_pm(const char *sysfs_path);
+extern int udevice_has_runtime_pm(struct udev_device* udevice);
#endif
\ No newline at end of file
diff --git a/src/tuning/runtime.cpp b/src/tuning/runtime.cpp
index 3201fdd..463372d 100644
--- a/src/tuning/runtime.cpp
+++ b/src/tuning/runtime.cpp
@@ -25,135 +25,83 @@
#include "tuning.h"
#include "tunable.h"
-#include "unistd.h"
#include "runtime.h"
#include <string.h>
#include <utility>
-#include <iostream>
-#include <fstream>
#include <unistd.h>
#include <sys/types.h>
-#include <dirent.h>
#include "../lib.h"
#include "../devices/runtime_pm.h"
-runtime_tunable::runtime_tunable(const char *path, const char *bus, const char *dev) :
tunable("", 0.4, _("Good"), _("Bad"),
_("Unknown"))
+runtime_tunable::runtime_tunable(struct udev_device* udev_device) :
+ tunable("", 0.4, _("Good"), _("Bad"),
_("Unknown")), tun_dev(udev_device)
{
- ifstream file;
- sprintf(runtime_path, "%s/power/control", path);
-
-
- sprintf(desc, _("Runtime PM for %s device %s"), bus, dev);
- if (!device_has_runtime_pm(path))
- sprintf(desc, _("%s device %s has no runtime power management"), bus, dev);
+ udev_device_ref(tun_dev);
+ const char *bus = udev_device_get_subsystem(udev_device);
+ const char *dev = udev_device_get_sysname(udev_device);
+ /* Give more accurate descriptions for PCI devices */
if (strcmp(bus, "pci") == 0) {
- char filename[4096];
- uint16_t vendor = 0, device = 0;
-
- sprintf(filename, "/sys/bus/%s/devices/%s/vendor", bus, dev);
-
- file.open(filename, ios::in);
- if (file) {
- file >> hex >> vendor;
- file.close();
- }
-
-
- sprintf(filename, "/sys/bus/%s/devices/%s/device", bus, dev);
- file.open(filename, ios::in);
- if (file) {
- file >> hex >> device;
- file.close();
- }
-
+ const char *uvendor = udev_device_get_sysattr_value(udev_device, "vendor");
+ const char *udevice = udev_device_get_sysattr_value(udev_device, "device");
+ uint16_t vendor = stoul(string(uvendor), NULL, 16);
+ uint16_t device = stoul(string(udevice), NULL, 16);
+ bus = "PCI";
if (vendor && device) {
- if (!device_has_runtime_pm(path))
- sprintf(desc, _("PCI Device %s has no runtime power management"),
pci_id_to_name(vendor, device, filename, 4095));
- else
- sprintf(desc, _("Runtime PM for PCI Device %s"), pci_id_to_name(vendor,
device, filename, 4095));
+ char filename[4096];
+ dev = pci_id_to_name(vendor, device, filename, 4095);
}
+ }
+ if (!udevice_has_runtime_pm(udev_device))
+ sprintf(desc, _("%s device %s has no runtime power management"), bus, dev);
+ else
+ sprintf(desc, _("Runtime PM for %s device %s"), bus, dev);
- }
- sprintf(toggle_good, "echo 'auto' > '%s';", runtime_path);
- sprintf(toggle_bad, "echo 'on' > '%s';", runtime_path);
+ const char *path = udev_device_get_syspath(udev_device);
+ sprintf(toggle_good, "echo 'auto' > '%s/power/control';",
path);
+ sprintf(toggle_bad, "echo 'on' > '%s/power/control';",
path);
}
-int runtime_tunable::good_bad(void)
+runtime_tunable::~runtime_tunable()
{
- string content;
-
- content = read_sysfs_string(runtime_path);
+ udev_device_unref(tun_dev);
+}
- if (strcmp(content.c_str(), "auto") == 0)
- return TUNE_GOOD;
- return TUNE_BAD;
+int runtime_tunable::good_bad(void)
+{
+ const char * str1 = udev_device_get_sysattr_value(tun_dev, "power/control");
+ return strcmp(str1, "auto") == 0 ? TUNE_GOOD : TUNE_BAD;
}
void runtime_tunable::toggle(void)
{
- int good;
- good = good_bad();
-
- if (good == TUNE_GOOD) {
- write_sysfs(runtime_path, "on");
- return;
- }
-
- write_sysfs(runtime_path, "auto");
+ std::string str = good_bad() == TUNE_GOOD ? "on" : "auto";
+ udev_device_set_sysattr_value(tun_dev, "power/control", &str[0]);
}
const char *runtime_tunable::toggle_script(void)
{
- int good;
- good = good_bad();
-
- if (good == TUNE_GOOD) {
- return toggle_bad;
- }
-
- return toggle_good;
+ return good_bad() == TUNE_GOOD ? toggle_bad : toggle_good;
}
-
-void add_runtime_tunables(const char *bus)
+static void add_runtime_callback(const char *path)
{
- struct dirent *entry;
- DIR *dir;
- char filename[4096];
-
- sprintf(filename, "/sys/bus/%s/devices/", bus);
- dir = opendir(filename);
- if (!dir)
- return;
- while (1) {
- class runtime_tunable *runtime;
-
- entry = readdir(dir);
+ struct udev_device *dev = udev_device_new_from_syspath(get_udev(), path);
+ runtime_tunable *runtime = new runtime_tunable(dev);
- if (!entry)
- break;
- if (entry->d_name[0] == '.')
- continue;
+ if (!udevice_has_runtime_pm(dev))
+ all_untunables.push_back(runtime);
+ else
+ all_tunables.push_back(runtime);
- sprintf(filename, "/sys/bus/%s/devices/%s/power/control", bus,
entry->d_name);
-
- if (access(filename, R_OK) != 0)
- continue;
-
-
- sprintf(filename, "/sys/bus/%s/devices/%s", bus, entry->d_name);
-
- runtime = new class runtime_tunable(filename, bus, entry->d_name);
-
- if (!device_has_runtime_pm(filename))
- all_untunables.push_back(runtime);
- else
- all_tunables.push_back(runtime);
+ /* Cleanup */
+ udev_device_unref(dev);
+}
- }
- closedir(dir);
+void add_runtime_tunables(const char *bus)
+{
+ process_subsystem(bus, add_runtime_callback, 1, "+power/control", NULL);
}
diff --git a/src/tuning/runtime.h b/src/tuning/runtime.h
index a3c3e20..29307dd 100644
--- a/src/tuning/runtime.h
+++ b/src/tuning/runtime.h
@@ -31,9 +31,11 @@
using namespace std;
class runtime_tunable : public tunable {
- char runtime_path[4096];
+ struct udev_device *tun_dev;
public:
- runtime_tunable(const char *runtime_path, const char *bus, const char *dev);
+ runtime_tunable(struct udev_device* udev);
+
+ ~runtime_tunable(void);
virtual int good_bad(void);
--
2.4.1
2:09 a.m.
New subject: [Powertop] [RFC 6/9] replace pciutils usage with udevhwdb
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
---
src/lib.cpp | 27 +++++++++++++++++++++++++++
src/lib.h | 1 +
src/tuning/runtime.cpp | 11 +++--------
3 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/src/lib.cpp b/src/lib.cpp
index 5a9c6be..399d2bc 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -314,6 +314,33 @@ void cleanup_udev()
}
}
+const char *upci_id_to_name(struct udev_device* dev, char* buffer)
+{
+ const char *uvendor = udev_device_get_sysattr_value(dev, "vendor");
+ const char *udevice = udev_device_get_sysattr_value(dev, "device");
+ uint16_t vendor = stoul(string(uvendor), NULL, 16);
+ uint16_t device = stoul(string(udevice), NULL, 16);
+ char modalias[64];
+ sprintf(modalias, "pci:v%08Xd%08X*", vendor, device);
+ const char *key1 = "ID_VENDOR_FROM_DATABASE";
+ const char *key2 = "ID_MODEL_FROM_DATABASE";
+
+ struct udev_hwdb *hwdb = udev_hwdb_new(udev_device_get_udev(dev));
+ struct udev_list_entry *entry;
+ udev_list_entry_foreach(entry, udev_hwdb_get_properties_list_entry(hwdb, modalias, 0))
+ if (strcmp(udev_list_entry_get_name(entry), key1) == 0)
+ uvendor = udev_list_entry_get_value(entry);
+ else if (strcmp(udev_list_entry_get_name(entry), key2) == 0)
+ udevice = udev_list_entry_get_value(entry);
+
+ if (uvendor && udevice) {
+ sprintf(buffer, "%s %s", uvendor, udevice);
+ return buffer;
+ }
+
+ return NULL;
+}
+
#ifndef HAVE_NO_PCI
static struct pci_access *pci_access;
diff --git a/src/lib.h b/src/lib.h
index ae0421f..464ce71 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -68,6 +68,7 @@ extern string read_sysfs_string(const char *format, const char *param);
extern void format_watts(double W, char *buffer, unsigned int len);
+extern const char *upci_id_to_name(struct udev_device* dev, char* buffer);
extern char *pci_id_to_name(uint16_t vendor, uint16_t device, char *buffer, int len);
extern void end_pci_access(void);
diff --git a/src/tuning/runtime.cpp b/src/tuning/runtime.cpp
index 463372d..ac65522 100644
--- a/src/tuning/runtime.cpp
+++ b/src/tuning/runtime.cpp
@@ -42,16 +42,11 @@ runtime_tunable::runtime_tunable(struct udev_device* udev_device) :
const char *dev = udev_device_get_sysname(udev_device);
/* Give more accurate descriptions for PCI devices */
+ char devname[4096];
if (strcmp(bus, "pci") == 0) {
- const char *uvendor = udev_device_get_sysattr_value(udev_device, "vendor");
- const char *udevice = udev_device_get_sysattr_value(udev_device, "device");
- uint16_t vendor = stoul(string(uvendor), NULL, 16);
- uint16_t device = stoul(string(udevice), NULL, 16);
+ if (upci_id_to_name(udev_device, devname))
+ dev = devname;
bus = "PCI";
- if (vendor && device) {
- char filename[4096];
- dev = pci_id_to_name(vendor, device, filename, 4095);
- }
}
if (!udevice_has_runtime_pm(udev_device))
--
2.4.1
2:15 a.m.
New subject: [Powertop] [RFC 6/9] replace pciutils usage with udevhwdb
On 6/1/2015 9:09 AM, Nanley Chery wrote:
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
no commit message?
also on some OSes the udevhwdb for PCI names is empty and the pciutils explicit file is
needed
6:33 a.m.
New subject: [Powertop] [RFC 6/9] replace pciutils usage with udevhwdb
On Mon, Jun 1, 2015 at 9:15 AM, Arjan van de Ven <arjan(a)linux.intel.com> wrote:
On 6/1/2015 9:09 AM, Nanley Chery wrote:
>
> Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
no commit message?
I've been sitting on these patches for a while and just
wanted to get
them out into the open. Most of my comments are in the cover letter.
I'll try to be better about commit messages in the future however.
also on some OSes the udevhwdb for PCI names is empty and the pciutils
explicit file is needed
I can't think of any work-around to this issue.
pciutils will have to
stick around.
2:09 a.m.
New subject: [Powertop] [RFC 7/9] runtime_pm: use udev
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
---
src/devices/runtime_pm.cpp | 98 +++++++++++++---------------------------------
1 file changed, 27 insertions(+), 71 deletions(-)
diff --git a/src/devices/runtime_pm.cpp b/src/devices/runtime_pm.cpp
index 3dfebf4..edcfb0f 100644
--- a/src/devices/runtime_pm.cpp
+++ b/src/devices/runtime_pm.cpp
@@ -29,7 +29,6 @@
#include <stdio.h>
#include <unistd.h>
#include <sys/types.h>
-#include <dirent.h>
#include "../parameters/parameters.h"
#include "../lib.h"
@@ -178,85 +177,42 @@ int device_has_runtime_pm(const char *sysfs_path)
return 0;
}
-static void do_bus(const char *bus)
+static void do_bus(const char *filename)
{
/* /sys/bus/pci/devices/0000\:00\:1f.0/power/runtime_suspended_time */
+ struct udev_device *udevice = udev_device_new_from_syspath(get_udev(), filename);
+ const char * name = udev_device_get_sysname(udevice);
+ const char * bus = udev_device_get_subsystem(udevice);
+ runtime_pmdevice *dev = new runtime_pmdevice(name, filename);
+
+ if (strcmp(bus, "i2c") == 0) {
+ char dev_name[4096];
+ const char *new_dev = udev_device_get_sysattr_value(udevice, "new_device");
+ const char *devname = udev_device_get_sysattr_value(udevice, "name");
+ sprintf(dev_name, _("I2C Device: %s"), devname);
+ sprintf(dev_name, _("I2C %s (%s): %s"), (new_dev ? _("Adapter") :
_("Device")), name, devname);
+ dev->set_human_name(dev_name);
+ }
- struct dirent *entry;
- DIR *dir;
- char filename[4096];
+ if (strcmp(bus, "pci") == 0) {
+ char dev_name[4096];
+ char devname[4096];
- sprintf(filename, "/sys/bus/%s/devices/", bus);
- dir = opendir(filename);
- if (!dir)
- return;
- while (1) {
- ifstream file;
- class runtime_pmdevice *dev;
- entry = readdir(dir);
-
- if (!entry)
- break;
- if (entry->d_name[0] == '.')
- continue;
-
- dev = new class runtime_pmdevice(entry->d_name, filename);
-
- if (strcmp(bus, "i2c") == 0) {
- string devname;
- char dev_name[4096];
- bool is_adapter = false;
-
- sprintf(filename, "/sys/bus/%s/devices/%s/new_device", bus,
entry->d_name);
- if (access(filename, W_OK) == 0)
- is_adapter = true;
-
- sprintf(filename, "/sys/bus/%s/devices/%s/name", bus, entry->d_name);
- file.open(filename, ios::in);
- if (file) {
- getline(file, devname);
- file.close();
- }
-
- sprintf(dev_name, _("I2C %s (%s): %s"), (is_adapter ? _("Adapter")
: _("Device")), entry->d_name, devname.c_str());
+ if (upci_id_to_name(udevice, devname)) {
+ sprintf(dev_name, _("PCI Device: %s"), devname);
dev->set_human_name(dev_name);
}
-
- if (strcmp(bus, "pci") == 0) {
- uint16_t vendor = 0, device = 0;
-
- sprintf(filename, "/sys/bus/%s/devices/%s/vendor", bus, entry->d_name);
-
- file.open(filename, ios::in);
- if (file) {
- file >> hex >> vendor;
- file.close();
- }
-
-
- sprintf(filename, "/sys/bus/%s/devices/%s/device", bus, entry->d_name);
- file.open(filename, ios::in);
- if (file) {
- file >> hex >> device;
- file.close();
- }
-
- if (vendor && device) {
- char devname[4096];
- sprintf(devname, _("PCI Device: %s"),
- pci_id_to_name(vendor, device, filename, 4095));
- dev->set_human_name(devname);
- }
- }
- all_devices.push_back(dev);
}
- closedir(dir);
+ all_devices.push_back(dev);
+
+ /* Cleanup */
+ udev_device_unref(udevice);
}
void create_all_runtime_pm_devices(void)
{
- do_bus("pci");
- do_bus("spi");
- do_bus("platform");
- do_bus("i2c");
+ process_subsystem("pci", do_bus, 0);
+ process_subsystem("spi", do_bus, 0);
+ process_subsystem("platform", do_bus, 0);
+ process_subsystem("i2c", do_bus, 0);
}
--
2.4.1
2:09 a.m.
New subject: [Powertop] [RFC 8/9] remove libpci and most dead code
Leave device_has_runtime_pm for now.
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
---
Android.mk | 1 -
README | 1 -
configure.ac | 15 ---------------
src/Makefile.am | 7 ++-----
src/lib.cpp | 45 ---------------------------------------------
src/lib.h | 2 --
src/main.cpp | 3 ---
7 files changed, 2 insertions(+), 72 deletions(-)
diff --git a/Android.mk b/Android.mk
index 2461233..2ebd8f1 100644
--- a/Android.mk
+++ b/Android.mk
@@ -4,7 +4,6 @@ include $(CLEAR_VARS)
LOCAL_MODULE_TAGS := debug
LOCAL_SHARED_LIBRARIES := libstlport \
libnl \
- libpci \
libtraceevnet \
LOCAL_MODULE := powertop
diff --git a/README b/README
index 6e76842..803bc6d 100644
--- a/README
+++ b/README
@@ -20,7 +20,6 @@ development environment.
in addition to that, PowerTOP requires the following components:
-pciutils-devel (is only required if you have PCI)
ncurses-devel (required)
libnl-devel (required)
kernel version => 2.6.38
diff --git a/configure.ac b/configure.ac
index 9af9604..bc16e23 100644
--- a/configure.ac
+++ b/configure.ac
@@ -117,11 +117,6 @@ PKG_CHECK_MODULES([NCURSES], [ncursesw ncurses], [LIBS="$LIBS
$ncurses_LIBS"], [
], [])
])
-has_libpci=0
-PKG_CHECK_MODULES([PCIUTILS], [libpci],[has_libpci=1], [
- AC_SEARCH_LIBS([pci_get_dev], [pci],[has_libpci=1], [has_libpci=0])
-])
-
has_libnl_ver=0
dnl libnl-2 provides only libnl-2.0.pc file, so we check for separate
@@ -138,16 +133,6 @@ AS_IF([test "$has_libnl_ver" -gt 1], [
AC_DEFINE([HAVE_LIBNL20], [1], [Define if you have libnl-2.0 or higher])
])
-AS_IF([test "$has_libpci" -eq 0], [
- AC_DEFINE([HAVE_NO_PCI], [1], [Define if pci is not supported])
- AC_MSG_WARN([
- ************* LIBPCI SUPPORT NOT CONFIGURED**************
- If you need or want pci support, please install libpci
- and re-configure PowerTOP.
- *********************************************************
- ])
-])
-
AC_SEARCH_LIBS([inet_aton], [resolv], [], [
AC_MSG_ERROR([libresolv is required but was not found])
], [])
diff --git a/src/Makefile.am b/src/Makefile.am
index ac2259c..c4326ec 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -133,8 +133,7 @@ powertop_CXXFLAGS = \
$(GLIB2_CFLAGS) \
$(LIBNL_CFLAGS) \
$(NCURSES_CFLAGS) \
- $(UDEV_CFLAGS) \
- $(PCIUTILS_CFLAGS)
+ $(UDEV_CFLAGS)
powertop_CPPFLAGS = \
@@ -143,8 +142,7 @@ powertop_CPPFLAGS = \
$(LIBNL_CFLAGS) \
$(LIBZ_CFLAGS) \
$(NCURSES_CFLAGS) \
- $(UDEV_CFLAGS) \
- $(PCIUTILS_CFLAGS)
+ $(UDEV_CFLAGS)
powertop_LDADD = \
../traceevent/libtraceevnet.la
@@ -155,7 +153,6 @@ AM_LDFLAGS = \
$(LIBZ_LIBS) \
$(NCURSES_LIBS) \
$(UDEV_LIBS) \
- $(PCIUTILS_LIBS) \
$(PTHREAD_LIBS) \
$(RESOLV_LIBS)
diff --git a/src/lib.cpp b/src/lib.cpp
index 399d2bc..d1ea35a 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -37,15 +37,8 @@
#include "lib.h"
-#ifndef HAVE_NO_PCI
-extern "C" {
-#include <pci/pci.h>
-}
-#endif
-
#include <stdio.h>
#include <stdint.h>
-#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/types.h>
@@ -341,44 +334,6 @@ const char *upci_id_to_name(struct udev_device* dev, char* buffer)
return NULL;
}
-#ifndef HAVE_NO_PCI
-static struct pci_access *pci_access;
-
-char *pci_id_to_name(uint16_t vendor, uint16_t device, char *buffer, int len)
-{
- char *ret;
-
- buffer[0] = 0;
-
- if (!pci_access) {
- pci_access = pci_alloc();
- pci_init(pci_access);
- }
-
- ret = pci_lookup_name(pci_access, buffer, len, PCI_LOOKUP_VENDOR | PCI_LOOKUP_DEVICE,
vendor, device);
-
- return ret;
-}
-
-void end_pci_access(void)
-{
- if (pci_access)
- pci_free_name_list(pci_access);
-}
-
-#else
-
-char *pci_id_to_name(uint16_t vendor, uint16_t device, char *buffer, int len)
-{
- return NULL;
-}
-
-void end_pci_access(void)
-{
-}
-
-#endif /* HAVE_NO_PCI */
-
int utf_ok = -1;
diff --git a/src/lib.h b/src/lib.h
index 464ce71..a577e3f 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -69,8 +69,6 @@ extern string read_sysfs_string(const char *format, const char *param);
extern void format_watts(double W, char *buffer, unsigned int len);
extern const char *upci_id_to_name(struct udev_device* dev, char* buffer);
-extern char *pci_id_to_name(uint16_t vendor, uint16_t device, char *buffer, int len);
-extern void end_pci_access(void);
extern struct udev * get_udev();
extern void cleanup_udev();
diff --git a/src/main.cpp b/src/main.cpp
index 65e54ac..1082f0e 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -282,7 +282,6 @@ void make_report(int time, char *workload, int iterations, char
*file)
learn_parameters(50, 0);
save_all_results("saved_results.powertop");
save_parameters("saved_parameters.powertop");
- end_pci_access();
exit(0);
}
@@ -475,7 +474,6 @@ int main(int argc, char **argv)
if (debug_learning) {
learn_parameters(1000, 1);
dump_parameter_bundle();
- end_pci_access();
exit(0);
}
if (!auto_tune)
@@ -512,7 +510,6 @@ int main(int argc, char **argv)
save_parameters("saved_parameters.powertop");
learn_parameters(500, 0);
save_parameters("saved_parameters.powertop");
- end_pci_access();
clear_tuning();
reset_display();
--
2.4.1
2:09 a.m.
New subject: [Powertop] [RFC 9/9] calibrate: use udev
Signed-off-by: Nanley Chery <nanley.g.chery(a)intel.com>
---
src/calibrate/calibrate.cpp | 44 ++++++++++++--------------------------------
1 file changed, 12 insertions(+), 32 deletions(-)
diff --git a/src/calibrate/calibrate.cpp b/src/calibrate/calibrate.cpp
index 98abfe9..107d8d9 100644
--- a/src/calibrate/calibrate.cpp
+++ b/src/calibrate/calibrate.cpp
@@ -88,29 +88,16 @@ static void restore_all_sysfs(void)
static void find_all_usb_callback(const char *d_name)
{
char filename[4096];
- ifstream file;
-
- sprintf(filename, "/sys/bus/usb/devices/%s/power/active_duration", d_name);
- if (access(filename, R_OK) != 0)
- return;
-
- sprintf(filename, "/sys/bus/usb/devices/%s/power/idVendor", d_name);
- file.open(filename, ios::in);
- if (file) {
- file.getline(filename, 4096);
- file.close();
- if (strcmp(filename, "1d6b") == 0)
- return;
- }
-
- sprintf(filename, "/sys/bus/usb/devices/%s/power/control", d_name);
+ sprintf(filename, "%s/power/control", d_name);
save_sysfs(filename);
usb_devices.push_back(filename);
}
static void find_all_usb(void)
{
- process_directory("/sys/bus/usb/devices/", find_all_usb_callback);
+ process_subsystem("usb", find_all_usb_callback, 2,
+ "+power/active_duration", NULL,
+ "-power/idVendor", "1d6b");
}
static void suspend_all_usb_devices(void)
@@ -124,16 +111,14 @@ static void suspend_all_usb_devices(void)
static void find_all_rfkill_callback(const char *d_name)
{
char filename[4096];
- sprintf(filename, "/sys/class/rfkill/%s/soft", d_name);
- if (access(filename, R_OK) != 0)
- return;
+ sprintf(filename, "%s/soft", d_name);
save_sysfs(filename);
rfkill_devices.push_back(filename);
}
static void find_all_rfkill(void)
{
- process_directory("/sys/class/rfkill/", find_all_rfkill_callback);
+ process_subsystem("rfkill", find_all_rfkill_callback, 1, "+soft",
NULL);
}
static void rfkill_all_radios(void)
@@ -154,19 +139,16 @@ static void unrfkill_all_radios(void)
static void find_backlight_callback(const char *d_name)
{
char filename[4096];
- sprintf(filename, "/sys/class/backlight/%s/brightness", d_name);
- if (access(filename, R_OK) != 0)
- return;
-
+ sprintf(filename, "%s/brightness", d_name);
save_sysfs(filename);
backlight_devices.push_back(filename);
- sprintf(filename, "/sys/class/backlight/%s/max_brightness", d_name);
+ sprintf(filename, "%s/max_brightness", d_name);
blmax = read_sysfs(filename);
}
static void find_backlight(void)
{
- process_directory("/sys/class/backlight/", find_backlight_callback);
+ process_subsystem("backlight", find_backlight_callback, 1,
"+brightness", NULL);
}
static void lower_backlight(void)
@@ -180,17 +162,15 @@ static void lower_backlight(void)
static void find_scsi_link_callback(const char *d_name)
{
char filename[4096];
- sprintf(filename, "/sys/class/scsi_host/%s/link_power_management_policy",
d_name);
- if (access(filename, R_OK)!=0)
- return;
-
+ sprintf(filename, "%s/link_power_management_policy", d_name);
save_sysfs(filename);
scsi_link_devices.push_back(filename);
}
static void find_scsi_link(void)
{
- process_directory("/sys/class/scsi_host/", find_scsi_link_callback);
+ process_subsystem("scsi_host", find_scsi_link_callback, 1,
+ "+link_power_management_policy", NULL);
}
static void set_scsi_link(const char *state)
--
2.4.1
2:09 a.m.
New subject: [Powertop] [RFC 0/9] Use udev for accessing sysfs
On 6/1/2015 9:09 AM, Nanley Chery wrote:
From: Nanley Chery <nanley.g.chery(a)intel.com>
This patch series changes PowerTOP's method of accessing sysfs (for
tunables mainly) from explicitly accessing files to using the standard
udev device management library and API.
The current codebase collects data on devices by probing sysfs files
directly with the usual low-level file handling API. This method is
not recommended by the Linux kernel maintainers because the sysfs
organization is not guaranteed to be stable over time. This can lead
to dependent applications breaking with newer kernel releases. Instead
using an abstraction layer, like udev, is suggested
(see https://www.kernel.org/doc/Documentation/sysfs-rules.txt).
.... how does this work on Android ?
6:23 a.m.
New subject: [Powertop] [RFC 0/9] Use udev for accessing sysfs
On Mon, Jun 1, 2015 at 9:09 AM, Arjan van de Ven <arjan(a)linux.intel.com> wrote:
On 6/1/2015 9:09 AM, Nanley Chery wrote:
>
> From: Nanley Chery <nanley.g.chery(a)intel.com>
>
> This patch series changes PowerTOP's method of accessing sysfs (for
> tunables mainly) from explicitly accessing files to using the standard
> udev device management library and API.
>
> The current codebase collects data on devices by probing sysfs files
> directly with the usual low-level file handling API. This method is
> not recommended by the Linux kernel maintainers because the sysfs
> organization is not guaranteed to be stable over time. This can lead
> to dependent applications breaking with newer kernel releases. Instead
> using an abstraction layer, like udev, is suggested
> (see https://www.kernel.org/doc/Documentation/sysfs-rules.txt).
.... how does this work on Android ?
That's a good question. I actually hadn't considered the affects on
that platform. It seems like there is no udev-like abstraction layer
on Android. It looks like the way forward would be to either keep a
separate folder for android specific code or to create more helper
functions to abstract how we access sysfs. The latter method seems
preferable because it wouldn't increase the complexity of the project
as a whole. Thoughts?
- Nanley
6:25 a.m.
New subject: [Powertop] [RFC 0/9] Use udev for accessing sysfs
On 6/3/2015 1:23 PM, Nanley Chery wrote:
>
> .... how does this work on Android ?
That's a good question. I actually hadn't considered the affects on
that platform. It seems like there is no udev-like abstraction layer
on Android. It looks like the way forward would be to either keep a
separate folder for android specific code or to create more helper
functions to abstract how we access sysfs. The latter method seems
preferable because it wouldn't increase the complexity of the project
as a whole. Thoughts?
but if we keep the code anyway, what's the benefit of going through one
more layer of indirection????
Am I missing something here?
7:10 a.m.
New subject: [Powertop] [RFC 0/9] Use udev for accessing sysfs
On Wed, Jun 3, 2015 at 1:25 PM, Arjan van de Ven <arjan(a)linux.intel.com> wrote:
On 6/3/2015 1:23 PM, Nanley Chery wrote:
>>
>> .... how does this work on Android ?
>
>
> That's a good question. I actually hadn't considered the affects on
> that platform. It seems like there is no udev-like abstraction layer
> on Android. It looks like the way forward would be to either keep a
> separate folder for android specific code or to create more helper
> functions to abstract how we access sysfs. The latter method seems
> preferable because it wouldn't increase the complexity of the project
> as a whole. Thoughts?
but if we keep the code anyway, what's the benefit of going through one
more layer of indirection????
Am I missing something here?
The main benefit would be increased protection from sysfs
reorganizations with progressive kernel releases. If this is actually
something we should be proactive about, I am not sure.
Udev should provide faster reading of sysfs files due to caching, but
I do not have any hard numbers to back this claim up.
Lastly there are some general wins in the realm of software
readability and maintainability (in my opinion). This should make the
process of adding more features and fixing bugs easier.
- Nanley
2664
days inactive
2666
days old
15 comments
2 participants
participants (2)
-
Arjan van de Ven -
Nanley Chery