Re: [PATCH v4 4/5] power: supply: max77759: add charger driver
From: Amit Sunil Dhamne
Date: Mon Jan 26 2026 - 20:09:00 EST
Hi Andre',
On 1/22/26 4:07 PM, Amit Sunil Dhamne wrote:
Hi Andre',
On 1/22/26 4:47 AM, André Draszik wrote:
Hi Amit,
Thanks for your patches, just a few minor comments below.
Thanks for your feedback!
On Wed, 2026-01-21 at 00:59 +0000, Amit Sunil Dhamne via B4 Relay wrote:
From: Amit Sunil Dhamne <amitsd@xxxxxxxxxx>Does it really make an impact on performance to specify unlikely? Also, I seem to
Add support for MAX77759 battery charger driver. This is a 4A 1-Cell
Li+/LiPoly dual input switch mode charger. While the device can support
USB & wireless charger inputs, this implementation only supports USB
input. This implementation supports both buck and boost modes.
Signed-off-by: Amit Sunil Dhamne <amitsd@xxxxxxxxxx>
---
MAINTAINERS | 6 +
drivers/power/supply/Kconfig | 11 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/max77759_charger.c | 737 ++++++++++++++++++++++++++++++++
4 files changed, 755 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 0d044a58cbfe0f2b97f3682a86708e1ece108e9f..38354964a85c34611b1b54e20651b360f3b9c11e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15546,6 +15546,12 @@ F: drivers/mfd/max77759.c
F: drivers/nvmem/max77759-nvmem.c
F: include/linux/mfd/max77759.h
+MAXIM MAX77759 BATTERY CHARGER DRIVER
+M: Amit Sunil Dhamne <amitsd@xxxxxxxxxx>
+L: linux-kernel@xxxxxxxxxxxxxxx
+S: Maintained
+F: drivers/power/supply/max77759_charger.c
+
MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
M: Javier Martinez Canillas <javier@xxxxxxxxxxxx>
L: linux-kernel@xxxxxxxxxxxxxxx
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 92f9f7aae92f249aa165e68dbcd4cebb569286ea..3a2cdb95c98e44324151ac2b86d740ae2923ee77 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -631,6 +631,17 @@ config CHARGER_MAX77705
help
Say Y to enable support for the Maxim MAX77705 battery charger.
+config CHARGER_MAX77759
+ tristate "Maxim MAX77759 battery charger driver"
+ depends on MFD_MAX77759 && REGULATOR
+ default MFD_MAX77759
+ help
+ Say M or Y here to enable the MAX77759 battery charger. MAX77759
+ charger is a function of the MAX77759 PMIC. This is a dual input
+ switch-mode charger. This driver supports buck and OTG boost modes.
+
+ If built as a module, it will be called max77759_charger.
+
config CHARGER_MAX77976
tristate "Maxim MAX77976 battery charger driver"
depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 4b79d5abc49a7fd1e37a26d0c89f94d9fe3a916f..6af905875ad5e3b393a7030405355b9a975870f6 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -128,3 +128,4 @@ obj-$(CONFIG_CHARGER_SURFACE) += surface_charger.o
obj-$(CONFIG_BATTERY_UG3105) += ug3105_battery.o
obj-$(CONFIG_CHARGER_QCOM_SMB2) += qcom_smbx.o
obj-$(CONFIG_FUEL_GAUGE_MM8013) += mm8013.o
+obj-$(CONFIG_CHARGER_MAX77759) += max77759_charger.o
diff --git a/drivers/power/supply/max77759_charger.c b/drivers/power/supply/max77759_charger.c
new file mode 100644
index 0000000000000000000000000000000000000000..34b5ea0967eb7b4716e81ee1a55227ac872493b0
--- /dev/null
+++ b/drivers/power/supply/max77759_charger.c
@@ -0,0 +1,737 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * max77759_charger.c - Battery charger driver for MAX77759 charger device.
+ *
+ * Copyright 2025 Google LLC.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/devm-helpers.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/math64.h>
+#include <linux/mfd/max77759.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/string_choices.h>
+
+/* Default values for Fast Charge Current & Float Voltage */
+#define CHG_CC_DEFAULT_UA 2266770
+#define CHG_FV_DEFAULT_MV 4300
+
+#define FOREACH_IRQ(S) \
+ S(AICL), \
+ S(CHGIN), \
+ S(CHG), \
+ S(INLIM), \
+ S(BAT_OILO), \
+ S(CHG_STA_CC), \
+ S(CHG_STA_CV), \
+ S(CHG_STA_TO), \
+ S(CHG_STA_DONE)
+
+#define GENERATE_ENUM(e) e
+#define GENERATE_STRING(s) #s
+
+enum {
+ FOREACH_IRQ(GENERATE_ENUM)
+};
+
+static const char *const chgr_irqs_str[] = {
+ FOREACH_IRQ(GENERATE_STRING)
+};
+
+#define NUM_IRQS ARRAY_SIZE(chgr_irqs_str)
+
+struct max77759_charger {
+ struct device *dev;
+ struct regmap *regmap;
+ struct power_supply *psy;
+ struct regulator_dev *chgin_otg_rdev;
+ struct notifier_block nb;
+ struct power_supply *tcpm_psy;
+ struct work_struct psy_work;
+ int irqs[NUM_IRQS];
+ struct mutex lock; /* protects the state below */
+ enum max77759_chgr_mode mode;
+};
+
+static inline int regval_to_val(int reg, int reg_offset, int step, int minval)
+{
+ return ((reg - reg_offset) * step) + minval;
+}
+
+static inline int val_to_regval(int val, int minval, int step, int reg_offset)
+{
+ s64 dividend;
+
+ if (unlikely(step == 0))
+ return reg_offset;
remember that the if branch is treated as unlikely anyway, but can't find any hard
evidence on that right now.
I was hoping it's performance improving even if negligible, though I don't have evidence to support either arguments. In any case, I kept it for readability.
+For these two functions above, have you considered using the APIs from
+ dividend = (s64)val - minval;
+ return DIV_S64_ROUND_CLOSEST(dividend, step) + reg_offset;
+}
include/linux/linear_range.h instead of duplicating in this driver? The
implementations of the above match linear_range_get_value() and
linear_range_get_selector_low() quite nicely.
I think it looks useful, will check it out.
+[...]
+static inline int unlock_prot_regs(struct max77759_charger *chg, bool unlock)
+{
+ return regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_06,
+ MAX77759_CHGR_REG_CHG_CNFG_06_CHGPROT, unlock
+ ? MAX77759_CHGR_REG_CHG_CNFG_06_CHGPROT : 0);
+}
+
+static irqreturn_t irq_handler(int irq, void *data)Generally, no 'else' is required after return.
+{
+ struct max77759_charger *chg = data;
+ struct device *dev = chg->dev;
+ int i;
+
+ for (i = 0; i < NUM_IRQS && chg->irqs[i] != irq; i++)
+ ;
+
+ if (i == NUM_IRQS) {
+ dev_err(dev, "Unable to handle irq=%d", irq);
+ return IRQ_NONE;
+ } else if (i == BAT_OILO) {
+ dev_warn(dev, "Battery over-current threshold crossed");
+ }
I will refactor it in the next rev.
+[...]
+ power_supply_changed(chg->psy);
+ return IRQ_HANDLED;
+}
+
+static void psy_work_item(struct work_struct *work)For all the possible errors in this function, should the driver try a bit
+{
+ struct max77759_charger *chg =
+ container_of(work, struct max77759_charger, psy_work);
+ union power_supply_propval current_limit, online;
+ int ret;
+
+ ret = power_supply_get_property(chg->tcpm_psy,
+ POWER_SUPPLY_PROP_CURRENT_MAX,
+ ¤t_limit);
+ if (ret) {
+ dev_err(chg->dev,
+ "Failed to get CURRENT_MAX psy property, ret=%d",
+ ret);
+ return;
+ }
+
+ ret = power_supply_get_property(chg->tcpm_psy, POWER_SUPPLY_PROP_ONLINE,
+ &online);
+ if (ret) {
+ dev_err(chg->dev,
+ "Failed to get ONLINE psy property, ret=%d",
+ ret);
+ return;
+ }
+
+ if (online.intval && current_limit.intval) {
+ ret = set_input_current_limit(chg, current_limit.intval);
+ if (ret)
+ dev_err(chg->dev,
+ "Unable to set current limit, ret=%d", ret);
+
+ charger_set_mode(chg, MAX77759_CHGR_MODE_CHG_BUCK_ON);
+ } else {
+ charger_set_mode(chg, MAX77759_CHGR_MODE_OFF);
+ }
harder, even if unlikely to occur?
Maybe we can do this:
On failure of either of the power_supply_get_property() calls or set_input_current_limit(), we should first turn off the charger (as that's the safest choice) and reschedule this work for a certain number of times (maybe 3). Obviously, if we recover from this state, we reset the error limit and don't reschedule this work anymore. Do you concur?
What if the current limit needed to be
reduced, e.g. due to thermal or any other reasons?
This specific piece of code is for setting current limit that is driven by USB Type-C subsystem. If the Type-C subsystem re-negotiates a PD contract this piece of code will re-run as this subsystem will be re-notified by TCPM. For cases where we need to cap the current limit due to thermal management, that's a hardware driven feature. It is enabled by enabling JEITA management. That's not in the scope atm.
Responded in the previous comment..
Could rescheduling the work be something to consider?
Following up on this comment, in case you missed it :)
Thanks,
Amit