Re: [PATCH v3 1/3] iio: configfs: Add configfs support to IIO

[Jonathan Cameron]

On 22/04/15 21:34, Octavian Purdila wrote:
> On Sat, Apr 18, 2015 at 12:30 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>>
>> On 15/04/15 23:15, Octavian Purdila wrote:
>>> On Wed, Apr 15, 2015 at 2:34 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>>>> On 15/04/15 21:58, Octavian Purdila wrote:
>>>>> On Sun, Apr 12, 2015 at 8:59 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>>>>>> On 10/04/15 14:43, Daniel Baluta wrote:
>>>>>>> On Thu, Apr 9, 2015 at 8:18 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>>>>>>>> On 08/04/15 14:30, Daniel Baluta wrote:
>>>>>>>>> On Mon, Apr 6, 2015 at 5:18 PM, Daniel Baluta <daniel.baluta@xxxxxxxxx> wrote:
>>>>>>>>>> This module is the core of IIO configfs. It creates the "iio" subsystem under
>>>>>>>>>> configfs mount point root, with one default group for "triggers".
>>>>>>>>>>
>>>>>>>>>> It offers basic interface for registering software trigger types. Next patches
>>>>>>>>>> will add "hrtimer" and "sysfs" trigger types. To add a new trigger type we must
>>>>>>>>>> create a new kernel module which implements iio_configfs_trigger.h interface.
>>>>>>>>>>
>>>>>>>>>> See Documentation/iio/iio_configfs.txt for more details on how configfs
>>>>>>>>>> support for IIO works.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Daniel Baluta <daniel.baluta@xxxxxxxxx>
>>>>>>>>>
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> I also need your feedback on the following problem.
>>>>>>>>>
>>>>>>>>> We would like to be able to create hrtimer triggers from within
>>>>>>>>> a kernel module. There are cases where we don't have an interrupt
>>>>>>>>> pin or the interrupt pin is not connected, and we would like
>>>>>>>>> that applications to run unmodified with these types of sensors too.
>>>>>>>> A reasonable aim perhaps, as opposed to locally implemented polling
>>>>>>>> all over the place (which should definitely not happen).
>>>>>>>
>>>>>>> Yes, exactly! :)
>>>>>> I'm actually beginning to have my doubts about whether my initial
>>>>>> response here was right.  Given your use cases and the complexity
>>>>>> of matching frequencies, I think you are almost always better
>>>>>> off implementing it in the drivers themselves (which can decide
>>>>>> if there is new data or not).
>>>>>
>>>>> I agree. Although I think having some helper functions in IIO core
>>>>> should help keep the driver modifications to a minimum.
>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> An alternative the zio guys used was to create timer
>>>>>>>> based triggers on the fly purely based on them being requested
>>>>>>>> (with an appropriate name IIRC)...  Doesn't quite solve your
>>>>>>>> problem though as still needs userspace changes.
>>>>>>>>>
>>>>>>>>> We will split the current design into 3 parts:
>>>>>>>>>
>>>>>>>>> (1) IIO trigger handling generic part, which offers an API
>>>>>>>>>      to register/unregister/get a reference to a trigger type
>>>>>>>>>
>>>>>>>>> (2) IIO configfs part that gets a reference to a trigger type and
>>>>>>>>> handles user requests to create/destroy a trigger.
>>>>>>>>>
>>>>>>>>> (3) IIO hrtimer driver that use the API at (1) for registering
>>>>>>>>> / deregistering a trigger type.
>>>>>>>>>
>>>>>>>>> Then, each driver in the case that it doesn't have a "real" trigger,
>>>>>>>>> will get a reference to a "hrtimer" trigger type and create
>>>>>>>>> a new "hrtimer" trigger.
>>>>>>>>>
>>>>>>>>> Does this look good to you? This could be easily done from
>>>>>>>>> userspace, but we will need to modify our userspace applications.
>>>>>>>> My initial thought is this really is a job for userspace, as should
>>>>>>>> be in most cases connecting up the device specific trigger as well
>>>>>>>> (as it's not always the right thing to use).
>>>>>>>>
>>>>>>>> In the general case it is far from obvious that an hrtimer is the
>>>>>>>> right option.  Many usecases would be better off with a sysfs trigger
>>>>>>>> or even running off a different interrupt based trigger entirely.
>>>>>>>>>
>>>>>>>>> Also, handling sampling_frequency/delay would be transparent to
>>>>>>>>> applications if we provide this in kernel API.
>>>>>>>> Not really as sampling frequency in this case should only be an
>>>>>>>> attribute of the trigger, not the device.  We only really allow
>>>>>>>> it for the device rather than always the triggers on the basis
>>>>>>>> that it has impacts on other device elements (events etc).
>>>>>>>
>>>>>>> Well, if the trigger is directly created from the driver then we will
>>>>>>> have a reference to a function that sets its delay.
>>>>>>>
>>>>>>> Each time the user sets the sampling frequency for the device
>>>>>>> with hit write_raw and there on IIO_CHAN_INFO_SAMP_FREQ
>>>>>>> case we also call set_delay(). Thus we always have have device
>>>>>>> sampling frequency in sync with trigger delay.
>>>>>>>
>>>>>>> I know this sounds crazy :) and I don't like it. I am not sure how
>>>>>>> to guarantee that device frequency is always in sync with trigger
>>>>>>> delay.
>>>>>> You can't that I can see.  Hence you've convinced me this is a bad
>>>>>> idea.
>>>>>
>>>>> I agree we can't accurately synchronize the device frequency with the
>>>>> trigger frequency, but I think we can get it do a good enough level. I
>>>>> also see no difference in accuracy in doing it from userspace or from
>>>>> the driver itself.
>>>>>
>>>>> Why do you think it is a bad idea to synchronize the trigger sample
>>>>> frequency from the device sample frequency handler? I think that if we
>>>>> want to provide the hrtimer trigger from the driver itself, we should
>>>>> do this as well, otherwise there is no real gain to userspace.
>>>>>
>>>> You will get beating problems.  So once in a while you'll either miss
>>>> a reading or you'll get a bonus one.
>>>>
>>>> The point about doing it in the driver is that you actually poll at a
>>>> higher frequency, but only fire a trigger if there is something there
>>>> (by reading the dataready register or equivalent).
>>>>
>>>> Thus your trigger acts more or less the same as a wired interrupt.
>>>> You can't do this with an external hrtimer trigger.
>>>
>>> Polling will have a large impact on power. I know that some people
>>> will care more about saving power and that is why I think its worth
>>> having the external hrtimer trigger option.
>>>
>> Two different use cases...
>>
>> Either you are interested in 'faking' the data ready signal, in which case polling
>> is the way to go as nothing else will get you there.  Note that ranged sleeps can be
>> used to limit the power usage pain.
>>
> 
> Sure, but ranged sleeps will use a hrtimer anyway :) And because of
> that we have no guarantee that we will not miss data.
Should be fine if you poll at double the sampling frequency.. or at least
it will loose samples very rarely.
> 
>> or you are interested in much lower frequency sampling in which case you use
>> an hrtimer trigger, but run it at way below the sample rate.  You will always
>> get new data, but there is no control of when exactly that data was grabbed
>> and you will loose samples.
>>
>> The first was what I thought we were discussing...
>>
>> Any attempt to synchronize timing is a non starter as it will beat as any two
>> similar frequencies will so you will loose data.
>>
> 
> Maybe synchronizing is a too stronger word here. What I meant was that
> when the device sampling frequency is changed to also change the
> hrtimer interval too (basically to 1/freq).
> 
> Yes, we will occasionally loose data but it is the best we can do if
> we don't have the interrupt available. Fully polling will waste too
> much power and ranged sleeps can still lead to missing samples.
Would polling at double the sampling frequency prove too power consuming?
That we can't do with a straight hrtimer trigger, but can be easily done
if we do it in driver (with out ever pushing missing data or doubled
data to userspace - assuming the device has a new data element that is
poll-able).

If you try to match you either get double samples (which might
be detectable, but we have no facility to indicate it in the buffer),
or half samples.

If anyone is trying to do integration either case could leave them
with a comically incorrect answer if undetected. It's bad enough even
with clean well timed data!

J

> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/