On 02/25/2025, Zijun Hu wrote: > On 2/25/2025 9:18 AM, William McVicker wrote: > > Hi Zijun and Rob, > > > > On 01/13/2025, Rob Herring wrote: > >> On Thu, Jan 09, 2025 at 09:27:00PM +0800, Zijun Hu wrote: > >>> From: Zijun Hu <quic_zijuhu@xxxxxxxxxxx> > >>> > >>> 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. > > > 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>