Re: [PATCH 10/10] power: supply: bq25890: Add new linux,iinlim-percentage property

From: Marek Vasut <marex@denx.de>
To: Hans de Goede <hdegoede@redhat.com>, Sebastian Reichel <sre@kernel.org>
Cc: linux-pm@vger.kernel.org
Subject: Re: [PATCH 10/10] power: supply: bq25890: Add new linux,iinlim-percentage property
Date: Sun, 27 Nov 2022 22:34:58 +0100	[thread overview]
Message-ID: <d2f2fe8f-9463-d496-dc4f-e3f27dd84526@denx.de> (raw)
In-Reply-To: <20221127180233.103678-11-hdegoede@redhat.com>

On 11/27/22 19:02, Hans de Goede wrote:
> Some devices, such as the Lenovo Yoga Tab 3 Pro (YT3-X90F) have
> multiple batteries with a separate bq25890 charger for each battery.
> 
> This requires the maximum current the external power-supply can deliver
> to be divided over the chargers. The Android vendor kernel shipped
> on the YT3-X90F divides this current with a 40/60 percent split so that
> batteries are done charging at approx. the same time if both were fully
> empty at the start.
> 
> Add support for a new "linux,iinlim-percentage" percentage property which
> can be set to indicate that a bq25890 charger should only use that
> percentage of the external power-supply's maximum current.
> 
> So far this new property is only used on x86/ACPI (non devicetree) devs,
> IOW it is not used in actual devicetree files. The devicetree-bindings
> maintainers have requested properties like these to not be added to the
> devicetree-bindings, so the new property is deliberately not added
> to the existing devicetree-bindings.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/power/supply/bq25890_charger.c | 24 +++++++++++++++++++++---
>   1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index b0d07ff24ace..2bd7721b969f 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -126,6 +126,7 @@ struct bq25890_device {
>   	bool read_back_init_data;
>   	bool force_hiz;
>   	u32 pump_express_vbus_max;
> +	u32 iinlim_percentage;

If this is percentage, u8 should be enough, right ?

>   	enum bq25890_chip_version chip_version;
>   	struct bq25890_init_data init_data;
>   	struct bq25890_state state;
> @@ -727,6 +728,18 @@ static int bq25890_power_supply_property_is_writeable(struct power_supply *psy,
>   	}
>   }
>   
> +/*
> + * If there are multiple chargers the maximum current the external power-supply
> + * can deliver needs to be divided over the chargers. This is done according
> + * to the bq->iinlim_percentage setting.
> + */
> +static int bq25890_charger_get_scaled_iinlim_regval(struct bq25890_device *bq,
> +						    int iinlim_ua)
> +{
> +	iinlim_ua = iinlim_ua * bq->iinlim_percentage / 100;

Can this ever add up to value above 100 ?
Should this use some clamp() ?

> +	return bq25890_find_idx(iinlim_ua, TBL_IINLIM);
> +}
> +
>   /* On the BQ25892 try to get charger-type info from our supplier */
>   static void bq25890_charger_external_power_changed(struct power_supply *psy)
>   {

[...]