diff mbox

[V4,1/6] ACPI / scan: Parse _CCA and setup device coherency

Message ID 1431724994-21601-2-git-send-email-Suravee.Suthikulpanit@amd.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

Suravee Suthikulpanit May 15, 2015, 9:23 p.m. UTC
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>
CC: Rafael J. Wysocki <rjw@rjwysocki.net>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Will Deacon <will.deacon@arm.com>
CC: Catalin Marinas <catalin.marinas@arm.com>
---
 drivers/acpi/Kconfig         |  3 +++
 drivers/acpi/acpi_platform.c | 10 +++++++---
 drivers/acpi/scan.c          | 40 ++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h      | 32 +++++++++++++++++++++++++++++++-
 include/linux/acpi.h         | 10 ++++++++++
 5 files changed, 91 insertions(+), 4 deletions(-)

Comments

Rafael J. Wysocki May 15, 2015, 11:53 p.m. UTC | #1
On Friday, May 15, 2015 04:23:09 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>
> CC: Rafael J. Wysocki <rjw@rjwysocki.net>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Will Deacon <will.deacon@arm.com>
> CC: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  drivers/acpi/Kconfig         |  3 +++
>  drivers/acpi/acpi_platform.c | 10 +++++++---
>  drivers/acpi/scan.c          | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h      | 32 +++++++++++++++++++++++++++++++-
>  include/linux/acpi.h         | 10 ++++++++++
>  5 files changed, 91 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index ab2cbb5..212735f 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -54,6 +54,9 @@ config ACPI_GENERIC_GSI
>  config ACPI_SYSTEM_POWER_STATES_SUPPORT
>  	bool
>  
> +config ACPI_CCA_REQUIRED
> +	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..f6bc438 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));

Shouldn't we generally do that in acpi_bind_one() for all bus types
that don't have specific handling rather than here?

>  		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..c56e66a 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,44 @@ 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);

Shouldn't the device's own _CCA take precedence?

> +	} 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 {

What about using acpi_handle_debug() here?

> +			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;
> +}
> +
>  void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
>  			     int type, unsigned long long sta)
>  {
> @@ -2155,6 +2194,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..2a05ffb 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;

I'd prefer to call this 'coherent_dma'.

> +	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),
> +	 * There are two approaches:
> +	 * 1. Do not support and disable DMA.
> +	 * 2. Support but rely on arch-specific cache maintenance for
> +	 * non-coherence DMA operations. ARM64 is one example.
> +	 *
> +	 * 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.
> +	 *
> +	 * See acpi_init_coherency() for more info.
> +	 */
> +	return adev && (adev->flags.is_coherent ||
> +			(adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64)));
> +}
> +
> +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 */
>
Suravee Suthikulpanit May 18, 2015, 10:38 p.m. UTC | #2
Hi Rafael,

On 5/15/2015 6:53 PM, Rafael J. Wysocki wrote:
> On Friday, May 15, 2015 04:23:09 PM Suravee Suthikulpanit wrote:
>> [...]
>> diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
>> index 4bf7559..f6bc438 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));
>
> Shouldn't we generally do that in acpi_bind_one() for all bus types
> that don't have specific handling rather than here?

I think that would also work, and makes sense. However, I'm not sure if 
this would help in the case when we are creating PCI end-point devices, 
since the CCA is specified at the host bridge node, and there is no ACPI 
companion for the end-point devices. It seems that patch 3/6 of this 
series is still needed.


>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> index 849b699..c56e66a 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,44 @@ 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);
>
> Shouldn't the device's own _CCA take precedence?
According to the ACPI specification, the parent's _CCA take precedence.

