Re: [PATCH 7/7] board: rockchip: add FriendlyElec NanoPi R3S

[Tianling Shen <cnsztl@gmail.com>]

Hi Quentin,

On 2025/1/14 22:48, Quentin Schulz wrote:
> Hi Tianling,
> 
> On 12/26/24 10:20 AM, Tianling Shen wrote:
>> The NanoPi R3S(as "R3S") is an open source platform with dual-Gbps
>> Ethernet ports designed and developed by FriendlyElec for IoT
>> applications.
>>
>> Specification:
>> - Rockchip RK3566
>> - 2GB LPDDR4X RAM
>> - optional 32GB eMMC module
>> - SD card slot
>> - 2x 1000 Base-T
>> - 3x LEDs (POWER, LAN, WAN)
>> - 2x Buttons (Reset, MaskROM)
>> - 1x USB 3.0 Port
>> - Type-C 5V 2A Power
>>
>> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
>> ---
>>   arch/arm/dts/rk3566-nanopi-r3s-u-boot.dtsi |  8 +++
>>   board/rockchip/evb_rk3568/MAINTAINERS      |  7 ++
>>   configs/nanopi-r3s-rk3566_defconfig        | 74 ++++++++++++++++++++++
>>   doc/board/rockchip/rockchip.rst            |  1 +
>>   4 files changed, 90 insertions(+)
>>   create mode 100644 arch/arm/dts/rk3566-nanopi-r3s-u-boot.dtsi
>>   create mode 100644 configs/nanopi-r3s-rk3566_defconfig
>>
>> diff --git a/arch/arm/dts/rk3566-nanopi-r3s-u-boot.dtsi b/arch/arm/ 
>> dts/rk3566-nanopi-r3s-u-boot.dtsi
>> new file mode 100644
>> index 0000000000..b66e5015d6
>> --- /dev/null
>> +++ b/arch/arm/dts/rk3566-nanopi-r3s-u-boot.dtsi
>> @@ -0,0 +1,8 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +#include "rk356x-u-boot.dtsi"
>> +
>> +&vcc5v0_usb {
>> +    /delete-property/ regulator-always-on;
>> +    /delete-property/ regulator-boot-on;
>> +};
> 
> Please justify this change, this seems very odd.

This change is based on commit 5b155997d445 "rockchip: rk3568-nanopi-r5: 
Update defconfig for NanoPi R5C and R5S" and it's copy&paste.

And another commit a9e9445ea2bb "rockchip: rk3568-nanopi-r5: Enable PCIe 
on NanoPi R5C and R5S" removes the vpcie3v3-supply prop, not sure why.

