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

[Waqar Hameed <waqar.hameed@axis.com>]

On Tue, Jun 02, 2026 at 18:17 +0200 Sebastian Reichel <sebastian.reichel@collabora.com> wrote:

> On Wed, May 20, 2026 at 05:49:38PM +0200, Waqar Hameed wrote:

[...]

> 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 :)

Sounds reasonable! I'm doing a thorough testing of this right now (with
some minor issues found) and will send an updated version of that soon
so we can have the base functionality merged.

>> > 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.

Alright, will do that!

>
>> >   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.

I'll send these as separate patches then.

>
>> >      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)

I think this sounds reasonable. Most future devices (if not all?) would
probably be covered with these three states.

>
> 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?

The USB specification actually lists several different methods [1]. I
agree that the wisest right now is to just leave this and wait for more
data points (from how other devices will expose this).

>
>> Friendly ping incoming!
>
> Sorry for being slow with reviews, I was quite busy and out of
> office in the whole May. 

No worries! I know how the workload can be as a maintainer (especially
if you're all by yourself). Thank you for taking your time! :)

> I suggest that the 3 things are being send on top of the updaed base
> driver :)

Yes, let's do baby steps here and merge base functionality first (as
discussed above) before adding all these extra features.

[1] `USB Type-C Spec R2.4 - October 2024.pdf`, A.3, p. 351. https://usb.org/sites/default/files/USB%20Type-C%202.4%20Release%20202410.zip