2003-01-13 20:45:06

by Dominik Brodowski

[permalink] [raw]
Subject: [PATCH 2.5.57] cpufreq: add sysfs interface

rediffed for 2.5.57

This patch adds a sysfs interface to the cpufreq core, and marks the
previous /proc/cpufreq interface as deprecated.

As in drivers/base/cpu.c a "CPU driver" is registered, cpufreq acts as
"interface" to this, offering the following files for each CPU
(in /system/devices/sys/cpu.../) where CPUfreq support is present

cpuinfo_min_freq (ro) - minimum frequency (in kHz) the CPU supports
cpuinfo_max_freq (ro) - maximum frequency (in kHz) the CPU supports
scaling_min_freq (rw) - minimum frequency (in kHz) cpufreq may scale
the CPU core to
scaling_max_freq (rw) - maximum frequency (in kHz) cpufreq may scale
the CPU core to
scaling_governor (rw) - governor == "A feedback device on a machine
or engine that is used to provide
automatic control, as of speed,
pressure, or temperature" [1, as noted
by David Kimdon]. Decides what frequency
is used. Currently, only "performance"
and "powersave" are supported, more may
be added later.

(In future, a file scaling_driver (ro) which shows what CPUfreq driver
is used (arm-sa1100, gx-suspmod, speedstep, longrun, powernow-k6,
...) might be added, and this driver will be allowed to add files
scaling_driver_* for driver-specific settings like "prefer fast FSB".
And scaling_governor_* files might offer settings for the governor.)

To implement this sysfs interface, the driver model "interface" code
is used. Unfortunately, it has a non-trivial locking bug in
drivers/base/intf.c: there's a down_write call for
cls->subsys.rwsem in add_intf(), which then calls add(), which may call
intf->add_device(), which may call interface_add_data(), which calls
kobject_register(), which calls kobject_add(), which then tries to
down_write cls->subsys.rwsem. Remember, that was already locked writable
in add_intf().

Because of that, interface_add_data() is commented out; this means
that no link in /system/class/cpu/cpufreq is added, and that the
dev-removal code isn't called. This shouldn't be a problem yet,
though; as no cpufreq driver I know of is capable of CPU hotplugging.

Dominik

[1] http://dictionary.reference.com/search?q=governor

diff -ruN linux-original/arch/i386/Kconfig linux/arch/i386/Kconfig
--- linux-original/arch/i386/Kconfig 2003-01-13 21:15:55.000000000 +0100
+++ linux/arch/i386/Kconfig 2003-01-13 21:16:28.000000000 +0100
@@ -949,15 +949,27 @@

If in doubt, say N.

+config CPU_FREQ_PROC_INTF
+ bool "/proc/cpufreq interface (DEPRECATED)"
+ depends on CPU_FREQ && PROC_FS
+ help
+ This enables the /proc/cpufreq interface for controlling
+ CPUFreq. Please note that it is recommended to use the sysfs
+ interface instead (which is built automatically).
+
+ For details, take a look at linux/Documentation/cpufreq.
+
+ If in doubt, say N.
+
config CPU_FREQ_24_API
bool "/proc/sys/cpu/ interface (2.4. / OLD)"
depends on CPU_FREQ
help
This enables the /proc/sys/cpu/ sysctl interface for controlling
CPUFreq, as known from the 2.4.-kernel patches for CPUFreq. 2.5
- uses /proc/cpufreq instead. Please note that some drivers do not
- work well with the 2.4. /proc/sys/cpu sysctl interface, so if in
- doubt, say N here.
+ uses a sysfs interface instead. Please note that some drivers do
+ not work well with the 2.4. /proc/sys/cpu sysctl interface,
+ so if in doubt, say N here.

For details, take a look at linux/Documentation/cpufreq.

diff -ruN linux-original/include/linux/cpufreq.h linux/include/linux/cpufreq.h
--- linux-original/include/linux/cpufreq.h 2003-01-13 21:08:18.000000000 +0100
+++ linux/include/linux/cpufreq.h 2003-01-13 21:16:28.000000000 +0100
@@ -17,6 +17,7 @@
#include <linux/config.h>
#include <linux/notifier.h>
#include <linux/threads.h>
+#include <linux/device.h>


/*********************************************************************
@@ -57,6 +58,7 @@
unsigned int max; /* in kHz */
unsigned int policy; /* see above */
struct cpufreq_cpuinfo cpuinfo; /* see above */
+ struct intf_data intf; /* interface data */
};

