Messages in this thread | | | From | Rob Herring <> | Date | Tue, 13 Jul 2021 13:34:05 -0600 | Subject | Re: [PATCH 1/5] dt-bindings: virtio: mmio: Add support for device subnode |
| |
On Tue, Jul 13, 2021 at 9:19 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 13-07-21, 08:43, Rob Herring wrote: > > On Tue, Jul 13, 2021 at 4:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > 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!
> > > 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?
> > > 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
| |