This patch series fixes several bugs related to dt_root_addr_cells and
dt_root_size_cells, and performs some cleanup.

Links to the previous related patches:

https://lore.kernel.org/lkml/CAL_JsqJxar7z+VcBXwPTw5-Et2oC9bQmH_CtMtKhoo_-=zN2XQ@mail.gmail.com/

Yuntao Wang (10):
  of/fdt: Introduce dt_root_addr_size_cells() and
    dt_root_addr_size_bytes()
  of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
    it
  of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
    it
  of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
    it
  of/fdt: Use dt_root_addr_size_bytes() instead of open-coding it
  of/fdt: Fix the len check in early_init_dt_check_for_elfcorehdr()
  of/fdt: Fix the len check in
    early_init_dt_check_for_usable_mem_range()
  of/fdt: Use dt_root_addr_size_bytes() instead of open-coding it
  of/fdt: Fix incorrect use of dt_root_addr_cells in
    early_init_dt_check_kho()
  of/address: Remove the incorrect and misleading comment

 drivers/of/address.c         |  4 ----
 drivers/of/fdt.c             | 14 +++++++-------
 drivers/of/of_reserved_mem.c |  6 +++---
 include/linux/of_fdt.h       | 11 +++++++++++
 4 files changed, 21 insertions(+), 14 deletions(-)

-- 
2.51.0

On Wed, Nov 12, 2025 at 10:35:10PM +0800, Yuntao Wang wrote:
> This patch series fixes several bugs related to dt_root_addr_cells and
> dt_root_size_cells, and performs some cleanup.
> 
> Links to the previous related patches:
> 
> https://lore.kernel.org/lkml/CAL_JsqJxar7z+VcBXwPTw5-Et2oC9bQmH_CtMtKhoo_-=zN2XQ@mail.gmail.com/
> 
> Yuntao Wang (10):
>   of/fdt: Introduce dt_root_addr_size_cells() and
>     dt_root_addr_size_bytes()
>   of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
>     it
>   of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
>     it
>   of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
>     it

Your aim in writing subjects should be to write something that is unique 
for every commit in the past or future. Because you can never make the 
same change twice, right? (I'm excluding 'fix typos/spelling' type 
commits). Certainly the same subject in one series is never right.

>   of/fdt: Use dt_root_addr_size_bytes() instead of open-coding it
>   of/fdt: Fix the len check in early_init_dt_check_for_elfcorehdr()
>   of/fdt: Fix the len check in
>     early_init_dt_check_for_usable_mem_range()
>   of/fdt: Use dt_root_addr_size_bytes() instead of open-coding it

This is not what I meant. We have multiple copies of this where only 
the property name changes: 

	prop = of_get_flat_dt_prop(node, "linux,elfcorehdr", &len);
	if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
		return;

	elfcorehdr_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
	elfcorehdr_size = dt_mem_next_cell(dt_root_size_cells, &prop);

Instead, add a function something like this:

static void early_init_dt_read_address(unsigned long node, const char 
*prop, u64 *addr, u64*size)
{
        prop = of_get_flat_dt_prop(node, prop, &len);
        if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
                return;

        *addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
        *size = dt_mem_next_cell(dt_root_size_cells, &prop);
}

Then we only have the length checks in one place.


That still leaves the cases with more than 1 entry open coded. So 
instead, to cover that case to something like this:

const __be32 *of_get_flat_dt_address_prop(unsigned long node, const char 
*propname, int *len)
{
	prop = of_get_flat_dt_prop(node, propname, &len);
	if (!prop || (*len % (dt_root_addr_cells + dt_root_size_cells))) {
		*len = 0;
		return NULL;
	}

	*len /= (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
	return prop;
}

And then a user would look something like this:

prop = of_get_flat_dt_address(node, "linux,usable-memory-range", &len);
for (i = 0; i < len; i++) {
	of_read_address_idx(prop, i, &addr, &size);
	...
}

Here 'len' is number of addr+size entries.

And the simple case of reading 1 entry could be just:

of_read_address_idx(of_get_flat_dt_address(node, "linux,elfcorehdr", NULL), 0, &addr, &size);

Rob

On Wed, 12 Nov 2025 14:55:51 -0600, Rob Herring <robh@kernel.org> wrote:

> On Wed, Nov 12, 2025 at 10:35:10PM +0800, Yuntao Wang wrote:
> > This patch series fixes several bugs related to dt_root_addr_cells and
> > dt_root_size_cells, and performs some cleanup.
> > 
> > Links to the previous related patches:
> > 
> > https://lore.kernel.org/lkml/CAL_JsqJxar7z+VcBXwPTw5-Et2oC9bQmH_CtMtKhoo_-=zN2XQ@mail.gmail.com/
> > 
> > Yuntao Wang (10):
> >   of/fdt: Introduce dt_root_addr_size_cells() and
> >     dt_root_addr_size_bytes()
> >   of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
> >     it
> >   of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
> >     it
> >   of/reserved_mem: Use dt_root_addr_size_bytes() instead of open-coding
> >     it
> 
> Your aim in writing subjects should be to write something that is unique 
> for every commit in the past or future. Because you can never make the 
> same change twice, right? (I'm excluding 'fix typos/spelling' type 
> commits). Certainly the same subject in one series is never right.
> 
> >   of/fdt: Use dt_root_addr_size_bytes() instead of open-coding it
> >   of/fdt: Fix the len check in early_init_dt_check_for_elfcorehdr()
> >   of/fdt: Fix the len check in
> >     early_init_dt_check_for_usable_mem_range()
> >   of/fdt: Use dt_root_addr_size_bytes() instead of open-coding it
> 
> This is not what I meant. We have multiple copies of this where only 
> the property name changes: 
> 
> 	prop = of_get_flat_dt_prop(node, "linux,elfcorehdr", &len);
> 	if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
> 		return;
> 
> 	elfcorehdr_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
> 	elfcorehdr_size = dt_mem_next_cell(dt_root_size_cells, &prop);
> 
> Instead, add a function something like this:
> 
> static void early_init_dt_read_address(unsigned long node, const char 
> *prop, u64 *addr, u64*size)
> {
>         prop = of_get_flat_dt_prop(node, prop, &len);
>         if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
>                 return;
> 
>         *addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
>         *size = dt_mem_next_cell(dt_root_size_cells, &prop);
> }
> 
> Then we only have the length checks in one place.
> 
> 
> That still leaves the cases with more than 1 entry open coded. So 
> instead, to cover that case to something like this:
> 
> const __be32 *of_get_flat_dt_address_prop(unsigned long node, const char 
> *propname, int *len)
> {
> 	prop = of_get_flat_dt_prop(node, propname, &len);
> 	if (!prop || (*len % (dt_root_addr_cells + dt_root_size_cells))) {
> 		*len = 0;
> 		return NULL;
> 	}
> 
> 	*len /= (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> 	return prop;
> }
> 
> And then a user would look something like this:
> 
> prop = of_get_flat_dt_address(node, "linux,usable-memory-range", &len);
> for (i = 0; i < len; i++) {
> 	of_read_address_idx(prop, i, &addr, &size);
> 	...
> }
> 
> Here 'len' is number of addr+size entries.
> 
> And the simple case of reading 1 entry could be just:
> 
> of_read_address_idx(of_get_flat_dt_address(node, "linux,elfcorehdr", NULL), 0, &addr, &size);
> 
> Rob

Hi Rob,

The link to the new patch series:

https://lore.kernel.org/linux-devicetree/20251113155104.226617-1-yuntao.wang@linux.dev/t/

Thanks,
Yuntao