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();
> > }
> >
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
