Re: [PATCH v5 3/5] counter: Add character device interface

[David Lechner]

> > > > > diff --git a/drivers/counter/counter-sysfs.c b/drivers/counter/counter-sysfs.c
> > > > > index e66ed99dd5ea..cefef61f170d 100644
> > > > > --- a/drivers/counter/counter-sysfs.c
> > > > > +++ b/drivers/counter/counter-sysfs.c
> > > >
> > > >
> > > > Not sure why sysfs changes are in the chrdev patch. Are these
> > > > changes related somehow?
> > >
> > > Sorry, I forgot to explain this in the cover letter. The changes here
> > > are only useful for the character device interface. These changes
> > > introduce the extensionZ_name and extensionZ_width sysfs attributes.
> > >
> > > In the character device interface, extensions are selected by their id
> > > number, and the value returned depends on the type of data. The new
> > > sysfs attributes introduced here allow users to match the id of an
> > > extension with its name, as well as the bit width of the value returned
> > > so that the user knows whether to use the value_u8 or value_u64 union
> > > member in struct counter_event.
> >
> > Are we sure that all value types will always be CPU-endian unsigned
> > integers? Or should we make an enum to describe the data type instead
> > of just the width?
>
> It should be safe to assume that the character device interface will
> only ever return CPU-endian unsigned integers. The device driver should
> handle the conversion of any strange endianness from the device before
> the character device interface, while userspace is the one responsible
> for interpreting the meaning of count in the context of the application.
>
> Let's create a scenario for the sake of example. Suppose we want to use
> a counter device to track the vertical position of a component moved by
> a linear actuator. The operator considers some vertical position as the
> horizon, where anything above would be a positive position and anything
> below a negative position. The counter device stores its count in
> big-endian format; but the system CPU expects little-endian.
>
> The flow of data for this scenario would look like the following (where
> BE = big-endian, LE = little-endian):
>
> +----------+         +---------------+          +--------+
> | Raw data | - BE -> | Device driver | -> LE -> | chrdev | - u64 ->
> +----------+         +---------------+          +--------+
>
> At this point, the userspace application would read the unsigned integer
> from the character device and determine how to interpret the position --
> whether the count be converted to a signed value to represent a negative
> physical position.
>
> Whether or not a position should be considered negative is dependent on
> the user application and context. Because the character device does not
> know the context of the user application, it should only provide
> unsigned integers in order to ensure a standard interface for counter
> devices; userspace will be responsible for interpreting those counts to
> something meaningful for the context of their applications.
>
> William Breathitt Gray

Sounds reasonable to me.