Re: [Patch v3 1/2] dt-bindings: make sid and broadcast reg optional

From: Thierry Reding
Date: Wed Apr 24 2024 - 12:26:45 EST


On Mon Apr 22, 2024 at 9:02 AM CEST, Krzysztof Kozlowski wrote:
> On 12/04/2024 15:05, Sumit Gupta wrote:
> > MC SID and Broadbast channel register access is restricted for Guest VM.
>
> Broadcast
>
> > Make both the regions as optional for SoC's from Tegra186 onwards.
>
> onward?
>
> > Tegra MC driver will skip access to the restricted registers from Guest
> > if the respective regions are not present in the memory-controller node
> > of Guest DT.
> >
> > Suggested-by: Thierry Reding <treding@xxxxxxxxxx>
> > Signed-off-by: Sumit Gupta <sumitg@xxxxxxxxxx>
> > ---
> > .../nvidia,tegra186-mc.yaml | 95 ++++++++++---------
> > 1 file changed, 49 insertions(+), 46 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
> > index 935d63d181d9..e0bd013ecca3 100644
> > --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
> > +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
> > @@ -34,11 +34,11 @@ properties:
> > - nvidia,tegra234-mc
> >
> > reg:
> > - minItems: 6
> > + minItems: 4
> > maxItems: 18
> >
> > reg-names:
> > - minItems: 6
> > + minItems: 4
> > maxItems: 18
> >
> > interrupts:
> > @@ -151,12 +151,13 @@ allOf:
> >
> > reg-names:
> > items:
> > - - const: sid
> > - - const: broadcast
> > - - const: ch0
> > - - const: ch1
> > - - const: ch2
> > - - const: ch3
> > + enum:
> > + - sid
> > + - broadcast
> > + - ch0
> > + - ch1
> > + - ch2
> > + - ch3
>
> I understand why sid and broadcast are becoming optional, but why order
> of the rest is now fully flexible?

The reason why the order of the rest doesn't matter is because we have
both reg and reg-names properties and so the order in which they appear
in the list doesn't matter. The only thing that matters is that the
entries of the reg and reg-names properties match.

> This does not even make sid/broadcast optional, but ch0!

Yeah, this ends up making all entries optional, which isn't what we
want. I don't know of a way to accurately express this in json-schema,
though. Do you?

If not, then maybe we need to resort to something like this and also
mention explicitly in some comment that it is sid and broadcast that are
optional.

Thierry

Attachment: signature.asc
Description: PGP signature