Re: [PATCH V2 4/4] cpufreq: Get rid of "struct global_attr"
On 22 February 2013 08:03, Rafael J. Wysocki wrote: > On Friday, February 22, 2013 07:47:44 AM Viresh Kumar wrote: >> On 22 February 2013 05:15, Rafael J. Wysocki wrote: >> > Why did you change all of the lines of this macro instead of changing just >> > the >> > one line you needed to change? >> >> I didn't like the indentation used within the macro. So did it. > > In general, things like that are for separate cleanup patches. If you mix > functional changes with cleanups, poeple get confused and it's difficult to > see > what's needed and what's "optional". > > I know it's tempting to fix stuff like that along with doing functional > changes and I do that sometimes. Not very often, though, and with care. Even i give similar comments sometimes but forget these while writing my patches :) Anyway, fixup: commit b1bbb99467d56140cf3a8a2b70e61b456aa46e48 Author: Viresh Kumar Date: Fri Feb 22 07:59:20 2013 +0530 fixup! cpufreq: Get rid of "struct global_attr" --- drivers/cpufreq/intel_pstate.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index e795134..49846b9 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -273,12 +273,12 @@ static void intel_pstate_debug_expose_params(void) /** debugfs end / /** sysfs begin / -#define show_one(file_name, object)\ -static ssize_t show_##file_name\ -(struct cpufreq_policy *policy, char *buf) \ -{ \ - return sprintf(buf, "%u\n", limits.object); \ -} +#define show_one(file_name, object)\ + static ssize_t show_##file_name \ + (struct cpufreq_policy *policy, char *buf) \ + { \ + return sprintf(buf, "%u\n", limits.object); \ + } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 4/4] cpufreq: Get rid of "struct global_attr"
On Friday, February 22, 2013 07:47:44 AM Viresh Kumar wrote: > On 22 February 2013 05:15, Rafael J. Wysocki wrote: > > Why did you change all of the lines of this macro instead of changing just > > the > > one line you needed to change? > > I didn't like the indentation used within the macro. So did it. In general, things like that are for separate cleanup patches. If you mix functional changes with cleanups, poeple get confused and it's difficult to see what's needed and what's "optional". I know it's tempting to fix stuff like that along with doing functional changes and I do that sometimes. Not very often, though, and with care. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 4/4] cpufreq: Get rid of "struct global_attr"
On 22 February 2013 05:15, Rafael J. Wysocki wrote: > Why did you change all of the lines of this macro instead of changing just the > one line you needed to change? I didn't like the indentation used within the macro. So did it. > Please don't do that. Okay. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 4/4] cpufreq: Get rid of "struct global_attr"
On Monday, February 11, 2013 01:20:03 PM Viresh Kumar wrote: > We don't need to keep two structures for file attributes, global_attr and > freq_attr. Lets use freq_attr only for cpufreq core and drivers. > > Signed-off-by: Viresh Kumar > --- > drivers/cpufreq/acpi-cpufreq.c | 9 - > drivers/cpufreq/intel_pstate.c | 30 +++--- > include/linux/cpufreq.h| 16 > 3 files changed, 19 insertions(+), 36 deletions(-) > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > index 937bc28..14219c3 100644 > --- a/drivers/cpufreq/acpi-cpufreq.c > +++ b/drivers/cpufreq/acpi-cpufreq.c > @@ -160,19 +160,18 @@ static ssize_t _store_boost(const char *buf, size_t > count) > return count; > } > > -static ssize_t store_global_boost(struct kobject *kobj, struct attribute > *attr, > - const char *buf, size_t count) > +static ssize_t store_global_boost(struct cpufreq_policy *policy, > + const char *buf, size_t count) > { > return _store_boost(buf, count); > } > > -static ssize_t show_global_boost(struct kobject *kobj, > - struct attribute *attr, char *buf) > +static ssize_t show_global_boost(struct cpufreq_policy *policy, char *buf) > { > return sprintf(buf, "%u\n", boost_enabled); > } > > -static struct global_attr global_boost = __ATTR(boost, 0644, > +static struct freq_attr global_boost = __ATTR(boost, 0644, > show_global_boost, > store_global_boost); > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c > index 86ad482..0f2d6a0 100644 > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -273,15 +273,15 @@ static void intel_pstate_debug_expose_params(void) > /** debugfs end / > > /** sysfs begin / > -#define show_one(file_name, object) \ > - static ssize_t show_##file_name \ > - (struct kobject *kobj, struct attribute *attr, char *buf) \ > - { \ > - return sprintf(buf, "%u\n", limits.object); \ > - } > +#define show_one(file_name, object) \ > +static ssize_t show_##file_name \ > +(struct cpufreq_policy *policy, char *buf) \ > +{\ > + return sprintf(buf, "%u\n", limits.object); \ > +} Why did you change all of the lines of this macro instead of changing just the one line you needed to change? Please don't do that. > -static ssize_t store_no_turbo(struct kobject *a, struct attribute *b, > - const char *buf, size_t count) > +static ssize_t store_no_turbo(struct cpufreq_policy *policy, const char *buf, > + size_t count) > { > unsigned int input; > int ret; > @@ -293,8 +293,8 @@ static ssize_t store_no_turbo(struct kobject *a, struct > attribute *b, > return count; > } > > -static ssize_t store_max_perf_pct(struct kobject *a, struct attribute *b, > - const char *buf, size_t count) > +static ssize_t store_max_perf_pct(struct cpufreq_policy *policy, > + const char *buf, size_t count) > { > unsigned int input; > int ret; > @@ -307,8 +307,8 @@ static ssize_t store_max_perf_pct(struct kobject *a, > struct attribute *b, > return count; > } > > -static ssize_t store_min_perf_pct(struct kobject *a, struct attribute *b, > - const char *buf, size_t count) > +static ssize_t store_min_perf_pct(struct cpufreq_policy *policy, > + const char *buf, size_t count) > { > unsigned int input; > int ret; > @@ -325,9 +325,9 @@ show_one(no_turbo, no_turbo); > show_one(max_perf_pct, max_perf_pct); > show_one(min_perf_pct, min_perf_pct); > > -define_one_global_rw(no_turbo); > -define_one_global_rw(max_perf_pct); > -define_one_global_rw(min_perf_pct); > +cpufreq_freq_attr_rw(no_turbo); > +cpufreq_freq_attr_rw(max_perf_pct); > +cpufreq_freq_attr_rw(min_perf_pct); > > static struct attribute *intel_pstate_attributes[] = { > _turbo.attr, > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index 0d84bfa..a092fcb 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -324,22 +324,6 @@ __ATTR(_name, _perm, show_##_name, NULL) > static struct freq_attr _name = \ > __ATTR(_name, 0644, show_##_name, store_##_name) > > -struct global_attr { > - struct attribute attr; > - ssize_t (*show)(struct kobject *kobj, > - struct attribute
Re: [PATCH V2 4/4] cpufreq: Get rid of struct global_attr
On Monday, February 11, 2013 01:20:03 PM Viresh Kumar wrote: We don't need to keep two structures for file attributes, global_attr and freq_attr. Lets use freq_attr only for cpufreq core and drivers. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- drivers/cpufreq/acpi-cpufreq.c | 9 - drivers/cpufreq/intel_pstate.c | 30 +++--- include/linux/cpufreq.h| 16 3 files changed, 19 insertions(+), 36 deletions(-) diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c index 937bc28..14219c3 100644 --- a/drivers/cpufreq/acpi-cpufreq.c +++ b/drivers/cpufreq/acpi-cpufreq.c @@ -160,19 +160,18 @@ static ssize_t _store_boost(const char *buf, size_t count) return count; } -static ssize_t store_global_boost(struct kobject *kobj, struct attribute *attr, - const char *buf, size_t count) +static ssize_t store_global_boost(struct cpufreq_policy *policy, + const char *buf, size_t count) { return _store_boost(buf, count); } -static ssize_t show_global_boost(struct kobject *kobj, - struct attribute *attr, char *buf) +static ssize_t show_global_boost(struct cpufreq_policy *policy, char *buf) { return sprintf(buf, %u\n, boost_enabled); } -static struct global_attr global_boost = __ATTR(boost, 0644, +static struct freq_attr global_boost = __ATTR(boost, 0644, show_global_boost, store_global_boost); diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 86ad482..0f2d6a0 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -273,15 +273,15 @@ static void intel_pstate_debug_expose_params(void) /** debugfs end / /** sysfs begin / -#define show_one(file_name, object) \ - static ssize_t show_##file_name \ - (struct kobject *kobj, struct attribute *attr, char *buf) \ - { \ - return sprintf(buf, %u\n, limits.object); \ - } +#define show_one(file_name, object) \ +static ssize_t show_##file_name \ +(struct cpufreq_policy *policy, char *buf) \ +{\ + return sprintf(buf, %u\n, limits.object); \ +} Why did you change all of the lines of this macro instead of changing just the one line you needed to change? Please don't do that. -static ssize_t store_no_turbo(struct kobject *a, struct attribute *b, - const char *buf, size_t count) +static ssize_t store_no_turbo(struct cpufreq_policy *policy, const char *buf, + size_t count) { unsigned int input; int ret; @@ -293,8 +293,8 @@ static ssize_t store_no_turbo(struct kobject *a, struct attribute *b, return count; } -static ssize_t store_max_perf_pct(struct kobject *a, struct attribute *b, - const char *buf, size_t count) +static ssize_t store_max_perf_pct(struct cpufreq_policy *policy, + const char *buf, size_t count) { unsigned int input; int ret; @@ -307,8 +307,8 @@ static ssize_t store_max_perf_pct(struct kobject *a, struct attribute *b, return count; } -static ssize_t store_min_perf_pct(struct kobject *a, struct attribute *b, - const char *buf, size_t count) +static ssize_t store_min_perf_pct(struct cpufreq_policy *policy, + const char *buf, size_t count) { unsigned int input; int ret; @@ -325,9 +325,9 @@ show_one(no_turbo, no_turbo); show_one(max_perf_pct, max_perf_pct); show_one(min_perf_pct, min_perf_pct); -define_one_global_rw(no_turbo); -define_one_global_rw(max_perf_pct); -define_one_global_rw(min_perf_pct); +cpufreq_freq_attr_rw(no_turbo); +cpufreq_freq_attr_rw(max_perf_pct); +cpufreq_freq_attr_rw(min_perf_pct); static struct attribute *intel_pstate_attributes[] = { no_turbo.attr, diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 0d84bfa..a092fcb 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -324,22 +324,6 @@ __ATTR(_name, _perm, show_##_name, NULL) static struct freq_attr _name = \ __ATTR(_name, 0644, show_##_name, store_##_name) -struct global_attr { - struct attribute attr; - ssize_t (*show)(struct kobject *kobj, - struct attribute *attr, char *buf); - ssize_t (*store)(struct kobject *a, struct attribute *b, -
Re: [PATCH V2 4/4] cpufreq: Get rid of struct global_attr
On 22 February 2013 05:15, Rafael J. Wysocki r...@sisk.pl wrote: Why did you change all of the lines of this macro instead of changing just the one line you needed to change? I didn't like the indentation used within the macro. So did it. Please don't do that. Okay. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 4/4] cpufreq: Get rid of struct global_attr
On Friday, February 22, 2013 07:47:44 AM Viresh Kumar wrote: On 22 February 2013 05:15, Rafael J. Wysocki r...@sisk.pl wrote: Why did you change all of the lines of this macro instead of changing just the one line you needed to change? I didn't like the indentation used within the macro. So did it. In general, things like that are for separate cleanup patches. If you mix functional changes with cleanups, poeple get confused and it's difficult to see what's needed and what's optional. I know it's tempting to fix stuff like that along with doing functional changes and I do that sometimes. Not very often, though, and with care. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 4/4] cpufreq: Get rid of struct global_attr
On 22 February 2013 08:03, Rafael J. Wysocki r...@sisk.pl wrote: On Friday, February 22, 2013 07:47:44 AM Viresh Kumar wrote: On 22 February 2013 05:15, Rafael J. Wysocki r...@sisk.pl wrote: Why did you change all of the lines of this macro instead of changing just the one line you needed to change? I didn't like the indentation used within the macro. So did it. In general, things like that are for separate cleanup patches. If you mix functional changes with cleanups, poeple get confused and it's difficult to see what's needed and what's optional. I know it's tempting to fix stuff like that along with doing functional changes and I do that sometimes. Not very often, though, and with care. Even i give similar comments sometimes but forget these while writing my patches :) Anyway, fixup: commit b1bbb99467d56140cf3a8a2b70e61b456aa46e48 Author: Viresh Kumar viresh.ku...@linaro.org Date: Fri Feb 22 07:59:20 2013 +0530 fixup! cpufreq: Get rid of struct global_attr --- drivers/cpufreq/intel_pstate.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index e795134..49846b9 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -273,12 +273,12 @@ static void intel_pstate_debug_expose_params(void) /** debugfs end / /** sysfs begin / -#define show_one(file_name, object)\ -static ssize_t show_##file_name\ -(struct cpufreq_policy *policy, char *buf) \ -{ \ - return sprintf(buf, %u\n, limits.object); \ -} +#define show_one(file_name, object)\ + static ssize_t show_##file_name \ + (struct cpufreq_policy *policy, char *buf) \ + { \ + return sprintf(buf, %u\n, limits.object); \ + } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/