> 
>> diff --git a/board/rockchip/evb_rk3568/MAINTAINERS b/board/rockchip/ 
>> evb_rk3568/MAINTAINERS
>> index 588134ecb2..b2780401a3 100644
>> --- a/board/rockchip/evb_rk3568/MAINTAINERS
>> +++ b/board/rockchip/evb_rk3568/MAINTAINERS
>> @@ -28,6 +28,13 @@ F:    configs/lubancat-2-rk3568_defconfig
>>   F:    arch/arm/dts/rk3568-lubancat-2.dts
>>   F:    arch/arm/dts/rk3568-lubancat-2-u-boot.dtsi
>> +NANOPI-R3S
>> +M:    Tianling Shen <cnsztl@gmail.com>
>> +R:    Jonas Karlman <jonas@kwiboo.se>
>> +S:    Maintained
>> +F:    configs/nanopi-r3s-rk3566_defconfig
>> +F:    arch/arm/dts/rk3566-nanopi-r3s-u-boot.dtsi
>> +
>>   NANOPI-R5C
>>   M:    Tianling Shen <cnsztl@gmail.com>
>>   R:    Jonas Karlman <jonas@kwiboo.se>
>> diff --git a/configs/nanopi-r3s-rk3566_defconfig b/configs/nanopi-r3s- 
>> rk3566_defconfig
>> new file mode 100644
>> index 0000000000..f21c703ca7
>> --- /dev/null
>> +++ b/configs/nanopi-r3s-rk3566_defconfig
>> @@ -0,0 +1,74 @@
>> +CONFIG_ARM=y
>> +CONFIG_SKIP_LOWLEVEL_INIT=y
>> +CONFIG_COUNTER_FREQUENCY=24000000
>> +CONFIG_ARCH_ROCKCHIP=y
>> +CONFIG_DEFAULT_DEVICE_TREE="rockchip/rk3566-nanopi-r3s"
>> +CONFIG_ROCKCHIP_RK3568=y
>> +CONFIG_SPL_SERIAL=y
>> +CONFIG_SYS_LOAD_ADDR=0xc00800
>> +CONFIG_DEBUG_UART_BASE=0xFE660000
>> +CONFIG_DEBUG_UART_CLOCK=24000000
>> +CONFIG_PCI=y
>> +CONFIG_DEBUG_UART=y
>> +CONFIG_FIT=y
>> +CONFIG_FIT_VERBOSE=y
>> +CONFIG_SPL_FIT_SIGNATURE=y
>> +CONFIG_SPL_LOAD_FIT=y
>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>> +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3566-nanopi-r3s.dtb"
>> +# CONFIG_DISPLAY_CPUINFO is not set
>> +CONFIG_DISPLAY_BOARDINFO_LATE=y
>> +CONFIG_SPL_MAX_SIZE=0x40000
>> +CONFIG_SPL_PAD_TO=0x7f8000
>> +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
>> +CONFIG_SPL_ATF=y
>> +CONFIG_CMD_GPIO=y
>> +CONFIG_CMD_GPT=y
>> +CONFIG_CMD_I2C=y
>> +CONFIG_CMD_MMC=y
>> +CONFIG_CMD_PCI=y
>> +CONFIG_CMD_USB=y
>> +CONFIG_CMD_PMIC=y
>> +CONFIG_CMD_REGULATOR=y
>> +# CONFIG_SPL_DOS_PARTITION is not set
>> +CONFIG_SPL_OF_CONTROL=y
>> +CONFIG_OF_LIVE=y
>> +CONFIG_OF_SPL_REMOVE_PROPS="clock-names interrupt-parent assigned- 
>> clocks assigned-clock-rates assigned-clock-parents"
>> +CONFIG_SPL_DM_SEQ_ALIAS=y
>> +CONFIG_SPL_REGMAP=y
>> +CONFIG_SPL_SYSCON=y
>> +CONFIG_SPL_CLK=y
>> +CONFIG_ROCKCHIP_GPIO=y
>> +CONFIG_SYS_I2C_ROCKCHIP=y
>> +CONFIG_LED=y
>> +CONFIG_LED_GPIO=y
>> +CONFIG_MISC=y
>> +CONFIG_SUPPORT_EMMC_RPMB=y
>> +CONFIG_MMC_DW=y
>> +CONFIG_MMC_DW_ROCKCHIP=y
>> +CONFIG_MMC_SDHCI=y
>> +CONFIG_MMC_SDHCI_SDMA=y
>> +CONFIG_MMC_SDHCI_ROCKCHIP=y
>> +CONFIG_PHY_REALTEK=y
>> +CONFIG_DWC_ETH_QOS=y
>> +CONFIG_DWC_ETH_QOS_ROCKCHIP=y
>> +CONFIG_RTL8169=y
>> +CONFIG_NVME_PCI=y
> 
> I didn't see any PCIe connector for an NVMe on the R3S[1]? Did I miss 
> something? The wiki seems to indicate there may be an NVME but that 
> could be just a bad copy-pasting[2]?

Yes, I checked this board and it does not have NVMe slot. I will send a 
new patch to disable it.

Thanks,
Tianling.

> 
> [1] https://wiki.friendlyelec.com/wiki/index.php/NanoPi_R3S
> [2] https://wiki.friendlyelec.com/wiki/index.php/ 
> NanoPi_R3S#Expand_Docker_Storage
> 
> Cheers,
> Quentin