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
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 <[email protected]>
---
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
This patch adds PMIC configurations for low-battery
monitoring by handling max77620 register CNFGGLBL1.
Signed-off-by: Laxman Dewangan <[email protected]>
Signed-off-by: Venkat Reddy Talla <[email protected]>
Signed-off-by: Mark Zhang <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Venkat Reddy Talla <[email protected]>
Signed-off-by: Mark Zhang <[email protected]>
---
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
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 <[email protected]>
---
.../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
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 <[email protected]>
> ---
> .../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
>
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> .../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
>>
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 <[email protected]>
>> ---
>> 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
>
On Wed, Jan 30, 2019 at 8:29 PM Mark Zhang <[email protected]> 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 <[email protected]>
> >> ---
> >> 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
On 31 January 2019 21:05:46 GMT, Rob Herring <[email protected]> wrote:
>On Wed, Jan 30, 2019 at 8:29 PM Mark Zhang <[email protected]> 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 <[email protected]>
>> >> ---
>> >> 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
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
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(-)
>
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
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