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

From: Suravee Suthikulanit
Date: Mon Nov 02 2015 - 10:52:14 EST


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@xxxxxxx> wrote:
From: Jeremy Linton <jeremy.linton@xxxxxxx>

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@xxxxxxx>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx>
CC: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
CC: Catalin Marinas <catalin.marinas@xxxxxxx>
CC: Rob Herring <robh+dt@xxxxxxxxxx>
CC: Will Deacon <will.deacon@xxxxxxx>
CC: Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/