[PATCH V5 0/9] PCI: ACPI: Setting up DMA coherency for PCI device from _CCA attribute

[PATCH V5 0/9] PCI: ACPI: Setting up DMA coherency for PCI device from _CCA attribute

[Suravee Suthikulpanit]

This patch series adds support to setup DMA coherency for PCI device using
the ACPI _CCA attribute. According to the ACPI spec, the _CCA attribute
is required for ARM64. Therefore, this patch is a pre-req for ACPI PCI
support for ARM64 which is currently in development.  Also, this should
not affect other architectures that does not define 
CONFIG_ACPI_CCA_REQUIRED, since the default value is coherent.

In the process, this series also introduces enum dev_dma_attr and a set
of APIs to query device DMA attribute. These APIs replace the obsolete
device_dma_is_coherent(), and acpi_check_dma().

I have also included a patch from Jeremy posted here:
    http://www.spinics.net/lists/linux-usb/msg128582.html

This patch series is now rebased from:
    https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next

This patch series has been tested on AMD Seattle RevB platform.
The git tree containing tested code and pre-req patches is available here:
    http://github.com/ssuthiku/linux.git pci-cca-v5

Changes from V4: (https://lkml.org/lkml/2015/10/21/535)
    * Clean up from Hanjun, Bjorn, and Thomas review comments
    * Rebased on top of Rafael's latest linux-next branch
    * Added patch 7/9 to fix the  pci_get_host_bridge_device leak
    * Added Acked-by from Bjorn
    * Added Reviewed-by from Hanjun

Changes from V3: (https://lkml.org/lkml/2015/8/26/389)
    * Clean up suggested by Bjorn
    * Introduce enum dev_dma_attr
    * Replace device_dma_is_coherent() and acpi_check_dma() with
      new APIs.

Changes from V2: (https://lkml.org/lkml/2015/8/25/549)
    * Return -ENOSUPP instead of -1 (per Rafael's suggestion)
    * Add WARN() when fail to setup DMA for PCI device when booting
      ACPI (per Arnd's suggestion)
    * Added Acked-by from Rob.
    * Minor clean up

Changes from V1: (https://lkml.org/lkml/2015/8/13/182)
    * Include patch 1 from Jeremy to enable support for _CCA=0
    * Clean up acpi_check_dma() per Bjorn suggestions
    * Split the original V1 patch into two patches (patch 3 and 4)

Jeremy Linton (1):
  ACPI: Honor ACPI _CCA attribute setting

Suravee Suthikulpanit (8):
  device property: Introducing enum dev_dma_attr
  ACPI: Adding DMA Attribute APIs for ACPI Device
  device property: Adding DMA Attribute APIs for Generic Devices
  device property: ACPI: Make use of the new DMA Attribute APIs
  device property: ACPI: Remove unused DMA APIs
  of/pci: Fix pci_get_host_bridge_device leak
  PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()
  PCI: ACPI: Add support for PCI device DMA coherency

 drivers/acpi/acpi_platform.c              |  7 +++++-
 drivers/acpi/glue.c                       |  8 +++---
 drivers/acpi/scan.c                       | 42 +++++++++++++++++++++++++++++++
 drivers/base/property.c                   | 32 +++++++++++++++++------
 drivers/crypto/ccp/ccp-platform.c         | 15 ++++++++---
 drivers/net/ethernet/amd/xgbe/xgbe-main.c |  8 +++++-
 drivers/of/of_pci.c                       | 20 ---------------
 drivers/pci/probe.c                       | 33 ++++++++++++++++++++++--
 include/acpi/acpi_bus.h                   | 36 +++-----------------------
 include/linux/acpi.h                      |  7 +++++-
 include/linux/of_pci.h                    |  3 ---
 include/linux/property.h                  | 10 +++++++-
 12 files changed, 144 insertions(+), 77 deletions(-)

-- 
2.1.0

[PATCH V5 1/9] ACPI: Honor ACPI _CCA attribute setting

[Suravee Suthikulpanit]

From: Jeremy Linton <jeremy.linton@arm.com>

ACPI configurations can now mark devices as noncoherent,
support that choice.

NOTE: This is required to support USB on ARM Juno Development Board.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Rob Herring <robh+dt@kernel.org>
CC: Will Deacon <will.deacon@arm.com>
CC: Rafael J. Wysocki <rjw@rjwysocki.net>
---
 include/acpi/acpi_bus.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index d11eff8..0f131d2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -407,7 +407,7 @@ static inline bool acpi_check_dma(struct acpi_device *adev, bool *coherent)
 	 * case 1. Do not support and disable DMA.
 	 * case 2. Support but rely on arch-specific cache maintenance for
 	 *         non-coherence DMA operations.
-	 * Currently, we implement case 1 above.
+	 * Currently, we implement case 2 above.
 	 *
 	 * For the case when _CCA is missing (i.e. cca_seen=0) and
 	 * platform specifies ACPI_CCA_REQUIRED, we do not support DMA,
@@ -415,7 +415,8 @@ static inline bool acpi_check_dma(struct acpi_device *adev, bool *coherent)
 	 *
 	 * See acpi_init_coherency() for more info.
 	 */
-	if (adev->flags.coherent_dma) {
+	if (adev->flags.coherent_dma ||
+	    (adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64))) {
 		ret = true;
 		if (coherent)
 			*coherent = adev->flags.coherent_dma;
-- 
2.1.0

[PATCH V5 2/9] device property: Introducing enum dev_dma_attr

[Suravee Suthikulpanit]

A device could have one of the following DMA attributes:
    * DMA not supported
    * DMA non-coherent
    * DMA coherent

So, this patch introduces enum dev_dma_attribute. This will be used by
new APIs introduced in later patches.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
CC: Rafael J. Wysocki <rjw@rjwysocki.net>
CC: Bjorn Helgaas <bhelgaas@google.com>
---
 include/linux/property.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 463de52..8eecf20 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -27,6 +27,12 @@ enum dev_prop_type {
 	DEV_PROP_MAX,
 };
 
+enum dev_dma_attr {
+	DEV_DMA_NOT_SUPPORTED,
+	DEV_DMA_NON_COHERENT,
+	DEV_DMA_COHERENT,
+};
+
 bool device_property_present(struct device *dev, const char *propname);
 int device_property_read_u8_array(struct device *dev, const char *propname,
 				  u8 *val, size_t nval);
-- 
2.1.0

[PATCH V5 3/9] ACPI: Adding DMA Attribute APIs for ACPI Device

[Suravee Suthikulpanit]

Adding acpi_get_dma_attr() to query DMA attributes of ACPI devices.
It returns the enum dev_dma_attr, which communicates DMA information
more clearly. This API replaces the acpi_check_dma(), which will be
removed in subsequent patch.

This patch also provides a convenient function, acpi_dma_supported(),
to check DMA support of the specified ACPI device.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
CC: Rafael J. Wysocki <rjw@rjwysocki.net>
---
 drivers/acpi/scan.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  3 +++
 include/linux/acpi.h    | 10 ++++++++++
 3 files changed, 55 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index daf9fc8..78d5f02 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1308,6 +1308,48 @@ void acpi_free_pnp_ids(struct acpi_device_pnp *pnp)
 	kfree(pnp->unique_id);
 }
 
+/**
+ * acpi_dma_supported - Check DMA support for the specified device.
+ * @adev: The pointer to acpi device
+ *
+ * Return false if DMA is not supported. Otherwise, return true
+ */
+bool acpi_dma_supported(struct acpi_device *adev)
+{
+	if (!adev)
+		return false;
+
+	if (adev->flags.cca_seen)
+		return true;
+
+	/*
+	* Per ACPI 6.0 sec 6.2.17, assume devices can do cache-coherent
+	* DMA on "Intel platforms".  Presumably that includes all x86 and
+	* ia64, and other arches will set CONFIG_ACPI_CCA_REQUIRED=y.
+	*/
+	if (!IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED))
+		return true;
+
+	return false;
+}
+
+/**
+ * acpi_get_dma_attr - Check the supported DMA attr for the specified device.
+ * @adev: The pointer to acpi device
+ *
+ * Return enum dev_dma_attr.
+ */
+enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
+{
+	if (!acpi_dma_supported(adev))
+		return DEV_DMA_NOT_SUPPORTED;
+
+	if (adev->flags.coherent_dma)
+		return DEV_DMA_COHERENT;
+	else
+		return DEV_DMA_NON_COHERENT;
+}
+
 static void acpi_init_coherency(struct acpi_device *adev)
 {
 	unsigned long long cca = 0;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 0f131d2..920b774 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -596,6 +596,9 @@ struct acpi_pci_root {
 
 /* helper */
 
+bool acpi_dma_supported(struct acpi_device *adev);
+enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
+
 struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
 					   u64 address, bool check_children);
 int acpi_is_root_bridge(acpi_handle);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 82f56bb..6527920 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -598,6 +598,16 @@ static inline bool acpi_check_dma(struct acpi_device *adev, bool *coherent)
 	return false;
 }
 
+static inline bool acpi_dma_supported(struct acpi_device *adev)
+{
+	return false;
+}
+
+static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
+{
+	return DEV_DMA_NOT_SUPPORTED;
+}
+
 #define ACPI_PTR(_ptr)	(NULL)
 
 #endif	/* !CONFIG_ACPI */
-- 
2.1.0

[PATCH V5 4/9] device property: Adding DMA Attribute APIs for Generic Devices

[Suravee Suthikulpanit]

The function device_dma_is_coherent() does not sufficiently
communicate device DMA attributes. Instead, this patch introduces
device_get_dma_attr(), which returns enum dev_dma_attr.
It replaces the acpi_check_dma(), which will be removed in
subsequent patch.

This also provides a convenient function, device_dma_supported(),
to check DMA support of the specified device.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
CC: Rafael J. Wysocki <rjw@rjwysocki.net>
---
 drivers/base/property.c  | 29 +++++++++++++++++++++++++++++
 include/linux/property.h |  4 ++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index de40623..05d57a2 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -611,6 +611,35 @@ bool device_dma_is_coherent(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(device_dma_is_coherent);
 
+bool device_dma_supported(struct device *dev)
+{
+	/* For DT, this is always supported.
+	 * For ACPI, this depends on CCA, which
+	 * is determined by the acpi_dma_supported().
+	 */
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+		return true;
+
+	return acpi_dma_supported(ACPI_COMPANION(dev));
+}
+EXPORT_SYMBOL_GPL(device_dma_supported);
+
+enum dev_dma_attr device_get_dma_attr(struct device *dev)
+{
+	enum dev_dma_attr attr = DEV_DMA_NOT_SUPPORTED;
+
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		if (of_dma_is_coherent(dev->of_node))
+			attr = DEV_DMA_COHERENT;
+		else
+			attr = DEV_DMA_NON_COHERENT;
+	} else
+		attr = acpi_get_dma_attr(ACPI_COMPANION(dev));
+
+	return attr;
+}
+EXPORT_SYMBOL_GPL(device_get_dma_attr);
+
 /**
  * device_get_phy_mode - Get phy mode for given device
  * @dev:	Pointer to the given device
diff --git a/include/linux/property.h b/include/linux/property.h
index 8eecf20..7200490 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -176,6 +176,10 @@ void device_add_property_set(struct device *dev, struct property_set *pset);
 
 bool device_dma_is_coherent(struct device *dev);
 
+bool device_dma_supported(struct device *dev);
+
+enum dev_dma_attr device_get_dma_attr(struct device *dev);
+
 int device_get_phy_mode(struct device *dev);
 
 void *device_get_mac_address(struct device *dev, char *addr, int alen);
-- 
2.1.0

[PATCH V5 5/9] device property: ACPI: Make use of the new DMA Attribute APIs

[Suravee Suthikulpanit]

Now that we have the new DMA attribute APIs, we can replace the older
acpi_check_dma() and device_dma_is_coherent().

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
CC: Rafael J. Wysocki <rjw@rjwysocki.net>
CC: Tom Lendacky <thomas.lendacky@amd.com>
CC: Herbert Xu <herbert@gondor.apana.org.au>
CC: David S. Miller <davem@davemloft.net>
---
 drivers/acpi/acpi_platform.c              |  7 ++++++-
 drivers/acpi/glue.c                       |  8 +++++---
 drivers/crypto/ccp/ccp-platform.c         | 15 +++++++++++----
 drivers/net/ethernet/amd/xgbe/xgbe-main.c |  8 +++++++-
 4 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index 06a67d5..296b7a1 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -103,7 +103,12 @@ 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 = acpi_check_dma(adev, NULL) ? DMA_BIT_MASK(32) : 0;
+
+	if (acpi_dma_supported(adev))
+		pdevinfo.dma_mask = DMA_BIT_MASK(32);
+	else
+		pdevinfo.dma_mask = 0;
+
 	pdev = platform_device_register_full(&pdevinfo);
 	if (IS_ERR(pdev))
 		dev_err(&adev->dev, "platform device creation failed: %ld\n",
diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 1470ae4..5ea5dc2 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -168,7 +168,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 	struct list_head *physnode_list;
 	unsigned int node_id;
 	int retval = -EINVAL;
-	bool coherent;
+	enum dev_dma_attr attr;
 
 	if (has_acpi_companion(dev)) {
 		if (acpi_dev) {
@@ -225,8 +225,10 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 	if (!has_acpi_companion(dev))
 		ACPI_COMPANION_SET(dev, acpi_dev);
 
-	if (acpi_check_dma(acpi_dev, &coherent))
-		arch_setup_dma_ops(dev, 0, 0, NULL, coherent);
+	attr = acpi_get_dma_attr(acpi_dev);
+	if (attr != DEV_DMA_NOT_SUPPORTED)
+		arch_setup_dma_ops(dev, 0, 0, NULL,
+				   attr == DEV_DMA_COHERENT);
 
 	acpi_physnode_link_name(physical_node_name, node_id);
 	retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
diff --git a/drivers/crypto/ccp/ccp-platform.c b/drivers/crypto/ccp/ccp-platform.c
index bb241c3..844118c 100644
--- a/drivers/crypto/ccp/ccp-platform.c
+++ b/drivers/crypto/ccp/ccp-platform.c
@@ -96,6 +96,7 @@ static int ccp_platform_probe(struct platform_device *pdev)
 	struct ccp_platform *ccp_platform;
 	struct device *dev = &pdev->dev;
 	struct acpi_device *adev = ACPI_COMPANION(dev);
+	enum dev_dma_attr attr;
 	struct resource *ior;
 	int ret;
 
@@ -122,18 +123,24 @@ static int ccp_platform_probe(struct platform_device *pdev)
 	}
 	ccp->io_regs = ccp->io_map;
 
-	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
-	if (ret) {
-		dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret);
+	attr = device_get_dma_attr(dev);
+	if (attr == DEV_DMA_NOT_SUPPORTED) {
+		dev_err(dev, "DMA is not supported");
 		goto e_err;
 	}
 
-	ccp_platform->coherent = device_dma_is_coherent(ccp->dev);
+	ccp_platform->coherent = (attr == DEV_DMA_COHERENT);
 	if (ccp_platform->coherent)
 		ccp->axcache = CACHE_WB_NO_ALLOC;
 	else
 		ccp->axcache = CACHE_NONE;
 
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
+	if (ret) {
+		dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret);
+		goto e_err;
+	}
+
 	dev_set_drvdata(dev, ccp);
 
 	ret = ccp_init(ccp);
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-main.c b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
index e83bd76..c607b3f 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-main.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
@@ -342,6 +342,7 @@ static int xgbe_probe(struct platform_device *pdev)
 	struct resource *res;
 	const char *phy_mode;
 	unsigned int i, phy_memnum, phy_irqnum;
+	enum dev_dma_attr attr;
 	int ret;
 
 	DBGPR("--> xgbe_probe\n");
@@ -609,7 +610,12 @@ static int xgbe_probe(struct platform_device *pdev)
 		goto err_io;
 
 	/* Set the DMA coherency values */
-	pdata->coherent = device_dma_is_coherent(pdata->dev);
+	attr = device_get_dma_attr(dev);
+	if (attr == DEV_DMA_NOT_SUPPORTED) {
+		dev_err(dev, "DMA is not supported");
+		goto err_io;
+	}
+	pdata->coherent = (attr == DEV_DMA_COHERENT);
 	if (pdata->coherent) {
 		pdata->axdomain = XGBE_DMA_OS_AXDOMAIN;
 		pdata->arcache = XGBE_DMA_OS_ARCACHE;
-- 
2.1.0

[PATCH V5 6/9] device property: ACPI: Remove unused DMA APIs

[Suravee Suthikulpanit]

These DMA APIs are replaced with the newer versions, which return
the enum dev_dma_attr. So, we can safely remove them.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
CC: Rafael J. Wysocki <rjw@rjwysocki.net>
---
 drivers/base/property.c  | 13 -------------
 include/acpi/acpi_bus.h  | 34 ----------------------------------
 include/linux/acpi.h     |  5 -----
 include/linux/property.h |  2 --
 4 files changed, 54 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 05d57a2..1325ff2 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -598,19 +598,6 @@ unsigned int device_get_child_node_count(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(device_get_child_node_count);
 
-bool device_dma_is_coherent(struct device *dev)
-{
-	bool coherent = false;
-
-	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
-		coherent = of_dma_is_coherent(dev->of_node);
-	else
-		acpi_check_dma(ACPI_COMPANION(dev), &coherent);
-
-	return coherent;
-}
-EXPORT_SYMBOL_GPL(device_dma_is_coherent);
-
 bool device_dma_supported(struct device *dev)
 {
 	/* For DT, this is always supported.
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 920b774..ad0a5ff 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -390,40 +390,6 @@ struct acpi_data_node {
 	struct completion kobj_done;
 };
 
-static inline bool acpi_check_dma(struct acpi_device *adev, bool *coherent)
-{
-	bool ret = false;
-
-	if (!adev)
-		return ret;
-
-	/**
-	 * Currently, we only support _CCA=1 (i.e. coherent_dma=1)
-	 * This should be equivalent to specifyig dma-coherent for
-	 * a device in OF.
-	 *
-	 * For the case when _CCA=0 (i.e. coherent_dma=0 && cca_seen=1),
-	 * There are two cases:
-	 * case 1. Do not support and disable DMA.
-	 * case 2. Support but rely on arch-specific cache maintenance for
-	 *         non-coherence DMA operations.
-	 * Currently, we implement case 2 above.
-	 *
-	 * For the case when _CCA is missing (i.e. cca_seen=0) and
-	 * platform specifies ACPI_CCA_REQUIRED, we do not support DMA,
-	 * and fallback to arch-specific default handling.
-	 *
-	 * See acpi_init_coherency() for more info.
-	 */
-	if (adev->flags.coherent_dma ||
-	    (adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64))) {
-		ret = true;
-		if (coherent)
-			*coherent = adev->flags.coherent_dma;
-	}
-	return ret;
-}
-
 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 6527920..fa2bbc0 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -593,11 +593,6 @@ static inline int acpi_device_modalias(struct device *dev,
 	return -ENODEV;
 }
 
-static inline bool acpi_check_dma(struct acpi_device *adev, bool *coherent)
-{
-	return false;
-}
-
 static inline bool acpi_dma_supported(struct acpi_device *adev)
 {
 	return false;
diff --git a/include/linux/property.h b/include/linux/property.h
index 7200490..0a3705a 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -174,8 +174,6 @@ struct property_set {
 
 void device_add_property_set(struct device *dev, struct property_set *pset);
 
-bool device_dma_is_coherent(struct device *dev);
-
 bool device_dma_supported(struct device *dev);
 
 enum dev_dma_attr device_get_dma_attr(struct device *dev);
-- 
2.1.0

[PATCH V5 7/9] of/pci: Fix pci_get_host_bridge_device leak

[Suravee Suthikulpanit]

In case of error, the current code return w/o calling
pci_put_host_bridge_device. This patch fixes this.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/of/of_pci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 5751dc5..a2f510c 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -129,10 +129,9 @@ void of_pci_dma_configure(struct pci_dev *pci_dev)
 	struct device *dev = &pci_dev->dev;
 	struct device *bridge = pci_get_host_bridge_device(pci_dev);
 
-	if (!bridge->parent)
-		return;
+	if (bridge->parent)
+		of_dma_configure(dev, bridge->parent->of_node);
 
-	of_dma_configure(dev, bridge->parent->of_node);
 	pci_put_host_bridge_device(bridge);
 }
 EXPORT_SYMBOL_GPL(of_pci_dma_configure);
-- 
2.1.0

[PATCH V5 8/9] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()

[Suravee Suthikulpanit]

This patch move of_pci_dma_configure() to a more generic
pci_dma_configure(), which can be extended by non-OF code (e.g. ACPI).

This has no functional change.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Acked-by: Rob Herring <robh+dt@kernel.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
CC: Rafael J. Wysocki <rjw@rjwysocki.net>
---
 drivers/of/of_pci.c    | 19 -------------------
 drivers/pci/probe.c    | 23 +++++++++++++++++++++--
 include/linux/of_pci.h |  3 ---
 3 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index a2f510c..b66ee4e 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -117,25 +117,6 @@ int of_get_pci_domain_nr(struct device_node *node)
 }
 EXPORT_SYMBOL_GPL(of_get_pci_domain_nr);
 
-/**
- * of_pci_dma_configure - Setup DMA configuration
- * @dev: ptr to pci_dev struct of the PCI device
- *
- * Function to update PCI devices's DMA configuration using the same
- * info from the OF node of host bridge's parent (if any).
- */
-void of_pci_dma_configure(struct pci_dev *pci_dev)
-{
-	struct device *dev = &pci_dev->dev;
-	struct device *bridge = pci_get_host_bridge_device(pci_dev);
-
-	if (bridge->parent)
-		of_dma_configure(dev, bridge->parent->of_node);
-
-	pci_put_host_bridge_device(bridge);
-}
-EXPORT_SYMBOL_GPL(of_pci_dma_configure);
-
 #if defined(CONFIG_OF_ADDRESS)
 /**
  * of_pci_get_host_bridge_resources - Parse PCI host bridge resources from DT
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8361d27..31e3eef 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -6,7 +6,7 @@
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/pci.h>
-#include <linux/of_pci.h>
+#include <linux/of_device.h>
 #include <linux/pci_hotplug.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -1633,6 +1633,25 @@ static void pci_set_msi_domain(struct pci_dev *dev)
 				   dev_get_msi_domain(&dev->bus->dev));
 }
 
+/**
+ * pci_dma_configure - Setup DMA configuration
+ * @dev: ptr to pci_dev struct of the PCI device
+ *
+ * Function to update PCI devices's DMA configuration using the same
+ * info from the OF node of host bridge's parent (if any).
+ */
+static void pci_dma_configure(struct pci_dev *dev)
+{
+	struct device *bridge = pci_get_host_bridge_device(dev);
+
+	if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
+		if (bridge->parent)
+			of_dma_configure(&dev->dev, bridge->parent->of_node);
+	}
+
+	pci_put_host_bridge_device(bridge);
+}
+
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 {
 	int ret;
@@ -1646,7 +1665,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	dev->dev.dma_mask = &dev->dma_mask;
 	dev->dev.dma_parms = &dev->dma_parms;
 	dev->dev.coherent_dma_mask = 0xffffffffull;
-	of_pci_dma_configure(dev);
+	pci_dma_configure(dev);
 
 	pci_set_dma_max_seg_size(dev, 65536);
 	pci_set_dma_seg_boundary(dev, 0xffffffff);
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29fd3fe..ce0e5ab 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -16,7 +16,6 @@ int of_pci_get_devfn(struct device_node *np);
 int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin);
 int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
 int of_get_pci_domain_nr(struct device_node *node);
-void of_pci_dma_configure(struct pci_dev *pci_dev);
 #else
 static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq)
 {
@@ -51,8 +50,6 @@ of_get_pci_domain_nr(struct device_node *node)
 {
 	return -1;
 }
-
-static inline void of_pci_dma_configure(struct pci_dev *pci_dev) { }
 #endif
 
 #if defined(CONFIG_OF_ADDRESS)
-- 
2.1.0

[PATCH V5 9/9] PCI: ACPI: Add support for PCI device DMA coherency

[Suravee Suthikulpanit]

This patch adds support for setting up PCI device DMA coherency from
ACPI _CCA object that should normally be specified in the DSDT node
of its PCI host bridge.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
CC: Rafael J. Wysocki <rjw@rjwysocki.net>
---
 drivers/pci/probe.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 31e3eef..40eed54 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/cpumask.h>
 #include <linux/pci-aspm.h>
+#include <linux/acpi.h>
 #include <asm-generic/pci-bridge.h>
 #include "pci.h"
 
@@ -1638,7 +1639,7 @@ static void pci_set_msi_domain(struct pci_dev *dev)
  * @dev: ptr to pci_dev struct of the PCI device
  *
  * Function to update PCI devices's DMA configuration using the same
- * info from the OF node of host bridge's parent (if any).
+ * info from the OF node or ACPI node of host bridge's parent (if any).
  */
 static void pci_dma_configure(struct pci_dev *dev)
 {
@@ -1647,6 +1648,15 @@ static void pci_dma_configure(struct pci_dev *dev)
 	if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
 		if (bridge->parent)
 			of_dma_configure(&dev->dev, bridge->parent->of_node);
+	} else if (has_acpi_companion(bridge)) {
+		struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
+		enum dev_dma_attr attr = acpi_get_dma_attr(adev);
+
+		if (attr == DEV_DMA_NOT_SUPPORTED)
+			dev_warn(&dev->dev, "DMA not supported.\n");
+		else
+			arch_setup_dma_ops(&dev->dev, 0, 0, NULL,
+					   attr == DEV_DMA_COHERENT);
 	}
 
 	pci_put_host_bridge_device(bridge);
-- 
2.1.0

Re: [PATCH V5 0/9] PCI: ACPI: Setting up DMA coherency for PCI device from _CCA attribute

[Hanjun Guo]

On 2015/10/29 6:50, Suravee Suthikulpanit wrote:
> This patch series adds support to setup DMA coherency for PCI device using
> the ACPI _CCA attribute. According to the ACPI spec, the _CCA attribute
> is required for ARM64. Therefore, this patch is a pre-req for ACPI PCI
> support for ARM64 which is currently in development.  Also, this should
> not affect other architectures that does not define 
> CONFIG_ACPI_CCA_REQUIRED, since the default value is coherent.
>
> In the process, this series also introduces enum dev_dma_attr and a set
> of APIs to query device DMA attribute. These APIs replace the obsolete
> device_dma_is_coherent(), and acpi_check_dma().
>
> I have also included a patch from Jeremy posted here:
>     http://www.spinics.net/lists/linux-usb/msg128582.html
>
> This patch series is now rebased from:
>     https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
>
> This patch series has been tested on AMD Seattle RevB platform.
> The git tree containing tested code and pre-req patches is available here:
>     http://github.com/ssuthiku/linux.git pci-cca-v5
>
> Changes from V4: (https://lkml.org/lkml/2015/10/21/535)
>     * Clean up from Hanjun, Bjorn, and Thomas review comments
>     * Rebased on top of Rafael's latest linux-next branch
>     * Added patch 7/9 to fix the  pci_get_host_bridge_device leak
>     * Added Acked-by from Bjorn
>     * Added Reviewed-by from Hanjun
>
> Changes from V3: (https://lkml.org/lkml/2015/8/26/389)
>     * Clean up suggested by Bjorn
>     * Introduce enum dev_dma_attr
>     * Replace device_dma_is_coherent() and acpi_check_dma() with
>       new APIs.
>
> Changes from V2: (https://lkml.org/lkml/2015/8/25/549)
>     * Return -ENOSUPP instead of -1 (per Rafael's suggestion)
>     * Add WARN() when fail to setup DMA for PCI device when booting
>       ACPI (per Arnd's suggestion)
>     * Added Acked-by from Rob.
>     * Minor clean up
>
> Changes from V1: (https://lkml.org/lkml/2015/8/13/182)
>     * Include patch 1 from Jeremy to enable support for _CCA=0
>     * Clean up acpi_check_dma() per Bjorn suggestions
>     * Split the original V1 patch into two patches (patch 3 and 4)
>
> Jeremy Linton (1):
>   ACPI: Honor ACPI _CCA attribute setting
>
> Suravee Suthikulpanit (8):
>   device property: Introducing enum dev_dma_attr
>   ACPI: Adding DMA Attribute APIs for ACPI Device
>   device property: Adding DMA Attribute APIs for Generic Devices
>   device property: ACPI: Make use of the new DMA Attribute APIs
>   device property: ACPI: Remove unused DMA APIs
>   of/pci: Fix pci_get_host_bridge_device leak
>   PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()
>   PCI: ACPI: Add support for PCI device DMA coherency
>
>  drivers/acpi/acpi_platform.c              |  7 +++++-
>  drivers/acpi/glue.c                       |  8 +++---
>  drivers/acpi/scan.c                       | 42 +++++++++++++++++++++++++++++++
>  drivers/base/property.c                   | 32 +++++++++++++++++------
>  drivers/crypto/ccp/ccp-platform.c         | 15 ++++++++---
>  drivers/net/ethernet/amd/xgbe/xgbe-main.c |  8 +++++-
>  drivers/of/of_pci.c                       | 20 ---------------
>  drivers/pci/probe.c                       | 33 ++++++++++++++++++++++--
>  include/acpi/acpi_bus.h                   | 36 +++-----------------------
>  include/linux/acpi.h                      |  7 +++++-
>  include/linux/of_pci.h                    |  3 ---
>  include/linux/property.h                  | 10 +++++++-
>  12 files changed, 144 insertions(+), 77 deletions(-)

I think I already add my reviewed-by, but again, for this patch set:

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

Re: [PATCH V5 0/9] PCI: ACPI: Setting up DMA coherency for PCI device from _CCA attribute

[Rafael J. Wysocki]

On Wednesday, October 28, 2015 03:50:45 PM Suravee Suthikulpanit wrote:
> This patch series adds support to setup DMA coherency for PCI device using
> the ACPI _CCA attribute. According to the ACPI spec, the _CCA attribute
> is required for ARM64. Therefore, this patch is a pre-req for ACPI PCI
> support for ARM64 which is currently in development.  Also, this should
> not affect other architectures that does not define 
> CONFIG_ACPI_CCA_REQUIRED, since the default value is coherent.
> 
> In the process, this series also introduces enum dev_dma_attr and a set
> of APIs to query device DMA attribute. These APIs replace the obsolete
> device_dma_is_coherent(), and acpi_check_dma().
> 
> I have also included a patch from Jeremy posted here:
>     http://www.spinics.net/lists/linux-usb/msg128582.html
> 
> This patch series is now rebased from:
>     https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> 
> This patch series has been tested on AMD Seattle RevB platform.
> The git tree containing tested code and pre-req patches is available here:
>     http://github.com/ssuthiku/linux.git pci-cca-v5
> 
> Changes from V4: (https://lkml.org/lkml/2015/10/21/535)
>     * Clean up from Hanjun, Bjorn, and Thomas review comments
>     * Rebased on top of Rafael's latest linux-next branch
>     * Added patch 7/9 to fix the  pci_get_host_bridge_device leak
>     * Added Acked-by from Bjorn
>     * Added Reviewed-by from Hanjun
> 
> Changes from V3: (https://lkml.org/lkml/2015/8/26/389)
>     * Clean up suggested by Bjorn
>     * Introduce enum dev_dma_attr
>     * Replace device_dma_is_coherent() and acpi_check_dma() with
>       new APIs.
> 
> Changes from V2: (https://lkml.org/lkml/2015/8/25/549)
>     * Return -ENOSUPP instead of -1 (per Rafael's suggestion)
>     * Add WARN() when fail to setup DMA for PCI device when booting
>       ACPI (per Arnd's suggestion)
>     * Added Acked-by from Rob.
>     * Minor clean up
> 
> Changes from V1: (https://lkml.org/lkml/2015/8/13/182)
>     * Include patch 1 from Jeremy to enable support for _CCA=0
>     * Clean up acpi_check_dma() per Bjorn suggestions
>     * Split the original V1 patch into two patches (patch 3 and 4)
> 
> Jeremy Linton (1):
>   ACPI: Honor ACPI _CCA attribute setting
> 
> Suravee Suthikulpanit (8):
>   device property: Introducing enum dev_dma_attr
>   ACPI: Adding DMA Attribute APIs for ACPI Device
>   device property: Adding DMA Attribute APIs for Generic Devices
>   device property: ACPI: Make use of the new DMA Attribute APIs
>   device property: ACPI: Remove unused DMA APIs
>   of/pci: Fix pci_get_host_bridge_device leak
>   PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()
>   PCI: ACPI: Add support for PCI device DMA coherency
> 
>  drivers/acpi/acpi_platform.c              |  7 +++++-
>  drivers/acpi/glue.c                       |  8 +++---
>  drivers/acpi/scan.c                       | 42 +++++++++++++++++++++++++++++++
>  drivers/base/property.c                   | 32 +++++++++++++++++------
>  drivers/crypto/ccp/ccp-platform.c         | 15 ++++++++---
>  drivers/net/ethernet/amd/xgbe/xgbe-main.c |  8 +++++-
>  drivers/of/of_pci.c                       | 20 ---------------
>  drivers/pci/probe.c                       | 33 ++++++++++++++++++++++--
>  include/acpi/acpi_bus.h                   | 36 +++-----------------------
>  include/linux/acpi.h                      |  7 +++++-
>  include/linux/of_pci.h                    |  3 ---
>  include/linux/property.h                  | 10 +++++++-
>  12 files changed, 144 insertions(+), 77 deletions(-)

Queued up for v4.4, but I'll include it into my second pull request in the
second half of the merge window.

Thanks,
Rafael

Re: [PATCH V5 0/9] PCI: ACPI: Setting up DMA coherency for PCI device from _CCA attribute

[Suravee Suthikulanit]

On 11/1/2015 7:01 PM, Rafael J. Wysocki wrote:
> Queued up for v4.4, but I'll include it into my second pull request in the
> second half of the merge window.
>
> Thanks,
> Rafael

Thank you,
Suravee

Re: [PATCH V5 8/9] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()

[Robin Murphy]

Hi Suravee,

On 28/10/15 22:50, Suravee Suthikulpanit wrote:
> This patch move of_pci_dma_configure() to a more generic
> pci_dma_configure(), which can be extended by non-OF code (e.g. ACPI).
>
> This has no functional change.

Unfortunately, it appears that it does...

> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Acked-by: Rob Herring <robh+dt@kernel.org>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
> CC: Rafael J. Wysocki <rjw@rjwysocki.net>
> ---
>   drivers/of/of_pci.c    | 19 -------------------
>   drivers/pci/probe.c    | 23 +++++++++++++++++++++--
>   include/linux/of_pci.h |  3 ---
>   3 files changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index a2f510c..b66ee4e 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -117,25 +117,6 @@ int of_get_pci_domain_nr(struct device_node *node)
>   }
>   EXPORT_SYMBOL_GPL(of_get_pci_domain_nr);
>
> -/**
> - * of_pci_dma_configure - Setup DMA configuration
> - * @dev: ptr to pci_dev struct of the PCI device
> - *
> - * Function to update PCI devices's DMA configuration using the same
> - * info from the OF node of host bridge's parent (if any).
> - */
> -void of_pci_dma_configure(struct pci_dev *pci_dev)
> -{
> -	struct device *dev = &pci_dev->dev;
> -	struct device *bridge = pci_get_host_bridge_device(pci_dev);
> -
> -	if (bridge->parent)
> -		of_dma_configure(dev, bridge->parent->of_node);
> -
> -	pci_put_host_bridge_device(bridge);
> -}
> -EXPORT_SYMBOL_GPL(of_pci_dma_configure);
> -
>   #if defined(CONFIG_OF_ADDRESS)
>   /**
>    * of_pci_get_host_bridge_resources - Parse PCI host bridge resources from DT
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8361d27..31e3eef 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -6,7 +6,7 @@
>   #include <linux/delay.h>
>   #include <linux/init.h>
>   #include <linux/pci.h>
> -#include <linux/of_pci.h>
> +#include <linux/of_device.h>
>   #include <linux/pci_hotplug.h>
>   #include <linux/slab.h>
>   #include <linux/module.h>
> @@ -1633,6 +1633,25 @@ static void pci_set_msi_domain(struct pci_dev *dev)
>   				   dev_get_msi_domain(&dev->bus->dev));
>   }
>
> +/**
> + * pci_dma_configure - Setup DMA configuration
> + * @dev: ptr to pci_dev struct of the PCI device
> + *
> + * Function to update PCI devices's DMA configuration using the same
> + * info from the OF node of host bridge's parent (if any).
> + */
> +static void pci_dma_configure(struct pci_dev *dev)
> +{
> +	struct device *bridge = pci_get_host_bridge_device(dev);
> +
> +	if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {

Previously I was seeing of_dma_configure, and thus of_iommu_configure, 
called for every PCI device on Juno. The check above now prevents this 
happening, since the PCI devices are probed directly from the bus and 
don't have OF nodes of their own. They now get left in some 
half-configured state where arch_setup_dma_ops isn't called either.

Should this be checking bridge->parent->of_node rather than 
dev->dev.of_node (which seems to work), or am I missing something?

Sorry I don't really have the bandwidth to look into this in detail 
myself, right now I'm just trying to get my magic hacks rebased ;)

Robin.

> +		if (bridge->parent)
> +			of_dma_configure(&dev->dev, bridge->parent->of_node);
> +	}
> +
> +	pci_put_host_bridge_device(bridge);
> +}
> +
>   void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>   {
>   	int ret;
> @@ -1646,7 +1665,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>   	dev->dev.dma_mask = &dev->dma_mask;
>   	dev->dev.dma_parms = &dev->dma_parms;
>   	dev->dev.coherent_dma_mask = 0xffffffffull;
> -	of_pci_dma_configure(dev);
> +	pci_dma_configure(dev);
>
>   	pci_set_dma_max_seg_size(dev, 65536);
>   	pci_set_dma_seg_boundary(dev, 0xffffffff);
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 29fd3fe..ce0e5ab 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -16,7 +16,6 @@ int of_pci_get_devfn(struct device_node *np);
>   int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin);
>   int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
>   int of_get_pci_domain_nr(struct device_node *node);
> -void of_pci_dma_configure(struct pci_dev *pci_dev);
>   #else
>   static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq)
>   {
> @@ -51,8 +50,6 @@ of_get_pci_domain_nr(struct device_node *node)
>   {
>   	return -1;
>   }
> -
> -static inline void of_pci_dma_configure(struct pci_dev *pci_dev) { }
>   #endif
>
>   #if defined(CONFIG_OF_ADDRESS)
>

Re: [PATCH V5 8/9] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()

[Robin Murphy]

+Arnd

On 17/11/15 15:00, Robin Murphy wrote:
> Hi Suravee,
>
> On 28/10/15 22:50, Suravee Suthikulpanit wrote:
>> This patch move of_pci_dma_configure() to a more generic
>> pci_dma_configure(), which can be extended by non-OF code (e.g. ACPI).
>>
>> This has no functional change.
>
> Unfortunately, it appears that it does...
>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>> Acked-by: Rob Herring <robh+dt@kernel.org>
>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
>> CC: Rafael J. Wysocki <rjw@rjwysocki.net>
[...]
>> +/**
>> + * pci_dma_configure - Setup DMA configuration
>> + * @dev: ptr to pci_dev struct of the PCI device
>> + *
>> + * Function to update PCI devices's DMA configuration using the same
>> + * info from the OF node of host bridge's parent (if any).
>> + */
>> +static void pci_dma_configure(struct pci_dev *dev)
>> +{
>> +    struct device *bridge = pci_get_host_bridge_device(dev);
>> +
>> +    if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
>
> Previously I was seeing of_dma_configure, and thus of_iommu_configure,
> called for every PCI device on Juno. The check above now prevents this
> happening, since the PCI devices are probed directly from the bus and
> don't have OF nodes of their own. They now get left in some
> half-configured state where arch_setup_dma_ops isn't called either.

Just to follow up on that, Arnd's patch to tidy up dma_get_ops (now 
queued[1]) makes this even worse, since preventing arch_setup_dma_ops 
being called means the PCI devices now get the dummy DMA ops which leave 
the drivers failing to probe at all, IOMMU hacks or not :(

Robin.

[1]:https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/commit/?h=fixes/core&id=1dccb598df549d892b6450c261da54cdd7af44b4

> Should this be checking bridge->parent->of_node rather than
> dev->dev.of_node (which seems to work), or am I missing something?
>
> Sorry I don't really have the bandwidth to look into this in detail
> myself, right now I'm just trying to get my magic hacks rebased ;)
>
> Robin.
>
>> +        if (bridge->parent)
>> +            of_dma_configure(&dev->dev, bridge->parent->of_node);
>> +    }
>> +
>> +    pci_put_host_bridge_device(bridge);
>> +}

Re: [PATCH V5 8/9] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()

[Arnd Bergmann]

On Wednesday 18 November 2015 12:00:32 Robin Murphy wrote:
> On 17/11/15 15:00, Robin Murphy wrote:
> > On 28/10/15 22:50, Suravee Suthikulpanit wrote:
> >> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >> Acked-by: Rob Herring <robh+dt@kernel.org>
> >> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> >> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
> >> CC: Rafael J. Wysocki <rjw@rjwysocki.net>
> [...]
> >> +/**
> >> + * pci_dma_configure - Setup DMA configuration
> >> + * @dev: ptr to pci_dev struct of the PCI device
> >> + *
> >> + * Function to update PCI devices's DMA configuration using the same
> >> + * info from the OF node of host bridge's parent (if any).
> >> + */
> >> +static void pci_dma_configure(struct pci_dev *dev)
> >> +{
> >> +    struct device *bridge = pci_get_host_bridge_device(dev);
> >> +
> >> +    if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
> >
> > Previously I was seeing of_dma_configure, and thus of_iommu_configure,
> > called for every PCI device on Juno. The check above now prevents this
> > happening, since the PCI devices are probed directly from the bus and
> > don't have OF nodes of their own. They now get left in some
> > half-configured state where arch_setup_dma_ops isn't called either.
> 
> Just to follow up on that, Arnd's patch to tidy up dma_get_ops (now 
> queued[1]) makes this even worse, since preventing arch_setup_dma_ops 
> being called means the PCI devices now get the dummy DMA ops which leave 
> the drivers failing to probe at all, IOMMU hacks or not 

Ok, glad we found that with my patch then. We really have to
configure the DMA (offset/size/coherency/iommu) for all devices that might
be masters, otherwise things can randomly go wrong.

	ARnd

Re: [PATCH V5 8/9] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()

[Suravee Suthikulanit]

On 11/18/2015 6:41 AM, Arnd Bergmann wrote:
> On Wednesday 18 November 2015 12:00:32 Robin Murphy wrote:
>> On 17/11/15 15:00, Robin Murphy wrote:
>>> On 28/10/15 22:50, Suravee Suthikulpanit wrote:
>>>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>>> Acked-by: Rob Herring <robh+dt@kernel.org>
>>>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>>>> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
>>>> CC: Rafael J. Wysocki <rjw@rjwysocki.net>
>> [...]
>>>> +/**
>>>> + * pci_dma_configure - Setup DMA configuration
>>>> + * @dev: ptr to pci_dev struct of the PCI device
>>>> + *
>>>> + * Function to update PCI devices's DMA configuration using the same
>>>> + * info from the OF node of host bridge's parent (if any).
>>>> + */
>>>> +static void pci_dma_configure(struct pci_dev *dev)
>>>> +{
>>>> +    struct device *bridge = pci_get_host_bridge_device(dev);
>>>> +
>>>> +    if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
>>>
>>> Previously I was seeing of_dma_configure, and thus of_iommu_configure,
>>> called for every PCI device on Juno. The check above now prevents this
>>> happening, since the PCI devices are probed directly from the bus and
>>> don't have OF nodes of their own. They now get left in some
>>> half-configured state where arch_setup_dma_ops isn't called either.
>>
>> Just to follow up on that, Arnd's patch to tidy up dma_get_ops (now
>> queued[1]) makes this even worse, since preventing arch_setup_dma_ops
>> being called means the PCI devices now get the dummy DMA ops which leave
>> the drivers failing to probe at all, IOMMU hacks or not
>
> Ok, glad we found that with my patch then. We really have to
> configure the DMA (offset/size/coherency/iommu) for all devices that might
> be masters, otherwise things can randomly go wrong.
>
> 	ARnd
>
I'm double checking this and will get back ASAP.

Suravee

Re: [PATCH V5 8/9] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()

[Suravee Suthikulanit]

Hi All,

On 11/18/2015 6:41 AM, Arnd Bergmann wrote:
> On Wednesday 18 November 2015 12:00:32 Robin Murphy wrote:
>> On 17/11/15 15:00, Robin Murphy wrote:
>>> On 28/10/15 22:50, Suravee Suthikulpanit wrote:
>>>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>>> Acked-by: Rob Herring <robh+dt@kernel.org>
>>>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>>>> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
>>>> CC: Rafael J. Wysocki <rjw@rjwysocki.net>
>> [...]
>>>> +/**
>>>> + * pci_dma_configure - Setup DMA configuration
>>>> + * @dev: ptr to pci_dev struct of the PCI device
>>>> + *
>>>> + * Function to update PCI devices's DMA configuration using the same
>>>> + * info from the OF node of host bridge's parent (if any).
>>>> + */
>>>> +static void pci_dma_configure(struct pci_dev *dev)
>>>> +{
>>>> +    struct device *bridge = pci_get_host_bridge_device(dev);
>>>> +
>>>> +    if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
>>>
>>> Previously I was seeing of_dma_configure, and thus of_iommu_configure,
>>> called for every PCI device on Juno. The check above now prevents this
>>> happening, since the PCI devices are probed directly from the bus and
>>> don't have OF nodes of their own. They now get left in some
>>> half-configured state where arch_setup_dma_ops isn't called either.
>>
>> Just to follow up on that, Arnd's patch to tidy up dma_get_ops (now
>> queued[1]) makes this even worse, since preventing arch_setup_dma_ops
>> being called means the PCI devices now get the dummy DMA ops which leave
>> the drivers failing to probe at all, IOMMU hacks or not
>
> Ok, glad we found that with my patch then. We really have to
> configure the DMA (offset/size/coherency/iommu) for all devices that might
> be masters, otherwise things can randomly go wrong.
>
> 	ARnd

Robin is correct. Thanks for catching the bug. Rafael, do you want me to 
submit just the fixed-up patch on top of what we had earlier. Or do you 
want a new revision (V6)?

Thanks,
Suravee

Re: [PATCH V5 8/9] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()

[Rafael J. Wysocki]

On Wednesday, November 18, 2015 10:54:59 AM Suravee Suthikulanit wrote:
> Hi All,
> 
> On 11/18/2015 6:41 AM, Arnd Bergmann wrote:
> > On Wednesday 18 November 2015 12:00:32 Robin Murphy wrote:
> >> On 17/11/15 15:00, Robin Murphy wrote:
> >>> On 28/10/15 22:50, Suravee Suthikulpanit wrote:
> >>>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>>> Acked-by: Rob Herring <robh+dt@kernel.org>
> >>>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> >>>> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
> >>>> CC: Rafael J. Wysocki <rjw@rjwysocki.net>
> >> [...]
> >>>> +/**
> >>>> + * pci_dma_configure - Setup DMA configuration
> >>>> + * @dev: ptr to pci_dev struct of the PCI device
> >>>> + *
> >>>> + * Function to update PCI devices's DMA configuration using the same
> >>>> + * info from the OF node of host bridge's parent (if any).
> >>>> + */
> >>>> +static void pci_dma_configure(struct pci_dev *dev)
> >>>> +{
> >>>> +    struct device *bridge = pci_get_host_bridge_device(dev);
> >>>> +
> >>>> +    if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
> >>>
> >>> Previously I was seeing of_dma_configure, and thus of_iommu_configure,
> >>> called for every PCI device on Juno. The check above now prevents this
> >>> happening, since the PCI devices are probed directly from the bus and
> >>> don't have OF nodes of their own. They now get left in some
> >>> half-configured state where arch_setup_dma_ops isn't called either.
> >>
> >> Just to follow up on that, Arnd's patch to tidy up dma_get_ops (now
> >> queued[1]) makes this even worse, since preventing arch_setup_dma_ops
> >> being called means the PCI devices now get the dummy DMA ops which leave
> >> the drivers failing to probe at all, IOMMU hacks or not
> >
> > Ok, glad we found that with my patch then. We really have to
> > configure the DMA (offset/size/coherency/iommu) for all devices that might
> > be masters, otherwise things can randomly go wrong.
> >
> > 	ARnd
> 
> Robin is correct. Thanks for catching the bug. Rafael, do you want me to 
> submit just the fixed-up patch on top of what we had earlier. Or do you 
> want a new revision (V6)?

This is in 4.4-rc1, right?  So new revisions surely won't work, will they?

Thanks,
Rafael