Slightly improve hardware description of Pine64 RockPro64

Add missing "avdd-0v9-supply" and "avdd-1v8-supply" properties to the "hdmi" node in the Pine64 RockPro64 board dtsi file. To achieve this, also add the associated "vcca_0v9" regulator that produces the 0.9 V supply, [1][2] which hasn't been defined previously in the board dtsi file.

This also eliminates the following warnings from the kernel log:

dwhdmi-rockchip ff940000.hdmi: supply avdd-0v9 not found, using dummy regulator dwhdmi-rockchip ff940000.hdmi: supply avdd-1v8 not found, using dummy regulator

There are no functional changes to the way board works with these additions, because the "vcc1v8_dvp" and "vcca_0v9" regulators are always enabled, [1][2] but these additions improve the accuracy of hardware description.

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

Fixes e4f3fb490967 ("arm64: dts: rockchip: add initial dts support for Rockpro64")

Cc [email protected]

Suggested-by Diederik de Haas <[email protected]>

Signed-off-by Dragan Simic <[email protected]>

---
 arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi +12 -0

index 69a9d6170649..47dc198706c8 100644
@@ -227,6 +227,16 @@ vcc5v0_usb: regulator-vcc5v0-usb {
 		vin-supply = <&vcc12v_dcin>;
 	};
 
+	vcca_0v9: regulator-vcca-0v9 {
+		compatible = "regulator-fixed";
+		regulator-name = "vcca_0v9";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <900000>;
+		regulator-max-microvolt = <900000>;
+		vin-supply = <&vcc3v3_sys>;
+	};
+
 	vdd_log: regulator-vdd-log {
 		compatible = "pwm-regulator";
 		pwms = <&pwm2 0 25000 1>;

@@ -312,6 +322,8 @@ &gmac {
 };
 
 &hdmi {
+	avdd-0v9-supply = <&vcca_0v9>;
+	avdd-1v8-supply = <&vcc1v8_dvp>;
 	ddc-i2c-bus = <&i2c3>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&hdmi_cec>;

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.

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/[email protected]/T/#u

Fixes bba821f5479e ("arm64: dts: rockchip: add PCIe nodes on rk3399-rockpro64")

Cc [email protected]

Cc Vincenzo Palazzo <[email protected]>

Cc Peter Geis <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Reported-by Diederik de Haas <[email protected]>

Tested-by Chris Vogel <[email protected]>

Signed-off-by Dragan Simic <[email protected]>

---
 arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi +3 -1

index 47dc198706c8..41ee381ff81f 100644
@@ -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";
 };
 

This is a small series that introduces small improvements to the way Pine64 RockPro64 [1] single-board-computer is described in the DT files. This applies to both production-run revisions of the RockPro64.

The introduced improvements boil down to eliminating some warnings from the kernel log, by adding a previously undefined regulator and by adding some previously missing references to the regulators.

[...]

Quoted 9 lines · Dragan Simic

This is a small series that introduces small improvements to the way Pine64 RockPro64 [1] single-board-computer is described in the DT files. This applies to both production-run revisions of the RockPro64.

The introduced improvements boil down to eliminating some warnings from the kernel log, by adding a previously undefined regulator and by adding some previously missing references to the regulators.

[...]

Applied, thanks!

[1/2] arm64: dts: rockchip: Add avdd HDMI supplies to RockPro64 board dtsi

      commit: bd1c959f37f384b477f51572331b0dc828bd009a
[2/2] arm64: dts: rockchip: Add missing PCIe supplies to RockPro64 board dtsi
      commit: 64ef4a4320e7aa3f0f267e01f170f52b90bf0b1b

I've moved the pcie12v supply up one line.
While in a mathematical sense it's true 12 > 3.3, we're sorting alphabetical, so it's 1?? < 3?? .

And yes I sympathize with 3.3 < 12, but also have come to appreciate not having overly many special cases :-)

Quoted 18 lines · Heiko Stuebner

Quoted 9 lines · Dragan Simic

This is a small series that introduces small improvements to the way Pine64 RockPro64 [1] single-board-computer is described in the DT files. This applies to both production-run revisions of the RockPro64.

The introduced improvements boil down to eliminating some warnings from the kernel log, by adding a previously undefined regulator and by adding some previously missing references to the regulators.

[...]

Applied, thanks!

[1/2] arm64: dts: rockchip: Add avdd HDMI supplies to RockPro64 board dtsi

      commit: bd1c959f37f384b477f51572331b0dc828bd009a
[2/2] arm64: dts: rockchip: Add missing PCIe supplies to RockPro64 board dtsi
      commit: 64ef4a4320e7aa3f0f267e01f170f52b90bf0b1b

Quoted 6 lines

I've moved the pcie12v supply up one line.
While in a mathematical sense it's true 12 > 3.3, we're sorting alphabetical, so it's 1?? < 3?? .

And yes I sympathize with 3.3 < 12, but also have come to appreciate not having overly many special cases :-)

Great, thanks! :)

I'm fine with the alphabetical ordering, albeit with some caveats described below, but the following part of the patch description should also be removed, if possible, so the patch description fully matches the introduced changes:

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.

