usb: dwc2: hcd_queue: Fix use of floating point literal
Subsystems (2)
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
@@ -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
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 itdelay = 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
Fixes 6ed30a7d8ec2 ("usb: dwc2: host: use hrtimer for NAK retries")Signed-off-by Nathan Chancellor <[email protected]>
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@@ -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
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 itdelay = 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
Fixes 6ed30a7d8ec2 ("usb: dwc2: host: use hrtimer for NAK retries")Signed-off-by Nathan Chancellor <[email protected]>Thanks for the patch!
Reviewed-by Nick Desaulniers <[email protected]>
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@@ -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
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 itdelay = 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.Fixes 6ed30a7d8ec2 ("usb: dwc2: host: use hrtimer for NAK retries")Signed-off-by Nathan Chancellor <[email protected]>
Quoted 7 lines
Quoted 2 lines
Thanks for the patch!
Reviewed-by Nick Desaulniers <[email protected]>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@@ -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 DesaulniersSignature · 2 lines
Thanks, ~Nick Desaulniers
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 itdelay = 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.Fixes 6ed30a7d8ec2 ("usb: dwc2: host: use hrtimer for NAK retries")Signed-off-by Nathan Chancellor <[email protected]>--- drivers/usb/dwc2/hcd_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/usb/dwc2/hcd_queue.c +0
-0
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?
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 itdelay = 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.Fixes 6ed30a7d8ec2 ("usb: dwc2: host: use hrtimer for NAK retries")Signed-off-by Nathan Chancellor <[email protected]>--- drivers/usb/dwc2/hcd_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/usb/dwc2/hcd_queue.c +0
-0drivers/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_SEChere 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_MSECwould better indicate the intent here?
Yes, that seems reasonable. I will send a v2.
Cheers,
Nathan