#define CPUFREQ_ADJUST (0)
diff -ruN linux-original/kernel/cpufreq.c linux/kernel/cpufreq.c
--- linux-original/kernel/cpufreq.c 2003-01-13 21:08:21.000000000 +0100
+++ linux/kernel/cpufreq.c 2003-01-13 21:16:28.000000000 +0100
@@ -20,12 +20,16 @@
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/spinlock.h>
+#include <linux/device.h>
+
+#ifdef CONFIG_CPU_FREQ_PROC_INTF
#include <linux/ctype.h>
#include <linux/proc_fs.h>
-
#include <asm/uaccess.h>
+#endif

#ifdef CONFIG_CPU_FREQ_24_API
+#include <linux/proc_fs.h>
#include <linux/sysctl.h>
#endif

@@ -75,11 +79,255 @@
#endif


+/*********************************************************************
+ * SYSFS INTERFACE *
+ *********************************************************************/
+
+/**
+ * cpufreq_parse_governor - parse a governor string
+ */
+static int cpufreq_parse_governor (char *str_governor, unsigned int *governor)
+{
+ if (!strnicmp(str_governor, "performance", 11)) {
+ *governor = CPUFREQ_POLICY_PERFORMANCE;
+ return 0;
+ } else if (!strnicmp(str_governor, "powersave", 9)) {
+ *governor = CPUFREQ_POLICY_POWERSAVE;
+ return 0;
+ } else
+ return -EINVAL;
+}
+
+
+/* forward declarations */
+static int cpufreq_add_dev (struct device * dev);
+static int cpufreq_remove_dev (struct intf_data * dev);
+
+/* drivers/base/cpu.c */
+extern struct device_class cpu_devclass;
+
+static struct device_interface cpufreq_interface = {
+ .name = "cpufreq",
+ .devclass = &cpu_devclass,
+ .add_device = &cpufreq_add_dev,
+ .remove_device = &cpufreq_remove_dev,
+ .kset = { .subsys = &cpu_devclass.subsys, },
+ .devnum = 0,
+};
+
+static inline int to_cpu_nr (struct device *dev)
+{
+ struct sys_device * cpu_sys_dev = container_of(dev, struct sys_device, dev);
+ return (cpu_sys_dev->id);
+}
+
+
+/**
+ * cpufreq_per_cpu_attr_read() / show_##file_name() - print out cpufreq information
+ *
+ * Write out information from cpufreq_driver->policy[cpu]; object must be
+ * "unsigned int".
+ */
+
+#define cpufreq_per_cpu_attr_read(file_name, object) \
+static ssize_t show_##file_name \
+(struct device *dev, char *buf) \
+{ \
+ unsigned int value = 0; \
+ \
+ if (!dev) \
+ return 0; \
+ \
+ down(&cpufreq_driver_sem); \
+ if (cpufreq_driver) \
+ value = cpufreq_driver->policy[to_cpu_nr(dev)].object; \
+ up(&cpufreq_driver_sem); \
+ \
+ return sprintf (buf, "%u\n", value); \
+}
+
+
+/**
+ * cpufreq_per_cpu_attr_write() / store_##file_name() - sysfs write access
+ */
+#define cpufreq_per_cpu_attr_write(file_name, object) \
+static ssize_t store_##file_name \
+(struct device *dev, const char *buf) \
+{ \
+ unsigned int ret = -EINVAL; \
+ struct cpufreq_policy policy; \
+ \
+ if (!dev) \
+ return 0; \
+ \
+ ret = cpufreq_get_policy(&policy, to_cpu_nr(dev)); \
+ if (ret) \
+ return ret; \
+ \
+ ret = sscanf (buf, "%u", &policy.object); \
+ if (ret != 1) \
+ return -EINVAL; \
+ \
+ ret = cpufreq_set_policy(&policy); \
+ if (ret) \
+ return ret; \
+ \
+ return strlen(buf); \
+}
+
+
+/**
+ * show_scaling_governor - show the current policy for the specified CPU
+ */
+static ssize_t show_scaling_governor (struct device *dev, char *buf)
+{
+ unsigned int value = 0;
+
+ if (!dev)
+ return 0;
+
+ down(&cpufreq_driver_sem);
+ if (cpufreq_driver)
+ value = cpufreq_driver->policy[to_cpu_nr(dev)].policy;
+ up(&cpufreq_driver_sem);
+
+ switch (value) {
+ case CPUFREQ_POLICY_POWERSAVE:
+ return sprintf(buf, "powersave\n");
+ case CPUFREQ_POLICY_PERFORMANCE:
+ return sprintf(buf, "performance\n");
+ }
+
+ return -EINVAL;
+}
+
+
+/**
+ * store_scaling_governor - store policy for the specified CPU
+ */
+static ssize_t store_scaling_governor (struct device *dev, const char *buf)
+{
+ unsigned int ret = -EINVAL;
+ char str_governor[16];
+ struct cpufreq_policy policy;
+
+ if (!dev)
+ return 0;
+
+ ret = cpufreq_get_policy(&policy, to_cpu_nr(dev));
+ if (ret)
+ return ret;
+
+ ret = sscanf (buf, "%15s", str_governor);
+ if (ret != 1)
+ return -EINVAL;
+
+ if (cpufreq_parse_governor(str_governor, &policy.policy))
+ return -EINVAL;
+
+ ret = cpufreq_set_policy(&policy);
+ if (ret)
+ return ret;
+
+ return strlen(buf);
+}
+
+
+/**
+ * cpufreq_per_cpu_attr_ro - read-only cpufreq per-CPU file
+ */
+#define cpufreq_per_cpu_attr_ro(file_name, object) \
+cpufreq_per_cpu_attr_read(file_name, object) \
+static DEVICE_ATTR(file_name, S_IRUGO, show_##file_name, NULL);
+
+
+/**
+ * cpufreq_per_cpu_attr_rw - read-write cpufreq per-CPU file
+ */
+#define cpufreq_per_cpu_attr_rw(file_name, object) \
+cpufreq_per_cpu_attr_read(file_name, object) \
+cpufreq_per_cpu_attr_write(file_name, object) \
+static DEVICE_ATTR(file_name, (S_IRUGO | S_IWUSR), show_##file_name, store_##file_name);
+
+
+/* create the file functions */
+cpufreq_per_cpu_attr_ro(cpuinfo_min_freq, cpuinfo.min_freq);
+cpufreq_per_cpu_attr_ro(cpuinfo_max_freq, cpuinfo.max_freq);
+cpufreq_per_cpu_attr_rw(scaling_min_freq, min);
+cpufreq_per_cpu_attr_rw(scaling_max_freq, max);
+
+static DEVICE_ATTR(scaling_governor, (S_IRUGO | S_IWUSR), show_scaling_governor, store_scaling_governor);
+
+
+/**
+ * cpufreq_add_dev - add a CPU device
+ *
+ * Adds the cpufreq interface for a CPU device.
+ */
+static int cpufreq_add_dev (struct device * dev)
+{
+ unsigned int cpu = to_cpu_nr(dev);
+ int ret = 0;
+
+ down(&cpufreq_driver_sem);
+ if (!cpufreq_driver) {
+ up(&cpufreq_driver_sem);
+ return -EINVAL;
+ }
+
+ /* prepare interface data */
+ cpufreq_driver->policy[cpu].intf.dev = dev;
+ cpufreq_driver->policy[cpu].intf.intf = &cpufreq_interface;
+ strncpy(cpufreq_driver->policy[cpu].intf.kobj.name, cpufreq_interface.name, KOBJ_NAME_LEN);
+ cpufreq_driver->policy[cpu].intf.kobj.parent = &(dev->kobj);
+ cpufreq_driver->policy[cpu].intf.kobj.kset = &(cpufreq_interface.kset);
+
+ /* add interface */
+ /* currently commented out due to deadlock */
+ //ret = interface_add_data(&(cpufreq_driver->policy[cpu].intf));
+ if (ret) {
+ up(&cpufreq_driver_sem);
+ return ret;
+ }
+
+ /* create files */
+ device_create_file (dev, &dev_attr_cpuinfo_min_freq);
+ device_create_file (dev, &dev_attr_cpuinfo_max_freq);
+ device_create_file (dev, &dev_attr_scaling_min_freq);
+ device_create_file (dev, &dev_attr_scaling_max_freq);
+ device_create_file (dev, &dev_attr_scaling_governor);
+
+ up(&cpufreq_driver_sem);
+ return ret;
+}
+
+
+/**
+ * cpufreq_remove_dev - remove a CPU device
+ *
+ * Removes the cpufreq interface for a CPU device. Is called with
+ * cpufreq_driver_sem locked.
+ */
+static int cpufreq_remove_dev (struct intf_data *intf)
+{
+ struct device * dev = intf->dev;
+
+ device_remove_file (dev, &dev_attr_cpuinfo_min_freq);
+ device_remove_file (dev, &dev_attr_cpuinfo_max_freq);
+ device_remove_file (dev, &dev_attr_scaling_min_freq);
+ device_remove_file (dev, &dev_attr_scaling_max_freq);
+ device_remove_file (dev, &dev_attr_scaling_governor);
+
+ return 0;
+}
+

/*********************************************************************
- * 2.6. API *
+ * /proc/cpufreq INTERFACE *
*********************************************************************/

+#ifdef CONFIG_CPU_FREQ_PROC_INTF
+
/**
* cpufreq_parse_policy - parse a policy string
* @input_string: the string to parse.
@@ -95,10 +343,9 @@
unsigned int min = 0;
unsigned int max = 0;
unsigned int cpu = 0;
- char policy_string[42] = {'\0'};
+ char str_governor[16];
struct cpufreq_policy current_policy;
unsigned int result = -EFAULT;
- unsigned int i = 0;

if (!policy)
return -EINVAL;
@@ -108,7 +355,7 @@
policy->policy = 0;
policy->cpu = CPUFREQ_ALL_CPUS;

- if (sscanf(input_string, "%d:%d:%d:%s", &cpu, &min, &max, policy_string) == 4)
+ if (sscanf(input_string, "%d:%d:%d:%15s", &cpu, &min, &max, str_governor) == 4)
{
policy->min = min;
policy->max = max;
@@ -116,7 +363,7 @@
result = 0;
goto scan_policy;
}
- if (sscanf(input_string, "%d%%%d%%%d%%%s", &cpu, &min, &max, policy_string) == 4)
+ if (sscanf(input_string, "%d%%%d%%%d%%%15s", &cpu, &min, &max, str_governor) == 4)
{
if (!cpufreq_get_policy(&current_policy, cpu)) {
policy->min = (min * current_policy.cpuinfo.max_freq) / 100;
@@ -127,7 +374,7 @@
}
}

- if (sscanf(input_string, "%d:%d:%s", &min, &max, policy_string) == 3)
+ if (sscanf(input_string, "%d:%d:%15s", &min, &max, str_governor) == 3)
{
policy->min = min;
policy->max = max;
@@ -135,7 +382,7 @@
goto scan_policy;
}

- if (sscanf(input_string, "%d%%%d%%%s", &min, &max, policy_string) == 3)
+ if (sscanf(input_string, "%d%%%d%%%15s", &min, &max, str_governor) == 3)
{
if (!cpufreq_get_policy(&current_policy, cpu)) {
policy->min = (min * current_policy.cpuinfo.max_freq) / 100;
@@ -148,36 +395,7 @@
return -EINVAL;

scan_policy:
-
- for (i=0;i<sizeof(policy_string);i++){
- if (policy_string[i]=='\0')
- break;
- policy_string[i] = tolower(policy_string[i]);
- }
-
- if (!strncmp(policy_string, "powersave", 6) ||
- !strncmp(policy_string, "eco", 3) ||
- !strncmp(policy_string, "batter", 6) ||
- !strncmp(policy_string, "low", 3))
- {
- result = 0;
- policy->policy = CPUFREQ_POLICY_POWERSAVE;
- }
- else if (!strncmp(policy_string, "performance",6) ||
- !strncmp(policy_string, "high",4) ||
- !strncmp(policy_string, "full",4))
- {
- result = 0;
- policy->policy = CPUFREQ_POLICY_PERFORMANCE;
- }
- else if (!cpufreq_get_policy(&current_policy, policy->cpu))
- {
- policy->policy = current_policy.policy;
- }
- else
- {
- policy->policy = 0;
- }
+ result = cpufreq_parse_governor(str_governor, &policy->policy);

return result;
}
@@ -197,8 +415,6 @@
__setup("cpufreq=", cpufreq_setup);


-#ifdef CONFIG_PROC_FS
-
/**
* cpufreq_proc_read - read /proc/cpufreq
*
@@ -345,12 +561,15 @@
remove_proc_entry("cpufreq", &proc_root);
return;
}
-#endif /* CONFIG_PROC_FS */
+#else
+#define cpufreq_proc_init() do {} while(0)
+#define cpufreq_proc_exit() do {} while(0)
+#endif /* CONFIG_CPU_FREQ_PROC_INTF */



/*********************************************************************
- * 2.4. COMPATIBLE API *
+ * /proc/sys/cpu/ INTERFACE *
*********************************************************************/

#ifdef CONFIG_CPU_FREQ_24_API
@@ -1055,7 +1274,9 @@
cpufreq_sysctl_init();
#endif

- return 0;
+ ret = interface_register(&cpufreq_interface);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(cpufreq_register);

@@ -1077,6 +1298,7 @@
return -EINVAL;
}

+ interface_unregister(&cpufreq_interface);
cpufreq_driver = NULL;

up(&cpufreq_driver_sem);


2003-01-13 21:17:28

by Patrick Mochel

[permalink] [raw]
Subject: Re: [PATCH 2.5.57] cpufreq: add sysfs interface


On Mon, 13 Jan 2003, Dominik Brodowski wrote:

> rediffed for 2.5.57
>
> This patch adds a sysfs interface to the cpufreq core, and marks the
> previous /proc/cpufreq interface as deprecated.

The following updates the patch to reflect the sysfs changes currently in
Linus's BK tree (reinstating the count parameter to sysfs store()
methods).


-pat

===== arch/i386/Kconfig 1.30 vs edited =====
--- 1.30/arch/i386/Kconfig Mon Jan 13 11:08:12 2003
+++ edited/arch/i386/Kconfig Mon Jan 13 14:15:39 2003
@@ -949,15 +949,27 @@

If in doubt, say N.

+config CPU_FREQ_PROC_INTF
+ bool "/proc/cpufreq interface (DEPRECATED)"
+ depends on CPU_FREQ && PROC_FS
+ help
+ This enables the /proc/cpufreq interface for controlling
+ CPUFreq. Please note that it is recommended to use the sysfs
+ interface instead (which is built automatically).
+
+ For details, take a look at linux/Documentation/cpufreq.
+
+ If in doubt, say N.
+
config CPU_FREQ_24_API
bool "/proc/sys/cpu/ interface (2.4. / OLD)"
depends on CPU_FREQ
help
This enables the /proc/sys/cpu/ sysctl interface for controlling
CPUFreq, as known from the 2.4.-kernel patches for CPUFreq. 2.5
- uses /proc/cpufreq instead. Please note that some drivers do not
- work well with the 2.4. /proc/sys/cpu sysctl interface, so if in
- doubt, say N here.
+ uses a sysfs interface instead. Please note that some drivers do
+ not work well with the 2.4. /proc/sys/cpu sysctl interface,
+ so if in doubt, say N here.