Admittedly, the patch itself indeed looks nicer [1] with the alphabetical ordering, :) (which is quasi-alphabetical, actually, please see below) but it might be better to have the DT guidelines adjusted to specify alphanumerical ordering as well. Even ls(1) can sort filenames alphanumerically, with its "-v" option that performs "natural sort of (version) numbers within text". [2]

The real trouble is that, if we were to apply the alphabetical ordering strictly, we'd end up with the following order of node properties in which 12 is before 1.8 and 3.3, which IMHO doesn't make much sense:

  vpcie0v9-supply = <&vcca_0v9>;
  vpcie12v-supply = <&vcc12v_dcin>;
  vpcie1v8-supply = <&vcca_1v8>;
  vpcie3v3-supply = <&vcc3v3_pcie>;

I'm hoping you'll agree that specifying alphanumerical ordering for the properties in the DTS coding style is the way to go, just like it's already specified for the ordering of the nodes. I'll go ahead and submit an appropriate patch for the DT guidelines.

[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?id=64ef4a4320e7aa3f0f267e01f170f52b90bf0b1b
[2] https://man.archlinux.org/man/ls.1.en#v

Hello Heiko,

Quoted 23 lines · Heiko Stuebner

Quoted 5 lines · Dragan Simic

This is a small series that introduces small improvements to the way Pine64 RockPro64 [1] single-board-computer is described in the DT files.
This applies to both production-run revisions of the RockPro64.

Quoted 7 lines

The introduced improvements boil down to eliminating some warnings from
the kernel log, by adding a previously undefined regulator and by adding
some previously missing references to the regulators.

[...]

Applied, thanks!

[1/2] arm64: dts: rockchip: Add avdd HDMI supplies to RockPro64 board dtsi

      commit: bd1c959f37f384b477f51572331b0dc828bd009a
[2/2] arm64: dts: rockchip: Add missing PCIe supplies to RockPro64 
board dtsi
      commit: 64ef4a4320e7aa3f0f267e01f170f52b90bf0b1b

Quoted 7 lines

I've moved the pcie12v supply up one line.
While in a mathematical sense it's true 12 > 3.3, we're sorting alphabetical, so it's 1?? < 3?? .

And yes I sympathize with 3.3 < 12, but also have come to appreciate not
having overly many special cases :-)

Great, thanks! :)

I'm fine with the alphabetical ordering, albeit with some caveats described below, but the following part of the patch description should also be removed, if possible, so the patch description fully matches the introduced changes:

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.

I'm hoping you'll agree that specifying alphanumerical ordering for the properties in the DTS coding style is the way to go, just like it's already specified for the ordering of the nodes. I'll go ahead and submit an appropriate patch for the DT guidelines.

Am Dienstag, 4. März 2025, 07:44:59 MEZ schrieb Dragan Simic:

Quoted 39 lines · Heiko Stuebner

Quoted 23 lines · Dragan Simic

Quoted 5 lines

This is a small series that introduces small improvements to the way Pine64 RockPro64 [1] single-board-computer is described in the DT files.
This applies to both production-run revisions of the RockPro64.

Quoted 7 lines

The introduced improvements boil down to eliminating some warnings from
the kernel log, by adding a previously undefined regulator and by adding
some previously missing references to the regulators.

[...]

Applied, thanks!

[1/2] arm64: dts: rockchip: Add avdd HDMI supplies to RockPro64 board dtsi

      commit: bd1c959f37f384b477f51572331b0dc828bd009a
[2/2] arm64: dts: rockchip: Add missing PCIe supplies to RockPro64
board dtsi
      commit: 64ef4a4320e7aa3f0f267e01f170f52b90bf0b1b

Quoted 7 lines

I've moved the pcie12v supply up one line.
While in a mathematical sense it's true 12 > 3.3, we're sorting alphabetical, so it's 1?? < 3?? .

And yes I sympathize with 3.3 < 12, but also have come to appreciate not
having overly many special cases :-)

Great, thanks! :)

I'm fine with the alphabetical ordering, albeit with some caveats described below, but the following part of the patch description should also be removed, if possible, so the patch description fully matches the introduced changes:

Quoted 6 lines

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.

I've amended the commit, dropping this block

Quoted 4 lines

I'm hoping you'll agree that specifying alphanumerical ordering for the properties in the DTS coding style is the way to go, just like it's already specified for the ordering of the nodes. I'll go ahead and submit an appropriate patch for the DT guidelines.

vpcie0v9-supply = <&vcca_0v9>;
vpcie1v8-supply = <&vcca_1v8>;
vpcie3v3-supply = <&vcc3v3_pcie>;
vpcie12v-supply = <&vcc12v_dcin>;

In the end I don't care _that_ much, but personally I find that alphanumerical ordering harder to read ;-) .

Because in the example above, my mind now constantly shouts "why is vpcie1... after vpcie3... ..... ooooh right, it's alpha-numerical"

But I can live with it I guess ;-) .
As 3.3 is smaller than 12 afterall.