On Tue, Sep 13, 2016 at 04:25:55PM +0100, Robin Murphy wrote: [...] > > +/** > > + * 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. > > + > > + 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. > > > + > > + 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(); > > } > > > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html