linux-kernel - Re: [PATCH v3 2/3] power: supply: Add bd718(15/28/78) charger driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdbZN3LB=iVwL0YLEoUOiPMSePdOF_NEGWuCncDAjWY4XA@mail.gmail.com>
Date: Thu, 28 Aug 2025 09:45:24 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: Andreas Kemnade <andreas@...nade.info>
Cc: Matti Vaittinen <mazziesaccount@...il.com>, Lee Jones <lee@...nel.org>, 
	Sebastian Reichel <sre@...nel.org>, linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, 
	Krzysztof Kozlowski <krzk@...nel.org>
Subject: Re: [PATCH v3 2/3] power: supply: Add bd718(15/28/78) charger driver

Hi Andreas / Cong,

On Thu, Aug 21, 2025 at 8:25 PM Andreas Kemnade <andreas@...nade.info> wrote:

> Add charger driver for ROHM BD718(15/28/78) PMIC charger block.
> It is a stripped down version of the driver here:
> https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/
>
> For the ease of review and to do a step-by-step approach remove all the
> coloumb counter related stuff and do not sneak in BD71827 support. That
> also avoids non-trivial rebasing of the above series.
>
> Changes besides that:
> Replace the custom property by a standard one and do not use megaohms
> for the current sense resistor.
>
> Signed-off-by: Andreas Kemnade <andreas@...nade.info>
> Reviewed-by: Matti Vaittinen <mazziesaccount@...il.com>

I think it looks good to merge:
Reviewed-by: Linus Walleij <linus.walleij@...aro.org>

It contains some interesting pointers for future work:

> +/* TODO: Verify the meaning of these interrupts */
> +BD_ISR_BAT(rechg_det, "Recharging", true)
> +BD_ISR_BAT(rechg_res, "Recharge ending", true)
> +BD_ISR_DUMMY(temp_transit, "Temperature transition")
> +BD_ISR_BAT(therm_rmv, "bd71815-therm-rmv", false)
> +BD_ISR_BAT(therm_det, "bd71815-therm-det", true)
> +BD_ISR_BAT(bat_dead, "bd71815-bat-dead", false)
> +BD_ISR_BAT(bat_short_res, "bd71815-bat-short-res", true)
> +BD_ISR_BAT(bat_short, "bd71815-bat-short-det", false)
> +BD_ISR_BAT(bat_low_res, "bd71815-bat-low-res", true)
> +BD_ISR_BAT(bat_low, "bd71815-bat-low-det", true)
> +BD_ISR_BAT(bat_ov_res, "bd71815-bat-over-res", true)

Some of these look like they should immediately shut down the
system, I suppose the battery charger does this autonomously
but it should probably also trigger an emergency shutdown
of Linux, right?

> +/* What should we do here? */
> +BD_ISR_BAT(bat_ov, "bd71815-bat-over-det", false)

At overvoltage all charging should stop, I guess the hardware
does that autonomouslyd and this is just to inform us that
this has happened.

> +BD_ISR_BAT(bat_mon_res, "bd71815-bat-mon-res", true)
> +BD_ISR_BAT(bat_mon, "bd71815-bat-mon-det", true)
> +BD_ISR_BAT(bat_cc_mon, "bd71815-bat-cc-mon2", false)
> +BD_ISR_BAT(bat_oc1_res, "bd71815-bat-oc1-res", true)
> +BD_ISR_BAT(bat_oc1, "bd71815-bat-oc1-det", false)
> +BD_ISR_BAT(bat_oc2_res, "bd71815-bat-oc2-res", true)
> +BD_ISR_BAT(bat_oc2, "bd71815-bat-oc2-det", false)
> +BD_ISR_BAT(bat_oc3_res, "bd71815-bat-oc3-res", true)
> +BD_ISR_BAT(bat_oc3, "bd71815-bat-oc3-det", false)
> +BD_ISR_BAT(temp_bat_low_res, "bd71815-temp-bat-low-res", true)
> +BD_ISR_BAT(temp_bat_low, "bd71815-temp-bat-low-det", true)
> +BD_ISR_BAT(temp_bat_hi_res, "bd71815-temp-bat-hi-res", true)
> +BD_ISR_BAT(temp_bat_hi, "bd71815-temp-bat-hi-det", true)

The "oc" stuff is "open circuit" and probably different interrupts
to indicate that a measurement of the open circuit voltage
is available in some register, which enables you do do more
precise capacity estimation, right?

If it is useful depends on if the device is e.g. flashed with
capacity tables vs OC voltage in the factory, and the charger
then has "deep knowledge" of the battery it is handling, I guess
this is how it works? Then this again is just informational, it
informs you of what the device-internal charging algorithm
is doing.

As for the temperature these are probably also just informational
if all these states are already handled by the hardware itself.

They all seem to be intended so that you can essentially put
these into custom sysfs or debugfs entries and then get out
a graphical state machine of the battery charger. That requires
deep knowledge of how the charger works (a datasheet)
and has limited use for a normal user.

Yours,
Linus Walleij