[PATCH V5 1/9] ACPI: Honor ACPI _CCA attribute setting

[Suravee Suthikulanit]

Hi Dennis / Hanjun,

On 11/2/2015 5:58 AM, Hanjun Guo wrote:
> Hi Dennis,
>
> On 11/02/2015 12:02 PM, Dennis Chen wrote:
>> On Thu, Oct 29, 2015 at 6:50 AM, Suravee Suthikulpanit
>> <Suravee.Suthikulpanit at amd.com> wrote:
>>> From: Jeremy Linton <jeremy.linton at arm.com>
>>>
>>> ACPI configurations can now mark devices as noncoherent,
>>> support that choice.
>>>
>>> NOTE: This is required to support USB on ARM Juno Development Board.
>>>
>>> Signed-off-by: Jeremy Linton <jeremy.linton at arm.com>
>>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit at amd.com>
>>> CC: Bjorn Helgaas <bhelgaas at google.com>
>>> CC: Catalin Marinas <catalin.marinas at arm.com>
>>> CC: Rob Herring <robh+dt at kernel.org>
>>> CC: Will Deacon <will.deacon at arm.com>
>>> CC: Rafael J. Wysocki <rjw at rjwysocki.net>
>>> ---
>>>   include/acpi/acpi_bus.h | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
>>> index d11eff8..0f131d2 100644
>>> --- a/include/acpi/acpi_bus.h
>>> +++ b/include/acpi/acpi_bus.h
>>> @@ -407,7 +407,7 @@ static inline bool acpi_check_dma(struct
>>> acpi_device *adev, bool *coherent)
>>>        * case 1. Do not support and disable DMA.
>>>        * case 2. Support but rely on arch-specific cache maintenance for
>>>        *         non-coherence DMA operations.
>>> -     * Currently, we implement case 1 above.
>>> +     * Currently, we implement case 2 above.
>>>        *
>>>        * For the case when _CCA is missing (i.e. cca_seen=0) and
>>>        * platform specifies ACPI_CCA_REQUIRED, we do not support DMA,
>>> @@ -415,7 +415,8 @@ static inline bool acpi_check_dma(struct
>>> acpi_device *adev, bool *coherent)
>>>        *
>>>        * See acpi_init_coherency() for more info.
>>>        */
>>> -    if (adev->flags.coherent_dma) {
>>> +    if (adev->flags.coherent_dma ||
>>> +        (adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64))) {
>>>           ret = true;
>>>           if (coherent)
>>>               *coherent = adev->flags.coherent_dma;
>>
>> Hi Suravee,
>>
>> The acpi_check_dma function has been removed in patch 6 of this patch
>> set, why it is still be used
>> here, am I missing something? If the acpi_check_dma will be used in
>> the future, personally I'd like
>
> I think this patch just to let people know that there is
> case that arch-specific cache maintenance is still needed
> for ACPI (such as Juno board), and in the later patches will
> cover this case.
>
> acpi_check_dma() will be replaced by acpi_get_dma_attr(),
> and in acpi_get_dma_attr() will cover that case and will
> be easily understood. (Suravee, correct me if I'm wrong :) )

Thanks Hanjun for filling in the info.

Yes, this is mainly to document the logic changes required by Juno, 
which would be more clear than just merging this change in the later patch.

>> to use IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED) while not CONFIG_ARM64
>> macro here,

We could have used CONFIG_ACPI_CCA_REQUIRED here, but this will be 
replaced by logic in patch 5, and removed in patch 6 anyways. So, I 
think it is okay. Eventually, the rest of the logic will be using 
CONFIG_ACPI_CCA_REQUIRED.

or since _CCA attribute
>> is arch-specific, it's reasonable to leave the _CCA handling policy to
>> the arch-specific code. For example,
>> with a link weak function like acpi_arch_check_dma() as a default
>> handling if no arch-specific code
>> provided, the actual _CCA handling will be implemented in the ARM,
>> Intel or other Arch if required.
>
> Actually Intel platform don't need _CCA and it's coherent
> in default, since _CCA is in ACPI spec, I would like it's
> handled in ACPI core.
>
> Thanks
> Hanjun

I also agree with Hanjun that the CCA parsing should be handled by the 
ACPI core driver. Since we are using the CONFIG_ACPI_CCA_REQUIRED, we 
should not need to have arch-specific code. If the ACPI spec gets more 
complicate in the future, we can revisit this. :)

Thanks,
Suravee