linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/5] Add battery charger driver support for MP2629
@ 2020-03-28  0:11 Saravanan Sekar
  2020-03-28  0:11 ` [PATCH v5 1/5] dt-bindings: mfd: add document bindings for mp2629 Saravanan Sekar
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Saravanan Sekar @ 2020-03-28  0:11 UTC (permalink / raw)
  To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars, pmeerw, sre
  Cc: devicetree, linux-kernel, linux-iio, linux-pm, Saravanan Sekar

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                  | 209 ++++++
 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         | 686 ++++++++++++++++++
 include/linux/mfd/mp2629.h                    |  29 +
 12 files changed, 1108 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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v5 1/5] dt-bindings: mfd: add document bindings for mp2629
  2020-03-28  0:11 [PATCH v5 0/5] Add battery charger driver support for MP2629 Saravanan Sekar
@ 2020-03-28  0:11 ` Saravanan Sekar
  2020-03-28  0:11 ` [PATCH v5 2/5] mfd: mp2629: Add support for mps battery charger Saravanan Sekar
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Saravanan Sekar @ 2020-03-28  0:11 UTC (permalink / raw)
  To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars, pmeerw, sre
  Cc: devicetree, linux-kernel, linux-iio, linux-pm, Saravanan Sekar

Add device tree binding information for mp2629 mfd driver.

Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 .../devicetree/bindings/mfd/mps,mp2629.yaml   | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/mps,mp2629.yaml

diff --git a/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml b/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
new file mode 100644
index 000000000000..3c3cd023256a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/mps,mp2629.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MP2629 Battery Charger PMIC from Monolithic Power System.
+
+maintainers:
+  - Saravanan Sekar <sravanhome@gmail.com>
+
+description: |
+  MP2629 is a PMIC providing battery charging and power supply for smartphones,
+  wireless camera and portable devices. Chip is contrlled over I2C.
+
+  The battery charge management device handles battery charger controller and
+  ADC IIO device for battery, system voltage
+
+properties:
+  compatible:
+    const: mps,mp2629
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+    description:
+      The first cell is the IRQ number, the second cell is the trigger type.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/input/linux-event-codes.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@4b {
+            compatible = "mps,mp2629";
+            reg = <0x4b>;
+
+            interrupt-controller;
+            interrupt-parent = <&gpio2>;
+            #interrupt-cells = <2>;
+            interrupts = <3 IRQ_TYPE_LEVEL_HIGH>;
+        };
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v5 2/5] mfd: mp2629: Add support for mps battery charger
  2020-03-28  0:11 [PATCH v5 0/5] Add battery charger driver support for MP2629 Saravanan Sekar
  2020-03-28  0:11 ` [PATCH v5 1/5] dt-bindings: mfd: add document bindings for mp2629 Saravanan Sekar
