| Message ID | 73becf4a75f15662b2dda5fba7cfeacdf3d866f8.1701268753.git.robin.murphy@arm.com |
|---|---|
| State | Accepted |
| Commit | ba503cf41c90e173f9421f7882d84b228bada97a |
| Headers | show |
| Series | dma-mapping: Clean up arch_setup_dma_ops() | expand |
On Wed, Nov 29, 2023 at 05:42:59PM +0000, Robin Murphy wrote: > Juggling start, end, and size values for a range is somewhat redundant > and a little hard to follow. Consolidate down to just using inclusive > start and end, which saves us worrying about size overflows for full > 64-bit ranges (note that passing a potentially-overflowed value through > to arch_setup_dma_ops() is benign for all current implementations, and > this is working towards removing that anyway). In iommu code I've been trying to use consistent language with other parts of the kernel like interval tree and maple tree: * In this file the term 'last' indicates an inclusive and closed interval, eg * [0,0] refers to a single PFN. 'end' means an open range, eg [0,0) refers to * no PFNs. Here I think you've swapped end to mean last? Regardless the change looks correct Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
On Wed, Nov 29, 2023 at 11:43 AM Robin Murphy <robin.murphy@arm.com> wrote: > > Juggling start, end, and size values for a range is somewhat redundant > and a little hard to follow. Consolidate down to just using inclusive > start and end, which saves us worrying about size overflows for full > 64-bit ranges (note that passing a potentially-overflowed value through > to arch_setup_dma_ops() is benign for all current implementations, and > this is working towards removing that anyway). > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > --- > drivers/of/device.c | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) Acked-by: Rob Herring <robh@kernel.org>
diff --git a/drivers/of/device.c b/drivers/of/device.c index 526a42cdf66e..51062a831970 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -97,7 +97,7 @@ int of_dma_configure_id(struct device *dev, struct device_node *np, const struct bus_dma_region *map = NULL; struct device_node *bus_np; u64 dma_start = 0; - u64 mask, end, size = 0; + u64 mask, end = 0; bool coherent; int ret; @@ -118,17 +118,15 @@ int of_dma_configure_id(struct device *dev, struct device_node *np, return ret == -ENODEV ? 0 : ret; } else { const struct bus_dma_region *r = map; - u64 dma_end = 0; /* Determine the overall bounds of all DMA regions */ for (dma_start = ~0; r->size; r++) { /* Take lower and upper limits */ if (r->dma_start < dma_start) dma_start = r->dma_start; - if (r->dma_start + r->size > dma_end) - dma_end = r->dma_start + r->size; + if (r->dma_start + r->size > end) + end = r->dma_start + r->size; } - size = dma_end - dma_start; } /* @@ -142,16 +140,15 @@ int of_dma_configure_id(struct device *dev, struct device_node *np, dev->dma_mask = &dev->coherent_dma_mask; } - if (!size && dev->coherent_dma_mask) - size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1); - else if (!size) - size = 1ULL << 32; + if (!end && dev->coherent_dma_mask) + end = dev->coherent_dma_mask; + else if (!end) + end = (1ULL << 32) - 1; /* * Limit coherent and dma mask based on size and default mask * set by the driver. */ - end = dma_start + size - 1; mask = DMA_BIT_MASK(ilog2(end) + 1); dev->coherent_dma_mask &= mask; *dev->dma_mask &= mask; @@ -177,7 +174,7 @@ int of_dma_configure_id(struct device *dev, struct device_node *np, dev_dbg(dev, "device is%sbehind an iommu\n", iommu ? " " : " not "); - arch_setup_dma_ops(dev, dma_start, size, iommu, coherent); + arch_setup_dma_ops(dev, dma_start, end - dma_start + 1, iommu, coherent); if (!iommu) of_dma_set_restricted_buffer(dev, np);
Juggling start, end, and size values for a range is somewhat redundant and a little hard to follow. Consolidate down to just using inclusive start and end, which saves us worrying about size overflows for full 64-bit ranges (note that passing a potentially-overflowed value through to arch_setup_dma_ops() is benign for all current implementations, and this is working towards removing that anyway). Signed-off-by: Robin Murphy <robin.murphy@arm.com> --- drivers/of/device.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)