Hi Ivan, On 09/25/2014 03:47 PM, Ivan T. Ivanov wrote: > > Hi Stan, few comment bellow. > > On Wed, 2014-09-24 at 15:56 +0300, Stanimir Varbanov wrote: >> 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@xxxxxxxxxx> >> Signed-off-by: Ivan T. Ivanov <iivanov@xxxxxxxxxx> >> --- >> drivers/iio/adc/Kconfig | 10 + >> drivers/iio/adc/Makefile | 1 + >> drivers/iio/adc/qcom-spmi-vadc.c | 1029 +++++++++++++++++++++++++ >> include/dt-bindings/iio/qcom,spmi-pmic-vadc.h | 119 +++ >> 4 files changed, 1159 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..ec48360 100644 >> --- a/drivers/iio/adc/Kconfig >> +++ b/drivers/iio/adc/Kconfig >> @@ -206,6 +206,16 @@ 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 > > + select REGMAP_SPMI I'd suggest MFD_SPMI_PMIC symbol of the parent driver instead? The parent MFD PMIC driver already selects REGMAP_SPMI and depends on SPMI. > > <snip> > >> + >> +static int vadc_read(struct vadc_priv *vadc, u16 offset, u8 *data) >> +{ >> + return regmap_bulk_read(vadc->regmap, vadc->base + offset, data, 1); > > just regmap_read() ? No, regmap_read() expects unsigned int *val as an argument hence I will need temporary variable. I had a comment to rework this on the first version of the patchset. > >> +} >> + > > General comment, most of the dev_dbg looks like legitimate dev_err's to me. I've decided to not flood console with messages. If someone wants to debug why the driver doesn't probed he can define DEBUG. -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html