On Sat, 8 Apr 2023 13:48:24 +0200
Patrik Dahlström <risca@xxxxxxxxxxxxxx> wrote:
> The palmas gpadc block has support for monitoring up to 2 ADC channels
> and issue an interrupt if they reach past a set threshold. This change
> hooks into the IIO events system and exposes to userspace the ability to
> configure these threshold values for each channel, but only allow up to
> 2 such thresholds to be enabled at any given time. Trying to enable a
> third channel will result in an error.
>
> Userspace is expected to input calibrated, as opposed to raw, values as
> threshold. However, it is not enough to do the opposite of what is done
> when converting the other way around. To account for tolerances in the
> ADC, the calculated raw threshold should be adjusted based on the ADC
> specifications for the device. These specifications include the integral
> nonlinearity (INL), offset, and gain error. To adjust the high
> threshold, use the following equation:
>
> (calibrated value + INL) * Gain error + offset = maximum value [1]
>
> Likewise, use the following equation for the low threshold:
>
> (calibrated value - INL) * Gain error - offset = minimum value
>
> The gain error is a combination of gain error, as listed in the
> datasheet, and gain error drift due to temperature and supply. The exact
> values for these specifications vary between palmas devices. This patch
> sets the values found in TWL6035, TWL6037 datasheet.
>
> [1] TI Application Report, SLIA087A, Guide to Using the GPADC in
> TPS65903x, TPS65917-Q1, TPS65919-Q1, and TPS65916 Devices.
>
> Signed-off-by: Patrik Dahlström <risca@xxxxxxxxxxxxxx>
0-day found some stuff we'd missed in here.
I've fixed it up and pushed out again.
> ---
...
> +static int palmas_gpadc_write_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int val, int val2)
> +{
> + struct palmas_gpadc *adc = iio_priv(indio_dev);
> + int adc_chan = chan->channel;
> + int old;
> + int ret;
> +
> + if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
> + return -EINVAL;
> +
> + mutex_lock(&adc->lock);
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + if (val < 0 || val > 0xFFF) {
> + ret = -EINVAL;
> + break;
Should be goto out_unlock; Found because old is undefined.
> + }
> + if (dir == IIO_EV_DIR_RISING) {
> + old = adc->thresholds[adc_chan].high;
> + adc->thresholds[adc_chan].high = val;
> + }
Whilst here should be } else {
Tidied both up force an update on the testing branch.
> + else {
> + old = adc->thresholds[adc_chan].low;
> + adc->thresholds[adc_chan].low = val;
> + }
> + ret = 0;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + if (val != old && palmas_gpadc_get_event(adc, adc_chan, dir))
> + ret = palmas_gpadc_reconfigure_event_channels(adc);
> +
> +out_unlock:
> + mutex_unlock(&adc->lock);
> +
> + return ret;
> +}
> +
