IIO hrtimer trigger

IIO hrtimer trigger

[Denis CIOCCA]

Hi Lars,

Thanks for your review.
I reviewed the code in accordance with your comments, for the other point
can you explain me better please?
You intend to use one driver to manage all triggers added by sysfs?

Thanks,
Denis

Re: IIO hrtimer trigger

[Lars-Peter Clausen]

On 09/29/2013 09:36 PM, Denis CIOCCA wrote:
> Hi Lars,
> 
> Thanks for your review.
> I reviewed the code in accordance with your comments, for the other point
> can you explain me better please?
> You intend to use one driver to manage all triggers added by sysfs?

Not necessarily, but I think we should have some common code that manages
the software triggers. But what I'm most concerned about is the userspace
ABI, since once we have added it, we have to maintain it forever. So the big
question do we think that the current ABI implemented by that patch is good
enough. Some thoughts:

* Should it maybe be called timer instead of hrtimer.
* Do we only want to allow names which follow "hrtimer-%d" or do we want to
allow arbitrary names.
* Do we want to have a top-level folder for each sw trigger type
* Is sysfs actually the right place for this, or should it go into configfs?
  Quote from Documentation/filesystems/configs:
  "configfs is a ram-based filesystem that provides the converse of
   sysfs's functionality.  Where sysfs is a filesystem-based view of
   kernel objects, configfs is a filesystem-based manager of kernel
   objects, or config_items. [...] Unlike sysfs, the
   lifetime of the representation is completely driven by userspace.  The
   kernel modules backing the items must respond to this."

I think especially the last one deserves some though.

- Lars

Re: IIO hrtimer trigger

[Jonathan Cameron]

On 10/03/13 18:10, Lars-Peter Clausen wrote:
> On 09/29/2013 09:36 PM, Denis CIOCCA wrote:
>> Hi Lars,
>>
>> Thanks for your review.
>> I reviewed the code in accordance with your comments, for the other point
>> can you explain me better please?
>> You intend to use one driver to manage all triggers added by sysfs?
> 
> Not necessarily, but I think we should have some common code that manages
> the software triggers.
That is fair enough.

> But what I'm most concerned about is the userspace
> ABI, since once we have added it, we have to maintain it forever. So the big
> question do we think that the current ABI implemented by that patch is good
> enough.
We are pretty much stuck with that for the sysfs trigger already...

> Some thoughts:
> 
> * Should it maybe be called timer instead of hrtimer.
Agreed.
> * Do we only want to allow names which follow "hrtimer-%d" or do we want to
> allow arbitrary names.
Arbitary would be fine.
> * Do we want to have a top-level folder for each sw trigger type
I'm not that bothered about this we are hardly talking a huge number of such
folders.
> * Is sysfs actually the right place for this, or should it go into configfs?
>   Quote from Documentation/filesystems/configs:
>   "configfs is a ram-based filesystem that provides the converse of
>    sysfs's functionality.  Where sysfs is a filesystem-based view of
>    kernel objects, configfs is a filesystem-based manager of kernel
>    objects, or config_items. [...] Unlike sysfs, the
>    lifetime of the representation is completely driven by userspace.  The
>    kernel modules backing the items must respond to this."
Hmm. maybe, I'm not sure how cleanly this would work and it adds an additional
dependency for all these types of drivers.  I'll take the lazy option:
Go on Lars, put together a full proposal on the actual interface ;)

Another vague thought was the on demand creation of timer based triggers
that I think zio provides.  Basically if a non existent trigger is requested
the subsystem figures out what is requested and creates it.  Not terribly
nice to implement, and to my mind unnecessary and possibly confusing...

Jonathan
> 
> I think especially the last one deserves some though.
> 
> - Lars
> 
> 
> 

Re: IIO hrtimer trigger

[Lars-Peter Clausen]

On 10/06/2013 08:15 PM, Jonathan Cameron wrote:
> On 10/03/13 18:10, Lars-Peter Clausen wrote:
>> On 09/29/2013 09:36 PM, Denis CIOCCA wrote:
>>> Hi Lars,
>>>
>>> Thanks for your review.
>>> I reviewed the code in accordance with your comments, for the other point
>>> can you explain me better please?
>>> You intend to use one driver to manage all triggers added by sysfs?
>>
>> Not necessarily, but I think we should have some common code that manages
>> the software triggers.
> That is fair enough.
> 
>> But what I'm most concerned about is the userspace
>> ABI, since once we have added it, we have to maintain it forever. So the big
>> question do we think that the current ABI implemented by that patch is good
>> enough.
> We are pretty much stuck with that for the sysfs trigger already...

