[PATCH 1/6] iio: adc: mp2629: fix wrong comparison of channel

[PATCH 1/6] iio: adc: mp2629: fix wrong comparison of channel

[Saravanan Sekar]

Input voltage channel enum is compared against iio address instead
of channel.

Fixes: 7abd9fb64682 ("iio: adc: mp2629: Add support for mp2629 ADC driver")
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 drivers/iio/adc/mp2629_adc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mp2629_adc.c b/drivers/iio/adc/mp2629_adc.c
index aca084f1e78a..e7fbfe92e884 100644
--- a/drivers/iio/adc/mp2629_adc.c
+++ b/drivers/iio/adc/mp2629_adc.c
@@ -73,7 +73,7 @@ static int mp2629_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 
-		if (chan->address == MP2629_INPUT_VOLT)
+		if (chan->channel == MP2629_INPUT_VOLT)
 			rval &= GENMASK(6, 0);
 		*val = rval;
 		return IIO_VAL_INT;
-- 
2.25.1

[PATCH 2/6] dt-bindings: mfd: Add mp2733 compatible

[Saravanan Sekar]

Add new compatible for mp2733 mfd driver.

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

diff --git a/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml b/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
index f91acc42d652..fe13869e2c58 100644
--- a/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
+++ b/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
@@ -19,6 +19,7 @@ description: |
 properties:
   compatible:
     const: mps,mp2629
+    const: mps,mp2733
 
   reg:
     maxItems: 1
-- 
2.25.1

[PATCH 3/6] mfd: mp2629: Add support for mps mp2733 battery charger

[Saravanan Sekar]

mp2733 is updated version of mp2629 battery charge management
device for single-cell Li-ion or Li-polymer battery. Additionally
supports usb fast-charge and higher range of input voltage.

Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 drivers/mfd/mp2629.c       | 31 +++++++++++++++++++++++--------
 include/linux/mfd/mp2629.h |  6 ++++++
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/mp2629.c b/drivers/mfd/mp2629.c
index 16840ec5fd1c..e0bbba9ca853 100644
--- a/drivers/mfd/mp2629.c
+++ b/drivers/mfd/mp2629.c
@@ -12,6 +12,7 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/mp2629.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
@@ -33,16 +34,36 @@ static const struct regmap_config mp2629_regmap_config = {
 	.max_register = 0x17,
 };
 
-static int mp2629_probe(struct i2c_client *client)
+static const struct of_device_id mp2629_of_match[] = {
+	{ .compatible = "mps,mp2629", .data = (void *)CHIP_ID_MP2629 },
+	{ .compatible = "mps,mp2733", .data = (void *)CHIP_ID_MP2733 },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mp2629_of_match);
+
+static int mp2629_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
 {
 	struct mp2629_data *ddata;
+	enum mp2xx_chip_id chip_id;
+	const struct of_device_id *of_id;
 	int ret;
 
+	if (client->dev.of_node) {
+		of_id = of_match_device(mp2629_of_match, &client->dev);
+		if (!of_id) {
+			dev_err(&client->dev, "Failed to match device\n");
+			return -ENODEV;
+		}
+		chip_id = (enum mp2xx_chip_id)of_id->data;
+	}
+
 	ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL);
 	if (!ddata)
 		return -ENOMEM;
 
 	ddata->dev = &client->dev;
+	ddata->chip_id = chip_id;
 	i2c_set_clientdata(client, ddata);
 
 	ddata->regmap = devm_regmap_init_i2c(client, &mp2629_regmap_config);
@@ -59,18 +80,12 @@ static int mp2629_probe(struct i2c_client *client)
 	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,
+	.probe	= mp2629_probe,
 };
 module_i2c_driver(mp2629_driver);
 
diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h
index 89b706900b57..ee0e65720c75 100644
--- a/include/linux/mfd/mp2629.h
+++ b/include/linux/mfd/mp2629.h
@@ -9,9 +9,15 @@
 #include <linux/device.h>
 #include <linux/regmap.h>
 
