[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bb863c8f-a92c-42d0-abc4-ff0b92f701c2@linux.microsoft.com>
Date: Wed, 12 Feb 2025 15:57:23 -0800
From: Roman Kisel <romank@...ux.microsoft.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: arnd@...db.de, bhelgaas@...gle.com, bp@...en8.de,
catalin.marinas@....com, conor+dt@...nel.org, dave.hansen@...ux.intel.com,
decui@...rosoft.com, haiyangz@...rosoft.com, hpa@...or.com,
krzk+dt@...nel.org, kw@...ux.com, kys@...rosoft.com, lpieralisi@...nel.org,
manivannan.sadhasivam@...aro.org, mingo@...hat.com, robh@...nel.org,
ssengar@...ux.microsoft.com, tglx@...utronix.de, wei.liu@...nel.org,
will@...nel.org, devicetree@...r.kernel.org, linux-arch@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-hyperv@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org, x86@...nel.org,
benhill@...rosoft.com, bperkins@...rosoft.com, sunilmut@...rosoft.com
Subject: Re: [PATCH hyperv-next v4 4/6] dt-bindings: microsoft,vmbus: Add GIC
and DMA coherence to the example
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@...ux.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@...000000: '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@...00000 {
>> + 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@...000000 {
>> 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@...00000 {
- compatible = "arm,gic-v3";
- reg = <0x0 0xfe200000 0x0 0x10000>, /* GIC Dist */
- <0x0 0xfe280000 0x0 0x200000>; /* GICR */
- interrupt-controller;
- #interrupt-cells = <3>;
- }
-
vmbus@...000000 {
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
Powered by blists - more mailing lists