Re: [PATCH v11 4/4] power: supply: bq25150 introduce the bq25150
On 5/28/20 9:43 AM, Andrew F. Davis wrote:
On 5/28/20 10:05 AM, Ricardo Rivera-Matos wrote:
+static int bq2515x_set_precharge_current(struct bq2515x_device *bq2515x,
+ int val)
+{
+ int ret;
+ unsigned int pchrgctrl;
+ unsigned int icharge_range;
+ unsigned int precharge_reg_code;
+ u16 precharge_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
+ u16 precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_1875_UA;
Why u16? looks like it gets promoted everywhere it's used anyway.
ACK
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, );
+ if (ret)
+ return ret;
+
+ icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
+
+ if (icharge_range) {
+ precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_3750_UA;
+ precharge_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
This is a little hard to read when we have a default value overwritten
in an if, it basically hides the else logic, suggest:
if (icharge_range) {
precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_3750_UA;
precharge_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
} else {
precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_1875_UA;
precharge_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
}
ACK. I originally had it as an if/else deal, but I got feedback it was
too verbose. It will stay verbose.
+ }
+ if (val > precharge_max_ua || val < BQ2515X_ICHG_MIN_UA)
+ return -EINVAL;
+
+ precharge_reg_code = val / precharge_multiplier;
+
+ ret = bq2515x_set_charge_disable(bq2515x, 1);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(bq2515x->regmap, BQ2515X_PCHRGCTRL,
+ BQ2515X_PRECHARGE_MASK, precharge_reg_code);
+ if (ret)
+ return ret;
+
+ return bq2515x_set_charge_disable(bq2515x, 0);
+}
[snip]
+
+static int bq2515x_set_ilim_lvl(struct bq2515x_device *bq2515x, int val)
+{
+ int i = 0;
+ unsigned int array_size = ARRAY_SIZE(bq2515x_ilim_lvl_values);
+
+ if (val >= bq2515x_ilim_lvl_values[array_size - 1]) {
Isn't this check the same as is done in first iteration of the below loop?
Andrew
ACK
+ i = array_size - 1;
+ } else {
+ for (i = array_size - 1; i > 0; i--) {
+ if (val >= bq2515x_ilim_lvl_values[i])
+ break;
+ }
+ }
+ return regmap_write(bq2515x->regmap, BQ2515X_ILIMCTRL, i);
+}
+
Re: [PATCH v11 4/4] power: supply: bq25150 introduce the bq25150
On 5/28/20 10:05 AM, Ricardo Rivera-Matos wrote:
> +static int bq2515x_set_precharge_current(struct bq2515x_device *bq2515x,
> + int val)
> +{
> + int ret;
> + unsigned int pchrgctrl;
> + unsigned int icharge_range;
> + unsigned int precharge_reg_code;
> + u16 precharge_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
> + u16 precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_1875_UA;
Why u16? looks like it gets promoted everywhere it's used anyway.
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, );
> + if (ret)
> + return ret;
> +
> + icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
> +
> + if (icharge_range) {
> + precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_3750_UA;
> + precharge_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
This is a little hard to read when we have a default value overwritten
in an if, it basically hides the else logic, suggest:
if (icharge_range) {
precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_3750_UA;
precharge_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
} else {
precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_1875_UA;
precharge_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
}
> + }
> + if (val > precharge_max_ua || val < BQ2515X_ICHG_MIN_UA)
> + return -EINVAL;
> +
> + precharge_reg_code = val / precharge_multiplier;
> +
> + ret = bq2515x_set_charge_disable(bq2515x, 1);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(bq2515x->regmap, BQ2515X_PCHRGCTRL,
> + BQ2515X_PRECHARGE_MASK, precharge_reg_code);
> + if (ret)
> + return ret;
> +
> + return bq2515x_set_charge_disable(bq2515x, 0);
> +}
[snip]
> +
> +static int bq2515x_set_ilim_lvl(struct bq2515x_device *bq2515x, int val)
> +{
> + int i = 0;
> + unsigned int array_size = ARRAY_SIZE(bq2515x_ilim_lvl_values);
> +
> + if (val >= bq2515x_ilim_lvl_values[array_size - 1]) {
Isn't this check the same as is done in first iteration of the below loop?
Andrew
> + i = array_size - 1;
> + } else {
> + for (i = array_size - 1; i > 0; i--) {
> + if (val >= bq2515x_ilim_lvl_values[i])
> + break;
> + }
> + }
> + return regmap_write(bq2515x->regmap, BQ2515X_ILIMCTRL, i);
> +}
> +
[PATCH v11 4/4] power: supply: bq25150 introduce the bq25150
Introduce the bq2515x family of chargers. The BQ2515X family of devices are highly integrated battery management ICs that integrate the most common functions for wearable devices namely a charger, an output voltage rail, ADC for battery and system monitoring, and a push-button controller. Datasheets: bq25150 - http://www.ti.com/lit/ds/symlink/bq25150.pdf bq25155 - http://www.ti.com/lit/ds/symlink/bq25155.pdf Signed-off-by: Ricardo Rivera-Matos --- drivers/power/supply/Kconfig | 13 + drivers/power/supply/Makefile |1 + drivers/power/supply/bq2515x_charger.c | 1159 3 files changed, 1173 insertions(+) create mode 100644 drivers/power/supply/bq2515x_charger.c diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index f3424fdce341..266193301e2d 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -589,6 +589,19 @@ config CHARGER_BQ24735 help Say Y to enable support for the TI BQ24735 battery charger. +config CHARGER_BQ2515X + tristate "TI BQ2515X battery charger family" + depends on I2C + depends on GPIOLIB || COMPILE_TEST + select REGMAP_I2C + help + Say Y to enable support for the TI BQ2515X family of battery + charging integrated circuits. The BQ2515X are highly integrated + battery charge management ICs that integrate the most common + functions for wearable devices, namely a charger, an output voltage + rail, ADC for battery and system monitoring, and push-button + controller. + config CHARGER_BQ25890 tristate "TI BQ25890 battery charger driver" depends on I2C diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile index 6c7da920ea83..8fcc175a7e22 100644 --- a/drivers/power/supply/Makefile +++ b/drivers/power/supply/Makefile @@ -80,6 +80,7 @@ obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o obj-$(CONFIG_CHARGER_BQ24257) += bq24257_charger.o obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o +obj-$(CONFIG_CHARGER_BQ2515X) += bq2515x_charger.o obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o diff --git a/drivers/power/supply/bq2515x_charger.c b/drivers/power/supply/bq2515x_charger.c new file mode 100644 index ..3e9d221e23bb --- /dev/null +++ b/drivers/power/supply/bq2515x_charger.c @@ -0,0 +1,1159 @@ +// SPDX-License-Identifier: GPL-2.0 +// BQ2515X Battery Charger Driver +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define BQ2515X_MANUFACTURER "Texas Instruments" + +#define BQ2515X_STAT0 0x00 +#define BQ2515X_STAT1 0x01 +#define BQ2515X_STAT2 0x02 +#define BQ2515X_FLAG0 0x03 +#define BQ2515X_FLAG1 0x04 +#define BQ2515X_FLAG2 0x05 +#define BQ2515X_FLAG3 0x06 +#define BQ2515X_MASK0 0x07 +#define BQ2515X_MASK1 0x08 +#define BQ2515X_MASK2 0x09 +#define BQ2515X_MASK3 0x0a +#define BQ2515X_VBAT_CTRL 0x12 +#define BQ2515X_ICHG_CTRL 0x13 +#define BQ2515X_PCHRGCTRL 0x14 +#define BQ2515X_TERMCTRL 0x15 +#define BQ2515X_BUVLO 0x16 +#define BQ2515X_CHARGERCTRL0 0x17 +#define BQ2515X_CHARGERCTRL1 0x18 +#define BQ2515X_ILIMCTRL 0x19 +#define BQ2515X_LDOCTRL0x1d +#define BQ2515X_MRCTRL 0x30 +#define BQ2515X_ICCTRL00x35 +#define BQ2515X_ICCTRL10x36 +#define BQ2515X_ICCTRL20x37 +#define BQ2515X_ADCCTRL0 0x40 +#define BQ2515X_ADCCTRL1 0x41 +#define BQ2515X_ADC_VBAT_M 0x42 +#define BQ2515X_ADC_VBAT_L 0x43 +#define BQ2515X_ADC_TS_M 0x44 +#define BQ2515X_ADC_TS_L 0x45 +#define BQ2515X_ADC_ICHG_M 0x46 +#define BQ2515X_ADC_ICHG_L 0x47 +#define BQ2515X_ADC_ADCIN_M0x48 +#define BQ2515X_ADC_ADCIN_L0x49 +#define BQ2515X_ADC_VIN_M 0x4a +#define BQ2515X_ADC_VIN_L 0x4b +#define BQ2515X_ADC_PMID_M 0x4c +#define BQ2515X_ADC_PMID_L 0x4d +#define BQ2515X_ADC_IIN_M 0x4e +#define BQ2515X_ADC_IIN_L 0x4f +#define BQ2515X_ADC_COMP1_M0x52 +#define BQ2515X_ADC_COMP1_L0X53 +#define BQ2515X_ADC_COMP2_M0X54 +#define BQ2515X_ADC_COMP2_L0x55 +#define BQ2515X_ADC_COMP3_M0x56 +#define BQ2515X_ADC_COMP3_L0x57 +#define BQ2515X_ADC_READ_EN0x58 +#define BQ2515X_TS_FASTCHGCTRL 0x61 +#define BQ2515X_TS_COLD0x62 +#define BQ2515X_TS_COOL0x63 +#define BQ2515X_TS_WARM0x64 +#define BQ2515X_TS_HOT 0x65 +#define BQ2515X_DEVICE_ID 0x6f + +#define BQ2515X_DEFAULT_ICHG_UA1 +#define BQ25150_DEFAULT_ILIM_UA
