[PATCH] board: rockchip: Add FriendlyElec NanoPi R6C

[Quentin Schulz]

Hi Sebastian,

On 6/5/24 5:36 PM, Sebastian Kropatsch wrote:
> [You don't often get email from seb-dev at mail.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> The NanoPi R6C is a SBC by FriendlyElec based on the Rockchip RK3588s.
> It comes with 4GB or 8GB of RAM, a microSD card slot, optional 32GB eMMC
> storage, one M.2 M-Key connector, one RTL8211F 1GbE and one RTL8125
> 2.5GbE Ethernet port, one USB 2.0 Type-A and one USB 3.0 Type-A port, a
> HDMI port, a 30-pin GPIO header as well as multiple buttons and LEDs.
> 
> Add initial support for this board using the upstream devicetree sources.
> 
> Tests in U-Boot proper:
> - Booting from eMMC works
> - 1GbE Ethernet works using the eth_eqos driver (tested by ping)
> - 2.5GbE Ethernet works using the eth_rtl8169 driver (tested by ping),
>    but the status LEDs on this specific port currently aren't working
> - NVMe SSD in M.2 socket does get recognized (tested with `nvme scan`
>    followed by `nvme details`)
> 
> Kernel commit:
> d5f1d7437451 ("arm64: dts: rockchip: Add support for NanoPi R6C")
> 
> Signed-off-by: Sebastian Kropatsch <seb-dev at mail.de>
> ---
> 
> Hello!
> 
> The Ethernet status LEDs which sit directly on the 2.5GbE port using the
> RTL8169 driver don't light up when connected and I couldn't figure out
> why. The other port with a RTL8211F has no problems with the LEDs.
> Have there been occurrences like this in combination with the RTL8125?
> I'm trying to figure out if this is something that could be solved in
> the devicetree or if this is a potential driver bug.
> 
> Secondly, the default active network device in U-Boot is the 1GbE one.
> I believe it would make sense to make the 2.5GbE device the default
> active one since this one is labeled "LAN", whereas the 1GbE is labeled
> "WAN". However, since the 2.5GbE device is PCIe-based, it only shows up
> in U-Boot proper after using the `pci enum` command (shows up as in gets
> listed in `net list` and `dm tree`).
> Do you have any tips on the preferred approach to handle this switch of
> the default active net device? Is this even a sensible thing to include
> in U-Boot in your opinion?
> 
> Thanks for your feedback!
> 
> Best regards,
> Sebastian
> 
> ---
>   arch/arm/dts/rk3588s-nanopi-r6c-u-boot.dtsi   |  3 +
>   arch/arm/mach-rockchip/rk3588/Kconfig         | 13 +++
>   board/friendlyelec/nanopi-r6c-rk3588s/Kconfig | 12 +++
>   .../nanopi-r6c-rk3588s/MAINTAINERS            |  7 ++
>   configs/nanopi-r6c-rk3588s_defconfig          | 83 +++++++++++++++++++
>   doc/board/rockchip/rockchip.rst               |  1 +
>   include/configs/nanopi-r6c-rk3588s.h          | 12 +++
>   7 files changed, 131 insertions(+)
>   create mode 100644 arch/arm/dts/rk3588s-nanopi-r6c-u-boot.dtsi
>   create mode 100644 board/friendlyelec/nanopi-r6c-rk3588s/Kconfig
>   create mode 100644 board/friendlyelec/nanopi-r6c-rk3588s/MAINTAINERS
>   create mode 100644 configs/nanopi-r6c-rk3588s_defconfig
>   create mode 100644 include/configs/nanopi-r6c-rk3588s.h
> 
> diff --git a/arch/arm/dts/rk3588s-nanopi-r6c-u-boot.dtsi b/arch/arm/dts/rk3588s-nanopi-r6c-u-boot.dtsi
> new file mode 100644
> index 0000000000..853ed58cfe
> --- /dev/null
> +++ b/arch/arm/dts/rk3588s-nanopi-r6c-u-boot.dtsi
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +#include "rk3588s-u-boot.dtsi"
> diff --git a/arch/arm/mach-rockchip/rk3588/Kconfig b/arch/arm/mach-rockchip/rk3588/Kconfig
> index 820e979abb..0e232ddfb9 100644
> --- a/arch/arm/mach-rockchip/rk3588/Kconfig
> +++ b/arch/arm/mach-rockchip/rk3588/Kconfig
> @@ -78,6 +78,18 @@ config TARGET_NANOPCT6_RK3588
>            Power: 5.5*2.1mm DC Jack, 12VDC input
>            Dimensions: 110x80x1.6mm (without case) / 86x114.5x30mm (with case)
> 
> +config TARGET_NANOPI_R6C_RK3588

Please add the S at the end of the symbol.

Otherwise looking good to me. I'm wondering if we shouldn't do something 
for boards which do not need a -u-boot.dtsi. Here it's needed because 
U-Boot would include rk3588-u-boot.dtsi automatically (because of 
CONFIG_SYS_SOC being rk3588) instead of rk3588s-u-boot.dtsi. Maybe it's 
too much headache for the benefit :)

Cheers,
Quentin