On 02/25/2025, Zijun Hu wrote:

On 2/25/2025 9:18 AM, William McVicker wrote:

quoted

Hi Zijun and Rob,

On 01/13/2025, Rob Herring wrote:

quoted

On Thu, Jan 09, 2025 at 09:27:00PM +0800, Zijun Hu wrote:

quoted

From: Zijun Hu <redacted>

According to DT spec, size of property 'alignment' is based on parent
node’s #size-cells property.

But __reserved_mem_alloc_size() wrongly uses @dt_root_addr_cells to get
the property obviously.

Fix by using @dt_root_size_cells instead of @dt_root_addr_cells.

I wonder if changing this might break someone. It's been this way for 
a long time. It might be better to change the spec or just read 
'alignment' as whatever size it happens to be (len / 4). It's not really 
the kernel's job to validate the DT. We should first have some 
validation in place to *know* if there are any current .dts files that 
would break. That would probably be easier to implement in dtc than 
dtschema. Cases of #address-cells != #size-cells should be pretty rare, 
but that was the default for OpenFirmware.

As the alignment is the base address alignment, it can be argued that 
"#address-cells" makes more sense to use than "#size-cells". So maybe 
the spec was a copy-n-paste error.

Yes, this breaks our Pixel downstream DT :( Also, the upstream Pixel 6 device
tree has cases where #address-cells != #size-cells.

it seems upstream upstream Pixel 6 has no property 'alignment'
git grep alignment arch/arm64/boot/dts/exynos/google/
so it should not be broken.

That's right. I was responding to Rob's statement about #address-cells !=
#size-cells being pretty rare. And wanted to give credance to the idea that
this change could possible break someone.

quoted

I would prefer to not have this change, but if that's not possible, could we
not backport it to all the stable branches? That way we can just force new
devices to fix this instead of existing devices on older LTS kernels?

the fix have stable and fix tags. not sure if we can control its
backporting. the fix has been backported to 6.1/6.6/6.12/6.13 automatically.

Right, I think it's already backported to the LTS kernels, but if it breaks any
in-tree users then we'd have to revert it. I just like Rob's idea to instead
change the spec for obvious reasons :)

Regards,
Will

<snip>