devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Heiko Stuebner <heiko@sntech.de>
To: Jianfeng Liu <liujianfeng1994@gmail.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org,
	krzysztof.kozlowski+dt@linaro.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	liujianfeng1994@gmail.com, robh@kernel.org, sfr@canb.auug.org.au
Subject: Re: [PATCH] arm64: dts: rockchip: remove startup-delay-us from vcc3v3_pcie2x1l0 on rock-5b
Date: Wed, 10 Apr 2024 06:31:48 +0200	[thread overview]
Message-ID: <3879529.iIbC2pHGDl@phil> (raw)
In-Reply-To: <20240403075916.1025550-1-liujianfeng1994@gmail.com>

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@fe170000 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.



  reply	other threads:[~2024-04-10  4:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-01  8:13 [PATCH] arm64: dts: rockchip: remove startup-delay-us from vcc3v3_pcie2x1l0 on rock-5b Jianfeng Liu
2024-04-02 10:39 ` Heiko Stübner
2024-04-03  7:59   ` Jianfeng Liu
2024-04-10  4:31     ` Heiko Stuebner [this message]
2024-04-09 10:32 ` Jianfeng Liu
2024-04-10  6:30 ` Shawn Lin
2024-04-10 17:33   ` Jianfeng Liu
2024-04-12 16:09   ` Sebastian Reichel
2024-05-15 18:12     ` Marc Giger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3879529.iIbC2pHGDl@phil \
    --to=heiko@sntech.de \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=liujianfeng1994@gmail.com \
    --cc=robh@kernel.org \
    --cc=sfr@canb.auug.org.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).