Re: [PATCH 1/3] dt-bindings: power: domain-idle-state: Add system-state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] dt-bindings: backwards compatibility broken by reusing domain-idle-state
--

commit 7783b35f06985a4e4112599684740ef1b9b25325
Author: Maulik Shah <maulik.shah@oss.qualcomm.com>

dt-bindings: power: domain-idle-state: Add system-state

Some domain idle states require system-wide coordination and are not
applicable while the platform is up and running. Add a boolean system-state
property to describe these states.

> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.yaml b/Documentation/devicetree/bindings/power/domain-idle-state.yaml
> index 4dd4f59bbbec3..35dab3cd6aedc 100644
> --- a/Documentation/devicetree/bindings/power/domain-idle-state.yaml
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.yaml
> @@ -45,6 +45,12 @@ patternProperties:
>            state will yield power benefits, after overcoming the overhead while
>            entering the idle state.
>  
> +      system-state:
> +        description:
> +          When present, this state requires system-wide coordination and is
> +          not applicable while the platform is up and running.
> +        type: boolean
> +

[Severity: Critical]
Does adding this property to nodes that still use the "domain-idle-state"
compatible string break backwards compatibility?

When an older kernel parses a Device Tree containing a system-only power
state, it traverses the domain-idle-states phandles and matches the
compatible = "domain-idle-state" string. Because older kernels do not
recognize the newly introduced system-state boolean property, they will
silently ignore it and incorrectly register the system-only state as a
regular runtime cpuidle state.

If the runtime governor selects this system-suspend state during normal
operation, the lack of system-wide coordination can cause the hardware to
hang or crash. Device Tree bindings form an ABI between firmware/bootloader
and the OS, meaning newer Device Trees must safely degrade on older kernels.

Could a new compatible string (e.g., compatible = "domain-system-state") or
a distinct linking property (e.g., domain-system-states) be introduced
instead, so older kernels naturally skip parsing it?

[ ... ]

> @@ -77,5 +83,13 @@ examples:
>          exit-latency-us = <40>;
>          min-residency-us = <80>;
>        };
> +
> +      domain_power_off: domain-power-off {
> +        compatible = "domain-idle-state";
> +        entry-latency-us = <100>;
> +        exit-latency-us = <200>;
> +        min-residency-us = <1000>;
> +        system-state;
> +      };
>      };
>  ...

[Severity: Critical]
Does this example demonstrate the compatibility issue? A state explicitly
restricted to system-wide coordination still uses the "domain-idle-state"
compatible string, which would render it structurally unsafe for older OS
releases to process.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-s2idle_state-v1-0-3c402c66f388@oss.qualcomm.com?part=1