linux-kernel - Re: [PATCH] power: supply: qcom_battmgr: Report battery capacity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <oa5okg7i2s6s7pxm5tn6nnanazze5lnnre4vnwrhopn5s5hsil@vhh22j6b5cvq>
Date: Tue, 27 May 2025 22:34:09 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Kornel Dulęba <korneld@...gle.com>
Cc: Sebastian Reichel <sre@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
        linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, chromeos-krk-upstreaming@...gle.com
Subject: Re: [PATCH] power: supply: qcom_battmgr: Report battery capacity

On Tue, May 27, 2025 at 12:18:07PM +0000, Kornel Dulęba wrote:
> Battery charge can be reported in several different ways. One of them is
> is charge percentage referred to as POWER_SUPPLY_PROP_CAPACITY in the
> power supply API. Currently the driver reports the capacity in this way
> on SM8350, but not on the newer variants referred to as SC8280XP in the
> driver. Although this is not a bug in itself, not reporting the
> percentage can confuse some userspace consumers.
> Mimic what is done in the ACPI driver (drivers/acpi/battery.c) and
> calculate the percentage capacity by dividing the current charge value
> by the full charge. Both values are expressed in either uWh, or
> in uAh.
> 
> Signed-off-by: Kornel Dulęba <korneld@...gle.com>
> ---
>  drivers/power/supply/qcom_battmgr.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
> index fe27676fbc7c..5ed5452ab51c 100644
> --- a/drivers/power/supply/qcom_battmgr.c
> +++ b/drivers/power/supply/qcom_battmgr.c
> @@ -577,6 +577,8 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
>  		val->intval = battmgr->status.capacity;
>  		break;
>  	case POWER_SUPPLY_PROP_CAPACITY:
> +		if (battmgr->status.percent == (unsigned int)-1)
> +			return -ENODATA;
>  		val->intval = battmgr->status.percent;
>  		break;
>  	case POWER_SUPPLY_PROP_TEMP:
> @@ -617,6 +619,7 @@ static const enum power_supply_property sc8280xp_bat_props[] = {
>  	POWER_SUPPLY_PROP_STATUS,
>  	POWER_SUPPLY_PROP_PRESENT,
>  	POWER_SUPPLY_PROP_TECHNOLOGY,
> +	POWER_SUPPLY_PROP_CAPACITY,
>  	POWER_SUPPLY_PROP_CYCLE_COUNT,
>  	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
>  	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -1063,6 +1066,21 @@ static void qcom_battmgr_sc8280xp_callback(struct qcom_battmgr *battmgr,
>  		battmgr->ac.online = source == BATTMGR_CHARGING_SOURCE_AC;
>  		battmgr->usb.online = source == BATTMGR_CHARGING_SOURCE_USB;
>  		battmgr->wireless.online = source == BATTMGR_CHARGING_SOURCE_WIRELESS;
> +		if (battmgr->info.last_full_capacity != 0) {
> +			/*
> +			 * 100 * battmgr->status.capacity can overflow a 32bit
> +			 * unsigned integer. Do a temporary cast to avoid that.
> +			 */
> +			battmgr->status.percent =
> +				(uint64_t)100 * battmgr->status.capacity /
> +				battmgr->info.last_full_capacity;

Can you use mult_frac(), preventing the overflow?

> +		} else {
> +			/*
> +			 * Let the sysfs handler know no data is available at
> +			 * this time.
> +			 */
> +			battmgr->status.percent = (unsigned int)-1;
> +		}
>  		break;
>  	case BATTMGR_BAT_DISCHARGE_TIME:
>  		battmgr->status.discharge_time = le32_to_cpu(resp->time);
> -- 
> 2.49.0.1151.ga128411c76-goog
> 

-- 
With best wishes
Dmitry