On Wed, Jul 01, 2020 at 05:21:38PM -0400, Jim Quinlan wrote:
> The new field 'dma_range_map' in struct device is used to facilitate the
> use of single or multiple offsets between mapping regions of cpu addrs and
> dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> capable of holding a single uniform offset and had no region bounds
> checking.
>
> The function of_dma_get_range() has been modified so that it takes a single
> argument -- the device node -- and returns a map, NULL, or an error code.
> The map is an array that holds the information regarding the DMA regions.
> Each range entry contains the address offset, the cpu_start address, the
> dma_start address, and the size of the region.
>
> of_dma_configure() is the typical manner to set range offsets but there are
> a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel
> driver code. These cases now invoke the function
> dma_attach_offset_range(dev, cpu_addr, dma_addr, size).
...
> + if (dev && dev->dma_range_map)
> + pfn -= (unsigned long)PFN_DOWN(dma_offset_from_phys_addr(dev,
> PFN_PHYS(pfn)));
Instead of casting use PHYS_PFN() and it will be consistent with latter in the
same line.
> + if (dev && dev->dma_range_map)
> + pfn += (unsigned long)PFN_DOWN(dma_offset_from_dma_addr(dev,
> addr));
Ditto.
...
> + dev_err(dev, "set dma_offset%08llx%s\n",
> KEYSTONE_HIGH_PHYS_START
> + - KEYSTONE_LOW_PHYS_START, ret ? " failed" : "");
Please, avoid such indentation.
Better split it to the three lines, argument per line (expect dev which will go
on the first one).
This applies to all similar places.
...
> unsigned long pfn = (dma_handle >> PAGE_SHIFT);
PHYS_PFN() / PFN_DOWN() ?
> + if (!WARN_ON(!dev) && dev->dma_range_map)
> + pfn += (unsigned long)PFN_DOWN(dma_offset_from_dma_addr(dev,
> dma_handle));
PHYS_PFN() ?
...
> + r = kcalloc(num_ranges + 1, sizeof(struct bus_dma_region), GFP_KERNEL);
sizeof(*r) ?
> + if (!r)
> + return ERR_PTR(-ENOMEM);
...
> + ret = IS_ERR(map) ? PTR_ERR(map) : 0;
PTR_ERR_OR_ZERO()
...
> + /* We want the offset map to be device-managed, so alloc & copy
> */
> + dev->dma_range_map = devm_kcalloc(dev, num_ranges + 1,
> sizeof(*r),
> + GFP_KERNEL);
The question is how many times per device lifetime this can be called?
...
> + if (!dev->dma_range_map)
> + return -ENOMEM;
> + memcpy((void *)dev->dma_range_map, map, sizeof(*r) * num_ranges
> + 1);
If it's continuous, perhaps kmemdup() ?
...
> + rc = IS_ERR(map) ? PTR_ERR(map) : 0;
PTR_ERR_OR_ZERO()
...
> + = dma_offset_from_phys_addr(dev,
> PFN_PHYS(mem->pfn_base));
> +
> + return (dma_addr_t)PFN_PHYS(mem->pfn_base) - dma_offset;
Looking at this more, I think you need to introduce in the same header (pfn.h)
something like:
#define PFN_DMA_ADDR()
#define DMA_ADDR_PFN()
--
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
