Re: [PATCH 1/1] PCI: tegra194: fix min() signedness when capping ASPM L1 entrance latency

[Manikanta Maddireddy <mmaddireddy@nvidia.com>]

On 08/04/26 5:28 pm, David Laight wrote:
> On Tue, 7 Apr 2026 20:27:49 +0530
> Manikanta Maddireddy <mmaddireddy@nvidia.com> wrote:
> 
>> The DT property "aspm-l1-entry-delay-ns" is converted to microseconds,
>> then encoded for the L1 entrance latency register field as ilog2(us) + 1,
>> clamped to the hardware maximum of 7.
>>
>> ilog2() returns int type, while the upper bound is 7U (unsigned int).
>> The min() macro is implemented with __careful_cmp(), which rejects mixed
>> signed and unsigned operands at compile time via BUILD_BUG_ON_MSG in
>> minmax.h; that check trips on this pair, notably when building with W=1.
>>
>> This combination fails to build (e.g. parisc allyesconfig, GCC 15, as
>> reported by the 0-day bot).
>>
>> Use min_t(u32, ilog2(us) + 1U, 7U) so both sides of the comparison are
>> unsigned and consistent with aspm_l1_enter_lat.
> 
> Adding 1U (rather than 1) is enough to make everything signed.
> Alternatively change the 7U to 7 and it will all be fine regardless of
> whether ilog2() returns a signed or unsigned result.
> 
> Remember min_t(u32, x, y) is min((u32)x, (u32)y) and you wouldn't put in
> casts like that for any other arithmetic operation.
> 
> Note that for the compile to fail there has to be a code path where
> ilog2(us) isn't known to generate a non-negative value.
> ilog2(us) (probably) ends up as 'fls(us) - 1'. If that is implemented using a
> compiler builtin (because there is a single instruction) then gcc knows that
> the input can't be zero (from the max()), so knows that fls() can't return 0
> (which it does for 0), so knows it is never negative and the checks in min()
> pass.
> 
> parisc may be one of the architectures that ends up with a real function
> for fls() so the compiler doesn't know the result of ilog2() is
> non-negative.
> 
> Just delete the U.
> 
> 	David

Thank you David for the review.

Hi Bjorn, Mani,

Let me know if you are OK with David's suggestion, I will send new patch 
as shown below

-	pcie->aspm_l1_enter_lat = min(ilog2(us) + 1, 7U);
+	pcie->aspm_l1_enter_lat = min(ilog2(us) + 1, 7);

> 
>>
>> Fixes: 4a44cd65c9dd ("PCI: tegra194: Use aspm-l1-entry-delay-ns DT property for L1 entrance latency")
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202604051407.AODe3ddZ-lkp@intel.com/
>> ---
>>   drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
>> index 393f75ce3df3..93d3452ac117 100644
>> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
>> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
>> @@ -1147,7 +1147,7 @@ static int tegra_pcie_dw_parse_dt(struct tegra_pcie_dw *pcie)
>>   	if (!ret) {
>>   		u32 us = max(val / 1000, 1U);
>>   
>> -		pcie->aspm_l1_enter_lat = min(ilog2(us) + 1, 7U);
>> +		pcie->aspm_l1_enter_lat = min_t(u32, ilog2(us) + 1U, 7U);
>>   	}
>>   
>>   	ret = of_property_read_u32(np, "num-lanes", &pcie->num_lanes);
> 

-- 
nvpublic