[PATCH 0/5] virtio: Parse virtio-device nodes from DT

[PATCH 0/5] virtio: Parse virtio-device nodes from DT

[Viresh Kumar]

Hi,

Currently the DT only provides support for following node types for virtio-mmio
nodes:

	virtio_mmio@a000000 {
		dma-coherent;
		interrupts = <0x00 0x10 0x01>;
		reg = <0x00 0xa000000 0x00 0x200>;
		compatible = "virtio,mmio";
	};

And each virtio-mmio corresponds to a virtio-device. But there is no way for
other users in the DT to show their dependency on virtio devices.

This patchset provides that support.

The first patch update virtio,mmio bindings to allow for device subnodes to be
present and the second patch updates the virtio-mmio driver to update the
of_node.

Other patches add bindings for i2c and gpio virtio devices (they have some
dependencies, mentioned in the patches).

Tested on x86 with qemu for arm64.

--
Viresh

Viresh Kumar (5):
  dt-bindings: virtio: mmio: Add support for device subnode
  virtio_mmio: Bind virtio device to device-tree node
  dt-bindings: i2c: Add bindings for i2c-virtio
  i2c: virtio: Update i2c-adapter's of_node
  dt-bindings: gpio: Add bindings for gpio-virtio

 .../devicetree/bindings/gpio/gpio-virtio.yaml | 67 +++++++++++++++++++
 .../devicetree/bindings/i2c/i2c-virtio.yaml   | 59 ++++++++++++++++
 .../devicetree/bindings/virtio/mmio.yaml      | 41 ++++++++++++
 drivers/i2c/busses/i2c-virtio.c               |  1 +
 drivers/virtio/virtio_mmio.c                  | 44 ++++++++++++
 include/dt-bindings/virtio/virtio_ids.h       |  1 +
 6 files changed, 213 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
 create mode 120000 include/dt-bindings/virtio/virtio_ids.h

-- 
2.31.1.272.g89b43f80a514

[PATCH 0/5] virtio: Parse virtio-device nodes from DT

[Viresh Kumar]

Hi,

Currently the DT only provides support for following node types for virtio-mmio
nodes:

	virtio_mmio@a000000 {
		dma-coherent;
		interrupts = <0x00 0x10 0x01>;
		reg = <0x00 0xa000000 0x00 0x200>;
		compatible = "virtio,mmio";
	};

And each virtio-mmio corresponds to a virtio-device. But there is no way for
other users in the DT to show their dependency on virtio devices.

This patchset provides that support.

The first patch update virtio,mmio bindings to allow for device subnodes to be
present and the second patch updates the virtio-mmio driver to update the
of_node.

Other patches add bindings for i2c and gpio virtio devices (they have some
dependencies, mentioned in the patches).

Tested on x86 with qemu for arm64.

--
Viresh

Viresh Kumar (5):
  dt-bindings: virtio: mmio: Add support for device subnode
  virtio_mmio: Bind virtio device to device-tree node
  dt-bindings: i2c: Add bindings for i2c-virtio
  i2c: virtio: Update i2c-adapter's of_node
  dt-bindings: gpio: Add bindings for gpio-virtio

 .../devicetree/bindings/gpio/gpio-virtio.yaml | 67 +++++++++++++++++++
 .../devicetree/bindings/i2c/i2c-virtio.yaml   | 59 ++++++++++++++++
 .../devicetree/bindings/virtio/mmio.yaml      | 41 ++++++++++++
 drivers/i2c/busses/i2c-virtio.c               |  1 +
 drivers/virtio/virtio_mmio.c                  | 44 ++++++++++++
 include/dt-bindings/virtio/virtio_ids.h       |  1 +
 6 files changed, 213 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
 create mode 120000 include/dt-bindings/virtio/virtio_ids.h

-- 
2.31.1.272.g89b43f80a514

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

Allow virtio,mmio nodes to contain device specific subnodes. Since each
virtio,mmio node can represent a single virtio device, each virtio node
is allowed to contain a maximum of one device specific subnode.

The device subnode must have the "reg" property, and its value must
match the virtio device ID used by the virtio mmio node.

A phandle to this device subnode can then be used by the users of the
virtio device.

Also add a symbolic link to uapi/linux/virtio_ids.h in order to use the
definitions here.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/virtio/mmio.yaml      | 41 +++++++++++++++++++
 include/dt-bindings/virtio/virtio_ids.h       |  1 +
 2 files changed, 42 insertions(+)
 create mode 120000 include/dt-bindings/virtio/virtio_ids.h

diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml b/Documentation/devicetree/bindings/virtio/mmio.yaml
index d46597028cf1..e5f9fe6ecb5e 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.yaml
+++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
@@ -31,6 +31,31 @@ title: virtio memory mapped devices
     description: Required for devices making accesses thru an IOMMU.
     maxItems: 1
 
+  "#address-cells":
+    const: 1
+    description:
+      The cell is the device ID if a device subnode is used.
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  '^[a-z0-9]+-virtio@[0-9]+$':
+    type: object
+    description: |
+      Exactly one node describing the virtio device. The name of the node isn't
+      significant but its phandle can be used to by an user of the virtio
+      device.
+
+    properties:
+      reg:
+        description:
+          Must contain the Virtio ID of the device.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+    required:
+      - reg
+
 required:
   - compatible
   - reg
@@ -57,4 +82,20 @@ additionalProperties: false
         #iommu-cells = <1>;
     };
 
+  - |
+    #include <dt-bindings/virtio/virtio_ids.h>
+
+    virtio@3200 {
+        compatible = "virtio,mmio";
+        reg = <0x3200 0x100>;
+        interrupts = <43>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-virtio@0 {
+            reg = <VIRTIO_ID_I2C_ADAPTER>;
+        };
+    };
+
 ...
diff --git a/include/dt-bindings/virtio/virtio_ids.h b/include/dt-bindings/virtio/virtio_ids.h
new file mode 120000
index 000000000000..6e59ba332216
--- /dev/null
+++ b/include/dt-bindings/virtio/virtio_ids.h
@@ -0,0 +1 @@
+../../uapi/linux/virtio_ids.h
\ No newline at end of file
-- 
2.31.1.272.g89b43f80a514

[PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

Allow virtio,mmio nodes to contain device specific subnodes. Since each
virtio,mmio node can represent a single virtio device, each virtio node
is allowed to contain a maximum of one device specific subnode.

The device subnode must have the "reg" property, and its value must
match the virtio device ID used by the virtio mmio node.

A phandle to this device subnode can then be used by the users of the
virtio device.

Also add a symbolic link to uapi/linux/virtio_ids.h in order to use the
definitions here.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/virtio/mmio.yaml      | 41 +++++++++++++++++++
 include/dt-bindings/virtio/virtio_ids.h       |  1 +
 2 files changed, 42 insertions(+)
 create mode 120000 include/dt-bindings/virtio/virtio_ids.h

diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml b/Documentation/devicetree/bindings/virtio/mmio.yaml
index d46597028cf1..e5f9fe6ecb5e 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.yaml
+++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
@@ -31,6 +31,31 @@ title: virtio memory mapped devices
     description: Required for devices making accesses thru an IOMMU.
     maxItems: 1
 
+  "#address-cells":
+    const: 1
+    description:
+      The cell is the device ID if a device subnode is used.
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  '^[a-z0-9]+-virtio@[0-9]+$':
+    type: object
+    description: |
+      Exactly one node describing the virtio device. The name of the node isn't
+      significant but its phandle can be used to by an user of the virtio
+      device.
+
+    properties:
+      reg:
+        description:
+          Must contain the Virtio ID of the device.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+    required:
+      - reg
+
 required:
   - compatible
   - reg
@@ -57,4 +82,20 @@ additionalProperties: false
         #iommu-cells = <1>;
     };
 
+  - |
+    #include <dt-bindings/virtio/virtio_ids.h>
+
+    virtio@3200 {
+        compatible = "virtio,mmio";
+        reg = <0x3200 0x100>;
+        interrupts = <43>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-virtio@0 {
+            reg = <VIRTIO_ID_I2C_ADAPTER>;
+        };
+    };
+
 ...
diff --git a/include/dt-bindings/virtio/virtio_ids.h b/include/dt-bindings/virtio/virtio_ids.h
new file mode 120000
index 000000000000..6e59ba332216
--- /dev/null
+++ b/include/dt-bindings/virtio/virtio_ids.h
@@ -0,0 +1 @@
+../../uapi/linux/virtio_ids.h
\ No newline at end of file
-- 
2.31.1.272.g89b43f80a514

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 2/5] virtio_mmio: Bind virtio device to device-tree node

[Viresh Kumar]

Bind the virtio device with its device protocol's sub-node. This will
help users of the virtio device to mention their dependencies on the
device in the DT file itself. Like GPIO pin users can use the phandle of
the device node, or the node may contain more subnodes to add i2c or spi
eeproms and other users.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/virtio/virtio_mmio.c | 44 ++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 56128b9c46eb..ae40546a66a3 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -551,11 +551,51 @@ static void virtio_mmio_release_dev(struct device *_d)
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
 	struct platform_device *pdev = vm_dev->pdev;
 