Unfortunately yes. I never liked its API and I still don't like it and we
have to live with it. But this doesn't mean we have to add more of the same.

> 
>> Some thoughts:
>>
>> * Should it maybe be called timer instead of hrtimer.
> Agreed.
>> * Do we only want to allow names which follow "hrtimer-%d" or do we want to
>> allow arbitrary names.
> Arbitary would be fine.
>> * Do we want to have a top-level folder for each sw trigger type
> I'm not that bothered about this we are hardly talking a huge number of such
> folders.
>> * Is sysfs actually the right place for this, or should it go into configfs?
>>   Quote from Documentation/filesystems/configs:
>>   "configfs is a ram-based filesystem that provides the converse of
>>    sysfs's functionality.  Where sysfs is a filesystem-based view of
>>    kernel objects, configfs is a filesystem-based manager of kernel
>>    objects, or config_items. [...] Unlike sysfs, the
>>    lifetime of the representation is completely driven by userspace.  The
>>    kernel modules backing the items must respond to this."
> Hmm. maybe, I'm not sure how cleanly this would work and it adds an additional
> dependency for all these types of drivers.  I'll take the lazy option:
> Go on Lars, put together a full proposal on the actual interface ;)

I'll do that but that might take a few weeks until I get to it.

> 
> Another vague thought was the on demand creation of timer based triggers
> that I think zio provides.  Basically if a non existent trigger is requested
> the subsystem figures out what is requested and creates it.  Not terribly
> nice to implement, and to my mind unnecessary and possibly confusing...
> 

I don't think that would work to nicely in our case.

> Jonathan
>>
>> I think especially the last one deserves some though.
>>
>> - Lars

Re: IIO hrtimer trigger

[Jonathan Cameron]

On 07/10/13 15:18, Lars-Peter Clausen wrote:
> On 10/06/2013 08:15 PM, Jonathan Cameron wrote:
>> On 10/03/13 18:10, Lars-Peter Clausen wrote:
>>> On 09/29/2013 09:36 PM, Denis CIOCCA wrote:
>>>> Hi Lars,
>>>>
>>>> Thanks for your review.
>>>> I reviewed the code in accordance with your comments, for the other point
>>>> can you explain me better please?
>>>> You intend to use one driver to manage all triggers added by sysfs?
>>>
>>> Not necessarily, but I think we should have some common code that manages
>>> the software triggers.
>> That is fair enough.
>>
>>> But what I'm most concerned about is the userspace
>>> ABI, since once we have added it, we have to maintain it forever. So the big
>>> question do we think that the current ABI implemented by that patch is good
>>> enough.
>> We are pretty much stuck with that for the sysfs trigger already...
>
> Unfortunately yes. I never liked its API and I still don't like it and we
> have to live with it. But this doesn't mean we have to add more of the same.
>
>>
>>> Some thoughts:
>>>
>>> * Should it maybe be called timer instead of hrtimer.
>> Agreed.
>>> * Do we only want to allow names which follow "hrtimer-%d" or do we want to
>>> allow arbitrary names.
>> Arbitary would be fine.
>>> * Do we want to have a top-level folder for each sw trigger type
>> I'm not that bothered about this we are hardly talking a huge number of such
>> folders.
>>> * Is sysfs actually the right place for this, or should it go into configfs?
>>>    Quote from Documentation/filesystems/configs:
>>>    "configfs is a ram-based filesystem that provides the converse of
>>>     sysfs's functionality.  Where sysfs is a filesystem-based view of
>>>     kernel objects, configfs is a filesystem-based manager of kernel
>>>     objects, or config_items. [...] Unlike sysfs, the
>>>     lifetime of the representation is completely driven by userspace.  The
>>>     kernel modules backing the items must respond to this."
>> Hmm. maybe, I'm not sure how cleanly this would work and it adds an additional
>> dependency for all these types of drivers.  I'll take the lazy option:
>> Go on Lars, put together a full proposal on the actual interface ;)
>
> I'll do that but that might take a few weeks until I get to it.
Bump.  Do we want to still wait for this, or should we just go ahead with the
hrtimer as is.  It may not be ideal, but it's useful and lets us kill off
some much worse options..
>
>>
>> Another vague thought was the on demand creation of timer based triggers
>> that I think zio provides.  Basically if a non existent trigger is requested
>> the subsystem figures out what is requested and creates it.  Not terribly
>> nice to implement, and to my mind unnecessary and possibly confusing...
>>
>
> I don't think that would work to nicely in our case.
>
>> Jonathan
>>>
>>> I think especially the last one deserves some though.
>>>
>>> - Lars

