Message-ID: <552A965B.7030108@kernel.org>
Date:	Sun, 12 Apr 2015 16:59:23 +0100
From:	Jonathan Cameron <jic23@...nel.org>
To:	Daniel Baluta <daniel.baluta@...el.com>
CC:	Joel Becker <jlbec@...lplan.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Hartmut Knaack <knaack.h@....de>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"octavian.purdila@...el.com" <octavian.purdila@...el.com>,
	Paul Bolle <pebolle@...cali.nl>, patrick.porlan@...el.com,
	adriana.reus@...el.com
Subject: Re: [PATCH v3 1/3] iio: configfs: Add configfs support to IIO

On 10/04/15 14:43, Daniel Baluta wrote:
> On Thu, Apr 9, 2015 at 8:18 PM, Jonathan Cameron <jic23@...nel.org> wrote:
>> On 08/04/15 14:30, Daniel Baluta wrote:
>>> On Mon, Apr 6, 2015 at 5:18 PM, Daniel Baluta <daniel.baluta@...el.com> 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@...el.com>
>>>
>>> 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).
> 
>>
>> 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.
> 
>>
>> Now sensible userspace ought to search for the trigger sysfs
>> directory and look there as well.
>>
>> I suppose if there is an awful lot of demand for this function
>> we could add a control knob somewhere that would set a 'default
>> trigger type' to be created for buffered devices that haven't
>> provided their own...  I'm not overly keen though.
>>>
> 
> This functionality would be very nice and useful, lets see if we can
> find some elegant way to do it.
I think this may well still be worth having, just not for your
particular case.  It would be useful for the devices that have
a 'sample now' signal of some type that allows the trigger to
control when they are sampling (and hence always match perfectly!)

J
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@...r.kernel.org
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists