Ivan T. Ivanov schrieb am 01.10.2014 18:14: > 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. > Hi, I spotted some issues, see inline. > Signed-off-by: Ivan T. Ivanov <iivanov@xxxxxxxxxx> > --- > > Changes since v2: > > - DT bindings fixed according comments. > - IADC driver now register 2 channels instead of just one. > - Fixed sense resistor Ohms dimensions. It is supposed that > values are around 0.010 Ohms not 10 Mega Ohms. > - Removed direct driver access to registers in Battery Monitor > peripheral address space. Which will impact correct internal > sense resistor calculation on some variants and vendors of > pm8110 and pm8226. This could be added once we figured out how > to pass/read PMIC device version from sub-function drivers. > > v2: http://www.gossamer-threads.com/lists/linux/kernel/2016613 > > .../devicetree/bindings/iio/adc/qcom,spmi-iadc.txt | 46 ++ > drivers/iio/adc/Kconfig | 11 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/qcom-spmi-iadc.c | 626 +++++++++++++++++++++ > 4 files changed, 684 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..833afd6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-iadc.txt > @@ -0,0 +1,46 @@ > +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 can measure the current > +through an external resistor(channel 1)or internal(built-in) resistor There are some whitespaces missing around the parenthesis. > +(channel 0). When using an external resistor it is to be described by > +qcom,external-resistor-micro-ohms property. > + > +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 > + > +- interrupts: > + Usage: optional > + Value type: <prop-encoded-array> > + Definition: End of ADC conversion. > + > +- qcom,external-resistor-micro-ohms: > + Usage: optional > + Value type: <u32> > + Definition: Sense resister value in micro Ohm. > + If not defined value of 10000 micro Ohms will be used. > + > +Example: > + /* IADC node */ > + pmic_iadc: iadc@3600 { > + compatible = "qcom,spmi-iadc"; > + reg = <0x3600 0x100>; > + interrupts = <0x0 0x36 0x0 IRQ_TYPE_EDGE_RISING>; > + qcom,external-resistor-micro-ohms = <10000>; > + #io-channel-cells = <1>; > + }; > + > + /* IIO client node */ > + bat { > + io-channels = <&pmic_iadc>; > + io-channel-names = "iadc 0"; > + }; > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 11b048a..ee1ad5b 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -206,6 +206,17 @@ config NAU7802 > To compile this driver as a module, choose M here: the > module will be called nau7802. > > +config QCOM_SPMI_IADC > + tristate "Qualcomm SPMI PMIC current ADC" > + depends on SPMI > + select REGMAP_SPMI > + help > + This is the IIO Current ADC driver for Qualcomm QPNP IADC Chip. > + > + The driver supports single mode operation to read from two > + channels (external or internal). Hardware have additional > + channels internally used for gain and offset calibration. To compile this driver as a module, choose M here: the module will be called qcom-spmi-iadc. > + > config TI_ADC081C > tristate "Texas Instruments ADC081C021/027" > depends on I2C > 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 Please sort in your entry in alphabetic order. > diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c > new file mode 100644 > index 0000000..f4328f2 > --- /dev/null > +++ b/drivers/iio/adc/qcom-spmi-iadc.c > @@ -0,0 +1,626 @@ > +/* > + * 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/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +/* 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_NOMINAL_RSENSE 0xf4 > +#define IADC_NOMINAL_RSENSE_SIGN_MASK BIT(7) > + > +#define IADC_REF_GAIN_MICRO_VOLTS 17857 > + > +#define IADC_INT_RSENSE_DEVIATION 15625 /* nano Ohms per bit */ > + > +#define IADC_INT_RSENSE_IDEAL_VALUE 10000 /* micro Ohms */ > +#define IADC_INT_RSENSE_DEFAULT_VALUE 7800 /* micro Ohms */ > +#define IADC_INT_RSENSE_DEFAULT_GF 9000 /* micro Ohms */ > +#define IADC_INT_RSENSE_DEFAULT_SMIC 9700 /* micro Ohms */ > + > +#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 channel list */ > +#define IADC_INT_RSENSE 0 > +#define IADC_EXT_RSENSE 1 > +#define IADC_GAIN_17P857MV 3 > +#define IADC_EXT_OFFSET_CSP_CSN 5 > +#define IADC_INT_OFFSET_CSP2_CSN2 6 > + > +/** > + * struct iadc_drv - IADC Current ADC device structure. You actually call it iadc_chip now. > + * @regmap: regmap for register read/write. > + * @dev: This device pointer. > + * @base: base offset for the ADC peripheral. > + * @rsense: Values of the internal and external sense resister in micro Ohms. > + * @poll_eoc: Poll for end of conversion instead of waiting for IRQ. > + * @offset_raw: Raw offset values for the internal and external channels. > + * @gain_raw: Raw gain of the channels. > + * @lock: ADC lock for access to the peripheral. > + * @complete: ADC notification after end of conversion interrupt is received. > + */ > +struct iadc_chip { > + struct regmap *regmap; > + struct device *dev; > + u16 base; > + bool poll_eoc; > + int rsense[2]; An unsigned integer type for rsense seems more appropriate to me, maybe u32? > + u16 offset_raw[2]; > + u16 gain_raw; > + struct mutex lock; > + struct completion complete; > +}; > + > +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 < 0) > + return ret; > + > + *data = val; > + return 0; > +} > + > +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_set_state(struct iadc_chip *iadc, bool state) > +{ > + u8 data = 0; > + > + if (state) > + data = IADC_EN_CTL1_SET; > + > + return iadc_write(iadc, IADC_EN_CTL1, data); The whole function body could just be consolidated to one line: return iadc_write(iadc, IADC_EN_CTL1, state ? IADC_EN_CTL1_SET : 0); > +} > + > +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_err(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_set_state(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) I think unsigned int interval_us would be more appropriate. > +{ > + int ret, count, retry; count and retry should also be unsigned. > + 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; I think using regmap_bulk_read() would be shorter and easier. > +} > + > +static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data) > +{ > + int wait, ret; wait should be unsigned. > + > + 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_set_state(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 isense_ua, vsense_uv, vsense_raw; vsense_raw perfectly fits into an int. For vsense_uv and isense_ua it depends, if IADC_REF_GAIN_MICRO_VOLTS could ever exceed 32768. But maybe s64 would be the better type for those two, see below. > + int ret, sign; > + u16 adc_raw; > + > + mutex_lock(&iadc->lock); I would move it down, as it is just necessary for IIO_CHAN_INFO_RAW. > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = iadc_do_conversion(iadc, chan->channel, &adc_raw); > + if (ret < 0) > + goto exit; > + > + vsense_raw = adc_raw - iadc->offset_raw[chan->channel]; > + > + vsense_uv = vsense_raw * IADC_REF_GAIN_MICRO_VOLTS; > + vsense_uv /= iadc->gain_raw - iadc->offset_raw[chan->channel]; > + > + sign = 1; > + if (vsense_uv < 0) > + sign = -1; If it is really necessary to use sign, why not just: sign = (vsense_uv < 0) -1 : 1; > + > + isense_ua = sign * vsense_uv; > + > + do_div(isense_ua, iadc->rsense[chan->channel]); I don't really understand, why you use do_div, since you discard the remainder. Wouldn't the following do the same, without removing the sign and applying it again, later? isense_ua = div_s64(vsense_uv, iadc->rsense[chan->channel]); > + > + isense_ua = sign * isense_ua; > + > + dev_dbg(iadc->dev, "offset %d, gain %d, adc %d, V=%ld,uV, I=%ld,uA\n", > + iadc->offset_raw[chan->channel], iadc->gain_raw, > + adc_raw, vsense_uv, isense_ua); > + > + *val = isense_ua; > + ret = IIO_VAL_INT; > + break; > + case IIO_CHAN_INFO_SCALE: > + *val = 0; > + *val2 = 1000; > + ret = IIO_VAL_INT_PLUS_MICRO; With the mutex_lock just for IIO_CHAN_INFO_RAW, you could return directly here. > + break; > + default: > + ret = -EINVAL; And here as well. > + 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_offset(struct iadc_chip *iadc) > +{ > + int ret; > + > + ret = iadc_do_conversion(iadc, IADC_GAIN_17P857MV, &iadc->gain_raw); > + if (ret < 0) > + return ret; > + > + ret = iadc_do_conversion(iadc, IADC_INT_OFFSET_CSP2_CSN2, > + &iadc->offset_raw[IADC_INT_RSENSE]); > + if (ret < 0) > + return ret; > + > + if ((iadc->gain_raw - iadc->offset_raw[IADC_INT_RSENSE]) == 0) { Just: if (iadc->gain_raw == iadc->offset_raw[IADC_INT_RSENSE]) { > + dev_err(iadc->dev, "error: internal offset %d gain %d\n", > + iadc->offset_raw[IADC_INT_RSENSE], iadc->gain_raw); > + return -EINVAL; > + } > + > + ret = iadc_do_conversion(iadc, IADC_EXT_OFFSET_CSP_CSN, > + &iadc->offset_raw[IADC_EXT_RSENSE]); > + if (ret < 0) > + return ret; > + > + if ((iadc->gain_raw - iadc->offset_raw[IADC_EXT_RSENSE]) == 0) { Same here: if (iadc->gain_raw == iadc->offset_raw[IADC_EXT_RSENSE]) { > + dev_err(iadc->dev, "error: external offset %d gain %d\n", > + iadc->offset_raw[IADC_EXT_RSENSE], iadc->gain_raw); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int iadc_version_check(struct iadc_chip *iadc) > +{ > + u8 revision, type, subtype; You could just use one variable and recylce it. > + 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 0; > +} > + > +static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node) > +{ > + int ret, sign, int_sense; > + u8 deviation; > + > + ret = of_property_read_u32(node, "qcom,external-resistor-micro-ohms", > + &iadc->rsense[IADC_EXT_RSENSE]); > + if (ret < 0) > + iadc->rsense[IADC_EXT_RSENSE] = IADC_INT_RSENSE_IDEAL_VALUE; > + > + if (!iadc->rsense[IADC_EXT_RSENSE]) { > + dev_err(iadc->dev, "qcom,external-resistor-micro-ohms can't be 0"); This exceeds the 80 character per line limit. > + return -EINVAL; > + } > + > + ret = iadc_read(iadc, IADC_NOMINAL_RSENSE, &deviation); > + if (ret < 0) > + return ret; > + > + sign = 1; > + if (deviation & IADC_NOMINAL_RSENSE_SIGN_MASK) > + sign = -1; Or simply: sign = (deviation & IADC_NOMINAL_RSENSE_SIGN_MASK) ? -1 : 1; > + > + deviation &= ~IADC_NOMINAL_RSENSE_SIGN_MASK; What is actually the format of deviation, when read from the device, signed magnitude representation format? Putting a comment about this could be a good idea. > + > + int_sense = IADC_INT_RSENSE_IDEAL_VALUE * 1000; > + int_sense += sign * deviation * IADC_INT_RSENSE_DEVIATION; > + int_sense /= 1000; /* micro Ohms */ > + > + iadc->rsense[IADC_INT_RSENSE] = int_sense; > + > + return 0; > +} > + > +static const struct iio_chan_spec iadc_channels[] = { > + { > + .type = IIO_CURRENT, > + .datasheet_name = "INTERNAL_RSENSE", > + .channel = 0, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_SCALE), > + .indexed = 1, > + }, > + { > + .type = IIO_CURRENT, > + .datasheet_name = "EXTERNAL_RSENSE", > + .channel = 1, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_SCALE), > + .indexed = 1, > + }, > +}; > + > +static int iadc_probe(struct platform_device *pdev) > +{ > + struct device_node *node = pdev->dev.of_node; > + struct device *dev = &pdev->dev; > + struct iio_dev *indio_dev; > + struct iadc_chip *iadc; > + int ret, irq_eoc, res[2]; Better use u32 for res? > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*iadc)); > + if (!indio_dev) > + return -ENOMEM; > + > + iadc = iio_priv(indio_dev); > + iadc->dev = dev; > + > + iadc->regmap = dev_get_regmap(dev->parent, NULL); > + if (!iadc->regmap) > + return -ENODEV; > + > + init_completion(&iadc->complete); > + mutex_init(&iadc->lock); > + > + platform_set_drvdata(pdev, iadc); Is this really necessary? > + > + ret = of_property_read_u32_array(node, "reg", res, 2); I don't see, where res[1] gets used, so is it really necessary to read it (and have it defined as array)? > + if (ret < 0) > + return -ENODEV; > + > + iadc->base = res[0]; > + > + 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, "sense resistors %d and %d micro Ohm\n", > + iadc->rsense[IADC_INT_RSENSE], > + iadc->rsense[IADC_EXT_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_offset(iadc); > + if (ret < 0) { > + dev_err(dev, "failed offset calibration\n"); > + return ret; > + } > + > + 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 = iadc_channels; > + indio_dev->num_channels = ARRAY_SIZE(iadc_channels); > + > + 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 linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
