On 2025-01-07 16:02, David Lechner wrote:
On 1/7/25 2:21 PM, Trevor Gamblin wrote:
On 2025-01-04 07:30, Jonathan Cameron wrote:
On Thu, 2 Jan 2025 13:19:19 -0500
Trevor Gamblin <tgamblin@xxxxxxxxxxxx> wrote:
On 2024-12-19 11:13, Jonathan Cameron wrote:
On Tue, 17 Dec 2024 16:47:28 -0500
Trevor Gamblin <tgamblin@xxxxxxxxxxxx> 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>
...
Maybe trick is to reorder into 3 conditions and set the value in a temporary integer.
int val_calc;
if (val > 0)
val_calc = val * 2 + val2 * 2 / MICRO;
else if (val < 0)
val_calc = -(val * 2 - val2 * 2 / MICRO);
else /* Only now does val2 sign matter as val == 0 */
val_calc = val2 * 2 / MICRO;
I've been testing out these simplifications (but using the scaling suggestion from below, which is great - for some reason I had it in my head that doing so wasn't an option).
These seem to have some issues with signs for particularly small calibbias values. I think it's because while my (val2 < 0) case was doing unnecessary clamping, the math itself was OK.
Mail is easier to read when wrapped to 80 chars. ;-)
My bad.
I did some more experimenting, and came up with a new version of the function that looks like this:
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;
}
/* Note that val2 > 0 if val != 0 and val2 range +- MICRO */
This comment doesn't seem 100% accurate. val2 range is (-MICRO, MICRO) if
val == 0 or [0, MICRO) if val != 0.
Alright, will fix this.
if (val < 0)
val_calc = val * scale - val2 * scale / MICRO;
else if (val2 < 0)
/* if val2 < 0 then val == 0 */
val_calc = -(-val2 * scale / MICRO);
Could also write this as `val2 * scale / (int)MICRO` lest someone try to remove
the double negative and break it (because MICRO is unsigned).
And this.
This also calls into question if MICRO and similar macros should actually be
unsigned because it can lead to subtle bugs since it is perfectly reasonable
to expect -1 * MICRO to be -1000000, but it isn't.
else
val_calc = val * scale + val2 * scale / MICRO;
val_calc /= 2;
return clamp_t(int, val_calc, S16_MIN, S16_MAX);
}
This seems to match all of the expected outputs for the pre-simplification version in this patch series when I test it. If there are no issues with it, I'll send a v2.
Probably not a big deal, but there is unhanded overflow when val is near S32_MAX
or S32_MIN.
Should I handle that with an extra call to clamp_t()?
