Re: [PATCH 1/2] power: supply: Add support for RN5T618/RC5T619 charger and fuel gauge

[Sebastian Reichel]

Hi,

Driver looks mostly good.

On Sat, Aug 15, 2020 at 06:56:09PM +0200, Andreas Kemnade wrote:
> [...]
> +static int rn5t618_battery_current_now(struct rn5t618_power_info *info,
> +				       union power_supply_propval *val)
> +{
> +	u16 res;
> +	int ret;
> +
> +	ret = rn5t618_battery_read_doublereg(info, RN5T618_CC_AVEREG1, &res);
> +	if (ret)
> +		return ret;
> +
> +	val->intval = res;
> +	/* 2's complement */
> +	if (val->intval & (1 << 13))
> +		val->intval = val->intval - (1 << 14);
> +
> +	/* negate current to be positive when discharging */
> +	val->intval *= -1000;

mh, the sign is not documented (which should be fixed). At least
sbs-battery does it the other way around (negative current when
discharging, positive otherwise). Some drivers do not support
signed current and always report positive values (e.g. ACPI driver).

What did you use as reference for swapping the sign?

> +	return 0;
> +}
> [...]
> +static const struct power_supply_desc rn5t618_adp_desc = {
> +	.name                   = "rn5t618-adp",
> +	.type                   = POWER_SUPPLY_TYPE_MAINS,
> +	.properties             = rn5t618_usb_props,

wrong property array, works by accident since usb and adp property
lists are the same.

> +	.num_properties         = ARRAY_SIZE(rn5t618_adp_props),
> +	.get_property           = rn5t618_adp_get_property,
> +};
> [...]

-- Sebastian

Attachment: signature.asc
Description: PGP signature