>
>> +	} 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 {
>
> What about using acpi_handle_debug() here?
Ok I can do that.

>> [...]
>> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
>> index 8de4fa9..2a05ffb 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;
>
> I'd prefer to call this 'coherent_dma'.

OK.

Thanks,

Suravee

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki May 19, 2015, 12:28 a.m. UTC | #3
On Monday, May 18, 2015 05:38:17 PM Suravee Suthikulanit wrote:
> Hi Rafael,
> 
> On 5/15/2015 6:53 PM, Rafael J. Wysocki wrote:
> > On Friday, May 15, 2015 04:23:09 PM Suravee Suthikulpanit wrote:
> >> [...]
> >> diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
> >> index 4bf7559..f6bc438 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));
> >
> > Shouldn't we generally do that in acpi_bind_one() for all bus types
> > that don't have specific handling rather than here?
> 
> I think that would also work, and makes sense. However, I'm not sure if 
> this would help in the case when we are creating PCI end-point devices, 
> since the CCA is specified at the host bridge node, and there is no ACPI 
> companion for the end-point devices. It seems that patch 3/6 of this 
> series is still needed.

Yes, PCI needs its own handling, but there are multiple bus types that
don't (SPI, I2C etc) in addition to the platform bus type.
Catalin Marinas May 20, 2015, 10:01 a.m. UTC | #4
On Fri, May 15, 2015 at 04:23:09PM -0500, Suravee Suthikulpanit wrote:
> +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),
> +	 * There are two approaches:
> +	 * 1. Do not support and disable DMA.
> +	 * 2. Support but rely on arch-specific cache maintenance for
> +	 * non-coherence DMA operations. ARM64 is one example.
> +	 *
> +	 * 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.
> +	 *
> +	 * See acpi_init_coherency() for more info.
> +	 */
> +	return adev && (adev->flags.is_coherent ||
> +			(adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64)));
> +}

I don't particularly like the check for CONFIG_ARM64 here but I
understand why it was added (I had the wrong impression that x86 can
cope with _CCA = 0).

Alternatively, we could leave it out (together with cca_seen) until
someone comes forward with a real use-case for _CCA = 0 on arm64. One
platform I'm aware of is Juno but even though it boot with ACPI, I
wouldn't call it a server platform.
Suravee Suthikulpanit May 20, 2015, 11:52 a.m. UTC | #5
On 5/20/2015 5:01 AM, Catalin Marinas wrote:
> On Fri, May 15, 2015 at 04:23:09PM -0500, Suravee Suthikulpanit wrote:
>> +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),
>> +	 * There are two approaches:
>> +	 * 1. Do not support and disable DMA.
>> +	 * 2. Support but rely on arch-specific cache maintenance for
>> +	 * non-coherence DMA operations. ARM64 is one example.
>> +	 *
>> +	 * 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.
>> +	 *
>> +	 * See acpi_init_coherency() for more info.
>> +	 */
>> +	return adev && (adev->flags.is_coherent ||
>> +			(adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64)));
>> +}
>
> I don't particularly like the check for CONFIG_ARM64 here but I
> understand why it was added (I had the wrong impression that x86 can
> cope with _CCA = 0).
>
> Alternatively, we could leave it out (together with cca_seen) until
> someone comes forward with a real use-case for _CCA = 0 on arm64. One
> platform I'm aware of is Juno but even though it boot with ACPI, I
> wouldn't call it a server platform.

Ok. That seems to be what Arnd would prefer as well.  Let's just leave 
the support for _CCA=0 out until it is needed then.

Thanks,
Suravee




--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann May 20, 2015, 12:04 p.m. UTC | #6
On Wednesday 20 May 2015 06:52:03 Suravee Suthikulanit wrote:
> On 5/20/2015 5:01 AM, Catalin Marinas wrote:
> > On Fri, May 15, 2015 at 04:23:09PM -0500, Suravee Suthikulpanit wrote:
> >> +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),
> >> +     * There are two approaches:
> >> +     * 1. Do not support and disable DMA.
> >> +     * 2. Support but rely on arch-specific cache maintenance for
> >> +     * non-coherence DMA operations. ARM64 is one example.
> >> +     *
> >> +     * 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.
> >> +     *
> >> +     * See acpi_init_coherency() for more info.
> >> +     */
> >> +    return adev && (adev->flags.is_coherent ||
> >> +                    (adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64)));
> >> +}
> >
> > I don't particularly like the check for CONFIG_ARM64 here but I
> > understand why it was added (I had the wrong impression that x86 can
> > cope with _CCA = 0).
> >
> > Alternatively, we could leave it out (together with cca_seen) until
> > someone comes forward with a real use-case for _CCA = 0 on arm64. One
> > platform I'm aware of is Juno but even though it boot with ACPI, I
> > wouldn't call it a server platform.
> 
> Ok. That seems to be what Arnd would prefer as well.  Let's just leave 
> the support for _CCA=0 out until it is needed then.
> 

