Re: [PATCH v3 2/2] arm64: tegra: fix aspm-l1-entry-delay-ns L1 latency cells

[Manikanta Maddireddy <mmaddireddy@nvidia.com>]

On 15/05/26 1:15 pm, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Modifying device tree hardware descriptions to work around a mathematical bug in the driver's latency encoding logic.
> - [Low] The commit message incorrectly states that the driver converts aspm-l1-entry-delay-ns to 'whole ms'.
> --
> 
> commit bbff94096c8fb5346fe688ef1d463490a2efe6df
> Author: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> 
> arm64: tegra: fix aspm-l1-entry-delay-ns L1 latency cells
> 
> This commit updates the aspm-l1-entry-delay-ns property in Tegra194 and
> Tegra234 device trees to larger values to adjust the L1 entrance latency
> codes programmed by the driver.
> 
>> The Tegra194 PCIe driver converts aspm-l1-entry-delay-ns to whole ms
>> with ceiling division, then derives the Synopsys DesignWare PORT_AFR L1
> 
> This isn't a bug, but there's a typo in the commit message. The driver
> uses DIV_ROUND_UP(val, 1000) which converts nanoseconds to microseconds,
> not milliseconds.
> 
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> index 1d659454a6f9f..7bbf0e892724d 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> @@ -2382,7 +2382,7 @@ pcie@14100000 {
>>   			nvidia,aspm-cmrt-us = <60>;
>>   			nvidia,aspm-pwr-on-t-us = <20>;
>>   			nvidia,aspm-l0s-entrance-latency-us = <3>;
>> -			aspm-l1-entry-delay-ns = <4000>;
>> +			aspm-l1-entry-delay-ns = <8000>;
> 
> Does this work around a driver calculation bug by modifying the device tree
> hardware descriptions?
> 
> Device tree properties are generally expected to accurately describe physical
> hardware in an OS-agnostic way. The issue appears to stem from the driver
> using order_base_2() to determine the latency bucket. For exact powers of
> two, this returns the exact log2 value. For example, order_base_2(4) is 2.
> Code 2 maps to the "2 us to <4 us" bucket, which is insufficient for exactly
> 4 us.
> 
> Could the driver's calculation logic be updated to handle boundary conditions
> properly, rather than artificially inflating the hardware values in the
> device tree files?
> 

PORT_AFR L1 entrance latency uses the same style of stepped microsecond
buckets as Endpoint L1 Acceptable Latency in the PCIe spec. On our 
platforms, DWC maps a delay in (2 us, 4 us] to encoding 010b (code 2,
"maximum of 4 us" in the driver table). For that rule, order_base_2(us) 
after DIV_ROUND_UP(ns, 1000) selects the correct code (e.g. 4000 ns → 4 
us → 010b).

- Manikanta

-- 
nvpublic