* [PATCH] power: supply: qcom_battmgr: Report battery capacity @ 2025-05-27 12:18 Kornel Dulęba 2025-05-27 19:11 ` kernel test robot 2025-05-27 19:34 ` Dmitry Baryshkov 0 siblings, 2 replies; 5+ messages in thread From: Kornel Dulęba @ 2025-05-27 12:18 UTC (permalink / raw) To: Sebastian Reichel, Bjorn Andersson Cc: linux-arm-msm, linux-pm, linux-kernel, chromeos-krk-upstreaming, Kornel Dulęba 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@google.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; + } 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] power: supply: qcom_battmgr: Report battery capacity 2025-05-27 12:18 [PATCH] power: supply: qcom_battmgr: Report battery capacity Kornel Dulęba @ 2025-05-27 19:11 ` kernel test robot 2025-05-27 19:34 ` Dmitry Baryshkov 1 sibling, 0 replies; 5+ messages in thread From: kernel test robot @ 2025-05-27 19:11 UTC (permalink / raw) To: Kornel Dulęba; +Cc: llvm, oe-kbuild-all Hi Kornel, kernel test robot noticed the following build errors: [auto build test ERROR on sre-power-supply/for-next] [also build test ERROR on linus/master v6.15 next-20250527] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Kornel-Dul-ba/power-supply-qcom_battmgr-Report-battery-capacity/20250527-202035 base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next patch link: https://lore.kernel.org/r/20250527121807.3597061-1-korneld%40google.com patch subject: [PATCH] power: supply: qcom_battmgr: Report battery capacity config: i386-buildonly-randconfig-002-20250527 (https://download.01.org/0day-ci/archive/20250528/202505280344.GjzOItSS-lkp@intel.com/config) compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250528/202505280344.GjzOItSS-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202505280344.GjzOItSS-lkp@intel.com/ All errors (new ones prefixed by >>, old ones prefixed by <<): WARNING: modpost: missing MODULE_DESCRIPTION() in lib/zlib_inflate/zlib_inflate.o >> ERROR: modpost: "__udivdi3" [drivers/power/supply/qcom_battmgr.ko] undefined! -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] power: supply: qcom_battmgr: Report battery capacity 2025-05-27 12:18 [PATCH] power: supply: qcom_battmgr: Report battery capacity Kornel Dulęba 2025-05-27 19:11 ` kernel test robot @ 2025-05-27 19:34 ` Dmitry Baryshkov 2025-05-28 9:57 ` Kornel Dulęba 1 sibling, 1 reply; 5+ messages in thread From: Dmitry Baryshkov @ 2025-05-27 19:34 UTC (permalink / raw) To: Kornel Dulęba Cc: Sebastian Reichel, Bjorn Andersson, linux-arm-msm, linux-pm, linux-kernel, chromeos-krk-upstreaming 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@google.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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] power: supply: qcom_battmgr: Report battery capacity 2025-05-27 19:34 ` Dmitry Baryshkov @ 2025-05-28 9:57 ` Kornel Dulęba 2025-05-28 11:07 ` Dmitry Baryshkov 0 siblings, 1 reply; 5+ messages in thread From: Kornel Dulęba @ 2025-05-28 9:57 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Sebastian Reichel, Bjorn Andersson, linux-arm-msm, linux-pm, linux-kernel, chromeos-krk-upstreaming On Tue, May 27, 2025 at 9:34 PM 'Dmitry Baryshkov' via chromeos-krk-upstreaming <chromeos-krk-upstreaming@google.com> wrote: > > 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@google.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? Good idea, but I don't think mult_frac() helps in cases where the dividend is smaller than the divider. Let's look at the sources: #define mult_frac(x, n, d) \ (...) typeof(x_) q = x_ / d_; \ typeof(x_) r = x_ % d_; \ q * n_ + r * n_ / d_; \ Since in our case x_ < d_, q = 0 and r = x_ then r * n_ will still result in an overflow. Unfortunately, the cast-and-divide approach won't work either. I received an email from a kernel test robot saying that this patch breaks a 32-bit only build. (">> ERROR: modpost: "__udivdi3" [drivers/power/supply/qcom_battmgr.ko] undefined!") See https://lore.kernel.org/oe-kbuild-all/202505280344.GjzOItSS-lkp@intel.com/ for details. I suppose I could just use a do_div with a temporary variable to work around that. I noticed that all data read from FW is multiplied by 1000, so I leveraged that instead: battmgr->status.percent = (100 * le32_to_cpu(resp->status.capacity)) / (battmgr->info.last_full_capacity / 1000); Any thoughts? > > > + } 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 > > -- > You received this message because you are subscribed to the Google Groups "chromeos-krk-upstreaming" group. > To unsubscribe from this group and stop receiving emails from it, send an email to chromeos-krk-upstreaming+unsubscribe@google.com. > To view this discussion visit https://groups.google.com/a/google.com/d/msgid/chromeos-krk-upstreaming/oa5okg7i2s6s7pxm5tn6nnanazze5lnnre4vnwrhopn5s5hsil%40vhh22j6b5cvq. > For more options, visit https://groups.google.com/a/google.com/d/optout. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] power: supply: qcom_battmgr: Report battery capacity 2025-05-28 9:57 ` Kornel Dulęba @ 2025-05-28 11:07 ` Dmitry Baryshkov 0 siblings, 0 replies; 5+ messages in thread From: Dmitry Baryshkov @ 2025-05-28 11:07 UTC (permalink / raw) To: Kornel Dulęba Cc: Sebastian Reichel, Bjorn Andersson, linux-arm-msm, linux-pm, linux-kernel, chromeos-krk-upstreaming On Wed, May 28, 2025 at 11:57:32AM +0200, Kornel Dulęba wrote: > On Tue, May 27, 2025 at 9:34 PM 'Dmitry Baryshkov' via > chromeos-krk-upstreaming <chromeos-krk-upstreaming@google.com> wrote: > > > > 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@google.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? > > Good idea, but I don't think mult_frac() helps in cases where the > dividend is smaller than the divider. Let's look at the sources: > #define mult_frac(x, n, d) \ > (...) > typeof(x_) q = x_ / d_; \ > typeof(x_) r = x_ % d_; \ > q * n_ + r * n_ / d_; \ > > Since in our case x_ < d_, q = 0 and r = x_ then r * n_ will still > result in an overflow. > > Unfortunately, the cast-and-divide approach won't work either. I > received an email from a kernel test robot saying that this patch > breaks a 32-bit only build. (">> ERROR: modpost: "__udivdi3" > [drivers/power/supply/qcom_battmgr.ko] undefined!") See > https://lore.kernel.org/oe-kbuild-all/202505280344.GjzOItSS-lkp@intel.com/ > for details. > > I suppose I could just use a do_div with a temporary variable to work > around that. I noticed that all data read from FW is multiplied by > 1000, so I leveraged that instead: > battmgr->status.percent = > (100 * le32_to_cpu(resp->status.capacity)) / > (battmgr->info.last_full_capacity / 1000); > > Any thoughts? LGTM. > > > > > > + } 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 > > > > -- > > You received this message because you are subscribed to the Google Groups "chromeos-krk-upstreaming" group. > > To unsubscribe from this group and stop receiving emails from it, send an email to chromeos-krk-upstreaming+unsubscribe@google.com. > > To view this discussion visit https://groups.google.com/a/google.com/d/msgid/chromeos-krk-upstreaming/oa5okg7i2s6s7pxm5tn6nnanazze5lnnre4vnwrhopn5s5hsil%40vhh22j6b5cvq. > > For more options, visit https://groups.google.com/a/google.com/d/optout. -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-28 11:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-27 12:18 [PATCH] power: supply: qcom_battmgr: Report battery capacity Kornel Dulęba 2025-05-27 19:11 ` kernel test robot 2025-05-27 19:34 ` Dmitry Baryshkov 2025-05-28 9:57 ` Kornel Dulęba 2025-05-28 11:07 ` Dmitry Baryshkov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.