| 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: <cctdvj7jsf5ng3ab5vyhzjn73u6wqye3kcrgfj4tugpd32zj4f@o5buyuu7mmns>
Date: Tue, 23 Sep 2025 00:28:33 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Dzmitry Sankouski <dsankouski@...il.com>
Cc: Chanwoo Choi <cw00.choi@...sung.com>,
Krzysztof Kozlowski <krzk@...nel.org>, Lee Jones <lee@...nel.org>,
Luca Ceresoli <luca.ceresoli@...tlin.com>, Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v5] power: supply: max77705_charger: implement aicl
feature
Hi,
On Sat, Sep 20, 2025 at 09:42:30PM +0300, Dzmitry Sankouski wrote:
> Adaptive input current allows charger to reduce it's current
> consumption, when source is not able to provide enough power.
>
> Signed-off-by: Dzmitry Sankouski <dsankouski@...il.com>
> ---
> This series consists of:
> - max77705: interrupt handling fix
> - max77705: make input current limit and charge current limit properties
> writable
> - max77705: add adaptive input current limit feature
> - max77705: switch to regfields
> - max77705: refactoring
> - max77976: change property for current charge limit value
> ---
> Changes in v5:
> - rebase on latest linux-next, dropping already applied patches
> - optimize code to drop is_aicl_irq_disabled variable
> - Link to v4: https://lore.kernel.org/r/20250918-max77705_77976_charger_improvement-v4-0-11ec9188f489@gmail.com
>
> Changes in v4:
> - fix commit message
> - use IRQF_TRIGGER_NONE, because non physical irqs
> - minor rename refactoring
> - rebase on latest linux-next
> - patch reorder: put fixes patch first
> - aicl feature cleanup
> - Link to v3: https://lore.kernel.org/r/20250911-max77705_77976_charger_improvement-v3-0-35203686fa29@gmail.com
>
> Changes in v3:
> - move interrupt request before interrupt handler work initialization
> - Link to v2: https://lore.kernel.org/r/20250909-max77705_77976_charger_improvement-v2-0-a8d2fba47159@gmail.com
>
> Changes in v2:
> - fix charger register protection unlock
> - Link to v1: https://lore.kernel.org/r/20250830-max77705_77976_charger_improvement-v1-0-e976db3fd432@gmail.com
> ---
> Changes in v5:
> - add _MS suffix to AICL_WORK_DELAY
> - optimize code to drop is_aicl_irq_disabled variable
>
> Changes in v4:
> - fix intendation
> - use IRQF_TRIGGER_NONE, because this is not physical irq
> - use dev_err_probe instead of pr_err
> - remove excessive chgin irq request
> - remove pr_infos
> ---
> drivers/power/supply/max77705_charger.c | 55 +++++++++++++++++++++++++++++++++
> include/linux/power/max77705_charger.h | 4 +++
> 2 files changed, 59 insertions(+)
>
> diff --git a/drivers/power/supply/max77705_charger.c b/drivers/power/supply/max77705_charger.c
> index b1a227bf72e2..ff1663b414f5 100644
> --- a/drivers/power/supply/max77705_charger.c
> +++ b/drivers/power/supply/max77705_charger.c
> @@ -40,6 +40,18 @@ static enum power_supply_property max77705_charger_props[] = {
> POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> };
>
> +static irqreturn_t max77705_aicl_irq(int irq, void *irq_drv_data)
> +{
> + struct max77705_charger_data *chg = irq_drv_data;
> +
> + disable_irq(chg->aicl_irq);
> +
> + queue_delayed_work(chg->wqueue, &chg->aicl_work,
> + msecs_to_jiffies(AICL_WORK_DELAY_MS));
> +
> + return IRQ_HANDLED;
> +}
> +
> static irqreturn_t max77705_chgin_irq(int irq, void *irq_drv_data)
> {
> struct max77705_charger_data *chg = irq_drv_data;
> @@ -445,6 +457,33 @@ static const struct power_supply_desc max77705_charger_psy_desc = {
> .set_property = max77705_set_property,
> };
>
> +static void max77705_aicl_isr_work(struct work_struct *work)
> +{
> + unsigned int regval, irq_status;
> + int err;
> + struct max77705_charger_data *chg =
> + container_of(work, struct max77705_charger_data, aicl_work.work);
> +
> + regmap_read(chg->regmap, MAX77705_CHG_REG_INT_OK, &irq_status);
> +
> + if (!(irq_status & BIT(MAX77705_AICL_I))) {
> + err = regmap_field_read(chg->rfield[MAX77705_CHG_CHGIN_LIM], ®val);
> + if (err < 0)
> + return;
> +
> + regval--;
> +
> + err = regmap_field_write(chg->rfield[MAX77705_CHG_CHGIN_LIM], regval);
> + if (err < 0)
> + return;
> +
> + queue_delayed_work(chg->wqueue, &chg->aicl_work,
> + msecs_to_jiffies(AICL_WORK_DELAY_MS));
> + } else {
> + enable_irq(chg->aicl_irq);
> + }
> +}
After looking at this again in this simpler version: Why do you
need the delayed work at all? It seems you can simplify to this:
static irqreturn_t max77705_aicl_irq(int irq, void *irq_drv_data)
{
struct max77705_charger_data *chg = irq_drv_data;
unsigned int regval, irq_status;
int err;
do {
regmap_read(chg->regmap, MAX77705_CHG_REG_INT_OK, &irq_status);
if (!(irq_status & BIT(MAX77705_AICL_I))) {
err = regmap_field_read(chg->rfield[MAX77705_CHG_CHGIN_LIM], ®val);
if (err < 0)
continue;
regval--;
err = regmap_field_write(chg->rfield[MAX77705_CHG_CHGIN_LIM], regval);
if (err < 0)
continue;
msleep(AICL_WORK_DELAY_MS);
}
} while(irq_status & BIT(MAX77705_AICL_I));
return IRQ_HANDLED;
}
Greetings,
-- Sebastian
> static void max77705_chgin_isr_work(struct work_struct *work)
> {
> struct max77705_charger_data *chg =
> @@ -617,6 +656,12 @@ static int max77705_charger_probe(struct i2c_client *i2c)
> goto destroy_wq;
> }
>
> + ret = devm_delayed_work_autocancel(dev, &chg->aicl_work, max77705_aicl_isr_work);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to initialize interrupt work\n");
> + goto destroy_wq;
> + }
> +
> ret = max77705_charger_initialize(chg);
> if (ret) {
> dev_err_probe(dev, ret, "failed to initialize charger IC\n");
> @@ -632,6 +677,16 @@ static int max77705_charger_probe(struct i2c_client *i2c)
> goto destroy_wq;
> }
>
> + chg->aicl_irq = regmap_irq_get_virq(irq_data, MAX77705_AICL_I);
> + ret = devm_request_threaded_irq(dev, chg->aicl_irq,
> + NULL, max77705_aicl_irq,
> + IRQF_TRIGGER_NONE,
> + "aicl-irq", chg);
> + if (ret) {
> + dev_err_probe(dev, ret, "Failed to Request aicl IRQ\n");
> + goto destroy_wq;
> + }
> +
> ret = max77705_charger_enable(chg);
> if (ret) {
> dev_err_probe(dev, ret, "failed to enable charge\n");
> diff --git a/include/linux/power/max77705_charger.h b/include/linux/power/max77705_charger.h
> index 6653abfdf747..031c1dc2485d 100644
> --- a/include/linux/power/max77705_charger.h
> +++ b/include/linux/power/max77705_charger.h
> @@ -123,6 +123,8 @@
> #define MAX77705_DISABLE_SKIP 1
> #define MAX77705_AUTO_SKIP 0
>
> +#define AICL_WORK_DELAY_MS 100
> +
> /* uA */
> #define MAX77705_CURRENT_CHGIN_STEP 25000
> #define MAX77705_CURRENT_CHG_STEP 50000
> @@ -185,7 +187,9 @@ struct max77705_charger_data {
> struct power_supply_battery_info *bat_info;
> struct workqueue_struct *wqueue;
> struct work_struct chgin_work;
> + struct delayed_work aicl_work;
> struct power_supply *psy_chg;
> + int aicl_irq;
> };
>
> #endif /* __MAX77705_CHARGER_H */
>
> ---
> base-commit: 846bd2225ec3cfa8be046655e02b9457ed41973e
> change-id: 20250830-max77705_77976_charger_improvement-e3f417bfaa56
>
> Best regards,
> --
> Dzmitry Sankouski <dsankouski@...il.com>
>
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists