diff mbox series

[V2.1,5/5] virtio: Bind virtio device to device-tree node

Message ID 3606cdcc637682a3eb401d617e6e247431b78ec6.1627019436.git.viresh.kumar@linaro.org
State Superseded
Headers show
Series None | expand

Commit Message

Viresh Kumar July 23, 2021, 6:11 a.m. UTC
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(-)

-- 
2.31.1.272.g89b43f80a514

Comments

Arnd Bergmann July 23, 2021, 8:59 a.m. UTC | #1
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>
Michael S. Tsirkin July 23, 2021, 10:33 a.m. UTC | #2
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
diff mbox series

Patch

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);