+enum mp2xx_chip_id {
+	CHIP_ID_MP2629,
+	CHIP_ID_MP2733,
+};
+
 struct mp2629_data {
 	struct device *dev;
 	struct regmap *regmap;
+	enum mp2xx_chip_id chip_id;
 };
 
 enum mp2629_adc_chan {
-- 
2.25.1

[PATCH 4/6] iio: adc: mp2629: restrict input voltage mask for mp2629

[Saravanan Sekar]

add support for mp2733 which is updated version of mp2629
with higher range of input voltage.

Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 drivers/iio/adc/mp2629_adc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mp2629_adc.c b/drivers/iio/adc/mp2629_adc.c
index e7fbfe92e884..28d048eefce3 100644
--- a/drivers/iio/adc/mp2629_adc.c
+++ b/drivers/iio/adc/mp2629_adc.c
@@ -64,6 +64,7 @@ static int mp2629_read_raw(struct iio_dev *indio_dev,
 			int *val, int *val2, long mask)
 {
 	struct mp2629_adc *info = iio_priv(indio_dev);
+	struct mp2629_data *ddata = dev_get_drvdata(info->dev);
 	unsigned int rval;
 	int ret;
 
@@ -73,8 +74,10 @@ static int mp2629_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 
-		if (chan->channel == MP2629_INPUT_VOLT)
+		if (chan->channel == MP2629_INPUT_VOLT &&
+		    ddata->chip_id == CHIP_ID_MP2629)
 			rval &= GENMASK(6, 0);
+
 		*val = rval;
 		return IIO_VAL_INT;
 
-- 
2.25.1

[PATCH 5/6] power: supply: Add support for mp2733 battery charger

[Saravanan Sekar]

mp2733 is updated version of mp2629 battery charge management
which supports usb fast-charge and higher range of input voltage.

Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 drivers/power/supply/mp2629_charger.c | 228 ++++++++++++++++++++++----
 1 file changed, 199 insertions(+), 29 deletions(-)

diff --git a/drivers/power/supply/mp2629_charger.c b/drivers/power/supply/mp2629_charger.c
index bf9c27b463a8..6f37bfc40e90 100644
--- a/drivers/power/supply/mp2629_charger.c
+++ b/drivers/power/supply/mp2629_charger.c
@@ -30,12 +30,15 @@
 #define MP2629_REG_INTERRUPT		0x0b
 #define MP2629_REG_STATUS		0x0c
 #define MP2629_REG_FAULT		0x0d
+#define MP2629_REG_FAST_CHARGE		0x17
 
 #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 MP2733_MASK_FAST_CHARGE		GENMASK(2, 1)
+#define MP2733_MASK_FAST_CHARGE_RESET	BIT(0)
 
 #define MP2629_INPUTSOURCE_CHANGE	GENMASK(7, 5)
 #define MP2629_CHARGING_CHANGE		GENMASK(4, 3)
@@ -62,6 +65,17 @@ enum mp2629_source_type {
 	MP2629_SOURCE_TYPE_OTG = 7,
 };
 
+enum mp2733_source_type {
+	MP2733_SOURCE_TYPE_NO_INPUT,
+	MP2733_SOURCE_TYPE_NON_STD,
+	MP2733_SOURCE_TYPE_APPLE_1p0,
+	MP2733_SOURCE_TYPE_APPLE_2p1,
+	MP2733_SOURCE_TYPE_APPLE_2p4,
+	MP2733_SOURCE_TYPE_SDP,
+	MP2733_SOURCE_TYPE_CDP,
+	MP2733_SOURCE_TYPE_DCP,
+};
+
 enum mp2629_field {
 	INPUT_ILIM,
 	INPUT_VLIM,
@@ -72,11 +86,30 @@ enum mp2629_field {
 	MP2629_MAX_FIELD
 };
 
+struct mp2629_prop {
+	int reg;
+	int mask;
+	int min;
+	int max;
+	int step;
+	int shift;
+};
+
+struct mp2xx_chip_info {
+	const struct reg_field *rfields;
+	struct mp2629_prop *chip_props;
+	bool has_impedance;
+	bool has_fast_charge;
+
+	int (*mpxx_get_usb_type)(unsigned int rval);
+};
+
 struct mp2629_charger {
 	struct device *dev;
 	int status;
 	int fault;
 
+	const struct mp2xx_chip_info *chip_info;
 	struct regmap *regmap;
 	struct regmap_field *regmap_fields[MP2629_MAX_FIELD];
 	struct mutex lock;
@@ -85,15 +118,6 @@ struct mp2629_charger {
 	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,
@@ -126,7 +150,25 @@ static enum power_supply_property mp2629_charger_bat_props[] = {
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
 };
 
-static struct mp2629_prop props[] = {
+static struct mp2629_prop mp2733_chip_props[] = {
+	MP2629_PROPS(INPUT_ILIM, 100000, 3250000, 50000),
+	MP2629_PROPS(INPUT_VLIM, 3800000, 15200000, 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 mp2733_reg_fields[] = {
+	[INPUT_ILIM]	= REG_FIELD(MP2629_REG_INPUT_ILIM, 0, 5),
+	[INPUT_VLIM]	= REG_FIELD(MP2629_REG_INPUT_VLIM, 0, 6),
+	[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 struct mp2629_prop mp2629_chip_props[] = {
 	MP2629_PROPS(INPUT_ILIM, 100000, 3250000, 50000),
 	MP2629_PROPS(INPUT_VLIM, 3800000, 5300000, 100000),
 	MP2629_PROPS(CHARGE_ILIM, 320000, 4520000, 40000),
@@ -174,6 +216,7 @@ static int mp2629_get_prop(struct mp2629_charger *charger,
 {
 	int ret;
 	unsigned int rval;
+	struct mp2629_prop *props = charger->chip_info->chip_props;
 
 	ret = regmap_field_read(charger->regmap_fields[fld], &rval);
 	if (ret)
@@ -189,6 +232,7 @@ static int mp2629_set_prop(struct mp2629_charger *charger,
 			   const union power_supply_propval *val)
 {
 	unsigned int rval;
+	struct mp2629_prop *props = charger->chip_info->chip_props;
 
 	if (val->intval < props[fld].min || val->intval > props[fld].max)
 		return -EINVAL;
@@ -311,6 +355,16 @@ static int mp2629_charger_battery_get_prop(struct power_supply *psy,
 		default:
 			val->intval = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
 		}
+
+		if (charger->chip_info->has_fast_charge) {
+			ret = regmap_read(charger->regmap,
+					  MP2629_REG_FAST_CHARGE, &rval);
+			if (ret)
+				break;
+
+			if (rval & BIT(6))
+				val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+		}
 		break;
 
 	default:
@@ -344,6 +398,58 @@ static int mp2629_charger_battery_set_prop(struct power_supply *psy,
 	}
 }
 
+static int mp2733_get_usb_type(unsigned int rval)
+{
+	int type;
+
+	switch (rval) {
+	case MP2733_SOURCE_TYPE_APPLE_1p0:
+	case MP2733_SOURCE_TYPE_APPLE_2p1:
+	case MP2733_SOURCE_TYPE_APPLE_2p4:
+		type = POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID;
+		break;
+	case MP2733_SOURCE_TYPE_SDP:
+		type = POWER_SUPPLY_USB_TYPE_SDP;
+		break;
+	case MP2733_SOURCE_TYPE_CDP:
+		type = POWER_SUPPLY_USB_TYPE_CDP;
+		break;
+	case MP2733_SOURCE_TYPE_DCP:
+		type = POWER_SUPPLY_USB_TYPE_DCP;
+		break;
+	default:
+		type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
+		break;
+	}
+
+	return type;
+}
+
+static int mp2629_get_usb_type(unsigned int rval)
+{
+	int type;
+
+	switch (rval) {
+	case MP2629_SOURCE_TYPE_SDP:
+		type = POWER_SUPPLY_USB_TYPE_SDP;
+		break;
+	case MP2629_SOURCE_TYPE_CDP:
+		type = POWER_SUPPLY_USB_TYPE_CDP;
+		break;
+	case MP2629_SOURCE_TYPE_DCP:
+		type = POWER_SUPPLY_USB_TYPE_DCP;
+		break;
+	case MP2629_SOURCE_TYPE_OTG:
+		type = POWER_SUPPLY_USB_TYPE_PD_DRP;
+		break;
+	default:
+		type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
+		break;
+	}
+
+	return type;
+}
+
 static int mp2629_charger_usb_get_prop(struct power_supply *psy,
 				enum power_supply_property psp,
 				union power_supply_propval *val)
@@ -367,23 +473,7 @@ static int mp2629_charger_usb_get_prop(struct power_supply *psy,
 			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;
-		}
+		val->intval = charger->chip_info->mpxx_get_usb_type(rval);
 		break;
 
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
@@ -556,6 +646,60 @@ static struct attribute *mp2629_charger_sysfs_attrs[] = {
 };
 ATTRIBUTE_GROUPS(mp2629_charger_sysfs);
 
+static ssize_t usb_fast_charge_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_INTERRUPT, &rval);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "USB DP %u:DM %u\n", !!(rval & BIT(2)),
+			 !!(rval & BIT(1)));
+}
+
+static ssize_t usb_fast_charge_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf,
+				     size_t count)
+{
+	struct mp2629_charger *charger = dev_get_drvdata(dev->parent);
+	unsigned int val;
+	int ret;
+
+	ret = kstrtouint(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	if (val > 3)
+		return -ERANGE;
+
+	ret = regmap_update_bits(charger->regmap, MP2629_REG_INTERRUPT,
+				 MP2733_MASK_FAST_CHARGE, val << 1);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(charger->regmap, MP2629_REG_INTERRUPT,
+				 MP2733_MASK_FAST_CHARGE_RESET,
+				 MP2733_MASK_FAST_CHARGE_RESET);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(usb_fast_charge);
+
+static struct attribute *mp2733_charger_sysfs_attrs[] = {
+	&dev_attr_usb_fast_charge.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(mp2733_charger_sysfs);
+
 static void mp2629_charger_disable(void *data)
 {
 	struct mp2629_charger *charger = data;
@@ -564,6 +708,25 @@ static void mp2629_charger_disable(void *data)
 					MP2629_MASK_CHARGE_CTRL, 0);
 }
 
+static const struct mp2xx_chip_info mp2xx_chip_info_tbl[] = {
+	[CHIP_ID_MP2629] = {
+		.rfields = mp2629_reg_fields,
+		.chip_props = mp2629_chip_props,
+		.has_impedance = 1,
+		.has_fast_charge = 0,
+
+		.mpxx_get_usb_type = mp2629_get_usb_type,
+	},
+	[CHIP_ID_MP2733] = {
+		.rfields = mp2733_reg_fields,
+		.chip_props = mp2733_chip_props,
+		.has_impedance = 0,
+		.has_fast_charge = 1,
+
+		.mpxx_get_usb_type = mp2733_get_usb_type,
+	},
+};
+
 static int mp2629_charger_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -578,6 +741,7 @@ static int mp2629_charger_probe(struct platform_device *pdev)
 
 	charger->regmap = ddata->regmap;
 	charger->dev = dev;
+	charger->chip_info = &mp2xx_chip_info_tbl[ddata->chip_id];
 	platform_set_drvdata(pdev, charger);
 
 	irq = platform_get_irq(to_platform_device(dev->parent), 0);
@@ -586,7 +750,8 @@ static int mp2629_charger_probe(struct platform_device *pdev)
 
 	for (i = 0; i < MP2629_MAX_FIELD; i++) {
 		charger->regmap_fields[i] = devm_regmap_field_alloc(dev,
-					charger->regmap, mp2629_reg_fields[i]);
+					charger->regmap,
+					charger->chip_info->rfields[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]);
@@ -613,7 +778,12 @@ static int mp2629_charger_probe(struct platform_device *pdev)
 	}
 
 	psy_cfg.drv_data = charger;
-	psy_cfg.attr_grp = mp2629_charger_sysfs_groups;
+	if (charger->chip_info->has_impedance)
+		psy_cfg.attr_grp = mp2629_charger_sysfs_groups;
+
+	if (charger->chip_info->has_fast_charge)
+		psy_cfg.attr_grp = mp2733_charger_sysfs_groups;
+
 	charger->battery = devm_power_supply_register(dev,
 					 &mp2629_battery_desc, &psy_cfg);
 	if (IS_ERR(charger->battery)) {
-- 
2.25.1

[PATCH 6/6] power: supply: mp2629: Add usb fast charge settings

[Saravanan Sekar]

Allows the user to change the usb device fast charge setting to advertise
host on enumeration helps to accelerate the charging cycle. Altering this
value resets usb existing connection.

Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 .../ABI/testing/sysfs-class-power-mp2629         | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-power-mp2629 b/Documentation/ABI/testing/sysfs-class-power-mp2629
index 914d67caac0d..42f37675aa0e 100644
--- a/Documentation/ABI/testing/sysfs-class-power-mp2629
+++ b/Documentation/ABI/testing/sysfs-class-power-mp2629
@@ -1,3 +1,19 @@
+What:		/sys/class/power_supply/mp2629_battery/usb_fast_charge
+Date:		April 2020
+KernelVersion:	5.19
+Description:
+		Represents a usb device fast charge settings.Altering this
+		value resets usb existing connection
+		USB DP:DM[0:0] 0.6V : Hi-Z
+		USB DP:DM[0:1] 3.3V : 0.6V
+		USB DP:DM[1:0] 0.6V : 0.6V
+		USB DP:DM[1:1] 0.6V : 3.3V
+
+                Access: Read, Write
+
+                Valid values: Represented in bit DP & DM setting. Valid
+			      range is [0, 3].
+
 What:		/sys/class/power_supply/mp2629_battery/batt_impedance_compen
 Date:		April 2020
 KernelVersion:	5.7
-- 
2.25.1

Re: [PATCH 3/6] mfd: mp2629: Add support for mps mp2733 battery charger

[Andy Shevchenko]

On Tue, Jun 14, 2022 at 5:17 PM Saravanan Sekar <sravanhome@gmail.com> wrote:
>
> mp2733 is updated version of mp2629 battery charge management
> device for single-cell Li-ion or Li-polymer battery. Additionally
> supports usb fast-charge and higher range of input voltage.

...

> +#include <linux/of_device.h>

What the original code misses is the mod_devicetable.h, and also see below.

...

> +static const struct of_device_id mp2629_of_match[] = {
> +       { .compatible = "mps,mp2629", .data = (void *)CHIP_ID_MP2629 },
> +       { .compatible = "mps,mp2733", .data = (void *)CHIP_ID_MP2733 },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, mp2629_of_match);

No need to move, see below.

...

> +static int mp2629_probe(struct i2c_client *client,
> +                       const struct i2c_device_id *id)

Why out of a sudden you moved from ->probe_new() to ->probe()?

> +       enum mp2xx_chip_id chip_id;
> +       const struct of_device_id *of_id;
>         int ret;
>
> +       if (client->dev.of_node) {
> +               of_id = of_match_device(mp2629_of_match, &client->dev);
> +               if (!of_id) {
> +                       dev_err(&client->dev, "Failed to match device\n");
> +                       return -ENODEV;
> +               }
> +               chip_id = (enum mp2xx_chip_id)of_id->data;
> +       }

This all is a single LoC only + property.h:

#include <linux/property.h>

     enum mp2xx_chip_id chip_id;

     chip_id = (uintptr_t)device_get_match_data(&client->dev);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/6] iio: adc: mp2629: restrict input voltage mask for mp2629

[Andy Shevchenko]

On Tue, Jun 14, 2022 at 5:17 PM Saravanan Sekar <sravanhome@gmail.com> wrote:
>
> add support for mp2733 which is updated version of mp2629
> with higher range of input voltage.

a higher

...

> +               if (chan->channel == MP2629_INPUT_VOLT &&
> +                   ddata->chip_id == CHIP_ID_MP2629)
>                         rval &= GENMASK(6, 0);

> +

(Semi-?)stray change. Up to you and Jonathan.

>                 *val = rval;


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 5/6] power: supply: Add support for mp2733 battery charger

[Andy Shevchenko]

On Tue, Jun 14, 2022 at 5:17 PM Saravanan Sekar <sravanhome@gmail.com> wrote:
>
> mp2733 is updated version of mp2629 battery charge management
> which supports usb fast-charge and higher range of input voltage.

USB

...

> +static int mp2733_get_usb_type(unsigned int rval)
> +{
> +       int type;

Unneeded, just return directly.

> +       switch (rval) {
> +       case MP2733_SOURCE_TYPE_APPLE_1p0:
> +       case MP2733_SOURCE_TYPE_APPLE_2p1:
> +       case MP2733_SOURCE_TYPE_APPLE_2p4:
> +               type = POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID;
> +               break;
> +       case MP2733_SOURCE_TYPE_SDP:
> +               type = POWER_SUPPLY_USB_TYPE_SDP;
> +               break;
> +       case MP2733_SOURCE_TYPE_CDP:
> +               type = POWER_SUPPLY_USB_TYPE_CDP;
> +               break;
> +       case MP2733_SOURCE_TYPE_DCP:
> +               type = POWER_SUPPLY_USB_TYPE_DCP;
> +               break;
> +       default:
> +               type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
> +               break;
> +       }
> +
> +       return type;
> +}
> +
> +static int mp2629_get_usb_type(unsigned int rval)
> +{
> +       int type;

Ditto.

> +       switch (rval) {
> +       case MP2629_SOURCE_TYPE_SDP:
> +               type = POWER_SUPPLY_USB_TYPE_SDP;
> +               break;
> +       case MP2629_SOURCE_TYPE_CDP:
> +               type = POWER_SUPPLY_USB_TYPE_CDP;
> +               break;
> +       case MP2629_SOURCE_TYPE_DCP:
> +               type = POWER_SUPPLY_USB_TYPE_DCP;
> +               break;
> +       case MP2629_SOURCE_TYPE_OTG:
> +               type = POWER_SUPPLY_USB_TYPE_PD_DRP;
> +               break;
> +       default:
> +               type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
> +               break;
> +       }
> +
> +       return type;
> +}

...

> +       return sprintf(buf, "USB DP %u:DM %u\n", !!(rval & BIT(2)),
> +                        !!(rval & BIT(1)));

sysfs_emit() ?

...

> +static const struct mp2xx_chip_info mp2xx_chip_info_tbl[] = {
> +       [CHIP_ID_MP2629] = {
> +               .rfields = mp2629_reg_fields,
> +               .chip_props = mp2629_chip_props,
> +               .has_impedance = 1,

> +               .has_fast_charge = 0,

Unneeded.

> +               .mpxx_get_usb_type = mp2629_get_usb_type,
> +       },
> +       [CHIP_ID_MP2733] = {
> +               .rfields = mp2733_reg_fields,
> +               .chip_props = mp2733_chip_props,

> +               .has_impedance = 0,

Ditto.

> +               .has_fast_charge = 1,
> +
> +               .mpxx_get_usb_type = mp2733_get_usb_type,
> +       },
> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 6/6] power: supply: mp2629: Add usb fast charge settings

[Andy Shevchenko]

On Tue, Jun 14, 2022 at 5:17 PM Saravanan Sekar <sravanhome@gmail.com> wrote:
>
> Allows the user to change the usb device fast charge setting to advertise
> host on enumeration helps to accelerate the charging cycle. Altering this
> value resets usb existing connection.

USB

...

> +KernelVersion: 5.19

I don't think it will be available at v5.19. v5.20?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/6] mfd: mp2629: Add support for mps mp2733 battery charger

[saravanan sekar]

Hello Andy,
Thanks for your time to review, I try fix all the review comments

On 14/06/22 18:05, Andy Shevchenko wrote:
> On Tue, Jun 14, 2022 at 5:17 PM Saravanan Sekar <sravanhome@gmail.com> wrote:
>>
>> mp2733 is updated version of mp2629 battery charge management
>> device for single-cell Li-ion or Li-polymer battery. Additionally
>> supports usb fast-charge and higher range of input voltage.
> 
> ...
> 
>> +#include <linux/of_device.h>
> 
> What the original code misses is the mod_devicetable.h, and also see below.
> 
> ...
> 
>> +static const struct of_device_id mp2629_of_match[] = {
>> +       { .compatible = "mps,mp2629", .data = (void *)CHIP_ID_MP2629 },
>> +       { .compatible = "mps,mp2733", .data = (void *)CHIP_ID_MP2733 },
>> +       { }
>> +};
>> +MODULE_DEVICE_TABLE(of, mp2629_of_match);
> 
> No need to move, see below.
> 
> ...
> 
>> +static int mp2629_probe(struct i2c_client *client,
>> +                       const struct i2c_device_id *id)
> 
> Why out of a sudden you moved from ->probe_new() to ->probe()?
> 
I was experiment to pass i2c_device_id table to differentiate, the used 
compatible. I will switch back to probe_new.

>> +       enum mp2xx_chip_id chip_id;
>> +       const struct of_device_id *of_id;
>>          int ret;
>>
>> +       if (client->dev.of_node) {
>> +               of_id = of_match_device(mp2629_of_match, &client->dev);
>> +               if (!of_id) {
>> +                       dev_err(&client->dev, "Failed to match device\n");
>> +                       return -ENODEV;
>> +               }
>> +               chip_id = (enum mp2xx_chip_id)of_id->data;
>> +       }
> 
> This all is a single LoC only + property.h:
> 
> #include <linux/property.h>
> 
>       enum mp2xx_chip_id chip_id;
> 
>       chip_id = (uintptr_t)device_get_match_data(&client->dev);
> 
sure.


Thanks,
Saravanan

Re: [PATCH 2/6] dt-bindings: mfd: Add mp2733 compatible

[Lee Jones]

On Tue, 14 Jun 2022, Saravanan Sekar wrote:

> Add new compatible for mp2733 mfd driver.
> 
> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
> ---
>  Documentation/devicetree/bindings/mfd/mps,mp2629.yaml | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/6] dt-bindings: mfd: Add mp2733 compatible

[Rob Herring]

On Wed, Jun 15, 2022 at 4:59 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Tue, 14 Jun 2022, Saravanan Sekar wrote:
>
> > Add new compatible for mp2733 mfd driver.
> >
> > Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/mfd/mps,mp2629.yaml | 1 +
> >  1 file changed, 1 insertion(+)
>
> Applied, thanks.

This breaks linux-next. Please apply v3 instead.

Rob

Re: [PATCH 2/6] dt-bindings: mfd: Add mp2733 compatible

[Lee Jones]

On Mon, 27 Jun 2022, Rob Herring wrote:

> On Wed, Jun 15, 2022 at 4:59 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Tue, 14 Jun 2022, Saravanan Sekar wrote:
> >
> > > Add new compatible for mp2733 mfd driver.
> > >
> > > Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
> > > ---
> > >  Documentation/devicetree/bindings/mfd/mps,mp2629.yaml | 1 +
> > >  1 file changed, 1 insertion(+)
> >
> > Applied, thanks.
> 
> This breaks linux-next. Please apply v3 instead.

Done, thanks.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog