Re: [PATCH v2 2/2] arm64: dts: rockchip: Add missing PCIe supplies to RockPro64 board dtsi

["Diederik de Haas" <didi.debian@cknow.org>]

[-- Attachment #1: Type: text/plain, Size: 5616 bytes --]

Hi Dragan,

On Sun Mar 2, 2025 at 7:48 PM CET, Dragan Simic wrote:
> Add missing "vpcie0v9-supply" and "vpcie1v8-supply" properties to the "pcie0"
> node in the Pine64 RockPro64 board dtsi file.  This eliminates the following
> warnings from the kernel log:
>
>   rockchip-pcie f8000000.pcie: supply vpcie1v8 not found, using dummy regulator
>   rockchip-pcie f8000000.pcie: supply vpcie0v9 not found, using dummy regulator
>
> These additions improve the accuracy of hardware description of the RockPro64
> and, in theory, they should result in no functional changes to the way board
> works after the changes, because the "vcca_0v9" and "vcca_1v8" regulators are
> always enabled. [1][2]  However, extended reliability testing, performed by
> Chris, [3] has proven that the age-old issues with some PCI Express cards,
> when used with a Pine64 RockPro64, are also resolved.

Thanks for this patch :-)

I 'reported' the issue based on the logs I saw from other people, but
now that I have a RockPro64 (version 2.1) myself, I can confirm that
without this patch I saw those warnings myself.

When booted up without a PCIe card in the slot, I also saw these errors:
[    5.106650] rockchip-pcie f8000000.pcie: PCIe link training gen1 timeout!
[    5.107373] rockchip-pcie f8000000.pcie: probe with driver rockchip-pcie failed with error -110

You indicated that that was due to there being no PCIe card inserted.

After applying this patch I booted up my RockPro64 and found that the
above mentioned warnings are indeed gone.

Then I inserted a Renesas Electronics Corp. uPD720201 USB 3.0 Host
Controller in the PCIe slot and booted up.
The above mentioned errors were indeed gone.
Furthermore ``lspci`` showed that card and when I plugged in an USB 3
drive in one of the ports, it was correctly detected and I could mount
the partition on my system. So feel free to include:

Tested-by: Diederik de Haas <didi.debian@cknow.org>

Cheers,
  Diederik

> Those issues were already mentioned in the commit 43853e843aa6 (arm64: dts:
> rockchip: Remove unsupported node from the Pinebook Pro dts, 2024-04-01),
> together with a brief description of the out-of-tree enumeration delay patch
> that reportedly resolves those issues.  In a nutshell, booting a RockPro64
> with some PCI Express cards attached to it caused a kernel oops. [4]
>
> Symptomatically enough, to the commit author's best knowledge, only the Pine64
> RockPro64, out of all RK3399-based boards and devices supported upstream, has
> been reported to suffer from those PCI Express issues, and only the RockPro64
> had some of the PCI Express supplies missing in its DT.  Thus, perhaps some
> weird timing issues exist that caused the "vcca_1v8" always-on regulator,
> which is part of the RK808 PMIC, to actually not be enabled before the PCI
> Express is initialized and enumerated on the RockPro64, causing oopses with
> some PCIe cards, and the aforementioned enumeration delay patch [4] probably
> acted as just a workaround for the underlying timing issue.
>
> Admittedly, the Pine64 RockPro64 is a bit specific board by having a standard
> PCI Express slot, allowing use of various standard cards, but pretty much
> standard PCI Express cards have been attached to other RK3399 boards as well,
> and the commit author is unaware ot such issues reported for them.
>
> It's quite hard to be sure that the PCI Express issues are fully resolved by
> these additions to the DT, without some really extensive and time-consuming
> testing.  However, these additions to the DT can result in good things and
> improvements anyway, making them perfectly safe from the standpoint of being
> unable to do any harm or cause some unforeseen regressions.
>
> Shuffle and reorder the "vpcie*-supply" properties a bit, so they're sorted
> alphanumerically, which is a bit more logical and more useful than having
> these properties listed in their strict alphabetical order.
>
> These changes apply to the both supported hardware revisions of the Pine64
> RockPro64, i.e. to the production-run revisions 2.0 and 2.1. [1][2]
>
> [1] https://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf
> [2] https://files.pine64.org/doc/rockpro64/rockpro64_v20-SCH.pdf
> [3] https://z9.de/hedgedoc/s/nF4d5G7rg#reboot-tests-for-PCIe-improvements
> [4] https://lore.kernel.org/lkml/20230509153912.515218-1-vincenzopalazzodev@gmail.com/T/#u
>
> Fixes: bba821f5479e ("arm64: dts: rockchip: add PCIe nodes on rk3399-rockpro64")
> Cc: stable@vger.kernel.org
> Cc: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
> Cc: Peter Geis <pgwipeout@gmail.com>
> Cc: Bjorn Helgaas <helgaas@kernel.org>
> Reported-by: Diederik de Haas <didi.debian@cknow.org>
> Tested-by: Chris Vogel <chris@z9.de>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> index 47dc198706c8..41ee381ff81f 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> @@ -673,8 +673,10 @@ &pcie0 {
>  	num-lanes = <4>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie_perst>;
> -	vpcie12v-supply = <&vcc12v_dcin>;
> +	vpcie0v9-supply = <&vcca_0v9>;
> +	vpcie1v8-supply = <&vcca_1v8>;
>  	vpcie3v3-supply = <&vcc3v3_pcie>;
> +	vpcie12v-supply = <&vcc12v_dcin>;
>  	status = "okay";
>  };
>  


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]