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@quicinc.com
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.
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?
Thanks, Will
Fixes: 3f0c82066448 ("drivers: of: add initialization code for dynamic reserved memory") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu quic_zijuhu@quicinc.com
drivers/of/of_reserved_mem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 45517b9e57b1add36bdf2109227ebbf7df631a66..d2753756d7c30adcbd52f57338e281c16d821488 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -409,12 +409,12 @@ static int __init __reserved_mem_alloc_size(unsigned long node, const char *unam prop = of_get_flat_dt_prop(node, "alignment", &len); if (prop) {
if (len != dt_root_addr_cells * sizeof(__be32)) {
if (len != dt_root_size_cells * sizeof(__be32)) { pr_err("invalid alignment property in '%s' node.\n", uname); return -EINVAL;
align = dt_mem_next_cell(dt_root_addr_cells, &prop);
align = dt_mem_next_cell(dt_root_size_cells, &prop);nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
-- 2.34.1