On Tue, Sep 13, 2016 at 04:25:55PM +0100, Robin Murphy wrote:

[...]

quoted

+/**
+ * iort_add_smmu_platform_device() - Allocate a platform device for SMMU
+ * @fwnode: IORT node associated fwnode handle
+ * @node: Pointer to SMMU ACPI IORT node
+ *
+ * Returns: 0 on success, <0 failure
+ */
+static int __init iort_add_smmu_platform_device(struct fwnode_handle *fwnode,
+						struct acpi_iort_node *node)
+{
+	struct platform_device *pdev;
+	struct resource *r;
+	enum dev_dma_attr attr;
+	int ret, count;
+	const struct iort_iommu_config *ops = iort_get_iommu_cfg(node);
+
+	if (!ops)
+		return -ENODEV;
+
+	pdev = platform_device_alloc(ops->name, PLATFORM_DEVID_AUTO);
+	if (!pdev)
+		return PTR_ERR(pdev);
+
+	count = ops->iommu_count_resources(node);
+
+	r = kcalloc(count, sizeof(*r), GFP_KERNEL);
+	if (!r) {
+		ret = -ENOMEM;
+		goto dev_put;
+	}
+
+	ops->iommu_init_resources(r, node);
+
+	ret = platform_device_add_resources(pdev, r, count);
+	/*
+	 * Resources are duplicated in platform_device_add_resources,
+	 * free their allocated memory
+	 */
+	kfree(r);
+
+	if (ret)
+		goto dev_put;
+
+	/*
+	 * Add a copy of IORT node pointer to platform_data to
+	 * be used to retrieve IORT data information.
+	 */
+	ret = platform_device_add_data(pdev, &node, sizeof(node));
+	if (ret)
+		goto dev_put;
+
+	pdev->dev.dma_mask = kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+	if (!pdev->dev.dma_mask) {
+		ret = -ENOMEM;
+		goto dev_put;
+	}

Since this is exclusively for creating SMMUs, and we know they should
never have weird shenanigans going on requiring different masks, I'd be
inclined to just point dev.dma_mask at dev.coherent_dma_mask and be done
with it.

That sounds reasonable yes, I will do.

quoted

+
+	pdev->dev.fwnode = fwnode;
+
+	/*
+	 * Set default dma mask value for the table walker,
+	 * to be overridden on probing with correct value.
+	 */
+	*pdev->dev.dma_mask = DMA_BIT_MASK(32);
+	pdev->dev.coherent_dma_mask = *pdev->dev.dma_mask;
+
+	attr = ops->iommu_is_coherent(node) ?
+			     DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
+
+	/* Configure DMA for the page table walker */
+	acpi_dma_configure(&pdev->dev, attr);

Oh look, some more code which would be simpler if acpi_dma_configure()
set the default mask itself ;)

Eheh I have no objections, apart from the effect that change can
have on non-ARM probing paths, it could also help remove some code
from ACPI core while at it.

Thanks !
Lorenzo

Robin.

quoted

+
+	ret = platform_device_add(pdev);
+	if (ret)
+		goto dma_deconfigure;
+
+	return 0;
+
+dma_deconfigure:
+	acpi_dma_deconfigure(&pdev->dev);
+	kfree(pdev->dev.dma_mask);
+
+dev_put:
+	platform_device_put(pdev);
+
+	return ret;
+}
+
+static acpi_status __init iort_match_iommu_callback(struct acpi_iort_node *node,
+						    void *context)
+{
+	int ret;
+	struct fwnode_handle *fwnode;
+
+	fwnode = iort_get_fwnode(node);
+
+	if (!fwnode)
+		return AE_NOT_FOUND;
+
+	ret = iort_add_smmu_platform_device(fwnode, node);
+	if (ret) {
+		pr_err("Error in platform device creation\n");
+		return AE_ERROR;
+	}
+
+	return AE_OK;
+}
+
+static void __init iort_smmu_init(void)
+{
+	iort_scan_node(ACPI_IORT_NODE_SMMU, iort_match_iommu_callback, NULL);
+	iort_scan_node(ACPI_IORT_NODE_SMMU_V3, iort_match_iommu_callback, NULL);
+}
+
 void __init acpi_iort_init(void)
 {
 	acpi_status status;

@@ -436,4 +566,5 @@ void __init acpi_iort_init(void)
 	}
 
 	acpi_probe_device_table(iort);
+	iort_smmu_init();
 }