Hi Christoph, On Tue, Jul 28, 2020 at 8:33 AM Christoph Hellwig <hch@xxxxxx> wrote: > > 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. Yes, that link needs to be deleted. > > > +int dma_set_offset_range(struct device *dev, phys_addr_t cpu_start, > > + dma_addr_t dma_start, u64 size)hh > > +{ > > + 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. Yes, your review-patch removed it but left the comment about the function returning -ENODEV. So I wasn't sure to leave it in or not. > > > + 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. I believe the overlap check was your suggestion or at least in your review-patch? I'm fine with just returning an error. > > 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. Whatever you think is best -- I would be quite happy if you could accept at least the "dma_range_map" commit. Of course I'd be most happy if the entire patchset were accepted, but perhaps you can just apply the "dma_range_map" commit, and I will continue to bang away at getting the N-1 PCIe-related commits ack'd and accepted. Thanks much! Jim Quinlan Broadcom STB _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel