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

[Saurabh Singh Sengar]

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 <ssengar@xxxxxxxxxxxxxxxxxxx>
> > ---
> >  .../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 <ssengar@xxxxxxxxxxxxxxxxxxx>
> > +
> > +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