Re: [PATCH hyperv-next v4 4/6] dt-bindings: microsoft,vmbus: Add GIC and DMA coherence to the example

[Roman Kisel <romank@linux.microsoft.com>]

On 2/11/2025 10:42 PM, Krzysztof Kozlowski wrote:
> On Tue, Feb 11, 2025 at 05:43:19PM -0800, Roman Kisel wrote:
>> The existing example lacks the GIC interrupt controller property
>> making it not possible to boot on ARM64, and it lacks the DMA
> 
> GIC controller is not relevant to this binding.
> 

Will remove, thank you for pointing that out!

>> coherence property making the kernel do more work on maintaining
>> CPU caches on ARM64 although the VMBus trancations are cache-coherent.
>>
>> Add the GIC node, specify DMA coherence, and define interrupt-parent
>> and interrupts properties in the example to provide a complete reference
>> for platforms utilizing GIC-based interrupts, and add the DMA coherence
>> property to not do extra work on the architectures where DMA defaults to
>> non cache-coherent.
>>
>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>> ---
>>   .../devicetree/bindings/bus/microsoft,vmbus.yaml      | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
> 
> Last time I said: not tested by automation.
> Now: I see automation build failures, although I do not see anything
> incorrect in the code, so that's a bit surprising. Please confirm that
> binding was tested on latest dtschema.

They weren't for which I am sorry. Read through

https://www.kernel.org/doc/html/v6.14-rc2/devicetree/bindings/writing-schema.html

and was able to see and fix the break by bringing the YAML to [1].
Getting now this

/Documentation/devicetree/bindings/bus/microsoft,vmbus.example.dtb: 
vmbus@ff0000000: 'dma-coherent', 'interrupts' do not match any of the 
regexes: 'pinctrl-[0-9]+'
         from schema $id: 
http://devicetree.org/schemas/bus/microsoft,vmbus.yaml#

so maybe I need to add some more to the "requires" section. Will follow
other examples as you suggested.

> 
>>
>> diff --git a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
>> index a8d40c766dcd..5ec69226ab85 100644
>> --- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
>> +++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
>> @@ -44,11 +44,22 @@ examples:
>>               #size-cells = <1>;
>>               ranges;
>>   
>> +            gic: intc@fe200000 {
>> +              compatible = "arm,gic-v3";
>> +              reg = <0x0 0xfe200000 0x0 0x10000>,   /* GIC Dist */
>> +                    <0x0 0xfe280000 0x0 0x200000>;  /* GICR */
>> +              interrupt-controller;
>> +              #interrupt-cells = <3>;
>> +            }
> 
> I fail to see how this is relevant here. This is example only of vmbus.
> Look how other bindings are done. Drop the example.

The bus refers to the interrupt controller, and I didn't have it, so
added it :) Now I in other examples that is not required, and the
tooling generates fake intc's. Appreciate your advice very much!

> 
> 
>> +
>>               vmbus@ff0000000 {
>>                   compatible = "microsoft,vmbus";
>>                   #address-cells = <2>;
>>                   #size-cells = <1>;
>>                   ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>;
>> +                dma-coherent;
>> +                interrupt-parent = <&gic>;
>> +                interrupts = <1 2 1>;
> 
> Use proper defines for known constants.

Will do as in [1], thank you!

> 

[1]

--- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
+++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
@@ -28,6 +28,7 @@ properties:
  required:
    - compatible
    - ranges
+  - interrupts
    - '#address-cells'
    - '#size-cells'

@@ -35,6 +36,8 @@ additionalProperties: false

  examples:
    - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
      soc {
          #address-cells = <2>;
          #size-cells = <1>;
@@ -44,14 +47,6 @@ examples:
              #size-cells = <1>;
              ranges;

-            gic: intc@fe200000 {
-              compatible = "arm,gic-v3";
-              reg = <0x0 0xfe200000 0x0 0x10000>,   /* GIC Dist */
-                    <0x0 0xfe280000 0x0 0x200000>;  /* GICR */
-              interrupt-controller;
-              #interrupt-cells = <3>;
-            }
-
              vmbus@ff0000000 {
                  compatible = "microsoft,vmbus";
                  #address-cells = <2>;
@@ -59,7 +54,7 @@ examples:
                  ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>;
                  dma-coherent;
                  interrupt-parent = <&gic>;
-                interrupts = <1 2 1>;
+                interrupts = <GIC_PPI 2 IRQ_TYPE_EDGE_RISING>;
              };
          };
      };

> Best regards,
> Krzysztof

-- 
Thank you,
Roman