lkml.org 
[lkml]   [2021]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/2] power: supply: bq25980 Apply datasheet revision changes
Hi,

On Thu, Feb 11, 2021 at 08:36:03AM +0100, Krzysztof Kozlowski wrote:
> On Wed, Feb 10, 2021 at 04:56:45PM -0600, Ricardo Rivera-Matos wrote:
> > The latest datasheet revision for BQ25980, BQ25975, and BQ25960 changed
> >
> > various register step sizes and offset values.
> >
> > This patch changes the following header file
> >
> > values for POWER_SUPPLY_PROP_CURRENT_NOW,
> >
> > POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
> >
> > POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
> >
> > POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> >
> > POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT, and POWER_SUPPLY_PROP_VOLTAGE_NOW.
> >
> > Additionally, this patch adjusts bq25980_get_input_curr_lim(),
> >
> > bq25980_set_input_curr_lim(), bq25980_get_const_charge_curr(), and
> >
> > bq25980_set_const_charge_curr() to perform the get/set math correctly.
>
> Your formatting is so odd, it is not readable. Please open "git log" and
> try to write something similar to existing commits, e.g. without
> additional blank line between lines.

Ack. This is a paint to read.

>
> >
> > 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;
>
> Does not look like related to changing offsets of register values in
> header.

well looking at the change as a whole the problem is that each
chip has different offset/step size now. Arguably this is an
atomic change. I would have queued it (fixing the commit message
while applying), but the patch also does some unrelated changes:

- introduced usage of clamp instead of min()/max() checks
(which is great, but should be separate commit!)
- rename variables to something more sensible in
bq25980_get_adc_vbat (again a great change, but should be
separate commit)

Ricardo, please submit a new version with these changes splitted
out.

-- Sebastian
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2021-04-05 18:11    [W:0.040 / U:21.576 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site