| Message ID | 1411046123-30625-1-git-send-email-iivanov@mm-sol.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Ivan, On Thursday 18 September 2014 06:45 PM, Ivan T. Ivanov wrote: > The current ADC is peripheral of Qualcomm SPMI PMIC chips. It has > 16 bits resolution and register space inside PMIC accessible across > SPMI bus. > > The driver registers itself through IIO interface. > > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> > --- > .../devicetree/bindings/iio/adc/qcom,spmi-iadc.txt | 61 ++ <snip> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 11b048a..77274e4 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -279,4 +279,15 @@ config XILINX_XADC > The driver can also be build as a module. If so, the module will be called > xilinx-xadc. > > +config QCOM_SPMI_IADC > + tristate "Qualcomm SPMI PMIC current ADC" > + select REGMAP_SPMI > + depends on IIO May be you need to add "SPMI" in depends on because without enabling that it breaks. > + help > + This is the IIO Current ADC driver for Qualcomm QPNP IADC Chip. > + > + The driver supports single mode operation to read from upto seven channel > + configuration that include reading the external/internal Rsense, CSP_EX, > + CSN_EX pair along with the gain and offset calibration. > + > endmenu <snip> > + > + return iadc_write(iadc, IADC_EN_CTL1, data); > +} > + > +static void iadc_status_show(struct iadc_chip *iadc) > +{ > + u8 mode, sta1, chan, dig, en, req; During compilation, getting a waring as below for all variables, rivers/iio/adc/qcom-spmi-iadc.c: In function ‘iadc_poll_wait_eoc’: drivers/iio/adc/qcom-spmi-iadc.c:227:10: warning: ‘en’ may be used uninitialized in this function [-Wmaybe-uninitialized] dev_warn(iadc->dev, ^ > + int ret; > + > + ret = iadc_read(iadc, IADC_MODE_CTL, &mode); > + if (ret < 0) > + return; > + > + ret = iadc_read(iadc, IADC_DIG_PARAM, &dig); Thanks, --Kiran -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Kiran, On Fri, 2014-09-19 at 15:49 +0530, Kiran Padwal wrote: > Hi Ivan, > > On Thursday 18 September 2014 06:45 PM, Ivan T. Ivanov wrote: > > The current ADC is peripheral of Qualcomm SPMI PMIC chips. It has > > 16 bits resolution and register space inside PMIC accessible across > > SPMI bus. > > > > The driver registers itself through IIO interface. > > > > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> > > --- > > .../devicetree/bindings/iio/adc/qcom,spmi-iadc.txt | 61 ++ > > <snip> > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 11b048a..77274e4 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -279,4 +279,15 @@ config XILINX_XADC > > The driver can also be build as a module. If so, the module will be called > > xilinx-xadc. > > > > +config QCOM_SPMI_IADC > > + tristate "Qualcomm SPMI PMIC current ADC" > > + select REGMAP_SPMI > > + depends on IIO > > May be you need to add "SPMI" in depends on because without enabling that it breaks. Right. > > > + help > > + This is the IIO Current ADC driver for Qualcomm QPNP IADC Chip. > > + > > + The driver supports single mode operation to read from upto seven channel > > + configuration that include reading the external/internal Rsense, CSP_EX, > > + CSN_EX pair along with the gain and offset calibration. > > + > > endmenu > > <snip> > > > + > > + return iadc_write(iadc, IADC_EN_CTL1, data); > > +} > > + > > +static void iadc_status_show(struct iadc_chip *iadc) > > +{ > > + u8 mode, sta1, chan, dig, en, req; > > During compilation, getting a waring as below for all variables, > > rivers/iio/adc/qcom-spmi-iadc.c: In function ‘iadc_poll_wait_eoc’: > drivers/iio/adc/qcom-spmi-iadc.c:227:10: warning: ‘en’ may be used uninitialized in this function [-Wmaybe-uninitialized] > dev_warn(iadc->dev, > ^ > Right. I am not sure why I am not getting this warning. > > + int ret; > > + > > + ret = iadc_read(iadc, IADC_MODE_CTL, &mode); > > + if (ret < 0) > > + return; > > + > > + ret = iadc_read(iadc, IADC_DIG_PARAM, &dig); > Thank you for review. Regards, Ivan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/09/14 14:15, Ivan T. Ivanov wrote: > The current ADC is peripheral of Qualcomm SPMI PMIC chips. It has > 16 bits resolution and register space inside PMIC accessible across > SPMI bus. > > The driver registers itself through IIO interface. > > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> A few comments from me inline... > --- > .../devicetree/bindings/iio/adc/qcom,spmi-iadc.txt | 61 ++ > drivers/iio/adc/Kconfig | 11 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/qcom-spmi-iadc.c | 700 +++++++++++++++++++++ > 4 files changed, 773 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-iadc.txt > create mode 100644 drivers/iio/adc/qcom-spmi-iadc.c > > diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-iadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-iadc.txt > new file mode 100644 > index 0000000..77be22a > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-iadc.txt > @@ -0,0 +1,61 @@ > +Qualcomm's SPMI PMIC current ADC > + > +QPNP PMIC current ADC (IADC) provides interface to clients to read current. > +A 16 bit ADC is used for current measurements. > + > +IADC node: > + > +- compatible: > + Usage: required > + Value type: <string> > + Definition: Should contain "qcom,spmi-iadc". > + > +- reg: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: IADC base address and length in the SPMI PMIC register map. > + TRIM_CNST_RDS register address and length(1) > + > +- interrupts: > + Usage: optional > + Value type: <prop-encoded-array> > + Definition: End of conversion interrupt number. If property is > + missing driver will use polling to detect end of conversion > + completion. > + > +- qcom,rsense: > + Usage: optional > + Value type: <u32> > + Definition: External sense register value in Ohm. Defining this > + propery will instruct ADC to use external ADC sense channel. > + If property is not defined ADC will use internal sense channel. > + > +- qcom,rds-trim: > + Usage: optional > + Value type: <u32> > + Definition: Calaculate internal sense resistor value based TRIM_CNST_RDS, calculate? > + IADC RDS and manufacturer type. > + 0: Read the IADC and SMBB trim register and apply the default > + RSENSE if conditions are met. > + 1: Read the IADC, SMBB trim register and manufacturer type and > + apply the default RSENSE if conditions are met. > + 2: Read the IADC, SMBB trim register and apply the default RSENSE > + if conditions are met. > + If property is not defined driver will use qcom,rsense value if > + defined or internal sense resistor value trimmed into register. > + > +Example: > + /* IADC node */ > + pmic_iadc: iadc@3600 { > + compatible = "qcom,spmi-iadc"; > + reg = <0x3600 0x100>, > + <0x12f1 0x1>; > + interrupts = <0x0 0x36 0x0 IRQ_TYPE_EDGE_RISING>; > + qcom,rds-trim = <0>; > + }; > + > + /* IIO client node */ > + bat { > + io-channels = <&pmic_iadc 0>; > + io-channel-names = "iadc"; > + }; > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 11b048a..77274e4 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -279,4 +279,15 @@ config XILINX_XADC > The driver can also be build as a module. If so, the module will be called > xilinx-xadc. > > +config QCOM_SPMI_IADC > + tristate "Qualcomm SPMI PMIC current ADC" > + select REGMAP_SPMI > + depends on IIO > + help > + This is the IIO Current ADC driver for Qualcomm QPNP IADC Chip. > + > + The driver supports single mode operation to read from upto seven channel channels? You only seem to register one... > + configuration that include reading the external/internal Rsense, CSP_EX, > + CSN_EX pair along with the gain and offset calibration. > + > endmenu > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index ad81b51..c790543 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -30,3 +30,4 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o > obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o > xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o > obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o > +obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o > diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c > new file mode 100644 > index 0000000..fef9168 > --- /dev/null > +++ b/drivers/iio/adc/qcom-spmi-iadc.c > @@ -0,0 +1,700 @@ > +/* > + * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/bitops.h> > +#include <linux/completion.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/iio/iio.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#include <linux/mutex.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +/* IADC IADC register and bit definition */ > +#define IADC_REVISION2 0x1 > +#define IADC_REVISION2_SUPPORTED_IADC 1 > + > +#define IADC_PERPH_TYPE 0x4 > +#define IADC_PERPH_TYPE_ADC 8 > + > +#define IADC_PERPH_SUBTYPE 0x5 > +#define IADC_PERPH_SUBTYPE_IADC 3 > + > +#define IADC_STATUS1 0x8 > +#define IADC_STATUS1_OP_MODE 4 > +#define IADC_STATUS1_REQ_STS BIT(1) > +#define IADC_STATUS1_EOC BIT(0) > +#define IADC_STATUS1_REQ_STS_EOC_MASK 0x3 > + > +#define IADC_MODE_CTL 0x40 > +#define IADC_OP_MODE_SHIFT 3 > +#define IADC_OP_MODE_NORMAL 0 > +#define IADC_TRIM_EN BIT(0) > + > +#define IADC_EN_CTL1 0x46 > +#define IADC_EN_CTL1_SET BIT(7) > + > +#define IADC_CH_SEL_CTL 0x48 > + > +#define IADC_DIG_PARAM 0x50 > +#define IADC_DIG_DEC_RATIO_SEL_SHIFT 2 > + > +#define IADC_HW_SETTLE_DELAY 0x51 > + > +#define IADC_CONV_REQ 0x52 > +#define IADC_CONV_REQ_SET BIT(7) > + > +#define IADC_FAST_AVG_CTL 0x5a > +#define IADC_FAST_AVG_EN 0x5b > +#define IADC_FAST_AVG_EN_SET BIT(7) > + > +#define IADC_PERH_RESET_CTL3 0xda > +#define IADC_FOLLOW_WARM_RB BIT(2) > + > +#define IADC_DATA0 0x60 > +#define IADC_DATA1 0x61 > + > +#define IADC_SEC_ACCESS 0xd0 > +#define IADC_SEC_ACCESS_DATA 0xa5 > + > +#define IADC_INT_TEST_VAL 0xe1 > +#define IADC_MSB_OFFSET 0xf2 > +#define IADC_LSB_OFFSET 0xf3 > + > +#define IADC_NOMINAL_RSENSE 0xf4 > +#define IADC_NOMINAL_RSENSE_SIGN_MASK BIT(7) > + > +#define IADC_REF_GAIN_MICRO_VOLTS 17857 > + > +#define IADC_INTERNAL_RSENSE_OHMS 10000000 > +#define IADC_RSENSE_OHMS_PER_BIT 15625 > + > +#define IADC_TRIM_CNST_RDS_MASK 0x7 > + > +#define IADC_FACTORY_GF 0 > +#define IADC_FACTORY_SMIC 1 > +#define IADC_FACTORY_TSMC 2 > + > +#define IADC_TRIM2_FULLSCALE 127 > + > +#define IADC_RSENSE_DEFAULT_VALUE 7800000 > +#define IADC_RSENSE_DEFAULT_GF 9000000 > +#define IADC_RSENSE_DEFAULT_SMIC 9700000 > + > +#define IADC_CONV_TIME_MIN_US 2000 > +#define IADC_CONV_TIME_MAX_US 2100 > + > +#define IADC_DEF_PRESCALING 0 /* 1:1 */ > +#define IADC_DEF_DECIMATION 0 /* 512 */ > +#define IADC_DEF_HW_SETTLE_TIME 0 /* 0 us */ > +#define IADC_DEF_AVG_SAMPLES 0 /* 1 sample */ > + > +/* IADC IADC channel list */ > +#define IADC_INTERNAL_RSENSE 0 > +#define IADC_EXTERNAL_RSENSE 1 > +#define IADC_ALT_LEAD_PAIR 2 > +#define IADC_GAIN_17P857MV 3 > +#define IADC_OFFSET_SHORT_CADC_LEADS 4 > +#define IADC_OFFSET_CSP_CSN 5 > +#define IADC_OFFSET_CSP2_CSN2 6 > +#define IADC_CHANNEL_COUNT 7 > + > +/** > + * struct iadc_drv - IADC Current ADC device structure. > + * @regmap: regmap for register read/write. > + * @dev: This device pointer. > + * @factory: Chip manufacturer. > + * @base: base offset for the ADC peripheral. > + * @trim_rds: Address of the Trim Constant Rds register. > + * @rsense: Sense register in Ohms. > + * @ext_sense: Using external sense resistor. > + * @poll_eoc: Poll for end of conversion instead of waiting for IRQ. > + * @offset_raw: Raw offset value for the channel. > + * @gain_raw: Raw gain of the channel. > + * @lock: ADC lock for access to the peripheral. > + * @complete: ADC notification after end of conversion interrupt is received. > + * @iio_chan: IIO channel as registered into framework. > + */ > +struct iadc_chip { > + struct regmap *regmap; > + struct device *dev; > + u8 factory; > + u16 base; > + u16 trim_rds; > + int rsense; > + bool ext_sense; > + bool poll_eoc; > + u16 offset_raw; > + u16 gain_raw; > + struct mutex lock; > + struct completion complete; > + struct iio_chan_spec iio_chan; As explained below, better to have this as static const outside of here. > +}; > + > +static int iadc_read(struct iadc_chip *iadc, u16 offset, u8 *data) > +{ > + unsigned int val; > + int ret; > + > + ret = regmap_read(iadc->regmap, iadc->base + offset, &val); > + if (!ret) > + *data = val; > + > + return ret; > +} Borderline whether these wrappers add significant value... I suppose they hide the application of the offset. > + > +static int iadc_write(struct iadc_chip *iadc, u16 offset, u8 data) > +{ > + return regmap_write(iadc->regmap, iadc->base + offset, data); > +} > + > +static int iadc_reset(struct iadc_chip *iadc) > +{ > + u8 data; > + int ret; > + > + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); > + if (ret < 0) > + return ret; > + > + ret = iadc_read(iadc, IADC_PERH_RESET_CTL3, &data); > + if (ret < 0) > + return ret; > + > + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); > + if (ret < 0) > + return ret; > + > + data |= IADC_FOLLOW_WARM_RB; > + > + return iadc_write(iadc, IADC_PERH_RESET_CTL3, data); > +} > + > +static int iadc_enable(struct iadc_chip *iadc, bool state) iadc_set_state prefered as enable(false) does not necessarily mean disable. > +{ > + u8 data = 0; > + > + if (state) > + data = IADC_EN_CTL1_SET; > + > + return iadc_write(iadc, IADC_EN_CTL1, data); > +} > + > +static void iadc_status_show(struct iadc_chip *iadc) > +{ > + u8 mode, sta1, chan, dig, en, req; > + int ret; > + > + ret = iadc_read(iadc, IADC_MODE_CTL, &mode); > + if (ret < 0) > + return; > + > + ret = iadc_read(iadc, IADC_DIG_PARAM, &dig); > + if (ret < 0) > + return; > + > + ret = iadc_read(iadc, IADC_CH_SEL_CTL, &chan); > + if (ret < 0) > + return; > + > + ret = iadc_read(iadc, IADC_CONV_REQ, &req); > + if (ret < 0) > + return; > + > + ret = iadc_read(iadc, IADC_STATUS1, &sta1); > + if (ret < 0) > + return; > + > + ret = iadc_read(iadc, IADC_EN_CTL1, &en); > + if (ret < 0) > + return; > + > + dev_warn(iadc->dev, > + "mode:%02x en:%02x chan:%02x dig:%02x req:%02x sta1:%02x\n", > + mode, en, chan, dig, req, sta1); > +} > + > +static int iadc_configure(struct iadc_chip *iadc, int channel) > +{ > + u8 decim, mode; > + int ret; > + > + /* Mode selection */ > + mode = (IADC_OP_MODE_NORMAL << IADC_OP_MODE_SHIFT) | IADC_TRIM_EN; > + ret = iadc_write(iadc, IADC_MODE_CTL, mode); > + if (ret < 0) > + return ret; > + > + /* Channel selection */ > + ret = iadc_write(iadc, IADC_CH_SEL_CTL, channel); > + if (ret < 0) > + return ret; > + > + /* Digital parameter setup */ > + decim = IADC_DEF_DECIMATION << IADC_DIG_DEC_RATIO_SEL_SHIFT; > + ret = iadc_write(iadc, IADC_DIG_PARAM, decim); > + if (ret < 0) > + return ret; > + > + /* HW settle time delay */ > + ret = iadc_write(iadc, IADC_HW_SETTLE_DELAY, IADC_DEF_HW_SETTLE_TIME); > + if (ret < 0) > + return ret; > + > + ret = iadc_write(iadc, IADC_FAST_AVG_CTL, IADC_DEF_AVG_SAMPLES); > + if (ret < 0) > + return ret; > + > + if (IADC_DEF_AVG_SAMPLES) > + ret = iadc_write(iadc, IADC_FAST_AVG_EN, IADC_FAST_AVG_EN_SET); > + else > + ret = iadc_write(iadc, IADC_FAST_AVG_EN, 0); > + > + if (ret < 0) > + return ret; > + > + if (!iadc->poll_eoc) > + reinit_completion(&iadc->complete); > + > + ret = iadc_enable(iadc, true); > + if (ret < 0) > + return ret; > + > + /* Request conversion */ > + return iadc_write(iadc, IADC_CONV_REQ, IADC_CONV_REQ_SET); > +} > + > +static int iadc_poll_wait_eoc(struct iadc_chip *iadc, int interval_us) > +{ > + int ret, count, retry; > + u8 sta1; > + > + retry = interval_us / IADC_CONV_TIME_MIN_US; > + > + for (count = 0; count < retry; count++) { > + ret = iadc_read(iadc, IADC_STATUS1, &sta1); > + if (ret < 0) > + return ret; > + > + sta1 &= IADC_STATUS1_REQ_STS_EOC_MASK; > + if (sta1 == IADC_STATUS1_EOC) > + return 0; > + > + usleep_range(IADC_CONV_TIME_MIN_US, IADC_CONV_TIME_MAX_US); > + } > + > + iadc_status_show(iadc); What is this for? Left over from debugging the driver? > + > + return -ETIMEDOUT; > +} > + > +static int iadc_read_result(struct iadc_chip *iadc, u16 *data) > +{ > + u8 lsb, msb; > + int ret; > + > + ret = iadc_read(iadc, IADC_DATA0, &lsb); > + if (ret < 0) > + return ret; > + > + ret = iadc_read(iadc, IADC_DATA1, &msb); > + if (ret < 0) > + return ret; > + > + *data = (msb << 8) | lsb; > + > + return 0; > +} > + > +static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data) > +{ > + int wait, ret; > + > + ret = iadc_configure(iadc, chan); > + if (ret < 0) > + goto exit; > + > + wait = BIT(IADC_DEF_AVG_SAMPLES) * IADC_CONV_TIME_MIN_US * 2; > + > + if (iadc->poll_eoc) { > + ret = iadc_poll_wait_eoc(iadc, wait); > + } else { > + ret = wait_for_completion_timeout(&iadc->complete, wait); > + if (!ret) > + ret = -ETIMEDOUT; > + else > + /* double check conversion status */ > + ret = iadc_poll_wait_eoc(iadc, IADC_CONV_TIME_MIN_US); > + } > + > + if (!ret) > + ret = iadc_read_result(iadc, data); > +exit: > + iadc_enable(iadc, false); > + if (ret < 0) > + dev_err(iadc->dev, "conversion failed\n"); > + > + return ret; > +} > + > +static int iadc_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct iadc_chip *iadc = iio_priv(indio_dev); > + long rsense_ua, rsense_uv, rsense_raw; > + int ret = -EINVAL, negative; > + u16 adc_raw; > + > + mutex_lock(&iadc->lock); > + > + switch (mask) { > + case IIO_CHAN_INFO_PROCESSED: > + ret = iadc_do_conversion(iadc, chan->channel, &adc_raw); > + if (ret < 0) > + goto exit; > + > + rsense_raw = adc_raw - iadc->offset_raw; > + > + rsense_uv = (rsense_raw * IADC_REF_GAIN_MICRO_VOLTS); > + rsense_uv /= iadc->gain_raw - iadc->offset_raw; > + > + negative = 0; > + if (rsense_uv < 0) { > + negative = 1; > + rsense_uv = -rsense_uv; > + } > + > + rsense_ua = rsense_uv; > + > + do_div(rsense_ua, iadc->rsense); > + > + if (negative) > + rsense_ua = -rsense_ua; > + > + dev_dbg(iadc->dev, "offset %d, gain %d, adc %d, V=%ld,uV, I=%ld,uA\n", > + iadc->offset_raw, iadc->gain_raw, adc_raw, > + rsense_uv, rsense_ua); > + > + *val = rsense_ua; Given the naming this seems unlikely to be in millamps? > + ret = IIO_VAL_INT; > + break; > + default: > + break; > + } > + > +exit: > + mutex_unlock(&iadc->lock); > + > + return ret; > +} > + > +static const struct iio_info iadc_info = { > + .read_raw = iadc_read_raw, > + .driver_module = THIS_MODULE, > +}; > + > +static irqreturn_t iadc_isr(int irq, void *dev_id) > +{ > + struct iadc_chip *iadc = dev_id; > + > + complete(&iadc->complete); > + > + return IRQ_HANDLED; > +} > + > +static int iadc_update_trim_offset(struct iadc_chip *iadc) > +{ > + u16 adc_raw; > + u8 lsb, msb; > + int ret, chan; > + > + ret = iadc_do_conversion(iadc, IADC_GAIN_17P857MV, &adc_raw); > + if (ret < 0) > + return ret; > + > + iadc->gain_raw = adc_raw; There is no real purpose in having the local variable adc_raw. It won't get written unless everything is fine anyway so why not use iadc->gain_raw directly in the do_conversion call. > + > + chan = IADC_OFFSET_CSP2_CSN2; > + if (iadc->ext_sense) > + chan = IADC_OFFSET_CSP_CSN; > + > + ret = iadc_do_conversion(iadc, chan, &adc_raw); > + if (ret < 0) > + return ret; > + > + iadc->offset_raw = adc_raw; > + > + if ((iadc->gain_raw - iadc->offset_raw) == 0) { > + dev_err(iadc->dev, "error: offset %d gain %d\n", > + iadc->offset_raw, iadc->gain_raw); > + return -EINVAL; > + } > + > + msb = adc_raw >> 8; > + lsb = adc_raw & 0xff; Do these directly where needed rather than bothering with local variables. > + > + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); > + if (ret < 0) > + return ret; > + > + ret = iadc_write(iadc, IADC_MSB_OFFSET, msb); > + if (ret < 0) > + return ret; > + > + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); > + if (ret < 0) > + return ret; > + > + return iadc_write(iadc, IADC_LSB_OFFSET, lsb); > +} > + > +static int iadc_version_check(struct iadc_chip *iadc) > +{ > + u8 revision, type, subtype; > + int ret; > + > + ret = iadc_read(iadc, IADC_PERPH_TYPE, &type); > + if (ret < 0) > + return ret; > + > + if (type < IADC_PERPH_TYPE_ADC) { > + dev_err(iadc->dev, "%d is not ADC\n", type); > + return -EINVAL; > + } > + > + ret = iadc_read(iadc, IADC_PERPH_SUBTYPE, &subtype); > + if (ret < 0) > + return ret; > + > + if (subtype < IADC_PERPH_SUBTYPE_IADC) { > + dev_err(iadc->dev, "%d is not IADC\n", subtype); > + return -EINVAL; > + } > + > + ret = iadc_read(iadc, IADC_REVISION2, &revision); > + if (ret < 0) > + return ret; > + > + if (revision < IADC_REVISION2_SUPPORTED_IADC) { > + dev_err(iadc->dev, "revision %d not supported\n", revision); > + return -EINVAL; > + } > + > + return iadc_read(iadc, IADC_INT_TEST_VAL, &iadc->factory); > +} > + > +static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node) > +{ > + unsigned int trim_val; > + u8 rsense, const_rds; > + int ret, negative; > + u32 trim_type; > + > + ret = of_property_read_u32(node, "qcom,rsense", &iadc->rsense); > + if (!ret) { > + iadc->ext_sense = true; > + return 0; > + } > + > + ret = iadc_read(iadc, IADC_NOMINAL_RSENSE, &rsense); > + if (ret < 0) > + return ret; > + > + negative = 0; > + if (rsense & IADC_NOMINAL_RSENSE_SIGN_MASK) > + negative = 1; I'm a little confused. The docs say that rsense is a resistor value in ohms (u32). Why does bit 7 allow encode other information? > + > + rsense &= ~IADC_NOMINAL_RSENSE_SIGN_MASK; > + > + iadc->rsense = IADC_INTERNAL_RSENSE_OHMS; > + if (negative) > + iadc->rsense -= rsense * IADC_RSENSE_OHMS_PER_BIT; > + else > + iadc->rsense += rsense * IADC_RSENSE_OHMS_PER_BIT; > + > + if (rsense < IADC_TRIM2_FULLSCALE) > + return 0; > + /* > + * Trim register is "saturated", check for specific trim value > + * based on manufacturer and RDS constant > + */ > + ret = of_property_read_u32(node, "qcom,rds-trim", &trim_type); > + if (ret) > + return 0; > + > + ret = regmap_read(iadc->regmap, iadc->trim_rds, &trim_val); > + if (ret < 0) > + return ret; > + > + const_rds = trim_val & IADC_TRIM_CNST_RDS_MASK; > + > + dev_dbg(iadc->dev, "factory %d trim type %d rsense %d const %d\n", > + iadc->factory, trim_type, rsense, const_rds); > + > + switch (trim_type) { > + case 0: > + if (const_rds == 2) > + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; So this is overwriting rsense if properties are obeyed. Would it not make sense to do this before using the rsense value above (if these constraints are not met?) > + break; > + case 1: > + if (const_rds >= 2) { > + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; > + } else if (const_rds < 2) { > + if (iadc->factory == IADC_FACTORY_GF) > + iadc->rsense = IADC_RSENSE_DEFAULT_GF; > + else if (iadc->factory == IADC_FACTORY_SMIC) > + iadc->rsense = IADC_RSENSE_DEFAULT_SMIC; > + } > + break; > + case 2: > + if (const_rds > 0 && const_rds <= 2) > + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int iadc_probe(struct platform_device *pdev) > +{ > + struct device_node *node = pdev->dev.of_node; > + struct device *dev = &pdev->dev; > + struct iio_chan_spec *iio_chan; > + struct iio_dev *indio_dev; > + struct iadc_chip *iadc; > + struct resource *res; > + struct regmap *regmap; > + int ret, irq_eoc; > + > + regmap = dev_get_regmap(dev->parent, NULL); > + if (!regmap) > + return -ENODEV; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*iadc)); > + if (!indio_dev) > + return -ENOMEM; Spinning the order of the regmap get and iio_device_alloc would perhaps give a cleaner result as you could then fill iadc->regmap directly... (marginal!) > + > + iadc = iio_priv(indio_dev); > + iadc->dev = dev; > + iadc->regmap = regmap; > + init_completion(&iadc->complete); > + mutex_init(&iadc->lock); > + > + platform_set_drvdata(pdev, iadc); > + > + res = platform_get_resource(pdev, IORESOURCE_REG, 0); > + if (!res) > + return -ENODEV; > + > + iadc->base = res->start; > + > + res = platform_get_resource(pdev, IORESOURCE_REG, 1); > + if (!res) > + return -ENODEV; > + > + iadc->trim_rds = res->start; > + > + ret = iadc_version_check(iadc); > + if (ret < 0) > + return -ENODEV; > + > + ret = iadc_rsense_read(iadc, node); > + if (ret < 0) > + return -ENODEV; > + > + dev_dbg(iadc->dev, "%s sense resistor %d Ohm\n", iadc->ext_sense ? > + "external" : "internal", iadc->rsense); > + > + irq_eoc = platform_get_irq(pdev, 0); > + if (irq_eoc == -EPROBE_DEFER) > + return irq_eoc; > + > + if (irq_eoc < 0) > + iadc->poll_eoc = true; > + > + ret = iadc_reset(iadc); > + if (ret < 0) { > + dev_err(dev, "reset failed\n"); > + return ret; > + } > + > + if (!iadc->poll_eoc) { > + ret = devm_request_irq(dev, irq_eoc, iadc_isr, 0, > + "spmi-iadc", iadc); > + if (!ret) > + enable_irq_wake(irq_eoc); > + else > + return ret; > + } else { > + device_init_wakeup(iadc->dev, 1); > + } > + > + ret = iadc_update_trim_offset(iadc); > + if (ret < 0) { > + dev_err(dev, "failed trim offset calibration\n"); > + return ret; > + } > + This stuff is basically constant configuration data, except that you have two choices. Just have two static const struct iio_chan entries outside here and pick between them based on ext_sense. Also, why give them different channel numbers? Looks like it is just to distinguish them in driver. If so, use address instead. e.g. static const struct iio_chan_spec iadc_channel_ext_rsense = { .type = IIO_CURRENT, .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), .address = 1, .datasheet_name = "EXTERNAL_RSENSE", }; static const struct iio_chan_spec iadc_channel_int_rsense = { .type = IIO_CURRENT, .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), .address = 0, .datasheet_name = "INTERNAL_RSENSE", }; No need to have a copy in iadc then... The only time dynamic allocation of iio_chan_spec structures makes sense is when there are lots of combinations and the static const approach becomes too unweildy. > + iio_chan = &iadc->iio_chan; > + iio_chan->type = IIO_CURRENT; > + iio_chan->indexed = 1; > + iio_chan->info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED); > + iio_chan->scan_type.sign = 's'; > + iio_chan->scan_type.realbits = 16; > + iio_chan->scan_type.storagebits = 16; > + > + if (iadc->ext_sense) { > + iio_chan->channel = 1; > + iio_chan->datasheet_name = "EXTERNAL_RSENSE"; > + } else { > + iio_chan->channel = 0; > + iio_chan->datasheet_name = "INTERNAL_RSENSE"; > + } > + > + indio_dev->dev.parent = dev; > + indio_dev->dev.of_node = node; > + indio_dev->name = pdev->name; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->info = &iadc_info; > + indio_dev->channels = iio_chan; > + indio_dev->num_channels = 1; > + > + return devm_iio_device_register(dev, indio_dev); > +} > + > +static const struct of_device_id iadc_match_table[] = { > + { .compatible = "qcom,spmi-iadc" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, iadc_match_table); > + > +static struct platform_driver iadc_driver = { > + .driver = { > + .name = "spmi-iadc", > + .of_match_table = iadc_match_table, > + }, > + .probe = iadc_probe, > +}; > +module_platform_driver(iadc_driver); > + > +MODULE_ALIAS("platform:spmi-iadc"); > +MODULE_DESCRIPTION("Qualcomm SPMI PMIC current ADC driver"); > +MODULE_LICENSE("GPL v2"); > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Sun, 2014-09-21 at 15:04 +0100, Jonathan Cameron wrote: > On 18/09/14 14:15, Ivan T. Ivanov wrote: <snip> > > +config QCOM_SPMI_IADC > > + tristate "Qualcomm SPMI PMIC current ADC" > > + select REGMAP_SPMI > > + depends on IIO > > + help > > + This is the IIO Current ADC driver for Qualcomm QPNP IADC Chip. > > + > > + The driver supports single mode operation to read from upto seven channel > channels? You only seem to register one... Will reword. > > + configuration that include reading the external/internal Rsense, CSP_EX, > > + CSN_EX pair along with the gain and offset calibration. > > + > > endmenu <snip> > > +struct iadc_chip { > > + struct regmap *regmap; > > + struct device *dev; > > + u8 factory; > > + u16 base; > > + u16 trim_rds; > > + int rsense; > > + bool ext_sense; > > + bool poll_eoc; > > + u16 offset_raw; > > + u16 gain_raw; > > + struct mutex lock; > > + struct completion complete; > > + struct iio_chan_spec iio_chan; > As explained below, better to have this as static const outside of > here. Thanks, already done as per Peter comments. > > +}; > > + > > +static int iadc_read(struct iadc_chip *iadc, u16 offset, u8 *data) > > +{ > > + unsigned int val; > > + int ret; > > + > > + ret = regmap_read(iadc->regmap, iadc->base + offset, &val); > > + if (!ret) > > + *data = val; > > + > > + return ret; > > +} > Borderline whether these wrappers add significant value... I suppose they > hide the application of the offset. Yes, and will like to keep them. > > + > > +static int iadc_write(struct iadc_chip *iadc, u16 offset, u8 data) > > +{ > > + return regmap_write(iadc->regmap, iadc->base + offset, data); > > +} > > + > > +static int iadc_reset(struct iadc_chip *iadc) > > +{ > > + u8 data; > > + int ret; > > + > > + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); > > + if (ret < 0) > > + return ret; > > + > > + ret = iadc_read(iadc, IADC_PERH_RESET_CTL3, &data); > > + if (ret < 0) > > + return ret; > > + > > + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); > > + if (ret < 0) > > + return ret; > > + > > + data |= IADC_FOLLOW_WARM_RB; > > + > > + return iadc_write(iadc, IADC_PERH_RESET_CTL3, data); > > +} > > + > > +static int iadc_enable(struct iadc_chip *iadc, bool state) > iadc_set_state prefered as enable(false) does not necessarily mean > disable. Ok. > > +static int iadc_poll_wait_eoc(struct iadc_chip *iadc, int interval_us) > > +{ > > + int ret, count, retry; > > + u8 sta1; > > + > > + retry = interval_us / IADC_CONV_TIME_MIN_US; > > + > > + for (count = 0; count < retry; count++) { > > + ret = iadc_read(iadc, IADC_STATUS1, &sta1); > > + if (ret < 0) > > + return ret; > > + > > + sta1 &= IADC_STATUS1_REQ_STS_EOC_MASK; > > + if (sta1 == IADC_STATUS1_EOC) > > + return 0; > > + > > + usleep_range(IADC_CONV_TIME_MIN_US, IADC_CONV_TIME_MAX_US); > > + } > > + > > + iadc_status_show(iadc); > What is this for? Left over from debugging the driver? This should not generally happen, but will be useful to see what was ADC configuration and statuses, when conversion time outs. > > + > > + return -ETIMEDOUT; > > +} > > + <snip> > > +static int iadc_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int *val, int *val2, long mask) > > +{ > > + struct iadc_chip *iadc = iio_priv(indio_dev); > > + long rsense_ua, rsense_uv, rsense_raw; > > + int ret = -EINVAL, negative; > > + u16 adc_raw; > > + > > + mutex_lock(&iadc->lock); > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_PROCESSED: > > + ret = iadc_do_conversion(iadc, chan->channel, &adc_raw); > > + if (ret < 0) > > + goto exit; > > + > > + rsense_raw = adc_raw - iadc->offset_raw; > > + > > + rsense_uv = (rsense_raw * IADC_REF_GAIN_MICRO_VOLTS); > > + rsense_uv /= iadc->gain_raw - iadc->offset_raw; > > + > > + negative = 0; > > + if (rsense_uv < 0) { > > + negative = 1; > > + rsense_uv = -rsense_uv; > > + } > > + > > + rsense_ua = rsense_uv; > > + > > + do_div(rsense_ua, iadc->rsense); > > + > > + if (negative) > > + rsense_ua = -rsense_ua; > > + > > + dev_dbg(iadc->dev, "offset %d, gain %d, adc %d, V=%ld,uV, I=%ld,uA\n", > > + iadc->offset_raw, iadc->gain_raw, adc_raw, > > + rsense_uv, rsense_ua); > > + > > + *val = rsense_ua; > Given the naming this seems unlikely to be in millamps? Yes, it is micro amps. I was unable to find what should be correct dimension. There is nothing in sysfs-bus-iio or bindings/iio. > > + ret = IIO_VAL_INT; > > + break; > > + default: > > + break; > > + } > > + > > +exit: > > + mutex_unlock(&iadc->lock); > > + > > + return ret; > > +} > > + <snip> > > + > > +static int iadc_update_trim_offset(struct iadc_chip *iadc) > > +{ > > + u16 adc_raw; > > + u8 lsb, msb; > > + int ret, chan; > > + > > + ret = iadc_do_conversion(iadc, IADC_GAIN_17P857MV, &adc_raw); > > + if (ret < 0) > > + return ret; > > + > > + iadc->gain_raw = adc_raw; > There is no real purpose in having the local variable adc_raw. > It won't get written unless everything is fine anyway so why > not use iadc->gain_raw directly in the do_conversion call. > > > + > > + chan = IADC_OFFSET_CSP2_CSN2; > > + if (iadc->ext_sense) > > + chan = IADC_OFFSET_CSP_CSN; > > + > > + ret = iadc_do_conversion(iadc, chan, &adc_raw); > > + if (ret < 0) > > + return ret; > > + > > + iadc->offset_raw = adc_raw; > > + > > + if ((iadc->gain_raw - iadc->offset_raw) == 0) { > > + dev_err(iadc->dev, "error: offset %d gain %d\n", > > + iadc->offset_raw, iadc->gain_raw); > > + return -EINVAL; > > + } > > + > > + msb = adc_raw >> 8; > > + lsb = adc_raw & 0xff; > Do these directly where needed rather than bothering with local > variables. Ok. No local variables, I get it :-) > > + > > + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); > > + if (ret < 0) > > + return ret; > > + > > + ret = iadc_write(iadc, IADC_MSB_OFFSET, msb); > > + if (ret < 0) > > + return ret; > > + > > + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); > > + if (ret < 0) > > + return ret; > > + > > + return iadc_write(iadc, IADC_LSB_OFFSET, lsb); > > +} > > + > > + > > +static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node) > > +{ > > + unsigned int trim_val; > > + u8 rsense, const_rds; > > + int ret, negative; > > + u32 trim_type; > > + > > + ret = of_property_read_u32(node, "qcom,rsense", &iadc->rsense); > > + if (!ret) { > > + iadc->ext_sense = true; > > + return 0; > > + } User specify external sense resistor value, which means external channel should be used. > > + > > + ret = iadc_read(iadc, IADC_NOMINAL_RSENSE, &rsense); > > + if (ret < 0) > > + return ret; > > + > > + negative = 0; > > + if (rsense & IADC_NOMINAL_RSENSE_SIGN_MASK) > > + negative = 1; > I'm a little confused. The docs say that rsense is a resistor value in > ohms (u32). Why does bit 7 allow encode other information? External sense resistor value not specified, driver will use internal ADC channel and internal sense resistor, so read what was trimmed during manufacturing process. Sense register value is read from register. If bit 7 of value is set value in register is negative. Register itself didn't contain value in ohms, but difference between desired and actual value. > > + > > + rsense &= ~IADC_NOMINAL_RSENSE_SIGN_MASK; > > + > > + iadc->rsense = IADC_INTERNAL_RSENSE_OHMS; > > + if (negative) > > + iadc->rsense -= rsense * IADC_RSENSE_OHMS_PER_BIT; > > + else > > + iadc->rsense += rsense * IADC_RSENSE_OHMS_PER_BIT; internal rsense = 10000000 + sign * 15625 * value; > > + > > + if (rsense < IADC_TRIM2_FULLSCALE) > > + return 0; > > + /* > > + * Trim register is "saturated", check for specific trim value > > + * based on manufacturer and RDS constant > > + */ > > + ret = of_property_read_u32(node, "qcom,rds-trim", &trim_type); > > + if (ret) > > + return 0; > > + > > + ret = regmap_read(iadc->regmap, iadc->trim_rds, &trim_val); > > + if (ret < 0) > > + return ret; > > + > > + const_rds = trim_val & IADC_TRIM_CNST_RDS_MASK; > > + > > + dev_dbg(iadc->dev, "factory %d trim type %d rsense %d const %d\n", > > + iadc->factory, trim_type, rsense, const_rds); > > + > > + switch (trim_type) { > > + case 0: > > + if (const_rds == 2) > > + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; > So this is overwriting rsense if properties are obeyed. Would it not > make sense to do this before using the rsense value above (if these > constraints are not met?) We are coming here only if value trimmed into IADC_NOMINAL_RSENSE register is saturated i.e, u8 value is 127, bit 7 is sign bit. Which means that internal resistor is far from desired value. And this depends on the factory used for manufacturing the chip. The whole logic here is simplified version, I believe, on the downstream implementation, which could be found here[1]. [1] https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/hwmon/qpnp-adc-current.c?h=msm-3.10 > > + break; > > + case 1: > > + if (const_rds >= 2) { > > + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; > > + } else if (const_rds < 2) { > > + if (iadc->factory == IADC_FACTORY_GF) > > + iadc->rsense = IADC_RSENSE_DEFAULT_GF; > > + else if (iadc->factory == IADC_FACTORY_SMIC) > > + iadc->rsense = IADC_RSENSE_DEFAULT_SMIC; > > + } > > + break; > > + case 2: > > + if (const_rds > 0 && const_rds <= 2) > > + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +static int iadc_probe(struct platform_device *pdev) > > +{ > > + struct device_node *node = pdev->dev.of_node; > > + struct device *dev = &pdev->dev; > > + struct iio_chan_spec *iio_chan; > > + struct iio_dev *indio_dev; > > + struct iadc_chip *iadc; > > + struct resource *res; > > + struct regmap *regmap; > > + int ret, irq_eoc; > > + > > + regmap = dev_get_regmap(dev->parent, NULL); > > + if (!regmap) > > + return -ENODEV; > > + > > + indio_dev = devm_iio_device_alloc(dev, sizeof(*iadc)); > > + if (!indio_dev) > > + return -ENOMEM; > Spinning the order of the regmap get and iio_device_alloc would perhaps > give a cleaner result as you could then fill iadc->regmap directly... > (marginal!) No local variables, right? :-) > > + > > + iadc = iio_priv(indio_dev); > > + iadc->dev = dev; > > + iadc->regmap = regmap; > > + init_completion(&iadc->complete); > > + mutex_init(&iadc->lock); > > + > > + platform_set_drvdata(pdev, iadc); > > + > > + res = platform_get_resource(pdev, IORESOURCE_REG, 0); > > + if (!res) > > + return -ENODEV; > > + > > + iadc->base = res->start; > > + > > + res = platform_get_resource(pdev, IORESOURCE_REG, 1); > > + if (!res) > > + return -ENODEV; > > + > > + iadc->trim_rds = res->start; > > + > > + ret = iadc_version_check(iadc); > > + if (ret < 0) > > + return -ENODEV; > > + > > + ret = iadc_rsense_read(iadc, node); > > + if (ret < 0) > > + return -ENODEV; > > + > > + dev_dbg(iadc->dev, "%s sense resistor %d Ohm\n", iadc->ext_sense ? > > + "external" : "internal", iadc->rsense); > > + > > + irq_eoc = platform_get_irq(pdev, 0); > > + if (irq_eoc == -EPROBE_DEFER) > > + return irq_eoc; > > + > > + if (irq_eoc < 0) > > + iadc->poll_eoc = true; > > + > > + ret = iadc_reset(iadc); > > + if (ret < 0) { > > + dev_err(dev, "reset failed\n"); > > + return ret; > > + } > > + > > + if (!iadc->poll_eoc) { > > + ret = devm_request_irq(dev, irq_eoc, iadc_isr, 0, > > + "spmi-iadc", iadc); > > + if (!ret) > > + enable_irq_wake(irq_eoc); > > + else > > + return ret; > > + } else { > > + device_init_wakeup(iadc->dev, 1); > > + } > > + > > + ret = iadc_update_trim_offset(iadc); > > + if (ret < 0) { > > + dev_err(dev, "failed trim offset calibration\n"); > > + return ret; > > + } > > + > This stuff is basically constant configuration data, except that you > have two choices. Just have two static const struct iio_chan entries > outside here and pick between them based on ext_sense. > Also, why give them different channel numbers? Looks like it is just > to distinguish them in driver. If so, use address instead. > e.g. > > static const struct iio_chan_spec iadc_channel_ext_rsense = { > .type = IIO_CURRENT, > .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > .address = 1, > .datasheet_name = "EXTERNAL_RSENSE", > }; > > static const struct iio_chan_spec iadc_channel_int_rsense = { > .type = IIO_CURRENT, > .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > .address = 0, > .datasheet_name = "INTERNAL_RSENSE", > }; > > No need to have a copy in iadc then... > > The only time dynamic allocation of iio_chan_spec structures > makes sense is when there are lots of combinations and the > static const approach becomes too unweildy. Already done. Thank you for review. Regards, Ivan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
<snip> > >>> +static int iadc_poll_wait_eoc(struct iadc_chip *iadc, int interval_us) >>> +{ >>> + int ret, count, retry; >>> + u8 sta1; >>> + >>> + retry = interval_us / IADC_CONV_TIME_MIN_US; >>> + >>> + for (count = 0; count < retry; count++) { >>> + ret = iadc_read(iadc, IADC_STATUS1, &sta1); >>> + if (ret < 0) >>> + return ret; >>> + >>> + sta1 &= IADC_STATUS1_REQ_STS_EOC_MASK; >>> + if (sta1 == IADC_STATUS1_EOC) >>> + return 0; >>> + >>> + usleep_range(IADC_CONV_TIME_MIN_US, IADC_CONV_TIME_MAX_US); >>> + } >>> + >>> + iadc_status_show(iadc); >> What is this for? Left over from debugging the driver? > > This should not generally happen, but will be useful to see what was > ADC configuration and statuses, when conversion time outs. Hmm. Fine I guess, just a lot of code for an unexpected condition Should probably be as dev_err if it occurs. > >>> + >>> + return -ETIMEDOUT; >>> +} >>> + > > <snip> > >>> +static int iadc_read_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, >>> + int *val, int *val2, long mask) >>> +{ >>> + struct iadc_chip *iadc = iio_priv(indio_dev); >>> + long rsense_ua, rsense_uv, rsense_raw; >>> + int ret = -EINVAL, negative; >>> + u16 adc_raw; >>> + >>> + mutex_lock(&iadc->lock); >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_PROCESSED: >>> + ret = iadc_do_conversion(iadc, chan->channel, &adc_raw); >>> + if (ret < 0) >>> + goto exit; >>> + >>> + rsense_raw = adc_raw - iadc->offset_raw; >>> + >>> + rsense_uv = (rsense_raw * IADC_REF_GAIN_MICRO_VOLTS); >>> + rsense_uv /= iadc->gain_raw - iadc->offset_raw; >>> + >>> + negative = 0; >>> + if (rsense_uv < 0) { >>> + negative = 1; >>> + rsense_uv = -rsense_uv; >>> + } >>> + >>> + rsense_ua = rsense_uv; >>> + >>> + do_div(rsense_ua, iadc->rsense); >>> + >>> + if (negative) >>> + rsense_ua = -rsense_ua; >>> + >>> + dev_dbg(iadc->dev, "offset %d, gain %d, adc %d, V=%ld,uV, I=%ld,uA\n", >>> + iadc->offset_raw, iadc->gain_raw, adc_raw, >>> + rsense_uv, rsense_ua); >>> + >>> + *val = rsense_ua; >> Given the naming this seems unlikely to be in millamps? > > Yes, it is micro amps. I was unable to find what should be correct > dimension. There is nothing in sysfs-bus-iio or bindings/iio. There is now (in the togreg branch of iio.git;) Wasn't until very recently though. Somehow it got lost in the move out of staging... The scaling comes form hwmon, With hindsight we should probably have ditched it in favour of using Volts and Amps as our base scaling. Less confusing, but too late now! > >>> + ret = IIO_VAL_INT; >>> + break; >>> + default: >>> + break; >>> + } >>> + >>> +exit: >>> + mutex_unlock(&iadc->lock); >>> + >>> + return ret; >>> +} >>> + > > <snip> >>> + >>> +static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node) >>> +{ >>> + unsigned int trim_val; >>> + u8 rsense, const_rds; >>> + int ret, negative; >>> + u32 trim_type; >>> + >>> + ret = of_property_read_u32(node, "qcom,rsense", &iadc->rsense); >>> + if (!ret) { >>> + iadc->ext_sense = true; >>> + return 0; >>> + } > > User specify external sense resistor value, which means external > channel should be used. > >>> + >>> + ret = iadc_read(iadc, IADC_NOMINAL_RSENSE, &rsense); >>> + if (ret < 0) >>> + return ret; >>> + >>> + negative = 0; >>> + if (rsense & IADC_NOMINAL_RSENSE_SIGN_MASK) >>> + negative = 1; >> I'm a little confused. The docs say that rsense is a resistor value in >> ohms (u32). Why does bit 7 allow encode other information? > > External sense resistor value not specified, driver will use internal > ADC channel and internal sense resistor, so read what was trimmed > during manufacturing process. Sense register value is read from > register. If bit 7 of value is set value in register is negative. > Register itself didn't contain value in ohms, but difference between > desired and actual value. Ah got it. Thanks for the explanation. Perhaps ensure that iadc->rsense is clearly in ohms wherease this local rsense is the register value (rename one of them!) > >>> + >>> + rsense &= ~IADC_NOMINAL_RSENSE_SIGN_MASK; >>> + >>> + iadc->rsense = IADC_INTERNAL_RSENSE_OHMS; >>> + if (negative) >>> + iadc->rsense -= rsense * IADC_RSENSE_OHMS_PER_BIT; >>> + else >>> + iadc->rsense += rsense * IADC_RSENSE_OHMS_PER_BIT; > > internal rsense = 10000000 + sign * 15625 * value; > >>> + >>> + if (rsense < IADC_TRIM2_FULLSCALE) >>> + return 0; >>> + /* >>> + * Trim register is "saturated", check for specific trim value >>> + * based on manufacturer and RDS constant >>> + */ >>> + ret = of_property_read_u32(node, "qcom,rds-trim", &trim_type); >>> + if (ret) >>> + return 0; >>> + >>> + ret = regmap_read(iadc->regmap, iadc->trim_rds, &trim_val); >>> + if (ret < 0) >>> + return ret; >>> + >>> + const_rds = trim_val & IADC_TRIM_CNST_RDS_MASK; >>> + >>> + dev_dbg(iadc->dev, "factory %d trim type %d rsense %d const %d\n", >>> + iadc->factory, trim_type, rsense, const_rds); >>> + >>> + switch (trim_type) { >>> + case 0: >>> + if (const_rds == 2) >>> + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; >> So this is overwriting rsense if properties are obeyed. Would it not >> make sense to do this before using the rsense value above (if these >> constraints are not met?) > > We are coming here only if value trimmed into IADC_NOMINAL_RSENSE > register is saturated i.e, u8 value is 127, bit 7 is sign bit. Which > means that internal resistor is far from desired value. And this > depends on the factory used for manufacturing the chip. The whole > logic here is simplified version, I believe, on the downstream > implementation, which could be found here[1]. > > [1] https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/hwmon/qpnp-adc-current.c?h=msm-3.10 > >>> + break; >>> + case 1: >>> + if (const_rds >= 2) { >>> + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; >>> + } else if (const_rds < 2) { >>> + if (iadc->factory == IADC_FACTORY_GF) >>> + iadc->rsense = IADC_RSENSE_DEFAULT_GF; >>> + else if (iadc->factory == IADC_FACTORY_SMIC) >>> + iadc->rsense = IADC_RSENSE_DEFAULT_SMIC; >>> + } >>> + break; >>> + case 2: >>> + if (const_rds > 0 && const_rds <= 2) >>> + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int iadc_probe(struct platform_device *pdev) >>> +{ >>> + struct device_node *node = pdev->dev.of_node; >>> + struct device *dev = &pdev->dev; >>> + struct iio_chan_spec *iio_chan; >>> + struct iio_dev *indio_dev; >>> + struct iadc_chip *iadc; >>> + struct resource *res; >>> + struct regmap *regmap; >>> + int ret, irq_eoc; >>> + >>> + regmap = dev_get_regmap(dev->parent, NULL); >>> + if (!regmap) >>> + return -ENODEV; >>> + >>> + indio_dev = devm_iio_device_alloc(dev, sizeof(*iadc)); >>> + if (!indio_dev) >>> + return -ENOMEM; >> Spinning the order of the regmap get and iio_device_alloc would perhaps >> give a cleaner result as you could then fill iadc->regmap directly... >> (marginal!) > > No local variables, right? :-) Well only when they actually make the code cleaner! > Thank you for review. You are welcome Jonathan > > Regards, > Ivan > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-iadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-iadc.txt new file mode 100644 index 0000000..77be22a --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-iadc.txt @@ -0,0 +1,61 @@ +Qualcomm's SPMI PMIC current ADC + +QPNP PMIC current ADC (IADC) provides interface to clients to read current. +A 16 bit ADC is used for current measurements. + +IADC node: + +- compatible: + Usage: required + Value type: <string> + Definition: Should contain "qcom,spmi-iadc". + +- reg: + Usage: required + Value type: <prop-encoded-array> + Definition: IADC base address and length in the SPMI PMIC register map. + TRIM_CNST_RDS register address and length(1) + +- interrupts: + Usage: optional + Value type: <prop-encoded-array> + Definition: End of conversion interrupt number. If property is + missing driver will use polling to detect end of conversion + completion. + +- qcom,rsense: + Usage: optional + Value type: <u32> + Definition: External sense register value in Ohm. Defining this + propery will instruct ADC to use external ADC sense channel. + If property is not defined ADC will use internal sense channel. + +- qcom,rds-trim: + Usage: optional + Value type: <u32> + Definition: Calaculate internal sense resistor value based TRIM_CNST_RDS, + IADC RDS and manufacturer type. + 0: Read the IADC and SMBB trim register and apply the default + RSENSE if conditions are met. + 1: Read the IADC, SMBB trim register and manufacturer type and + apply the default RSENSE if conditions are met. + 2: Read the IADC, SMBB trim register and apply the default RSENSE + if conditions are met. + If property is not defined driver will use qcom,rsense value if + defined or internal sense resistor value trimmed into register. + +Example: + /* IADC node */ + pmic_iadc: iadc@3600 { + compatible = "qcom,spmi-iadc"; + reg = <0x3600 0x100>, + <0x12f1 0x1>; + interrupts = <0x0 0x36 0x0 IRQ_TYPE_EDGE_RISING>; + qcom,rds-trim = <0>; + }; + + /* IIO client node */ + bat { + io-channels = <&pmic_iadc 0>; + io-channel-names = "iadc"; + }; diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 11b048a..77274e4 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -279,4 +279,15 @@ config XILINX_XADC The driver can also be build as a module. If so, the module will be called xilinx-xadc. +config QCOM_SPMI_IADC + tristate "Qualcomm SPMI PMIC current ADC" + select REGMAP_SPMI + depends on IIO + help + This is the IIO Current ADC driver for Qualcomm QPNP IADC Chip. + + The driver supports single mode operation to read from upto seven channel + configuration that include reading the external/internal Rsense, CSP_EX, + CSN_EX pair along with the gain and offset calibration. + endmenu diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index ad81b51..c790543 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -30,3 +30,4 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o +obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c new file mode 100644 index 0000000..fef9168 --- /dev/null +++ b/drivers/iio/adc/qcom-spmi-iadc.c @@ -0,0 +1,700 @@ +/* + * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/bitops.h> +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/iio/iio.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#include <linux/mutex.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +/* IADC IADC register and bit definition */ +#define IADC_REVISION2 0x1 +#define IADC_REVISION2_SUPPORTED_IADC 1 + +#define IADC_PERPH_TYPE 0x4 +#define IADC_PERPH_TYPE_ADC 8 + +#define IADC_PERPH_SUBTYPE 0x5 +#define IADC_PERPH_SUBTYPE_IADC 3 + +#define IADC_STATUS1 0x8 +#define IADC_STATUS1_OP_MODE 4 +#define IADC_STATUS1_REQ_STS BIT(1) +#define IADC_STATUS1_EOC BIT(0) +#define IADC_STATUS1_REQ_STS_EOC_MASK 0x3 + +#define IADC_MODE_CTL 0x40 +#define IADC_OP_MODE_SHIFT 3 +#define IADC_OP_MODE_NORMAL 0 +#define IADC_TRIM_EN BIT(0) + +#define IADC_EN_CTL1 0x46 +#define IADC_EN_CTL1_SET BIT(7) + +#define IADC_CH_SEL_CTL 0x48 + +#define IADC_DIG_PARAM 0x50 +#define IADC_DIG_DEC_RATIO_SEL_SHIFT 2 + +#define IADC_HW_SETTLE_DELAY 0x51 + +#define IADC_CONV_REQ 0x52 +#define IADC_CONV_REQ_SET BIT(7) + +#define IADC_FAST_AVG_CTL 0x5a +#define IADC_FAST_AVG_EN 0x5b +#define IADC_FAST_AVG_EN_SET BIT(7) + +#define IADC_PERH_RESET_CTL3 0xda +#define IADC_FOLLOW_WARM_RB BIT(2) + +#define IADC_DATA0 0x60 +#define IADC_DATA1 0x61 + +#define IADC_SEC_ACCESS 0xd0 +#define IADC_SEC_ACCESS_DATA 0xa5 + +#define IADC_INT_TEST_VAL 0xe1 +#define IADC_MSB_OFFSET 0xf2 +#define IADC_LSB_OFFSET 0xf3 + +#define IADC_NOMINAL_RSENSE 0xf4 +#define IADC_NOMINAL_RSENSE_SIGN_MASK BIT(7) + +#define IADC_REF_GAIN_MICRO_VOLTS 17857 + +#define IADC_INTERNAL_RSENSE_OHMS 10000000 +#define IADC_RSENSE_OHMS_PER_BIT 15625 + +#define IADC_TRIM_CNST_RDS_MASK 0x7 + +#define IADC_FACTORY_GF 0 +#define IADC_FACTORY_SMIC 1 +#define IADC_FACTORY_TSMC 2 + +#define IADC_TRIM2_FULLSCALE 127 + +#define IADC_RSENSE_DEFAULT_VALUE 7800000 +#define IADC_RSENSE_DEFAULT_GF 9000000 +#define IADC_RSENSE_DEFAULT_SMIC 9700000 + +#define IADC_CONV_TIME_MIN_US 2000 +#define IADC_CONV_TIME_MAX_US 2100 + +#define IADC_DEF_PRESCALING 0 /* 1:1 */ +#define IADC_DEF_DECIMATION 0 /* 512 */ +#define IADC_DEF_HW_SETTLE_TIME 0 /* 0 us */ +#define IADC_DEF_AVG_SAMPLES 0 /* 1 sample */ + +/* IADC IADC channel list */ +#define IADC_INTERNAL_RSENSE 0 +#define IADC_EXTERNAL_RSENSE 1 +#define IADC_ALT_LEAD_PAIR 2 +#define IADC_GAIN_17P857MV 3 +#define IADC_OFFSET_SHORT_CADC_LEADS 4 +#define IADC_OFFSET_CSP_CSN 5 +#define IADC_OFFSET_CSP2_CSN2 6 +#define IADC_CHANNEL_COUNT 7 + +/** + * struct iadc_drv - IADC Current ADC device structure. + * @regmap: regmap for register read/write. + * @dev: This device pointer. + * @factory: Chip manufacturer. + * @base: base offset for the ADC peripheral. + * @trim_rds: Address of the Trim Constant Rds register. + * @rsense: Sense register in Ohms. + * @ext_sense: Using external sense resistor. + * @poll_eoc: Poll for end of conversion instead of waiting for IRQ. + * @offset_raw: Raw offset value for the channel. + * @gain_raw: Raw gain of the channel. + * @lock: ADC lock for access to the peripheral. + * @complete: ADC notification after end of conversion interrupt is received. + * @iio_chan: IIO channel as registered into framework. + */ +struct iadc_chip { + struct regmap *regmap; + struct device *dev; + u8 factory; + u16 base; + u16 trim_rds; + int rsense; + bool ext_sense; + bool poll_eoc; + u16 offset_raw; + u16 gain_raw; + struct mutex lock; + struct completion complete; + struct iio_chan_spec iio_chan; +}; + +static int iadc_read(struct iadc_chip *iadc, u16 offset, u8 *data) +{ + unsigned int val; + int ret; + + ret = regmap_read(iadc->regmap, iadc->base + offset, &val); + if (!ret) + *data = val; + + return ret; +} + +static int iadc_write(struct iadc_chip *iadc, u16 offset, u8 data) +{ + return regmap_write(iadc->regmap, iadc->base + offset, data); +} + +static int iadc_reset(struct iadc_chip *iadc) +{ + u8 data; + int ret; + + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); + if (ret < 0) + return ret; + + ret = iadc_read(iadc, IADC_PERH_RESET_CTL3, &data); + if (ret < 0) + return ret; + + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); + if (ret < 0) + return ret; + + data |= IADC_FOLLOW_WARM_RB; + + return iadc_write(iadc, IADC_PERH_RESET_CTL3, data); +} + +static int iadc_enable(struct iadc_chip *iadc, bool state) +{ + u8 data = 0; + + if (state) + data = IADC_EN_CTL1_SET; + + return iadc_write(iadc, IADC_EN_CTL1, data); +} + +static void iadc_status_show(struct iadc_chip *iadc) +{ + u8 mode, sta1, chan, dig, en, req; + int ret; + + ret = iadc_read(iadc, IADC_MODE_CTL, &mode); + if (ret < 0) + return; + + ret = iadc_read(iadc, IADC_DIG_PARAM, &dig); + if (ret < 0) + return; + + ret = iadc_read(iadc, IADC_CH_SEL_CTL, &chan); + if (ret < 0) + return; + + ret = iadc_read(iadc, IADC_CONV_REQ, &req); + if (ret < 0) + return; + + ret = iadc_read(iadc, IADC_STATUS1, &sta1); + if (ret < 0) + return; + + ret = iadc_read(iadc, IADC_EN_CTL1, &en); + if (ret < 0) + return; + + dev_warn(iadc->dev, + "mode:%02x en:%02x chan:%02x dig:%02x req:%02x sta1:%02x\n", + mode, en, chan, dig, req, sta1); +} + +static int iadc_configure(struct iadc_chip *iadc, int channel) +{ + u8 decim, mode; + int ret; + + /* Mode selection */ + mode = (IADC_OP_MODE_NORMAL << IADC_OP_MODE_SHIFT) | IADC_TRIM_EN; + ret = iadc_write(iadc, IADC_MODE_CTL, mode); + if (ret < 0) + return ret; + + /* Channel selection */ + ret = iadc_write(iadc, IADC_CH_SEL_CTL, channel); + if (ret < 0) + return ret; + + /* Digital parameter setup */ + decim = IADC_DEF_DECIMATION << IADC_DIG_DEC_RATIO_SEL_SHIFT; + ret = iadc_write(iadc, IADC_DIG_PARAM, decim); + if (ret < 0) + return ret; + + /* HW settle time delay */ + ret = iadc_write(iadc, IADC_HW_SETTLE_DELAY, IADC_DEF_HW_SETTLE_TIME); + if (ret < 0) + return ret; + + ret = iadc_write(iadc, IADC_FAST_AVG_CTL, IADC_DEF_AVG_SAMPLES); + if (ret < 0) + return ret; + + if (IADC_DEF_AVG_SAMPLES) + ret = iadc_write(iadc, IADC_FAST_AVG_EN, IADC_FAST_AVG_EN_SET); + else + ret = iadc_write(iadc, IADC_FAST_AVG_EN, 0); + + if (ret < 0) + return ret; + + if (!iadc->poll_eoc) + reinit_completion(&iadc->complete); + + ret = iadc_enable(iadc, true); + if (ret < 0) + return ret; + + /* Request conversion */ + return iadc_write(iadc, IADC_CONV_REQ, IADC_CONV_REQ_SET); +} + +static int iadc_poll_wait_eoc(struct iadc_chip *iadc, int interval_us) +{ + int ret, count, retry; + u8 sta1; + + retry = interval_us / IADC_CONV_TIME_MIN_US; + + for (count = 0; count < retry; count++) { + ret = iadc_read(iadc, IADC_STATUS1, &sta1); + if (ret < 0) + return ret; + + sta1 &= IADC_STATUS1_REQ_STS_EOC_MASK; + if (sta1 == IADC_STATUS1_EOC) + return 0; + + usleep_range(IADC_CONV_TIME_MIN_US, IADC_CONV_TIME_MAX_US); + } + + iadc_status_show(iadc); + + return -ETIMEDOUT; +} + +static int iadc_read_result(struct iadc_chip *iadc, u16 *data) +{ + u8 lsb, msb; + int ret; + + ret = iadc_read(iadc, IADC_DATA0, &lsb); + if (ret < 0) + return ret; + + ret = iadc_read(iadc, IADC_DATA1, &msb); + if (ret < 0) + return ret; + + *data = (msb << 8) | lsb; + + return 0; +} + +static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data) +{ + int wait, ret; + + ret = iadc_configure(iadc, chan); + if (ret < 0) + goto exit; + + wait = BIT(IADC_DEF_AVG_SAMPLES) * IADC_CONV_TIME_MIN_US * 2; + + if (iadc->poll_eoc) { + ret = iadc_poll_wait_eoc(iadc, wait); + } else { + ret = wait_for_completion_timeout(&iadc->complete, wait); + if (!ret) + ret = -ETIMEDOUT; + else + /* double check conversion status */ + ret = iadc_poll_wait_eoc(iadc, IADC_CONV_TIME_MIN_US); + } + + if (!ret) + ret = iadc_read_result(iadc, data); +exit: + iadc_enable(iadc, false); + if (ret < 0) + dev_err(iadc->dev, "conversion failed\n"); + + return ret; +} + +static int iadc_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct iadc_chip *iadc = iio_priv(indio_dev); + long rsense_ua, rsense_uv, rsense_raw; + int ret = -EINVAL, negative; + u16 adc_raw; + + mutex_lock(&iadc->lock); + + switch (mask) { + case IIO_CHAN_INFO_PROCESSED: + ret = iadc_do_conversion(iadc, chan->channel, &adc_raw); + if (ret < 0) + goto exit; + + rsense_raw = adc_raw - iadc->offset_raw; + + rsense_uv = (rsense_raw * IADC_REF_GAIN_MICRO_VOLTS); + rsense_uv /= iadc->gain_raw - iadc->offset_raw; + + negative = 0; + if (rsense_uv < 0) { + negative = 1; + rsense_uv = -rsense_uv; + } + + rsense_ua = rsense_uv; + + do_div(rsense_ua, iadc->rsense); + + if (negative) + rsense_ua = -rsense_ua; + + dev_dbg(iadc->dev, "offset %d, gain %d, adc %d, V=%ld,uV, I=%ld,uA\n", + iadc->offset_raw, iadc->gain_raw, adc_raw, + rsense_uv, rsense_ua); + + *val = rsense_ua; + ret = IIO_VAL_INT; + break; + default: + break; + } + +exit: + mutex_unlock(&iadc->lock); + + return ret; +} + +static const struct iio_info iadc_info = { + .read_raw = iadc_read_raw, + .driver_module = THIS_MODULE, +}; + +static irqreturn_t iadc_isr(int irq, void *dev_id) +{ + struct iadc_chip *iadc = dev_id; + + complete(&iadc->complete); + + return IRQ_HANDLED; +} + +static int iadc_update_trim_offset(struct iadc_chip *iadc) +{ + u16 adc_raw; + u8 lsb, msb; + int ret, chan; + + ret = iadc_do_conversion(iadc, IADC_GAIN_17P857MV, &adc_raw); + if (ret < 0) + return ret; + + iadc->gain_raw = adc_raw; + + chan = IADC_OFFSET_CSP2_CSN2; + if (iadc->ext_sense) + chan = IADC_OFFSET_CSP_CSN; + + ret = iadc_do_conversion(iadc, chan, &adc_raw); + if (ret < 0) + return ret; + + iadc->offset_raw = adc_raw; + + if ((iadc->gain_raw - iadc->offset_raw) == 0) { + dev_err(iadc->dev, "error: offset %d gain %d\n", + iadc->offset_raw, iadc->gain_raw); + return -EINVAL; + } + + msb = adc_raw >> 8; + lsb = adc_raw & 0xff; + + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); + if (ret < 0) + return ret; + + ret = iadc_write(iadc, IADC_MSB_OFFSET, msb); + if (ret < 0) + return ret; + + ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA); + if (ret < 0) + return ret; + + return iadc_write(iadc, IADC_LSB_OFFSET, lsb); +} + +static int iadc_version_check(struct iadc_chip *iadc) +{ + u8 revision, type, subtype; + int ret; + + ret = iadc_read(iadc, IADC_PERPH_TYPE, &type); + if (ret < 0) + return ret; + + if (type < IADC_PERPH_TYPE_ADC) { + dev_err(iadc->dev, "%d is not ADC\n", type); + return -EINVAL; + } + + ret = iadc_read(iadc, IADC_PERPH_SUBTYPE, &subtype); + if (ret < 0) + return ret; + + if (subtype < IADC_PERPH_SUBTYPE_IADC) { + dev_err(iadc->dev, "%d is not IADC\n", subtype); + return -EINVAL; + } + + ret = iadc_read(iadc, IADC_REVISION2, &revision); + if (ret < 0) + return ret; + + if (revision < IADC_REVISION2_SUPPORTED_IADC) { + dev_err(iadc->dev, "revision %d not supported\n", revision); + return -EINVAL; + } + + return iadc_read(iadc, IADC_INT_TEST_VAL, &iadc->factory); +} + +static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node) +{ + unsigned int trim_val; + u8 rsense, const_rds; + int ret, negative; + u32 trim_type; + + ret = of_property_read_u32(node, "qcom,rsense", &iadc->rsense); + if (!ret) { + iadc->ext_sense = true; + return 0; + } + + ret = iadc_read(iadc, IADC_NOMINAL_RSENSE, &rsense); + if (ret < 0) + return ret; + + negative = 0; + if (rsense & IADC_NOMINAL_RSENSE_SIGN_MASK) + negative = 1; + + rsense &= ~IADC_NOMINAL_RSENSE_SIGN_MASK; + + iadc->rsense = IADC_INTERNAL_RSENSE_OHMS; + if (negative) + iadc->rsense -= rsense * IADC_RSENSE_OHMS_PER_BIT; + else + iadc->rsense += rsense * IADC_RSENSE_OHMS_PER_BIT; + + if (rsense < IADC_TRIM2_FULLSCALE) + return 0; + /* + * Trim register is "saturated", check for specific trim value + * based on manufacturer and RDS constant + */ + ret = of_property_read_u32(node, "qcom,rds-trim", &trim_type); + if (ret) + return 0; + + ret = regmap_read(iadc->regmap, iadc->trim_rds, &trim_val); + if (ret < 0) + return ret; + + const_rds = trim_val & IADC_TRIM_CNST_RDS_MASK; + + dev_dbg(iadc->dev, "factory %d trim type %d rsense %d const %d\n", + iadc->factory, trim_type, rsense, const_rds); + + switch (trim_type) { + case 0: + if (const_rds == 2) + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; + break; + case 1: + if (const_rds >= 2) { + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; + } else if (const_rds < 2) { + if (iadc->factory == IADC_FACTORY_GF) + iadc->rsense = IADC_RSENSE_DEFAULT_GF; + else if (iadc->factory == IADC_FACTORY_SMIC) + iadc->rsense = IADC_RSENSE_DEFAULT_SMIC; + } + break; + case 2: + if (const_rds > 0 && const_rds <= 2) + iadc->rsense = IADC_RSENSE_DEFAULT_VALUE; + break; + default: + return -EINVAL; + } + + return 0; +} + +static int iadc_probe(struct platform_device *pdev) +{ + struct device_node *node = pdev->dev.of_node; + struct device *dev = &pdev->dev; + struct iio_chan_spec *iio_chan; + struct iio_dev *indio_dev; + struct iadc_chip *iadc; + struct resource *res; + struct regmap *regmap; + int ret, irq_eoc; + + regmap = dev_get_regmap(dev->parent, NULL); + if (!regmap) + return -ENODEV; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*iadc)); + if (!indio_dev) + return -ENOMEM; + + iadc = iio_priv(indio_dev); + iadc->dev = dev; + iadc->regmap = regmap; + init_completion(&iadc->complete); + mutex_init(&iadc->lock); + + platform_set_drvdata(pdev, iadc); + + res = platform_get_resource(pdev, IORESOURCE_REG, 0); + if (!res) + return -ENODEV; + + iadc->base = res->start; + + res = platform_get_resource(pdev, IORESOURCE_REG, 1); + if (!res) + return -ENODEV; + + iadc->trim_rds = res->start; + + ret = iadc_version_check(iadc); + if (ret < 0) + return -ENODEV; + + ret = iadc_rsense_read(iadc, node); + if (ret < 0) + return -ENODEV; + + dev_dbg(iadc->dev, "%s sense resistor %d Ohm\n", iadc->ext_sense ? + "external" : "internal", iadc->rsense); + + irq_eoc = platform_get_irq(pdev, 0); + if (irq_eoc == -EPROBE_DEFER) + return irq_eoc; + + if (irq_eoc < 0) + iadc->poll_eoc = true; + + ret = iadc_reset(iadc); + if (ret < 0) { + dev_err(dev, "reset failed\n"); + return ret; + } + + if (!iadc->poll_eoc) { + ret = devm_request_irq(dev, irq_eoc, iadc_isr, 0, + "spmi-iadc", iadc); + if (!ret) + enable_irq_wake(irq_eoc); + else + return ret; + } else { + device_init_wakeup(iadc->dev, 1); + } + + ret = iadc_update_trim_offset(iadc); + if (ret < 0) { + dev_err(dev, "failed trim offset calibration\n"); + return ret; + } + + iio_chan = &iadc->iio_chan; + iio_chan->type = IIO_CURRENT; + iio_chan->indexed = 1; + iio_chan->info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED); + iio_chan->scan_type.sign = 's'; + iio_chan->scan_type.realbits = 16; + iio_chan->scan_type.storagebits = 16; + + if (iadc->ext_sense) { + iio_chan->channel = 1; + iio_chan->datasheet_name = "EXTERNAL_RSENSE"; + } else { + iio_chan->channel = 0; + iio_chan->datasheet_name = "INTERNAL_RSENSE"; + } + + indio_dev->dev.parent = dev; + indio_dev->dev.of_node = node; + indio_dev->name = pdev->name; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->info = &iadc_info; + indio_dev->channels = iio_chan; + indio_dev->num_channels = 1; + + return devm_iio_device_register(dev, indio_dev); +} + +static const struct of_device_id iadc_match_table[] = { + { .compatible = "qcom,spmi-iadc" }, + { } +}; +MODULE_DEVICE_TABLE(of, iadc_match_table); + +static struct platform_driver iadc_driver = { + .driver = { + .name = "spmi-iadc", + .of_match_table = iadc_match_table, + }, + .probe = iadc_probe, +}; +module_platform_driver(iadc_driver); + +MODULE_ALIAS("platform:spmi-iadc"); +MODULE_DESCRIPTION("Qualcomm SPMI PMIC current ADC driver"); +MODULE_LICENSE("GPL v2");
The current ADC is peripheral of Qualcomm SPMI PMIC chips. It has 16 bits resolution and register space inside PMIC accessible across SPMI bus. The driver registers itself through IIO interface. Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> --- .../devicetree/bindings/iio/adc/qcom,spmi-iadc.txt | 61 ++ drivers/iio/adc/Kconfig | 11 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/qcom-spmi-iadc.c | 700 +++++++++++++++++++++ 4 files changed, 773 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-iadc.txt create mode 100644 drivers/iio/adc/qcom-spmi-iadc.c