[RFC PATCH 0/2] PCI: get DMA configuration from parent device

[RFC PATCH 0/2] PCI: get DMA configuration from parent device

[Murali Karicheri]

Keystone PCI devices requires the dma_pfn_offset to be set correctly
so that the PCI devices get the right DMA mask to function. This
patch adds a helper function to get this configuration from the root
bridge's parent device. The probe.c code now calls this helper to set
the default dma configuration if the parent device is dma capable.

Typically, dma-ranges are defined in the DT node of the SoC and gets
updated in the root bridge's parent device structure. My original
patch for this was at [1] which was NACK-ed and this is an attempt
to implement a better solution. This may have side effects that I am
unware of. So sending as a RFC patch to get feedback before sending
the formal patch. Please review and provide me the comment so that
I can incorporate the same.

[1] http://www.gossamer-threads.com/lists/linux/kernel/2024591

Murali Karicheri (2):
  common: dma-mapping: introduce dma_get_parent_cfg() helper
  PCI: get device dma configuration from parent

 drivers/base/dma-mapping.c  |   18 ++++++++++++++++++
 drivers/pci/probe.c         |   20 +++++++++++++++++---
 include/linux/dma-mapping.h |    3 +++
 3 files changed, 38 insertions(+), 3 deletions(-)

-- 
1.7.9.5

[RFC PATCH 0/2] PCI: get DMA configuration from parent device

[Murali Karicheri]

Keystone PCI devices requires the dma_pfn_offset to be set correctly
so that the PCI devices get the right DMA mask to function. This
patch adds a helper function to get this configuration from the root
bridge's parent device. The probe.c code now calls this helper to set
the default dma configuration if the parent device is dma capable.

Typically, dma-ranges are defined in the DT node of the SoC and gets
updated in the root bridge's parent device structure. My original
patch for this was at [1] which was NACK-ed and this is an attempt
to implement a better solution. This may have side effects that I am
unware of. So sending as a RFC patch to get feedback before sending
the formal patch. Please review and provide me the comment so that
I can incorporate the same.

[1] http://www.gossamer-threads.com/lists/linux/kernel/2024591

Murali Karicheri (2):
  common: dma-mapping: introduce dma_get_parent_cfg() helper
  PCI: get device dma configuration from parent

 drivers/base/dma-mapping.c  |   18 ++++++++++++++++++
 drivers/pci/probe.c         |   20 +++++++++++++++++---
 include/linux/dma-mapping.h |    3 +++
 3 files changed, 38 insertions(+), 3 deletions(-)

-- 
1.7.9.5

[RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Murali Karicheri]

Now, in Kernel, parent device's DMA parameters has to be applied to
the child as is - to enable DMA support for the device. Usually this
is happened in places where parent device manually instantiates child
device such as in drivers/pci/probe.c (pci_device_add() for example).

Now DMA configuration is represented in device data structure not only
by DMA mask and DMA params, it also includes dma_pfn_offset at least.
Hence introduce common dma_get_parent_cfg() helper to apply dma
configuration from parent to child, and use __weak to allow arch to
override it if needed.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/base/dma-mapping.c  |   18 ++++++++++++++++++
 include/linux/dma-mapping.h |    3 +++
 2 files changed, 21 insertions(+)

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 9e8bbdd..5322426 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -339,3 +339,21 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
 	vunmap(cpu_addr);
 }
 #endif
+
+int __weak dma_get_parent_cfg(struct device *dev, struct device *parent)
+{
+	struct device *temp = parent;
+
+	if (!temp)
+		temp = dev->parent;
+
+	if (temp && is_device_dma_capable(temp)) {
+		dev->dma_mask	= temp->dma_mask;
+		dev->coherent_dma_mask = temp->coherent_dma_mask;
+		dev->dma_parms	= temp->dma_parms;
+		dev->dma_pfn_offset = temp->dma_pfn_offset;
+		return 0;
+	}
+	return -EINVAL;
+}
+EXPORT_SYMBOL(dma_get_parent_cfg);
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index d5d3881..eb080d6 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -127,6 +127,9 @@ static inline int dma_coerce_mask_and_coherent(struct device *dev, u64 mask)
 	return dma_set_mask_and_coherent(dev, mask);
 }
 
+extern int __weak dma_get_parent_cfg(struct device *dev,
+					struct device *parent);
+
 extern u64 dma_get_required_mask(struct device *dev);
 
 #ifndef set_arch_dma_coherent_ops
-- 
1.7.9.5

[RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Murali Karicheri]

Now, in Kernel, parent device's DMA parameters has to be applied to
the child as is - to enable DMA support for the device. Usually this
is happened in places where parent device manually instantiates child
device such as in drivers/pci/probe.c (pci_device_add() for example).

Now DMA configuration is represented in device data structure not only
by DMA mask and DMA params, it also includes dma_pfn_offset at least.
Hence introduce common dma_get_parent_cfg() helper to apply dma
configuration from parent to child, and use __weak to allow arch to
override it if needed.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/base/dma-mapping.c  |   18 ++++++++++++++++++
 include/linux/dma-mapping.h |    3 +++
 2 files changed, 21 insertions(+)

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 9e8bbdd..5322426 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -339,3 +339,21 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
 	vunmap(cpu_addr);
 }
 #endif
+
+int __weak dma_get_parent_cfg(struct device *dev, struct device *parent)
+{
+	struct device *temp = parent;
+
+	if (!temp)
+		temp = dev->parent;
+
+	if (temp && is_device_dma_capable(temp)) {
+		dev->dma_mask	= temp->dma_mask;
+		dev->coherent_dma_mask = temp->coherent_dma_mask;
+		dev->dma_parms	= temp->dma_parms;
+		dev->dma_pfn_offset = temp->dma_pfn_offset;
+		return 0;
+	}
+	return -EINVAL;
+}
+EXPORT_SYMBOL(dma_get_parent_cfg);
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index d5d3881..eb080d6 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -127,6 +127,9 @@ static inline int dma_coerce_mask_and_coherent(struct device *dev, u64 mask)
 	return dma_set_mask_and_coherent(dev, mask);
 }
 
