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

[David Lechner <david@lechnology.com>]

>>>>> 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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel