From: Arnd Bergmann <[email protected]>
The argument to do_div() is a 32-bit integer, and it was read from a
32-bit register so there is no point in doing a 64-bit division on it.
On 32-bit arm, do_div() causes a compile-time warning here:
include/asm-generic/div64.h:238:22: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types]
238 | __rem = __div64_32(&(n), __base); \
| ^~~~
| |
| unsigned int *
drivers/power/supply/qcom_battmgr.c:1130:4: note: in expansion of macro 'do_div'
1130 | do_div(battmgr->status.percent, 100);
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/power/supply/qcom_battmgr.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
index ec31f887184f..de77df97b3a4 100644
--- a/drivers/power/supply/qcom_battmgr.c
+++ b/drivers/power/supply/qcom_battmgr.c
@@ -1126,8 +1126,7 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
battmgr->info.charge_type = le32_to_cpu(resp->intval.value);
break;
case BATT_CAPACITY:
- battmgr->status.percent = le32_to_cpu(resp->intval.value);
- do_div(battmgr->status.percent, 100);
+ battmgr->status.percent = le32_to_cpu(resp->intval.value) / 100;
break;
case BATT_VOLT_OCV:
battmgr->status.voltage_ocv = le32_to_cpu(resp->intval.value);
--
2.39.1
On 14.02.2023 14:20, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The argument to do_div() is a 32-bit integer, and it was read from a
> 32-bit register so there is no point in doing a 64-bit division on it.
>
> On 32-bit arm, do_div() causes a compile-time warning here:
>
> include/asm-generic/div64.h:238:22: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types]
> 238 | __rem = __div64_32(&(n), __base); \
> | ^~~~
> | |
> | unsigned int *
> drivers/power/supply/qcom_battmgr.c:1130:4: note: in expansion of macro 'do_div'
> 1130 | do_div(battmgr->status.percent, 100);
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
> drivers/power/supply/qcom_battmgr.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
> index ec31f887184f..de77df97b3a4 100644
> --- a/drivers/power/supply/qcom_battmgr.c
> +++ b/drivers/power/supply/qcom_battmgr.c
> @@ -1126,8 +1126,7 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
> battmgr->info.charge_type = le32_to_cpu(resp->intval.value);
> break;
> case BATT_CAPACITY:
> - battmgr->status.percent = le32_to_cpu(resp->intval.value);
> - do_div(battmgr->status.percent, 100);
> + battmgr->status.percent = le32_to_cpu(resp->intval.value) / 100;
> break;
> case BATT_VOLT_OCV:
> battmgr->status.voltage_ocv = le32_to_cpu(resp->intval.value);
Hi,
On Tue, Feb 14, 2023 at 02:36:03PM +0100, Konrad Dybcio wrote:
> On 14.02.2023 14:20, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > The argument to do_div() is a 32-bit integer, and it was read from a
> > 32-bit register so there is no point in doing a 64-bit division on it.
> >
> > On 32-bit arm, do_div() causes a compile-time warning here:
> >
> > include/asm-generic/div64.h:238:22: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types]
> > 238 | __rem = __div64_32(&(n), __base); \
> > | ^~~~
> > | |
> > | unsigned int *
> > drivers/power/supply/qcom_battmgr.c:1130:4: note: in expansion of macro 'do_div'
> > 1130 | do_div(battmgr->status.percent, 100);
> >
> > Signed-off-by: Arnd Bergmann <[email protected]>
> > ---
> Reviewed-by: Konrad Dybcio <[email protected]>
Needs to go through the Qualcomm tree:
Acked-by: Sebastian Reichel <[email protected]>
-- Sebastian
> > drivers/power/supply/qcom_battmgr.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
> > index ec31f887184f..de77df97b3a4 100644
> > --- a/drivers/power/supply/qcom_battmgr.c
> > +++ b/drivers/power/supply/qcom_battmgr.c
> > @@ -1126,8 +1126,7 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
> > battmgr->info.charge_type = le32_to_cpu(resp->intval.value);
> > break;
> > case BATT_CAPACITY:
> > - battmgr->status.percent = le32_to_cpu(resp->intval.value);
> > - do_div(battmgr->status.percent, 100);
> > + battmgr->status.percent = le32_to_cpu(resp->intval.value) / 100;
> > break;
> > case BATT_VOLT_OCV:
> > battmgr->status.voltage_ocv = le32_to_cpu(resp->intval.value);
On Tue, Feb 14, 2023 at 2:23 PM Arnd Bergmann <[email protected]> wrote:
> From: Arnd Bergmann <[email protected]>
>
> The argument to do_div() is a 32-bit integer, and it was read from a
> 32-bit register so there is no point in doing a 64-bit division on it.
>
> On 32-bit arm, do_div() causes a compile-time warning here:
>
> include/asm-generic/div64.h:238:22: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types]
> 238 | __rem = __div64_32(&(n), __base); \
> | ^~~~
> | |
> | unsigned int *
> drivers/power/supply/qcom_battmgr.c:1130:4: note: in expansion of macro 'do_div'
> 1130 | do_div(battmgr->status.percent, 100);
>
> Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Linus,
On Tue, Feb 14, 2023 at 02:20:42PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The argument to do_div() is a 32-bit integer, and it was read from a
> 32-bit register so there is no point in doing a 64-bit division on it.
>
> On 32-bit arm, do_div() causes a compile-time warning here:
>
> include/asm-generic/div64.h:238:22: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types]
> 238 | __rem = __div64_32(&(n), __base); \
> | ^~~~
> | |
> | unsigned int *
> drivers/power/supply/qcom_battmgr.c:1130:4: note: in expansion of macro 'do_div'
> 1130 | do_div(battmgr->status.percent, 100);
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/power/supply/qcom_battmgr.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
> index ec31f887184f..de77df97b3a4 100644
> --- a/drivers/power/supply/qcom_battmgr.c
> +++ b/drivers/power/supply/qcom_battmgr.c
> @@ -1126,8 +1126,7 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
> battmgr->info.charge_type = le32_to_cpu(resp->intval.value);
> break;
> case BATT_CAPACITY:
> - battmgr->status.percent = le32_to_cpu(resp->intval.value);
> - do_div(battmgr->status.percent, 100);
> + battmgr->status.percent = le32_to_cpu(resp->intval.value) / 100;
> break;
> case BATT_VOLT_OCV:
> battmgr->status.voltage_ocv = le32_to_cpu(resp->intval.value);
> --
> 2.39.1
>
Would you be able to take this patch directly? It seems obviously
correctTM, has an ack from Sebastian [1], and without it, 32-bit
allmodconfig builds are broken [2] (the other warning in that log has a
fix on the way to you soon).
[1]: https://lore.kernel.org/[email protected]/
[2]: https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/builds/2MPmxwvmQ7FdpiMhdQN2ZJhcoUP/build.log
Cheers,
Nathan
On Wed, Mar 1, 2023 at 10:16 AM Nathan Chancellor <[email protected]> wrote:
>
> Would you be able to take this patch directly? It seems obviously
> correctTM, has an ack from Sebastian [1], and without it, 32-bit
> allmodconfig builds are broken [2] (the other warning in that log has a
> fix on the way to you soon).
Ok, I've taken it directly.
However, the whole "seems obviously correct" is true in the sense that
it now doesn't complain (and doesn't overwrite an "int" with a 64-bit
value.
The actual code still looks odd. Is that return value for
BATT_CAPACITY truly in that odd "hundredths of percent" format, where
dividing it by 100 gives you whole percent?
Because "hundredths of percent" strikes me as a very odd interface.
Even for some firmware interface. I realize that anything is possible
with strange firmware, but still...
Linus
Hi,
On Wed, Mar 01, 2023 at 10:50:33AM -0800, Linus Torvalds wrote:
> On Wed, Mar 1, 2023 at 10:16 AM Nathan Chancellor <[email protected]> wrote:
> > Would you be able to take this patch directly? It seems obviously
> > correctTM, has an ack from Sebastian [1], and without it, 32-bit
> > allmodconfig builds are broken [2] (the other warning in that log has a
> > fix on the way to you soon).
>
> Ok, I've taken it directly.
>
> However, the whole "seems obviously correct" is true in the sense that
> it now doesn't complain (and doesn't overwrite an "int" with a 64-bit
> value.
>
> The actual code still looks odd. Is that return value for
> BATT_CAPACITY truly in that odd "hundredths of percent" format,
> where dividing it by 100 gives you whole percent?
>
> Because "hundredths of percent" strikes me as a very odd interface.
> Even for some firmware interface. I realize that anything is possible
> with strange firmware, but still...
I don't have the documentation for this Qualcomm interface, but I
remember somebody from Qualcomm asking for a power-supply property
exposing "hundredths of percent" to userspace some years ago (with
the rationale, that percent was not precise enough).
For reference: The upstream solution for that is exposing ENERGY_NOW
and ENERGY_FULL in µWh (or CHARGE_NOW + CHARGE_FULL in µAh depending
on the fuel-gauge's capabilities), which is even more precise.
-- Sebastian