Re: [PATCH v2 2/5] dt-bindings: iommu: mediatek: Fix interrupt count constraint for new SoCs

[AngeloGioacchino Del Regno]

Il 02/10/24 07:01, Macpaul Lin ha scritto:
> On 9/30/24 16:49, AngeloGioacchino Del Regno wrote:
> > Il 26/09/24 13:14, Macpaul Lin ha scritto:
> > > The infra-iommu node in mt8195.dtsi was triggering a CHECK_DTBS error due
> > > to an excessively long 'interrupts' property. The error message was:
>
> [snip]
>
> > > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml 
> > > b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> > > index ea6b0f5f24de..fdd2996d2a31 100644
> > > --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> > > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> > > @@ -96,7 +96,8 @@ properties:
> > >       maxItems: 1
> > >     interrupts:
> > > -    maxItems: 1
> > > +    minItems: 1
> > > +    maxItems: 5
> > >     clocks:
> > >       items:
> > > @@ -210,6 +211,28 @@ allOf:
> > >         required:
> > >           - mediatek,larbs
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - mediatek,mt8195-iommu-infra
> > > +
> > > +    then:
> > > +      properties:
> > > +        interrupts:
> > > +          description: |
> >
> > Do you really need to keep the formatting?
> >
> > If you rephrase that as:
> >
> > The infra IOMMU in MT8195 has five banks: each features one set
> > of APB registers for the normal world (set 0), one
>
> Shouldn't we use a 'three' here?

Oops, yes, that's three. I wrote 'one' but described three. Heh!

> Three APB register sets for the protected world 1, protected world 2,
> and protected world 3.

three APB register sets for the protected world (sets 1/2/3) -- or
three APB register sets for the protected world (sets 1-3)

I mean, repeating "protected world X" three times is too much I think :-)

> > for the protected
> > world (sets 1-3) and one for the secure world (set 4), and each set
> > has its own interrupt. Therefore, five interrupts are needed.
> >
> > ...you won't need the bar :-)
>
> Thanks for the suggestion. The description has been moved to
> top common property in v3, and v4,
> hence the bar is still required to explain the
> others SOCs. I'll try to rephrase the description for MT8195 also.

Sure. You're welcome!

> > > +            The IOMMU of MT8195 has 5 banks: 0/1/2/3/4.
> > > +            Each bank has a set of APB registers corresponding to the
> > > +            normal world, protected world 1/2/3, and secure world, respectively.
> > > +            Therefore, 5 interrupt numbers are needed.
> > > +          maxItems: 5
> >
> > minItems: 5
> >
> > Cheers,
> > Angelo
>
> Thanks
> Macpaul Lin