Re: IIO hrtimer trigger

[Lars-Peter Clausen]

On 06/18/2014 09:47 PM, Jonathan Cameron wrote:
> On 07/10/13 15:18, Lars-Peter Clausen wrote:
>> On 10/06/2013 08:15 PM, Jonathan Cameron wrote:
>>> On 10/03/13 18:10, Lars-Peter Clausen wrote:
>>>> On 09/29/2013 09:36 PM, Denis CIOCCA wrote:
>>>>> Hi Lars,
>>>>>
>>>>> Thanks for your review.
>>>>> I reviewed the code in accordance with your comments, for the other point
>>>>> can you explain me better please?
>>>>> You intend to use one driver to manage all triggers added by sysfs?
>>>>
>>>> Not necessarily, but I think we should have some common code that manages
>>>> the software triggers.
>>> That is fair enough.
>>>
>>>> But what I'm most concerned about is the userspace
>>>> ABI, since once we have added it, we have to maintain it forever. So the
>>>> big
>>>> question do we think that the current ABI implemented by that patch is good
>>>> enough.
>>> We are pretty much stuck with that for the sysfs trigger already...
>>
>> Unfortunately yes. I never liked its API and I still don't like it and we
>> have to live with it. But this doesn't mean we have to add more of the same.
>>
>>>
>>>> Some thoughts:
>>>>
>>>> * Should it maybe be called timer instead of hrtimer.
>>> Agreed.
>>>> * Do we only want to allow names which follow "hrtimer-%d" or do we want to
>>>> allow arbitrary names.
>>> Arbitary would be fine.
>>>> * Do we want to have a top-level folder for each sw trigger type
>>> I'm not that bothered about this we are hardly talking a huge number of such
>>> folders.
>>>> * Is sysfs actually the right place for this, or should it go into
>>>> configfs?
>>>>    Quote from Documentation/filesystems/configs:
>>>>    "configfs is a ram-based filesystem that provides the converse of
>>>>     sysfs's functionality.  Where sysfs is a filesystem-based view of
>>>>     kernel objects, configfs is a filesystem-based manager of kernel
>>>>     objects, or config_items. [...] Unlike sysfs, the
>>>>     lifetime of the representation is completely driven by userspace.  The
>>>>     kernel modules backing the items must respond to this."
>>> Hmm. maybe, I'm not sure how cleanly this would work and it adds an
>>> additional
>>> dependency for all these types of drivers.  I'll take the lazy option:
>>> Go on Lars, put together a full proposal on the actual interface ;)
>>
>> I'll do that but that might take a few weeks until I get to it.
> Bump.  Do we want to still wait for this, or should we just go ahead with the
> hrtimer as is.  It may not be ideal, but it's useful and lets us kill off
> some much worse options..

I'd really prefer to not compromise on user-space ABI. Quick hacks are 
sometimes fine for kernel internal things since we can clean them up later. 
But for userspace ABI we'll be stuck with it forever.

Re: IIO hrtimer trigger

[Jonathan Cameron]

