linux-kernel - Re: [PATCH] arm64: dts: rockchip: remove startup-delay-us from vcc3v3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3879529.iIbC2pHGDl@phil>
Date: Wed, 10 Apr 2024 06:31:48 +0200
From: Heiko Stuebner <heiko@...ech.de>
To: Jianfeng Liu <liujianfeng1994@...il.com>
Cc: conor+dt@...nel.org, devicetree@...r.kernel.org,
 krzysztof.kozlowski+dt@...aro.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org,
 liujianfeng1994@...il.com, robh@...nel.org, sfr@...b.auug.org.au
Subject:
 Re: [PATCH] arm64: dts: rockchip: remove startup-delay-us from
 vcc3v3_pcie2x1l0 on rock-5b

Am Mittwoch, 3. April 2024, 09:59:16 CEST schrieb Jianfeng Liu:
> Hi Heiko,
> 
> Tue, 02 Apr 2024 12:39:17 +0200, Heiko Stübner wrote:
> >Does the pcie driver enable the regulator too late somehow?
> The pcie driver will enable the regulator imediately when it is probed.
> I added log at when driver is probed and when regulator is enabled.
> Here is the log with "startup-delay-us = <50000>":
> ```
> [    1.572991] rockchip-dw-pcie a40800000.pcie: rockchip_pcie_probe start
> [    1.573697] rockchip-dw-pcie a40800000.pcie: going to enable vpcie3v3 regulator
> [    1.575194] rockchip-dw-pcie a40800000.pcie: enable vpcie3v3 regulator done
> ```
> 
> And here is the log without "startup-delay-us":
> ```
> [    1.518490] rockchip-dw-pcie a40800000.pcie: rockchip_pcie_probe start
> [    1.518603] rockchip-dw-pcie a40800000.pcie: going to enable vpcie3v3 regulator
> [    1.518610] rockchip-dw-pcie a40800000.pcie: enable vpcie3v3 regulator done
> ```
> 
> We can see startup-delay-us will delay the driver probe.
> 
> I also take a look at rockchip's SDK kernel, their pci driver is probed
> very late:
> ```
> [    3.398682] dw-pcie fe170000.pcie: invalid resource
> [    3.398686] dw-pcie fe170000.pcie: Failed to initialize host
> [    3.398688] dw-pcie: probe of fe170000.pcie failed with error -22
> [    3.399396] rk-pcie fe170000.pcie: invalid prsnt-gpios property in node
> [    3.399410] rk-pcie fe170000.pcie: Looking up vpcie3v3-supply from device tree
> [    3.405195] rk-pcie fe170000.pcie: host bridge /pcie@...70000 ranges:
> [    3.405253] rk-pcie fe170000.pcie:       IO 0x00f2100000..0x00f21fffff -> 0x00f2100000
> [    3.405283] rk-pcie fe170000.pcie:      MEM 0x00f2200000..0x00f2ffffff -> 0x00f2200000
> [    3.405310] rk-pcie fe170000.pcie:      MEM 0x0980000000..0x09bfffffff -> 0x0980000000
> [    3.405372] rk-pcie fe170000.pcie: iATU unroll: enabled
> [    3.405381] rk-pcie fe170000.pcie: iATU regions: 8 ob, 8 ib, align 64K, limit 8G
> [    3.666917] rk-pcie fe170000.pcie: PCIe Link up, LTSSM is 0x30011
> [    3.666932] rk-pcie fe170000.pcie: PCIe Gen.1 x1 link up
> [    3.667139] rk-pcie fe170000.pcie: PCI host bridge to bus 0002:20
> ```
> 
> And it is reported that startup-delay-us is necessary in rockchip's SDK
> kernel. But in mainline kernel it is different.

that's not directly what I meant.

I.e. if the behaviour changes with arbitary delay changes, it points
very much to some sort of timing issue in the pcie driver itself.

That's why I asked about the regulator turning on, because if the enable
call returns 50ms earlier or later should never influence the behaviour
of the driver.

For example other threads could also just hinder the kernel from
continuing the pcie probe even after the regulator is on - again
leading to undefined behaviour, as you seem to be experiencing as
described in your mail from yesterday.