linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on
@ 2021-02-09 23:05 Ricardo Rivera-Matos
  2021-02-09 23:05 ` [PATCH 2/2] power: supply: bq25980: Moves properties from battery node Ricardo Rivera-Matos
  2021-02-10  8:20 ` [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on Krzysztof Kozlowski
  0 siblings, 2 replies; 7+ messages in thread
From: Ricardo Rivera-Matos @ 2021-02-09 23:05 UTC (permalink / raw)
  To: sre, linux-pm, linux-kernel; +Cc: Ricardo Rivera-Matos

fix: corrects various register step size and offset values

fix: corrects bq25980_get_input_curr_lim() and bq25980_set_input_curr_lim()

fix: corrects bq25980_get_const_charge_curr() and bq25980_set_const_charge_curr()

fix: corrects BQ25960_BATOVP_MIN_uV, BQ25960_BATOVP_OFFSET_uV,

BQ25960_BATOVP_STEP_uV, and BQ25960_BATOVP_MAX_uV

fix: corrects busocp_sc_min and busocp_byp_min members

fix: removes unnecessary polarity check from bq25980_get_adc_ibus()

fix: removes unnecessary polarity check from bq25980_get_adc_ibat()

fix: clamps ibat_adc to match datasheet change

Fixes: 5069185fc18e ("power: supply: bq25980: Add support for the BQ259xx family")
Signed-off-by: Ricardo Rivera-Matos <r-rivera-matos@ti.com>
---
 drivers/power/supply/bq25980_charger.c | 141 ++++++++++++++++++++-----
 drivers/power/supply/bq25980_charger.h |  77 ++++++++++----
 2 files changed, 173 insertions(+), 45 deletions(-)

diff --git a/drivers/power/supply/bq25980_charger.c b/drivers/power/supply/bq25980_charger.c
index 530ff4025b31..7c489a9e8877 100644
--- a/drivers/power/supply/bq25980_charger.c
+++ b/drivers/power/supply/bq25980_charger.c
@@ -52,6 +52,10 @@ struct bq25980_chip_info {
 	int busocp_byp_max;
 	int busocp_sc_min;
 	int busocp_byp_min;
+	int busocp_sc_step;
+	int busocp_byp_step;
+	int busocp_sc_offset;
+	int busocp_byp_offset;
 
 	int busovp_sc_def;
 	int busovp_byp_def;
@@ -73,6 +77,20 @@ struct bq25980_chip_info {
 
 	int batocp_def;
 	int batocp_max;
+	int batocp_min;
+	int batocp_step;
+
+	int vbus_adc_step;
+	int vbus_adc_offset;
+
+	int ibus_adc_step;
+	int ibus_adc_offset;
+
+	int vbat_adc_step;
+	int vbat_adc_offset;
+
+	int ibat_adc_step;
+	int ibat_adc_offset;
 };
 
 struct bq25980_init_data {
@@ -275,13 +293,22 @@ static int bq25980_watchdog_time[BQ25980_NUM_WD_VAL] = {5000, 10000, 50000,
 static int bq25980_get_input_curr_lim(struct bq25980_device *bq)
 {
 	unsigned int busocp_reg_code;
+	int offset, step;
 	int ret;
 
+	if (bq->state.bypass) {
+		step = bq->chip_info->busocp_byp_step;
+		offset = bq->chip_info->busocp_byp_offset;
+	} else {
+		step = bq->chip_info->busocp_sc_step;
+		offset = bq->chip_info->busocp_sc_offset;
+	}
+
 	ret = regmap_read(bq->regmap, BQ25980_BUSOCP, &busocp_reg_code);
 	if (ret)
 		return ret;
 
-	return (busocp_reg_code * BQ25980_BUSOCP_STEP_uA) + BQ25980_BUSOCP_OFFSET_uA;
+	return (busocp_reg_code * step) + offset;
 }
 
 static int bq25980_set_hiz(struct bq25980_device *bq, int setting)
@@ -293,6 +320,7 @@ static int bq25980_set_hiz(struct bq25980_device *bq, int setting)
 static int bq25980_set_input_curr_lim(struct bq25980_device *bq, int busocp)
 {
 	unsigned int busocp_reg_code;
+	int step, offset;
 	int ret;
 
 	if (!busocp)
@@ -303,13 +331,17 @@ static int bq25980_set_input_curr_lim(struct bq25980_device *bq, int busocp)
 	if (busocp < BQ25980_BUSOCP_MIN_uA)
 		busocp = BQ25980_BUSOCP_MIN_uA;
 
-	if (bq->state.bypass)
+	if (bq->state.bypass) {
 		busocp = min(busocp, bq->chip_info->busocp_sc_max);
-	else
+		step = bq->chip_info->busocp_byp_step;
+		offset = bq->chip_info->busocp_byp_offset;
+	} else {
 		busocp = min(busocp, bq->chip_info->busocp_byp_max);
+		step = bq->chip_info->busocp_sc_step;
+		offset = bq->chip_info->busocp_sc_offset;
+	}
 
-	busocp_reg_code = (busocp - BQ25980_BUSOCP_OFFSET_uA)
-						/ BQ25980_BUSOCP_STEP_uA;
+	busocp_reg_code = (busocp - offset) / step;
 
 	ret = regmap_write(bq->regmap, BQ25980_BUSOCP, busocp_reg_code);
 	if (ret)
@@ -374,6 +406,7 @@ static int bq25980_set_input_volt_lim(struct bq25980_device *bq, int busovp)
 
 static int bq25980_get_const_charge_curr(struct bq25980_device *bq)
 {
+	int step = bq->chip_info->batocp_step;
 	unsigned int batocp_reg_code;
 	int ret;
 
@@ -381,19 +414,20 @@ static int bq25980_get_const_charge_curr(struct bq25980_device *bq)
 	if (ret)
 		return ret;
 
-	return (batocp_reg_code & BQ25980_BATOCP_MASK) *
-						BQ25980_BATOCP_STEP_uA;
+	return (batocp_reg_code & BQ25980_BATOCP_MASK) * step;
 }
 
 static int bq25980_set_const_charge_curr(struct bq25980_device *bq, int batocp)
 {
+	int step = bq->chip_info->batocp_step;
+	int max = bq->chip_info->batocp_max;
+	int min = bq->chip_info->batocp_min;
 	unsigned int batocp_reg_code;
 	int ret;
 
-	batocp = max(batocp, BQ25980_BATOCP_MIN_uA);
-	batocp = min(batocp, bq->chip_info->batocp_max);
+	batocp = clamp(batocp, min, max);
 
-	batocp_reg_code = batocp / BQ25980_BATOCP_STEP_uA;
+	batocp_reg_code = batocp / step;
 
 	ret = regmap_update_bits(bq->regmap, BQ25980_BATOCP,
 				BQ25980_BATOCP_MASK, batocp_reg_code);
@@ -490,10 +524,8 @@ static int bq25980_get_adc_ibus(struct bq25980_device *bq)
 
 	ibus_adc = (ibus_adc_msb << 8) | ibus_adc_lsb;
 
-	if (ibus_adc_msb & BQ25980_ADC_POLARITY_BIT)
-		return ((ibus_adc ^ 0xffff) + 1) * BQ25980_ADC_CURR_STEP_uA;
-
-	return ibus_adc * BQ25980_ADC_CURR_STEP_uA;
+	return (ibus_adc * bq->chip_info->ibus_adc_step) +
+					bq->chip_info->ibus_adc_offset;
 }
 
 static int bq25980_get_adc_vbus(struct bq25980_device *bq)
@@ -512,7 +544,8 @@ static int bq25980_get_adc_vbus(struct bq25980_device *bq)
 
 	vbus_adc = (vbus_adc_msb << 8) | vbus_adc_lsb;
 
-	return vbus_adc * BQ25980_ADC_VOLT_STEP_uV;
+	return (vbus_adc * bq->chip_info->vbus_adc_step) +
+					bq->chip_info->vbus_adc_offset;
 }
 
 static int bq25980_get_ibat_adc(struct bq25980_device *bq)
@@ -531,29 +564,31 @@ static int bq25980_get_ibat_adc(struct bq25980_device *bq)
 
 	ibat_adc = (ibat_adc_msb << 8) | ibat_adc_lsb;
 
-	if (ibat_adc_msb & BQ25980_ADC_POLARITY_BIT)
-		return ((ibat_adc ^ 0xffff) + 1) * BQ25980_ADC_CURR_STEP_uA;
+	ibat_adc = clamp(ibat_adc, BQ25960_ADC_IBAT_MIN_CODE,
+					BQ25960_ADC_IBAT_MAX_CODE);
 
-	return ibat_adc * BQ25980_ADC_CURR_STEP_uA;
+	return (ibat_adc * bq->chip_info->ibat_adc_step) +
+					bq->chip_info->ibat_adc_offset;
 }
 
 static int bq25980_get_adc_vbat(struct bq25980_device *bq)
 {
-	int vsys_adc_lsb, vsys_adc_msb;
-	u16 vsys_adc;
+	int vbat_adc_lsb, vbat_adc_msb;
+	u16 vbat_adc;
 	int ret;
 
-	ret = regmap_read(bq->regmap, BQ25980_VBAT_ADC_MSB, &vsys_adc_msb);
+	ret = regmap_read(bq->regmap, BQ25980_VBAT_ADC_MSB, &vbat_adc_msb);
 	if (ret)
 		return ret;
 
-	ret = regmap_read(bq->regmap, BQ25980_VBAT_ADC_LSB, &vsys_adc_lsb);
+	ret = regmap_read(bq->regmap, BQ25980_VBAT_ADC_LSB, &vbat_adc_lsb);
 	if (ret)
 		return ret;
 
-	vsys_adc = (vsys_adc_msb << 8) | vsys_adc_lsb;
+	vbat_adc = (vbat_adc_msb << 8) | vbat_adc_lsb;
 
-	return vsys_adc * BQ25980_ADC_VOLT_STEP_uV;
+	return (vbat_adc * bq->chip_info->vbat_adc_step) +
+					bq->chip_info->vbat_adc_offset;
 }
 
 static int bq25980_get_state(struct bq25980_device *bq,
@@ -984,6 +1019,10 @@ static const struct bq25980_chip_info bq25980_chip_info_tbl[] = {
 		.busocp_sc_max = BQ25980_BUSOCP_SC_MAX_uA,
 		.busocp_byp_max = BQ25980_BUSOCP_BYP_MAX_uA,
 		.busocp_byp_min = BQ25980_BUSOCP_MIN_uA,
+		.busocp_sc_step = BQ25980_BUSOCP_STEP_uA,
+		.busocp_byp_step = BQ25980_BUSOCP_STEP_uA,
+		.busocp_sc_offset = BQ25980_BUSOCP_OFFSET_uA,
+		.busocp_byp_offset = BQ25980_BUSOCP_OFFSET_uA,
 
 		.busovp_sc_def = BQ25980_BUSOVP_DFLT_uV,
 		.busovp_byp_def = BQ25980_BUSOVP_BYPASS_DFLT_uV,
@@ -1004,6 +1043,20 @@ static const struct bq25980_chip_info bq25980_chip_info_tbl[] = {
 
 		.batocp_def = BQ25980_BATOCP_DFLT_uA,
 		.batocp_max = BQ25980_BATOCP_MAX_uA,
+		.batocp_min = BQ25980_BATOCP_MIN_uA,
+		.batocp_step = BQ25980_BATOCP_STEP_uA,
+
+		.vbus_adc_step = BQ25980_ADC_VBUS_STEP_uV,
+		.vbus_adc_offset = BQ25980_ADC_VBUS_OFFSET_uV,
+
+		.ibus_adc_step = BQ25980_ADC_IBUS_STEP_uA,
+		.ibus_adc_offset = BQ25980_ADC_IBUS_OFFSET_uA,
+
+		.vbat_adc_step = BQ25980_ADC_VBAT_STEP_uV,
+		.vbat_adc_offset = BQ25980_ADC_VBAT_OFFSET_uV,
+
+		.ibat_adc_step = BQ25980_ADC_IBAT_STEP_uA,
+		.ibat_adc_offset = BQ25980_ADC_IBAT_OFFSET_uA,
 	},
 
 	[BQ25975] = {
@@ -1015,6 +1068,10 @@ static const struct bq25980_chip_info bq25980_chip_info_tbl[] = {
 		.busocp_sc_max = BQ25975_BUSOCP_SC_MAX_uA,
 		.busocp_byp_min = BQ25980_BUSOCP_MIN_uA,
 		.busocp_byp_max = BQ25975_BUSOCP_BYP_MAX_uA,
+		.busocp_sc_step = BQ25980_BUSOCP_STEP_uA,
+		.busocp_byp_step = BQ25980_BUSOCP_STEP_uA,
+		.busocp_sc_offset = BQ25980_BUSOCP_OFFSET_uA,
+		.busocp_byp_offset = BQ25980_BUSOCP_OFFSET_uA,
 
 		.busovp_sc_def = BQ25975_BUSOVP_DFLT_uV,
 		.busovp_byp_def = BQ25975_BUSOVP_BYPASS_DFLT_uV,
@@ -1035,6 +1092,20 @@ static const struct bq25980_chip_info bq25980_chip_info_tbl[] = {
 
 		.batocp_def = BQ25980_BATOCP_DFLT_uA,
 		.batocp_max = BQ25980_BATOCP_MAX_uA,
+		.batocp_min = BQ25980_BATOCP_MIN_uA,
+		.batocp_step = BQ25980_BATOCP_STEP_uA,
+
+		.vbus_adc_step = BQ25975_ADC_VBUS_STEP_uV,
+		.vbus_adc_offset = BQ25975_ADC_VBUS_OFFSET_uV,
+
+		.ibus_adc_step = BQ25975_ADC_IBUS_STEP_uA,
+		.ibus_adc_offset = BQ25975_ADC_IBUS_OFFSET_uA,
+
+		.vbat_adc_step = BQ25975_ADC_VBAT_STEP_uV,
+		.vbat_adc_offset = BQ25975_ADC_VBAT_OFFSET_uV,
+
+		.ibat_adc_step = BQ25975_ADC_IBAT_STEP_uA,
+		.ibat_adc_offset = BQ25975_ADC_IBAT_OFFSET_uA,
 	},
 
 	[BQ25960] = {
@@ -1042,10 +1113,14 @@ static const struct bq25980_chip_info bq25980_chip_info_tbl[] = {
 		.regmap_config = &bq25960_regmap_config,
 
 		.busocp_def = BQ25960_BUSOCP_DFLT_uA,
-		.busocp_sc_min = BQ25960_BUSOCP_SC_MAX_uA,
+		.busocp_sc_min = BQ25960_BUSOCP_SC_MIN_uA,
 		.busocp_sc_max = BQ25960_BUSOCP_SC_MAX_uA,
-		.busocp_byp_min = BQ25960_BUSOCP_SC_MAX_uA,
+		.busocp_byp_min = BQ25960_BUSOCP_BYP_MIN_uA,
 		.busocp_byp_max = BQ25960_BUSOCP_BYP_MAX_uA,
+		.busocp_sc_step = BQ25960_BUSOCP_SC_STEP_uA,
+		.busocp_byp_step = BQ25960_BUSOCP_BYP_STEP_uA,
+		.busocp_sc_offset = BQ25960_BUSOCP_SC_OFFSET_uA,
+		.busocp_byp_offset = BQ25960_BUSOCP_BYP_OFFSET_uA,
 
 		.busovp_sc_def = BQ25975_BUSOVP_DFLT_uV,
 		.busovp_byp_def = BQ25975_BUSOVP_BYPASS_DFLT_uV,
@@ -1066,6 +1141,20 @@ static const struct bq25980_chip_info bq25980_chip_info_tbl[] = {
 
 		.batocp_def = BQ25960_BATOCP_DFLT_uA,
 		.batocp_max = BQ25960_BATOCP_MAX_uA,
+		.batocp_min = BQ25960_BATOCP_MIN_uA,
+		.batocp_step = BQ25960_BATOCP_STEP_uA,
+
+		.vbus_adc_step = BQ25960_ADC_VBUS_STEP_uV,
+		.vbus_adc_offset = BQ25960_ADC_VBUS_OFFSET_uV,
+
+		.ibus_adc_step = BQ25960_ADC_IBUS_STEP_uA,
+		.ibus_adc_offset = BQ25960_ADC_IBUS_OFFSET_uA,
+
+		.vbat_adc_step = BQ25960_ADC_VBAT_STEP_uV,
+		.vbat_adc_offset = BQ25960_ADC_VBAT_OFFSET_uV,
+
+		.ibat_adc_step = BQ25960_ADC_IBAT_STEP_uA,
+		.ibat_adc_offset = BQ25960_ADC_IBAT_OFFSET_uA,
 	},
 };
 
diff --git a/drivers/power/supply/bq25980_charger.h b/drivers/power/supply/bq25980_charger.h
index 39f94eba5f6c..7394bd9d7263 100644
--- a/drivers/power/supply/bq25980_charger.h
+++ b/drivers/power/supply/bq25980_charger.h
@@ -66,22 +66,29 @@
 #define BQ25980_DEGLITCH_TIME		0x39
 #define BQ25980_CHRGR_CTRL_6	0x3A
 
-#define BQ25980_BUSOCP_STEP_uA		250000
-#define BQ25980_BUSOCP_OFFSET_uA	1000000
+#define BQ25980_BUSOCP_STEP_uA		262500
+#define BQ25980_BUSOCP_OFFSET_uA	1050000
 
-#define BQ25980_BUSOCP_DFLT_uA		4250000
-#define BQ25975_BUSOCP_DFLT_uA		4250000
+#define BQ25960_BUSOCP_SC_STEP_uA	254375
+#define BQ25960_BUSOCP_SC_OFFSET_uA	1017500
+#define BQ25960_BUSOCP_BYP_STEP_uA	261875
+#define BQ25960_BUSOCP_BYP_OFFSET_uA	1047500
+
+#define BQ25980_BUSOCP_DFLT_uA		4462500
+#define BQ25975_BUSOCP_DFLT_uA		4462500
 #define BQ25960_BUSOCP_DFLT_uA		3250000
 
-#define BQ25980_BUSOCP_MIN_uA		1000000
+#define BQ25980_BUSOCP_MIN_uA		1050000
+#define BQ25960_BUSOCP_SC_MIN_uA	1017500
+#define BQ25960_BUSOCP_BYP_MIN_uA	1047500
 
-#define BQ25980_BUSOCP_SC_MAX_uA	5750000
-#define BQ25975_BUSOCP_SC_MAX_uA	5750000
-#define BQ25960_BUSOCP_SC_MAX_uA	3750000
+#define BQ25980_BUSOCP_SC_MAX_uA	6037500
+#define BQ25975_BUSOCP_SC_MAX_uA	6037500
+#define BQ25960_BUSOCP_SC_MAX_uA	4578750
 
-#define BQ25980_BUSOCP_BYP_MAX_uA	8500000
-#define BQ25975_BUSOCP_BYP_MAX_uA	8500000
-#define BQ25960_BUSOCP_BYP_MAX_uA	5750000
+#define BQ25980_BUSOCP_BYP_MAX_uA	8925000
+#define BQ25975_BUSOCP_BYP_MAX_uA	8925000
+#define BQ25960_BUSOCP_BYP_MAX_uA	6808750
 
 #define BQ25980_BUSOVP_SC_STEP_uV	100000
 #define BQ25975_BUSOVP_SC_STEP_uV	50000
@@ -120,11 +127,11 @@
 
 #define BQ25980_BATOVP_STEP_uV		20000
 #define BQ25975_BATOVP_STEP_uV		10000
-#define BQ25960_BATOVP_STEP_uV		10000
+#define BQ25960_BATOVP_STEP_uV		9985
 
 #define BQ25980_BATOVP_OFFSET_uV	7000000
 #define BQ25975_BATOVP_OFFSET_uV	3500000
-#define BQ25960_BATOVP_OFFSET_uV	3500000
+#define BQ25960_BATOVP_OFFSET_uV	3491000
 
 #define BQ25980_BATOVP_DFLT_uV		14000000
 #define BQ25975_BATOVP_DFLT_uV		8900000
@@ -132,24 +139,26 @@
 
 #define BQ25980_BATOVP_MIN_uV		7000000
 #define BQ25975_BATOVP_MIN_uV		3500000
-#define BQ25960_BATOVP_MIN_uV		3500000
+#define BQ25960_BATOVP_MIN_uV		3491000
 
 #define BQ25980_BATOVP_MAX_uV		9540000
 #define BQ25975_BATOVP_MAX_uV		4770000
-#define BQ25960_BATOVP_MAX_uV		4770000
+#define BQ25960_BATOVP_MAX_uV		4759000
 
 #define BQ25980_BATOCP_STEP_uA		100000
+#define BQ25960_BATOCP_STEP_uA		102500
 
 #define BQ25980_BATOCP_MASK		GENMASK(6, 0)
 
 #define BQ25980_BATOCP_DFLT_uA		8100000
-#define BQ25960_BATOCP_DFLT_uA		6100000
+#define BQ25960_BATOCP_DFLT_uA		7277500
 
 #define BQ25980_BATOCP_MIN_uA		2000000
+#define BQ25960_BATOCP_MIN_uA		2050000
 
 #define BQ25980_BATOCP_MAX_uA		11000000
 #define BQ25975_BATOCP_MAX_uA		11000000
-#define BQ25960_BATOCP_MAX_uA		7000000
+#define BQ25960_BATOCP_MAX_uA		8712500
 
 #define BQ25980_ENABLE_HIZ		0xff
 #define BQ25980_DISABLE_HIZ		0x0
@@ -165,10 +174,40 @@
 #define BQ25980_EN_HIZ			BIT(6)
 #define BQ25980_ADC_EN			BIT(7)
 
-#define BQ25980_ADC_VOLT_STEP_uV        1000
-#define BQ25980_ADC_CURR_STEP_uA        1000
 #define BQ25980_ADC_POLARITY_BIT	BIT(7)
 
+#define BQ25980_ADC_IBUS_STEP_uA	1070
+#define BQ25980_ADC_IBUS_OFFSET_uA	0
+#define BQ25980_ADC_VBUS_STEP_uV	1007
+#define BQ25980_ADC_VBUS_OFFSET_uV	-60000
+
+#define BQ25980_ADC_IBAT_STEP_uA	1000
+#define BQ25980_ADC_IBAT_OFFSET_uA	0
+#define BQ25980_ADC_VBAT_STEP_uV	1006
+#define BQ25980_ADC_VBAT_OFFSET_uV	0
+
+#define BQ25975_ADC_IBUS_STEP_uA	1070
+#define BQ25975_ADC_IBUS_OFFSET_uA	0
+#define BQ25975_ADC_VBUS_STEP_uV	1003
+#define BQ25975_ADC_VBUS_OFFSET_uV	-40000
+
+#define BQ25975_ADC_IBAT_STEP_uA	1000
+#define BQ25975_ADC_IBAT_OFFSET_uA	12000
+#define BQ25975_ADC_VBAT_STEP_uV	1001
+#define BQ25975_ADC_VBAT_OFFSET_uV	10000
+
+#define BQ25960_ADC_IBUS_STEP_uA	1000
+#define BQ25960_ADC_IBUS_OFFSET_uA	65000
+#define BQ25960_ADC_VBUS_STEP_uV	1002
+#define BQ25960_ADC_VBUS_OFFSET_uV	0
+
+#define BQ25960_ADC_IBAT_STEP_uA	999
+#define BQ25960_ADC_IBAT_OFFSET_uA	-150
+#define BQ25960_ADC_IBAT_MIN_CODE	0x96
+#define BQ25960_ADC_IBAT_MAX_CODE	0x2ee0
+#define BQ25960_ADC_VBAT_STEP_uV	1017
+#define BQ25960_ADC_VBAT_OFFSET_uV	1000
+
 #define BQ25980_WATCHDOG_MASK	GENMASK(4, 3)
 #define BQ25980_WATCHDOG_DIS	BIT(2)
 #define BQ25980_WATCHDOG_MAX	300000
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] power: supply: bq25980: Moves properties from battery node
  2021-02-09 23:05 [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on Ricardo Rivera-Matos
@ 2021-02-09 23:05 ` Ricardo Rivera-Matos
  2021-02-10  8:23   ` Krzysztof Kozlowski
  2021-02-10  8:20 ` [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on Krzysztof Kozlowski
  1 sibling, 1 reply; 7+ messages in thread
From: Ricardo Rivera-Matos @ 2021-02-09 23:05 UTC (permalink / raw)
  To: sre, linux-pm, linux-kernel; +Cc: Ricardo Rivera-Matos

fix: exposes POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT on the

charger node

fix: exposes POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE on the

charger node

fix: eliminates unnecessary set_property for the battery node

Fixes: 5069185fc18e ("power: supply: bq25980: Add support for the BQ259xx family")
Signed-off-by: Ricardo Rivera-Matos <r-rivera-matos@ti.com>
---
 drivers/power/supply/bq25980_charger.c | 40 ++++++++------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/power/supply/bq25980_charger.c b/drivers/power/supply/bq25980_charger.c
index 7c489a9e8877..ac73e2c19238 100644
--- a/drivers/power/supply/bq25980_charger.c
+++ b/drivers/power/supply/bq25980_charger.c
@@ -641,33 +641,6 @@ static int bq25980_get_state(struct bq25980_device *bq,
 	return 0;
 }
 
-static int bq25980_set_battery_property(struct power_supply *psy,
-				enum power_supply_property psp,
-				const union power_supply_propval *val)
-{
-	struct bq25980_device *bq = power_supply_get_drvdata(psy);
-	int ret = 0;
-
-	switch (psp) {
-	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
-		ret = bq25980_set_const_charge_curr(bq, val->intval);
-		if (ret)
-			return ret;
-		break;
-
-	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
-		ret = bq25980_set_const_charge_volt(bq, val->intval);
-		if (ret)
-			return ret;
-		break;
-
-	default:
-		return -EINVAL;
-	}
-
-	return ret;
-}
-
 static int bq25980_get_battery_property(struct power_supply *psy,
 				enum power_supply_property psp,
 				union power_supply_propval *val)
@@ -736,6 +709,18 @@ static int bq25980_set_charger_property(struct power_supply *psy,
 			return ret;
 		break;
 
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+		ret = bq25980_set_const_charge_curr(bq, val->intval);
+		if (ret)
+			return ret;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		ret = bq25980_set_const_charge_volt(bq, val->intval);
+		if (ret)
+			return ret;
+		break;
+
 	default:
 		return -EINVAL;
 	}
@@ -957,7 +942,6 @@ static struct power_supply_desc bq25980_battery_desc = {
 	.name			= "bq25980-battery",
 	.type			= POWER_SUPPLY_TYPE_BATTERY,
 	.get_property		= bq25980_get_battery_property,
-	.set_property		= bq25980_set_battery_property,
 	.properties		= bq25980_battery_props,
 	.num_properties		= ARRAY_SIZE(bq25980_battery_props),
 	.property_is_writeable	= bq25980_property_is_writeable,
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on
  2021-02-09 23:05 [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on Ricardo Rivera-Matos
  2021-02-09 23:05 ` [PATCH 2/2] power: supply: bq25980: Moves properties from battery node Ricardo Rivera-Matos
@ 2021-02-10  8:20 ` Krzysztof Kozlowski
  2021-02-10 19:50   ` [EXTERNAL] " Ricardo Rivera-Matos
  1 sibling, 1 reply; 7+ messages in thread
From: Krzysztof Kozlowski @ 2021-02-10  8:20 UTC (permalink / raw)
  To: Ricardo Rivera-Matos; +Cc: sre, linux-pm, linux-kernel

On Wed, 10 Feb 2021 at 00:52, Ricardo Rivera-Matos
<r-rivera-matos@ti.com> wrote:
>
> fix: corrects various register step size and offset values
>
> fix: corrects bq25980_get_input_curr_lim() and bq25980_set_input_curr_lim()
>
> fix: corrects bq25980_get_const_charge_curr() and bq25980_set_const_charge_curr()
>
> fix: corrects BQ25960_BATOVP_MIN_uV, BQ25960_BATOVP_OFFSET_uV,
>
> BQ25960_BATOVP_STEP_uV, and BQ25960_BATOVP_MAX_uV
>
> fix: corrects busocp_sc_min and busocp_byp_min members
>
> fix: removes unnecessary polarity check from bq25980_get_adc_ibus()
>
> fix: removes unnecessary polarity check from bq25980_get_adc_ibat()
>
> fix: clamps ibat_adc to match datasheet change

Thanks for the patch.

Only one fix at a time and please exactly describe what is being fixed
using proper sentences (starting with capital letter, ending with a
full stop... and usually description needs multiple of such
sentences). You add here multiple changes without proper description
of a problem being fixed. This is not the correct style of a patch.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] power: supply: bq25980: Moves properties from battery node
  2021-02-09 23:05 ` [PATCH 2/2] power: supply: bq25980: Moves properties from battery node Ricardo Rivera-Matos
@ 2021-02-10  8:23   ` Krzysztof Kozlowski
  2021-02-10 19:51     ` [EXTERNAL] " Ricardo Rivera-Matos
  0 siblings, 1 reply; 7+ messages in thread
From: Krzysztof Kozlowski @ 2021-02-10  8:23 UTC (permalink / raw)
  To: Ricardo Rivera-Matos; +Cc: sre, linux-pm, linux-kernel

On Wed, 10 Feb 2021 at 00:52, Ricardo Rivera-Matos
<r-rivera-matos@ti.com> wrote:
>
> fix: exposes POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT on the
>
> charger node

Why is this a fix? Why is exposing this property wrong? What is the
problem here? Why do you start sentences with a small letter? Your
commit message should answer such questions.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [EXTERNAL] Re: [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on
  2021-02-10  8:20 ` [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on Krzysztof Kozlowski
@ 2021-02-10 19:50   ` Ricardo Rivera-Matos
  2021-02-10 20:38     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 7+ messages in thread
From: Ricardo Rivera-Matos @ 2021-02-10 19:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski; +Cc: sre, linux-pm, linux-kernel

Krzysztof,

On 2/10/21 2:20 AM, Krzysztof Kozlowski wrote:
> On Wed, 10 Feb 2021 at 00:52, Ricardo Rivera-Matos
> <r-rivera-matos@ti.com> wrote:
>> fix: corrects various register step size and offset values
>>
>> fix: corrects bq25980_get_input_curr_lim() and bq25980_set_input_curr_lim()
>>
>> fix: corrects bq25980_get_const_charge_curr() and bq25980_set_const_charge_curr()
>>
>> fix: corrects BQ25960_BATOVP_MIN_uV, BQ25960_BATOVP_OFFSET_uV,
>>
>> BQ25960_BATOVP_STEP_uV, and BQ25960_BATOVP_MAX_uV
>>
>> fix: corrects busocp_sc_min and busocp_byp_min members
>>
>> fix: removes unnecessary polarity check from bq25980_get_adc_ibus()
>>
>> fix: removes unnecessary polarity check from bq25980_get_adc_ibat()
>>
>> fix: clamps ibat_adc to match datasheet change
> Thanks for the patch.
>
> Only one fix at a time and please exactly describe what is being fixed
> using proper sentences (starting with capital letter, ending with a
> full stop... and usually description needs multiple of such
> sentences). You add here multiple changes without proper description
> of a problem being fixed. This is not the correct style of a patch.
ACK, this patch is meant to implement changes brought on by a new 
datasheet revision. The revision tweaked the register step size and 
offset values to improve the accuracy. I can rebase and reword the patch 
if that works for you.
>
> Best regards,
> Krzysztof
Best Regards,
Ricardo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [EXTERNAL] Re: [PATCH 2/2] power: supply: bq25980: Moves properties from battery node
  2021-02-10  8:23   ` Krzysztof Kozlowski
@ 2021-02-10 19:51     ` Ricardo Rivera-Matos
  0 siblings, 0 replies; 7+ messages in thread
From: Ricardo Rivera-Matos @ 2021-02-10 19:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski; +Cc: sre, linux-pm, linux-kernel



On 2/10/21 2:23 AM, Krzysztof Kozlowski wrote:
> On Wed, 10 Feb 2021 at 00:52, Ricardo Rivera-Matos
> <r-rivera-matos@ti.com> wrote:
>> fix: exposes POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT on the
>>
>> charger node
> Why is this a fix? Why is exposing this property wrong? What is the
> problem here? Why do you start sentences with a small letter? Your
> commit message should answer such questions.
ACK, I will rebase and reword this commit message
>
> Best regards,
> Krzysztof
Best Regards,
Ricardo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [EXTERNAL] Re: [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on
  2021-02-10 19:50   ` [EXTERNAL] " Ricardo Rivera-Matos
@ 2021-02-10 20:38     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Kozlowski @ 2021-02-10 20:38 UTC (permalink / raw)
  To: Ricardo Rivera-Matos; +Cc: sre, linux-pm, linux-kernel

On Wed, Feb 10, 2021 at 01:50:03PM -0600, Ricardo Rivera-Matos wrote:
> Krzysztof,
> 
> On 2/10/21 2:20 AM, Krzysztof Kozlowski wrote:
> > On Wed, 10 Feb 2021 at 00:52, Ricardo Rivera-Matos
> > <r-rivera-matos@ti.com> wrote:
> > > fix: corrects various register step size and offset values
> > > 
> > > fix: corrects bq25980_get_input_curr_lim() and bq25980_set_input_curr_lim()
> > > 
> > > fix: corrects bq25980_get_const_charge_curr() and bq25980_set_const_charge_curr()
> > > 
> > > fix: corrects BQ25960_BATOVP_MIN_uV, BQ25960_BATOVP_OFFSET_uV,
> > > 
> > > BQ25960_BATOVP_STEP_uV, and BQ25960_BATOVP_MAX_uV
> > > 
> > > fix: corrects busocp_sc_min and busocp_byp_min members
> > > 
> > > fix: removes unnecessary polarity check from bq25980_get_adc_ibus()
> > > 
> > > fix: removes unnecessary polarity check from bq25980_get_adc_ibat()
> > > 
> > > fix: clamps ibat_adc to match datasheet change
> > Thanks for the patch.
> > 
> > Only one fix at a time and please exactly describe what is being fixed
> > using proper sentences (starting with capital letter, ending with a
> > full stop... and usually description needs multiple of such
> > sentences). You add here multiple changes without proper description
> > of a problem being fixed. This is not the correct style of a patch.
> ACK, this patch is meant to implement changes brought on by a new datasheet
> revision.

Only one "fix" in commit msg mentions datasheet.

> The revision tweaked the register step size and offset values to
> improve the accuracy. I can rebase and reword the patch if that works for
> you.

Yes, please. "corrects bq25980_get_input_curr_lim() and
bq25980_set_input_curr_lim()" tells me absolutely nothing what was wrong
and how it's get corrected.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-02-10 20:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-09 23:05 [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on Ricardo Rivera-Matos
2021-02-09 23:05 ` [PATCH 2/2] power: supply: bq25980: Moves properties from battery node Ricardo Rivera-Matos
2021-02-10  8:23   ` Krzysztof Kozlowski
2021-02-10 19:51     ` [EXTERNAL] " Ricardo Rivera-Matos
2021-02-10  8:20 ` [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on Krzysztof Kozlowski
2021-02-10 19:50   ` [EXTERNAL] " Ricardo Rivera-Matos
2021-02-10 20:38     ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).