drivers/of/address.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
The function of_dma_get_max_cpu_address() for a device node should choose
the highest cpu address among the ones that subtree nodes can access.
However, there was a bug of choosing the lowest cpu address and this
commit is to fix it.
Signed-off-by: Joonwon Kang <kjw1627@gmail.com>
---
drivers/of/address.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index f0f8f0dd191c..5e984e0d372b 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -969,6 +969,7 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
{
phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
struct of_range_parser parser;
+ phys_addr_t max_subtree_max_addr = PHYS_ADDR_MAX;
phys_addr_t subtree_max_addr;
struct device_node *child;
struct of_range range;
@@ -992,10 +993,17 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
for_each_available_child_of_node(np, child) {
subtree_max_addr = of_dma_get_max_cpu_address(child);
- if (max_cpu_addr > subtree_max_addr)
- max_cpu_addr = subtree_max_addr;
+ if (subtree_max_addr == PHYS_ADDR_MAX)
+ continue;
+
+ if (max_subtree_max_addr == PHYS_ADDR_MAX)
+ max_subtree_max_addr = subtree_max_addr;
+ else
+ max_subtree_max_addr = max(max_subtree_max_addr, subtree_max_addr);
}
+ max_cpu_addr = min(max_cpu_addr, max_subtree_max_addr);
+
return max_cpu_addr;
}
--
2.46.0On Sun, Jul 27, 2025 at 1:01 PM Joonwon Kang <kjw1627@gmail.com> wrote:
>
> The function of_dma_get_max_cpu_address() for a device node should choose
> the highest cpu address among the ones that nodes can access.
> However, there was a bug of choosing the lowest cpu address and this
> commit is to fix it.
Please provide a test case in the DT unittests or at least details on
the DT that is affected by the bug.
> Signed-off-by: Joonwon Kang <kjw1627@gmail.com>
> ---
> drivers/of/address.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index f0f8f0dd191c..5e984e0d372b 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -969,6 +969,7 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> {
> phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
> struct of_range_parser parser;
> + phys_addr_t max_subtree_max_addr = PHYS_ADDR_MAX;
> phys_addr_t subtree_max_addr;
> struct device_node *child;
> struct of_range range;
> @@ -992,10 +993,17 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
>
> for_each_available_child_of_node(np, child) {
> subtree_max_addr = of_dma_get_max_cpu_address(child);
> - if (max_cpu_addr > subtree_max_addr)
> - max_cpu_addr = subtree_max_addr;
> + if (subtree_max_addr == PHYS_ADDR_MAX)
> + continue;
> +
> + if (max_subtree_max_addr == PHYS_ADDR_MAX)
> + max_subtree_max_addr = subtree_max_addr;
> + else
> + max_subtree_max_addr = max(max_subtree_max_addr, subtree_max_addr);
> }
>
> + max_cpu_addr = min(max_cpu_addr, max_subtree_max_addr);
> +
> return max_cpu_addr;
> }
>
> --
> 2.46.0
>
On Sun, Jul 27, 2025 at 1:01 PM Joonwon Kang <kjw1627@gmail.com> wrote:
> >
> > The function of_dma_get_max_cpu_address() for a device node should choose
> > the highest cpu address among the ones that nodes can access.
> > However, there was a bug of choosing the lowest cpu address and this
> > commit is to fix it.
>
> Please provide a test case in the DT unittests or at least details on
> the DT that is affected by the bug.
While working on the DT unittests, I got two questions to which I had failed to
have clear answers. Let's assume that the device tree looks as follows.
parent_bus@... {
#address-cells = <1>;
#size-cells = <1>;
dma-ranges = <0x0 0x0 0x1000>;
child_bus@... {
#address-cells = <1>;
#size-cells = <1>;
/* Note that the size part exceeds the `parent_bus`' dma size. */
dma-ranges = <0x0 0x0 0x2000>;
child_device_1@... {
/*
* Note that the size part exceeds the `child_bus`' dma size and
* also the `parent_bus`' dma size.
*/
reg = <0x0 0x3000>;
};
child_device_2@... {
/*
* Note that the address part transitively exceeds the
*`parent_bus`' end address.
*/
reg = <0x1000 0x1000>
};
};
another_child_bus@... {
#address-cells = <1>;
#size-cells = <1>;
dma-ranges = <0x0 0x0 0x300>;
};
};
Q1: What is the expected output of `of_dma_get_max_cpu_address(parent_bus)`?
I think it should be 0xfff since the `dma-ranges` in the `child_bus` should be
capped to the parent max cpu address instead of treating it as if the
`dma-ranges` in the `child_bus` does not exist. The current expectation is
0x2ff which is for `another_child_bus` based on the existing test case
in drivers/of/unittest.c and drivers/of/tests-address.dtsi.
Q2: `of_dma_get_max_cpu_address(child_device_1, reg_prop, &addr, &length)`
returns a success with `addr` set to 0x0 and `length` set to 0x3000. Similarly,
`of_translate_dma_address(child_device_1, reg_prop)` returns a success. On the
other hand, both functions for `child_device_2` return a failure since the
address is out of parent ranges. I think those functions should also fail
for `child_device_1` since the dma "end" address of the `child_device_1` node
is not valid in the first place. Are the current behaviors of both functions
intended?
> > Signed-off-by: Joonwon Kang <kjw1627@gmail.com>
> > ---
> > drivers/of/address.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index f0f8f0dd191c..5e984e0d372b 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -969,6 +969,7 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> > {
> > phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
> > struct of_range_parser parser;
> > + phys_addr_t max_subtree_max_addr = PHYS_ADDR_MAX;
> > phys_addr_t subtree_max_addr;
> > struct device_node *child;
> > struct of_range range;
> > @@ -992,10 +993,17 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> >
> > for_each_available_child_of_node(np, child) {
> > subtree_max_addr = of_dma_get_max_cpu_address(child);
> > - if (max_cpu_addr > subtree_max_addr)
> > - max_cpu_addr = subtree_max_addr;
> > + if (subtree_max_addr == PHYS_ADDR_MAX)
> > + continue;
> > +
> > + if (max_subtree_max_addr == PHYS_ADDR_MAX)
> > + max_subtree_max_addr = subtree_max_addr;
> > + else
> > + max_subtree_max_addr = max(max_subtree_max_addr, subtree_max_addr);
> > }
> >
> > + max_cpu_addr = min(max_cpu_addr, max_subtree_max_addr);
> > +
> > return max_cpu_addr;
> > }
> >
> > --
> > 2.46.0
> >
On Mon, Aug 4, 2025 at 11:43 AM Joonwon Kang <kjw1627@gmail.com> wrote:
>
> On Sun, Jul 27, 2025 at 1:01 PM Joonwon Kang <kjw1627@gmail.com> wrote:
> > >
> > > The function of_dma_get_max_cpu_address() for a device node should choose
> > > the highest cpu address among the ones that nodes can access.
> > > However, there was a bug of choosing the lowest cpu address and this
> > > commit is to fix it.
> >
> > Please provide a test case in the DT unittests or at least details on
> > the DT that is affected by the bug.
>
> While working on the DT unittests, I got two questions to which I had failed to
> have clear answers. Let's assume that the device tree looks as follows.
>
> parent_bus@... {
> #address-cells = <1>;
> #size-cells = <1>;
> dma-ranges = <0x0 0x0 0x1000>;
>
> child_bus@... {
> #address-cells = <1>;
> #size-cells = <1>;
> /* Note that the size part exceeds the `parent_bus`' dma size. */
> dma-ranges = <0x0 0x0 0x2000>;
>
> child_device_1@... {
> /*
> * Note that the size part exceeds the `child_bus`' dma size and
> * also the `parent_bus`' dma size.
> */
> reg = <0x0 0x3000>;
dma-ranges is irrelevant for 'reg'. 'ranges' applies to 'reg'.
> };
>
> child_device_2@... {
> /*
> * Note that the address part transitively exceeds the
> *`parent_bus`' end address.
> */
> reg = <0x1000 0x1000>
> };
> };
>
> another_child_bus@... {
> #address-cells = <1>;
> #size-cells = <1>;
> dma-ranges = <0x0 0x0 0x300>;
> };
> };
>
> Q1: What is the expected output of `of_dma_get_max_cpu_address(parent_bus)`?
> I think it should be 0xfff since the `dma-ranges` in the `child_bus` should be
> capped to the parent max cpu address instead of treating it as if the
> `dma-ranges` in the `child_bus` does not exist. The current expectation is
> 0x2ff which is for `another_child_bus` based on the existing test case
> in drivers/of/unittest.c and drivers/of/tests-address.dtsi.
0x2FF is correct. The max address returned is the minimum.
>
> Q2: `of_dma_get_max_cpu_address(child_device_1, reg_prop, &addr, &length)`
> returns a success with `addr` set to 0x0 and `length` set to 0x3000. Similarly,
> `of_translate_dma_address(child_device_1, reg_prop)` returns a success. On the
> other hand, both functions for `child_device_2` return a failure since the
> address is out of parent ranges. I think those functions should also fail
> for `child_device_1` since the dma "end" address of the `child_device_1` node
> is not valid in the first place. Are the current behaviors of both functions
> intended?
Passing in child_device_1 is invalid. It doesn't contain dma-ranges,
so it will return PHYS_ADDR_MAX.
Passing 'reg' into the DMA translation functions is invalid. 'reg' has
0 to do with DMA addresses.
Rob
>
> > > Signed-off-by: Joonwon Kang <kjw1627@gmail.com>
> > > ---
> > > drivers/of/address.c | 12 ++++++++++--
> > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > index f0f8f0dd191c..5e984e0d372b 100644
> > > --- a/drivers/of/address.c
> > > +++ b/drivers/of/address.c
> > > @@ -969,6 +969,7 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> > > {
> > > phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
> > > struct of_range_parser parser;
> > > + phys_addr_t max_subtree_max_addr = PHYS_ADDR_MAX;
> > > phys_addr_t subtree_max_addr;
> > > struct device_node *child;
> > > struct of_range range;
> > > @@ -992,10 +993,17 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> > >
> > > for_each_available_child_of_node(np, child) {
> > > subtree_max_addr = of_dma_get_max_cpu_address(child);
> > > - if (max_cpu_addr > subtree_max_addr)
> > > - max_cpu_addr = subtree_max_addr;
> > > + if (subtree_max_addr == PHYS_ADDR_MAX)
> > > + continue;
> > > +
> > > + if (max_subtree_max_addr == PHYS_ADDR_MAX)
> > > + max_subtree_max_addr = subtree_max_addr;
> > > + else
> > > + max_subtree_max_addr = max(max_subtree_max_addr, subtree_max_addr);
> > > }
> > >
> > > + max_cpu_addr = min(max_cpu_addr, max_subtree_max_addr);
> > > +
> > > return max_cpu_addr;
> > > }
> > >
> > > --
> > > 2.46.0
> > >
© 2016 - 2025 Red Hat, Inc.