2021-07-26 04:53:23

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V3 1/5] dt-bindings: virtio: Add binding for virtio devices

Allow virtio device sub-nodes to be added to the virtio mmio or pci
nodes. The compatible property for virtio device must be of format
"virtio,<DID>", where DID is virtio device ID in hexadecimal format.

Signed-off-by: Viresh Kumar <[email protected]>
---
.../devicetree/bindings/virtio/mmio.yaml | 2 +-
.../bindings/virtio/virtio-device.yaml | 47 +++++++++++++++++++
2 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/virtio/virtio-device.yaml

diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml b/Documentation/devicetree/bindings/virtio/mmio.yaml
index d46597028cf1..1b91553f87c6 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.yaml
+++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
@@ -36,7 +36,7 @@ title: virtio memory mapped devices
- reg
- interrupts

-additionalProperties: false
+additionalProperties: true

examples:
- |
diff --git a/Documentation/devicetree/bindings/virtio/virtio-device.yaml b/Documentation/devicetree/bindings/virtio/virtio-device.yaml
new file mode 100644
index 000000000000..15cb6df8c98a
--- /dev/null
+++ b/Documentation/devicetree/bindings/virtio/virtio-device.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/virtio/virtio-device.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio device bindings
+
+maintainers:
+ - Viresh Kumar <[email protected]>
+
+description:
+ These bindings are applicable to virtio devices irrespective of the bus they
+ are bound to, like mmio or pci.
+
+# We need a select here so we don't match all nodes with 'virtio,mmio'
+properties:
+ $nodename:
+ pattern: '^[a-z0-9]+-virtio(-[a-z0-9]+)?$'
+ description: |
+ Exactly one node describing the virtio device. The name of the node isn't
+ significant but its phandle can be used to by a user of the virtio device.
+
+ compatible:
+ pattern: "^virtio,[0-9a-f]+$"
+ description: Virtio device nodes.
+ "virtio,DID", where DID is the virtio device id. The textual
+ representation of DID shall be in lower case hexadecimal with leading
+ zeroes suppressed.
+
+required:
+ - compatible
+
+additionalProperties: true
+
+examples:
+ - |
+ [email protected] {
+ compatible = "virtio,mmio";
+ reg = <0x3000 0x100>;
+ interrupts = <43>;
+
+ i2c-virtio {
+ compatible = "virtio,22";
+ };
+ };
+...
--
2.31.1.272.g89b43f80a514


2021-07-26 15:02:49

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH V3 1/5] dt-bindings: virtio: Add binding for virtio devices

On Sun, Jul 25, 2021 at 10:52 PM Viresh Kumar <[email protected]> wrote:
>
> Allow virtio device sub-nodes to be added to the virtio mmio or pci
> nodes. The compatible property for virtio device must be of format
> "virtio,<DID>", where DID is virtio device ID in hexadecimal format.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> .../devicetree/bindings/virtio/mmio.yaml | 2 +-
> .../bindings/virtio/virtio-device.yaml | 47 +++++++++++++++++++
> 2 files changed, 48 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/virtio/virtio-device.yaml
>
> diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml b/Documentation/devicetree/bindings/virtio/mmio.yaml
> index d46597028cf1..1b91553f87c6 100644
> --- a/Documentation/devicetree/bindings/virtio/mmio.yaml
> +++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
> @@ -36,7 +36,7 @@ title: virtio memory mapped devices
> - reg
> - interrupts
>
> -additionalProperties: false
> +additionalProperties: true

That just allows for any random property. What you want is child nodes only:

addtionalProperties:
type: object

Or you could reference virtio-device.yaml here.

>
> examples:
> - |
> diff --git a/Documentation/devicetree/bindings/virtio/virtio-device.yaml b/Documentation/devicetree/bindings/virtio/virtio-device.yaml
> new file mode 100644
> index 000000000000..15cb6df8c98a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/virtio/virtio-device.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/virtio/virtio-device.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Virtio device bindings
> +
> +maintainers:
> + - Viresh Kumar <[email protected]>
> +
> +description:
> + These bindings are applicable to virtio devices irrespective of the bus they
> + are bound to, like mmio or pci.
> +
> +# We need a select here so we don't match all nodes with 'virtio,mmio'
> +properties:
> + $nodename:
> + pattern: '^[a-z0-9]+-virtio(-[a-z0-9]+)?$'

Node names aren't based on the bus they are on, but their class.
You'll need to drop this.

> + description: |
> + Exactly one node describing the virtio device. The name of the node isn't
> + significant but its phandle can be used to by a user of the virtio device.
> +
> + compatible:
> + pattern: "^virtio,[0-9a-f]+$"

DID is only 4 chars? If so, "^virtio,[0-9a-f]{1,4}$"

> + description: Virtio device nodes.
> + "virtio,DID", where DID is the virtio device id. The textual
> + representation of DID shall be in lower case hexadecimal with leading
> + zeroes suppressed.
> +
> +required:
> + - compatible
> +
> +additionalProperties: true
> +
> +examples:
> + - |
> + [email protected] {
> + compatible = "virtio,mmio";
> + reg = <0x3000 0x100>;
> + interrupts = <43>;
> +
> + i2c-virtio {
> + compatible = "virtio,22";
> + };
> + };
> +...
> --
> 2.31.1.272.g89b43f80a514
>

