Re: [PATCH v9 4/5] dt-bindings: riscv: Add binding for CBQRI controllers

[Drew Fustini]

On Sat, Oct 03, 2026 at 07:19:10AM +0000, sashiko-bot@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] dt-bindings: riscv: cbqri properties unconditionally required against intent
> --
> 
> commit 5dcd168e7171a1ca9e65e913bdb55cea23dc0357
> Author: Drew Fustini <fustini@xxxxxxxxxx>
> 
> dt-bindings: riscv: Add binding for CBQRI controllers
> 
> This commit introduces the device tree binding for RISC-V Capacity and Bandwidth
> QoS Register Interface (CBQRI) controllers. The binding covers capacity and
> bandwidth controllers, including properties for resource-control IDs,
> monitoring-counter IDs, and cache phandles.
> 
> > diff --git a/Documentation/devicetree/bindings/riscv/riscv,cbqri.yaml b/Documentation/devicetree/bindings/riscv/riscv,cbqri.yaml
> > new file mode 100644
> > index 0000000000000..0924758ebdde6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/riscv/riscv,cbqri.yaml
> > @@ -0,0 +1,97 @@
> 
> [ ... ]
> 
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: riscv,cbqri-capacity-controller
> > +    then:
> > +      required:
> > +        - riscv,cbqri-rcid
> > +        - riscv,cbqri-cache
> 
> [Severity: Medium]
> Does this code introduce a regression by unconditionally requiring these
> properties for all capacity controllers?
> 
> The commit message explicitly describes the riscv,cbqri-cache phandle as
> optional.
> 
> Additionally, the binding description notes a controller may implement only
> monitoring without allocation capabilities, meaning it would lack RCIDs for
> allocation operations.
> 
> Because the allOf block rigidly requires both properties for any
> riscv,cbqri-capacity-controller, this appears to contradict the hardware
> flexibility and will cause schema validation to fail for valid device trees
> describing monitoring-only or non-CPU-cache capacity controllers.

This is not an issue.

riscv,cbqri-cache is optional for the binding as a whole because a
bandwidth controller has no cache to point at. For a capacity
controller, it is required because the cache level and the harts sharing
the cache are taken from that node.

The required properties describe the hardware that currently exists. A
monitoring-only capacity controller would have no RCIDs to describe, and
the binding can be extended to cover it when such hardware exists.

Drew