On Mon, 9 May 2022 12:53:12 +0100 Caleb Connolly <caleb.connolly@xxxxxxxxxx> wrote: > On 01/05/2022 18:38, Jonathan Cameron wrote: > > On Fri, 29 Apr 2022 23:09:00 +0100 > > Caleb Connolly <caleb.connolly@xxxxxxxxxx> wrote: > > > >> The Round Robin ADC is responsible for reading data about the rate of > >> charge from the USB or DC input ports, it can also read the battery > >> ID (resistence), skin temperature and the die temperature of the pmic. > >> It is found on the PMI8998 and PM660 Qualcomm PMICs. > >> > >> Signed-off-by: Caleb Connolly <caleb.connolly@xxxxxxxxxx> > > Hi Caleb, > Hi Jonathan, > > Thanks for spotting this, I completely missed it... Yeah this should be > IIO_INFO_PROCESSED, the battery ID calculation doesn't fit in the > raw/offset/scale format. > > > > I took another quick read through of this and noticed that the battery channel > > is providing on IIO_INFO_RAW but there is code for IIO_INFO_PROCESSED. > > > > Something gone wrong along the way? If all we need is to change it to > > BIT(IIO_INFO_PROCESSED) I can do that whilst applying or you can do a v15 if > > you prefer. > That would be hugely appreciated, thanks a lot. > > Given other reply I just sent suggesting you do a v15 and Lee might want to just pick up the series, with above fixed. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> Either way works as long as there is an immutable branch somewhere. Thanks, Jonathan > > Thanks, > > > > Jonathan > > > >> --- > > > >> diff --git a/drivers/iio/adc/qcom-spmi-rradc.c b/drivers/iio/adc/qcom-spmi-rradc.c > >> new file mode 100644 > >> index 000000000000..c437546d8a4c > >> --- /dev/null > >> +++ b/drivers/iio/adc/qcom-spmi-rradc.c > > > > > > .. > > > >> + > >> +/* > >> + * These functions explicitly cast int64_t to int. > >> + * They will never overflow, as the values are small enough. > > > > See below. I don't think this gets used... > > > >> + */ > >> +static int rradc_post_process_batt_id(struct rradc_chip *chip, u16 adc_code, > >> + int *result_ohms) > >> +{ > >> + uint32_t current_value; > >> + int64_t r_id; > >> + > >> + current_value = chip->batt_id_data; > >> + r_id = ((int64_t)adc_code * RR_ADC_FS_VOLTAGE_MV); > >> + r_id = div64_s64(r_id, (RR_ADC_CHAN_MSB * current_value)); > >> + *result_ohms = (int)(r_id * MILLI); > >> + > >> + return 0; > >> +} > >> + > > > > > >> + > >> +static int rradc_read_raw(struct iio_dev *indio_dev, > >> + struct iio_chan_spec const *chan_spec, int *val, > >> + int *val2, long mask) > >> +{ > >> + struct rradc_chip *chip = iio_priv(indio_dev); > >> + const struct rradc_channel *chan; > >> + int ret; > >> + u16 adc_code; > >> + > >> + if (chan_spec->address >= RR_ADC_CHAN_MAX) { > >> + dev_err(chip->dev, "Invalid channel index:%lu\n", > >> + chan_spec->address); > >> + return -EINVAL; > >> + } > >> + > >> + switch (mask) { > >> + case IIO_CHAN_INFO_SCALE: > >> + return rradc_read_scale(chip, chan_spec->address, val, val2); > >> + case IIO_CHAN_INFO_OFFSET: > >> + return rradc_read_offset(chip, chan_spec->address, val); > >> + case IIO_CHAN_INFO_RAW: > >> + ret = rradc_do_conversion(chip, chan_spec->address, &adc_code); > >> + if (ret < 0) > >> + return ret; > >> + > >> + *val = adc_code; > >> + return IIO_VAL_INT; > >> + case IIO_CHAN_INFO_PROCESSED: > > > > This doesn't seem to apply to any channels.... > > > >> + chan = &rradc_chans[chan_spec->address]; > >> + if (!chan->scale_fn) > >> + return -EINVAL; > >> + ret = rradc_do_conversion(chip, chan_spec->address, &adc_code); > >> + if (ret < 0) > >> + return ret; > >> + > >> + *val = chan->scale_fn(chip, adc_code, val); > >> + return IIO_VAL_INT; > >> + default: > >> + return -EINVAL; > >> + } > >> +} > >> + > >> +static int rradc_read_label(struct iio_dev *indio_dev, > >> + struct iio_chan_spec const *chan, char *label) > >> +{ > >> + return snprintf(label, PAGE_SIZE, "%s\n", > >> + rradc_chans[chan->address].label); > >> +} > >> + > >> +static const struct iio_info rradc_info = { > >> + .read_raw = rradc_read_raw, > >> + .read_label = rradc_read_label, > >> +}; > >> + > >> +static const struct rradc_channel rradc_chans[RR_ADC_CHAN_MAX] = { > >> + { > >> + .label = "batt_id", > >> + .scale_fn = rradc_post_process_batt_id, > >> + .lsb = RR_ADC_BATT_ID_5_LSB, > >> + .status = RR_ADC_BATT_ID_STS, > >> + .size = 6, > >> + .trigger_addr = RR_ADC_BATT_ID_TRIGGER, > >> + .trigger_mask = BIT(0), > >> + }, { > >> + .label = "batt", > >> + .lsb = RR_ADC_BATT_THERM_LSB, > >> + .status = RR_ADC_BATT_THERM_STS, > >> + .size = 2, > >> + .trigger_addr = RR_ADC_BATT_THERM_TRIGGER, > >> + }, { > >> + .label = "pmi8998_skin", > >> + .lsb = RR_ADC_SKIN_TEMP_LSB, > >> + .status = RR_ADC_AUX_THERM_STS, > >> + .size = 2, > >> + .trigger_addr = RR_ADC_AUX_THERM_TRIGGER, > >> + }, { > >> + .label = "usbin_i", > >> + .lsb = RR_ADC_USB_IN_I_LSB, > >> + .status = RR_ADC_USB_IN_I_STS, > >> + .size = 2, > >> + .trigger_addr = RR_ADC_USB_IN_I_TRIGGER, > >> + }, { > >> + .label = "usbin_v", > >> + .lsb = RR_ADC_USB_IN_V_LSB, > >> + .status = RR_ADC_USB_IN_V_STS, > >> + .size = 2, > >> + .trigger_addr = RR_ADC_USB_IN_V_TRIGGER, > >> + .trigger_mask = BIT(7), > >> + }, { > >> + .label = "dcin_i", > >> + .lsb = RR_ADC_DC_IN_I_LSB, > >> + .status = RR_ADC_DC_IN_I_STS, > >> + .size = 2, > >> + .trigger_addr = RR_ADC_DC_IN_I_TRIGGER, > >> + }, { > >> + .label = "dcin_v", > >> + .lsb = RR_ADC_DC_IN_V_LSB, > >> + .status = RR_ADC_DC_IN_V_STS, > >> + .size = 2, > >> + .trigger_addr = RR_ADC_DC_IN_V_TRIGGER, > >> + }, { > >> + .label = "pmi8998_die", > >> + .lsb = RR_ADC_PMI_DIE_TEMP_LSB, > >> + .status = RR_ADC_PMI_DIE_TEMP_STS, > >> + .size = 2, > >> + .trigger_addr = RR_ADC_PMI_DIE_TEMP_TRIGGER, > >> + .trigger_mask = RR_ADC_TRIGGER_EVERY_CYCLE, > >> + }, { > >> + .label = "chg", > >> + .lsb = RR_ADC_CHARGER_TEMP_LSB, > >> + .status = RR_ADC_CHARGER_TEMP_STS, > >> + .size = 2, > >> + .trigger_addr = RR_ADC_CHARGER_TEMP_TRIGGER, > >> + }, { > >> + .label = "gpio", > >> + .lsb = RR_ADC_GPIO_LSB, > >> + .status = RR_ADC_GPIO_STS, > >> + .size = 2, > >> + .trigger_addr = RR_ADC_GPIO_TRIGGER, > >> + }, > >> +}; > >> + > >> +static const struct iio_chan_spec rradc_iio_chans[RR_ADC_CHAN_MAX] = { > >> + { > >> + .type = IIO_RESISTANCE, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > >> + .address = RR_ADC_BATT_ID, > >> + .channel = 0, > >> + .indexed = 1, > >> + }, { > >> + .type = IIO_TEMP, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > >> + .address = RR_ADC_BATT_THERM, > >> + .channel = 0, > >> + .indexed = 1, > >> + }, { > >> + .type = IIO_TEMP, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > >> + BIT(IIO_CHAN_INFO_SCALE) | > >> + BIT(IIO_CHAN_INFO_OFFSET), > >> + .address = RR_ADC_SKIN_TEMP, > >> + .channel = 1, > >> + .indexed = 1, > >> + }, { > >> + .type = IIO_CURRENT, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > >> + BIT(IIO_CHAN_INFO_SCALE), > >> + .address = RR_ADC_USBIN_I, > >> + .channel = 0, > >> + .indexed = 1, > >> + }, { > >> + .type = IIO_VOLTAGE, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > >> + BIT(IIO_CHAN_INFO_SCALE), > >> + .address = RR_ADC_USBIN_V, > >> + .channel = 0, > >> + .indexed = 1, > >> + }, { > >> + .type = IIO_CURRENT, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > >> + BIT(IIO_CHAN_INFO_SCALE), > >> + .address = RR_ADC_DCIN_I, > >> + .channel = 1, > >> + .indexed = 1, > >> + }, { > >> + .type = IIO_VOLTAGE, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > >> + BIT(IIO_CHAN_INFO_SCALE), > >> + .address = RR_ADC_DCIN_V, > >> + .channel = 1, > >> + .indexed = 1, > >> + }, { > >> + .type = IIO_TEMP, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > >> + BIT(IIO_CHAN_INFO_SCALE) | > >> + BIT(IIO_CHAN_INFO_OFFSET), > >> + .address = RR_ADC_DIE_TEMP, > >> + .channel = 2, > >> + .indexed = 1, > >> + }, { > >> + .type = IIO_TEMP, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > >> + BIT(IIO_CHAN_INFO_OFFSET) | > >> + BIT(IIO_CHAN_INFO_SCALE), > >> + .address = RR_ADC_CHG_TEMP, > >> + .channel = 3, > >> + .indexed = 1, > >> + }, { > >> + .type = IIO_VOLTAGE, > >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > >> + BIT(IIO_CHAN_INFO_SCALE), > >> + .address = RR_ADC_GPIO, > >> + .channel = 2, > >> + .indexed = 1, > >> + }, > >> +}; > >> + > > >