Re: [EXTERNAL] Re: [PATCH v9 2/2] power: supply: bq256xx: Introduce the BQ256XX charger driver
Sebastian,
On 1/5/21 3:26 PM, Sebastian Reichel wrote:
Hi,
On Tue, Jan 05, 2021 at 02:29:49PM -0600, Ricardo Rivera-Matos wrote:
The BQ256XX family of devices are highly integrated buck chargers
for single cell batteries.
Signed-off-by: Ricardo Rivera-Matos
v9 - resolves two warnings issued by kernel test robot
changelog needs to be below --- btw.
(so that git am does not pick it up :))
ACK
---
[...]
+ ret = bq256xx_parse_dt(bq, psy_cfg, dev);
+ if (ret) {
+ dev_err(dev, "Failed to read device tree properties%d\n", ret);
+ return ret;
+ }
[...]
If you want to change psy_cfg, you need to pass it by reference
and not by value (i.e. use _cfg here and a pointer as argument
of bq256xx_parse_dt). Providing psy_cfg like this creates a copy
of the struct.
ACK, understood.
Did you runtime test this version? It should crash when accessing
the properties because of psy_cfg.drv_data being NULL.
ACK, I did not, my mistake. v10 will get tested on the actual hardware.
[...]
+ ret = bq256xx_power_supply_init(bq, psy_cfg, dev);
+ if (ret) {
+ dev_err(dev, "Failed to register power supply\n");
+ return ret;
+ }
Here it's also better to just provide the address of psy_cfg
(but not strictly necessary).
-- Sebastian
Thanks,
Ricardo
Re: [PATCH v9 2/2] power: supply: bq256xx: Introduce the BQ256XX charger driver
Hi,
On Tue, Jan 05, 2021 at 02:29:49PM -0600, Ricardo Rivera-Matos wrote:
> The BQ256XX family of devices are highly integrated buck chargers
> for single cell batteries.
>
> Signed-off-by: Ricardo Rivera-Matos
>
> v9 - resolves two warnings issued by kernel test robot
changelog needs to be below --- btw.
(so that git am does not pick it up :))
> ---
> [...]
> + ret = bq256xx_parse_dt(bq, psy_cfg, dev);
> + if (ret) {
> + dev_err(dev, "Failed to read device tree properties%d\n", ret);
> + return ret;
> + }
> [...]
If you want to change psy_cfg, you need to pass it by reference
and not by value (i.e. use _cfg here and a pointer as argument
of bq256xx_parse_dt). Providing psy_cfg like this creates a copy
of the struct.
Did you runtime test this version? It should crash when accessing
the properties because of psy_cfg.drv_data being NULL.
> [...]
> + ret = bq256xx_power_supply_init(bq, psy_cfg, dev);
> + if (ret) {
> + dev_err(dev, "Failed to register power supply\n");
> + return ret;
> + }
Here it's also better to just provide the address of psy_cfg
(but not strictly necessary).
-- Sebastian
signature.asc
Description: PGP signature
[PATCH v9 2/2] power: supply: bq256xx: Introduce the BQ256XX charger driver
The BQ256XX family of devices are highly integrated buck chargers for single cell batteries. Signed-off-by: Ricardo Rivera-Matos v9 - resolves two warnings issued by kernel test robot --- drivers/power/supply/Kconfig | 11 + drivers/power/supply/Makefile |1 + drivers/power/supply/bq256xx_charger.c | 1745 3 files changed, 1757 insertions(+) create mode 100644 drivers/power/supply/bq256xx_charger.c diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index eec646c568b7..cedef501e683 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -645,6 +645,17 @@ config CHARGER_BQ25980 Say Y to enable support for the TI BQ25980, BQ25975 and BQ25960 series of fast battery chargers. +config CHARGER_BQ256XX + tristate "TI BQ256XX battery charger driver" + depends on I2C + depends on GPIOLIB || COMPILE_TEST + select REGMAP_I2C + help + Say Y to enable support for the TI BQ256XX battery chargers. The + BQ256XX family of devices are highly-integrated, switch-mode battery + charge management and system power path management devices for single + cell Li-ion and Li-polymer batteries. + config CHARGER_SMB347 tristate "Summit Microelectronics SMB3XX Battery Charger" depends on I2C diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile index dd4b86318cd9..ae322b1da1ed 100644 --- a/drivers/power/supply/Makefile +++ b/drivers/power/supply/Makefile @@ -85,6 +85,7 @@ obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o obj-$(CONFIG_CHARGER_BQ2515X) += bq2515x_charger.o obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o obj-$(CONFIG_CHARGER_BQ25980) += bq25980_charger.o +obj-$(CONFIG_CHARGER_BQ256XX) += bq256xx_charger.o obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o obj-$(CONFIG_CHARGER_TPS65217) += tps65217_charger.o diff --git a/drivers/power/supply/bq256xx_charger.c b/drivers/power/supply/bq256xx_charger.c new file mode 100644 index ..32d6e99e0443 --- /dev/null +++ b/drivers/power/supply/bq256xx_charger.c @@ -0,0 +1,1745 @@ +// SPDX-License-Identifier: GPL-2.0 +// BQ256XX Battery Charger Driver +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define BQ256XX_MANUFACTURER "Texas Instruments" + +#define BQ256XX_INPUT_CURRENT_LIMIT0x00 +#define BQ256XX_CHARGER_CONTROL_0 0x01 +#define BQ256XX_CHARGE_CURRENT_LIMIT 0x02 +#define BQ256XX_PRECHG_AND_TERM_CURR_LIM 0x03 +#define BQ256XX_BATTERY_VOLTAGE_LIMIT 0x04 +#define BQ256XX_CHARGER_CONTROL_1 0x05 +#define BQ256XX_CHARGER_CONTROL_2 0x06 +#define BQ256XX_CHARGER_CONTROL_3 0x07 +#define BQ256XX_CHARGER_STATUS_0 0x08 +#define BQ256XX_CHARGER_STATUS_1 0x09 +#define BQ256XX_CHARGER_STATUS_2 0x0a +#define BQ256XX_PART_INFORMATION 0x0b +#define BQ256XX_CHARGER_CONTROL_4 0x0c + +#define BQ256XX_IINDPM_MASKGENMASK(4, 0) +#define BQ256XX_IINDPM_STEP_uA 10 +#define BQ256XX_IINDPM_OFFSET_uA 10 +#define BQ256XX_IINDPM_MIN_uA 10 +#define BQ256XX_IINDPM_MAX_uA 320 +#define BQ256XX_IINDPM_DEF_uA 240 + +#define BQ256XX_VINDPM_MASKGENMASK(3, 0) +#define BQ256XX_VINDPM_STEP_uV 10 +#define BQ256XX_VINDPM_OFFSET_uV 390 +#define BQ256XX_VINDPM_MIN_uV 390 +#define BQ256XX_VINDPM_MAX_uV 540 +#define BQ256XX_VINDPM_DEF_uV 450 + +#define BQ256XX_VBATREG_MASK GENMASK(7, 3) +#define BQ2560X_VBATREG_STEP_uV32000 +#define BQ2560X_VBATREG_OFFSET_uV 3856000 +#define BQ2560X_VBATREG_MIN_uV 3856000 +#define BQ2560X_VBATREG_MAX_uV 4624000 +#define BQ2560X_VBATREG_DEF_uV 4208000 +#define BQ25601D_VBATREG_OFFSET_uV 3847000 +#define BQ25601D_VBATREG_MIN_uV3847000 +#define BQ25601D_VBATREG_MAX_uV4615000 +#define BQ25601D_VBATREG_DEF_uV4199000 +#define BQ2561X_VBATREG_STEP_uV1 +#define BQ25611D_VBATREG_MIN_uV3494000 +#define BQ25611D_VBATREG_MAX_uV451 +#define BQ25611D_VBATREG_DEF_uV419 +#define BQ25618_VBATREG_MIN_uV 3504000 +#define BQ25618_VBATREG_MAX_uV 450 +#define BQ25618_VBATREG_DEF_uV 420 +#define BQ256XX_VBATREG_BIT_SHIFT 3 +#define BQ2561X_VBATREG_THRESH 0x8 +#define BQ25611D_VBATREG_THRESH_uV 429 +#define BQ25618_VBATREG_THRESH_uV 430 + +#define BQ256XX_ITERM_MASK
