| Message ID | 026ad5f274d64d46590623f9f3a04b8abfbe62d7.1626947324.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | virtio: Add virtio-device bindings | expand |
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
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
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);
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(-) -- 2.31.1.272.g89b43f80a514