+	of_node_put(vdev->dev.of_node);
 	devm_kfree(&pdev->dev, vm_dev);
 }
 
 /* Platform device */
 
+static int virtio_mmio_of_init(struct virtio_device *vdev)
+{
+	struct device_node *np, *pnode = vdev->dev.parent->of_node;
+	int ret, count;
+	u32 reg;
+
+	if (!pnode)
+		return 0;
+
+	count = of_get_available_child_count(pnode);
+	if (!count)
+		return 0;
+
+	/* There can be only 1 child node */
+	if (WARN_ON(count > 1))
+		return -EINVAL;
+
+	np = of_get_next_available_child(pnode, NULL);
+	if (WARN_ON(!np))
+		return -ENODEV;
+
+	ret = of_property_read_u32(np, "reg", &reg);
+	if (ret < 0)
+		goto out;
+
+	/* The reg field should match the device id */
+	if (WARN_ON(reg != vdev->id.device)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	vdev->dev.of_node = np;
+	return 0;
+
+out:
+	of_node_put(np);
+	return ret;
+}
+
 static int virtio_mmio_probe(struct platform_device *pdev)
 {
 	struct virtio_mmio_device *vm_dev;
@@ -621,6 +661,10 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 	if (rc)
 		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
 
+	rc = virtio_mmio_of_init(&vm_dev->vdev);
+	if (rc)
+		return rc;
+
 	platform_set_drvdata(pdev, vm_dev);
 
 	rc = register_virtio_device(&vm_dev->vdev);
-- 
2.31.1.272.g89b43f80a514

[PATCH 2/5] virtio_mmio: Bind virtio device to device-tree node

[Viresh Kumar]

Bind the virtio device with its device protocol's sub-node. This will
help users of the virtio device to mention their dependencies on the
device in the DT file itself. Like GPIO pin users can use the phandle of
the device node, or the node may contain more subnodes to add i2c or spi
eeproms and other users.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/virtio/virtio_mmio.c | 44 ++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 56128b9c46eb..ae40546a66a3 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -551,11 +551,51 @@ static void virtio_mmio_release_dev(struct device *_d)
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
 	struct platform_device *pdev = vm_dev->pdev;
 
+	of_node_put(vdev->dev.of_node);
 	devm_kfree(&pdev->dev, vm_dev);
 }
 
 /* Platform device */
 
+static int virtio_mmio_of_init(struct virtio_device *vdev)
+{
+	struct device_node *np, *pnode = vdev->dev.parent->of_node;
+	int ret, count;
+	u32 reg;
+
+	if (!pnode)
+		return 0;
+
+	count = of_get_available_child_count(pnode);
+	if (!count)
+		return 0;
+
+	/* There can be only 1 child node */
+	if (WARN_ON(count > 1))
+		return -EINVAL;
+
+	np = of_get_next_available_child(pnode, NULL);
+	if (WARN_ON(!np))
+		return -ENODEV;
+
+	ret = of_property_read_u32(np, "reg", &reg);
+	if (ret < 0)
+		goto out;
+
+	/* The reg field should match the device id */
+	if (WARN_ON(reg != vdev->id.device)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	vdev->dev.of_node = np;
+	return 0;
+
+out:
+	of_node_put(np);
+	return ret;
+}
+
 static int virtio_mmio_probe(struct platform_device *pdev)
 {
 	struct virtio_mmio_device *vm_dev;
@@ -621,6 +661,10 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 	if (rc)
 		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
 
+	rc = virtio_mmio_of_init(&vm_dev->vdev);
+	if (rc)
+		return rc;
+
 	platform_set_drvdata(pdev, vm_dev);
 
 	rc = register_virtio_device(&vm_dev->vdev);
-- 
2.31.1.272.g89b43f80a514

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 3/5] dt-bindings: i2c: Add bindings for i2c-virtio

[Viresh Kumar]

i2c-virtio represents a virtio I2C device and this patch adds binding
for the same. The i2c-virtio subnode can be part of a virtio,mmio node
and is based on its binding.

Cc: Wolfram Sang <wsa@kernel.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/i2c/i2c-virtio.yaml   | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-virtio.yaml

diff --git a/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml b/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
new file mode 100644
index 000000000000..6b6538f9a3d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/i2c-virtio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio memory mapped I2C Adapter
+
+maintainers:
+  - Viresh Kumar <viresh.kumar@linaro.org>
+
+description:
+  Virtio I2C device, see /schemas/virtio/mmio.yaml for more details.
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  $nodename:
+    pattern: '^i2c-virtio@[0-9]+$'
+
+  reg:
+    description:
+      The cell is the device ID of the I2C device (VIRTIO_ID_I2C_ADAPTER) as per
+      dt-bindings/virtio/virtio_ids.h.
+    const: 34
+    $ref: /schemas/virtio/mmio.yaml#/properties/reg
+
+required:
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/virtio/virtio_ids.h>
+
+    virtio@3000 {
+        compatible = "virtio,mmio";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-virtio@0 {
+            reg = <VIRTIO_ID_I2C_ADAPTER>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            light-sensor@1c {
+                compatible = "dynaimage,al3320a";
+                reg = <0x20>;
+            };
+        };
+    };
+
+...
-- 
2.31.1.272.g89b43f80a514

[PATCH 3/5] dt-bindings: i2c: Add bindings for i2c-virtio

[Viresh Kumar]

i2c-virtio represents a virtio I2C device and this patch adds binding
for the same. The i2c-virtio subnode can be part of a virtio,mmio node
and is based on its binding.

Cc: Wolfram Sang <wsa@kernel.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/i2c/i2c-virtio.yaml   | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-virtio.yaml

diff --git a/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml b/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
new file mode 100644
index 000000000000..6b6538f9a3d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/i2c-virtio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio memory mapped I2C Adapter
+
+maintainers:
+  - Viresh Kumar <viresh.kumar@linaro.org>
+
+description:
+  Virtio I2C device, see /schemas/virtio/mmio.yaml for more details.
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  $nodename:
+    pattern: '^i2c-virtio@[0-9]+$'
+
+  reg:
+    description:
+      The cell is the device ID of the I2C device (VIRTIO_ID_I2C_ADAPTER) as per
+      dt-bindings/virtio/virtio_ids.h.
+    const: 34
+    $ref: /schemas/virtio/mmio.yaml#/properties/reg
+
+required:
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/virtio/virtio_ids.h>
+
+    virtio@3000 {
+        compatible = "virtio,mmio";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-virtio@0 {
+            reg = <VIRTIO_ID_I2C_ADAPTER>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            light-sensor@1c {
+                compatible = "dynaimage,al3320a";
+                reg = <0x20>;
+            };
+        };
+    };
+
+...
-- 
2.31.1.272.g89b43f80a514

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 4/5] i2c: virtio: Update i2c-adapter's of_node

[Viresh Kumar]

Set of_node of the adapter from the virtio device to enable automatic
parsing the of the I2C devices present in DT.

Cc: Wolfram Sang <wsa@kernel.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Depends on:

https://lore.kernel.org/linux-i2c/984ebecaf697058eb73389ed14ead9dd6d38fb53.1625796246.git.jie.deng@intel.com/
---
 drivers/i2c/busses/i2c-virtio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
index 0139cdc33cae..cb8cfae2748f 100644
--- a/drivers/i2c/busses/i2c-virtio.c
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -223,6 +223,7 @@ static int virtio_i2c_probe(struct virtio_device *vdev)
 		 "i2c_virtio at virtio bus %d", vdev->index);
 	vi->adap.algo = &virtio_algorithm;
 	vi->adap.dev.parent = &vdev->dev;
