[PATCH v2 0/4] Add max77620 charging & low battery support

[PATCH v2 0/4] Add max77620 charging & low battery support

[Mark Zhang]

This patch set adds support for max77620 backup battery charging and
low battery monitoring.

Changes in v2:
- Add devicetree binding documentation

Mark Zhang (4):
  mfd: max77620: Add backup battery charger support
  mfd: max77620: add documentation for backup battery charging
  mfd: max77620: Add low battery monitor support
  mfd: max77620: add documentation for low battery monitoring

 .../devicetree/bindings/mfd/max77620.txt      |  34 +++++
 drivers/mfd/max77620.c                        | 137 +++++++++++++++++-
 2 files changed, 170 insertions(+), 1 deletion(-)

-- 
2.19.2

[PATCH v2 0/4] Add max77620 charging & low battery support

[Mark Zhang]

This patch set adds support for max77620 backup battery charging and
low battery monitoring.

Changes in v2:
- Add devicetree binding documentation

Mark Zhang (4):
  mfd: max77620: Add backup battery charger support
  mfd: max77620: add documentation for backup battery charging
  mfd: max77620: Add low battery monitor support
  mfd: max77620: add documentation for low battery monitoring

 .../devicetree/bindings/mfd/max77620.txt      |  34 +++++
 drivers/mfd/max77620.c                        | 137 +++++++++++++++++-
 2 files changed, 170 insertions(+), 1 deletion(-)

-- 
2.19.2

[PATCH v2 1/4] mfd: max77620: Add backup battery charger support

[Mark Zhang]

Add PMIC configurations for backup battery charger, which
is a constant voltage and constant current style charger
with a series output resistance.

The max77620 register CNFGBBC(addr: 0x04) defines the
parameters of backup battery charger. This patch adds
support for it.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Venkat Reddy Talla <vreddytalla@nvidia.com>
Signed-off-by: Mark Zhang <markz@nvidia.com>
---
 drivers/mfd/max77620.c | 80 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c
index d8ddd1a6f304..f58143103185 100644
--- a/drivers/mfd/max77620.c
+++ b/drivers/mfd/max77620.c
@@ -398,6 +398,82 @@ static int max77620_initialise_fps(struct max77620_chip *chip)
 	return 0;
 }
 
+static int max77620_init_backup_battery_charging(struct max77620_chip *chip)
+{
+	struct device *dev = chip->dev;
+	struct device_node *np;
+	u32 pval;
+	u8 config;
+	int charging_current;
+	int charging_voltage;
+	int resistor;
+	int ret;
+
+	np = of_get_child_by_name(dev->of_node, "backup-battery");
+	if (!np) {
+		dev_info(dev, "Backup battery charging support disabled\n");
+		ret = regmap_update_bits(chip->rmap, MAX77620_REG_CNFGBBC,
+					 MAX77620_CNFGBBC_ENABLE, 0);
+		if (ret < 0)
+			dev_err(dev, "Failed to update CNFGBBC: %d\n", ret);
+		return ret;
+	}
+
+	ret = of_property_read_u32(np,
+			"maxim,backup-battery-charging-current", &pval);
+	charging_current = (!ret) ? pval : 50;
+
+	ret = of_property_read_u32(np,
+			"maxim,backup-battery-charging-voltage", &pval);
+	charging_voltage = (!ret) ? pval : 2500000;
+	charging_voltage /= 1000;
+
+	ret = of_property_read_u32(np,
+			"maxim,backup-battery-output-resister", &pval);
+	resistor = (!ret) ? pval : 1000;
+
+	config = MAX77620_CNFGBBC_ENABLE;
+	if (charging_current <= 50)
+		config |= 0 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else if (charging_current <= 100)
+		config |= 3 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else if (charging_current <= 200)
+		config |= 0 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else if (charging_current <= 400)
+		config |= 3 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else if (charging_current <= 600)
+		config |= 1 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else
+		config |= 2 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+
+	if (charging_current > 100)
+		config |= MAX77620_CNFGBBC_LOW_CURRENT_DISABLE;
+
+	if (charging_voltage <= 2500)
+		config |= 0 << MAX77620_CNFGBBC_VOLTAGE_SHIFT;
+	else if (charging_voltage <= 3000)
+		config |= 1 << MAX77620_CNFGBBC_VOLTAGE_SHIFT;
+	else if (charging_voltage <= 3300)
+		config |= 2 << MAX77620_CNFGBBC_VOLTAGE_SHIFT;
+	else
+		config |= 3 << MAX77620_CNFGBBC_VOLTAGE_SHIFT;
+
+	if (resistor <= 100)
+		config |= 0 << MAX77620_CNFGBBC_RESISTOR_SHIFT;
+	else if (resistor <= 1000)
+		config |= 1 << MAX77620_CNFGBBC_RESISTOR_SHIFT;
+	else if (resistor <= 3000)
+		config |= 2 << MAX77620_CNFGBBC_RESISTOR_SHIFT;
+	else if (resistor <= 6000)
+		config |= 3 << MAX77620_CNFGBBC_RESISTOR_SHIFT;
+
+	ret = regmap_write(chip->rmap, MAX77620_REG_CNFGBBC, config);
+	if (ret < 0)
+		dev_err(dev, "Reg 0x%02x write failed, %d\n",
+				MAX77620_REG_CNFGBBC, ret);
+	return ret;
+}
+
 static int max77620_read_es_version(struct max77620_chip *chip)
 {
 	unsigned int val;
@@ -483,6 +559,10 @@ static int max77620_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
+	ret = max77620_init_backup_battery_charging(chip);
+	if (ret < 0)
+		return ret;
+
 	ret =  devm_mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
 				    mfd_cells, n_mfd_cells, NULL, 0,
 				    regmap_irq_get_domain(chip->top_irq_data));
-- 
2.19.2

[PATCH v2 1/4] mfd: max77620: Add backup battery charger support

[Mark Zhang]

Add PMIC configurations for backup battery charger, which
is a constant voltage and constant current style charger
with a series output resistance.

The max77620 register CNFGBBC(addr: 0x04) defines the
parameters of backup battery charger. This patch adds
support for it.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Venkat Reddy Talla <vreddytalla@nvidia.com>
Signed-off-by: Mark Zhang <markz@nvidia.com>
---
 drivers/mfd/max77620.c | 80 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c
index d8ddd1a6f304..f58143103185 100644
--- a/drivers/mfd/max77620.c
+++ b/drivers/mfd/max77620.c
@@ -398,6 +398,82 @@ static int max77620_initialise_fps(struct max77620_chip *chip)
 	return 0;
 }
 
