On Tue, Jul 13, 2021 at 9:19 AM Viresh Kumar [off-list ref] wrote:

On 13-07-21, 08:43, Rob Herring wrote:

quoted

On Tue, Jul 13, 2021 at 4:50 AM Viresh Kumar [off-list ref] wrote:

quoted

Allow virtio,mmio nodes to contain device specific subnodes. Since each
virtio,mmio node can represent a single virtio device, each virtio node
is allowed to contain a maximum of one device specific subnode.

Doesn't sound like we need 2 nodes here. Just add I2C devices as child
nodes. You could add a more specific compatible string, but the
protocol is discoverable, so that shouldn't be necessary.

I am not sure if it will be a problem, but you can clarify it better.

The parent node (virtio,mmio) is used to create a platform device,
virtio-mmio, (and so assigned as its of_node) and we create the
virtio-device from probe() of this virtio-mmio device.

Is it going to be a problem if two devices in kernel use the same
of_node ?

There shouldn't be. We have nodes be multiple providers (e.g clocks
and resets) already.

Are there cases where we would need to get the device
pointer from the of_node ? Then we will have two here.

Rarely...

In any case, should these potential kernel issues be dictating the DT
binding design? No!

quoted

BTW, what's the usecase for these protocols? A standard interface to
firmware controlled I2C, GPIO, etc.?

Right now we are looking to control devices in the host machine from
guests. That's what Linaro's project stratos is doing. There are other
people who want to use this for other kind of remote control stuff,
maybe from firmware.

Project stratos means nothing to me.

Direct userspace access to I2C, GPIO, etc. has its issues, we're going
to repeat that with guests?

quoted

quoted

diff --git a/include/dt-bindings/virtio/virtio_ids.h b/include/dt-bindings/virtio/virtio_ids.h
new file mode 120000
index 000000000000..6e59ba332216
--- /dev/null
+++ b/include/dt-bindings/virtio/virtio_ids.h

@@ -0,0 +1 @@
+../../uapi/linux/virtio_ids.h

This will break the devicetree-rebasing tree I think. DT files
shouldn't reference kernel files.

We already do this for linux-event-codes.h and so I thought it is the
right way of doing it :)

Humm, maybe it's okay. Please double check then...

Else we can create a new copy, which will be a mess, or use hardcoded
values.

Though you may not need the header based on what Arnd and I have suggested.

Rob