On 3/7/21 1:04 PM, Jonathan Cameron wrote: > On Sun, 7 Mar 2021 10:40:18 +0100 > Lars-Peter Clausen <lars@xxxxxxxxxx> wrote: > >> On 1/17/21 4:38 PM, Jonathan Cameron wrote: >>> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> >>> >>> 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@xxxxxx> >>> Reported-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> >>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> >>> --- >>> 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 > >
