Re: [PATCH v14 05/10] iio: adc: qcom-spmi-rradc: introduce round robin adc

From: Caleb Connolly
Date: Mon May 09 2022 - 07:53:24 EST




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.

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)