+static int max77620_init_backup_battery_charging(struct max77620_chip *chip)
+{
+	struct device *dev = chip->dev;
+	struct device_node *np;
+	u32 pval;
+	u8 config;
+	int charging_current;
+	int charging_voltage;
+	int resistor;
+	int ret;
+
+	np = of_get_child_by_name(dev->of_node, "backup-battery");
+	if (!np) {
+		dev_info(dev, "Backup battery charging support disabled\n");
+		ret = regmap_update_bits(chip->rmap, MAX77620_REG_CNFGBBC,
+					 MAX77620_CNFGBBC_ENABLE, 0);
+		if (ret < 0)
+			dev_err(dev, "Failed to update CNFGBBC: %d\n", ret);
+		return ret;
+	}
+
+	ret = of_property_read_u32(np,
+			"maxim,backup-battery-charging-current", &pval);
+	charging_current = (!ret) ? pval : 50;
+
+	ret = of_property_read_u32(np,
+			"maxim,backup-battery-charging-voltage", &pval);
+	charging_voltage = (!ret) ? pval : 2500000;
+	charging_voltage /= 1000;
+
+	ret = of_property_read_u32(np,
+			"maxim,backup-battery-output-resister", &pval);
+	resistor = (!ret) ? pval : 1000;
+
+	config = MAX77620_CNFGBBC_ENABLE;
+	if (charging_current <= 50)
+		config |= 0 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else if (charging_current <= 100)
+		config |= 3 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else if (charging_current <= 200)
+		config |= 0 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else if (charging_current <= 400)
+		config |= 3 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else if (charging_current <= 600)
+		config |= 1 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+	else
+		config |= 2 << MAX77620_CNFGBBC_CURRENT_SHIFT;
+
+	if (charging_current > 100)
+		config |= MAX77620_CNFGBBC_LOW_CURRENT_DISABLE;
+
+	if (charging_voltage <= 2500)
+		config |= 0 << MAX77620_CNFGBBC_VOLTAGE_SHIFT;
+	else if (charging_voltage <= 3000)
+		config |= 1 << MAX77620_CNFGBBC_VOLTAGE_SHIFT;
+	else if (charging_voltage <= 3300)
+		config |= 2 << MAX77620_CNFGBBC_VOLTAGE_SHIFT;
+	else
+		config |= 3 << MAX77620_CNFGBBC_VOLTAGE_SHIFT;
+
+	if (resistor <= 100)
+		config |= 0 << MAX77620_CNFGBBC_RESISTOR_SHIFT;
+	else if (resistor <= 1000)
+		config |= 1 << MAX77620_CNFGBBC_RESISTOR_SHIFT;
+	else if (resistor <= 3000)
+		config |= 2 << MAX77620_CNFGBBC_RESISTOR_SHIFT;
+	else if (resistor <= 6000)
+		config |= 3 << MAX77620_CNFGBBC_RESISTOR_SHIFT;
+
+	ret = regmap_write(chip->rmap, MAX77620_REG_CNFGBBC, config);
+	if (ret < 0)
+		dev_err(dev, "Reg 0x%02x write failed, %d\n",
+				MAX77620_REG_CNFGBBC, ret);
+	return ret;
+}
+
 static int max77620_read_es_version(struct max77620_chip *chip)
 {
 	unsigned int val;
@@ -483,6 +559,10 @@ static int max77620_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
+	ret = max77620_init_backup_battery_charging(chip);
+	if (ret < 0)
+		return ret;
+
 	ret =  devm_mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
 				    mfd_cells, n_mfd_cells, NULL, 0,
 				    regmap_irq_get_domain(chip->top_irq_data));
-- 
2.19.2

[PATCH v2 2/4] mfd: max77620: add documentation for backup battery charging

[Mark Zhang]

Adding documentation for 3 new backup battery charging dts
properties:
- maxim,backup-battery-charging-current
- maxim,backup-battery-charging-voltage
- maxim,backup-battery-output-resister

Signed-off-by: Mark Zhang <markz@nvidia.com>
---
 .../devicetree/bindings/mfd/max77620.txt      | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
index 9c16d51cc15b..484b17e4fba5 100644
--- a/Documentation/devicetree/bindings/mfd/max77620.txt
+++ b/Documentation/devicetree/bindings/mfd/max77620.txt
@@ -122,6 +122,26 @@ For DT binding details of different sub modules like GPIO, pincontrol,
 regulator, power, please refer respective device-tree binding document
 under their respective sub-system directories.
 
+Backup Battery:
+==============
+This sub-node configure charging backup battery of the device. Device has
+support of charging the backup battery. The subnode name is "backup-battery".
+The property for backup-battery child nodes as:
+Presence of this child node will enable the backup battery charging.
+
+Optional properties:
+	-maxim,backup-battery-charging-current: Charging current setting.
+			The device supports 50/100/200/400/600/800uA.
+			If this property is unavailable then it will
+			charge with 50uA.
+	-maxim,backup-battery-charging-voltage: Charging Voltage Limit Setting.
+			Device supports 2500000/3000000/3300000/350000uV.
+			Default will be set to 2500mV. The voltage will be roundoff
+			to nearest lower side if other than above is configured.
+	-maxim,backup-battery-output-resister: Output resistor on Ohm.
+			Device supports 100/1000/3000/6000 Ohms.
+			Default will be set to 1000 Ohm.
+
 Example:
 --------
 #include <dt-bindings/mfd/max77620.h>
-- 
2.19.2

[PATCH v2 2/4] mfd: max77620: add documentation for backup battery charging

[Mark Zhang]

Adding documentation for 3 new backup battery charging dts
properties:
- maxim,backup-battery-charging-current
- maxim,backup-battery-charging-voltage
- maxim,backup-battery-output-resister

Signed-off-by: Mark Zhang <markz@nvidia.com>
---
 .../devicetree/bindings/mfd/max77620.txt      | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
index 9c16d51cc15b..484b17e4fba5 100644
--- a/Documentation/devicetree/bindings/mfd/max77620.txt
+++ b/Documentation/devicetree/bindings/mfd/max77620.txt
@@ -122,6 +122,26 @@ For DT binding details of different sub modules like GPIO, pincontrol,
 regulator, power, please refer respective device-tree binding document
 under their respective sub-system directories.
 