@ 2020-03-28  0:11 ` Saravanan Sekar
  2020-03-28 10:45   ` Andy Shevchenko
  2020-03-28  0:11 ` [PATCH v5 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver Saravanan Sekar
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Saravanan Sekar @ 2020-03-28  0:11 UTC (permalink / raw)
  To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars, pmeerw, sre
  Cc: devicetree, linux-kernel, linux-iio, linux-pm, Saravanan Sekar

mp2629 is a highly-integrated switching-mode battery charge management
device for single-cell Li-ion or Li-polymer battery.

Add MFD core enables chip access for ADC driver for battery readings,
and a power supply battery-charger driver

Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 drivers/mfd/Kconfig        |  9 ++++
 drivers/mfd/Makefile       |  2 +
 drivers/mfd/mp2629.c       | 86 ++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/mp2629.h | 20 +++++++++
 4 files changed, 117 insertions(+)
 create mode 100644 drivers/mfd/mp2629.c
 create mode 100644 include/linux/mfd/mp2629.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3c547ed575e6..85be799795aa 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C
 	help
 	  Select this if your MC13xxx is connected via an I2C bus.
 
+config MFD_MP2629
+	tristate "Monolithic power system MP2629 ADC and Battery charger"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Select this option to enable support for monolithic power system
+	  battery charger. This provides ADC, thermal, battery charger power
+	  management functions on the systems.
+
 config MFD_MXS_LRADC
 	tristate "Freescale i.MX23/i.MX28 LRADC"
 	depends on ARCH_MXS || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f935d10cbf0f..d6c210f96d02 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -170,6 +170,8 @@ obj-$(CONFIG_MFD_MAX8925)	+= max8925.o
 obj-$(CONFIG_MFD_MAX8997)	+= max8997.o max8997-irq.o
 obj-$(CONFIG_MFD_MAX8998)	+= max8998.o max8998-irq.o
 
+obj-$(CONFIG_MFD_MP2629)	+= mp2629.o
+
 pcf50633-objs			:= pcf50633-core.o pcf50633-irq.o
 obj-$(CONFIG_MFD_PCF50633)	+= pcf50633.o
 obj-$(CONFIG_PCF50633_ADC)	+= pcf50633-adc.o
diff --git a/drivers/mfd/mp2629.c b/drivers/mfd/mp2629.c
new file mode 100644
index 000000000000..46242b1cdf24
--- /dev/null
+++ b/drivers/mfd/mp2629.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * MP2629 parent driver for ADC and battery charger
+ *
+ * Copyright 2020 Monolithic Power Systems, Inc
+ *
+ * Author: Saravanan Sekar <sravanhome@gmail.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/mp2629.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+enum {
+	MP2629_MFD_ADC,
+	MP2629_MFD_CHARGER,
+	MP2629_MFD_MAX
+};
+
+static const struct mfd_cell mp2629mfd[] = {
+	[MP2629_MFD_ADC] = {
+		.name = "mp2629_adc",
+		.of_compatible = "mps,mp2629_adc",
+	},
+	[MP2629_MFD_CHARGER] = {
+		.name = "mp2629_charger",
+		.of_compatible = "mps,mp2629_charger",
+	}
+};
+
+static const struct regmap_config mp2629_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x17,
+};
+
+static int mp2629_probe(struct i2c_client *client)
+{
+	struct mp2629_info *ddata;
+	int ret;
+
+	ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL);
+	if (!ddata)
+		return -ENOMEM;
+
+	ddata->dev = &client->dev;
+	i2c_set_clientdata(client, ddata);
+
+	ddata->regmap = devm_regmap_init_i2c(client, &mp2629_regmap_config);
+	if (IS_ERR(ddata->regmap)) {
+		dev_err(ddata->dev, "Failed to allocate regmap!\n");
+		return PTR_ERR(ddata->regmap);
+	}
+
+	ret = devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_NONE, mp2629mfd,
+				ARRAY_SIZE(mp2629mfd), NULL,
+				0, NULL);
+	if (ret)
+		dev_err(ddata->dev, "Failed to register sub-devices %d\n", ret);
+
+	return ret;
+}
+
+static const struct of_device_id mp2629_of_match[] = {
+	{ .compatible = "mps,mp2629"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mp2629_of_match);
+
+static struct i2c_driver mp2629_driver = {
+	.driver = {
+		.name = "mp2629",
+		.of_match_table = mp2629_of_match,
+	},
+	.probe_new	= mp2629_probe,
+};
+module_i2c_driver(mp2629_driver);
+
+MODULE_AUTHOR("Saravanan Sekar <sravanhome@gmail.com>");
+MODULE_DESCRIPTION("MP2629 Battery charger parent driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h
new file mode 100644
index 000000000000..7f071f1f352b
--- /dev/null
+++ b/include/linux/mfd/mp2629.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * register definitions for MP2629 charger
+ *
+ * Copyright 2020 Monolithic Power Systems, Inc
+ */
+
+#ifndef __MP2629_H__
+#define __MP2629_H__
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+struct mp2629_info {
+	struct device *dev;
+	struct regmap *regmap;
+};
+
+#endif
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v5 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver
  2020-03-28  0:11 [PATCH v5 0/5] Add battery charger driver support for MP2629 Saravanan Sekar
  2020-03-28  0:11 ` [PATCH v5 1/5] dt-bindings: mfd: add document bindings for mp2629 Saravanan Sekar
  2020-03-28  0:11 ` [PATCH v5 2/5] mfd: mp2629: Add support for mps battery charger Saravanan Sekar
@ 2020-03-28  0:11 ` Saravanan Sekar
  2020-03-28 10:52   ` Andy Shevchenko
  2020-03-28  0:11 ` [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger Saravanan Sekar
  2020-03-28  0:11 ` [PATCH v5 5/5] MAINTAINERS: Add entry for mp2629 Battery Charger driver Saravanan Sekar
  4 siblings, 1 reply; 17+ messages in thread
From: Saravanan Sekar @ 2020-03-28  0:11 UTC (permalink / raw)
  To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars, pmeerw, sre
  Cc: devicetree, linux-kernel, linux-iio, linux-pm, Saravanan Sekar

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 <sravanhome@gmail.com>
---
 drivers/iio/adc/Kconfig      |  10 ++
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/mp2629_adc.c | 209 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/mp2629.h   |   9 ++
 4 files changed, 229 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..39a421026dff
--- /dev/null
+++ b/drivers/iio/adc/mp2629_adc.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MP2629 Driver for ADC
+ *
+ * Copyright 2020 Monolithic Power Systems, Inc
+ *
+ * Author: Saravanan Sekar <sravanhome@gmail.com>
+ */
+
+#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 < 0)
+			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;
+	}
+
+	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;
+	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 = dev_name(dev);
+	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 = 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(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);
+
+	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 <sravanhome@gmail.com>");
+MODULE_DESCRIPTION("MP2629 ADC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h
index 7f071f1f352b..cf4ec4febefe 100644
--- a/include/linux/mfd/mp2629.h
+++ b/include/linux/mfd/mp2629.h
@@ -17,4 +17,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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger
  2020-03-28  0:11 [PATCH v5 0/5] Add battery charger driver support for MP2629 Saravanan Sekar
                   ` (2 preceding siblings ...)
  2020-03-28  0:11 ` [PATCH v5 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver Saravanan Sekar
@ 2020-03-28  0:11 ` Saravanan Sekar
  2020-03-28 11:02   ` Andy Shevchenko
  2020-03-28  0:11 ` [PATCH v5 5/5] MAINTAINERS: Add entry for mp2629 Battery Charger driver Saravanan Sekar
  4 siblings, 1 reply; 17+ messages in thread
From: Saravanan Sekar @ 2020-03-28  0:11 UTC (permalink / raw)
  To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars, pmeerw, sre
  Cc: devicetree, linux-kernel, linux-iio, linux-pm, Saravanan Sekar

The mp2629 provides switching-mode battery charge management for
single-cell Li-ion or Li-polymer battery. Driver supports the
access/control input source and battery charging parameters.

Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 drivers/power/supply/Kconfig          |  10 +
 drivers/power/supply/Makefile         |   1 +
 drivers/power/supply/mp2629_charger.c | 686 ++++++++++++++++++++++++++
 3 files changed, 697 insertions(+)
 create mode 100644 drivers/power/supply/mp2629_charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index f3424fdce341..05b3f66946ad 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -541,6 +541,16 @@ config CHARGER_MAX8998
 	  Say Y to enable support for the battery charger control sysfs and
 	  platform data of MAX8998/LP3974 PMICs.
 
+config CHARGER_MP2629
+	tristate "Monolithic power system MP2629 Battery charger"
+	depends on MFD_MP2629
+	depends on MP2629_ADC
+	depends on IIO
+	help
+	  Select this option to enable support for Monolithic power system
+	  Battery charger. This driver provies Battery charger power management
+	  functions on the systems.
+
 config CHARGER_QCOM_SMBB
 	tristate "Qualcomm Switch-Mode Battery Charger and Boost"
 	depends on MFD_SPMI_PMIC || COMPILE_TEST
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 6c7da920ea83..41cb64f09e49 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -75,6 +75,7 @@ obj-$(CONFIG_CHARGER_MAX77650)	+= max77650-charger.o
 obj-$(CONFIG_CHARGER_MAX77693)	+= max77693_charger.o
 obj-$(CONFIG_CHARGER_MAX8997)	+= max8997_charger.o
 obj-$(CONFIG_CHARGER_MAX8998)	+= max8998_charger.o
+obj-$(CONFIG_CHARGER_MP2629)	+= mp2629_charger.o
 obj-$(CONFIG_CHARGER_QCOM_SMBB)	+= qcom_smbb.o
 obj-$(CONFIG_CHARGER_BQ2415X)	+= bq2415x_charger.o
 obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
diff --git a/drivers/power/supply/mp2629_charger.c b/drivers/power/supply/mp2629_charger.c
new file mode 100644
index 000000000000..6a780d35c4a3
--- /dev/null
+++ b/drivers/power/supply/mp2629_charger.c
@@ -0,0 +1,686 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MP2629 battery charger driver
+ *
+ * Copyright 2020 Monolithic Power Systems, Inc
+ *
+ * Author: Saravanan Sekar <sravanhome@gmail.com>
+ */
+
+#include <linux/iio/consumer.h>
+#include <linux/iio/types.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/mp2629.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/workqueue.h>
+
+#define MP2629_REG_INPUT_ILIM		0x00
+#define MP2629_REG_INPUT_VLIM		0x01
+#define MP2629_REG_CHARGE_CTRL		0x04
+#define MP2629_REG_CHARGE_ILIM		0x05
+#define MP2629_REG_PRECHARGE		0x06
+#define MP2629_REG_TERM_CURRENT		0x06
+#define MP2629_REG_CHARGE_VLIM		0x07
+#define MP2629_REG_TIMER_CTRL		0x08
+#define MP2629_REG_IMPEDANCE_COMP	0x09
+#define MP2629_REG_INTERRUPT		0x0b
+#define MP2629_REG_STATUS		0x0c
+#define MP2629_REG_FAULT		0x0d
+
+#define MP2629_MASK_INPUT_TYPE		GENMASK(7, 5)
+#define MP2629_MASK_CHARGE_TYPE		GENMASK(4, 3)
+#define MP2629_MASK_CHARGE_CTRL		GENMASK(5, 4)
+#define MP2629_MASK_WDOG_CTRL		GENMASK(5, 4)
+#define MP2629_MASK_IMPEDANCE		GENMASK(7, 4)
+
+#define MP2629_INPUTSOURCE_CHANGE	GENMASK(7, 5)
+#define MP2629_CHARGING_CHANGE		GENMASK(4, 3)
+#define MP2629_FAULT_BATTERY		BIT(3)
+#define MP2629_FAULT_THERMAL		BIT(4)
+#define MP2629_FAULT_INPUT		BIT(5)
+#define MP2629_FAULT_OTG		BIT(6)
+
+#define MP2629_MAX_BATT_CAPACITY	100
+
+#define MP2629_PROPS(_idx, _min, _max, _step)		\
+	[_idx] = {					\
+		.min	= _min,				\
+		.max	= _max,				\
+		.step	= _step,			\
+}
+
+enum mp2629_source_type {
+	MP2629_SOURCE_TYPE_NO_INPUT,
+	MP2629_SOURCE_TYPE_NON_STD,
+	MP2629_SOURCE_TYPE_SDP,
+	MP2629_SOURCE_TYPE_CDP,
+	MP2629_SOURCE_TYPE_DCP,
+	MP2629_SOURCE_TYPE_OTG = 7,
+};
+
+enum mp2629_field {
+	INPUT_ILIM,
+	INPUT_VLIM,
+	CHARGE_ILIM,
+	CHARGE_VLIM,
+	PRECHARGE,
+	TERM_CURRENT,
+	MP2629_MAX_FIELD
+};
+
+struct mp2629_charger {
+	struct device *dev;
+	struct work_struct charger_work;
+	int status;
+	int fault;
+
+	struct regmap *regmap;
+	struct regmap_field *regmap_fields[MP2629_MAX_FIELD];
+	struct mutex lock;
+	struct power_supply *usb;
+	struct power_supply *battery;
+	struct iio_channel *iiochan[MP2629_ADC_CHAN_END];
+};
+
+struct mp2629_prop {
+	int reg;
+	int mask;
+	int min;
+	int max;
+	int step;
+	int shift;
+};
+
+static enum power_supply_usb_type mp2629_usb_types[] = {
+	POWER_SUPPLY_USB_TYPE_SDP,
+	POWER_SUPPLY_USB_TYPE_DCP,
+	POWER_SUPPLY_USB_TYPE_CDP,
+	POWER_SUPPLY_USB_TYPE_PD_DRP,
+	POWER_SUPPLY_USB_TYPE_UNKNOWN
+};
+
+static enum power_supply_property mp2629_charger_usb_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_USB_TYPE,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+	POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
+};
+
+static enum power_supply_property mp2629_charger_bat_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
+	POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+};
+
+static struct mp2629_prop props[] = {
+	MP2629_PROPS(INPUT_ILIM, 100000, 3250000, 50000),
+	MP2629_PROPS(INPUT_VLIM, 3800000, 5300000, 100000),
+	MP2629_PROPS(CHARGE_ILIM, 320000, 4520000, 40000),
+	MP2629_PROPS(CHARGE_VLIM, 3400000, 4670000, 10000),
+	MP2629_PROPS(PRECHARGE, 120000, 720000, 40000),
+	MP2629_PROPS(TERM_CURRENT, 80000, 680000, 40000),
+};
+
+static const struct reg_field mp2629_reg_fields[] = {
+	[INPUT_ILIM]	= REG_FIELD(MP2629_REG_INPUT_ILIM, 0, 5),
+	[INPUT_VLIM]	= REG_FIELD(MP2629_REG_INPUT_VLIM, 0, 3),
+	[CHARGE_ILIM]	= REG_FIELD(MP2629_REG_CHARGE_ILIM, 0, 6),
+	[CHARGE_VLIM]	= REG_FIELD(MP2629_REG_CHARGE_VLIM, 1, 7),
+	[PRECHARGE]	= REG_FIELD(MP2629_REG_PRECHARGE, 4, 7),
+	[TERM_CURRENT]	= REG_FIELD(MP2629_REG_TERM_CURRENT, 0, 3),
+};
+
+static char *adc_chan_name[] = {
+	"mp2629-batt-volt",
+	"mp2629-system-volt",
+	"mp2629-input-volt",
+	"mp2629-batt-current",
+	"mp2629-input-current",
+};
+
+static int mp2629_read_adc(struct mp2629_charger *charger,
+			   enum mp2629_adc_chan ch,
+			   union power_supply_propval *val)
+{
+	int ret;
+	int chval;
+
+	ret = iio_read_channel_processed(charger->iiochan[ch], &chval);
+	if (ret < 0)
+		return ret;
+
+	val->intval = chval * 1000;
+
+	return 0;
+}
+
+static int mp2629_get_prop(struct mp2629_charger *charger,
+			   enum mp2629_field fld,
+			   union power_supply_propval *val)
+{
+	int ret;
+	unsigned int rval;
+
+	ret = regmap_field_read(charger->regmap_fields[fld], &rval);
+	if (ret)
+		return ret;
+
+	val->intval = (rval * props[fld].step) + props[fld].min;
+
+	return 0;
+}
+
+static int mp2629_set_prop(struct mp2629_charger *charger,
+			   enum mp2629_field fld,
+			   const union power_supply_propval *val)
+{
+	unsigned int rval;
+
+	if (val->intval < props[fld].min || val->intval > props[fld].max)
+		return -EINVAL;
+
+	rval = (val->intval - props[fld].min) / props[fld].step;
+	return regmap_field_write(charger->regmap_fields[fld], rval);
+}
+
+static int mp2629_get_battery_capacity(struct mp2629_charger *charger,
+				       union power_supply_propval *val)
+{
+	union power_supply_propval vnow, vlim;
+	int ret;
+
+	ret = mp2629_read_adc(charger, MP2629_BATT_VOLT, &vnow);
+	if (ret)
+		return ret;
+
+	ret = mp2629_get_prop(charger, CHARGE_VLIM, &vlim);
+	if (ret)
+		return ret;
+
+	val->intval = (vnow.intval * 100) / vlim.intval;
+	val->intval = min(val->intval, MP2629_MAX_BATT_CAPACITY);
+
+	return 0;
+}
+
+static int mp2629_charger_battery_get_prop(struct power_supply *psy,
+					enum power_supply_property psp,
+					union power_supply_propval *val)
+{
+	struct mp2629_charger *charger = dev_get_drvdata(psy->dev.parent);
+	unsigned int rval;
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = mp2629_read_adc(charger, MP2629_BATT_VOLT, val);
+		break;
+
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = mp2629_read_adc(charger, MP2629_BATT_CURRENT, val);
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		val->intval = 4520000;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
+		val->intval = 4670000;
+		break;
+
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = mp2629_get_battery_capacity(charger, val);
+		break;
+
+	case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
+		ret = mp2629_get_prop(charger, TERM_CURRENT, val);
+		break;
+
+	case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
+		ret = mp2629_get_prop(charger, PRECHARGE, val);
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		ret = mp2629_get_prop(charger, CHARGE_VLIM, val);
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+		ret = mp2629_get_prop(charger, CHARGE_ILIM, val);
+		break;
+
+	case POWER_SUPPLY_PROP_HEALTH:
+		if (!charger->fault)
+			val->intval = POWER_SUPPLY_HEALTH_GOOD;
+		if (MP2629_FAULT_BATTERY & charger->fault)
+			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+		else if (MP2629_FAULT_THERMAL & charger->fault)
+			val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+		else if (MP2629_FAULT_INPUT & charger->fault)
+			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+		break;
+
+	case POWER_SUPPLY_PROP_STATUS:
+		ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+		if (ret)
+			break;
+
+		rval = (rval & MP2629_MASK_CHARGE_TYPE) >> 3;
+		switch (rval) {
+		case 0x00:
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+			break;
+		case 0x01:
+		case 0x10:
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+			break;
+		case 0x11:
+			val->intval = POWER_SUPPLY_STATUS_FULL;
+		}
+		break;
+
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+		if (ret)
+			break;
+
+		rval = (rval & MP2629_MASK_CHARGE_TYPE) >> 3;
+		switch (rval) {
+		case 0x00:
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+			break;
+		case 0x01:
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+			break;
+		case 0x10:
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
+			break;
+		default:
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+		}
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int mp2629_charger_battery_set_prop(struct power_supply *psy,
+					enum power_supply_property psp,
+					const union power_supply_propval *val)
+{
+	struct mp2629_charger *charger = dev_get_drvdata(psy->dev.parent);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
+		return mp2629_set_prop(charger, TERM_CURRENT, val);
+
+	case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
+		return mp2629_set_prop(charger, PRECHARGE, val);
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		return mp2629_set_prop(charger, CHARGE_VLIM, val);
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+		return mp2629_set_prop(charger, CHARGE_ILIM, val);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mp2629_charger_usb_get_prop(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct mp2629_charger *charger = dev_get_drvdata(psy->dev.parent);
+	unsigned int rval;
+	int ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+		if (ret)
+			break;
+
+		val->intval = !!(rval & MP2629_MASK_INPUT_TYPE);
+		break;
+
+	case POWER_SUPPLY_PROP_USB_TYPE:
+		ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+		if (ret)
+			break;
+
+		rval = (rval & MP2629_MASK_INPUT_TYPE) >> 5;
+		switch (rval) {
+		case MP2629_SOURCE_TYPE_SDP:
+			val->intval = POWER_SUPPLY_USB_TYPE_SDP;
+			break;
+		case MP2629_SOURCE_TYPE_CDP:
+			val->intval = POWER_SUPPLY_USB_TYPE_CDP;
+			break;
+		case MP2629_SOURCE_TYPE_DCP:
+			val->intval = POWER_SUPPLY_USB_TYPE_DCP;
+			break;
+		case MP2629_SOURCE_TYPE_OTG:
+			val->intval = POWER_SUPPLY_USB_TYPE_PD_DRP;
+			break;
+		default:
+			val->intval = POWER_SUPPLY_USB_TYPE_UNKNOWN;
+			break;
+		}
+		break;
+
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = mp2629_read_adc(charger, MP2629_INPUT_VOLT, val);
+		break;
+
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = mp2629_read_adc(charger, MP2629_INPUT_CURRENT, val);
+		break;
+
+	case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
+		ret = mp2629_get_prop(charger, INPUT_VLIM, val);
+		break;
+
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = mp2629_get_prop(charger, INPUT_ILIM, val);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int mp2629_charger_usb_set_prop(struct power_supply *psy,
+				enum power_supply_property psp,
+				const union power_supply_propval *val)
+{
+	struct mp2629_charger *charger = dev_get_drvdata(psy->dev.parent);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
+		return mp2629_set_prop(charger, INPUT_VLIM, val);
+
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		return mp2629_set_prop(charger, INPUT_ILIM, val);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mp2629_charger_battery_prop_writeable(struct power_supply *psy,
+				     enum power_supply_property psp)
+{
+	return ((psp == POWER_SUPPLY_PROP_PRECHARGE_CURRENT) ||
+		(psp == POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT) ||
+		(psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT) ||
+		(psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE));
+}
+
+static int mp2629_charger_usb_prop_writeable(struct power_supply *psy,
+				     enum power_supply_property psp)
+{
+	return ((psp == POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT) ||
+		(psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT));
+}
+
+static void mp2629_charger_work(struct work_struct *work)
+{
+	struct mp2629_charger *charger;
+	unsigned int rval;
+	int ret;
+
+	charger = container_of(work, struct mp2629_charger, charger_work);
+
+	mutex_lock(&charger->lock);
+
+	ret = regmap_read(charger->regmap, MP2629_REG_FAULT, &rval);
+	if (ret)
+		goto unlock;
+
+	if (rval) {
+		charger->fault = rval;
+		if (MP2629_FAULT_BATTERY & rval)
+			dev_err(charger->dev, "Battery fault OVP");
+		else if (MP2629_FAULT_THERMAL & rval)
+			dev_err(charger->dev, "Thermal shutdown fault");
+		else if (MP2629_FAULT_INPUT & rval)
+			dev_err(charger->dev, "no input or input OVP");
+		else if (MP2629_FAULT_OTG & rval)
+			dev_err(charger->dev, "VIN overloaded");
+
+		goto unlock;
+	}
+
+	ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+	if (ret)
+		goto unlock;
+
+	if (rval & MP2629_INPUTSOURCE_CHANGE)
+		power_supply_changed(charger->usb);
+	else if (rval & MP2629_CHARGING_CHANGE)
+		power_supply_changed(charger->battery);
+
+unlock:
+	mutex_unlock(&charger->lock);
+}
+
+static irqreturn_t mp2629_irq_handler(int irq, void *dev_id)
+{
+	struct mp2629_charger *charger = dev_id;
+
+	schedule_work(&charger->charger_work);
+	return IRQ_HANDLED;
+}
+
+static const struct power_supply_desc mp2629_usb_desc = {
+	.name		= "mp2629_usb",
+	.type		= POWER_SUPPLY_TYPE_USB,
+	.usb_types      = mp2629_usb_types,
+	.num_usb_types  = ARRAY_SIZE(mp2629_usb_types),
+	.properties	= mp2629_charger_usb_props,
+	.num_properties	= ARRAY_SIZE(mp2629_charger_usb_props),
+	.get_property	= mp2629_charger_usb_get_prop,
+	.set_property	= mp2629_charger_usb_set_prop,
+	.property_is_writeable = mp2629_charger_usb_prop_writeable,
+};
+
+static const struct power_supply_desc mp2629_battery_desc = {
+	.name		= "mp2629_battery",
+	.type		= POWER_SUPPLY_TYPE_BATTERY,
+	.properties	= mp2629_charger_bat_props,
+	.num_properties	= ARRAY_SIZE(mp2629_charger_bat_props),
+	.get_property	= mp2629_charger_battery_get_prop,
+	.set_property	= mp2629_charger_battery_set_prop,
+	.property_is_writeable = mp2629_charger_battery_prop_writeable,
+};
+
+static ssize_t batt_impedance_compensation_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct mp2629_charger *charger = dev_get_drvdata(dev->parent);
+	unsigned int rval;
+	int ret;
+
+	ret = regmap_read(charger->regmap, MP2629_REG_IMPEDANCE_COMP, &rval);
+	if (ret)
+		return ret;
+
+	rval = (rval >> 4) * 10;
+	return sprintf(buf, "%d mohm\n", rval);
+}
+
+static ssize_t batt_impedance_compensation_store(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf,
+					    size_t count)
+{
+	struct mp2629_charger *charger = dev_get_drvdata(dev->parent);
+	int val;
+	int ret;
+
+	ret = kstrtoint(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	if (val < 0 && val > 140)
+		return -ERANGE;
+
+	/* multiples of 10 mohm so round off */
+	val = val / 10;
+	ret = regmap_update_bits(charger->regmap, MP2629_REG_IMPEDANCE_COMP,
+					MP2629_MASK_IMPEDANCE, val << 4);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(batt_impedance_compensation);
+
+static struct attribute *mp2629_charger_sysfs_attrs[] = {
+	&dev_attr_batt_impedance_compensation.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(mp2629_charger_sysfs);
+
+static int mp2629_charger_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mp2629_info *ddata = dev_get_drvdata(dev->parent);
+	struct mp2629_charger *charger;
+	struct power_supply_config psy_cfg = {NULL};
+	int ret, i, irq;
+
+	charger = devm_kzalloc(dev, sizeof(*charger), GFP_KERNEL);
+	if (!charger)
+		return -ENOMEM;
+
+	charger->regmap = ddata->regmap;
+	charger->dev = dev;
+	platform_set_drvdata(pdev, charger);
+
+	for (i = 0; i < MP2629_MAX_FIELD; i++) {
+		charger->regmap_fields[i] = devm_regmap_field_alloc(dev,
+					charger->regmap, mp2629_reg_fields[i]);
+		if (IS_ERR(charger->regmap_fields[i])) {
+			dev_err(dev, "regmap field alloc fail %d\n", i);
+			return PTR_ERR(charger->regmap_fields[i]);
+		}
+	}
+
+	for (i = 0; i < MP2629_ADC_CHAN_END; i++) {
+		charger->iiochan[i] = iio_channel_get(dev, adc_chan_name[i]);
+		if (IS_ERR(charger->iiochan[i])) {
+			ret = PTR_ERR(charger->iiochan[i]);
+			goto iio_fail;
+		}
+	}
+
+	charger->usb = devm_power_supply_register(dev, &mp2629_usb_desc, NULL);
+	if (IS_ERR(charger->usb)) {
+		ret = PTR_ERR(charger->usb);
+		goto iio_fail;
+	}
+
+	psy_cfg.drv_data = charger;
+	psy_cfg.attr_grp = mp2629_charger_sysfs_groups;
+	charger->battery = devm_power_supply_register(dev,
+					 &mp2629_battery_desc, &psy_cfg);
+	if (IS_ERR(charger->battery)) {
+		ret = PTR_ERR(charger->battery);
+		goto iio_fail;
+	}
+
+	ret = regmap_update_bits(charger->regmap, MP2629_REG_CHARGE_CTRL,
+					MP2629_MASK_CHARGE_CTRL, BIT(4));
+	if (ret) {
+		dev_err(dev, "enable charge fail: %d\n", ret);
+		goto iio_fail;
+	}
+
+	regmap_update_bits(charger->regmap, MP2629_REG_TIMER_CTRL,
+					MP2629_MASK_WDOG_CTRL, 0);
+
+	INIT_WORK(&charger->charger_work, mp2629_charger_work);
+	mutex_init(&charger->lock);
+
+	irq = platform_get_irq(to_platform_device(pdev->dev.parent), 0);
+	if (irq) {
+		ret = devm_request_irq(dev, irq, mp2629_irq_handler,
+				 IRQF_TRIGGER_RISING, "mp2629-charger",
+				 charger);
+		if (ret) {
+			dev_err(dev, "failed to request gpio IRQ\n");
+			goto iio_fail;
+		}
+	}
+
+	regmap_update_bits(charger->regmap, MP2629_REG_INTERRUPT,
+				GENMASK(6, 5), BIT(6) | BIT(5));
+
+	return 0;
+
+iio_fail:
+	while (i--)
+		iio_channel_release(charger->iiochan[i]);
+
+	dev_err(dev, "driver register fail: %d\n", ret);
+	return ret;
+}
+
+static int mp2629_charger_remove(struct platform_device *pdev)
+{
+	struct mp2629_charger *charger = platform_get_drvdata(pdev);
+	int i;
+
+	cancel_work_sync(&charger->charger_work);
+
+	for (i = 0; i < MP2629_ADC_CHAN_END; i++)
+		iio_channel_release(charger->iiochan[i]);
+
+	regmap_update_bits(charger->regmap, MP2629_REG_CHARGE_CTRL,
+					MP2629_MASK_CHARGE_CTRL, 0);
+	return 0;
+}
+
+static const struct of_device_id mp2629_charger_of_match[] = {
+	{ .compatible = "mps,mp2629_charger"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, mp2629_charger_of_match);
+
+static struct platform_driver mp2629_charger_driver = {
+	.driver = {
+		.name = "mp2629_charger",
+		.of_match_table = mp2629_charger_of_match,
+	},
+	.probe		= mp2629_charger_probe,
+	.remove		= mp2629_charger_remove,
+};
+module_platform_driver(mp2629_charger_driver);
+
+MODULE_AUTHOR("Saravanan Sekar <sravanhome@gmail.com>");
+MODULE_DESCRIPTION("MP2629 Charger driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v5 5/5] MAINTAINERS: Add entry for mp2629 Battery Charger driver
  2020-03-28  0:11 [PATCH v5 0/5] Add battery charger driver support for MP2629 Saravanan Sekar
                   ` (3 preceding siblings ...)
  2020-03-28  0:11 ` [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger Saravanan Sekar
@ 2020-03-28  0:11 ` Saravanan Sekar
  4 siblings, 0 replies; 17+ messages in thread
From: Saravanan Sekar @ 2020-03-28  0:11 UTC (permalink / raw)
  To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars, pmeerw, sre
  Cc: devicetree, linux-kernel, linux-iio, linux-pm, Saravanan Sekar

Add MAINTAINERS entry for Monolithic Power Systems mp2629 Charger driver.

Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 32a95d162f06..0f82d5a7a614 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11358,10 +11358,15 @@ F:	drivers/tty/mxser.*
 MONOLITHIC POWER SYSTEM PMIC DRIVER
 M:	Saravanan Sekar <sravanhome@gmail.com>
 S:	Maintained
+F:	Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
 F:	Documentation/devicetree/bindings/regulator/mps,mp*.yaml
+F:	drivers/iio/adc/mp2629_adc.c
+F:	drivers/mfd/mp2629.c
+F:	drivers/power/supply/mp2629_charger.c
 F:	drivers/regulator/mp5416.c
 F:	drivers/regulator/mpq7920.c
 F:	drivers/regulator/mpq7920.h
+F:	include/linux/mfd/mp2629.h
 
 MR800 AVERMEDIA USB FM RADIO DRIVER
 M:	Alexey Klimov <klimov.linux@gmail.com>
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 2/5] mfd: mp2629: Add support for mps battery charger
  2020-03-28  0:11 ` [PATCH v5 2/5] mfd: mp2629: Add support for mps battery charger Saravanan Sekar
@ 2020-03-28 10:45   ` Andy Shevchenko
  2020-03-28 10:51     ` saravanan sekar
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-28 10:45 UTC (permalink / raw)
  To: Saravanan Sekar
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> wrote:
>
> mp2629 is a highly-integrated switching-mode battery charge management
> device for single-cell Li-ion or Li-polymer battery.
>
> Add MFD core enables chip access for ADC driver for battery readings,
> and a power supply battery-charger driver

...

> +#ifndef __MP2629_H__
> +#define __MP2629_H__

> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>

None of these header is in use here.

struct device;
struct regmap;

would be enough.

> +struct mp2629_info {
> +       struct device *dev;
> +       struct regmap *regmap;
> +};
> +
> +#endif

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 2/5] mfd: mp2629: Add support for mps battery charger
  2020-03-28 10:45   ` Andy Shevchenko
@ 2020-03-28 10:51     ` saravanan sekar
  2020-03-28 11:06       ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: saravanan sekar @ 2020-03-28 10:51 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

Hi Andy,

On 28/03/20 11:45 am, Andy Shevchenko wrote:
> On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> wrote:
>> mp2629 is a highly-integrated switching-mode battery charge management
>> device for single-cell Li-ion or Li-polymer battery.
>>
>> Add MFD core enables chip access for ADC driver for battery readings,
>> and a power supply battery-charger driver
> ...
>
>> +#ifndef __MP2629_H__
>> +#define __MP2629_H__
>> +#include <linux/device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/types.h>
> None of these header is in use here.
>
> struct device;
> struct regmap;
>
> would be enough.

Yesterday I conveyed to you that Lee is not recommended to use forward 
declaration and asked me

to use includes, then you agreed with the same. Again same comments !! 
sorry I am lost.

>> +struct mp2629_info {
>> +       struct device *dev;
>> +       struct regmap *regmap;
>> +};
>> +
>> +#endif

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver
  2020-03-28  0:11 ` [PATCH v5 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver Saravanan Sekar
@ 2020-03-28 10:52   ` Andy Shevchenko
  2020-03-28 17:51     ` Jonathan Cameron
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-28 10:52 UTC (permalink / raw)
  To: Saravanan Sekar
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> 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.

...

> +               ret = regmap_read(info->regmap, chan->address, &rval);
> +               if (ret < 0)

' < 0' is not needed for regmap call.

..

> +               case MP2629_INPUT_CURRENT:
> +                       *val = 133;
> +                       *val2 = 10;
> +                       return IIO_VAL_FRACTIONAL;
> +
> +               default:
> +                       return -EINVAL;
> +               }
> +
> +       default:
> +               return -EINVAL;
> +       }

> +
> +       return 0;

Do you really need this? Looks to me as dead code.

...

> +       indio_dev->name = dev_name(dev);

Shouldn't be this a part number?
I heard something, so, I might be mistaken, but I hope maintainers
will help here.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger
  2020-03-28  0:11 ` [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger Saravanan Sekar
@ 2020-03-28 11:02   ` Andy Shevchenko
  2020-03-28 11:03     ` Andy Shevchenko
  2020-03-28 11:29     ` saravanan sekar
  0 siblings, 2 replies; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-28 11:02 UTC (permalink / raw)
  To: Saravanan Sekar
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> wrote:
>
> The mp2629 provides switching-mode battery charge management for
> single-cell Li-ion or Li-polymer battery. Driver supports the
> access/control input source and battery charging parameters.

...

> +#include <linux/module.h>
> +#include <linux/of_device.h>

Missed header bits.h.

...

> +       ret = iio_read_channel_processed(charger->iiochan[ch], &chval);

> +       if (ret < 0)

Is it possible to get positive returned value?

> +               return ret;

...

> +       val->intval = (rval * props[fld].step) + props[fld].min;

Too many parentheses.

...

> +       return ((psp == POWER_SUPPLY_PROP_PRECHARGE_CURRENT) ||
> +               (psp == POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT) ||
> +               (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT) ||
> +               (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE));

Ditto.

...

> +       return ((psp == POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT) ||
> +               (psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT));

Ditto.

...

> +       return sprintf(buf, "%d mohm\n", rval);

Hmm... For units we usually have separate node, but it is up to
maintainer, I dunno what the common practice is there.

...

> +       int val;
> +       int ret;
> +
> +       ret = kstrtoint(buf, 10, &val);
> +       if (ret)
> +               return ret;
> +

> +       if (val < 0 && val > 140)

What the point to convert negative values in the first place? kstrtouint()

> +               return -ERANGE;

...

> +       struct power_supply_config psy_cfg = {NULL};

{ 0 }

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger
  2020-03-28 11:02   ` Andy Shevchenko
@ 2020-03-28 11:03     ` Andy Shevchenko
  2020-03-28 11:29     ` saravanan sekar
  1 sibling, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-28 11:03 UTC (permalink / raw)
  To: Saravanan Sekar
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

On Sat, Mar 28, 2020 at 1:02 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> wrote:

> > +#include <linux/of_device.h>

Forget to tell, I haven't found evidence of the above being used.

> Missed header bits.h.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 2/5] mfd: mp2629: Add support for mps battery charger
  2020-03-28 10:51     ` saravanan sekar
@ 2020-03-28 11:06       ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-28 11:06 UTC (permalink / raw)
  To: saravanan sekar
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

On Sat, Mar 28, 2020 at 12:51 PM saravanan sekar <sravanhome@gmail.com> wrote:
>
> Hi Andy,
>
> On 28/03/20 11:45 am, Andy Shevchenko wrote:
> > On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> wrote:
> >> mp2629 is a highly-integrated switching-mode battery charge management
> >> device for single-cell Li-ion or Li-polymer battery.
> >>
> >> Add MFD core enables chip access for ADC driver for battery readings,
> >> and a power supply battery-charger driver
> > ...
> >
> >> +#ifndef __MP2629_H__
> >> +#define __MP2629_H__
> >> +#include <linux/device.h>
> >> +#include <linux/regmap.h>
> >> +#include <linux/types.h>
> > None of these header is in use here.
> >
> > struct device;
> > struct regmap;
> >
> > would be enough.
>
> Yesterday I conveyed to you that Lee is not recommended to use forward
> declaration and asked me
>
> to use includes, then you agreed with the same. Again same comments !!
> sorry I am lost.

Ah, okay. I missed that.
But types.h is still not needed, all data types are provided either by
C or by rest two headers.

> >> +struct mp2629_info {
> >> +       struct device *dev;
> >> +       struct regmap *regmap;
> >> +};
> >> +
> >> +#endif

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger
  2020-03-28 11:02   ` Andy Shevchenko
  2020-03-28 11:03     ` Andy Shevchenko
@ 2020-03-28 11:29     ` saravanan sekar
  2020-03-28 18:44       ` Andy Shevchenko
  1 sibling, 1 reply; 17+ messages in thread
From: saravanan sekar @ 2020-03-28 11:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

Hi Andy,

On 28/03/20 12:02 pm, Andy Shevchenko wrote:
> On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> wrote:
>> The mp2629 provides switching-mode battery charge management for
>> single-cell Li-ion or Li-polymer battery. Driver supports the
>> access/control input source and battery charging parameters.
> ...
>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
> Missed header bits.h.
>
> ...
>
>> +       ret = iio_read_channel_processed(charger->iiochan[ch], &chval);
>> +       if (ret < 0)
> Is it possible to get positive returned value?
Really not for processed
>> +               return ret;
> ...
>
>> +       val->intval = (rval * props[fld].step) + props[fld].min;
> Too many parentheses.
>
> ...
>
>> +       return ((psp == POWER_SUPPLY_PROP_PRECHARGE_CURRENT) ||
>> +               (psp == POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT) ||
>> +               (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT) ||
>> +               (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE));
> Ditto.
I think I misunderstood you previous review comment "Redundant 
parentheses", no sure what is the expectation
>
> ...
>
>> +       return ((psp == POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT) ||
>> +               (psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT));
> Ditto.
>
> ...
>
>> +       return sprintf(buf, "%d mohm\n", rval);
> Hmm... For units we usually have separate node, but it is up to
> maintainer, I dunno what the common practice is there.
>
> ...
>
>> +       int val;
>> +       int ret;
>> +
>> +       ret = kstrtoint(buf, 10, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (val < 0 && val > 140)
> What the point to convert negative values in the first place? kstrtouint()
Done
>> +               return -ERANGE;
> ...
>
>> +       struct power_supply_config psy_cfg = {NULL};
> { 0 }
>
NULL to make compiler happy.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver
  2020-03-28 10:52   ` Andy Shevchenko
@ 2020-03-28 17:51     ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2020-03-28 17:51 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Saravanan Sekar, Lee Jones, Rob Herring, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

On Sat, 28 Mar 2020 12:52:11 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> 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.  
> 
> ...
> 
> > +               ret = regmap_read(info->regmap, chan->address, &rval);
> > +               if (ret < 0)  
> 
> ' < 0' is not needed for regmap call.
> 
> ..
> 
> > +               case MP2629_INPUT_CURRENT:
> > +                       *val = 133;
> > +                       *val2 = 10;
> > +                       return IIO_VAL_FRACTIONAL;
> > +
> > +               default:
> > +                       return -EINVAL;
> > +               }
> > +
> > +       default:
> > +               return -EINVAL;
> > +       }  
> 
> > +
> > +       return 0;  
> 
> Do you really need this? Looks to me as dead code.
> 
> ...
> 
> > +       indio_dev->name = dev_name(dev);  
> 
> Shouldn't be this a part number?
> I heard something, so, I might be mistaken, but I hope maintainers
> will help here.

It should indeed.  I have a nasty habit of missing this in
review so thanks for pointing it out!

Jonathan


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger
  2020-03-28 11:29     ` saravanan sekar
@ 2020-03-28 18:44       ` Andy Shevchenko
  2020-03-29 10:34         ` saravanan sekar
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-28 18:44 UTC (permalink / raw)
  To: saravanan sekar
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

On Sat, Mar 28, 2020 at 1:29 PM saravanan sekar <sravanhome@gmail.com> wrote:
> On 28/03/20 12:02 pm, Andy Shevchenko wrote:
> > On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> wrote:

...

> >> +       val->intval = (rval * props[fld].step) + props[fld].min;
> > Too many parentheses.
> >
> > ...
> >
> >> +       return ((psp == POWER_SUPPLY_PROP_PRECHARGE_CURRENT) ||
> >> +               (psp == POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT) ||
> >> +               (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT) ||
> >> +               (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE));
> > Ditto.
> I think I misunderstood you previous review comment "Redundant
> parentheses", no sure what is the expectation

(At least) surrounding pair is not needed, return (a == b) || (c == d);

> > ...
> >
> >> +       return ((psp == POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT) ||
> >> +               (psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT));
> > Ditto.

...

> >> +       struct power_supply_config psy_cfg = {NULL};
> > { 0 }
> >
> NULL to make compiler happy.

Hmm... Can you share warning / error compiler issued in 0 case?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger
  2020-03-28 18:44       ` Andy Shevchenko
@ 2020-03-29 10:34         ` saravanan sekar
  2020-03-29 11:17           ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: saravanan sekar @ 2020-03-29 10:34 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

Hi Andy,

On 28/03/20 7:44 pm, Andy Shevchenko wrote:
> On Sat, Mar 28, 2020 at 1:29 PM saravanan sekar <sravanhome@gmail.com> wrote:
>> On 28/03/20 12:02 pm, Andy Shevchenko wrote:
>>> On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> wrote:
> ...
>
>>>> +       val->intval = (rval * props[fld].step) + props[fld].min;
>>> Too many parentheses.
>>>
>>> ...
>>>
>>>> +       return ((psp == POWER_SUPPLY_PROP_PRECHARGE_CURRENT) ||
>>>> +               (psp == POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT) ||
>>>> +               (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT) ||
>>>> +               (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE));
>>> Ditto.
>> I think I misunderstood you previous review comment "Redundant
>> parentheses", no sure what is the expectation
> (At least) surrounding pair is not needed, return (a == b) || (c == d);
ok, I will remove outer ().
>>> ...
>>>
>>>> +       return ((psp == POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT) ||
>>>> +               (psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT));
>>> Ditto.
> ...
>
>>>> +       struct power_supply_config psy_cfg = {NULL};
>>> { 0 }
>>>
>> NULL to make compiler happy.
> Hmm... Can you share warning / error compiler issued in 0 case?
>
Please see the 0-day warning.

"Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
 >> drivers/power/supply/mp2629_charger.c:584:47: sparse: sparse: Using 
plain integer as NULL pointer"


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger
  2020-03-29 10:34         ` saravanan sekar
@ 2020-03-29 11:17           ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-29 11:17 UTC (permalink / raw)
  To: saravanan sekar
  Cc: Lee Jones, Rob Herring, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Sebastian Reichel,
	devicetree, Linux Kernel Mailing List, linux-iio, Linux PM

On Sun, Mar 29, 2020 at 1:34 PM saravanan sekar <sravanhome@gmail.com> wrote:
> On 28/03/20 7:44 pm, Andy Shevchenko wrote:
> > On Sat, Mar 28, 2020 at 1:29 PM saravanan sekar <sravanhome@gmail.com> wrote:
> >> On 28/03/20 12:02 pm, Andy Shevchenko wrote:
> >>> On Sat, Mar 28, 2020 at 2:12 AM Saravanan Sekar <sravanhome@gmail.com> wrote:

...

> >>>> +       struct power_supply_config psy_cfg = {NULL};
> >>> { 0 }
> >>>
> >> NULL to make compiler happy.
> > Hmm... Can you share warning / error compiler issued in 0 case?
> >
> Please see the 0-day warning.
>
> "Reported-by: kbuild test robot <lkp@intel.com>
> sparse warnings: (new ones prefixed by >>)
>  >> drivers/power/supply/mp2629_charger.c:584:47: sparse: sparse: Using
> plain integer as NULL pointer"

I see. Grepping the code shows that for this certain structure other
drivers are using simple '{}'. Can you align with them?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-03-29 11:17 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-28  0:11 [PATCH v5 0/5] Add battery charger driver support for MP2629 Saravanan Sekar
2020-03-28  0:11 ` [PATCH v5 1/5] dt-bindings: mfd: add document bindings for mp2629 Saravanan Sekar
2020-03-28  0:11 ` [PATCH v5 2/5] mfd: mp2629: Add support for mps battery charger Saravanan Sekar
2020-03-28 10:45   ` Andy Shevchenko
2020-03-28 10:51     ` saravanan sekar
2020-03-28 11:06       ` Andy Shevchenko
2020-03-28  0:11 ` [PATCH v5 3/5] iio: adc: mp2629: Add support for mp2629 ADC driver Saravanan Sekar
2020-03-28 10:52   ` Andy Shevchenko
2020-03-28 17:51     ` Jonathan Cameron
2020-03-28  0:11 ` [PATCH v5 4/5] power: supply: Add support for mps mp2629 battery charger Saravanan Sekar
2020-03-28 11:02   ` Andy Shevchenko
2020-03-28 11:03     ` Andy Shevchenko
2020-03-28 11:29     ` saravanan sekar
2020-03-28 18:44       ` Andy Shevchenko
2020-03-29 10:34         ` saravanan sekar
2020-03-29 11:17           ` Andy Shevchenko
2020-03-28  0:11 ` [PATCH v5 5/5] MAINTAINERS: Add entry for mp2629 Battery Charger driver Saravanan Sekar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).