Re: [PATCH v9 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset

[Christoph Hellwig <hch@lst.de>]

A few tiny nitpicks:

The subject should have the dma-mapping prefix, this doesn't
really touch the device core.

> -	rc = of_dma_get_range(np, &dma_addr, &paddr, &size);
> +	rc = of_dma_get_range(np, &map);
> +	rc = PTR_ERR_OR_ZERO(map);

I don't think you need the PTR_ERR_OR_ZERO line here, of_dma_get_range
returns the error.

> +int dma_set_offset_range(struct device *dev, phys_addr_t cpu_start,
> +			 dma_addr_t dma_start, u64 size)
> +{
> +	struct bus_dma_region *map;
> +	u64 offset = (u64)cpu_start - (u64)dma_start;
> +
> +	if (!dev)
> +		return -ENODEV;

I don't think we need the NULL protection here, all DMA API calls
expect a device.

> +	if (!offset)
> +		return 0;
> +
> +	/*
> +	 * See if a map already exists and we already encompass the new range:
> +	 */
> +	if (dev->dma_range_map) {
> +		if (dma_range_overlaps(dev, cpu_start, dma_start, size, offset))
> +			return 0;
> +		dev_err(dev, "attempt to add conflicting DMA range to existing map\n");
> +		return -EINVAL;
> +	}

And here why do we need the overlap check at all?  I'd be tempted to
always return an error for this case.

What is the plan to merge this?  Do you want all this to go into one
tree, or get as many bits into the applicable trees for 5.9 and then
finish up for 5.10?  If the former I can apply it to the dma-mapping
tree and just fix up the nitpicks.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu