This patch series introduce support for _CCA object, which is currently used mainly by ARM64 platform to specify DMA coherency attribute for devices when booting with ACPI.
A copy of ACPIv6 can be found here: http://www.uefi.org/sites/default/files/resources/ACPI_6.0.pdf
This patch also introduces 2 new APIS: 1. acpi_dma_is_coherent() as part of ACPI API. 2. device_dma_is_coherent() as part of unified device property API.
This simplifies the logic in device drivers to determine device coherency attribute regardless of booting with DT vs ACPI.
This has been tested on AMD-Seattle platform, which implements _CCA object as described in the AMD Opteron A1100 Series Processor ACPI Porting Guide:
http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2012/10/Seattle_ACPI_...
Changes from V2 (https://lkml.org/lkml/2015/5/5/510): * Reword ACPI_MUST_HAVE_CCA to ACPI_CCA_REQUIRED (per Rafael) * Reword ACPI_SUPPORT_CCA_ZERO to ARCH64_SUPPORT_ACPI_CCA_ZERO (per Rafael and Arnd) * Misc code styling clean up (per Rafael) * Only print missing _CCA warning message in debug mode. * Refactor logic in acpi_setup_device_dma() into if acpi_dma_is_supported() then call arch_setup_dma_ops(). * Do not allocate device dma_mask if !acpi_dma_is_supported() (per Arnd). * Re-use the dummy functions with the same signature.
Changes from V1 (https://lkml.org/lkml/2015/4/29/290): * Remove supports for 32-bit ARM since doesn't currently supporting ACPI (Per Catalin suggestions.) * Do not call arch_setup_dma_ops() and when _CCA is missing. (per Arnd suggestion) * Add CONFIG_ACPI_SUPPORT_CCA_ZERO kernel config flag to allow architectures to specify the behavior when _CCA=0. * Add dummy_dma_ops for ARM64 (per Catalin suggestions). * Fixed build error when ACPI is not configured by defining acpi_dma_is_coherent() for when CONFIG_ACPI is not set. * Introduce device_dma_is_coherent(). * Use device_dma_is_coherent in crypto/ccp and amd-xgbe driver.
Changes from RFC: (https://lkml.org/lkml/2015/4/1/389) * New logic for deriving and propagating coherent attribute from parent devices. (by Mark) * Introducing acpi_dma_is_coherent() API (Per Tom suggestion) * Introducing CONFIG_ACPI_MUST_HAVE_CCA kernel configuration. * Rebased to linux-4.1-rc1
Suravee Suthikulpanit (5): ACPI / scan: Parse _CCA and setup device coherency arm64 : Introduce support for ACPI _CCA object device property: Introduces device_dma_is_coherent() crypto: ccp - Unify coherency checking logic with device_dma_is_coherent() amd-xgbe: Unify coherency checking logic with device_dma_is_coherent()
arch/arm64/Kconfig | 2 + arch/arm64/include/asm/dma-mapping.h | 18 +++++- arch/arm64/mm/dma-mapping.c | 92 +++++++++++++++++++++++++++++++ drivers/acpi/Kconfig | 6 ++ drivers/acpi/acpi_platform.c | 10 +++- drivers/acpi/scan.c | 42 ++++++++++++++ drivers/base/property.c | 12 ++++ drivers/crypto/ccp/ccp-platform.c | 60 +------------------- drivers/net/ethernet/amd/xgbe/xgbe-main.c | 27 +-------- include/acpi/acpi_bus.h | 32 ++++++++++- include/linux/acpi.h | 10 ++++ include/linux/property.h | 2 + 12 files changed, 222 insertions(+), 91 deletions(-)
This patch implements support for ACPI _CCA object, which is introduced in ACPIv5.1, can be used for specifying device DMA coherency attribute.
The parsing logic traverses device namespace to parse coherency information, and stores it in acpi_device_flags. Then uses it to call arch_setup_dma_ops() when creating each device enumerated in DSDT during ACPI scan.
This patch also introduces acpi_dma_is_coherent(), which provides an interface for device drivers to check the coherency information similarly to the of_dma_is_coherent().
Signed-off-by: Mark Salter msalter@redhat.com Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com --- drivers/acpi/Kconfig | 6 ++++++ drivers/acpi/acpi_platform.c | 10 +++++++--- drivers/acpi/scan.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/acpi/acpi_bus.h | 32 +++++++++++++++++++++++++++++++- include/linux/acpi.h | 10 ++++++++++ 5 files changed, 96 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index ab2cbb5..7822149 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -54,6 +54,12 @@ config ACPI_GENERIC_GSI config ACPI_SYSTEM_POWER_STATES_SUPPORT bool
+config ACPI_CCA_REQUIRED + bool + +config ARM64_SUPPORT_ACPI_CCA_ZERO + bool + config ACPI_SLEEP bool depends on SUSPEND || HIBERNATION diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 4bf7559..a084ea0 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -103,14 +103,18 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev) pdevinfo.res = resources; pdevinfo.num_res = count; pdevinfo.fwnode = acpi_fwnode_handle(adev); - pdevinfo.dma_mask = DMA_BIT_MASK(32); + pdevinfo.dma_mask = acpi_dma_is_supported(adev)? DMA_BIT_MASK(32): 0; pdev = platform_device_register_full(&pdevinfo); - if (IS_ERR(pdev)) + if (IS_ERR(pdev)) { dev_err(&adev->dev, "platform device creation failed: %ld\n", PTR_ERR(pdev)); - else + } else { + if (acpi_dma_is_supported(adev)) + arch_setup_dma_ops(&pdev->dev, 0, 0, NULL, + acpi_dma_is_coherent(adev)); dev_dbg(&adev->dev, "created platform device %s\n", dev_name(&pdev->dev)); + }
kfree(resources); return pdev; diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 849b699..0976dc2 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -11,6 +11,7 @@ #include <linux/kthread.h> #include <linux/dmi.h> #include <linux/nls.h> +#include <linux/dma-mapping.h>
#include <asm/pgtable.h>
@@ -2137,6 +2138,46 @@ void acpi_free_pnp_ids(struct acpi_device_pnp *pnp) kfree(pnp->unique_id); }
+static void acpi_init_coherency(struct acpi_device *adev) +{ + unsigned long long cca = 0; + acpi_status status; + struct acpi_device *parent = adev->parent; + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; + + if (parent && parent->flags.cca_seen) { + /* + * From ACPI spec, OSPM will ignore _CCA if an ancestor + * already saw one. + */ + adev->flags.cca_seen = 1; + cca = acpi_dma_is_coherent(parent); + } else { + status = acpi_evaluate_integer(adev->handle, "_CCA", + NULL, &cca); + if (ACPI_SUCCESS(status)) { + adev->flags.cca_seen = 1; + } else if (!IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED)) { + /* + * If architecture does not specify that _CCA is + * required for DMA-able devices (e.g. x86), + * we default to _CCA=1. + */ + cca = 1; + } else { + acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &buffer); + pr_debug("ACPI device %s is missing _CCA.\n", + (char *) buffer.pointer); + kfree(buffer.pointer); + } + } + + adev->flags.is_coherent = cca; + if (acpi_dma_is_supported(adev)) + arch_setup_dma_ops(&adev->dev, 0, 0, NULL, + acpi_dma_is_coherent(adev)); +} + void acpi_init_device_object(struct acpi_device *device, acpi_handle handle, int type, unsigned long long sta) { @@ -2155,6 +2196,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle, device->flags.visited = false; device_initialize(&device->dev); dev_set_uevent_suppress(&device->dev, true); + acpi_init_coherency(device); }
void acpi_device_add_finalize(struct acpi_device *device) diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 8de4fa9..17fb630 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -208,7 +208,9 @@ struct acpi_device_flags { u32 visited:1; u32 hotplug_notify:1; u32 is_dock_station:1; - u32 reserved:23; + u32 is_coherent:1; + u32 cca_seen:1; + u32 reserved:21; };
/* File System */ @@ -380,6 +382,34 @@ struct acpi_device { void (*remove)(struct acpi_device *); };
+static inline bool acpi_dma_is_supported(struct acpi_device *adev) +{ + /** + * Currently, we mainly support _CCA=1 (i.e. is_coherent=1) + * This should be equivalent to specifyig dma-coherent for + * a device in OF. + * + * For the case when _CCA=0 (i.e. is_coherent=0 && cca_seen=1), + * we would rely on arch-specific cache maintenance for + * non-coherence DMA operations if architecture specifies + * _XXX_SUPPORT_CCA_ZERO. Otherwise, we do not support + * DMA on this device and fallback to arch-specific default + * handling. + * + * For the case when _CCA is missing (i.e. cca_seen=0) but + * platform specifies ACPI_CCA_REQUIRED, we do not support DMA, + * and fallback to arch-specific default handling. + */ + return adev && (adev->flags.is_coherent || + (adev->flags.cca_seen && + IS_ENABLED(CONFIG_ARM64_SUPPORT_ACPI_CCA_ZERO))); +} + +static inline bool acpi_dma_is_coherent(struct acpi_device *adev) +{ + return adev && adev->flags.is_coherent; +} + static inline bool is_acpi_node(struct fwnode_handle *fwnode) { return fwnode && fwnode->type == FWNODE_ACPI; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index b10c4a6..baccf3b 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -583,6 +583,16 @@ static inline int acpi_device_modalias(struct device *dev, return -ENODEV; }
+static inline bool acpi_dma_is_supported(struct acpi_device *adev) +{ + return false; +} + +static inline bool acpi_dma_is_coherent(struct acpi_device *adev) +{ + return false; +} + #define ACPI_PTR(_ptr) (NULL)
#endif /* !CONFIG_ACPI */
On Thursday, May 07, 2015 07:37:12 PM Suravee Suthikulpanit wrote:
This patch implements support for ACPI _CCA object, which is introduced in ACPIv5.1, can be used for specifying device DMA coherency attribute.
The parsing logic traverses device namespace to parse coherency information, and stores it in acpi_device_flags. Then uses it to call arch_setup_dma_ops() when creating each device enumerated in DSDT during ACPI scan.
This patch also introduces acpi_dma_is_coherent(), which provides an interface for device drivers to check the coherency information similarly to the of_dma_is_coherent().
Signed-off-by: Mark Salter msalter@redhat.com Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com
drivers/acpi/Kconfig | 6 ++++++ drivers/acpi/acpi_platform.c | 10 +++++++--- drivers/acpi/scan.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/acpi/acpi_bus.h | 32 +++++++++++++++++++++++++++++++- include/linux/acpi.h | 10 ++++++++++ 5 files changed, 96 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index ab2cbb5..7822149 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -54,6 +54,12 @@ config ACPI_GENERIC_GSI config ACPI_SYSTEM_POWER_STATES_SUPPORT bool +config ACPI_CCA_REQUIRED
- bool
+config ARM64_SUPPORT_ACPI_CCA_ZERO
Hmm. I guess the Arnd's idea what to simply use CONFIG_ARM64 directly instead of adding this new option.
- bool
config ACPI_SLEEP bool depends on SUSPEND || HIBERNATION diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 4bf7559..a084ea0 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -103,14 +103,18 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev) pdevinfo.res = resources; pdevinfo.num_res = count; pdevinfo.fwnode = acpi_fwnode_handle(adev);
- pdevinfo.dma_mask = DMA_BIT_MASK(32);
- pdevinfo.dma_mask = acpi_dma_is_supported(adev)? DMA_BIT_MASK(32): 0;
Spaces before the "?" and ":", please.
pdev = platform_device_register_full(&pdevinfo);
- if (IS_ERR(pdev))
- if (IS_ERR(pdev)) { dev_err(&adev->dev, "platform device creation failed: %ld\n", PTR_ERR(pdev));
- else
- } else {
if (acpi_dma_is_supported(adev))arch_setup_dma_ops(&pdev->dev, 0, 0, NULL,acpi_dma_is_coherent(adev));
OK, so I understand why this is needed, but ->
dev_dbg(&adev->dev, "created platform device %s\n", dev_name(&pdev->dev));
- }
kfree(resources); return pdev; diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 849b699..0976dc2 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -11,6 +11,7 @@ #include <linux/kthread.h> #include <linux/dmi.h> #include <linux/nls.h> +#include <linux/dma-mapping.h> #include <asm/pgtable.h> @@ -2137,6 +2138,46 @@ void acpi_free_pnp_ids(struct acpi_device_pnp *pnp) kfree(pnp->unique_id); } +static void acpi_init_coherency(struct acpi_device *adev) +{
- unsigned long long cca = 0;
- acpi_status status;
- struct acpi_device *parent = adev->parent;
- struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
- if (parent && parent->flags.cca_seen) {
/** From ACPI spec, OSPM will ignore _CCA if an ancestor* already saw one.*/adev->flags.cca_seen = 1;cca = acpi_dma_is_coherent(parent);- } else {
status = acpi_evaluate_integer(adev->handle, "_CCA",NULL, &cca);if (ACPI_SUCCESS(status)) {adev->flags.cca_seen = 1;} else if (!IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED)) {/** If architecture does not specify that _CCA is* required for DMA-able devices (e.g. x86),* we default to _CCA=1.*/cca = 1;} else {acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &buffer);pr_debug("ACPI device %s is missing _CCA.\n",(char *) buffer.pointer);kfree(buffer.pointer);}- }
- adev->flags.is_coherent = cca;
- if (acpi_dma_is_supported(adev))
arch_setup_dma_ops(&adev->dev, 0, 0, NULL,acpi_dma_is_coherent(adev));
Why do we need this one? adev->dev is not a device, it is an ACPI namespace node representation. Why do you want to set up DMA ops for it?
Would you set up DMA ops for a struct device_node?
+}
void acpi_init_device_object(struct acpi_device *device, acpi_handle handle, int type, unsigned long long sta) { @@ -2155,6 +2196,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle, device->flags.visited = false; device_initialize(&device->dev); dev_set_uevent_suppress(&device->dev, true);
- acpi_init_coherency(device);
} void acpi_device_add_finalize(struct acpi_device *device) diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 8de4fa9..17fb630 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -208,7 +208,9 @@ struct acpi_device_flags { u32 visited:1; u32 hotplug_notify:1; u32 is_dock_station:1;
- u32 reserved:23;
- u32 is_coherent:1;
- u32 cca_seen:1;
- u32 reserved:21;
}; /* File System */ @@ -380,6 +382,34 @@ struct acpi_device { void (*remove)(struct acpi_device *); }; +static inline bool acpi_dma_is_supported(struct acpi_device *adev) +{
- /**
* Currently, we mainly support _CCA=1 (i.e. is_coherent=1)* This should be equivalent to specifyig dma-coherent for* a device in OF.** For the case when _CCA=0 (i.e. is_coherent=0 && cca_seen=1),* we would rely on arch-specific cache maintenance for* non-coherence DMA operations if architecture specifies* _XXX_SUPPORT_CCA_ZERO. Otherwise, we do not support* DMA on this device and fallback to arch-specific default* handling.** For the case when _CCA is missing (i.e. cca_seen=0) but* platform specifies ACPI_CCA_REQUIRED, we do not support DMA,* and fallback to arch-specific default handling.*/- return adev && (adev->flags.is_coherent ||
(adev->flags.cca_seen &&IS_ENABLED(CONFIG_ARM64_SUPPORT_ACPI_CCA_ZERO)));
So what exactly would be wrong with using IS_ENABLED(CONFIG_ARM64) here?
+}
+static inline bool acpi_dma_is_coherent(struct acpi_device *adev) +{
- return adev && adev->flags.is_coherent;
+}
static inline bool is_acpi_node(struct fwnode_handle *fwnode) { return fwnode && fwnode->type == FWNODE_ACPI; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index b10c4a6..baccf3b 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -583,6 +583,16 @@ static inline int acpi_device_modalias(struct device *dev, return -ENODEV; } +static inline bool acpi_dma_is_supported(struct acpi_device *adev) +{
- return false;
+}
+static inline bool acpi_dma_is_coherent(struct acpi_device *adev) +{
- return false;
+}
#define ACPI_PTR(_ptr) (NULL) #endif /* !CONFIG_ACPI */
On Fri, May 08, 2015 at 10:53:59PM +0200, Rafael J. Wysocki wrote:
On Thursday, May 07, 2015 07:37:12 PM Suravee Suthikulpanit wrote:
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index ab2cbb5..7822149 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -54,6 +54,12 @@ config ACPI_GENERIC_GSI config ACPI_SYSTEM_POWER_STATES_SUPPORT bool +config ACPI_CCA_REQUIRED
- bool
+config ARM64_SUPPORT_ACPI_CCA_ZERO
Hmm. I guess the Arnd's idea what to simply use CONFIG_ARM64 directly instead of adding this new option.
I agree.
+static inline bool acpi_dma_is_supported(struct acpi_device *adev) +{
- /**
* Currently, we mainly support _CCA=1 (i.e. is_coherent=1)* This should be equivalent to specifyig dma-coherent for* a device in OF.** For the case when _CCA=0 (i.e. is_coherent=0 && cca_seen=1),* we would rely on arch-specific cache maintenance for* non-coherence DMA operations if architecture specifies* _XXX_SUPPORT_CCA_ZERO. Otherwise, we do not support* DMA on this device and fallback to arch-specific default* handling.** For the case when _CCA is missing (i.e. cca_seen=0) but* platform specifies ACPI_CCA_REQUIRED, we do not support DMA,* and fallback to arch-specific default handling.*/- return adev && (adev->flags.is_coherent ||
(adev->flags.cca_seen &&IS_ENABLED(CONFIG_ARM64_SUPPORT_ACPI_CCA_ZERO)));So what exactly would be wrong with using IS_ENABLED(CONFIG_ARM64) here?
I'm not sure I follow why we need to check for ARM64 here at all. Can we not just have something like:
return adev && (!IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED) || adev->flags.cca_seen)
On Monday, May 11, 2015 05:16:27 PM Catalin Marinas wrote:
On Fri, May 08, 2015 at 10:53:59PM +0200, Rafael J. Wysocki wrote:
On Thursday, May 07, 2015 07:37:12 PM Suravee Suthikulpanit wrote:
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index ab2cbb5..7822149 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -54,6 +54,12 @@ config ACPI_GENERIC_GSI config ACPI_SYSTEM_POWER_STATES_SUPPORT bool +config ACPI_CCA_REQUIRED
- bool
+config ARM64_SUPPORT_ACPI_CCA_ZERO
Hmm. I guess the Arnd's idea what to simply use CONFIG_ARM64 directly instead of adding this new option.
I agree.
+static inline bool acpi_dma_is_supported(struct acpi_device *adev) +{
- /**
* Currently, we mainly support _CCA=1 (i.e. is_coherent=1)* This should be equivalent to specifyig dma-coherent for* a device in OF.** For the case when _CCA=0 (i.e. is_coherent=0 && cca_seen=1),* we would rely on arch-specific cache maintenance for* non-coherence DMA operations if architecture specifies* _XXX_SUPPORT_CCA_ZERO. Otherwise, we do not support* DMA on this device and fallback to arch-specific default* handling.** For the case when _CCA is missing (i.e. cca_seen=0) but* platform specifies ACPI_CCA_REQUIRED, we do not support DMA,* and fallback to arch-specific default handling.*/- return adev && (adev->flags.is_coherent ||
(adev->flags.cca_seen &&IS_ENABLED(CONFIG_ARM64_SUPPORT_ACPI_CCA_ZERO)));So what exactly would be wrong with using IS_ENABLED(CONFIG_ARM64) here?
I'm not sure I follow why we need to check for ARM64 here at all. Can we not just have something like:
return adev && (!IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED) || adev->flags.cca_seen)
If _CCA returns 0 on non-ARM64, DMA is not supported for this device, so in that case the function should return 'false' while the above check will make it return 'true'.
On 5/11/2015 8:20 PM, Rafael J. Wysocki wrote:
On Monday, May 11, 2015 05:16:27 PM Catalin Marinas wrote:
On Fri, May 08, 2015 at 10:53:59PM +0200, Rafael J. Wysocki wrote:
On Thursday, May 07, 2015 07:37:12 PM Suravee Suthikulpanit wrote:
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index ab2cbb5..7822149 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -54,6 +54,12 @@ config ACPI_GENERIC_GSI config ACPI_SYSTEM_POWER_STATES_SUPPORT bool
+config ACPI_CCA_REQUIRED
- bool
+config ARM64_SUPPORT_ACPI_CCA_ZERO
Hmm. I guess the Arnd's idea what to simply use CONFIG_ARM64 directly instead of adding this new option.
I agree.
+static inline bool acpi_dma_is_supported(struct acpi_device *adev) +{
- /**
* Currently, we mainly support _CCA=1 (i.e. is_coherent=1)* This should be equivalent to specifyig dma-coherent for* a device in OF.** For the case when _CCA=0 (i.e. is_coherent=0 && cca_seen=1),* we would rely on arch-specific cache maintenance for* non-coherence DMA operations if architecture specifies* _XXX_SUPPORT_CCA_ZERO. Otherwise, we do not support* DMA on this device and fallback to arch-specific default* handling.** For the case when _CCA is missing (i.e. cca_seen=0) but* platform specifies ACPI_CCA_REQUIRED, we do not support DMA,* and fallback to arch-specific default handling.*/- return adev && (adev->flags.is_coherent ||
(adev->flags.cca_seen &&IS_ENABLED(CONFIG_ARM64_SUPPORT_ACPI_CCA_ZERO)));So what exactly would be wrong with using IS_ENABLED(CONFIG_ARM64) here?
I'm not sure I follow why we need to check for ARM64 here at all. Can we not just have something like:
return adev && (!IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED) || adev->flags.cca_seen)
If _CCA returns 0 on non-ARM64, DMA is not supported for this device, so in that case the function should return 'false' while the above check will make it return 'true'.
The main idea is basically to allow architecture to decide if it wants to specify if it wants to support _CCA=0. Currently, there are two approaches. 1. Do not support and disable DMA 2. Support and default to what architecture would normally do for non-coherent DMA.
Since, ARM64 being the only platform, which supports ACPI and would support _CCA=0. I can just put CONFIG_ARM64 then as Arnd and Rafael mentioned.
Thanks, Suravee
From http://www.uefi.org/sites/default/files/resources/ACPI_6.0.pdf,
section 6.2.17 _CCA states that ARM platforms require ACPI _CCA object to be specified for DMA-cabpable devices. This patch introduces ACPI_MUST_HAVE_CCA in arm64 Kconfig to specify such requirement.
In case _CCA is missing, arm64 would assign dummy_dma_ops to disable DMA capability of the device.
Signed-off-by: Mark Salter msalter@redhat.com Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com --- arch/arm64/Kconfig | 2 + arch/arm64/include/asm/dma-mapping.h | 18 ++++++- arch/arm64/mm/dma-mapping.c | 92 ++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 4269dba..c7227e8 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1,5 +1,6 @@ config ARM64 def_bool y + select ACPI_CCA_REQUIRED if ACPI select ACPI_GENERIC_GSI if ACPI select ACPI_REDUCED_HARDWARE_ONLY if ACPI select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE @@ -19,6 +20,7 @@ config ARM64 select ARM_GIC_V2M if PCI_MSI select ARM_GIC_V3 select ARM_GIC_V3_ITS if PCI_MSI + select ARM64_SUPPORT_ACPI_CCA_ZERO if ACPI select BUILDTIME_EXTABLE_SORT select CLONE_BACKWARDS select COMMON_CLK diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h index 9437e3d..f0d6d0b 100644 --- a/arch/arm64/include/asm/dma-mapping.h +++ b/arch/arm64/include/asm/dma-mapping.h @@ -18,6 +18,7 @@
#ifdef __KERNEL__
+#include <linux/acpi.h> #include <linux/types.h> #include <linux/vmalloc.h>
@@ -28,13 +29,23 @@
#define DMA_ERROR_CODE (~(dma_addr_t)0) extern struct dma_map_ops *dma_ops; +extern struct dma_map_ops dummy_dma_ops;
static inline struct dma_map_ops *__generic_dma_ops(struct device *dev) { - if (unlikely(!dev) || !dev->archdata.dma_ops) + if (unlikely(!dev)) return dma_ops; - else + else if (dev->archdata.dma_ops) return dev->archdata.dma_ops; + else if (acpi_disabled) + return dma_ops; + + /* + * When ACPI is enabled, if arch_set_dma_ops is not called, + * we will disable device DMA capability by setting it + * to dummy_dma_ops. + */ + return &dummy_dma_ops; }
static inline struct dma_map_ops *get_dma_ops(struct device *dev) @@ -48,6 +59,9 @@ static inline struct dma_map_ops *get_dma_ops(struct device *dev) static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, struct iommu_ops *iommu, bool coherent) { + if (!acpi_disabled && !dev->archdata.dma_ops) + dev->archdata.dma_ops = dma_ops; + dev->archdata.dma_coherent = coherent; } #define arch_setup_dma_ops arch_setup_dma_ops diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index ef7d112..6e6d6ad 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -415,6 +415,98 @@ out: return -ENOMEM; }
+/******************************************** + * The following APIs are for dummy DMA ops * + ********************************************/ + +static void *__dummy_alloc(struct device *dev, size_t size, + dma_addr_t *dma_handle, gfp_t flags, + struct dma_attrs *attrs) +{ + return NULL; +} + +static void __dummy_free(struct device *dev, size_t size, + void *vaddr, dma_addr_t dma_handle, + struct dma_attrs *attrs) +{ +} + +static int __dummy_mmap(struct device *dev, + struct vm_area_struct *vma, + void *cpu_addr, dma_addr_t dma_addr, size_t size, + struct dma_attrs *attrs) +{ + return -ENXIO; +} + +static dma_addr_t __dummy_map_page(struct device *dev, struct page *page, + unsigned long offset, size_t size, + enum dma_data_direction dir, + struct dma_attrs *attrs) +{ + return DMA_ERROR_CODE; +} + +static void __dummy_unmap_page(struct device *dev, dma_addr_t dev_addr, + size_t size, enum dma_data_direction dir, + struct dma_attrs *attrs) +{ +} + +static int __dummy_map_sg(struct device *dev, struct scatterlist *sgl, + int nelems, enum dma_data_direction dir, + struct dma_attrs *attrs) +{ + return 0; +} + +static void __dummy_unmap_sg(struct device *dev, + struct scatterlist *sgl, int nelems, + enum dma_data_direction dir, + struct dma_attrs *attrs) +{ +} + +static void __dummy_sync_single(struct device *dev, + dma_addr_t dev_addr, size_t size, + enum dma_data_direction dir) +{ +} + +static void __dummy_sync_sg(struct device *dev, + struct scatterlist *sgl, int nelems, + enum dma_data_direction dir) +{ +} + +static int __dummy_mapping_error(struct device *hwdev, dma_addr_t dma_addr) +{ + return 1; +} + +static int __dummy_dma_supported(struct device *hwdev, u64 mask) +{ + return 0; +} + +struct dma_map_ops dummy_dma_ops = { + .alloc = __dummy_alloc, + .free = __dummy_free, + .mmap = __dummy_mmap, + .map_page = __dummy_map_page, + .unmap_page = __dummy_unmap_page, + .map_sg = __dummy_map_sg, + .unmap_sg = __dummy_unmap_sg, + .sync_single_for_cpu = __dummy_sync_single, + .sync_single_for_device = __dummy_sync_single, + .sync_sg_for_cpu = __dummy_sync_sg, + .sync_sg_for_device = __dummy_sync_sg, + .mapping_error = __dummy_mapping_error, + .dma_supported = __dummy_dma_supported, +}; +EXPORT_SYMBOL(dummy_dma_ops); + static int __init arm64_dma_init(void) { int ret;
On Thursday, May 07, 2015 07:37:13 PM Suravee Suthikulpanit wrote:
From http://www.uefi.org/sites/default/files/resources/ACPI_6.0.pdf, section 6.2.17 _CCA states that ARM platforms require ACPI _CCA object to be specified for DMA-cabpable devices. This patch introduces ACPI_MUST_HAVE_CCA in arm64 Kconfig to specify such requirement.
In case _CCA is missing, arm64 would assign dummy_dma_ops to disable DMA capability of the device.
Signed-off-by: Mark Salter msalter@redhat.com Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com
This won't go in without any ACKs from the ARM64 maintainers.
arch/arm64/Kconfig | 2 + arch/arm64/include/asm/dma-mapping.h | 18 ++++++- arch/arm64/mm/dma-mapping.c | 92 ++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 4269dba..c7227e8 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1,5 +1,6 @@ config ARM64 def_bool y
- select ACPI_CCA_REQUIRED if ACPI select ACPI_GENERIC_GSI if ACPI select ACPI_REDUCED_HARDWARE_ONLY if ACPI select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
@@ -19,6 +20,7 @@ config ARM64 select ARM_GIC_V2M if PCI_MSI select ARM_GIC_V3 select ARM_GIC_V3_ITS if PCI_MSI
- select ARM64_SUPPORT_ACPI_CCA_ZERO if ACPI select BUILDTIME_EXTABLE_SORT select CLONE_BACKWARDS select COMMON_CLK
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h index 9437e3d..f0d6d0b 100644 --- a/arch/arm64/include/asm/dma-mapping.h +++ b/arch/arm64/include/asm/dma-mapping.h @@ -18,6 +18,7 @@ #ifdef __KERNEL__ +#include <linux/acpi.h> #include <linux/types.h> #include <linux/vmalloc.h> @@ -28,13 +29,23 @@ #define DMA_ERROR_CODE (~(dma_addr_t)0) extern struct dma_map_ops *dma_ops; +extern struct dma_map_ops dummy_dma_ops; static inline struct dma_map_ops *__generic_dma_ops(struct device *dev) {
- if (unlikely(!dev) || !dev->archdata.dma_ops)
- if (unlikely(!dev)) return dma_ops;
- else
- else if (dev->archdata.dma_ops) return dev->archdata.dma_ops;
- else if (acpi_disabled)
return dma_ops;- /*
* When ACPI is enabled, if arch_set_dma_ops is not called,* we will disable device DMA capability by setting it* to dummy_dma_ops.*/- return &dummy_dma_ops;
} static inline struct dma_map_ops *get_dma_ops(struct device *dev) @@ -48,6 +59,9 @@ static inline struct dma_map_ops *get_dma_ops(struct device *dev) static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, struct iommu_ops *iommu, bool coherent) {
- if (!acpi_disabled && !dev->archdata.dma_ops)
dev->archdata.dma_ops = dma_ops;- dev->archdata.dma_coherent = coherent;
} #define arch_setup_dma_ops arch_setup_dma_ops diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index ef7d112..6e6d6ad 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -415,6 +415,98 @@ out: return -ENOMEM; } +/********************************************
- The following APIs are for dummy DMA ops *
- ********************************************/
+static void *__dummy_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t flags,struct dma_attrs *attrs)+{
- return NULL;
+}
+static void __dummy_free(struct device *dev, size_t size,
void *vaddr, dma_addr_t dma_handle,struct dma_attrs *attrs)+{ +}
+static int __dummy_mmap(struct device *dev,
struct vm_area_struct *vma,void *cpu_addr, dma_addr_t dma_addr, size_t size,struct dma_attrs *attrs)+{
- return -ENXIO;
+}
+static dma_addr_t __dummy_map_page(struct device *dev, struct page *page,
unsigned long offset, size_t size,enum dma_data_direction dir,struct dma_attrs *attrs)+{
- return DMA_ERROR_CODE;
+}
+static void __dummy_unmap_page(struct device *dev, dma_addr_t dev_addr,
size_t size, enum dma_data_direction dir,struct dma_attrs *attrs)+{ +}
+static int __dummy_map_sg(struct device *dev, struct scatterlist *sgl,
int nelems, enum dma_data_direction dir,struct dma_attrs *attrs)+{
- return 0;
+}
+static void __dummy_unmap_sg(struct device *dev,
struct scatterlist *sgl, int nelems,enum dma_data_direction dir,struct dma_attrs *attrs)+{ +}
+static void __dummy_sync_single(struct device *dev,
dma_addr_t dev_addr, size_t size,enum dma_data_direction dir)+{ +}
+static void __dummy_sync_sg(struct device *dev,
struct scatterlist *sgl, int nelems,enum dma_data_direction dir)+{ +}
+static int __dummy_mapping_error(struct device *hwdev, dma_addr_t dma_addr) +{
- return 1;
+}
+static int __dummy_dma_supported(struct device *hwdev, u64 mask) +{
- return 0;
+}
+struct dma_map_ops dummy_dma_ops = {
- .alloc = __dummy_alloc,
- .free = __dummy_free,
- .mmap = __dummy_mmap,
- .map_page = __dummy_map_page,
- .unmap_page = __dummy_unmap_page,
- .map_sg = __dummy_map_sg,
- .unmap_sg = __dummy_unmap_sg,
- .sync_single_for_cpu = __dummy_sync_single,
- .sync_single_for_device = __dummy_sync_single,
- .sync_sg_for_cpu = __dummy_sync_sg,
- .sync_sg_for_device = __dummy_sync_sg,
- .mapping_error = __dummy_mapping_error,
- .dma_supported = __dummy_dma_supported,
+}; +EXPORT_SYMBOL(dummy_dma_ops);
static int __init arm64_dma_init(void) { int ret;
On Thu, May 07, 2015 at 07:37:13PM -0500, Suravee Suthikulpanit wrote:
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 4269dba..c7227e8 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1,5 +1,6 @@ config ARM64 def_bool y
- select ACPI_CCA_REQUIRED if ACPI select ACPI_GENERIC_GSI if ACPI select ACPI_REDUCED_HARDWARE_ONLY if ACPI select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
@@ -19,6 +20,7 @@ config ARM64 select ARM_GIC_V2M if PCI_MSI select ARM_GIC_V3 select ARM_GIC_V3_ITS if PCI_MSI
- select ARM64_SUPPORT_ACPI_CCA_ZERO if ACPI
As per the other sub-thread, I don't think we need this option at all.
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h index 9437e3d..f0d6d0b 100644 --- a/arch/arm64/include/asm/dma-mapping.h +++ b/arch/arm64/include/asm/dma-mapping.h @@ -18,6 +18,7 @@ #ifdef __KERNEL__ +#include <linux/acpi.h> #include <linux/types.h> #include <linux/vmalloc.h> @@ -28,13 +29,23 @@ #define DMA_ERROR_CODE (~(dma_addr_t)0) extern struct dma_map_ops *dma_ops; +extern struct dma_map_ops dummy_dma_ops; static inline struct dma_map_ops *__generic_dma_ops(struct device *dev) {
- if (unlikely(!dev) || !dev->archdata.dma_ops)
- if (unlikely(!dev)) return dma_ops;
- else
- else if (dev->archdata.dma_ops) return dev->archdata.dma_ops;
- else if (acpi_disabled)
return dma_ops;- /*
* When ACPI is enabled, if arch_set_dma_ops is not called,* we will disable device DMA capability by setting it* to dummy_dma_ops.*/- return &dummy_dma_ops;
}
The code looks fine to me but Arnd had some comments that I didn't fully understand (dropping dummy_map_ops in favour of simply setting dma_mask to NULL; I don't think the existing swiotlb ops would behave in a way that always return NULL).
Currently, device drivers, which support both OF and ACPI, need to call two separate APIs, of_dma_is_coherent() and acpi_dma_is_coherent()) to determine device coherency attribute.
This patch simplifies this process by introducing a new device property API, device_dma_is_coherent(), which calls the appropriate interface based on the booting architecture.
Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com --- drivers/base/property.c | 12 ++++++++++++ include/linux/property.h | 2 ++ 2 files changed, 14 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c index 1d0b116..8123c6e 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -14,6 +14,7 @@ #include <linux/export.h> #include <linux/kernel.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/property.h>
/** @@ -519,3 +520,14 @@ unsigned int device_get_child_node_count(struct device *dev) return count; } EXPORT_SYMBOL_GPL(device_get_child_node_count); + +bool device_dma_is_coherent(struct device *dev) +{ + if (IS_ENABLED(CONFIG_OF) && dev->of_node) + return of_dma_is_coherent(dev->of_node); + else if (has_acpi_companion(dev)) + return acpi_dma_is_coherent(acpi_node(dev->fwnode)); + + return false; +} +EXPORT_SYMBOL_GPL(device_dma_is_coherent); diff --git a/include/linux/property.h b/include/linux/property.h index de8bdf4..76ebde9 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -164,4 +164,6 @@ struct property_set {
void device_add_property_set(struct device *dev, struct property_set *pset);
+bool device_dma_is_coherent(struct device *dev); + #endif /* _LINUX_PROPERTY_H_ */
On 5/7/15 5:37 PM, Suravee Suthikulpanit wrote:
Currently, device drivers, which support both OF and ACPI, need to call two separate APIs, of_dma_is_coherent() and acpi_dma_is_coherent()) to determine device coherency attribute.
This patch simplifies this process by introducing a new device property API, device_dma_is_coherent(), which calls the appropriate interface based on the booting architecture.
Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com
drivers/base/property.c | 12 ++++++++++++ include/linux/property.h | 2 ++ 2 files changed, 14 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c index 1d0b116..8123c6e 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -14,6 +14,7 @@ #include <linux/export.h> #include <linux/kernel.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/property.h>
/** @@ -519,3 +520,14 @@ unsigned int device_get_child_node_count(struct device *dev) return count; } EXPORT_SYMBOL_GPL(device_get_child_node_count);
+bool device_dma_is_coherent(struct device *dev) +{
- if (IS_ENABLED(CONFIG_OF) && dev->of_node)
Do you really need that IS_ENABLED(CONFIG_OF) ? In other words, dev->of_node should be null for !CONFIG_OF
Regards, Santosh
On Thursday, May 07, 2015 09:12:00 PM santosh.shilimkar@oracle.com wrote:
On 5/7/15 5:37 PM, Suravee Suthikulpanit wrote:
Currently, device drivers, which support both OF and ACPI, need to call two separate APIs, of_dma_is_coherent() and acpi_dma_is_coherent()) to determine device coherency attribute.
This patch simplifies this process by introducing a new device property API, device_dma_is_coherent(), which calls the appropriate interface based on the booting architecture.
Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com
drivers/base/property.c | 12 ++++++++++++ include/linux/property.h | 2 ++ 2 files changed, 14 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c index 1d0b116..8123c6e 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -14,6 +14,7 @@ #include <linux/export.h> #include <linux/kernel.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/property.h>
/** @@ -519,3 +520,14 @@ unsigned int device_get_child_node_count(struct device *dev) return count; } EXPORT_SYMBOL_GPL(device_get_child_node_count);
+bool device_dma_is_coherent(struct device *dev) +{
- if (IS_ENABLED(CONFIG_OF) && dev->of_node)
Do you really need that IS_ENABLED(CONFIG_OF) ? In other words, dev->of_node should be null for !CONFIG_OF
Yes, but IS_ENABLED(CONFIG_OF) causes the check to be optimized away by the compiler if CONFIG_OF is not enabled.
On 5/8/2015 1:49 PM, Rafael J. Wysocki wrote:
On Thursday, May 07, 2015 09:12:00 PM santosh.shilimkar@oracle.com wrote:
On 5/7/15 5:37 PM, Suravee Suthikulpanit wrote:
Currently, device drivers, which support both OF and ACPI, need to call two separate APIs, of_dma_is_coherent() and acpi_dma_is_coherent()) to determine device coherency attribute.
This patch simplifies this process by introducing a new device property API, device_dma_is_coherent(), which calls the appropriate interface based on the booting architecture.
Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com
drivers/base/property.c | 12 ++++++++++++ include/linux/property.h | 2 ++ 2 files changed, 14 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c index 1d0b116..8123c6e 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -14,6 +14,7 @@ #include <linux/export.h> #include <linux/kernel.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/property.h>
/** @@ -519,3 +520,14 @@ unsigned int device_get_child_node_count(struct device *dev) return count; } EXPORT_SYMBOL_GPL(device_get_child_node_count);
+bool device_dma_is_coherent(struct device *dev) +{
- if (IS_ENABLED(CONFIG_OF) && dev->of_node)
Do you really need that IS_ENABLED(CONFIG_OF) ? In other words, dev->of_node should be null for !CONFIG_OF
Yes, but IS_ENABLED(CONFIG_OF) causes the check to be optimized away by the compiler if CONFIG_OF is not enabled.
Sure but my point was why you need it when just 'dev->of_node' check is enough. May be I missed something.
Regards, Santosh
On Friday, May 08, 2015 01:27:00 PM santosh shilimkar wrote:
On 5/8/2015 1:49 PM, Rafael J. Wysocki wrote:
On Thursday, May 07, 2015 09:12:00 PM santosh.shilimkar@oracle.com wrote:
On 5/7/15 5:37 PM, Suravee Suthikulpanit wrote:
Currently, device drivers, which support both OF and ACPI, need to call two separate APIs, of_dma_is_coherent() and acpi_dma_is_coherent()) to determine device coherency attribute.
This patch simplifies this process by introducing a new device property API, device_dma_is_coherent(), which calls the appropriate interface based on the booting architecture.
Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com
drivers/base/property.c | 12 ++++++++++++ include/linux/property.h | 2 ++ 2 files changed, 14 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c index 1d0b116..8123c6e 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -14,6 +14,7 @@ #include <linux/export.h> #include <linux/kernel.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/property.h>
/** @@ -519,3 +520,14 @@ unsigned int device_get_child_node_count(struct device *dev) return count; } EXPORT_SYMBOL_GPL(device_get_child_node_count);
+bool device_dma_is_coherent(struct device *dev) +{
- if (IS_ENABLED(CONFIG_OF) && dev->of_node)
Do you really need that IS_ENABLED(CONFIG_OF) ? In other words, dev->of_node should be null for !CONFIG_OF
Yes, but IS_ENABLED(CONFIG_OF) causes the check to be optimized away by the compiler if CONFIG_OF is not enabled.
Sure but my point was why you need it when just 'dev->of_node' check is enough. May be I missed something.
dev->of_node is present when CONFIG_OF is not enabled too. Without the IS_ENABLED(CONFIG_OF) this becomes a pointless pointer check that will always evaluate to 'false' on systems without CONFIG_OF, AFAICS.
On 5/8/2015 1:58 PM, Rafael J. Wysocki wrote:
On Friday, May 08, 2015 01:27:00 PM santosh shilimkar wrote:
On 5/8/2015 1:49 PM, Rafael J. Wysocki wrote:
On Thursday, May 07, 2015 09:12:00 PM santosh.shilimkar@oracle.com wrote:
On 5/7/15 5:37 PM, Suravee Suthikulpanit wrote:
Currently, device drivers, which support both OF and ACPI, need to call two separate APIs, of_dma_is_coherent() and acpi_dma_is_coherent()) to determine device coherency attribute.
This patch simplifies this process by introducing a new device property API, device_dma_is_coherent(), which calls the appropriate interface based on the booting architecture.
Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com
drivers/base/property.c | 12 ++++++++++++ include/linux/property.h | 2 ++ 2 files changed, 14 insertions(+)diff --git a/drivers/base/property.c b/drivers/base/property.c index 1d0b116..8123c6e 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -14,6 +14,7 @@ #include <linux/export.h> #include <linux/kernel.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/property.h>
/**@@ -519,3 +520,14 @@ unsigned int device_get_child_node_count(struct device *dev) return count; } EXPORT_SYMBOL_GPL(device_get_child_node_count);
+bool device_dma_is_coherent(struct device *dev) +{
- if (IS_ENABLED(CONFIG_OF) && dev->of_node)
Do you really need that IS_ENABLED(CONFIG_OF) ? In other words, dev->of_node should be null for !CONFIG_OF
Yes, but IS_ENABLED(CONFIG_OF) causes the check to be optimized away by the compiler if CONFIG_OF is not enabled.
Sure but my point was why you need it when just 'dev->of_node' check is enough. May be I missed something.
dev->of_node is present when CONFIG_OF is not enabled too. Without the IS_ENABLED(CONFIG_OF) this becomes a pointless pointer check that will always evaluate to 'false' on systems without CONFIG_OF, AFAICS.
Got it now. Thanks for expanding it.
Regards, Santosh
Currently, the driver has separate logic to determine device coherency for DT vs ACPI. This patch simplifies the code with a call to device_dma_is_coherent().
Signed-off-by: Tom Lendacky thomas.lendacky@amd.com Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com --- drivers/crypto/ccp/ccp-platform.c | 60 +-------------------------------------- 1 file changed, 1 insertion(+), 59 deletions(-)
diff --git a/drivers/crypto/ccp/ccp-platform.c b/drivers/crypto/ccp/ccp-platform.c index b1c20b2..e446781 100644 --- a/drivers/crypto/ccp/ccp-platform.c +++ b/drivers/crypto/ccp/ccp-platform.c @@ -90,58 +90,6 @@ static struct resource *ccp_find_mmio_area(struct ccp_device *ccp) return NULL; }
-#ifdef CONFIG_ACPI -static int ccp_acpi_support(struct ccp_device *ccp) -{ - struct ccp_platform *ccp_platform = ccp->dev_specific; - struct acpi_device *adev = ACPI_COMPANION(ccp->dev); - acpi_handle handle; - acpi_status status; - unsigned long long data; - int cca; - - /* Retrieve the device cache coherency value */ - handle = adev->handle; - do { - status = acpi_evaluate_integer(handle, "_CCA", NULL, &data); - if (!ACPI_FAILURE(status)) { - cca = data; - break; - } - } while (!ACPI_FAILURE(status)); - - if (ACPI_FAILURE(status)) { - dev_err(ccp->dev, "error obtaining acpi coherency value\n"); - return -EINVAL; - } - - ccp_platform->coherent = !!cca; - - return 0; -} -#else /* CONFIG_ACPI */ -static int ccp_acpi_support(struct ccp_device *ccp) -{ - return -EINVAL; -} -#endif - -#ifdef CONFIG_OF -static int ccp_of_support(struct ccp_device *ccp) -{ - struct ccp_platform *ccp_platform = ccp->dev_specific; - - ccp_platform->coherent = of_dma_is_coherent(ccp->dev->of_node); - - return 0; -} -#else -static int ccp_of_support(struct ccp_device *ccp) -{ - return -EINVAL; -} -#endif - static int ccp_platform_probe(struct platform_device *pdev) { struct ccp_device *ccp; @@ -182,13 +130,7 @@ static int ccp_platform_probe(struct platform_device *pdev) goto e_err; }
- if (ccp_platform->use_acpi) - ret = ccp_acpi_support(ccp); - else - ret = ccp_of_support(ccp); - if (ret) - goto e_err; - + ccp_platform->coherent = device_dma_is_coherent(ccp->dev); if (ccp_platform->coherent) ccp->axcache = CACHE_WB_NO_ALLOC; else
Currently, amd-xgbe driver has separate logic to determine device coherency for DT vs. ACPI. This patch simplifies the code with a call to device_dma_is_coherent().
Signed-off-by: Tom Lendacky thomas.lendacky@amd.com Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com --- drivers/net/ethernet/amd/xgbe/xgbe-main.c | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-main.c b/drivers/net/ethernet/amd/xgbe/xgbe-main.c index 7149053..6d2c702 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-main.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-main.c @@ -168,13 +168,8 @@ static void xgbe_init_all_fptrs(struct xgbe_prv_data *pdata) #ifdef CONFIG_ACPI static int xgbe_acpi_support(struct xgbe_prv_data *pdata) { - struct acpi_device *adev = pdata->adev; struct device *dev = pdata->dev; u32 property; - acpi_handle handle; - acpi_status status; - unsigned long long data; - int cca; int ret;
/* Obtain the system clock setting */ @@ -195,24 +190,6 @@ static int xgbe_acpi_support(struct xgbe_prv_data *pdata) } pdata->ptpclk_rate = property;
- /* Retrieve the device cache coherency value */ - handle = adev->handle; - do { - status = acpi_evaluate_integer(handle, "_CCA", NULL, &data); - if (!ACPI_FAILURE(status)) { - cca = data; - break; - } - - status = acpi_get_parent(handle, &handle); - } while (!ACPI_FAILURE(status)); - - if (ACPI_FAILURE(status)) { - dev_err(dev, "error obtaining acpi coherency value\n"); - return -EINVAL; - } - pdata->coherent = !!cca; - return 0; } #else /* CONFIG_ACPI */ @@ -243,9 +220,6 @@ static int xgbe_of_support(struct xgbe_prv_data *pdata) } pdata->ptpclk_rate = clk_get_rate(pdata->ptpclk);
- /* Retrieve the device cache coherency value */ - pdata->coherent = of_dma_is_coherent(dev->of_node); - return 0; } #else /* CONFIG_OF */ @@ -364,6 +338,7 @@ static int xgbe_probe(struct platform_device *pdev) goto err_io;
/* Set the DMA coherency values */ + pdata->coherent = device_dma_is_coherent(pdata->dev); if (pdata->coherent) { pdata->axdomain = XGBE_DMA_OS_AXDOMAIN; pdata->arcache = XGBE_DMA_OS_ARCACHE;
-
Catalin Marinas -
Rafael J. Wysocki -
santosh shilimkar -
santosh.shilimkar＠oracle.com
-
Suravee Suthikulanit -
Suravee Suthikulpanit