Re: [PATCH v4] power: supply: bq27xxx: detect bq27541 behind bq27411 compatible

[Andrew Davis <afd@ti.com>]

On 9/27/26 5:31 PM, Pali Rohár wrote:
> On Monday 28 September 2026 00:13:43 David Heidelberg wrote:
>> On 28/09/2026 00:00, Pali Rohár wrote:
>>> On Monday 28 September 2026 00:17:17 Rinat Muhamedgaliev via B4 Relay wrote:
>>>> From: Rinat Muhamedgaliev <rinat.muhamedgaliev@gmail.com>
>>>>
>>>> OnePlus 6 and 6T replacement battery packs may contain either a bq27411 or a bq27541 fuel gauge at I2C address 0x55. The device tree currently identifies the gauge as bq27411, but a bq27541 uses a different register map and then reports invalid battery values.
>>>>
>>>> Read the DeviceType control subcommand when probing a bq27411. Keep the existing profile for DeviceType 0x0421, but select the bq27541 profile for DeviceType 0x0541. This retains the established DT ABI and supports replacement packs without introducing a generic compatible.
>>>
>>> Hello! I have not read the previous versions of the patch or its
>>> discussion. But I have two points which could useful for future.
>>>
>>> In a past I was solving similar problem (there are more possible
>>> endpoint types in DT and hardcoding any of them cause replugging
>>> issues). And the solution was to "improve" bootloader to load the DTB
>>> file (from the storage) and then on-the-fly in RAM modify it to contains
>>> current configuration of endpoint device. This allowed to boot new
>>> kernel, and also old kernel without any modification of kernel or DTB
>>> file. So it retained the established DT ABI for kernel too.
>>
>> I think the patch here has two aspects here.
>>
>> 1st is verification on which chip it does run, which is something we don't
>> need discuss. This should be there forever. If nothing else, if different
>> chip than declared one is detected, the driver should report big warning.
>> This was missing.
> 
> My suggestion (but only for new things / drivers to prevent any
> compatibility issues) is to report / return fatal probe errors when
> verification fails.
> 
>> 2nd part is more tricky - having generic device-tree compatibles is
>> something opposite what DT trying to achieve. Here it's more like "a
>> workaround". The driver already loaded, using right i2c addr, it can easily
>> switch the device version and it saves us a lot of troubles people have
>> today (mostly for existing deployments).
> 
> Here we are trying to mix two opposite things: static device-tree with
> non-static hotpluggable / repluggable hardware. Similar problem has any
> hotpluggable bus (PCIe, USB, SDIO, ...) which needs to be described in
> device-tree (because bus itself static and burn into the chipset itself)
> but endpoint nodes on the bus in DT are non-static.
> 

There might be a solution already in the I2C framework as part of
the auto detection callback [0]. A lot of the later BQ27xxx devices
have the DEVICE_TYPE register at this same offset. Although auto
detection doesn't help with getting this driver module loaded in
the first place..