For details, take a look at linux/Documentation/cpufreq.

===== include/linux/cpufreq.h 1.9 vs edited =====
--- 1.9/include/linux/cpufreq.h Fri Jan 10 15:16:10 2003
+++ edited/include/linux/cpufreq.h Mon Jan 13 14:15:39 2003
@@ -17,6 +17,7 @@
#include <linux/config.h>
#include <linux/notifier.h>
#include <linux/threads.h>
+#include <linux/device.h>


/*********************************************************************
@@ -57,6 +58,7 @@
unsigned int max; /* in kHz */
unsigned int policy; /* see above */
struct cpufreq_cpuinfo cpuinfo; /* see above */
+ struct intf_data intf; /* interface data */
};

#define CPUFREQ_ADJUST (0)
===== kernel/cpufreq.c 1.10 vs edited =====
--- 1.10/kernel/cpufreq.c Fri Jan 10 15:16:10 2003
+++ edited/kernel/cpufreq.c Mon Jan 13 14:25:01 2003
@@ -20,12 +20,16 @@
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/spinlock.h>
+#include <linux/device.h>
+
+#ifdef CONFIG_CPU_FREQ_PROC_INTF
#include <linux/ctype.h>
#include <linux/proc_fs.h>
-
#include <asm/uaccess.h>
+#endif

