Re: [PATCH 1/2] dt-bindings: devfreq: Document support for Airoha Subsystem devfreq

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The compatible string `airoha,devfreq` is overly generic and lacks an SoC-specific identifier.
- [Medium] The binding introduces purely virtual nodes with OS-specific names (`*-devfreq`) instead of describing the actual hardware topology.
- [Low] Typos and grammatical errors in the binding description.
--

--- Patch [1]: [PATCH 1/2] dt-bindings: devfreq: Document support for Airoha Subsystem devfreq ---
commit 372abcdd0c5df2cbcf3b1b6f064f1949fb955f9d
Author: Christian Marangi <ansuelsmth@gmail.com>

dt-bindings: devfreq: Document support for Airoha Subsystem devfreq

This commit documents the devicetree bindings for Airoha SoC subsystems
that scale to specific operating performance points (OPP) based on CPU
clock reference.

> diff --git a/Documentation/devicetree/bindings/devfreq/airoha,devfreq.yaml b/Documentation/devicetree/bindings/devfreq/airoha,devfreq.yaml
> new file mode 100644
> index 0000000000000..2f84486cb2096
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/airoha,devfreq.yaml
> @@ -0,0 +1,114 @@
[ ... ]
> +description:
> +  Airoha SoC contains multiple subsystem that are scaled to a
> +  specific OPP taking the CPU clock as reference. Such sybsystem
> +  can enter idle or performance state based on the system load
> +  and power consumption conditions.
> +
> +  Example of such subsystem are GSW (Gigabit Switch), NPU (Custom

[Severity: Low]
This isn't a bug, but there are a few typos in this description. Should
"multiple subsystem" and "Example of such subsystem" be plural? Also,
"sybsystem" appears to be misspelled.

> +  Risc Co-Processor for Network acceleration), SOE (Crypto Module) BUS...
> +
> +properties:
> +  compatible:
> +    const: airoha,devfreq

[Severity: Medium]
Will using a generic module string like airoha,devfreq prevent the OS from
distinguishing between hardware variants on future Airoha SoCs? Devicetree
best practices typically mandate SoC-specific compatible strings, such as
airoha,en7523-devfreq, to cleanly handle future hardware variations or quirks
without breaking the DT ABI.

[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/clock/en7523-clk.h>
[ ... ]
> +    npu-devfreq {
> +        compatible = "airoha,devfreq";
> +        operating-points-v2 = <&npu_opp_table>;
> +        clocks = <&scuclk EN7523_CLK_NPU>;
> +    };
> +
> +    emi-devfreq {
> +        compatible = "airoha,devfreq";
> +        operating-points-v2 = <&emi_opp_table>;
> +        clocks = <&scuclk EN7523_CLK_EMI>;
> +    };

[Severity: Medium]
Do these example nodes describe Linux driver frameworks rather than the actual
hardware topology? 

Since devicetree is meant to describe hardware, it appears these virtual nodes
lack hardware resources like reg properties and use Linux subsystem terminology
(devfreq) in their names purely to bind a Linux driver. Should the clocks and
OPP tables be defined directly within the hardware device nodes they scale,
such as the NPU, EMI, or GSW nodes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810143057.651478-1-ansuelsmth@gmail.com?part=1