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...@bp.renesas.com> > > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad...@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: > > >