Re: [RFC/PATCHv2 2/2] counter: introduce support for Intel QEP Encoder

[William Breathitt Gray <vilhelm.gray@gmail.com>]

On Tue, Sep 24, 2019 at 04:46:57PM -0500, David Lechner wrote:
> On 9/22/19 6:35 PM, William Breathitt Gray wrote:
> > On Thu, Sep 19, 2019 at 11:03:05AM +0300, Felipe Balbi wrote:
> >> Add support for Intel PSE Quadrature Encoder
> >>
> >> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> >> ---
> >>
> >> Changes since v1:
> >> 	- Many more private sysfs files converted over to counter interface
> >>
> >>
> >> How do you want me to model this device's Capture Compare Mode (see
> >> below)?
> > 
> > Hi Felipe,
> > 
> > I'm CCing Fabien and David as they may be interested in the timestamps
> > discussion. See below for some ideas I have on implementing this.
> > 
> 
> Could be an interesting read (thread from my first counter driver):
> 
> https://lore.kernel.org/linux-iio/1b913919-beb9-34e7-d915-6bcc40eeee1d@lechnology.com/
> 
> What would be useful to me is something like the buffer feature in iio
> where a timestamp is associated with a count and stored in a buffer so that
> we can look at a window of all values recorded in the last 20ms. Being able
> to access this via mmap would be very helpful for performance (running on
> 300MHz ARM). Anything to do with timestamps in sysfs is probably not useful
> unless it is a rare event, like a watchdog timeout.

I'm CCing Jonathan Cameron since I'm not familiar with how IIO handles
timestamps and buffers. I don't want to reinvent something that is
working well, so hopefully we can reuse the IIO timestamp design for the
Counter subsystem.

I would argue that a human-readable timestamps printout is useful for
certain applications (e.g. a tally counter attached to a fault line: a
human administrator will be able to review previous fault times).
However as you point out, a low latency operation is necessary for
performance critical applications.

Although you are correct that mmap is a good low latency operation to
get access to a timestamp buffer, I'm afraid giving direct access to
memory like that will lead to many incompatible representations of
timestamp data (e.g. variations in endianness, signedness, data size,
etc.). I would like a standardized representation for this data that
userspace applications can expect to receive and interpret, especially
when time is widely represented as an unsigned integer.

Felipe suggested the creation of a counter_event structure so that users
can poll on an attribute. This kind of behavior is useful for notifying
users of interrupts and other events, but I think we should restrict the
use of the read call on these sysfs attributes to just human-readable
data. Instead, perhaps ioctl calls can be used to facilitate binary data
transfers.

For example, we can define a COUNTER_GET_TIMESTAMPS_IOCTL ioctl request
that returns a counter_timestamps structure with a timestamps array
populated:

        struct counter_timestamps{
                size_t num_timestamps;
        	unsigned int *timestamps;
        }

That would allow quick access to the timestamps data, while also
restricting it to a standard representation that all userspace
applications can follow and interpret. In addition, this won't interfer
with polling, so users can still wait for an interrupt and then decide
whether they want to use the slower human-readable printout (via read)
or the faster binary data access (via ioctl).

William Breathitt Gray