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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
