* [PATCH v2 0/2] Intial support for voltage ADC @ 2014-09-11 15:13 Stanimir Varbanov 2014-09-11 15:13 ` [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver Stanimir Varbanov 2014-09-11 15:13 ` [PATCH v2 2/2] DT: iio: vadc: document dt binding Stanimir Varbanov 0 siblings, 2 replies; 23+ messages in thread From: Stanimir Varbanov @ 2014-09-11 15:13 UTC (permalink / raw) To: Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Jonathan Cameron, Arnd Bergmann Cc: linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Hartmut Knaack, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Stanimir Varbanov Here is version 2. Changes since v1: - the review comments from Peter Meerwald and Hartmut Knaack are taken into account - the driver private sysfs attributes are dropped out. - dt binding document is changed in the section of adc channel nodes - now the adc channel number is represented by "reg" property. - various cleanups found during preparation of version 2. regards, Stan ------------------------------------------------------------------------------- Introduce inital support for VADC inside Qualcomm SPMI PMICs. The vadc driver is reworked version of the same downstream driver but now it is implemented through Industrial IO core. Comments are welcome! Stanimir Varbanov (2): iio: vadc: Qualcomm SPMI PMIC voltage ADC driver DT: iio: vadc: document dt binding .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt | 123 +++ drivers/iio/adc/Kconfig | 11 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/qcom-spmi-vadc.c | 999 ++++++++++++++++++++ include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ 5 files changed, 1253 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-11 15:13 [PATCH v2 0/2] Intial support for voltage ADC Stanimir Varbanov @ 2014-09-11 15:13 ` Stanimir Varbanov 2014-09-12 23:27 ` Hartmut Knaack 2014-09-11 15:13 ` [PATCH v2 2/2] DT: iio: vadc: document dt binding Stanimir Varbanov 1 sibling, 1 reply; 23+ messages in thread From: Stanimir Varbanov @ 2014-09-11 15:13 UTC (permalink / raw) To: Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Jonathan Cameron, Arnd Bergmann Cc: linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Hartmut Knaack, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Stanimir Varbanov, Ivan T. Ivanov The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has 15bits resolution and register space inside PMIC accessible across SPMI bus. The vadc driver registers itself through IIO interface. Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> --- drivers/iio/adc/Kconfig | 11 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/qcom-spmi-vadc.c | 999 +++++++++++++++++++++++++ include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ 4 files changed, 1130 insertions(+), 0 deletions(-) create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 11b048a..08700d4 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_VADC + tristate "Qualcomm SPMI PMIC voltage ADC" + depends on SPMI + help + Say yes here if you want support for the Qualcomm SPMI PMIC voltage ADC. + + The driver supports reading the HKADC, XOADC through the ADC AMUX arbiter. + The VADC includes support for the conversion sequencer. The driver + supports reading the ADC through the AMUX channels for external pull-ups + simultaneously. + 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..d5d18f4 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -22,6 +22,7 @@ obj-$(CONFIG_MCP320X) += mcp320x.o obj-$(CONFIG_MCP3422) += mcp3422.o obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o obj-$(CONFIG_NAU7802) += nau7802.o +obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c new file mode 100644 index 0000000..e30eb04 --- /dev/null +++ b/drivers/iio/adc/qcom-spmi-vadc.c @@ -0,0 +1,999 @@ +/* + * 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/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <linux/log2.h> + +#include <dt-bindings/iio/qcom,spmi-pmic-vadc.h> + +/* VADC register and bit definition */ +#define VADC_REVISION2 0x1 +#define VADC_REVISION2_SUPPORTED_VADC 1 + +#define VADC_PERPH_TYPE 0x4 +#define VADC_PERPH_TYPE_ADC 8 + +#define VADC_PERPH_SUBTYPE 0x5 +#define VADC_PERPH_SUBTYPE_VADC 1 + +#define VADC_STATUS1 0x8 +#define VADC_STATUS1_OP_MODE 4 +#define VADC_STATUS1_REQ_STS BIT(1) +#define VADC_STATUS1_EOC BIT(0) +#define VADC_STATUS1_REQ_STS_EOC_MASK 0x3 + +#define VADC_MODE_CTL 0x40 +#define VADC_OP_MODE_SHIFT 3 +#define VADC_OP_MODE_NORMAL 0 +#define VADC_AMUX_TRIM_EN BIT(1) +#define VADC_ADC_TRIM_EN BIT(0) + +#define VADC_EN_CTL1 0x46 +#define VADC_EN_CTL1_SET BIT(7) + +#define VADC_ADC_CH_SEL_CTL 0x48 + +#define VADC_ADC_DIG_PARAM 0x50 +#define VADC_ADC_DIG_DEC_RATIO_SEL_SHIFT 2 + +#define VADC_HW_SETTLE_DELAY 0x51 + +#define VADC_CONV_REQ 0x52 +#define VADC_CONV_REQ_SET BIT(7) + +#define VADC_FAST_AVG_CTL 0x5a +#define VADC_FAST_AVG_EN 0x5b +#define VADC_FAST_AVG_EN_SET BIT(7) + +#define VADC_ACCESS 0xd0 +#define VADC_ACCESS_DATA 0xa5 + +#define VADC_PERH_RESET_CTL3 0xda +#define VADC_FOLLOW_WARM_RB BIT(2) + +#define VADC_DATA0 0x60 +#define VADC_DATA1 0x61 + +#define VADC_CONV_TIME_MIN_US 2000 +#define VADC_CONV_TIME_MAX_US 2100 + +/* Min ADC code represents 0V */ +#define VADC_MIN_ADC_CODE 0x6000 +/* Max ADC code represents full-scale range of 1.8V */ +#define VADC_MAX_ADC_CODE 0xA800 + +#define VADC_ABSOLUTE_RANGE_UV 625000 +#define VADC_RATIOMETRIC_RANGE_UV 1800000 + +#define VADC_DEF_PRESCALING 0 /* 1:1 */ +#define VADC_DEF_DECIMATION 0 /* 512 */ +#define VADC_DEF_HW_SETTLE_TIME 0 /* 0 us */ +#define VADC_DEF_AVG_SAMPLES 0 /* 1 sample */ +#define VADC_DEF_CALIB_TYPE VADC_CALIB_ABSOLUTE + +#define VADC_DECIMATION_MIN 512 +#define VADC_DECIMATION_MAX 4096 + +#define VADC_HW_SETTLE_DELAY_MAX 10000 +#define VADC_AVG_SAMPLES_MAX 512 + +/* + * VADC_CALIB_ABSOLUTE: Uses the 625mV and 1.25V reference channels. + * VADC_CALIB_RATIOMETRIC: Uses the reference Voltage/GND for calibration. + */ +enum vadc_calibration { + VADC_CALIB_ABSOLUTE = 0, + VADC_CALIB_RATIOMETRIC +}; + +/** + * struct vadc_linear_graph - Represent ADC characteristics. + * @dy: Numerator slope to calculate the gain. + * @dx: Denominator slope to calculate the gain. + * @vref: A/D word of the voltage reference used for the channel. + * @gnd: A/D word of the ground reference used for the channel. + * + * Each ADC device has different offset and gain parameters which are + * computed to calibrate the device. + */ +struct vadc_linear_graph { + s32 dy; + s32 dx; + s32 vref; + s32 gnd; +}; + +/** + * struct vadc_prescaling - Represent scaling ratio for ADC input. + * @num: The inverse numerator of the gain applied to the input channel. + * @den: The inverse denominator of the gain applied to the input channel. + */ +struct vadc_prescaling { + u32 num; + u32 den; +}; + +/** + * struct vadc_result - Represent the res of the QPNP ADC. + * @adc_code: The pre-calibrated digital output of a given ADC relative to the + * the ADC reference. + * @measurement: In units specific for a given ADC; most ADC uses reference + * voltage but some ADC uses reference current. This measurement + * here is a number relative to a reference of a given ADC. + * @physical: The data meaningful for each individual channel whether it is + * voltage, current, temperature, etc. + */ +struct vadc_result { + s32 adc_code; + s64 measurement; + s64 physical; +}; + +/** + * struct vadc_channel - QPNP VADC amux channel property. + * @number - Channel number, refer to the channel list. + * @calibration - Calibration type. + * @decimation - Sampling rate supported for the channel. + * @prescaling - Channel scaling performed on the input signal. + * @hw_settle_time - The time between AMUX being configured and the + * start of conversion. + * @avg_samples - Ability to provide single result from the ADC + * that is an average of multiple measurements. + */ +struct vadc_channel { + int number; + enum vadc_calibration calibration; + unsigned decimation; + unsigned prescaling; + unsigned hw_settle_time; + unsigned avg_samples; +}; + +/** + * struct vadc_priv - VADC private structure. + * @regmap - pointer to struct regmap + * @dev - pointer to struct device. + * @base - base address for the ADC peripheral. + * @nchannels - number of ADC channels + * @channels - AMUX properties representing the ADC peripheral. + * @is_ref_measured - does reference points are measured. + * @poll_eoc - do we use polling instead of interrupt. + * @complete - ADC result notification after interrupt is received. + * @graph - store parameters for calibration. + */ +struct vadc_priv { + struct regmap *regmap; + struct device *dev; + u16 base; + unsigned nchannels; + struct vadc_channel *channels; + bool is_ref_measured; + bool poll_eoc; + struct completion complete; + struct vadc_linear_graph graph[2]; +}; + +static const struct vadc_prescaling vadc_prescale[] = { + {1, 1}, + {1, 3}, + {1, 4}, + {1, 6}, + {1, 20}, + {1, 8}, + {10, 81}, + {1, 10} +}; + +static int vadc_read(struct vadc_priv *vadc, u16 offset, u8 *data) +{ + return regmap_bulk_read(vadc->regmap, vadc->base + offset, data, 1); +} + +static int vadc_write(struct vadc_priv *vadc, u16 offset, u8 data) +{ + return regmap_write(vadc->regmap, vadc->base + offset, data); +} + +static int vadc_reset(struct vadc_priv *vadc) +{ + u8 data; + int rc; + + rc = vadc_write(vadc, VADC_ACCESS, VADC_ACCESS_DATA); + if (rc) + return rc; + + rc = vadc_read(vadc, VADC_PERH_RESET_CTL3, &data); + if (rc) + return rc; + + rc = vadc_write(vadc, VADC_ACCESS, VADC_ACCESS_DATA); + if (rc) + return rc; + + data |= VADC_FOLLOW_WARM_RB; + + return vadc_write(vadc, VADC_PERH_RESET_CTL3, data); +} + +static int vadc_enable(struct vadc_priv *vadc, bool state) +{ + return vadc_write(vadc, VADC_EN_CTL1, state ? VADC_EN_CTL1_SET : 0); +} + +static int vadc_request_conversion(struct vadc_priv *vadc) +{ + return vadc_write(vadc, VADC_CONV_REQ, VADC_CONV_REQ_SET); +} + +static void vadc_status_show(struct vadc_priv *vadc) +{ + u8 mode, sta1, chan, dig, en, req; + int rc; + + rc = vadc_read(vadc, VADC_MODE_CTL, &mode); + if (rc) + return; + + rc = vadc_read(vadc, VADC_ADC_DIG_PARAM, &dig); + if (rc) + return; + + rc = vadc_read(vadc, VADC_ADC_CH_SEL_CTL, &chan); + if (rc) + return; + + rc = vadc_read(vadc, VADC_CONV_REQ, &req); + if (rc) + return; + + rc = vadc_read(vadc, VADC_STATUS1, &sta1); + if (rc) + return; + + rc = vadc_read(vadc, VADC_EN_CTL1, &en); + if (rc) + return; + + dev_dbg(vadc->dev, + "mode:%02x en:%02x chan:%02x dig:%02x req:%02x sta1:%02x\n", + mode, en, chan, dig, req, sta1); +} + +static int vadc_configure(struct vadc_priv *vadc, struct vadc_channel *vchan) +{ + u8 decim, mode_ctrl; + int rc; + + /* Mode selection */ + mode_ctrl = (VADC_OP_MODE_NORMAL << VADC_OP_MODE_SHIFT) | + VADC_ADC_TRIM_EN | VADC_AMUX_TRIM_EN; + rc = vadc_write(vadc, VADC_MODE_CTL, mode_ctrl); + if (rc) + return rc; + + /* Channel selection */ + rc = vadc_write(vadc, VADC_ADC_CH_SEL_CTL, vchan->number); + if (rc) + return rc; + + /* Digital parameter setup */ + decim = vchan->decimation << VADC_ADC_DIG_DEC_RATIO_SEL_SHIFT; + rc = vadc_write(vadc, VADC_ADC_DIG_PARAM, decim); + if (rc) + return rc; + + /* HW settle time delay */ + rc = vadc_write(vadc, VADC_HW_SETTLE_DELAY, vchan->hw_settle_time); + if (rc) + return rc; + + rc = vadc_write(vadc, VADC_FAST_AVG_CTL, vchan->avg_samples); + if (rc) + return rc; + + if (vchan->avg_samples) + rc = vadc_write(vadc, VADC_FAST_AVG_EN, VADC_FAST_AVG_EN_SET); + else + rc = vadc_write(vadc, VADC_FAST_AVG_EN, 0); + + return rc; +} + +static int vadc_poll_wait_eoc(struct vadc_priv *vadc, unsigned int interval_us) +{ + unsigned int count, retry; + u8 sta1; + int rc; + + retry = interval_us / VADC_CONV_TIME_MIN_US; + + for (count = 0; count < retry; count++) { + rc = vadc_read(vadc, VADC_STATUS1, &sta1); + if (rc) + return rc; + + sta1 &= VADC_STATUS1_REQ_STS_EOC_MASK; + if (sta1 == VADC_STATUS1_EOC) + return 0; + + usleep_range(VADC_CONV_TIME_MIN_US, VADC_CONV_TIME_MAX_US); + } + + vadc_status_show(vadc); + + return -ETIMEDOUT; +} + +static int vadc_read_result(struct vadc_priv *vadc, u32 *data) +{ + u8 lsb, msb; + int rc; + + rc = vadc_read(vadc, VADC_DATA0, &lsb); + if (rc) + return rc; + + rc = vadc_read(vadc, VADC_DATA1, &msb); + if (rc) + return rc; + + *data = clamp_t(u32, (msb << 8) | lsb, VADC_MIN_ADC_CODE, + VADC_MAX_ADC_CODE); + + return 0; +} + +static struct vadc_channel *vadc_find_channel(struct vadc_priv *vadc, int num) +{ + unsigned i; + + for (i = 0; i < vadc->nchannels; i++) + if (vadc->channels[i].number == num) + return &vadc->channels[num]; + + dev_dbg(vadc->dev, "no such channel %02x\n", num); + + return NULL; +} + +static int vadc_do_conversion(struct vadc_priv *vadc, + struct vadc_channel *vchan, s32 *data) +{ + unsigned int timeout; + int rc; + + rc = vadc_configure(vadc, vchan); + if (rc) + return rc; + + if (!vadc->poll_eoc) + reinit_completion(&vadc->complete); + + rc = vadc_enable(vadc, true); + if (rc) + return rc; + + rc = vadc_request_conversion(vadc); + if (rc) + goto err_disable; + + timeout = BIT(vchan->avg_samples) * VADC_CONV_TIME_MIN_US * 2; + + if (vadc->poll_eoc) { + rc = vadc_poll_wait_eoc(vadc, timeout); + } else { + rc = wait_for_completion_timeout(&vadc->complete, + msecs_to_jiffies(1000)); + if (!rc) { + rc = -ETIMEDOUT; + goto err_disable; + } + + /* double check conversion status */ + rc = vadc_poll_wait_eoc(vadc, VADC_CONV_TIME_MIN_US * 2); + if (rc) + goto err_disable; + } + + rc = vadc_read_result(vadc, data); + +err_disable: + vadc_enable(vadc, false); + + if (rc) + dev_dbg(vadc->dev, "conversion failed\n"); + + return rc; +} + +static int vadc_measure_reference_points(struct vadc_priv *vadc) +{ + struct vadc_channel *vchan; + s32 read_1, read_2; + int rc = -EINVAL; + + vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV; + vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV; + + vchan = vadc_find_channel(vadc, VADC_REF_1250MV); + if (!vchan) + goto err; + + rc = vadc_do_conversion(vadc, vchan, &read_1); + if (rc) + goto err; + + /* Try with buffered 625mV channel first */ + vchan = vadc_find_channel(vadc, VADC_SPARE1); + if (!vchan) { + vchan = vadc_find_channel(vadc, VADC_REF_625MV); + if (!vchan) { + rc = -EINVAL; + goto err; + } + } + + rc = vadc_do_conversion(vadc, vchan, &read_2); + if (rc) + goto err; + + if (read_1 == read_2) { + rc = -EINVAL; + goto err; + } + + vadc->graph[VADC_CALIB_ABSOLUTE].dy = read_1 - read_2; + vadc->graph[VADC_CALIB_ABSOLUTE].vref = read_1; + vadc->graph[VADC_CALIB_ABSOLUTE].gnd = read_2; + + /* Ratiometric calibration */ + vchan = vadc_find_channel(vadc, VADC_VDD_VADC); + if (!vchan) { + rc = -EINVAL; + goto err; + } + + rc = vadc_do_conversion(vadc, vchan, &read_1); + if (rc) + goto err; + + vchan = vadc_find_channel(vadc, VADC_GND_REF); + if (!vchan) { + rc = -EINVAL; + goto err; + } + + rc = vadc_do_conversion(vadc, vchan, &read_2); + if (rc) + goto err; + + if (read_1 == read_2) { + rc = -EINVAL; + goto err; + } + + vadc->graph[VADC_CALIB_RATIOMETRIC].dy = read_1 - read_2; + vadc->graph[VADC_CALIB_RATIOMETRIC].vref = read_1; + vadc->graph[VADC_CALIB_RATIOMETRIC].gnd = read_2; +err: + if (rc) + dev_dbg(vadc->dev, "measure reference points failed\n"); + + return rc; +} + +static void vadc_calibrate(struct vadc_priv *vadc, + const struct vadc_channel *vchan, + struct vadc_result *res) +{ + const struct vadc_prescaling *prescale; + bool negative = false; + s64 voltage; + + voltage = res->adc_code - vadc->graph[vchan->calibration].gnd; + voltage *= vadc->graph[vchan->calibration].dx; + + if (voltage < 0) { + negative = true; + voltage = -voltage; + } + + do_div(voltage, vadc->graph[vchan->calibration].dy); + if (negative) + voltage = -voltage; + + if (vchan->calibration == VADC_CALIB_ABSOLUTE) + voltage += vadc->graph[vchan->calibration].dx; + + if (voltage < 0) + voltage = 0; + + prescale = &vadc_prescale[vchan->prescaling]; + + res->measurement = voltage * prescale->den; + + do_div(res->measurement, prescale->num); + + res->physical = res->measurement; +} + +static unsigned vadc_decimation_from_dt(unsigned int value) +{ + if (!is_power_of_2(value) || value < VADC_DECIMATION_MIN || + value > VADC_DECIMATION_MAX) + return -EINVAL; + + return __ffs64(value / VADC_DECIMATION_MIN); +} + +static int vadc_prescaling_from_dt(unsigned int num, unsigned int den) +{ + unsigned int pre; + + for (pre = 0; pre < ARRAY_SIZE(vadc_prescale); pre++) + if (vadc_prescale[pre].num == num && + vadc_prescale[pre].den == den) + break; + + if (pre == ARRAY_SIZE(vadc_prescale)) + return -EINVAL; + + return pre; +} + +static int vadc_hw_settle_time_from_dt(unsigned int value) +{ + if ((value <= 1000 && value % 100) || (value > 1000 && value % 2000)) + return -EINVAL; + + if (value <= 1000) + value /= 100; + else + value = value / 2000 + 10; + + return value; +} + +static int vadc_avg_samples_from_dt(unsigned int value) +{ + if (!is_power_of_2(value) || value > VADC_AVG_SAMPLES_MAX) + return -EINVAL; + + return __ffs64(value); +} + +static int +vadc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct vadc_priv *vadc = iio_priv(indio_dev); + struct vadc_channel *vchan; + struct vadc_result result; + int rc; + + vchan = vadc_find_channel(vadc, chan->channel); + if (!vchan) + return -EINVAL; + + if (!vadc->is_ref_measured) { + rc = vadc_measure_reference_points(vadc); + if (rc) + return rc; + + vadc->is_ref_measured = true; + } + + switch (mask) { + case IIO_CHAN_INFO_PROCESSED: + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); + if (rc) + return rc; + + vadc_calibrate(vadc, vchan, &result); + + *val = result.physical; + rc = IIO_VAL_INT; + break; + default: + rc = -EINVAL; + break; + } + + return rc; +} + +static const struct iio_info vadc_info = { + .read_raw = vadc_read_raw, + .driver_module = THIS_MODULE, +}; + +#define VADC_CHAN(_id, _pre) \ + [VADC_##_id] = { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = VADC_##_id, \ + .address = _pre, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ + .datasheet_name = __stringify(VADC_##_id), \ + .scan_type = { \ + .sign = 's', \ + .realbits = 15, \ + .storagebits = 16, \ + .endianness = IIO_CPU, \ + }, \ + }, + +static const struct iio_chan_spec vadc_channels[] = { + VADC_CHAN(USBIN, 4) /* 0x00 */ + VADC_CHAN(DCIN, 4) + VADC_CHAN(VCHG_SNS, 3) + VADC_CHAN(SPARE1_03, 1) + VADC_CHAN(USB_ID_MV, 1) + VADC_CHAN(VCOIN, 1) + VADC_CHAN(VBAT_SNS, 1) + VADC_CHAN(VSYS, 1) + VADC_CHAN(DIE_TEMP, 0) + VADC_CHAN(REF_625MV, 0) + VADC_CHAN(REF_1250MV, 0) + VADC_CHAN(CHG_TEMP, 0) + VADC_CHAN(SPARE1, 0) + VADC_CHAN(SPARE2, 0) + VADC_CHAN(GND_REF, 0) + VADC_CHAN(VDD_VADC, 0) /* 0x0f */ + + VADC_CHAN(P_MUX1_1_1, 0) /* 0x10 */ + VADC_CHAN(P_MUX2_1_1, 0) + VADC_CHAN(P_MUX3_1_1, 0) + VADC_CHAN(P_MUX4_1_1, 0) + VADC_CHAN(P_MUX5_1_1, 0) + VADC_CHAN(P_MUX6_1_1, 0) + VADC_CHAN(P_MUX7_1_1, 0) + VADC_CHAN(P_MUX8_1_1, 0) + VADC_CHAN(P_MUX9_1_1, 0) + VADC_CHAN(P_MUX10_1_1, 0) + VADC_CHAN(P_MUX11_1_1, 0) + VADC_CHAN(P_MUX12_1_1, 0) + VADC_CHAN(P_MUX13_1_1, 0) + VADC_CHAN(P_MUX14_1_1, 0) + VADC_CHAN(P_MUX15_1_1, 0) + VADC_CHAN(P_MUX16_1_1, 0) /* 0x1f */ + + VADC_CHAN(P_MUX1_1_3, 1) /* 0x20 */ + VADC_CHAN(P_MUX2_1_3, 1) + VADC_CHAN(P_MUX3_1_3, 1) + VADC_CHAN(P_MUX4_1_3, 1) + VADC_CHAN(P_MUX5_1_3, 1) + VADC_CHAN(P_MUX6_1_3, 1) + VADC_CHAN(P_MUX7_1_3, 1) + VADC_CHAN(P_MUX8_1_3, 1) + VADC_CHAN(P_MUX9_1_3, 1) + VADC_CHAN(P_MUX10_1_3, 1) + VADC_CHAN(P_MUX11_1_3, 1) + VADC_CHAN(P_MUX12_1_3, 1) + VADC_CHAN(P_MUX13_1_3, 1) + VADC_CHAN(P_MUX14_1_3, 1) + VADC_CHAN(P_MUX15_1_3, 1) + VADC_CHAN(P_MUX16_1_3, 1) /* 0x2f */ + + VADC_CHAN(LR_MUX1_BAT_THERM, 0) /* 0x30 */ + VADC_CHAN(LR_MUX2_BAT_ID, 0) + VADC_CHAN(LR_MUX3_XO_THERM, 0) + VADC_CHAN(LR_MUX4_AMUX_THM1, 0) + VADC_CHAN(LR_MUX5_AMUX_THM2, 0) + VADC_CHAN(LR_MUX6_AMUX_THM3, 0) + VADC_CHAN(LR_MUX7_HW_ID, 0) + VADC_CHAN(LR_MUX8_AMUX_THM4, 0) + VADC_CHAN(LR_MUX9_AMUX_THM5, 0) + VADC_CHAN(LR_MUX10_USB_ID, 0) + VADC_CHAN(AMUX_PU1, 0) + VADC_CHAN(AMUX_PU2, 0) + VADC_CHAN(LR_MUX3_BUF_XO_THERM, 0) /* 0x3c */ + + VADC_CHAN(LR_MUX1_PU1_BAT_THERM, 0) /* 0x70 */ + VADC_CHAN(LR_MUX2_PU1_BAT_ID, 0) + VADC_CHAN(LR_MUX3_PU1_XO_THERM, 0) + VADC_CHAN(LR_MUX4_PU1_AMUX_THM1, 0) + VADC_CHAN(LR_MUX5_PU1_AMUX_THM2, 0) + VADC_CHAN(LR_MUX6_PU1_AMUX_THM3, 0) + VADC_CHAN(LR_MUX7_PU1_AMUX_HW_ID, 0) + VADC_CHAN(LR_MUX8_PU1_AMUX_THM4, 0) + VADC_CHAN(LR_MUX9_PU1_AMUX_THM5, 0) + VADC_CHAN(LR_MUX10_PU1_AMUX_USB_ID, 0) /* 0x79 */ + VADC_CHAN(LR_MUX3_BUF_PU1_XO_THERM, 0) /* 0x7c */ + + VADC_CHAN(LR_MUX1_PU2_BAT_THERM, 0) /* 0xb0 */ + VADC_CHAN(LR_MUX2_PU2_BAT_ID, 0) + VADC_CHAN(LR_MUX3_PU2_XO_THERM, 0) + VADC_CHAN(LR_MUX4_PU2_AMUX_THM1, 0) + VADC_CHAN(LR_MUX5_PU2_AMUX_THM2, 0) + VADC_CHAN(LR_MUX6_PU2_AMUX_THM3, 0) + VADC_CHAN(LR_MUX7_PU2_AMUX_HW_ID, 0) + VADC_CHAN(LR_MUX8_PU2_AMUX_THM4, 0) + VADC_CHAN(LR_MUX9_PU2_AMUX_THM5, 0) + VADC_CHAN(LR_MUX10_PU2_AMUX_USB_ID, 0) /* 0xb9 */ + VADC_CHAN(LR_MUX3_BUF_PU2_XO_THERM, 0) /* 0xbc */ + + VADC_CHAN(LR_MUX1_PU1_PU2_BAT_THERM, 0) /* 0xf0 */ + VADC_CHAN(LR_MUX2_PU1_PU2_BAT_ID, 0) + VADC_CHAN(LR_MUX3_PU1_PU2_XO_THERM, 0) + VADC_CHAN(LR_MUX4_PU1_PU2_AMUX_THM1, 0) + VADC_CHAN(LR_MUX5_PU1_PU2_AMUX_THM2, 0) + VADC_CHAN(LR_MUX6_PU1_PU2_AMUX_THM3, 0) + VADC_CHAN(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0) + VADC_CHAN(LR_MUX8_PU1_PU2_AMUX_THM4, 0) + VADC_CHAN(LR_MUX9_PU1_PU2_AMUX_THM5, 0) + VADC_CHAN(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0) /* 0xf9 */ + VADC_CHAN(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0) /* 0xfc */ +}; + +static int +vadc_get_dt_channel_data(struct vadc_priv *vadc, struct device_node *node) +{ + struct vadc_channel *vchan; + u32 channel, value, varr[2]; + int rc, pre, time, avg, decim; + const char *name = node->name; + + rc = of_property_read_u32(node, "reg", &channel); + if (rc) { + dev_dbg(vadc->dev, "invalid channel number %s\n", name); + return -EINVAL; + } + + if (channel >= vadc->nchannels) { + dev_dbg(vadc->dev, "%s invalid channel number %d\n", name, + channel); + return -EINVAL; + } + + vchan = &vadc->channels[channel]; + + /* the channel is active */ + vchan->number = channel; + + rc = of_property_read_u32(node, "qcom,decimation", &value); + if (!rc) { + decim = vadc_decimation_from_dt(value); + if (decim < 0) { + dev_dbg(vadc->dev, "%02x invalid decimation %d\n", + channel, value); + return -EINVAL; + } + vchan->decimation = decim; + } + + rc = of_property_read_u32_array(node, "qcom,pre-scaling", varr, 2); + if (!rc) { + pre = vadc_prescaling_from_dt(varr[0], varr[1]); + if (pre < 0) { + dev_dbg(vadc->dev, + "%02x invalid pre-scaling <%d %d>\n", + channel, varr[0], varr[1]); + return -EINVAL; + } + vchan->prescaling = pre; + } + + rc = of_property_read_u32(node, "qcom,hw-settle-time", &value); + if (!rc) { + time = vadc_hw_settle_time_from_dt(value); + if (time < 0) { + dev_dbg(vadc->dev, + "%02x invalid hw-settle-time %d, us\n", + channel, value); + return -EINVAL; + } + vchan->hw_settle_time = time; + } + + rc = of_property_read_u32(node, "qcom,avg-samples", &value); + if (!rc) { + avg = vadc_avg_samples_from_dt(value); + if (avg < 0) { + dev_dbg(vadc->dev, "%02x invalid avg-samples %d\n", + channel, value); + return -EINVAL; + } + vchan->avg_samples = avg; + } + + if (of_property_read_bool(node, "qcom,ratiometric")) + vchan->calibration = VADC_CALIB_RATIOMETRIC; + else + vchan->calibration = VADC_CALIB_ABSOLUTE; + + dev_dbg(vadc->dev, "%02x name %s\n", channel, name); + + return 0; +} + +static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node) +{ + struct device_node *child; + int rc; + + vadc->poll_eoc = of_property_read_bool(node, "qcom,poll-eoc"); + + for_each_available_child_of_node(node, child) { + rc = vadc_get_dt_channel_data(vadc, child); + if (rc < 0) + return rc; + } + + return 0; +} + +static irqreturn_t vadc_isr(int irq, void *dev_id) +{ + struct vadc_priv *vadc = dev_id; + + complete(&vadc->complete); + + return IRQ_HANDLED; +} + +static int vadc_version_check(struct vadc_priv *vadc) +{ + u8 revision, type, subtype; + int rc; + + rc = vadc_read(vadc, VADC_PERPH_TYPE, &type); + if (rc < 0) + return rc; + + if (type < VADC_PERPH_TYPE_ADC) { + dev_dbg(vadc->dev, "%d is not ADC\n", type); + return -EINVAL; + } + + rc = vadc_read(vadc, VADC_PERPH_SUBTYPE, &subtype); + if (rc < 0) + return rc; + + if (subtype < VADC_PERPH_SUBTYPE_VADC) { + dev_dbg(vadc->dev, "%d is not VADC\n", subtype); + return -EINVAL; + } + + rc = vadc_read(vadc, VADC_REVISION2, &revision); + if (rc < 0) + return rc; + + if (revision < VADC_REVISION2_SUPPORTED_VADC) { + dev_dbg(vadc->dev, "revision %d not supported\n", revision); + return -EINVAL; + } + + return 0; +} + +static int vadc_probe(struct platform_device *pdev) +{ + struct device_node *node = pdev->dev.of_node; + struct device *dev = &pdev->dev; + struct iio_dev *indio_dev; + struct vadc_channel *vchan; + struct vadc_priv *vadc; + struct resource *res; + struct regmap *regmap; + int rc, irq_eoc, n; + + regmap = dev_get_regmap(dev->parent, NULL); + if (!regmap) + return -ENODEV; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*vadc)); + if (!indio_dev) + return -ENOMEM; + + vadc = iio_priv(indio_dev); + vadc->dev = dev; + vadc->regmap = regmap; + vadc->is_ref_measured = false; + init_completion(&vadc->complete); + + vadc->nchannels = ARRAY_SIZE(vadc_channels); + vadc->channels = devm_kcalloc(dev, vadc->nchannels, + sizeof(*vadc->channels), GFP_KERNEL); + if (!vadc->channels) + return -ENOMEM; + + for (n = 0; n < vadc->nchannels; n++) { + vchan = &vadc->channels[n]; + /* set default channel properties */ + vchan->number = -1; /* inactive */ + vchan->prescaling = vadc_channels[n].address; + vchan->decimation = VADC_DEF_DECIMATION; + vchan->hw_settle_time = VADC_DEF_HW_SETTLE_TIME; + vchan->avg_samples = VADC_DEF_AVG_SAMPLES; + vchan->calibration = VADC_DEF_CALIB_TYPE; + } + + platform_set_drvdata(pdev, vadc); + + res = platform_get_resource(pdev, IORESOURCE_REG, 0); + if (!res) + return -ENODEV; + + vadc->base = res->start; + + rc = vadc_version_check(vadc); + if (rc < 0) + return -ENODEV; + + rc = vadc_get_dt_data(vadc, node); + if (rc < 0) + return rc; + + if (!vadc->poll_eoc) { + irq_eoc = platform_get_irq(pdev, 0); + if (irq_eoc < 0) + return irq_eoc; + + rc = devm_request_irq(dev, irq_eoc, vadc_isr, 0, + "spmi-vadc", vadc); + if (!rc) + enable_irq_wake(irq_eoc); + else + return rc; + } else { + device_init_wakeup(vadc->dev, true); + } + + rc = vadc_reset(vadc); + if (rc < 0) { + dev_dbg(dev, "reset failed\n"); + return rc; + } + + 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 = &vadc_info; + indio_dev->channels = vadc_channels; + indio_dev->num_channels = ARRAY_SIZE(vadc_channels); + + return devm_iio_device_register(dev, indio_dev); +} + +static const struct of_device_id vadc_match_table[] = { + { .compatible = "qcom,spmi-vadc" }, + { } +}; +MODULE_DEVICE_TABLE(of, vadc_match_table); + +static struct platform_driver vadc_driver = { + .driver = { + .name = "spmi-vadc", + .of_match_table = vadc_match_table, + }, + .probe = vadc_probe, +}; +module_platform_driver(vadc_driver); + +MODULE_ALIAS("platform:spmi-vadc"); +MODULE_DESCRIPTION("Qualcomm SPMI PMIC voltage ADC driver"); +MODULE_LICENSE("GPL v2"); diff --git a/include/dt-bindings/iio/qcom,spmi-pmic-vadc.h b/include/dt-bindings/iio/qcom,spmi-pmic-vadc.h new file mode 100644 index 0000000..d543f24 --- /dev/null +++ b/include/dt-bindings/iio/qcom,spmi-pmic-vadc.h @@ -0,0 +1,119 @@ +/* + * 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. + */ + +#ifndef _DT_BINDINGS_QCOM_PMIC_ADC_H +#define _DT_BINDINGS_QCOM_PMIC_ADC_H + +/* Voltage ADC channels */ +#define VADC_USBIN 0x00 +#define VADC_DCIN 0x01 +#define VADC_VCHG_SNS 0x02 +#define VADC_SPARE1_03 0x03 +#define VADC_USB_ID_MV 0x04 +#define VADC_VCOIN 0x05 +#define VADC_VBAT_SNS 0x06 +#define VADC_VSYS 0x07 +#define VADC_DIE_TEMP 0x08 +#define VADC_REF_625MV 0x09 +#define VADC_REF_1250MV 0x0a +#define VADC_CHG_TEMP 0x0b +#define VADC_SPARE1 0x0c +#define VADC_SPARE2 0x0d +#define VADC_GND_REF 0x0e +#define VADC_VDD_VADC 0x0f + +#define VADC_P_MUX1_1_1 0x10 +#define VADC_P_MUX2_1_1 0x11 +#define VADC_P_MUX3_1_1 0x12 +#define VADC_P_MUX4_1_1 0x13 +#define VADC_P_MUX5_1_1 0x14 +#define VADC_P_MUX6_1_1 0x15 +#define VADC_P_MUX7_1_1 0x16 +#define VADC_P_MUX8_1_1 0x17 +#define VADC_P_MUX9_1_1 0x18 +#define VADC_P_MUX10_1_1 0x19 +#define VADC_P_MUX11_1_1 0x1a +#define VADC_P_MUX12_1_1 0x1b +#define VADC_P_MUX13_1_1 0x1c +#define VADC_P_MUX14_1_1 0x1d +#define VADC_P_MUX15_1_1 0x1e +#define VADC_P_MUX16_1_1 0x1f + +#define VADC_P_MUX1_1_3 0x20 +#define VADC_P_MUX2_1_3 0x21 +#define VADC_P_MUX3_1_3 0x22 +#define VADC_P_MUX4_1_3 0x23 +#define VADC_P_MUX5_1_3 0x24 +#define VADC_P_MUX6_1_3 0x25 +#define VADC_P_MUX7_1_3 0x26 +#define VADC_P_MUX8_1_3 0x27 +#define VADC_P_MUX9_1_3 0x28 +#define VADC_P_MUX10_1_3 0x29 +#define VADC_P_MUX11_1_3 0x2a +#define VADC_P_MUX12_1_3 0x2b +#define VADC_P_MUX13_1_3 0x2c +#define VADC_P_MUX14_1_3 0x2d +#define VADC_P_MUX15_1_3 0x2e +#define VADC_P_MUX16_1_3 0x2f + +#define VADC_LR_MUX1_BAT_THERM 0x30 +#define VADC_LR_MUX2_BAT_ID 0x31 +#define VADC_LR_MUX3_XO_THERM 0x32 +#define VADC_LR_MUX4_AMUX_THM1 0x33 +#define VADC_LR_MUX5_AMUX_THM2 0x34 +#define VADC_LR_MUX6_AMUX_THM3 0x35 +#define VADC_LR_MUX7_HW_ID 0x36 +#define VADC_LR_MUX8_AMUX_THM4 0x37 +#define VADC_LR_MUX9_AMUX_THM5 0x38 +#define VADC_LR_MUX10_USB_ID 0x39 +#define VADC_AMUX_PU1 0x3a +#define VADC_AMUX_PU2 0x3b +#define VADC_LR_MUX3_BUF_XO_THERM 0x3c + +#define VADC_LR_MUX1_PU1_BAT_THERM 0x70 +#define VADC_LR_MUX2_PU1_BAT_ID 0x71 +#define VADC_LR_MUX3_PU1_XO_THERM 0x72 +#define VADC_LR_MUX4_PU1_AMUX_THM1 0x73 +#define VADC_LR_MUX5_PU1_AMUX_THM2 0x74 +#define VADC_LR_MUX6_PU1_AMUX_THM3 0x75 +#define VADC_LR_MUX7_PU1_AMUX_HW_ID 0x76 +#define VADC_LR_MUX8_PU1_AMUX_THM4 0x77 +#define VADC_LR_MUX9_PU1_AMUX_THM5 0x78 +#define VADC_LR_MUX10_PU1_AMUX_USB_ID 0x79 +#define VADC_LR_MUX3_BUF_PU1_XO_THERM 0x7c + +#define VADC_LR_MUX1_PU2_BAT_THERM 0xb0 +#define VADC_LR_MUX2_PU2_BAT_ID 0xb1 +#define VADC_LR_MUX3_PU2_XO_THERM 0xb2 +#define VADC_LR_MUX4_PU2_AMUX_THM1 0xb3 +#define VADC_LR_MUX5_PU2_AMUX_THM2 0xb4 +#define VADC_LR_MUX6_PU2_AMUX_THM3 0xb5 +#define VADC_LR_MUX7_PU2_AMUX_HW_ID 0xb6 +#define VADC_LR_MUX8_PU2_AMUX_THM4 0xb7 +#define VADC_LR_MUX9_PU2_AMUX_THM5 0xb8 +#define VADC_LR_MUX10_PU2_AMUX_USB_ID 0xb9 +#define VADC_LR_MUX3_BUF_PU2_XO_THERM 0xbc + +#define VADC_LR_MUX1_PU1_PU2_BAT_THERM 0xf0 +#define VADC_LR_MUX2_PU1_PU2_BAT_ID 0xf1 +#define VADC_LR_MUX3_PU1_PU2_XO_THERM 0xf2 +#define VADC_LR_MUX4_PU1_PU2_AMUX_THM1 0xf3 +#define VADC_LR_MUX5_PU1_PU2_AMUX_THM2 0xf4 +#define VADC_LR_MUX6_PU1_PU2_AMUX_THM3 0xf5 +#define VADC_LR_MUX7_PU1_PU2_AMUX_HW_ID 0xf6 +#define VADC_LR_MUX8_PU1_PU2_AMUX_THM4 0xf7 +#define VADC_LR_MUX9_PU1_PU2_AMUX_THM5 0xf8 +#define VADC_LR_MUX10_PU1_PU2_AMUX_USB_ID 0xf9 +#define VADC_LR_MUX3_BUF_PU1_PU2_XO_THERM 0xfc + +#endif /* _DT_BINDINGS_QCOM_PMIC_ADC_H */ -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-11 15:13 ` [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver Stanimir Varbanov @ 2014-09-12 23:27 ` Hartmut Knaack 2014-09-13 17:27 ` Jonathan Cameron [not found] ` <54138151.8010902-Mmb7MZpHnFY@public.gmane.org> 0 siblings, 2 replies; 23+ messages in thread From: Hartmut Knaack @ 2014-09-12 23:27 UTC (permalink / raw) To: Stanimir Varbanov, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Jonathan Cameron, Arnd Bergmann Cc: linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov Stanimir Varbanov schrieb, Am 11.09.2014 17:13: > The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has > 15bits resolution and register space inside PMIC accessible across > SPMI bus. > > The vadc driver registers itself through IIO interface. > Looks already pretty good. Things you should consider in regard of common coding style are to use the variable name ret instead of rc, since it is used in almost all adc drivers and thus makes reviewing a bit easier. Besides that, you seem to use unsigned as well as unsigned int, so to be consistent, please stick to one of them. Other comments in line. > Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> > --- > drivers/iio/adc/Kconfig | 11 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/qcom-spmi-vadc.c | 999 +++++++++++++++++++++++++ > include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ > 4 files changed, 1130 insertions(+), 0 deletions(-) > create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c > create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 11b048a..08700d4 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_VADC > + tristate "Qualcomm SPMI PMIC voltage ADC" > + depends on SPMI > + help > + Say yes here if you want support for the Qualcomm SPMI PMIC voltage ADC. > + > + The driver supports reading the HKADC, XOADC through the ADC AMUX arbiter. > + The VADC includes support for the conversion sequencer. The driver > + supports reading the ADC through the AMUX channels for external pull-ups > + simultaneously. > + > 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..d5d18f4 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -22,6 +22,7 @@ obj-$(CONFIG_MCP320X) += mcp320x.o > obj-$(CONFIG_MCP3422) += mcp3422.o > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > obj-$(CONFIG_NAU7802) += nau7802.o > +obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o > obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o > obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o > obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o > diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c > new file mode 100644 > index 0000000..e30eb04 > --- /dev/null > +++ b/drivers/iio/adc/qcom-spmi-vadc.c > @@ -0,0 +1,999 @@ > +/* > + * 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/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > +#include <linux/log2.h> > + > +#include <dt-bindings/iio/qcom,spmi-pmic-vadc.h> > + > +/* VADC register and bit definition */ > +#define VADC_REVISION2 0x1 > +#define VADC_REVISION2_SUPPORTED_VADC 1 > + > +#define VADC_PERPH_TYPE 0x4 > +#define VADC_PERPH_TYPE_ADC 8 > + > +#define VADC_PERPH_SUBTYPE 0x5 > +#define VADC_PERPH_SUBTYPE_VADC 1 > + > +#define VADC_STATUS1 0x8 > +#define VADC_STATUS1_OP_MODE 4 > +#define VADC_STATUS1_REQ_STS BIT(1) > +#define VADC_STATUS1_EOC BIT(0) > +#define VADC_STATUS1_REQ_STS_EOC_MASK 0x3 > + > +#define VADC_MODE_CTL 0x40 > +#define VADC_OP_MODE_SHIFT 3 > +#define VADC_OP_MODE_NORMAL 0 > +#define VADC_AMUX_TRIM_EN BIT(1) > +#define VADC_ADC_TRIM_EN BIT(0) > + > +#define VADC_EN_CTL1 0x46 > +#define VADC_EN_CTL1_SET BIT(7) > + > +#define VADC_ADC_CH_SEL_CTL 0x48 > + > +#define VADC_ADC_DIG_PARAM 0x50 > +#define VADC_ADC_DIG_DEC_RATIO_SEL_SHIFT 2 > + > +#define VADC_HW_SETTLE_DELAY 0x51 > + > +#define VADC_CONV_REQ 0x52 > +#define VADC_CONV_REQ_SET BIT(7) > + > +#define VADC_FAST_AVG_CTL 0x5a > +#define VADC_FAST_AVG_EN 0x5b > +#define VADC_FAST_AVG_EN_SET BIT(7) > + > +#define VADC_ACCESS 0xd0 > +#define VADC_ACCESS_DATA 0xa5 > + > +#define VADC_PERH_RESET_CTL3 0xda > +#define VADC_FOLLOW_WARM_RB BIT(2) > + > +#define VADC_DATA0 0x60 > +#define VADC_DATA1 0x61 > + > +#define VADC_CONV_TIME_MIN_US 2000 > +#define VADC_CONV_TIME_MAX_US 2100 > + > +/* Min ADC code represents 0V */ > +#define VADC_MIN_ADC_CODE 0x6000 > +/* Max ADC code represents full-scale range of 1.8V */ > +#define VADC_MAX_ADC_CODE 0xA800 > + > +#define VADC_ABSOLUTE_RANGE_UV 625000 > +#define VADC_RATIOMETRIC_RANGE_UV 1800000 > + > +#define VADC_DEF_PRESCALING 0 /* 1:1 */ > +#define VADC_DEF_DECIMATION 0 /* 512 */ > +#define VADC_DEF_HW_SETTLE_TIME 0 /* 0 us */ > +#define VADC_DEF_AVG_SAMPLES 0 /* 1 sample */ > +#define VADC_DEF_CALIB_TYPE VADC_CALIB_ABSOLUTE > + > +#define VADC_DECIMATION_MIN 512 > +#define VADC_DECIMATION_MAX 4096 > + > +#define VADC_HW_SETTLE_DELAY_MAX 10000 > +#define VADC_AVG_SAMPLES_MAX 512 > + > +/* > + * VADC_CALIB_ABSOLUTE: Uses the 625mV and 1.25V reference channels. > + * VADC_CALIB_RATIOMETRIC: Uses the reference Voltage/GND for calibration. > + */ > +enum vadc_calibration { > + VADC_CALIB_ABSOLUTE = 0, > + VADC_CALIB_RATIOMETRIC > +}; > + > +/** > + * struct vadc_linear_graph - Represent ADC characteristics. > + * @dy: Numerator slope to calculate the gain. > + * @dx: Denominator slope to calculate the gain. > + * @vref: A/D word of the voltage reference used for the channel. > + * @gnd: A/D word of the ground reference used for the channel. > + * > + * Each ADC device has different offset and gain parameters which are > + * computed to calibrate the device. > + */ > +struct vadc_linear_graph { > + s32 dy; > + s32 dx; > + s32 vref; > + s32 gnd; > +}; > + > +/** > + * struct vadc_prescaling - Represent scaling ratio for ADC input. > + * @num: The inverse numerator of the gain applied to the input channel. > + * @den: The inverse denominator of the gain applied to the input channel. > + */ > +struct vadc_prescaling { > + u32 num; > + u32 den; > +}; > + > +/** > + * struct vadc_result - Represent the res of the QPNP ADC. ... Represents the result... > + * @adc_code: The pre-calibrated digital output of a given ADC relative to the > + * the ADC reference. > + * @measurement: In units specific for a given ADC; most ADC uses reference > + * voltage but some ADC uses reference current. This measurement > + * here is a number relative to a reference of a given ADC. > + * @physical: The data meaningful for each individual channel whether it is > + * voltage, current, temperature, etc. > + */ > +struct vadc_result { > + s32 adc_code; > + s64 measurement; > + s64 physical; > +}; > + > +/** > + * struct vadc_channel - QPNP VADC amux channel property. > + * @number - Channel number, refer to the channel list. > + * @calibration - Calibration type. > + * @decimation - Sampling rate supported for the channel. > + * @prescaling - Channel scaling performed on the input signal. > + * @hw_settle_time - The time between AMUX being configured and the > + * start of conversion. > + * @avg_samples - Ability to provide single result from the ADC > + * that is an average of multiple measurements. > + */ > +struct vadc_channel { > + int number; > + enum vadc_calibration calibration; > + unsigned decimation; > + unsigned prescaling; > + unsigned hw_settle_time; > + unsigned avg_samples; > +}; > + > +/** > + * struct vadc_priv - VADC private structure. > + * @regmap - pointer to struct regmap > + * @dev - pointer to struct device. > + * @base - base address for the ADC peripheral. > + * @nchannels - number of ADC channels Missing period at end of line of @regmap and @nchannels. > + * @channels - AMUX properties representing the ADC peripheral. > + * @is_ref_measured - does reference points are measured. This is expressed wrong. > + * @poll_eoc - do we use polling instead of interrupt. > + * @complete - ADC result notification after interrupt is received. > + * @graph - store parameters for calibration. > + */ > +struct vadc_priv { > + struct regmap *regmap; > + struct device *dev; > + u16 base; > + unsigned nchannels; > + struct vadc_channel *channels; > + bool is_ref_measured; > + bool poll_eoc; > + struct completion complete; > + struct vadc_linear_graph graph[2]; > +}; > + > +static const struct vadc_prescaling vadc_prescale[] = { > + {1, 1}, > + {1, 3}, > + {1, 4}, > + {1, 6}, > + {1, 20}, > + {1, 8}, > + {10, 81}, > + {1, 10} > +}; > + > +static int vadc_read(struct vadc_priv *vadc, u16 offset, u8 *data) > +{ > + return regmap_bulk_read(vadc->regmap, vadc->base + offset, data, 1); > +} > + > +static int vadc_write(struct vadc_priv *vadc, u16 offset, u8 data) > +{ > + return regmap_write(vadc->regmap, vadc->base + offset, data); > +} > + > +static int vadc_reset(struct vadc_priv *vadc) > +{ > + u8 data; > + int rc; > + > + rc = vadc_write(vadc, VADC_ACCESS, VADC_ACCESS_DATA); > + if (rc) > + return rc; > + > + rc = vadc_read(vadc, VADC_PERH_RESET_CTL3, &data); > + if (rc) > + return rc; > + > + rc = vadc_write(vadc, VADC_ACCESS, VADC_ACCESS_DATA); > + if (rc) > + return rc; > + > + data |= VADC_FOLLOW_WARM_RB; > + > + return vadc_write(vadc, VADC_PERH_RESET_CTL3, data); > +} > + > +static int vadc_enable(struct vadc_priv *vadc, bool state) > +{ > + return vadc_write(vadc, VADC_EN_CTL1, state ? VADC_EN_CTL1_SET : 0); > +} > + > +static int vadc_request_conversion(struct vadc_priv *vadc) > +{ > + return vadc_write(vadc, VADC_CONV_REQ, VADC_CONV_REQ_SET); > +} > + > +static void vadc_status_show(struct vadc_priv *vadc) > +{ > + u8 mode, sta1, chan, dig, en, req; > + int rc; > + > + rc = vadc_read(vadc, VADC_MODE_CTL, &mode); > + if (rc) > + return; > + > + rc = vadc_read(vadc, VADC_ADC_DIG_PARAM, &dig); > + if (rc) > + return; > + > + rc = vadc_read(vadc, VADC_ADC_CH_SEL_CTL, &chan); > + if (rc) > + return; > + > + rc = vadc_read(vadc, VADC_CONV_REQ, &req); > + if (rc) > + return; > + > + rc = vadc_read(vadc, VADC_STATUS1, &sta1); > + if (rc) > + return; > + > + rc = vadc_read(vadc, VADC_EN_CTL1, &en); > + if (rc) > + return; > + > + dev_dbg(vadc->dev, > + "mode:%02x en:%02x chan:%02x dig:%02x req:%02x sta1:%02x\n", > + mode, en, chan, dig, req, sta1); > +} > + > +static int vadc_configure(struct vadc_priv *vadc, struct vadc_channel *vchan) > +{ > + u8 decim, mode_ctrl; > + int rc; > + > + /* Mode selection */ > + mode_ctrl = (VADC_OP_MODE_NORMAL << VADC_OP_MODE_SHIFT) | > + VADC_ADC_TRIM_EN | VADC_AMUX_TRIM_EN; > + rc = vadc_write(vadc, VADC_MODE_CTL, mode_ctrl); > + if (rc) > + return rc; > + > + /* Channel selection */ > + rc = vadc_write(vadc, VADC_ADC_CH_SEL_CTL, vchan->number); > + if (rc) > + return rc; > + > + /* Digital parameter setup */ > + decim = vchan->decimation << VADC_ADC_DIG_DEC_RATIO_SEL_SHIFT; > + rc = vadc_write(vadc, VADC_ADC_DIG_PARAM, decim); > + if (rc) > + return rc; > + > + /* HW settle time delay */ > + rc = vadc_write(vadc, VADC_HW_SETTLE_DELAY, vchan->hw_settle_time); > + if (rc) > + return rc; > + > + rc = vadc_write(vadc, VADC_FAST_AVG_CTL, vchan->avg_samples); > + if (rc) > + return rc; > + > + if (vchan->avg_samples) > + rc = vadc_write(vadc, VADC_FAST_AVG_EN, VADC_FAST_AVG_EN_SET); > + else > + rc = vadc_write(vadc, VADC_FAST_AVG_EN, 0); > + > + return rc; > +} > + > +static int vadc_poll_wait_eoc(struct vadc_priv *vadc, unsigned int interval_us) > +{ > + unsigned int count, retry; > + u8 sta1; > + int rc; > + > + retry = interval_us / VADC_CONV_TIME_MIN_US; > + > + for (count = 0; count < retry; count++) { > + rc = vadc_read(vadc, VADC_STATUS1, &sta1); > + if (rc) > + return rc; > + > + sta1 &= VADC_STATUS1_REQ_STS_EOC_MASK; > + if (sta1 == VADC_STATUS1_EOC) > + return 0; > + > + usleep_range(VADC_CONV_TIME_MIN_US, VADC_CONV_TIME_MAX_US); > + } > + > + vadc_status_show(vadc); > + > + return -ETIMEDOUT; > +} > + > +static int vadc_read_result(struct vadc_priv *vadc, u32 *data) > +{ > + u8 lsb, msb; > + int rc; > + > + rc = vadc_read(vadc, VADC_DATA0, &lsb); > + if (rc) > + return rc; > + > + rc = vadc_read(vadc, VADC_DATA1, &msb); Better have a wrapper function vadc_read_word()? > + if (rc) > + return rc; > + > + *data = clamp_t(u32, (msb << 8) | lsb, VADC_MIN_ADC_CODE, > + VADC_MAX_ADC_CODE); Although Peter suggested to use u32 for *data, I think it would be cleaner to use s32 here. It would be more consistent, as *data is s32 in vadc_do_conversion() as well as read_1 and read_2 in vadc_measure_reference_points(). It seems messy to me to pass a s32 by reference to a function, which expects a reference to an u32. Any other opinions on this? > + > + return 0; > +} > + > +static struct vadc_channel *vadc_find_channel(struct vadc_priv *vadc, int num) > +{ > + unsigned i; > + > + for (i = 0; i < vadc->nchannels; i++) > + if (vadc->channels[i].number == num) > + return &vadc->channels[num]; > + > + dev_dbg(vadc->dev, "no such channel %02x\n", num); > + > + return NULL; > +} > + > +static int vadc_do_conversion(struct vadc_priv *vadc, > + struct vadc_channel *vchan, s32 *data) > +{ > + unsigned int timeout; > + int rc; > + > + rc = vadc_configure(vadc, vchan); > + if (rc) > + return rc; > + > + if (!vadc->poll_eoc) > + reinit_completion(&vadc->complete); > + > + rc = vadc_enable(vadc, true); > + if (rc) > + return rc; > + > + rc = vadc_request_conversion(vadc); > + if (rc) > + goto err_disable; > + > + timeout = BIT(vchan->avg_samples) * VADC_CONV_TIME_MIN_US * 2; > + > + if (vadc->poll_eoc) { > + rc = vadc_poll_wait_eoc(vadc, timeout); > + } else { > + rc = wait_for_completion_timeout(&vadc->complete, > + msecs_to_jiffies(1000)); > + if (!rc) { > + rc = -ETIMEDOUT; > + goto err_disable; > + } > + > + /* double check conversion status */ > + rc = vadc_poll_wait_eoc(vadc, VADC_CONV_TIME_MIN_US * 2); > + if (rc) > + goto err_disable; > + } > + > + rc = vadc_read_result(vadc, data); > + > +err_disable: > + vadc_enable(vadc, false); > + > + if (rc) > + dev_dbg(vadc->dev, "conversion failed\n"); > + > + return rc; > +} > + > +static int vadc_measure_reference_points(struct vadc_priv *vadc) > +{ > + struct vadc_channel *vchan; > + s32 read_1, read_2; > + int rc = -EINVAL; > + > + vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV; > + vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV; > + > + vchan = vadc_find_channel(vadc, VADC_REF_1250MV); > + if (!vchan) > + goto err; > + > + rc = vadc_do_conversion(vadc, vchan, &read_1); > + if (rc) > + goto err; > + > + /* Try with buffered 625mV channel first */ > + vchan = vadc_find_channel(vadc, VADC_SPARE1); > + if (!vchan) { > + vchan = vadc_find_channel(vadc, VADC_REF_625MV); > + if (!vchan) { > + rc = -EINVAL; > + goto err; > + } > + } > + > + rc = vadc_do_conversion(vadc, vchan, &read_2); > + if (rc) > + goto err; > + > + if (read_1 == read_2) { > + rc = -EINVAL; > + goto err; > + } > + > + vadc->graph[VADC_CALIB_ABSOLUTE].dy = read_1 - read_2; > + vadc->graph[VADC_CALIB_ABSOLUTE].vref = read_1; > + vadc->graph[VADC_CALIB_ABSOLUTE].gnd = read_2; > + > + /* Ratiometric calibration */ > + vchan = vadc_find_channel(vadc, VADC_VDD_VADC); > + if (!vchan) { > + rc = -EINVAL; > + goto err; > + } > + > + rc = vadc_do_conversion(vadc, vchan, &read_1); > + if (rc) > + goto err; > + > + vchan = vadc_find_channel(vadc, VADC_GND_REF); > + if (!vchan) { > + rc = -EINVAL; > + goto err; > + } > + > + rc = vadc_do_conversion(vadc, vchan, &read_2); > + if (rc) > + goto err; > + > + if (read_1 == read_2) { > + rc = -EINVAL; > + goto err; > + } > + > + vadc->graph[VADC_CALIB_RATIOMETRIC].dy = read_1 - read_2; > + vadc->graph[VADC_CALIB_RATIOMETRIC].vref = read_1; > + vadc->graph[VADC_CALIB_RATIOMETRIC].gnd = read_2; > +err: > + if (rc) > + dev_dbg(vadc->dev, "measure reference points failed\n"); > + > + return rc; > +} > + > +static void vadc_calibrate(struct vadc_priv *vadc, > + const struct vadc_channel *vchan, > + struct vadc_result *res) > +{ > + const struct vadc_prescaling *prescale; > + bool negative = false; > + s64 voltage; > + > + voltage = res->adc_code - vadc->graph[vchan->calibration].gnd; > + voltage *= vadc->graph[vchan->calibration].dx; > + > + if (voltage < 0) { > + negative = true; > + voltage = -voltage; > + } > + > + do_div(voltage, vadc->graph[vchan->calibration].dy); > + if (negative) > + voltage = -voltage; > + > + if (vchan->calibration == VADC_CALIB_ABSOLUTE) > + voltage += vadc->graph[vchan->calibration].dx; > + > + if (voltage < 0) > + voltage = 0; > + > + prescale = &vadc_prescale[vchan->prescaling]; > + > + res->measurement = voltage * prescale->den; > + > + do_div(res->measurement, prescale->num); > + > + res->physical = res->measurement; > +} > + > +static unsigned vadc_decimation_from_dt(unsigned int value) static int vadc_decimation_from_dt > +{ > + if (!is_power_of_2(value) || value < VADC_DECIMATION_MIN || > + value > VADC_DECIMATION_MAX) > + return -EINVAL; > + > + return __ffs64(value / VADC_DECIMATION_MIN); > +} > + > +static int vadc_prescaling_from_dt(unsigned int num, unsigned int den) Better use u32 num, u32 den? > +{ > + unsigned int pre; > + > + for (pre = 0; pre < ARRAY_SIZE(vadc_prescale); pre++) > + if (vadc_prescale[pre].num == num && > + vadc_prescale[pre].den == den) > + break; > + > + if (pre == ARRAY_SIZE(vadc_prescale)) > + return -EINVAL; > + > + return pre; > +} > + > +static int vadc_hw_settle_time_from_dt(unsigned int value) > +{ > + if ((value <= 1000 && value % 100) || (value > 1000 && value % 2000)) > + return -EINVAL; > + > + if (value <= 1000) > + value /= 100; > + else > + value = value / 2000 + 10; > + > + return value; > +} > + > +static int vadc_avg_samples_from_dt(unsigned int value) Better use u32 value? > +{ > + if (!is_power_of_2(value) || value > VADC_AVG_SAMPLES_MAX) > + return -EINVAL; > + > + return __ffs64(value); > +} > + > +static int Don't wrap line here. > +vadc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct vadc_priv *vadc = iio_priv(indio_dev); > + struct vadc_channel *vchan; > + struct vadc_result result; > + int rc; > + > + vchan = vadc_find_channel(vadc, chan->channel); > + if (!vchan) > + return -EINVAL; > + > + if (!vadc->is_ref_measured) { > + rc = vadc_measure_reference_points(vadc); > + if (rc) > + return rc; > + > + vadc->is_ref_measured = true; > + } > + > + switch (mask) { > + case IIO_CHAN_INFO_PROCESSED: > + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); > + if (rc) > + return rc; > + > + vadc_calibrate(vadc, vchan, &result); > + > + *val = result.physical; > + rc = IIO_VAL_INT; return IIO_VAL_INT; > + break; > + default: > + rc = -EINVAL; > + break; Drop default case, or leave empty. > + } > + > + return rc; return -EINVAL; > +} > + > +static const struct iio_info vadc_info = { > + .read_raw = vadc_read_raw, > + .driver_module = THIS_MODULE, > +}; > + > +#define VADC_CHAN(_id, _pre) \ > + [VADC_##_id] = { \ > + .type = IIO_VOLTAGE, \ > + .indexed = 1, \ > + .channel = VADC_##_id, \ > + .address = _pre, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ > + .datasheet_name = __stringify(VADC_##_id), \ > + .scan_type = { \ > + .sign = 's', \ > + .realbits = 15, \ > + .storagebits = 16, \ > + .endianness = IIO_CPU, \ > + }, \ > + }, > + > +static const struct iio_chan_spec vadc_channels[] = { > + VADC_CHAN(USBIN, 4) /* 0x00 */ > + VADC_CHAN(DCIN, 4) > + VADC_CHAN(VCHG_SNS, 3) > + VADC_CHAN(SPARE1_03, 1) > + VADC_CHAN(USB_ID_MV, 1) > + VADC_CHAN(VCOIN, 1) > + VADC_CHAN(VBAT_SNS, 1) > + VADC_CHAN(VSYS, 1) > + VADC_CHAN(DIE_TEMP, 0) > + VADC_CHAN(REF_625MV, 0) > + VADC_CHAN(REF_1250MV, 0) > + VADC_CHAN(CHG_TEMP, 0) > + VADC_CHAN(SPARE1, 0) > + VADC_CHAN(SPARE2, 0) > + VADC_CHAN(GND_REF, 0) > + VADC_CHAN(VDD_VADC, 0) /* 0x0f */ > + > + VADC_CHAN(P_MUX1_1_1, 0) /* 0x10 */ > + VADC_CHAN(P_MUX2_1_1, 0) > + VADC_CHAN(P_MUX3_1_1, 0) > + VADC_CHAN(P_MUX4_1_1, 0) > + VADC_CHAN(P_MUX5_1_1, 0) > + VADC_CHAN(P_MUX6_1_1, 0) > + VADC_CHAN(P_MUX7_1_1, 0) > + VADC_CHAN(P_MUX8_1_1, 0) > + VADC_CHAN(P_MUX9_1_1, 0) > + VADC_CHAN(P_MUX10_1_1, 0) > + VADC_CHAN(P_MUX11_1_1, 0) > + VADC_CHAN(P_MUX12_1_1, 0) > + VADC_CHAN(P_MUX13_1_1, 0) > + VADC_CHAN(P_MUX14_1_1, 0) > + VADC_CHAN(P_MUX15_1_1, 0) > + VADC_CHAN(P_MUX16_1_1, 0) /* 0x1f */ > + > + VADC_CHAN(P_MUX1_1_3, 1) /* 0x20 */ > + VADC_CHAN(P_MUX2_1_3, 1) > + VADC_CHAN(P_MUX3_1_3, 1) > + VADC_CHAN(P_MUX4_1_3, 1) > + VADC_CHAN(P_MUX5_1_3, 1) > + VADC_CHAN(P_MUX6_1_3, 1) > + VADC_CHAN(P_MUX7_1_3, 1) > + VADC_CHAN(P_MUX8_1_3, 1) > + VADC_CHAN(P_MUX9_1_3, 1) > + VADC_CHAN(P_MUX10_1_3, 1) > + VADC_CHAN(P_MUX11_1_3, 1) > + VADC_CHAN(P_MUX12_1_3, 1) > + VADC_CHAN(P_MUX13_1_3, 1) > + VADC_CHAN(P_MUX14_1_3, 1) > + VADC_CHAN(P_MUX15_1_3, 1) > + VADC_CHAN(P_MUX16_1_3, 1) /* 0x2f */ > + > + VADC_CHAN(LR_MUX1_BAT_THERM, 0) /* 0x30 */ > + VADC_CHAN(LR_MUX2_BAT_ID, 0) > + VADC_CHAN(LR_MUX3_XO_THERM, 0) > + VADC_CHAN(LR_MUX4_AMUX_THM1, 0) > + VADC_CHAN(LR_MUX5_AMUX_THM2, 0) > + VADC_CHAN(LR_MUX6_AMUX_THM3, 0) > + VADC_CHAN(LR_MUX7_HW_ID, 0) > + VADC_CHAN(LR_MUX8_AMUX_THM4, 0) > + VADC_CHAN(LR_MUX9_AMUX_THM5, 0) > + VADC_CHAN(LR_MUX10_USB_ID, 0) > + VADC_CHAN(AMUX_PU1, 0) > + VADC_CHAN(AMUX_PU2, 0) > + VADC_CHAN(LR_MUX3_BUF_XO_THERM, 0) /* 0x3c */ > + > + VADC_CHAN(LR_MUX1_PU1_BAT_THERM, 0) /* 0x70 */ > + VADC_CHAN(LR_MUX2_PU1_BAT_ID, 0) > + VADC_CHAN(LR_MUX3_PU1_XO_THERM, 0) > + VADC_CHAN(LR_MUX4_PU1_AMUX_THM1, 0) > + VADC_CHAN(LR_MUX5_PU1_AMUX_THM2, 0) > + VADC_CHAN(LR_MUX6_PU1_AMUX_THM3, 0) > + VADC_CHAN(LR_MUX7_PU1_AMUX_HW_ID, 0) > + VADC_CHAN(LR_MUX8_PU1_AMUX_THM4, 0) > + VADC_CHAN(LR_MUX9_PU1_AMUX_THM5, 0) > + VADC_CHAN(LR_MUX10_PU1_AMUX_USB_ID, 0) /* 0x79 */ > + VADC_CHAN(LR_MUX3_BUF_PU1_XO_THERM, 0) /* 0x7c */ > + > + VADC_CHAN(LR_MUX1_PU2_BAT_THERM, 0) /* 0xb0 */ > + VADC_CHAN(LR_MUX2_PU2_BAT_ID, 0) > + VADC_CHAN(LR_MUX3_PU2_XO_THERM, 0) > + VADC_CHAN(LR_MUX4_PU2_AMUX_THM1, 0) > + VADC_CHAN(LR_MUX5_PU2_AMUX_THM2, 0) > + VADC_CHAN(LR_MUX6_PU2_AMUX_THM3, 0) > + VADC_CHAN(LR_MUX7_PU2_AMUX_HW_ID, 0) > + VADC_CHAN(LR_MUX8_PU2_AMUX_THM4, 0) > + VADC_CHAN(LR_MUX9_PU2_AMUX_THM5, 0) > + VADC_CHAN(LR_MUX10_PU2_AMUX_USB_ID, 0) /* 0xb9 */ > + VADC_CHAN(LR_MUX3_BUF_PU2_XO_THERM, 0) /* 0xbc */ > + > + VADC_CHAN(LR_MUX1_PU1_PU2_BAT_THERM, 0) /* 0xf0 */ > + VADC_CHAN(LR_MUX2_PU1_PU2_BAT_ID, 0) > + VADC_CHAN(LR_MUX3_PU1_PU2_XO_THERM, 0) > + VADC_CHAN(LR_MUX4_PU1_PU2_AMUX_THM1, 0) > + VADC_CHAN(LR_MUX5_PU1_PU2_AMUX_THM2, 0) > + VADC_CHAN(LR_MUX6_PU1_PU2_AMUX_THM3, 0) > + VADC_CHAN(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0) > + VADC_CHAN(LR_MUX8_PU1_PU2_AMUX_THM4, 0) > + VADC_CHAN(LR_MUX9_PU1_PU2_AMUX_THM5, 0) > + VADC_CHAN(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0) /* 0xf9 */ > + VADC_CHAN(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0) /* 0xfc */ > +}; > + > +static int > +vadc_get_dt_channel_data(struct vadc_priv *vadc, struct device_node *node) > +{ > + struct vadc_channel *vchan; > + u32 channel, value, varr[2]; > + int rc, pre, time, avg, decim; Drop pre, time, avg and decim and reuse rc instead? > + const char *name = node->name; > + > + rc = of_property_read_u32(node, "reg", &channel); > + if (rc) { > + dev_dbg(vadc->dev, "invalid channel number %s\n", name); > + return -EINVAL; > + } > + > + if (channel >= vadc->nchannels) { > + dev_dbg(vadc->dev, "%s invalid channel number %d\n", name, > + channel); > + return -EINVAL; > + } > + > + vchan = &vadc->channels[channel]; > + > + /* the channel is active */ > + vchan->number = channel; > + > + rc = of_property_read_u32(node, "qcom,decimation", &value); > + if (!rc) { > + decim = vadc_decimation_from_dt(value); > + if (decim < 0) { > + dev_dbg(vadc->dev, "%02x invalid decimation %d\n", > + channel, value); > + return -EINVAL; return decim; > + } > + vchan->decimation = decim; > + } > + > + rc = of_property_read_u32_array(node, "qcom,pre-scaling", varr, 2); > + if (!rc) { > + pre = vadc_prescaling_from_dt(varr[0], varr[1]); > + if (pre < 0) { > + dev_dbg(vadc->dev, > + "%02x invalid pre-scaling <%d %d>\n", > + channel, varr[0], varr[1]); > + return -EINVAL; return pre; > + } > + vchan->prescaling = pre; > + } > + > + rc = of_property_read_u32(node, "qcom,hw-settle-time", &value); > + if (!rc) { > + time = vadc_hw_settle_time_from_dt(value); > + if (time < 0) { > + dev_dbg(vadc->dev, > + "%02x invalid hw-settle-time %d, us\n", > + channel, value); > + return -EINVAL; return time; > + } > + vchan->hw_settle_time = time; > + } > + > + rc = of_property_read_u32(node, "qcom,avg-samples", &value); > + if (!rc) { > + avg = vadc_avg_samples_from_dt(value); > + if (avg < 0) { > + dev_dbg(vadc->dev, "%02x invalid avg-samples %d\n", > + channel, value); > + return -EINVAL; return avg; > + } > + vchan->avg_samples = avg; > + } > + > + if (of_property_read_bool(node, "qcom,ratiometric")) > + vchan->calibration = VADC_CALIB_RATIOMETRIC; > + else > + vchan->calibration = VADC_CALIB_ABSOLUTE; Or simply vchan->calibration = of_property_read_bool(node, "qcom,ratiometric")? > + > + dev_dbg(vadc->dev, "%02x name %s\n", channel, name); > + > + return 0; > +} > + > +static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node) > +{ > + struct device_node *child; > + int rc; > + > + vadc->poll_eoc = of_property_read_bool(node, "qcom,poll-eoc"); > + > + for_each_available_child_of_node(node, child) { > + rc = vadc_get_dt_channel_data(vadc, child); > + if (rc < 0) > + return rc; > + } > + > + return 0; > +} > + > +static irqreturn_t vadc_isr(int irq, void *dev_id) > +{ > + struct vadc_priv *vadc = dev_id; > + > + complete(&vadc->complete); > + > + return IRQ_HANDLED; > +} > + > +static int vadc_version_check(struct vadc_priv *vadc) > +{ > + u8 revision, type, subtype; > + int rc; > + > + rc = vadc_read(vadc, VADC_PERPH_TYPE, &type); > + if (rc < 0) > + return rc; > + > + if (type < VADC_PERPH_TYPE_ADC) { > + dev_dbg(vadc->dev, "%d is not ADC\n", type); > + return -EINVAL; > + } > + > + rc = vadc_read(vadc, VADC_PERPH_SUBTYPE, &subtype); > + if (rc < 0) > + return rc; > + > + if (subtype < VADC_PERPH_SUBTYPE_VADC) { > + dev_dbg(vadc->dev, "%d is not VADC\n", subtype); > + return -EINVAL; > + } > + > + rc = vadc_read(vadc, VADC_REVISION2, &revision); > + if (rc < 0) > + return rc; > + > + if (revision < VADC_REVISION2_SUPPORTED_VADC) { > + dev_dbg(vadc->dev, "revision %d not supported\n", revision); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int vadc_probe(struct platform_device *pdev) > +{ > + struct device_node *node = pdev->dev.of_node; > + struct device *dev = &pdev->dev; > + struct iio_dev *indio_dev; > + struct vadc_channel *vchan; > + struct vadc_priv *vadc; > + struct resource *res; > + struct regmap *regmap; > + int rc, irq_eoc, n; unsigned int n? > + > + regmap = dev_get_regmap(dev->parent, NULL); > + if (!regmap) > + return -ENODEV; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*vadc)); > + if (!indio_dev) > + return -ENOMEM; > + > + vadc = iio_priv(indio_dev); > + vadc->dev = dev; > + vadc->regmap = regmap; > + vadc->is_ref_measured = false; > + init_completion(&vadc->complete); > + > + vadc->nchannels = ARRAY_SIZE(vadc_channels); > + vadc->channels = devm_kcalloc(dev, vadc->nchannels, > + sizeof(*vadc->channels), GFP_KERNEL); > + if (!vadc->channels) > + return -ENOMEM; > + > + for (n = 0; n < vadc->nchannels; n++) { > + vchan = &vadc->channels[n]; > + /* set default channel properties */ > + vchan->number = -1; /* inactive */ > + vchan->prescaling = vadc_channels[n].address; > + vchan->decimation = VADC_DEF_DECIMATION; > + vchan->hw_settle_time = VADC_DEF_HW_SETTLE_TIME; > + vchan->avg_samples = VADC_DEF_AVG_SAMPLES; > + vchan->calibration = VADC_DEF_CALIB_TYPE; > + } > + > + platform_set_drvdata(pdev, vadc); > + > + res = platform_get_resource(pdev, IORESOURCE_REG, 0); > + if (!res) > + return -ENODEV; > + > + vadc->base = res->start; > + > + rc = vadc_version_check(vadc); > + if (rc < 0) > + return -ENODEV; > + > + rc = vadc_get_dt_data(vadc, node); > + if (rc < 0) > + return rc; > + > + if (!vadc->poll_eoc) { > + irq_eoc = platform_get_irq(pdev, 0); > + if (irq_eoc < 0) > + return irq_eoc; > + > + rc = devm_request_irq(dev, irq_eoc, vadc_isr, 0, > + "spmi-vadc", vadc); > + if (!rc) > + enable_irq_wake(irq_eoc); > + else > + return rc; > + } else { > + device_init_wakeup(vadc->dev, true); > + } > + > + rc = vadc_reset(vadc); > + if (rc < 0) { > + dev_dbg(dev, "reset failed\n"); > + return rc; > + } > + > + 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 = &vadc_info; > + indio_dev->channels = vadc_channels; > + indio_dev->num_channels = ARRAY_SIZE(vadc_channels); > + > + return devm_iio_device_register(dev, indio_dev); > +} > + > +static const struct of_device_id vadc_match_table[] = { > + { .compatible = "qcom,spmi-vadc" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, vadc_match_table); > + > +static struct platform_driver vadc_driver = { > + .driver = { > + .name = "spmi-vadc", > + .of_match_table = vadc_match_table, > + }, > + .probe = vadc_probe, > +}; > +module_platform_driver(vadc_driver); > + > +MODULE_ALIAS("platform:spmi-vadc"); > +MODULE_DESCRIPTION("Qualcomm SPMI PMIC voltage ADC driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/dt-bindings/iio/qcom,spmi-pmic-vadc.h b/include/dt-bindings/iio/qcom,spmi-pmic-vadc.h > new file mode 100644 > index 0000000..d543f24 > --- /dev/null > +++ b/include/dt-bindings/iio/qcom,spmi-pmic-vadc.h > @@ -0,0 +1,119 @@ > +/* > + * 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. > + */ > + > +#ifndef _DT_BINDINGS_QCOM_PMIC_ADC_H > +#define _DT_BINDINGS_QCOM_PMIC_ADC_H > + > +/* Voltage ADC channels */ > +#define VADC_USBIN 0x00 > +#define VADC_DCIN 0x01 > +#define VADC_VCHG_SNS 0x02 > +#define VADC_SPARE1_03 0x03 > +#define VADC_USB_ID_MV 0x04 > +#define VADC_VCOIN 0x05 > +#define VADC_VBAT_SNS 0x06 > +#define VADC_VSYS 0x07 > +#define VADC_DIE_TEMP 0x08 > +#define VADC_REF_625MV 0x09 > +#define VADC_REF_1250MV 0x0a > +#define VADC_CHG_TEMP 0x0b > +#define VADC_SPARE1 0x0c > +#define VADC_SPARE2 0x0d > +#define VADC_GND_REF 0x0e > +#define VADC_VDD_VADC 0x0f > + > +#define VADC_P_MUX1_1_1 0x10 > +#define VADC_P_MUX2_1_1 0x11 > +#define VADC_P_MUX3_1_1 0x12 > +#define VADC_P_MUX4_1_1 0x13 > +#define VADC_P_MUX5_1_1 0x14 > +#define VADC_P_MUX6_1_1 0x15 > +#define VADC_P_MUX7_1_1 0x16 > +#define VADC_P_MUX8_1_1 0x17 > +#define VADC_P_MUX9_1_1 0x18 > +#define VADC_P_MUX10_1_1 0x19 > +#define VADC_P_MUX11_1_1 0x1a > +#define VADC_P_MUX12_1_1 0x1b > +#define VADC_P_MUX13_1_1 0x1c > +#define VADC_P_MUX14_1_1 0x1d > +#define VADC_P_MUX15_1_1 0x1e > +#define VADC_P_MUX16_1_1 0x1f > + > +#define VADC_P_MUX1_1_3 0x20 > +#define VADC_P_MUX2_1_3 0x21 > +#define VADC_P_MUX3_1_3 0x22 > +#define VADC_P_MUX4_1_3 0x23 > +#define VADC_P_MUX5_1_3 0x24 > +#define VADC_P_MUX6_1_3 0x25 > +#define VADC_P_MUX7_1_3 0x26 > +#define VADC_P_MUX8_1_3 0x27 > +#define VADC_P_MUX9_1_3 0x28 > +#define VADC_P_MUX10_1_3 0x29 > +#define VADC_P_MUX11_1_3 0x2a > +#define VADC_P_MUX12_1_3 0x2b > +#define VADC_P_MUX13_1_3 0x2c > +#define VADC_P_MUX14_1_3 0x2d > +#define VADC_P_MUX15_1_3 0x2e > +#define VADC_P_MUX16_1_3 0x2f > + > +#define VADC_LR_MUX1_BAT_THERM 0x30 > +#define VADC_LR_MUX2_BAT_ID 0x31 > +#define VADC_LR_MUX3_XO_THERM 0x32 > +#define VADC_LR_MUX4_AMUX_THM1 0x33 > +#define VADC_LR_MUX5_AMUX_THM2 0x34 > +#define VADC_LR_MUX6_AMUX_THM3 0x35 > +#define VADC_LR_MUX7_HW_ID 0x36 > +#define VADC_LR_MUX8_AMUX_THM4 0x37 > +#define VADC_LR_MUX9_AMUX_THM5 0x38 > +#define VADC_LR_MUX10_USB_ID 0x39 > +#define VADC_AMUX_PU1 0x3a > +#define VADC_AMUX_PU2 0x3b > +#define VADC_LR_MUX3_BUF_XO_THERM 0x3c > + > +#define VADC_LR_MUX1_PU1_BAT_THERM 0x70 > +#define VADC_LR_MUX2_PU1_BAT_ID 0x71 > +#define VADC_LR_MUX3_PU1_XO_THERM 0x72 > +#define VADC_LR_MUX4_PU1_AMUX_THM1 0x73 > +#define VADC_LR_MUX5_PU1_AMUX_THM2 0x74 > +#define VADC_LR_MUX6_PU1_AMUX_THM3 0x75 > +#define VADC_LR_MUX7_PU1_AMUX_HW_ID 0x76 > +#define VADC_LR_MUX8_PU1_AMUX_THM4 0x77 > +#define VADC_LR_MUX9_PU1_AMUX_THM5 0x78 > +#define VADC_LR_MUX10_PU1_AMUX_USB_ID 0x79 > +#define VADC_LR_MUX3_BUF_PU1_XO_THERM 0x7c > + > +#define VADC_LR_MUX1_PU2_BAT_THERM 0xb0 > +#define VADC_LR_MUX2_PU2_BAT_ID 0xb1 > +#define VADC_LR_MUX3_PU2_XO_THERM 0xb2 > +#define VADC_LR_MUX4_PU2_AMUX_THM1 0xb3 > +#define VADC_LR_MUX5_PU2_AMUX_THM2 0xb4 > +#define VADC_LR_MUX6_PU2_AMUX_THM3 0xb5 > +#define VADC_LR_MUX7_PU2_AMUX_HW_ID 0xb6 > +#define VADC_LR_MUX8_PU2_AMUX_THM4 0xb7 > +#define VADC_LR_MUX9_PU2_AMUX_THM5 0xb8 > +#define VADC_LR_MUX10_PU2_AMUX_USB_ID 0xb9 > +#define VADC_LR_MUX3_BUF_PU2_XO_THERM 0xbc > + > +#define VADC_LR_MUX1_PU1_PU2_BAT_THERM 0xf0 > +#define VADC_LR_MUX2_PU1_PU2_BAT_ID 0xf1 > +#define VADC_LR_MUX3_PU1_PU2_XO_THERM 0xf2 > +#define VADC_LR_MUX4_PU1_PU2_AMUX_THM1 0xf3 > +#define VADC_LR_MUX5_PU1_PU2_AMUX_THM2 0xf4 > +#define VADC_LR_MUX6_PU1_PU2_AMUX_THM3 0xf5 > +#define VADC_LR_MUX7_PU1_PU2_AMUX_HW_ID 0xf6 > +#define VADC_LR_MUX8_PU1_PU2_AMUX_THM4 0xf7 > +#define VADC_LR_MUX9_PU1_PU2_AMUX_THM5 0xf8 > +#define VADC_LR_MUX10_PU1_PU2_AMUX_USB_ID 0xf9 > +#define VADC_LR_MUX3_BUF_PU1_PU2_XO_THERM 0xfc > + > +#endif /* _DT_BINDINGS_QCOM_PMIC_ADC_H */ > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-12 23:27 ` Hartmut Knaack @ 2014-09-13 17:27 ` Jonathan Cameron [not found] ` <54147E97.60808-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [not found] ` <54138151.8010902-Mmb7MZpHnFY@public.gmane.org> 1 sibling, 1 reply; 23+ messages in thread From: Jonathan Cameron @ 2014-09-13 17:27 UTC (permalink / raw) To: Hartmut Knaack, Stanimir Varbanov, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann Cc: linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov On 13/09/14 00:27, Hartmut Knaack wrote: > Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >> 15bits resolution and register space inside PMIC accessible across >> SPMI bus. >> >> The vadc driver registers itself through IIO interface. >> > Looks already pretty good. Things you should consider in regard of common coding style are to use the variable name ret instead of rc, since it is used in almost all adc drivers and thus makes reviewing a bit easier. Besides that, you seem to use unsigned as well as unsigned int, so to be consistent, please stick to one of them. Other comments in line. A few additional comments from me. My biggest question is whether you are actually making life difficult for yourself by having vadc_channels and vadc->channels (don't like the similar naming btw!) in different orders. I think you can move the ordering into the device tree reading code rather than doing it in lots of other places. Hence rather than an order based on the device tree description, put the data into a fixed ofer in vadc->channels. Entirely possible I'm missing something though :) >> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> >> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> >> --- >> drivers/iio/adc/Kconfig | 11 + >> drivers/iio/adc/Makefile | 1 + >> drivers/iio/adc/qcom-spmi-vadc.c | 999 +++++++++++++++++++++++++ >> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >> 4 files changed, 1130 insertions(+), 0 deletions(-) >> create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c >> create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h >> >> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig >> index 11b048a..08700d4 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_VADC >> + tristate "Qualcomm SPMI PMIC voltage ADC" >> + depends on SPMI >> + help >> + Say yes here if you want support for the Qualcomm SPMI PMIC voltage ADC. >> + >> + The driver supports reading the HKADC, XOADC through the ADC AMUX arbiter. >> + The VADC includes support for the conversion sequencer. The driver >> + supports reading the ADC through the AMUX channels for external pull-ups >> + simultaneously. >> + >> 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..d5d18f4 100644 >> --- a/drivers/iio/adc/Makefile >> +++ b/drivers/iio/adc/Makefile >> @@ -22,6 +22,7 @@ obj-$(CONFIG_MCP320X) += mcp320x.o >> obj-$(CONFIG_MCP3422) += mcp3422.o >> obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o >> obj-$(CONFIG_NAU7802) += nau7802.o >> +obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o >> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o >> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o >> obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o >> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c >> new file mode 100644 >> index 0000000..e30eb04 >> --- /dev/null >> +++ b/drivers/iio/adc/qcom-spmi-vadc.c >> @@ -0,0 +1,999 @@ >> +/* >> + * 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/module.h> >> +#include <linux/of.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> +#include <linux/slab.h> >> +#include <linux/log2.h> >> + >> +#include <dt-bindings/iio/qcom,spmi-pmic-vadc.h> >> + >> +/* VADC register and bit definition */ >> +#define VADC_REVISION2 0x1 >> +#define VADC_REVISION2_SUPPORTED_VADC 1 >> + >> +#define VADC_PERPH_TYPE 0x4 >> +#define VADC_PERPH_TYPE_ADC 8 >> + >> +#define VADC_PERPH_SUBTYPE 0x5 >> +#define VADC_PERPH_SUBTYPE_VADC 1 >> + >> +#define VADC_STATUS1 0x8 >> +#define VADC_STATUS1_OP_MODE 4 >> +#define VADC_STATUS1_REQ_STS BIT(1) >> +#define VADC_STATUS1_EOC BIT(0) >> +#define VADC_STATUS1_REQ_STS_EOC_MASK 0x3 >> + >> +#define VADC_MODE_CTL 0x40 >> +#define VADC_OP_MODE_SHIFT 3 >> +#define VADC_OP_MODE_NORMAL 0 >> +#define VADC_AMUX_TRIM_EN BIT(1) >> +#define VADC_ADC_TRIM_EN BIT(0) >> + >> +#define VADC_EN_CTL1 0x46 >> +#define VADC_EN_CTL1_SET BIT(7) >> + >> +#define VADC_ADC_CH_SEL_CTL 0x48 >> + >> +#define VADC_ADC_DIG_PARAM 0x50 >> +#define VADC_ADC_DIG_DEC_RATIO_SEL_SHIFT 2 >> + >> +#define VADC_HW_SETTLE_DELAY 0x51 >> + >> +#define VADC_CONV_REQ 0x52 >> +#define VADC_CONV_REQ_SET BIT(7) >> + >> +#define VADC_FAST_AVG_CTL 0x5a >> +#define VADC_FAST_AVG_EN 0x5b >> +#define VADC_FAST_AVG_EN_SET BIT(7) >> + >> +#define VADC_ACCESS 0xd0 >> +#define VADC_ACCESS_DATA 0xa5 >> + >> +#define VADC_PERH_RESET_CTL3 0xda >> +#define VADC_FOLLOW_WARM_RB BIT(2) >> + >> +#define VADC_DATA0 0x60 >> +#define VADC_DATA1 0x61 >> + >> +#define VADC_CONV_TIME_MIN_US 2000 >> +#define VADC_CONV_TIME_MAX_US 2100 >> + >> +/* Min ADC code represents 0V */ >> +#define VADC_MIN_ADC_CODE 0x6000 >> +/* Max ADC code represents full-scale range of 1.8V */ >> +#define VADC_MAX_ADC_CODE 0xA800 >> + >> +#define VADC_ABSOLUTE_RANGE_UV 625000 >> +#define VADC_RATIOMETRIC_RANGE_UV 1800000 >> + >> +#define VADC_DEF_PRESCALING 0 /* 1:1 */ >> +#define VADC_DEF_DECIMATION 0 /* 512 */ >> +#define VADC_DEF_HW_SETTLE_TIME 0 /* 0 us */ >> +#define VADC_DEF_AVG_SAMPLES 0 /* 1 sample */ >> +#define VADC_DEF_CALIB_TYPE VADC_CALIB_ABSOLUTE >> + >> +#define VADC_DECIMATION_MIN 512 >> +#define VADC_DECIMATION_MAX 4096 >> + >> +#define VADC_HW_SETTLE_DELAY_MAX 10000 >> +#define VADC_AVG_SAMPLES_MAX 512 >> + >> +/* >> + * VADC_CALIB_ABSOLUTE: Uses the 625mV and 1.25V reference channels. >> + * VADC_CALIB_RATIOMETRIC: Uses the reference Voltage/GND for calibration. >> + */ >> +enum vadc_calibration { >> + VADC_CALIB_ABSOLUTE = 0, >> + VADC_CALIB_RATIOMETRIC >> +}; >> + >> +/** >> + * struct vadc_linear_graph - Represent ADC characteristics. >> + * @dy: Numerator slope to calculate the gain. >> + * @dx: Denominator slope to calculate the gain. >> + * @vref: A/D word of the voltage reference used for the channel. >> + * @gnd: A/D word of the ground reference used for the channel. >> + * >> + * Each ADC device has different offset and gain parameters which are >> + * computed to calibrate the device. >> + */ >> +struct vadc_linear_graph { >> + s32 dy; >> + s32 dx; >> + s32 vref; >> + s32 gnd; >> +}; >> + >> +/** >> + * struct vadc_prescaling - Represent scaling ratio for ADC input. >> + * @num: The inverse numerator of the gain applied to the input channel. >> + * @den: The inverse denominator of the gain applied to the input channel. >> + */ >> +struct vadc_prescaling { >> + u32 num; >> + u32 den; >> +}; >> + >> +/** >> + * struct vadc_result - Represent the res of the QPNP ADC. > ... Represents the result... >> + * @adc_code: The pre-calibrated digital output of a given ADC relative to the >> + * the ADC reference. >> + * @measurement: In units specific for a given ADC; most ADC uses reference >> + * voltage but some ADC uses reference current. This measurement >> + * here is a number relative to a reference of a given ADC. >> + * @physical: The data meaningful for each individual channel whether it is >> + * voltage, current, temperature, etc. >> + */ >> +struct vadc_result { >> + s32 adc_code; >> + s64 measurement; >> + s64 physical; >> +}; >> + >> +/** >> + * struct vadc_channel - QPNP VADC amux channel property. >> + * @number - Channel number, refer to the channel list. >> + * @calibration - Calibration type. >> + * @decimation - Sampling rate supported for the channel. >> + * @prescaling - Channel scaling performed on the input signal. >> + * @hw_settle_time - The time between AMUX being configured and the >> + * start of conversion. >> + * @avg_samples - Ability to provide single result from the ADC >> + * that is an average of multiple measurements. >> + */ >> +struct vadc_channel { >> + int number; >> + enum vadc_calibration calibration; >> + unsigned decimation; >> + unsigned prescaling; >> + unsigned hw_settle_time; >> + unsigned avg_samples; >> +}; >> + >> +/** >> + * struct vadc_priv - VADC private structure. >> + * @regmap - pointer to struct regmap >> + * @dev - pointer to struct device. >> + * @base - base address for the ADC peripheral. >> + * @nchannels - number of ADC channels > Missing period at end of line of @regmap and @nchannels. >> + * @channels - AMUX properties representing the ADC peripheral. >> + * @is_ref_measured - does reference points are measured. > This is expressed wrong. >> + * @poll_eoc - do we use polling instead of interrupt. >> + * @complete - ADC result notification after interrupt is received. >> + * @graph - store parameters for calibration. >> + */ >> +struct vadc_priv { >> + struct regmap *regmap; >> + struct device *dev; >> + u16 base; >> + unsigned nchannels; >> + struct vadc_channel *channels; >> + bool is_ref_measured; >> + bool poll_eoc; >> + struct completion complete; >> + struct vadc_linear_graph graph[2]; >> +}; >> + >> +static const struct vadc_prescaling vadc_prescale[] = { >> + {1, 1}, >> + {1, 3}, >> + {1, 4}, >> + {1, 6}, >> + {1, 20}, >> + {1, 8}, >> + {10, 81}, >> + {1, 10} >> +}; >> + >> +static int vadc_read(struct vadc_priv *vadc, u16 offset, u8 *data) >> +{ >> + return regmap_bulk_read(vadc->regmap, vadc->base + offset, data, 1); >> +} >> + >> +static int vadc_write(struct vadc_priv *vadc, u16 offset, u8 data) >> +{ >> + return regmap_write(vadc->regmap, vadc->base + offset, data); >> +} >> + >> +static int vadc_reset(struct vadc_priv *vadc) >> +{ >> + u8 data; >> + int rc; >> + >> + rc = vadc_write(vadc, VADC_ACCESS, VADC_ACCESS_DATA); >> + if (rc) >> + return rc; >> + >> + rc = vadc_read(vadc, VADC_PERH_RESET_CTL3, &data); >> + if (rc) >> + return rc; >> + >> + rc = vadc_write(vadc, VADC_ACCESS, VADC_ACCESS_DATA); >> + if (rc) >> + return rc; >> + >> + data |= VADC_FOLLOW_WARM_RB; >> + >> + return vadc_write(vadc, VADC_PERH_RESET_CTL3, data); >> +} >> + >> +static int vadc_enable(struct vadc_priv *vadc, bool state) >> +{ >> + return vadc_write(vadc, VADC_EN_CTL1, state ? VADC_EN_CTL1_SET : 0); >> +} >> + >> +static int vadc_request_conversion(struct vadc_priv *vadc) >> +{ >> + return vadc_write(vadc, VADC_CONV_REQ, VADC_CONV_REQ_SET); >> +} >> + >> +static void vadc_status_show(struct vadc_priv *vadc) >> +{ >> + u8 mode, sta1, chan, dig, en, req; >> + int rc; >> + >> + rc = vadc_read(vadc, VADC_MODE_CTL, &mode); >> + if (rc) >> + return; >> + >> + rc = vadc_read(vadc, VADC_ADC_DIG_PARAM, &dig); >> + if (rc) >> + return; >> + >> + rc = vadc_read(vadc, VADC_ADC_CH_SEL_CTL, &chan); >> + if (rc) >> + return; >> + >> + rc = vadc_read(vadc, VADC_CONV_REQ, &req); >> + if (rc) >> + return; >> + >> + rc = vadc_read(vadc, VADC_STATUS1, &sta1); >> + if (rc) >> + return; >> + >> + rc = vadc_read(vadc, VADC_EN_CTL1, &en); >> + if (rc) >> + return; >> + >> + dev_dbg(vadc->dev, >> + "mode:%02x en:%02x chan:%02x dig:%02x req:%02x sta1:%02x\n", >> + mode, en, chan, dig, req, sta1); >> +} >> + >> +static int vadc_configure(struct vadc_priv *vadc, struct vadc_channel *vchan) >> +{ >> + u8 decim, mode_ctrl; >> + int rc; >> + >> + /* Mode selection */ >> + mode_ctrl = (VADC_OP_MODE_NORMAL << VADC_OP_MODE_SHIFT) | >> + VADC_ADC_TRIM_EN | VADC_AMUX_TRIM_EN; >> + rc = vadc_write(vadc, VADC_MODE_CTL, mode_ctrl); >> + if (rc) >> + return rc; >> + >> + /* Channel selection */ >> + rc = vadc_write(vadc, VADC_ADC_CH_SEL_CTL, vchan->number); >> + if (rc) >> + return rc; >> + >> + /* Digital parameter setup */ >> + decim = vchan->decimation << VADC_ADC_DIG_DEC_RATIO_SEL_SHIFT; >> + rc = vadc_write(vadc, VADC_ADC_DIG_PARAM, decim); >> + if (rc) >> + return rc; >> + >> + /* HW settle time delay */ >> + rc = vadc_write(vadc, VADC_HW_SETTLE_DELAY, vchan->hw_settle_time); >> + if (rc) >> + return rc; >> + >> + rc = vadc_write(vadc, VADC_FAST_AVG_CTL, vchan->avg_samples); >> + if (rc) >> + return rc; >> + >> + if (vchan->avg_samples) >> + rc = vadc_write(vadc, VADC_FAST_AVG_EN, VADC_FAST_AVG_EN_SET); >> + else >> + rc = vadc_write(vadc, VADC_FAST_AVG_EN, 0); >> + >> + return rc; >> +} >> + >> +static int vadc_poll_wait_eoc(struct vadc_priv *vadc, unsigned int interval_us) >> +{ >> + unsigned int count, retry; >> + u8 sta1; >> + int rc; >> + >> + retry = interval_us / VADC_CONV_TIME_MIN_US; >> + >> + for (count = 0; count < retry; count++) { >> + rc = vadc_read(vadc, VADC_STATUS1, &sta1); >> + if (rc) >> + return rc; >> + >> + sta1 &= VADC_STATUS1_REQ_STS_EOC_MASK; >> + if (sta1 == VADC_STATUS1_EOC) >> + return 0; >> + >> + usleep_range(VADC_CONV_TIME_MIN_US, VADC_CONV_TIME_MAX_US); >> + } >> + >> + vadc_status_show(vadc); >> + >> + return -ETIMEDOUT; >> +} >> + >> +static int vadc_read_result(struct vadc_priv *vadc, u32 *data) >> +{ >> + u8 lsb, msb; >> + int rc; >> + >> + rc = vadc_read(vadc, VADC_DATA0, &lsb); >> + if (rc) >> + return rc; >> + >> + rc = vadc_read(vadc, VADC_DATA1, &msb); > Better have a wrapper function vadc_read_word()? >> + if (rc) >> + return rc; >> + >> + *data = clamp_t(u32, (msb << 8) | lsb, VADC_MIN_ADC_CODE, >> + VADC_MAX_ADC_CODE); > Although Peter suggested to use u32 for *data, I think it would be cleaner to use s32 here. It would be more consistent, as *data is s32 in vadc_do_conversion() as well as read_1 and read_2 in vadc_measure_reference_points(). It seems messy to me to pass a s32 by reference to a function, which expects a reference to an u32. > Any other opinions on this? >> + >> + return 0; >> +} >> + >> +static struct vadc_channel *vadc_find_channel(struct vadc_priv *vadc, int num) >> +{ >> + unsigned i; >> + >> + for (i = 0; i < vadc->nchannels; i++) >> + if (vadc->channels[i].number == num) >> + return &vadc->channels[num]; >> + >> + dev_dbg(vadc->dev, "no such channel %02x\n", num); >> + >> + return NULL; >> +} >> + >> +static int vadc_do_conversion(struct vadc_priv *vadc, >> + struct vadc_channel *vchan, s32 *data) >> +{ >> + unsigned int timeout; >> + int rc; >> + >> + rc = vadc_configure(vadc, vchan); >> + if (rc) >> + return rc; >> + >> + if (!vadc->poll_eoc) >> + reinit_completion(&vadc->complete); >> + >> + rc = vadc_enable(vadc, true); >> + if (rc) >> + return rc; >> + >> + rc = vadc_request_conversion(vadc); >> + if (rc) >> + goto err_disable; >> + >> + timeout = BIT(vchan->avg_samples) * VADC_CONV_TIME_MIN_US * 2; >> + >> + if (vadc->poll_eoc) { >> + rc = vadc_poll_wait_eoc(vadc, timeout); >> + } else { >> + rc = wait_for_completion_timeout(&vadc->complete, >> + msecs_to_jiffies(1000)); >> + if (!rc) { >> + rc = -ETIMEDOUT; >> + goto err_disable; >> + } >> + >> + /* double check conversion status */ >> + rc = vadc_poll_wait_eoc(vadc, VADC_CONV_TIME_MIN_US * 2); >> + if (rc) >> + goto err_disable; >> + } >> + >> + rc = vadc_read_result(vadc, data); >> + >> +err_disable: >> + vadc_enable(vadc, false); >> + >> + if (rc) >> + dev_dbg(vadc->dev, "conversion failed\n"); >> + >> + return rc; >> +} >> + >> +static int vadc_measure_reference_points(struct vadc_priv *vadc) >> +{ >> + struct vadc_channel *vchan; >> + s32 read_1, read_2; >> + int rc = -EINVAL; >> + >> + vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV; >> + vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV; >> + >> + vchan = vadc_find_channel(vadc, VADC_REF_1250MV); >> + if (!vchan) >> + goto err; >> + >> + rc = vadc_do_conversion(vadc, vchan, &read_1); >> + if (rc) >> + goto err; >> + >> + /* Try with buffered 625mV channel first */ >> + vchan = vadc_find_channel(vadc, VADC_SPARE1); >> + if (!vchan) { >> + vchan = vadc_find_channel(vadc, VADC_REF_625MV); >> + if (!vchan) { >> + rc = -EINVAL; >> + goto err; >> + } >> + } >> + >> + rc = vadc_do_conversion(vadc, vchan, &read_2); >> + if (rc) >> + goto err; >> + >> + if (read_1 == read_2) { >> + rc = -EINVAL; >> + goto err; >> + } >> + >> + vadc->graph[VADC_CALIB_ABSOLUTE].dy = read_1 - read_2; >> + vadc->graph[VADC_CALIB_ABSOLUTE].vref = read_1; >> + vadc->graph[VADC_CALIB_ABSOLUTE].gnd = read_2; >> + >> + /* Ratiometric calibration */ >> + vchan = vadc_find_channel(vadc, VADC_VDD_VADC); >> + if (!vchan) { >> + rc = -EINVAL; >> + goto err; >> + } >> + >> + rc = vadc_do_conversion(vadc, vchan, &read_1); >> + if (rc) >> + goto err; >> + >> + vchan = vadc_find_channel(vadc, VADC_GND_REF); >> + if (!vchan) { >> + rc = -EINVAL; >> + goto err; >> + } >> + >> + rc = vadc_do_conversion(vadc, vchan, &read_2); >> + if (rc) >> + goto err; >> + >> + if (read_1 == read_2) { >> + rc = -EINVAL; >> + goto err; >> + } >> + >> + vadc->graph[VADC_CALIB_RATIOMETRIC].dy = read_1 - read_2; >> + vadc->graph[VADC_CALIB_RATIOMETRIC].vref = read_1; >> + vadc->graph[VADC_CALIB_RATIOMETRIC].gnd = read_2; >> +err: >> + if (rc) >> + dev_dbg(vadc->dev, "measure reference points failed\n"); >> + >> + return rc; >> +} >> + >> +static void vadc_calibrate(struct vadc_priv *vadc, >> + const struct vadc_channel *vchan, >> + struct vadc_result *res) >> +{ >> + const struct vadc_prescaling *prescale; >> + bool negative = false; >> + s64 voltage; >> + >> + voltage = res->adc_code - vadc->graph[vchan->calibration].gnd; >> + voltage *= vadc->graph[vchan->calibration].dx; >> + >> + if (voltage < 0) { >> + negative = true; >> + voltage = -voltage; >> + } >> + >> + do_div(voltage, vadc->graph[vchan->calibration].dy); >> + if (negative) >> + voltage = -voltage; >> + >> + if (vchan->calibration == VADC_CALIB_ABSOLUTE) >> + voltage += vadc->graph[vchan->calibration].dx; >> + >> + if (voltage < 0) >> + voltage = 0; >> + >> + prescale = &vadc_prescale[vchan->prescaling]; >> + >> + res->measurement = voltage * prescale->den; >> + >> + do_div(res->measurement, prescale->num); >> + >> + res->physical = res->measurement; >> +} >> + >> +static unsigned vadc_decimation_from_dt(unsigned int value) > static int vadc_decimation_from_dt >> +{ >> + if (!is_power_of_2(value) || value < VADC_DECIMATION_MIN || >> + value > VADC_DECIMATION_MAX) >> + return -EINVAL; >> + >> + return __ffs64(value / VADC_DECIMATION_MIN); >> +} >> + >> +static int vadc_prescaling_from_dt(unsigned int num, unsigned int den) > Better use u32 num, u32 den? >> +{ >> + unsigned int pre; >> + >> + for (pre = 0; pre < ARRAY_SIZE(vadc_prescale); pre++) >> + if (vadc_prescale[pre].num == num && >> + vadc_prescale[pre].den == den) >> + break; >> + >> + if (pre == ARRAY_SIZE(vadc_prescale)) >> + return -EINVAL; >> + >> + return pre; >> +} >> + >> +static int vadc_hw_settle_time_from_dt(unsigned int value) > >> +{ >> + if ((value <= 1000 && value % 100) || (value > 1000 && value % 2000)) >> + return -EINVAL; >> + >> + if (value <= 1000) >> + value /= 100; >> + else >> + value = value / 2000 + 10; >> + >> + return value; >> +} >> + >> +static int vadc_avg_samples_from_dt(unsigned int value) > Better use u32 value? >> +{ >> + if (!is_power_of_2(value) || value > VADC_AVG_SAMPLES_MAX) >> + return -EINVAL; >> + >> + return __ffs64(value); >> +} >> + >> +static int > Don't wrap line here. >> +vadc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, >> + int *val, int *val2, long mask) >> +{ >> + struct vadc_priv *vadc = iio_priv(indio_dev); >> + struct vadc_channel *vchan; >> + struct vadc_result result; >> + int rc; >> + It is a bit of a pitty we can't avoid this lookup. Normally I'd suggest putting an index in chan->address but you've already used that. The purpose of this is to (I think) allow you to have the private data stored in a random order... What is the benefit of doing that? (see various comments elsewhere) >> + vchan = vadc_find_channel(vadc, chan->channel); >> + if (!vchan) >> + return -EINVAL; >> + >> + if (!vadc->is_ref_measured) { >> + rc = vadc_measure_reference_points(vadc); >> + if (rc) >> + return rc; >> + >> + vadc->is_ref_measured = true; >> + } >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_PROCESSED: >> + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); >> + if (rc) >> + return rc; >> + >> + vadc_calibrate(vadc, vchan, &result); >> + >> + *val = result.physical; I'm a little suspicious here. Are the resulting values in milivolts for all the channels? Very handy if so, but seems a little unlikely with 15 bit ADC that you'd have no part of greater accuracy than a milivolt. >> + rc = IIO_VAL_INT; > return IIO_VAL_INT; >> + break; >> + default: >> + rc = -EINVAL; >> + break; > Drop default case, or leave empty. >> + } >> + >> + return rc; > return -EINVAL; >> +} >> + >> +static const struct iio_info vadc_info = { >> + .read_raw = vadc_read_raw, >> + .driver_module = THIS_MODULE, >> +}; >> + >> +#define VADC_CHAN(_id, _pre) \ >> + [VADC_##_id] = { \ >> + .type = IIO_VOLTAGE, \ A few of the below look to be temp sensors. If they are hardwired in some way to this functionality (i.e. is it on chip) then it might be nice to reflect this in the channel type. >> + .indexed = 1, \ >> + .channel = VADC_##_id, \ >> + .address = _pre, \ >> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ >> + .datasheet_name = __stringify(VADC_##_id), \ >> + .scan_type = { \ >> + .sign = 's', \ >> + .realbits = 15, \ >> + .storagebits = 16, \ >> + .endianness = IIO_CPU, \ >> + }, \ >> + }, >> + >> +static const struct iio_chan_spec vadc_channels[] = { >> + VADC_CHAN(USBIN, 4) /* 0x00 */ >> + VADC_CHAN(DCIN, 4) >> + VADC_CHAN(VCHG_SNS, 3) >> + VADC_CHAN(SPARE1_03, 1) >> + VADC_CHAN(USB_ID_MV, 1) >> + VADC_CHAN(VCOIN, 1) >> + VADC_CHAN(VBAT_SNS, 1) >> + VADC_CHAN(VSYS, 1) >> + VADC_CHAN(DIE_TEMP, 0) >> + VADC_CHAN(REF_625MV, 0) >> + VADC_CHAN(REF_1250MV, 0) >> + VADC_CHAN(CHG_TEMP, 0) >> + VADC_CHAN(SPARE1, 0) >> + VADC_CHAN(SPARE2, 0) >> + VADC_CHAN(GND_REF, 0) >> + VADC_CHAN(VDD_VADC, 0) /* 0x0f */ >> + >> + VADC_CHAN(P_MUX1_1_1, 0) /* 0x10 */ >> + VADC_CHAN(P_MUX2_1_1, 0) >> + VADC_CHAN(P_MUX3_1_1, 0) >> + VADC_CHAN(P_MUX4_1_1, 0) >> + VADC_CHAN(P_MUX5_1_1, 0) >> + VADC_CHAN(P_MUX6_1_1, 0) >> + VADC_CHAN(P_MUX7_1_1, 0) >> + VADC_CHAN(P_MUX8_1_1, 0) >> + VADC_CHAN(P_MUX9_1_1, 0) >> + VADC_CHAN(P_MUX10_1_1, 0) >> + VADC_CHAN(P_MUX11_1_1, 0) >> + VADC_CHAN(P_MUX12_1_1, 0) >> + VADC_CHAN(P_MUX13_1_1, 0) >> + VADC_CHAN(P_MUX14_1_1, 0) >> + VADC_CHAN(P_MUX15_1_1, 0) >> + VADC_CHAN(P_MUX16_1_1, 0) /* 0x1f */ >> + >> + VADC_CHAN(P_MUX1_1_3, 1) /* 0x20 */ >> + VADC_CHAN(P_MUX2_1_3, 1) >> + VADC_CHAN(P_MUX3_1_3, 1) >> + VADC_CHAN(P_MUX4_1_3, 1) >> + VADC_CHAN(P_MUX5_1_3, 1) >> + VADC_CHAN(P_MUX6_1_3, 1) >> + VADC_CHAN(P_MUX7_1_3, 1) >> + VADC_CHAN(P_MUX8_1_3, 1) >> + VADC_CHAN(P_MUX9_1_3, 1) >> + VADC_CHAN(P_MUX10_1_3, 1) >> + VADC_CHAN(P_MUX11_1_3, 1) >> + VADC_CHAN(P_MUX12_1_3, 1) >> + VADC_CHAN(P_MUX13_1_3, 1) >> + VADC_CHAN(P_MUX14_1_3, 1) >> + VADC_CHAN(P_MUX15_1_3, 1) >> + VADC_CHAN(P_MUX16_1_3, 1) /* 0x2f */ >> + >> + VADC_CHAN(LR_MUX1_BAT_THERM, 0) /* 0x30 */ >> + VADC_CHAN(LR_MUX2_BAT_ID, 0) >> + VADC_CHAN(LR_MUX3_XO_THERM, 0) >> + VADC_CHAN(LR_MUX4_AMUX_THM1, 0) >> + VADC_CHAN(LR_MUX5_AMUX_THM2, 0) >> + VADC_CHAN(LR_MUX6_AMUX_THM3, 0) >> + VADC_CHAN(LR_MUX7_HW_ID, 0) >> + VADC_CHAN(LR_MUX8_AMUX_THM4, 0) >> + VADC_CHAN(LR_MUX9_AMUX_THM5, 0) >> + VADC_CHAN(LR_MUX10_USB_ID, 0) >> + VADC_CHAN(AMUX_PU1, 0) >> + VADC_CHAN(AMUX_PU2, 0) >> + VADC_CHAN(LR_MUX3_BUF_XO_THERM, 0) /* 0x3c */ >> + >> + VADC_CHAN(LR_MUX1_PU1_BAT_THERM, 0) /* 0x70 */ >> + VADC_CHAN(LR_MUX2_PU1_BAT_ID, 0) >> + VADC_CHAN(LR_MUX3_PU1_XO_THERM, 0) >> + VADC_CHAN(LR_MUX4_PU1_AMUX_THM1, 0) >> + VADC_CHAN(LR_MUX5_PU1_AMUX_THM2, 0) >> + VADC_CHAN(LR_MUX6_PU1_AMUX_THM3, 0) >> + VADC_CHAN(LR_MUX7_PU1_AMUX_HW_ID, 0) >> + VADC_CHAN(LR_MUX8_PU1_AMUX_THM4, 0) >> + VADC_CHAN(LR_MUX9_PU1_AMUX_THM5, 0) >> + VADC_CHAN(LR_MUX10_PU1_AMUX_USB_ID, 0) /* 0x79 */ >> + VADC_CHAN(LR_MUX3_BUF_PU1_XO_THERM, 0) /* 0x7c */ >> + >> + VADC_CHAN(LR_MUX1_PU2_BAT_THERM, 0) /* 0xb0 */ >> + VADC_CHAN(LR_MUX2_PU2_BAT_ID, 0) >> + VADC_CHAN(LR_MUX3_PU2_XO_THERM, 0) >> + VADC_CHAN(LR_MUX4_PU2_AMUX_THM1, 0) >> + VADC_CHAN(LR_MUX5_PU2_AMUX_THM2, 0) >> + VADC_CHAN(LR_MUX6_PU2_AMUX_THM3, 0) >> + VADC_CHAN(LR_MUX7_PU2_AMUX_HW_ID, 0) >> + VADC_CHAN(LR_MUX8_PU2_AMUX_THM4, 0) >> + VADC_CHAN(LR_MUX9_PU2_AMUX_THM5, 0) >> + VADC_CHAN(LR_MUX10_PU2_AMUX_USB_ID, 0) /* 0xb9 */ >> + VADC_CHAN(LR_MUX3_BUF_PU2_XO_THERM, 0) /* 0xbc */ >> + >> + VADC_CHAN(LR_MUX1_PU1_PU2_BAT_THERM, 0) /* 0xf0 */ >> + VADC_CHAN(LR_MUX2_PU1_PU2_BAT_ID, 0) >> + VADC_CHAN(LR_MUX3_PU1_PU2_XO_THERM, 0) >> + VADC_CHAN(LR_MUX4_PU1_PU2_AMUX_THM1, 0) >> + VADC_CHAN(LR_MUX5_PU1_PU2_AMUX_THM2, 0) >> + VADC_CHAN(LR_MUX6_PU1_PU2_AMUX_THM3, 0) >> + VADC_CHAN(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0) >> + VADC_CHAN(LR_MUX8_PU1_PU2_AMUX_THM4, 0) >> + VADC_CHAN(LR_MUX9_PU1_PU2_AMUX_THM5, 0) >> + VADC_CHAN(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0) /* 0xf9 */ >> + VADC_CHAN(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0) /* 0xfc */ >> +}; >> + >> +static int >> +vadc_get_dt_channel_data(struct vadc_priv *vadc, struct device_node *node) >> +{ >> + struct vadc_channel *vchan; >> + u32 channel, value, varr[2]; >> + int rc, pre, time, avg, decim; > Drop pre, time, avg and decim and reuse rc instead? >> + const char *name = node->name; >> + >> + rc = of_property_read_u32(node, "reg", &channel); >> + if (rc) { >> + dev_dbg(vadc->dev, "invalid channel number %s\n", name); >> + return -EINVAL; >> + } >> + >> + if (channel >= vadc->nchannels) { >> + dev_dbg(vadc->dev, "%s invalid channel number %d\n", name, >> + channel); >> + return -EINVAL; >> + } >> + >> + vchan = &vadc->channels[channel]; Could you not have these in the same order as the iio_chan_spec array? Hence move the lookups that are scattered elsewhere in the driver to here? >> + >> + /* the channel is active */ >> + vchan->number = channel; >> + >> + rc = of_property_read_u32(node, "qcom,decimation", &value); >> + if (!rc) { >> + decim = vadc_decimation_from_dt(value); >> + if (decim < 0) { >> + dev_dbg(vadc->dev, "%02x invalid decimation %d\n", >> + channel, value); >> + return -EINVAL; > return decim; >> + } >> + vchan->decimation = decim; >> + } >> + >> + rc = of_property_read_u32_array(node, "qcom,pre-scaling", varr, 2); >> + if (!rc) { >> + pre = vadc_prescaling_from_dt(varr[0], varr[1]); >> + if (pre < 0) { >> + dev_dbg(vadc->dev, >> + "%02x invalid pre-scaling <%d %d>\n", >> + channel, varr[0], varr[1]); >> + return -EINVAL; > return pre; >> + } >> + vchan->prescaling = pre; >> + } >> + >> + rc = of_property_read_u32(node, "qcom,hw-settle-time", &value); >> + if (!rc) { >> + time = vadc_hw_settle_time_from_dt(value); >> + if (time < 0) { >> + dev_dbg(vadc->dev, >> + "%02x invalid hw-settle-time %d, us\n", >> + channel, value); >> + return -EINVAL; > return time; >> + } >> + vchan->hw_settle_time = time; >> + } >> + >> + rc = of_property_read_u32(node, "qcom,avg-samples", &value); >> + if (!rc) { >> + avg = vadc_avg_samples_from_dt(value); >> + if (avg < 0) { >> + dev_dbg(vadc->dev, "%02x invalid avg-samples %d\n", >> + channel, value); >> + return -EINVAL; > return avg; >> + } >> + vchan->avg_samples = avg; >> + } >> + >> + if (of_property_read_bool(node, "qcom,ratiometric")) >> + vchan->calibration = VADC_CALIB_RATIOMETRIC; >> + else >> + vchan->calibration = VADC_CALIB_ABSOLUTE; > Or simply vchan->calibration = of_property_read_bool(node, "qcom,ratiometric")? >> + >> + dev_dbg(vadc->dev, "%02x name %s\n", channel, name); >> + >> + return 0; >> +} >> + >> +static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node) >> +{ >> + struct device_node *child; >> + int rc; >> + >> + vadc->poll_eoc = of_property_read_bool(node, "qcom,poll-eoc"); >> + >> + for_each_available_child_of_node(node, child) { >> + rc = vadc_get_dt_channel_data(vadc, child); >> + if (rc < 0) >> + return rc; >> + } >> + >> + return 0; >> +} >> + >> +static irqreturn_t vadc_isr(int irq, void *dev_id) >> +{ >> + struct vadc_priv *vadc = dev_id; >> + >> + complete(&vadc->complete); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static int vadc_version_check(struct vadc_priv *vadc) >> +{ >> + u8 revision, type, subtype; >> + int rc; >> + >> + rc = vadc_read(vadc, VADC_PERPH_TYPE, &type); >> + if (rc < 0) >> + return rc; >> + >> + if (type < VADC_PERPH_TYPE_ADC) { >> + dev_dbg(vadc->dev, "%d is not ADC\n", type); >> + return -EINVAL; >> + } >> + >> + rc = vadc_read(vadc, VADC_PERPH_SUBTYPE, &subtype); >> + if (rc < 0) >> + return rc; >> + >> + if (subtype < VADC_PERPH_SUBTYPE_VADC) { >> + dev_dbg(vadc->dev, "%d is not VADC\n", subtype); >> + return -EINVAL; >> + } >> + >> + rc = vadc_read(vadc, VADC_REVISION2, &revision); >> + if (rc < 0) >> + return rc; >> + >> + if (revision < VADC_REVISION2_SUPPORTED_VADC) { >> + dev_dbg(vadc->dev, "revision %d not supported\n", revision); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static int vadc_probe(struct platform_device *pdev) >> +{ >> + struct device_node *node = pdev->dev.of_node; >> + struct device *dev = &pdev->dev; >> + struct iio_dev *indio_dev; >> + struct vadc_channel *vchan; >> + struct vadc_priv *vadc; >> + struct resource *res; >> + struct regmap *regmap; >> + int rc, irq_eoc, n; > unsigned int n? >> + >> + regmap = dev_get_regmap(dev->parent, NULL); >> + if (!regmap) >> + return -ENODEV; >> + >> + indio_dev = devm_iio_device_alloc(dev, sizeof(*vadc)); >> + if (!indio_dev) >> + return -ENOMEM; >> + >> + vadc = iio_priv(indio_dev); >> + vadc->dev = dev; >> + vadc->regmap = regmap; >> + vadc->is_ref_measured = false; >> + init_completion(&vadc->complete); >> + >> + vadc->nchannels = ARRAY_SIZE(vadc_channels); >> + vadc->channels = devm_kcalloc(dev, vadc->nchannels, >> + sizeof(*vadc->channels), GFP_KERNEL); Interesting. This is always the same size as the vadc_channels so you might as well keep them in the same order and simplify various corners of the code. >> + if (!vadc->channels) >> + return -ENOMEM; >> + I wonder if we can't vadc->channels rather more different from vadc_channels? Perhaps vadc->channelspriv or similar? Confused me a little at first. >> + for (n = 0; n < vadc->nchannels; n++) { >> + vchan = &vadc->channels[n]; >> + /* set default channel properties */ >> + vchan->number = -1; /* inactive */ >> + vchan->prescaling = vadc_channels[n].address; >> + vchan->decimation = VADC_DEF_DECIMATION; >> + vchan->hw_settle_time = VADC_DEF_HW_SETTLE_TIME; >> + vchan->avg_samples = VADC_DEF_AVG_SAMPLES; >> + vchan->calibration = VADC_DEF_CALIB_TYPE; >> + } >> + >> + platform_set_drvdata(pdev, vadc); >> + >> + res = platform_get_resource(pdev, IORESOURCE_REG, 0); >> + if (!res) >> + return -ENODEV; >> + >> + vadc->base = res->start; >> + >> + rc = vadc_version_check(vadc); >> + if (rc < 0) >> + return -ENODEV; >> + >> + rc = vadc_get_dt_data(vadc, node); >> + if (rc < 0) >> + return rc; >> + Do we need an explicit flag to indicate poll mode rather than just using the absense of the irq being specified to select that mode? I guess the explicit flag makes it harder to get things wrong. >> + if (!vadc->poll_eoc) { >> + irq_eoc = platform_get_irq(pdev, 0); >> + if (irq_eoc < 0) >> + return irq_eoc; >> + >> + rc = devm_request_irq(dev, irq_eoc, vadc_isr, 0, >> + "spmi-vadc", vadc); >> + if (!rc) >> + enable_irq_wake(irq_eoc); >> + else >> + return rc; >> + } else { >> + device_init_wakeup(vadc->dev, true); >> + } >> + >> + rc = vadc_reset(vadc); >> + if (rc < 0) { >> + dev_dbg(dev, "reset failed\n"); >> + return rc; >> + } >> + >> + 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 = &vadc_info; >> + indio_dev->channels = vadc_channels; >> + indio_dev->num_channels = ARRAY_SIZE(vadc_channels); >> + >> + return devm_iio_device_register(dev, indio_dev); >> +} >> + >> +static const struct of_device_id vadc_match_table[] = { >> + { .compatible = "qcom,spmi-vadc" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, vadc_match_table); >> + >> +static struct platform_driver vadc_driver = { >> + .driver = { >> + .name = "spmi-vadc", >> + .of_match_table = vadc_match_table, >> + }, >> + .probe = vadc_probe, >> +}; >> +module_platform_driver(vadc_driver); >> + >> +MODULE_ALIAS("platform:spmi-vadc"); >> +MODULE_DESCRIPTION("Qualcomm SPMI PMIC voltage ADC driver"); >> +MODULE_LICENSE("GPL v2"); >> diff --git a/include/dt-bindings/iio/qcom,spmi-pmic-vadc.h b/include/dt-bindings/iio/qcom,spmi-pmic-vadc.h >> new file mode 100644 >> index 0000000..d543f24 >> --- /dev/null >> +++ b/include/dt-bindings/iio/qcom,spmi-pmic-vadc.h >> @@ -0,0 +1,119 @@ >> +/* >> + * 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. >> + */ >> + >> +#ifndef _DT_BINDINGS_QCOM_PMIC_ADC_H >> +#define _DT_BINDINGS_QCOM_PMIC_ADC_H >> + >> +/* Voltage ADC channels */ >> +#define VADC_USBIN 0x00 >> +#define VADC_DCIN 0x01 >> +#define VADC_VCHG_SNS 0x02 >> +#define VADC_SPARE1_03 0x03 >> +#define VADC_USB_ID_MV 0x04 >> +#define VADC_VCOIN 0x05 >> +#define VADC_VBAT_SNS 0x06 >> +#define VADC_VSYS 0x07 >> +#define VADC_DIE_TEMP 0x08 >> +#define VADC_REF_625MV 0x09 >> +#define VADC_REF_1250MV 0x0a >> +#define VADC_CHG_TEMP 0x0b >> +#define VADC_SPARE1 0x0c >> +#define VADC_SPARE2 0x0d >> +#define VADC_GND_REF 0x0e >> +#define VADC_VDD_VADC 0x0f >> + >> +#define VADC_P_MUX1_1_1 0x10 >> +#define VADC_P_MUX2_1_1 0x11 >> +#define VADC_P_MUX3_1_1 0x12 >> +#define VADC_P_MUX4_1_1 0x13 >> +#define VADC_P_MUX5_1_1 0x14 >> +#define VADC_P_MUX6_1_1 0x15 >> +#define VADC_P_MUX7_1_1 0x16 >> +#define VADC_P_MUX8_1_1 0x17 >> +#define VADC_P_MUX9_1_1 0x18 >> +#define VADC_P_MUX10_1_1 0x19 >> +#define VADC_P_MUX11_1_1 0x1a >> +#define VADC_P_MUX12_1_1 0x1b >> +#define VADC_P_MUX13_1_1 0x1c >> +#define VADC_P_MUX14_1_1 0x1d >> +#define VADC_P_MUX15_1_1 0x1e >> +#define VADC_P_MUX16_1_1 0x1f >> + >> +#define VADC_P_MUX1_1_3 0x20 >> +#define VADC_P_MUX2_1_3 0x21 >> +#define VADC_P_MUX3_1_3 0x22 >> +#define VADC_P_MUX4_1_3 0x23 >> +#define VADC_P_MUX5_1_3 0x24 >> +#define VADC_P_MUX6_1_3 0x25 >> +#define VADC_P_MUX7_1_3 0x26 >> +#define VADC_P_MUX8_1_3 0x27 >> +#define VADC_P_MUX9_1_3 0x28 >> +#define VADC_P_MUX10_1_3 0x29 >> +#define VADC_P_MUX11_1_3 0x2a >> +#define VADC_P_MUX12_1_3 0x2b >> +#define VADC_P_MUX13_1_3 0x2c >> +#define VADC_P_MUX14_1_3 0x2d >> +#define VADC_P_MUX15_1_3 0x2e >> +#define VADC_P_MUX16_1_3 0x2f >> + >> +#define VADC_LR_MUX1_BAT_THERM 0x30 >> +#define VADC_LR_MUX2_BAT_ID 0x31 >> +#define VADC_LR_MUX3_XO_THERM 0x32 >> +#define VADC_LR_MUX4_AMUX_THM1 0x33 >> +#define VADC_LR_MUX5_AMUX_THM2 0x34 >> +#define VADC_LR_MUX6_AMUX_THM3 0x35 >> +#define VADC_LR_MUX7_HW_ID 0x36 >> +#define VADC_LR_MUX8_AMUX_THM4 0x37 >> +#define VADC_LR_MUX9_AMUX_THM5 0x38 >> +#define VADC_LR_MUX10_USB_ID 0x39 >> +#define VADC_AMUX_PU1 0x3a >> +#define VADC_AMUX_PU2 0x3b >> +#define VADC_LR_MUX3_BUF_XO_THERM 0x3c >> + >> +#define VADC_LR_MUX1_PU1_BAT_THERM 0x70 >> +#define VADC_LR_MUX2_PU1_BAT_ID 0x71 >> +#define VADC_LR_MUX3_PU1_XO_THERM 0x72 >> +#define VADC_LR_MUX4_PU1_AMUX_THM1 0x73 >> +#define VADC_LR_MUX5_PU1_AMUX_THM2 0x74 >> +#define VADC_LR_MUX6_PU1_AMUX_THM3 0x75 >> +#define VADC_LR_MUX7_PU1_AMUX_HW_ID 0x76 >> +#define VADC_LR_MUX8_PU1_AMUX_THM4 0x77 >> +#define VADC_LR_MUX9_PU1_AMUX_THM5 0x78 >> +#define VADC_LR_MUX10_PU1_AMUX_USB_ID 0x79 >> +#define VADC_LR_MUX3_BUF_PU1_XO_THERM 0x7c >> + >> +#define VADC_LR_MUX1_PU2_BAT_THERM 0xb0 >> +#define VADC_LR_MUX2_PU2_BAT_ID 0xb1 >> +#define VADC_LR_MUX3_PU2_XO_THERM 0xb2 >> +#define VADC_LR_MUX4_PU2_AMUX_THM1 0xb3 >> +#define VADC_LR_MUX5_PU2_AMUX_THM2 0xb4 >> +#define VADC_LR_MUX6_PU2_AMUX_THM3 0xb5 >> +#define VADC_LR_MUX7_PU2_AMUX_HW_ID 0xb6 >> +#define VADC_LR_MUX8_PU2_AMUX_THM4 0xb7 >> +#define VADC_LR_MUX9_PU2_AMUX_THM5 0xb8 >> +#define VADC_LR_MUX10_PU2_AMUX_USB_ID 0xb9 >> +#define VADC_LR_MUX3_BUF_PU2_XO_THERM 0xbc >> + >> +#define VADC_LR_MUX1_PU1_PU2_BAT_THERM 0xf0 >> +#define VADC_LR_MUX2_PU1_PU2_BAT_ID 0xf1 >> +#define VADC_LR_MUX3_PU1_PU2_XO_THERM 0xf2 >> +#define VADC_LR_MUX4_PU1_PU2_AMUX_THM1 0xf3 >> +#define VADC_LR_MUX5_PU1_PU2_AMUX_THM2 0xf4 >> +#define VADC_LR_MUX6_PU1_PU2_AMUX_THM3 0xf5 >> +#define VADC_LR_MUX7_PU1_PU2_AMUX_HW_ID 0xf6 >> +#define VADC_LR_MUX8_PU1_PU2_AMUX_THM4 0xf7 >> +#define VADC_LR_MUX9_PU1_PU2_AMUX_THM5 0xf8 >> +#define VADC_LR_MUX10_PU1_PU2_AMUX_USB_ID 0xf9 >> +#define VADC_LR_MUX3_BUF_PU1_PU2_XO_THERM 0xfc >> + >> +#endif /* _DT_BINDINGS_QCOM_PMIC_ADC_H */ >> > > -- > 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 > ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <54147E97.60808-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-13 17:27 ` Jonathan Cameron @ 2014-09-15 14:12 ` Stanimir Varbanov 0 siblings, 0 replies; 23+ messages in thread From: Stanimir Varbanov @ 2014-09-15 14:12 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins Hi Jonathan, Thanks for the review! On 09/13/2014 08:27 PM, Jonathan Cameron wrote: > On 13/09/14 00:27, Hartmut Knaack wrote: >> Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >>> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >>> 15bits resolution and register space inside PMIC accessible across >>> SPMI bus. >>> >>> The vadc driver registers itself through IIO interface. >>> >> Looks already pretty good. Things you should consider in regard of common coding style are to use the variable name ret instead of rc, since it is used in almost all adc drivers and thus makes reviewing a bit easier. Besides that, you seem to use unsigned as well as unsigned int, so to be consistent, please stick to one of them. Other comments in line. > > A few additional comments from me. My biggest question is whether > you are actually making life difficult for yourself by having > vadc_channels and vadc->channels (don't like the similar naming btw!) > in different orders. I think you can move the ordering into the device > tree reading code rather than doing it in lots of other places. Hence > rather than an order based on the device tree description, put the > data into a fixed ofer in vadc->channels. > > Entirely possible I'm missing something though :) >>> Signed-off-by: Stanimir Varbanov <svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >>> Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >>> --- >>> drivers/iio/adc/Kconfig | 11 + >>> drivers/iio/adc/Makefile | 1 + >>> drivers/iio/adc/qcom-spmi-vadc.c | 999 +++++++++++++++++++++++++ >>> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >>> 4 files changed, 1130 insertions(+), 0 deletions(-) >>> create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c >>> create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h <snip> >>> + >>> +static int >> Don't wrap line here. >>> +vadc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, >>> + int *val, int *val2, long mask) >>> +{ >>> + struct vadc_priv *vadc = iio_priv(indio_dev); >>> + struct vadc_channel *vchan; >>> + struct vadc_result result; >>> + int rc; >>> + > It is a bit of a pitty we can't avoid this lookup. Normally I'd suggest > putting an index in chan->address but you've already used that. > The purpose of this is to (I think) allow you to have the private > data stored in a random order... What is the benefit of doing that? > (see various comments elsewhere) So the vadc_channels array describe all possible vadc channels for all supported PMICs from this driver. On the other side vadc->channels pointer should contain only the channels described in DT. Thus we need a below function to check is the current channel is active for the current DT (current PMIC version). This is because we have in vadc_channels the full set of channels but not every supported PMIC have support for them. I agree that this peace of code is not so clear. So I will try to rework this and register to the IIO core only those channels that have channel descriptions in DT. Also I wonder can I use iio_chan_spec::address field as a pointer to private structure with vadc channel properties like decimation, prescale etc. got from DT or the default values. > >>> + vchan = vadc_find_channel(vadc, chan->channel); >>> + if (!vchan) >>> + return -EINVAL; >>> + >>> + if (!vadc->is_ref_measured) { >>> + rc = vadc_measure_reference_points(vadc); >>> + if (rc) >>> + return rc; >>> + >>> + vadc->is_ref_measured = true; >>> + } >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_PROCESSED: >>> + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); >>> + if (rc) >>> + return rc; >>> + >>> + vadc_calibrate(vadc, vchan, &result); >>> + >>> + *val = result.physical; > I'm a little suspicious here. Are the resulting values in milivolts for > all the channels? Very handy if so, but seems a little unlikely with 15 bit > ADC that you'd have no part of greater accuracy than a milivolt. In fact *val is in microvolts. What is the expected unit from IIO ADC users? >>> + rc = IIO_VAL_INT; >> return IIO_VAL_INT; >>> + break; >>> + default: >>> + rc = -EINVAL; >>> + break; >> Drop default case, or leave empty. >>> + } >>> + >>> + return rc; >> return -EINVAL; >>> +} >>> + >>> +static const struct iio_info vadc_info = { >>> + .read_raw = vadc_read_raw, >>> + .driver_module = THIS_MODULE, >>> +}; >>> + >>> +#define VADC_CHAN(_id, _pre) \ >>> + [VADC_##_id] = { \ >>> + .type = IIO_VOLTAGE, \ > A few of the below look to be temp sensors. If they are hardwired > in some way to this functionality (i.e. is it on chip) then it might be > nice to reflect this in the channel type. There are a dedicated channels to measure temperature. Those channels have connected thermistor but I don't think it is on chip. So the returned adc code is in microvolts and we have a translation table to convert measured voltage to miliCelsius. I thought that if I mark the channel type as IIO_TEMP then the user would expect the returned units to be miliCelsius. If that assumption is not correct I can change the type of those channels. >>> + .indexed = 1, \ >>> + .channel = VADC_##_id, \ >>> + .address = _pre, \ >>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ >>> + .datasheet_name = __stringify(VADC_##_id), \ >>> + .scan_type = { \ >>> + .sign = 's', \ >>> + .realbits = 15, \ >>> + .storagebits = 16, \ >>> + .endianness = IIO_CPU, \ >>> + }, \ >>> + }, >>> + >>> +static const struct iio_chan_spec vadc_channels[] = { >>> + VADC_CHAN(USBIN, 4) /* 0x00 */ >>> + VADC_CHAN(DCIN, 4) >>> + VADC_CHAN(VCHG_SNS, 3) >>> + VADC_CHAN(SPARE1_03, 1) >>> + VADC_CHAN(USB_ID_MV, 1) >>> + VADC_CHAN(VCOIN, 1) >>> + VADC_CHAN(VBAT_SNS, 1) >>> + VADC_CHAN(VSYS, 1) >>> + VADC_CHAN(DIE_TEMP, 0) >>> + VADC_CHAN(REF_625MV, 0) >>> + VADC_CHAN(REF_1250MV, 0) >>> + VADC_CHAN(CHG_TEMP, 0) >>> + VADC_CHAN(SPARE1, 0) >>> + VADC_CHAN(SPARE2, 0) >>> + VADC_CHAN(GND_REF, 0) >>> + VADC_CHAN(VDD_VADC, 0) /* 0x0f */ >>> + >>> + VADC_CHAN(P_MUX1_1_1, 0) /* 0x10 */ >>> + VADC_CHAN(P_MUX2_1_1, 0) >>> + VADC_CHAN(P_MUX3_1_1, 0) >>> + VADC_CHAN(P_MUX4_1_1, 0) >>> + VADC_CHAN(P_MUX5_1_1, 0) >>> + VADC_CHAN(P_MUX6_1_1, 0) >>> + VADC_CHAN(P_MUX7_1_1, 0) >>> + VADC_CHAN(P_MUX8_1_1, 0) >>> + VADC_CHAN(P_MUX9_1_1, 0) >>> + VADC_CHAN(P_MUX10_1_1, 0) >>> + VADC_CHAN(P_MUX11_1_1, 0) >>> + VADC_CHAN(P_MUX12_1_1, 0) >>> + VADC_CHAN(P_MUX13_1_1, 0) >>> + VADC_CHAN(P_MUX14_1_1, 0) >>> + VADC_CHAN(P_MUX15_1_1, 0) >>> + VADC_CHAN(P_MUX16_1_1, 0) /* 0x1f */ >>> + >>> + VADC_CHAN(P_MUX1_1_3, 1) /* 0x20 */ >>> + VADC_CHAN(P_MUX2_1_3, 1) >>> + VADC_CHAN(P_MUX3_1_3, 1) >>> + VADC_CHAN(P_MUX4_1_3, 1) >>> + VADC_CHAN(P_MUX5_1_3, 1) >>> + VADC_CHAN(P_MUX6_1_3, 1) >>> + VADC_CHAN(P_MUX7_1_3, 1) >>> + VADC_CHAN(P_MUX8_1_3, 1) >>> + VADC_CHAN(P_MUX9_1_3, 1) >>> + VADC_CHAN(P_MUX10_1_3, 1) >>> + VADC_CHAN(P_MUX11_1_3, 1) >>> + VADC_CHAN(P_MUX12_1_3, 1) >>> + VADC_CHAN(P_MUX13_1_3, 1) >>> + VADC_CHAN(P_MUX14_1_3, 1) >>> + VADC_CHAN(P_MUX15_1_3, 1) >>> + VADC_CHAN(P_MUX16_1_3, 1) /* 0x2f */ >>> + >>> + VADC_CHAN(LR_MUX1_BAT_THERM, 0) /* 0x30 */ >>> + VADC_CHAN(LR_MUX2_BAT_ID, 0) >>> + VADC_CHAN(LR_MUX3_XO_THERM, 0) >>> + VADC_CHAN(LR_MUX4_AMUX_THM1, 0) >>> + VADC_CHAN(LR_MUX5_AMUX_THM2, 0) >>> + VADC_CHAN(LR_MUX6_AMUX_THM3, 0) >>> + VADC_CHAN(LR_MUX7_HW_ID, 0) >>> + VADC_CHAN(LR_MUX8_AMUX_THM4, 0) >>> + VADC_CHAN(LR_MUX9_AMUX_THM5, 0) >>> + VADC_CHAN(LR_MUX10_USB_ID, 0) >>> + VADC_CHAN(AMUX_PU1, 0) >>> + VADC_CHAN(AMUX_PU2, 0) >>> + VADC_CHAN(LR_MUX3_BUF_XO_THERM, 0) /* 0x3c */ >>> + >>> + VADC_CHAN(LR_MUX1_PU1_BAT_THERM, 0) /* 0x70 */ >>> + VADC_CHAN(LR_MUX2_PU1_BAT_ID, 0) >>> + VADC_CHAN(LR_MUX3_PU1_XO_THERM, 0) >>> + VADC_CHAN(LR_MUX4_PU1_AMUX_THM1, 0) >>> + VADC_CHAN(LR_MUX5_PU1_AMUX_THM2, 0) >>> + VADC_CHAN(LR_MUX6_PU1_AMUX_THM3, 0) >>> + VADC_CHAN(LR_MUX7_PU1_AMUX_HW_ID, 0) >>> + VADC_CHAN(LR_MUX8_PU1_AMUX_THM4, 0) >>> + VADC_CHAN(LR_MUX9_PU1_AMUX_THM5, 0) >>> + VADC_CHAN(LR_MUX10_PU1_AMUX_USB_ID, 0) /* 0x79 */ >>> + VADC_CHAN(LR_MUX3_BUF_PU1_XO_THERM, 0) /* 0x7c */ >>> + >>> + VADC_CHAN(LR_MUX1_PU2_BAT_THERM, 0) /* 0xb0 */ >>> + VADC_CHAN(LR_MUX2_PU2_BAT_ID, 0) >>> + VADC_CHAN(LR_MUX3_PU2_XO_THERM, 0) >>> + VADC_CHAN(LR_MUX4_PU2_AMUX_THM1, 0) >>> + VADC_CHAN(LR_MUX5_PU2_AMUX_THM2, 0) >>> + VADC_CHAN(LR_MUX6_PU2_AMUX_THM3, 0) >>> + VADC_CHAN(LR_MUX7_PU2_AMUX_HW_ID, 0) >>> + VADC_CHAN(LR_MUX8_PU2_AMUX_THM4, 0) >>> + VADC_CHAN(LR_MUX9_PU2_AMUX_THM5, 0) >>> + VADC_CHAN(LR_MUX10_PU2_AMUX_USB_ID, 0) /* 0xb9 */ >>> + VADC_CHAN(LR_MUX3_BUF_PU2_XO_THERM, 0) /* 0xbc */ >>> + >>> + VADC_CHAN(LR_MUX1_PU1_PU2_BAT_THERM, 0) /* 0xf0 */ >>> + VADC_CHAN(LR_MUX2_PU1_PU2_BAT_ID, 0) >>> + VADC_CHAN(LR_MUX3_PU1_PU2_XO_THERM, 0) >>> + VADC_CHAN(LR_MUX4_PU1_PU2_AMUX_THM1, 0) >>> + VADC_CHAN(LR_MUX5_PU1_PU2_AMUX_THM2, 0) >>> + VADC_CHAN(LR_MUX6_PU1_PU2_AMUX_THM3, 0) >>> + VADC_CHAN(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0) >>> + VADC_CHAN(LR_MUX8_PU1_PU2_AMUX_THM4, 0) >>> + VADC_CHAN(LR_MUX9_PU1_PU2_AMUX_THM5, 0) >>> + VADC_CHAN(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0) /* 0xf9 */ >>> + VADC_CHAN(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0) /* 0xfc */ >>> +}; >>> + >>> +static int >>> +vadc_get_dt_channel_data(struct vadc_priv *vadc, struct device_node *node) >>> +{ >>> + struct vadc_channel *vchan; >>> + u32 channel, value, varr[2]; >>> + int rc, pre, time, avg, decim; >> Drop pre, time, avg and decim and reuse rc instead? >>> + const char *name = node->name; >>> + >>> + rc = of_property_read_u32(node, "reg", &channel); >>> + if (rc) { >>> + dev_dbg(vadc->dev, "invalid channel number %s\n", name); >>> + return -EINVAL; >>> + } >>> + >>> + if (channel >= vadc->nchannels) { >>> + dev_dbg(vadc->dev, "%s invalid channel number %d\n", name, >>> + channel); >>> + return -EINVAL; >>> + } >>> + >>> + vchan = &vadc->channels[channel]; > Could you not have these in the same order as the iio_chan_spec array? > Hence move the lookups that are scattered elsewhere in the driver to here? > <snip> >>> +static int vadc_probe(struct platform_device *pdev) >>> +{ >>> + struct device_node *node = pdev->dev.of_node; >>> + struct device *dev = &pdev->dev; >>> + struct iio_dev *indio_dev; >>> + struct vadc_channel *vchan; >>> + struct vadc_priv *vadc; >>> + struct resource *res; >>> + struct regmap *regmap; >>> + int rc, irq_eoc, n; >> unsigned int n? >>> + >>> + regmap = dev_get_regmap(dev->parent, NULL); >>> + if (!regmap) >>> + return -ENODEV; >>> + >>> + indio_dev = devm_iio_device_alloc(dev, sizeof(*vadc)); >>> + if (!indio_dev) >>> + return -ENOMEM; >>> + >>> + vadc = iio_priv(indio_dev); >>> + vadc->dev = dev; >>> + vadc->regmap = regmap; >>> + vadc->is_ref_measured = false; >>> + init_completion(&vadc->complete); >>> + >>> + vadc->nchannels = ARRAY_SIZE(vadc_channels); >>> + vadc->channels = devm_kcalloc(dev, vadc->nchannels, >>> + sizeof(*vadc->channels), GFP_KERNEL); > Interesting. This is always the same size as the vadc_channels so you > might as well keep them in the same order and simplify various corners of > the code. >>> + if (!vadc->channels) >>> + return -ENOMEM; >>> + > I wonder if we can't vadc->channels rather more different from vadc_channels? > Perhaps vadc->channelspriv or similar? Confused me a little at first. >>> + for (n = 0; n < vadc->nchannels; n++) { >>> + vchan = &vadc->channels[n]; >>> + /* set default channel properties */ >>> + vchan->number = -1; /* inactive */ >>> + vchan->prescaling = vadc_channels[n].address; >>> + vchan->decimation = VADC_DEF_DECIMATION; >>> + vchan->hw_settle_time = VADC_DEF_HW_SETTLE_TIME; >>> + vchan->avg_samples = VADC_DEF_AVG_SAMPLES; >>> + vchan->calibration = VADC_DEF_CALIB_TYPE; >>> + } >>> + >>> + platform_set_drvdata(pdev, vadc); >>> + >>> + res = platform_get_resource(pdev, IORESOURCE_REG, 0); >>> + if (!res) >>> + return -ENODEV; >>> + >>> + vadc->base = res->start; >>> + >>> + rc = vadc_version_check(vadc); >>> + if (rc < 0) >>> + return -ENODEV; >>> + >>> + rc = vadc_get_dt_data(vadc, node); >>> + if (rc < 0) >>> + return rc; >>> + > Do we need an explicit flag to indicate poll mode rather than just > using the absense of the irq being specified to select that mode? I will think about this. Maybe I will check the returned value from platform_get_irq to see is the IRQ resource exist. If the IRQ doesn't exist I will fallback to polling. <snip> -- regards, Stan ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver @ 2014-09-15 14:12 ` Stanimir Varbanov 0 siblings, 0 replies; 23+ messages in thread From: Stanimir Varbanov @ 2014-09-15 14:12 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov Hi Jonathan, Thanks for the review! On 09/13/2014 08:27 PM, Jonathan Cameron wrote: > On 13/09/14 00:27, Hartmut Knaack wrote: >> Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >>> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >>> 15bits resolution and register space inside PMIC accessible across >>> SPMI bus. >>> >>> The vadc driver registers itself through IIO interface. >>> >> Looks already pretty good. Things you should consider in regard of common coding style are to use the variable name ret instead of rc, since it is used in almost all adc drivers and thus makes reviewing a bit easier. Besides that, you seem to use unsigned as well as unsigned int, so to be consistent, please stick to one of them. Other comments in line. > > A few additional comments from me. My biggest question is whether > you are actually making life difficult for yourself by having > vadc_channels and vadc->channels (don't like the similar naming btw!) > in different orders. I think you can move the ordering into the device > tree reading code rather than doing it in lots of other places. Hence > rather than an order based on the device tree description, put the > data into a fixed ofer in vadc->channels. > > Entirely possible I'm missing something though :) >>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> >>> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> >>> --- >>> drivers/iio/adc/Kconfig | 11 + >>> drivers/iio/adc/Makefile | 1 + >>> drivers/iio/adc/qcom-spmi-vadc.c | 999 +++++++++++++++++++++++++ >>> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >>> 4 files changed, 1130 insertions(+), 0 deletions(-) >>> create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c >>> create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h <snip> >>> + >>> +static int >> Don't wrap line here. >>> +vadc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, >>> + int *val, int *val2, long mask) >>> +{ >>> + struct vadc_priv *vadc = iio_priv(indio_dev); >>> + struct vadc_channel *vchan; >>> + struct vadc_result result; >>> + int rc; >>> + > It is a bit of a pitty we can't avoid this lookup. Normally I'd suggest > putting an index in chan->address but you've already used that. > The purpose of this is to (I think) allow you to have the private > data stored in a random order... What is the benefit of doing that? > (see various comments elsewhere) So the vadc_channels array describe all possible vadc channels for all supported PMICs from this driver. On the other side vadc->channels pointer should contain only the channels described in DT. Thus we need a below function to check is the current channel is active for the current DT (current PMIC version). This is because we have in vadc_channels the full set of channels but not every supported PMIC have support for them. I agree that this peace of code is not so clear. So I will try to rework this and register to the IIO core only those channels that have channel descriptions in DT. Also I wonder can I use iio_chan_spec::address field as a pointer to private structure with vadc channel properties like decimation, prescale etc. got from DT or the default values. > >>> + vchan = vadc_find_channel(vadc, chan->channel); >>> + if (!vchan) >>> + return -EINVAL; >>> + >>> + if (!vadc->is_ref_measured) { >>> + rc = vadc_measure_reference_points(vadc); >>> + if (rc) >>> + return rc; >>> + >>> + vadc->is_ref_measured = true; >>> + } >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_PROCESSED: >>> + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); >>> + if (rc) >>> + return rc; >>> + >>> + vadc_calibrate(vadc, vchan, &result); >>> + >>> + *val = result.physical; > I'm a little suspicious here. Are the resulting values in milivolts for > all the channels? Very handy if so, but seems a little unlikely with 15 bit > ADC that you'd have no part of greater accuracy than a milivolt. In fact *val is in microvolts. What is the expected unit from IIO ADC users? >>> + rc = IIO_VAL_INT; >> return IIO_VAL_INT; >>> + break; >>> + default: >>> + rc = -EINVAL; >>> + break; >> Drop default case, or leave empty. >>> + } >>> + >>> + return rc; >> return -EINVAL; >>> +} >>> + >>> +static const struct iio_info vadc_info = { >>> + .read_raw = vadc_read_raw, >>> + .driver_module = THIS_MODULE, >>> +}; >>> + >>> +#define VADC_CHAN(_id, _pre) \ >>> + [VADC_##_id] = { \ >>> + .type = IIO_VOLTAGE, \ > A few of the below look to be temp sensors. If they are hardwired > in some way to this functionality (i.e. is it on chip) then it might be > nice to reflect this in the channel type. There are a dedicated channels to measure temperature. Those channels have connected thermistor but I don't think it is on chip. So the returned adc code is in microvolts and we have a translation table to convert measured voltage to miliCelsius. I thought that if I mark the channel type as IIO_TEMP then the user would expect the returned units to be miliCelsius. If that assumption is not correct I can change the type of those channels. >>> + .indexed = 1, \ >>> + .channel = VADC_##_id, \ >>> + .address = _pre, \ >>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ >>> + .datasheet_name = __stringify(VADC_##_id), \ >>> + .scan_type = { \ >>> + .sign = 's', \ >>> + .realbits = 15, \ >>> + .storagebits = 16, \ >>> + .endianness = IIO_CPU, \ >>> + }, \ >>> + }, >>> + >>> +static const struct iio_chan_spec vadc_channels[] = { >>> + VADC_CHAN(USBIN, 4) /* 0x00 */ >>> + VADC_CHAN(DCIN, 4) >>> + VADC_CHAN(VCHG_SNS, 3) >>> + VADC_CHAN(SPARE1_03, 1) >>> + VADC_CHAN(USB_ID_MV, 1) >>> + VADC_CHAN(VCOIN, 1) >>> + VADC_CHAN(VBAT_SNS, 1) >>> + VADC_CHAN(VSYS, 1) >>> + VADC_CHAN(DIE_TEMP, 0) >>> + VADC_CHAN(REF_625MV, 0) >>> + VADC_CHAN(REF_1250MV, 0) >>> + VADC_CHAN(CHG_TEMP, 0) >>> + VADC_CHAN(SPARE1, 0) >>> + VADC_CHAN(SPARE2, 0) >>> + VADC_CHAN(GND_REF, 0) >>> + VADC_CHAN(VDD_VADC, 0) /* 0x0f */ >>> + >>> + VADC_CHAN(P_MUX1_1_1, 0) /* 0x10 */ >>> + VADC_CHAN(P_MUX2_1_1, 0) >>> + VADC_CHAN(P_MUX3_1_1, 0) >>> + VADC_CHAN(P_MUX4_1_1, 0) >>> + VADC_CHAN(P_MUX5_1_1, 0) >>> + VADC_CHAN(P_MUX6_1_1, 0) >>> + VADC_CHAN(P_MUX7_1_1, 0) >>> + VADC_CHAN(P_MUX8_1_1, 0) >>> + VADC_CHAN(P_MUX9_1_1, 0) >>> + VADC_CHAN(P_MUX10_1_1, 0) >>> + VADC_CHAN(P_MUX11_1_1, 0) >>> + VADC_CHAN(P_MUX12_1_1, 0) >>> + VADC_CHAN(P_MUX13_1_1, 0) >>> + VADC_CHAN(P_MUX14_1_1, 0) >>> + VADC_CHAN(P_MUX15_1_1, 0) >>> + VADC_CHAN(P_MUX16_1_1, 0) /* 0x1f */ >>> + >>> + VADC_CHAN(P_MUX1_1_3, 1) /* 0x20 */ >>> + VADC_CHAN(P_MUX2_1_3, 1) >>> + VADC_CHAN(P_MUX3_1_3, 1) >>> + VADC_CHAN(P_MUX4_1_3, 1) >>> + VADC_CHAN(P_MUX5_1_3, 1) >>> + VADC_CHAN(P_MUX6_1_3, 1) >>> + VADC_CHAN(P_MUX7_1_3, 1) >>> + VADC_CHAN(P_MUX8_1_3, 1) >>> + VADC_CHAN(P_MUX9_1_3, 1) >>> + VADC_CHAN(P_MUX10_1_3, 1) >>> + VADC_CHAN(P_MUX11_1_3, 1) >>> + VADC_CHAN(P_MUX12_1_3, 1) >>> + VADC_CHAN(P_MUX13_1_3, 1) >>> + VADC_CHAN(P_MUX14_1_3, 1) >>> + VADC_CHAN(P_MUX15_1_3, 1) >>> + VADC_CHAN(P_MUX16_1_3, 1) /* 0x2f */ >>> + >>> + VADC_CHAN(LR_MUX1_BAT_THERM, 0) /* 0x30 */ >>> + VADC_CHAN(LR_MUX2_BAT_ID, 0) >>> + VADC_CHAN(LR_MUX3_XO_THERM, 0) >>> + VADC_CHAN(LR_MUX4_AMUX_THM1, 0) >>> + VADC_CHAN(LR_MUX5_AMUX_THM2, 0) >>> + VADC_CHAN(LR_MUX6_AMUX_THM3, 0) >>> + VADC_CHAN(LR_MUX7_HW_ID, 0) >>> + VADC_CHAN(LR_MUX8_AMUX_THM4, 0) >>> + VADC_CHAN(LR_MUX9_AMUX_THM5, 0) >>> + VADC_CHAN(LR_MUX10_USB_ID, 0) >>> + VADC_CHAN(AMUX_PU1, 0) >>> + VADC_CHAN(AMUX_PU2, 0) >>> + VADC_CHAN(LR_MUX3_BUF_XO_THERM, 0) /* 0x3c */ >>> + >>> + VADC_CHAN(LR_MUX1_PU1_BAT_THERM, 0) /* 0x70 */ >>> + VADC_CHAN(LR_MUX2_PU1_BAT_ID, 0) >>> + VADC_CHAN(LR_MUX3_PU1_XO_THERM, 0) >>> + VADC_CHAN(LR_MUX4_PU1_AMUX_THM1, 0) >>> + VADC_CHAN(LR_MUX5_PU1_AMUX_THM2, 0) >>> + VADC_CHAN(LR_MUX6_PU1_AMUX_THM3, 0) >>> + VADC_CHAN(LR_MUX7_PU1_AMUX_HW_ID, 0) >>> + VADC_CHAN(LR_MUX8_PU1_AMUX_THM4, 0) >>> + VADC_CHAN(LR_MUX9_PU1_AMUX_THM5, 0) >>> + VADC_CHAN(LR_MUX10_PU1_AMUX_USB_ID, 0) /* 0x79 */ >>> + VADC_CHAN(LR_MUX3_BUF_PU1_XO_THERM, 0) /* 0x7c */ >>> + >>> + VADC_CHAN(LR_MUX1_PU2_BAT_THERM, 0) /* 0xb0 */ >>> + VADC_CHAN(LR_MUX2_PU2_BAT_ID, 0) >>> + VADC_CHAN(LR_MUX3_PU2_XO_THERM, 0) >>> + VADC_CHAN(LR_MUX4_PU2_AMUX_THM1, 0) >>> + VADC_CHAN(LR_MUX5_PU2_AMUX_THM2, 0) >>> + VADC_CHAN(LR_MUX6_PU2_AMUX_THM3, 0) >>> + VADC_CHAN(LR_MUX7_PU2_AMUX_HW_ID, 0) >>> + VADC_CHAN(LR_MUX8_PU2_AMUX_THM4, 0) >>> + VADC_CHAN(LR_MUX9_PU2_AMUX_THM5, 0) >>> + VADC_CHAN(LR_MUX10_PU2_AMUX_USB_ID, 0) /* 0xb9 */ >>> + VADC_CHAN(LR_MUX3_BUF_PU2_XO_THERM, 0) /* 0xbc */ >>> + >>> + VADC_CHAN(LR_MUX1_PU1_PU2_BAT_THERM, 0) /* 0xf0 */ >>> + VADC_CHAN(LR_MUX2_PU1_PU2_BAT_ID, 0) >>> + VADC_CHAN(LR_MUX3_PU1_PU2_XO_THERM, 0) >>> + VADC_CHAN(LR_MUX4_PU1_PU2_AMUX_THM1, 0) >>> + VADC_CHAN(LR_MUX5_PU1_PU2_AMUX_THM2, 0) >>> + VADC_CHAN(LR_MUX6_PU1_PU2_AMUX_THM3, 0) >>> + VADC_CHAN(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0) >>> + VADC_CHAN(LR_MUX8_PU1_PU2_AMUX_THM4, 0) >>> + VADC_CHAN(LR_MUX9_PU1_PU2_AMUX_THM5, 0) >>> + VADC_CHAN(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0) /* 0xf9 */ >>> + VADC_CHAN(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0) /* 0xfc */ >>> +}; >>> + >>> +static int >>> +vadc_get_dt_channel_data(struct vadc_priv *vadc, struct device_node *node) >>> +{ >>> + struct vadc_channel *vchan; >>> + u32 channel, value, varr[2]; >>> + int rc, pre, time, avg, decim; >> Drop pre, time, avg and decim and reuse rc instead? >>> + const char *name = node->name; >>> + >>> + rc = of_property_read_u32(node, "reg", &channel); >>> + if (rc) { >>> + dev_dbg(vadc->dev, "invalid channel number %s\n", name); >>> + return -EINVAL; >>> + } >>> + >>> + if (channel >= vadc->nchannels) { >>> + dev_dbg(vadc->dev, "%s invalid channel number %d\n", name, >>> + channel); >>> + return -EINVAL; >>> + } >>> + >>> + vchan = &vadc->channels[channel]; > Could you not have these in the same order as the iio_chan_spec array? > Hence move the lookups that are scattered elsewhere in the driver to here? > <snip> >>> +static int vadc_probe(struct platform_device *pdev) >>> +{ >>> + struct device_node *node = pdev->dev.of_node; >>> + struct device *dev = &pdev->dev; >>> + struct iio_dev *indio_dev; >>> + struct vadc_channel *vchan; >>> + struct vadc_priv *vadc; >>> + struct resource *res; >>> + struct regmap *regmap; >>> + int rc, irq_eoc, n; >> unsigned int n? >>> + >>> + regmap = dev_get_regmap(dev->parent, NULL); >>> + if (!regmap) >>> + return -ENODEV; >>> + >>> + indio_dev = devm_iio_device_alloc(dev, sizeof(*vadc)); >>> + if (!indio_dev) >>> + return -ENOMEM; >>> + >>> + vadc = iio_priv(indio_dev); >>> + vadc->dev = dev; >>> + vadc->regmap = regmap; >>> + vadc->is_ref_measured = false; >>> + init_completion(&vadc->complete); >>> + >>> + vadc->nchannels = ARRAY_SIZE(vadc_channels); >>> + vadc->channels = devm_kcalloc(dev, vadc->nchannels, >>> + sizeof(*vadc->channels), GFP_KERNEL); > Interesting. This is always the same size as the vadc_channels so you > might as well keep them in the same order and simplify various corners of > the code. >>> + if (!vadc->channels) >>> + return -ENOMEM; >>> + > I wonder if we can't vadc->channels rather more different from vadc_channels? > Perhaps vadc->channelspriv or similar? Confused me a little at first. >>> + for (n = 0; n < vadc->nchannels; n++) { >>> + vchan = &vadc->channels[n]; >>> + /* set default channel properties */ >>> + vchan->number = -1; /* inactive */ >>> + vchan->prescaling = vadc_channels[n].address; >>> + vchan->decimation = VADC_DEF_DECIMATION; >>> + vchan->hw_settle_time = VADC_DEF_HW_SETTLE_TIME; >>> + vchan->avg_samples = VADC_DEF_AVG_SAMPLES; >>> + vchan->calibration = VADC_DEF_CALIB_TYPE; >>> + } >>> + >>> + platform_set_drvdata(pdev, vadc); >>> + >>> + res = platform_get_resource(pdev, IORESOURCE_REG, 0); >>> + if (!res) >>> + return -ENODEV; >>> + >>> + vadc->base = res->start; >>> + >>> + rc = vadc_version_check(vadc); >>> + if (rc < 0) >>> + return -ENODEV; >>> + >>> + rc = vadc_get_dt_data(vadc, node); >>> + if (rc < 0) >>> + return rc; >>> + > Do we need an explicit flag to indicate poll mode rather than just > using the absense of the irq being specified to select that mode? I will think about this. Maybe I will check the returned value from platform_get_irq to see is the IRQ resource exist. If the IRQ doesn't exist I will fallback to polling. <snip> -- regards, Stan ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <5416F3E2.3030009-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-15 14:12 ` Stanimir Varbanov @ 2014-09-15 16:11 ` Jonathan Cameron -1 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-09-15 16:11 UTC (permalink / raw) To: Stanimir Varbanov Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins On September 15, 2014 3:12:50 PM GMT+01:00, Stanimir Varbanov <svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> wrote: >Hi Jonathan, > >Thanks for the review! > >On 09/13/2014 08:27 PM, Jonathan Cameron wrote: >> On 13/09/14 00:27, Hartmut Knaack wrote: >>> Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >>>> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >>>> 15bits resolution and register space inside PMIC accessible across >>>> SPMI bus. >>>> >>>> The vadc driver registers itself through IIO interface. >>>> >>> Looks already pretty good. Things you should consider in regard of >common coding style are to use the variable name ret instead of rc, >since it is used in almost all adc drivers and thus makes reviewing a >bit easier. Besides that, you seem to use unsigned as well as unsigned >int, so to be consistent, please stick to one of them. Other comments >in line. >> >> A few additional comments from me. My biggest question is whether >> you are actually making life difficult for yourself by having >> vadc_channels and vadc->channels (don't like the similar naming btw!) >> in different orders. I think you can move the ordering into the >device >> tree reading code rather than doing it in lots of other places. >Hence >> rather than an order based on the device tree description, put the >> data into a fixed ofer in vadc->channels. >> >> Entirely possible I'm missing something though :) >>>> Signed-off-by: Stanimir Varbanov <svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >>>> Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >>>> --- >>>> drivers/iio/adc/Kconfig | 11 + >>>> drivers/iio/adc/Makefile | 1 + >>>> drivers/iio/adc/qcom-spmi-vadc.c | 999 >+++++++++++++++++++++++++ >>>> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >>>> 4 files changed, 1130 insertions(+), 0 deletions(-) >>>> create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c >>>> create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h > ><snip> > >>>> + >>>> +static int >>> Don't wrap line here. >>>> +vadc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec >const *chan, >>>> + int *val, int *val2, long mask) >>>> +{ >>>> + struct vadc_priv *vadc = iio_priv(indio_dev); >>>> + struct vadc_channel *vchan; >>>> + struct vadc_result result; >>>> + int rc; >>>> + >> It is a bit of a pitty we can't avoid this lookup. Normally I'd >suggest >> putting an index in chan->address but you've already used that. >> The purpose of this is to (I think) allow you to have the private >> data stored in a random order... What is the benefit of doing that? >> (see various comments elsewhere) > >So the vadc_channels array describe all possible vadc channels for all >supported PMICs from this driver. On the other side vadc->channels >pointer should contain only the channels described in DT. > >Thus we need a below function to check is the current channel is active >for the current DT (current PMIC version). This is because we have in >vadc_channels the full set of channels but not every supported PMIC >have >support for them. > >I agree that this peace of code is not so clear. So I will try to >rework >this and register to the IIO core only those channels that have channel >descriptions in DT. Cool > >Also I wonder can I use iio_chan_spec::address field as a pointer to >private structure with vadc channel properties like decimation, >prescale >etc. got from DT or the default values. Yes or an index into an array of them perhaps? > >> >>>> + vchan = vadc_find_channel(vadc, chan->channel); >>>> + if (!vchan) >>>> + return -EINVAL; >>>> + >>>> + if (!vadc->is_ref_measured) { >>>> + rc = vadc_measure_reference_points(vadc); >>>> + if (rc) >>>> + return rc; >>>> + >>>> + vadc->is_ref_measured = true; >>>> + } >>>> + >>>> + switch (mask) { >>>> + case IIO_CHAN_INFO_PROCESSED: >>>> + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); >>>> + if (rc) >>>> + return rc; >>>> + >>>> + vadc_calibrate(vadc, vchan, &result); >>>> + >>>> + *val = result.physical; >> I'm a little suspicious here. Are the resulting values in milivolts >for >> all the channels? Very handy if so, but seems a little unlikely with >15 bit >> ADC that you'd have no part of greater accuracy than a milivolt. > >In fact *val is in microvolts. What is the expected unit from IIO ADC >users? See Documentation/ABI/sysfs-bus-iio Millivolts I think... We copied hwmon where possible. > >>>> + rc = IIO_VAL_INT; >>> return IIO_VAL_INT; >>>> + break; >>>> + default: >>>> + rc = -EINVAL; >>>> + break; >>> Drop default case, or leave empty. >>>> + } >>>> + >>>> + return rc; >>> return -EINVAL; >>>> +} >>>> + >>>> +static const struct iio_info vadc_info = { >>>> + .read_raw = vadc_read_raw, >>>> + .driver_module = THIS_MODULE, >>>> +}; >>>> + >>>> +#define VADC_CHAN(_id, _pre) \ >>>> + [VADC_##_id] = { \ >>>> + .type = IIO_VOLTAGE, \ >> A few of the below look to be temp sensors. If they are hardwired >> in some way to this functionality (i.e. is it on chip) then it might >be >> nice to reflect this in the channel type. > >There are a dedicated channels to measure temperature. Those channels >have connected thermistor but I don't think it is on chip. So the >returned adc code is in microvolts and we have a translation table to >convert measured voltage to miliCelsius. I thought that if I mark the >channel type as IIO_TEMP then the user would expect the returned units >to be miliCelsius. If that assumption is not correct I can change the >type of those channels. You are correct. Leave them as voltages. If they were hardwired inside the chip it would be different. > >>>> + .indexed = 1, \ >>>> + .channel = VADC_##_id, \ >>>> + .address = _pre, \ >>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ >>>> + .datasheet_name = __stringify(VADC_##_id), \ >>>> + .scan_type = { \ >>>> + .sign = 's', \ >>>> + .realbits = 15, \ >>>> + .storagebits = 16, \ >>>> + .endianness = IIO_CPU, \ >>>> + }, \ >>>> + }, >>>> + >>>> +static const struct iio_chan_spec vadc_channels[] = { >>>> + VADC_CHAN(USBIN, 4) /* 0x00 */ >>>> + VADC_CHAN(DCIN, 4) >>>> + VADC_CHAN(VCHG_SNS, 3) >>>> + VADC_CHAN(SPARE1_03, 1) >>>> + VADC_CHAN(USB_ID_MV, 1) >>>> + VADC_CHAN(VCOIN, 1) >>>> + VADC_CHAN(VBAT_SNS, 1) >>>> + VADC_CHAN(VSYS, 1) >>>> + VADC_CHAN(DIE_TEMP, 0) >>>> + VADC_CHAN(REF_625MV, 0) >>>> + VADC_CHAN(REF_1250MV, 0) >>>> + VADC_CHAN(CHG_TEMP, 0) >>>> + VADC_CHAN(SPARE1, 0) >>>> + VADC_CHAN(SPARE2, 0) >>>> + VADC_CHAN(GND_REF, 0) >>>> + VADC_CHAN(VDD_VADC, 0) /* 0x0f */ >>>> + >>>> + VADC_CHAN(P_MUX1_1_1, 0) /* 0x10 */ >>>> + VADC_CHAN(P_MUX2_1_1, 0) >>>> + VADC_CHAN(P_MUX3_1_1, 0) >>>> + VADC_CHAN(P_MUX4_1_1, 0) >>>> + VADC_CHAN(P_MUX5_1_1, 0) >>>> + VADC_CHAN(P_MUX6_1_1, 0) >>>> + VADC_CHAN(P_MUX7_1_1, 0) >>>> + VADC_CHAN(P_MUX8_1_1, 0) >>>> + VADC_CHAN(P_MUX9_1_1, 0) >>>> + VADC_CHAN(P_MUX10_1_1, 0) >>>> + VADC_CHAN(P_MUX11_1_1, 0) >>>> + VADC_CHAN(P_MUX12_1_1, 0) >>>> + VADC_CHAN(P_MUX13_1_1, 0) >>>> + VADC_CHAN(P_MUX14_1_1, 0) >>>> + VADC_CHAN(P_MUX15_1_1, 0) >>>> + VADC_CHAN(P_MUX16_1_1, 0) /* 0x1f */ >>>> + >>>> + VADC_CHAN(P_MUX1_1_3, 1) /* 0x20 */ >>>> + VADC_CHAN(P_MUX2_1_3, 1) >>>> + VADC_CHAN(P_MUX3_1_3, 1) >>>> + VADC_CHAN(P_MUX4_1_3, 1) >>>> + VADC_CHAN(P_MUX5_1_3, 1) >>>> + VADC_CHAN(P_MUX6_1_3, 1) >>>> + VADC_CHAN(P_MUX7_1_3, 1) >>>> + VADC_CHAN(P_MUX8_1_3, 1) >>>> + VADC_CHAN(P_MUX9_1_3, 1) >>>> + VADC_CHAN(P_MUX10_1_3, 1) >>>> + VADC_CHAN(P_MUX11_1_3, 1) >>>> + VADC_CHAN(P_MUX12_1_3, 1) >>>> + VADC_CHAN(P_MUX13_1_3, 1) >>>> + VADC_CHAN(P_MUX14_1_3, 1) >>>> + VADC_CHAN(P_MUX15_1_3, 1) >>>> + VADC_CHAN(P_MUX16_1_3, 1) /* 0x2f */ >>>> + >>>> + VADC_CHAN(LR_MUX1_BAT_THERM, 0) /* 0x30 */ >>>> + VADC_CHAN(LR_MUX2_BAT_ID, 0) >>>> + VADC_CHAN(LR_MUX3_XO_THERM, 0) >>>> + VADC_CHAN(LR_MUX4_AMUX_THM1, 0) >>>> + VADC_CHAN(LR_MUX5_AMUX_THM2, 0) >>>> + VADC_CHAN(LR_MUX6_AMUX_THM3, 0) >>>> + VADC_CHAN(LR_MUX7_HW_ID, 0) >>>> + VADC_CHAN(LR_MUX8_AMUX_THM4, 0) >>>> + VADC_CHAN(LR_MUX9_AMUX_THM5, 0) >>>> + VADC_CHAN(LR_MUX10_USB_ID, 0) >>>> + VADC_CHAN(AMUX_PU1, 0) >>>> + VADC_CHAN(AMUX_PU2, 0) >>>> + VADC_CHAN(LR_MUX3_BUF_XO_THERM, 0) /* 0x3c */ >>>> + >>>> + VADC_CHAN(LR_MUX1_PU1_BAT_THERM, 0) /* 0x70 */ >>>> + VADC_CHAN(LR_MUX2_PU1_BAT_ID, 0) >>>> + VADC_CHAN(LR_MUX3_PU1_XO_THERM, 0) >>>> + VADC_CHAN(LR_MUX4_PU1_AMUX_THM1, 0) >>>> + VADC_CHAN(LR_MUX5_PU1_AMUX_THM2, 0) >>>> + VADC_CHAN(LR_MUX6_PU1_AMUX_THM3, 0) >>>> + VADC_CHAN(LR_MUX7_PU1_AMUX_HW_ID, 0) >>>> + VADC_CHAN(LR_MUX8_PU1_AMUX_THM4, 0) >>>> + VADC_CHAN(LR_MUX9_PU1_AMUX_THM5, 0) >>>> + VADC_CHAN(LR_MUX10_PU1_AMUX_USB_ID, 0) /* 0x79 */ >>>> + VADC_CHAN(LR_MUX3_BUF_PU1_XO_THERM, 0) /* 0x7c */ >>>> + >>>> + VADC_CHAN(LR_MUX1_PU2_BAT_THERM, 0) /* 0xb0 */ >>>> + VADC_CHAN(LR_MUX2_PU2_BAT_ID, 0) >>>> + VADC_CHAN(LR_MUX3_PU2_XO_THERM, 0) >>>> + VADC_CHAN(LR_MUX4_PU2_AMUX_THM1, 0) >>>> + VADC_CHAN(LR_MUX5_PU2_AMUX_THM2, 0) >>>> + VADC_CHAN(LR_MUX6_PU2_AMUX_THM3, 0) >>>> + VADC_CHAN(LR_MUX7_PU2_AMUX_HW_ID, 0) >>>> + VADC_CHAN(LR_MUX8_PU2_AMUX_THM4, 0) >>>> + VADC_CHAN(LR_MUX9_PU2_AMUX_THM5, 0) >>>> + VADC_CHAN(LR_MUX10_PU2_AMUX_USB_ID, 0) /* 0xb9 */ >>>> + VADC_CHAN(LR_MUX3_BUF_PU2_XO_THERM, 0) /* 0xbc */ >>>> + >>>> + VADC_CHAN(LR_MUX1_PU1_PU2_BAT_THERM, 0) /* 0xf0 */ >>>> + VADC_CHAN(LR_MUX2_PU1_PU2_BAT_ID, 0) >>>> + VADC_CHAN(LR_MUX3_PU1_PU2_XO_THERM, 0) >>>> + VADC_CHAN(LR_MUX4_PU1_PU2_AMUX_THM1, 0) >>>> + VADC_CHAN(LR_MUX5_PU1_PU2_AMUX_THM2, 0) >>>> + VADC_CHAN(LR_MUX6_PU1_PU2_AMUX_THM3, 0) >>>> + VADC_CHAN(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0) >>>> + VADC_CHAN(LR_MUX8_PU1_PU2_AMUX_THM4, 0) >>>> + VADC_CHAN(LR_MUX9_PU1_PU2_AMUX_THM5, 0) >>>> + VADC_CHAN(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0) /* 0xf9 */ >>>> + VADC_CHAN(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0) /* 0xfc */ >>>> +}; >>>> + >>>> +static int >>>> +vadc_get_dt_channel_data(struct vadc_priv *vadc, struct >device_node *node) >>>> +{ >>>> + struct vadc_channel *vchan; >>>> + u32 channel, value, varr[2]; >>>> + int rc, pre, time, avg, decim; >>> Drop pre, time, avg and decim and reuse rc instead? >>>> + const char *name = node->name; >>>> + >>>> + rc = of_property_read_u32(node, "reg", &channel); >>>> + if (rc) { >>>> + dev_dbg(vadc->dev, "invalid channel number %s\n", name); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + if (channel >= vadc->nchannels) { >>>> + dev_dbg(vadc->dev, "%s invalid channel number %d\n", name, >>>> + channel); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + vchan = &vadc->channels[channel]; >> Could you not have these in the same order as the iio_chan_spec >array? >> Hence move the lookups that are scattered elsewhere in the driver to >here? >> > ><snip> > >>>> +static int vadc_probe(struct platform_device *pdev) >>>> +{ >>>> + struct device_node *node = pdev->dev.of_node; >>>> + struct device *dev = &pdev->dev; >>>> + struct iio_dev *indio_dev; >>>> + struct vadc_channel *vchan; >>>> + struct vadc_priv *vadc; >>>> + struct resource *res; >>>> + struct regmap *regmap; >>>> + int rc, irq_eoc, n; >>> unsigned int n? >>>> + >>>> + regmap = dev_get_regmap(dev->parent, NULL); >>>> + if (!regmap) >>>> + return -ENODEV; >>>> + >>>> + indio_dev = devm_iio_device_alloc(dev, sizeof(*vadc)); >>>> + if (!indio_dev) >>>> + return -ENOMEM; >>>> + >>>> + vadc = iio_priv(indio_dev); >>>> + vadc->dev = dev; >>>> + vadc->regmap = regmap; >>>> + vadc->is_ref_measured = false; >>>> + init_completion(&vadc->complete); >>>> + >>>> + vadc->nchannels = ARRAY_SIZE(vadc_channels); >>>> + vadc->channels = devm_kcalloc(dev, vadc->nchannels, >>>> + sizeof(*vadc->channels), GFP_KERNEL); >> Interesting. This is always the same size as the vadc_channels so >you >> might as well keep them in the same order and simplify various >corners of >> the code. >>>> + if (!vadc->channels) >>>> + return -ENOMEM; >>>> + >> I wonder if we can't vadc->channels rather more different from >vadc_channels? >> Perhaps vadc->channelspriv or similar? Confused me a little at first. >>>> + for (n = 0; n < vadc->nchannels; n++) { >>>> + vchan = &vadc->channels[n]; >>>> + /* set default channel properties */ >>>> + vchan->number = -1; /* inactive */ >>>> + vchan->prescaling = vadc_channels[n].address; >>>> + vchan->decimation = VADC_DEF_DECIMATION; >>>> + vchan->hw_settle_time = VADC_DEF_HW_SETTLE_TIME; >>>> + vchan->avg_samples = VADC_DEF_AVG_SAMPLES; >>>> + vchan->calibration = VADC_DEF_CALIB_TYPE; >>>> + } >>>> + >>>> + platform_set_drvdata(pdev, vadc); >>>> + >>>> + res = platform_get_resource(pdev, IORESOURCE_REG, 0); >>>> + if (!res) >>>> + return -ENODEV; >>>> + >>>> + vadc->base = res->start; >>>> + >>>> + rc = vadc_version_check(vadc); >>>> + if (rc < 0) >>>> + return -ENODEV; >>>> + >>>> + rc = vadc_get_dt_data(vadc, node); >>>> + if (rc < 0) >>>> + return rc; >>>> + >> Do we need an explicit flag to indicate poll mode rather than just >> using the absense of the irq being specified to select that mode? > >I will think about this. Maybe I will check the returned value from >platform_get_irq to see is the IRQ resource exist. If the IRQ doesn't >exist I will fallback to polling. That was my thought. > ><snip> -- Sent from my Android phone with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver @ 2014-09-15 16:11 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-09-15 16:11 UTC (permalink / raw) To: Stanimir Varbanov Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov On September 15, 2014 3:12:50 PM GMT+01:00, Stanimir Varbanov <svarbanov@mm-sol.com> wrote: >Hi Jonathan, > >Thanks for the review! > >On 09/13/2014 08:27 PM, Jonathan Cameron wrote: >> On 13/09/14 00:27, Hartmut Knaack wrote: >>> Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >>>> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >>>> 15bits resolution and register space inside PMIC accessible across >>>> SPMI bus. >>>> >>>> The vadc driver registers itself through IIO interface. >>>> >>> Looks already pretty good. Things you should consider in regard of >common coding style are to use the variable name ret instead of rc, >since it is used in almost all adc drivers and thus makes reviewing a >bit easier. Besides that, you seem to use unsigned as well as unsigned >int, so to be consistent, please stick to one of them. Other comments >in line. >> >> A few additional comments from me. My biggest question is whether >> you are actually making life difficult for yourself by having >> vadc_channels and vadc->channels (don't like the similar naming btw!) >> in different orders. I think you can move the ordering into the >device >> tree reading code rather than doing it in lots of other places. >Hence >> rather than an order based on the device tree description, put the >> data into a fixed ofer in vadc->channels. >> >> Entirely possible I'm missing something though :) >>>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> >>>> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> >>>> --- >>>> drivers/iio/adc/Kconfig | 11 + >>>> drivers/iio/adc/Makefile | 1 + >>>> drivers/iio/adc/qcom-spmi-vadc.c | 999 >+++++++++++++++++++++++++ >>>> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >>>> 4 files changed, 1130 insertions(+), 0 deletions(-) >>>> create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c >>>> create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h > ><snip> > >>>> + >>>> +static int >>> Don't wrap line here. >>>> +vadc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec >const *chan, >>>> + int *val, int *val2, long mask) >>>> +{ >>>> + struct vadc_priv *vadc = iio_priv(indio_dev); >>>> + struct vadc_channel *vchan; >>>> + struct vadc_result result; >>>> + int rc; >>>> + >> It is a bit of a pitty we can't avoid this lookup. Normally I'd >suggest >> putting an index in chan->address but you've already used that. >> The purpose of this is to (I think) allow you to have the private >> data stored in a random order... What is the benefit of doing that? >> (see various comments elsewhere) > >So the vadc_channels array describe all possible vadc channels for all >supported PMICs from this driver. On the other side vadc->channels >pointer should contain only the channels described in DT. > >Thus we need a below function to check is the current channel is active >for the current DT (current PMIC version). This is because we have in >vadc_channels the full set of channels but not every supported PMIC >have >support for them. > >I agree that this peace of code is not so clear. So I will try to >rework >this and register to the IIO core only those channels that have channel >descriptions in DT. Cool > >Also I wonder can I use iio_chan_spec::address field as a pointer to >private structure with vadc channel properties like decimation, >prescale >etc. got from DT or the default values. Yes or an index into an array of them perhaps? > >> >>>> + vchan = vadc_find_channel(vadc, chan->channel); >>>> + if (!vchan) >>>> + return -EINVAL; >>>> + >>>> + if (!vadc->is_ref_measured) { >>>> + rc = vadc_measure_reference_points(vadc); >>>> + if (rc) >>>> + return rc; >>>> + >>>> + vadc->is_ref_measured = true; >>>> + } >>>> + >>>> + switch (mask) { >>>> + case IIO_CHAN_INFO_PROCESSED: >>>> + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); >>>> + if (rc) >>>> + return rc; >>>> + >>>> + vadc_calibrate(vadc, vchan, &result); >>>> + >>>> + *val = result.physical; >> I'm a little suspicious here. Are the resulting values in milivolts >for >> all the channels? Very handy if so, but seems a little unlikely with >15 bit >> ADC that you'd have no part of greater accuracy than a milivolt. > >In fact *val is in microvolts. What is the expected unit from IIO ADC >users? See Documentation/ABI/sysfs-bus-iio Millivolts I think... We copied hwmon where possible. > >>>> + rc = IIO_VAL_INT; >>> return IIO_VAL_INT; >>>> + break; >>>> + default: >>>> + rc = -EINVAL; >>>> + break; >>> Drop default case, or leave empty. >>>> + } >>>> + >>>> + return rc; >>> return -EINVAL; >>>> +} >>>> + >>>> +static const struct iio_info vadc_info = { >>>> + .read_raw = vadc_read_raw, >>>> + .driver_module = THIS_MODULE, >>>> +}; >>>> + >>>> +#define VADC_CHAN(_id, _pre) \ >>>> + [VADC_##_id] = { \ >>>> + .type = IIO_VOLTAGE, \ >> A few of the below look to be temp sensors. If they are hardwired >> in some way to this functionality (i.e. is it on chip) then it might >be >> nice to reflect this in the channel type. > >There are a dedicated channels to measure temperature. Those channels >have connected thermistor but I don't think it is on chip. So the >returned adc code is in microvolts and we have a translation table to >convert measured voltage to miliCelsius. I thought that if I mark the >channel type as IIO_TEMP then the user would expect the returned units >to be miliCelsius. If that assumption is not correct I can change the >type of those channels. You are correct. Leave them as voltages. If they were hardwired inside the chip it would be different. > >>>> + .indexed = 1, \ >>>> + .channel = VADC_##_id, \ >>>> + .address = _pre, \ >>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ >>>> + .datasheet_name = __stringify(VADC_##_id), \ >>>> + .scan_type = { \ >>>> + .sign = 's', \ >>>> + .realbits = 15, \ >>>> + .storagebits = 16, \ >>>> + .endianness = IIO_CPU, \ >>>> + }, \ >>>> + }, >>>> + >>>> +static const struct iio_chan_spec vadc_channels[] = { >>>> + VADC_CHAN(USBIN, 4) /* 0x00 */ >>>> + VADC_CHAN(DCIN, 4) >>>> + VADC_CHAN(VCHG_SNS, 3) >>>> + VADC_CHAN(SPARE1_03, 1) >>>> + VADC_CHAN(USB_ID_MV, 1) >>>> + VADC_CHAN(VCOIN, 1) >>>> + VADC_CHAN(VBAT_SNS, 1) >>>> + VADC_CHAN(VSYS, 1) >>>> + VADC_CHAN(DIE_TEMP, 0) >>>> + VADC_CHAN(REF_625MV, 0) >>>> + VADC_CHAN(REF_1250MV, 0) >>>> + VADC_CHAN(CHG_TEMP, 0) >>>> + VADC_CHAN(SPARE1, 0) >>>> + VADC_CHAN(SPARE2, 0) >>>> + VADC_CHAN(GND_REF, 0) >>>> + VADC_CHAN(VDD_VADC, 0) /* 0x0f */ >>>> + >>>> + VADC_CHAN(P_MUX1_1_1, 0) /* 0x10 */ >>>> + VADC_CHAN(P_MUX2_1_1, 0) >>>> + VADC_CHAN(P_MUX3_1_1, 0) >>>> + VADC_CHAN(P_MUX4_1_1, 0) >>>> + VADC_CHAN(P_MUX5_1_1, 0) >>>> + VADC_CHAN(P_MUX6_1_1, 0) >>>> + VADC_CHAN(P_MUX7_1_1, 0) >>>> + VADC_CHAN(P_MUX8_1_1, 0) >>>> + VADC_CHAN(P_MUX9_1_1, 0) >>>> + VADC_CHAN(P_MUX10_1_1, 0) >>>> + VADC_CHAN(P_MUX11_1_1, 0) >>>> + VADC_CHAN(P_MUX12_1_1, 0) >>>> + VADC_CHAN(P_MUX13_1_1, 0) >>>> + VADC_CHAN(P_MUX14_1_1, 0) >>>> + VADC_CHAN(P_MUX15_1_1, 0) >>>> + VADC_CHAN(P_MUX16_1_1, 0) /* 0x1f */ >>>> + >>>> + VADC_CHAN(P_MUX1_1_3, 1) /* 0x20 */ >>>> + VADC_CHAN(P_MUX2_1_3, 1) >>>> + VADC_CHAN(P_MUX3_1_3, 1) >>>> + VADC_CHAN(P_MUX4_1_3, 1) >>>> + VADC_CHAN(P_MUX5_1_3, 1) >>>> + VADC_CHAN(P_MUX6_1_3, 1) >>>> + VADC_CHAN(P_MUX7_1_3, 1) >>>> + VADC_CHAN(P_MUX8_1_3, 1) >>>> + VADC_CHAN(P_MUX9_1_3, 1) >>>> + VADC_CHAN(P_MUX10_1_3, 1) >>>> + VADC_CHAN(P_MUX11_1_3, 1) >>>> + VADC_CHAN(P_MUX12_1_3, 1) >>>> + VADC_CHAN(P_MUX13_1_3, 1) >>>> + VADC_CHAN(P_MUX14_1_3, 1) >>>> + VADC_CHAN(P_MUX15_1_3, 1) >>>> + VADC_CHAN(P_MUX16_1_3, 1) /* 0x2f */ >>>> + >>>> + VADC_CHAN(LR_MUX1_BAT_THERM, 0) /* 0x30 */ >>>> + VADC_CHAN(LR_MUX2_BAT_ID, 0) >>>> + VADC_CHAN(LR_MUX3_XO_THERM, 0) >>>> + VADC_CHAN(LR_MUX4_AMUX_THM1, 0) >>>> + VADC_CHAN(LR_MUX5_AMUX_THM2, 0) >>>> + VADC_CHAN(LR_MUX6_AMUX_THM3, 0) >>>> + VADC_CHAN(LR_MUX7_HW_ID, 0) >>>> + VADC_CHAN(LR_MUX8_AMUX_THM4, 0) >>>> + VADC_CHAN(LR_MUX9_AMUX_THM5, 0) >>>> + VADC_CHAN(LR_MUX10_USB_ID, 0) >>>> + VADC_CHAN(AMUX_PU1, 0) >>>> + VADC_CHAN(AMUX_PU2, 0) >>>> + VADC_CHAN(LR_MUX3_BUF_XO_THERM, 0) /* 0x3c */ >>>> + >>>> + VADC_CHAN(LR_MUX1_PU1_BAT_THERM, 0) /* 0x70 */ >>>> + VADC_CHAN(LR_MUX2_PU1_BAT_ID, 0) >>>> + VADC_CHAN(LR_MUX3_PU1_XO_THERM, 0) >>>> + VADC_CHAN(LR_MUX4_PU1_AMUX_THM1, 0) >>>> + VADC_CHAN(LR_MUX5_PU1_AMUX_THM2, 0) >>>> + VADC_CHAN(LR_MUX6_PU1_AMUX_THM3, 0) >>>> + VADC_CHAN(LR_MUX7_PU1_AMUX_HW_ID, 0) >>>> + VADC_CHAN(LR_MUX8_PU1_AMUX_THM4, 0) >>>> + VADC_CHAN(LR_MUX9_PU1_AMUX_THM5, 0) >>>> + VADC_CHAN(LR_MUX10_PU1_AMUX_USB_ID, 0) /* 0x79 */ >>>> + VADC_CHAN(LR_MUX3_BUF_PU1_XO_THERM, 0) /* 0x7c */ >>>> + >>>> + VADC_CHAN(LR_MUX1_PU2_BAT_THERM, 0) /* 0xb0 */ >>>> + VADC_CHAN(LR_MUX2_PU2_BAT_ID, 0) >>>> + VADC_CHAN(LR_MUX3_PU2_XO_THERM, 0) >>>> + VADC_CHAN(LR_MUX4_PU2_AMUX_THM1, 0) >>>> + VADC_CHAN(LR_MUX5_PU2_AMUX_THM2, 0) >>>> + VADC_CHAN(LR_MUX6_PU2_AMUX_THM3, 0) >>>> + VADC_CHAN(LR_MUX7_PU2_AMUX_HW_ID, 0) >>>> + VADC_CHAN(LR_MUX8_PU2_AMUX_THM4, 0) >>>> + VADC_CHAN(LR_MUX9_PU2_AMUX_THM5, 0) >>>> + VADC_CHAN(LR_MUX10_PU2_AMUX_USB_ID, 0) /* 0xb9 */ >>>> + VADC_CHAN(LR_MUX3_BUF_PU2_XO_THERM, 0) /* 0xbc */ >>>> + >>>> + VADC_CHAN(LR_MUX1_PU1_PU2_BAT_THERM, 0) /* 0xf0 */ >>>> + VADC_CHAN(LR_MUX2_PU1_PU2_BAT_ID, 0) >>>> + VADC_CHAN(LR_MUX3_PU1_PU2_XO_THERM, 0) >>>> + VADC_CHAN(LR_MUX4_PU1_PU2_AMUX_THM1, 0) >>>> + VADC_CHAN(LR_MUX5_PU1_PU2_AMUX_THM2, 0) >>>> + VADC_CHAN(LR_MUX6_PU1_PU2_AMUX_THM3, 0) >>>> + VADC_CHAN(LR_MUX7_PU1_PU2_AMUX_HW_ID, 0) >>>> + VADC_CHAN(LR_MUX8_PU1_PU2_AMUX_THM4, 0) >>>> + VADC_CHAN(LR_MUX9_PU1_PU2_AMUX_THM5, 0) >>>> + VADC_CHAN(LR_MUX10_PU1_PU2_AMUX_USB_ID, 0) /* 0xf9 */ >>>> + VADC_CHAN(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0) /* 0xfc */ >>>> +}; >>>> + >>>> +static int >>>> +vadc_get_dt_channel_data(struct vadc_priv *vadc, struct >device_node *node) >>>> +{ >>>> + struct vadc_channel *vchan; >>>> + u32 channel, value, varr[2]; >>>> + int rc, pre, time, avg, decim; >>> Drop pre, time, avg and decim and reuse rc instead? >>>> + const char *name = node->name; >>>> + >>>> + rc = of_property_read_u32(node, "reg", &channel); >>>> + if (rc) { >>>> + dev_dbg(vadc->dev, "invalid channel number %s\n", name); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + if (channel >= vadc->nchannels) { >>>> + dev_dbg(vadc->dev, "%s invalid channel number %d\n", name, >>>> + channel); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + vchan = &vadc->channels[channel]; >> Could you not have these in the same order as the iio_chan_spec >array? >> Hence move the lookups that are scattered elsewhere in the driver to >here? >> > ><snip> > >>>> +static int vadc_probe(struct platform_device *pdev) >>>> +{ >>>> + struct device_node *node = pdev->dev.of_node; >>>> + struct device *dev = &pdev->dev; >>>> + struct iio_dev *indio_dev; >>>> + struct vadc_channel *vchan; >>>> + struct vadc_priv *vadc; >>>> + struct resource *res; >>>> + struct regmap *regmap; >>>> + int rc, irq_eoc, n; >>> unsigned int n? >>>> + >>>> + regmap = dev_get_regmap(dev->parent, NULL); >>>> + if (!regmap) >>>> + return -ENODEV; >>>> + >>>> + indio_dev = devm_iio_device_alloc(dev, sizeof(*vadc)); >>>> + if (!indio_dev) >>>> + return -ENOMEM; >>>> + >>>> + vadc = iio_priv(indio_dev); >>>> + vadc->dev = dev; >>>> + vadc->regmap = regmap; >>>> + vadc->is_ref_measured = false; >>>> + init_completion(&vadc->complete); >>>> + >>>> + vadc->nchannels = ARRAY_SIZE(vadc_channels); >>>> + vadc->channels = devm_kcalloc(dev, vadc->nchannels, >>>> + sizeof(*vadc->channels), GFP_KERNEL); >> Interesting. This is always the same size as the vadc_channels so >you >> might as well keep them in the same order and simplify various >corners of >> the code. >>>> + if (!vadc->channels) >>>> + return -ENOMEM; >>>> + >> I wonder if we can't vadc->channels rather more different from >vadc_channels? >> Perhaps vadc->channelspriv or similar? Confused me a little at first. >>>> + for (n = 0; n < vadc->nchannels; n++) { >>>> + vchan = &vadc->channels[n]; >>>> + /* set default channel properties */ >>>> + vchan->number = -1; /* inactive */ >>>> + vchan->prescaling = vadc_channels[n].address; >>>> + vchan->decimation = VADC_DEF_DECIMATION; >>>> + vchan->hw_settle_time = VADC_DEF_HW_SETTLE_TIME; >>>> + vchan->avg_samples = VADC_DEF_AVG_SAMPLES; >>>> + vchan->calibration = VADC_DEF_CALIB_TYPE; >>>> + } >>>> + >>>> + platform_set_drvdata(pdev, vadc); >>>> + >>>> + res = platform_get_resource(pdev, IORESOURCE_REG, 0); >>>> + if (!res) >>>> + return -ENODEV; >>>> + >>>> + vadc->base = res->start; >>>> + >>>> + rc = vadc_version_check(vadc); >>>> + if (rc < 0) >>>> + return -ENODEV; >>>> + >>>> + rc = vadc_get_dt_data(vadc, node); >>>> + if (rc < 0) >>>> + return rc; >>>> + >> Do we need an explicit flag to indicate poll mode rather than just >> using the absense of the irq being specified to select that mode? > >I will think about this. Maybe I will check the returned value from >platform_get_irq to see is the IRQ resource exist. If the IRQ doesn't >exist I will fallback to polling. That was my thought. > ><snip> -- Sent from my Android phone with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <4dbc485f-599a-4b50-854c-c2e1c44d4810-2ueSQiBKiTY7tOexoI0I+QC/G2K4zDHf@public.gmane.org>]
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-15 16:11 ` Jonathan Cameron @ 2014-09-18 9:57 ` Stanimir Varbanov -1 siblings, 0 replies; 23+ messages in thread From: Stanimir Varbanov @ 2014-09-18 9:57 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins Hi Jonathan, On 09/15/2014 07:11 PM, Jonathan Cameron wrote: > > > On September 15, 2014 3:12:50 PM GMT+01:00, Stanimir Varbanov <svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> wrote: >> Hi Jonathan, >> >> Thanks for the review! >> >> On 09/13/2014 08:27 PM, Jonathan Cameron wrote: >>> On 13/09/14 00:27, Hartmut Knaack wrote: >>>> Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >>>>> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >>>>> 15bits resolution and register space inside PMIC accessible across >>>>> SPMI bus. >>>>> >>>>> The vadc driver registers itself through IIO interface. >>>>> >>>> Looks already pretty good. Things you should consider in regard of >> common coding style are to use the variable name ret instead of rc, >> since it is used in almost all adc drivers and thus makes reviewing a >> bit easier. Besides that, you seem to use unsigned as well as unsigned >> int, so to be consistent, please stick to one of them. Other comments >> in line. >>> >>> A few additional comments from me. My biggest question is whether >>> you are actually making life difficult for yourself by having >>> vadc_channels and vadc->channels (don't like the similar naming btw!) >>> in different orders. I think you can move the ordering into the >> device >>> tree reading code rather than doing it in lots of other places. >> Hence >>> rather than an order based on the device tree description, put the >>> data into a fixed ofer in vadc->channels. >>> >>> Entirely possible I'm missing something though :) >>>>> Signed-off-by: Stanimir Varbanov <svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >>>>> Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >>>>> --- >>>>> drivers/iio/adc/Kconfig | 11 + >>>>> drivers/iio/adc/Makefile | 1 + >>>>> drivers/iio/adc/qcom-spmi-vadc.c | 999 >> +++++++++++++++++++++++++ >>>>> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >>>>> 4 files changed, 1130 insertions(+), 0 deletions(-) >>>>> create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c >>>>> create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h <snip> >>>>> + vchan = vadc_find_channel(vadc, chan->channel); >>>>> + if (!vchan) >>>>> + return -EINVAL; >>>>> + >>>>> + if (!vadc->is_ref_measured) { >>>>> + rc = vadc_measure_reference_points(vadc); >>>>> + if (rc) >>>>> + return rc; >>>>> + >>>>> + vadc->is_ref_measured = true; >>>>> + } >>>>> + >>>>> + switch (mask) { >>>>> + case IIO_CHAN_INFO_PROCESSED: >>>>> + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); >>>>> + if (rc) >>>>> + return rc; >>>>> + >>>>> + vadc_calibrate(vadc, vchan, &result); >>>>> + >>>>> + *val = result.physical; >>> I'm a little suspicious here. Are the resulting values in milivolts >> for >>> all the channels? Very handy if so, but seems a little unlikely with >> 15 bit >>> ADC that you'd have no part of greater accuracy than a milivolt. >> >> In fact *val is in microvolts. What is the expected unit from IIO ADC >> users? > See Documentation/ABI/sysfs-bus-iio > Millivolts I think... We copied hwmon where possible. I'm a bit confused about these units. I searched references of iio_read_channel_processed() and found a few. The iio_hwmon expecting milivolts. On the other side lp8788-charger.c registers a get_property method in charger-manager.c, which expects microvolts in get_batt_uV(). I also wonder how to implement IIO read_raw() method so as not to lose precision (if assume we must return millivolts). As far I can see it should be some combination of IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_SCALE masks. Any hints? >> >>>>> + rc = IIO_VAL_INT; >>>> return IIO_VAL_INT; >>>>> + break; >>>>> + default: >>>>> + rc = -EINVAL; >>>>> + break; >>>> Drop default case, or leave empty. >>>>> + } >>>>> + >>>>> + return rc; >>>> return -EINVAL; >>>>> +} -- regards, Stan ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver @ 2014-09-18 9:57 ` Stanimir Varbanov 0 siblings, 0 replies; 23+ messages in thread From: Stanimir Varbanov @ 2014-09-18 9:57 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov Hi Jonathan, On 09/15/2014 07:11 PM, Jonathan Cameron wrote: > > > On September 15, 2014 3:12:50 PM GMT+01:00, Stanimir Varbanov <svarbanov@mm-sol.com> wrote: >> Hi Jonathan, >> >> Thanks for the review! >> >> On 09/13/2014 08:27 PM, Jonathan Cameron wrote: >>> On 13/09/14 00:27, Hartmut Knaack wrote: >>>> Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >>>>> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >>>>> 15bits resolution and register space inside PMIC accessible across >>>>> SPMI bus. >>>>> >>>>> The vadc driver registers itself through IIO interface. >>>>> >>>> Looks already pretty good. Things you should consider in regard of >> common coding style are to use the variable name ret instead of rc, >> since it is used in almost all adc drivers and thus makes reviewing a >> bit easier. Besides that, you seem to use unsigned as well as unsigned >> int, so to be consistent, please stick to one of them. Other comments >> in line. >>> >>> A few additional comments from me. My biggest question is whether >>> you are actually making life difficult for yourself by having >>> vadc_channels and vadc->channels (don't like the similar naming btw!) >>> in different orders. I think you can move the ordering into the >> device >>> tree reading code rather than doing it in lots of other places. >> Hence >>> rather than an order based on the device tree description, put the >>> data into a fixed ofer in vadc->channels. >>> >>> Entirely possible I'm missing something though :) >>>>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> >>>>> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> >>>>> --- >>>>> drivers/iio/adc/Kconfig | 11 + >>>>> drivers/iio/adc/Makefile | 1 + >>>>> drivers/iio/adc/qcom-spmi-vadc.c | 999 >> +++++++++++++++++++++++++ >>>>> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >>>>> 4 files changed, 1130 insertions(+), 0 deletions(-) >>>>> create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c >>>>> create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h <snip> >>>>> + vchan = vadc_find_channel(vadc, chan->channel); >>>>> + if (!vchan) >>>>> + return -EINVAL; >>>>> + >>>>> + if (!vadc->is_ref_measured) { >>>>> + rc = vadc_measure_reference_points(vadc); >>>>> + if (rc) >>>>> + return rc; >>>>> + >>>>> + vadc->is_ref_measured = true; >>>>> + } >>>>> + >>>>> + switch (mask) { >>>>> + case IIO_CHAN_INFO_PROCESSED: >>>>> + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); >>>>> + if (rc) >>>>> + return rc; >>>>> + >>>>> + vadc_calibrate(vadc, vchan, &result); >>>>> + >>>>> + *val = result.physical; >>> I'm a little suspicious here. Are the resulting values in milivolts >> for >>> all the channels? Very handy if so, but seems a little unlikely with >> 15 bit >>> ADC that you'd have no part of greater accuracy than a milivolt. >> >> In fact *val is in microvolts. What is the expected unit from IIO ADC >> users? > See Documentation/ABI/sysfs-bus-iio > Millivolts I think... We copied hwmon where possible. I'm a bit confused about these units. I searched references of iio_read_channel_processed() and found a few. The iio_hwmon expecting milivolts. On the other side lp8788-charger.c registers a get_property method in charger-manager.c, which expects microvolts in get_batt_uV(). I also wonder how to implement IIO read_raw() method so as not to lose precision (if assume we must return millivolts). As far I can see it should be some combination of IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_SCALE masks. Any hints? >> >>>>> + rc = IIO_VAL_INT; >>>> return IIO_VAL_INT; >>>>> + break; >>>>> + default: >>>>> + rc = -EINVAL; >>>>> + break; >>>> Drop default case, or leave empty. >>>>> + } >>>>> + >>>>> + return rc; >>>> return -EINVAL; >>>>> +} -- regards, Stan ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <541AAC80.1090708-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-18 9:57 ` Stanimir Varbanov @ 2014-09-21 13:29 ` Jonathan Cameron -1 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-09-21 13:29 UTC (permalink / raw) To: Stanimir Varbanov Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins On 18/09/14 10:57, Stanimir Varbanov wrote: > Hi Jonathan, > > On 09/15/2014 07:11 PM, Jonathan Cameron wrote: >> >> >> On September 15, 2014 3:12:50 PM GMT+01:00, Stanimir Varbanov <svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> wrote: >>> Hi Jonathan, >>> >>> Thanks for the review! >>> >>> On 09/13/2014 08:27 PM, Jonathan Cameron wrote: >>>> On 13/09/14 00:27, Hartmut Knaack wrote: >>>>> Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >>>>>> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >>>>>> 15bits resolution and register space inside PMIC accessible across >>>>>> SPMI bus. >>>>>> >>>>>> The vadc driver registers itself through IIO interface. >>>>>> >>>>> Looks already pretty good. Things you should consider in regard of >>> common coding style are to use the variable name ret instead of rc, >>> since it is used in almost all adc drivers and thus makes reviewing a >>> bit easier. Besides that, you seem to use unsigned as well as unsigned >>> int, so to be consistent, please stick to one of them. Other comments >>> in line. >>>> >>>> A few additional comments from me. My biggest question is whether >>>> you are actually making life difficult for yourself by having >>>> vadc_channels and vadc->channels (don't like the similar naming btw!) >>>> in different orders. I think you can move the ordering into the >>> device >>>> tree reading code rather than doing it in lots of other places. >>> Hence >>>> rather than an order based on the device tree description, put the >>>> data into a fixed ofer in vadc->channels. >>>> >>>> Entirely possible I'm missing something though :) >>>>>> Signed-off-by: Stanimir Varbanov <svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >>>>>> Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >>>>>> --- >>>>>> drivers/iio/adc/Kconfig | 11 + >>>>>> drivers/iio/adc/Makefile | 1 + >>>>>> drivers/iio/adc/qcom-spmi-vadc.c | 999 >>> +++++++++++++++++++++++++ >>>>>> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >>>>>> 4 files changed, 1130 insertions(+), 0 deletions(-) >>>>>> create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c >>>>>> create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h > > <snip> > >>>>>> + vchan = vadc_find_channel(vadc, chan->channel); >>>>>> + if (!vchan) >>>>>> + return -EINVAL; >>>>>> + >>>>>> + if (!vadc->is_ref_measured) { >>>>>> + rc = vadc_measure_reference_points(vadc); >>>>>> + if (rc) >>>>>> + return rc; >>>>>> + >>>>>> + vadc->is_ref_measured = true; >>>>>> + } >>>>>> + >>>>>> + switch (mask) { >>>>>> + case IIO_CHAN_INFO_PROCESSED: >>>>>> + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); >>>>>> + if (rc) >>>>>> + return rc; >>>>>> + >>>>>> + vadc_calibrate(vadc, vchan, &result); >>>>>> + >>>>>> + *val = result.physical; >>>> I'm a little suspicious here. Are the resulting values in milivolts >>> for >>>> all the channels? Very handy if so, but seems a little unlikely with >>> 15 bit >>>> ADC that you'd have no part of greater accuracy than a milivolt. >>> >>> In fact *val is in microvolts. What is the expected unit from IIO ADC >>> users? >> See Documentation/ABI/sysfs-bus-iio >> Millivolts I think... We copied hwmon where possible. > > I'm a bit confused about these units. I searched references of > iio_read_channel_processed() and found a few. > > The iio_hwmon expecting milivolts. On the other side lp8788-charger.c > registers a get_property method in charger-manager.c, which expects > microvolts in get_batt_uV(). It's definitely meant to be millivolts (lifted from hwmon a while back). See Documentation/ABI/testing/sysfs-bus-iio Looks like we have a bug in lp8788-charger - it might be matched with one in lp8788-adc, but then there will be a bug there... Cc'd Milo Kim. > > I also wonder how to implement IIO read_raw() method so as not to lose > precision (if assume we must return millivolts). As far I can see it > should be some combination of IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_SCALE > masks. Any hints? Use val and val2 and a return type of IIO_VAL_INT_PLUS_MICRO. So if you have n microvolts val = n div 1000; val2 = n mod 1000 * 1000; This makes sense if you are returning a processed value. Otherwise, use a scale infomask element (and support in read raw) to ensure that rawvalue*scale is in millivolts So if n is in microvolts it would simply be 1000. (this would all have been more obvious if we hadn't copied hwmon and had just used volts, but such is history). As for in kernel users... iio_read_channel_raw and iio_read_channel_processed assume we are looking for an integer value out, so you'll want to call iio_read_channel_raw and apply iio_convert_raw_to_processed with appropriately adjusted scale value. > >>> >>>>>> + rc = IIO_VAL_INT; >>>>> return IIO_VAL_INT; >>>>>> + break; >>>>>> + default: >>>>>> + rc = -EINVAL; >>>>>> + break; >>>>> Drop default case, or leave empty. >>>>>> + } >>>>>> + >>>>>> + return rc; >>>>> return -EINVAL; >>>>>> +} > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver @ 2014-09-21 13:29 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-09-21 13:29 UTC (permalink / raw) To: Stanimir Varbanov Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov, Kim, Milo On 18/09/14 10:57, Stanimir Varbanov wrote: > Hi Jonathan, > > On 09/15/2014 07:11 PM, Jonathan Cameron wrote: >> >> >> On September 15, 2014 3:12:50 PM GMT+01:00, Stanimir Varbanov <svarbanov@mm-sol.com> wrote: >>> Hi Jonathan, >>> >>> Thanks for the review! >>> >>> On 09/13/2014 08:27 PM, Jonathan Cameron wrote: >>>> On 13/09/14 00:27, Hartmut Knaack wrote: >>>>> Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >>>>>> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >>>>>> 15bits resolution and register space inside PMIC accessible across >>>>>> SPMI bus. >>>>>> >>>>>> The vadc driver registers itself through IIO interface. >>>>>> >>>>> Looks already pretty good. Things you should consider in regard of >>> common coding style are to use the variable name ret instead of rc, >>> since it is used in almost all adc drivers and thus makes reviewing a >>> bit easier. Besides that, you seem to use unsigned as well as unsigned >>> int, so to be consistent, please stick to one of them. Other comments >>> in line. >>>> >>>> A few additional comments from me. My biggest question is whether >>>> you are actually making life difficult for yourself by having >>>> vadc_channels and vadc->channels (don't like the similar naming btw!) >>>> in different orders. I think you can move the ordering into the >>> device >>>> tree reading code rather than doing it in lots of other places. >>> Hence >>>> rather than an order based on the device tree description, put the >>>> data into a fixed ofer in vadc->channels. >>>> >>>> Entirely possible I'm missing something though :) >>>>>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> >>>>>> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> >>>>>> --- >>>>>> drivers/iio/adc/Kconfig | 11 + >>>>>> drivers/iio/adc/Makefile | 1 + >>>>>> drivers/iio/adc/qcom-spmi-vadc.c | 999 >>> +++++++++++++++++++++++++ >>>>>> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >>>>>> 4 files changed, 1130 insertions(+), 0 deletions(-) >>>>>> create mode 100644 drivers/iio/adc/qcom-spmi-vadc.c >>>>>> create mode 100644 include/dt-bindings/iio/qcom,spmi-pmic-vadc.h > > <snip> > >>>>>> + vchan = vadc_find_channel(vadc, chan->channel); >>>>>> + if (!vchan) >>>>>> + return -EINVAL; >>>>>> + >>>>>> + if (!vadc->is_ref_measured) { >>>>>> + rc = vadc_measure_reference_points(vadc); >>>>>> + if (rc) >>>>>> + return rc; >>>>>> + >>>>>> + vadc->is_ref_measured = true; >>>>>> + } >>>>>> + >>>>>> + switch (mask) { >>>>>> + case IIO_CHAN_INFO_PROCESSED: >>>>>> + rc = vadc_do_conversion(vadc, vchan, &result.adc_code); >>>>>> + if (rc) >>>>>> + return rc; >>>>>> + >>>>>> + vadc_calibrate(vadc, vchan, &result); >>>>>> + >>>>>> + *val = result.physical; >>>> I'm a little suspicious here. Are the resulting values in milivolts >>> for >>>> all the channels? Very handy if so, but seems a little unlikely with >>> 15 bit >>>> ADC that you'd have no part of greater accuracy than a milivolt. >>> >>> In fact *val is in microvolts. What is the expected unit from IIO ADC >>> users? >> See Documentation/ABI/sysfs-bus-iio >> Millivolts I think... We copied hwmon where possible. > > I'm a bit confused about these units. I searched references of > iio_read_channel_processed() and found a few. > > The iio_hwmon expecting milivolts. On the other side lp8788-charger.c > registers a get_property method in charger-manager.c, which expects > microvolts in get_batt_uV(). It's definitely meant to be millivolts (lifted from hwmon a while back). See Documentation/ABI/testing/sysfs-bus-iio Looks like we have a bug in lp8788-charger - it might be matched with one in lp8788-adc, but then there will be a bug there... Cc'd Milo Kim. > > I also wonder how to implement IIO read_raw() method so as not to lose > precision (if assume we must return millivolts). As far I can see it > should be some combination of IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_SCALE > masks. Any hints? Use val and val2 and a return type of IIO_VAL_INT_PLUS_MICRO. So if you have n microvolts val = n div 1000; val2 = n mod 1000 * 1000; This makes sense if you are returning a processed value. Otherwise, use a scale infomask element (and support in read raw) to ensure that rawvalue*scale is in millivolts So if n is in microvolts it would simply be 1000. (this would all have been more obvious if we hadn't copied hwmon and had just used volts, but such is history). As for in kernel users... iio_read_channel_raw and iio_read_channel_processed assume we are looking for an integer value out, so you'll want to call iio_read_channel_raw and apply iio_convert_raw_to_processed with appropriately adjusted scale value. > >>> >>>>>> + rc = IIO_VAL_INT; >>>>> return IIO_VAL_INT; >>>>>> + break; >>>>>> + default: >>>>>> + rc = -EINVAL; >>>>>> + break; >>>>> Drop default case, or leave empty. >>>>>> + } >>>>>> + >>>>>> + return rc; >>>>> return -EINVAL; >>>>>> +} > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-21 13:29 ` Jonathan Cameron (?) @ 2014-09-22 1:01 ` Kim, Milo -1 siblings, 0 replies; 23+ messages in thread From: Kim, Milo @ 2014-09-22 1:01 UTC (permalink / raw) To: Jonathan Cameron, Stanimir Varbanov Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Hello Jonathan and Stanimir, > >> See Documentation/ABI/sysfs-bus-iio > >> Millivolts I think... We copied hwmon where possible. > > > > I'm a bit confused about these units. I searched references of > > iio_read_channel_processed() and found a few. > > > > The iio_hwmon expecting milivolts. On the other side lp8788-charger.c > > registers a get_property method in charger-manager.c, which expects > > microvolts in get_batt_uV(). > It's definitely meant to be millivolts (lifted from hwmon a while back). > See Documentation/ABI/testing/sysfs-bus-iio > > Looks like we have a bug in lp8788-charger - it might be matched with one in > lp8788-adc, but then there will be a bug there... > > Cc'd Milo Kim. 'lp8788-charger' registers not charger-manager but power-supply subsystem. 'lp8788-adc' is the IIO driver. 'lp8788-charger' is the IIO consumer of lp8788-adc. (How to communicate between lp8788-adc and lp8788-charger) 1. Application requests vbatt(battery voltage) by accessing /sys/class/power/ 2. lp8788-charger asks lp8788-adc to get converted vbatt value 3. lp8788-adc get the battery ADC from the device. It returns calculated micro voltage to lp8788-charger. 4. lp8788-charger reports this uV value to the application 5. Application uses uV or converts to mV or V unit. It's up to the app. Please note that battery app accesses not /sys/bus/iio but /sys/class/power/. Best regards, Milo ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver @ 2014-09-22 1:01 ` Kim, Milo 0 siblings, 0 replies; 23+ messages in thread From: Kim, Milo @ 2014-09-22 1:01 UTC (permalink / raw) To: Jonathan Cameron, Stanimir Varbanov Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov SGVsbG8gSm9uYXRoYW4gYW5kIFN0YW5pbWlyLA0KDQo+ID4+IFNlZSBEb2N1bWVudGF0aW9uL0FC SS9zeXNmcy1idXMtaWlvDQo+ID4+IE1pbGxpdm9sdHMgSSB0aGluay4uLiBXZSBjb3BpZWQgaHdt b24gd2hlcmUgcG9zc2libGUuDQo+ID4NCj4gPiBJJ20gYSBiaXQgY29uZnVzZWQgYWJvdXQgdGhl c2UgdW5pdHMuIEkgc2VhcmNoZWQgcmVmZXJlbmNlcyBvZg0KPiA+IGlpb19yZWFkX2NoYW5uZWxf cHJvY2Vzc2VkKCkgYW5kIGZvdW5kIGEgZmV3Lg0KPiA+DQo+ID4gVGhlIGlpb19od21vbiBleHBl Y3RpbmcgbWlsaXZvbHRzLiBPbiB0aGUgb3RoZXIgc2lkZSBscDg3ODgtY2hhcmdlci5jDQo+ID4g cmVnaXN0ZXJzIGEgZ2V0X3Byb3BlcnR5IG1ldGhvZCBpbiBjaGFyZ2VyLW1hbmFnZXIuYywgd2hp Y2ggZXhwZWN0cw0KPiA+IG1pY3Jvdm9sdHMgaW4gZ2V0X2JhdHRfdVYoKS4NCj4gSXQncyBkZWZp bml0ZWx5IG1lYW50IHRvIGJlIG1pbGxpdm9sdHMgKGxpZnRlZCBmcm9tIGh3bW9uIGEgd2hpbGUg YmFjaykuDQo+IFNlZSBEb2N1bWVudGF0aW9uL0FCSS90ZXN0aW5nL3N5c2ZzLWJ1cy1paW8NCj4g DQo+IExvb2tzIGxpa2Ugd2UgaGF2ZSBhIGJ1ZyBpbiBscDg3ODgtY2hhcmdlciAtIGl0IG1pZ2h0 IGJlIG1hdGNoZWQgd2l0aCBvbmUgaW4NCj4gbHA4Nzg4LWFkYywgYnV0IHRoZW4gdGhlcmUgd2ls bCBiZSBhIGJ1ZyB0aGVyZS4uLg0KPiANCj4gQ2MnZCBNaWxvIEtpbS4NCg0KJ2xwODc4OC1jaGFy Z2VyJyByZWdpc3RlcnMgbm90IGNoYXJnZXItbWFuYWdlciBidXQgcG93ZXItc3VwcGx5IHN1YnN5 c3RlbS4NCidscDg3ODgtYWRjJyBpcyB0aGUgSUlPIGRyaXZlci4NCidscDg3ODgtY2hhcmdlcicg aXMgdGhlIElJTyBjb25zdW1lciBvZiBscDg3ODgtYWRjLg0KDQooSG93IHRvIGNvbW11bmljYXRl IGJldHdlZW4gbHA4Nzg4LWFkYyBhbmQgbHA4Nzg4LWNoYXJnZXIpDQoxLiBBcHBsaWNhdGlvbiBy ZXF1ZXN0cyB2YmF0dChiYXR0ZXJ5IHZvbHRhZ2UpIGJ5IGFjY2Vzc2luZyAvc3lzL2NsYXNzL3Bv d2VyLw0KMi4gbHA4Nzg4LWNoYXJnZXIgYXNrcyBscDg3ODgtYWRjIHRvIGdldCBjb252ZXJ0ZWQg dmJhdHQgdmFsdWUNCjMuIGxwODc4OC1hZGMgZ2V0IHRoZSBiYXR0ZXJ5IEFEQyBmcm9tIHRoZSBk ZXZpY2UuDQogICBJdCByZXR1cm5zIGNhbGN1bGF0ZWQgbWljcm8gdm9sdGFnZSB0byBscDg3ODgt Y2hhcmdlci4NCjQuIGxwODc4OC1jaGFyZ2VyIHJlcG9ydHMgdGhpcyB1ViB2YWx1ZSB0byB0aGUg YXBwbGljYXRpb24NCjUuIEFwcGxpY2F0aW9uIHVzZXMgdVYgb3IgY29udmVydHMgdG8gbVYgb3Ig ViB1bml0LiBJdCdzIHVwIHRvIHRoZSBhcHAuDQoNClBsZWFzZSBub3RlIHRoYXQgYmF0dGVyeSBh cHAgYWNjZXNzZXMgbm90IC9zeXMvYnVzL2lpbyBidXQgL3N5cy9jbGFzcy9wb3dlci8uDQoNCkJl c3QgcmVnYXJkcywNCk1pbG8NCg== ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver @ 2014-09-22 1:01 ` Kim, Milo 0 siblings, 0 replies; 23+ messages in thread From: Kim, Milo @ 2014-09-22 1:01 UTC (permalink / raw) To: Jonathan Cameron, Stanimir Varbanov Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1569 bytes --] Hello Jonathan and Stanimir, > >> See Documentation/ABI/sysfs-bus-iio > >> Millivolts I think... We copied hwmon where possible. > > > > I'm a bit confused about these units. I searched references of > > iio_read_channel_processed() and found a few. > > > > The iio_hwmon expecting milivolts. On the other side lp8788-charger.c > > registers a get_property method in charger-manager.c, which expects > > microvolts in get_batt_uV(). > It's definitely meant to be millivolts (lifted from hwmon a while back). > See Documentation/ABI/testing/sysfs-bus-iio > > Looks like we have a bug in lp8788-charger - it might be matched with one in > lp8788-adc, but then there will be a bug there... > > Cc'd Milo Kim. 'lp8788-charger' registers not charger-manager but power-supply subsystem. 'lp8788-adc' is the IIO driver. 'lp8788-charger' is the IIO consumer of lp8788-adc. (How to communicate between lp8788-adc and lp8788-charger) 1. Application requests vbatt(battery voltage) by accessing /sys/class/power/ 2. lp8788-charger asks lp8788-adc to get converted vbatt value 3. lp8788-adc get the battery ADC from the device. It returns calculated micro voltage to lp8788-charger. 4. lp8788-charger reports this uV value to the application 5. Application uses uV or converts to mV or V unit. It's up to the app. Please note that battery app accesses not /sys/bus/iio but /sys/class/power/. Best regards, Milo ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-22 1:01 ` Kim, Milo (?) (?) @ 2014-09-22 6:28 ` Jonathan Cameron -1 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-09-22 6:28 UTC (permalink / raw) To: Kim, Milo, Stanimir Varbanov Cc: Hartmut Knaack, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann, linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David <collinsd@codeaurora.org>,"Ivan T. Ivanov" <iivanov@mm-sol.com> Message-ID: <4f726fe9-707e-41bd-bcc7-188516b6f47a@email.android.com> On September 22, 2014 2:01:12 AM GMT+01:00, "Kim, Milo" <Milo.Kim@ti.com> wrote: >Hello Jonathan and Stanimir, > >> >> See Documentation/ABI/sysfs-bus-iio >> >> Millivolts I think... We copied hwmon where possible. >> > >> > I'm a bit confused about these units. I searched references of >> > iio_read_channel_processed() and found a few. >> > >> > The iio_hwmon expecting milivolts. On the other side >lp8788-charger.c >> > registers a get_property method in charger-manager.c, which expects >> > microvolts in get_batt_uV(). >> It's definitely meant to be millivolts (lifted from hwmon a while >back). >> See Documentation/ABI/testing/sysfs-bus-iio >> >> Looks like we have a bug in lp8788-charger - it might be matched with >one in >> lp8788-adc, but then there will be a bug there... >> >> Cc'd Milo Kim. > >'lp8788-charger' registers not charger-manager but power-supply >subsystem. >'lp8788-adc' is the IIO driver. >'lp8788-charger' is the IIO consumer of lp8788-adc. > >(How to communicate between lp8788-adc and lp8788-charger) >1. Application requests vbatt(battery voltage) by accessing >/sys/class/power/ >2. lp8788-charger asks lp8788-adc to get converted vbatt value >3. lp8788-adc get the battery ADC from the device. > It returns calculated micro voltage to lp8788-charger. It should be in millivolts... See below. >4. lp8788-charger reports this uV value to the application >5. Application uses uV or converts to mV or V unit. It's up to the app. > >Please note that battery app accesses not /sys/bus/iio but >/sys/class/power/. Indeed but the value actually returned is that from iio_read_channel_processed which calls iio_channel_read (or does application of scale locally though the effect is the same) with the processed value. This then calls read_raw and so gets a value that should be in millivolts. Hence the query. Looks to me like the ADC driver has wrong scale value? > >Best regards, >Milo -- Sent from my Android phone with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <54138151.8010902-Mmb7MZpHnFY@public.gmane.org>]
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver 2014-09-12 23:27 ` Hartmut Knaack @ 2014-09-15 14:16 ` Stanimir Varbanov [not found] ` <54138151.8010902-Mmb7MZpHnFY@public.gmane.org> 1 sibling, 0 replies; 23+ messages in thread From: Stanimir Varbanov @ 2014-09-15 14:16 UTC (permalink / raw) To: Hartmut Knaack Cc: Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Jonathan Cameron, Arnd Bergmann, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins Thanks for the comments! On 09/13/2014 02:27 AM, Hartmut Knaack wrote: > Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >> 15bits resolution and register space inside PMIC accessible across >> SPMI bus. >> >> The vadc driver registers itself through IIO interface. >> > Looks already pretty good. Things you should consider in regard of common coding style are to use the variable name ret instead of rc, since it is used in almost all adc drivers and thus makes reviewing a bit easier. Besides that, you seem to use unsigned as well as unsigned int, so to be consistent, please stick to one of them. Other comments in line. OK, I will take into account your comments. <snip> -- regards, Stan ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver @ 2014-09-15 14:16 ` Stanimir Varbanov 0 siblings, 0 replies; 23+ messages in thread From: Stanimir Varbanov @ 2014-09-15 14:16 UTC (permalink / raw) To: Hartmut Knaack Cc: Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Jonathan Cameron, Arnd Bergmann, linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov Thanks for the comments! On 09/13/2014 02:27 AM, Hartmut Knaack wrote: > Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >> 15bits resolution and register space inside PMIC accessible across >> SPMI bus. >> >> The vadc driver registers itself through IIO interface. >> > Looks already pretty good. Things you should consider in regard of common coding style are to use the variable name ret instead of rc, since it is used in almost all adc drivers and thus makes reviewing a bit easier. Besides that, you seem to use unsigned as well as unsigned int, so to be consistent, please stick to one of them. Other comments in line. OK, I will take into account your comments. <snip> -- regards, Stan ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 2/2] DT: iio: vadc: document dt binding 2014-09-11 15:13 [PATCH v2 0/2] Intial support for voltage ADC Stanimir Varbanov 2014-09-11 15:13 ` [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver Stanimir Varbanov @ 2014-09-11 15:13 ` Stanimir Varbanov [not found] ` <1410448403-19402-3-git-send-email-svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> 1 sibling, 1 reply; 23+ messages in thread From: Stanimir Varbanov @ 2014-09-11 15:13 UTC (permalink / raw) To: Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Jonathan Cameron, Arnd Bergmann Cc: linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Hartmut Knaack, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Stanimir Varbanov, Ivan T. Ivanov Document DT binding for Qualcomm SPMI PMIC voltage ADC driver. Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> --- .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt | 123 ++++++++++++++++++++ 1 files changed, 123 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt new file mode 100644 index 0000000..5abb491 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt @@ -0,0 +1,123 @@ +Qualcomm's SPMI PMIC voltage ADC + +SPMI PMIC voltage ADC (VADC) provides interface to clients to read +voltage. A 15 bit ADC is used for voltage measurements. There are multiple +peripherals to the VADC and the scope of the driver is to provide interface +for the USR peripheral of the VADC. + +VADC node: + +- compatible: + Usage: required + Value type: <string> + Definition: Should contain "qcom,spmi-vadc". + +- reg: + Usage: required + Value type: <u32> + Definition: Base address in the SPMI PMIC register map. + +- address-cells: + Usage: required + Value type: <u32> + Definition: Must be one. + +- size-cells: + Usage: required + Value type: <u32> + Definition: Must be zero. + +- interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: End of convertion interrupt number. + +- qcom,poll-eoc: + Usage: optional + Value type: <empty> + Definition: Use polling instead of interrupt for end of conversion + completion. + +Channel node properties: + +- reg: + Usage: required + Value type: <u32> + Definition: AMUX channel number. + See include/dt-bindings/iio/qcom,spmi-pmic-vadc.h + +- qcom,decimation: + Usage: optional + Value type: <u32> + Definition: Sampling rate to use for the individual channel measurement. + Quicker measurements can be made by reducing decimation ratio. + Valid values are 512, 1024, 2048, 4096. + If property is not found, default value of 512 will be used. + +- qcom,pre-scaling: + Usage: optional + Value type: <u32 array> + Definition: Used for scaling the channel input signal before the signal is + fed to VADC. The configuration for this node is to know the + pre-determined ratio and use it for post scaling. Select one from + the following options. + <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10> + If property is not found default value depending of chip will be used. + +- qcom,ratiometric: + Usage: optional + Value type: <empty> + Definition: Channel calibration type. If this property is specified + VADC will use the VDD reference(1.8V) and GND for channel + calibration. If property is not found, channel will be + calibrated with 625mV and 1.25V reference channels. + +- qcom,hw-settle-time: + Usage: optional + Value type: <u32> + Definition: Time between AMUX getting configured and the ADC starting + conversion. Delay = 100us * (value) for value < 11, and + 2ms * (value - 10) otherwise. + Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800, + 900 us and 1, 2, 4, 6, 8, 10 ms + If property is not found, channel will use 0us. + +- qcom,avg-samples: + Usage: optional + Value type: <u32> + Definition: Number of samples to be used for measurement. + Fast averaging provides the option to obtain a single measurement + from the ADC that is an average of multiple samples. The value + selected is 2^(value). + Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512 + If property is not found, 1 sample will be used. + +NOTE: At least one channel node is required. + +Example: + /* VADC node */ + pmic_vadc: vadc@3100 { + compatible = "qcom,spmi-vadc"; + reg = <0x3100 0x100>; + interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>; + #address-cells = <1>; + #size-cells = <0>; + #io-channel-cells = <1>; + io-channel-ranges; + + /* Channel node */ + usb_id_nopull { + reg = <VADC_LR_MUX10_USB_ID>; + qcom,decimation = <512>; + qcom,ratiometric; + qcom,hw-settle-time = <200>; + qcom,avg-samples = <1>; + qcom,pre-scaling = <1 3>; + }; + }; + + /* IIO client node */ + usb { + io-channels = <&pmic_vadc VADC_LR_MUX10_USB_ID>; + io-channel-names = "vadc"; + }; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1410448403-19402-3-git-send-email-svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v2 2/2] DT: iio: vadc: document dt binding 2014-09-11 15:13 ` [PATCH v2 2/2] DT: iio: vadc: document dt binding Stanimir Varbanov @ 2014-09-12 23:35 ` Hartmut Knaack 0 siblings, 0 replies; 23+ messages in thread From: Hartmut Knaack @ 2014-09-12 23:35 UTC (permalink / raw) To: Stanimir Varbanov, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Jonathan Cameron, Arnd Bergmann Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov Stanimir Varbanov schrieb, Am 11.09.2014 17:13: > Document DT binding for Qualcomm SPMI PMIC voltage ADC > driver. > Still one typo left. > Signed-off-by: Stanimir Varbanov <svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> > Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> > --- > .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt | 123 ++++++++++++++++++++ > 1 files changed, 123 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt > > diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt > new file mode 100644 > index 0000000..5abb491 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt > @@ -0,0 +1,123 @@ > +Qualcomm's SPMI PMIC voltage ADC > + > +SPMI PMIC voltage ADC (VADC) provides interface to clients to read > +voltage. A 15 bit ADC is used for voltage measurements. There are multiple > +peripherals to the VADC and the scope of the driver is to provide interface > +for the USR peripheral of the VADC. > + > +VADC node: > + > +- compatible: > + Usage: required > + Value type: <string> > + Definition: Should contain "qcom,spmi-vadc". > + > +- reg: > + Usage: required > + Value type: <u32> > + Definition: Base address in the SPMI PMIC register map. > + > +- address-cells: > + Usage: required > + Value type: <u32> > + Definition: Must be one. > + > +- size-cells: > + Usage: required > + Value type: <u32> > + Definition: Must be zero. > + > +- interrupts: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: End of convertion interrupt number. Typo: conversion > + > +- qcom,poll-eoc: > + Usage: optional > + Value type: <empty> > + Definition: Use polling instead of interrupt for end of conversion > + completion. > + > +Channel node properties: > + > +- reg: > + Usage: required > + Value type: <u32> > + Definition: AMUX channel number. > + See include/dt-bindings/iio/qcom,spmi-pmic-vadc.h > + > +- qcom,decimation: > + Usage: optional > + Value type: <u32> > + Definition: Sampling rate to use for the individual channel measurement. > + Quicker measurements can be made by reducing decimation ratio. > + Valid values are 512, 1024, 2048, 4096. > + If property is not found, default value of 512 will be used. > + > +- qcom,pre-scaling: > + Usage: optional > + Value type: <u32 array> > + Definition: Used for scaling the channel input signal before the signal is > + fed to VADC. The configuration for this node is to know the > + pre-determined ratio and use it for post scaling. Select one from > + the following options. > + <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10> > + If property is not found default value depending of chip will be used. > + > +- qcom,ratiometric: > + Usage: optional > + Value type: <empty> > + Definition: Channel calibration type. If this property is specified > + VADC will use the VDD reference(1.8V) and GND for channel > + calibration. If property is not found, channel will be > + calibrated with 625mV and 1.25V reference channels. > + > +- qcom,hw-settle-time: > + Usage: optional > + Value type: <u32> > + Definition: Time between AMUX getting configured and the ADC starting > + conversion. Delay = 100us * (value) for value < 11, and > + 2ms * (value - 10) otherwise. > + Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800, > + 900 us and 1, 2, 4, 6, 8, 10 ms > + If property is not found, channel will use 0us. > + > +- qcom,avg-samples: > + Usage: optional > + Value type: <u32> > + Definition: Number of samples to be used for measurement. > + Fast averaging provides the option to obtain a single measurement > + from the ADC that is an average of multiple samples. The value > + selected is 2^(value). > + Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512 > + If property is not found, 1 sample will be used. > + > +NOTE: At least one channel node is required. > + > +Example: > + /* VADC node */ > + pmic_vadc: vadc@3100 { > + compatible = "qcom,spmi-vadc"; > + reg = <0x3100 0x100>; > + interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>; > + #address-cells = <1>; > + #size-cells = <0>; > + #io-channel-cells = <1>; > + io-channel-ranges; > + > + /* Channel node */ > + usb_id_nopull { > + reg = <VADC_LR_MUX10_USB_ID>; > + qcom,decimation = <512>; > + qcom,ratiometric; > + qcom,hw-settle-time = <200>; > + qcom,avg-samples = <1>; > + qcom,pre-scaling = <1 3>; > + }; > + }; > + > + /* IIO client node */ > + usb { > + io-channels = <&pmic_vadc VADC_LR_MUX10_USB_ID>; > + io-channel-names = "vadc"; > + }; > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/2] DT: iio: vadc: document dt binding @ 2014-09-12 23:35 ` Hartmut Knaack 0 siblings, 0 replies; 23+ messages in thread From: Hartmut Knaack @ 2014-09-12 23:35 UTC (permalink / raw) To: Stanimir Varbanov, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Jonathan Cameron, Arnd Bergmann Cc: linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov Stanimir Varbanov schrieb, Am 11.09.2014 17:13: > Document DT binding for Qualcomm SPMI PMIC voltage ADC > driver. > Still one typo left. > Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> > --- > .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt | 123 ++++++++++++++++++++ > 1 files changed, 123 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt > > diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt > new file mode 100644 > index 0000000..5abb491 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt > @@ -0,0 +1,123 @@ > +Qualcomm's SPMI PMIC voltage ADC > + > +SPMI PMIC voltage ADC (VADC) provides interface to clients to read > +voltage. A 15 bit ADC is used for voltage measurements. There are multiple > +peripherals to the VADC and the scope of the driver is to provide interface > +for the USR peripheral of the VADC. > + > +VADC node: > + > +- compatible: > + Usage: required > + Value type: <string> > + Definition: Should contain "qcom,spmi-vadc". > + > +- reg: > + Usage: required > + Value type: <u32> > + Definition: Base address in the SPMI PMIC register map. > + > +- address-cells: > + Usage: required > + Value type: <u32> > + Definition: Must be one. > + > +- size-cells: > + Usage: required > + Value type: <u32> > + Definition: Must be zero. > + > +- interrupts: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: End of convertion interrupt number. Typo: conversion > + > +- qcom,poll-eoc: > + Usage: optional > + Value type: <empty> > + Definition: Use polling instead of interrupt for end of conversion > + completion. > + > +Channel node properties: > + > +- reg: > + Usage: required > + Value type: <u32> > + Definition: AMUX channel number. > + See include/dt-bindings/iio/qcom,spmi-pmic-vadc.h > + > +- qcom,decimation: > + Usage: optional > + Value type: <u32> > + Definition: Sampling rate to use for the individual channel measurement. > + Quicker measurements can be made by reducing decimation ratio. > + Valid values are 512, 1024, 2048, 4096. > + If property is not found, default value of 512 will be used. > + > +- qcom,pre-scaling: > + Usage: optional > + Value type: <u32 array> > + Definition: Used for scaling the channel input signal before the signal is > + fed to VADC. The configuration for this node is to know the > + pre-determined ratio and use it for post scaling. Select one from > + the following options. > + <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10> > + If property is not found default value depending of chip will be used. > + > +- qcom,ratiometric: > + Usage: optional > + Value type: <empty> > + Definition: Channel calibration type. If this property is specified > + VADC will use the VDD reference(1.8V) and GND for channel > + calibration. If property is not found, channel will be > + calibrated with 625mV and 1.25V reference channels. > + > +- qcom,hw-settle-time: > + Usage: optional > + Value type: <u32> > + Definition: Time between AMUX getting configured and the ADC starting > + conversion. Delay = 100us * (value) for value < 11, and > + 2ms * (value - 10) otherwise. > + Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800, > + 900 us and 1, 2, 4, 6, 8, 10 ms > + If property is not found, channel will use 0us. > + > +- qcom,avg-samples: > + Usage: optional > + Value type: <u32> > + Definition: Number of samples to be used for measurement. > + Fast averaging provides the option to obtain a single measurement > + from the ADC that is an average of multiple samples. The value > + selected is 2^(value). > + Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512 > + If property is not found, 1 sample will be used. > + > +NOTE: At least one channel node is required. > + > +Example: > + /* VADC node */ > + pmic_vadc: vadc@3100 { > + compatible = "qcom,spmi-vadc"; > + reg = <0x3100 0x100>; > + interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>; > + #address-cells = <1>; > + #size-cells = <0>; > + #io-channel-cells = <1>; > + io-channel-ranges; > + > + /* Channel node */ > + usb_id_nopull { > + reg = <VADC_LR_MUX10_USB_ID>; > + qcom,decimation = <512>; > + qcom,ratiometric; > + qcom,hw-settle-time = <200>; > + qcom,avg-samples = <1>; > + qcom,pre-scaling = <1 3>; > + }; > + }; > + > + /* IIO client node */ > + usb { > + io-channels = <&pmic_vadc VADC_LR_MUX10_USB_ID>; > + io-channel-names = "vadc"; > + }; > ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <5413832F.6040503-Mmb7MZpHnFY@public.gmane.org>]
* Re: [PATCH v2 2/2] DT: iio: vadc: document dt binding 2014-09-12 23:35 ` Hartmut Knaack @ 2014-09-13 17:32 ` Jonathan Cameron -1 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-09-13 17:32 UTC (permalink / raw) To: Hartmut Knaack, Stanimir Varbanov, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov On 13/09/14 00:35, Hartmut Knaack wrote: > Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >> Document DT binding for Qualcomm SPMI PMIC voltage ADC >> driver. >> > Still one typo left. >> Signed-off-by: Stanimir Varbanov <svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >> Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> >> --- >> .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt | 123 ++++++++++++++++++++ >> 1 files changed, 123 insertions(+), 0 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt >> >> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt >> new file mode 100644 >> index 0000000..5abb491 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt >> @@ -0,0 +1,123 @@ >> +Qualcomm's SPMI PMIC voltage ADC >> + >> +SPMI PMIC voltage ADC (VADC) provides interface to clients to read >> +voltage. A 15 bit ADC is used for voltage measurements. There are multiple >> +peripherals to the VADC and the scope of the driver is to provide interface >> +for the USR peripheral of the VADC. >> + >> +VADC node: >> + >> +- compatible: >> + Usage: required >> + Value type: <string> >> + Definition: Should contain "qcom,spmi-vadc". >> + >> +- reg: >> + Usage: required >> + Value type: <u32> >> + Definition: Base address in the SPMI PMIC register map. >> + >> +- address-cells: >> + Usage: required >> + Value type: <u32> >> + Definition: Must be one. >> + >> +- size-cells: >> + Usage: required >> + Value type: <u32> >> + Definition: Must be zero. >> + >> +- interrupts: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: End of convertion interrupt number. > Typo: conversion >> + >> +- qcom,poll-eoc: >> + Usage: optional >> + Value type: <empty> >> + Definition: Use polling instead of interrupt for end of conversion >> + completion. >> + >> +Channel node properties: >> + >> +- reg: >> + Usage: required >> + Value type: <u32> >> + Definition: AMUX channel number. >> + See include/dt-bindings/iio/qcom,spmi-pmic-vadc.h >> + >> +- qcom,decimation: >> + Usage: optional >> + Value type: <u32> >> + Definition: Sampling rate to use for the individual channel measurement. >> + Quicker measurements can be made by reducing decimation ratio. >> + Valid values are 512, 1024, 2048, 4096. >> + If property is not found, default value of 512 will be used. >> + >> +- qcom,pre-scaling: >> + Usage: optional >> + Value type: <u32 array> >> + Definition: Used for scaling the channel input signal before the signal is >> + fed to VADC. The configuration for this node is to know the >> + pre-determined ratio and use it for post scaling. Select one from >> + the following options. >> + <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10> >> + If property is not found default value depending of chip will be used. >> + >> +- qcom,ratiometric: >> + Usage: optional >> + Value type: <empty> >> + Definition: Channel calibration type. If this property is specified >> + VADC will use the VDD reference(1.8V) and GND for channel >> + calibration. If property is not found, channel will be >> + calibrated with 625mV and 1.25V reference channels. >> + >> +- qcom,hw-settle-time: >> + Usage: optional >> + Value type: <u32> >> + Definition: Time between AMUX getting configured and the ADC starting >> + conversion. Delay = 100us * (value) for value < 11, and >> + 2ms * (value - 10) otherwise. >> + Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800, >> + 900 us and 1, 2, 4, 6, 8, 10 ms >> + If property is not found, channel will use 0us. >> + >> +- qcom,avg-samples: >> + Usage: optional >> + Value type: <u32> >> + Definition: Number of samples to be used for measurement. >> + Fast averaging provides the option to obtain a single measurement >> + from the ADC that is an average of multiple samples. The value >> + selected is 2^(value). >> + Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512 >> + If property is not found, 1 sample will be used. >> + >> +NOTE: At least one channel node is required. Why? As far as I can see it will register all the channels anyway. If that is not the intent you need to build the iio_chan_spec array rather than using a static one. I'm fine with it providing channels with sensible defaults if they aren't overridden, but then it should work fine with no channel nodes. Jonathan >> + >> +Example: >> + /* VADC node */ >> + pmic_vadc: vadc@3100 { >> + compatible = "qcom,spmi-vadc"; >> + reg = <0x3100 0x100>; >> + interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + #io-channel-cells = <1>; >> + io-channel-ranges; >> + >> + /* Channel node */ >> + usb_id_nopull { >> + reg = <VADC_LR_MUX10_USB_ID>; >> + qcom,decimation = <512>; >> + qcom,ratiometric; >> + qcom,hw-settle-time = <200>; >> + qcom,avg-samples = <1>; >> + qcom,pre-scaling = <1 3>; >> + }; >> + }; >> + >> + /* IIO client node */ >> + usb { >> + io-channels = <&pmic_vadc VADC_LR_MUX10_USB_ID>; >> + io-channel-names = "vadc"; >> + }; >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/2] DT: iio: vadc: document dt binding @ 2014-09-13 17:32 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-09-13 17:32 UTC (permalink / raw) To: Hartmut Knaack, Stanimir Varbanov, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala, Mark Rutland, Grant Likely, Arnd Bergmann Cc: linux-arm-msm, linux-kernel, linux-iio, devicetree, Greg Kroah-Hartman, Lars-Peter Clausen, Angelo Compagnucci, Doug Anderson, Fugang Duan, Johannes Thumshirn, Jean Delvare, Philippe Reynes, Lee Jones, Josh Cartwright, Stephen Boyd, David Collins, Ivan T. Ivanov On 13/09/14 00:35, Hartmut Knaack wrote: > Stanimir Varbanov schrieb, Am 11.09.2014 17:13: >> Document DT binding for Qualcomm SPMI PMIC voltage ADC >> driver. >> > Still one typo left. >> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com> >> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com> >> --- >> .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt | 123 ++++++++++++++++++++ >> 1 files changed, 123 insertions(+), 0 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt >> >> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt >> new file mode 100644 >> index 0000000..5abb491 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt >> @@ -0,0 +1,123 @@ >> +Qualcomm's SPMI PMIC voltage ADC >> + >> +SPMI PMIC voltage ADC (VADC) provides interface to clients to read >> +voltage. A 15 bit ADC is used for voltage measurements. There are multiple >> +peripherals to the VADC and the scope of the driver is to provide interface >> +for the USR peripheral of the VADC. >> + >> +VADC node: >> + >> +- compatible: >> + Usage: required >> + Value type: <string> >> + Definition: Should contain "qcom,spmi-vadc". >> + >> +- reg: >> + Usage: required >> + Value type: <u32> >> + Definition: Base address in the SPMI PMIC register map. >> + >> +- address-cells: >> + Usage: required >> + Value type: <u32> >> + Definition: Must be one. >> + >> +- size-cells: >> + Usage: required >> + Value type: <u32> >> + Definition: Must be zero. >> + >> +- interrupts: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: End of convertion interrupt number. > Typo: conversion >> + >> +- qcom,poll-eoc: >> + Usage: optional >> + Value type: <empty> >> + Definition: Use polling instead of interrupt for end of conversion >> + completion. >> + >> +Channel node properties: >> + >> +- reg: >> + Usage: required >> + Value type: <u32> >> + Definition: AMUX channel number. >> + See include/dt-bindings/iio/qcom,spmi-pmic-vadc.h >> + >> +- qcom,decimation: >> + Usage: optional >> + Value type: <u32> >> + Definition: Sampling rate to use for the individual channel measurement. >> + Quicker measurements can be made by reducing decimation ratio. >> + Valid values are 512, 1024, 2048, 4096. >> + If property is not found, default value of 512 will be used. >> + >> +- qcom,pre-scaling: >> + Usage: optional >> + Value type: <u32 array> >> + Definition: Used for scaling the channel input signal before the signal is >> + fed to VADC. The configuration for this node is to know the >> + pre-determined ratio and use it for post scaling. Select one from >> + the following options. >> + <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10> >> + If property is not found default value depending of chip will be used. >> + >> +- qcom,ratiometric: >> + Usage: optional >> + Value type: <empty> >> + Definition: Channel calibration type. If this property is specified >> + VADC will use the VDD reference(1.8V) and GND for channel >> + calibration. If property is not found, channel will be >> + calibrated with 625mV and 1.25V reference channels. >> + >> +- qcom,hw-settle-time: >> + Usage: optional >> + Value type: <u32> >> + Definition: Time between AMUX getting configured and the ADC starting >> + conversion. Delay = 100us * (value) for value < 11, and >> + 2ms * (value - 10) otherwise. >> + Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800, >> + 900 us and 1, 2, 4, 6, 8, 10 ms >> + If property is not found, channel will use 0us. >> + >> +- qcom,avg-samples: >> + Usage: optional >> + Value type: <u32> >> + Definition: Number of samples to be used for measurement. >> + Fast averaging provides the option to obtain a single measurement >> + from the ADC that is an average of multiple samples. The value >> + selected is 2^(value). >> + Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512 >> + If property is not found, 1 sample will be used. >> + >> +NOTE: At least one channel node is required. Why? As far as I can see it will register all the channels anyway. If that is not the intent you need to build the iio_chan_spec array rather than using a static one. I'm fine with it providing channels with sensible defaults if they aren't overridden, but then it should work fine with no channel nodes. Jonathan >> + >> +Example: >> + /* VADC node */ >> + pmic_vadc: vadc@3100 { >> + compatible = "qcom,spmi-vadc"; >> + reg = <0x3100 0x100>; >> + interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + #io-channel-cells = <1>; >> + io-channel-ranges; >> + >> + /* Channel node */ >> + usb_id_nopull { >> + reg = <VADC_LR_MUX10_USB_ID>; >> + qcom,decimation = <512>; >> + qcom,ratiometric; >> + qcom,hw-settle-time = <200>; >> + qcom,avg-samples = <1>; >> + qcom,pre-scaling = <1 3>; >> + }; >> + }; >> + >> + /* IIO client node */ >> + usb { >> + io-channels = <&pmic_vadc VADC_LR_MUX10_USB_ID>; >> + io-channel-names = "vadc"; >> + }; >> > > -- > 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 > ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2014-09-22 6:28 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-11 15:13 [PATCH v2 0/2] Intial support for voltage ADC Stanimir Varbanov
2014-09-11 15:13 ` [PATCH v2 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver Stanimir Varbanov
2014-09-12 23:27 ` Hartmut Knaack
2014-09-13 17:27 ` Jonathan Cameron
[not found] ` <54147E97.60808-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2014-09-15 14:12 ` Stanimir Varbanov
2014-09-15 14:12 ` Stanimir Varbanov
[not found] ` <5416F3E2.3030009-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
2014-09-15 16:11 ` Jonathan Cameron
2014-09-15 16:11 ` Jonathan Cameron
[not found] ` <4dbc485f-599a-4b50-854c-c2e1c44d4810-2ueSQiBKiTY7tOexoI0I+QC/G2K4zDHf@public.gmane.org>
2014-09-18 9:57 ` Stanimir Varbanov
2014-09-18 9:57 ` Stanimir Varbanov
[not found] ` <541AAC80.1090708-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
2014-09-21 13:29 ` Jonathan Cameron
2014-09-21 13:29 ` Jonathan Cameron
2014-09-22 1:01 ` Kim, Milo
2014-09-22 1:01 ` Kim, Milo
2014-09-22 1:01 ` Kim, Milo
2014-09-22 6:28 ` Jonathan Cameron
[not found] ` <54138151.8010902-Mmb7MZpHnFY@public.gmane.org>
2014-09-15 14:16 ` Stanimir Varbanov
2014-09-15 14:16 ` Stanimir Varbanov
2014-09-11 15:13 ` [PATCH v2 2/2] DT: iio: vadc: document dt binding Stanimir Varbanov
[not found] ` <1410448403-19402-3-git-send-email-svarbanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
2014-09-12 23:35 ` Hartmut Knaack
2014-09-12 23:35 ` Hartmut Knaack
[not found] ` <5413832F.6040503-Mmb7MZpHnFY@public.gmane.org>
2014-09-13 17:32 ` Jonathan Cameron
2014-09-13 17:32 ` Jonathan Cameron
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.