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
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
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@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.
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.
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.
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
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@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.
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>
On Tue, Feb 25, 2025 at 09:46:54AM -0800, William McVicker wrote:
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@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 thought downstream kept kernels and DTs in sync, so the dts could be fixed?
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 :)
While if it is downstream, it doesn't exist, I'm reverting this for now. We need the tools to check this and look at other projects to see what they expect. Then we can think about changing the spec.
Rob
On 2025/2/27 03:45, Rob Herring wrote:
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 🙂
While if it is downstream, it doesn't exist, I'm reverting this for now.
perhaps, it is better for us to slow down here.
1) This change does not break any upstream code. is there downstream code which is publicly visible and is broken by this change ?
2) IMO, the spec may be right. The type of size is enough to express any alignment wanted. For several kernel allocators. type of 'alignment' should be the type of 'size', NOT the type of 'address'
We need the tools to check this and look at other projects to see what they expect. Then we can think about changing the spec.
On Wed, Feb 26, 2025 at 2:31 PM Zijun Hu zijun_hu@icloud.com wrote:
On 2025/2/27 03:45, Rob Herring wrote:
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 🙂
While if it is downstream, it doesn't exist, I'm reverting this for now.
perhaps, it is better for us to slow down here.
- This change does not break any upstream code. is there downstream code which is publicly visible and is broken by this change ?
We don't know that unless you tested every dts file. We only know that no one has reported an issue yet.
Even if we did test everything, there are DT's that aren't in the kernel tree. It's not like this downstream DT is using some undocumented binding or questionable things. It's a standard binding.
Every time this code is touched, it breaks. This is not even the only breakage right now[1].
- IMO, the spec may be right. The type of size is enough to express any alignment wanted. For several kernel allocators. type of 'alignment' should be the type of 'size', NOT the type of 'address'
As I said previously, it can be argued either way.
Rob
[1] https://lore.kernel.org/all/20250226115044.zw44p5dxlhy5eoni@pengutronix.de/
I thought downstream kept kernels and DTs in sync, so the dts could be fixed?
For Pixel the kernel and DT are in sync, but I'm not sure about other devices. The problem in general though is now everyone would need to do a special coordination when updating to the newer LTS version to make sure their kernel matches the new DT.
On 02/26/2025, Rob Herring wrote:
On Wed, Feb 26, 2025 at 2:31 PM Zijun Hu zijun_hu@icloud.com wrote:
On 2025/2/27 03:45, Rob Herring wrote:
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 🙂
While if it is downstream, it doesn't exist, I'm reverting this for now.
perhaps, it is better for us to slow down here.
- This change does not break any upstream code. is there downstream code which is publicly visible and is broken by this change ?
We don't know that unless you tested every dts file. We only know that no one has reported an issue yet.
Even if we did test everything, there are DT's that aren't in the kernel tree. It's not like this downstream DT is using some undocumented binding or questionable things. It's a standard binding.
Every time this code is touched, it breaks. This is not even the only breakage right now[1].
You can find the Pixel 6/7/8/9 device trees on android.googlesource.com. You can see for zuma based devices (Pixel 9 for example) they have this [1]:
&reserved_memory { #address-cells = <2>; #size-cells = <1>; vstream: vstream { compatible = "shared-dma-pool"; reusable; size = <0x4800000>; alignment = <0x0 0x00010000>; alloc-ranges = <0x9 0x80000000 0x80000000>, <0x9 0x00000000 0x80000000>, <0x8 0x80000000 0x80000000>, <0x0 0x80000000 0x80000000>; };
I understand this code is downstream, but as a general principle we shouldn't break backwards compatibilty.
Thanks, Will
[1] https://android.googlesource.com/kernel/devices/google/zuma/+/refs/heads/and...
- IMO, the spec may be right. The type of size is enough to express any alignment wanted. For several kernel allocators. type of 'alignment' should be the type of 'size', NOT the type of 'address'
As I said previously, it can be argued either way.
Rob
[1] https://lore.kernel.org/all/20250226115044.zw44p5dxlhy5eoni@pengutronix.de/
On 2025/2/27 05:36, William McVicker wrote:
Every time this code is touched, it breaks. This is not even the only breakage right now[1].
You can find the Pixel 6/7/8/9 device trees on android.googlesource.com. You can see for zuma based devices (Pixel 9 for example) they have this [1]:
&reserved_memory { #address-cells = <2>; #size-cells = <1>; vstream: vstream { compatible = "shared-dma-pool"; reusable; size = <0x4800000>; alignment = <0x0 0x00010000>; alloc-ranges = <0x9 0x80000000 0x80000000>, <0x9 0x00000000 0x80000000>, <0x8 0x80000000 0x80000000>, <0x0 0x80000000 0x80000000>; };
I understand this code is downstream, but as a general principle we shouldn't break backwards compatibilty.
this is not backward compatibility issue. it is a downstream bug instead.
normally, you need to write DTS according to relevant DT binding spec or DT spec.
i can't access the link you shared due to my country's GFW. does google kernel have extra binding spec about size of property 'alignment'?
IMO, downstream maintainers may needs to fix this issue by if the upstream fix is picked up. - alignment = <0x0 0x00010000>; + alignment = <0x00010000>;
actually, "The importance of getting code into the mainline" within Documentation/process/1.Intro.rst encourages upstream your code to avoid such issue.
On 2025/2/27 05:30, Rob Herring wrote:
this change ?
We don't know that unless you tested every dts file. We only know that no one has reported an issue yet.
Sorry, my mistake to post the question here for convenience.
actually, i want to ask William this question, and he/she shared applet of the downstream code.
Even if we did test everything, there are DT's that aren't in the kernel tree. It's not like this downstream DT is using some undocumented binding or questionable things. It's a standard binding.
IMO, that may be a downstream bug since they don't refer to binding spec to set property 'alignment'.
Every time this code is touched, it breaks. This is not even the only breakage right now[1].
indeed.
- IMO, the spec may be right. The type of size is enough to express any alignment wanted. For several kernel allocators. type of 'alignment' should be the type of 'size', NOT the type of 'address'
As I said previously, it can be argued either way.
Rob
[1] https://lore.kernel.org/all/20250226115044.zw44p5dxlhy5eoni@pengutronix.de/
On 2025/2/27 05:30, Rob Herring wrote:
Every time this code is touched, it breaks. This is not even the only breakage right now[1].
- IMO, the spec may be right. The type of size is enough to express any alignment wanted. For several kernel allocators. type of 'alignment' should be the type of 'size', NOT the type of 'address'
As I said previously, it can be argued either way.
Rob
[1] https://lore.kernel.org/all/20250226115044.zw44p5dxlhy5eoni@pengutronix.de/
this issue may be a downstream issue as well.
based on comments. they expects the CMA area is within 4G range.
but the wrong alloc-ranges config make the range (0x42000000 + 0xc0000000) > 4G across 4G boundary.
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
-
Rob Herring -
Rob Herring (Arm)
-
William McVicker -
Zijun Hu -
Zijun Hu