Re: [PATCH v7 8/8] PCC: Enable PCC only when needed

[Sudeep Holla <sudeep.holla@arm.com>]

Hi Rafael,

On 21/07/15 15:34, Rafael J. Wysocki wrote:
> Hi Sudeep,
>
> On Tue, Jul 21, 2015 at 11:23 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 20/07/15 23:04, Rafael J. Wysocki wrote:
>>>
>>> On Monday, July 20, 2015 03:22:37 PM Sudeep Holla wrote:
>>>>
>>>>
>>>> On 09/07/15 19:04, Ashwin Chaugule wrote:
>>>>>
>>>>> CPPC is the first client to make use of the PCC Mailbox channel. So
>>>>> enable it only when CPPC is also enabled.
>>>>>
>>>> This sounds like a reverse dependency to me. So if there's some client
>>>> unrelated to CPPC using PCC, CPPC_LIB needs to be selected to enable PCC
>>>> ?
>>>
>>>
>>> No.  The other client will need to select PCC too.
>>
>>
>> Yes the PCC users/clients selecting PCC is fine and that's already
>> done(i.e. ACPI_CPPC_LIB selects PCC). I still don't understand the need
>> for this change, also how will other clients possibly select PCC which
>> now depends on CPPC_LIB ? e.g. if we have
>>
>> config ACPI_XYZ_LIB
>>          select PCC
>>
>> config ACPI_XYZ
>>          select ACPI_XYZ_LIB
>>
>> Won't this shout warning: (ACPI_XYZ_LIB && ACPI_CPPC_LIB) selects PCC
>> which has unmet direct dependencies (MAILBOX && ACPI && ACPI_CPPC_LIB)
>> if ACPI_CPPC_LIB is not selected ?
>
> That depends on the "depends on" clauses used.  Selecting itself
> doesn't cause any dependencies to appear.
>

Agreed and I am absolutely fine with that. But if you look at this 
patch, it does

config PCC
	bool "Platform Communication Channel Driver"
	depends on ACPI && ACPI_CPPC_LIB

I am fine with ACPI_CPPC_LIB selecting PCC which is already done in
earlier patch. I am against making PCC depend on ACPI_CPPC_LIB.

Lets assume it's fine to do that. Now if another feature XYZ as in my
example say on x86 selects PCC then you will *get warnings* as we will
not able to select PCC without selecting ACPI_CPPC_LIB after this patch
and x86 will never select ACPI_CPPC_LIB. I did test something like my
XYZ example and it did trigger the warning as I suspected.

I am sorry either I am missing to understand again or unable to express
the problem here :(

>> OK, for now we enable ACPI_CPPC_LIB on ARM64 and not on x86. When x86
>> has a PCC client how will that select PCC without ACPI_CPPC_LIB. Sorry
>> if I am missing to understand something.
>
> Presumably, the new feature will have a Kconfig option associated with
> it and it will do "select PCC" too.
>

As I say I am fine with that and no arguments there.

Regards,
Sudeep