changes in v7:
- fixed probe/remove order, managed and unmanaged call mix use in adc.
- Documentation dual license, i2c node with controller address
changes in v6:
- removed includes types.h in mfd, of_device.h in adc.
- fixed review comments parentheses, err check, kstrtouint
changes in v5:
- removed platfrom data stored in mfd and directly accessed mfd struct in child
- fixed spell check and capitalization in mfd and documentation
changes in v4:
- fixed capitalization in mfg Kconfig and documentation
changes in v3:
- regmap for children passed using platform data and remove mfd driver info
access directly from children
changes in v2:
- removed EXPORT_SYMBOL of register set/get helper
- regmap bit filed used, fixed other review comments
This patch series add support for Battery charger control driver for Monolithic
Power System's MP2629 chipset, includes MFD driver for ADC battery & input
power supply measurement and battery charger control driver.
Thanks,
Saravanan
Saravanan Sekar (5):
dt-bindings: mfd: add document bindings for mp2629
mfd: mp2629: Add support for mps battery charger
iio: adc: mp2629: Add support for mp2629 ADC driver
power: supply: Add support for mps mp2629 battery charger
MAINTAINERS: Add entry for mp2629 Battery Charger driver
.../devicetree/bindings/mfd/mps,mp2629.yaml | 60 ++
MAINTAINERS | 5 +
drivers/iio/adc/Kconfig | 10 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mp2629_adc.c | 208 ++++++
drivers/mfd/Kconfig | 9 +
drivers/mfd/Makefile | 2 +
drivers/mfd/mp2629.c | 86 +++
drivers/power/supply/Kconfig | 10 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/mp2629_charger.c | 687 ++++++++++++++++++
include/linux/mfd/mp2629.h | 28 +
12 files changed, 1107 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
create mode 100644 drivers/iio/adc/mp2629_adc.c
create mode 100644 drivers/mfd/mp2629.c
create mode 100644 drivers/power/supply/mp2629_charger.c
create mode 100644 include/linux/mfd/mp2629.h
--
2.17.1
Add support for 8-bit resolution ADC readings for input power
supply and battery charging measurement. Provides voltage, current
readings to mp2629 power supply driver.
Signed-off-by: Saravanan Sekar <[email protected]>
---
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mp2629_adc.c | 208 +++++++++++++++++++++++++++++++++++
include/linux/mfd/mp2629.h | 9 ++
4 files changed, 228 insertions(+)
create mode 100644 drivers/iio/adc/mp2629_adc.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 82e33082958c..ef0c0cd31855 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -680,6 +680,16 @@ config MESON_SARADC
To compile this driver as a module, choose M here: the
module will be called meson_saradc.
+config MP2629_ADC
+ tristate "Monolithic MP2629 ADC driver"
+ depends on MFD_MP2629
+ help
+ Say yes to have support for battery charger IC MP2629 ADC device
+ accessed over I2C.
+
+ This driver provides ADC conversion of system, input power supply
+ and battery voltage & current information.
+
config NAU7802
tristate "Nuvoton NAU7802 ADC driver"
depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 919228900df9..f14416c245a6 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
+obj-$(CONFIG_MP2629_ADC) += mp2629_adc.o
obj-$(CONFIG_MXS_LRADC_ADC) += mxs-lradc-adc.o
obj-$(CONFIG_NAU7802) += nau7802.o
obj-$(CONFIG_NPCM_ADC) += npcm_adc.o
diff --git a/drivers/iio/adc/mp2629_adc.c b/drivers/iio/adc/mp2629_adc.c
new file mode 100644
index 000000000000..e32d5bbe97df
--- /dev/null
+++ b/drivers/iio/adc/mp2629_adc.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MP2629 Driver for ADC
+ *
+ * Copyright 2020 Monolithic Power Systems, Inc
+ *
+ * Author: Saravanan Sekar <[email protected]>
+ */
+
+#include <linux/iio/driver.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/machine.h>
+#include <linux/mfd/mp2629.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MP2629_REG_ADC_CTRL 0x03
+#define MP2629_REG_BATT_VOLT 0x0e
+#define MP2629_REG_SYSTEM_VOLT 0x0f
+#define MP2629_REG_INPUT_VOLT 0x11
+#define MP2629_REG_BATT_CURRENT 0x12
+#define MP2629_REG_INPUT_CURRENT 0x13
+
+#define MP2629_ADC_START BIT(7)
+#define MP2629_ADC_CONTINUOUS BIT(6)
+
+#define MP2629_MAP(_mp, _mpc) IIO_MAP(#_mp, "mp2629_charger", "mp2629-"_mpc)
+
+#define MP2629_ADC_CHAN(_ch, _type) { \
+ .type = _type, \
+ .indexed = 1, \
+ .datasheet_name = #_ch, \
+ .channel = MP2629_ ## _ch, \
+ .address = MP2629_REG_ ## _ch, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+}
+
+struct mp2629_adc {
+ struct regmap *regmap;
+ struct device *dev;
+};
+
+static struct iio_chan_spec mp2629_channels[] = {
+ MP2629_ADC_CHAN(BATT_VOLT, IIO_VOLTAGE),
+ MP2629_ADC_CHAN(SYSTEM_VOLT, IIO_VOLTAGE),
+ MP2629_ADC_CHAN(INPUT_VOLT, IIO_VOLTAGE),
+ MP2629_ADC_CHAN(BATT_CURRENT, IIO_CURRENT),
+ MP2629_ADC_CHAN(INPUT_CURRENT, IIO_CURRENT)
+};
+
+static struct iio_map mp2629_adc_maps[] = {
+ MP2629_MAP(BATT_VOLT, "batt-volt"),
+ MP2629_MAP(SYSTEM_VOLT, "system-volt"),
+ MP2629_MAP(INPUT_VOLT, "input-volt"),
+ MP2629_MAP(BATT_CURRENT, "batt-current"),
+ MP2629_MAP(INPUT_CURRENT, "input-current")
+};
+
+static int mp2629_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct mp2629_adc *info = iio_priv(indio_dev);
+ unsigned int rval;
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = regmap_read(info->regmap, chan->address, &rval);
+ if (ret)
+ return ret;
+
+ if (chan->address == MP2629_INPUT_VOLT)
+ rval &= GENMASK(6, 0);
+ *val = rval;
+ return IIO_VAL_INT;
+
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->channel) {
+ case MP2629_BATT_VOLT:
+ case MP2629_SYSTEM_VOLT:
+ *val = 20;
+ return IIO_VAL_INT;
+
+ case MP2629_INPUT_VOLT:
+ *val = 60;
+ return IIO_VAL_INT;
+
+ case MP2629_BATT_CURRENT:
+ *val = 175;
+ *val2 = 10;
+ return IIO_VAL_FRACTIONAL;
+
+ case MP2629_INPUT_CURRENT:
+ *val = 133;
+ *val2 = 10;
+ return IIO_VAL_FRACTIONAL;
+
+ default:
+ return -EINVAL;
+ }
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info mp2629_adc_info = {
+ .read_raw = &mp2629_read_raw,
+};
+
+static int mp2629_adc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mp2629_info *ddata = dev_get_drvdata(dev->parent);
+ struct mp2629_adc *info;
+ struct iio_dev *indio_dev;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*info));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ info = iio_priv(indio_dev);
+ info->regmap = ddata->regmap;
+ info->dev = dev;
+ platform_set_drvdata(pdev, indio_dev);
+
+ ret = iio_map_array_register(indio_dev, mp2629_adc_maps);
+ if (ret) {
+ dev_err(dev, "IIO maps register fail: %d\n", ret);
+ return ret;
+ }
+
+ indio_dev->name = "mp2629-adc";
+ indio_dev->dev.parent = dev;
+ indio_dev->channels = mp2629_channels;
+ indio_dev->num_channels = ARRAY_SIZE(mp2629_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &mp2629_adc_info;
+
+ ret = regmap_update_bits(ddata->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_START | MP2629_ADC_CONTINUOUS,
+ MP2629_ADC_START | MP2629_ADC_CONTINUOUS);
+ if (ret) {
+ dev_err(dev, "adc enable fail: %d\n", ret);
+ goto fail_unmap;
+ }
+
+ ret = iio_device_register(indio_dev);
+ if (ret) {
+ dev_err(dev, "IIO device register fail: %d\n", ret);
+ goto fail_disable;
+ }
+
+ return 0;
+
+fail_disable:
+ regmap_update_bits(ddata->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_CONTINUOUS, 0);
+ regmap_update_bits(ddata->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_START, 0);
+
+fail_unmap:
+ iio_map_array_unregister(indio_dev);
+
+ return ret;
+}
+
+static int mp2629_adc_remove(struct platform_device *pdev)
+{
+ struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+ struct mp2629_adc *info = iio_priv(indio_dev);
+
+ iio_map_array_unregister(indio_dev);
+
+ regmap_update_bits(info->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_CONTINUOUS, 0);
+ regmap_update_bits(info->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_START, 0);
+
+ iio_device_unregister(indio_dev);
+
+ return 0;
+}
+
+static const struct of_device_id mp2629_adc_of_match[] = {
+ { .compatible = "mps,mp2629_adc"},
+ {}
+};
+MODULE_DEVICE_TABLE(of, mp2629_adc_of_match);
+
+static struct platform_driver mp2629_adc_driver = {
+ .driver = {
+ .name = "mp2629_adc",
+ .of_match_table = mp2629_adc_of_match,
+ },
+ .probe = mp2629_adc_probe,
+ .remove = mp2629_adc_remove,
+};
+module_platform_driver(mp2629_adc_driver);
+
+MODULE_AUTHOR("Saravanan Sekar <[email protected]>");
+MODULE_DESCRIPTION("MP2629 ADC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h
index 06985e41fc6f..83ebfab41691 100644
--- a/include/linux/mfd/mp2629.h
+++ b/include/linux/mfd/mp2629.h
@@ -16,4 +16,13 @@ struct mp2629_info {
struct regmap *regmap;
};
+enum mp2629_adc_chan {
+ MP2629_BATT_VOLT,
+ MP2629_SYSTEM_VOLT,
+ MP2629_INPUT_VOLT,
+ MP2629_BATT_CURRENT,
+ MP2629_INPUT_CURRENT,
+ MP2629_ADC_CHAN_END
+};
+
#endif
--
2.17.1
On Fri, Apr 10, 2020 at 11:19 PM Saravanan Sekar <[email protected]> wrote:
>
> changes in v7:
> - fixed probe/remove order, managed and unmanaged call mix use in adc.
> - Documentation dual license, i2c node with controller address
Overall looks good to me, FWIW,
Reviewed-by: Andy Shevchenko <[email protected]>
One question though in reply to patch 4.
> changes in v6:
> - removed includes types.h in mfd, of_device.h in adc.
> - fixed review comments parentheses, err check, kstrtouint
>
> changes in v5:
> - removed platfrom data stored in mfd and directly accessed mfd struct in child
> - fixed spell check and capitalization in mfd and documentation
>
> changes in v4:
> - fixed capitalization in mfg Kconfig and documentation
>
> changes in v3:
> - regmap for children passed using platform data and remove mfd driver info
> access directly from children
>
> changes in v2:
> - removed EXPORT_SYMBOL of register set/get helper
> - regmap bit filed used, fixed other review comments
>
> This patch series add support for Battery charger control driver for Monolithic
> Power System's MP2629 chipset, includes MFD driver for ADC battery & input
> power supply measurement and battery charger control driver.
>
> Thanks,
> Saravanan
>
> Saravanan Sekar (5):
> dt-bindings: mfd: add document bindings for mp2629
> mfd: mp2629: Add support for mps battery charger
> iio: adc: mp2629: Add support for mp2629 ADC driver
> power: supply: Add support for mps mp2629 battery charger
> MAINTAINERS: Add entry for mp2629 Battery Charger driver
>
> .../devicetree/bindings/mfd/mps,mp2629.yaml | 60 ++
> MAINTAINERS | 5 +
> drivers/iio/adc/Kconfig | 10 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/mp2629_adc.c | 208 ++++++
> drivers/mfd/Kconfig | 9 +
> drivers/mfd/Makefile | 2 +
> drivers/mfd/mp2629.c | 86 +++
> drivers/power/supply/Kconfig | 10 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/mp2629_charger.c | 687 ++++++++++++++++++
> include/linux/mfd/mp2629.h | 28 +
> 12 files changed, 1107 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
> create mode 100644 drivers/iio/adc/mp2629_adc.c
> create mode 100644 drivers/mfd/mp2629.c
> create mode 100644 drivers/power/supply/mp2629_charger.c
> create mode 100644 include/linux/mfd/mp2629.h
>
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
On Fri, 10 Apr 2020 22:19:46 +0200
Saravanan Sekar <[email protected]> wrote:
> Add support for 8-bit resolution ADC readings for input power
> supply and battery charging measurement. Provides voltage, current
> readings to mp2629 power supply driver.
>
> Signed-off-by: Saravanan Sekar <[email protected]>
I got a bit hung up on the double free in the previous version so didn't
mention that the ordering was also fundamentally wrong in remove.
It should be the reverse of that done in probe so that we go through
a consistent set of states in each direction.
Also a note inline about registering the map, and hence allowing in kernel
users before the device is turned on which seems a bit odd.
Otherwise looks good,
Jonathan
> ---
> drivers/iio/adc/Kconfig | 10 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/mp2629_adc.c | 208 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/mp2629.h | 9 ++
> 4 files changed, 228 insertions(+)
> create mode 100644 drivers/iio/adc/mp2629_adc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 82e33082958c..ef0c0cd31855 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -680,6 +680,16 @@ config MESON_SARADC
> To compile this driver as a module, choose M here: the
> module will be called meson_saradc.
>
> +config MP2629_ADC
> + tristate "Monolithic MP2629 ADC driver"
> + depends on MFD_MP2629
> + help
> + Say yes to have support for battery charger IC MP2629 ADC device
> + accessed over I2C.
> +
> + This driver provides ADC conversion of system, input power supply
> + and battery voltage & current information.
> +
> config NAU7802
> tristate "Nuvoton NAU7802 ADC driver"
> depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 919228900df9..f14416c245a6 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_MCP3911) += mcp3911.o
> obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
> obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
> obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
> +obj-$(CONFIG_MP2629_ADC) += mp2629_adc.o
> obj-$(CONFIG_MXS_LRADC_ADC) += mxs-lradc-adc.o
> obj-$(CONFIG_NAU7802) += nau7802.o
> obj-$(CONFIG_NPCM_ADC) += npcm_adc.o
> diff --git a/drivers/iio/adc/mp2629_adc.c b/drivers/iio/adc/mp2629_adc.c
> new file mode 100644
> index 000000000000..e32d5bbe97df
> --- /dev/null
> +++ b/drivers/iio/adc/mp2629_adc.c
> @@ -0,0 +1,208 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * MP2629 Driver for ADC
> + *
> + * Copyright 2020 Monolithic Power Systems, Inc
> + *
> + * Author: Saravanan Sekar <[email protected]>
> + */
> +
> +#include <linux/iio/driver.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/machine.h>
> +#include <linux/mfd/mp2629.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define MP2629_REG_ADC_CTRL 0x03
> +#define MP2629_REG_BATT_VOLT 0x0e
> +#define MP2629_REG_SYSTEM_VOLT 0x0f
> +#define MP2629_REG_INPUT_VOLT 0x11
> +#define MP2629_REG_BATT_CURRENT 0x12
> +#define MP2629_REG_INPUT_CURRENT 0x13
> +
> +#define MP2629_ADC_START BIT(7)
> +#define MP2629_ADC_CONTINUOUS BIT(6)
> +
> +#define MP2629_MAP(_mp, _mpc) IIO_MAP(#_mp, "mp2629_charger", "mp2629-"_mpc)
> +
> +#define MP2629_ADC_CHAN(_ch, _type) { \
> + .type = _type, \
> + .indexed = 1, \
> + .datasheet_name = #_ch, \
> + .channel = MP2629_ ## _ch, \
> + .address = MP2629_REG_ ## _ch, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +struct mp2629_adc {
> + struct regmap *regmap;
> + struct device *dev;
> +};
> +
> +static struct iio_chan_spec mp2629_channels[] = {
> + MP2629_ADC_CHAN(BATT_VOLT, IIO_VOLTAGE),
> + MP2629_ADC_CHAN(SYSTEM_VOLT, IIO_VOLTAGE),
> + MP2629_ADC_CHAN(INPUT_VOLT, IIO_VOLTAGE),
> + MP2629_ADC_CHAN(BATT_CURRENT, IIO_CURRENT),
> + MP2629_ADC_CHAN(INPUT_CURRENT, IIO_CURRENT)
> +};
> +
> +static struct iio_map mp2629_adc_maps[] = {
> + MP2629_MAP(BATT_VOLT, "batt-volt"),
> + MP2629_MAP(SYSTEM_VOLT, "system-volt"),
> + MP2629_MAP(INPUT_VOLT, "input-volt"),
> + MP2629_MAP(BATT_CURRENT, "batt-current"),
> + MP2629_MAP(INPUT_CURRENT, "input-current")
> +};
> +
> +static int mp2629_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct mp2629_adc *info = iio_priv(indio_dev);
> + unsigned int rval;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = regmap_read(info->regmap, chan->address, &rval);
> + if (ret)
> + return ret;
> +
> + if (chan->address == MP2629_INPUT_VOLT)
> + rval &= GENMASK(6, 0);
> + *val = rval;
> + return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->channel) {
> + case MP2629_BATT_VOLT:
> + case MP2629_SYSTEM_VOLT:
> + *val = 20;
> + return IIO_VAL_INT;
> +
> + case MP2629_INPUT_VOLT:
> + *val = 60;
> + return IIO_VAL_INT;
> +
> + case MP2629_BATT_CURRENT:
> + *val = 175;
> + *val2 = 10;
> + return IIO_VAL_FRACTIONAL;
> +
> + case MP2629_INPUT_CURRENT:
> + *val = 133;
> + *val2 = 10;
> + return IIO_VAL_FRACTIONAL;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info mp2629_adc_info = {
> + .read_raw = &mp2629_read_raw,
> +};
> +
> +static int mp2629_adc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mp2629_info *ddata = dev_get_drvdata(dev->parent);
> + struct mp2629_adc *info;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*info));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + info = iio_priv(indio_dev);
> + info->regmap = ddata->regmap;
> + info->dev = dev;
> + platform_set_drvdata(pdev, indio_dev);
> +
> + ret = iio_map_array_register(indio_dev, mp2629_adc_maps);
> + if (ret) {
> + dev_err(dev, "IIO maps register fail: %d\n", ret);
> + return ret;
> + }
It may well not matter, but it is probably more logical to register
the map (and hence allow consumers) after turning the device on.
> +
> + indio_dev->name = "mp2629-adc";
> + indio_dev->dev.parent = dev;
> + indio_dev->channels = mp2629_channels;
> + indio_dev->num_channels = ARRAY_SIZE(mp2629_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &mp2629_adc_info;
> +
> + ret = regmap_update_bits(ddata->regmap, MP2629_REG_ADC_CTRL,
> + MP2629_ADC_START | MP2629_ADC_CONTINUOUS,
> + MP2629_ADC_START | MP2629_ADC_CONTINUOUS);
> + if (ret) {
> + dev_err(dev, "adc enable fail: %d\n", ret);
> + goto fail_unmap;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(dev, "IIO device register fail: %d\n", ret);
> + goto fail_disable;
> + }
> +
> + return 0;
> +
> +fail_disable:
> + regmap_update_bits(ddata->regmap, MP2629_REG_ADC_CTRL,
> + MP2629_ADC_CONTINUOUS, 0);
> + regmap_update_bits(ddata->regmap, MP2629_REG_ADC_CTRL,
> + MP2629_ADC_START, 0);
> +
> +fail_unmap:
Nitpick but that's a missleading name. There is a big difference between
registering a map and mapping something. fail_map_unregister would be
better here.
> + iio_map_array_unregister(indio_dev);
> +
> + return ret;
> +}
> +
> +static int mp2629_adc_remove(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct mp2629_adc *info = iio_priv(indio_dev);
> +
The order in here is wrong. It should be the reverse of what we
have in probe.
So start by removing the interfaces.
iio_device_unregister..
then disable the device
then remove the map.
However, there is a reasonable arguement that logically the
device should be on before your register the map.
> + iio_map_array_unregister(indio_dev);
> +
> + regmap_update_bits(info->regmap, MP2629_REG_ADC_CTRL,
> + MP2629_ADC_CONTINUOUS, 0);
> + regmap_update_bits(info->regmap, MP2629_REG_ADC_CTRL,
> + MP2629_ADC_START, 0);
> +
> + iio_device_unregister(indio_dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mp2629_adc_of_match[] = {
> + { .compatible = "mps,mp2629_adc"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, mp2629_adc_of_match);
> +
> +static struct platform_driver mp2629_adc_driver = {
> + .driver = {
> + .name = "mp2629_adc",
> + .of_match_table = mp2629_adc_of_match,
> + },
> + .probe = mp2629_adc_probe,
> + .remove = mp2629_adc_remove,
> +};
> +module_platform_driver(mp2629_adc_driver);
> +
> +MODULE_AUTHOR("Saravanan Sekar <[email protected]>");
> +MODULE_DESCRIPTION("MP2629 ADC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h
> index 06985e41fc6f..83ebfab41691 100644
> --- a/include/linux/mfd/mp2629.h
> +++ b/include/linux/mfd/mp2629.h
> @@ -16,4 +16,13 @@ struct mp2629_info {
> struct regmap *regmap;
> };
>
> +enum mp2629_adc_chan {
> + MP2629_BATT_VOLT,
> + MP2629_SYSTEM_VOLT,
> + MP2629_INPUT_VOLT,
> + MP2629_BATT_CURRENT,
> + MP2629_INPUT_CURRENT,
> + MP2629_ADC_CHAN_END
> +};
> +
> #endif