Re: [PATCH v1 3/3] arm64: dts: exynos990: define all PERIC USI nodes

From: Tudor Ambarus
Date: Thu Feb 13 2025 - 02:38:53 EST




On 2/12/25 11:40 PM, Denzeel Oliva wrote:
> Universal Serial Interface (USI) supports three types of serial interface
> such as Universal Asynchronous Receiver and Transmitter (UART), Serial
> Peripheral Interface (SPI), and Inter-Integrated Circuit (I2C).
> Each protocols can be working independently and configured as one of
> those using external configuration inputs.
>
> Exynos990 SoC defines 18 USI nodes in PERIC0/1 blocks.
> Nodes have different depths from 64-256 bytes.

for the reviewer's peace of mind you shall specify whether you tested at
least an i2c, uart and spi node.

>
> Signed-off-by: Denzeel Oliva <wachiturroxd150@xxxxxxxxx>
> ---
> arch/arm64/boot/dts/exynos/exynos990.dtsi | 1693 +++++++++++++++++++++
> 1 file changed, 1693 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos990.dtsi b/arch/arm64/boot/dts/exynos/exynos990.dtsi
> index aa056fdae..22ec92a45 100644
> --- a/arch/arm64/boot/dts/exynos/exynos990.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos990.dtsi
> @@ -7,6 +7,7 @@
>
> #include <dt-bindings/clock/samsung,exynos990.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/soc/samsung,exynos-usi.h>
>
> / {
> compatible = "samsung,exynos990";
> @@ -248,6 +249,808 @@ sysreg_peric0: syscon@10420000 {
> clocks = <&cmu_peric0 CLK_GOUT_PERIC0_SYSREG_PCLK>;
> };
>
> + usi_uart: usi@105400c0 {
> + compatible = "samsung,exynos990-usi", "samsung,exynos850-usi";
> + reg = <0x105400c0 0x20>;
> + samsung,sysreg = <&sysreg_peric0 0x1000>;
> + samsung,mode = <USI_V2_UART>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + clocks = <&cmu_peric0 CLK_GOUT_PERIC0_TOP0_PCLK_4>,
> + <&cmu_peric0 CLK_GOUT_PERIC0_TOP0_IPCLK_4>;
> + clock-names = "pclk", "ipclk";
> + status = "disabled";
> +
> + serial_0: serial@10540000 {
> + compatible = "samsung,exynos990-uart",
> + "samsung,exynos8895-uart";
> + reg = <0x10540000 0xc0>;
> + interrupts = <GIC_SPI 391 IRQ_TYPE_LEVEL_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart0_bus>;
> + clocks = <&cmu_peric0 CLK_GOUT_PERIC0_TOP0_PCLK_4>,
> + <&cmu_peric0 CLK_GOUT_PERIC0_TOP0_IPCLK_4>;
> + clock-names = "uart", "clk_uart_baud0";
> + samsung,uart-fifosize = <256>;
> + status = "disabled";

node properties shall be specified in a specific order. Follow similar
nodes that are already accepted, gs101 is one.

> + };
> + };
> +
> + usi0: usi@105500c0 {

cut

> +
> + hsi2c_0: i2c@10550000 {

cut

> +
> + spi_0: spi@10550000 {

cut

> + serial_2: serial@10550000 {

why not serial_0 since you're in USI0.

> + };
> +
> + usi_i2c_0: usi@105600c0 {
> + compatible = "samsung,exynos990-usi", "samsung,exynos850-usi";
> + reg = <0x105600c0 0x20>;
> + samsung,sysreg = <&sysreg_peric0 0x1008>;
> + samsung,mode = <USI_V2_I2C>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + clocks = <&cmu_peric0 CLK_GOUT_PERIC0_TOP0_PCLK_6>,
> + <&cmu_peric0 CLK_GOUT_PERIC0_TOP0_IPCLK_6>;
> + clock-names = "pclk", "ipclk";
> + status = "disabled";
> +
> + hsi2c_1: i2c@10560000 {
> + compatible = "samsung,exynos990-hsi2c",
> + "samsung,exynosautov9-hsi2c";
> + reg = <0x10560000 0xc0>;
> + interrupts = <GIC_SPI 398 IRQ_TYPE_LEVEL_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&hsi2c1_bus>;
> + clocks = <&cmu_peric0 CLK_GOUT_PERIC0_TOP0_IPCLK_6>,
> + <&cmu_peric0 CLK_GOUT_PERIC0_TOP0_PCLK_6>;
> + clock-names = "hsi2c", "hsi2c_pclk";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };

shouldn't you define serial and SPI too?

> + };
> +

cut

> + spi_8: spi@108e0000 {
> + compatible = "samsung,exynos990-spi";
> + reg = <0x108e0000 0x30>;
> + interrupts = <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi8_bus>;
> + clocks = <&cmu_peric1 CLK_GOUT_PERIC1_TOP0_PCLK_14>,
> + <&cmu_peric1 CLK_GOUT_PERIC1_TOP0_IPCLK_14>;
> + clock-names = "spi", "spi_busclk0";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + fifo-depth = <256>;

that's a first. Does downstream define any SPI node with 256 bytes
FIFOs? Would you please point me to the downstream sources?

Cheers,
ta