Hi, On 29/04/2020 14:11:34-0400, William Breathitt Gray wrote: > Over the past couple years we have noticed some shortcomings with the > Counter sysfs interface. Although useful in the majority of situations, > there are certain use-cases where interacting through sysfs attributes > can become cumbersome and inefficient. A desire to support more advanced > functionality such as timestamps, multi-axis positioning tables, and > other such latency-sensitive applications, has motivated a reevaluation > of the Counter subsystem. I believe a character device interface will be > helpful for this more niche area of counter device use. > > To quell any concerns from the offset: this patchset makes no changes to > the existing Counter sysfs userspace interface -- existing userspace > applications will continue to work with no modifications necessary. I > request that driver maintainers please test their applications to verify > that this is true, and report any discrepancies if they arise. > On that topic, I'm wondering why the counter subsystem uses /sys/bus instead of /sys/class that would be more natural for a class of devices. I can't see how counters would be considered busses. I think you should consider moving it over to /sys/class (even if deprecating /sys/bus/counter will be long). > Interaction with Counter character devices occurs via ioctl commands. > This allows userspace applications to access and set counter data using > native C datatypes rather than working through string translations. > I agree with David that you should consider using read to retrieve the counter data as this will simplify interrupt handling/polling and blocking/non-blocking reads can be used by an application. ABI wise, this can also be a good move as you could always consider having an ioctl requesting a specific format when reading the device so you are not stuck with the initial format you are going to choose. > 2. Should device driver callbacks return int or long? I sometimes see > error values returned as long (e.g. PTR_ERR(), the file_operations > structure's ioctl callbacks, etc.); when is it necessary to return > long as opposed to int? > You should use a long if you ever have to return a point as it is guaranteed to have the correct size. Else, just stick to an int if you are not going to overflow it. > 3. I only implemented the unlocked_ioctl callback. Should I implement a > compat_ioctl callback as well? > The compat_ioctl is to handle 32bit userspace running on a 64bit kernel. If your structures have the same size in both cases, then you don't have to implement compat_ioctl. Have a look at Documentation/driver-api/ioctl.rst -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com