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 <[email protected]>
---
drivers/of/device.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
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);
--
2.39.2.101.g768bb238c484.dirty
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 <[email protected]>
Jason
On Wed, Nov 29, 2023 at 11:43 AM Robin Murphy <[email protected]> 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 <[email protected]>
> ---
> drivers/of/device.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
Acked-by: Rob Herring <[email protected]>