+Backup Battery:
+==============
+This sub-node configure charging backup battery of the device. Device has
+support of charging the backup battery. The subnode name is "backup-battery".
+The property for backup-battery child nodes as:
+Presence of this child node will enable the backup battery charging.
+
+Optional properties:
+	-maxim,backup-battery-charging-current: Charging current setting.
+			The device supports 50/100/200/400/600/800uA.
+			If this property is unavailable then it will
+			charge with 50uA.
+	-maxim,backup-battery-charging-voltage: Charging Voltage Limit Setting.
+			Device supports 2500000/3000000/3300000/350000uV.
+			Default will be set to 2500mV. The voltage will be roundoff
+			to nearest lower side if other than above is configured.
+	-maxim,backup-battery-output-resister: Output resistor on Ohm.
+			Device supports 100/1000/3000/6000 Ohms.
+			Default will be set to 1000 Ohm.
+
 Example:
 --------
 #include <dt-bindings/mfd/max77620.h>
-- 
2.19.2

[PATCH v2 3/4] mfd: max77620: Add low battery monitor support

[Mark Zhang]

This patch adds PMIC configurations for low-battery
monitoring by handling max77620 register CNFGGLBL1.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Venkat Reddy Talla <vreddytalla@nvidia.com>
Signed-off-by: Mark Zhang <markz@nvidia.com>
---
 drivers/mfd/max77620.c | 57 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c
index f58143103185..9e50d145afd8 100644
--- a/drivers/mfd/max77620.c
+++ b/drivers/mfd/max77620.c
@@ -474,6 +474,57 @@ static int max77620_init_backup_battery_charging(struct max77620_chip *chip)
 	return ret;
 }
 
+static int max77620_init_low_battery_monitor(struct max77620_chip *chip)
+{
+	struct device *dev = chip->dev;
+	struct device_node *np;
+	bool pval;
+	u8 mask = 0;
+	u8 val = 0;
+	int ret;
+
+	np = of_get_child_by_name(dev->of_node, "low-battery-monitor");
+	if (!np) {
+		dev_info(dev, "Low battery monitoring support disabled\n");
+		return 0;
+	}
+
+	pval = of_property_read_bool(np, "maxim,low-battery-dac-enable");
+	if (pval) {
+		mask |= MAX77620_CNFGGLBL1_LBDAC_EN;
+		val |= MAX77620_CNFGGLBL1_LBDAC_EN;
+	}
+
+	pval = of_property_read_bool(np, "maxim,low-battery-dac-disable");
+	if (pval)
+		mask |= MAX77620_CNFGGLBL1_LBDAC_EN;
+
+	pval = of_property_read_bool(np, "maxim,low-battery-shutdown-enable");
+	if (pval) {
+		mask |= MAX77620_CNFGGLBL1_MPPLD;
+		val |= MAX77620_CNFGGLBL1_MPPLD;
+	}
+
+	pval = of_property_read_bool(np, "maxim,low-battery-shutdown-disable");
+	if (pval)
+		mask |= MAX77620_CNFGGLBL1_MPPLD;
+
+	pval = of_property_read_bool(np, "maxim,low-battery-reset-enable");
+	if (pval) {
+		mask |= MAX77620_CNFGGLBL1_LBRSTEN;
+		val |= MAX77620_CNFGGLBL1_LBRSTEN;
+	}
+
+	pval = of_property_read_bool(np, "maxim,low-battery-reset-disable");
+	if (pval)
+		mask |= MAX77620_CNFGGLBL1_LBRSTEN;
+
+	ret = regmap_update_bits(chip->rmap, MAX77620_REG_CNFGGLBL1, mask, val);
+	if (ret < 0)
+		dev_err(dev, "Reg CNFGGLBL1 update failed: %d\n", ret);
+	return ret;
+}
+
 static int max77620_read_es_version(struct max77620_chip *chip)
 {
 	unsigned int val;
@@ -563,7 +614,11 @@ static int max77620_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
-	ret =  devm_mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
+	ret = max77620_init_low_battery_monitor(chip);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
 				    mfd_cells, n_mfd_cells, NULL, 0,
 				    regmap_irq_get_domain(chip->top_irq_data));
 	if (ret < 0) {
-- 
2.19.2

[PATCH v2 3/4] mfd: max77620: Add low battery monitor support

[Mark Zhang]

This patch adds PMIC configurations for low-battery
monitoring by handling max77620 register CNFGGLBL1.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Venkat Reddy Talla <vreddytalla@nvidia.com>
Signed-off-by: Mark Zhang <markz@nvidia.com>
---
 drivers/mfd/max77620.c | 57 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c
index f58143103185..9e50d145afd8 100644
--- a/drivers/mfd/max77620.c
+++ b/drivers/mfd/max77620.c
@@ -474,6 +474,57 @@ static int max77620_init_backup_battery_charging(struct max77620_chip *chip)
 	return ret;
 }
 
