Skip to contentSkip to conversation

usb: dwc2: hcd_queue: Fix use of floating point literal

PATCH Superseded Nathan Chancellor 3 participants 6 messages Nov 4, 2021 – Nov 5, 2021

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 usb: dwc2: hcd_queue: Fix use of floating point literal

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/usb/dwc2/hcd_queue.c:1744:25: error: expression requires 'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it

                         delay = ktime_set(0, DWC2_RETRY_WAIT_DELAY);
                                             ^
 drivers/usb/dwc2/hcd_queue.c:62:34: note: expanded from macro 'DWC2_RETRY_WAIT_DELAY'
 #define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
                                 ^

1 error 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 a floating point value in this driver, as the value is only used as a parameter to a function that expects an integer type. Use USEC_PER_SEC, which is the same integer value as '1E6L', to avoid changing functionality but fix the error.

---
 drivers/usb/dwc2/hcd_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/dwc2/hcd_queue.c +1 -1
index 89a788326c56..bdf1927e1be1 100644
@@ -59,7 +59,7 @@
 #define DWC2_UNRESERVE_DELAY (msecs_to_jiffies(5))
 
 /* If we get a NAK, wait this long before retrying */
-#define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
+#define DWC2_RETRY_WAIT_DELAY (1 * USEC_PER_SEC)
 
 /**
  * dwc2_periodic_channel_available() - Checks that a channel is available for a

base-commit: d4439a1189f93d0ac1eaf0197db8e6b3e197d5c7

Signature · 1 line
2.34.0.rc0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 23 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/usb/dwc2/hcd_queue.c:1744:25: error: expression requires 'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it

                         delay = ktime_set(0, DWC2_RETRY_WAIT_DELAY);
                                             ^
 drivers/usb/dwc2/hcd_queue.c:62:34: note: expanded from macro 'DWC2_RETRY_WAIT_DELAY'
 #define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
                                 ^

1 error 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 a floating point value in this driver, as the value is only used as a parameter to a function that expects an integer type. Use USEC_PER_SEC, which is the same integer value as '1E6L', to avoid changing functionality but fix the error.

Quoted 4 lines

Thanks for the patch!

Quoted 22 lines
---
 drivers/usb/dwc2/hcd_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/dwc2/hcd_queue.c +1 -1
index 89a788326c56..bdf1927e1be1 100644
@@ -59,7 +59,7 @@
 #define DWC2_UNRESERVE_DELAY (msecs_to_jiffies(5))
 
 /* If we get a NAK, wait this long before retrying */
-#define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
+#define DWC2_RETRY_WAIT_DELAY (1 * USEC_PER_SEC)
 
 /**
  * dwc2_periodic_channel_available() - Checks that a channel is available for a

base-commit: d4439a1189f93d0ac1eaf0197db8e6b3e197d5c7 --
2.34.0.rc0

Signature · 2 lines
Thanks,
~Nick Desaulniers
#2 lore raw
Quoted 25 lines · Nick Desaulniers
Quoted 23 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/usb/dwc2/hcd_queue.c:1744:25: error: expression requires 'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it

                         delay = ktime_set(0, DWC2_RETRY_WAIT_DELAY);
                                             ^
 drivers/usb/dwc2/hcd_queue.c:62:34: note: expanded from macro 'DWC2_RETRY_WAIT_DELAY'
 #define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
                                 ^

1 error 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 a floating point value in this driver, as the value is only used as a parameter to a function that expects an integer type. Use USEC_PER_SEC, which is the same integer value as '1E6L', to avoid changing functionality but fix the error.

Quoted 7 lines
Quoted 4 lines

Thanks for the patch!

hmm...should we explicitly #include <vdso/time.h> for this declaration? It may work due to transitive includes, but that's generally considered brittle should those transitive includes change in the future.

Quoted 28 lines
Quoted 19 lines
---
 drivers/usb/dwc2/hcd_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/dwc2/hcd_queue.c +1 -1
index 89a788326c56..bdf1927e1be1 100644
@@ -59,7 +59,7 @@
 #define DWC2_UNRESERVE_DELAY (msecs_to_jiffies(5))
 
 /* If we get a NAK, wait this long before retrying */
