Re: [RFC PATCH 2/7] iio:ABI docs: Avoid repitition of triggerX/sampling_frequency

[Fabrice Gasnier <fabrice.gasnier@foss.st.com>]

On 3/7/21 1:04 PM, Jonathan Cameron wrote:
> On Sun, 7 Mar 2021 10:40:18 +0100
> Lars-Peter Clausen <lars@metafoo.de> wrote:
> 
>> On 1/17/21 4:38 PM, Jonathan Cameron wrote:
>>> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>>
>>> The timer-stm32 provided a little more specific information than the main
>>> docs about the value of 0 corresponding to stopping sampling.  Given that
>>> this makes sense in general, move that statement over to the main docs
>>> and drop the version in sysfs-bus-iio-timer-stm32
>>>
>>> Fixes
>>> $ scripts/get_abi.pl validate
>>> /sys/bus/iio/devices/triggerX/sampling_frequency is defined 2 times:  ./Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:92  ./Documentation/ABI/testing/sysfs-bus-iio:45
>>>
>>> Cc: Fabrice Gasnier <fabrice.gasnier@st.com>
>>> Reported-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>> ---
>>>   Documentation/ABI/testing/sysfs-bus-iio             | 2 ++
>>>   Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 | 8 --------
>>>   2 files changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>>> index d2dd9cc280f9..9b5ceb22363d 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>>> @@ -55,6 +55,8 @@ Description:
>>>   		direct access interfaces, it may be found in any of the
>>>   		relevant directories.  If it affects all of the above
>>>   		then it is to be found in the base device directory.
>>> +		Note a value 0 where supported, will correspond to sampling
>>> +		stopping.  
>>
>> Hm, I don't like this. 0 should be an invalid value for the standard 
>> ABI. Enabling/disabling of the trigger should be controlled by whether 
>> there are any active trigger consumers.
>>
>> The stm32-timer-trigger implements a non-standard ABI and I think we 
>> need to document this explicitly. The driver does not have a set_state 
>> callback. So the frequency property is used to enable/disable the 
>> trigger, but as said above, for standard compliant triggers that should 
>> not be the case.
>>
> Good point.  Ideally we'd also fix the stm32-timer-trigger to have
> an explicit enable / disable (even if that works by setting the frequency
> to 0 under the hook)

Hi Jonathan,

I'd like to come up with something here...
The trivial use case is: the stm32-timer triggers the stm32-adc (by HW).
In this case, "set_trigger_state" perfectly fits the needs (looks like).

But such change doesn't seems trivial when considering one timer can
trig another timer in hardware here. The "set_trigger_state" isn't
called in such a case.
So, I'm not sure what could be suitable to implement explicit
enable/disable of the timer trigger ?

Just to share some thoughts here: For sure the stm32-timer-trigger would
need additional changes (I haven't forget other mail thread on the
preset attribute). I believe also some functionalities should be moved
to the counter framework. But, even provided this, I think there will
still be a need for an enable or disable attribute, for the trigger part.

Perhaps adding an enable/disable attribute could be a way here ?
If yes, would you have some suggestion on the naming ?

Please advice,
Thanks in advance,
Fabrice

> 
> For now I've dropped the patch until this is resolved.
> 
> Thanks,
> 
> Jonathan
> 
>