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@xxxxxxxxxxxxxxx> > >> --- > >> > >> 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@xxxxxxxxxxxxxx/ > > 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