diff mbox series

[V3,5/5] virtio: Bind virtio device to device-tree node

Message ID 454a58f998b0d16847d72a97b32192829fab2c8c.1627273794.git.viresh.kumar@linaro.org
State Superseded
Headers show
Series virtio: Add virtio-device bindings | expand

Commit Message

Viresh Kumar July 26, 2021, 4:51 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.

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

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

---
 drivers/virtio/virtio.c | 57 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 3 deletions(-)

-- 
2.31.1.272.g89b43f80a514

Comments

Alexey Kardashevskiy Sept. 13, 2021, 9:19 a.m. UTC | #1 
On 26/07/2021 14:51, 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.

> 

> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

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

> ---

>  drivers/virtio/virtio.c | 57 ++++++++++++++++++++++++++++++++++++++---

>  1 file changed, 54 insertions(+), 3 deletions(-)

> 

> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c

> index 4b15c00c0a0a..d001e84a5b23 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,43 @@ 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_of_node(dev->dev.parent);

> +	char compat[] = "virtio,XXXXXXXX"; /* Reserve enough space 32-bit id */

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

> +

> +	BUG_ON(snprintf(compat, sizeof(compat), "virtio,%x", dev->id.device) >=

> +	       sizeof(compat));

> +

> +	if (!of_device_is_compatible(np, compat)) {



This broke powerpc/pseries as there these virtio devices are PCI so
there is no "compat" - PCI vendor id/device ids play role of "compat".
Thanks,




> +		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 +384,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 +407,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);

> 


-- 
Alexey
Michael S. Tsirkin Sept. 13, 2021, 9:45 a.m. UTC | #2 
On Mon, Sep 13, 2021 at 07:19:17PM +1000, Alexey Kardashevskiy wrote:
> 

> 

> On 26/07/2021 14:51, 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.

> > 

> > Reviewed-by: Arnd Bergmann <arnd@arndb.de>

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

> > ---

> >  drivers/virtio/virtio.c | 57 ++++++++++++++++++++++++++++++++++++++---

> >  1 file changed, 54 insertions(+), 3 deletions(-)

> > 

> > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c

> > index 4b15c00c0a0a..d001e84a5b23 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,43 @@ 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_of_node(dev->dev.parent);

> > +	char compat[] = "virtio,XXXXXXXX"; /* Reserve enough space 32-bit id */

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

> > +

> > +	BUG_ON(snprintf(compat, sizeof(compat), "virtio,%x", dev->id.device) >=

> > +	       sizeof(compat));

> > +

> > +	if (!of_device_is_compatible(np, compat)) {

> 

> 

> This broke powerpc/pseries as there these virtio devices are PCI so

> there is no "compat" - PCI vendor id/device ids play role of "compat".

> Thanks,


Hmm now that you say this I wonder why do we bother
with this check, too. When can this be invoked for something
that is not a virtio device? And is it enough to just
skip of_node initialization then?


> 

> 

> 

> > +		ret = -EINVAL;



So basically ret = 0 above?


> > +		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 +384,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 +407,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);

> > 

> 

> -- 

> Alexey
Alexey Kardashevskiy Sept. 13, 2021, 10:27 a.m. UTC | #3 
On 13/09/2021 19:45, Michael S. Tsirkin wrote:
> On Mon, Sep 13, 2021 at 07:19:17PM +1000, Alexey Kardashevskiy wrote:

>>

>>

>> On 26/07/2021 14:51, 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.

>>>

>>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

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

>>> ---

>>>  drivers/virtio/virtio.c | 57 ++++++++++++++++++++++++++++++++++++++---

>>>  1 file changed, 54 insertions(+), 3 deletions(-)

>>>

>>> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c

>>> index 4b15c00c0a0a..d001e84a5b23 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,43 @@ 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_of_node(dev->dev.parent);

>>> +	char compat[] = "virtio,XXXXXXXX"; /* Reserve enough space 32-bit id */

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

>>> +

>>> +	BUG_ON(snprintf(compat, sizeof(compat), "virtio,%x", dev->id.device) >=

>>> +	       sizeof(compat));

>>> +

>>> +	if (!of_device_is_compatible(np, compat)) {

>>

>>

>> This broke powerpc/pseries as there these virtio devices are PCI so

>> there is no "compat" - PCI vendor id/device ids play role of "compat".

>> Thanks,

> 

> Hmm now that you say this I wonder why do we bother

> with this check, too. When can this be invoked for something

> that is not a virtio device? And is it enough to just

> skip of_node initialization then?



I am not following here, the problem device is virtio-scsi which is
virtio-derived, or you meant that virtio which hosts virtio-bus?

> 

>>

>>

>>

>>> +		ret = -EINVAL;

> 

> 

> So basically ret = 0 above?



Yup, this does fix it. Thanks,

> 

> 

>>> +		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 +384,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 +407,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);

>>>

>>

>> -- 

>> Alexey

> 


-- 
Alexey
Guenter Roeck Sept. 13, 2021, 2:49 p.m. UTC | #4 
On Mon, Jul 26, 2021 at 10:21:45AM +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.

> 

> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

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


This patch causes a boot failure on sparc64: The virtio device no longer
instantiates. Reverting this patch fixes the problem. Bisect log attached.

Guenter