#ifdef CONFIG_CPU_FREQ_24_API
+#include <linux/proc_fs.h>
#include <linux/sysctl.h>
#endif

@@ -75,11 +79,256 @@
#endif


+/*********************************************************************
+ * SYSFS INTERFACE *
+ *********************************************************************/
+
+/**
+ * cpufreq_parse_governor - parse a governor string
+ */
+static int cpufreq_parse_governor (char *str_governor, unsigned int *governor)
+{
+ if (!strnicmp(str_governor, "performance", 11)) {
+ *governor = CPUFREQ_POLICY_PERFORMANCE;
+ return 0;
+ } else if (!strnicmp(str_governor, "powersave", 9)) {
+ *governor = CPUFREQ_POLICY_POWERSAVE;
+ return 0;
+ } else
+ return -EINVAL;
+}
+
+
+/* forward declarations */
+static int cpufreq_add_dev (struct device * dev);
+static int cpufreq_remove_dev (struct intf_data * dev);
+
+/* drivers/base/cpu.c */
+extern struct device_class cpu_devclass;
+
+static struct device_interface cpufreq_interface = {
+ .name = "cpufreq",
+ .devclass = &cpu_devclass,
+ .add_device = &cpufreq_add_dev,
+ .remove_device = &cpufreq_remove_dev,
+ .kset = { .subsys = &cpu_devclass.subsys, },
+ .devnum = 0,
+};
+
+static inline int to_cpu_nr (struct device *dev)
+{
+ struct sys_device * cpu_sys_dev = container_of(dev, struct sys_device, dev);
+ return (cpu_sys_dev->id);
+}
+
+
+/**
+ * cpufreq_per_cpu_attr_read() / show_##file_name() - print out cpufreq information
+ *
+ * Write out information from cpufreq_driver->policy[cpu]; object must be
+ * "unsigned int".
+ */
+
+#define cpufreq_per_cpu_attr_read(file_name, object) \
+static ssize_t show_##file_name \
+(struct device *dev, char *buf) \
+{ \
+ unsigned int value = 0; \
+ \
+ if (!dev) \
+ return 0; \
+ \
+ down(&cpufreq_driver_sem); \
+ if (cpufreq_driver) \
+ value = cpufreq_driver->policy[to_cpu_nr(dev)].object; \
+ up(&cpufreq_driver_sem); \
+ \
+ return sprintf (buf, "%u\n", value); \
+}
+
+
+/**
+ * cpufreq_per_cpu_attr_write() / store_##file_name() - sysfs write access
+ */
+#define cpufreq_per_cpu_attr_write(file_name, object) \
+static ssize_t store_##file_name \
+(struct device *dev, const char *buf, size_t count) \
+{ \
+ unsigned int ret = -EINVAL; \
+ struct cpufreq_policy policy; \
+ \
+ if (!dev) \
+ return 0; \
+ \
+ ret = cpufreq_get_policy(&policy, to_cpu_nr(dev)); \
+ if (ret) \
+ return ret; \
+ \
+ ret = sscanf (buf, "%u", &policy.object); \
+ if (ret != 1) \
+ return -EINVAL; \
+ \
+ ret = cpufreq_set_policy(&policy); \
+ if (ret) \
+ return ret; \
+ \
+ return count; \
+}
+
+
+/**
+ * show_scaling_governor - show the current policy for the specified CPU
+ */
+static ssize_t show_scaling_governor (struct device *dev, char *buf)
+{
+ unsigned int value = 0;
+
+ if (!dev)
+ return 0;
+
+ down(&cpufreq_driver_sem);
+ if (cpufreq_driver)
+ value = cpufreq_driver->policy[to_cpu_nr(dev)].policy;
+ up(&cpufreq_driver_sem);
+
+ switch (value) {
+ case CPUFREQ_POLICY_POWERSAVE:
+ return sprintf(buf, "powersave\n");
+ case CPUFREQ_POLICY_PERFORMANCE:
+ return sprintf(buf, "performance\n");
+ }
+
+ return -EINVAL;
+}
+
+
+/**
+ * store_scaling_governor - store policy for the specified CPU
+ */
+static ssize_t
+store_scaling_governor (struct device *dev, const char *buf, size_t count)
+{
+ unsigned int ret = -EINVAL;
+ char str_governor[16];
+ struct cpufreq_policy policy;
+
+ if (!dev)
+ return 0;
+
+ ret = cpufreq_get_policy(&policy, to_cpu_nr(dev));
+ if (ret)
+ return ret;
+
+ ret = sscanf (buf, "%15s", str_governor);
+ if (ret != 1)
+ return -EINVAL;
+
+ if (cpufreq_parse_governor(str_governor, &policy.policy))
+ return -EINVAL;
+
+ ret = cpufreq_set_policy(&policy);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+
+/**
+ * cpufreq_per_cpu_attr_ro - read-only cpufreq per-CPU file
+ */
+#define cpufreq_per_cpu_attr_ro(file_name, object) \
+cpufreq_per_cpu_attr_read(file_name, object) \
+static DEVICE_ATTR(file_name, S_IRUGO, show_##file_name, NULL);
+
+
+/**
+ * cpufreq_per_cpu_attr_rw - read-write cpufreq per-CPU file
+ */
+#define cpufreq_per_cpu_attr_rw(file_name, object) \
+cpufreq_per_cpu_attr_read(file_name, object) \
+cpufreq_per_cpu_attr_write(file_name, object) \
+static DEVICE_ATTR(file_name, (S_IRUGO | S_IWUSR), show_##file_name, store_##file_name);
+
+
+/* create the file functions */
+cpufreq_per_cpu_attr_ro(cpuinfo_min_freq, cpuinfo.min_freq);
+cpufreq_per_cpu_attr_ro(cpuinfo_max_freq, cpuinfo.max_freq);
+cpufreq_per_cpu_attr_rw(scaling_min_freq, min);
+cpufreq_per_cpu_attr_rw(scaling_max_freq, max);
+
+static DEVICE_ATTR(scaling_governor, (S_IRUGO | S_IWUSR), show_scaling_governor, store_scaling_governor);
+
+
+/**
+ * cpufreq_add_dev - add a CPU device
+ *
+ * Adds the cpufreq interface for a CPU device.
+ */
+static int cpufreq_add_dev (struct device * dev)
+{
+ unsigned int cpu = to_cpu_nr(dev);
+ int ret = 0;
+
+ down(&cpufreq_driver_sem);
+ if (!cpufreq_driver) {
+ up(&cpufreq_driver_sem);
+ return -EINVAL;
+ }
+
+ /* prepare interface data */
+ cpufreq_driver->policy[cpu].intf.dev = dev;
+ cpufreq_driver->policy[cpu].intf.intf = &cpufreq_interface;
+ strncpy(cpufreq_driver->policy[cpu].intf.kobj.name, cpufreq_interface.name, KOBJ_NAME_LEN);
+ cpufreq_driver->policy[cpu].intf.kobj.parent = &(dev->kobj);
+ cpufreq_driver->policy[cpu].intf.kobj.kset = &(cpufreq_interface.kset);
+
+ /* add interface */
+ /* currently commented out due to deadlock */
+ //ret = interface_add_data(&(cpufreq_driver->policy[cpu].intf));
+ if (ret) {
+ up(&cpufreq_driver_sem);
+ return ret;
+ }
+
+ /* create files */
+ device_create_file (dev, &dev_attr_cpuinfo_min_freq);
+ device_create_file (dev, &dev_attr_cpuinfo_max_freq);
+ device_create_file (dev, &dev_attr_scaling_min_freq);
+ device_create_file (dev, &dev_attr_scaling_max_freq);
+ device_create_file (dev, &dev_attr_scaling_governor);
+
+ up(&cpufreq_driver_sem);
+ return ret;
+}
+
+
+/**
+ * cpufreq_remove_dev - remove a CPU device
+ *
+ * Removes the cpufreq interface for a CPU device. Is called with
+ * cpufreq_driver_sem locked.
+ */
+static int cpufreq_remove_dev (struct intf_data *intf)
+{
+ struct device * dev = intf->dev;
+
+ device_remove_file (dev, &dev_attr_cpuinfo_min_freq);
+ device_remove_file (dev, &dev_attr_cpuinfo_max_freq);
+ device_remove_file (dev, &dev_attr_scaling_min_freq);
+ device_remove_file (dev, &dev_attr_scaling_max_freq);
+ device_remove_file (dev, &dev_attr_scaling_governor);
+
+ return 0;
+}
+

/*********************************************************************
- * 2.6. API *
+ * /proc/cpufreq INTERFACE *
*********************************************************************/

+#ifdef CONFIG_CPU_FREQ_PROC_INTF
+
/**
* cpufreq_parse_policy - parse a policy string
* @input_string: the string to parse.
@@ -95,10 +344,9 @@
unsigned int min = 0;
unsigned int max = 0;
unsigned int cpu = 0;
- char policy_string[42] = {'\0'};
+ char str_governor[16];
struct cpufreq_policy current_policy;
unsigned int result = -EFAULT;
- unsigned int i = 0;

if (!policy)
return -EINVAL;
@@ -108,7 +356,7 @@
policy->policy = 0;
policy->cpu = CPUFREQ_ALL_CPUS;

- if (sscanf(input_string, "%d:%d:%d:%s", &cpu, &min, &max, policy_string) == 4)
+ if (sscanf(input_string, "%d:%d:%d:%15s", &cpu, &min, &max, str_governor) == 4)
{
policy->min = min;
policy->max = max;
@@ -116,7 +364,7 @@
result = 0;
goto scan_policy;
}
- if (sscanf(input_string, "%d%%%d%%%d%%%s", &cpu, &min, &max, policy_string) == 4)
+ if (sscanf(input_string, "%d%%%d%%%d%%%15s", &cpu, &min, &max, str_governor) == 4)
{
if (!cpufreq_get_policy(&current_policy, cpu)) {
policy->min = (min * current_policy.cpuinfo.max_freq) / 100;
@@ -127,7 +375,7 @@
}
}

- if (sscanf(input_string, "%d:%d:%s", &min, &max, policy_string) == 3)
+ if (sscanf(input_string, "%d:%d:%15s", &min, &max, str_governor) == 3)
{
policy->min = min;
policy->max = max;
@@ -135,7 +383,7 @@
goto scan_policy;
}

- if (sscanf(input_string, "%d%%%d%%%s", &min, &max, policy_string) == 3)
+ if (sscanf(input_string, "%d%%%d%%%15s", &min, &max, str_governor) == 3)
{
if (!cpufreq_get_policy(&current_policy, cpu)) {
policy->min = (min * current_policy.cpuinfo.max_freq) / 100;
@@ -148,36 +396,7 @@
return -EINVAL;

scan_policy:
-
- for (i=0;i<sizeof(policy_string);i++){
- if (policy_string[i]=='\0')
- break;
- policy_string[i] = tolower(policy_string[i]);
- }
-
- if (!strncmp(policy_string, "powersave", 6) ||
- !strncmp(policy_string, "eco", 3) ||
- !strncmp(policy_string, "batter", 6) ||
- !strncmp(policy_string, "low", 3))
- {
- result = 0;
- policy->policy = CPUFREQ_POLICY_POWERSAVE;
- }
- else if (!strncmp(policy_string, "performance",6) ||
- !strncmp(policy_string, "high",4) ||
- !strncmp(policy_string, "full",4))
- {
- result = 0;
- policy->policy = CPUFREQ_POLICY_PERFORMANCE;
- }
- else if (!cpufreq_get_policy(&current_policy, policy->cpu))
- {
- policy->policy = current_policy.policy;
- }
- else
- {
- policy->policy = 0;
- }
+ result = cpufreq_parse_governor(str_governor, &policy->policy);

return result;
}
@@ -197,8 +416,6 @@
__setup("cpufreq=", cpufreq_setup);


-#ifdef CONFIG_PROC_FS
-
/**
* cpufreq_proc_read - read /proc/cpufreq
*
@@ -345,12 +562,15 @@
remove_proc_entry("cpufreq", &proc_root);
return;
}
-#endif /* CONFIG_PROC_FS */
+#else
+#define cpufreq_proc_init() do {} while(0)
+#define cpufreq_proc_exit() do {} while(0)
+#endif /* CONFIG_CPU_FREQ_PROC_INTF */



/*********************************************************************
- * 2.4. COMPATIBLE API *
+ * /proc/sys/cpu/ INTERFACE *
*********************************************************************/

#ifdef CONFIG_CPU_FREQ_24_API
@@ -1055,7 +1275,9 @@
cpufreq_sysctl_init();
#endif

- return 0;
+ ret = interface_register(&cpufreq_interface);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(cpufreq_register);

@@ -1077,6 +1299,7 @@
return -EINVAL;
}

+ interface_unregister(&cpufreq_interface);
cpufreq_driver = NULL;

up(&cpufreq_driver_sem);

2003-01-16 21:43:43

by Martin Waitz

[permalink] [raw]
Subject: [PATCH] work around deadlock in add_intf

hi :)


the note for cpufreq sysfs support regarding locking in add_intf
has bitten me, too

i don't have a really good idea how to change the locking
so that it just works, so i wrote a workaround

the following patch allows to add interfaces again for me,
it just deferres the actual device additions:


--- src/linux-2.5/drivers/base/intf.c Mon Jan 13 09:49:02 2003
+++ src/linux-rc/drivers/base/intf.c Thu Jan 16 11:13:19 2003
@@ -132,6 +132,22 @@


/**
+ * delayed_add - version of add() called via schedule_work
+ * @_data: pointer to arguments
+ */
+static void delayed_add(void *_data)
+{
+ struct {
+ struct device_interface *intf;
+ struct device *dev;
+ struct work_struct work;
+ } *data = _data;
+
+ add(data->intf, data->dev);
+ kfree(data);
+}
+
+/**
* add_intf - add class's devices to interface.
* @intf: interface.
*
@@ -142,12 +158,24 @@
*/
static void add_intf(struct device_interface * intf)
{
+ struct {
+ struct device_interface *intf;
+ struct device *dev;
+ struct work_struct work;
+ } *data;
struct device_class * cls = intf->devclass;
struct list_head * entry;

down_write(&cls->subsys.rwsem);
- list_for_each(entry,&cls->devices.list)
- add(intf,to_dev(entry));
+ list_for_each(entry,&cls->devices.list) {
+ /* add will lock subsys.rwsem via interface_add_data, */
+ /* do it after lock is released */
+ data = kmalloc(sizeof(*data), GFP_KERNEL);
+ INIT_WORK(&data->work, delayed_add, data);
+ data->intf = intf;
+ data->dev = to_dev(entry);
+ schedule_work(&data->work);
+ }
up_write(&cls->subsys.rwsem);
}



--
CU, / Friedrich-Alexander University Erlangen, Germany
Martin Waitz // [Tali on IRCnet] [tali.home.pages.de] _________
______________/// - - - - - - - - - - - - - - - - - - - - ///
dies ist eine manuell generierte mail, sie beinhaltet //
tippfehler und ist auch ohne grossbuchstaben gueltig. /
-
Wer bereit ist, grundlegende Freiheiten aufzugeben, um sich
kurzfristige Sicherheit zu verschaffen, der hat weder Freiheit
noch Sicherheit verdient. Benjamin Franklin (1706 - 1790)


Attachments:
(No filename) (2.32 kB)
(No filename) (189.00 B)
Download all attachments