diff mbox

[1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config

Message ID 1470874646-70570-2-git-send-email-srinivas.pandruvada@linux.intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Srinivas Pandruvada Aug. 11, 2016, 12:17 a.m. UTC
Some newer x86 platforms have support for both _CPC and _PSS object. So
kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
defined.
Also for legacy systems with only _PSS, we shouldn't bail out if
acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/acpi/Kconfig            | 1 -
 drivers/acpi/processor_driver.c | 5 ++++-
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

Alexey Klimov Aug. 12, 2016, 9:13 a.m. UTC | #1
(adding Sudeep and Prashanth in c/c)

On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada wrote:
> Some newer x86 platforms have support for both _CPC and _PSS object. So
> kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
> restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
> defined.
> Also for legacy systems with only _PSS, we shouldn't bail out if
> acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/acpi/Kconfig            | 1 -
>  drivers/acpi/processor_driver.c | 5 ++++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 445ce28..c6bb6aa 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -227,7 +227,6 @@ config ACPI_MCFG
>  config ACPI_CPPC_LIB
>  	bool
>  	depends on ACPI_PROCESSOR
> -	depends on !ACPI_CPU_FREQ_PSS
>  	select MAILBOX
>  	select PCC
>  	help
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 0553aee..0e0b629 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct acpi_device *device)
>  		return 0;
>  
>  	result = acpi_cppc_processor_probe(pr);
> -	if (result)
> +	if (result) {
> +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
>  		return -ENODEV;
> +#endif
> +	}
>  
>  	if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>  		acpi_processor_power_init(pr);

If PSS is not defined and kernel fails to probe CPPC then why we should not
execute acpi_processor_power_init()?

Best regards,
Alexey

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srinivas Pandruvada Aug. 12, 2016, 12:34 p.m. UTC | #2
On Fri, 2016-08-12 at 10:13 +0100, Alexey Klimov wrote:
> > 
> (adding Sudeep and Prashanth in c/c)
> 
> On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada wrote:

[...]

> >  	result = acpi_cppc_processor_probe(pr);
> > -	if (result)
> > +	if (result) {
> > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> >  		return -ENODEV;
> > +#endif
> > +	}
> >  
> >  	if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> > &acpi_idle_driver)
> >  		acpi_processor_power_init(pr);
> If PSS is not defined and kernel fails to probe CPPC then why we
> should not
> execute acpi_processor_power_init()?
Did I change the current behavior? Currently when 
acpi_cppc_processor_probe() fails, then -ENODEV is returned.

Thanks,
Srinivas

> 
> Best regards,
> Alexey
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Prakash, Prashanth Aug. 12, 2016, 4:04 p.m. UTC | #3
Hi Alexey,

On 8/12/2016 3:13 AM, Alexey Klimov wrote:
> (adding Sudeep and Prashanth in c/c)
>
> On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada wrote:
>> Some newer x86 platforms have support for both _CPC and _PSS object. So
>> kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
>> restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
>> defined.
>> Also for legacy systems with only _PSS, we shouldn't bail out if
>> acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.
>>
>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>> ---
>>  drivers/acpi/Kconfig            | 1 -
>>  drivers/acpi/processor_driver.c | 5 ++++-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 445ce28..c6bb6aa 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -227,7 +227,6 @@ config ACPI_MCFG
>>  config ACPI_CPPC_LIB
>>  	bool
>>  	depends on ACPI_PROCESSOR
>> -	depends on !ACPI_CPU_FREQ_PSS
>>  	select MAILBOX
>>  	select PCC
>>  	help
>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>> index 0553aee..0e0b629 100644
>> --- a/drivers/acpi/processor_driver.c
>> +++ b/drivers/acpi/processor_driver.c
>> @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct acpi_device *device)
>>  		return 0;
>>  
>>  	result = acpi_cppc_processor_probe(pr);
>> -	if (result)
>> +	if (result) {
>> +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
>>  		return -ENODEV;
>> +#endif
>> +	}
>>  
>>  	if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>>  		acpi_processor_power_init(pr);
> If PSS is not defined and kernel fails to probe CPPC then why we should not
> execute acpi_processor_power_init()?
Returning on cppc probe failure looks like a bug. We can just print
a warning and continue to acpi_processor_power_init().