---
# bad: [6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f] Linux 5.15-rc1
# good: [926de8c4326c14fcf35f1de142019043597a4fac] Merge tag 'acpi-5.15-rc1-3' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
git bisect start 'HEAD' '926de8c4326c'
# good: [8177a5c96229ff24da1e362789e359b68b4f34f5] Merge tag 'libata-5.15-2021-09-11' of git://git.kernel.dk/linux-block
git bisect good 8177a5c96229ff24da1e362789e359b68b4f34f5
# bad: [78e709522d2c012cb0daad2e668506637bffb7c2] Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
git bisect bad 78e709522d2c012cb0daad2e668506637bffb7c2
# bad: [7bc7f61897b66bef78bb5952e3d1e9f3aaf9ccca] Documentation: Add documentation for VDUSE
git bisect bad 7bc7f61897b66bef78bb5952e3d1e9f3aaf9ccca
# bad: [41116599a0731f4cd451e9d191d879ab45e31945] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
git bisect bad 41116599a0731f4cd451e9d191d879ab45e31945
# good: [5262912ef3cfc5e518892c3d67fb36412cb813e2] vdpa/mlx5: Add support for control VQ and MAC setting
git bisect good 5262912ef3cfc5e518892c3d67fb36412cb813e2
# good: [7f815fce08d563006e43d1b7d2f9a0a4f3b832f3] dt-bindings: i2c: Add bindings for i2c-virtio
git bisect good 7f815fce08d563006e43d1b7d2f9a0a4f3b832f3
# good: [d5a8680dfab0547a4ecd708b1fe9de48598a6757] uapi: virtio_ids: Sync ids with specification
git bisect good d5a8680dfab0547a4ecd708b1fe9de48598a6757
# bad: [9af8f1061646e8e22b66413bedf7b3e2ab3d69e5] virtio/vsock: rename 'EOR' to 'EOM' bit.
git bisect bad 9af8f1061646e8e22b66413bedf7b3e2ab3d69e5
# bad: [694a1116b405d887c893525a6766b390989c8606] virtio: Bind virtio device to device-tree node
git bisect bad 694a1116b405d887c893525a6766b390989c8606
# first bad commit: [694a1116b405d887c893525a6766b390989c8606] virtio: Bind virtio device to device-tree node
Guenter Roeck Sept. 13, 2021, 4:19 p.m. UTC | #5 
On Mon, Sep 13, 2021 at 07:49:07AM -0700, Guenter Roeck wrote:
> On Mon, Jul 26, 2021 at 10:21:45AM +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.

> > 

> > Reviewed-by: Arnd Bergmann <arnd@arndb.de>

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

> 

> This patch causes a boot failure on sparc64: The virtio device no longer

> instantiates. Reverting this patch fixes the problem. Bisect log attached.

> 


In case it matters: The problem is here:

+       if (!of_device_is_compatible(np, compat)) {
+               ret = -EINVAL;
+               goto out;
+       }

Guenter

> Guenter

> 

> ---

> # bad: [6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f] Linux 5.15-rc1

> # good: [926de8c4326c14fcf35f1de142019043597a4fac] Merge tag 'acpi-5.15-rc1-3' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm

> git bisect start 'HEAD' '926de8c4326c'

> # good: [8177a5c96229ff24da1e362789e359b68b4f34f5] Merge tag 'libata-5.15-2021-09-11' of git://git.kernel.dk/linux-block

> git bisect good 8177a5c96229ff24da1e362789e359b68b4f34f5

> # bad: [78e709522d2c012cb0daad2e668506637bffb7c2] Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost

> git bisect bad 78e709522d2c012cb0daad2e668506637bffb7c2

> # bad: [7bc7f61897b66bef78bb5952e3d1e9f3aaf9ccca] Documentation: Add documentation for VDUSE

> git bisect bad 7bc7f61897b66bef78bb5952e3d1e9f3aaf9ccca

> # bad: [41116599a0731f4cd451e9d191d879ab45e31945] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.

> git bisect bad 41116599a0731f4cd451e9d191d879ab45e31945

> # good: [5262912ef3cfc5e518892c3d67fb36412cb813e2] vdpa/mlx5: Add support for control VQ and MAC setting

> git bisect good 5262912ef3cfc5e518892c3d67fb36412cb813e2

> # good: [7f815fce08d563006e43d1b7d2f9a0a4f3b832f3] dt-bindings: i2c: Add bindings for i2c-virtio

> git bisect good 7f815fce08d563006e43d1b7d2f9a0a4f3b832f3

> # good: [d5a8680dfab0547a4ecd708b1fe9de48598a6757] uapi: virtio_ids: Sync ids with specification

> git bisect good d5a8680dfab0547a4ecd708b1fe9de48598a6757

> # bad: [9af8f1061646e8e22b66413bedf7b3e2ab3d69e5] virtio/vsock: rename 'EOR' to 'EOM' bit.

> git bisect bad 9af8f1061646e8e22b66413bedf7b3e2ab3d69e5

> # bad: [694a1116b405d887c893525a6766b390989c8606] virtio: Bind virtio device to device-tree node

> git bisect bad 694a1116b405d887c893525a6766b390989c8606

> # first bad commit: [694a1116b405d887c893525a6766b390989c8606] virtio: Bind virtio device to device-tree node
diff mbox series

Patch

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 4b15c00c0a0a..d001e84a5b23 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,43 @@  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_of_node(dev->dev.parent);
+	char compat[] = "virtio,XXXXXXXX"; /* Reserve enough space 32-bit id */
+	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;
+
+	BUG_ON(snprintf(compat, sizeof(compat), "virtio,%x", dev->id.device) >=
+	       sizeof(compat));
+
+	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 +384,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 +407,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);