This patch series is to fix bugs and improve codes for drivers/of/*.
Signed-off-by: Zijun Hu quic_zijuhu@quicinc.com --- Changes in v4: - Remove 2 modalias relevant patches, and add more patches. - Link to v3: https://lore.kernel.org/r/20241217-of_core_fix-v3-0-3bc49a2e8bda@quicinc.com
Changes in v3: - Drop 2 applied patches and pick up patch 4/7 again - Fix build error for patch 6/7. - Include of_private.h instead of function declaration for patch 2/7 - Correct tile and commit messages. - Link to v2: https://lore.kernel.org/r/20241216-of_core_fix-v2-0-e69b8f60da63@quicinc.com
Changes in v2: - Drop applied/conflict/TBD patches. - Correct based on Rob's comments. - Link to v1: https://lore.kernel.org/r/20241206-of_core_fix-v1-0-dc28ed56bec3@quicinc.com
--- Zijun Hu (14): of: Correct child specifier used as input of the 2nd nexus node of: Do not expose of_alias_scan() and correct its comments of: Make of_property_present() applicable to all kinds of property of: property: Use of_property_present() for of_fwnode_property_present() of: Fix available buffer size calculating error in API of_device_uevent_modalias() of: property: Avoiding using uninitialized variable @imaplen in parse_interrupt_map() of: property: Fix potential fwnode reference's argument count got out of range of: Remove a duplicated code block of: reserved-memory: Fix using wrong number of cells to get property 'alignment' of: reserved-memory: Do not make kmemleak ignore freed address of: reserved-memory: Warn for missing static reserved memory regions of: reserved-memory: Move an assignment to effective place in __reserved_mem_alloc_size() of/fdt: Check fdt_get_mem_rsv() error in early_init_fdt_scan_reserved_mem() of: Improve __of_add_property_sysfs() readability
drivers/of/address.c | 21 +++------------------ drivers/of/base.c | 7 +++---- drivers/of/device.c | 14 ++++++++++---- drivers/of/fdt.c | 7 ++++++- drivers/of/fdt_address.c | 21 ++++----------------- drivers/of/kobj.c | 3 ++- drivers/of/of_private.h | 20 ++++++++++++++++++++ drivers/of/of_reserved_mem.c | 15 ++++++++++----- drivers/of/pdt.c | 2 ++ drivers/of/property.c | 9 +++++++-- include/linux/of.h | 24 ++++++++++++------------ 11 files changed, 79 insertions(+), 64 deletions(-) --- base-commit: 456f3000f82571697d23c255c451cfcfb5c9ae75 change-id: 20241206-of_core_fix-dc3021a06418
Best regards,
From: Zijun Hu quic_zijuhu@quicinc.com
API of_parse_phandle_with_args_map() will use wrong input for nexus node Nexus_2 as shown below:
Node_1 Nexus_1 Nexus_2 &Nexus_1,arg_1 -> arg_1,&Nexus_2,arg_2' -> &Nexus_2,arg_2 -> arg_2,... map-pass-thru=<...>
Nexus_1's output arg_2 should be used as input of Nexus_2, but the API wrongly uses arg_2' instead which != arg_2 due to Nexus_1's map-pass-thru.
Fix by always making @match_array point to @initial_match_array into which to store nexus output.
Fixes: bd6f2fd5a1d5 ("of: Support parsing phandle argument lists through a nexus node") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu quic_zijuhu@quicinc.com --- drivers/of/base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c index bf18d5997770eb81e47e749198dd505a35203d10..969b99838655534915882abe358814d505c6f748 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1536,7 +1536,6 @@ int of_parse_phandle_with_args_map(const struct device_node *np, * specifier into the out_args structure, keeping the * bits specified in <list>-map-pass-thru. */ - match_array = map - new_size; for (i = 0; i < new_size; i++) { __be32 val = *(map - new_size + i);
@@ -1545,6 +1544,7 @@ int of_parse_phandle_with_args_map(const struct device_node *np, val |= cpu_to_be32(out_args->args[i]) & pass[i]; }
+ initial_match_array[i] = val; out_args->args[i] = be32_to_cpu(val); } out_args->args_count = list_size = new_size;
On Thu, 09 Jan 2025 21:26:52 +0800, Zijun Hu wrote:
From: Zijun Hu quic_zijuhu@quicinc.com
API of_parse_phandle_with_args_map() will use wrong input for nexus node Nexus_2 as shown below:
Node_1 Nexus_1 Nexus_2
&Nexus_1,arg_1 -> arg_1,&Nexus_2,arg_2' -> &Nexus_2,arg_2 -> arg_2,... map-pass-thru=<...>
Nexus_1's output arg_2 should be used as input of Nexus_2, but the API wrongly uses arg_2' instead which != arg_2 due to Nexus_1's map-pass-thru.
Fix by always making @match_array point to @initial_match_array into which to store nexus output.
Fixes: bd6f2fd5a1d5 ("of: Support parsing phandle argument lists through a nexus node") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu quic_zijuhu@quicinc.com
drivers/of/base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks!
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.
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;
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.
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
From: Zijun Hu quic_zijuhu@quicinc.com
For child node of /reserved-memory, its property 'reg' may contain multiple regions, but fdt_scan_reserved_mem_reg_nodes() only takes into account the first region, and miss remaining regions.
Give warning message when missing remaining regions.
Fixes: 8a6e02d0c00e ("of: reserved_mem: Restructure how the reserved memory regions are processed") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu quic_zijuhu@quicinc.com --- drivers/of/of_reserved_mem.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 03a8f03ed1da165d6d7bf907d931857260888225..e2da88d7706ab3c208386e29f31350178e67cd14 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -263,6 +263,11 @@ void __init fdt_scan_reserved_mem_reg_nodes(void) uname); continue; } + + if (len > t_len) + pr_warn("%s() ignores %d regions in node '%s'\n", + __func__, len / t_len - 1, uname); + base = dt_mem_next_cell(dt_root_addr_cells, &prop); size = dt_mem_next_cell(dt_root_size_cells, &prop);
On Thu, Jan 09, 2025 at 09:27:02PM +0800, Zijun Hu wrote:
From: Zijun Hu quic_zijuhu@quicinc.com
For child node of /reserved-memory, its property 'reg' may contain multiple regions, but fdt_scan_reserved_mem_reg_nodes() only takes into account the first region, and miss remaining regions.
Give warning message when missing remaining regions.
Can't we just fix it to support more than 1 entry?
Fixes: 8a6e02d0c00e ("of: reserved_mem: Restructure how the reserved memory regions are processed") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu quic_zijuhu@quicinc.com
drivers/of/of_reserved_mem.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 03a8f03ed1da165d6d7bf907d931857260888225..e2da88d7706ab3c208386e29f31350178e67cd14 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -263,6 +263,11 @@ void __init fdt_scan_reserved_mem_reg_nodes(void) uname); continue; }
if (len > t_len)
pr_warn("%s() ignores %d regions in node '%s'\n",
__func__, len / t_len - 1, uname);
- base = dt_mem_next_cell(dt_root_addr_cells, &prop); size = dt_mem_next_cell(dt_root_size_cells, &prop);
-- 2.34.1
On 2025/1/14 07:16, Rob Herring wrote:
From: Zijun Hu quic_zijuhu@quicinc.com
For child node of /reserved-memory, its property 'reg' may contain multiple regions, but fdt_scan_reserved_mem_reg_nodes() only takes into account the first region, and miss remaining regions.
Give warning message when missing remaining regions.
Can't we just fix it to support more than 1 entry?
actually, i ever considered supporting more entries. and find out there are no simple approach to achieve this aim.
it may need to change 'struct reserved_mem' to support multiple regions that may also involve 4 RESERVEDMEM_OF_DECLARE instances.
RESERVEDMEM_OF_DECLARE(tegra210_emc_table, "nvidia,tegra210-emc-table", tegra210_emc_table_init); RESERVEDMEM_OF_DECLARE(dma, "shared-dma-pool", rmem_dma_setup); RESERVEDMEM_OF_DECLARE(cma, "shared-dma-pool", rmem_cma_setup); RESERVEDMEM_OF_DECLARE(dma, "restricted-dma-pool", rmem_swiotlb_setup);
so i finally chose this warning way due to simplification and low risk.
linux-stable-mirror@lists.linaro.org