Yes, that would be best (as I said repeatedly ;-) )

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Catalin Marinas May 21, 2015, 1:01 p.m. UTC | #7
On Wed, May 20, 2015 at 02:04:02PM +0200, Arnd Bergmann wrote:
> On Wednesday 20 May 2015 06:52:03 Suravee Suthikulanit wrote:
> > On 5/20/2015 5:01 AM, Catalin Marinas wrote:
> > > On Fri, May 15, 2015 at 04:23:09PM -0500, Suravee Suthikulpanit wrote:
> > >> +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),
> > >> +     * There are two approaches:
> > >> +     * 1. Do not support and disable DMA.
> > >> +     * 2. Support but rely on arch-specific cache maintenance for
> > >> +     * non-coherence DMA operations. ARM64 is one example.
> > >> +     *
> > >> +     * 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.
> > >> +     *
> > >> +     * See acpi_init_coherency() for more info.
> > >> +     */
> > >> +    return adev && (adev->flags.is_coherent ||
> > >> +                    (adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64)));
> > >> +}
> > >
> > > I don't particularly like the check for CONFIG_ARM64 here but I
> > > understand why it was added (I had the wrong impression that x86 can
> > > cope with _CCA = 0).
> > >
> > > Alternatively, we could leave it out (together with cca_seen) until
> > > someone comes forward with a real use-case for _CCA = 0 on arm64. One
> > > platform I'm aware of is Juno but even though it boot with ACPI, I
> > > wouldn't call it a server platform.
> > 
> > Ok. That seems to be what Arnd would prefer as well.  Let's just leave 
> > the support for _CCA=0 out until it is needed then.
> 
> Yes, that would be best (as I said repeatedly ;-) )

I'm sure it won't be long before someone asks for this feature ;).
Arnd Bergmann May 21, 2015, 1:24 p.m. UTC | #8
On Thursday 21 May 2015 14:01:16 Catalin Marinas wrote:
> On Wed, May 20, 2015 at 02:04:02PM +0200, Arnd Bergmann wrote:
> > On Wednesday 20 May 2015 06:52:03 Suravee Suthikulanit wrote:
> > > 
> > > Ok. That seems to be what Arnd would prefer as well.  Let's just leave 
> > > the support for _CCA=0 out until it is needed then.
> > 
> > Yes, that would be best (as I said repeatedly  )
> 
> I'm sure it won't be long before someone asks for this feature ;).

If nothing else, we get to publically shame them for building crappy
hardware then ;-)

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ab2cbb5..212735f 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -54,6 +54,9 @@  config ACPI_GENERIC_GSI
 config ACPI_SYSTEM_POWER_STATES_SUPPORT
 	bool
 
+config ACPI_CCA_REQUIRED
+	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..f6bc438 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..c56e66a 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,44 @@  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;
+}
+
 void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
 			     int type, unsigned long long sta)
 {
@@ -2155,6 +2194,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..2a05ffb 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),
+	 * There are two approaches:
+	 * 1. Do not support and disable DMA.
+	 * 2. Support but rely on arch-specific cache maintenance for
+	 * non-coherence DMA operations. ARM64 is one example.
+	 *
+	 * 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.
+	 *
+	 * See acpi_init_coherency() for more info.
+	 */
+	return adev && (adev->flags.is_coherent ||
+			(adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64)));
+}
+
+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 */