Re: [PATCH v10 4/5] arm64: tegra: Reorder reg and reg-names to match bindings

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect register offset for PCIe Link Capabilities causes garbage ICC bandwidth calculation
--

--- Patch [4]: [PATCH v10 4/5] arm64: tegra: Reorder reg and reg-names to match bindings ---
commit 1aff20381185113aa8fbe2813f31f1849b4105ad
Author: Thierry Reding <[email protected]>

arm64: tegra: Reorder reg and reg-names to match bindings

This commit reorders the "reg" and "reg-names" properties for the tegra264
PCIe controller nodes so that the ECAM region is listed last. This ensures
that the first entry matches the unit-address in the device tree nodes.

> diff --git a/arch/arm64/boot/dts/nvidia/tegra264.dtsi b/arch/arm64/boot/dts/nvidia/tegra264.dtsi
> index 013228a817237..ab1e83d856fd9 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra264.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra264.dtsi
> @@ -3537,11 +3537,11 @@ cmdqv4: cmdqv@b200000 {
>  
>  		pci@c000000 {
>  			compatible = "nvidia,tegra264-pcie";
> -			reg = <0xd0 0xb0000000 0x0 0x10000000>,
> -			      <0x00 0x0c000000 0x0 0x00004000>,
> +			reg = <0x00 0x0c000000 0x0 0x00004000>,
>  			      <0x00 0x0c004000 0x0 0x00001000>,
> -			      <0x00 0x0c005000 0x0 0x00001000>;
> -			reg-names = "ecam", "xal", "xtl", "xtl-pri";
> +			      <0x00 0x0c005000 0x0 0x00001000>,
> +			      <0xd0 0xb0000000 0x0 0x10000000>;
> +			reg-names = "xal", "xtl", "xtl-pri", "ecam";

[Severity: High]
Is the offset for XTL_RC_PCIE_CFG_LINK_CAPS in the tegra264 PCIe driver
correct? The PCIe Capability structure starts at 0x48, making the 32-bit
Link Capabilities register offset 0x48 + 0x0c = 0x54.

However, the driver defines it as 0x56:

drivers/pci/controller/pcie-tegra264.c:
	#define XTL_RC_PCIE_CFG_LINK_CAPS		0x56

When the link is down and the port supports hotplug, tegra264_pcie_icc_set()
calculates bandwidth using this offset:

drivers/pci/controller/pcie-tegra264.c:tegra264_pcie_icc_set() {
	...
		value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_CAPS);
		speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, value);
		width = FIELD_GET(PCI_EXP_LNKCAP_MLW, value);
	...
}

Does this readw() at offset 0x56 extract the upper 16 bits of the Link
Capabilities register instead of the lower 16 bits?

If so, this would cause FIELD_GET() to extract garbage data for both speed
and width (bits 16-19 for SLS and 20-25 for MLW), which could result in a
0 MB/s bandwidth request and starve the PCIe controller when a device is
later hotplugged.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4