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.
I am ok for keeping it or removing it. Please let me know.
>
> > 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.
Initially GPU was not working on our platform. Then after debugging found that it needs, bus clock
to make it work. This information is missing in dt binding and I need to find this info from source code.
That is the reason, even if it is optional, I have documented with same name here.
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:
> >
