Re: [PATCH v4 3/6] dt-bindings: arm: Add MPAM MSC binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] dt-bindings: arm: arm,mpam-msc: mismatched reg and ranges cell counts in example
- [Low] dt-bindings: arm: arm,mpam-msc: missing top-level required property list
- [Low] dt-bindings: arm: arm,mpam-msc: example violates dt-check-style rules
--

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

dt-bindings: arm: Add MPAM MSC binding

This commit adds the devicetree binding schema for the Arm Memory System
Resource Partitioning and Monitoring (MPAM) Memory System Component (MSC)
using the arm,mpam-msc compatible string.

> 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..523978900d3de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml
> @@ -0,0 +1,52 @@

[ ... ]

> +properties:
> +  compatible:
> +    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 top-level required block listing compatible and
reg before additionalProperties: false?

In drivers/resctrl/mpam_devices.c, do_mpam_msc_drv_probe() unconditionally
maps MMIO resource 0 for these nodes:

    if (msc->iface == MPAM_IFACE_MMIO) {
        void __iomem *io;

        io = devm_platform_get_and_ioremap_resource(pdev, 0,
                                                    &msc_res);
        if (IS_ERR(io)) {
            dev_err_once(dev, "Failed to map MSC base address\n");
            return ERR_CAST(io);
        }

Without a required list in Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml,
devicetree nodes matching arm,mpam-msc that omit reg will pass dtbs_check
validation and fail at runtime during probe.

> +
> +examples:
> +  - |
> +    L3: cache-controller@30000000 {
> +        compatible = "foo,a-l3-cache", "cache";
> +        cache-level = <3>;
> +        cache-unified;

[Severity: Low]
Does this example trigger check_property_order() and check_unused_labels()
warnings when running scripts/dtc/dt-check-style --mode=strict on
Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml?

In cache-controller@30000000, the L3 label is never referenced in the example
(and uses uppercase characters), and cache-level and cache-unified are placed
before reg and ranges rather than after standard addressing properties.

> +        reg = <0x0 0x30000000 0x0 0x2000>;
> +        ranges = <0x0 0x30000000 0x2000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;

[Severity: Low]
Are the cell counts in reg and ranges consistent with the default example bus
here?

When make dt_binding_check extracts the example in
Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml, the surrounding
example-0 parent node defaults to #address-cells = <1> and #size-cells = <1>.

In cache-controller@30000000, ranges = <0x0 0x30000000 0x2000> uses 3 cells
(1 child address cell, 1 parent address cell, and 1 child size cell), which
matches a 1-cell parent bus. However, reg = <0x0 0x30000000 0x0 0x2000> uses
4 cells as if the parent bus had #address-cells = <2> and #size-cells = <2>,
causing it to be decoded under the 1-cell parent as two separate 32-bit
regions at base address 0x0 (<0x0 0x30000000> and <0x0 0x2000>) rather than a
single region at 0x30000000.

Should reg be <0x30000000 0x2000> to match the 1-cell parent bus and ranges?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-mpam-resctrl-dt-knp-support-v4-0-bdda1e445cba@oss.qualcomm.com?part=3