Message-ID: <9e610ccd-bf2c-f20e-c562-b316e880a5d4@lechnology.com>
Date:   Fri, 6 Jul 2018 13:25:00 -0500
From:   David Lechner <david@...hnology.com>
To:     Jonathan Cameron <jonathan.cameron@...wei.com>,
        Linus Walleij <linus.walleij@...aro.org>
Cc:     William Breathitt Gray <vilhelm.gray@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-iio@...r.kernel.org,
        Fabrice Gasnier <fabrice.gasnier@...com>,
        Benjamin Gaignard <benjamin.gaignard@...com>,
        Rob Herring <robh+dt@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald <pmeerw@...erw.net>,
        Mark Rutland <mark.rutland@....com>
Subject: Re: [v7,03/10] docs: Add Generic Counter interface documentation

On 07/06/2018 12:15 PM, Jonathan Cameron wrote:
> On Wed, 4 Jul 2018 19:23:26 +0200
> Linus Walleij <linus.walleij@...aro.org> wrote:
> 
>> On Tue, Jul 3, 2018 at 4:16 PM William Breathitt Gray
>> <vilhelm.gray@...il.com> wrote:
>>> On Mon, Jul 02, 2018 at 02:37:53PM -0500, David Lechner wrote:
>>>> On 06/21/2018 04:07 PM, William Breathitt Gray wrote:
>>>>> +Userspace Interface
>>>>> +===================
>>>>> +
>>>>> +Several sysfs attributes are generated by the Generic Counter interface,
>>>>> +and reside under the /sys/bus/counter/devices/counterX directory, where
>>>>> +counterX refers to the respective counter device. Please see
>>>>> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
>>>>> +information on each Generic Counter interface sysfs attribute.
>>>>> +
>>>>> +Through these sysfs attributes, programs and scripts may interact with
>>>>> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
>>>>> +counter devices.
>>>>> +
>>>>
>>>> Have you considered a character device in addition to the sysfs interface?
>>>>
>>>> I basically have many of the same concerns that resulted in a char dev for
>>>> gpio[1].
>>>>
>>>> - With sysfs, you *can* technically poll for events, but then you have to
>>>>    seek and read or re-open the file.
> 
> For this to be relevant we need some type of self clocking sampling of a counter,
> so far this hasn't really been true for William's devices - they tend to have
> internal monitoring and you just 'ask' them when you want to know the rotation.
> Sure if we have 'events' such as soft limit switches in the hardware, then
> we'd want some sort of event chrdev (personally I think these should be separate
> from the main data flow - but that's a detail).
> 
>>>> - File permissions are annoying if you want a non root user to be able to
>>>>    use the device.
> 
> They aren't a whole lot different for a chrdev.  In both cases you can allow
> a non root user write access if you want to.
> 
>>>> - A single program can't claim exclusive access to a device.
> 
> Agreed.  Though that only matters for control, why do you care if someone
> else can read.  In IIO we get around this by 'generally' blocking settings
> changes that will a process that is sampling data via the chrdev.
> It's not a hard and fast rule though.  I really don't like configuration
> via chrdevs as for complex devices you end up with a non self describing
> interface with a lot of complexity.
> 
> The exceptions are things like the media controller frameworks, but they
> are very very heavyweight for an simple devices like counters.
> 
>>>> - There is no automatic cleanup if a userspace program accessing the device
>>>>     crashes.
> 
> For these devices, the question is what sort of cleanup makes sense?
> 
> Often they are freerunning so the most you could do is power down if you knew
> no one cared, but for an encoder you may well want to continue tracking even
> if no one is looking right now.
> 
> I can think of reasons you 'might' want to tidy up, but we'd need real
> driver code to show the necessity of this one.
> 
>>>>
>>>> [1]: https://www.elinux.org/images/7/74/Elce2017_new_GPIO_interface.pdf
>>>
>>> Those look like good technical reasons for implementing a character
>>> device for the Generic Counter interface. I chose to implement the sysfs
>>> interface because I was using the IIO code as a reference, but I
>>> personally don't have much opposition to a character device in addition.
>>
>> IIO is also using a character device for outputting events and sensor
>> data. In IIO sysfs is only used for configuring what events and
>> data should come out of the character device.
> 
> Yes, with the addition that we typically provide data readback as well.
> For some simple devices which are slow and are actually polled to get
> a reading, there is not a lot of point in implementing the chrdev route
> so in IIO it is optional.
>>
>>> I'd like to get Jonathan's opinion on this as well if possible -- I
>>> vaguely recall us considering this option briefly last year when the
>>> Generic Counter interface was still in its beginnings. I've CC'd Linus
>>> Walleij as well for input as the GPIO maintainer.
> 
> I'm not against it, but I do want to see use cases that are not
> satisfied by sysfs first.
> 
> So far we've no seen them but sounds like you might have one David!
> 

Basically, we are implementing a counter in the PRU on TI Sitara, so
we can make it do just about whatever we want. Although, I'm trying to
keep it similar to the eQEP.

Powered by blists - more mailing lists