Messages in this thread | | | Date | Mon, 9 May 2022 12:53:12 +0100 | Subject | Re: [PATCH v14 05/10] iio: adc: qcom-spmi-rradc: introduce round robin adc | From | Caleb Connolly <> |
| |
On 01/05/2022 18:38, Jonathan Cameron wrote: > On Fri, 29 Apr 2022 23:09:00 +0100 > Caleb Connolly <caleb.connolly@linaro.org> 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@linaro.org> > 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. > > 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, >> + }, >> +}; >> + >
-- Kind Regards, Caleb (they/he)
| |