For configuration of fuel gauge alert for a low level state of charge
interrupt we add a function to config level threshold and a device tree
binding property to set it in flatned device tree node.
Now we can use "maxim,alert-low-soc-level" property with the values from
1% up to 32% to configure alert interrupt threshold.
Signed-off-by: Matheus Castello <[email protected]>
---
drivers/power/supply/max17040_battery.c | 96 +++++++++++++++++++++----
1 file changed, 82 insertions(+), 14 deletions(-)
diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
index 75459f76d02c..c48a691cbd7b 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -29,6 +29,9 @@
#define MAX17040_DELAY 1000
#define MAX17040_BATTERY_FULL 95
+#define MAX17040_ATHD_MASK 0xFFC0
+#define MAX17040_ATHD_DEFAULT_POWER_UP 4
+
struct max17040_chip {
struct i2c_client *client;
struct delayed_work work;
@@ -43,6 +46,8 @@ struct max17040_chip {
int soc;
/* State Of Charge */
int status;
+ /* Low alert threshold from 32% to 1% of the State of Charge */
+ u32 low_soc_alert;
};
static int max17040_get_property(struct power_supply *psy,
@@ -99,6 +104,21 @@ static void max17040_reset(struct i2c_client *client)
max17040_write_reg(client, MAX17040_CMD, 0x0054);
}
+static int max17040_set_low_soc_alert(struct i2c_client *client, u32 level)
+{
+ int ret;
+ u16 data;
+
+ level = 32 - level;
+ data = max17040_read_reg(client, MAX17040_RCOMP);
+ /* clear the alrt bit and set LSb 5 bits */
+ data &= MAX17040_ATHD_MASK;
+ data |= level;
+ ret = max17040_write_reg(client, MAX17040_RCOMP, data);
+
+ return ret;
+}
+
static void max17040_get_vcell(struct i2c_client *client)
{
struct max17040_chip *chip = i2c_get_clientdata(client);
@@ -115,7 +135,6 @@ static void max17040_get_soc(struct i2c_client *client)
u16 soc;
soc = max17040_read_reg(client, MAX17040_SOC);
-
chip->soc = (soc >> 8);
}
@@ -161,6 +180,24 @@ static void max17040_get_status(struct i2c_client *client)
chip->status = POWER_SUPPLY_STATUS_FULL;
}
+static int max17040_get_of_data(struct max17040_chip *chip)
+{
+ struct device *dev = &chip->client->dev;
+ struct device_node *np = dev->of_node;
+ int ret = 0;
+
+ if (of_property_read_u32(np, "maxim,alert-low-soc-level",
+ &chip->low_soc_alert)) {
+ chip->low_soc_alert = MAX17040_ATHD_DEFAULT_POWER_UP;
+ } else if (chip->low_soc_alert <= 0 ||
+ chip->low_soc_alert >= 33) {
+ /* low_soc_alert is not between 1% and 32% */
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
static void max17040_check_changes(struct i2c_client *client)
{
max17040_get_vcell(client);
@@ -192,9 +229,27 @@ static irqreturn_t max17040_thread_handler(int id, void *dev)
/* send uevent */
power_supply_changed(chip->battery);
+ /* reset alert bit */
+ max17040_set_low_soc_alert(client, chip->low_soc_alert);
+
return IRQ_HANDLED;
}
+static int max17040_enable_alert_irq(struct max17040_chip *chip)
+{
+ struct i2c_client *client = chip->client;
+ unsigned int flags;
+ int ret;
+
+ dev_info(&client->dev, "IRQ: enabled\n");
+ flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
+ ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+ max17040_thread_handler, flags,
+ chip->battery->desc->name, chip);
+
+ return ret;
+}
+
static enum power_supply_property max17040_battery_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
@@ -216,6 +271,7 @@ static int max17040_probe(struct i2c_client *client,
struct i2c_adapter *adapter = client->adapter;
struct power_supply_config psy_cfg = {};
struct max17040_chip *chip;
+ int ret;
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
return -EIO;
@@ -226,6 +282,12 @@ static int max17040_probe(struct i2c_client *client,
chip->client = client;
chip->pdata = client->dev.platform_data;
+ ret = max17040_get_of_data(chip);
+ if (ret) {
+ dev_err(&client->dev,
+ "failed: low SOC alert OF data out of bounds\n");
+ return ret;
+ }
i2c_set_clientdata(client, chip);
psy_cfg.drv_data = chip;
@@ -242,20 +304,26 @@ static int max17040_probe(struct i2c_client *client,
/* check interrupt */
if (client->irq) {
- int ret;
- unsigned int flags;
-
- dev_info(&client->dev, "IRQ: enabled\n");
- flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
- ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
- max17040_thread_handler, flags,
- chip->battery->desc->name,
- chip);
-
- if (ret) {
- client->irq = 0;
+ if (of_device_is_compatible(client->dev.of_node,
+ "maxim,max77836-battery")) {
+ ret = max17040_set_low_soc_alert(client,
+ chip->low_soc_alert);
+ if (ret) {
+ dev_err(&client->dev,
+ "Failed to set low SOC alert: err %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = max17040_enable_alert_irq(chip);
+ if (ret) {
+ client->irq = 0;
+ dev_warn(&client->dev,
+ "Failed to get IRQ err %d\n", ret);
+ }
+ } else {
dev_warn(&client->dev,
- "Failed to get IRQ err %d\n", ret);
+ "Device not compatible for IRQ");
}
}
--
2.24.0.rc2
On Tue, Nov 05, 2019 at 02:42:17AM -0300, Matheus Castello wrote:
> For configuration of fuel gauge alert for a low level state of charge
> interrupt we add a function to config level threshold and a device tree
> binding property to set it in flatned device tree node.
>
> Now we can use "maxim,alert-low-soc-level" property with the values from
> 1% up to 32% to configure alert interrupt threshold.
>
> Signed-off-by: Matheus Castello <[email protected]>
> ---
> drivers/power/supply/max17040_battery.c | 96 +++++++++++++++++++++----
> 1 file changed, 82 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
> index 75459f76d02c..c48a691cbd7b 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -29,6 +29,9 @@
> #define MAX17040_DELAY 1000
> #define MAX17040_BATTERY_FULL 95
>
> +#define MAX17040_ATHD_MASK 0xFFC0
> +#define MAX17040_ATHD_DEFAULT_POWER_UP 4
> +
> struct max17040_chip {
> struct i2c_client *client;
> struct delayed_work work;
> @@ -43,6 +46,8 @@ struct max17040_chip {
> int soc;
> /* State Of Charge */
> int status;
> + /* Low alert threshold from 32% to 1% of the State of Charge */
> + u32 low_soc_alert;
> };
>
> static int max17040_get_property(struct power_supply *psy,
> @@ -99,6 +104,21 @@ static void max17040_reset(struct i2c_client *client)
> max17040_write_reg(client, MAX17040_CMD, 0x0054);
> }
>
> +static int max17040_set_low_soc_alert(struct i2c_client *client, u32 level)
> +{
> + int ret;
> + u16 data;
> +
> + level = 32 - level;
> + data = max17040_read_reg(client, MAX17040_RCOMP);
> + /* clear the alrt bit and set LSb 5 bits */
> + data &= MAX17040_ATHD_MASK;
> + data |= level;
> + ret = max17040_write_reg(client, MAX17040_RCOMP, data);
> +
> + return ret;
> +}
> +
> static void max17040_get_vcell(struct i2c_client *client)
> {
> struct max17040_chip *chip = i2c_get_clientdata(client);
> @@ -115,7 +135,6 @@ static void max17040_get_soc(struct i2c_client *client)
> u16 soc;
>
> soc = max17040_read_reg(client, MAX17040_SOC);
> -
> chip->soc = (soc >> 8);
> }
>
> @@ -161,6 +180,24 @@ static void max17040_get_status(struct i2c_client *client)
> chip->status = POWER_SUPPLY_STATUS_FULL;
> }
>
> +static int max17040_get_of_data(struct max17040_chip *chip)
> +{
> + struct device *dev = &chip->client->dev;
> + struct device_node *np = dev->of_node;
> + int ret = 0;
> +
> + if (of_property_read_u32(np, "maxim,alert-low-soc-level",
> + &chip->low_soc_alert)) {
> + chip->low_soc_alert = MAX17040_ATHD_DEFAULT_POWER_UP;
> + } else if (chip->low_soc_alert <= 0 ||
> + chip->low_soc_alert >= 33) {
> + /* low_soc_alert is not between 1% and 32% */
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> static void max17040_check_changes(struct i2c_client *client)
> {
> max17040_get_vcell(client);
> @@ -192,9 +229,27 @@ static irqreturn_t max17040_thread_handler(int id, void *dev)
> /* send uevent */
> power_supply_changed(chip->battery);
>
> + /* reset alert bit */
> + max17040_set_low_soc_alert(client, chip->low_soc_alert);
> +
> return IRQ_HANDLED;
> }
>
> +static int max17040_enable_alert_irq(struct max17040_chip *chip)
> +{
It does not make really sense to move this code to separate function in
this patch. This should be done in patch 1/5. Otherwise you add a code
in patch 1 and later in patch 4 you immediately rearrange it. This
raises eybrows and gives a hint that patchset is not well structured.
> + struct i2c_client *client = chip->client;
> + unsigned int flags;
> + int ret;
> +
> + dev_info(&client->dev, "IRQ: enabled\n");
While at it, get rid of dev_info here. It does not bring any useful
information. All this is available in /proc/interrupts.
Best regards,
Krzysztof
> + flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
> + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> + max17040_thread_handler, flags,
> + chip->battery->desc->name, chip);
> +
> + return ret;
> +}
> +
> static enum power_supply_property max17040_battery_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_ONLINE,
> @@ -216,6 +271,7 @@ static int max17040_probe(struct i2c_client *client,
> struct i2c_adapter *adapter = client->adapter;
> struct power_supply_config psy_cfg = {};
> struct max17040_chip *chip;
> + int ret;
>
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
> return -EIO;
> @@ -226,6 +282,12 @@ static int max17040_probe(struct i2c_client *client,
>
> chip->client = client;
> chip->pdata = client->dev.platform_data;
> + ret = max17040_get_of_data(chip);
> + if (ret) {
> + dev_err(&client->dev,
> + "failed: low SOC alert OF data out of bounds\n");
> + return ret;
> + }
>
> i2c_set_clientdata(client, chip);
> psy_cfg.drv_data = chip;
> @@ -242,20 +304,26 @@ static int max17040_probe(struct i2c_client *client,
>
> /* check interrupt */
> if (client->irq) {
> - int ret;
> - unsigned int flags;
> -
> - dev_info(&client->dev, "IRQ: enabled\n");
> - flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
> - ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> - max17040_thread_handler, flags,
> - chip->battery->desc->name,
> - chip);
> -
> - if (ret) {
> - client->irq = 0;
> + if (of_device_is_compatible(client->dev.of_node,
> + "maxim,max77836-battery")) {
> + ret = max17040_set_low_soc_alert(client,
> + chip->low_soc_alert);
> + if (ret) {
> + dev_err(&client->dev,
> + "Failed to set low SOC alert: err %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = max17040_enable_alert_irq(chip);
> + if (ret) {
> + client->irq = 0;
> + dev_warn(&client->dev,
> + "Failed to get IRQ err %d\n", ret);
> + }
> + } else {
> dev_warn(&client->dev,
> - "Failed to get IRQ err %d\n", ret);
> + "Device not compatible for IRQ");
> }
> }
>
> --
> 2.24.0.rc2
>
This series add IRQ handler for low level SOC alert, define a devicetree
binding attribute to configure the alert level threshold and check for
changes in SOC and power supply status for send uevents.
Max17043/17044 have a pin for alert host about low level state of charge and
this alert can be configured in a threshold from 1% up to 32% of SOC.
Tested on Toradex Colibri iMX7D, with a SparkFun Lipo Fuel Gauge module
based on MAXIM MAX17043.
Thanks Krzysztof for your time reviewing it. Let me know what you think about
the fixes.
Changes since v5:
(Suggested by Krzysztof Kozlowski)
- Rearrange code and add max17040_enable_alert_irq on patch 1/5
- Remove useless dev_info
Changes since v4:
(Suggested by Krzysztof Kozlowski)
- Fix code style and alignment issues
- Keep IRQF_TRIGGER_FALLING | IRQF_ONESHOT instead client->flags
(Suggested by Rob Herring)
- Add reference to the MFD description
- Fix the dt-bindings commit description
Matheus Castello (5):
power: supply: max17040: Add IRQ handler for low SOC alert
dt-bindings: power: supply: Max17040: Add DT bindings for max17040
fuel gauge
devicetree: mfd: max14577: Add reference to max14040_battery.txt
descriptions
power: supply: max17040: Config alert SOC low level threshold from FDT
power: supply: max17040: Send uevent in SOC and status change
.../devicetree/bindings/mfd/max14577.txt | 2 +
.../power/supply/max17040_battery.txt | 33 ++++
drivers/power/supply/max17040_battery.c | 141 +++++++++++++++++-
3 files changed, 171 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/supply/max17040_battery.txt
--
2.24.0.rc2
Documentation of max17040 based fuel gauge characteristics.
For configure low level state of charge threshold alert signaled from
max17043/max17044 we add "maxim,alert-low-soc-level" property.
Signed-off-by: Matheus Castello <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../power/supply/max17040_battery.txt | 33 +++++++++++++++++++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/max17040_battery.txt
diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
new file mode 100644
index 000000000000..f2d0b22b5f79
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
@@ -0,0 +1,33 @@
+max17040_battery
+~~~~~~~~~~~~~~~~
+
+Required properties :
+ - compatible : "maxim,max17040" or "maxim,max77836-battery"
+ - reg: i2c slave address
+
+Optional properties :
+- maxim,alert-low-soc-level : The alert threshold that sets the state of
+ charge level (%) where an interrupt is
+ generated. Can be configured from 1 up to 32
+ (%). If skipped the power up default value of
+ 4 (%) will be used.
+- interrupts : Interrupt line see Documentation/devicetree/
+ bindings/interrupt-controller/interrupts.txt
+- wakeup-source : This device has wakeup capabilities. Use this
+ property to use alert low SOC level interrupt
+ as wake up source.
+
+Optional properties support interrupt functionality for alert low state of
+charge level, present in some ICs in the same family, and should be used with
+compatible "maxim,max77836-battery".
+
+Example:
+
+ battery-fuel-gauge@36 {
+ compatible = "maxim,max77836-battery";
+ reg = <0x36>;
+ maxim,alert-low-soc-level = <10>;
+ interrupt-parent = <&gpio7>;
+ interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+ wakeup-source;
+ };
--
2.24.0.rc2
On Thu, 07 Nov 2019, Matheus Castello wrote:
> This series add IRQ handler for low level SOC alert, define a devicetree
> binding attribute to configure the alert level threshold and check for
> changes in SOC and power supply status for send uevents.
>
> Max17043/17044 have a pin for alert host about low level state of charge and
> this alert can be configured in a threshold from 1% up to 32% of SOC.
>
> Tested on Toradex Colibri iMX7D, with a SparkFun Lipo Fuel Gauge module
> based on MAXIM MAX17043.
>
> Thanks Krzysztof for your time reviewing it. Let me know what you think about
> the fixes.
>
> Changes since v5:
> (Suggested by Krzysztof Kozlowski)
> - Rearrange code and add max17040_enable_alert_irq on patch 1/5
> - Remove useless dev_info
Just something to bear in mind in the future:
When submitting subsequent versions, could you please do so as
separate sets? Attaching them to previous submissions gets confusing
real quick.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thu, 7 Nov 2019 00:17:07 -0300, Matheus Castello wrote:
> Documentation of max17040 based fuel gauge characteristics.
> For configure low level state of charge threshold alert signaled from
> max17043/max17044 we add "maxim,alert-low-soc-level" property.
>
> Signed-off-by: Matheus Castello <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> ---
> .../power/supply/max17040_battery.txt | 33 +++++++++++++++++++
> 1 file changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/supply/max17040_battery.txt
>
Reviewed-by: Rob Herring <[email protected]>