Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1260329 > unrolled thread
| Started by | Dennis Chen <dennis.chen@arm.com> |
|---|---|
| First post | 2015-11-02 05:10 +0100 |
| Last post | 2015-11-03 07:00 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
Re: [PATCH V5 1/9] ACPI: Honor ACPI _CCA attribute setting Dennis Chen <dennis.chen@arm.com> - 2015-11-02 05:10 +0100
Re: [PATCH V5 1/9] ACPI: Honor ACPI _CCA attribute setting Hanjun Guo <hanjun.guo@linaro.org> - 2015-11-02 13:00 +0100
Re: [PATCH V5 1/9] ACPI: Honor ACPI _CCA attribute setting Dennis Chen <dennis.chen@arm.com> - 2015-11-03 07:00 +0100
| From | Dennis Chen <dennis.chen@arm.com> |
|---|---|
| Date | 2015-11-02 05:10 +0100 |
| Subject | Re: [PATCH V5 1/9] ACPI: Honor ACPI _CCA attribute setting |
| Message-ID | <qqeBr-EF-5@gated-at.bofh.it> |
On Thu, Oct 29, 2015 at 6:50 AM, Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> wrote:
> From: Jeremy Linton <jeremy.linton@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@arm.com>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: Catalin Marinas <catalin.marinas@arm.com>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Will Deacon <will.deacon@arm.com>
> CC: Rafael J. Wysocki <rjw@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
to use IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED) while not CONFIG_ARM64 macro here, 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.
Thanks,
Dennis
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Hanjun Guo <hanjun.guo@linaro.org> |
|---|---|
| Date | 2015-11-02 13:00 +0100 |
| Message-ID | <qqlWh-4QZ-11@gated-at.bofh.it> |
| In reply to | #1260329 |
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@amd.com> wrote:
>> From: Jeremy Linton <jeremy.linton@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@arm.com>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>> CC: Bjorn Helgaas <bhelgaas@google.com>
>> CC: Catalin Marinas <catalin.marinas@arm.com>
>> CC: Rob Herring <robh+dt@kernel.org>
>> CC: Will Deacon <will.deacon@arm.com>
>> CC: Rafael J. Wysocki <rjw@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 :) )
> to use IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED) while not CONFIG_ARM64 macro here, 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Dennis Chen <dennis.chen@arm.com> |
|---|---|
| Date | 2015-11-03 07:00 +0100 |
| Message-ID | <qqCNs-70t-7@gated-at.bofh.it> |
| In reply to | #1260572 |
On Mon, Nov 02, 2015 at 09:51:46AM -0600, Suravee Suthikulanit wrote:
> 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@amd.com> wrote:
> >>>From: Jeremy Linton <jeremy.linton@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@arm.com>
> >>>Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>>CC: Bjorn Helgaas <bhelgaas@google.com>
> >>>CC: Catalin Marinas <catalin.marinas@arm.com>
> >>>CC: Rob Herring <robh+dt@kernel.org>
> >>>CC: Will Deacon <will.deacon@arm.com>
> >>>CC: Rafael J. Wysocki <rjw@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.
>
Clear.
> >>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. :)
>
Hmm, seems I have no objection currently if we only think about intel and
arm arch. Things maybe a little bit complicated if more Archs becomes
ACPI awareness, if any. Good to see the patch set upstream soon :) Thank you
Suravee and Hanjun.
> Thanks,
> Suravee
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web