| Message ID | 20211221090447.1567-1-vincent.whitchurch@axis.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | um: virtio_uml: allow probing from devicetree | expand |
On Tue, 2021-12-21 at 10:04 +0100, Vincent Whitchurch wrote: > Allow the virtio_uml device to be probed from the devicetree so that > sub-devices can be specified using the standard virtio bindings, for > example: > > virtio@1 { > compatible = "virtio,uml"; > socket-path = "i2c.sock"; > virtio-device-id = <0x22>; > Given this, maybe it should modify Documentation/devicetree/bindings/virtio/virtio-device.yaml? Or actually add a new Documentation/devicetree/bindings/virtio/uml.yaml I guess? +Rob, because I'm not really into any of this. Also, I'm not even sure we should/need to document the DT bits that are basically only used for testing in the first place? Code looks good to me. johannes
On Tue, Dec 21, 2021 at 09:48:26PM +0100, Johannes Berg wrote: > On Tue, 2021-12-21 at 10:04 +0100, Vincent Whitchurch wrote: > > Allow the virtio_uml device to be probed from the devicetree so that > > sub-devices can be specified using the standard virtio bindings, for > > example: > > > > virtio@1 { > > compatible = "virtio,uml"; > > socket-path = "i2c.sock"; > > virtio-device-id = <0x22>; > > > > Given this, maybe it should modify > Documentation/devicetree/bindings/virtio/virtio-device.yaml? Or actually > add a new Documentation/devicetree/bindings/virtio/uml.yaml I guess? > > +Rob, because I'm not really into any of this. > > Also, I'm not even sure we should/need to document the DT bits that are > basically only used for testing in the first place? I wasn't sure either, but Rob was OK with not documenting some other bindings which are only used for testing[0], so I assumed that that applied here too: [0] https://lore.kernel.org/all/5baa1ae6.1c69fb81.847f2.3ab1@mx.google.com/ Also, DT bindings are supposed to be generic and based on what the hardware has, but here we have no hardware and something very Linux and UML-specific. > Code looks good to me. Thanks!
On 22/12/2021 10:34, Vincent Whitchurch wrote: > On Tue, Dec 21, 2021 at 09:48:26PM +0100, Johannes Berg wrote: >> On Tue, 2021-12-21 at 10:04 +0100, Vincent Whitchurch wrote: >>> Allow the virtio_uml device to be probed from the devicetree so that >>> sub-devices can be specified using the standard virtio bindings, for >>> example: >>> >>> virtio@1 { >>> compatible = "virtio,uml"; >>> socket-path = "i2c.sock"; >>> virtio-device-id = <0x22>; >>> >> >> Given this, maybe it should modify >> Documentation/devicetree/bindings/virtio/virtio-device.yaml? Or actually >> add a new Documentation/devicetree/bindings/virtio/uml.yaml I guess? >> >> +Rob, because I'm not really into any of this. >> >> Also, I'm not even sure we should/need to document the DT bits that are >> basically only used for testing in the first place? > > I wasn't sure either, but Rob was OK with not documenting some other > bindings which are only used for testing[0], so I assumed that that > applied here too: > > [0] https://lore.kernel.org/all/5baa1ae6.1c69fb81.847f2.3ab1@mx.google.com/ > > Also, DT bindings are supposed to be generic and based on what the > hardware has, but here we have no hardware and something very Linux and > UML-specific. This will probably need to be added to the general UML documentation. I will take care of that once the patches are in the tree. Brgds, > >> Code looks good to me. > > Thanks! >
----- Ursprüngliche Mail ----- > Von: "anton ivanov" <anton.ivanov@cambridgegreys.com> >> Also, DT bindings are supposed to be generic and based on what the >> hardware has, but here we have no hardware and something very Linux and >> UML-specific. > > This will probably need to be added to the general UML documentation. I > will take care of that once the patches are in the tree. That would be great! Patch applied. Thanks, //richard
On 22/12/2021 10:34, Vincent Whitchurch wrote: > On Tue, Dec 21, 2021 at 09:48:26PM +0100, Johannes Berg wrote: >> On Tue, 2021-12-21 at 10:04 +0100, Vincent Whitchurch wrote: >>> Allow the virtio_uml device to be probed from the devicetree so that >>> sub-devices can be specified using the standard virtio bindings, for >>> example: >>> >>> virtio@1 { >>> compatible = "virtio,uml"; >>> socket-path = "i2c.sock"; >>> virtio-device-id = <0x22>; >>> >> >> Given this, maybe it should modify >> Documentation/devicetree/bindings/virtio/virtio-device.yaml? Or actually >> add a new Documentation/devicetree/bindings/virtio/uml.yaml I guess? >> >> +Rob, because I'm not really into any of this. >> >> Also, I'm not even sure we should/need to document the DT bits that are >> basically only used for testing in the first place? If we start adding the UML devices themselves to the DT, we might as well add all of them. In the doc patch have described the DT support as mostly for development at this point. It can be a good alternative to the endless command line (especially for complex devices like f.e. l2tpv3). > > I wasn't sure either, but Rob was OK with not documenting some other > bindings which are only used for testing[0], so I assumed that that > applied here too: > > [0] https://lore.kernel.org/all/5baa1ae6.1c69fb81.847f2.3ab1@mx.google.com/ > > Also, DT bindings are supposed to be generic and based on what the > hardware has, but here we have no hardware and something very Linux and > UML-specific. > >> Code looks good to me. > > Thanks! > Brgds,
diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c index d51e445df797..3e4fa0f262d3 100644 --- a/arch/um/drivers/virtio_uml.c +++ b/arch/um/drivers/virtio_uml.c @@ -21,6 +21,7 @@ * Based on Virtio MMIO driver by Pawel Moll, copyright 2011-2014, ARM Ltd. */ #include <linux/module.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/virtio.h> @@ -49,6 +50,7 @@ struct virtio_uml_platform_data { struct virtio_uml_device { struct virtio_device vdev; struct platform_device *pdev; + struct virtio_uml_platform_data *pdata; spinlock_t sock_lock; int sock, req_fd, irq; @@ -149,7 +151,7 @@ static int vhost_user_recv(struct virtio_uml_device *vu_dev, if (rc == -ECONNRESET && vu_dev->registered) { struct virtio_uml_platform_data *pdata; - pdata = vu_dev->pdev->dev.platform_data; + pdata = vu_dev->pdata; virtio_break_device(&vu_dev->vdev); schedule_work(&pdata->conn_broken_wk); @@ -1113,21 +1115,63 @@ void virtio_uml_set_no_vq_suspend(struct virtio_device *vdev, no_vq_suspend ? "dis" : "en"); } +static void vu_of_conn_broken(struct work_struct *wk) +{ + /* + * We can't remove the device from the devicetree so the only thing we + * can do is warn. + */ + WARN_ON(1); +} + /* Platform device */ +static struct virtio_uml_platform_data * +virtio_uml_create_pdata(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct virtio_uml_platform_data *pdata; + int ret; + + if (!np) + return ERR_PTR(-EINVAL); + + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return ERR_PTR(-ENOMEM); + + INIT_WORK(&pdata->conn_broken_wk, vu_of_conn_broken); + pdata->pdev = pdev; + + ret = of_property_read_string(np, "socket-path", &pdata->socket_path); + if (ret) + return ERR_PTR(ret); + + ret = of_property_read_u32(np, "virtio-device-id", + &pdata->virtio_device_id); + if (ret) + return ERR_PTR(ret); + + return pdata; +} + static int virtio_uml_probe(struct platform_device *pdev) { struct virtio_uml_platform_data *pdata = pdev->dev.platform_data; struct virtio_uml_device *vu_dev; int rc; - if (!pdata) - return -EINVAL; + if (!pdata) { + pdata = virtio_uml_create_pdata(pdev); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); + } vu_dev = kzalloc(sizeof(*vu_dev), GFP_KERNEL); if (!vu_dev) return -ENOMEM; + vu_dev->pdata = pdata; vu_dev->vdev.dev.parent = &pdev->dev; vu_dev->vdev.dev.release = virtio_uml_release_dev; vu_dev->vdev.config = &virtio_uml_config_ops;
Allow the virtio_uml device to be probed from the devicetree so that sub-devices can be specified using the standard virtio bindings, for example: virtio@1 { compatible = "virtio,uml"; socket-path = "i2c.sock"; virtio-device-id = <0x22>; i2c-controller { compatible = "virtio,device22"; #address-cells = <0x01>; #size-cells = <0x00>; light-sensor@01 { compatible = "ti,opt3001"; reg = <0x01>; }; }; }; Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> --- Notes: Requires the UML devicetree support I posted a couple of weeks ago: https://lore.kernel.org/all/20211208151123.29313-1-vincent.whitchurch@axis.com/ arch/um/drivers/virtio_uml.c | 50 +++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-)