[PATCH] cpufreq: acpi: Allow new dynamics attributes to be added to acpi_cpufreq_attr

[PATCH] cpufreq: acpi: Allow new dynamics attributes to be added to acpi_cpufreq_attr

[Viresh Kumar]

acpi_cpufreq_attr contains at least one dynamically populated (removed)
attribute today, cpb. But the code isn't friendly enough for new
attributes to be populated in a similar way.

Make some changes to allow new attributes to be easily added to the
struct.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Srinivas,

This should make it easy for you to add another dynamic entry into the
acpi_cpufreq_attr structure.

 drivers/cpufreq/acpi-cpufreq.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 59a7b380fbe2..c37617ddcc9e 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -866,7 +866,7 @@ static struct freq_attr *acpi_cpufreq_attr[] = {
 	&cpufreq_freq_attr_scaling_available_freqs,
 	&freqdomain_cpus,
 #ifdef CONFIG_X86_ACPI_CPUFREQ_CPB
-	&cpb,
+	NULL,	/* Extra space for cpb if required */
 #endif
 	NULL,
 };
@@ -917,6 +917,7 @@ static void acpi_cpufreq_boost_exit(void)
 
 static int __init acpi_cpufreq_init(void)
 {
+	struct freq_attr **attr;
 	int ret;
 
 	if (acpi_disabled)
@@ -932,6 +933,10 @@ static int __init acpi_cpufreq_init(void)
 	if (ret)
 		return ret;
 
+	/* Find first empty entry */
+	for (attr = acpi_cpufreq_attr; *attr; attr++)
+		;
+
 #ifdef CONFIG_X86_ACPI_CPUFREQ_CPB
 	/* this is a sysfs file with a strange name and an even stranger
 	 * semantic - per CPU instantiation, but system global effect.
@@ -939,17 +944,11 @@ static int __init acpi_cpufreq_init(void)
 	 * only if configured. This is considered legacy code, which
 	 * will probably be removed at some point in the future.
 	 */
-	if (!check_amd_hwpstate_cpu(0)) {
-		struct freq_attr **attr;
-
+	if (check_amd_hwpstate_cpu(0))
+		*attr++ = &cpb;
+	else
 		pr_debug("CPB unsupported, do not expose it\n");
 
-		for (attr = acpi_cpufreq_attr; *attr; attr++)
-			if (*attr == &cpb) {
-				*attr = NULL;
-				break;
-			}
-	}
 #endif
 	acpi_cpufreq_boost_init();
 
-- 
2.7.1.410.g6faf27b

Re: [PATCH] cpufreq: acpi: Allow new dynamics attributes to be added to acpi_cpufreq_attr

[Rafael J. Wysocki]

On Thu, Mar 3, 2016 at 6:40 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> acpi_cpufreq_attr contains at least one dynamically populated (removed)
> attribute today, cpb. But the code isn't friendly enough for new
> attributes to be populated in a similar way.
>
> Make some changes to allow new attributes to be easily added to the
> struct.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Srinivas,
>
> This should make it easy for you to add another dynamic entry into the
> acpi_cpufreq_attr structure.
>
>  drivers/cpufreq/acpi-cpufreq.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 59a7b380fbe2..c37617ddcc9e 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -866,7 +866,7 @@ static struct freq_attr *acpi_cpufreq_attr[] = {
>         &cpufreq_freq_attr_scaling_available_freqs,
>         &freqdomain_cpus,
>  #ifdef CONFIG_X86_ACPI_CPUFREQ_CPB
> -       &cpb,
> +       NULL,   /* Extra space for cpb if required */
>  #endif
>         NULL,
>  };
>
> @@ -917,6 +917,7 @@ static void acpi_cpufreq_boost_exit(void)
>
>  static int __init acpi_cpufreq_init(void)
>  {
> +       struct freq_attr **attr;
>         int ret;
>
>         if (acpi_disabled)
> @@ -932,6 +933,10 @@ static int __init acpi_cpufreq_init(void)
>         if (ret)
>                 return ret;
>
> +       /* Find first empty entry */
> +       for (attr = acpi_cpufreq_attr; *attr; attr++)
> +               ;
> +
>  #ifdef CONFIG_X86_ACPI_CPUFREQ_CPB
>         /* this is a sysfs file with a strange name and an even stranger
>          * semantic - per CPU instantiation, but system global effect.
> @@ -939,17 +944,11 @@ static int __init acpi_cpufreq_init(void)
>          * only if configured. This is considered legacy code, which
>          * will probably be removed at some point in the future.
>          */
> -       if (!check_amd_hwpstate_cpu(0)) {
> -               struct freq_attr **attr;
> -
> +       if (check_amd_hwpstate_cpu(0))
> +               *attr++ = &cpb;
> +       else
>                 pr_debug("CPB unsupported, do not expose it\n");
>
> -               for (attr = acpi_cpufreq_attr; *attr; attr++)
> -                       if (*attr == &cpb) {
> -                               *attr = NULL;
> -                               break;
> -                       }
> -       }
>  #endif
>         acpi_cpufreq_boost_init();

OK, this makes sense.

The table definition starts to look somewhat ugly with all of those
NULLs at the end, but then adding the base_frequency thing to it would
be easy.

Srinivas, can you please take this and rebase your patch on top of it?
 Or if you prefer, I can take it into my linux-next branch.

Thanks,
Rafael

Re: [PATCH] cpufreq: acpi: Allow new dynamics attributes to be added to acpi_cpufreq_attr

[Viresh Kumar]

On 03-03-16, 18:29, Rafael J. Wysocki wrote:
> Srinivas, can you please take this and rebase your patch on top of it?
>  Or if you prefer, I can take it into my linux-next branch.

Right, so you need to apply this patch as Srinivas already sent a separate patch
for his stuff, rebased over this.

-- 
viresh

Re: [PATCH] cpufreq: acpi: Allow new dynamics attributes to be added to acpi_cpufreq_attr

[Rafael J. Wysocki]

On Wed, Mar 9, 2016 at 10:41 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 03-03-16, 18:29, Rafael J. Wysocki wrote:
>> Srinivas, can you please take this and rebase your patch on top of it?
>>  Or if you prefer, I can take it into my linux-next branch.
>
> Right, so you need to apply this patch as Srinivas already sent a separate patch
> for his stuff, rebased over this.

I know, but I'm deferring that one as we aren't entirely sure about
the approach yet.

Thanks,
Rafael

Re: [PATCH] cpufreq: acpi: Allow new dynamics attributes to be added to acpi_cpufreq_attr

[Viresh Kumar]

On 09-03-16, 14:13, Rafael J. Wysocki wrote:
> On Wed, Mar 9, 2016 at 10:41 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > On 03-03-16, 18:29, Rafael J. Wysocki wrote:
> >> Srinivas, can you please take this and rebase your patch on top of it?
> >>  Or if you prefer, I can take it into my linux-next branch.
> >
> > Right, so you need to apply this patch as Srinivas already sent a separate patch
> > for his stuff, rebased over this.
> 
> I know, but I'm deferring that one as we aren't entirely sure about
> the approach yet.

I might have missed, but what are we not sure about here ? I thought, all three
of us agreed that this is a good idea ..

-- 
viresh

Re: [PATCH] cpufreq: acpi: Allow new dynamics attributes to be added to acpi_cpufreq_attr

[Rafael J. Wysocki]

On Wed, Mar 16, 2016 at 5:53 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 09-03-16, 14:13, Rafael J. Wysocki wrote:
>> On Wed, Mar 9, 2016 at 10:41 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> > On 03-03-16, 18:29, Rafael J. Wysocki wrote:
>> >> Srinivas, can you please take this and rebase your patch on top of it?
>> >>  Or if you prefer, I can take it into my linux-next branch.
>> >
>> > Right, so you need to apply this patch as Srinivas already sent a separate patch
>> > for his stuff, rebased over this.
>>
>> I know, but I'm deferring that one as we aren't entirely sure about
>> the approach yet.
>
> I might have missed, but what are we not sure about here ? I thought, all three
> of us agreed that this is a good idea ..

We are not sure if the new attribute is needed.