Hi Sebastian,
On 6/6/24 6:25 PM, Sebastian Kropatsch wrote:
[You don't often get email from seb-dev@mail.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi Quentin,
thanks for your feedback!
Am 06.06.2024 um 16:53 schrieb Quentin Schulz:
Hi Sebastian,
On 6/5/24 5:36 PM, Sebastian Kropatsch wrote:
[You don't often get email from seb-dev@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@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.
u-boot-next currently has two other RK3588s boards, which are Rock5A and Indiedroid Nova. Both do not have the S at the end of the symbol:
config TARGET_ROCK5A_RK3588 [...] config TARGET_NOVA_RK3588
Which is why I haven't included the S. Should we update those as well? I suppose in this case I should also update '/include/configs/nanopi- r6c-rk3588s.h' which currently reads:
Ah well, this went through the review process :) It's not a big deal anyway, so we can do a search and replace later if necessary.
diff --git a/include/configs/nanopi-r6c-rk3588s.h b/include/configs/nanopi-r6c-rk3588s.h new file mode 100644 index 0000000000..e05856f3ce --- /dev/null +++ b/include/configs/nanopi-r6c-rk3588s.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef __NANOPI_R6C_RK3588_H +#define __NANOPI_R6C_RK3588_H
+#define ROCKCHIP_DEVICE_SETTINGS \ + "stdout=serial,vidconsole\0" \ + "stderr=serial,vidconsole\0"
+#include <configs/rk3588_common.h>
+#endif /* __NANOPI_R6C_RK3588_H */
(end '/include/configs/nanopi-r6c-rk3588s.h') If this should not be updated to add an S, let me know. I'll send a v2 when I know :)
Nah, this is a local define, don't care too much if there's a typo in there as the only purpose is that no other file actually define it :)
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
Ah yes, I was wondering if my mostly empty rk3588s-nanopi-r6c- u-boot.dtsi was even needed, since I don't know much about the inner workings of U-Boot. I believe it would be nice to not need this file, but I have no idea how complex and maintainable an implementation would be. If it's complex I'd say no, since having those *u-boot.dtsi files don't hurt that much in my opinion :)
This whole rk3588 is an rk3588s++ but actually we use "rk3588" for everything but actually no, the Device Tree base is "rk3588s" is a bit of a pain to deal with in different places :) Probably a similar issue with the RK3566 vs RK3568 I guess. And it's probably not going to be much more fun with the RK3582 which is an RK3588s--.
A few options right now tht comes to mind: - add a new CONFIG_SYS_SOC for rk3588s and add symlinks wherever necessary to point to rk3588 implementations - add a new config symbol for use here: https://elixir.bootlin.com/u-boot/latest/source/scripts/Makefile.lib#L162 - make U-Boot try recursively a right dash-stripped verison of the dts, e.g.: rk3588s-nanopi-r6c would also try rk3588s-nanopi-u-boot.dtsi and rk3588s-u-boot.dtsi
I think we're good anyway, so
Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Thanks! Quentin