On Thu, Oct 29, 2015 at 6:50 AM, Suravee Suthikulpanit <[email protected]> wrote:
> From: Jeremy Linton <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> CC: Bjorn Helgaas <[email protected]>
> CC: Catalin Marinas <[email protected]>
> CC: Rob Herring <[email protected]>
> CC: Will Deacon <[email protected]>
> CC: Rafael J. Wysocki <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Hi Dennis,
On 11/02/2015 12:02 PM, Dennis Chen wrote:
> On Thu, Oct 29, 2015 at 6:50 AM, Suravee Suthikulpanit <[email protected]> wrote:
>> From: Jeremy Linton <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>> CC: Bjorn Helgaas <[email protected]>
>> CC: Catalin Marinas <[email protected]>
>> CC: Rob Herring <[email protected]>
>> CC: Will Deacon <[email protected]>
>> CC: Rafael J. Wysocki <[email protected]>
>> ---
>> 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
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
>> <[email protected]> wrote:
>>> From: Jeremy Linton <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>>> CC: Bjorn Helgaas <[email protected]>
>>> CC: Catalin Marinas <[email protected]>
>>> CC: Rob Herring <[email protected]>
>>> CC: Will Deacon <[email protected]>
>>> CC: Rafael J. Wysocki <[email protected]>
>>> ---
>>> 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
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
> >><[email protected]> wrote:
> >>>From: Jeremy Linton <[email protected]>
> >>>
> >>>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 <[email protected]>
> >>>Signed-off-by: Suravee Suthikulpanit <[email protected]>
> >>>CC: Bjorn Helgaas <[email protected]>
> >>>CC: Catalin Marinas <[email protected]>
> >>>CC: Rob Herring <[email protected]>
> >>>CC: Will Deacon <[email protected]>
> >>>CC: Rafael J. Wysocki <[email protected]>
> >>>---
> >>> 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
>