From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yx1-f49.google.com (mail-yx1-f49.google.com [74.125.224.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 329723D34BB for ; Fri, 15 May 2026 22:22:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.224.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778883779; cv=none; b=iQK7MeCTR+k/DmAl3XQezSYok4xzVd7yL9olp3nYkU9f7jvFeYFy0S5wJ3I0zAR4z1W140ZLwUJtpMl7eZzukJvQSuoTc1nlYi8Q7EZDDSg2ieQ6WrVsGAhst19gZlDi1zwi9+WgROdbxByrvU0DSh8Q+4crDYkg86rrrHSEhps= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778883779; c=relaxed/simple; bh=rDH8B+QqgndpGE3qse8f/2bDTYp/j4uHy9m1Q2rSD20=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=GGcf+5WI+CVcsbQJMqQBtvyQZWnj3/zGbuC7GP7Q+h8A8/+nBAgiHnD8YIkf6ZTAphUWDSemslT+hvjSwRwO4iVOdvEqminWnqadvb4sdXlvsgOmgls7mEmw0cOYsxWGlZTcz6rFFXZ47RrhvXut8YTq7UNIAlRRqaZTFwt6wYs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=OA078xnY; arc=none smtp.client-ip=74.125.224.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="OA078xnY" Received: by mail-yx1-f49.google.com with SMTP id 956f58d0204a3-651bc83e74aso697804d50.2 for ; Fri, 15 May 2026 15:22:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1778883777; x=1779488577; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=5m7nm9/acQjAEPOHXxWuk1T8HUQRk5NC7/L7t4zgkds=; b=OA078xnY3CDSdzAvwGg+bjxKcVgDkkAGpYYnYGC3D++Pqb7HvX/XHleyqM9XGCLdL0 cBx09u9gaftVwsOOwpEjWPNlNlsi2weSQ2doV6AmnVWgLQ99DvuTaa399chxxMEiR8JV q3RX2Da2Tx8WJII4YLCKqGW7ezdF1iC1ssIDWriwXXVQIHybvMkhJBOV4Bja5uaUiuDY mVUUX+0nXGfYJMvjHfZyFPBuBzs36TM4ZW+OlbfaxhQ9JfjEd0tW8xcg2XPSUHG5Wwt6 ACXxc8W8AM0JnThXmMsf7a/thA3oSjuAPva2iKscDlD4K964eSjnyWILPshtg62ni8ZB q+7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778883777; x=1779488577; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=5m7nm9/acQjAEPOHXxWuk1T8HUQRk5NC7/L7t4zgkds=; b=rr037AwtaefqjbFMf/Vj0PBuSmGmoH3JPsV6SMrhmENZovPw8juzOxkXmeKLgQCPI5 X24c3sdP7IQShC37guojEt+QEyCucvpsSn9FYKrwer4avQRE9QXx340UC3VnsOHN0Gwj wkYxzRzRHgVBZS03TVbyvBZUYJaPX3lW1kn1rPIb1TjQRt4dQdg+m5U14oxKzgtYepzk YONkRzDXghBudxcQokk1hDEghH0SaeetIe1Z+Fg9ddy4ryHWbvWW7StI9EdbK/HGgDhO lCnT3BOiCtcpH2bmdbslREesoJDf7kRU4uDCfhcznSn9Uf89xJ7iXmEqA0Te/ugfxhdi X3nA== X-Forwarded-Encrypted: i=1; AFNElJ+sFMVcjk3MjhIN6QznlMoeacAA7nroJuUdWow68JBfPEos/odF/xuo9566zaCwH9rCjiuluPFtwBAZhao=@vger.kernel.org X-Gm-Message-State: AOJu0YwEfkGalt6DcIC9JaY7F7i/uQFQ3dSk5CeS6Ma4ObDctGQ6hcFK WV+FTuPZ+5Rnwo3470xZ4AufaDUTeeXbnVDsmkWX4SOi2gKhT+F3hCBHMbV4dAHD5w== X-Gm-Gg: Acq92OEmpZFZRGrfqHICXeFjz6VLMRSUer36lkJ/4QaVBoRj9Fc86kJJq3hixjZPJ+y kxT/Ld08X1T9JIp9OUp2r/k2QxFH8/LwYB3liLzm+jvNOC2GOcEsU5bPb159T/FgbmiQ3V46l7C rje43wlBUPdmDnUGwWWArpwzKNP+VM1Kn5NEYAI+MHx1+1FUeIjHcC5iru2WKwrzfyyJEW4vZgr 6obKT0X+gcLKsewGqfEf2XQpmjqVOdKWqvs7TNHrqsDXvcU+E+YAhQPE0zaEp6i/BlNhzErRB2M WGNXB3fIN9e+3Bx8CXEL13riI61GggZVFdkYRwo35VBb/atua3sTKvKN9UdWgzbNZgpQ8SY5T31 blvrWciTnH6JAJgANyTuc3ufKOoQmgyUaLdPyB0X/0LHmVLgKLB5YcZxPfV87eFCCkNuqszdjkl Tf/FM8vu1hUOMk3SjS2lMFOOKpD7IorEDCYfLnsDHuMLoVLjFDxOGJKvYsz92O6B6cT0DjkZqUm 9mP0NajGg== X-Received: by 2002:a05:690e:4403:b0:65c:2738:c687 with SMTP id 956f58d0204a3-65e2270ef58mr4744685d50.21.1778883776819; Fri, 15 May 2026 15:22:56 -0700 (PDT) Received: from ?IPV6:2600:1700:4570:89a0:9ee1:b971:dc61:3e74? ([2600:1700:4570:89a0:9ee1:b971:dc61:3e74]) by smtp.gmail.com with ESMTPSA id 956f58d0204a3-65e0dbcda4bsm3144803d50.18.2026.05.15.15.22.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 15 May 2026 15:22:56 -0700 (PDT) Message-ID: <14fa7e1b-89e8-498f-90ea-f82586c5d6b9@google.com> Date: Fri, 15 May 2026 15:22:54 -0700 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/3] power: supply: max17042: add handler for energy_now property To: Hans de Goede , Sebastian Reichel , Badhri Jagan Sridharan , Heikki Krogerus , Greg Kroah-Hartman , Krzysztof Kozlowski , Marek Szyprowski , Sebastian Krzyszkowiak , Purism Kernel Team Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, =?UTF-8?Q?Andr=C3=A9_Draszik?= , Tudor Ambarus , Peter Griffin , RD Babiera , Kyle Tso References: <20260515-batt-status-v1-0-fed6b7d8cea7@google.com> <20260515-batt-status-v1-3-fed6b7d8cea7@google.com> <17f49974-1f39-4b4c-8577-da33da7f1cc4@kernel.org> From: Amit Sunil Dhamne Content-Language: en-US In-Reply-To: <17f49974-1f39-4b4c-8577-da33da7f1cc4@kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hi Hans, Thanks for the review. On 5/15/26 12:25 AM, Hans de Goede wrote: > Hi, > > On 15-May-26 07:48, Amit Sunil Dhamne via B4 Relay wrote: >> From: Amit Sunil Dhamne >> >> Add handler to report power_supply_prop_energy_now so that users can get >> current SoC in uWH. Additionally, add helper functions to get avg_vcell >> and repcap values in uv and uah units respectively to avoid code >> duplication. >> >> Signed-off-by: Amit Sunil Dhamne > NACK for multiple reasons: > > 1. We don't want to do this in all drivers which only support > charge_now and not energy_now, instead the TCPM driver should > convert charge_now to energy_now when necessary itself so that > the TCPM code will work with all battery type power-supply drivers > not just those which provide energy_now. Got it. I will pivot and update the TCPM driver to fetch charge_now and a voltage property (voltage_avg) from the battery power supply, performing the energy_now calculation locally. > > 2. Having energy_now without energy_full is problematic and will > confuse userspace which prefers energy_* over charge_* since > userspace will now miss a reference value for full to report > a charging progress percentage. I also wonder how this works > on the TCPM side does the Battery Status response message not > have a full value / percentage ? Only the present state of charge of the battery (in units of 0.1Wh) is sent out to the port partner as part of Battery Status response message per USB PD spec. `energy_full` and `energy_full_design` are sent out as part of Battery Capacity response message. I plan to send out the patches to support Batt Caps soon. > > 3. IIRC userspace (upower) picks either energy_* or charge_* > values depending on which are present. I'm not sure if having > both will not confuse userspace. As mentioned in 2. having both > while one of them has an incomplete set of properties is sure > to confuse userspace. I see. I will drop this patch for now. Thanks, Amit > > Regards, > > Hans > > > > > >> --- >> drivers/power/supply/max17042_battery.c | 60 ++++++++++++++++++++++++++------- >> 1 file changed, 47 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c >> index 167fb3fb3732..e16eb6985b70 100644 >> --- a/drivers/power/supply/max17042_battery.c >> +++ b/drivers/power/supply/max17042_battery.c >> @@ -81,6 +81,7 @@ static enum power_supply_property max17042_battery_props[] = { >> POWER_SUPPLY_PROP_CHARGE_NOW, >> POWER_SUPPLY_PROP_CHARGE_COUNTER, >> POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT, >> + POWER_SUPPLY_PROP_ENERGY_NOW, >> POWER_SUPPLY_PROP_TEMP, >> POWER_SUPPLY_PROP_TEMP_ALERT_MIN, >> POWER_SUPPLY_PROP_TEMP_ALERT_MAX, >> @@ -95,6 +96,36 @@ static enum power_supply_property max17042_battery_props[] = { >> POWER_SUPPLY_PROP_CURRENT_AVG, >> }; >> >> +static int max17042_get_repcap_uah(struct max17042_chip *chip, u64 *rep_cap) >> +{ >> + u32 data; >> + int ret; >> + >> + ret = regmap_read(chip->regmap, MAX17042_RepCap, &data); >> + if (ret < 0) >> + return ret; >> + >> + *rep_cap = data * 5000000ll; >> + *rep_cap *= chip->task_period; >> + do_div(*rep_cap, MAX17042_DEFAULT_TASK_PERIOD); >> + do_div(*rep_cap, chip->pdata->r_sns); >> + >> + return 0; >> +} >> + >> +static int max17042_get_avgvcell_uv(struct max17042_chip *chip, u32 *vcell) >> +{ >> + int ret; >> + >> + ret = regmap_read(chip->regmap, MAX17042_AvgVCELL, vcell); >> + if (ret < 0) >> + return ret; >> + >> + *vcell = (*vcell * 625) / 8; >> + >> + return 0; >> +} >> + >> static int max17042_get_temperature(struct max17042_chip *chip, int *temp) >> { >> int ret; >> @@ -180,14 +211,12 @@ static int max17042_get_battery_health(struct max17042_chip *chip, int *health) >> int temp, vavg, vbatt, ret; >> u32 val; >> >> - ret = regmap_read(chip->regmap, MAX17042_AvgVCELL, &val); >> + ret = max17042_get_avgvcell_uv(chip, &val); >> if (ret < 0) >> goto health_error; >> >> - /* bits [0-3] unused */ >> - vavg = val * 625 / 8; >> /* Convert to millivolts */ >> - vavg /= 1000; >> + vavg = val / 1000; >> >> ret = regmap_read(chip->regmap, MAX17042_VCELL, &val); >> if (ret < 0) >> @@ -304,11 +333,10 @@ static int max17042_get_property(struct power_supply *psy, >> val->intval = data * 625 / 8; >> break; >> case POWER_SUPPLY_PROP_VOLTAGE_AVG: >> - ret = regmap_read(map, MAX17042_AvgVCELL, &data); >> + ret = max17042_get_avgvcell_uv(chip, &data); >> if (ret < 0) >> return ret; >> - >> - val->intval = data * 625 / 8; >> + val->intval = data; >> break; >> case POWER_SUPPLY_PROP_VOLTAGE_OCV: >> ret = regmap_read(map, MAX17042_OCVInternal, &data); >> @@ -350,14 +378,9 @@ static int max17042_get_property(struct power_supply *psy, >> val->intval = data64; >> break; >> case POWER_SUPPLY_PROP_CHARGE_NOW: >> - ret = regmap_read(map, MAX17042_RepCap, &data); >> + ret = max17042_get_repcap_uah(chip, &data64); >> if (ret < 0) >> return ret; >> - >> - data64 = data * 5000000ll; >> - data64 *= chip->task_period; >> - do_div(data64, MAX17042_DEFAULT_TASK_PERIOD); >> - do_div(data64, chip->pdata->r_sns); >> val->intval = data64; >> break; >> case POWER_SUPPLY_PROP_CHARGE_COUNTER: >> @@ -370,6 +393,17 @@ static int max17042_get_property(struct power_supply *psy, >> data64 = div_s64(data64, MAX17042_DEFAULT_TASK_PERIOD); >> val->intval = div_s64(data64, chip->pdata->r_sns); >> break; >> + case POWER_SUPPLY_PROP_ENERGY_NOW: >> + ret = max17042_get_repcap_uah(chip, &data64); >> + if (ret < 0) >> + return ret; >> + >> + ret = max17042_get_avgvcell_uv(chip, &data); >> + if (ret < 0) >> + return ret; >> + >> + val->intval = data64 * data / 1000000; >> + break; >> case POWER_SUPPLY_PROP_TEMP: >> ret = max17042_get_temperature(chip, &val->intval); >> if (ret < 0) >>