-#define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
+#define DWC2_RETRY_WAIT_DELAY (1 * USEC_PER_SEC)
 
 /**
  * dwc2_periodic_channel_available() - Checks that a channel is available for a
Quoted 4 lines

base-commit: d4439a1189f93d0ac1eaf0197db8e6b3e197d5c7 --
2.34.0.rc0

--
Thanks,
~Nick Desaulniers

Signature · 2 lines
Thanks,
~Nick Desaulniers
#3 lore raw
Quoted 31 lines · Nick Desaulniers
Quoted 30 lines · Nick Desaulniers
Quoted 27 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/usb/dwc2/hcd_queue.c:1744:25: error: expression requires 'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it

                         delay = ktime_set(0, DWC2_RETRY_WAIT_DELAY);
                                             ^
 drivers/usb/dwc2/hcd_queue.c:62:34: note: expanded from macro 'DWC2_RETRY_WAIT_DELAY'
 #define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
                                 ^

1 error 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 a floating point value in this driver, as the value is only used as a parameter to a function that expects an integer type. Use USEC_PER_SEC, which is the same integer value as '1E6L', to avoid changing functionality but fix the error.

Quoted 7 lines
Quoted 2 lines

Thanks for the patch!

hmm...should we explicitly #include <vdso/time.h> for this declaration? It may work due to transitive includes, but that's generally considered brittle should those transitive includes change in the future.

This file uses ktime_t, which comes from include/linux/ktime.h, which eventually includes include/vdso/time64.h. include/vdso/time64.h is only included in three header files and no drivers so I do not think this is that brittle. I am happy to change that if the maintainers feel it is worth the extra include.

Quoted 34 lines
Quoted 28 lines
Quoted 19 lines
---
 drivers/usb/dwc2/hcd_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/dwc2/hcd_queue.c +1 -1
index 89a788326c56..bdf1927e1be1 100644
@@ -59,7 +59,7 @@
 #define DWC2_UNRESERVE_DELAY (msecs_to_jiffies(5))
 
 /* If we get a NAK, wait this long before retrying */
-#define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
+#define DWC2_RETRY_WAIT_DELAY (1 * USEC_PER_SEC)
 
 /**
  * dwc2_periodic_channel_available() - Checks that a channel is available for a
Quoted 4 lines

base-commit: d4439a1189f93d0ac1eaf0197db8e6b3e197d5c7 --
2.34.0.rc0

--
Thanks,
~Nick Desaulniers

Signature · 2 lines
Thanks,
~Nick Desaulniers
#4 lore raw
In reply to Nathan Chancellor
Quoted 35 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/usb/dwc2/hcd_queue.c:1744:25: error: expression requires 'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it

                         delay = ktime_set(0, DWC2_RETRY_WAIT_DELAY);
                                             ^
 drivers/usb/dwc2/hcd_queue.c:62:34: note: expanded from macro 'DWC2_RETRY_WAIT_DELAY'
 #define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
                                 ^

1 error 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 a floating point value in this driver, as the value is only used as a parameter to a function that expects an integer type. Use USEC_PER_SEC, which is the same integer value as '1E6L', to avoid changing functionality but fix the error.

---
 drivers/usb/dwc2/hcd_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/dwc2/hcd_queue.c +0 -0
index 89a788326c56..bdf1927e1be1 100644

drivers/usb/dwc2/hcd_queue.c line 62

Quoted 5 lines
drivers/usb/dwc2/hcd_queue.c +1 -1
@@ -59,7 +59,7 @@
 #define DWC2_UNRESERVE_DELAY (msecs_to_jiffies(5))
 
 /* If we get a NAK, wait this long before retrying */
-#define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
+#define DWC2_RETRY_WAIT_DELAY (1 * USEC_PER_SEC)

Using USEC_PER_SEC here seems quite weird. This is used as:

	delay = ktime_set(0, DWC2_RETRY_WAIT_DELAY);

so the units are nanoseconds.

Maybe NSEC_PER_MSEC would better indicate the intent here?

#5 lore raw
Quoted 44 lines · John Keeping
Quoted 35 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/usb/dwc2/hcd_queue.c:1744:25: error: expression requires 'long double' type support, but target 'x86_64-unknown-linux-gnu' does not support it

                         delay = ktime_set(0, DWC2_RETRY_WAIT_DELAY);
                                             ^
 drivers/usb/dwc2/hcd_queue.c:62:34: note: expanded from macro 'DWC2_RETRY_WAIT_DELAY'
 #define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
                                 ^

1 error 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 a floating point value in this driver, as the value is only used as a parameter to a function that expects an integer type. Use USEC_PER_SEC, which is the same integer value as '1E6L', to avoid changing functionality but fix the error.

---
 drivers/usb/dwc2/hcd_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/dwc2/hcd_queue.c +0 -0
index 89a788326c56..bdf1927e1be1 100644

drivers/usb/dwc2/hcd_queue.c line 62

Quoted 5 lines
drivers/usb/dwc2/hcd_queue.c +1 -1
@@ -59,7 +59,7 @@
 #define DWC2_UNRESERVE_DELAY (msecs_to_jiffies(5))
 
 /* If we get a NAK, wait this long before retrying */
-#define DWC2_RETRY_WAIT_DELAY (1 * 1E6L)
+#define DWC2_RETRY_WAIT_DELAY (1 * USEC_PER_SEC)

Using USEC_PER_SEC here seems quite weird. This is used as:

Quoted 5 lines
	delay = ktime_set(0, DWC2_RETRY_WAIT_DELAY);

so the units are nanoseconds.

Maybe NSEC_PER_MSEC would better indicate the intent here?

Yes, that seems reasonable. I will send a v2.

Cheers,
Nathan