2020-03-22 22:47:23

by saravanan sekar

[permalink] [raw]
Subject: [PATCH v4 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver

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 | 214 +++++++++++++++++++++++++++++++++++
include/linux/mfd/mp2629.h | 9 ++
4 files changed, 234 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..6967c695d177
--- /dev/null
+++ b/drivers/iio/adc/mp2629_adc.c
@@ -0,0 +1,214 @@
+// 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/platform_device.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regulator/consumer.h>
+#include <linux/sysfs.h>
+#include <linux/regmap.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/machine.h>
+#include <linux/iio/driver.h>
+#include <linux/mfd/mp2629.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 < 0)
+ return ret;
+
+ if (chan->address == MP2629_INPUT_VOLT)
+ rval &= 0x7f;
+ *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;
+ }
+
+ return 0;
+}
+
+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;
+ void **pdata = pdev->dev.platform_data;
+ 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 = *pdata;
+ 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 = dev_name(dev);
+ indio_dev->dev.parent = dev;
+ indio_dev->dev.of_node = pdev->dev.of_node;
+ 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(info->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 = devm_iio_device_register(dev, indio_dev);
+ if (ret) {
+ dev_err(dev, "IIO device register fail: %d\n", ret);
+ goto fail_disable;
+ }
+
+ return 0;
+
+fail_disable:
+ 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);
+
+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);
+
+ 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_map_array_unregister(indio_dev);
+ 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 371e44330ba8..8b9c717e1fb0 100644
--- a/include/linux/mfd/mp2629.h
+++ b/include/linux/mfd/mp2629.h
@@ -19,4 +19,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


2020-03-22 23:33:20

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver

On Mon, Mar 23, 2020 at 12:47 AM 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.

...

> +#include <linux/platform_device.h>

> +#include <linux/of_device.h>

Don't see users of it.

> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regulator/consumer.h>

> +#include <linux/sysfs.h>

Any users?

> +#include <linux/regmap.h>

Perhaps ordered?

> +#include <linux/iio/iio.h>
> +#include <linux/iio/machine.h>
> +#include <linux/iio/driver.h>

+ blank line?

> +#include <linux/mfd/mp2629.h>

...

> +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 < 0)
> + return ret;
> +
> + if (chan->address == MP2629_INPUT_VOLT)

> + rval &= 0x7f;

GENMASK() ?

> + *val = rval;
> + return IIO_VAL_INT;

> + return 0;
> +}

...

> + void **pdata = pdev->dev.platform_data;

Same Qs as per other patch.

...

> + indio_dev->dev.of_node = pdev->dev.of_node;

Jonathan, doesn't IIO core do this for all?

--
With Best Regards,
Andy Shevchenko

2020-03-28 14:43:11

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver

On Mon, 23 Mar 2020 01:32:34 +0200
Andy Shevchenko <[email protected]> wrote:

> On Mon, Mar 23, 2020 at 12:47 AM 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.
>
> ...
>
> > +#include <linux/platform_device.h>
>
> > +#include <linux/of_device.h>
>
> Don't see users of it.
>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/regulator/consumer.h>
>
> > +#include <linux/sysfs.h>
>
> Any users?
>
> > +#include <linux/regmap.h>
>
> Perhaps ordered?
>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/machine.h>
> > +#include <linux/iio/driver.h>
>
> + blank line?
>
> > +#include <linux/mfd/mp2629.h>
>
> ...
>
> > +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 < 0)
> > + return ret;
> > +
> > + if (chan->address == MP2629_INPUT_VOLT)
>
> > + rval &= 0x7f;
>
> GENMASK() ?
>
> > + *val = rval;
> > + return IIO_VAL_INT;
>
> > + return 0;
> > +}
>
> ...
>
> > + void **pdata = pdev->dev.platform_data;
>
> Same Qs as per other patch.
>
> ...
>
> > + indio_dev->dev.of_node = pdev->dev.of_node;
>
> Jonathan, doesn't IIO core do this for all?
>

