Re: [EXTERNAL] Re: [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on
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 > > 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
Re: [EXTERNAL] Re: [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on
Krzysztof, On 2/10/21 2:20 AM, Krzysztof Kozlowski wrote: On Wed, 10 Feb 2021 at 00:52, Ricardo Rivera-Matos 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
Re: [PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on
On Wed, 10 Feb 2021 at 00:52, Ricardo Rivera-Matos 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
[PATCH 1/2] power: supply: bq25980: Applies multiple fixes brought on
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
---
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, 1, 5,
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, _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 /
