| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7c326abd0d48c77c4b7df64da87870644d093757.camel@linaro.org>
Date: Wed, 26 Nov 2025 07:36:54 +0000
From: André Draszik <andre.draszik@...aro.org>
To: amitsd@...gle.com, Sebastian Reichel <sre@...nel.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Lee Jones <lee@...nel.org>, Greg Kroah-Hartman
<gregkh@...uxfoundation.org>, Badhri Jagan Sridharan <badhri@...gle.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Peter Griffin
<peter.griffin@...aro.org>, Tudor Ambarus <tudor.ambarus@...aro.org>, Alim
Akhtar <alim.akhtar@...sung.com>
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
devicetree@...r.kernel.org, linux-usb@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, RD
Babiera <rdbabiera@...gle.com>, Kyle Tso <kyletso@...gle.com>
Subject: Re: [PATCH 5/6] power: supply: max77759: add charger driver
Hi Amit,
Just a quick comment below for something I noticed during a brief look.
On Sun, 2025-11-23 at 08:35 +0000, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@...gle.com>
>
> 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@...gle.com>
> ---
> MAINTAINERS | 7 +
> drivers/mfd/max77759.c | 3 +-
> drivers/power/supply/Kconfig | 11 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/max77759_charger.c | 866 ++++++++++++++++++++++++++++++++
> 5 files changed, 887 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fed6cd812d79..f1b1015c08b5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15053,6 +15053,13 @@ 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@...gle.com>
> +L: linux-kernel@...r.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
> +F: drivers/power/supply/max77759_charger.c
> +
> MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
> M: Javier Martinez Canillas <javier@...hile0.org>
> L: linux-kernel@...r.kernel.org
> diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
> index 5fe22884f362..8a22838be1b0 100644
> --- a/drivers/mfd/max77759.c
> +++ b/drivers/mfd/max77759.c
> @@ -349,7 +349,8 @@ static const struct mfd_cell max77759_maxq_cells[] = {
> };
>
> static const struct mfd_cell max77759_charger_cells[] = {
> - MFD_CELL_RES("max77759-charger", max77759_charger_resources),
> + MFD_CELL_OF("max77759-charger", max77759_charger_resources, NULL, 0, 0,
> + "maxim,max77759-charger"),
> };
>
> int max77759_maxq_command(struct max77759 *max77759,
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 79ddb006e2da..b97990cc0b92 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -1074,4 +1074,15 @@ config FUEL_GAUGE_MM8013
> the state of charge, temperature, cycle count, actual and design
> capacity, etc.
>
> +config CHARGER_MAX77759
> + tristate "MAX77759 Charger Driver"
> + depends on MFD_MAX77759
> + default MFD_MAX77759
> + help
> + Say M or Y here to enable the MAX77759 Charger Driver. 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.
> +
> endif # POWER_SUPPLY
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index f943c9150b32..12669734cfe3 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -122,3 +122,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 000000000000..51637e87182b
> --- /dev/null
> +++ b/drivers/power/supply/max77759_charger.c
> @@ -0,0 +1,866 @@
> +// 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>
> +
> +/* CHG_INT_OK */
> +#define CHG_INT_OK_AICL_OK BIT(7)
> +#define CHG_INT_OK_CHGIN_OK BIT(6)
> +#define CHG_INT_OK_CHG_OK BIT(4)
> +#define CHG_INT_OK_INLIM_OK BIT(2)
> +
[...]
> +static irqreturn_t irq_handler(int irq, void *data)
> +{
> + struct max77759_charger *chg = data;
> + u32 irq_status, chgint_ok, idx = 0;
> + int ret;
> +
> + if (irq == chg->irq[0])
> + idx = 0;
> + else
> + idx = 1;
> +
> + ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT + idx,
> + &irq_status);
> + if (ret) {
> + dev_err(chg->dev, "regmap_read_error idx=%d ret=%d", idx, ret);
> + return IRQ_HANDLED;
> + }
> +
> + regmap_write(chg->regmap, MAX77759_CHGR_REG_CHG_INT + idx,
> + irq_status);
> + regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT_OK, &chgint_ok);
> +
> + if (idx == 0) {
> + if (irq_status & MAX77759_CHGR_REG_CHG_INT_AICL)
> + dev_dbg(chg->dev, "AICL mode: %s",
> + str_no_yes(chgint_ok & CHG_INT_OK_AICL_OK));
> +
> + if (irq_status & MAX77759_CHGR_REG_CHG_INT_CHGIN)
> + dev_dbg(chg->dev, "CHGIN input valid: %s",
> + str_yes_no(chgint_ok & CHG_INT_OK_CHGIN_OK));
> +
> + if (irq_status & MAX77759_CHGR_REG_CHG_INT_CHG)
> + dev_dbg(chg->dev, "CHG status okay/off: %s",
> + str_yes_no(chgint_ok & CHG_INT_OK_CHG_OK));
> +
> + if (irq_status & MAX77759_CHGR_REG_CHG_INT_INLIM)
> + dev_dbg(chg->dev, "Current Limit reached: %s",
> + str_no_yes(chgint_ok & CHG_INT_OK_INLIM_OK));
> + } else {
> + if (irq_status & MAX77759_CHGR_REG_CHG_INT2_BAT_OILO)
> + dev_dbg(chg->dev,
> + "Battery over-current threshold crossed");
> +
> + if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC)
> + dev_dbg(chg->dev, "Charger reached CC stage");
> +
> + if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV)
> + dev_dbg(chg->dev, "Charger reached CV stage");
> +
> + if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO)
> + dev_dbg(chg->dev, "Charger reached TO stage");
> +
> + if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE)
> + dev_dbg(chg->dev, "Charger reached Done stage");
> + }
> +
> + power_supply_changed(chg->psy);
> + return IRQ_HANDLED;
> +}
> +
> +static int max77759_init_irqhandler(struct max77759_charger *chg)
> +{
> + static const char * const irq_res_names[] = { "INT1", "INT2" };
> + struct device *dev = chg->dev;
> + unsigned long irq_flags;
> + struct irq_data *irqd;
> + int *irq = chg->irq;
> + int ret, i;
> +
> + for (i = 0; i < 2; i++) {
> + irq[i] = platform_get_irq_byname(to_platform_device(dev),
> + irq_res_names[i]);
> + if (irq[i] < 0) {
> + dev_err(dev, "unable to find %s irq", irq_res_names[i]);
> + return irq[i];
> + }
> +
> + irq_flags = IRQF_ONESHOT | IRQF_SHARED;
> + irqd = irq_get_irq_data(irq[i]);
> + if (irqd)
> + irq_flags |= irqd_get_trigger_type(irqd);
> +
> + ret = devm_request_threaded_irq(dev, irq[i], NULL, irq_handler,
> + irq_flags, dev_name(dev), chg);
> + if (ret) {
> + dev_err(dev, "Unable to register threaded irq handler");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
The way you're requesting the IRQ here I don't think you need to modify
max77759_chgr_irqs[] in patch 4 at all. You're requesting the interrupt
and are manually demultiplexing and acknowledging the actual event in your
IRQ handler.
Your change in patch 4 would instead allow your charger driver to call
request_irq for each individual bit of the interrupt register(s), letting
regmap-irq demultiplex and acknowledge the event for you before calling
your handler(s). As an example, see how the ACPM result interrupt is
requested in max77759_add_chained_maxq().
It's probably cleaner to let regmap-irq deal with demultiplexing etc.
Cheers,
Andre
Powered by blists - more mailing lists