Re: [PATCH 2/2] arm64: dts: rockchip: Add FriendlyARM NanoPi R3S board

[Jonas Karlman <jonas@kwiboo.se>]

Hi Tianling,

On 2024-10-22 19:38, Tianling Shen wrote:
> Hi Jonas,
> 
> On Wed, Oct 23, 2024 at 12:49 AM Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Hi Tianling,
>>
>> On 2024-10-20 19:39, 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/arm64/boot/dts/rockchip/Makefile         |   1 +
>>>  .../boot/dts/rockchip/rk3566-nanopi-r3s.dts   | 554 ++++++++++++++++++
>>>  2 files changed, 555 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-nanopi-r3s.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
>>> index 8e1025cb5d20..e641033d23d1 100644
>>> --- a/arch/arm64/boot/dts/rockchip/Makefile
>>> +++ b/arch/arm64/boot/dts/rockchip/Makefile
>>> @@ -109,6 +109,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-soquartz-model-a.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-box-demo.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-lckfb-tspi.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-lubancat-1.dtb
>>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-nanopi-r3s.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-fastrhino-r66s.dtb
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-nanopi-r3s.dts b/arch/arm64/boot/dts/rockchip/rk3566-nanopi-r3s.dts
>>> new file mode 100644
>>> index 000000000000..951dd9bcb4b0
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-nanopi-r3s.dts
>>> @@ -0,0 +1,554 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>>> +/*
>>> + * Copyright (c) 2020 Rockchip Electronics Co., Ltd.
>>> + *
>>> + * Copyright (c) 2024 FriendlyElec Computer Tech. Co., Ltd.
>>> + * (http://www.friendlyarm.com/

This could probably be updated to use https://www.friendlyelec.com/

>>> + *
>>> + * Copyright (c) 2024 Tianling Shen <cnsztl@gmail.com>
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/input/input.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +#include <dt-bindings/pinctrl/rockchip.h>
>>> +#include <dt-bindings/soc/rockchip,vop2.h>
>>> +#include "rk3566.dtsi"
>>> +
>>> +/ {
>>> +     model = "FriendlyARM NanoPi R3S";
>>
>> Please use the marketing name, and the name added in binding:
>>
>>    FriendlyElec NanoPi R3S
> 
> Sorry for the copy & paste error, will fix it in new patch.

The commit subject could probably also be updated :-)

> 
>>
>>> +     compatible = "friendlyarm,nanopi-r3s", "rockchip,rk3566";
>>> +
>>> +     aliases {
>>> +             ethernet0 = &gmac1;
>>> +             mmc0 = &sdmmc0;
>>> +             mmc1 = &sdhci;
>>
>> This board comes with on-board eMMC this please use mmc0 as alias for
>> &sdhci; and mmc1 for sd-card.
> 
> Is using mmc0 for sdhci a common practice for eMMC boards?
> The eMMC module on R3S is optional and users may get boards without it.

Typically any non-removable storage (emmc) is listed before removable
storage (sd-card) options. Also U-Boot will try to override and use
mmc0=sdhci and mmc1=sdmmc0 for all rk356x boards, so for EFI booing
using control FDT the alias may be different.

Regards,
Jonas

[snip]