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

[Gwendal Grignou <gwendal@chromium.org>]

On Tue, Jan 12, 2021 at 6:22 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 6:09 AM 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 && \
>
> This && seems strange...
>
> > cd /sys/bus/iio/devices/triggerX ;
This is just for testing. X in triggerX should be replaced with the
newly created trigger.
>
> ...because if the above fails, the below becomes a total train wreck.
>
> > for i in 1 .1 .01 .001 ; do
> >   echo $i > sampling_frequency
> >   cat sampling_frequency
> > done
>
> Something like this perhaps (note {} and also I dropped unneeded ; and
> whitespace)
>   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
>   }
>
> ...
>
> > +       if (!val || val > NSEC_PER_SEC * 1000)
> >                 return -EINVAL;
>
> > +       info->period = NSEC_PER_SEC * 1000;
>
> I didn't get these * 1000 parts, why not define and use PSEC_PER_SEC?
Indeed only Nano and Femto SEC_PER_SEC are defined.

>
> I'll send a patch soon. You may include it in your series as prerequisite.
Thanks,

Gwendal.
>
> --
> With Best Regards,
> Andy Shevchenko