Re: [RFC PATCH 0/2] Add driver for TI BQ25630 charger

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 3832 bytes --]

Hi,

On Wed, May 20, 2026 at 05:49:38PM +0200, Waqar Hameed wrote:
> On Fri, Feb 27, 2026 at 16:35 +0100 Waqar Hameed <waqar.hameed@axis.com> wrote:
> > This patch series contains a fully working driver for the basic
> > functionality for the new TI BQ25630 charger (see datasheet [1]).

Driver itself LGTM. I think it would be sensible to merge that while
the extra features are being prepared, but the DT binding must be
fixed first :)

> > However, some functionality has no straightforward implementation. The
> > following features have therefore been left out and hopefully we can
> > have some design discussions to reach a clear resolution for the next
> > patch version (hence the RFC tag):
> >
> >   1. The USB OTG functionality (i.e. power *out* from the device) would
> >      probably need a minor refactorization to use the MFD sub-system to
> >      also register a regulator driver. Looking at the bq257xx driver,
> >      this should be the preferred design?

If the regulator is the only part making this a MFD, just create it
in the charger driver itself. There are a few examples, just grep
for 'struct regulator_ops' in drivers/power/supply.

> >   2. Other drivers add a custom `sysfs` attributes for BATFET control.
> >      See for example rt9471 and bq24190. Is this the preferred approach?
> >      Should we add a new power `sysfs` class ABI for this? (There is a
> >      TODO left in the code for this.)
> >      
> >      I reckon it is quite common to have BATFET control for chargers,
> >      i.e. being able to set them in "ship mode", "stand-by mode",
> >      "shutdown mode" or "idle mode" (example values taken from the
> >      `BATFET_CTRL` register field from datasheet [1])?

Yes. I usually ask to use custom properties until there are a few
users to avoid cluttering the general ABI. I think BATFET is a good
candidate. Please add a new property for that one.

> >   3. This device has liquid detection and corrosion mitigation. I
> >      couldn't find any existing device driver with this kind of
> >      functionality. The datasheet [1] even mentions "patent pending",
> >      although it refers to the USB type-C Specification 2.3... :)

This is something I also expect to see in more devices; I know a
couple of Android devices offer this. But indeed nothing has been
submitted so far.

> >      When liquid is detected in the charging port, an interrupt is
> >      fired. Likewise, an interrupt can be fired when the port is dry
> >      enough (according to some configured threshold value). My initial
> >      thought was that maybe we can add "liquid detected" to the `health`
> >      `sysfs` ABI?

Sounds good to me.

> >      However, the question still remains though how one
> >      should enable/disable and set threshold values for this (new power
> >      class `sysfs` ABI or a custom one only for this driver)?
> >
> > [1] https://www.ti.com/lit/gpn/bq25630

I had a quick look at the datasheet. I think a new standard property
would be sensible:

LIQUID_DETECTION_CONTROL with possible values
    off = Not liquid detection is being performed
    auto = Periodic liquid detection checks are being done
    once = Writing this will perform a single check (and return to off)

For the other controls I'm not sure how generalized they are. E.g.
for TLQD and ILQD it offers 4 steps, but not explicit current
(seconds and amps). OTOH for the voltage explicit values are listed.
Maybe just leave them to the default until somebody actually need
this configurable?

> Friendly ping incoming!

Sorry for being slow with reviews, I was quite busy and out of
office in the whole May. I suggest that the 3 things are being
send on top of the updaed base driver :)

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]