Re: [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family

[Sebastian Reichel]

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@xxxxxx>
> ---

[...]

> +/* 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: signature.asc
Description: PGP signature