Re: [PATCH v2 12/17] iio: adc: axp20x_adc: Add support for AXP192

Jonathan Cameron
2022-06-08T13:35:23Z

On Tue, 7 Jun 2022 16:53:19 +0100
Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote:

> The AXP192 is identical to the AXP20x, except for the addition of
> two more GPIO ADC channels.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>

Hi Aidan,

A few things follow through from review of previous 2 patches.
Otherwise looks good to me.

Thanks,

Jonathan

> AXP20X_ADC_CHANNEL(AXP20X_ACIN_V, "acin_v", IIO_VOLTAGE,
> AXP20X_ACIN_V_ADC_H),
> @@ -250,6 +314,15 @@ static int axp20x_adc_raw(struct iio_dev *indio_dev,
> int ret, size;
>
> switch (info->data->axp20x_id) {
> + case AXP192_ID:
> + /* Battery current ADCs on the AXP192 are 13 bits. */
> + if (chan->type == IIO_CURRENT &&
> + (chan->channel == AXP20X_BATT_CHRG_I || chan->channel == AXP20X_BATT_DISCHRG_I))

Ah, you'll be needing two _res variables as suggested in earlier patch.

> + size = 13;
> + else
> + size = 12;
> + break;
> +
> case AXP202_ID:
> case AXP209_ID:
> /*
> @@ -276,6 +349,44 @@ static int axp20x_adc_raw(struct iio_dev *indio_dev,
> return IIO_VAL_INT;
> }

...

> +static int axp192_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
> + int *val)
> +{
> + struct axp20x_adc_iio *info = iio_priv(indio_dev);
> + unsigned int regval;
> + int ret;
> +
> + ret = regmap_read(info->regmap, AXP192_GPIO30_IN_RANGE, &regval);
> + if (ret < 0)
> + return ret;
> +
> + switch (channel) {
> + case AXP192_GPIO0_V:
> + regval &= AXP192_GPIO30_IN_RANGE_GPIO0;

As per earlier patch, FIELD_GET() would act as 'documentation'
of what is going on here, even though it'll make no real difference.

> + break;
> +
> + case AXP192_GPIO1_V:
> + regval &= AXP192_GPIO30_IN_RANGE_GPIO1;
> + break;
> +
> + case AXP192_GPIO2_V:
> + regval &= AXP192_GPIO30_IN_RANGE_GPIO2;
> + break;
> +
> + case AXP192_GPIO3_V:
> + regval &= AXP192_GPIO30_IN_RANGE_GPIO3;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + *val = regval ? 700000 : 0;
> + return IIO_VAL_INT;
> +}
> +