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@xxxxxxxxxxxxxx>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > > ---
> > > 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:
> > >