On June 19, 2014 11:57:50 AM GMT+01:00, Lars-Peter Clausen <lars@metafoo.de> wrote:
>On 06/18/2014 09:47 PM, Jonathan Cameron wrote:
>> On 07/10/13 15:18, Lars-Peter Clausen wrote:
>>> On 10/06/2013 08:15 PM, Jonathan Cameron wrote:
>>>> On 10/03/13 18:10, Lars-Peter Clausen wrote:
>>>>> On 09/29/2013 09:36 PM, Denis CIOCCA wrote:
>>>>>> Hi Lars,
>>>>>>
>>>>>> Thanks for your review.
>>>>>> I reviewed the code in accordance with your comments, for the
>other point
>>>>>> can you explain me better please?
>>>>>> You intend to use one driver to manage all triggers added by
>sysfs?
>>>>>
>>>>> Not necessarily, but I think we should have some common code that
>manages
>>>>> the software triggers.
>>>> That is fair enough.
>>>>
>>>>> But what I'm most concerned about is the userspace
>>>>> ABI, since once we have added it, we have to maintain it forever.
>So the
>>>>> big
>>>>> question do we think that the current ABI implemented by that
>patch is good
>>>>> enough.
>>>> We are pretty much stuck with that for the sysfs trigger already...
>>>
>>> Unfortunately yes. I never liked its API and I still don't like it
>and we
>>> have to live with it. But this doesn't mean we have to add more of
>the same.
>>>
>>>>
>>>>> Some thoughts:
>>>>>
>>>>> * Should it maybe be called timer instead of hrtimer.
>>>> Agreed.
>>>>> * Do we only want to allow names which follow "hrtimer-%d" or do
>we want to
>>>>> allow arbitrary names.
>>>> Arbitary would be fine.
>>>>> * Do we want to have a top-level folder for each sw trigger type
>>>> I'm not that bothered about this we are hardly talking a huge
>number of such
>>>> folders.
>>>>> * Is sysfs actually the right place for this, or should it go into
>>>>> configfs?
>>>>>    Quote from Documentation/filesystems/configs:
>>>>>    "configfs is a ram-based filesystem that provides the converse
>of
>>>>>     sysfs's functionality.  Where sysfs is a filesystem-based view
>of
>>>>>     kernel objects, configfs is a filesystem-based manager of
>kernel
>>>>>     objects, or config_items. [...] Unlike sysfs, the
>>>>>     lifetime of the representation is completely driven by
>userspace.  The
>>>>>     kernel modules backing the items must respond to this."
>>>> Hmm. maybe, I'm not sure how cleanly this would work and it adds an
>>>> additional
>>>> dependency for all these types of drivers.  I'll take the lazy
>option:
>>>> Go on Lars, put together a full proposal on the actual interface ;)
>>>
>>> I'll do that but that might take a few weeks until I get to it.
>> Bump.  Do we want to still wait for this, or should we just go ahead
>with the
>> hrtimer as is.  It may not be ideal, but it's useful and lets us kill
>off
>> some much worse options..
>
>I'd really prefer to not compromise on user-space ABI. Quick hacks are 
>sometimes fine for kernel internal things since we can clean them up
>later. 
>But for userspace ABI we'll be stuck with it forever.
I am slightly less bothered by this than normal because we obliged to keep the sysfs
 trigger interface around indefinitely anyway and this would be much the same.

 I am yet to be convinced that dragging in configfs makes sense...

Still, other purpose of restarting this thread was to hope to move it forward!

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: IIO hrtimer trigger

[Ezequiel Garcia]

Hello Lars,

I'm picking this discussion, since we're also interested in using an hrtimer
trigger.

> On 10/06/2013 08:15 PM, Jonathan Cameron wrote:
>> We are pretty much stuck with that for the sysfs trigger already...
>
> Unfortunately yes. I never liked its API and I still don't like it and we
> have to live with it. But this doesn't mean we have to add more of the same.

So the reason why the hrtimer trigger hasn't been merged in its current form is
because we're still discussing the ABI, right?

Can you elaborate on why you don't like the sysfs trigger API? If you can give
me some hints I can try to cook a patch for the configfs hrtimer trigger.

Thanks!
Ezequiel

Re: IIO hrtimer trigger

[Lars-Peter Clausen]

On 09/05/2014 08:22 PM, Ezequiel Garcia wrote:
> Hello Lars,
>
> I'm picking this discussion, since we're also interested in using an hrtimer
> trigger.
>
>> On 10/06/2013 08:15 PM, Jonathan Cameron wrote:
>>> We are pretty much stuck with that for the sysfs trigger already...
>>
>> Unfortunately yes. I never liked its API and I still don't like it and we
>> have to live with it. But this doesn't mean we have to add more of the same.
>
> So the reason why the hrtimer trigger hasn't been merged in its current form is
> because we're still discussing the ABI, right?

I think the configfs documentation explains this quite well:

"Where sysfs is a filesystem-based view of kernel objects, configfs is a 
filesystem-based manager of kernel objects, or config_items." [1]

>
> Can you elaborate on why you don't like the sysfs trigger API? If you can give
> me some hints I can try to cook a patch for the configfs hrtimer trigger.

That would be great. I unfortunately only have a very foggy view of how the 
ABI and API should look like.

But I think the basic interface is have a toplevel iio configfs folder, 
maybe a subfolder called "triggers" and than a subfolder for each software 
trigger. E.g. "timer" (We shouldn't call it hrtimer cause that is just a 
implementation detail). Then in those trigger folders you can do mkdir to 
create a new instance of that trigger. In the folder there might be trigger 
specific config settings exposed as files.

- Lars

[1] https://www.kernel.org/doc/Documentation/filesystems/configfs/configfs.txt