power: reset: ltc2952: Fix use of floating point literals

PATCH In review Nathan Chancellor 2 participants 3 messages Nov 5, 2021 – Dec 31, 2021 System reset/shutdown drivers

Share & tools

Nathan Chancellor

author

2021-11-05 15:23 UTC #0 lore raw

PATCH v2 1/1 power: reset: ltc2952: Fix use of floating point literals

A new commit in LLVM causes an error on the use of 'long double' when '-mno-x87' is used, which the kernel does through an alias, '-mno-80387' (see the LLVM commit below for more details around why it does this).

drivers/power/reset/ltc2952-poweroff.c:162:28: error: expression requires 'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it

        data->wde_interval = 300L * 1E6L;
                                  ^
drivers/power/reset/ltc2952-poweroff.c:162:21: error: expression requires  'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it
        data->wde_interval = 300L * 1E6L;
                           ^
drivers/power/reset/ltc2952-poweroff.c:163:41: error: expression requires  'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it
        data->trigger_delay = ktime_set(2, 500L*1E6L);
                                               ^

3 errors generated.

This happens due to the use of a 'long double' literal. The 'E6' part of '1E6L' causes the literal to be a 'double' then the 'L' suffix promotes it to 'long double'.

There is no visible reason for floating point values in this driver, as the values are only assigned to integer types. Use NSEC_PER_MSEC, which is the same integer value as '1E6L', to avoid changing functionality but fix the error.

Fixes 6647156c00cc ("power: reset: add LTC2952 poweroff driver")

Link https://github.com/ClangBuiltLinux/linux/issues/1497

Link https://github.com/llvm/llvm-project/commit/a8083d42b1c346e21623a1d36d1f0cadd7801d83

Signed-off-by Nathan Chancellor <[email protected]>

Reviewed-by Nick Desaulniers <[email protected]>

---

v1 -> v2: https://lore.kernel.org/r/[email protected]/

* A separate review pointed out that NSEC_PER_MSEC is a better choice than USEC_PER_SEC because ktime_t is nanoseconds and the few functions that take these values work in nanoseconds. The value is the same but the documentation is better.

* Pick up Nick's review tag.

 drivers/power/reset/ltc2952-poweroff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/power/reset/ltc2952-poweroff.c +2 -2

index fbb344353fe4..65d9528cc989 100644
@@ -159,8 +159,8 @@ static void ltc2952_poweroff_kill(void)
 
 static void ltc2952_poweroff_default(struct ltc2952_poweroff *data)
 {
-	data->wde_interval = 300L * 1E6L;
-	data->trigger_delay = ktime_set(2, 500L*1E6L);
+	data->wde_interval = 300L * NSEC_PER_MSEC;
+	data->trigger_delay = ktime_set(2, 500L * NSEC_PER_MSEC);
 
 	hrtimer_init(&data->timer_trigger, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	data->timer_trigger.function = ltc2952_poweroff_timer_trigger;

base-commit: d4439a1189f93d0ac1eaf0197db8e6b3e197d5c7

Signature · 1 line

2.34.0.rc0

Sebastian Reichel

2021-11-16 14:26 UTC #1 lore raw

Hi,

Quoted 25 lines · Nathan Chancellor

A new commit in LLVM causes an error on the use of 'long double' when '-mno-x87' is used, which the kernel does through an alias, '-mno-80387' (see the LLVM commit below for more details around why it does this).
drivers/power/reset/ltc2952-poweroff.c:162:28: error: expression requires 'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it
        data->wde_interval = 300L * 1E6L;
                                  ^
drivers/power/reset/ltc2952-poweroff.c:162:21: error: expression requires  'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it
        data->wde_interval = 300L * 1E6L;
                           ^
drivers/power/reset/ltc2952-poweroff.c:163:41: error: expression requires  'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it
        data->trigger_delay = ktime_set(2, 500L*1E6L);
                                               ^
3 errors generated.
This happens due to the use of a 'long double' literal. The 'E6' part of '1E6L' causes the literal to be a 'double' then the 'L' suffix promotes it to 'long double'.
There is no visible reason for floating point values in this driver, as the values are only assigned to integer types. Use NSEC_PER_MSEC, which is the same integer value as '1E6L', to avoid changing functionality but fix the error.

Quoted 6 lines

Fixes 6647156c00cc ("power: reset: add LTC2952 poweroff driver")
Link https://github.com/ClangBuiltLinux/linux/issues/1497
Link https://github.com/llvm/llvm-project/commit/a8083d42b1c346e21623a1d36d1f0cadd7801d83
Signed-off-by Nathan Chancellor <[email protected]>
Reviewed-by Nick Desaulniers <[email protected]>
---

Thanks, queued to power-supply's fixes branch.

-- Sebastian

Quoted 33 lines

v1 -> v2: https://lore.kernel.org/r/[email protected]/
* A separate review pointed out that NSEC_PER_MSEC is a better choice than USEC_PER_SEC because ktime_t is nanoseconds and the few functions that take these values work in nanoseconds. The value is the same but the documentation is better.
* Pick up Nick's review tag.
 drivers/power/reset/ltc2952-poweroff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/power/reset/ltc2952-poweroff.c +2 -2
index fbb344353fe4..65d9528cc989 100644
@@ -159,8 +159,8 @@ static void ltc2952_poweroff_kill(void)
 
 static void ltc2952_poweroff_default(struct ltc2952_poweroff *data)
 {
-	data->wde_interval = 300L * 1E6L;
-	data->trigger_delay = ktime_set(2, 500L*1E6L);
+	data->wde_interval = 300L * NSEC_PER_MSEC;
+	data->trigger_delay = ktime_set(2, 500L * NSEC_PER_MSEC);
 
 	hrtimer_init(&data->timer_trigger, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	data->timer_trigger.function = ltc2952_poweroff_timer_trigger;
base-commit: d4439a1189f93d0ac1eaf0197db8e6b3e197d5c7
Signature · 1 line
2.34.0.rc0

Nathan Chancellor

author

2021-12-31 22:02 UTC #2 lore raw

Hi Sebastian,

Quoted 28 lines · Sebastian Reichel

Hi,
Quoted 25 lines · Nathan Chancellor
A new commit in LLVM causes an error on the use of 'long double' when '-mno-x87' is used, which the kernel does through an alias, '-mno-80387' (see the LLVM commit below for more details around why it does this).
drivers/power/reset/ltc2952-poweroff.c:162:28: error: expression requires 'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it
        data->wde_interval = 300L * 1E6L;
                                  ^
drivers/power/reset/ltc2952-poweroff.c:162:21: error: expression requires  'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it
        data->wde_interval = 300L * 1E6L;
                           ^
drivers/power/reset/ltc2952-poweroff.c:163:41: error: expression requires  'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it
        data->trigger_delay = ktime_set(2, 500L*1E6L);
                                               ^
3 errors generated.
This happens due to the use of a 'long double' literal. The 'E6' part of '1E6L' causes the literal to be a 'double' then the 'L' suffix promotes it to 'long double'.
There is no visible reason for floating point values in this driver, as the values are only assigned to integer types. Use NSEC_PER_MSEC, which is the same integer value as '1E6L', to avoid changing functionality but fix the error.

Quoted 8 lines

Quoted 6 lines
Fixes 6647156c00cc ("power: reset: add LTC2952 poweroff driver")
Link https://github.com/ClangBuiltLinux/linux/issues/1497
Link https://github.com/llvm/llvm-project/commit/a8083d42b1c346e21623a1d36d1f0cadd7801d83
Signed-off-by Nathan Chancellor <[email protected]>
Reviewed-by Nick Desaulniers <[email protected]>
---
Thanks, queued to power-supply's fixes branch.

Is there a timeline for getting this to Linus, since it has been a month and a half now since this was applied? It breaks several x86_64 configs with tip of tree clang, which harms our testing of the upcoming release, and other people are now tripping over this and reporting it on the LLVM issue tracker:

https://github.com/llvm/llvm-project/issues/52924

Also, I noticed that your fixes branch is not flowing into linux-next, meaning this is not fixed there either. Would it be possible for you to ask Stephen Rothwell to add it?

Cheers,
Nathan

Quoted 33 lines

Quoted 33 lines
v1 -> v2: https://lore.kernel.org/r/[email protected]/
* A separate review pointed out that NSEC_PER_MSEC is a better choice than USEC_PER_SEC because ktime_t is nanoseconds and the few functions that take these values work in nanoseconds. The value is the same but the documentation is better.
* Pick up Nick's review tag.
 drivers/power/reset/ltc2952-poweroff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/power/reset/ltc2952-poweroff.c +2 -2
index fbb344353fe4..65d9528cc989 100644
@@ -159,8 +159,8 @@ static void ltc2952_poweroff_kill(void)
 
 static void ltc2952_poweroff_default(struct ltc2952_poweroff *data)
 {
-	data->wde_interval = 300L * 1E6L;
-	data->trigger_delay = ktime_set(2, 500L*1E6L);
+	data->wde_interval = 300L * NSEC_PER_MSEC;
+	data->trigger_delay = ktime_set(2, 500L * NSEC_PER_MSEC);
 
 	hrtimer_init(&data->timer_trigger, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	data->timer_trigger.function = ltc2952_poweroff_timer_trigger;
base-commit: d4439a1189f93d0ac1eaf0197db8e6b3e197d5c7
Signature · 1 line
2.34.0.rc0