diff mbox series

[v5,4/6] dt-bindings: gpu: v3d: Add additional examples to improve binding checks

Message ID 20250316-v3d-gpu-reset-fixes-v5-4-9779cdb12f06@igalia.com (mailing list archive)
State New, archived
Headers show
Series drm/v3d: Fix GPU reset issues on the Raspberry Pi 5 | expand

Commit Message

Maíra Canal March 16, 2025, 2:15 p.m. UTC
To prevent future changes that might inadvertently break the ABI, add
more examples to the binding. These examples improve coverage and help
ensure `make dt_binding_check` produces more robust validation results.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 34 ++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

Comments

Krzysztof Kozlowski March 16, 2025, 4:43 p.m. UTC | #1
On Sun, Mar 16, 2025 at 11:15:11AM -0300, Maíra Canal wrote:
> To prevent future changes that might inadvertently break the ABI, add
> more examples to the binding. These examples improve coverage and help

Examples are not related to ABI at all.

> ensure `make dt_binding_check` produces more robust validation results.

No, don't add more examples differing by one property. Keep one/two
examples.

> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 34 ++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
> index 766a310ab653855d7cc9a80f18c2083218fe307e..39b8f0ee1f727628307d758844008ae1189902b2 100644
> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
> @@ -123,6 +123,38 @@ allOf:
>  additionalProperties: false
>  
>  examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/soc/bcm2835-pm.h>
> +
> +    gpu@7ec00000 {
> +      compatible= "brcm,2711-v3d";
> +      reg = <0x7ec00000 0x4000>,
> +            <0x7ec04000 0x4000>;
> +      reg-names = "hub", "core0";
> +
> +      power-domains = <&pm BCM2835_POWER_DOMAIN_GRAFX_V3D>;

That's the only notable difference - one new property.

> +      resets = <&pm BCM2835_RESET_V3D>;
> +      interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> +    };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/soc/bcm2835-pm.h>
> +
> +    gpu@2000000 {
> +      compatible = "brcm,2712-v3d";
> +      reg = <0x02000000 0x4000>,
> +            <0x02008000 0x6000>,
> +            <0x02030800 0x0700>;
> +      reg-names = "hub", "core0", "sms";
> +
> +      power-domains = <&pm BCM2835_POWER_DOMAIN_GRAFX_V3D>;
> +      resets = <&pm BCM2835_RESET_V3D>;
> +      interrupts = <GIC_SPI 250 IRQ_TYPE_LEVEL_HIGH>,
> +                   <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;

No differences here at all.

Best regards,
Krzysztof
Maíra Canal March 16, 2025, 5:42 p.m. UTC | #2
Hi Krzysztof,

On 16/03/25 13:43, Krzysztof Kozlowski wrote:
> On Sun, Mar 16, 2025 at 11:15:11AM -0300, Maíra Canal wrote:
>> To prevent future changes that might inadvertently break the ABI, add
>> more examples to the binding. These examples improve coverage and help
> 
> Examples are not related to ABI at all.
> 
>> ensure `make dt_binding_check` produces more robust validation results.
> 
> No, don't add more examples differing by one property. Keep one/two
> examples.

I had the intention to add examples to avoid people from changing the
reg order in the future. For example, we changed the register order when
we converted the binding from txt to YAML. My goal was to avoid such
thing to happen again.

 From the feedback, I'll drop this patch. Thanks!

Best Regards,
- Maíra

> 
>>
>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>> ---
>>   .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 34 ++++++++++++++++++++--
>>   1 file changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> index 766a310ab653855d7cc9a80f18c2083218fe307e..39b8f0ee1f727628307d758844008ae1189902b2 100644
>> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> @@ -123,6 +123,38 @@ allOf:
>>   additionalProperties: false
>>   
>>   examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/soc/bcm2835-pm.h>
>> +
>> +    gpu@7ec00000 {
>> +      compatible= "brcm,2711-v3d";
>> +      reg = <0x7ec00000 0x4000>,
>> +            <0x7ec04000 0x4000>;
>> +      reg-names = "hub", "core0";
>> +
>> +      power-domains = <&pm BCM2835_POWER_DOMAIN_GRAFX_V3D>;
> 
> That's the only notable difference - one new property.
> 
>> +      resets = <&pm BCM2835_RESET_V3D>;
>> +      interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
>> +    };
>> +
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/soc/bcm2835-pm.h>
>> +
>> +    gpu@2000000 {
>> +      compatible = "brcm,2712-v3d";
>> +      reg = <0x02000000 0x4000>,
>> +            <0x02008000 0x6000>,
>> +            <0x02030800 0x0700>;
>> +      reg-names = "hub", "core0", "sms";
>> +
>> +      power-domains = <&pm BCM2835_POWER_DOMAIN_GRAFX_V3D>;
>> +      resets = <&pm BCM2835_RESET_V3D>;
>> +      interrupts = <GIC_SPI 250 IRQ_TYPE_LEVEL_HIGH>,
>> +                   <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
> 
> No differences here at all.
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski March 17, 2025, 7:30 a.m. UTC | #3
On 16/03/2025 18:42, Maíra Canal wrote:
> Hi Krzysztof,
> 
> On 16/03/25 13:43, Krzysztof Kozlowski wrote:
>> On Sun, Mar 16, 2025 at 11:15:11AM -0300, Maíra Canal wrote:
>>> To prevent future changes that might inadvertently break the ABI, add
>>> more examples to the binding. These examples improve coverage and help
>>
>> Examples are not related to ABI at all.
>>
>>> ensure `make dt_binding_check` produces more robust validation results.
>>
>> No, don't add more examples differing by one property. Keep one/two
>> examples.
> 
> I had the intention to add examples to avoid people from changing the
> reg order in the future. For example, we changed the register order when

Example does not stop that at all. Changes nothing here.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
index 766a310ab653855d7cc9a80f18c2083218fe307e..39b8f0ee1f727628307d758844008ae1189902b2 100644
--- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
@@ -123,6 +123,38 @@  allOf:
 additionalProperties: false
 
 examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/soc/bcm2835-pm.h>
+
+    gpu@7ec00000 {
+      compatible= "brcm,2711-v3d";
+      reg = <0x7ec00000 0x4000>,
+            <0x7ec04000 0x4000>;
+      reg-names = "hub", "core0";
+
+      power-domains = <&pm BCM2835_POWER_DOMAIN_GRAFX_V3D>;
+      resets = <&pm BCM2835_RESET_V3D>;
+      interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/soc/bcm2835-pm.h>
+
+    gpu@2000000 {
+      compatible = "brcm,2712-v3d";
+      reg = <0x02000000 0x4000>,
+            <0x02008000 0x6000>,
+            <0x02030800 0x0700>;
+      reg-names = "hub", "core0", "sms";
+
+      power-domains = <&pm BCM2835_POWER_DOMAIN_GRAFX_V3D>;
+      resets = <&pm BCM2835_RESET_V3D>;
+      interrupts = <GIC_SPI 250 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
+    };
+
   - |
     gpu@f1200000 {
       compatible = "brcm,7268-v3d";
@@ -134,5 +166,3 @@  examples:
       interrupts = <0 78 4>,
                    <0 77 4>;
     };
-
-...