From: Marc Giger <gigerstyle@gmail.com>
To: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Shawn Lin <shawn.lin@rock-chips.com>,
Jianfeng Liu <liujianfeng1994@gmail.com>,
robh@kernel.org, krzysztof.kozlowski+dt@linaro.org,
conor+dt@kernel.org, heiko@sntech.de, sfr@canb.auug.org.au,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rockchip@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: dts: rockchip: remove startup-delay-us from vcc3v3_pcie2x1l0 on rock-5b
Date: Wed, 15 May 2024 20:12:58 +0200 [thread overview]
Message-ID: <20240515201258.090f754f@mgi.gigerstyle.ch> (raw)
In-Reply-To: <273slx5qkz6ja5qlfjgcaukuzifzsxkdabsld3qodvxaekbzet@ipir56a6afj3>
Hi,
On Fri, 12 Apr 2024 18:09:13 +0200
Sebastian Reichel <sebastian.reichel@collabora.com> wrote:
> Hi,
>
> On Wed, Apr 10, 2024 at 02:30:16PM +0800, Shawn Lin wrote:
> > Hi Jianfeng,
> >
> > On 2024/4/1 16:13, Jianfeng Liu wrote:
> > > Property startup-delay-us is copied from vendor dts and it will
> > > make kernel not detect pcie wifi device. If I run command:
> > > "echo 1 > /sys/bus/pci/rescan", pcie wifi device is detected, but
> > > my wifi device RTL8822CE failed to load driver. Another device
> > > RTL8723BE can load driver but no wifi signal is detected.
> > >
> > > Removing this property will fix issues above.
> > >
> > > Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> >
> > startup-delay-us just make sure the power rail is stable before
> > any action is taken to start the link, preventing the device from
> > unable to work stably. So it shouldn't be the root cause I think.
> >
> > Could you help try this patch to checkout if it works for you?
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > index d684214..df30127 100644
> > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > @@ -167,7 +167,7 @@ static int rockchip_pcie_start_link(struct
> > dw_pcie *pci) struct rockchip_pcie *rockchip =
> > to_rockchip_pcie(pci);
> >
> > /* Reset device */
> > - gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
> > + //gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
>
> Is this removal actually needed?
>
> >
> > rockchip_pcie_enable_ltssm(rockchip);
> >
> > @@ -180,7 +180,7 @@ static int rockchip_pcie_start_link(struct
> > dw_pcie *pci)
> > * We need more extra time as before, rather than setting
> > just
> > * 100us as we don't know how long should the device need
> > to reset. */
> > - msleep(100);
> > + msleep(300);
> > gpiod_set_value_cansleep(rockchip->rst_gpio, 1);
> >
> > return 0;
> > @@ -311,6 +311,8 @@ static int rockchip_pcie_probe(struct
> > platform_device *pdev)
> > if (ret)
> > return ret;
> >
> > + gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
>
> I suppose it makes sense to use GPIOD_OUT_LOW in
> rockchip_pcie_resource_get(), so that the GPIO is requested low from
> the start instead of being high for a very short amount of time.
>
I see the very same issue as the original reporter on a Orange Pi 5
(rk3588s) and a Orange Pi 5 plus (rk3588). In my case the onboard
ethernet interfaces and/or the nvme drive are randomly not
initialized properly. This is with Linux 6.8.9. Funny enough it seemed
to work all the time on 6.8.4 without any issued (having the bifurcation
patches applied).
After applying the following patch (with the incorporated suggestions
from Sebastian) everything seems to work on the Opi5 plus but on the
Opi5 the nvme drive still wan't to properly initialize.
In dmesg the only difference in repect to pcie/nvme I can see is that on
a working system the following to lines are shown whereby they are
missing when the nvme was not initialized properly:
nvme nvme0: Shutdown timeout set to 10 seconds
nvme nvme0: 8/0/0 default/read/poll queues
---
debian-linux.orig/drivers/pci/controller/dwc/pcie-dw-rockchip.c
2024-05-02 14:35:35.000000000 +0000 +++
debian-linux/drivers/pci/controller/dwc/pcie-dw-rockchip.c
2024-05-14 19:25:18.519434456 +0000 @@ -180,7 +180,7 @@
* We need more extra time as before, rather than setting just
* 100us as we don't know how long should the device need to
reset. */
- msleep(100);
+ msleep(300);
gpiod_set_value_cansleep(rockchip->rst_gpio, 1);
return 0;
@@ -240,7 +240,7 @@
return PTR_ERR(rockchip->apb_base);
rockchip->rst_gpio = devm_gpiod_get_optional(&pdev->dev,
"reset",
- GPIOD_OUT_HIGH);
+ GPIOD_OUT_LOW);
if (IS_ERR(rockchip->rst_gpio))
return PTR_ERR(rockchip->rst_gpio);
@@ -311,6 +311,8 @@
if (ret)
return ret;
+ gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
+
/* DON'T MOVE ME: must be enable before PHY init */
rockchip->vpcie3v3 = devm_regulator_get_optional(dev,
"vpcie3v3"); if (IS_ERR(rockchip->vpcie3v3)) {
Thanks,
Marc
prev parent reply other threads:[~2024-05-15 18:13 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
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 [this message]
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=20240515201258.090f754f@mgi.gigerstyle.ch \
--to=gigerstyle@gmail.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=heiko@sntech.de \
--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=sebastian.reichel@collabora.com \
--cc=sfr@canb.auug.org.au \
--cc=shawn.lin@rock-chips.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).