On Mon, 17 Jun 2024 17:05:32 -0500
Chris Morgan <macroalpha82@xxxxxxxxx> wrote:
> From: Chris Morgan <macromorgan@xxxxxxxxxxx>
>
> Add support for the AXP717 ADC. The AXP717 differs from other ADCs
> in this series by utilizing a 14 bit ADC for all channels (a full 16
> bits with the first 2 digits reserved). It also differs by lacking a
> battery discharge current channel.
>
> Note that while the current charge channel itself is included in this
> driver for the AXP717 and listed in the datasheet, no scale or offset
> was given for this channel. For now no scale or offset is provided in
> this driver.
>
> Signed-off-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
Hi Chris
A few minor comments inline,
Thanks,
Jonathan
> ---
> drivers/iio/adc/axp20x_adc.c | 167 +++++++++++++++++++++++++++++++++--
> 1 file changed, 160 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index d6c51b0f48e3..f35ba2c11e1b 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -5,6 +5,7 @@
> * Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxx>
> */
>
> +#include <asm/unaligned.h>
> #include <linux/bitfield.h>
> #include <linux/completion.h>
> #include <linux/interrupt.h>
> @@ -27,6 +28,8 @@
>
> #define AXP22X_ADC_EN1_MASK (GENMASK(7, 5) | BIT(0))
>
> +#define AXP717_ADC_EN1_MASK GENMASK(5, 0)
> +
> #define AXP20X_GPIO10_IN_RANGE_GPIO0 BIT(0)
> #define AXP20X_GPIO10_IN_RANGE_GPIO1 BIT(1)
>
> @@ -35,6 +38,11 @@
>
> #define AXP22X_ADC_RATE_HZ(x) ((ilog2((x) / 100) << 6) & AXP20X_ADC_RATE_MASK)
>
> +#define AXP717_ADC_DATA_TS 0x00
> +#define AXP717_ADC_DATA_TEMP 0x01
> +
> +#define AXP717_ADC_DATA_MASK 0x3fff
GENMASK()
> +static int axp717_adc_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val)
> +{
> + struct axp20x_adc_iio *info = iio_priv(indio_dev);
> + u8 bulk_reg[2];
> + int ret;
> +
> + /*
> + * A generic "ADC data" channel is used for TS, tdie, vmid,
> + * and vbackup. This channel must both first be enabled and
> + * also selected before it can be read.
> + */
> + switch (chan->channel) {
> + case AXP717_TS_IN:
> + regmap_write(info->regmap, AXP717_ADC_DATA_SEL,
> + AXP717_ADC_DATA_TS);
> + break;
> + case AXP717_DIE_TEMP_V:
> + regmap_write(info->regmap, AXP717_ADC_DATA_SEL,
> + AXP717_ADC_DATA_TEMP);
> + break;
> +
> + default:
> + break;
> + }
> +
> + /*
> + * All channels are 14 bits, with the first 2 bits on the high
> + * register reserved and the remaining bits as the ADC value.
> + */
> + ret = regmap_bulk_read(info->regmap, chan->address, bulk_reg, 2);
> + if (ret < 0)
> + return ret;
> +
> + *val = get_unaligned_be16(bulk_reg) & AXP717_ADC_DATA_MASK;
FIELD_GET() preferred as then I don't have to check if DATA_MASK includes
the 0th bit.
> + return IIO_VAL_INT;
> +}
> +
