On Thu, Mar 3, 2016 at 6:40 AM, Viresh Kumar <viresh.ku...@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.ku...@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
