2023-01-16 17:10:47

by Saurabh Singh Sengar

[permalink] [raw]
Subject: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

Add dt-bindings for Hyper-V VMBus

Signed-off-by: Saurabh Sengar <[email protected]>
---
.../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
1 file changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hv/msft,vmbus.yaml

diff --git a/Documentation/devicetree/bindings/hv/msft,vmbus.yaml b/Documentation/devicetree/bindings/hv/msft,vmbus.yaml
new file mode 100644
index 0000000..66cb426
--- /dev/null
+++ b/Documentation/devicetree/bindings/hv/msft,vmbus.yaml
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/hv/msft,vmbus.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microsoft Hyper-V VMBus device tree bindings
+
+maintainers:
+ - Saurabh Sengar <[email protected]>
+
+description:
+ VMBus is a software bus that implement the protocols for communication
+ between the root or host OS and guest OSs (virtual machines).
+
+properties:
+ compatible:
+ const: msft,vmbus
+
+ ranges :
+ const: <0x00 0x00 0x0f 0xf0000000 0x10000000>
+
+required:
+ - compatible
+ - ranges
+
+examples:
+ - |
+ vmbus {
+ #address-cells = <0x02>;
+ #size-cells = <0x01>;
+ compatible = "msft,vmbus";
+ ranges = <0x00 0x00 0x0f 0xf0000000 0x10000000>;
+ };
--
1.8.3.1


2023-01-16 19:11:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

On 16/01/2023 17:48, Saurabh Sengar wrote:
> Add dt-bindings for Hyper-V VMBus
>
> Signed-off-by: Saurabh Sengar <[email protected]>
> ---
> .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++

Also, there is no "hv" hardware, so that's not correct location. If your
bindings describe firmware, this should go to firmware. Otherwise, this
does not look like suitable for DT. We do not describe software stuff in DT.

Best regards,
Krzysztof

2023-01-16 19:52:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

On 16/01/2023 17:48, Saurabh Sengar wrote:
> Add dt-bindings for Hyper-V VMBus

Missing full stop.

Subject: drop second/last, redundant "dt-bindings for". The
"dt-bindings" prefix is already stating that these are bindings.

>
> Signed-off-by: Saurabh Sengar <[email protected]>
> ---
> .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hv/msft,vmbus.yaml
>
> diff --git a/Documentation/devicetree/bindings/hv/msft,vmbus.yaml b/Documentation/devicetree/bindings/hv/msft,vmbus.yaml
> new file mode 100644
> index 0000000..66cb426
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hv/msft,vmbus.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/hv/msft,vmbus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microsoft Hyper-V VMBus device tree bindings

Drop "device tree bindings"

> +
> +maintainers:
> + - Saurabh Sengar <[email protected]>
> +
> +description:
> + VMBus is a software bus that implement the protocols for communication
> + between the root or host OS and guest OSs (virtual machines).

Why this cannot be auto-discoverable? Why do you need OF for this?

> +
> +properties:
> + compatible:
> + const: msft,vmbus
> +
> + ranges :
> + const: <0x00 0x00 0x0f 0xf0000000 0x10000000>

Did you test the bindings?

This property does not look correct. If you have static addresses, you
do not need OF. What do you want to discover here?

> +
> +required:
> + - compatible
> + - ranges
> +
> +examples:
> + - |
> + vmbus {

Use 4 spaces for example indentation.

> + #address-cells = <0x02>;
> + #size-cells = <0x01>;

That's not correct style. Drop hex notation. Drop leading zeros.

But anyway you did not test the bindings. This cannot work. Try.

> + compatible = "msft,vmbus";

compatible is a first property.

> + ranges = <0x00 0x00 0x0f 0xf0000000 0x10000000>;

What do you translate? There is no reg, no unit address.

> + };

Best regards,
Krzysztof

2023-01-17 01:37:20

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus


On Mon, 16 Jan 2023 08:48:08 -0800, Saurabh Sengar wrote:
> Add dt-bindings for Hyper-V VMBus
>
> Signed-off-by: Saurabh Sengar <[email protected]>
> ---
> .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hv/msft,vmbus.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/hv/msft,vmbus.yaml:20:9: [warning] too many spaces before colon (colons)
./Documentation/devicetree/bindings/hv/msft,vmbus.yaml:30:1: [error] syntax error: found character '\t' that cannot start any token (syntax)