>> We'll try to do some proper autodetection at bootloader level later
>> (together with camera focus coil detection, which is provided by two vendors
>> on different i2c addresses).
> 
> This is what I saw more times... because it solved problem of "generic DT"
> and "hacks in kernel drivers" by completely hiding the problem from
> kernel and DT files point of view.
> 
>>>
>>> If there are more requests for these replugging support in bq27xxx, what
>>> about improving the whole bq27xxx driver to do autodetection of any
>>> plugged battery and take any explicit device tree identifier as a
>>> generic? If I remember correctly, it is not possible detect the whole
>>> type, but at least something is possible. This could solve this problem
>>> too, but would require some larger rewrite of driver. And it would make
>>> sense only if there are more requests for such functionality. I agree
>>> that it would probably do not make sense for one device.
>>
>> By DT principles we shouldn't introduce any generic compatibles.
> 
> Yes. That is balancing between principles, real HW and how it is already
> used by kernel drivers. I do not have any opinion for this as it looks
> like that every solution would have some gaps or issues.
> 
> Sometimes the easiest (in a way of writing the code) solution is better
> even if it does not fully match the design or principles.
> 
>> David
>>
>>>
>>> I'm not opposing the change, I'm just writing ideas for future, maybe
>>> they could be useful for some future decisions...
>>>
>>>> Tested on OnePlus 6T (fajita) with DeviceType 0x0541: voltage, state of charge, and temperature were reported correctly. Testing on hardware with DeviceType 0x0421 would be appreciated.
>>>>
>>>> Signed-off-by: Rinat Muhamedgaliev <rinat.muhamedgaliev@gmail.com>
>>>> ---
>>>> OnePlus 6 and 6T replacement battery packs can contain either a bq27411 or a
>>>> bq27541 fuel gauge. The latter requires a different register map and produces
>>>> invalid battery readings when interpreted as a bq27411.
>>>>
>>>> v4 drops the proposed generic DT compatible and DTS changes. The I2C driver
>>>> instead reads DeviceType while probing the existing bq27411 compatible, and
>>>> selects the bq27541 profile if the device reports 0x0541.
>>>>
>>>> The bq27541 path was tested on a OnePlus 6T. Testing on an OnePlus 6 or 6T
>>>> whose fuel gauge reports DeviceType 0x0421 (bq27411) would be appreciated.
>>>>
>>>> Changes in v4:
>>>> - Drop the generic compatible and binding update.
>>>> - Keep the established OnePlus DTS unchanged.
>>>> - Detect bq27541 from DeviceType in the bq27411 probe path.
>>>> - Send as a new thread.
>>>> ---
>>>>    drivers/power/supply/bq27xxx_battery_i2c.c | 37 ++++++++++++++++++++++++++++++
>>>>    1 file changed, 37 insertions(+)
>>>>
>>>> diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
>>>> index 94b00bb89c17..732164423423 100644
>>>> --- a/drivers/power/supply/bq27xxx_battery_i2c.c
>>>> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c
>>>> @@ -16,6 +16,11 @@
>>>>    static DEFINE_IDR(battery_id);
>>>>    static DEFINE_MUTEX(battery_mutex);
>>>> +#define BQ27XXX_REG_CTRL		0x00
>>>> +#define BQ27XXX_DEVICE_TYPE		0x0001
>>>> +#define BQ27411_DEVICE_TYPE		0x0421
>>>> +#define BQ27541_DEVICE_TYPE		0x0541
>>>> +
>>>>    static irqreturn_t bq27xxx_battery_irq_handler_thread(int irq, void *data)
>>>>    {
>>>>    	struct bq27xxx_device_info *di = data;
>>>> @@ -136,6 +141,32 @@ static int bq27xxx_battery_i2c_bulk_write(struct bq27xxx_device_info *di,
>>>>    	return 0;
>>>>    }
>>>> +static int bq27xxx_battery_i2c_check_device_type(struct bq27xxx_device_info *di)
>>>> +{
>>>> +	int ret;
>>>> +
>>>> +	ret = di->bus.write(di, BQ27XXX_REG_CTRL, BQ27XXX_DEVICE_TYPE,
>>>> +			    false);
>>>> +	if (ret < 0)
>>>> +		return ret;
>>>> +
>>>> +	ret = di->bus.read(di, BQ27XXX_REG_CTRL, false);
>>>> +	if (ret < 0)
>>>> +		return ret;
>>>> +
>>>> +	switch (ret) {
>>>> +	case BQ27411_DEVICE_TYPE:
>>>> +		return 0;
>>>> +	case BQ27541_DEVICE_TYPE:
>>>> +		dev_warn(di->dev, "detected bq27541 instead of bq27411\n");
>>>> +		di->chip = BQ27541;
>>>> +		return 0;
>>>> +	default:
>>>> +		dev_err(di->dev, "unsupported device type 0x%04x\n", ret);
>>>> +		return -ENODEV;
>>>> +	}
>>>> +}
>>>> +
>>>>    static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
>>>>    				     const struct i2c_device_id *id)
>>>>    {
>>>> @@ -169,6 +200,12 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
>>>>    	di->bus.read_bulk = bq27xxx_battery_i2c_bulk_read;
>>>>    	di->bus.write_bulk = bq27xxx_battery_i2c_bulk_write;
>>>> +	if (di->chip == BQ27411) {

Why only this one device? Feels very specific to your exact usecase.
If you add all the devices supported by this driver which have the
DEVICE_TYPE register, it might end up being easier to list the devices
to *not* check.

But maybe going though that many datasheets is asking too much, for
now this is still a good starting point and more device checks can
always be added on later.

Andrew

[0] https://github.com/torvalds/linux/blob/master/include/linux/i2c.h#L299

>>>> +		ret = bq27xxx_battery_i2c_check_device_type(di);
>>>> +		if (ret)
>>>> +			goto err_failed;
>>>> +	}
>>>> +
>>>>    	ret = bq27xxx_battery_setup(di);
>>>>    	if (ret)
>>>>    		goto err_failed;
>>>>
>>>> ---
>>>> base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476
>>>> change-id: 20260927-master-bd0afd9696ef
>>>>
>>>> Best regards,
>>>> --
>>>> Rinat Muhamedgaliev <rinat.muhamedgaliev@gmail.com>
>>>>
>>>>
>>