+extern int __weak dma_get_parent_cfg(struct device *dev,
+					struct device *parent);
+
 extern u64 dma_get_required_mask(struct device *dev);
 
 #ifndef set_arch_dma_coherent_ops
-- 
1.7.9.5

[RFC PATCH 2/2] PCI: get device dma configuration from parent

[Murali Karicheri]

This patch makes use of the helper function dma_get_parent_cfg() to
get dma configuration from the parent of the root bus bridge device.
One particular comfiguration that is required for Keystone PCI devices
to function correctly is the dma_pfn_offset that has to be correctly
set in the pci device's device structure in order for getting the
dma_mask right for the device.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/pci/probe.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c8ca98c..8f4d55b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1519,6 +1519,8 @@ static void pci_init_capabilities(struct pci_dev *dev)
 
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 {
+	struct pci_bus *temp_bus = bus;
+	struct device *parent;
 	int ret;
 
 	pci_configure_device(dev);
@@ -1527,9 +1529,21 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	dev->dev.release = pci_release_dev;
 
 	set_dev_node(&dev->dev, pcibus_to_node(bus));
-	dev->dev.dma_mask = &dev->dma_mask;
-	dev->dev.dma_parms = &dev->dma_parms;
-	dev->dev.coherent_dma_mask = 0xffffffffull;
+	/*
+	 * look for host bridge and check if the dma parameters
+	 * can be copied from parent
+	 */
+	while (!pci_is_root_bus(temp_bus))
+		temp_bus = temp_bus->parent;
+
+	parent = temp_bus->bridge->parent;
+
+	/* override with parent dma configuration if available */
+	if (dma_get_parent_cfg(&dev->dev, parent) < 0) {
+		dev->dev.dma_mask = &dev->dma_mask;
+		dev->dev.dma_parms = &dev->dma_parms;
+		dev->dev.coherent_dma_mask = 0xffffffffull;
+	}
 
 	pci_set_dma_max_seg_size(dev, 65536);
 	pci_set_dma_seg_boundary(dev, 0xffffffff);
-- 
1.7.9.5

[RFC PATCH 2/2] PCI: get device dma configuration from parent

[Murali Karicheri]

This patch makes use of the helper function dma_get_parent_cfg() to
get dma configuration from the parent of the root bus bridge device.
One particular comfiguration that is required for Keystone PCI devices
to function correctly is the dma_pfn_offset that has to be correctly
set in the pci device's device structure in order for getting the
dma_mask right for the device.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/pci/probe.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c8ca98c..8f4d55b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1519,6 +1519,8 @@ static void pci_init_capabilities(struct pci_dev *dev)
 
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 {
+	struct pci_bus *temp_bus = bus;
+	struct device *parent;
 	int ret;
 
 	pci_configure_device(dev);
@@ -1527,9 +1529,21 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	dev->dev.release = pci_release_dev;
 
 	set_dev_node(&dev->dev, pcibus_to_node(bus));
-	dev->dev.dma_mask = &dev->dma_mask;
-	dev->dev.dma_parms = &dev->dma_parms;
-	dev->dev.coherent_dma_mask = 0xffffffffull;
+	/*
+	 * look for host bridge and check if the dma parameters
+	 * can be copied from parent
+	 */
+	while (!pci_is_root_bus(temp_bus))
+		temp_bus = temp_bus->parent;
+
+	parent = temp_bus->bridge->parent;
+
+	/* override with parent dma configuration if available */
+	if (dma_get_parent_cfg(&dev->dev, parent) < 0) {
+		dev->dev.dma_mask = &dev->dma_mask;
+		dev->dev.dma_parms = &dev->dma_parms;
+		dev->dev.coherent_dma_mask = 0xffffffffull;
+	}
 
 	pci_set_dma_max_seg_size(dev, 65536);
 	pci_set_dma_seg_boundary(dev, 0xffffffff);
-- 
1.7.9.5

Re: [RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Arnd Bergmann]

On Wednesday 17 December 2014 13:02:23 Murali Karicheri wrote:
> Now, in Kernel, parent device's DMA parameters has to be applied to
> the child as is - to enable DMA support for the device. Usually this
> is happened in places where parent device manually instantiates child
> device such as in drivers/pci/probe.c (pci_device_add() for example).
> 
> Now DMA configuration is represented in device data structure not only
> by DMA mask and DMA params, it also includes dma_pfn_offset at least.
> Hence introduce common dma_get_parent_cfg() helper to apply dma
> configuration from parent to child, and use __weak to allow arch to
> override it if needed.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>  drivers/base/dma-mapping.c  |   18 ++++++++++++++++++
>  include/linux/dma-mapping.h |    3 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index 9e8bbdd..5322426 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -339,3 +339,21 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
>  	vunmap(cpu_addr);
>  }
>  #endif
> +
> +int __weak dma_get_parent_cfg(struct device *dev, struct device *parent)
> +{
> +	struct device *temp = parent;
> +
> +	if (!temp)
> +		temp = dev->parent;
> +
> +	if (temp && is_device_dma_capable(temp)) {
> +		dev->dma_mask	= temp->dma_mask;
> +		dev->coherent_dma_mask = temp->coherent_dma_mask;

As discussed, setting the pointers like this is always wrong,
so don't do it.

What's wrong with using arch_setup_dma_ops() from PCI as suggested
previously?

	Arnd

[RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Arnd Bergmann]

On Wednesday 17 December 2014 13:02:23 Murali Karicheri wrote:
> Now, in Kernel, parent device's DMA parameters has to be applied to
> the child as is - to enable DMA support for the device. Usually this
> is happened in places where parent device manually instantiates child
> device such as in drivers/pci/probe.c (pci_device_add() for example).
> 
> Now DMA configuration is represented in device data structure not only
> by DMA mask and DMA params, it also includes dma_pfn_offset at least.
> Hence introduce common dma_get_parent_cfg() helper to apply dma
> configuration from parent to child, and use __weak to allow arch to
> override it if needed.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>  drivers/base/dma-mapping.c  |   18 ++++++++++++++++++
>  include/linux/dma-mapping.h |    3 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index 9e8bbdd..5322426 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -339,3 +339,21 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
>  	vunmap(cpu_addr);
>  }
>  #endif
> +
> +int __weak dma_get_parent_cfg(struct device *dev, struct device *parent)
> +{
> +	struct device *temp = parent;
> +
> +	if (!temp)
> +		temp = dev->parent;
> +
> +	if (temp && is_device_dma_capable(temp)) {
> +		dev->dma_mask	= temp->dma_mask;
> +		dev->coherent_dma_mask = temp->coherent_dma_mask;

As discussed, setting the pointers like this is always wrong,
so don't do it.

What's wrong with using arch_setup_dma_ops() from PCI as suggested
previously?

	Arnd

Re: [RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Murali Karicheri]

On 12/17/2014 04:56 PM, Arnd Bergmann wrote:
> On Wednesday 17 December 2014 13:02:23 Murali Karicheri wrote:
>> Now, in Kernel, parent device's DMA parameters has to be applied to
>> the child as is - to enable DMA support for the device. Usually this
>> is happened in places where parent device manually instantiates child
>> device such as in drivers/pci/probe.c (pci_device_add() for example).
>>
>> Now DMA configuration is represented in device data structure not only
>> by DMA mask and DMA params, it also includes dma_pfn_offset at least.
>> Hence introduce common dma_get_parent_cfg() helper to apply dma
>> configuration from parent to child, and use __weak to allow arch to
>> override it if needed.
>>
>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com>
>> ---
>>   drivers/base/dma-mapping.c  |   18 ++++++++++++++++++
>>   include/linux/dma-mapping.h |    3 +++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
>> index 9e8bbdd..5322426 100644
>> --- a/drivers/base/dma-mapping.c
>> +++ b/drivers/base/dma-mapping.c
>> @@ -339,3 +339,21 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
>>   	vunmap(cpu_addr);
>>   }
>>   #endif
>> +
>> +int __weak dma_get_parent_cfg(struct device *dev, struct device *parent)
>> +{
>> +	struct device *temp = parent;
>> +
>> +	if (!temp)
>> +		temp = dev->parent;
>> +
>> +	if (temp&&  is_device_dma_capable(temp)) {
>> +		dev->dma_mask	= temp->dma_mask;
>> +		dev->coherent_dma_mask = temp->coherent_dma_mask;
>
> As discussed, setting the pointers like this is always wrong,
> so don't do it.
>
> What's wrong with using arch_setup_dma_ops() from PCI as suggested
> previously?
>
> 	Arnd

Arnd,

Thanks for the review.

I had originally written a code based on that line as below. But 
dma-ranges property is also used by ppc and other architectures in the 
pci device DT node. So I wasn't sure how this code impact PCI driver 
functionality on those platforms. Hence used a simpler change as all 
that is needed for keystone is to get the dma_pfn_offset rightly set in 
the pci slave device.

Initially I had a function implemented as below for this in of_pci.c.

+ * of_pci_dma_configure - Setup DMA configuration for a pci device
+ * @dev:       pci device to apply DMA configuration
+ *
+ * Try to get devices's DMA configuration from DT and update it
+ * accordingly. This is a similar to of_dma_configure() for platform
+ * devices.
+ *
+ */
+void of_pci_dma_configure(struct pci_dev *dev)
+{
+       struct device *host_bridge, *parent;
+       struct pci_bus *bus = dev->bus;
+       u64 dma_addr, paddr, size;
+       int ret;
+
+       while (!pci_is_root_bus(bus))
+               bus = bus->parent;
+       host_bridge = bus->bridge;
+
+       parent = host_bridge->parent;
+       if (parent->of_node) {
+               /*
+                * if dma-coherent property exist, call arch hook to setup
+                * dma coherent operations.
+                */
+               if (of_dma_is_coherent(parent->of_node)) {
+                       set_arch_dma_coherent_ops(&dev->dev);
+                       dev_info(&dev->dev, "device is dma coherent\n");
+               }
+
+               /*
+                * if dma-ranges property doesn't exist - just return else
+                * setup the dma offset
+                */
+               ret = of_dma_get_range(parent->of_node, &dma_addr, 
&paddr, &size);
+               if (ret < 0) {
+                       dev_info(&dev->dev, "no dma range information to 
setup\n");
+                       printk("no dma range information to setup\n");
+                       return;
+               }
+
+               /* DMA ranges found. Calculate and set dma_pfn_offset */
+               dev->dev.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
+               dev_info(&dev->dev, "dma_pfn_offset(%#08lx)\n", 
dev->dev.dma_pfn_offset);
+       }
+}
+EXPORT_SYMBOL_GPL(of_pci_dma_configure);


and then called from pci/probe.c as

@@ -1527,6 +1528,9 @@ void pci_device_add(struct pci_dev *dev, struct 
pci_bus *bus)
         dev->dev.dma_parms = &dev->dma_parms;
         dev->dev.coherent_dma_mask = 0xffffffffull;

+       /* Get the DMA configuration from root bridge's parent if present */
+       of_pci_dma_configure(dev);

If you think this is a better way to implement this, I can work on a 
patch set using this approach. Let me know.

-- 
Murali Karicheri
Linux Kernel, Texas Instruments

[RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Murali Karicheri]

On 12/17/2014 04:56 PM, Arnd Bergmann wrote:
> On Wednesday 17 December 2014 13:02:23 Murali Karicheri wrote:
>> Now, in Kernel, parent device's DMA parameters has to be applied to
>> the child as is - to enable DMA support for the device. Usually this
>> is happened in places where parent device manually instantiates child
>> device such as in drivers/pci/probe.c (pci_device_add() for example).
>>
>> Now DMA configuration is represented in device data structure not only
>> by DMA mask and DMA params, it also includes dma_pfn_offset at least.
>> Hence introduce common dma_get_parent_cfg() helper to apply dma
>> configuration from parent to child, and use __weak to allow arch to
>> override it if needed.
>>
>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com>
>> ---
>>   drivers/base/dma-mapping.c  |   18 ++++++++++++++++++
>>   include/linux/dma-mapping.h |    3 +++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
>> index 9e8bbdd..5322426 100644
>> --- a/drivers/base/dma-mapping.c
>> +++ b/drivers/base/dma-mapping.c
>> @@ -339,3 +339,21 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
>>   	vunmap(cpu_addr);
>>   }
>>   #endif
>> +
>> +int __weak dma_get_parent_cfg(struct device *dev, struct device *parent)
>> +{
>> +	struct device *temp = parent;
>> +
>> +	if (!temp)
>> +		temp = dev->parent;
>> +
>> +	if (temp&&  is_device_dma_capable(temp)) {
>> +		dev->dma_mask	= temp->dma_mask;
>> +		dev->coherent_dma_mask = temp->coherent_dma_mask;
>
> As discussed, setting the pointers like this is always wrong,
> so don't do it.
>
> What's wrong with using arch_setup_dma_ops() from PCI as suggested
> previously?
>
> 	Arnd

Arnd,

Thanks for the review.

I had originally written a code based on that line as below. But 
dma-ranges property is also used by ppc and other architectures in the 
pci device DT node. So I wasn't sure how this code impact PCI driver 
functionality on those platforms. Hence used a simpler change as all 
that is needed for keystone is to get the dma_pfn_offset rightly set in 
the pci slave device.

Initially I had a function implemented as below for this in of_pci.c.

+ * of_pci_dma_configure - Setup DMA configuration for a pci device
+ * @dev:       pci device to apply DMA configuration
+ *
+ * Try to get devices's DMA configuration from DT and update it
+ * accordingly. This is a similar to of_dma_configure() for platform
+ * devices.
+ *
+ */
+void of_pci_dma_configure(struct pci_dev *dev)
+{
+       struct device *host_bridge, *parent;
+       struct pci_bus *bus = dev->bus;
+       u64 dma_addr, paddr, size;
+       int ret;
+
+       while (!pci_is_root_bus(bus))
+               bus = bus->parent;
+       host_bridge = bus->bridge;
+
+       parent = host_bridge->parent;
+       if (parent->of_node) {
+               /*
+                * if dma-coherent property exist, call arch hook to setup
+                * dma coherent operations.
+                */
+               if (of_dma_is_coherent(parent->of_node)) {
+                       set_arch_dma_coherent_ops(&dev->dev);
+                       dev_info(&dev->dev, "device is dma coherent\n");
+               }
+
+               /*
+                * if dma-ranges property doesn't exist - just return else
+                * setup the dma offset
+                */
+               ret = of_dma_get_range(parent->of_node, &dma_addr, 
&paddr, &size);
+               if (ret < 0) {
+                       dev_info(&dev->dev, "no dma range information to 
setup\n");
+                       printk("no dma range information to setup\n");
+                       return;
+               }
+
+               /* DMA ranges found. Calculate and set dma_pfn_offset */
+               dev->dev.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
+               dev_info(&dev->dev, "dma_pfn_offset(%#08lx)\n", 
dev->dev.dma_pfn_offset);
+       }
+}
+EXPORT_SYMBOL_GPL(of_pci_dma_configure);


and then called from pci/probe.c as

@@ -1527,6 +1528,9 @@ void pci_device_add(struct pci_dev *dev, struct 
pci_bus *bus)
         dev->dev.dma_parms = &dev->dma_parms;
         dev->dev.coherent_dma_mask = 0xffffffffull;

+       /* Get the DMA configuration from root bridge's parent if present */
+       of_pci_dma_configure(dev);

If you think this is a better way to implement this, I can work on a 
patch set using this approach. Let me know.

-- 
Murali Karicheri
Linux Kernel, Texas Instruments

Re: [RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Arnd Bergmann]

On Wednesday 17 December 2014 18:24:43 Murali Karicheri wrote:
> On 12/17/2014 04:56 PM, Arnd Bergmann wrote:
> > On Wednesday 17 December 2014 13:02:23 Murali Karicheri wrote:
> >
> > What's wrong with using arch_setup_dma_ops() from PCI as suggested
> > previously?
> >

+Will Deacon

> I had originally written a code based on that line as below. But 
> dma-ranges property is also used by ppc and other architectures in the 
> pci device DT node. So I wasn't sure how this code impact PCI driver 
> functionality on those platforms. Hence used a simpler change as all 
> that is needed for keystone is to get the dma_pfn_offset rightly set in 
> the pci slave device.

But in your patch, you don't call arch_setup_dma_ops() at all.

> Initially I had a function implemented as below for this in of_pci.c.
> 
> + * of_pci_dma_configure - Setup DMA configuration for a pci device
> + * @dev:       pci device to apply DMA configuration
> + *
> + * Try to get devices's DMA configuration from DT and update it
> + * accordingly. This is a similar to of_dma_configure() for platform
> + * devices.
> + *
> + */
> +void of_pci_dma_configure(struct pci_dev *dev)
> +{
> +       struct device *host_bridge, *parent;
> +       struct pci_bus *bus = dev->bus;
> +       u64 dma_addr, paddr, size;
> +       int ret;
> +
> +       while (!pci_is_root_bus(bus))
> +               bus = bus->parent;
> +       host_bridge = bus->bridge;
> +
> +       parent = host_bridge->parent;
> +       if (parent->of_node) {

so far it looks good, although we may want to introduce
a helper function to get the of_node.

> +               /*
> +                * if dma-coherent property exist, call arch hook to setup
> +                * dma coherent operations.
> +                */
> +               if (of_dma_is_coherent(parent->of_node)) {
> +                       set_arch_dma_coherent_ops(&dev->dev);
> +                       dev_info(&dev->dev, "device is dma coherent\n");
> +               }

set_arch_dma_coherent_ops no longer exists. Just keep the flag in a
local variable

> +               /*
> +                * if dma-ranges property doesn't exist - just return else
> +                * setup the dma offset
> +                */
> +               ret = of_dma_get_range(parent->of_node, &dma_addr, 
> &paddr, &size);
> +               if (ret < 0) {
> +                       dev_info(&dev->dev, "no dma range information to 
> setup\n");
> +                       printk("no dma range information to setup\n");
> +                       return;
> +               }
> +
> +               /* DMA ranges found. Calculate and set dma_pfn_offset */
> +               dev->dev.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
> +               dev_info(&dev->dev, "dma_pfn_offset(%#08lx)\n", 
> dev->dev.dma_pfn_offset);

Same for the offset and size here, then pass all of the above into
arch_setup_dma_ops. This is also where we need to hook up the iommu
support once we decide how to make that work with the ARM SMMU.

Will, I think we may have a problem on ARM64 now, since we only replaced
set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
this? Without that, we don't have any coherent operations on ARM64 any
more, unless I'm missing something.

	Arnd

[RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Arnd Bergmann]

On Wednesday 17 December 2014 18:24:43 Murali Karicheri wrote:
> On 12/17/2014 04:56 PM, Arnd Bergmann wrote:
> > On Wednesday 17 December 2014 13:02:23 Murali Karicheri wrote:
> >
> > What's wrong with using arch_setup_dma_ops() from PCI as suggested
> > previously?
> >

+Will Deacon

> I had originally written a code based on that line as below. But 
> dma-ranges property is also used by ppc and other architectures in the 
> pci device DT node. So I wasn't sure how this code impact PCI driver 
> functionality on those platforms. Hence used a simpler change as all 
> that is needed for keystone is to get the dma_pfn_offset rightly set in 
> the pci slave device.

But in your patch, you don't call arch_setup_dma_ops() at all.

> Initially I had a function implemented as below for this in of_pci.c.
> 
> + * of_pci_dma_configure - Setup DMA configuration for a pci device
> + * @dev:       pci device to apply DMA configuration
> + *
> + * Try to get devices's DMA configuration from DT and update it
> + * accordingly. This is a similar to of_dma_configure() for platform
> + * devices.
> + *
> + */
> +void of_pci_dma_configure(struct pci_dev *dev)
> +{
> +       struct device *host_bridge, *parent;
> +       struct pci_bus *bus = dev->bus;
> +       u64 dma_addr, paddr, size;
> +       int ret;
> +
> +       while (!pci_is_root_bus(bus))
> +               bus = bus->parent;
> +       host_bridge = bus->bridge;
> +
> +       parent = host_bridge->parent;
> +       if (parent->of_node) {

so far it looks good, although we may want to introduce
a helper function to get the of_node.

> +               /*
> +                * if dma-coherent property exist, call arch hook to setup
> +                * dma coherent operations.
> +                */
> +               if (of_dma_is_coherent(parent->of_node)) {
> +                       set_arch_dma_coherent_ops(&dev->dev);
> +                       dev_info(&dev->dev, "device is dma coherent\n");
> +               }

set_arch_dma_coherent_ops no longer exists. Just keep the flag in a
local variable

> +               /*
> +                * if dma-ranges property doesn't exist - just return else
> +                * setup the dma offset
> +                */
> +               ret = of_dma_get_range(parent->of_node, &dma_addr, 
> &paddr, &size);
> +               if (ret < 0) {
> +                       dev_info(&dev->dev, "no dma range information to 
> setup\n");
> +                       printk("no dma range information to setup\n");
> +                       return;
> +               }
> +
> +               /* DMA ranges found. Calculate and set dma_pfn_offset */
> +               dev->dev.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
> +               dev_info(&dev->dev, "dma_pfn_offset(%#08lx)\n", 
> dev->dev.dma_pfn_offset);

Same for the offset and size here, then pass all of the above into
arch_setup_dma_ops. This is also where we need to hook up the iommu
support once we decide how to make that work with the ARM SMMU.

Will, I think we may have a problem on ARM64 now, since we only replaced
set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
this? Without that, we don't have any coherent operations on ARM64 any
more, unless I'm missing something.

	Arnd

Re: [RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Catalin Marinas]

On Thu, Dec 18, 2014 at 12:09:17AM +0000, Arnd Bergmann wrote:
> Will, I think we may have a problem on ARM64 now, since we only replaced
> set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
> this? Without that, we don't have any coherent operations on ARM64 any
> more, unless I'm missing something.

Good point, I think Will forgot about arm64 (weird ;)).

I'll push this out since Will is on holiday, probably shortly after
-rc1.

-----8<--------------------------------------------

>From 40ec7fd94b4efbd5d74185a062433f71c6576e20 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Thu, 18 Dec 2014 17:13:49 +0000
Subject: [PATCH] arm64: Replace set_arch_dma_coherent_ops with
 arch_setup_dma_ops

Commit a3a60f81ee6f (dma-mapping: replace set_arch_dma_coherent_ops with
arch_setup_dma_ops) changes the of_dma_configure() arch dma_ops callback
to arch_setup_dma_ops but only the arch/arm code is updated. Subsequent
commit 97890ba9289c (dma-mapping: detect and configure IOMMU in
of_dma_configure) changes the arch_setup_dma_ops() prototype further to
handle iommu. The patch makes the corresponding arm64 changes.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/dma-mapping.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index d34189bceff7..9ce3e680ae1c 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -52,13 +52,14 @@ static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
 	dev->archdata.dma_ops = ops;
 }
 
-static inline int set_arch_dma_coherent_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)
 {
-	dev->archdata.dma_coherent = true;
-	set_dma_ops(dev, &coherent_swiotlb_dma_ops);
-	return 0;
+	dev->archdata.dma_coherent = coherent;
+	if (coherent)
+		set_dma_ops(dev, &coherent_swiotlb_dma_ops);
 }
-#define set_arch_dma_coherent_ops	set_arch_dma_coherent_ops
+#define arch_setup_dma_ops	arch_setup_dma_ops
 
 /* do not use this function in a driver */
 static inline bool is_device_dma_coherent(struct device *dev)

Re: [RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Catalin Marinas]

On Thu, Dec 18, 2014 at 12:09:17AM +0000, Arnd Bergmann wrote:
> Will, I think we may have a problem on ARM64 now, since we only replaced
> set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
> this? Without that, we don't have any coherent operations on ARM64 any
> more, unless I'm missing something.

Good point, I think Will forgot about arm64 (weird ;)).

I'll push this out since Will is on holiday, probably shortly after
-rc1.

-----8<--------------------------------------------

>From 40ec7fd94b4efbd5d74185a062433f71c6576e20 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Thu, 18 Dec 2014 17:13:49 +0000
Subject: [PATCH] arm64: Replace set_arch_dma_coherent_ops with
 arch_setup_dma_ops

Commit a3a60f81ee6f (dma-mapping: replace set_arch_dma_coherent_ops with
arch_setup_dma_ops) changes the of_dma_configure() arch dma_ops callback
to arch_setup_dma_ops but only the arch/arm code is updated. Subsequent
commit 97890ba9289c (dma-mapping: detect and configure IOMMU in
of_dma_configure) changes the arch_setup_dma_ops() prototype further to
handle iommu. The patch makes the corresponding arm64 changes.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/dma-mapping.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index d34189bceff7..9ce3e680ae1c 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -52,13 +52,14 @@ static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
 	dev->archdata.dma_ops = ops;
 }
 
-static inline int set_arch_dma_coherent_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)
 {
-	dev->archdata.dma_coherent = true;
-	set_dma_ops(dev, &coherent_swiotlb_dma_ops);
-	return 0;
+	dev->archdata.dma_coherent = coherent;
+	if (coherent)
+		set_dma_ops(dev, &coherent_swiotlb_dma_ops);
 }
-#define set_arch_dma_coherent_ops	set_arch_dma_coherent_ops
+#define arch_setup_dma_ops	arch_setup_dma_ops
 
 /* do not use this function in a driver */
 static inline bool is_device_dma_coherent(struct device *dev)

[RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Catalin Marinas]

On Thu, Dec 18, 2014 at 12:09:17AM +0000, Arnd Bergmann wrote:
> Will, I think we may have a problem on ARM64 now, since we only replaced
> set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
> this? Without that, we don't have any coherent operations on ARM64 any
> more, unless I'm missing something.

Good point, I think Will forgot about arm64 (weird ;)).

I'll push this out since Will is on holiday, probably shortly after
-rc1.

-----8<--------------------------------------------

>From 40ec7fd94b4efbd5d74185a062433f71c6576e20 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Thu, 18 Dec 2014 17:13:49 +0000
Subject: [PATCH] arm64: Replace set_arch_dma_coherent_ops with
 arch_setup_dma_ops

Commit a3a60f81ee6f (dma-mapping: replace set_arch_dma_coherent_ops with
arch_setup_dma_ops) changes the of_dma_configure() arch dma_ops callback
to arch_setup_dma_ops but only the arch/arm code is updated. Subsequent
commit 97890ba9289c (dma-mapping: detect and configure IOMMU in
of_dma_configure) changes the arch_setup_dma_ops() prototype further to
handle iommu. The patch makes the corresponding arm64 changes.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/dma-mapping.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index d34189bceff7..9ce3e680ae1c 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -52,13 +52,14 @@ static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
 	dev->archdata.dma_ops = ops;
 }
 
-static inline int set_arch_dma_coherent_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)
 {
-	dev->archdata.dma_coherent = true;
-	set_dma_ops(dev, &coherent_swiotlb_dma_ops);
-	return 0;
+	dev->archdata.dma_coherent = coherent;
+	if (coherent)
+		set_dma_ops(dev, &coherent_swiotlb_dma_ops);
 }
-#define set_arch_dma_coherent_ops	set_arch_dma_coherent_ops
+#define arch_setup_dma_ops	arch_setup_dma_ops
 
 /* do not use this function in a driver */
 static inline bool is_device_dma_coherent(struct device *dev)

Re: [RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Murali Karicheri]

On 12/17/2014 07:09 PM, Arnd Bergmann wrote:
> On Wednesday 17 December 2014 18:24:43 Murali Karicheri wrote:
>> On 12/17/2014 04:56 PM, Arnd Bergmann wrote:
>>> On Wednesday 17 December 2014 13:02:23 Murali Karicheri wrote:
>>>
>>> What's wrong with using arch_setup_dma_ops() from PCI as suggested
>>> previously?
>>>
>
> +Will Deacon
>
>> I had originally written a code based on that line as below. But
>> dma-ranges property is also used by ppc and other architectures in the
>> pci device DT node. So I wasn't sure how this code impact PCI driver
>> functionality on those platforms. Hence used a simpler change as all
>> that is needed for keystone is to get the dma_pfn_offset rightly set in
>> the pci slave device.
>
> But in your patch, you don't call arch_setup_dma_ops() at all.
>
>> Initially I had a function implemented as below for this in of_pci.c.
>>
>> + * of_pci_dma_configure - Setup DMA configuration for a pci device
>> + * @dev:       pci device to apply DMA configuration
>> + *
>> + * Try to get devices's DMA configuration from DT and update it
>> + * accordingly. This is a similar to of_dma_configure() for platform
>> + * devices.
>> + *
>> + */
>> +void of_pci_dma_configure(struct pci_dev *dev)
>> +{
>> +       struct device *host_bridge, *parent;
>> +       struct pci_bus *bus = dev->bus;
>> +       u64 dma_addr, paddr, size;
>> +       int ret;
>> +
>> +       while (!pci_is_root_bus(bus))
>> +               bus = bus->parent;
>> +       host_bridge = bus->bridge;
>> +
>> +       parent = host_bridge->parent;
>> +       if (parent->of_node) {
>
> so far it looks good, although we may want to introduce
> a helper function to get the of_node.

Will add

>
>> +               /*
>> +                * if dma-coherent property exist, call arch hook to setup
>> +                * dma coherent operations.
>> +                */
>> +               if (of_dma_is_coherent(parent->of_node)) {
>> +                       set_arch_dma_coherent_ops(&dev->dev);
>> +                       dev_info(&dev->dev, "device is dma coherent\n");
>> +               }
>
> set_arch_dma_coherent_ops no longer exists. Just keep the flag in a
> local variable

Ok.

>
>> +               /*
>> +                * if dma-ranges property doesn't exist - just return else
>> +                * setup the dma offset
>> +                */
>> +               ret = of_dma_get_range(parent->of_node,&dma_addr,
>> &paddr,&size);
>> +               if (ret<  0) {
>> +                       dev_info(&dev->dev, "no dma range information to
>> setup\n");
>> +                       printk("no dma range information to setup\n");
>> +                       return;
>> +               }
>> +
>> +               /* DMA ranges found. Calculate and set dma_pfn_offset */
>> +               dev->dev.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
>> +               dev_info(&dev->dev, "dma_pfn_offset(%#08lx)\n",
>> dev->dev.dma_pfn_offset);
>
> Same for the offset and size here, then pass all of the above into
> arch_setup_dma_ops. This is also where we need to hook up the iommu
> support once we decide how to make that work with the ARM SMMU.

Ok. I will make this similar to of_dma_configure() that can be called 
from pci/probe.c

Murali
>
> Will, I think we may have a problem on ARM64 now, since we only replaced
> set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
> this? Without that, we don't have any coherent operations on ARM64 any
> more, unless I'm missing something.
>
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Murali Karicheri
Linux Kernel, Texas Instruments

[RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Murali Karicheri]

On 12/17/2014 07:09 PM, Arnd Bergmann wrote:
> On Wednesday 17 December 2014 18:24:43 Murali Karicheri wrote:
>> On 12/17/2014 04:56 PM, Arnd Bergmann wrote:
>>> On Wednesday 17 December 2014 13:02:23 Murali Karicheri wrote:
>>>
>>> What's wrong with using arch_setup_dma_ops() from PCI as suggested
>>> previously?
>>>
>
> +Will Deacon
>
>> I had originally written a code based on that line as below. But
>> dma-ranges property is also used by ppc and other architectures in the
>> pci device DT node. So I wasn't sure how this code impact PCI driver
>> functionality on those platforms. Hence used a simpler change as all
>> that is needed for keystone is to get the dma_pfn_offset rightly set in
>> the pci slave device.
>
> But in your patch, you don't call arch_setup_dma_ops() at all.
>
>> Initially I had a function implemented as below for this in of_pci.c.
>>
>> + * of_pci_dma_configure - Setup DMA configuration for a pci device
>> + * @dev:       pci device to apply DMA configuration
>> + *
>> + * Try to get devices's DMA configuration from DT and update it
>> + * accordingly. This is a similar to of_dma_configure() for platform
>> + * devices.
>> + *
>> + */
>> +void of_pci_dma_configure(struct pci_dev *dev)
>> +{
>> +       struct device *host_bridge, *parent;
>> +       struct pci_bus *bus = dev->bus;
>> +       u64 dma_addr, paddr, size;
>> +       int ret;
>> +
>> +       while (!pci_is_root_bus(bus))
>> +               bus = bus->parent;
>> +       host_bridge = bus->bridge;
>> +
>> +       parent = host_bridge->parent;
>> +       if (parent->of_node) {
>
> so far it looks good, although we may want to introduce
> a helper function to get the of_node.

Will add

>
>> +               /*
>> +                * if dma-coherent property exist, call arch hook to setup
>> +                * dma coherent operations.
>> +                */
>> +               if (of_dma_is_coherent(parent->of_node)) {
>> +                       set_arch_dma_coherent_ops(&dev->dev);
>> +                       dev_info(&dev->dev, "device is dma coherent\n");
>> +               }
>
> set_arch_dma_coherent_ops no longer exists. Just keep the flag in a
> local variable

Ok.

>
>> +               /*
>> +                * if dma-ranges property doesn't exist - just return else
>> +                * setup the dma offset
>> +                */
>> +               ret = of_dma_get_range(parent->of_node,&dma_addr,
>> &paddr,&size);
>> +               if (ret<  0) {
>> +                       dev_info(&dev->dev, "no dma range information to
>> setup\n");
>> +                       printk("no dma range information to setup\n");
>> +                       return;
>> +               }
>> +
>> +               /* DMA ranges found. Calculate and set dma_pfn_offset */
>> +               dev->dev.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
>> +               dev_info(&dev->dev, "dma_pfn_offset(%#08lx)\n",
>> dev->dev.dma_pfn_offset);
>
> Same for the offset and size here, then pass all of the above into
> arch_setup_dma_ops. This is also where we need to hook up the iommu
> support once we decide how to make that work with the ARM SMMU.

Ok. I will make this similar to of_dma_configure() that can be called 
from pci/probe.c

Murali
>
> Will, I think we may have a problem on ARM64 now, since we only replaced
> set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
> this? Without that, we don't have any coherent operations on ARM64 any
> more, unless I'm missing something.
>
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Murali Karicheri
Linux Kernel, Texas Instruments

Re: [RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Will Deacon]

On Thu, Dec 18, 2014 at 05:20:12PM +0000, Catalin Marinas wrote:
> On Thu, Dec 18, 2014 at 12:09:17AM +0000, Arnd Bergmann wrote:
> > Will, I think we may have a problem on ARM64 now, since we only replaced
> > set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
> > this? Without that, we don't have any coherent operations on ARM64 any
> > more, unless I'm missing something.
> 
> Good point, I think Will forgot about arm64 (weird ;)).

Oops, I think arm64 didn't implement set_arch_dma_coherent_ops when I
started that series and I forgot to refresh it.

> I'll push this out since Will is on holiday, probably shortly after
> -rc1.
> 
> -----8<--------------------------------------------
> 
> From 40ec7fd94b4efbd5d74185a062433f71c6576e20 Mon Sep 17 00:00:00 2001
> From: Catalin Marinas <catalin.marinas@arm.com>
> Date: Thu, 18 Dec 2014 17:13:49 +0000
> Subject: [PATCH] arm64: Replace set_arch_dma_coherent_ops with
>  arch_setup_dma_ops
> 
> Commit a3a60f81ee6f (dma-mapping: replace set_arch_dma_coherent_ops with
> arch_setup_dma_ops) changes the of_dma_configure() arch dma_ops callback
> to arch_setup_dma_ops but only the arch/arm code is updated. Subsequent
> commit 97890ba9289c (dma-mapping: detect and configure IOMMU in
> of_dma_configure) changes the arch_setup_dma_ops() prototype further to
> handle iommu. The patch makes the corresponding arm64 changes.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/dma-mapping.h | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
> index d34189bceff7..9ce3e680ae1c 100644
> --- a/arch/arm64/include/asm/dma-mapping.h
> +++ b/arch/arm64/include/asm/dma-mapping.h
> @@ -52,13 +52,14 @@ static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
>  	dev->archdata.dma_ops = ops;
>  }
>  
> -static inline int set_arch_dma_coherent_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)
>  {
> -	dev->archdata.dma_coherent = true;
> -	set_dma_ops(dev, &coherent_swiotlb_dma_ops);
> -	return 0;
> +	dev->archdata.dma_coherent = coherent;
> +	if (coherent)
> +		set_dma_ops(dev, &coherent_swiotlb_dma_ops);
>  }
> -#define set_arch_dma_coherent_ops	set_arch_dma_coherent_ops
> +#define arch_setup_dma_ops	arch_setup_dma_ops

Acked-by: Will Deacon <will.deacon@arm.com>

Thanks for fixing this.

Will

Re: [RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Will Deacon]

On Thu, Dec 18, 2014 at 05:20:12PM +0000, Catalin Marinas wrote:
> On Thu, Dec 18, 2014 at 12:09:17AM +0000, Arnd Bergmann wrote:
> > Will, I think we may have a problem on ARM64 now, since we only replaced
> > set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
> > this? Without that, we don't have any coherent operations on ARM64 any
> > more, unless I'm missing something.
> 
> Good point, I think Will forgot about arm64 (weird ;)).

Oops, I think arm64 didn't implement set_arch_dma_coherent_ops when I
started that series and I forgot to refresh it.

> I'll push this out since Will is on holiday, probably shortly after
> -rc1.
> 
> -----8<--------------------------------------------
> 
> From 40ec7fd94b4efbd5d74185a062433f71c6576e20 Mon Sep 17 00:00:00 2001
> From: Catalin Marinas <catalin.marinas@arm.com>
> Date: Thu, 18 Dec 2014 17:13:49 +0000
> Subject: [PATCH] arm64: Replace set_arch_dma_coherent_ops with
>  arch_setup_dma_ops
> 
> Commit a3a60f81ee6f (dma-mapping: replace set_arch_dma_coherent_ops with
> arch_setup_dma_ops) changes the of_dma_configure() arch dma_ops callback
> to arch_setup_dma_ops but only the arch/arm code is updated. Subsequent
> commit 97890ba9289c (dma-mapping: detect and configure IOMMU in
> of_dma_configure) changes the arch_setup_dma_ops() prototype further to
> handle iommu. The patch makes the corresponding arm64 changes.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/dma-mapping.h | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
> index d34189bceff7..9ce3e680ae1c 100644
> --- a/arch/arm64/include/asm/dma-mapping.h
> +++ b/arch/arm64/include/asm/dma-mapping.h
> @@ -52,13 +52,14 @@ static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
>  	dev->archdata.dma_ops = ops;
>  }
>  
> -static inline int set_arch_dma_coherent_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)
>  {
> -	dev->archdata.dma_coherent = true;
> -	set_dma_ops(dev, &coherent_swiotlb_dma_ops);
> -	return 0;
> +	dev->archdata.dma_coherent = coherent;
> +	if (coherent)
> +		set_dma_ops(dev, &coherent_swiotlb_dma_ops);
>  }
> -#define set_arch_dma_coherent_ops	set_arch_dma_coherent_ops
> +#define arch_setup_dma_ops	arch_setup_dma_ops

Acked-by: Will Deacon <will.deacon@arm.com>

Thanks for fixing this.

Will

[RFC PATCH 1/2] common: dma-mapping: introduce dma_get_parent_cfg() helper

[Will Deacon]

On Thu, Dec 18, 2014 at 05:20:12PM +0000, Catalin Marinas wrote:
> On Thu, Dec 18, 2014 at 12:09:17AM +0000, Arnd Bergmann wrote:
> > Will, I think we may have a problem on ARM64 now, since we only replaced
> > set_arch_dma_coherent_ops on ARM32 but not ARM64. Can you send a fix for
> > this? Without that, we don't have any coherent operations on ARM64 any
> > more, unless I'm missing something.
> 
> Good point, I think Will forgot about arm64 (weird ;)).

Oops, I think arm64 didn't implement set_arch_dma_coherent_ops when I
started that series and I forgot to refresh it.

> I'll push this out since Will is on holiday, probably shortly after
> -rc1.
> 
> -----8<--------------------------------------------
> 
> From 40ec7fd94b4efbd5d74185a062433f71c6576e20 Mon Sep 17 00:00:00 2001
> From: Catalin Marinas <catalin.marinas@arm.com>
> Date: Thu, 18 Dec 2014 17:13:49 +0000
> Subject: [PATCH] arm64: Replace set_arch_dma_coherent_ops with
>  arch_setup_dma_ops
> 
> Commit a3a60f81ee6f (dma-mapping: replace set_arch_dma_coherent_ops with
> arch_setup_dma_ops) changes the of_dma_configure() arch dma_ops callback
> to arch_setup_dma_ops but only the arch/arm code is updated. Subsequent
> commit 97890ba9289c (dma-mapping: detect and configure IOMMU in
> of_dma_configure) changes the arch_setup_dma_ops() prototype further to
> handle iommu. The patch makes the corresponding arm64 changes.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/dma-mapping.h | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
> index d34189bceff7..9ce3e680ae1c 100644
> --- a/arch/arm64/include/asm/dma-mapping.h
> +++ b/arch/arm64/include/asm/dma-mapping.h
> @@ -52,13 +52,14 @@ static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
>  	dev->archdata.dma_ops = ops;
>  }
>  
> -static inline int set_arch_dma_coherent_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)
>  {
> -	dev->archdata.dma_coherent = true;
> -	set_dma_ops(dev, &coherent_swiotlb_dma_ops);
> -	return 0;
> +	dev->archdata.dma_coherent = coherent;
> +	if (coherent)
> +		set_dma_ops(dev, &coherent_swiotlb_dma_ops);
>  }
> -#define set_arch_dma_coherent_ops	set_arch_dma_coherent_ops
> +#define arch_setup_dma_ops	arch_setup_dma_ops

Acked-by: Will Deacon <will.deacon@arm.com>

Thanks for fixing this.

Will