Re: [PATCH v2] iio: hrtimer: Allow sub Hz granularity

[Jonathan Cameron <jic23@kernel.org>]

On Wed,  9 Dec 2020 14:46:44 -0800
Gwendal Grignou <gwendal@chromium.org> wrote:

> Allow setting frequency below 1Hz or sub 1Hz precision.
> Useful for slow sensors like ALS.
> 
> Test frequency is set properly:
> modprobe iio-trig-hrtimer && \
> mkdir /sys/kernel/config/iio/triggers/hrtimer/t1 && \
> cd /sys/bus/iio/devices/triggerX ;
> for i in 1 .1 .01 .001 ; do
>   echo $i > sampling_frequency
>   cat sampling_frequency
> done
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Just to note as Andy raised the ABI docs question. 
The attribute this is providing is more generic than just for the hrtimer
trigger and as such is in the main ABI docs.  The reason is that
samping_frequency is able to occur in various different places depending
on where it has side effects.  In particular a dataready trigger for
a device with variable sampling frequency will usually be in the
iio:\deviceX directory because it will have affects on anti aliasing filters
so has an impact even if you are just reading from sysfs.
We could add specific docs for this case (though because all ABI docs
for a given named attribute need to be in one file, it would be rather
messy).

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to see if we missed anything.

Thanks,

Jonathan


> ---
> Changes since v1:
> - Added documentation.
> 
>  Documentation/iio/iio_configfs.rst     |  1 +
>  drivers/iio/trigger/iio-trig-hrtimer.c | 25 +++++++++++++++----------
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/iio/iio_configfs.rst b/Documentation/iio/iio_configfs.rst
> index ecbfdb3afef7e..428419f8732c4 100644
> --- a/Documentation/iio/iio_configfs.rst
> +++ b/Documentation/iio/iio_configfs.rst
> @@ -99,3 +99,4 @@ Each trigger can have one or more attributes specific to the trigger type.
>  
>  "hrtimer" trigger type doesn't have any configurable attribute from /config dir.
>  It does introduce the sampling_frequency attribute to trigger directory.
> +That attribute sets the polling frequency in Hz, with mHz precision.
> diff --git a/drivers/iio/trigger/iio-trig-hrtimer.c b/drivers/iio/trigger/iio-trig-hrtimer.c
> index a5e670726717f..06acd6dc79a8c 100644
> --- a/drivers/iio/trigger/iio-trig-hrtimer.c
> +++ b/drivers/iio/trigger/iio-trig-hrtimer.c
> @@ -22,7 +22,7 @@
>  struct iio_hrtimer_info {
>  	struct iio_sw_trigger swt;
>  	struct hrtimer timer;
> -	unsigned long sampling_frequency;
> +	int sampling_frequency[2];
>  	ktime_t period;
>  };
>  
> @@ -38,7 +38,9 @@ ssize_t iio_hrtimer_show_sampling_frequency(struct device *dev,
>  	struct iio_trigger *trig = to_iio_trigger(dev);
>  	struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
>  
> -	return snprintf(buf, PAGE_SIZE, "%lu\n", info->sampling_frequency);
> +	return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO,
> +			ARRAY_SIZE(info->sampling_frequency),
> +			info->sampling_frequency);
>  }
>  
>  static
> @@ -48,18 +50,21 @@ ssize_t iio_hrtimer_store_sampling_frequency(struct device *dev,
>  {
>  	struct iio_trigger *trig = to_iio_trigger(dev);
>  	struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
> -	unsigned long val;
> -	int ret;
> +	unsigned long long val;
> +	int integer, fract, ret;
>  
> -	ret = kstrtoul(buf, 10, &val);
> +	ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
>  	if (ret)
>  		return ret;
>  
> -	if (!val || val > NSEC_PER_SEC)
> +	val = fract + 1000 * integer;
> +
> +	if (!val || val > NSEC_PER_SEC * 1000)
>  		return -EINVAL;
>  
> -	info->sampling_frequency = val;
> -	info->period = NSEC_PER_SEC / val;
> +	info->sampling_frequency[0] = integer;
> +	info->sampling_frequency[1] = fract * 1000;
> +	info->period = NSEC_PER_SEC * 1000 / val;
>  
>  	return len;
>  }
> @@ -135,8 +140,8 @@ static struct iio_sw_trigger *iio_trig_hrtimer_probe(const char *name)
>  	hrtimer_init(&trig_info->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>  	trig_info->timer.function = iio_hrtimer_trig_handler;
>  
> -	trig_info->sampling_frequency = HRTIMER_DEFAULT_SAMPLING_FREQUENCY;
> -	trig_info->period = NSEC_PER_SEC / trig_info->sampling_frequency;
> +	trig_info->sampling_frequency[0] = HRTIMER_DEFAULT_SAMPLING_FREQUENCY;
> +	trig_info->period = NSEC_PER_SEC / trig_info->sampling_frequency[0];
>  
>  	ret = iio_trigger_register(trig_info->swt.trigger);
>  	if (ret)