Thanks,
Prashanth
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
hotran Aug. 12, 2016, 4:32 p.m. UTC | #4
Hi,

On Fri, Aug 12, 2016 at 9:04 AM, Prakash, Prashanth
<pprakash@codeaurora.org> wrote:
> Hi Alexey,
>
> On 8/12/2016 3:13 AM, Alexey Klimov wrote:
>> (adding Sudeep and Prashanth in c/c)
>>
>> On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada wrote:
>>> Some newer x86 platforms have support for both _CPC and _PSS object. So
>>> kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
>>> restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
>>> defined.
>>> Also for legacy systems with only _PSS, we shouldn't bail out if
>>> acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.
>>>
>>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>>> ---
>>>  drivers/acpi/Kconfig            | 1 -
>>>  drivers/acpi/processor_driver.c | 5 ++++-
>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index 445ce28..c6bb6aa 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -227,7 +227,6 @@ config ACPI_MCFG
>>>  config ACPI_CPPC_LIB
>>>      bool
>>>      depends on ACPI_PROCESSOR
>>> -    depends on !ACPI_CPU_FREQ_PSS
>>>      select MAILBOX
>>>      select PCC
>>>      help
>>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>>> index 0553aee..0e0b629 100644
>>> --- a/drivers/acpi/processor_driver.c
>>> +++ b/drivers/acpi/processor_driver.c
>>> @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct acpi_device *device)
>>>              return 0;
>>>
>>>      result = acpi_cppc_processor_probe(pr);
>>> -    if (result)
>>> +    if (result) {
>>> +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
>>>              return -ENODEV;
>>> +#endif
>>> +    }
>>>
>>>      if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>>>              acpi_processor_power_init(pr);
>> If PSS is not defined and kernel fails to probe CPPC then why we should not
>> execute acpi_processor_power_init()?
> Returning on cppc probe failure looks like a bug. We can just print
> a warning and continue to acpi_processor_power_init().

Yes, it is. We should continue. I saw an issue about that. If the CPPC
probe fails, CPUidle can NOT be registered.

Thanks
Hoan

>
> Thanks,
> Prashanth
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
hotran Aug. 12, 2016, 4:35 p.m. UTC | #5
Hi Srinivas,

On Wed, Aug 10, 2016 at 5:17 PM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> Some newer x86 platforms have support for both _CPC and _PSS object. So
> kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
> restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
> defined.
> Also for legacy systems with only _PSS, we shouldn't bail out if
> acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/acpi/Kconfig            | 1 -
>  drivers/acpi/processor_driver.c | 5 ++++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 445ce28..c6bb6aa 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -227,7 +227,6 @@ config ACPI_MCFG
>  config ACPI_CPPC_LIB
>         bool
>         depends on ACPI_PROCESSOR
> -       depends on !ACPI_CPU_FREQ_PSS

From ACPI 6.1 spec, if _CPC is present, its use supersedes the use of
PSS. So I think, config ACPI_CPU_FREQ_PSS should depend on
!ACPI_CPPC_LIB.

Thanks
Hoan

>         select MAILBOX
>         select PCC
>         help
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 0553aee..0e0b629 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct acpi_device *device)
>                 return 0;
>
>         result = acpi_cppc_processor_probe(pr);
> -       if (result)
> +       if (result) {
> +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
>                 return -ENODEV;
> +#endif
> +       }
>
>         if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>                 acpi_processor_power_init(pr);
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srinivas Pandruvada Aug. 12, 2016, 4:52 p.m. UTC | #6
On Fri, 2016-08-12 at 09:35 -0700, Hoan Tran wrote:
> Hi Srinivas,
> 
> On Wed, Aug 10, 2016 at 5:17 PM, Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > 
> > Some newer x86 platforms have support for both _CPC and _PSS
> > object. So
> > kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So
> > remove
> > restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS
> > is not
> > defined.
> > Also for legacy systems with only _PSS, we shouldn't bail out if
> > acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also
> > defined.
> > 
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel
> > .com>
> > ---
> >  drivers/acpi/Kconfig            | 1 -
> >  drivers/acpi/processor_driver.c | 5 ++++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 445ce28..c6bb6aa 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -227,7 +227,6 @@ config ACPI_MCFG
> >  config ACPI_CPPC_LIB
> >         bool
> >         depends on ACPI_PROCESSOR
> > -       depends on !ACPI_CPU_FREQ_PSS
> From ACPI 6.1 spec, if _CPC is present, its use supersedes the use of
> PSS. So I think, config ACPI_CPU_FREQ_PSS should depend on
> !ACPI_CPPC_LIB.
> 
Distro want to have a single binary kernel image, so they will turn on
all configs. So this is not a compile time decision.
On runtime if the ACPI contains both tables than _CPC should be used
(but that also if the kernel is capable of handling _CPC, as legacy
kernel will not).

