[PATCH v2 0/2] ARM: l2c: honour status = "disabled" on the L2 cache node

[PATCH v2 0/2] ARM: l2c: honour status = "disabled" on the L2 cache node

[Agustin Santiago Isasmendi]

Hi Russell,

l2x0_of_init() finds the L2 cache node by compatible and never looks at
its status property, so a device tree cannot keep the L2 cache off.
Patch 2 makes it leave a disabled node alone; patch 1 notes in the
binding that the firmware must then leave the cache disabled.

The user is the ZTE ZX279128S (dual Cortex-A9, L2C-310 r3p3), where a
PCIe read on one CPU deadlocks against an L2C-310 line operation by
physical address or a Cache Sync on the other CPU, and the board hangs
within seconds of WiFi traffic.  Running with the L2 cache off avoids
it, and that was suggested as the fallback for the platform when a
lock-based workaround was found too invasive for generic code:

  https://lore.kernel.org/r/20261110153522.1620088-1-isasmendi.agus@gmail.com

Whether the ZX279128S device tree disables the cache is a decision for
that thread; this series only makes the property work.

No in-tree L2 cache node is disabled, so existing device trees are not
affected.  Tested on the ZTE ZXHN H3600 (not in mainline yet, on top of
Navid Ghahremani's v2 platform series): with the node disabled the cache
stays off (control register 0) and the system runs normally; with it
enabled the cache comes up as before.  Builds without warnings with W=1
(board config and multi_v7_defconfig); checkpatch --strict and
dt_binding_check are clean.

Changes in v2 (after Arnd's review):
- drop the clean-invalidate-disable path for a cache the firmware left
  enabled, and with it the "factor out the number of ways" patch: the
  firmware is responsible for leaving the cache off, the kernel does not
  touch a disabled node;
- new patch 1 saying so in the binding.
v1: https://lore.kernel.org/r/20261111133930.3665251-1-isasmendi.agus@gmail.com

Agustin Santiago Isasmendi (2):
  dt-bindings: cache: l2c2x0: a disabled node is left to the firmware
  ARM: l2c: honour status = "disabled" on the L2 cache node

 Documentation/devicetree/bindings/cache/l2c2x0.yaml | 3 +++
 arch/arm/mm/cache-l2x0.c                            | 5 +++++
 2 files changed, 8 insertions(+)


base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.30.2

[PATCH v2 1/2] dt-bindings: cache: l2c2x0: a disabled node is left to the firmware

[Agustin Santiago Isasmendi]

l2x0_of_init() is about to honour status = "disabled" on the L2 cache
node by leaving the controller alone.  Say that the firmware must then
leave the cache disabled: the kernel does not check or change its state.

Assisted-by: LLM
Signed-off-by: Agustin Santiago Isasmendi <isasmendi.agus@gmail.com>
---
 Documentation/devicetree/bindings/cache/l2c2x0.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/cache/l2c2x0.yaml b/Documentation/devicetree/bindings/cache/l2c2x0.yaml
index ee604117ffb3..15b5d8c1ebdf 100644
--- a/Documentation/devicetree/bindings/cache/l2c2x0.yaml
+++ b/Documentation/devicetree/bindings/cache/l2c2x0.yaml
@@ -24,6 +24,9 @@ description: |+
     early secure boot code. Thus no need to deal with their configuration
     in the kernel at all.
 
+  Note 2: A node with status = "disabled" is not touched by the kernel at
+    all: the firmware must leave the cache disabled.
+
 allOf:
   - $ref: /schemas/cache-controller.yaml#
 
-- 
2.30.2

[PATCH v2 2/2] ARM: l2c: honour status = "disabled" on the L2 cache node

[Agustin Santiago Isasmendi]

l2x0_of_init() looks the L2 cache node up by compatible and enables the
cache whatever its status property says, so a device tree cannot keep
the L2 cache off.

On the ZTE ZX279128S (dual Cortex-A9, L2C-310 r3p3) a PCIe read on one
CPU deadlocks against an L2C-310 line operation by physical address or a
Cache Sync on the other CPU, and the board hangs within seconds of WiFi
traffic.  Running with the L2 cache off avoids the deadlock, so a board
needs a way to say so.

Honour the status property, as the newer cache controller drivers
already do (drivers/cache/ax45mp_cache.c, starfive_starlink_cache.c):
leave a disabled node alone.  The firmware is responsible for leaving
the cache disabled in that case; the kernel does not check.  No in-tree
L2 cache node is disabled, so nothing changes for existing device trees.

Tested on the ZTE ZXHN H3600: with the node disabled the cache stays
off and the system runs normally; with it enabled the cache comes up as
before.

Assisted-by: LLM
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Agustin Santiago Isasmendi <isasmendi.agus@gmail.com>
---
 arch/arm/mm/cache-l2x0.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 470867160076..5d6a4c040a50 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1772,6 +1772,11 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
 	if (!np)
 		return -ENODEV;
 
+	if (!of_device_is_available(np)) {
+		pr_info("L2C: disabled by the device tree\n");
+		return -ENODEV;
+	}
+
 	if (of_address_to_resource(np, 0, &res))
 		return -ENODEV;
 
-- 
2.30.2

Re: [PATCH v2 2/2] ARM: l2c: honour status = "disabled" on the L2 cache node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ARM: l2c: device_node refcount leak on disabled DT node in
  l2x0_of_init()
--

commit 98b76cb0386da8baf1e847208a1b97f4eab12a69
Author: Agustin Santiago Isasmendi <isasmendi.agus@gmail.com>

ARM: l2c: honour status = "disabled" on the L2 cache node

This commit checks of_device_is_available() in l2x0_of_init() so that an
L2 cache controller node marked disabled in the device tree is skipped.

> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index 470867160076f..5d6a4c040a506 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -1772,6 +1772,11 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
>  	if (!np)
>  		return -ENODEV;
>  
> +	if (!of_device_is_available(np)) {
> +		pr_info("L2C: disabled by the device tree\n");
> +		return -ENODEV;
> +	}

[Severity: Medium]
Does this leak a reference to np?  of_find_matching_node() returns np with
its refcount incremented, and unlike the drivers/cache/ drivers mentioned
in the commit message (which declare np with __free(device_node)),
l2x0_of_init() uses a plain struct device_node *np and returns here
without calling of_node_put(np).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112072953.989664-1-isasmendi.agus@gmail.com?part=2