+	vi->adap.dev.of_node = vdev->dev.of_node;
 	i2c_set_adapdata(&vi->adap, vi);
 
 	/*
-- 
2.31.1.272.g89b43f80a514

[PATCH 4/5] i2c: virtio: Update i2c-adapter's of_node

[Viresh Kumar]

Set of_node of the adapter from the virtio device to enable automatic
parsing the of the I2C devices present in DT.

Cc: Wolfram Sang <wsa@kernel.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Depends on:

https://lore.kernel.org/linux-i2c/984ebecaf697058eb73389ed14ead9dd6d38fb53.1625796246.git.jie.deng@intel.com/
---
 drivers/i2c/busses/i2c-virtio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
index 0139cdc33cae..cb8cfae2748f 100644
--- a/drivers/i2c/busses/i2c-virtio.c
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -223,6 +223,7 @@ static int virtio_i2c_probe(struct virtio_device *vdev)
 		 "i2c_virtio at virtio bus %d", vdev->index);
 	vi->adap.algo = &virtio_algorithm;
 	vi->adap.dev.parent = &vdev->dev;
+	vi->adap.dev.of_node = vdev->dev.of_node;
 	i2c_set_adapdata(&vi->adap, vi);
 
 	/*
-- 
2.31.1.272.g89b43f80a514

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 5/5] dt-bindings: gpio: Add bindings for gpio-virtio

[Viresh Kumar]

gpio-virtio represents a virtio GPIO controller and this patch adds
binding for the same. The gpio-virtio subnode can be part of a
virtio,mmio node and is based on its binding.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Depends on:

https://lore.kernel.org/lkml/7c716c2eb7ace5b5a560d8502af93101dbb53d24.1626170146.git.viresh.kumar@linaro.org/
---
 .../devicetree/bindings/gpio/gpio-virtio.yaml | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-virtio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
new file mode 100644
index 000000000000..c813cdfd60fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-virtio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio memory mapped GPIO controller
+
+maintainers:
+  - Viresh Kumar <viresh.kumar@linaro.org>
+
+description:
+  Virtio GPIO controller, see /schemas/virtio/mmio.yaml for more details.
+
+allOf:
+  - $ref: /schemas/gpio/gpio.yaml#
+
+properties:
+  $nodename:
+    pattern: '^gpio-virtio@[0-9]+$'
+
+  reg:
+    description:
+      The cell is the device ID of the GPIO device (VIRTIO_ID_GPIO) as per
+      dt-bindings/virtio/virtio_ids.h.
+    const: 41
+    $ref: /schemas/virtio/mmio.yaml#/properties/reg
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - reg
+  - gpio-controller
+  - "#gpio-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/virtio/virtio_ids.h>
+
+    virtio@3000 {
+        compatible = "virtio,mmio";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        gpio-virtio@0 {
+            reg = <VIRTIO_ID_GPIO>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+        };
+    };
+
+...
-- 
2.31.1.272.g89b43f80a514

[PATCH 5/5] dt-bindings: gpio: Add bindings for gpio-virtio

[Viresh Kumar]

gpio-virtio represents a virtio GPIO controller and this patch adds
binding for the same. The gpio-virtio subnode can be part of a
virtio,mmio node and is based on its binding.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Depends on:

https://lore.kernel.org/lkml/7c716c2eb7ace5b5a560d8502af93101dbb53d24.1626170146.git.viresh.kumar@linaro.org/
---
 .../devicetree/bindings/gpio/gpio-virtio.yaml | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-virtio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
new file mode 100644
index 000000000000..c813cdfd60fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-virtio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio memory mapped GPIO controller
+
+maintainers:
+  - Viresh Kumar <viresh.kumar@linaro.org>
+
+description:
+  Virtio GPIO controller, see /schemas/virtio/mmio.yaml for more details.
+
+allOf:
+  - $ref: /schemas/gpio/gpio.yaml#
+
+properties:
+  $nodename:
+    pattern: '^gpio-virtio@[0-9]+$'
+
+  reg:
+    description:
+      The cell is the device ID of the GPIO device (VIRTIO_ID_GPIO) as per
+      dt-bindings/virtio/virtio_ids.h.
+    const: 41
+    $ref: /schemas/virtio/mmio.yaml#/properties/reg
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - reg
+  - gpio-controller
+  - "#gpio-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/virtio/virtio_ids.h>
+
+    virtio@3000 {
+        compatible = "virtio,mmio";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        gpio-virtio@0 {
+            reg = <VIRTIO_ID_GPIO>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+        };
+    };
+
+...
-- 
2.31.1.272.g89b43f80a514

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/5] virtio_mmio: Bind virtio device to device-tree node

[Arnd Bergmann]

On Tue, Jul 13, 2021 at 12:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Bind the virtio device with its device protocol's sub-node. This will
> help users of the virtio device to mention their dependencies on the
> device in the DT file itself. Like GPIO pin users can use the phandle of
> the device node, or the node may contain more subnodes to add i2c or spi
> eeproms and other users.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/virtio/virtio_mmio.c | 44 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)

Hi Viresh,

I don't see anything in this patch that is specific to virtio-mmio, as
opposed to
virtio-pci. It would be better to move this into the virtio core code so it can
be called independently of the transport that is used for virtio.

The PCI code has similar code that will set vdev->dev.parent->of_node
for a virtio-pci device, as long as that node is present.

        Arnd

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Arnd Bergmann]

On Tue, Jul 13, 2021 at 12:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:

> +
> +    virtio@3200 {
> +        compatible = "virtio,mmio";
> +        reg = <0x3200 0x100>;
> +        interrupts = <43>;
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        i2c-virtio@0 {
> +            reg = <VIRTIO_ID_I2C_ADAPTER>;
> +        };
> +    };

This works, but it seems oddly inconsistent with the way we do the same thing
for PCI, USB and MMC devices that normally don't need device tree properties but
can optionally have those.

All of the above use the "compatible" property to identify the device,
rather than
using the "reg" property. Neither of them is actually great here,
since we already
know what the device is and how to talk to it, but I'd still prefer doing this
with

       compatible = "virtio,34";

or similar, where 34 is the numerical value of VIRTIO_ID_I2C_ADAPTER.
This would then be required in the virtio-i2c binding.
I think you can skip the #address-cells/#size-cells then.

       Arnd

Re: [PATCH 5/5] dt-bindings: gpio: Add bindings for gpio-virtio

[Rob Herring]

On Tue, 13 Jul 2021 16:20:34 +0530, Viresh Kumar wrote:
> gpio-virtio represents a virtio GPIO controller and this patch adds
> binding for the same. The gpio-virtio subnode can be part of a
> virtio,mmio node and is based on its binding.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Depends on:
> 
> https://lore.kernel.org/lkml/7c716c2eb7ace5b5a560d8502af93101dbb53d24.1626170146.git.viresh.kumar@linaro.org/
> ---
>  .../devicetree/bindings/gpio/gpio-virtio.yaml | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-virtio.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:

dtschema/dtc warnings/errors:
Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/virtio/mmio.yaml'
xargs: dt-doc-validate: exited with status 255; aborting
Documentation/devicetree/bindings/gpio/gpio-virtio.example.dts:19:18: fatal error: dt-bindings/virtio/virtio_ids.h: No such file or directory
   19 |         #include <dt-bindings/virtio/virtio_ids.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/gpio/gpio-virtio.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1416: dt_binding_check] Error 2
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1504545

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

Re: [PATCH 3/5] dt-bindings: i2c: Add bindings for i2c-virtio

[Rob Herring]

On Tue, 13 Jul 2021 16:20:32 +0530, Viresh Kumar wrote:
> i2c-virtio represents a virtio I2C device and this patch adds binding
> for the same. The i2c-virtio subnode can be part of a virtio,mmio node
> and is based on its binding.
> 
> Cc: Wolfram Sang <wsa@kernel.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  .../devicetree/bindings/i2c/i2c-virtio.yaml   | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-virtio.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:

dtschema/dtc warnings/errors:
Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/virtio/mmio.yaml'
xargs: dt-doc-validate: exited with status 255; aborting
Documentation/devicetree/bindings/i2c/i2c-virtio.example.dts:19:18: fatal error: dt-bindings/virtio/virtio_ids.h: No such file or directory
   19 |         #include <dt-bindings/virtio/virtio_ids.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/i2c/i2c-virtio.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1416: dt_binding_check] Error 2
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1504542

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

Re: [PATCH 3/5] dt-bindings: i2c: Add bindings for i2c-virtio

[Rob Herring]

On Tue, 13 Jul 2021 16:20:32 +0530, Viresh Kumar wrote:
> i2c-virtio represents a virtio I2C device and this patch adds binding
> for the same. The i2c-virtio subnode can be part of a virtio,mmio node
> and is based on its binding.
> 
> Cc: Wolfram Sang <wsa@kernel.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  .../devicetree/bindings/i2c/i2c-virtio.yaml   | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-virtio.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:

dtschema/dtc warnings/errors:
Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/virtio/mmio.yaml'
xargs: dt-doc-validate: exited with status 255; aborting
Documentation/devicetree/bindings/i2c/i2c-virtio.example.dts:19:18: fatal error: dt-bindings/virtio/virtio_ids.h: No such file or directory
   19 |         #include <dt-bindings/virtio/virtio_ids.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/i2c/i2c-virtio.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1416: dt_binding_check] Error 2
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1504542

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 5/5] dt-bindings: gpio: Add bindings for gpio-virtio

[Rob Herring]

On Tue, 13 Jul 2021 16:20:34 +0530, Viresh Kumar wrote:
> gpio-virtio represents a virtio GPIO controller and this patch adds
> binding for the same. The gpio-virtio subnode can be part of a
> virtio,mmio node and is based on its binding.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Depends on:
> 
> https://lore.kernel.org/lkml/7c716c2eb7ace5b5a560d8502af93101dbb53d24.1626170146.git.viresh.kumar@linaro.org/
> ---
>  .../devicetree/bindings/gpio/gpio-virtio.yaml | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-virtio.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:

dtschema/dtc warnings/errors:
Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/virtio/mmio.yaml'
xargs: dt-doc-validate: exited with status 255; aborting
Documentation/devicetree/bindings/gpio/gpio-virtio.example.dts:19:18: fatal error: dt-bindings/virtio/virtio_ids.h: No such file or directory
   19 |         #include <dt-bindings/virtio/virtio_ids.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/gpio/gpio-virtio.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1416: dt_binding_check] Error 2
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1504545

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Rob Herring]

On Tue, Jul 13, 2021 at 4:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Allow virtio,mmio nodes to contain device specific subnodes. Since each
> virtio,mmio node can represent a single virtio device, each virtio node
> is allowed to contain a maximum of one device specific subnode.

Doesn't sound like we need 2 nodes here. Just add I2C devices as child
nodes. You could add a more specific compatible string, but the
protocol is discoverable, so that shouldn't be necessary.

BTW, what's the usecase for these protocols? A standard interface to
firmware controlled I2C, GPIO, etc.?

> The device subnode must have the "reg" property, and its value must
> match the virtio device ID used by the virtio mmio node.
>
> A phandle to this device subnode can then be used by the users of the
> virtio device.
>
> Also add a symbolic link to uapi/linux/virtio_ids.h in order to use the
> definitions here.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  .../devicetree/bindings/virtio/mmio.yaml      | 41 +++++++++++++++++++
>  include/dt-bindings/virtio/virtio_ids.h       |  1 +
>  2 files changed, 42 insertions(+)
>  create mode 120000 include/dt-bindings/virtio/virtio_ids.h
>
> diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml b/Documentation/devicetree/bindings/virtio/mmio.yaml
> index d46597028cf1..e5f9fe6ecb5e 100644
> --- a/Documentation/devicetree/bindings/virtio/mmio.yaml
> +++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
> @@ -31,6 +31,31 @@ title: virtio memory mapped devices
>      description: Required for devices making accesses thru an IOMMU.
>      maxItems: 1
>
> +  "#address-cells":
> +    const: 1
> +    description:
> +      The cell is the device ID if a device subnode is used.
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  '^[a-z0-9]+-virtio@[0-9]+$':
> +    type: object
> +    description: |
> +      Exactly one node describing the virtio device. The name of the node isn't
> +      significant but its phandle can be used to by an user of the virtio
> +      device.
> +
> +    properties:
> +      reg:
> +        description:
> +          Must contain the Virtio ID of the device.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +    required:
> +      - reg
> +
>  required:
>    - compatible
>    - reg
> @@ -57,4 +82,20 @@ additionalProperties: false
>          #iommu-cells = <1>;
>      };
>
> +  - |
> +    #include <dt-bindings/virtio/virtio_ids.h>
> +
> +    virtio@3200 {
> +        compatible = "virtio,mmio";
> +        reg = <0x3200 0x100>;
> +        interrupts = <43>;
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        i2c-virtio@0 {
> +            reg = <VIRTIO_ID_I2C_ADAPTER>;
> +        };
> +    };
> +
>  ...
> diff --git a/include/dt-bindings/virtio/virtio_ids.h b/include/dt-bindings/virtio/virtio_ids.h
> new file mode 120000
> index 000000000000..6e59ba332216
> --- /dev/null
> +++ b/include/dt-bindings/virtio/virtio_ids.h
> @@ -0,0 +1 @@
> +../../uapi/linux/virtio_ids.h

This will break the devicetree-rebasing tree I think. DT files
shouldn't reference kernel files.

Rob

Re: [PATCH 5/5] dt-bindings: gpio: Add bindings for gpio-virtio

[Rob Herring]

On Tue, Jul 13, 2021 at 4:51 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> gpio-virtio represents a virtio GPIO controller and this patch adds
> binding for the same. The gpio-virtio subnode can be part of a
> virtio,mmio node and is based on its binding.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Depends on:
>
> https://lore.kernel.org/lkml/7c716c2eb7ace5b5a560d8502af93101dbb53d24.1626170146.git.viresh.kumar@linaro.org/
> ---
>  .../devicetree/bindings/gpio/gpio-virtio.yaml | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
> new file mode 100644
> index 000000000000..c813cdfd60fd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/gpio-virtio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Virtio memory mapped GPIO controller
> +
> +maintainers:
> +  - Viresh Kumar <viresh.kumar@linaro.org>
> +
> +description:
> +  Virtio GPIO controller, see /schemas/virtio/mmio.yaml for more details.
> +
> +allOf:
> +  - $ref: /schemas/gpio/gpio.yaml#
> +
> +properties:
> +  $nodename:
> +    pattern: '^gpio-virtio@[0-9]+$'
> +
> +  reg:
> +    description:
> +      The cell is the device ID of the GPIO device (VIRTIO_ID_GPIO) as per
> +      dt-bindings/virtio/virtio_ids.h.
> +    const: 41
> +    $ref: /schemas/virtio/mmio.yaml#/properties/reg
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +required:
> +  - reg
> +  - gpio-controller
> +  - "#gpio-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/virtio/virtio_ids.h>
> +
> +    virtio@3000 {
> +        compatible = "virtio,mmio";
> +        reg = <0x3000 0x100>;
> +        interrupts = <41>;
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        gpio-virtio@0 {
> +            reg = <VIRTIO_ID_GPIO>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +            interrupt-controller;
> +            #interrupt-cells = <2>;

Similar to I2C, all this can just be added to the parent node.

Rob

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 13-07-21, 08:43, Rob Herring wrote:
> On Tue, Jul 13, 2021 at 4:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > Allow virtio,mmio nodes to contain device specific subnodes. Since each
> > virtio,mmio node can represent a single virtio device, each virtio node
> > is allowed to contain a maximum of one device specific subnode.
> 
> Doesn't sound like we need 2 nodes here. Just add I2C devices as child
> nodes. You could add a more specific compatible string, but the
> protocol is discoverable, so that shouldn't be necessary.

I am not sure if it will be a problem, but you can clarify it better.

The parent node (virtio,mmio) is used to create a platform device,
virtio-mmio, (and so assigned as its of_node) and we create the
virtio-device from probe() of this virtio-mmio device.

Is it going to be a problem if two devices in kernel use the same
of_node ? Are there cases where we would need to get the device
pointer from the of_node ? Then we will have two here.

> BTW, what's the usecase for these protocols? A standard interface to
> firmware controlled I2C, GPIO, etc.?

Right now we are looking to control devices in the host machine from
guests. That's what Linaro's project stratos is doing. There are other
people who want to use this for other kind of remote control stuff,
maybe from firmware.

> > diff --git a/include/dt-bindings/virtio/virtio_ids.h b/include/dt-bindings/virtio/virtio_ids.h
> > new file mode 120000
> > index 000000000000..6e59ba332216
> > --- /dev/null
> > +++ b/include/dt-bindings/virtio/virtio_ids.h
> > @@ -0,0 +1 @@
> > +../../uapi/linux/virtio_ids.h
> 
> This will break the devicetree-rebasing tree I think. DT files
> shouldn't reference kernel files.

We already do this for linux-event-codes.h and so I thought it is the
right way of doing it :)

Else we can create a new copy, which will be a mess, or use hardcoded
values.

-- 
viresh

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 13-07-21, 08:43, Rob Herring wrote:
> On Tue, Jul 13, 2021 at 4:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > Allow virtio,mmio nodes to contain device specific subnodes. Since each
> > virtio,mmio node can represent a single virtio device, each virtio node
> > is allowed to contain a maximum of one device specific subnode.
> 
> Doesn't sound like we need 2 nodes here. Just add I2C devices as child
> nodes. You could add a more specific compatible string, but the
> protocol is discoverable, so that shouldn't be necessary.

I am not sure if it will be a problem, but you can clarify it better.

The parent node (virtio,mmio) is used to create a platform device,
virtio-mmio, (and so assigned as its of_node) and we create the
virtio-device from probe() of this virtio-mmio device.

Is it going to be a problem if two devices in kernel use the same
of_node ? Are there cases where we would need to get the device
pointer from the of_node ? Then we will have two here.

> BTW, what's the usecase for these protocols? A standard interface to
> firmware controlled I2C, GPIO, etc.?

Right now we are looking to control devices in the host machine from
guests. That's what Linaro's project stratos is doing. There are other
people who want to use this for other kind of remote control stuff,
maybe from firmware.

> > diff --git a/include/dt-bindings/virtio/virtio_ids.h b/include/dt-bindings/virtio/virtio_ids.h
> > new file mode 120000
> > index 000000000000..6e59ba332216
> > --- /dev/null
> > +++ b/include/dt-bindings/virtio/virtio_ids.h
> > @@ -0,0 +1 @@
> > +../../uapi/linux/virtio_ids.h
> 
> This will break the devicetree-rebasing tree I think. DT files
> shouldn't reference kernel files.

We already do this for linux-event-codes.h and so I thought it is the
right way of doing it :)

Else we can create a new copy, which will be a mess, or use hardcoded
values.

-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Rob Herring]

On Tue, Jul 13, 2021 at 9:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 13-07-21, 08:43, Rob Herring wrote:
> > On Tue, Jul 13, 2021 at 4:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > Allow virtio,mmio nodes to contain device specific subnodes. Since each
> > > virtio,mmio node can represent a single virtio device, each virtio node
> > > is allowed to contain a maximum of one device specific subnode.
> >
> > Doesn't sound like we need 2 nodes here. Just add I2C devices as child
> > nodes. You could add a more specific compatible string, but the
> > protocol is discoverable, so that shouldn't be necessary.
>
> I am not sure if it will be a problem, but you can clarify it better.
>
> The parent node (virtio,mmio) is used to create a platform device,
> virtio-mmio, (and so assigned as its of_node) and we create the
> virtio-device from probe() of this virtio-mmio device.
>
> Is it going to be a problem if two devices in kernel use the same
> of_node ?

There shouldn't be. We have nodes be multiple providers (e.g clocks
and resets) already.

> Are there cases where we would need to get the device
> pointer from the of_node ? Then we will have two here.

Rarely...

In any case, should these potential kernel issues be dictating the DT
binding design? No!

>
> > BTW, what's the usecase for these protocols? A standard interface to
> > firmware controlled I2C, GPIO, etc.?
>
> Right now we are looking to control devices in the host machine from
> guests. That's what Linaro's project stratos is doing. There are other
> people who want to use this for other kind of remote control stuff,
> maybe from firmware.

Project stratos means nothing to me.

Direct userspace access to I2C, GPIO, etc. has its issues, we're going
to repeat that with guests?

> > > diff --git a/include/dt-bindings/virtio/virtio_ids.h b/include/dt-bindings/virtio/virtio_ids.h
> > > new file mode 120000
> > > index 000000000000..6e59ba332216
> > > --- /dev/null
> > > +++ b/include/dt-bindings/virtio/virtio_ids.h
> > > @@ -0,0 +1 @@
> > > +../../uapi/linux/virtio_ids.h
> >
> > This will break the devicetree-rebasing tree I think. DT files
> > shouldn't reference kernel files.
>
> We already do this for linux-event-codes.h and so I thought it is the
> right way of doing it :)

Humm, maybe it's okay. Please double check then...

> Else we can create a new copy, which will be a mess, or use hardcoded
> values.

Though you may not need the header based on what Arnd and I have suggested.

Rob

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Arnd Bergmann]

On Tue, Jul 13, 2021 at 9:35 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Tue, Jul 13, 2021 at 9:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 13-07-21, 08:43, Rob Herring wrote:
> > > On Tue, Jul 13, 2021 at 4:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > >
> > > > Allow virtio,mmio nodes to contain device specific subnodes. Since each
> > > > virtio,mmio node can represent a single virtio device, each virtio node
> > > > is allowed to contain a maximum of one device specific subnode.
> > >
> > > Doesn't sound like we need 2 nodes here. Just add I2C devices as child
> > > nodes. You could add a more specific compatible string, but the
> > > protocol is discoverable, so that shouldn't be necessary.
> >
> > I am not sure if it will be a problem, but you can clarify it better.
>
> > The parent node (virtio,mmio) is used to create a platform device,
> > virtio-mmio, (and so assigned as its of_node) and we create the
> > virtio-device from probe() of this virtio-mmio device.
> >
> > Is it going to be a problem if two devices in kernel use the same
> > of_node ?
>
> There shouldn't be. We have nodes be multiple providers (e.g clocks
> and resets) already.

I think this would be a little different, but it can still work. There is in
fact already some precedent of doing this, with Jean-Philippe's virtio-iommu
binding, which is documented in both

Documentation/devicetree/bindings/virtio/iommu.txt
Documentation/devicetree/bindings/virtio/mmio.txt

Unfortunately, those are still slightly different from where I think we should
be going here, but it's probably close enough to fit into the general
system.

What we have with virtio-iommu is two special hacks:
 - on virtio-mmio, a node with 'compatible="virtio,mmio"' may optionally
   have an '#iommu-cells=<1>', in which case we assume it's an iommu.
 - for virtio-pci, the node has the standard PCI 'reg' property but a special
   'compatible="virtio,pci-iommu"' property that I think is different from any
   other PCI node.

I think for other virtio devices, we should come up with a way to define a
binding per device (i2c, gpio, ...) without needing to cram this into the
"virtio,mmio" binding or coming up with special compatible strings for
PCI devices.

Having a child device for the virtio device type gives a better separation
here, since it lets you have two nodes with 'compatible' strings that each
make sense for their respective parent buses: The parent is either a PCI
device or a plain mmio based device, and the child is a virtio device with
its own namespace for compatible values. As you say, the downside is
that this requires an extra node that is redundant because there is always
a 1:1 relation with its parent.

Having a combined node gets rid of the redundancy but if we want to
identify the device for the purpose of defining a custom binding, it would have
to have two compatible strings, something like

compatible="virtio,mmio", "virtio,device34";

for a virtio-mmio device of device-id 34 (i2c), or a PCI device with

compatible="pci1af4,1041", "virtio,device34";

which also does not quite feel right.

>> On Tue, Jul 13, 2021 at 9:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 13-07-21, 08:43, Rob Herring wrote:
> > > On Tue, Jul 13, 2021 at 4:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > > BTW, what's the usecase for these protocols? A standard interface to
> > > firmware controlled I2C, GPIO, etc.?
> >
> > Right now we are looking to control devices in the host machine from
> > guests. That's what Linaro's project stratos is doing. There are other
> > people who want to use this for other kind of remote control stuff,
> > maybe from firmware.
>
> Project stratos means nothing to me.
>
> Direct userspace access to I2C, GPIO, etc. has its issues, we're going
> to repeat that with guests?

Passing through the i2c or gpio access from a Linux host is just one
way to use it, you could do the same with an emulated i2c device
from qemu, and you could have a fake i2c device behind a virtio-i2c
controller.

         Arnd

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 13-07-21, 13:34, Rob Herring wrote:
> On Tue, Jul 13, 2021 at 9:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > We already do this for linux-event-codes.h and so I thought it is the
> > right way of doing it :)
> 
> Humm, maybe it's okay. Please double check then...
> 
> > Else we can create a new copy, which will be a mess, or use hardcoded
> > values.
> 
> Though you may not need the header based on what Arnd and I have suggested.

Yeah, we may not need it at all. New node or not, reg property will
get converted to a compatible anyway..

Thanks.

-- 
viresh

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 13-07-21, 13:34, Rob Herring wrote:
> On Tue, Jul 13, 2021 at 9:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > We already do this for linux-event-codes.h and so I thought it is the
> > right way of doing it :)
> 
> Humm, maybe it's okay. Please double check then...
> 
> > Else we can create a new copy, which will be a mess, or use hardcoded
> > values.
> 
> Though you may not need the header based on what Arnd and I have suggested.

Yeah, we may not need it at all. New node or not, reg property will
get converted to a compatible anyway..

Thanks.

-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 13-07-21, 22:34, Arnd Bergmann wrote:
> On Tue, Jul 13, 2021 at 9:35 PM Rob Herring <robh+dt@kernel.org> wrote:
> > There shouldn't be. We have nodes be multiple providers (e.g clocks
> > and resets) already.
> 
> I think this would be a little different, but it can still work. There is in
> fact already some precedent of doing this, with Jean-Philippe's virtio-iommu
> binding, which is documented in both
> 
> Documentation/devicetree/bindings/virtio/iommu.txt
> Documentation/devicetree/bindings/virtio/mmio.txt
> 
> Unfortunately, those are still slightly different from where I think we should
> be going here, but it's probably close enough to fit into the general
> system.
> 
> What we have with virtio-iommu is two special hacks:
>  - on virtio-mmio, a node with 'compatible="virtio,mmio"' may optionally
>    have an '#iommu-cells=<1>', in which case we assume it's an iommu.
>  - for virtio-pci, the node has the standard PCI 'reg' property but a special
>    'compatible="virtio,pci-iommu"' property that I think is different from any
>    other PCI node.
> 
> I think for other virtio devices, we should come up with a way to define a
> binding per device (i2c, gpio, ...) without needing to cram this into the
> "virtio,mmio" binding or coming up with special compatible strings for
> PCI devices.
> 
> Having a child device for the virtio device type gives a better separation
> here, since it lets you have two nodes with 'compatible' strings that each
> make sense for their respective parent buses: The parent is either a PCI
> device or a plain mmio based device, and the child is a virtio device with
> its own namespace for compatible values. As you say, the downside is
> that this requires an extra node that is redundant because there is always
> a 1:1 relation with its parent.
> 
> Having a combined node gets rid of the redundancy but if we want to
> identify the device for the purpose of defining a custom binding, it would have
> to have two compatible strings, something like
> 
> compatible="virtio,mmio", "virtio,device34";
> 
> for a virtio-mmio device of device-id 34 (i2c), or a PCI device with
> 
> compatible="pci1af4,1041", "virtio,device34";
> 
> which also does not quite feel right.

I agree that even if the device is discoverable at runtime, we should
still have some sort of stuff in DT to distinguish the devices, and
"virtio,deviceDID" sounds good enough for that, considering that we
already do it for USB, etc.

And I am fine with both the ways, a new node or just using the parent
node. So whatever you guys decide is fine.

> > Direct userspace access to I2C, GPIO, etc. has its issues, we're going
> > to repeat that with guests?
> 
> Passing through the i2c or gpio access from a Linux host is just one
> way to use it, you could do the same with an emulated i2c device
> from qemu, and you could have a fake i2c device behind a virtio-i2c
> controller.

Or it can be firmware controlled device as well, as Rob said earlier.
I think that's what Vincent will be doing for SCMI.

Though all I have tested until now is direct userspace access.

-- 
viresh

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 13-07-21, 22:34, Arnd Bergmann wrote:
> On Tue, Jul 13, 2021 at 9:35 PM Rob Herring <robh+dt@kernel.org> wrote:
> > There shouldn't be. We have nodes be multiple providers (e.g clocks
> > and resets) already.
> 
> I think this would be a little different, but it can still work. There is in
> fact already some precedent of doing this, with Jean-Philippe's virtio-iommu
> binding, which is documented in both
> 
> Documentation/devicetree/bindings/virtio/iommu.txt
> Documentation/devicetree/bindings/virtio/mmio.txt
> 
> Unfortunately, those are still slightly different from where I think we should
> be going here, but it's probably close enough to fit into the general
> system.
> 
> What we have with virtio-iommu is two special hacks:
>  - on virtio-mmio, a node with 'compatible="virtio,mmio"' may optionally
>    have an '#iommu-cells=<1>', in which case we assume it's an iommu.
>  - for virtio-pci, the node has the standard PCI 'reg' property but a special
>    'compatible="virtio,pci-iommu"' property that I think is different from any
>    other PCI node.
> 
> I think for other virtio devices, we should come up with a way to define a
> binding per device (i2c, gpio, ...) without needing to cram this into the
> "virtio,mmio" binding or coming up with special compatible strings for
> PCI devices.
> 
> Having a child device for the virtio device type gives a better separation
> here, since it lets you have two nodes with 'compatible' strings that each
> make sense for their respective parent buses: The parent is either a PCI
> device or a plain mmio based device, and the child is a virtio device with
> its own namespace for compatible values. As you say, the downside is
> that this requires an extra node that is redundant because there is always
> a 1:1 relation with its parent.
> 
> Having a combined node gets rid of the redundancy but if we want to
> identify the device for the purpose of defining a custom binding, it would have
> to have two compatible strings, something like
> 
> compatible="virtio,mmio", "virtio,device34";
> 
> for a virtio-mmio device of device-id 34 (i2c), or a PCI device with
> 
> compatible="pci1af4,1041", "virtio,device34";
> 
> which also does not quite feel right.

I agree that even if the device is discoverable at runtime, we should
still have some sort of stuff in DT to distinguish the devices, and
"virtio,deviceDID" sounds good enough for that, considering that we
already do it for USB, etc.

And I am fine with both the ways, a new node or just using the parent
node. So whatever you guys decide is fine.

> > Direct userspace access to I2C, GPIO, etc. has its issues, we're going
> > to repeat that with guests?
> 
> Passing through the i2c or gpio access from a Linux host is just one
> way to use it, you could do the same with an emulated i2c device
> from qemu, and you could have a fake i2c device behind a virtio-i2c
> controller.

Or it can be firmware controlled device as well, as Rob said earlier.
I think that's what Vincent will be doing for SCMI.

Though all I have tested until now is direct userspace access.

-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 13-07-21, 14:32, Arnd Bergmann wrote:
> On Tue, Jul 13, 2021 at 12:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
> > +
> > +    virtio@3200 {
> > +        compatible = "virtio,mmio";
> > +        reg = <0x3200 0x100>;
> > +        interrupts = <43>;
> > +
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        i2c-virtio@0 {
> > +            reg = <VIRTIO_ID_I2C_ADAPTER>;
> > +        };
> > +    };
> 
> This works, but it seems oddly inconsistent with the way we do the same thing
> for PCI, USB and MMC devices that normally don't need device tree properties but
> can optionally have those.
> 
> All of the above use the "compatible" property to identify the device,
> rather than
> using the "reg" property. Neither of them is actually great here,
> since we already
> know what the device is and how to talk to it, but I'd still prefer doing this
> with
> 
>        compatible = "virtio,34";
> 
> or similar, where 34 is the numerical value of VIRTIO_ID_I2C_ADAPTER.
> This would then be required in the virtio-i2c binding.
> I think you can skip the #address-cells/#size-cells then.

That works, sure.

I think I misunderstood it when you said it earlier and thought that
you are asking to add compatible in the parent node itself and so did
it this way.

Though that may be the way we will end up doing it now :)

-- 
viresh

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 13-07-21, 14:32, Arnd Bergmann wrote:
> On Tue, Jul 13, 2021 at 12:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
> > +
> > +    virtio@3200 {
> > +        compatible = "virtio,mmio";
> > +        reg = <0x3200 0x100>;
> > +        interrupts = <43>;
> > +
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        i2c-virtio@0 {
> > +            reg = <VIRTIO_ID_I2C_ADAPTER>;
> > +        };
> > +    };
> 
> This works, but it seems oddly inconsistent with the way we do the same thing
> for PCI, USB and MMC devices that normally don't need device tree properties but
> can optionally have those.
> 
> All of the above use the "compatible" property to identify the device,
> rather than
> using the "reg" property. Neither of them is actually great here,
> since we already
> know what the device is and how to talk to it, but I'd still prefer doing this
> with
> 
>        compatible = "virtio,34";
> 
> or similar, where 34 is the numerical value of VIRTIO_ID_I2C_ADAPTER.
> This would then be required in the virtio-i2c binding.
> I think you can skip the #address-cells/#size-cells then.

That works, sure.

I think I misunderstood it when you said it earlier and thought that
you are asking to add compatible in the parent node itself and so did
it this way.

Though that may be the way we will end up doing it now :)

-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/5] virtio_mmio: Bind virtio device to device-tree node

[Viresh Kumar]

On 13-07-21, 14:26, Arnd Bergmann wrote:
> On Tue, Jul 13, 2021 at 12:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > Bind the virtio device with its device protocol's sub-node. This will
> > help users of the virtio device to mention their dependencies on the
> > device in the DT file itself. Like GPIO pin users can use the phandle of
> > the device node, or the node may contain more subnodes to add i2c or spi
> > eeproms and other users.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  drivers/virtio/virtio_mmio.c | 44 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> 
> Hi Viresh,
> 
> I don't see anything in this patch that is specific to virtio-mmio, as
> opposed to
> virtio-pci. It would be better to move this into the virtio core code so it can
> be called independently of the transport that is used for virtio.
> 
> The PCI code has similar code that will set vdev->dev.parent->of_node
> for a virtio-pci device, as long as that node is present.

Sure.
-- 
viresh

Re: [PATCH 2/5] virtio_mmio: Bind virtio device to device-tree node

[Viresh Kumar]

On 13-07-21, 14:26, Arnd Bergmann wrote:
> On Tue, Jul 13, 2021 at 12:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > Bind the virtio device with its device protocol's sub-node. This will
> > help users of the virtio device to mention their dependencies on the
> > device in the DT file itself. Like GPIO pin users can use the phandle of
> > the device node, or the node may contain more subnodes to add i2c or spi
> > eeproms and other users.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  drivers/virtio/virtio_mmio.c | 44 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> 
> Hi Viresh,
> 
> I don't see anything in this patch that is specific to virtio-mmio, as
> opposed to
> virtio-pci. It would be better to move this into the virtio core code so it can
> be called independently of the transport that is used for virtio.
> 
> The PCI code has similar code that will set vdev->dev.parent->of_node
> for a virtio-pci device, as long as that node is present.

Sure.
-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Jean-Philippe Brucker]

On Tue, Jul 13, 2021 at 10:34:03PM +0200, Arnd Bergmann wrote:
> > > Is it going to be a problem if two devices in kernel use the same
> > > of_node ?
> >
> > There shouldn't be. We have nodes be multiple providers (e.g clocks
> > and resets) already.
> 
> I think this would be a little different, but it can still work. There is in
> fact already some precedent of doing this, with Jean-Philippe's virtio-iommu
> binding, which is documented in both
> 
> Documentation/devicetree/bindings/virtio/iommu.txt
> Documentation/devicetree/bindings/virtio/mmio.txt
> 
> Unfortunately, those are still slightly different from where I think we should
> be going here, but it's probably close enough to fit into the general
> system.
> 
> What we have with virtio-iommu is two special hacks:
>  - on virtio-mmio, a node with 'compatible="virtio,mmio"' may optionally
>    have an '#iommu-cells=<1>', in which case we assume it's an iommu.
>  - for virtio-pci, the node has the standard PCI 'reg' property but a special
>    'compatible="virtio,pci-iommu"' property that I think is different from any
>    other PCI node.

Yes in retrospect I don't think the compatible property on the PCI
endpoint node is necessary nor useful, we could deprecate it. The OS gets
the IOMMU topology information early from 'iommus', 'iommu-map' and
'#iommu-cells' properties, which is the only reason we need this PCI
endpoint explicitly described in DT. The rest is discovered while probing
just like virtio-mmio.

Thanks,
Jean

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Jean-Philippe Brucker]

On Tue, Jul 13, 2021 at 10:34:03PM +0200, Arnd Bergmann wrote:
> > > Is it going to be a problem if two devices in kernel use the same
> > > of_node ?
> >
> > There shouldn't be. We have nodes be multiple providers (e.g clocks
> > and resets) already.
> 
> I think this would be a little different, but it can still work. There is in
> fact already some precedent of doing this, with Jean-Philippe's virtio-iommu
> binding, which is documented in both
> 
> Documentation/devicetree/bindings/virtio/iommu.txt
> Documentation/devicetree/bindings/virtio/mmio.txt
> 
> Unfortunately, those are still slightly different from where I think we should
> be going here, but it's probably close enough to fit into the general
> system.
> 
> What we have with virtio-iommu is two special hacks:
>  - on virtio-mmio, a node with 'compatible="virtio,mmio"' may optionally
>    have an '#iommu-cells=<1>', in which case we assume it's an iommu.
>  - for virtio-pci, the node has the standard PCI 'reg' property but a special
>    'compatible="virtio,pci-iommu"' property that I think is different from any
>    other PCI node.

Yes in retrospect I don't think the compatible property on the PCI
endpoint node is necessary nor useful, we could deprecate it. The OS gets
the IOMMU topology information early from 'iommus', 'iommu-map' and
'#iommu-cells' properties, which is the only reason we need this PCI
endpoint explicitly described in DT. The rest is discovered while probing
just like virtio-mmio.

Thanks,
Jean
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 14-07-21, 07:56, Viresh Kumar wrote:
> I agree that even if the device is discoverable at runtime, we should
> still have some sort of stuff in DT to distinguish the devices, and
> "virtio,deviceDID" sounds good enough for that, considering that we
> already do it for USB, etc.
> 
> And I am fine with both the ways, a new node or just using the parent
> node. So whatever you guys decide is fine.

I tried to write and see what it would look like after using the
existing nodes for mmio/pci and here is what I got.  (I couldn't find
any virtio-pci bindings and so stayed away from adding any reference
to it here).

Does that look better ?

-- 
viresh

-------------------------8<-------------------------
diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml b/Documentation/devicetree/bindings/virtio/mmio.yaml
index d46597028cf1..324b810e51a5 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.yaml
+++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
@@ -15,7 +15,8 @@ title: virtio memory mapped devices
 
 properties:
   compatible:
-    const: virtio,mmio
+    contains:
+      const: virtio,mmio
 
   reg:
     maxItems: 1
@@ -36,7 +37,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..9cfe090ea65f
--- /dev/null
+++ b/Documentation/devicetree/bindings/virtio/virtio-device.yaml
@@ -0,0 +1,49 @@
+# 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 <viresh.kumar@linaro.org>
+
+description:
+  These bindings are applicable to virtio devices irrespective of the bus they
+  are bound to, like mmio or pci.
+
+allOf:
+  - $ref: /schemas/virtio/mmio.yaml#
+
+# We need a select here so we don't match all nodes with 'virtio,mmio'
+select:
+  properties:
+    compatible:
+      contains:
+        pattern: '^virtio,[0-9]+$'
+  required:
+    - compatible
+
+properties:
+  compatible:
+    contains:
+      oneOf:
+        - items:
+          - const: virtio,mmio
+          - pattern: '^virtio,[0-9]+$'
+
+required:
+  - compatible
+
+additionalProperties: true
+
+examples:
+  - |
+    i2c: i2c-virtio@3000 {
+        compatible = "virtio,mmio", "virtio,34";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
new file mode 100644
index 000000000000..8115ba794557
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-virtio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio GPIO controller
+
+maintainers:
+  - Viresh Kumar <viresh.kumar@linaro.org>
+
+allOf:
+  - $ref: /schemas/gpio/gpio.yaml#
+  - $ref: /schemas/virtio/virtio-device.yaml#
+
+# We need a select here so we don't match all nodes with 'virtio,mmio'
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - virtio,41
+  required:
+    - compatible
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - const: virtio,mmio
+        - const: virtio,41
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - gpio-controller
+  - "#gpio-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    gpio: gpio-virtio@3000 {
+        compatible = "virtio,mmio", "virtio,41";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+
+        gpio-controller;
+        #gpio-cells = <2>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml b/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
new file mode 100644
index 000000000000..43e9910920d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/i2c-virtio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio I2C Adapter
+
+maintainers:
+  - Viresh Kumar <viresh.kumar@linaro.org>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+  - $ref: /schemas/virtio/virtio-device.yaml#
+
+# We need a select here so we don't match all nodes with 'virtio,mmio'
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - virtio,34
+  required:
+    - compatible
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - const: virtio,mmio
+        - const: virtio,34
+
+required:
+  - compatible
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c: i2c-virtio@3000 {
+        compatible = "virtio,mmio", "virtio,34";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@1c {
+            compatible = "dynaimage,al3320a";
+            reg = <0x20>;
+        };
+    };
+
+...

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 14-07-21, 07:56, Viresh Kumar wrote:
> I agree that even if the device is discoverable at runtime, we should
> still have some sort of stuff in DT to distinguish the devices, and
> "virtio,deviceDID" sounds good enough for that, considering that we
> already do it for USB, etc.
> 
> And I am fine with both the ways, a new node or just using the parent
> node. So whatever you guys decide is fine.

I tried to write and see what it would look like after using the
existing nodes for mmio/pci and here is what I got.  (I couldn't find
any virtio-pci bindings and so stayed away from adding any reference
to it here).

Does that look better ?

-- 
viresh

-------------------------8<-------------------------
diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml b/Documentation/devicetree/bindings/virtio/mmio.yaml
index d46597028cf1..324b810e51a5 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.yaml
+++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
@@ -15,7 +15,8 @@ title: virtio memory mapped devices
 
 properties:
   compatible:
-    const: virtio,mmio
+    contains:
+      const: virtio,mmio
 
   reg:
     maxItems: 1
@@ -36,7 +37,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..9cfe090ea65f
--- /dev/null
+++ b/Documentation/devicetree/bindings/virtio/virtio-device.yaml
@@ -0,0 +1,49 @@
+# 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 <viresh.kumar@linaro.org>
+
+description:
+  These bindings are applicable to virtio devices irrespective of the bus they
+  are bound to, like mmio or pci.
+
+allOf:
+  - $ref: /schemas/virtio/mmio.yaml#
+
+# We need a select here so we don't match all nodes with 'virtio,mmio'
+select:
+  properties:
+    compatible:
+      contains:
+        pattern: '^virtio,[0-9]+$'
+  required:
+    - compatible
+
+properties:
+  compatible:
+    contains:
+      oneOf:
+        - items:
+          - const: virtio,mmio
+          - pattern: '^virtio,[0-9]+$'
+
+required:
+  - compatible
+
+additionalProperties: true
+
+examples:
+  - |
+    i2c: i2c-virtio@3000 {
+        compatible = "virtio,mmio", "virtio,34";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
new file mode 100644
index 000000000000..8115ba794557
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-virtio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio GPIO controller
+
+maintainers:
+  - Viresh Kumar <viresh.kumar@linaro.org>
+
+allOf:
+  - $ref: /schemas/gpio/gpio.yaml#
+  - $ref: /schemas/virtio/virtio-device.yaml#
+
+# We need a select here so we don't match all nodes with 'virtio,mmio'
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - virtio,41
+  required:
+    - compatible
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - const: virtio,mmio
+        - const: virtio,41
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - gpio-controller
+  - "#gpio-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    gpio: gpio-virtio@3000 {
+        compatible = "virtio,mmio", "virtio,41";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+
+        gpio-controller;
+        #gpio-cells = <2>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml b/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
new file mode 100644
index 000000000000..43e9910920d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/i2c-virtio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio I2C Adapter
+
+maintainers:
+  - Viresh Kumar <viresh.kumar@linaro.org>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+  - $ref: /schemas/virtio/virtio-device.yaml#
+
+# We need a select here so we don't match all nodes with 'virtio,mmio'
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - virtio,34
+  required:
+    - compatible
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - const: virtio,mmio
+        - const: virtio,34
+
+required:
+  - compatible
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c: i2c-virtio@3000 {
+        compatible = "virtio,mmio", "virtio,34";
+        reg = <0x3000 0x100>;
+        interrupts = <41>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@1c {
+            compatible = "dynaimage,al3320a";
+            reg = <0x20>;
+        };
+    };
+
+...
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Rob Herring]

On Tue, Jul 13, 2021 at 2:34 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Tue, Jul 13, 2021 at 9:35 PM Rob Herring <robh+dt@kernel.org> wrote:
> > On Tue, Jul 13, 2021 at 9:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > On 13-07-21, 08:43, Rob Herring wrote:
> > > > On Tue, Jul 13, 2021 at 4:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > > >
> > > > > Allow virtio,mmio nodes to contain device specific subnodes. Since each
> > > > > virtio,mmio node can represent a single virtio device, each virtio node
> > > > > is allowed to contain a maximum of one device specific subnode.
> > > >
> > > > Doesn't sound like we need 2 nodes here. Just add I2C devices as child
> > > > nodes. You could add a more specific compatible string, but the
> > > > protocol is discoverable, so that shouldn't be necessary.
> > >
> > > I am not sure if it will be a problem, but you can clarify it better.
> >
> > > The parent node (virtio,mmio) is used to create a platform device,
> > > virtio-mmio, (and so assigned as its of_node) and we create the
> > > virtio-device from probe() of this virtio-mmio device.
> > >
> > > Is it going to be a problem if two devices in kernel use the same
> > > of_node ?
> >
> > There shouldn't be. We have nodes be multiple providers (e.g clocks
> > and resets) already.
>
> I think this would be a little different, but it can still work. There is in
> fact already some precedent of doing this, with Jean-Philippe's virtio-iommu
> binding, which is documented in both
>
> Documentation/devicetree/bindings/virtio/iommu.txt
> Documentation/devicetree/bindings/virtio/mmio.txt
>
> Unfortunately, those are still slightly different from where I think we should
> be going here, but it's probably close enough to fit into the general
> system.
>
> What we have with virtio-iommu is two special hacks:
>  - on virtio-mmio, a node with 'compatible="virtio,mmio"' may optionally
>    have an '#iommu-cells=<1>', in which case we assume it's an iommu.
>  - for virtio-pci, the node has the standard PCI 'reg' property but a special
>    'compatible="virtio,pci-iommu"' property that I think is different from any
>    other PCI node.

How is that different? PCI device can be a VID/PID compatible or
omitted, but can also be a "typical" compatible string.

> I think for other virtio devices, we should come up with a way to define a
> binding per device (i2c, gpio, ...) without needing to cram this into the
> "virtio,mmio" binding or coming up with special compatible strings for
> PCI devices.
>
> Having a child device for the virtio device type gives a better separation
> here, since it lets you have two nodes with 'compatible' strings that each
> make sense for their respective parent buses: The parent is either a PCI
> device or a plain mmio based device, and the child is a virtio device with
> its own namespace for compatible values. As you say, the downside is
> that this requires an extra node that is redundant because there is always
> a 1:1 relation with its parent.
>
> Having a combined node gets rid of the redundancy but if we want to
> identify the device for the purpose of defining a custom binding, it would have
> to have two compatible strings, something like
>
> compatible="virtio,mmio", "virtio,device34";

The order seems backwards here. 'virtio,device34' is more specific.
Though I guess the meanings are orthogonal.

> for a virtio-mmio device of device-id 34 (i2c), or a PCI device with
>
> compatible="pci1af4,1041", "virtio,device34";

But this seems the right order. Though does '1041' have any specific
meaning or device IDs are just dynamically assigned? It seems to be
the latter from my brief scan of the code.

> which also does not quite feel right.

I guess it comes down to is 'virtio,mmio' providing a bus or is it
just a device? I guess a bus (so 2 nodes) does make sense here.
'virtio,mmio' defines how you access/discover the virtio queues (the
bus) and the functional device (i2c, gpio, iommu, etc.) is accessed
via the virtio queues.

Rob

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Arnd Bergmann]

On Wed, Jul 14, 2021 at 5:43 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Tue, Jul 13, 2021 at 2:34 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > On Tue, Jul 13, 2021 at 9:35 PM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > What we have with virtio-iommu is two special hacks:
> >  - on virtio-mmio, a node with 'compatible="virtio,mmio"' may optionally
> >    have an '#iommu-cells=<1>', in which case we assume it's an iommu.
> >  - for virtio-pci, the node has the standard PCI 'reg' property but a special
> >    'compatible="virtio,pci-iommu"' property that I think is different from any
> >    other PCI node.
>
> How is that different? PCI device can be a VID/PID compatible or
> omitted, but can also be a "typical" compatible string.

Ok, my mistake then, I though the VID/PID compatible was mandated

> > I think for other virtio devices, we should come up with a way to define a
> > binding per device (i2c, gpio, ...) without needing to cram this into the
> > "virtio,mmio" binding or coming up with special compatible strings for
> > PCI devices.
> >
> > Having a child device for the virtio device type gives a better separation
> > here, since it lets you have two nodes with 'compatible' strings that each
> > make sense for their respective parent buses: The parent is either a PCI
> > device or a plain mmio based device, and the child is a virtio device with
> > its own namespace for compatible values. As you say, the downside is
> > that this requires an extra node that is redundant because there is always
> > a 1:1 relation with its parent.
> >
> > Having a combined node gets rid of the redundancy but if we want to
> > identify the device for the purpose of defining a custom binding, it would have
> > to have two compatible strings, something like
> >
> > compatible="virtio,mmio", "virtio,device34";
>
> The order seems backwards here. 'virtio,device34' is more specific.
> Though I guess the meanings are orthogonal.

Right, one defines the transport and the other defines the device behind
the transport.

> > for a virtio-mmio device of device-id 34 (i2c), or a PCI device with
> >
> > compatible="pci1af4,1041", "virtio,device34";
>
> But this seems the right order. Though does '1041' have any specific
> meaning or device IDs are just dynamically assigned? It seems to be
> the latter from my brief scan of the code.

It's the assigned PCI vendor/device pair for virtio, all virtio-pci devices
have to be "pci1af4,1041", just like all virtio-mmio devices are
"virtio,mmio".

> > which also does not quite feel right.
>
> I guess it comes down to is 'virtio,mmio' providing a bus or is it
> just a device? I guess a bus (so 2 nodes) does make sense here.
> 'virtio,mmio' defines how you access/discover the virtio queues (the
> bus) and the functional device (i2c, gpio, iommu, etc.) is accessed
> via the virtio queues.

It's not really a bus since there is only ever one device behind it.
A better analogy would be your 'serdev' framework: You could
have a 8250 or a pl011 uart, and behind that have a mouse, GPS
receiver or bluetooth dongle.

In Documentation/devicetree/bindings/serial/serial.yaml, you also
have two nodes for a single device, so we could follow that
example.

        Arnd

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 14-07-21, 23:07, Arnd Bergmann wrote:
> On Wed, Jul 14, 2021 at 5:43 PM Rob Herring <robh+dt@kernel.org> wrote:
> > I guess it comes down to is 'virtio,mmio' providing a bus or is it
> > just a device? I guess a bus (so 2 nodes) does make sense here.
> > 'virtio,mmio' defines how you access/discover the virtio queues (the
> > bus) and the functional device (i2c, gpio, iommu, etc.) is accessed
> > via the virtio queues.
> 
> It's not really a bus since there is only ever one device behind it.
> A better analogy would be your 'serdev' framework: You could
> have a 8250 or a pl011 uart, and behind that have a mouse, GPS
> receiver or bluetooth dongle.
> 
> In Documentation/devicetree/bindings/serial/serial.yaml, you also
> have two nodes for a single device, so we could follow that
> example.

So two device nodes is final then ? Pretty much like how this patchset did it
already ? I need to get rid of reg thing and use "virtio,DID" though.

-- 
viresh

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Viresh Kumar]

On 14-07-21, 23:07, Arnd Bergmann wrote:
> On Wed, Jul 14, 2021 at 5:43 PM Rob Herring <robh+dt@kernel.org> wrote:
> > I guess it comes down to is 'virtio,mmio' providing a bus or is it
> > just a device? I guess a bus (so 2 nodes) does make sense here.
> > 'virtio,mmio' defines how you access/discover the virtio queues (the
> > bus) and the functional device (i2c, gpio, iommu, etc.) is accessed
> > via the virtio queues.
> 
> It's not really a bus since there is only ever one device behind it.
> A better analogy would be your 'serdev' framework: You could
> have a 8250 or a pl011 uart, and behind that have a mouse, GPS
> receiver or bluetooth dongle.
> 
> In Documentation/devicetree/bindings/serial/serial.yaml, you also
> have two nodes for a single device, so we could follow that
> example.

So two device nodes is final then ? Pretty much like how this patchset did it
already ? I need to get rid of reg thing and use "virtio,DID" though.

-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode

[Arnd Bergmann]

On Mon, Jul 19, 2021 at 12:34 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 14-07-21, 23:07, Arnd Bergmann wrote:
> > On Wed, Jul 14, 2021 at 5:43 PM Rob Herring <robh+dt@kernel.org> wrote:
> > > I guess it comes down to is 'virtio,mmio' providing a bus or is it
> > > just a device? I guess a bus (so 2 nodes) does make sense here.
> > > 'virtio,mmio' defines how you access/discover the virtio queues (the
> > > bus) and the functional device (i2c, gpio, iommu, etc.) is accessed
> > > via the virtio queues.
> >
> > It's not really a bus since there is only ever one device behind it.
> > A better analogy would be your 'serdev' framework: You could
> > have a 8250 or a pl011 uart, and behind that have a mouse, GPS
> > receiver or bluetooth dongle.
> >
> > In Documentation/devicetree/bindings/serial/serial.yaml, you also
> > have two nodes for a single device, so we could follow that
> > example.
>
> So two device nodes is final then ? Pretty much like how this patchset did it
> already ? I need to get rid of reg thing and use "virtio,DID" though.

Let's give Rob another day to reply here. I think two nodes is easier
to get working than one node, even when we continue supporting the
current iommu binding that relies on a single node, but we could make
it work either way if necessary.

       Arnd