| Message ID | 20230315072902.9298-12-ychuang570808@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce Nuvoton ma35d1 SoC | expand |
Hi Lee, Thanks for your attentions. On 2023/3/17 上午 12:44, Lee Jones wrote: > On Thu, 16 Mar 2023, Arnd Bergmann wrote: > >> On Wed, Mar 15, 2023, at 08:28, Jacky Huang wrote: >>> + mem: memory@80000000 { >>> + device_type = "memory"; >>> + reg = <0x00000000 0x80000000 0 0x20000000>; /* 512M DRAM */ >>> + }; >>> +}; >> In most machines, the memory size is detected by the boot loader >> and filled in the dtb in memory before starting the kernel, so >> you should not need two separate files here for the two common >> memory configurations. >> >> Since the machine is called 'som', I would assume that this is a >> module that is integrated on another board, so more commonly one >> would have a dtsi file for the som in addition to the one for the >> soc, and have all the components of the module listed in this >> file, while the dts file that includes the som.dtsi lists the >> devices on the carrier board and enables the on-chip devices >> that are connected to the outside. > It's using syscon and simple-mfd by the looks of it. > > -- > Lee Jones [李琼斯] We just copy from what other chips's dts have done. This seems be defined in Documentation\devicetree\bindings\numa.txt. Best regards, Jacky Huang
On Sat, Mar 18, 2023, at 14:17, Jacky Huang wrote: > On 2023/3/16 下午 10:17, Arnd Bergmann wrote: >> On Wed, Mar 15, 2023, at 08:28, Jacky Huang wrote: >>> + mem: memory@80000000 { >>> + device_type = "memory"; >>> + reg = <0x00000000 0x80000000 0 0x20000000>; /* 512M DRAM */ >>> + }; >>> +}; >> In most machines, the memory size is detected by the boot loader >> and filled in the dtb in memory before starting the kernel, so >> you should not need two separate files here for the two common >> memory configurations. > > > On ma35d1, memory size is determined early before uboot. > > BL1 (MaskROM boot code) -> BL2 (arm-trust-firmware) -> BL32 (op-tee) & > BL33 (uboot). > The DDR was initialized in BL2 stage with a selected DDR setting, which > is hard coded, including DDR size. > > We searched the arm64 dts and found that almost all vendors claimed > memory size in board level dtsi/dts. This seems to be common. > > So, can we have it unchanged? I see the memory size encoded in about one out of three .dts files, which is more than I expected. It's clearly not harmful to have it listed in the dts, it just shouldn't be necessary. If it helps you with your current u-boot, then leave it in, but consider adding detection logic into u-boot so it can override the value in the dtb file at boot time. >> Since the machine is called 'som', I would assume that this is a >> module that is integrated on another board, so more commonly one >> would have a dtsi file for the som in addition to the one for the >> soc, and have all the components of the module listed in this >> file, while the dts file that includes the som.dtsi lists the >> devices on the carrier board and enables the on-chip devices >> that are connected to the outside. >> > > You are right, ma35d1 som have a base board, and a cpu board on it. > > It is a good suggestion that we should have a dtsi for som base board. > > Consider that we are in the initial submit, and such a dtsi will be an empty > file at this stage. So, I would like to do it when peripheral drivers > upstream started. Is it ok? It's not a big deal either way. I if you want to keep it only with one dts file and one dtsi file, that's fine, but maybe rename the dts file based on the name of the carrier rather than the SoM in this case. Arnd
On 18/03/2023 07:07, Jacky Huang wrote: > >> >>> + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_RAW(0x13) | >>> + IRQ_TYPE_LEVEL_HIGH)>; >>> + }; >>> + >>> + uart0:serial@40700000 { >>> + compatible = "nuvoton,ma35d1-uart"; >>> + reg = <0x0 0x40700000 0x0 0x100>; >>> + interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>; >>> + clocks = <&clk UART0_GATE>; >>> + status = "okay"; >> Why? Drop the line... or convert it to disabled. Otherwise, why every >> SoC has serial0 enabled? Is it used internally? > > > uart0 is on all the way since this SoC booting from the MaskROM boot code, > > load arm-trusted-firmware, load bootloader, and finally load linux kernel. > > uart0 is also the Linux console. Are you sure? Maybe my board has UART0 disconnected. Best regards, Krzysztof
Dear Krzyszto, On 2023/3/19 下午 07:06, Krzysztof Kozlowski wrote: > On 18/03/2023 07:07, Jacky Huang wrote: >>>> + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_RAW(0x13) | >>>> + IRQ_TYPE_LEVEL_HIGH)>; >>>> + }; >>>> + >>>> + uart0:serial@40700000 { >>>> + compatible = "nuvoton,ma35d1-uart"; >>>> + reg = <0x0 0x40700000 0x0 0x100>; >>>> + interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>; >>>> + clocks = <&clk UART0_GATE>; >>>> + status = "okay"; >>> Why? Drop the line... or convert it to disabled. Otherwise, why every >>> SoC has serial0 enabled? Is it used internally? >> >> uart0 is on all the way since this SoC booting from the MaskROM boot code, >> >> load arm-trusted-firmware, load bootloader, and finally load linux kernel. >> >> uart0 is also the Linux console. > Are you sure? Maybe my board has UART0 disconnected. > > Best regards, > Krzysztof OK, I will have the uart0 disabled in dtsi, and enabled it in dts. Best regards, Jacky Huang
Dear Arnd, On 2023/3/18 下午 10:04, Arnd Bergmann wrote: > On Sat, Mar 18, 2023, at 14:17, Jacky Huang wrote: >> On 2023/3/16 下午 10:17, Arnd Bergmann wrote: >>> On Wed, Mar 15, 2023, at 08:28, Jacky Huang wrote: >>>> + mem: memory@80000000 { >>>> + device_type = "memory"; >>>> + reg = <0x00000000 0x80000000 0 0x20000000>; /* 512M DRAM */ >>>> + }; >>>> +}; >>> In most machines, the memory size is detected by the boot loader >>> and filled in the dtb in memory before starting the kernel, so >>> you should not need two separate files here for the two common >>> memory configurations. >> >> On ma35d1, memory size is determined early before uboot. >> >> BL1 (MaskROM boot code) -> BL2 (arm-trust-firmware) -> BL32 (op-tee) & >> BL33 (uboot). >> The DDR was initialized in BL2 stage with a selected DDR setting, which >> is hard coded, including DDR size. >> >> We searched the arm64 dts and found that almost all vendors claimed >> memory size in board level dtsi/dts. This seems to be common. >> >> So, can we have it unchanged? > I see the memory size encoded in about one out of three .dts files, > which is more than I expected. It's clearly not harmful to have it > listed in the dts, it just shouldn't be necessary. > > If it helps you with your current u-boot, then leave it in, but > consider adding detection logic into u-boot so it can override > the value in the dtb file at boot time. Thank you for your understanding. As more drivers are added, I think this memory size encoded will look less conspicuous. In fact, in the previous arm9 project, we did detect the memory size by uboot, and then passed it to the kernel. If there is a need in the future, we will consider to support it in ma35d1. >>> Since the machine is called 'som', I would assume that this is a >>> module that is integrated on another board, so more commonly one >>> would have a dtsi file for the som in addition to the one for the >>> soc, and have all the components of the module listed in this >>> file, while the dts file that includes the som.dtsi lists the >>> devices on the carrier board and enables the on-chip devices >>> that are connected to the outside. >>> >> You are right, ma35d1 som have a base board, and a cpu board on it. >> >> It is a good suggestion that we should have a dtsi for som base board. >> >> Consider that we are in the initial submit, and such a dtsi will be an empty >> file at this stage. So, I would like to do it when peripheral drivers >> upstream started. Is it ok? > It's not a big deal either way. I if you want to keep it only with > one dts file and one dtsi file, that's fine, but maybe rename the dts > file based on the name of the carrier rather than the SoM in this > case. > > Arnd Thank you. As the dts names are consistent with the ma35d1 BSP on linux-5.10.y, we would like to keep the consistence still. Best regards, Jacky Huang
diff --git a/arch/arm64/boot/dts/nuvoton/Makefile b/arch/arm64/boot/dts/nuvoton/Makefile index a99dab90472a..c11ab4eac9c7 100644 --- a/arch/arm64/boot/dts/nuvoton/Makefile +++ b/arch/arm64/boot/dts/nuvoton/Makefile @@ -1,2 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 dtb-$(CONFIG_ARCH_NPCM) += nuvoton-npcm845-evb.dtb +dtb-$(CONFIG_ARCH_NUVOTON) += ma35d1-iot-512m.dtb +dtb-$(CONFIG_ARCH_NUVOTON) += ma35d1-som-256m.dtb diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts new file mode 100644 index 000000000000..dffcaef1e6d8 --- /dev/null +++ b/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2023 Nuvoton Technology Corp. + * Author: Shan-Chun Hung <schung@nuvoton.com> + * Jacky huang <ychuang3@nuvoton.com> + */ + +/dts-v1/; +#include "ma35d1.dtsi" + +/ { + model = "Nuvoton MA35D1-IoT"; + compatible = "nuvoton,ma35d1-iot", "nuvoton,ma35d1"; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + mem: memory@80000000 { + device_type = "memory"; + reg = <0x00000000 0x80000000 0 0x20000000>; /* 512M DRAM */ + }; +}; + diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts new file mode 100644 index 000000000000..3e6c3d5469ac --- /dev/null +++ b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2023 Nuvoton Technology Corp. + * Author: Shan-Chun Hung <schung@nuvoton.com> + * Jacky huang <ychuang3@nuvoton.com> + */ + +/dts-v1/; +#include "ma35d1.dtsi" + +/ { + model = "Nuvoton MA35D1-SOM"; + compatible = "nuvoton,ma35d1-som", "nuvoton,ma35d1"; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + mem: memory@80000000 { + device_type = "memory"; + reg = <0x00000000 0x80000000 0 0x10000000>; /* 256M DRAM */ + }; +}; diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi new file mode 100644 index 000000000000..8c855f6b330a --- /dev/null +++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi @@ -0,0 +1,272 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2023 Nuvoton Technology Corp. + * Author: Shan-Chun Hung <schung@nuvoton.com> + * Jacky huang <ychuang3@nuvoton.com> + */ + +#include <dt-bindings/interrupt-controller/arm-gic.h> +#include <dt-bindings/input/input.h> +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/clock/nuvoton,ma35d1-clk.h> +#include <dt-bindings/reset/nuvoton,ma35d1-reset.h> + +/ { + compatible = "nuvoton,ma35d1"; + interrupt-parent = <&gic>; + #address-cells = <2>; + #size-cells = <2>; + + aliases { + serial0 = &uart0; + serial1 = &uart1; + serial2 = &uart2; + serial3 = &uart3; + serial4 = &uart4; + serial5 = &uart5; + serial6 = &uart6; + serial7 = &uart7; + serial8 = &uart8; + serial9 = &uart9; + serial10 = &uart10; + serial11 = &uart11; + serial12 = &uart12; + serial13 = &uart13; + serial14 = &uart14; + serial15 = &uart15; + serial16 = &uart16; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + cpus { + #address-cells = <2>; + #size-cells = <0>; + cpu0: cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a35"; + reg = <0x0 0x0>; + enable-method = "psci"; + next-level-cache = <&L2_0>; + }; + cpu1: cpu@1 { + device_type = "cpu"; + compatible = "arm,cortex-a35"; + reg = <0x0 0x1>; + enable-method = "psci"; + next-level-cache = <&L2_0>; + }; + L2_0: l2-cache0 { + compatible = "cache"; + cache-level = <2>; + }; + }; + + psci { + compatible = "arm,psci-0.2"; + method = "smc"; + }; + + clk_hxt: clock_hxt { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <24000000>; + clock-output-names = "clk_hxt"; + }; + + timer { + compatible = "arm,armv8-timer"; + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | + IRQ_TYPE_LEVEL_LOW)>, /* Physical Secure */ + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | + IRQ_TYPE_LEVEL_LOW)>, /* Physical Non-Secure */ + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | + IRQ_TYPE_LEVEL_LOW)>, /* Virtual */ + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | + IRQ_TYPE_LEVEL_LOW)>; /* Hypervisor */ + clock-frequency = <12000000>; + interrupt-parent = <&gic>; + }; + + sys: system-management@40460000 { + compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd"; + reg = <0x0 0x40460000 0x0 0x200>; + + reset: reset-controller { + compatible = "nuvoton,ma35d1-reset"; + regmap = <&sys>; + #reset-cells = <1>; + }; + }; + + clk: clock-controller@40460200 { + compatible = "nuvoton,ma35d1-clk", "syscon"; + reg = <0x00000000 0x40460200 0x0 0x100>; + #clock-cells = <1>; + clocks = <&clk_hxt>; + clock-names = "clk_hxt"; + assigned-clocks = <&clk CAPLL>, + <&clk DDRPLL>, + <&clk APLL>, + <&clk EPLL>, + <&clk VPLL>; + assigned-clock-rates = <800000000>, + <266000000>, + <180000000>, + <500000000>, + <102000000>; + nuvoton,pll-mode = <0>, <1>, <0>, <0>, <0>; + nuvoton,sys = <&sys>; + }; + + gic: interrupt-controller@50801000 { + compatible = "arm,gic-400"; + #interrupt-cells = <3>; + interrupt-parent = <&gic>; + interrupt-controller; + reg = <0x0 0x50801000 0 0x1000>, /* GICD */ + <0x0 0x50802000 0 0x2000>, /* GICC */ + <0x0 0x50804000 0 0x2000>, /* GICH */ + <0x0 0x50806000 0 0x2000>; /* GICV */ + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_RAW(0x13) | + IRQ_TYPE_LEVEL_HIGH)>; + }; + + uart0:serial@40700000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40700000 0x0 0x100>; + interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART0_GATE>; + status = "okay"; + }; + + uart1:serial@40710000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40710000 0x0 0x100>; + interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART1_GATE>; + status = "disabled"; + }; + + uart2:serial@40720000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40720000 0x0 0x100>; + interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART2_GATE>; + status = "disabled"; + }; + + uart3:serial@40730000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40730000 0x0 0x100>; + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART3_GATE>; + status = "disabled"; + }; + + uart4:serial@40740000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40740000 0x0 0x100>; + interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART4_GATE>; + status = "disabled"; + }; + + uart5:serial@40750000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40750000 0x0 0x100>; + interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART5_GATE>; + status = "disabled"; + }; + + uart6:serial@40760000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40760000 0x0 0x100>; + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART6_GATE>; + status = "disabled"; + }; + + uart7:serial@40770000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40770000 0x0 0x100>; + interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART7_GATE>; + status = "disabled"; + }; + + uart8:serial@40780000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40780000 0x0 0x100>; + interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART8_GATE>; + status = "disabled"; + }; + + uart9:serial@40790000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40790000 0x0 0x100>; + interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART9_GATE>; + status = "disabled"; + }; + + uart10:serial@407a0000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x407a0000 0x0 0x100>; + interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART10_GATE>; + status = "disabled"; + }; + + uart11:serial@407b0000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x407b0000 0x0 0x100>; + interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART11_GATE>; + status = "disabled"; + }; + + uart12:serial@407c0000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x407c0000 0x0 0x100>; + interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART12_GATE>; + status = "disabled"; + }; + + uart13:serial@407d0000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x407d0000 0x0 0x100>; + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART13_GATE>; + status = "disabled"; + }; + + uart14:serial@407e0000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x407e0000 0x0 0x100>; + interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART14_GATE>; + status = "disabled"; + }; + + uart15:serial@407f0000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x407f0000 0x0 0x100>; + interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART15_GATE>; + status = "disabled"; + }; + + uart16:serial@40880000 { + compatible = "nuvoton,ma35d1-uart"; + reg = <0x0 0x40880000 0x0 0x100>; + interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk UART16_GATE>; + status = "disabled"; + }; +};