2021-07-26 16:09:05

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V3 1/5] dt-bindings: virtio: Add binding for virtio devices

On Mon, Jul 26, 2021 at 4:57 PM Rob Herring <[email protected]> wrote:
> On Sun, Jul 25, 2021 at 10:52 PM Viresh Kumar <[email protected]> wrote:
> > + description: |
> > + Exactly one node describing the virtio device. The name of the node isn't
> > + significant but its phandle can be used to by a user of the virtio device.
> > +
> > + compatible:
> > + pattern: "^virtio,[0-9a-f]+$"
>
> DID is only 4 chars? If so, "^virtio,[0-9a-f]{1,4}$"

Any opinion on whether this should have any namespace prefix (or infix, I guess)
after "virtio,"?

I previously suggested making it "virtio,device[0-9a-f]{1,4}$", which would
make it clearer that the following digits are the device ID rather
than something
else we might define in the future. Viresh picked this version because it's
somewhat more consistent with other subsystems.

Arnd

2021-07-26 20:37:48

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH V3 1/5] dt-bindings: virtio: Add binding for virtio devices

On Mon, Jul 26, 2021 at 9:54 AM Arnd Bergmann <[email protected]> wrote:
>
> On Mon, Jul 26, 2021 at 4:57 PM Rob Herring <[email protected]> wrote:
> > On Sun, Jul 25, 2021 at 10:52 PM Viresh Kumar <[email protected]> wrote:
> > > + description: |
> > > + Exactly one node describing the virtio device. The name of the node isn't
> > > + significant but its phandle can be used to by a user of the virtio device.
> > > +
> > > + compatible:
> > > + pattern: "^virtio,[0-9a-f]+$"
> >
> > DID is only 4 chars? If so, "^virtio,[0-9a-f]{1,4}$"
>
> Any opinion on whether this should have any namespace prefix (or infix, I guess)
> after "virtio,"?
>
> I previously suggested making it "virtio,device[0-9a-f]{1,4}$", which would
> make it clearer that the following digits are the device ID rather
> than something
> else we might define in the future. Viresh picked this version because it's
> somewhat more consistent with other subsystems.

I'm fine either way, though I do find just a number a bit strange. So
I'd lean toward adding 'device' or even just a 'd'.

BTW, what happens if/when the device protocol is rev'ed? A new DID or
is there a separate revision that's discoverable?

Rob

2021-07-26 21:22:21

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V3 1/5] dt-bindings: virtio: Add binding for virtio devices

On Mon, Jul 26, 2021 at 10:37 PM Rob Herring <[email protected]> wrote:
>
> On Mon, Jul 26, 2021 at 9:54 AM Arnd Bergmann <[email protected]> wrote:
> >
> > On Mon, Jul 26, 2021 at 4:57 PM Rob Herring <[email protected]> wrote:
> > > On Sun, Jul 25, 2021 at 10:52 PM Viresh Kumar <[email protected]> wrote:
> > > > + description: |
> > > > + Exactly one node describing the virtio device. The name of the node isn't
> > > > + significant but its phandle can be used to by a user of the virtio device.
> > > > +
> > > > + compatible:
> > > > + pattern: "^virtio,[0-9a-f]+$"
> > >
> > > DID is only 4 chars? If so, "^virtio,[0-9a-f]{1,4}$"
> >
> > Any opinion on whether this should have any namespace prefix (or infix, I guess)
> > after "virtio,"?
> >
> > I previously suggested making it "virtio,device[0-9a-f]{1,4}$", which would
> > make it clearer that the following digits are the device ID rather
> > than something
> > else we might define in the future. Viresh picked this version because it's
> > somewhat more consistent with other subsystems.
>
> I'm fine either way, though I do find just a number a bit strange. So
> I'd lean toward adding 'device' or even just a 'd'.

I don't think just 'd' would be a good idea since it is indistinguishable from
a hexadecimal character. 'dev' would work though.

> BTW, what happens if/when the device protocol is rev'ed? A new DID or
> is there a separate revision that's discoverable?

This should normally be done using feature bits that are negotiated
between the two sides, and if only one side can do it, they use the
old revision.

There could be a new device ID but I don't think that has happened so far.

Arnd

2021-07-27 05:10:57

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V3 1/5] dt-bindings: virtio: Add binding for virtio devices

On 26-07-21, 08:57, Rob Herring wrote:
> On Sun, Jul 25, 2021 at 10:52 PM Viresh Kumar <[email protected]> wrote:
> > + description: |
> > + Exactly one node describing the virtio device. The name of the node isn't
> > + significant but its phandle can be used to by a user of the virtio device.
> > +
> > + compatible:
> > + pattern: "^virtio,[0-9a-f]+$"
>
> DID is only 4 chars? If so, "^virtio,[0-9a-f]{1,4}$"

It is 32 bit actually, so making this {1,8}.

--
viresh