[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250907112214.36b8564a@jic23-huawei>
Date: Sun, 7 Sep 2025 11:22:14 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>, Nuno Sá
<nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/6] iio: adc: ad7124: add filter support
On Fri, 05 Sep 2025 13:12:00 -0500
David Lechner <dlechner@...libre.com> wrote:
> Add support to the ad7124 driver for selecting the filter type.
>
> The filter type has an influence on the effective sampling frequency of
> each channel. For sinc3+pf{1,2,3,4}, the sampling frequency is fixed.
> For sinc{3,4} (without post filter), there is a factor of 3 or 4
> depending on the filter type. For the extra +sinc1, there is an extra
> averaging factor that depends on the power mode.
>
> In order to select the closest sampling frequency for each filter type,
> we keep a copy of the requested sampling frequency. This way, if the
> user sets the sampling frequency first and then selects the filter type,
> the sampling frequency will still be as close as possible to the
> requested value.
>
> Since we always either have the SINGLE_CYCLE bit set or have more than
> one channel enabled, the sampling frequency is always using the
> "zero-latency" calculation from the data sheet. This is only documented
> for the basic sinc{3,4} filters, so the other filter types had to be
> inferred and confirmed through testing.
>
> Since the flat filter type list consists of multiple register fields,
> the struct ad7124_channel_config::filter_type field is changed to the
> enum ad7124_filter_type type to avoid nested switch statements in a
> lot of places.
>
> Signed-off-by: David Lechner <dlechner@...libre.com>
One really trivial comment inline. Not worth a v2 for just that.
However, this is complex enough code I'd like to keep this on list anyway
for a little longer to see if anyone else has review comments.
Jonathan
> -static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel,
> - unsigned int odr, unsigned int odr_micro)
> +static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel)
> {
> + struct ad7124_channel_config *cfg = &st->channels[channel].cfg;
> unsigned int fclk, factor, odr_sel_bits;
>
> fclk = ad7124_get_fclk_hz(st);
> + factor = ad7124_get_fs_factor(st, channel);
>
> /*
> * FS[10:0] = fCLK / (fADC x 32 * N) where:
> @@ -310,9 +362,9 @@ static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel
> * FS[10:0] are the bits in the filter register
> * FS[10:0] can have a value from 1 to 2047
> */
> - factor = 32 * 4; /* N = 4 for default sinc4 filter. */
> - odr_sel_bits = DIV_ROUND_CLOSEST(fclk, odr * factor +
> - odr_micro * factor / MICRO);
> + odr_sel_bits = DIV_ROUND_CLOSEST(fclk, cfg->requested_odr * factor +
> + cfg->requested_odr_micro *
> + factor / MICRO);
This second parameter is getting complex enough, perhaps worth introducing
a local variable?
divisor = cfg->requested_odr * factor +
cfg->requested_odr_micro * factor / MICRO;
perhaps, or local variables for odr and odr_micro to keep this unchanged in this path.
> odr_sel_bits = clamp(odr_sel_bits, 1, 2047);
>
> if (odr_sel_bits != st->channels[channel].cfg.odr_sel_bits)
Powered by blists - more mailing lists