Thanks,
Srinivas

> Thanks
> Hoan
> 
> > 
> >         select MAILBOX
> >         select PCC
> >         help
> > diff --git a/drivers/acpi/processor_driver.c
> > b/drivers/acpi/processor_driver.c
> > index 0553aee..0e0b629 100644
> > --- a/drivers/acpi/processor_driver.c
> > +++ b/drivers/acpi/processor_driver.c
> > @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct
> > acpi_device *device)
> >                 return 0;
> > 
> >         result = acpi_cppc_processor_probe(pr);
> > -       if (result)
> > +       if (result) {
> > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> >                 return -ENODEV;
> > +#endif
> > +       }
> > 
> >         if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> > &acpi_idle_driver)
> >                 acpi_processor_power_init(pr);
> > --
> > 2.7.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srinivas Pandruvada Aug. 12, 2016, 4:53 p.m. UTC | #7
On Fri, 2016-08-12 at 09:32 -0700, Hoan Tran wrote:
> Hi,
> 
> On Fri, Aug 12, 2016 at 9:04 AM, Prakash, Prashanth
> <pprakash@codeaurora.org> wrote:
> > 
> > Hi Alexey,
> > 
> > On 8/12/2016 3:13 AM, Alexey Klimov wrote:
> > > 
> > > (adding Sudeep and Prashanth in c/c)
> > > 
> > > On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada
> > > wrote:
> > > > 
> > > > Some newer x86 platforms have support for both _CPC and _PSS
> > > > object. So
> > > > kernel config can have both ACPI_CPU_FREQ_PSS and
> > > > ACPI_CPPC_LIB. So remove
> > > > restriction for ACPI_CPPC_LIB to build only when
> > > > ACPI_CPU_FREQ_PSS is not
> > > > defined.
> > > > Also for legacy systems with only _PSS, we shouldn't bail out
> > > > if
> > > > acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also
> > > > defined.
> > > > 
> > > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.i
> > > > ntel.com>
> > > > ---
> > > >  drivers/acpi/Kconfig            | 1 -
> > > >  drivers/acpi/processor_driver.c | 5 ++++-
> > > >  2 files changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > > > index 445ce28..c6bb6aa 100644
> > > > --- a/drivers/acpi/Kconfig
> > > > +++ b/drivers/acpi/Kconfig
> > > > @@ -227,7 +227,6 @@ config ACPI_MCFG
> > > >  config ACPI_CPPC_LIB
> > > >      bool
> > > >      depends on ACPI_PROCESSOR
> > > > -    depends on !ACPI_CPU_FREQ_PSS
> > > >      select MAILBOX
> > > >      select PCC
> > > >      help
> > > > diff --git a/drivers/acpi/processor_driver.c
> > > > b/drivers/acpi/processor_driver.c
> > > > index 0553aee..0e0b629 100644
> > > > --- a/drivers/acpi/processor_driver.c
> > > > +++ b/drivers/acpi/processor_driver.c
> > > > @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct
> > > > acpi_device *device)
> > > >              return 0;
> > > > 
> > > >      result = acpi_cppc_processor_probe(pr);
> > > > -    if (result)
> > > > +    if (result) {
> > > > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> > > >              return -ENODEV;
> > > > +#endif
> > > > +    }
> > > > 
> > > >      if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> > > > &acpi_idle_driver)
> > > >              acpi_processor_power_init(pr);
> > > If PSS is not defined and kernel fails to probe CPPC then why we
> > > should not
> > > execute acpi_processor_power_init()?
> > Returning on cppc probe failure looks like a bug. We can just print
> > a warning and continue to acpi_processor_power_init().
> Yes, it is. We should continue. I saw an issue about that. If the
> CPPC
> probe fails, CPUidle can NOT be registered.

