diff mbox series

[02/21] dt-bindings: gpu: img: Further constrain clocks

Message ID 20241105-sets-bxs-4-64-patch-v1-v1-2-4ed30e865892@imgtec.com (mailing list archive)
State New
Headers show
Series Imagination BXS-4-64 MC1 GPU support | expand

Commit Message

Matt Coster Nov. 5, 2024, 3:58 p.m. UTC
All Imagination GPUs use three clocks: core, mem and sys. All reasonably
modern Imagination GPUs also support a single-clock mode where the SoC
only hooks up core and the other two are derived internally. On GPUs which
support this mode, it is the default and most commonly used integration.

Codify this "1 or 3" constraint in our bindings and hang the specifics off
the vendor compatible string to mirror the integration-time choice.

Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 27 +++++++++++++++-------
 1 file changed, 19 insertions(+), 8 deletions(-)

Comments

Conor Dooley Nov. 5, 2024, 6:16 p.m. UTC | #1
On Tue, Nov 05, 2024 at 03:58:08PM +0000, Matt Coster wrote:
> All Imagination GPUs use three clocks: core, mem and sys. All reasonably
> modern Imagination GPUs also support a single-clock mode where the SoC
> only hooks up core and the other two are derived internally. On GPUs which
> support this mode, it is the default and most commonly used integration.
> 
> Codify this "1 or 3" constraint in our bindings and hang the specifics off
> the vendor compatible string to mirror the integration-time choice.
> 
> Signed-off-by: Matt Coster <matt.coster@imgtec.com>
> ---
>  .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 27 +++++++++++++++-------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> index ef7070daf213277d0190fe319e202fdc597337d4..6924831d3e9dd9b2b052ca8f9d7228ff25526532 100644
> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> @@ -30,15 +30,20 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    minItems: 1
> -    maxItems: 3
> +    oneOf:
> +      - minItems: 1
> +        maxItems: 1
> +      - minItems: 3
> +        maxItems: 3

Just put the outer constraints here and...

>    clock-names:
> -    items:
> -      - const: core
> -      - const: mem
> -      - const: sys
> -    minItems: 1
> +    oneOf:
> +      - items:
> +          - const: core
> +      - items:
> +          - const: core
> +          - const: mem
> +          - const: sys
>  
>    interrupts:
>      maxItems: 1
> @@ -56,15 +61,21 @@ required:
>  additionalProperties: false
>  
>  allOf:
> +  # Vendor integrations using a single clock domain
>    - if:
>        properties:
>          compatible:
>            contains:
> -            const: ti,am62-gpu
> +            anyOf:
> +              - const: ti,am62-gpu
>      then:
>        properties:
>          clocks:
> +          minItems: 1
>            maxItems: 1

...adjust the constraints in conditional bits. Setting minItems to 1
should be a nop too. Pretty sure what you already had here was actually
already sufficient.

Cheers,
Conor.

> +        clock-names:
> +          items:
> +            - const: core
>  
>  examples:
>    - |
> 
> -- 
> 2.47.0
>
Matt Coster Nov. 6, 2024, 10:17 a.m. UTC | #2
On 05/11/2024 18:16, Conor Dooley wrote:
> On Tue, Nov 05, 2024 at 03:58:08PM +0000, Matt Coster wrote:
>> All Imagination GPUs use three clocks: core, mem and sys. All reasonably
>> modern Imagination GPUs also support a single-clock mode where the SoC
>> only hooks up core and the other two are derived internally. On GPUs which
>> support this mode, it is the default and most commonly used integration.
>>
>> Codify this "1 or 3" constraint in our bindings and hang the specifics off
>> the vendor compatible string to mirror the integration-time choice.
>>
>> Signed-off-by: Matt Coster <matt.coster@imgtec.com>
>> ---
>>  .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 27 +++++++++++++++-------
>>  1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
>> index ef7070daf213277d0190fe319e202fdc597337d4..6924831d3e9dd9b2b052ca8f9d7228ff25526532 100644
>> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
>> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
>> @@ -30,15 +30,20 @@ properties:
>>      maxItems: 1
>>  
>>    clocks:
>> -    minItems: 1
>> -    maxItems: 3
>> +    oneOf:
>> +      - minItems: 1
>> +        maxItems: 1
>> +      - minItems: 3
>> +        maxItems: 3
> 
> Just put the outer constraints here and...
> 
>>    clock-names:
>> -    items:
>> -      - const: core
>> -      - const: mem
>> -      - const: sys
>> -    minItems: 1
>> +    oneOf:
>> +      - items:
>> +          - const: core
>> +      - items:
>> +          - const: core
>> +          - const: mem
>> +          - const: sys
>>  
>>    interrupts:
>>      maxItems: 1
>> @@ -56,15 +61,21 @@ required:
>>  additionalProperties: false
>>  
>>  allOf:
>> +  # Vendor integrations using a single clock domain
>>    - if:
>>        properties:
>>          compatible:
>>            contains:
>> -            const: ti,am62-gpu
>> +            anyOf:
>> +              - const: ti,am62-gpu
>>      then:
>>        properties:
>>          clocks:
>> +          minItems: 1
>>            maxItems: 1
> 
> ...adjust the constraints in conditional bits. Setting minItems to 1
> should be a nop too. Pretty sure what you already had here was actually
> already sufficient.
> 
> Cheers,
> Conor.