Nope. I'm not totally sure it's always safe to do so
as we have some weird parent structures in some cases.
A quick grep suggests that we may be fine though, or
alternatively be able to get away with a set it if not
already set approach.

I'll take a look when I get some time. It would be nice
to clean this up.

Jonathan




2020-03-28 18:51:41

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver

On Sat, Mar 28, 2020 at 4:42 PM Jonathan Cameron <[email protected]> wrote:
> On Mon, 23 Mar 2020 01:32:34 +0200
> Andy Shevchenko <[email protected]> wrote:
> > On Mon, Mar 23, 2020 at 12:47 AM Saravanan Sekar <[email protected]> wrote:

...

> > > + indio_dev->dev.of_node = pdev->dev.of_node;
> >
> > Jonathan, doesn't IIO core do this for all?
> >
>
> Nope. I'm not totally sure it's always safe to do so
> as we have some weird parent structures in some cases.
> A quick grep suggests that we may be fine though, or
> alternatively be able to get away with a set it if not
> already set approach.
>
> I'll take a look when I get some time. It would be nice
> to clean this up.

We may follow the GPIO subsystem's approach, i.e. if there is no node
provided take the one from the supplied struct device.
But it may have side effects. So, it's completely up to you.

--
With Best Regards,
Andy Shevchenko

2020-03-29 10:40:27

by saravanan sekar

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver

Hi Andy,

On 28/03/20 3:42 pm, Jonathan Cameron wrote:
> On Mon, 23 Mar 2020 01:32:34 +0200
> Andy Shevchenko <[email protected]> wrote:
>
>> On Mon, Mar 23, 2020 at 12:47 AM 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.
>> ...
>>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/of_device.h>
>> Don't see users of it.
Forgot to reply, its needed since I used struct of_device
"error: field name not in record or union initializer
  { .compatible = "mps,mp2629_charger"},"
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/regulator/consumer.h>
>>> +#include <linux/sysfs.h>
>> Any users?
>>
>>> +#include <linux/regmap.h>
>> Perhaps ordered?
>>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/machine.h>
>>> +#include <linux/iio/driver.h>
>> + blank line?
>>
>>> +#include <linux/mfd/mp2629.h>
>> ...
>>
>>> +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 < 0)
>>> + return ret;
>>> +
>>> + if (chan->address == MP2629_INPUT_VOLT)
>>> + rval &= 0x7f;
>> GENMASK() ?
>>
>>> + *val = rval;
>>> + return IIO_VAL_INT;
>>> + return 0;
>>> +}
>> ...
>>
>>> + void **pdata = pdev->dev.platform_data;
>> Same Qs as per other patch.
>>
>> ...
>>
>>> + indio_dev->dev.of_node = pdev->dev.of_node;
>> Jonathan, doesn't IIO core do this for all?
>>
> Nope. I'm not totally sure it's always safe to do so
> as we have some weird parent structures in some cases.
> A quick grep suggests that we may be fine though, or
> alternatively be able to get away with a set it if not
> already set approach.
>
> I'll take a look when I get some time. It would be nice
> to clean this up.
>
> Jonathan
>
>
>
>

2020-03-29 11:10:04

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver

On Sun, Mar 29, 2020 at 1:37 PM saravanan sekar <[email protected]> wrote:
> On 28/03/20 3:42 pm, Jonathan Cameron wrote:
> > On Mon, 23 Mar 2020 01:32:34 +0200
> > Andy Shevchenko <[email protected]> wrote:
> >> On Mon, Mar 23, 2020 at 12:47 AM Saravanan Sekar <[email protected]> wrote:

...

> >>> +#include <linux/of_device.h>
> >> Don't see users of it.
> Forgot to reply, its needed since I used struct of_device
> "error: field name not in record or union initializer
> { .compatible = "mps,mp2629_charger"},"

Yes, and my comment still stays. The error you get due to lack of
proper header, i.e. mod_devicetable.h.

--
With Best Regards,
Andy Shevchenko