Re: [PATCH v5 1/2] dt-bindings: perf: marvell: Add CN20K DDR PMU binding

Krzysztof Kozlowski
2026-04-14T06:37:13Z

On Mon, Apr 13, 2026 at 05:12:06PM +0000, sashiko-bot@kernel.org wrote:
> Sashiko AI review found 2 potential issue(s):
> - [Low] DT binding example unit address and generic node name mismatch.
> - [Low] Unnecessary duplication of binding files instead of updating the existing schema.
> --
>
> commit e2d439a05897c67e7bc90170f7cf6e218505769a
> Author: Geetha sowjanya <gakula@marvell.com>
>
> dt-bindings: perf: marvell: Add CN20K DDR PMU binding
>
> This commit adds a new device tree binding for the DDR Performance Monitoring
> Unit found in Marvell CN20K SoCs. The hardware provides counters to monitor
> DDR traffic and is accessed via a dedicated MMIO region.
>
> > diff --git a/Documentation/devicetree/bindings/perf/marvell,cn20k-ddr-pmu.yaml b/Documentation/devicetree/bindings/perf/marvell,cn20k-ddr-pmu.yaml
> > new file mode 100644
> > index 0000000000000..cc6aa760de49e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/perf/marvell,cn20k-ddr-pmu.yaml
>
> The commit message notes that the CN20K DDR PMU is functionally equivalent
> to the CN10K DDR PMU. Does it make sense to consolidate this by adding the new
> compatible string to the existing marvell-cn10k-ddr.yaml schema instead of
> introducing a new duplicate file?
>

That's valid comment, I did not notice there is a file which is almost
identical. Why isn't this part of that file?

> Splitting schemas is typically recommended only when conditionals become
> too unwieldy.
>
> [ ... ]
>
> > +examples:
> > + - |
> > + bus {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + ddr-pmu@c200000000 {
> > + compatible = "marvell,cn20k-ddr-pmu";
> > + reg = <0xc200 0x00000000 0x0 0x100000>;
> > + };
> > + };
>
> Is there a unit address mismatch here? The 64-bit base address formed by the
> reg property is 0xc20000000000, but the node name uses @c200000000, missing two
> trailing zeros. This can trigger a unit_address_vs_reg regression during
> dt_binding_check.
>

Also valid.

Best regards,
Krzysztof