Is there an implicit constraint ensuring "clocks" and "clock-names" are
the same size? I wasn't sure if we could rely on that, hence the
slightly odd constraint on "clocks". The only real goal here is to
codify that you can't have one of "mem" or "sys". it's both or nothing.

Cheers,
Matt

>> +        clock-names:
>> +          items:
>> +            - const: core
>>  
>>  examples:
>>    - |
>>
>> -- 
>> 2.47.0
>>
Conor Dooley Nov. 6, 2024, 5:06 p.m. UTC | #3
On Wed, Nov 06, 2024 at 10:17:53AM +0000, Matt Coster wrote:
> On 05/11/2024 18:16, Conor Dooley wrote:
> > On Tue, Nov 05, 2024 at 03:58:08PM +0000, Matt Coster wrote:
> >> All Imagination GPUs use three clocks: core, mem and sys. All reasonably
> >> modern Imagination GPUs also support a single-clock mode where the SoC
> >> only hooks up core and the other two are derived internally. On GPUs which
> >> support this mode, it is the default and most commonly used integration.
> >>
> >> Codify this "1 or 3" constraint in our bindings and hang the specifics off
> >> the vendor compatible string to mirror the integration-time choice.
> >>
> >> Signed-off-by: Matt Coster <matt.coster@imgtec.com>
> >> ---
> >>  .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 27 +++++++++++++++-------
> >>  1 file changed, 19 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> >> index ef7070daf213277d0190fe319e202fdc597337d4..6924831d3e9dd9b2b052ca8f9d7228ff25526532 100644
> >> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> >> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> >> @@ -30,15 +30,20 @@ properties:
> >>      maxItems: 1
> >>  
> >>    clocks:
> >> -    minItems: 1
> >> -    maxItems: 3
> >> +    oneOf:
> >> +      - minItems: 1
> >> +        maxItems: 1
> >> +      - minItems: 3
> >> +        maxItems: 3
> > 
> > Just put the outer constraints here and...
> > 
> >>    clock-names:
> >> -    items:
> >> -      - const: core
> >> -      - const: mem
> >> -      - const: sys
> >> -    minItems: 1
> >> +    oneOf:
> >> +      - items:
> >> +          - const: core
> >> +      - items:
> >> +          - const: core
> >> +          - const: mem
> >> +          - const: sys
> >>  
> >>    interrupts:
> >>      maxItems: 1
> >> @@ -56,15 +61,21 @@ required:
> >>  additionalProperties: false
> >>  
> >>  allOf:
> >> +  # Vendor integrations using a single clock domain
> >>    - if:
> >>        properties:
> >>          compatible:
> >>            contains:
> >> -            const: ti,am62-gpu
> >> +            anyOf:
> >> +              - const: ti,am62-gpu
> >>      then:
> >>        properties:
> >>          clocks:
> >> +          minItems: 1
> >>            maxItems: 1
> > 
> > ...adjust the constraints in conditional bits. Setting minItems to 1
> > should be a nop too. Pretty sure what you already had here was actually
> > already sufficient.
> > 
> > Cheers,
> > Conor.
> 
> Is there an implicit constraint ensuring "clocks" and "clock-names" are
> the same size? I wasn't sure if we could rely on that, hence the
> slightly odd constraint on "clocks". The only real goal here is to
> codify that you can't have one of "mem" or "sys". it's both or nothing.

You can just constrain clock-names: maxItems: 1 like you have already
done for clocks. The items list that was already in the file enforces
that the first clock provided must be core. When you add your new
compatible you can set clock{-names}: minItems: 3 for it, and that will
ensure that either 1 clock (core) or all 3 are the only options.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
index ef7070daf213277d0190fe319e202fdc597337d4..6924831d3e9dd9b2b052ca8f9d7228ff25526532 100644
--- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
+++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
@@ -30,15 +30,20 @@  properties:
     maxItems: 1
 
   clocks:
-    minItems: 1
-    maxItems: 3
+    oneOf:
+      - minItems: 1
+        maxItems: 1
+      - minItems: 3
+        maxItems: 3
 
   clock-names:
-    items:
-      - const: core
-      - const: mem
-      - const: sys
-    minItems: 1
+    oneOf:
+      - items:
+          - const: core
+      - items:
+          - const: core
+          - const: mem
+          - const: sys
 
   interrupts:
     maxItems: 1
@@ -56,15 +61,21 @@  required:
 additionalProperties: false
 
 allOf:
+  # Vendor integrations using a single clock domain
   - if:
       properties:
         compatible:
           contains:
-            const: ti,am62-gpu
+            anyOf:
+              - const: ti,am62-gpu
     then:
       properties:
         clocks:
+          minItems: 1
           maxItems: 1
+        clock-names:
+          items:
+            - const: core
 
 examples:
   - |