+static int max77620_init_low_battery_monitor(struct max77620_chip *chip)
+{
+	struct device *dev = chip->dev;
+	struct device_node *np;
+	bool pval;
+	u8 mask = 0;
+	u8 val = 0;
+	int ret;
+
+	np = of_get_child_by_name(dev->of_node, "low-battery-monitor");
+	if (!np) {
+		dev_info(dev, "Low battery monitoring support disabled\n");
+		return 0;
+	}
+
+	pval = of_property_read_bool(np, "maxim,low-battery-dac-enable");
+	if (pval) {
+		mask |= MAX77620_CNFGGLBL1_LBDAC_EN;
+		val |= MAX77620_CNFGGLBL1_LBDAC_EN;
+	}
+
+	pval = of_property_read_bool(np, "maxim,low-battery-dac-disable");
+	if (pval)
+		mask |= MAX77620_CNFGGLBL1_LBDAC_EN;
+
+	pval = of_property_read_bool(np, "maxim,low-battery-shutdown-enable");
+	if (pval) {
+		mask |= MAX77620_CNFGGLBL1_MPPLD;
+		val |= MAX77620_CNFGGLBL1_MPPLD;
+	}
+
+	pval = of_property_read_bool(np, "maxim,low-battery-shutdown-disable");
+	if (pval)
+		mask |= MAX77620_CNFGGLBL1_MPPLD;
+
+	pval = of_property_read_bool(np, "maxim,low-battery-reset-enable");
+	if (pval) {
+		mask |= MAX77620_CNFGGLBL1_LBRSTEN;
+		val |= MAX77620_CNFGGLBL1_LBRSTEN;
+	}
+
+	pval = of_property_read_bool(np, "maxim,low-battery-reset-disable");
+	if (pval)
+		mask |= MAX77620_CNFGGLBL1_LBRSTEN;
+
+	ret = regmap_update_bits(chip->rmap, MAX77620_REG_CNFGGLBL1, mask, val);
+	if (ret < 0)
+		dev_err(dev, "Reg CNFGGLBL1 update failed: %d\n", ret);
+	return ret;
+}
+
 static int max77620_read_es_version(struct max77620_chip *chip)
 {
 	unsigned int val;
@@ -563,7 +614,11 @@ static int max77620_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
-	ret =  devm_mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
+	ret = max77620_init_low_battery_monitor(chip);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
 				    mfd_cells, n_mfd_cells, NULL, 0,
 				    regmap_irq_get_domain(chip->top_irq_data));
 	if (ret < 0) {
-- 
2.19.2

[PATCH v2 4/4] mfd: max77620: add documentation for low battery monitoring

[Mark Zhang]

Adding documentation for low battery monitor properties:
- maxim,low-battery-dac-enable
- maxim,low-battery-dac-disable
- maxim,low-battery-shutdown-enable
- maxim,low-battery-shutdown-disable
- maxim,low-battery-reset-enable
- maxim,low-battery-reset-disable

Signed-off-by: Mark Zhang <markz@nvidia.com>
---
 Documentation/devicetree/bindings/mfd/max77620.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
index 484b17e4fba5..5fed0a463b80 100644
--- a/Documentation/devicetree/bindings/mfd/max77620.txt
+++ b/Documentation/devicetree/bindings/mfd/max77620.txt
@@ -142,6 +142,20 @@ Optional properties:
 			Device supports 100/1000/3000/6000 Ohms.
 			Default will be set to 1000 Ohm.
 
+Low-Battery Monitor:
+==================
+This sub-node configure low battery monitor configuration registers. Device has
+support for low-battery monitor configuration through child DT node
+"low-battery-monitor".
+
+Optional properties:
+	- maxim,low-battery-dac-enable: Enable low battery DAC.
+	- maxim,low-battery-dac-disable: Disable low battery DAC.
+	- maxim,low-battery-shutdown-enable: Enable low battery shutdown.
+	- maxim,low-battery-shutdown-disable: Disable low battery shutdown.
+	- maxim,low-battery-reset-enable: Enable low battery reset.
+	- maxim,low-battery-reset-disable: Disable low battery reset.
+
 Example:
 --------
 #include <dt-bindings/mfd/max77620.h>
-- 
2.19.2

[PATCH v2 4/4] mfd: max77620: add documentation for low battery monitoring

[Mark Zhang]

Adding documentation for low battery monitor properties:
- maxim,low-battery-dac-enable
- maxim,low-battery-dac-disable
- maxim,low-battery-shutdown-enable
- maxim,low-battery-shutdown-disable
- maxim,low-battery-reset-enable
- maxim,low-battery-reset-disable

Signed-off-by: Mark Zhang <markz@nvidia.com>
---
 Documentation/devicetree/bindings/mfd/max77620.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
index 484b17e4fba5..5fed0a463b80 100644
--- a/Documentation/devicetree/bindings/mfd/max77620.txt
+++ b/Documentation/devicetree/bindings/mfd/max77620.txt
@@ -142,6 +142,20 @@ Optional properties:
 			Device supports 100/1000/3000/6000 Ohms.
 			Default will be set to 1000 Ohm.
 
+Low-Battery Monitor:
+==================
+This sub-node configure low battery monitor configuration registers. Device has
+support for low-battery monitor configuration through child DT node
+"low-battery-monitor".
+
+Optional properties:
+	- maxim,low-battery-dac-enable: Enable low battery DAC.
+	- maxim,low-battery-dac-disable: Disable low battery DAC.
+	- maxim,low-battery-shutdown-enable: Enable low battery shutdown.
+	- maxim,low-battery-shutdown-disable: Disable low battery shutdown.
+	- maxim,low-battery-reset-enable: Enable low battery reset.
+	- maxim,low-battery-reset-disable: Disable low battery reset.
+
 Example:
 --------
 #include <dt-bindings/mfd/max77620.h>
-- 
2.19.2

Re: [PATCH v2 2/4] mfd: max77620: add documentation for backup battery charging

[Rob Herring]

On Tue, Jan 29, 2019 at 04:55:29PM +0800, Mark Zhang wrote:
> Adding documentation for 3 new backup battery charging dts
> properties:
> - maxim,backup-battery-charging-current
> - maxim,backup-battery-charging-voltage
> - maxim,backup-battery-output-resister
> 
> Signed-off-by: Mark Zhang <markz@nvidia.com>
> ---
>  .../devicetree/bindings/mfd/max77620.txt      | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
> index 9c16d51cc15b..484b17e4fba5 100644
> --- a/Documentation/devicetree/bindings/mfd/max77620.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77620.txt
> @@ -122,6 +122,26 @@ For DT binding details of different sub modules like GPIO, pincontrol,
>  regulator, power, please refer respective device-tree binding document
>  under their respective sub-system directories.
>  
> +Backup Battery:
> +==============
> +This sub-node configure charging backup battery of the device. Device has
> +support of charging the backup battery. The subnode name is "backup-battery".
> +The property for backup-battery child nodes as:
> +Presence of this child node will enable the backup battery charging.
> +
> +Optional properties:
> +	-maxim,backup-battery-charging-current: Charging current setting.
> +			The device supports 50/100/200/400/600/800uA.
> +			If this property is unavailable then it will
> +			charge with 50uA.
> +	-maxim,backup-battery-charging-voltage: Charging Voltage Limit Setting.
> +			Device supports 2500000/3000000/3300000/350000uV.
> +			Default will be set to 2500mV. The voltage will be roundoff
> +			to nearest lower side if other than above is configured.
> +	-maxim,backup-battery-output-resister: Output resistor on Ohm.
> +			Device supports 100/1000/3000/6000 Ohms.
> +			Default will be set to 1000 Ohm.

These need property unit suffixes as defined in property-units.txt. The 
names are kind of long, so perhaps shorten them a bit.

> +
>  Example:
>  --------
>  #include <dt-bindings/mfd/max77620.h>
> -- 
> 2.19.2
> 

Re: [PATCH v2 4/4] mfd: max77620: add documentation for low battery monitoring

[Rob Herring]

On Tue, Jan 29, 2019 at 04:55:31PM +0800, Mark Zhang wrote:
> Adding documentation for low battery monitor properties:
> - maxim,low-battery-dac-enable
> - maxim,low-battery-dac-disable
> - maxim,low-battery-shutdown-enable
> - maxim,low-battery-shutdown-disable
> - maxim,low-battery-reset-enable
> - maxim,low-battery-reset-disable
> 
> Signed-off-by: Mark Zhang <markz@nvidia.com>
> ---
>  Documentation/devicetree/bindings/mfd/max77620.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
> index 484b17e4fba5..5fed0a463b80 100644
> --- a/Documentation/devicetree/bindings/mfd/max77620.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77620.txt
> @@ -142,6 +142,20 @@ Optional properties:
>  			Device supports 100/1000/3000/6000 Ohms.
>  			Default will be set to 1000 Ohm.
>  
> +Low-Battery Monitor:
> +==================
> +This sub-node configure low battery monitor configuration registers. Device has
> +support for low-battery monitor configuration through child DT node
> +"low-battery-monitor".
> +
> +Optional properties:
> +	- maxim,low-battery-dac-enable: Enable low battery DAC.
> +	- maxim,low-battery-dac-disable: Disable low battery DAC.
> +	- maxim,low-battery-shutdown-enable: Enable low battery shutdown.
> +	- maxim,low-battery-shutdown-disable: Disable low battery shutdown.
> +	- maxim,low-battery-reset-enable: Enable low battery reset.
> +	- maxim,low-battery-reset-disable: Disable low battery reset.

Do you really need 3 states with the 3rd being prop not present.

Rob

Re: [PATCH v2 2/4] mfd: max77620: add documentation for backup battery charging

[Mark Zhang]

On 1/31/2019 3:51 AM, Rob Herring wrote:
> On Tue, Jan 29, 2019 at 04:55:29PM +0800, Mark Zhang wrote:
>> Adding documentation for 3 new backup battery charging dts
>> properties:
>> - maxim,backup-battery-charging-current
>> - maxim,backup-battery-charging-voltage
>> - maxim,backup-battery-output-resister
>>
>> Signed-off-by: Mark Zhang <markz@nvidia.com>
>> ---
>>  .../devicetree/bindings/mfd/max77620.txt      | 20 +++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
>> index 9c16d51cc15b..484b17e4fba5 100644
>> --- a/Documentation/devicetree/bindings/mfd/max77620.txt
>> +++ b/Documentation/devicetree/bindings/mfd/max77620.txt
>> @@ -122,6 +122,26 @@ For DT binding details of different sub modules like GPIO, pincontrol,
>>  regulator, power, please refer respective device-tree binding document
>>  under their respective sub-system directories.
>>  
>> +Backup Battery:
>> +==============
>> +This sub-node configure charging backup battery of the device. Device has
>> +support of charging the backup battery. The subnode name is "backup-battery".
>> +The property for backup-battery child nodes as:
>> +Presence of this child node will enable the backup battery charging.
>> +
>> +Optional properties:
>> +	-maxim,backup-battery-charging-current: Charging current setting.
>> +			The device supports 50/100/200/400/600/800uA.
>> +			If this property is unavailable then it will
>> +			charge with 50uA.
>> +	-maxim,backup-battery-charging-voltage: Charging Voltage Limit Setting.
>> +			Device supports 2500000/3000000/3300000/350000uV.
>> +			Default will be set to 2500mV. The voltage will be roundoff
>> +			to nearest lower side if other than above is configured.
>> +	-maxim,backup-battery-output-resister: Output resistor on Ohm.
>> +			Device supports 100/1000/3000/6000 Ohms.
>> +			Default will be set to 1000 Ohm.
> 
> These need property unit suffixes as defined in property-units.txt. The 
> names are kind of long, so perhaps shorten them a bit.
> 

Thanks Rob. Given these properties will be under "backup-battery" section, so how about
changing them to:
- maxim,charging-current-microamp
- maxim,charging-voltage-microvolt
- maxim,output-resister-ohms

Mark

>> +
>>  Example:
>>  --------
>>  #include <dt-bindings/mfd/max77620.h>
>> -- 
>> 2.19.2
>>

Re: [PATCH v2 2/4] mfd: max77620: add documentation for backup battery charging

[Mark Zhang]

On 1/31/2019 3:51 AM, Rob Herring wrote:
> On Tue, Jan 29, 2019 at 04:55:29PM +0800, Mark Zhang wrote:
>> Adding documentation for 3 new backup battery charging dts
>> properties:
>> - maxim,backup-battery-charging-current
>> - maxim,backup-battery-charging-voltage
>> - maxim,backup-battery-output-resister
>>
>> Signed-off-by: Mark Zhang <markz@nvidia.com>
>> ---
>>  .../devicetree/bindings/mfd/max77620.txt      | 20 +++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
>> index 9c16d51cc15b..484b17e4fba5 100644
>> --- a/Documentation/devicetree/bindings/mfd/max77620.txt
>> +++ b/Documentation/devicetree/bindings/mfd/max77620.txt
>> @@ -122,6 +122,26 @@ For DT binding details of different sub modules like GPIO, pincontrol,
>>  regulator, power, please refer respective device-tree binding document
>>  under their respective sub-system directories.
>>  
>> +Backup Battery:
>> +==============
>> +This sub-node configure charging backup battery of the device. Device has
>> +support of charging the backup battery. The subnode name is "backup-battery".
>> +The property for backup-battery child nodes as:
>> +Presence of this child node will enable the backup battery charging.
>> +
>> +Optional properties:
>> +	-maxim,backup-battery-charging-current: Charging current setting.
>> +			The device supports 50/100/200/400/600/800uA.
>> +			If this property is unavailable then it will
>> +			charge with 50uA.
>> +	-maxim,backup-battery-charging-voltage: Charging Voltage Limit Setting.
>> +			Device supports 2500000/3000000/3300000/350000uV.
>> +			Default will be set to 2500mV. The voltage will be roundoff
>> +			to nearest lower side if other than above is configured.
>> +	-maxim,backup-battery-output-resister: Output resistor on Ohm.
>> +			Device supports 100/1000/3000/6000 Ohms.
>> +			Default will be set to 1000 Ohm.
> 
> These need property unit suffixes as defined in property-units.txt. The 
> names are kind of long, so perhaps shorten them a bit.
> 

Thanks Rob. Given these properties will be under "backup-battery" section, so how about
changing them to:
- maxim,charging-current-microamp
- maxim,charging-voltage-microvolt
- maxim,output-resister-ohms

Mark

>> +
>>  Example:
>>  --------
>>  #include <dt-bindings/mfd/max77620.h>
>> -- 
>> 2.19.2
>>

Re: [PATCH v2 4/4] mfd: max77620: add documentation for low battery monitoring

[Mark Zhang]

On 1/31/2019 3:53 AM, Rob Herring wrote:
> On Tue, Jan 29, 2019 at 04:55:31PM +0800, Mark Zhang wrote:
>> Adding documentation for low battery monitor properties:
>> - maxim,low-battery-dac-enable
>> - maxim,low-battery-dac-disable
>> - maxim,low-battery-shutdown-enable
>> - maxim,low-battery-shutdown-disable
>> - maxim,low-battery-reset-enable
>> - maxim,low-battery-reset-disable
>>
>> Signed-off-by: Mark Zhang <markz@nvidia.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/max77620.txt | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
>> index 484b17e4fba5..5fed0a463b80 100644
>> --- a/Documentation/devicetree/bindings/mfd/max77620.txt
>> +++ b/Documentation/devicetree/bindings/mfd/max77620.txt
>> @@ -142,6 +142,20 @@ Optional properties:
>>  			Device supports 100/1000/3000/6000 Ohms.
>>  			Default will be set to 1000 Ohm.
>>  
>> +Low-Battery Monitor:
>> +==================
>> +This sub-node configure low battery monitor configuration registers. Device has
>> +support for low-battery monitor configuration through child DT node
>> +"low-battery-monitor".
>> +
>> +Optional properties:
>> +	- maxim,low-battery-dac-enable: Enable low battery DAC.
>> +	- maxim,low-battery-dac-disable: Disable low battery DAC.
>> +	- maxim,low-battery-shutdown-enable: Enable low battery shutdown.
>> +	- maxim,low-battery-shutdown-disable: Disable low battery shutdown.
>> +	- maxim,low-battery-reset-enable: Enable low battery reset.
>> +	- maxim,low-battery-reset-disable: Disable low battery reset.
> 
> Do you really need 3 states with the 3rd being prop not present.

Yeah, so I think we can just keep 3 of them and shorten the names a little bit
(lbm stands for "low battery monitoring"):
- maxim,lbm-dac-enable
- maxim,lbm-shutdown-enable
- maxim,lbm-reset-enable

Does this look good to you?
Mark

> 
> Rob
> 

Re: [PATCH v2 4/4] mfd: max77620: add documentation for low battery monitoring

[Mark Zhang]

On 1/31/2019 3:53 AM, Rob Herring wrote:
> On Tue, Jan 29, 2019 at 04:55:31PM +0800, Mark Zhang wrote:
>> Adding documentation for low battery monitor properties:
>> - maxim,low-battery-dac-enable
>> - maxim,low-battery-dac-disable
>> - maxim,low-battery-shutdown-enable
>> - maxim,low-battery-shutdown-disable
>> - maxim,low-battery-reset-enable
>> - maxim,low-battery-reset-disable
>>
>> Signed-off-by: Mark Zhang <markz@nvidia.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/max77620.txt | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
>> index 484b17e4fba5..5fed0a463b80 100644
>> --- a/Documentation/devicetree/bindings/mfd/max77620.txt
>> +++ b/Documentation/devicetree/bindings/mfd/max77620.txt
>> @@ -142,6 +142,20 @@ Optional properties:
>>  			Device supports 100/1000/3000/6000 Ohms.
>>  			Default will be set to 1000 Ohm.
>>  
>> +Low-Battery Monitor:
>> +==================
>> +This sub-node configure low battery monitor configuration registers. Device has
>> +support for low-battery monitor configuration through child DT node
>> +"low-battery-monitor".
>> +
>> +Optional properties:
>> +	- maxim,low-battery-dac-enable: Enable low battery DAC.
>> +	- maxim,low-battery-dac-disable: Disable low battery DAC.
>> +	- maxim,low-battery-shutdown-enable: Enable low battery shutdown.
>> +	- maxim,low-battery-shutdown-disable: Disable low battery shutdown.
>> +	- maxim,low-battery-reset-enable: Enable low battery reset.
>> +	- maxim,low-battery-reset-disable: Disable low battery reset.
> 
> Do you really need 3 states with the 3rd being prop not present.

Yeah, so I think we can just keep 3 of them and shorten the names a little bit
(lbm stands for "low battery monitoring"):
- maxim,lbm-dac-enable
- maxim,lbm-shutdown-enable
- maxim,lbm-reset-enable

Does this look good to you?
Mark

> 
> Rob
> 

Re: [PATCH v2 4/4] mfd: max77620: add documentation for low battery monitoring

[Rob Herring]

On Wed, Jan 30, 2019 at 8:29 PM Mark Zhang <markz@nvidia.com> wrote:
>
> On 1/31/2019 3:53 AM, Rob Herring wrote:
> > On Tue, Jan 29, 2019 at 04:55:31PM +0800, Mark Zhang wrote:
> >> Adding documentation for low battery monitor properties:
> >> - maxim,low-battery-dac-enable
> >> - maxim,low-battery-dac-disable
> >> - maxim,low-battery-shutdown-enable
> >> - maxim,low-battery-shutdown-disable
> >> - maxim,low-battery-reset-enable
> >> - maxim,low-battery-reset-disable
> >>
> >> Signed-off-by: Mark Zhang <markz@nvidia.com>
> >> ---
> >>  Documentation/devicetree/bindings/mfd/max77620.txt | 14 ++++++++++++++
> >>  1 file changed, 14 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
> >> index 484b17e4fba5..5fed0a463b80 100644
> >> --- a/Documentation/devicetree/bindings/mfd/max77620.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/max77620.txt
> >> @@ -142,6 +142,20 @@ Optional properties:
> >>                      Device supports 100/1000/3000/6000 Ohms.
> >>                      Default will be set to 1000 Ohm.
> >>
> >> +Low-Battery Monitor:
> >> +==================
> >> +This sub-node configure low battery monitor configuration registers. Device has
> >> +support for low-battery monitor configuration through child DT node
> >> +"low-battery-monitor".

Missed this the first time, but there's not really any reason for
these to be in a child node.

> >> +
> >> +Optional properties:
> >> +    - maxim,low-battery-dac-enable: Enable low battery DAC.
> >> +    - maxim,low-battery-dac-disable: Disable low battery DAC.
> >> +    - maxim,low-battery-shutdown-enable: Enable low battery shutdown.
> >> +    - maxim,low-battery-shutdown-disable: Disable low battery shutdown.
> >> +    - maxim,low-battery-reset-enable: Enable low battery reset.
> >> +    - maxim,low-battery-reset-disable: Disable low battery reset.
> >
> > Do you really need 3 states with the 3rd being prop not present.
>
> Yeah, so I think we can just keep 3 of them and shorten the names a little bit
> (lbm stands for "low battery monitoring"):
> - maxim,lbm-dac-enable
> - maxim,lbm-shutdown-enable
> - maxim,lbm-reset-enable
>
> Does this look good to you?

Yes. However, are these 3 mutually exclusive? If so, then perhaps
'maxim,low-battery-mode = "<dac|shutdown|reset>"'?

Rob

Re: [PATCH v2 4/4] mfd: max77620: add documentation for low battery monitoring

[Billy laws]

On 31 January 2019 21:05:46 GMT, Rob Herring <robh@kernel.org> wrote:
>On Wed, Jan 30, 2019 at 8:29 PM Mark Zhang <markz@nvidia.com> wrote:
>>
>> On 1/31/2019 3:53 AM, Rob Herring wrote:
>> > On Tue, Jan 29, 2019 at 04:55:31PM +0800, Mark Zhang wrote:
>> >> Adding documentation for low battery monitor properties:
>> >> - maxim,low-battery-dac-enable
>> >> - maxim,low-battery-dac-disable
>> >> - maxim,low-battery-shutdown-enable
>> >> - maxim,low-battery-shutdown-disable
>> >> - maxim,low-battery-reset-enable
>> >> - maxim,low-battery-reset-disable
>> >>
>> >> Signed-off-by: Mark Zhang <markz@nvidia.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/mfd/max77620.txt | 14
>++++++++++++++
>> >>  1 file changed, 14 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt
>b/Documentation/devicetree/bindings/mfd/max77620.txt
>> >> index 484b17e4fba5..5fed0a463b80 100644
>> >> --- a/Documentation/devicetree/bindings/mfd/max77620.txt
>> >> +++ b/Documentation/devicetree/bindings/mfd/max77620.txt
>> >> @@ -142,6 +142,20 @@ Optional properties:
>> >>                      Device supports 100/1000/3000/6000 Ohms.
>> >>                      Default will be set to 1000 Ohm.
>> >>
>> >> +Low-Battery Monitor:
>> >> +==================
>> >> +This sub-node configure low battery monitor configuration
>registers. Device has
>> >> +support for low-battery monitor configuration through child DT
>node
>> >> +"low-battery-monitor".
>
>Missed this the first time, but there's not really any reason for
>these to be in a child node.
>
>> >> +
>> >> +Optional properties:
>> >> +    - maxim,low-battery-dac-enable: Enable low battery DAC.
>> >> +    - maxim,low-battery-dac-disable: Disable low battery DAC.
>> >> +    - maxim,low-battery-shutdown-enable: Enable low battery
>shutdown.
>> >> +    - maxim,low-battery-shutdown-disable: Disable low battery
>shutdown.
>> >> +    - maxim,low-battery-reset-enable: Enable low battery reset.
>> >> +    - maxim,low-battery-reset-disable: Disable low battery reset.
>> >
>> > Do you really need 3 states with the 3rd being prop not present.
>>
>> Yeah, so I think we can just keep 3 of them and shorten the names a
>little bit
>> (lbm stands for "low battery monitoring"):
>> - maxim,lbm-dac-enable
>> - maxim,lbm-shutdown-enable
>> - maxim,lbm-reset-enable
>>
>> Does this look good to you?
>
>Yes. However, are these 3 mutually exclusive? If so, then perhaps
>'maxim,low-battery-mode = "<dac|shutdown|reset>"'?
I agree, would also allow the addition of the voltage cut off point configuration by just adding more defines.(current forces 3.something v cutoff)
>
>Rob

Re: [PATCH v2 0/4] Add max77620 charging & low battery support

[Lee Jones]

On Tue, 29 Jan 2019, Mark Zhang wrote:

> This patch set adds support for max77620 backup battery charging and
> low battery monitoring.
> 
> Changes in v2:
> - Add devicetree binding documentation
> 
> Mark Zhang (4):
>   mfd: max77620: Add backup battery charger support
>   mfd: max77620: add documentation for backup battery charging
>   mfd: max77620: Add low battery monitor support
>   mfd: max77620: add documentation for low battery monitoring
> 
>  .../devicetree/bindings/mfd/max77620.txt      |  34 +++++
>  drivers/mfd/max77620.c                        | 137 +++++++++++++++++-

All of this needs moving out to the correct subsystem.

drivers/power/supply/max77620-battery.c looks right.

>  2 files changed, 170 insertions(+), 1 deletion(-)

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

Re: [PATCH v2 0/4] Add max77620 charging & low battery support

[Mark Zhang]

On 2/7/2019 4:48 PM, Lee Jones wrote:
> On Tue, 29 Jan 2019, Mark Zhang wrote:
> 
>> This patch set adds support for max77620 backup battery charging and
>> low battery monitoring.
>>
>> Changes in v2:
>> - Add devicetree binding documentation
>>
>> Mark Zhang (4):
>>   mfd: max77620: Add backup battery charger support
>>   mfd: max77620: add documentation for backup battery charging
>>   mfd: max77620: Add low battery monitor support
>>   mfd: max77620: add documentation for low battery monitoring
>>
>>  .../devicetree/bindings/mfd/max77620.txt      |  34 +++++
>>  drivers/mfd/max77620.c                        | 137 +++++++++++++++++-
> 
> All of this needs moving out to the correct subsystem.
> 
> drivers/power/supply/max77620-battery.c looks right.

Actually max77620 is not a power supply device. This patch set adds 2
features:
- Backup battery charger. The RTC in max77620 is supplied from a backup
battery and consumes 2.0uA when no other power sources are available. So
basically this is not a system battery charger, it's just for RTC.
- Low battery monitoring. This is for monitoring the system main battery
voltage, so max77620 can shutdown or reset the SoC accordingly. But I
think this doesn't conform the idea of "power supply" driver as well.

Mark
> 
>>  2 files changed, 170 insertions(+), 1 deletion(-)
> 

Re: [PATCH v2 0/4] Add max77620 charging & low battery support

[Mark Zhang]

On 2/7/2019 4:48 PM, Lee Jones wrote:
> On Tue, 29 Jan 2019, Mark Zhang wrote:
> 
>> This patch set adds support for max77620 backup battery charging and
>> low battery monitoring.
>>
>> Changes in v2:
>> - Add devicetree binding documentation
>>
>> Mark Zhang (4):
>>   mfd: max77620: Add backup battery charger support
>>   mfd: max77620: add documentation for backup battery charging
>>   mfd: max77620: Add low battery monitor support
>>   mfd: max77620: add documentation for low battery monitoring
>>
>>  .../devicetree/bindings/mfd/max77620.txt      |  34 +++++
>>  drivers/mfd/max77620.c                        | 137 +++++++++++++++++-
> 
> All of this needs moving out to the correct subsystem.
> 
> drivers/power/supply/max77620-battery.c looks right.

Actually max77620 is not a power supply device. This patch set adds 2
features:
- Backup battery charger. The RTC in max77620 is supplied from a backup
battery and consumes 2.0uA when no other power sources are available. So
basically this is not a system battery charger, it's just for RTC.
- Low battery monitoring. This is for monitoring the system main battery
voltage, so max77620 can shutdown or reset the SoC accordingly. But I
think this doesn't conform the idea of "power supply" driver as well.

Mark
> 
>>  2 files changed, 170 insertions(+), 1 deletion(-)
> 

Re: [PATCH v2 0/4] Add max77620 charging & low battery support

[Lee Jones]

On Tue, 12 Feb 2019, Mark Zhang wrote:

> On 2/7/2019 4:48 PM, Lee Jones wrote:
> > On Tue, 29 Jan 2019, Mark Zhang wrote:
> > 
> >> This patch set adds support for max77620 backup battery charging and
> >> low battery monitoring.
> >>
> >> Changes in v2:
> >> - Add devicetree binding documentation
> >>
> >> Mark Zhang (4):
> >>   mfd: max77620: Add backup battery charger support
> >>   mfd: max77620: add documentation for backup battery charging
> >>   mfd: max77620: Add low battery monitor support
> >>   mfd: max77620: add documentation for low battery monitoring
> >>
> >>  .../devicetree/bindings/mfd/max77620.txt      |  34 +++++
> >>  drivers/mfd/max77620.c                        | 137 +++++++++++++++++-
> > 
> > All of this needs moving out to the correct subsystem.
> > 
> > drivers/power/supply/max77620-battery.c looks right.
> 
> Actually max77620 is not a power supply device. This patch set adds 2
> features:
> - Backup battery charger. The RTC in max77620 is supplied from a backup
> battery and consumes 2.0uA when no other power sources are available. So
> basically this is not a system battery charger, it's just for RTC.
> - Low battery monitoring. This is for monitoring the system main battery
> voltage, so max77620 can shutdown or reset the SoC accordingly. But I
> think this doesn't conform the idea of "power supply" driver as well.

Most other battery handling seems to happen in
drivers/power/supply/*.battery*.  If that's not the right location,
then you need to find a place for it to go.  MFDs do not provide
useful functionality per say - that is the role of the child devices.

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

Re: [PATCH v2 0/4] Add max77620 charging & low battery support

[Mark Zhang]

On 2/12/2019 4:04 PM, Lee Jones wrote:
> On Tue, 12 Feb 2019, Mark Zhang wrote:
> 
>> On 2/7/2019 4:48 PM, Lee Jones wrote:
>>> On Tue, 29 Jan 2019, Mark Zhang wrote:
>>>
>>>> This patch set adds support for max77620 backup battery charging and
>>>> low battery monitoring.
>>>>
>>>> Changes in v2:
>>>> - Add devicetree binding documentation
>>>>
>>>> Mark Zhang (4):
>>>>   mfd: max77620: Add backup battery charger support
>>>>   mfd: max77620: add documentation for backup battery charging
>>>>   mfd: max77620: Add low battery monitor support
>>>>   mfd: max77620: add documentation for low battery monitoring
>>>>
>>>>  .../devicetree/bindings/mfd/max77620.txt      |  34 +++++
>>>>  drivers/mfd/max77620.c                        | 137 +++++++++++++++++-
>>>
>>> All of this needs moving out to the correct subsystem.
>>>
>>> drivers/power/supply/max77620-battery.c looks right.
>>
>> Actually max77620 is not a power supply device. This patch set adds 2
>> features:
>> - Backup battery charger. The RTC in max77620 is supplied from a backup
>> battery and consumes 2.0uA when no other power sources are available. So
>> basically this is not a system battery charger, it's just for RTC.
>> - Low battery monitoring. This is for monitoring the system main battery
>> voltage, so max77620 can shutdown or reset the SoC accordingly. But I
>> think this doesn't conform the idea of "power supply" driver as well.
> 
> Most other battery handling seems to happen in
> drivers/power/supply/*.battery*.  If that's not the right location,
> then you need to find a place for it to go.  MFDs do not provide
> useful functionality per say - that is the role of the child devices.
> 

Understood. Looking at "enum power_supply_property", battery handling
drivers are able to get battery capacity/current
voltage/temperature/status/... and etc, which is making sense. But my
patch set doesn't do this kind of things at all so I think the power
supply framework doesn't fit here. Let me check other similar device
drivers... hope to get inspired. Thanks Lee.

Mark

Re: [PATCH v2 0/4] Add max77620 charging & low battery support

[Mark Zhang]

On 2/12/2019 4:04 PM, Lee Jones wrote:
> On Tue, 12 Feb 2019, Mark Zhang wrote:
> 
>> On 2/7/2019 4:48 PM, Lee Jones wrote:
>>> On Tue, 29 Jan 2019, Mark Zhang wrote:
>>>
>>>> This patch set adds support for max77620 backup battery charging and
>>>> low battery monitoring.
>>>>
>>>> Changes in v2:
>>>> - Add devicetree binding documentation
>>>>
>>>> Mark Zhang (4):
>>>>   mfd: max77620: Add backup battery charger support
>>>>   mfd: max77620: add documentation for backup battery charging
>>>>   mfd: max77620: Add low battery monitor support
>>>>   mfd: max77620: add documentation for low battery monitoring
>>>>
>>>>  .../devicetree/bindings/mfd/max77620.txt      |  34 +++++
>>>>  drivers/mfd/max77620.c                        | 137 +++++++++++++++++-
>>>
>>> All of this needs moving out to the correct subsystem.
>>>
>>> drivers/power/supply/max77620-battery.c looks right.
>>
>> Actually max77620 is not a power supply device. This patch set adds 2
>> features:
>> - Backup battery charger. The RTC in max77620 is supplied from a backup
>> battery and consumes 2.0uA when no other power sources are available. So
>> basically this is not a system battery charger, it's just for RTC.
>> - Low battery monitoring. This is for monitoring the system main battery
>> voltage, so max77620 can shutdown or reset the SoC accordingly. But I
>> think this doesn't conform the idea of "power supply" driver as well.
> 
> Most other battery handling seems to happen in
> drivers/power/supply/*.battery*.  If that's not the right location,
> then you need to find a place for it to go.  MFDs do not provide
> useful functionality per say - that is the role of the child devices.
> 

Understood. Looking at "enum power_supply_property", battery handling
drivers are able to get battery capacity/current
voltage/temperature/status/... and etc, which is making sense. But my
patch set doesn't do this kind of things at all so I think the power
supply framework doesn't fit here. Let me check other similar device
drivers... hope to get inspired. Thanks Lee.

Mark