Re: [PATCH V5 1/2] virtio-gpio: Add the device specification

[Viresh Kumar <viresh.kumar@linaro.org>]

On 19-07-21, 12:40, Arnd Bergmann wrote:
> I still don't see it as an important enough part of the interface to be encoded
> in the virtio spec. In the most likely use cases I can think of, you would never
> need more than one gpio controller in a virtio machine, since you can have
> an arbitrary number of lines connected to it already.
> 
> This also simplifies the host side, as it avoids requiring the host to configure
> a name that ends up being the same almost always.
> 
> If Linus or Bartosz think we should have the name, leave it in there, otherwise
> I'd remove it.

I don't have a really good argument for keeping it, dropping it for now unless
someone thinks it is important. :)

> Encoding the number of gpio lines in the config registers makes sense to me,
> but I don't see how you can sensibly get away without some other structure
> to describe what the lines are. It doesn't have to be DT, but that's what we use
> in practice.

Yes, we normally end up defining them in DT via "gpio-line-names" binding, but I
think it would be better to get them over the protocol instead here as this
should be self sufficient. Something like DT will still be required to link
users to the GPIO device, and that's outside of the scope of the protocol, so DT
works well there.

> > That may not always work and this count of refcounting at the backend daemon may
> > not be correct.
> >
> > So, the guest calls set-direction-out-with-val-1, the backend activates the GPIO
> > pin, sets direction/value and then deactivates it ? That won't work, right ?
> >
> > On those lines, I think these operations must be renamed to ACTIVATE/DEACTIVATE
> > instead of REQUEST/FREE, that is what we want to do here.
> 
> From the drivers you pointed to,

There were more, I just didn't get to them :)

> it seems to be something like a 'disconnected'
> state where the pin is neither input nor output, so this could perhaps
> get reflected
> in the interface as well, by having each pin be one of IN/OUT/NONE. Not sure
> if that is any better.

I think keeping APIs like ACTIVATE/DEACTIVATE makes it cleaner, than adding
another state of the PIN itself.

-- 
viresh