| Message ID | 1675188609-20913-6-git-send-email-ssengar@linux.microsoft.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Device tree support for Hyper-V VMBus driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 31/01/2023 19:10, Saurabh Sengar wrote: > Add dt-bindings for Hyper-V VMBus > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> > --- > .../bindings/hypervisor/msft,vmbus.yaml | 50 +++++++++++++++++++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > MAINTAINERS | 1 + > 3 files changed, 53 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml > > diff --git a/Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml b/Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml > new file mode 100644 > index 000000000000..8f50d6097c48 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml > @@ -0,0 +1,50 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hypervisor/msft,vmbus.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microsoft Hyper-V VMBus device tree bindings This is a friendly reminder during the review process. It seems my previous comments were not fully addressed. Maybe my feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. (other places as well...) Best regards, Krzysztof
On 31/01/2023 19:10, Saurabh Sengar wrote: > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> > + - | > + / { > + compatible = "foo"; > + model = "foo"; > + #address-cells = <0x02>; > + #size-cells = <0x02>; Except previous comments (all of them were ignored), also: Drop entire part. Not related, not correct, not helping and you cannot have top level nodes in example. > + > + vmbus@ff0000000 { > + #address-cells = <0x02>; > + #size-cells = <0x01>; > + compatible = "msft,vmbus"; > + ranges = <0x00 0x00 0x0f 0xf0000000 0x10000000>; > + }; > + }; > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml > index 1f7a519a936f..ab74ea97535f 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > @@ -876,6 +876,8 @@ patternProperties: > deprecated: true > "^mscc,.*": > description: Microsemi Corporation > + "^msft,.*": > + description: Microsoft Corporation Don't duplicate vendor prefixes. Drop and use correct prefix. Best regards, Krzysztof
On Tue, Jan 31, 2023 at 07:51:20PM +0100, Krzysztof Kozlowski wrote: > On 31/01/2023 19:10, Saurabh Sengar wrote: > > Add dt-bindings for Hyper-V VMBus > > > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> > > --- > > .../bindings/hypervisor/msft,vmbus.yaml | 50 +++++++++++++++++++ > > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > > MAINTAINERS | 1 + > > 3 files changed, 53 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml > > > > diff --git a/Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml b/Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml > > new file mode 100644 > > index 000000000000..8f50d6097c48 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml > > @@ -0,0 +1,50 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/hypervisor/msft,vmbus.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Microsoft Hyper-V VMBus device tree bindings > > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all > requested changes or keep discussing them. > > Thank you. > > (other places as well...) Hi Krzysztof, Thank you for the review. Sorry I missed this, will fix in V3. The patches have gone significant modification, and I have thought I have fixed all the comments you have provided hence didnt send the follow up discussion. Apparently, I may have missed few will look again and fix in next version. Regards, Saurabh > > Best regards, > Krzysztof
On Tue, Jan 31, 2023 at 07:54:56PM +0100, Krzysztof Kozlowski wrote: > On 31/01/2023 19:10, Saurabh Sengar wrote: > > > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> > > > + - | > > + / { > > + compatible = "foo"; > > + model = "foo"; > > + #address-cells = <0x02>; > > + #size-cells = <0x02>; > > Except previous comments (all of them were ignored), Thank you for your comments, I have tried to address them all in this version but I may have missed few. I will go and look again all the emails, but if there is any thing which you can point again and we can start a new dicussion from here will be very helpful. I think one concern was related to use of 'reg' or 'ranges', and I thought this patch will give clarity that I intend to use 'ranges' without any child node. Is this acceptable ? > also: > Drop entire part. Not related, not correct, not helping and you cannot > have top level nodes in example. If I dont use the top level node, the parent address cells are assumed to be 1, and I get below warning. If there is better way to address this warning, please suggest I will work on it. Warning (ranges_format): /example-0/vmbus@ff0000000:ranges: "ranges" property has invalid length (20 bytes) (parent #address-cells == 1, child #address-cells == 2, #size-cells == 1) > > > + > > + vmbus@ff0000000 { > > + #address-cells = <0x02>; > > + #size-cells = <0x01>; > > + compatible = "msft,vmbus"; > > + ranges = <0x00 0x00 0x0f 0xf0000000 0x10000000>; > > + }; > > + }; > > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml > > index 1f7a519a936f..ab74ea97535f 100644 > > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > > @@ -876,6 +876,8 @@ patternProperties: > > deprecated: true > > "^mscc,.*": > > description: Microsemi Corporation > > + "^msft,.*": > > + description: Microsoft Corporation > > Don't duplicate vendor prefixes. Drop and use correct prefix. Sure, will fix this. Regards, Saurabh > > Best regards, > Krzysztof
On 01/02/2023 02:57, Saurabh Singh Sengar wrote: > On Tue, Jan 31, 2023 at 07:54:56PM +0100, Krzysztof Kozlowski wrote: >> On 31/01/2023 19:10, Saurabh Sengar wrote: >>> >>> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> >> >>> + - | >>> + / { >>> + compatible = "foo"; >>> + model = "foo"; >>> + #address-cells = <0x02>; >>> + #size-cells = <0x02>; >> >> Except previous comments (all of them were ignored), > > Thank you for your comments, I have tried to address them all in this version > but I may have missed few. I will go and look again all the emails, but if > there is any thing which you can point again and we can start a new dicussion > from here will be very helpful. > > I think one concern was related to use of 'reg' or 'ranges', and I > thought this patch will give clarity that I intend to use 'ranges' > without any child node. Is this acceptable ? There were several comments. > > >> also: >> Drop entire part. Not related, not correct, not helping and you cannot >> have top level nodes in example. > > If I dont use the top level node, the parent address cells are assumed to be 1, > and I get below warning. If there is better way to address this warning, please > suggest I will work on it. > > Warning (ranges_format): /example-0/vmbus@ff0000000:ranges: "ranges" property has invalid length (20 bytes) (parent #address-cells == 1, child #address-cells == 2, #size-cells == 1) Then use soc just like every other example. Best regards, Krzysztof
On Wed, Feb 01, 2023 at 08:23:48AM +0100, Krzysztof Kozlowski wrote: > On 01/02/2023 02:57, Saurabh Singh Sengar wrote: > > On Tue, Jan 31, 2023 at 07:54:56PM +0100, Krzysztof Kozlowski wrote: > >> On 31/01/2023 19:10, Saurabh Sengar wrote: > >>> > >>> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> > >> > >>> + - | > >>> + / { > >>> + compatible = "foo"; > >>> + model = "foo"; > >>> + #address-cells = <0x02>; > >>> + #size-cells = <0x02>; > >> > >> Except previous comments (all of them were ignored), > > > > Thank you for your comments, I have tried to address them all in this version > > but I may have missed few. I will go and look again all the emails, but if > > there is any thing which you can point again and we can start a new dicussion > > from here will be very helpful. > > > > I think one concern was related to use of 'reg' or 'ranges', and I > > thought this patch will give clarity that I intend to use 'ranges' > > without any child node. Is this acceptable ? > > There were several comments. Ok, let me reply to all of your comments from previous thread to avoid any confusion. Hope this is fine. > > > > > > >> also: > >> Drop entire part. Not related, not correct, not helping and you cannot > >> have top level nodes in example. > > > > If I dont use the top level node, the parent address cells are assumed to be 1, > > and I get below warning. If there is better way to address this warning, please > > suggest I will work on it. > > > > Warning (ranges_format): /example-0/vmbus@ff0000000:ranges: "ranges" property has invalid length (20 bytes) (parent #address-cells == 1, child #address-cells == 2, #size-cells == 1) > > Then use soc just like every other example. Thanks for suggestion, I will fix this in V3 Regards, Saurabh > > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml b/Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml new file mode 100644 index 000000000000..8f50d6097c48 --- /dev/null +++ b/Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml @@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hypervisor/msft,vmbus.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microsoft Hyper-V VMBus device tree bindings + +maintainers: + - Saurabh Sengar <ssengar@linux.microsoft.com> + +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: true + + '#address-cells': + const: 2 + + '#size-cells': + const: 1 + +required: + - compatible + - ranges + - '#address-cells' + - '#size-cells' + +additionalProperties: false + +examples: + - | + / { + compatible = "foo"; + model = "foo"; + #address-cells = <0x02>; + #size-cells = <0x02>; + + vmbus@ff0000000 { + #address-cells = <0x02>; + #size-cells = <0x01>; + compatible = "msft,vmbus"; + ranges = <0x00 0x00 0x0f 0xf0000000 0x10000000>; + }; + }; diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 1f7a519a936f..ab74ea97535f 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -876,6 +876,8 @@ patternProperties: deprecated: true "^mscc,.*": description: Microsemi Corporation + "^msft,.*": + description: Microsoft Corporation "^msi,.*": description: Micro-Star International Co. Ltd. "^mstar,.*": diff --git a/MAINTAINERS b/MAINTAINERS index 9bc5b88b723a..c749782b0cff 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9496,6 +9496,7 @@ S: Supported T: git git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git F: Documentation/ABI/stable/sysfs-bus-vmbus F: Documentation/ABI/testing/debugfs-hyperv +F: Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml F: Documentation/virt/hyperv F: Documentation/networking/device_drivers/ethernet/microsoft/netvsc.rst F: arch/arm64/hyperv
Add dt-bindings for Hyper-V VMBus Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> --- .../bindings/hypervisor/msft,vmbus.yaml | 50 +++++++++++++++++++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + MAINTAINERS | 1 + 3 files changed, 53 insertions(+) create mode 100644 Documentation/devicetree/bindings/hypervisor/msft,vmbus.yaml