dtschema/dtc warnings/errors:
make[1]: *** Deleting file 'Documentation/devicetree/bindings/hv/msft,vmbus.example.dts'
Documentation/devicetree/bindings/hv/msft,vmbus.yaml:30:1: found a tab character where an indentation space is expected
make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/hv/msft,vmbus.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/hv/msft,vmbus.yaml:30:1: found a tab character where an indentation space is expected
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hv/msft,vmbus.yaml: ignoring, error parsing file
make: *** [Makefile:1508: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

2023-01-17 15:56:13

by Saurabh Singh Sengar

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

On Mon, Jan 16, 2023 at 07:55:13PM +0100, Krzysztof Kozlowski wrote:
> On 16/01/2023 17:48, Saurabh Sengar wrote:
> > Add dt-bindings for Hyper-V VMBus
> >
> > Signed-off-by: Saurabh Sengar <[email protected]>
> > ---
> > .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
>
> Also, there is no "hv" hardware, so that's not correct location. If your
> bindings describe firmware, this should go to firmware. Otherwise, this
> does not look like suitable for DT. We do not describe software stuff in DT.

VMBus is a virtual device this is simmilar to virtio. I can rename this folder to vmbus.

>
> Best regards,
> Krzysztof

2023-01-17 16:31:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

On 17/01/2023 16:13, Saurabh Singh Sengar wrote:
> On Mon, Jan 16, 2023 at 07:55:13PM +0100, Krzysztof Kozlowski wrote:
>> On 16/01/2023 17:48, Saurabh Sengar wrote:
>>> Add dt-bindings for Hyper-V VMBus
>>>
>>> Signed-off-by: Saurabh Sengar <[email protected]>
>>> ---
>>> .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
>>
>> Also, there is no "hv" hardware, so that's not correct location. If your
>> bindings describe firmware, this should go to firmware. Otherwise, this
>> does not look like suitable for DT. We do not describe software stuff in DT.
>
> VMBus is a virtual device this is simmilar to virtio. I can rename this folder to vmbus.
>

Then virtio directory. The directories are per subsystems (hardware
classes).

Best regards,
Krzysztof

2023-01-17 17:19:33

by Saurabh Singh Sengar

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

On Tue, Jan 17, 2023 at 04:41:22PM +0100, Krzysztof Kozlowski wrote:
> On 17/01/2023 16:13, Saurabh Singh Sengar wrote:
> > On Mon, Jan 16, 2023 at 07:55:13PM +0100, Krzysztof Kozlowski wrote:
> >> On 16/01/2023 17:48, Saurabh Sengar wrote:
> >>> Add dt-bindings for Hyper-V VMBus
> >>>
> >>> Signed-off-by: Saurabh Sengar <[email protected]>
> >>> ---
> >>> .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
> >>
> >> Also, there is no "hv" hardware, so that's not correct location. If your
> >> bindings describe firmware, this should go to firmware. Otherwise, this
> >> does not look like suitable for DT. We do not describe software stuff in DT.
> >
> > VMBus is a virtual device this is simmilar to virtio. I can rename this folder to vmbus.
> >
>
> Then virtio directory. The directories are per subsystems (hardware
> classes).

Apologies if I was not clear, I meant to say this is a device conceptually
similar to virtio. But this driver has nothing to do with virtio, we should
be creating a new folder for it OR I am fine moving it under bus if that's
okay.

>
> Best regards,
> Krzysztof

2023-01-20 11:56:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

On 17/01/2023 16:52, Saurabh Singh Sengar wrote:
> On Tue, Jan 17, 2023 at 04:41:22PM +0100, Krzysztof Kozlowski wrote:
>> On 17/01/2023 16:13, Saurabh Singh Sengar wrote:
>>> On Mon, Jan 16, 2023 at 07:55:13PM +0100, Krzysztof Kozlowski wrote:
>>>> On 16/01/2023 17:48, Saurabh Sengar wrote:
>>>>> Add dt-bindings for Hyper-V VMBus
>>>>>
>>>>> Signed-off-by: Saurabh Sengar <[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
>>>>
>>>> Also, there is no "hv" hardware, so that's not correct location. If your
>>>> bindings describe firmware, this should go to firmware. Otherwise, this
>>>> does not look like suitable for DT. We do not describe software stuff in DT.
>>>
>>> VMBus is a virtual device this is simmilar to virtio. I can rename this folder to vmbus.
>>>
>>
>> Then virtio directory. The directories are per subsystems (hardware
>> classes).
>
> Apologies if I was not clear, I meant to say this is a device conceptually
> similar to virtio. But this driver has nothing to do with virtio, we should

Bindings are for hardware, not drivers, so if the device serves the same
purpose, it's driver differences do not matter.

> be creating a new folder for it OR I am fine moving it under bus if that's
> okay.

Since you do not have children here, it's not really a bus to fit under
bus directory...

Probably this should go together with virtio bindings to dedicated
hypervisor interfaces directory. We do not create directories for
specific solutions (implementations) with only one or few bindings.
Directories are for entire classes.

Best regards,
Krzysztof

2023-01-20 13:20:06

by Saurabh Singh Sengar

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

On Fri, Jan 20, 2023 at 12:43:40PM +0100, Krzysztof Kozlowski wrote:
> On 17/01/2023 16:52, Saurabh Singh Sengar wrote:
> > On Tue, Jan 17, 2023 at 04:41:22PM +0100, Krzysztof Kozlowski wrote:
> >> On 17/01/2023 16:13, Saurabh Singh Sengar wrote:
> >>> On Mon, Jan 16, 2023 at 07:55:13PM +0100, Krzysztof Kozlowski wrote:
> >>>> On 16/01/2023 17:48, Saurabh Sengar wrote:
> >>>>> Add dt-bindings for Hyper-V VMBus
> >>>>>
> >>>>> Signed-off-by: Saurabh Sengar <[email protected]>
> >>>>> ---
> >>>>> .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
> >>>>
> >>>> Also, there is no "hv" hardware, so that's not correct location. If your
> >>>> bindings describe firmware, this should go to firmware. Otherwise, this
> >>>> does not look like suitable for DT. We do not describe software stuff in DT.
> >>>
> >>> VMBus is a virtual device this is simmilar to virtio. I can rename this folder to vmbus.
> >>>
> >>
> >> Then virtio directory. The directories are per subsystems (hardware
> >> classes).
> >
> > Apologies if I was not clear, I meant to say this is a device conceptually
> > similar to virtio. But this driver has nothing to do with virtio, we should
>
> Bindings are for hardware, not drivers, so if the device serves the same
> purpose, it's driver differences do not matter.
>
> > be creating a new folder for it OR I am fine moving it under bus if that's
> > okay.
>
> Since you do not have children here, it's not really a bus to fit under
> bus directory...
>
> Probably this should go together with virtio bindings to dedicated
> hypervisor interfaces directory. We do not create directories for
> specific solutions (implementations) with only one or few bindings.
> Directories are for entire classes.

I am OK to keep it anywhere, but I believe virtio is not its correct place. I am also
concern how will the virtio maintainers will perceive it. Ideally we should be renaming
virtio to virtualization OR hypervisor OR something more generic where both virtio and
VMBus can co-exist. Please let me know if renaming virtio is acceptable.

>
> Best regards,
> Krzysztof

2023-01-21 20:49:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

On 20/01/2023 13:51, Saurabh Singh Sengar wrote:
> On Fri, Jan 20, 2023 at 12:43:40PM +0100, Krzysztof Kozlowski wrote:
>> On 17/01/2023 16:52, Saurabh Singh Sengar wrote:
>>> On Tue, Jan 17, 2023 at 04:41:22PM +0100, Krzysztof Kozlowski wrote:
>>>> On 17/01/2023 16:13, Saurabh Singh Sengar wrote:
>>>>> On Mon, Jan 16, 2023 at 07:55:13PM +0100, Krzysztof Kozlowski wrote:
>>>>>> On 16/01/2023 17:48, Saurabh Sengar wrote:
>>>>>>> Add dt-bindings for Hyper-V VMBus
>>>>>>>
>>>>>>> Signed-off-by: Saurabh Sengar <[email protected]>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
>>>>>>
>>>>>> Also, there is no "hv" hardware, so that's not correct location. If your
>>>>>> bindings describe firmware, this should go to firmware. Otherwise, this
>>>>>> does not look like suitable for DT. We do not describe software stuff in DT.
>>>>>
>>>>> VMBus is a virtual device this is simmilar to virtio. I can rename this folder to vmbus.
>>>>>
>>>>
>>>> Then virtio directory. The directories are per subsystems (hardware
>>>> classes).
>>>
>>> Apologies if I was not clear, I meant to say this is a device conceptually
>>> similar to virtio. But this driver has nothing to do with virtio, we should
>>
>> Bindings are for hardware, not drivers, so if the device serves the same
>> purpose, it's driver differences do not matter.
>>
>>> be creating a new folder for it OR I am fine moving it under bus if that's
>>> okay.
>>
>> Since you do not have children here, it's not really a bus to fit under
>> bus directory...
>>
>> Probably this should go together with virtio bindings to dedicated
>> hypervisor interfaces directory. We do not create directories for
>> specific solutions (implementations) with only one or few bindings.
>> Directories are for entire classes.
>
> I am OK to keep it anywhere, but I believe virtio is not its correct place. I am also
> concern how will the virtio maintainers will perceive it. Ideally we should be renaming
> virtio to virtualization OR hypervisor OR something more generic where both virtio and
> VMBus can co-exist. Please let me know if renaming virtio is acceptable.

Yes, that's what I was thinking about. I think all of these should be in
one place, but named differently (with updates to MAINTAINERS place).

Best regards,
Krzysztof

2023-02-01 10:53:18

by Saurabh Singh Sengar

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: hv: Add dt-bindings for VMBus

On Mon, Jan 16, 2023 at 07:53:07PM +0100, Krzysztof Kozlowski wrote:
> On 16/01/2023 17:48, Saurabh Sengar wrote:
> > Add dt-bindings for Hyper-V VMBus
>
> Missing full stop.
>
> Subject: drop second/last, redundant "dt-bindings for". The
> "dt-bindings" prefix is already stating that these are bindings.

Will fix in v3.

>
> >
> > Signed-off-by: Saurabh Sengar <[email protected]>
> > ---
> > .../devicetree/bindings/hv/msft,vmbus.yaml | 34 ++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/hv/msft,vmbus.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hv/msft,vmbus.yaml b/Documentation/devicetree/bindings/hv/msft,vmbus.yaml
> > new file mode 100644
> > index 0000000..66cb426
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hv/msft,vmbus.yaml
> > @@ -0,0 +1,34 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/hv/msft,vmbus.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microsoft Hyper-V VMBus device tree bindings
>
> Drop "device tree bindings"

Will fix in v3

>
> > +
> > +maintainers:
> > + - Saurabh Sengar <[email protected]>
> > +
> > +description:
> > + VMBus is a software bus that implement the protocols for communication
> > + between the root or host OS and guest OSs (virtual machines).
>
> Why this cannot be auto-discoverable? Why do you need OF for this?

This is a virtulization device, and I guess we have discussed this in greater
length in other thread.

>
> > +
> > +properties:
> > + compatible:
> > + const: msft,vmbus
> > +
> > + ranges :
> > + const: <0x00 0x00 0x0f 0xf0000000 0x10000000>
>
> Did you test the bindings?
>
> This property does not look correct. If you have static addresses, you
> do not need OF. What do you want to discover here?

fixed in v2

>
> > +
> > +required:
> > + - compatible
> > + - ranges
> > +
> > +examples:
> > + - |
> > + vmbus {
>
> Use 4 spaces for example indentation.

Fix in v2

>
> > + #address-cells = <0x02>;
> > + #size-cells = <0x01>;
>
> That's not correct style. Drop hex notation. Drop leading zeros.

Will fix in v3

>
> But anyway you did not test the bindings. This cannot work. Try.
>
> > + compatible = "msft,vmbus";
>
> compatible is a first property.

fixed in v2

>
> > + ranges = <0x00 0x00 0x0f 0xf0000000 0x10000000>;
>
> What do you translate? There is no reg, no unit address.

Commented on v2 thread, if there is any further concern using ranges
please let me know.

>
> > + };
>
> Best regards,
> Krzysztof