Re: [PATCH] PCI/ASPM: Use 64-bit arithmetic instead of 32-bit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andy,

Quoting Andy Shevchenko <andy.shevchenko@xxxxxxxxx>:

On Tue, Feb 13, 2018 at 6:59 PM, Gustavo A. R. Silva
<garsilva@xxxxxxxxxxxxxx> wrote:
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.

(u64)threshold_us ?


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 makes sense?


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 5.5.3.3.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.

Thanks
--
Gustavo













[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux