Re: [PATCH v5 3/7] dt-bindings: arm: Add MPAM MSC binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Low] dt-bindings: arm: arm,mpam-msc: 4-cell reg on 1-cell bus in
  example
- [Low] dt-bindings: arm: arm,mpam-msc: missing top-level required
  property list
- [Low] dt-bindings: arm: arm,mpam-msc: property order and unused labels
  in example
--

commit fcdcd9b567c42204dbc32cc6d1155a3dddba1926
Author: Rob Herring <robh@kernel.org>

dt-bindings: arm: Add MPAM MSC binding

This commit adds the Devicetree binding schema for Arm Memory System
Resource Partitioning and Monitoring (MPAM) Memory System Component (MSC)
nodes.

> diff --git a/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml b/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml
> new file mode 100644
> index 0000000000000..b52599b47a1cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml
> @@ -0,0 +1,62 @@
[ ... ]
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: arm,mpam-msc                   # Further details are discoverable
> +      - items:
> +          - const: arm,mpam-memory-controller-msc
> +          - const: arm,mpam-msc
> +
> +  reg:
> +    maxItems: 1
> +    description: A memory region containing registers as defined in the MPAM
> +      specification.
> +
> +  arm,not-ready-us:
> +    description: The maximum time in microseconds for monitoring data to be
> +      accurate after a settings change. For more information, see the
> +      Not-Ready (NRDY) bit description in the MPAM specification.
> +
> +additionalProperties: false

[Severity: Low]
Should this schema include a required section listing compatible and reg?
do_mpam_msc_drv_probe() unconditionally maps MMIO resource 0 when probing
DT MSC nodes, so nodes omitting reg will pass dtbs_check and then fail to
probe at runtime.

> +
> +examples:
> +  - |
> +    L3: cache-controller@30000000 {
> +        compatible = "foo,a-l3-cache", "cache";
> +        cache-level = <3>;
> +        cache-unified;
> +        reg = <0x0 0x30000000 0x0 0x2000>;
> +        ranges = <0x0 0x30000000 0x2000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;

[Severity: Low]
Should reg in cache-controller@30000000 be <0x30000000 0x2000>?  The
default example bus uses #address-cells = <1> and #size-cells = <1> (as
used by ranges here and by msc@40000000 below), so a 4-cell reg is parsed
as two regions starting at 0x0 and triggers a dtc simple_bus_reg unit
address mismatch warning during dt_binding_check.

> +
> +        msc@0 {
> +            compatible = "arm,mpam-msc";
> +            reg = <0x0 0x2000>;
> +            arm,not-ready-us = <1>;
> +            /* CPU affinity implied by parent cache node  */
> +        };
> +    };
> +
> +    ddr_msc: msc@40000000 {
> +        compatible = "arm,mpam-memory-controller-msc", "arm,mpam-msc";
> +        reg = <0x40000000 0x1000>;
> +        arm,not-ready-us = <10>;
> +    };

[Severity: Low]
This isn't a bug, but scripts/dtc/dt-check-style reports two style issues
in this example: in cache-controller@30000000, reg and ranges should come
immediately after compatible before cache-level and cache-unified, and the
unused labels L3 and ddr_msc should be dropped.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-mpam-resctrl-dt-knp-support-v5-0-150335a79381@oss.qualcomm.com?part=3