* [PATCH V2 0/5] virtio: Add virtio-device bindings
@ 2021-07-22 9:56 Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 1/5] dt-bindings: virtio: Add binding for virtio devices Viresh Kumar
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Viresh Kumar @ 2021-07-22 9:56 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin, Rob Herring, Arnd Bergmann,
Jean-Philippe Brucker, Bartosz Golaszewski, Linus Walleij,
Viresh Kumar
Cc: Vincent Guittot, Bill Mills, Alex Bennée, Enrico Weigelt,
metux IT consult, Jie Deng, devicetree, linux-kernel,
virtualization, linux-gpio, linux-i2c, Wolfram Sang
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";
};
Here, 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 adds virtio-device bindings to allow for device sub-nodes to be
present and the second patch updates the virtio core to update the of_node.
Other patches add bindings for i2c and gpio devices.
Tested on x86 with qemu for arm64.
V1->V2:
- The changes (both binding and code) are made at virtio level, instead of
virtio-mmio. This allows the same to be used by all device types, irrespective
of the transport mechanism.
- Dropped the reg property and used compatible in the form "virtio,<DID>".
- Dropped dt-bindings/virtio/virtio_ids.h.
- Add a patch to sync virtio-ids from spec, required for the last patch.
--
Viresh
Viresh Kumar (5):
dt-bindings: virtio: Add binding for virtio devices
dt-bindings: i2c: Add bindings for i2c-virtio
dt-bindings: gpio: Add bindings for gpio-virtio
uapi: virtio_ids: Sync ids with specification
virtio: Bind virtio device to device-tree node
.../devicetree/bindings/gpio/gpio-virtio.yaml | 60 +++++++++++++++++
.../devicetree/bindings/i2c/i2c-virtio.yaml | 51 ++++++++++++++
.../devicetree/bindings/virtio/mmio.yaml | 2 +-
.../bindings/virtio/virtio-device.yaml | 47 +++++++++++++
drivers/virtio/virtio.c | 67 ++++++++++++++++++-
include/uapi/linux/virtio_ids.h | 12 ++++
6 files changed, 235 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
create mode 100644 Documentation/devicetree/bindings/virtio/virtio-device.yaml
--
2.31.1.272.g89b43f80a514
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V2 1/5] dt-bindings: virtio: Add binding for virtio devices
2021-07-22 9:56 [PATCH V2 0/5] virtio: Add virtio-device bindings Viresh Kumar
@ 2021-07-22 9:56 ` Viresh Kumar
2021-07-22 10:28 ` Arnd Bergmann
2021-07-22 9:56 ` [PATCH V2 2/5] dt-bindings: i2c: Add bindings for i2c-virtio Viresh Kumar
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2021-07-22 9:56 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin, Rob Herring, Arnd Bergmann,
Jean-Philippe Brucker, Viresh Kumar
Cc: Vincent Guittot, Bill Mills, Alex Bennée, Enrico Weigelt,
metux IT consult, Jie Deng, devicetree, linux-kernel,
virtualization
Allow virtio device sub-nodes to be added to the virtio mmio or pci
nodes. The compatible property for virtio device must be of format
"virtio,<DID>", where DID is virtio device ID in hexadecimal format.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
.../devicetree/bindings/virtio/mmio.yaml | 2 +-
.../bindings/virtio/virtio-device.yaml | 47 +++++++++++++++++++
2 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/virtio/virtio-device.yaml
diff --git a/Documentation/devicetree/bindings/virtio/mmio.yaml b/Documentation/devicetree/bindings/virtio/mmio.yaml
index d46597028cf1..1b91553f87c6 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.yaml
+++ b/Documentation/devicetree/bindings/virtio/mmio.yaml
@@ -36,7 +36,7 @@ title: virtio memory mapped devices
- reg
- interrupts
-additionalProperties: false
+additionalProperties: true
examples:
- |
diff --git a/Documentation/devicetree/bindings/virtio/virtio-device.yaml b/Documentation/devicetree/bindings/virtio/virtio-device.yaml
new file mode 100644
index 000000000000..15cb6df8c98a
--- /dev/null
+++ b/Documentation/devicetree/bindings/virtio/virtio-device.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/virtio/virtio-device.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio device bindings
+
+maintainers:
+ - Viresh Kumar <viresh.kumar@linaro.org>
+
+description:
+ These bindings are applicable to virtio devices irrespective of the bus they
+ are bound to, like mmio or pci.
+
+# We need a select here so we don't match all nodes with 'virtio,mmio'
+properties:
+ $nodename:
+ pattern: '^[a-z0-9]+-virtio(-[a-z0-9]+)?$'
+ description: |
+ Exactly one node describing the virtio device. The name of the node isn't
+ significant but its phandle can be used to by a user of the virtio device.
+
+ compatible:
+ pattern: "^virtio,[0-9a-f]+$"
+ description: Virtio device nodes.
+ "virtio,DID", where DID is the virtio device id. The textual
+ representation of DID shall be in lower case hexadecimal with leading
+ zeroes suppressed.
+
+required:
+ - compatible
+
+additionalProperties: true
+
+examples:
+ - |
+ virtio@3000 {
+ compatible = "virtio,mmio";
+ reg = <0x3000 0x100>;
+ interrupts = <43>;
+
+ i2c-virtio {
+ compatible = "virtio,22";
+ };
+ };
+...
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V2 2/5] dt-bindings: i2c: Add bindings for i2c-virtio
2021-07-22 9:56 [PATCH V2 0/5] virtio: Add virtio-device bindings Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 1/5] dt-bindings: virtio: Add binding for virtio devices Viresh Kumar
@ 2021-07-22 9:56 ` Viresh Kumar
2021-07-22 14:45 ` Wolfram Sang
2021-07-22 9:56 ` [PATCH V2 3/5] dt-bindings: gpio: Add bindings for gpio-virtio Viresh Kumar
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2021-07-22 9:56 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin, Rob Herring, Arnd Bergmann,
Jean-Philippe Brucker, Viresh Kumar
Cc: Vincent Guittot, Bill Mills, Alex Bennée, Enrico Weigelt,
metux IT consult, Jie Deng, devicetree, linux-kernel,
virtualization, Wolfram Sang, linux-i2c
This patch adds binding for virtio I2C device, it is based on
virtio-device bindings.
Cc: Wolfram Sang <wsa@kernel.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
.../devicetree/bindings/i2c/i2c-virtio.yaml | 51 +++++++++++++++++++
1 file changed, 51 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..0381d9065287
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
@@ -0,0 +1,51 @@
+# 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#
+
+description:
+ Virtio I2C device, see /schemas/virtio/virtio-device.yaml for more details.
+
+properties:
+ $nodename:
+ pattern: '^i2c-virtio(-[a-z0-9]+)?$'
+
+ compatible:
+ const: virtio,22
+
+required:
+ - compatible
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ virtio@3000 {
+ compatible = "virtio,mmio";
+ reg = <0x3000 0x100>;
+ interrupts = <41>;
+
+ i2c-virtio {
+ compatible = "virtio,22";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ light-sensor@1c {
+ compatible = "dynaimage,al3320a";
+ reg = <0x20>;
+ };
+ };
+ };
+
+...
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V2 3/5] dt-bindings: gpio: Add bindings for gpio-virtio
2021-07-22 9:56 [PATCH V2 0/5] virtio: Add virtio-device bindings Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 1/5] dt-bindings: virtio: Add binding for virtio devices Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 2/5] dt-bindings: i2c: Add bindings for i2c-virtio Viresh Kumar
@ 2021-07-22 9:56 ` Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 4/5] uapi: virtio_ids: Sync ids with specification Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 5/5] virtio: Bind virtio device to device-tree node Viresh Kumar
4 siblings, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2021-07-22 9:56 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin, Rob Herring, Arnd Bergmann,
Jean-Philippe Brucker, Linus Walleij, Bartosz Golaszewski,
Viresh Kumar
Cc: Vincent Guittot, Bill Mills, Alex Bennée, Enrico Weigelt,
metux IT consult, Jie Deng, devicetree, linux-kernel,
virtualization, linux-gpio
This patch adds binding for virtio GPIO controller, it is based on
virtio-device bindings.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
.../devicetree/bindings/gpio/gpio-virtio.yaml | 60 +++++++++++++++++++
1 file changed, 60 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..96108cfb7a08
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
@@ -0,0 +1,60 @@
+# 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#
+
+description:
+ Virtio GPIO controller, see /schemas/virtio/virtio-device.yaml for more
+ details.
+
+properties:
+ $nodename:
+ pattern: '^gpio-virtio(-[a-z0-9]+)?$'
+
+ compatible:
+ const: virtio,29
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+
+ interrupt-controller: true
+
+ "#interrupt-cells":
+ const: 2
+
+required:
+ - compatible
+ - gpio-controller
+ - "#gpio-cells"
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ virtio@3000 {
+ compatible = "virtio,mmio";
+ reg = <0x3000 0x100>;
+ interrupts = <41>;
+
+ gpio: gpio-virtio {
+ compatible = "virtio,29";
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+ };
+
+...
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V2 4/5] uapi: virtio_ids: Sync ids with specification
2021-07-22 9:56 [PATCH V2 0/5] virtio: Add virtio-device bindings Viresh Kumar
` (2 preceding siblings ...)
2021-07-22 9:56 ` [PATCH V2 3/5] dt-bindings: gpio: Add bindings for gpio-virtio Viresh Kumar
@ 2021-07-22 9:56 ` Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 5/5] virtio: Bind virtio device to device-tree node Viresh Kumar
4 siblings, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2021-07-22 9:56 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin, Rob Herring, Arnd Bergmann,
Jean-Philippe Brucker
Cc: Viresh Kumar, Vincent Guittot, Bill Mills, Alex Bennée,
Enrico Weigelt, metux IT consult, Jie Deng, devicetree,
linux-kernel, virtualization
This synchronizes the virtio ids with the latest list from virtio
specification.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Replaces:
https://lore.kernel.org/lkml/7c716c2eb7ace5b5a560d8502af93101dbb53d24.1626170146.git.viresh.kumar@linaro.org/
include/uapi/linux/virtio_ids.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 70a8057ad4bb..3c8e11820fdb 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -54,8 +54,20 @@
#define VIRTIO_ID_SOUND 25 /* virtio sound */
#define VIRTIO_ID_FS 26 /* virtio filesystem */
#define VIRTIO_ID_PMEM 27 /* virtio pmem */
+#define VIRTIO_ID_RPMB 28 /* virtio rpmb */
#define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
+#define VIRTIO_ID_VIDEO_ENCODER 30 /* virtio video encoder */
+#define VIRTIO_ID_VIDEO_DECODER 31 /* virtio video decoder */
+#define VIRTIO_ID_SCMI 32 /* virtio scmi */
+#define VIRTIO_ID_NITRO_SEC_MOD 33 /* virtio nitro secure module*/
+#define VIRTIO_ID_I2C_ADAPTER 34 /* virtio i2c adapter */
+#define VIRTIO_ID_WATCHDOG 35 /* virtio watchdog */
+#define VIRTIO_ID_CAN 36 /* virtio can */
+#define VIRTIO_ID_DMABUF 37 /* virtio dmabuf */
+#define VIRTIO_ID_PARAM_SERV 38 /* virtio parameter server */
+#define VIRTIO_ID_AUDIO_POLICY 39 /* virtio audio policy */
#define VIRTIO_ID_BT 40 /* virtio bluetooth */
+#define VIRTIO_ID_GPIO 41 /* virtio gpio */
/*
* Virtio Transitional IDs
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V2 5/5] virtio: Bind virtio device to device-tree node
2021-07-22 9:56 [PATCH V2 0/5] virtio: Add virtio-device bindings Viresh Kumar
` (3 preceding siblings ...)
2021-07-22 9:56 ` [PATCH V2 4/5] uapi: virtio_ids: Sync ids with specification Viresh Kumar
@ 2021-07-22 9:56 ` Viresh Kumar
2021-07-22 14:52 ` Arnd Bergmann
2021-07-23 6:11 ` [PATCH V2.1 " Viresh Kumar
4 siblings, 2 replies; 14+ messages in thread
From: Viresh Kumar @ 2021-07-22 9:56 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin, Rob Herring, Arnd Bergmann,
Jean-Philippe Brucker
Cc: Viresh Kumar, Vincent Guittot, Bill Mills, Alex Bennée,
Enrico Weigelt, metux IT consult, Jie Deng, devicetree,
linux-kernel, virtualization
Bind the virtio devices with their of_node. This will help users of the
virtio devices to mention their dependencies on the device in the DT
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.c | 67 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 64 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 4b15c00c0a0a..87bee5b966c3 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -4,6 +4,7 @@
#include <linux/virtio_config.h>
#include <linux/module.h>
#include <linux/idr.h>
+#include <linux/of.h>
#include <uapi/linux/virtio_ids.h>
/* Unique numbering for virtio devices. */
@@ -292,6 +293,9 @@ static int virtio_dev_remove(struct device *_d)
/* Acknowledge the device's existence again. */
virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
+
+ of_node_put(dev->dev.of_node);
+
return 0;
}
@@ -319,6 +323,53 @@ void unregister_virtio_driver(struct virtio_driver *driver)
}
EXPORT_SYMBOL_GPL(unregister_virtio_driver);
+/* Virtio device compatibles and IDs */
+static const struct of_device_id of_virtio_devices[] = {
+ { .compatible = "virtio,22", .data = (void *)VIRTIO_ID_I2C_ADAPTER },
+ { .compatible = "virtio,29", .data = (void *)VIRTIO_ID_GPIO },
+ { }
+};
+
+static int virtio_device_of_init(struct virtio_device *dev)
+{
+ struct device_node *np, *pnode = dev->dev.parent->of_node;
+ const struct of_device_id *match;
+ int ret, count;
+
+ 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;
+
+ match = of_match_node(of_virtio_devices, np);
+ if (!match) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ if ((unsigned long)match->data != dev->id.device) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ dev->dev.of_node = np;
+ return 0;
+
+out:
+ of_node_put(np);
+ return ret;
+}
+
/**
* register_virtio_device - register virtio device
* @dev : virtio device to be registered
@@ -343,6 +394,10 @@ int register_virtio_device(struct virtio_device *dev)
dev->index = err;
dev_set_name(&dev->dev, "virtio%u", dev->index);
+ err = virtio_device_of_init(dev);
+ if (err)
+ goto out_ida_remove;
+
spin_lock_init(&dev->config_lock);
dev->config_enabled = false;
dev->config_change_pending = false;
@@ -362,10 +417,16 @@ int register_virtio_device(struct virtio_device *dev)
*/
err = device_add(&dev->dev);
if (err)
- ida_simple_remove(&virtio_index_ida, dev->index);
+ goto out_of_node_put;
+
+ return 0;
+
+out_of_node_put:
+ of_node_put(dev->dev.of_node);
+out_ida_remove:
+ ida_simple_remove(&virtio_index_ida, dev->index);
out:
- if (err)
- virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
+ virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
return err;
}
EXPORT_SYMBOL_GPL(register_virtio_device);
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH V2 1/5] dt-bindings: virtio: Add binding for virtio devices
2021-07-22 9:56 ` [PATCH V2 1/5] dt-bindings: virtio: Add binding for virtio devices Viresh Kumar
@ 2021-07-22 10:28 ` Arnd Bergmann
2021-07-22 10:32 ` Viresh Kumar
0 siblings, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2021-07-22 10:28 UTC (permalink / raw)
To: Viresh Kumar
Cc: Jason Wang, Michael S. Tsirkin, Rob Herring,
Jean-Philippe Brucker, Vincent Guittot, Bill Mills,
Alex Bennée, Enrico Weigelt, metux IT consult, Jie Deng,
DTML, Linux Kernel Mailing List,
open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE
On Thu, Jul 22, 2021 at 11:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Allow virtio device sub-nodes to be added to the virtio mmio or pci
> nodes. The compatible property for virtio device must be of format
> "virtio,<DID>", where DID is virtio device ID in hexadecimal format.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Looks good to me overall. One question would be for the exact format of
the 'compatible' string. After seeing this version, I would slightly prefer
"virtio,device22" over "virtio,22". I think in the previous version
we had mentioned both, but not actually decided on which one to use.
Arnd
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 1/5] dt-bindings: virtio: Add binding for virtio devices
2021-07-22 10:28 ` Arnd Bergmann
@ 2021-07-22 10:32 ` Viresh Kumar
0 siblings, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2021-07-22 10:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jason Wang, Michael S. Tsirkin, Rob Herring,
Jean-Philippe Brucker, Vincent Guittot, Bill Mills,
Alex Bennée, Enrico Weigelt, metux IT consult, Jie Deng,
DTML, Linux Kernel Mailing List,
open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE
On 22-07-21, 12:28, Arnd Bergmann wrote:
> On Thu, Jul 22, 2021 at 11:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > Allow virtio device sub-nodes to be added to the virtio mmio or pci
> > nodes. The compatible property for virtio device must be of format
> > "virtio,<DID>", where DID is virtio device ID in hexadecimal format.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> Looks good to me overall. One question would be for the exact format of
> the 'compatible' string. After seeing this version, I would slightly prefer
> "virtio,device22" over "virtio,22". I think in the previous version
> we had mentioned both, but not actually decided on which one to use.
Right, so there were few doubts I had.
- Should I use "device" as well or not.
- Should I use hex value or decimal.
I went with what's already done with USB/PCI, and so dropped device
and used hex values.
I don't have any strong opinions about using or dropping "device", I
am fine with whatever You/Rob suggest.
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/5] dt-bindings: i2c: Add bindings for i2c-virtio
2021-07-22 9:56 ` [PATCH V2 2/5] dt-bindings: i2c: Add bindings for i2c-virtio Viresh Kumar
@ 2021-07-22 14:45 ` Wolfram Sang
0 siblings, 0 replies; 14+ messages in thread
From: Wolfram Sang @ 2021-07-22 14:45 UTC (permalink / raw)
To: Viresh Kumar
Cc: Jason Wang, Michael S. Tsirkin, Rob Herring, Arnd Bergmann,
Jean-Philippe Brucker, Vincent Guittot, Bill Mills,
Alex Bennée, Enrico Weigelt, metux IT consult, Jie Deng,
devicetree, linux-kernel, virtualization, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 401 bytes --]
On Thu, Jul 22, 2021 at 03:26:40PM +0530, Viresh Kumar wrote:
> This patch adds binding for virtio I2C device, it is based on
> virtio-device bindings.
>
> Cc: Wolfram Sang <wsa@kernel.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
I'll leave the review for Rob. I suppose the series should go in via
some other tree, so for that:
Acked-by: Wolfram Sang <wsa@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 5/5] virtio: Bind virtio device to device-tree node
2021-07-22 9:56 ` [PATCH V2 5/5] virtio: Bind virtio device to device-tree node Viresh Kumar
@ 2021-07-22 14:52 ` Arnd Bergmann
2021-07-23 1:30 ` Viresh Kumar
2021-07-23 6:11 ` [PATCH V2.1 " Viresh Kumar
1 sibling, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2021-07-22 14:52 UTC (permalink / raw)
To: Viresh Kumar
Cc: Jason Wang, Michael S. Tsirkin, Rob Herring,
Jean-Philippe Brucker, Vincent Guittot, Bill Mills,
Alex Bennée, Enrico Weigelt, metux IT consult, Jie Deng,
DTML, Linux Kernel Mailing List,
open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE
On Thu, Jul 22, 2021 at 11:56 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> +/* Virtio device compatibles and IDs */
> +static const struct of_device_id of_virtio_devices[] = {
> + { .compatible = "virtio,22", .data = (void *)VIRTIO_ID_I2C_ADAPTER },
> + { .compatible = "virtio,29", .data = (void *)VIRTIO_ID_GPIO },
> + { }
> +};
> +
> +static int virtio_device_of_init(struct virtio_device *dev)
> +{
> + struct device_node *np, *pnode = dev->dev.parent->of_node;
> + const struct of_device_id *match;
> + int ret, count;
> +
> + 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;
> +
> + match = of_match_node(of_virtio_devices, np);
> + if (!match) {
> + ret = -ENODEV;
> + goto out;
> + }
I think it would be better not to have to enumerate the of_virtio_devices[]
strings, but instead use of_device_is_compatible() to match against
"virtio,%d". Otherwise we end up modifying this function for every
virtio driver that needs a binding.
Arnd
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 5/5] virtio: Bind virtio device to device-tree node
2021-07-22 14:52 ` Arnd Bergmann
@ 2021-07-23 1:30 ` Viresh Kumar
0 siblings, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2021-07-23 1:30 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jason Wang, Michael S. Tsirkin, Rob Herring,
Jean-Philippe Brucker, Vincent Guittot, Bill Mills,
Alex Bennée, Enrico Weigelt, metux IT consult, Jie Deng,
DTML, Linux Kernel Mailing List,
open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE
On 22-07-21, 16:52, Arnd Bergmann wrote:
> On Thu, Jul 22, 2021 at 11:56 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > +/* Virtio device compatibles and IDs */
> > +static const struct of_device_id of_virtio_devices[] = {
> > + { .compatible = "virtio,22", .data = (void *)VIRTIO_ID_I2C_ADAPTER },
> > + { .compatible = "virtio,29", .data = (void *)VIRTIO_ID_GPIO },
> > + { }
> > +};
> > +
> > +static int virtio_device_of_init(struct virtio_device *dev)
> > +{
> > + struct device_node *np, *pnode = dev->dev.parent->of_node;
> > + const struct of_device_id *match;
> > + int ret, count;
> > +
> > + 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;
> > +
> > + match = of_match_node(of_virtio_devices, np);
> > + if (!match) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
>
> I think it would be better not to have to enumerate the of_virtio_devices[]
> strings, but instead use of_device_is_compatible() to match against
> "virtio,%d". Otherwise we end up modifying this function for every
> virtio driver that needs a binding.
Yeah, will do that.
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V2.1 5/5] virtio: Bind virtio device to device-tree node
2021-07-22 9:56 ` [PATCH V2 5/5] virtio: Bind virtio device to device-tree node Viresh Kumar
2021-07-22 14:52 ` Arnd Bergmann
@ 2021-07-23 6:11 ` Viresh Kumar
2021-07-23 8:59 ` Arnd Bergmann
2021-07-23 10:33 ` Michael S. Tsirkin
1 sibling, 2 replies; 14+ messages in thread
From: Viresh Kumar @ 2021-07-23 6:11 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin, Rob Herring, Arnd Bergmann,
Jean-Philippe Brucker
Cc: Viresh Kumar, Vincent Guittot, Bill Mills, Alex Bennée,
Enrico Weigelt, metux IT consult, Jie Deng, devicetree,
linux-kernel, virtualization
Bind the virtio devices with their of_node. This will help users of the
virtio devices to mention their dependencies on the device in the DT
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>
---
V2->V2.1
- Remove list of virtio device and use of_device_is_compatible() instead.
drivers/virtio/virtio.c | 56 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 53 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 4b15c00c0a0a..7c56b3416895 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -4,6 +4,7 @@
#include <linux/virtio_config.h>
#include <linux/module.h>
#include <linux/idr.h>
+#include <linux/of.h>
#include <uapi/linux/virtio_ids.h>
/* Unique numbering for virtio devices. */
@@ -292,6 +293,9 @@ static int virtio_dev_remove(struct device *_d)
/* Acknowledge the device's existence again. */
virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
+
+ of_node_put(dev->dev.of_node);
+
return 0;
}
@@ -319,6 +323,42 @@ void unregister_virtio_driver(struct virtio_driver *driver)
}
EXPORT_SYMBOL_GPL(unregister_virtio_driver);
+static int virtio_device_of_init(struct virtio_device *dev)
+{
+ struct device_node *np, *pnode = dev->dev.parent->of_node;
+ int ret, count;
+ char compat[12];
+
+ 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;
+
+ snprintf(compat, sizeof(compat), "virtio,%x", dev->id.device);
+
+ if (!of_device_is_compatible(np, compat)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ dev->dev.of_node = np;
+ return 0;
+
+out:
+ of_node_put(np);
+ return ret;
+}
+
/**
* register_virtio_device - register virtio device
* @dev : virtio device to be registered
@@ -343,6 +383,10 @@ int register_virtio_device(struct virtio_device *dev)
dev->index = err;
dev_set_name(&dev->dev, "virtio%u", dev->index);
+ err = virtio_device_of_init(dev);
+ if (err)
+ goto out_ida_remove;
+
spin_lock_init(&dev->config_lock);
dev->config_enabled = false;
dev->config_change_pending = false;
@@ -362,10 +406,16 @@ int register_virtio_device(struct virtio_device *dev)
*/
err = device_add(&dev->dev);
if (err)
- ida_simple_remove(&virtio_index_ida, dev->index);
+ goto out_of_node_put;
+
+ return 0;
+
+out_of_node_put:
+ of_node_put(dev->dev.of_node);
+out_ida_remove:
+ ida_simple_remove(&virtio_index_ida, dev->index);
out:
- if (err)
- virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
+ virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
return err;
}
EXPORT_SYMBOL_GPL(register_virtio_device);
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH V2.1 5/5] virtio: Bind virtio device to device-tree node
2021-07-23 6:11 ` [PATCH V2.1 " Viresh Kumar
@ 2021-07-23 8:59 ` Arnd Bergmann
2021-07-23 10:33 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2021-07-23 8:59 UTC (permalink / raw)
To: Viresh Kumar
Cc: Jason Wang, Michael S. Tsirkin, Rob Herring,
Jean-Philippe Brucker, Vincent Guittot, Bill Mills,
Alex Bennée, Enrico Weigelt, metux IT consult, Jie Deng,
DTML, Linux Kernel Mailing List,
open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE
On Fri, Jul 23, 2021 at 8:12 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Bind the virtio devices with their of_node. This will help users of the
> virtio devices to mention their dependencies on the device in the DT
> 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>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2.1 5/5] virtio: Bind virtio device to device-tree node
2021-07-23 6:11 ` [PATCH V2.1 " Viresh Kumar
2021-07-23 8:59 ` Arnd Bergmann
@ 2021-07-23 10:33 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2021-07-23 10:33 UTC (permalink / raw)
To: Viresh Kumar
Cc: Jason Wang, Rob Herring, Arnd Bergmann, Jean-Philippe Brucker,
Vincent Guittot, Bill Mills, Alex Bennée, Enrico Weigelt,
metux IT consult, Jie Deng, devicetree, linux-kernel,
virtualization
On Fri, Jul 23, 2021 at 11:41:31AM +0530, Viresh Kumar wrote:
> Bind the virtio devices with their of_node. This will help users of the
> virtio devices to mention their dependencies on the device in the DT
> 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>
> ---
> V2->V2.1
> - Remove list of virtio device and use of_device_is_compatible() instead.
>
> drivers/virtio/virtio.c | 56 ++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 4b15c00c0a0a..7c56b3416895 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -4,6 +4,7 @@
> #include <linux/virtio_config.h>
> #include <linux/module.h>
> #include <linux/idr.h>
> +#include <linux/of.h>
> #include <uapi/linux/virtio_ids.h>
>
> /* Unique numbering for virtio devices. */
> @@ -292,6 +293,9 @@ static int virtio_dev_remove(struct device *_d)
>
> /* Acknowledge the device's existence again. */
> virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
> +
> + of_node_put(dev->dev.of_node);
> +
> return 0;
> }
>
> @@ -319,6 +323,42 @@ void unregister_virtio_driver(struct virtio_driver *driver)
> }
> EXPORT_SYMBOL_GPL(unregister_virtio_driver);
>
> +static int virtio_device_of_init(struct virtio_device *dev)
> +{
> + struct device_node *np, *pnode = dev->dev.parent->of_node;
dev_of_node? I think gcc will then be smart enough to
make this a nop with !IS_ENABLED(CONFIG_OF).
> + int ret, count;
> + char compat[12];
I think this assumes device id is 16 bits but it's defined as u32
just in case.
If it's ever extended we will then get
into need to handle snprintf errors which is currently missing.
To keep things simple we can do
char compat[] = "virtio,XXXXXXXX";
> +
> + 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;
> +
> + snprintf(compat, sizeof(compat), "virtio,%x", dev->id.device);
worth checking this returns < sizeof(compat) and BUG_ON.
> +
> + if (!of_device_is_compatible(np, compat)) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + dev->dev.of_node = np;
> + return 0;
> +
> +out:
> + of_node_put(np);
> + return ret;
> +}
> +
> /**
> * register_virtio_device - register virtio device
> * @dev : virtio device to be registered
> @@ -343,6 +383,10 @@ int register_virtio_device(struct virtio_device *dev)
> dev->index = err;
> dev_set_name(&dev->dev, "virtio%u", dev->index);
>
> + err = virtio_device_of_init(dev);
> + if (err)
> + goto out_ida_remove;
> +
> spin_lock_init(&dev->config_lock);
> dev->config_enabled = false;
> dev->config_change_pending = false;
> @@ -362,10 +406,16 @@ int register_virtio_device(struct virtio_device *dev)
> */
> err = device_add(&dev->dev);
> if (err)
> - ida_simple_remove(&virtio_index_ida, dev->index);
> + goto out_of_node_put;
> +
> + return 0;
> +
> +out_of_node_put:
> + of_node_put(dev->dev.of_node);
> +out_ida_remove:
> + ida_simple_remove(&virtio_index_ida, dev->index);
> out:
> - if (err)
> - virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
> + virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
> return err;
> }
> EXPORT_SYMBOL_GPL(register_virtio_device);
> --
> 2.31.1.272.g89b43f80a514
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-07-23 10:33 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22 9:56 [PATCH V2 0/5] virtio: Add virtio-device bindings Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 1/5] dt-bindings: virtio: Add binding for virtio devices Viresh Kumar
2021-07-22 10:28 ` Arnd Bergmann
2021-07-22 10:32 ` Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 2/5] dt-bindings: i2c: Add bindings for i2c-virtio Viresh Kumar
2021-07-22 14:45 ` Wolfram Sang
2021-07-22 9:56 ` [PATCH V2 3/5] dt-bindings: gpio: Add bindings for gpio-virtio Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 4/5] uapi: virtio_ids: Sync ids with specification Viresh Kumar
2021-07-22 9:56 ` [PATCH V2 5/5] virtio: Bind virtio device to device-tree node Viresh Kumar
2021-07-22 14:52 ` Arnd Bergmann
2021-07-23 1:30 ` Viresh Kumar
2021-07-23 6:11 ` [PATCH V2.1 " Viresh Kumar
2021-07-23 8:59 ` Arnd Bergmann
2021-07-23 10:33 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).