I wanted to keep the existing functionality as is. But I can submit
another patch on top of it to ignore cppc probe failure.

Thanks,
Srinivas


> Thanks
> Hoan
> 
> > 
> > 
> > Thanks,
> > Prashanth
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
hotran Aug. 12, 2016, 4:58 p.m. UTC | #8
Hi Srinivas,

On Fri, Aug 12, 2016 at 9:52 AM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> On Fri, 2016-08-12 at 09:35 -0700, Hoan Tran wrote:
>> Hi Srinivas,
>>
>> On Wed, Aug 10, 2016 at 5:17 PM, Srinivas Pandruvada
>> <srinivas.pandruvada@linux.intel.com> wrote:
>> >
>> > Some newer x86 platforms have support for both _CPC and _PSS
>> > object. So
>> > kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So
>> > remove
>> > restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS
>> > is not
>> > defined.
>> > Also for legacy systems with only _PSS, we shouldn't bail out if
>> > acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also
>> > defined.
>> >
>> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel
>> > .com>
>> > ---
>> >  drivers/acpi/Kconfig            | 1 -
>> >  drivers/acpi/processor_driver.c | 5 ++++-
>> >  2 files changed, 4 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> > index 445ce28..c6bb6aa 100644
>> > --- a/drivers/acpi/Kconfig
>> > +++ b/drivers/acpi/Kconfig
>> > @@ -227,7 +227,6 @@ config ACPI_MCFG
>> >  config ACPI_CPPC_LIB
>> >         bool
>> >         depends on ACPI_PROCESSOR
>> > -       depends on !ACPI_CPU_FREQ_PSS
>> From ACPI 6.1 spec, if _CPC is present, its use supersedes the use of
>> PSS. So I think, config ACPI_CPU_FREQ_PSS should depend on
>> !ACPI_CPPC_LIB.
>>
> Distro want to have a single binary kernel image, so they will turn on
> all configs. So this is not a compile time decision.
> On runtime if the ACPI contains both tables than _CPC should be used
> (but that also if the kernel is capable of handling _CPC, as legacy
> kernel will not).

Agreed. If so, this "depends on" is not need, right ? And a runtime
code will be added to decide which is used instead ?

Thanks
Hoan

