On Mon, Aug 24, 2020 at 03:30:20PM -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).
... > + /* > + * Record all info in the generic DMA ranges array for struct device. > + */ > + *map = r; > + for_each_of_range(&parser, &range) { > + pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n", > + range.bus_addr, range.cpu_addr, range.size); > + r->cpu_start = range.cpu_addr; > + r->dma_start = range.bus_addr; > + r->size = range.size; > + r->offset = (u64)range.cpu_addr - (u64)range.bus_addr; What's the point in explicit castings to the same type? > + r++; > + } ... > + phys_addr_t paddr; > + dma_addr_t dma_addr; > + struct device dev_bogus; > unittest(paddr == expect_paddr, > - "of_dma_get_range wrong phys addr (%llx) on node > %pOF", paddr, np); > + "of_dma_get_range: wrong phys addr %llx (expecting > %llx) on node %pOF\n", > + (u64)paddr, expect_paddr, np); %llx -> %pap > unittest(dma_addr == expect_dma_addr, > - "of_dma_get_range wrong DMA addr (%llx) on node %pOF", > dma_addr, np); > + "of_dma_get_range: wrong DMA addr %llx (expecting > %llx) on node %pOF\n", > + (u64)dma_addr, expect_dma_addr, np); %llx -> %pad ... > + if (mem->use_dev_dma_pfn_offset) { > + u64 base_addr = PFN_PHYS((u64)mem->pfn_base); Do we need explicit casting here? > + > + return base_addr - dma_offset_from_phys_addr(dev, base_addr); > + } ... > +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->dma_range_map) { > + dev_err(dev, "attempt to add DMA range to existing map\n"); > + return -EINVAL; > + } Wouldn't be better to do an assignment of offset here? > + if (!offset) > + return 0; > + > + map = kcalloc(2, sizeof(*map), GFP_KERNEL); > + if (!map) > + return -ENOMEM; > + map[0].cpu_start = cpu_start; > + map[0].dma_start = dma_start; > + map[0].offset = offset; > + map[0].size = size; > + dev->dma_range_map = map; > + > + return 0; > +} -- With Best Regards, Andy Shevchenko _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel