Re: [PATCH v3 10/10] riscv: dts: eswin: add HiFive Premier P550 board device tree

[Samuel Holland]

Hi Ariel,

On 2025-04-14 7:55 AM, Ariel D'Alessandro wrote:
> Hi Pinkesh,
> 
> On 4/10/25 12:25 PM, Pinkesh Vaghela wrote:
>> From: Min Lin <linmin@xxxxxxxxxxxxxxxxxx>
>>
>> Add initial board data for HiFive Premier P550 Development board
>>
>> Currently the data populated in this DT file describes the board
>> DRAM configuration, UART and GPIO.
>>
>> Signed-off-by: Min Lin <linmin@xxxxxxxxxxxxxxxxxx>
>> Co-developed-by: Pinkesh Vaghela <pinkesh.vaghela@xxxxxxxxxxxxxx>
>> Signed-off-by: Pinkesh Vaghela <pinkesh.vaghela@xxxxxxxxxxxxxx>
>> Reviewed-by: Samuel Holland <samuel.holland@xxxxxxxxxx>
>> Tested-by: Samuel Holland <samuel.holland@xxxxxxxxxx>
>> ---
>>   arch/riscv/boot/dts/Makefile                  |  1 +
>>   arch/riscv/boot/dts/eswin/Makefile            |  2 ++
>>   .../dts/eswin/eic7700-hifive-premier-p550.dts | 29 +++++++++++++++++++
>>   3 files changed, 32 insertions(+)
>>   create mode 100644 arch/riscv/boot/dts/eswin/Makefile
>>   create mode 100644 arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
>>
>> diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
>> index 64a898da9aee..29a97a663ea2 100644
>> --- a/arch/riscv/boot/dts/Makefile
>> +++ b/arch/riscv/boot/dts/Makefile
>> @@ -1,6 +1,7 @@
>>   # SPDX-License-Identifier: GPL-2.0
>>   subdir-y += allwinner
>>   subdir-y += canaan
>> +subdir-y += eswin
>>   subdir-y += microchip
>>   subdir-y += renesas
>>   subdir-y += sifive
>> diff --git a/arch/riscv/boot/dts/eswin/Makefile b/arch/riscv/boot/dts/eswin/
>> Makefile
>> new file mode 100644
>> index 000000000000..224101ae471e
>> --- /dev/null
>> +++ b/arch/riscv/boot/dts/eswin/Makefile
>> @@ -0,0 +1,2 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +dtb-$(CONFIG_ARCH_ESWIN) += eic7700-hifive-premier-p550.dtb
>> diff --git a/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts b/arch/
>> riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
>> new file mode 100644
>> index 000000000000..131ed1fc6b2e
>> --- /dev/null
>> +++ b/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
>> @@ -0,0 +1,29 @@
>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>> +/*
>> + * Copyright (c) 2024, Beijing ESWIN Computing Technology Co., Ltd.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "eic7700.dtsi"
>> +
>> +/ {
>> +    compatible = "sifive,hifive-premier-p550", "eswin,eic7700";
>> +    model = "SiFive HiFive Premier P550";
>> +
>> +    aliases {
>> +        serial0 = &uart0;
>> +    };
>> +
>> +    chosen {
>> +        stdout-path = "serial0:115200n8";
>> +    };
>> +};
>> +
>> +&uart0 {
>> +    status = "okay";
>> +};
>> +
>> +&uart2 {
>> +    status = "okay";
>> +};
> 
> Although commit log says that this includes DRAM configuration, looks like it's
> missing? In order to test this patchset, had to add this following memory
> definition (picked from vendor kernel repository):
> 
>     L50: memory@80000000 {
>             compatible = "sifive,axi4-mem-port", "sifive,axi4-port",
> "sifive,mem-port";
>             device_type = "memory";
>             reg = <0x0 0x80000000 0x7f 0x80000000>;
>             sifive,port-width-bytes = <32>;
>     };

That is a misstatement in the commit message. The memory node is not included in
the static devicetree because the amount of RAM installed on the board is
variable. It is the responsibility of firmware to provide the memory map, either
through EFI or by patching the memory node into the DT at runtime. I believe the
current BSP U-Boot does the former but not the latter.

Regards,
Samuel