| Message ID | cover.1740941097.git.dsimic@manjaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Slightly improve hardware description of Pine64 RockPro64 | expand |
On Sun, 02 Mar 2025 19:48:02 +0100, Dragan Simic wrote: > 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 :-) Best regards,
Hello Heiko, On 2025-03-03 23:36, Heiko Stuebner wrote: > On Sun, 02 Mar 2025 19:48:02 +0100, Dragan Simic wrote: >> 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 :-) 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
On 2025-03-04 07:44, Dragan Simic wrote: > On 2025-03-03 23:36, Heiko Stuebner wrote: >> On Sun, 02 Mar 2025 19:48:02 +0100, Dragan Simic wrote: >>> 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 :-) > > 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 For future reference and for anyone interested, here's a link to the above-mentioned DTS coding style patch: https://lore.kernel.org/linux-kernel/09d6f2fc111b3d6e58987336944f93ec36b65118.1741071107.git.dsimic@manjaro.org/T/#u
Am Dienstag, 4. März 2025, 07:44:59 MEZ schrieb Dragan Simic: > Hello Heiko, > > On 2025-03-03 23:36, Heiko Stuebner wrote: > > On Sun, 02 Mar 2025 19:48:02 +0100, Dragan Simic wrote: > >> 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 :-) > > 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've amended the commit, dropping this block > 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. Heiko
Hello Heiko, On 2025-03-04 22:52, Heiko Stübner wrote: > Am Dienstag, 4. März 2025, 07:44:59 MEZ schrieb Dragan Simic: >> On 2025-03-03 23:36, Heiko Stuebner wrote: >> > On Sun, 02 Mar 2025 19:48:02 +0100, Dragan Simic wrote: >> >> 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 :-) >> >> 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've amended the commit, dropping this block Thanks! >> 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. Oh, I know very well first-hand how alphanumerical ordering may look and feel strange at first... :) I used ls(1) without the "-v" option for many, many years, and the things looked a bit strange after I decided to add "-v" to its set of options at some point, but after a short period of getting used to it, alphanumerical sorting became so logical and useful to me that alphabetical ordering is actually now harder to read and is looking a bit strange to me. :)
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. The patch 2/2 also results in reliability improvements, by reportedly resolving (some of) the longstanding issues resulting from using certain PCI Express cards with the Pine64 RockPro64. Changes in v2: - Dissolved the patch 2/2 notes into its description, according to the numerous power cycling/rebooting tests performed on a Pine64 RockPro64 by @ChriChri and reported in the #rock64 channel on irc.pine64.org - Adjusted the cover letter to match the latest test results - Added "Fixes" and "Cc: stable" tags to both patches in the series - Added "Tested-by" to the patch 2/2, thanks to Chris' testing [2] - Added a few "Cc" tags to the patch 2/2, to make sure people involved in the earlier related patch [3] are in the loop Link to v1: https://lore.kernel.org/linux-rockchip/cover.1740748439.git.dsimic@manjaro.org/T/#u [1] https://wiki.pine64.org/wiki/ROCKPro64 [2] https://z9.de/hedgedoc/s/nF4d5G7rg#reboot-tests-for-PCIe-improvements [3] https://lore.kernel.org/lkml/20230509153912.515218-1-vincenzopalazzodev@gmail.com/T/#u Dragan Simic (2): arm64: dts: rockchip: Add avdd HDMI supplies to RockPro64 board dtsi arm64: dts: rockchip: Add missing PCIe supplies to RockPro64 board dtsi .../boot/dts/rockchip/rk3399-rockpro64.dtsi | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)