Re: [PATCH] PCI/ASPM: Use 64-bit arithmetic instead of 32-bit
- Date: Tue, 13 Feb 2018 13:05:50 -0600
- From: "Gustavo A. R. Silva" <garsilva@xxxxxxxxxxxxxx>
- Subject: Re: [PATCH] PCI/ASPM: Use 64-bit arithmetic instead of 32-bit
Quoting Andy Shevchenko <andy.shevchenko@xxxxxxxxx>:
On Tue, Feb 13, 2018 at 6:59 PM, Gustavo A. R. Silva
Add suffix ULL to constant 1000 in order to give the compiler complete
information about the proper arithmetic to use. Notice that this
constant is used in a context that expects an expression of type
u64 (64 bits, unsigned).
The expression threshold_us * 1000 is currently being evaluated
using 32-bit arithmetic.
- u64 threshold_ns = threshold_us * 1000;
+ u64 threshold_ns = threshold_us * 1000ULL;
Shouldn't be other way around, i.e.
Either way works. The thing is that casting threshold_us to u64 may
imply that there is something wrong with threshold_us, which does not
seem to be the case. So adding the suffix ULL to the constant 1000 is
good enough to make the expression be evaluated using 64-bit
arithmetic instead of 32-bit.
But, again, either way works.
But still the question. have you checked all callers? Does it even
The proposed patch was due to fact that currently threshold_ns is of
type u64. But based on the following piece of code (which is the only
piece of code from where encode_l12_threshold is being called):
* Based on PCIe r3.1, sec 18.104.22.168.1, Figures 5-16 and 5-17, and
* Table 5-11. T(POWER_OFF) is at most 2us and T(L1.2) is at
* least 4us.
l1_2_threshold = 2 + 4 + t_common_mode + t_power_on;
encode_l12_threshold(l1_2_threshold, &scale, &value);
It seems to me that it makes no sense for threshold_ns to be of type
u64, because the expression threshold_us * 1000 will never exceed the
32-bit limits. So if you agree I can send a patch to change its type
to u32 instead.