| Message ID | 20240617220535.359021-6-macroalpha82@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Add Battery and USB Supply for AXP717 | expand |
On Mon, Jun 17, 2024 at 05:05:32PM -0500, Chris Morgan wrote: > From: Chris Morgan <macromorgan@hotmail.com> > > 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. Maybe charge and discharge share the same 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@hotmail.com> > --- > 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@free-electrons.com> > */ > > +#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 > + > #define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4) > #define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK) > #define AXP813_TS_GPIO0_ADC_RATE_HZ(x) AXP20X_ADC_RATE_HZ(x) > @@ -98,6 +106,18 @@ enum axp22x_adc_channel_i { > AXP22X_BATT_DISCHRG_I, > }; > > +enum axp717_adc_channel_v { > + AXP717_BATT_V = 0, > + AXP717_TS_IN, > + AXP717_VBUS_V, > + AXP717_VSYS_V, > + AXP717_DIE_TEMP_V, Please also add VMID and backup battery voltage. > +}; > + > +enum axp717_adc_channel_i { > + AXP717_BATT_CHRG_I = 5, > +}; > + > enum axp813_adc_channel_v { > AXP813_TS_IN = 0, > AXP813_GPIO0_V, > @@ -152,6 +172,22 @@ static struct iio_map axp22x_maps[] = { > }, { /* sentinel */ } > }; > > +static struct iio_map axp717_maps[] = { > + { > + .consumer_dev_name = "axp20x-usb-power-supply", > + .consumer_channel = "vbus_v", > + .adc_channel_label = "vbus_v", > + }, { > + .consumer_dev_name = "axp20x-battery-power-supply", > + .consumer_channel = "batt_v", > + .adc_channel_label = "batt_v", > + }, { > + .consumer_dev_name = "axp20x-battery-power-supply", > + .consumer_channel = "batt_chrg_i", > + .adc_channel_label = "batt_chrg_i", > + }, > +}; > + > /* > * Channels are mapped by physical system. Their channels share the same index. > * i.e. acin_i is in_current0_raw and acin_v is in_voltage0_raw. > @@ -210,6 +246,25 @@ static const struct iio_chan_spec axp22x_adc_channels[] = { > AXP22X_TS_ADC_H), > }; > > +/* > + * Scale and offset is unknown for temp, ts, and batt_chrg_i channels > + * for now. Leaving scale and offset undefined for now. > + */ > +static const struct iio_chan_spec axp717_adc_channels[] = { > + AXP20X_ADC_CHANNEL(AXP717_BATT_V, "batt_v", IIO_VOLTAGE, > + AXP717_BATT_V_H), > + AXP20X_ADC_CHANNEL(AXP717_TS_IN, "ts_v", IIO_VOLTAGE, > + AXP717_ADC_DATA_H), > + AXP20X_ADC_CHANNEL(AXP717_VBUS_V, "vbus_v", IIO_VOLTAGE, > + AXP717_VBUS_V_H), > + AXP20X_ADC_CHANNEL(AXP717_VSYS_V, "vsys_v", IIO_VOLTAGE, > + AXP717_VSYS_V_H), > + AXP20X_ADC_CHANNEL(AXP717_DIE_TEMP_V, "pmic_temp", IIO_TEMP, > + AXP717_ADC_DATA_H), > + AXP20X_ADC_CHANNEL(AXP717_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT, > + AXP717_BATT_CHRG_I_H), > +}; > + > static const struct iio_chan_spec axp813_adc_channels[] = { > { > .type = IIO_TEMP, > @@ -269,6 +324,44 @@ static int axp22x_adc_raw(struct iio_dev *indio_dev, > return IIO_VAL_INT; > } > > +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; > + return IIO_VAL_INT; > +} > + > static int axp813_adc_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, int *val) > { > @@ -425,6 +518,27 @@ static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val, > } > } > > +static int axp717_adc_scale(struct iio_chan_spec const *chan, int *val, > + int *val2) > +{ > + switch (chan->type) { > + case IIO_VOLTAGE: > + *val = 1; > + return IIO_VAL_INT; > + > + case IIO_CURRENT: > + *val = 1; > + return IIO_VAL_INT; > + > + case IIO_TEMP: > + *val = 100; > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } > +} > + > static int axp813_adc_scale(struct iio_chan_spec const *chan, int *val, > int *val2) > { > @@ -529,6 +643,22 @@ static int axp22x_read_raw(struct iio_dev *indio_dev, > } > } > > +static int axp717_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + switch (mask) { > + case IIO_CHAN_INFO_SCALE: > + return axp717_adc_scale(chan, val, val2); > + > + case IIO_CHAN_INFO_RAW: > + return axp717_adc_raw(indio_dev, chan, val); > + > + default: > + return -EINVAL; > + } > +} > + > static int axp813_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, int *val, > int *val2, long mask) > @@ -593,6 +723,10 @@ static const struct iio_info axp22x_adc_iio_info = { > .read_raw = axp22x_read_raw, > }; > > +static const struct iio_info axp717_adc_iio_info = { > + .read_raw = axp717_read_raw, > +}; > + > static const struct iio_info axp813_adc_iio_info = { > .read_raw = axp813_read_raw, > }; > @@ -622,7 +756,9 @@ struct axp_data { > const struct iio_info *iio_info; > int num_channels; > struct iio_chan_spec const *channels; > + unsigned long adc_en1; > unsigned long adc_en1_mask; > + unsigned long adc_en2; > unsigned long adc_en2_mask; > int (*adc_rate)(struct axp20x_adc_iio *info, > int rate); > @@ -633,7 +769,9 @@ static const struct axp_data axp20x_data = { > .iio_info = &axp20x_adc_iio_info, > .num_channels = ARRAY_SIZE(axp20x_adc_channels), > .channels = axp20x_adc_channels, > + .adc_en1 = AXP20X_ADC_EN1, > .adc_en1_mask = AXP20X_ADC_EN1_MASK, > + .adc_en2 = AXP20X_ADC_EN2, > .adc_en2_mask = AXP20X_ADC_EN2_MASK, > .adc_rate = axp20x_adc_rate, > .maps = axp20x_maps, > @@ -643,15 +781,26 @@ static const struct axp_data axp22x_data = { > .iio_info = &axp22x_adc_iio_info, > .num_channels = ARRAY_SIZE(axp22x_adc_channels), > .channels = axp22x_adc_channels, > + .adc_en1 = AXP20X_ADC_EN1, > .adc_en1_mask = AXP22X_ADC_EN1_MASK, > .adc_rate = axp22x_adc_rate, > .maps = axp22x_maps, > }; Please split out the changes generalizing the driver into a separate patch. That makes it easier to follow. ChenYu > +static const struct axp_data axp717_data = { > + .iio_info = &axp717_adc_iio_info, > + .num_channels = ARRAY_SIZE(axp717_adc_channels), > + .channels = axp717_adc_channels, > + .adc_en1 = AXP717_ADC_CH_EN_CONTROL, > + .adc_en1_mask = AXP717_ADC_EN1_MASK, > + .maps = axp717_maps, > +}; > + > static const struct axp_data axp813_data = { > .iio_info = &axp813_adc_iio_info, > .num_channels = ARRAY_SIZE(axp813_adc_channels), > .channels = axp813_adc_channels, > + .adc_en1 = AXP20X_ADC_EN1, > .adc_en1_mask = AXP22X_ADC_EN1_MASK, > .adc_rate = axp813_adc_rate, > .maps = axp22x_maps, > @@ -660,6 +809,7 @@ static const struct axp_data axp813_data = { > static const struct of_device_id axp20x_adc_of_match[] = { > { .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, }, > { .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, }, > + { .compatible = "x-powers,axp717-adc", .data = (void *)&axp717_data, }, > { .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, }, > { /* sentinel */ } > }; > @@ -668,6 +818,7 @@ MODULE_DEVICE_TABLE(of, axp20x_adc_of_match); > static const struct platform_device_id axp20x_adc_id_match[] = { > { .name = "axp20x-adc", .driver_data = (kernel_ulong_t)&axp20x_data, }, > { .name = "axp22x-adc", .driver_data = (kernel_ulong_t)&axp22x_data, }, > + { .name = "axp717-adc", .driver_data = (kernel_ulong_t)&axp717_data, }, > { .name = "axp813-adc", .driver_data = (kernel_ulong_t)&axp813_data, }, > { /* sentinel */ }, > }; > @@ -709,15 +860,17 @@ static int axp20x_probe(struct platform_device *pdev) > indio_dev->channels = info->data->channels; > > /* Enable the ADCs on IP */ > - regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask); > + regmap_write(info->regmap, info->data->adc_en1, > + info->data->adc_en1_mask); > > if (info->data->adc_en2_mask) > - regmap_update_bits(info->regmap, AXP20X_ADC_EN2, > + regmap_update_bits(info->regmap, info->data->adc_en2, > info->data->adc_en2_mask, > info->data->adc_en2_mask); > > /* Configure ADCs rate */ > - info->data->adc_rate(info, 100); > + if (info->data->adc_rate) > + info->data->adc_rate(info, 100); > > ret = iio_map_array_register(indio_dev, info->data->maps); > if (ret < 0) { > @@ -737,10 +890,10 @@ static int axp20x_probe(struct platform_device *pdev) > iio_map_array_unregister(indio_dev); > > fail_map: > - regmap_write(info->regmap, AXP20X_ADC_EN1, 0); > + regmap_write(info->regmap, info->data->adc_en1, 0); > > if (info->data->adc_en2_mask) > - regmap_write(info->regmap, AXP20X_ADC_EN2, 0); > + regmap_write(info->regmap, info->data->adc_en2, 0); > > return ret; > } > @@ -753,10 +906,10 @@ static void axp20x_remove(struct platform_device *pdev) > iio_device_unregister(indio_dev); > iio_map_array_unregister(indio_dev); > > - regmap_write(info->regmap, AXP20X_ADC_EN1, 0); > + regmap_write(info->regmap, info->data->adc_en1, 0); > > if (info->data->adc_en2_mask) > - regmap_write(info->regmap, AXP20X_ADC_EN2, 0); > + regmap_write(info->regmap, info->data->adc_en2, 0); > } > > static struct platform_driver axp20x_adc_driver = { > -- > 2.34.1 >
On Mon, 17 Jun 2024 17:05:32 -0500 Chris Morgan <macroalpha82@gmail.com> wrote: > From: Chris Morgan <macromorgan@hotmail.com> > > 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@hotmail.com> 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@free-electrons.com> > */ > > +#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; > +} > +
On Sun, Jun 23, 2024 at 01:27:25PM +0800, Chen-Yu Tsai wrote: > On Mon, Jun 17, 2024 at 05:05:32PM -0500, Chris Morgan wrote: > > From: Chris Morgan <macromorgan@hotmail.com> > > > > 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. > > Maybe charge and discharge share the same channel? I can't confirm that. I've enabled multiple power draws and don't notice a change on this channel consistent with such a draw. Power draws include backlight, GPIO vibrators, and turning on/off regulators. > > > 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@hotmail.com> > > --- > > 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@free-electrons.com> > > */ > > > > +#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 > > + > > #define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4) > > #define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK) > > #define AXP813_TS_GPIO0_ADC_RATE_HZ(x) AXP20X_ADC_RATE_HZ(x) > > @@ -98,6 +106,18 @@ enum axp22x_adc_channel_i { > > AXP22X_BATT_DISCHRG_I, > > }; > > > > +enum axp717_adc_channel_v { > > + AXP717_BATT_V = 0, > > + AXP717_TS_IN, > > + AXP717_VBUS_V, > > + AXP717_VSYS_V, > > + AXP717_DIE_TEMP_V, > > Please also add VMID and backup battery voltage. Will do, I'll make sure all ADC channels are here, even if they aren't used. > > > +}; > > + > > +enum axp717_adc_channel_i { > > + AXP717_BATT_CHRG_I = 5, > > +}; > > + > > enum axp813_adc_channel_v { > > AXP813_TS_IN = 0, > > AXP813_GPIO0_V, > > @@ -152,6 +172,22 @@ static struct iio_map axp22x_maps[] = { > > }, { /* sentinel */ } > > }; > > > > +static struct iio_map axp717_maps[] = { > > + { > > + .consumer_dev_name = "axp20x-usb-power-supply", > > + .consumer_channel = "vbus_v", > > + .adc_channel_label = "vbus_v", > > + }, { > > + .consumer_dev_name = "axp20x-battery-power-supply", > > + .consumer_channel = "batt_v", > > + .adc_channel_label = "batt_v", > > + }, { > > + .consumer_dev_name = "axp20x-battery-power-supply", > > + .consumer_channel = "batt_chrg_i", > > + .adc_channel_label = "batt_chrg_i", > > + }, > > +}; > > + > > /* > > * Channels are mapped by physical system. Their channels share the same index. > > * i.e. acin_i is in_current0_raw and acin_v is in_voltage0_raw. > > @@ -210,6 +246,25 @@ static const struct iio_chan_spec axp22x_adc_channels[] = { > > AXP22X_TS_ADC_H), > > }; > > > > +/* > > + * Scale and offset is unknown for temp, ts, and batt_chrg_i channels > > + * for now. Leaving scale and offset undefined for now. > > + */ > > +static const struct iio_chan_spec axp717_adc_channels[] = { > > + AXP20X_ADC_CHANNEL(AXP717_BATT_V, "batt_v", IIO_VOLTAGE, > > + AXP717_BATT_V_H), > > + AXP20X_ADC_CHANNEL(AXP717_TS_IN, "ts_v", IIO_VOLTAGE, > > + AXP717_ADC_DATA_H), > > + AXP20X_ADC_CHANNEL(AXP717_VBUS_V, "vbus_v", IIO_VOLTAGE, > > + AXP717_VBUS_V_H), > > + AXP20X_ADC_CHANNEL(AXP717_VSYS_V, "vsys_v", IIO_VOLTAGE, > > + AXP717_VSYS_V_H), > > + AXP20X_ADC_CHANNEL(AXP717_DIE_TEMP_V, "pmic_temp", IIO_TEMP, > > + AXP717_ADC_DATA_H), > > + AXP20X_ADC_CHANNEL(AXP717_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT, > > + AXP717_BATT_CHRG_I_H), > > +}; > > + > > static const struct iio_chan_spec axp813_adc_channels[] = { > > { > > .type = IIO_TEMP, > > @@ -269,6 +324,44 @@ static int axp22x_adc_raw(struct iio_dev *indio_dev, > > return IIO_VAL_INT; > > } > > > > +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; > > + return IIO_VAL_INT; > > +} > > + > > static int axp813_adc_raw(struct iio_dev *indio_dev, > > struct iio_chan_spec const *chan, int *val) > > { > > @@ -425,6 +518,27 @@ static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val, > > } > > } > > > > +static int axp717_adc_scale(struct iio_chan_spec const *chan, int *val, > > + int *val2) > > +{ > > + switch (chan->type) { > > + case IIO_VOLTAGE: > > + *val = 1; > > + return IIO_VAL_INT; > > + > > + case IIO_CURRENT: > > + *val = 1; > > + return IIO_VAL_INT; > > + > > + case IIO_TEMP: > > + *val = 100; > > + return IIO_VAL_INT; > > + > > + default: > > + return -EINVAL; > > + } > > +} > > + > > static int axp813_adc_scale(struct iio_chan_spec const *chan, int *val, > > int *val2) > > { > > @@ -529,6 +643,22 @@ static int axp22x_read_raw(struct iio_dev *indio_dev, > > } > > } > > > > +static int axp717_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, int *val, > > + int *val2, long mask) > > +{ > > + switch (mask) { > > + case IIO_CHAN_INFO_SCALE: > > + return axp717_adc_scale(chan, val, val2); > > + > > + case IIO_CHAN_INFO_RAW: > > + return axp717_adc_raw(indio_dev, chan, val); > > + > > + default: > > + return -EINVAL; > > + } > > +} > > + > > static int axp813_read_raw(struct iio_dev *indio_dev, > > struct iio_chan_spec const *chan, int *val, > > int *val2, long mask) > > @@ -593,6 +723,10 @@ static const struct iio_info axp22x_adc_iio_info = { > > .read_raw = axp22x_read_raw, > > }; > > > > +static const struct iio_info axp717_adc_iio_info = { > > + .read_raw = axp717_read_raw, > > +}; > > + > > static const struct iio_info axp813_adc_iio_info = { > > .read_raw = axp813_read_raw, > > }; > > @@ -622,7 +756,9 @@ struct axp_data { > > const struct iio_info *iio_info; > > int num_channels; > > struct iio_chan_spec const *channels; > > + unsigned long adc_en1; > > unsigned long adc_en1_mask; > > + unsigned long adc_en2; > > unsigned long adc_en2_mask; > > int (*adc_rate)(struct axp20x_adc_iio *info, > > int rate); > > @@ -633,7 +769,9 @@ static const struct axp_data axp20x_data = { > > .iio_info = &axp20x_adc_iio_info, > > .num_channels = ARRAY_SIZE(axp20x_adc_channels), > > .channels = axp20x_adc_channels, > > + .adc_en1 = AXP20X_ADC_EN1, > > .adc_en1_mask = AXP20X_ADC_EN1_MASK, > > + .adc_en2 = AXP20X_ADC_EN2, > > .adc_en2_mask = AXP20X_ADC_EN2_MASK, > > .adc_rate = axp20x_adc_rate, > > .maps = axp20x_maps, > > @@ -643,15 +781,26 @@ static const struct axp_data axp22x_data = { > > .iio_info = &axp22x_adc_iio_info, > > .num_channels = ARRAY_SIZE(axp22x_adc_channels), > > .channels = axp22x_adc_channels, > > + .adc_en1 = AXP20X_ADC_EN1, > > .adc_en1_mask = AXP22X_ADC_EN1_MASK, > > .adc_rate = axp22x_adc_rate, > > .maps = axp22x_maps, > > }; > > Please split out the changes generalizing the driver into a separate > patch. That makes it easier to follow. > Acknowledged. > > ChenYu > > > +static const struct axp_data axp717_data = { > > + .iio_info = &axp717_adc_iio_info, > > + .num_channels = ARRAY_SIZE(axp717_adc_channels), > > + .channels = axp717_adc_channels, > > + .adc_en1 = AXP717_ADC_CH_EN_CONTROL, > > + .adc_en1_mask = AXP717_ADC_EN1_MASK, > > + .maps = axp717_maps, > > +}; > > + > > static const struct axp_data axp813_data = { > > .iio_info = &axp813_adc_iio_info, > > .num_channels = ARRAY_SIZE(axp813_adc_channels), > > .channels = axp813_adc_channels, > > + .adc_en1 = AXP20X_ADC_EN1, > > .adc_en1_mask = AXP22X_ADC_EN1_MASK, > > .adc_rate = axp813_adc_rate, > > .maps = axp22x_maps, > > @@ -660,6 +809,7 @@ static const struct axp_data axp813_data = { > > static const struct of_device_id axp20x_adc_of_match[] = { > > { .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, }, > > { .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, }, > > + { .compatible = "x-powers,axp717-adc", .data = (void *)&axp717_data, }, > > { .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, }, > > { /* sentinel */ } > > }; > > @@ -668,6 +818,7 @@ MODULE_DEVICE_TABLE(of, axp20x_adc_of_match); > > static const struct platform_device_id axp20x_adc_id_match[] = { > > { .name = "axp20x-adc", .driver_data = (kernel_ulong_t)&axp20x_data, }, > > { .name = "axp22x-adc", .driver_data = (kernel_ulong_t)&axp22x_data, }, > > + { .name = "axp717-adc", .driver_data = (kernel_ulong_t)&axp717_data, }, > > { .name = "axp813-adc", .driver_data = (kernel_ulong_t)&axp813_data, }, > > { /* sentinel */ }, > > }; > > @@ -709,15 +860,17 @@ static int axp20x_probe(struct platform_device *pdev) > > indio_dev->channels = info->data->channels; > > > > /* Enable the ADCs on IP */ > > - regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask); > > + regmap_write(info->regmap, info->data->adc_en1, > > + info->data->adc_en1_mask); > > > > if (info->data->adc_en2_mask) > > - regmap_update_bits(info->regmap, AXP20X_ADC_EN2, > > + regmap_update_bits(info->regmap, info->data->adc_en2, > > info->data->adc_en2_mask, > > info->data->adc_en2_mask); > > > > /* Configure ADCs rate */ > > - info->data->adc_rate(info, 100); > > + if (info->data->adc_rate) > > + info->data->adc_rate(info, 100); > > > > ret = iio_map_array_register(indio_dev, info->data->maps); > > if (ret < 0) { > > @@ -737,10 +890,10 @@ static int axp20x_probe(struct platform_device *pdev) > > iio_map_array_unregister(indio_dev); > > > > fail_map: > > - regmap_write(info->regmap, AXP20X_ADC_EN1, 0); > > + regmap_write(info->regmap, info->data->adc_en1, 0); > > > > if (info->data->adc_en2_mask) > > - regmap_write(info->regmap, AXP20X_ADC_EN2, 0); > > + regmap_write(info->regmap, info->data->adc_en2, 0); > > > > return ret; > > } > > @@ -753,10 +906,10 @@ static void axp20x_remove(struct platform_device *pdev) > > iio_device_unregister(indio_dev); > > iio_map_array_unregister(indio_dev); > > > > - regmap_write(info->regmap, AXP20X_ADC_EN1, 0); > > + regmap_write(info->regmap, info->data->adc_en1, 0); > > > > if (info->data->adc_en2_mask) > > - regmap_write(info->regmap, AXP20X_ADC_EN2, 0); > > + regmap_write(info->regmap, info->data->adc_en2, 0); > > } > > > > static struct platform_driver axp20x_adc_driver = { > > -- > > 2.34.1 > > Thank you for the feedback, Chris
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@free-electrons.com> */ +#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 + #define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4) #define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK) #define AXP813_TS_GPIO0_ADC_RATE_HZ(x) AXP20X_ADC_RATE_HZ(x) @@ -98,6 +106,18 @@ enum axp22x_adc_channel_i { AXP22X_BATT_DISCHRG_I, }; +enum axp717_adc_channel_v { + AXP717_BATT_V = 0, + AXP717_TS_IN, + AXP717_VBUS_V, + AXP717_VSYS_V, + AXP717_DIE_TEMP_V, +}; + +enum axp717_adc_channel_i { + AXP717_BATT_CHRG_I = 5, +}; + enum axp813_adc_channel_v { AXP813_TS_IN = 0, AXP813_GPIO0_V, @@ -152,6 +172,22 @@ static struct iio_map axp22x_maps[] = { }, { /* sentinel */ } }; +static struct iio_map axp717_maps[] = { + { + .consumer_dev_name = "axp20x-usb-power-supply", + .consumer_channel = "vbus_v", + .adc_channel_label = "vbus_v", + }, { + .consumer_dev_name = "axp20x-battery-power-supply", + .consumer_channel = "batt_v", + .adc_channel_label = "batt_v", + }, { + .consumer_dev_name = "axp20x-battery-power-supply", + .consumer_channel = "batt_chrg_i", + .adc_channel_label = "batt_chrg_i", + }, +}; + /* * Channels are mapped by physical system. Their channels share the same index. * i.e. acin_i is in_current0_raw and acin_v is in_voltage0_raw. @@ -210,6 +246,25 @@ static const struct iio_chan_spec axp22x_adc_channels[] = { AXP22X_TS_ADC_H), }; +/* + * Scale and offset is unknown for temp, ts, and batt_chrg_i channels + * for now. Leaving scale and offset undefined for now. + */ +static const struct iio_chan_spec axp717_adc_channels[] = { + AXP20X_ADC_CHANNEL(AXP717_BATT_V, "batt_v", IIO_VOLTAGE, + AXP717_BATT_V_H), + AXP20X_ADC_CHANNEL(AXP717_TS_IN, "ts_v", IIO_VOLTAGE, + AXP717_ADC_DATA_H), + AXP20X_ADC_CHANNEL(AXP717_VBUS_V, "vbus_v", IIO_VOLTAGE, + AXP717_VBUS_V_H), + AXP20X_ADC_CHANNEL(AXP717_VSYS_V, "vsys_v", IIO_VOLTAGE, + AXP717_VSYS_V_H), + AXP20X_ADC_CHANNEL(AXP717_DIE_TEMP_V, "pmic_temp", IIO_TEMP, + AXP717_ADC_DATA_H), + AXP20X_ADC_CHANNEL(AXP717_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT, + AXP717_BATT_CHRG_I_H), +}; + static const struct iio_chan_spec axp813_adc_channels[] = { { .type = IIO_TEMP, @@ -269,6 +324,44 @@ static int axp22x_adc_raw(struct iio_dev *indio_dev, return IIO_VAL_INT; } +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; + return IIO_VAL_INT; +} + static int axp813_adc_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val) { @@ -425,6 +518,27 @@ static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val, } } +static int axp717_adc_scale(struct iio_chan_spec const *chan, int *val, + int *val2) +{ + switch (chan->type) { + case IIO_VOLTAGE: + *val = 1; + return IIO_VAL_INT; + + case IIO_CURRENT: + *val = 1; + return IIO_VAL_INT; + + case IIO_TEMP: + *val = 100; + return IIO_VAL_INT; + + default: + return -EINVAL; + } +} + static int axp813_adc_scale(struct iio_chan_spec const *chan, int *val, int *val2) { @@ -529,6 +643,22 @@ static int axp22x_read_raw(struct iio_dev *indio_dev, } } +static int axp717_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + switch (mask) { + case IIO_CHAN_INFO_SCALE: + return axp717_adc_scale(chan, val, val2); + + case IIO_CHAN_INFO_RAW: + return axp717_adc_raw(indio_dev, chan, val); + + default: + return -EINVAL; + } +} + static int axp813_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) @@ -593,6 +723,10 @@ static const struct iio_info axp22x_adc_iio_info = { .read_raw = axp22x_read_raw, }; +static const struct iio_info axp717_adc_iio_info = { + .read_raw = axp717_read_raw, +}; + static const struct iio_info axp813_adc_iio_info = { .read_raw = axp813_read_raw, }; @@ -622,7 +756,9 @@ struct axp_data { const struct iio_info *iio_info; int num_channels; struct iio_chan_spec const *channels; + unsigned long adc_en1; unsigned long adc_en1_mask; + unsigned long adc_en2; unsigned long adc_en2_mask; int (*adc_rate)(struct axp20x_adc_iio *info, int rate); @@ -633,7 +769,9 @@ static const struct axp_data axp20x_data = { .iio_info = &axp20x_adc_iio_info, .num_channels = ARRAY_SIZE(axp20x_adc_channels), .channels = axp20x_adc_channels, + .adc_en1 = AXP20X_ADC_EN1, .adc_en1_mask = AXP20X_ADC_EN1_MASK, + .adc_en2 = AXP20X_ADC_EN2, .adc_en2_mask = AXP20X_ADC_EN2_MASK, .adc_rate = axp20x_adc_rate, .maps = axp20x_maps, @@ -643,15 +781,26 @@ static const struct axp_data axp22x_data = { .iio_info = &axp22x_adc_iio_info, .num_channels = ARRAY_SIZE(axp22x_adc_channels), .channels = axp22x_adc_channels, + .adc_en1 = AXP20X_ADC_EN1, .adc_en1_mask = AXP22X_ADC_EN1_MASK, .adc_rate = axp22x_adc_rate, .maps = axp22x_maps, }; +static const struct axp_data axp717_data = { + .iio_info = &axp717_adc_iio_info, + .num_channels = ARRAY_SIZE(axp717_adc_channels), + .channels = axp717_adc_channels, + .adc_en1 = AXP717_ADC_CH_EN_CONTROL, + .adc_en1_mask = AXP717_ADC_EN1_MASK, + .maps = axp717_maps, +}; + static const struct axp_data axp813_data = { .iio_info = &axp813_adc_iio_info, .num_channels = ARRAY_SIZE(axp813_adc_channels), .channels = axp813_adc_channels, + .adc_en1 = AXP20X_ADC_EN1, .adc_en1_mask = AXP22X_ADC_EN1_MASK, .adc_rate = axp813_adc_rate, .maps = axp22x_maps, @@ -660,6 +809,7 @@ static const struct axp_data axp813_data = { static const struct of_device_id axp20x_adc_of_match[] = { { .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, }, { .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, }, + { .compatible = "x-powers,axp717-adc", .data = (void *)&axp717_data, }, { .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, }, { /* sentinel */ } }; @@ -668,6 +818,7 @@ MODULE_DEVICE_TABLE(of, axp20x_adc_of_match); static const struct platform_device_id axp20x_adc_id_match[] = { { .name = "axp20x-adc", .driver_data = (kernel_ulong_t)&axp20x_data, }, { .name = "axp22x-adc", .driver_data = (kernel_ulong_t)&axp22x_data, }, + { .name = "axp717-adc", .driver_data = (kernel_ulong_t)&axp717_data, }, { .name = "axp813-adc", .driver_data = (kernel_ulong_t)&axp813_data, }, { /* sentinel */ }, }; @@ -709,15 +860,17 @@ static int axp20x_probe(struct platform_device *pdev) indio_dev->channels = info->data->channels; /* Enable the ADCs on IP */ - regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask); + regmap_write(info->regmap, info->data->adc_en1, + info->data->adc_en1_mask); if (info->data->adc_en2_mask) - regmap_update_bits(info->regmap, AXP20X_ADC_EN2, + regmap_update_bits(info->regmap, info->data->adc_en2, info->data->adc_en2_mask, info->data->adc_en2_mask); /* Configure ADCs rate */ - info->data->adc_rate(info, 100); + if (info->data->adc_rate) + info->data->adc_rate(info, 100); ret = iio_map_array_register(indio_dev, info->data->maps); if (ret < 0) { @@ -737,10 +890,10 @@ static int axp20x_probe(struct platform_device *pdev) iio_map_array_unregister(indio_dev); fail_map: - regmap_write(info->regmap, AXP20X_ADC_EN1, 0); + regmap_write(info->regmap, info->data->adc_en1, 0); if (info->data->adc_en2_mask) - regmap_write(info->regmap, AXP20X_ADC_EN2, 0); + regmap_write(info->regmap, info->data->adc_en2, 0); return ret; } @@ -753,10 +906,10 @@ static void axp20x_remove(struct platform_device *pdev) iio_device_unregister(indio_dev); iio_map_array_unregister(indio_dev); - regmap_write(info->regmap, AXP20X_ADC_EN1, 0); + regmap_write(info->regmap, info->data->adc_en1, 0); if (info->data->adc_en2_mask) - regmap_write(info->regmap, AXP20X_ADC_EN2, 0); + regmap_write(info->regmap, info->data->adc_en2, 0); } static struct platform_driver axp20x_adc_driver = {