Re: [v7,03/10] docs: Add Generic Counter interface documentation

[David Lechner]

On 07/06/2018 12:15 PM, Jonathan Cameron wrote:
> On Wed, 4 Jul 2018 19:23:26 +0200
> Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> > On Tue, Jul 3, 2018 at 4:16 PM William Breathitt Gray
> > <vilhelm.gray@xxxxxxxxx> wrote:
> > > On Mon, Jul 02, 2018 at 02:37:53PM -0500, David Lechner wrote:
> > > > On 06/21/2018 04:07 PM, William Breathitt Gray wrote:
> > > > > +Userspace Interface
> > > > > +===================
> > > > > +
> > > > > +Several sysfs attributes are generated by the Generic Counter interface,
> > > > > +and reside under the /sys/bus/counter/devices/counterX directory, where
> > > > > +counterX refers to the respective counter device. Please see
> > > > > +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
> > > > > +information on each Generic Counter interface sysfs attribute.
> > > > > +
> > > > > +Through these sysfs attributes, programs and scripts may interact with
> > > > > +the Generic Counter paradigm Counts, Signals, and Synapses of respective
> > > > > +counter devices.
> > > > > +
> > > >
> > > > Have you considered a character device in addition to the sysfs interface?
> > > >
> > > > I basically have many of the same concerns that resulted in a char dev for
> > > > gpio[1].
> > > >
> > > > - With sysfs, you *can* technically poll for events, but then you have to
> > > >    seek and read or re-open the file.
>
> For this to be relevant we need some type of self clocking sampling of a counter,
> so far this hasn't really been true for William's devices - they tend to have
> internal monitoring and you just 'ask' them when you want to know the rotation.
> Sure if we have 'events' such as soft limit switches in the hardware, then
> we'd want some sort of event chrdev (personally I think these should be separate
> from the main data flow - but that's a detail).
>
> > > > - File permissions are annoying if you want a non root user to be able to
> > > >    use the device.
>
> They aren't a whole lot different for a chrdev.  In both cases you can allow
> a non root user write access if you want to.
>
> > > > - A single program can't claim exclusive access to a device.
>
> Agreed.  Though that only matters for control, why do you care if someone
> else can read.  In IIO we get around this by 'generally' blocking settings
> changes that will a process that is sampling data via the chrdev.
> It's not a hard and fast rule though.  I really don't like configuration
> via chrdevs as for complex devices you end up with a non self describing
> interface with a lot of complexity.
>
> The exceptions are things like the media controller frameworks, but they
> are very very heavyweight for an simple devices like counters.
>
> > > > - There is no automatic cleanup if a userspace program accessing the device
> > > >     crashes.
>
> For these devices, the question is what sort of cleanup makes sense?
>
> Often they are freerunning so the most you could do is power down if you knew
> no one cared, but for an encoder you may well want to continue tracking even
> if no one is looking right now.
>
> I can think of reasons you 'might' want to tidy up, but we'd need real
> driver code to show the necessity of this one.
>
> > > > [1]: https://www.elinux.org/images/7/74/Elce2017_new_GPIO_interface.pdf
> > >
> > > Those look like good technical reasons for implementing a character
> > > device for the Generic Counter interface. I chose to implement the sysfs
> > > interface because I was using the IIO code as a reference, but I
> > > personally don't have much opposition to a character device in addition.
> >
> > IIO is also using a character device for outputting events and sensor
> > data. In IIO sysfs is only used for configuring what events and
> > data should come out of the character device.
>
> Yes, with the addition that we typically provide data readback as well.
> For some simple devices which are slow and are actually polled to get
> a reading, there is not a lot of point in implementing the chrdev route
> so in IIO it is optional.
> > > I'd like to get Jonathan's opinion on this as well if possible -- I
> > > vaguely recall us considering this option briefly last year when the
> > > Generic Counter interface was still in its beginnings. I've CC'd Linus
> > > Walleij as well for input as the GPIO maintainer.
>
> I'm not against it, but I do want to see use cases that are not
> satisfied by sysfs first.
>
> So far we've no seen them but sounds like you might have one David!

Basically, we are implementing a counter in the PRU on TI Sitara, so
we can make it do just about whatever we want. Although, I'm trying to
keep it similar to the eQEP.