From 65f4be07ad1010f41dbe329398da881a38051c98 Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Sun, 9 Feb 2025 20:58:54 +0800 Subject: of: unittest: Add a case to test if API of_irq_parse_one() leaks refcount To test if of_irq_parse_one(@int_gen_dev, i, ...) will leak refcount of @i_th_phandle. int_gen_dev { ... interrupts-extended = ..., <&i_th_phandle ...>, ...; ... }; Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250209-of_irq_fix-v2-1-93e3a2659aa7@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/unittest.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) (limited to 'drivers') diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index f88ddb1cf5d7..48aec4695fff 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -1654,6 +1654,50 @@ static void __init of_unittest_parse_interrupts_extended(void) of_node_put(np); } +#if IS_ENABLED(CONFIG_OF_DYNAMIC) +static void __init of_unittest_irq_refcount(void) +{ + struct of_phandle_args args; + struct device_node *intc0, *int_ext0; + unsigned int ref_c0, ref_c1, ref_c2; + int rc; + bool passed; + + if (of_irq_workarounds & OF_IMAP_OLDWORLD_MAC) + return; + + intc0 = of_find_node_by_path("/testcase-data/interrupts/intc0"); + int_ext0 = of_find_node_by_path("/testcase-data/interrupts/interrupts-extended0"); + if (!intc0 || !int_ext0) { + pr_err("missing testcase data\n"); + goto out; + } + + /* Test refcount for API of_irq_parse_one() */ + passed = true; + ref_c0 = OF_KREF_READ(intc0); + ref_c1 = ref_c0 + 1; + memset(&args, 0, sizeof(args)); + rc = of_irq_parse_one(int_ext0, 0, &args); + ref_c2 = OF_KREF_READ(intc0); + of_node_put(args.np); + + passed &= !rc; + passed &= (args.np == intc0); + passed &= (args.args_count == 1); + passed &= (args.args[0] == 1); + passed &= (ref_c1 == ref_c2); + unittest(passed, "IRQ refcount case #1 failed, original(%u) expected(%u) got(%u)\n", + ref_c0, ref_c1, ref_c2); + +out: + of_node_put(int_ext0); + of_node_put(intc0); +} +#else +static inline void __init of_unittest_irq_refcount(void) { } +#endif + static const struct of_device_id match_node_table[] = { { .data = "A", .name = "name0", }, /* Name alone is lowest priority */ { .data = "B", .type = "type1", }, /* followed by type alone */ @@ -4324,6 +4368,7 @@ static int __init of_unittest(void) of_unittest_changeset_prop(); of_unittest_parse_interrupts(); of_unittest_parse_interrupts_extended(); + of_unittest_irq_refcount(); of_unittest_dma_get_max_cpu_address(); of_unittest_parse_dma_ranges(); of_unittest_pci_dma_ranges(); -- cgit v1.2.3-59-g8ed1b From 0cb58d6c7b558a69957fabe159bfb184196e1e8d Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Sun, 9 Feb 2025 20:58:55 +0800 Subject: of/irq: Fix device node refcount leakage in API of_irq_parse_one() of_irq_parse_one(@int_gen_dev, i, ...) will leak refcount of @i_th_phandle int_gen_dev { ... interrupts-extended = ..., <&i_th_phandle ...>, ...; ... }; Refcount of @i_th_phandle is increased by of_parse_phandle_with_args() but is not decreased by API of_irq_parse_one() before return, so causes refcount leakage. Rework the refcounting to use __free() cleanup and simplify the code to have a single call to of_irq_parse_raw(). Also add comments about refcount of node @out_irq->np got by the API. Fixes: 79d9701559a9 ("of/irq: create interrupts-extended property") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250209-of_irq_fix-v2-2-93e3a2659aa7@quicinc.com [robh: Use __free() to do puts] Signed-off-by: Rob Herring (Arm) --- drivers/of/irq.c | 59 ++++++++++++++++++++++++++------------------------------ 1 file changed, 27 insertions(+), 32 deletions(-) (limited to 'drivers') diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 6c843d54ebb1..95456e918681 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -16,6 +16,7 @@ #define pr_fmt(fmt) "OF: " fmt +#include #include #include #include @@ -339,10 +340,12 @@ EXPORT_SYMBOL_GPL(of_irq_parse_raw); * This function resolves an interrupt for a node by walking the interrupt tree, * finding which interrupt controller node it is attached to, and returning the * interrupt specifier that can be used to retrieve a Linux IRQ number. + * + * Note: refcount of node @out_irq->np is increased by 1 on success. */ int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_args *out_irq) { - struct device_node *p; + struct device_node __free(device_node) *p = NULL; const __be32 *addr; u32 intsize; int i, res, addr_len; @@ -367,41 +370,33 @@ int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_ar /* Try the new-style interrupts-extended first */ res = of_parse_phandle_with_args(device, "interrupts-extended", "#interrupt-cells", index, out_irq); - if (!res) - return of_irq_parse_raw(addr_buf, out_irq); - - /* Look for the interrupt parent. */ - p = of_irq_find_parent(device); - if (p == NULL) - return -EINVAL; - - /* Get size of interrupt specifier */ - if (of_property_read_u32(p, "#interrupt-cells", &intsize)) { - res = -EINVAL; - goto out; - } - - pr_debug(" parent=%pOF, intsize=%d\n", p, intsize); + if (!res) { + p = out_irq->np; + } else { + /* Look for the interrupt parent. */ + p = of_irq_find_parent(device); + /* Get size of interrupt specifier */ + if (!p || of_property_read_u32(p, "#interrupt-cells", &intsize)) + return -EINVAL; + + pr_debug(" parent=%pOF, intsize=%d\n", p, intsize); + + /* Copy intspec into irq structure */ + out_irq->np = p; + out_irq->args_count = intsize; + for (i = 0; i < intsize; i++) { + res = of_property_read_u32_index(device, "interrupts", + (index * intsize) + i, + out_irq->args + i); + if (res) + return res; + } - /* Copy intspec into irq structure */ - out_irq->np = p; - out_irq->args_count = intsize; - for (i = 0; i < intsize; i++) { - res = of_property_read_u32_index(device, "interrupts", - (index * intsize) + i, - out_irq->args + i); - if (res) - goto out; + pr_debug(" intspec=%d\n", *out_irq->args); } - pr_debug(" intspec=%d\n", *out_irq->args); - - /* Check if there are any interrupt-map translations to process */ - res = of_irq_parse_raw(addr_buf, out_irq); - out: - of_node_put(p); - return res; + return of_irq_parse_raw(addr_buf, out_irq); } EXPORT_SYMBOL_GPL(of_irq_parse_one); -- cgit v1.2.3-59-g8ed1b From f8647991e07f42d1525c63cfa5a021b3869ef630 Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Sun, 9 Feb 2025 20:58:56 +0800 Subject: of: unittest: Add a case to test if API of_irq_parse_raw() leaks refcount To test if of_irq_parse_raw(), invoked by of_irq_parse_one(), will leak refcount of interrupt combo node consisting of controller and nexus. Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250209-of_irq_fix-v2-3-93e3a2659aa7@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/unittest-data/tests-interrupts.dtsi | 13 +++++++++++++ drivers/of/unittest.c | 24 +++++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/unittest-data/tests-interrupts.dtsi b/drivers/of/unittest-data/tests-interrupts.dtsi index 7c9f31cc131b..4ccb54f91c30 100644 --- a/drivers/of/unittest-data/tests-interrupts.dtsi +++ b/drivers/of/unittest-data/tests-interrupts.dtsi @@ -50,6 +50,13 @@ interrupt-map = <0x5000 1 2 &test_intc0 15>; }; + test_intc_intmap0: intc-intmap0 { + #interrupt-cells = <1>; + #address-cells = <1>; + interrupt-controller; + interrupt-map = <0x6000 1 &test_intc_intmap0 0x7000 2>; + }; + interrupts0 { interrupt-parent = <&test_intc0>; interrupts = <1>, <2>, <3>, <4>; @@ -60,6 +67,12 @@ interrupts = <1>, <2>, <3>, <4>; }; + interrupts2 { + reg = <0x6000 0x100>; + interrupt-parent = <&test_intc_intmap0>; + interrupts = <1>; + }; + interrupts-extended0 { reg = <0x5000 0x100>; /* diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 48aec4695fff..64d301893af7 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -1659,6 +1659,7 @@ static void __init of_unittest_irq_refcount(void) { struct of_phandle_args args; struct device_node *intc0, *int_ext0; + struct device_node *int2, *intc_intmap0; unsigned int ref_c0, ref_c1, ref_c2; int rc; bool passed; @@ -1668,7 +1669,9 @@ static void __init of_unittest_irq_refcount(void) intc0 = of_find_node_by_path("/testcase-data/interrupts/intc0"); int_ext0 = of_find_node_by_path("/testcase-data/interrupts/interrupts-extended0"); - if (!intc0 || !int_ext0) { + intc_intmap0 = of_find_node_by_path("/testcase-data/interrupts/intc-intmap0"); + int2 = of_find_node_by_path("/testcase-data/interrupts/interrupts2"); + if (!intc0 || !int_ext0 || !intc_intmap0 || !int2) { pr_err("missing testcase data\n"); goto out; } @@ -1690,7 +1693,26 @@ static void __init of_unittest_irq_refcount(void) unittest(passed, "IRQ refcount case #1 failed, original(%u) expected(%u) got(%u)\n", ref_c0, ref_c1, ref_c2); + /* Test refcount for API of_irq_parse_raw() */ + passed = true; + ref_c0 = OF_KREF_READ(intc_intmap0); + ref_c1 = ref_c0 + 1; + memset(&args, 0, sizeof(args)); + rc = of_irq_parse_one(int2, 0, &args); + ref_c2 = OF_KREF_READ(intc_intmap0); + of_node_put(args.np); + + passed &= !rc; + passed &= (args.np == intc_intmap0); + passed &= (args.args_count == 1); + passed &= (args.args[0] == 2); + passed &= (ref_c1 == ref_c2); + unittest(passed, "IRQ refcount case #2 failed, original(%u) expected(%u) got(%u)\n", + ref_c0, ref_c1, ref_c2); + out: + of_node_put(int2); + of_node_put(intc_intmap0); of_node_put(int_ext0); of_node_put(intc0); } -- cgit v1.2.3-59-g8ed1b From ff93e7213d6cc8d9a7b0bc64f70ed26094e168f3 Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Sun, 9 Feb 2025 20:58:57 +0800 Subject: of/irq: Fix device node refcount leakage in API of_irq_parse_raw() if the node @out_irq->np got by of_irq_parse_raw() is a combo node which consists of both controller and nexus, namely, of_irq_parse_raw() returns due to condition (@ipar == @newpar), then the node's refcount was increased twice, hence causes refcount leakage. Fix by putting @out_irq->np refcount before returning due to the condition. Also add comments about refcount of node @out_irq->np got by the API. Fixes: 041284181226 ("of/irq: Allow matching of an interrupt-map local to an interrupt controller") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250209-of_irq_fix-v2-4-93e3a2659aa7@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/irq.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers') diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 95456e918681..1a018726fb0e 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -166,6 +166,8 @@ const __be32 *of_irq_parse_imap_parent(const __be32 *imap, int len, struct of_ph * the specifier for each map, and then returns the translated map. * * Return: 0 on success and a negative number on error + * + * Note: refcount of node @out_irq->np is increased by 1 on success. */ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq) { @@ -311,6 +313,12 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq) addrsize = (imap - match_array) - intsize; if (ipar == newpar) { + /* + * We got @ipar's refcount, but the refcount was + * gotten again by of_irq_parse_imap_parent() via its + * alias @newpar. + */ + of_node_put(ipar); pr_debug("%pOF interrupt-map entry to self\n", ipar); return 0; } -- cgit v1.2.3-59-g8ed1b From bbf71f44aaf241d853759a71de7e7ebcdb89be3d Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Sun, 9 Feb 2025 20:58:58 +0800 Subject: of/irq: Fix device node refcount leakages in of_irq_count() of_irq_count() invokes of_irq_parse_one() to count IRQs, and successful invocation of the later will get device node @irq.np refcount, but the former does not put the refcount before next iteration invocation, hence causes device node refcount leakages. Fix by putting @irq.np refcount before the next iteration invocation. Fixes: 3da5278727a8 ("of/irq: Rework of_irq_count()") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250209-of_irq_fix-v2-5-93e3a2659aa7@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/irq.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 1a018726fb0e..75b471327903 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -508,8 +508,10 @@ int of_irq_count(struct device_node *dev) struct of_phandle_args irq; int nr = 0; - while (of_irq_parse_one(dev, nr, &irq) == 0) + while (of_irq_parse_one(dev, nr, &irq) == 0) { + of_node_put(irq.np); nr++; + } return nr; } -- cgit v1.2.3-59-g8ed1b From 962a2805e47b933876ba0e4c488d9e89ced2dd29 Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Sun, 9 Feb 2025 20:58:59 +0800 Subject: of/irq: Fix device node refcount leakage in API irq_of_parse_and_map() In irq_of_parse_and_map(), refcount of device node @oirq.np was got by successful of_irq_parse_one() invocation, but it does not put the refcount before return, so causes @oirq.np refcount leakage. Fix by putting @oirq.np refcount before return. Fixes: e3873444990d ("of/irq: Move irq_of_parse_and_map() to common code") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250209-of_irq_fix-v2-6-93e3a2659aa7@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/irq.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 75b471327903..2f8dcb77a800 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -39,11 +39,15 @@ unsigned int irq_of_parse_and_map(struct device_node *dev, int index) { struct of_phandle_args oirq; + unsigned int ret; if (of_irq_parse_one(dev, index, &oirq)) return 0; - return irq_create_of_mapping(&oirq); + ret = irq_create_of_mapping(&oirq); + of_node_put(oirq.np); + + return ret; } EXPORT_SYMBOL_GPL(irq_of_parse_and_map); -- cgit v1.2.3-59-g8ed1b From 708124d9e6e7ac5ebf927830760679136b23fdf0 Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Sun, 9 Feb 2025 20:59:00 +0800 Subject: of/irq: Fix device node refcount leakages in of_irq_init() of_irq_init() will leak interrupt controller device node refcounts in two places as explained below: 1) Leak refcounts of both @desc->dev and @desc->interrupt_parent when suffers @desc->irq_init_cb() failure. 2) Leak refcount of @desc->interrupt_parent when cleans up list @intc_desc_list in the end. Refcounts of both @desc->dev and @desc->interrupt_parent were got in the first loop, but of_irq_init() does not put them before kfree(@desc) in places mentioned above, so causes refcount leakages. Fix by putting refcounts involved before kfree(@desc). Fixes: 8363ccb917c6 ("of/irq: add missing of_node_put") Fixes: c71a54b08201 ("of/irq: introduce of_irq_init") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250209-of_irq_fix-v2-7-93e3a2659aa7@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/irq.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers') diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 2f8dcb77a800..f5459ad50f36 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -632,6 +632,8 @@ void __init of_irq_init(const struct of_device_id *matches) __func__, desc->dev, desc->dev, desc->interrupt_parent); of_node_clear_flag(desc->dev, OF_POPULATED); + of_node_put(desc->interrupt_parent); + of_node_put(desc->dev); kfree(desc); continue; } @@ -662,6 +664,7 @@ void __init of_irq_init(const struct of_device_id *matches) err: list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) { list_del(&desc->list); + of_node_put(desc->interrupt_parent); of_node_put(desc->dev); kfree(desc); } -- cgit v1.2.3-59-g8ed1b From 4bafd71a38c2f96901fee99325c4451161e4c9bd Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Sun, 9 Feb 2025 20:59:01 +0800 Subject: of/irq: Add comments about refcount for API of_irq_find_parent() Add comments about refcount of the node returned by of_irq_find_parent(). Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250209-of_irq_fix-v2-8-93e3a2659aa7@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/of/irq.c b/drivers/of/irq.c index f5459ad50f36..f8ad79b9b1c9 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -55,8 +55,8 @@ EXPORT_SYMBOL_GPL(irq_of_parse_and_map); * of_irq_find_parent - Given a device node, find its interrupt parent node * @child: pointer to device node * - * Return: A pointer to the interrupt parent node, or NULL if the interrupt - * parent could not be determined. + * Return: A pointer to the interrupt parent node with refcount increased + * or NULL if the interrupt parent could not be determined. */ struct device_node *of_irq_find_parent(struct device_node *child) { -- cgit v1.2.3-59-g8ed1b From 5275e8b5293f65cc82a5ee5eab02dd573b911d6e Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Sun, 9 Feb 2025 20:59:02 +0800 Subject: of: resolver: Simplify of_resolve_phandles() using __free() Use the __free() cleanup to simplify of_resolve_phandles() and remove all the goto's. Signed-off-by: Rob Herring (Arm) --- drivers/of/resolver.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) (limited to 'drivers') diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 779db058c42f..c871e35d4921 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -250,24 +250,20 @@ static int adjust_local_phandle_references(const struct device_node *local_fixup int of_resolve_phandles(struct device_node *overlay) { struct device_node *child, *local_fixups, *refnode; - struct device_node *tree_symbols, *overlay_fixups; + struct device_node *overlay_fixups; struct property *prop; const char *refpath; phandle phandle, phandle_delta; int err; - tree_symbols = NULL; - if (!overlay) { pr_err("null overlay\n"); - err = -EINVAL; - goto out; + return -EINVAL; } if (!of_node_check_flag(overlay, OF_DETACHED)) { pr_err("overlay not detached\n"); - err = -EINVAL; - goto out; + return -EINVAL; } phandle_delta = live_tree_max_phandle() + 1; @@ -279,7 +275,7 @@ int of_resolve_phandles(struct device_node *overlay) err = adjust_local_phandle_references(local_fixups, overlay, phandle_delta); if (err) - goto out; + return err; overlay_fixups = NULL; @@ -288,16 +284,13 @@ int of_resolve_phandles(struct device_node *overlay) overlay_fixups = child; } - if (!overlay_fixups) { - err = 0; - goto out; - } + if (!overlay_fixups) + return 0; - tree_symbols = of_find_node_by_path("/__symbols__"); + struct device_node __free(device_node) *tree_symbols = of_find_node_by_path("/__symbols__"); if (!tree_symbols) { pr_err("no symbols in root of device tree.\n"); - err = -EINVAL; - goto out; + return -EINVAL; } for_each_property_of_node(overlay_fixups, prop) { @@ -311,14 +304,12 @@ int of_resolve_phandles(struct device_node *overlay) if (err) { pr_err("node label '%s' not found in live devicetree symbols table\n", prop->name); - goto out; + return err; } refnode = of_find_node_by_path(refpath); - if (!refnode) { - err = -ENOENT; - goto out; - } + if (!refnode) + return -ENOENT; phandle = refnode->phandle; of_node_put(refnode); @@ -328,11 +319,8 @@ int of_resolve_phandles(struct device_node *overlay) break; } -out: if (err) pr_err("overlay phandle fixup failed: %d\n", err); - of_node_put(tree_symbols); - return err; } EXPORT_SYMBOL_GPL(of_resolve_phandles); -- cgit v1.2.3-59-g8ed1b From a46a0805635d07de50c2ac71588345323c13b2f9 Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Mon, 24 Feb 2025 17:01:55 -0600 Subject: of: resolver: Fix device node refcount leakage in of_resolve_phandles() In of_resolve_phandles(), refcount of device node @local_fixups will be increased if the for_each_child_of_node() exits early, but nowhere to decrease the refcount, so cause refcount leakage for the node. Fix by using __free() on @local_fixups. Fixes: da56d04c806a ("of/resolver: Switch to new local fixups format.") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250209-of_irq_fix-v2-9-93e3a2659aa7@quicinc.com [robh: Use __free() instead] Signed-off-by: Rob Herring (Arm) --- drivers/of/resolver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index c871e35d4921..2caad365a665 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -249,8 +249,9 @@ static int adjust_local_phandle_references(const struct device_node *local_fixup */ int of_resolve_phandles(struct device_node *overlay) { - struct device_node *child, *local_fixups, *refnode; + struct device_node *child, *refnode; struct device_node *overlay_fixups; + struct device_node __free(device_node) *local_fixups = NULL; struct property *prop; const char *refpath; phandle phandle, phandle_delta; -- cgit v1.2.3-59-g8ed1b From 56d733bb8f99a572e3b35a307057e019c1974026 Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Mon, 24 Feb 2025 22:27:57 +0800 Subject: of: Compare property names by of_prop_cmp() in of_alias_scan() For these pseudo property names 'name', 'phandle' and 'linux,phandle': Use dedicated property name comparison macro of_prop_cmp() instead of strcmp() in of_alias_scan() to: - Make property name comparison consistent. - Prepare for introducing private is_pseudo_property() later. Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250224-of_bugfix-v1-1-03640ae8c3a6@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/base.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/of/base.c b/drivers/of/base.c index af6c68bbb427..d2d41601136b 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1855,9 +1855,9 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align)) int id, len; /* Skip those we do not want to proceed */ - if (!strcmp(pp->name, "name") || - !strcmp(pp->name, "phandle") || - !strcmp(pp->name, "linux,phandle")) + if (!of_prop_cmp(pp->name, "name") || + !of_prop_cmp(pp->name, "phandle") || + !of_prop_cmp(pp->name, "linux,phandle")) continue; np = of_find_node_by_path(pp->value); -- cgit v1.2.3-59-g8ed1b From f443029c9a6e9515582ee2dfe7014a9be8a4a98a Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Mon, 24 Feb 2025 22:27:58 +0800 Subject: of: Introduce and apply private is_pseudo_property() There are several places which check if a property name is one of 'name'|'phandle'|'linux,phandle'. Introduce and apply private is_pseudo_property() for the check. Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250224-of_bugfix-v1-2-03640ae8c3a6@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/base.c | 4 +--- drivers/of/of_private.h | 7 +++++++ drivers/of/overlay.c | 4 +--- drivers/of/resolver.c | 4 +--- 4 files changed, 10 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/of/base.c b/drivers/of/base.c index d2d41601136b..001ff6ce4abf 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1855,9 +1855,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align)) int id, len; /* Skip those we do not want to proceed */ - if (!of_prop_cmp(pp->name, "name") || - !of_prop_cmp(pp->name, "phandle") || - !of_prop_cmp(pp->name, "linux,phandle")) + if (is_pseudo_property(pp->name)) continue; np = of_find_node_by_path(pp->value); diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index f3e1193c8ded..b0c077867bf4 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -208,4 +208,11 @@ static void __maybe_unused of_dump_addr(const char *s, const __be32 *addr, int n static void __maybe_unused of_dump_addr(const char *s, const __be32 *addr, int na) { } #endif +static inline bool is_pseudo_property(const char *prop_name) +{ + return !of_prop_cmp(prop_name, "name") || + !of_prop_cmp(prop_name, "phandle") || + !of_prop_cmp(prop_name, "linux,phandle"); +} + #endif /* _LINUX_OF_PRIVATE_H */ diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 434f6dd6a86c..5a51c52b9729 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -304,9 +304,7 @@ static int add_changeset_property(struct overlay_changeset *ovcs, int ret = 0; if (target->in_livetree) - if (!of_prop_cmp(overlay_prop->name, "name") || - !of_prop_cmp(overlay_prop->name, "phandle") || - !of_prop_cmp(overlay_prop->name, "linux,phandle")) + if (is_pseudo_property(overlay_prop->name)) return 0; if (target->in_livetree) diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 2caad365a665..86424e145919 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -161,9 +161,7 @@ static int adjust_local_phandle_references(const struct device_node *local_fixup for_each_property_of_node(local_fixups, prop_fix) { /* skip properties added automatically */ - if (!of_prop_cmp(prop_fix->name, "name") || - !of_prop_cmp(prop_fix->name, "phandle") || - !of_prop_cmp(prop_fix->name, "linux,phandle")) + if (is_pseudo_property(prop_fix->name)) continue; if ((prop_fix->length % 4) != 0 || prop_fix->length == 0) -- cgit v1.2.3-59-g8ed1b From b41838312e24f69d28d1b81c9b9beef55f31215d Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Mon, 24 Feb 2025 22:27:59 +0800 Subject: of: Correct property name comparison in __of_add_property() __of_add_property() compares property name by strcmp(), and that is improper for SPARC which wants strcasecmp(). Fix by using dedicated property name comparison macro of_prop_cmp(). Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250224-of_bugfix-v1-3-03640ae8c3a6@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/base.c b/drivers/of/base.c index 001ff6ce4abf..c810014957e8 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1651,7 +1651,7 @@ int __of_add_property(struct device_node *np, struct property *prop) prop->next = NULL; next = &np->properties; while (*next) { - if (strcmp(prop->name, (*next)->name) == 0) { + if (of_prop_cmp(prop->name, (*next)->name) == 0) { /* duplicate ! don't insert it */ rc = -EEXIST; goto out_unlock; -- cgit v1.2.3-59-g8ed1b From 161e7e4671e6eb09b1d9ae61dbcf48f4c2b337b7 Mon Sep 17 00:00:00 2001 From: Zijun Hu Date: Mon, 24 Feb 2025 22:28:01 +0800 Subject: of/platform: Do not use of_get_property() to test property presence Use of_property_present() instead of of_get_property() to test property 'compatible' presence in of_platform_bus_create(). Signed-off-by: Zijun Hu Link: https://lore.kernel.org/r/20250224-of_bugfix-v1-5-03640ae8c3a6@quicinc.com Signed-off-by: Rob Herring (Arm) --- drivers/of/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/platform.c b/drivers/of/platform.c index c6d8afb284e8..242172e4b875 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -334,7 +334,7 @@ static int of_platform_bus_create(struct device_node *bus, int rc = 0; /* Make sure it has a compatible property */ - if (strict && (!of_get_property(bus, "compatible", NULL))) { + if (strict && (!of_property_present(bus, "compatible"))) { pr_debug("%s() - skipping %pOF, no compatible prop\n", __func__, bus); return 0; -- cgit v1.2.3-59-g8ed1b From 3367838f5549d48172bce068ee958f715ae80428 Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Wed, 12 Mar 2025 16:29:36 -0500 Subject: of/platform: Use typed accessors rather than of_get_property() Use the typed of_property_* functions rather than of_get_property() which leaks pointers to DT data without any control of the lifetime. Signed-off-by: "Rob Herring (Arm)" Link: https://lore.kernel.org/r/20250312212937.1067088-1-robh@kernel.org Signed-off-by: Rob Herring (Arm) --- drivers/of/platform.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 242172e4b875..f77cb19973a5 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -536,8 +536,8 @@ static int __init of_platform_default_populate_init(void) * ignore errors for the rest. */ for_each_node_by_type(node, "display") { - if (!of_get_property(node, "linux,opened", NULL) || - !of_get_property(node, "linux,boot-display", NULL)) + if (!of_property_read_bool(node, "linux,opened") || + !of_property_read_bool(node, "linux,boot-display")) continue; dev = of_platform_device_create(node, "of-display", NULL); of_node_put(node); @@ -551,7 +551,7 @@ static int __init of_platform_default_populate_init(void) char buf[14]; const char *of_display_format = "of-display.%d"; - if (!of_get_property(node, "linux,opened", NULL) || node == boot_display) + if (!of_property_read_bool(node, "linux,opened") || node == boot_display) continue; ret = snprintf(buf, sizeof(buf), of_display_format, display_number++); if (ret < sizeof(buf)) -- cgit v1.2.3-59-g8ed1b From 590f5d6752f7d951d3549b259c1436940131703b Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Wed, 12 Mar 2025 16:29:46 -0500 Subject: of: Move of_prop_val_eq() next to the single user There's only a single user of of_prop_val_eq(), so move it to overlay.c. This removes one case of exposing struct property outside of the DT code. Signed-off-by: "Rob Herring (Arm)" Link: https://lore.kernel.org/r/20250312212947.1067337-1-robh@kernel.org Signed-off-by: Rob Herring (Arm) --- drivers/of/overlay.c | 6 ++++++ include/linux/of.h | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 5a51c52b9729..1af6f52d0708 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -84,6 +84,12 @@ static int devicetree_state_flags; #define DTSF_APPLY_FAIL 0x01 #define DTSF_REVERT_FAIL 0x02 +static int of_prop_val_eq(const struct property *p1, const struct property *p2) +{ + return p1->length == p2->length && + !memcmp(p1->value, p2->value, (size_t)p1->length); +} + /* * If a changeset apply or revert encounters an error, an attempt will * be made to undo partial changes, but may fail. If the undo fails diff --git a/include/linux/of.h b/include/linux/of.h index 86bf8f073111..e95a02db321a 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -908,12 +908,6 @@ static inline const void *of_device_get_match_data(const struct device *dev) #define of_node_cmp(s1, s2) strcasecmp((s1), (s2)) #endif -static inline int of_prop_val_eq(const struct property *p1, const struct property *p2) -{ - return p1->length == p2->length && - !memcmp(p1->value, p2->value, (size_t)p1->length); -} - #define for_each_property_of_node(dn, pp) \ for (pp = dn->properties; pp != NULL; pp = pp->next) -- cgit v1.2.3-59-g8ed1b From 7f623466b690a6da5b9b9fc821c8bffe11fea5ff Mon Sep 17 00:00:00 2001 From: Konrad Dybcio Date: Wed, 19 Mar 2025 15:25:57 +0100 Subject: of: address: Expand nonposted-mmio to non-Apple Silicon platforms The nE memory attribute may be utilized by various implementations, not limited to Apple Silicon platforms. Drop the early CONFIG_ARCH_APPLE check. Signed-off-by: Konrad Dybcio Link: https://lore.kernel.org/r/20250319-topic-nonposted_mmio-v1-1-dfb886fbd15f@oss.qualcomm.com Signed-off-by: Rob Herring (Arm) --- drivers/of/address.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'drivers') diff --git a/drivers/of/address.c b/drivers/of/address.c index 125833e5ce52..0ed35a4f9205 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -1028,15 +1028,9 @@ EXPORT_SYMBOL_GPL(of_dma_is_coherent); * * Returns true if the "nonposted-mmio" property was found for * the device's bus. - * - * This is currently only enabled on builds that support Apple ARM devices, as - * an optimization. */ static bool of_mmio_is_nonposted(const struct device_node *np) { - if (!IS_ENABLED(CONFIG_ARCH_APPLE)) - return false; - struct device_node *parent __free(device_node) = of_get_parent(np); if (!parent) return false; -- cgit v1.2.3-59-g8ed1b From 47026c4ffedd2cd664e3f70f63c4149c56355f37 Mon Sep 17 00:00:00 2001 From: Konrad Dybcio Date: Wed, 19 Mar 2025 15:25:58 +0100 Subject: of: address: Allow to specify nonposted-mmio per-device Certain IP blocks may strictly require/expect a nE mapping to function correctly, while others may be fine without it (which is preferred for performance reasons). Allow specifying nonposted-mmio on a per-device basis. Signed-off-by: Konrad Dybcio Link: https://lore.kernel.org/r/20250319-topic-nonposted_mmio-v1-2-dfb886fbd15f@oss.qualcomm.com Signed-off-by: Rob Herring (Arm) --- drivers/of/address.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/of/address.c b/drivers/of/address.c index 0ed35a4f9205..cb2212b13375 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -1032,10 +1032,11 @@ EXPORT_SYMBOL_GPL(of_dma_is_coherent); static bool of_mmio_is_nonposted(const struct device_node *np) { struct device_node *parent __free(device_node) = of_get_parent(np); - if (!parent) - return false; - return of_property_read_bool(parent, "nonposted-mmio"); + if (of_property_read_bool(np, "nonposted-mmio")) + return true; + + return parent && of_property_read_bool(parent, "nonposted-mmio"); } static int __of_address_to_resource(struct device_node *dev, int index, int bar_no, -- cgit v1.2.3-59-g8ed1b