On Mon, 13 Jan 2025 11:49:49 -0500
Trevor Gamblin <tgamblin@xxxxxxxxxxxx> wrote:
> On 2025-01-13 09:35, Nuno Sá wrote:
> > On Thu, 2025-01-09 at 13:47 -0500, Trevor Gamblin wrote:
> >> Add support for the ad4695's oversampling feature when SPI offload is
> >> available. This allows the ad4695 to set oversampling ratios on a
> >> per-channel basis, raising the effective-number-of-bits from 16
> >> (OSR == 1) to 17 (4), 18 (16), or 19 (64) for a given sample (i.e. one
> >> full cycle through the auto-sequencer). The logic for reading and
> >> writing sampling frequency for a given channel is also adjusted based on
> >> the current oversampling ratio.
> >>
> >> The non-offload case isn't supported as there isn't a good way to
> >> trigger the CNV pin in this mode. Support could be added in the future
> >> if a use-case arises.
> >>
> >> Signed-off-by: Trevor Gamblin <tgamblin@xxxxxxxxxxxx>
> >> ---
> > LGTM, just one small thing inline... Either way:
> >
> > Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> >
> >> drivers/iio/adc/ad4695.c | 333 ++++++++++++++++++++++++++++++++++++++++++----
> >> -
> >> 1 file changed, 303 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
> >> index c8cd73d19e86..0caaeaa310ed 100644
> >> --- a/drivers/iio/adc/ad4695.c
> >> +++ b/drivers/iio/adc/ad4695.c
> >> @@ -79,6 +79,7 @@
> >> #define AD4695_REG_CONFIG_IN_MODE BIT(6)
> >> #define AD4695_REG_CONFIG_IN_PAIR GENMASK(5, 4)
> >> #define AD4695_REG_CONFIG_IN_AINHIGHZ_EN BIT(3)
> >> +#define AD4695_REG_CONFIG_IN_OSR_SET GENMASK(1, 0)
> >> #define AD4695_REG_UPPER_IN(n) (0x0040 | (2 * (n)))
> >> #define AD4695_REG_LOWER_IN(n) (0x0060 | (2 * (n)))
> >> #define AD4695_REG_HYST_IN(n) (0x0080 | (2 * (n)))
> >> @@ -127,6 +128,7 @@ struct ad4695_channel_config {
> >> bool bipolar;
> >> enum ad4695_in_pair pin_pairing;
> >> unsigned int common_mode_mv;
> >> + unsigned int oversampling_ratio;
> >> };
> >>
> > ...
> >
> >> +
> >> +static unsigned int ad4695_get_calibbias(int val, int val2, int osr)
> >> +{
> >> + int val_calc, scale;
> >> +
> >> + switch (osr) {
> >> + case 4:
> >> + scale = 4;
> >> + break;
> >> + case 16:
> >> + scale = 2;
> >> + break;
> >> + case 64:
> >> + scale = 1;
> >> + break;
> >> + default:
> >> + scale = 8;
> >> + break;
> >> + }
> >> +
> >> + val = clamp_t(int, val, S32_MIN / 8, S32_MAX / 8);
> >> +
> > Why not clamp()? AFAICS, we have the same type on all the arguments. I also
> > think clamp*() macros got the same improvements as min/max() ones which means
> > that using the ones with explicit casts are not so often needed anymore. My
> > understanding is also that those macros are not that encouraged as it's easy to
> > go wrong with the casts.
> I have no preference, this is just a recent habitual use of clamp_t(). If
> clamp() is preferred I can send a v3. Or maybe Jonathan can tweak it
> when it is
I've left it as clamp_T for now. We can always follow up with a series
to tidy these up general.
Series applied though a bit provisionally as the SPI offload set needed
a few tweaks that might get changed.
Pushed out as testing for now.
Thanks,
Jonathan
>
> eventually applied?
>
> - Trevor
>
> >
> > - Nuno Sá