>
> Thanks,
> Srinivas
>
>> Thanks
>> Hoan
>>
>> >
>> >         select MAILBOX
>> >         select PCC
>> >         help
>> > diff --git a/drivers/acpi/processor_driver.c
>> > b/drivers/acpi/processor_driver.c
>> > index 0553aee..0e0b629 100644
>> > --- a/drivers/acpi/processor_driver.c
>> > +++ b/drivers/acpi/processor_driver.c
>> > @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct
>> > acpi_device *device)
>> >                 return 0;
>> >
>> >         result = acpi_cppc_processor_probe(pr);
>> > -       if (result)
>> > +       if (result) {
>> > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
>> >                 return -ENODEV;
>> > +#endif
>> > +       }
>> >
>> >         if (!cpuidle_get_driver() || cpuidle_get_driver() ==
>> > &acpi_idle_driver)
>> >                 acpi_processor_power_init(pr);
>> > --
>> > 2.7.4
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-
>> > acpi" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm"
>> in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srinivas Pandruvada Aug. 12, 2016, 5:16 p.m. UTC | #9
On Fri, 2016-08-12 at 09:58 -0700, Hoan Tran wrote:
> Hi Srinivas,
> 
> On Fri, Aug 12, 2016 at 9:52 AM, Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > 
> > On Fri, 2016-08-12 at 09:35 -0700, Hoan Tran wrote:
> > > 
> > > Hi Srinivas,
> > > 
> > > On Wed, Aug 10, 2016 at 5:17 PM, Srinivas Pandruvada
> > > <srinivas.pandruvada@linux.intel.com> wrote:
> > > > 
> > > > 
> > > > Some newer x86 platforms have support for both _CPC and _PSS
> > > > object. So
> > > > kernel config can have both ACPI_CPU_FREQ_PSS and
> > > > ACPI_CPPC_LIB. So
> > > > remove
> > > > restriction for ACPI_CPPC_LIB to build only when
> > > > ACPI_CPU_FREQ_PSS
> > > > is not
> > > > defined.
> > > > Also for legacy systems with only _PSS, we shouldn't bail out
> > > > if
> > > > acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also
> > > > defined.
> > > > 
> > > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.i
> > > > ntel
> > > > .com>
> > > > ---
> > > >  drivers/acpi/Kconfig            | 1 -
> > > >  drivers/acpi/processor_driver.c | 5 ++++-
> > > >  2 files changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > > > index 445ce28..c6bb6aa 100644
> > > > --- a/drivers/acpi/Kconfig
> > > > +++ b/drivers/acpi/Kconfig
> > > > @@ -227,7 +227,6 @@ config ACPI_MCFG
> > > >  config ACPI_CPPC_LIB
> > > >         bool
> > > >         depends on ACPI_PROCESSOR
> > > > -       depends on !ACPI_CPU_FREQ_PSS
> > > From ACPI 6.1 spec, if _CPC is present, its use supersedes the
> > > use of
> > > PSS. So I think, config ACPI_CPU_FREQ_PSS should depend on
> > > !ACPI_CPPC_LIB.
> > > 
> > Distro want to have a single binary kernel image, so they will turn
> > on
> > all configs. So this is not a compile time decision.
> > On runtime if the ACPI contains both tables than _CPC should be
> > used
> > (but that also if the kernel is capable of handling _CPC, as legacy
> > kernel will not).
> Agreed. If so, this "depends on" is not need, right ? And a runtime
> code will be added to decide which is used instead ?
Yes, _CPC will be used if present. 

Thanks,
Srinivas

> 
> Thanks
> Hoan
> 
> > 
> > 
> > Thanks,
> > Srinivas
> > 
> > > 
> > > Thanks
> > > Hoan
> > > 
> > > > 
> > > > 
> > > >         select MAILBOX
> > > >         select PCC
> > > >         help
> > > > diff --git a/drivers/acpi/processor_driver.c
> > > > b/drivers/acpi/processor_driver.c
> > > > index 0553aee..0e0b629 100644
> > > > --- a/drivers/acpi/processor_driver.c
> > > > +++ b/drivers/acpi/processor_driver.c
> > > > @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct
> > > > acpi_device *device)
> > > >                 return 0;
> > > > 
> > > >         result = acpi_cppc_processor_probe(pr);
> > > > -       if (result)
> > > > +       if (result) {
> > > > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> > > >                 return -ENODEV;
> > > > +#endif
> > > > +       }
> > > > 
> > > >         if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> > > > &acpi_idle_driver)
> > > >                 acpi_processor_power_init(pr);
> > > > --
> > > > 2.7.4
> > > > 
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe
> > > > linux-
> > > > acpi" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.h
> > > > tml
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-
> > > pm"
> > > in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.htm
> > > l
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 445ce28..c6bb6aa 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -227,7 +227,6 @@  config ACPI_MCFG
 config ACPI_CPPC_LIB
 	bool
 	depends on ACPI_PROCESSOR
-	depends on !ACPI_CPU_FREQ_PSS
 	select MAILBOX
 	select PCC
 	help
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 0553aee..0e0b629 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -245,8 +245,11 @@  static int __acpi_processor_start(struct acpi_device *device)
 		return 0;
 
 	result = acpi_cppc_processor_probe(pr);
-	if (result)
+	if (result) {
+#ifndef CONFIG_ACPI_CPU_FREQ_PSS
 		return -ENODEV;
+#endif
+	}
 
 	if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
 		acpi_processor_power_init(pr);