All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Reichel <sre@kernel.org>
To: Dan Murphy <dmurphy@ti.com>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family
Date: Sun, 20 Oct 2019 14:43:35 +0200	[thread overview]
Message-ID: <20191020124335.gssoiyjmdtpygwqy@earth.universe> (raw)
In-Reply-To: <20190930143137.21624-3-dmurphy@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1813 bytes --]

Hi,

Here are some more notes.

On Mon, Sep 30, 2019 at 09:31:37AM -0500, Dan Murphy wrote:
> Introduce the bq25150 and bq25155 supply chargers.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---

[...]

> +/* initial field values, converted to register values */
> +struct bq2515x_init_data {
> +	u8 ichg;	/* charge current		*/
> +	u8 vreg;	/* regulation voltage		*/

The following 4 entries seem to be unused.

> +	u8 iterm;	/* termination current		*/
> +	u8 iprechg;	/* precharge current		*/
> +	u8 sysvmin;	/* minimum system voltage limit */
> +	u8 ilim;	/* ILIM current contol		*/
> +};

[...]

> +static int get_const_charge_current(struct bq2515x_device *bq2515x)
> +{
> +	int ret;
> +	int intval;
> +	int iin_msb;
> +	int iin_lsb;
> +	u16 ichg_measurement;
> +	int ilim_val, ichg_multiplier;
> +
> +	if (!bq2515x_is_ps_online(bq2515x))
> +		return -ENODATA;
> +
> +	ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_IIN_M, &iin_msb);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_IIN_L, &iin_lsb);
> +	if (ret < 0)
> +		return ret;
> +
> +	ichg_measurement = (iin_msb << 8) | iin_lsb;
> +	ret = regmap_read(bq2515x->regmap, BQ2515X_ILIMCTRL, &ilim_val);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ilim_val >= BQ2515X_ILIM_150MA)
> +		ichg_multiplier = 350;
> +	else
> +		ichg_multiplier = 750;
> +
> +	intval = (ichg_measurement * 100 / BQ2515X_DIVISOR) * ichg_multiplier;
> +	return intval / 100;
> +}
> [...]

I just checked the calculation and you return mA. This is incorrect,
since the power-supply ABI uses µA. Please also re-check the other
calculations. You can find some information about the power-supply
subsystem here:

Documentation/power/power_supply_class.rst

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2019-10-20 12:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-30 14:31 [PATCH 1/3] power_supply: Add additional health properties to the header Dan Murphy
2019-09-30 14:31 ` [PATCH 2/3] dt: power: Add the bq2515x family dt bindings Dan Murphy
2019-10-20 12:30   ` Sebastian Reichel
2019-10-21 19:36     ` Dan Murphy
2019-09-30 14:31 ` [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family Dan Murphy
2019-10-20 12:15   ` Sebastian Reichel
2019-10-23 19:02     ` Dan Murphy
2019-10-20 12:43   ` Sebastian Reichel [this message]
2019-10-20 12:25 ` [PATCH 1/3] power_supply: Add additional health properties to the header Sebastian Reichel
2019-10-21 19:35   ` Dan Murphy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191020124335.gssoiyjmdtpygwqy@earth.universe \
    --to=sre@kernel.org \
    --cc=dmurphy@ti.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.