Hi Sebastian,
On 6/6/24 6:25 PM, Sebastian Kropatsch wrote:
[You don't often get email from seb-...@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-...@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-...@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.sch...@cherry.de>
Thanks!
Quentin
