Re: [PATCH V3 1/2] arm64: dts: rockchip: Add rk3588-orangepi-5b device tree and refactor

[Jimmy Hon]

On Sun, Oct 13, 2024 at 4:33 PM Cenk Uluisik
<cenk.uluisik@xxxxxxxxxxxxxx> wrote:
>
> Implements a slightly modified rk3588s-orangepi-5b.dts from the vendor.
> Unfortunately the &wireless_bluetooth and &wireless_wlan overlays don't seem
> to compile, so I removed them for now:
>
> Error: arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5b.dts:20.1-20 Label or path wireless_bluetooth not found
> Error: arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5b.dts:24.1-15 Label or path wireless_wlan not found
>
> Bigger parts of the rk3588s-orangepi-5.dts file were moved into a new
> rk3588s-orangepi-5.dtsi file, so that both device trees from the orangepi-5 and 5b include from it and avoid including from the .dts.
>
> How does this board differ from the original Orange Pi 5?
>   - builtin eMMC storage
>   - no SPI NOR flash (u-boot, preboot etc. initiates from within the eMMC
>     storage)
>   - ap6275p Wifi module (like the Orange Pi 5 Plus)
>   - builtin BlueTooth module
>
> Beside that everything is exactly the same as far as I know.

Another difference is the Orange Pi 5 has a M.2 NVMe M-key PCI 2.0x1
slot (hooked to combphy0_ps) whereas the Orange Pi 5b uses combphy0_ps
for the WiFi.
I mention this because the vendor kernel defines each boards pcie2 differently
https://github.com/orangepi-xunlong/linux-orangepi/blob/orange-pi-6.1-rk35xx/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts#L441
https://github.com/orangepi-xunlong/linux-orangepi/blob/orange-pi-6.1-rk35xx/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5b.dts#L373

The Orange Pi 5 with the M.2 socket has a regulator defined hooked to
"GPIO0_C5" (i.e. PCIE_PWREN_H) whereas the Orange Pi 5B has GPIO0_C5
hooked to BT_WAKE_HOST.

> --- a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> @@ -1,766 +1,10 @@
> -// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> -

> -       vcc3v3_pcie20: vcc3v3-pcie20-regulator {
> -               compatible = "regulator-fixed";
> -               enable-active-high;
> -               gpios = <&gpio0 RK_PC5 GPIO_ACTIVE_HIGH>;
> -               regulator-name = "vcc3v3_pcie20";
> -               regulator-boot-on;
> -               regulator-min-microvolt = <1800000>;
> -               regulator-max-microvolt = <1800000>;
> -               startup-delay-us = <50000>;
> -               vin-supply = <&vcc5v0_sys>;
> -       };
As mentioned at the top, you may want to only define this in the
Orange Pi 5 board, and leave it out of the Orange Pi 5B board.

> -&pcie2x1l2 {
> -       reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
> -       vpcie3v3-supply = <&vcc3v3_pcie20>;
> -       status = "okay";
> -};
Then this can be defined differently between the Orange Pi 5 vs Orange Pi 5b.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> similarity index 99%
> copy from arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> copy to arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> index feea6b20a6bf..739c4d9f58e0 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> @@ -10,9 +10,6 @@
>  #include "rk3588s.dtsi"
>
>  / {
> -       model = "Xunlong Orange Pi 5";
> -       compatible = "xunlong,orangepi-5", "rockchip,rk3588s";
> -
>         aliases {
>                 ethernet0 = &gmac1;
>                 mmc0 = &sdmmc;
Since the sdhci is enabled for the Orange Pi 5b, it'd be nice to add
an alias for it.

Heiko, can we change the sdmmc alias to be mmc1, and let the sdhci be
mmc0? That way it's consistent with all the other rk3588 DTS? A change
like this could break existing users if they coded using /dev/mmc0
device file.

Seems like it's only the NanoPi and Orange Pi 5 rk3588 that use a
different convention. The Orange Pi 5 Plus is consistent with the
other rk3588 device trees.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts?h=v6.11#n20

This is the new default that rockchip wants to use.
https://github.com/orangepi-xunlong/linux-orangepi/commit/bce92d16b230b8e93c2831fb7768839fd7bbab04

Orange Pi flipped it in their 5.10 kernel.
https://github.com/orangepi-xunlong/linux-orangepi/commit/7e6c3163aa7e58b19730aa2aa259f1bb957cbca0#diff-0c7ddd2f22091009f8e7a4970aa293bfae425f25d0fe2c19418b886ec9eab3fa

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5b.dts
> new file mode 100644
> index 000000000000..049227af0252
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5b.dts
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +/dts-v1/;
> +
> +#include "rk3588s-orangepi-5.dtsi"
> +
> +/ {
> +       model = "Xunlong Orange Pi 5B";
> +       compatible = "rockchip,rk3588s-orangepi-5b", "rockchip,rk3588";
The second part of the compatible should be "rockchip,rk3588s" to
match the binding.
> +};
> +
> +&sdhci {
> +       status = "okay";
> +};
The mainline dtsi does not have the sdhci node elaborated on for the
Orange Pi 5 like the vendor kernel does. Do you want to add it to the
common dtsi?
https://github.com/orangepi-xunlong/linux-orangepi/blob/orange-pi-6.1-rk35xx/arch/arm64/boot/dts/rockchip/rk3588s-orangepi.dtsi#L461-L470
The Orange Pi 5 Plus has a similar sdhci node definition in mainline at
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts?h=v6.11#n433
> +
> +&sfc {
> +       status = "disabled";
> +};
Since the sfc is a difference between board variants, would it be a
better practice to mark the node disabled in the common dtsi, and mark
it enabled only in the 5.dts?

> \ No newline at end of file
> --
> 2.46.0
>