[PATCH v2 1/3] dt-bindings: gpu: mali-bifrost: Document RZ/G2L support

[Biju Das]

Hi,

> Subject: RE: [PATCH v2 1/3] dt-bindings: gpu: mali-bifrost: Document
> RZ/G2L support
> 
> Hi Robin,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH v2 1/3] dt-bindings: gpu: mali-bifrost: Document
> > RZ/G2L support
> >
> > On 2021-12-06 15:00, Biju Das wrote:
> > > The Renesas RZ/G2{L, LC} SoC (a.k.a R9A07G044) has a Bifrost
> > > Mali-G31 GPU, add a compatible string for it.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz at bp.renesas.com>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj at bp.renesas.com>
> > > ---
> > > v1->v2:
> > >   * Updated minItems for resets as 2
> > >   * Documented optional property reset-names
> > >   * Documented reset-names as required property for RZ/G2L SoC.
> > > ---
> > >   .../bindings/gpu/arm,mali-bifrost.yaml        | 39
> ++++++++++++++++++-
> > >   1 file changed, 37 insertions(+), 2 deletions(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > > index 6f98dd55fb4c..c3b2f4ddd520 100644
> > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > > @@ -19,6 +19,7 @@ properties:
> > >             - amlogic,meson-g12a-mali
> > >             - mediatek,mt8183-mali
> > >             - realtek,rtd1619-mali
> > > +          - renesas,r9a07g044-mali
> > >             - rockchip,px30-mali
> > >             - rockchip,rk3568-mali
> > >         - const: arm,mali-bifrost # Mali Bifrost GPU model/revision
> > > is fully discoverable @@ -27,19 +28,30 @@ properties:
> > >       maxItems: 1
> > >
> > >     interrupts:
> > > +    minItems: 3
> > >       items:
> > >         - description: Job interrupt
> > >         - description: MMU interrupt
> > >         - description: GPU interrupt
> > > +      - description: EVENT interrupt
> >
> > I believe we haven't bothered with the "Event" interrupt so far since
> > there's no real meaningful use for it - it seems the downstream
> > binding for Arm's kbase driver doesn't mention it either.
> 
> I agree.
> But DT binding describes the H/W. Our SoC, mention about Event interrupt.
> That is the reason I have documented it.

> >
> > >     interrupt-names:
> > > +    minItems: 3
> > >       items:
> > >         - const: job
> > >         - const: mmu
> > >         - const: gpu
> > > +      - const: event
> > >
> > >     clocks:
> > > -    maxItems: 1
> > > +    minItems: 1
> > > +    maxItems: 3
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: gpu
> > > +      - const: bus
> > > +      - const: bus_ace
> >
> > Note that the Bifrost GPUs themselves all only have a single external
> > clock and reset (unexcitingly named "CLK" and "RESETn" respectively,
> > FWIW). I can't help feeling wary that defining additional names for
> > vendor integration details in the core binding may quickly grow into a
> > mess of mutually-incompatible sets of values, for no great benefit. At
> > the very least, it would seem more sensible to put them in the
> > SoC-specific conditional schemas.
> 

I agree, All optional properties like clock-names and reset-names should go in the SoC-specific conditional schemas.
I will make clock-names and reset-names to true and handle it in the SoC-specific conditional schemas.

I will send V3, incorporating the above. 

Regards,
Biju

 

> 
> >
> > Robin.
> >
> > >
> > >     mali-supply: true
> > >
> > > @@ -52,7 +64,14 @@ properties:
> > >       maxItems: 3
> > >
> > >     resets:
> > > -    maxItems: 2
> > > +    minItems: 2
> > > +    maxItems: 3
> > > +
> > > +  reset-names:
> > > +    items:
> > > +      - const: rst
> > > +      - const: axi_rst
> > > +      - const: ace_rst
> > >
> > >     "#cooling-cells":
> > >       const: 2
> > > @@ -113,6 +132,22 @@ allOf:
> > >           - sram-supply
> > >           - power-domains
> > >           - power-domain-names
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: renesas,r9a07g044-mali
> > > +    then:
> > > +      properties:
> > > +        interrupt-names:
> > > +          minItems: 4
> > > +        clock-names:
> > > +          minItems: 3
> > > +      required:
> > > +        - clock-names
> > > +        - power-domains
> > > +        - resets
> > > +        - reset-names
> > >       else:
> > >         properties:
> > >           power-domains:
> > >