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

[Manivannan Sadhasivam]

On Thu, Apr 09, 2026 at 12:35:04PM +0530, Manikanta Maddireddy wrote:
> 
> 
> On 08/04/26 5:28 pm, David Laight wrote:
> > On Tue, 7 Apr 2026 20:27:49 +0530
> > Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx> 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);
> 

Sure, go ahead.

- Mani

> > 
> > > 
> > > Fixes: 4a44cd65c9dd ("PCI: tegra194: Use aspm-l1-entry-delay-ns DT property for L1 entrance latency")
> > > Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
> > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202604051407.AODe3ddZ-lkp@xxxxxxxxx/
> > > ---
> > >   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
> 

-- 
மணிவண்ணன் சதாசிவம்