Re: [PATCH 2/3] arm64: dts: exynosautov9: add exynosautov9-usi.dtsi

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 29/06/2022 03:56, Chanho Park 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.
> Exynos Auto v9 SoC support 12 USIs. When a USI uses two pins such as i2c
> and 3 wire uarts(RX/TX only), we can use remain two pins as i2c mode.
> So, we can define one USI node that includes serial/spi and hsi2c.
> usi_i2c nodes can be used only for i2c mode.
> 
> We can have below combinations for one USI.
> 1) The usi node is used either 4 pin uart or 4 pin spi
>  -> No usi_i2c can be used
> 2) The usi node is used 2 pin uart(RX/TX) and i2c(SDA/SCL)
>  -> usi_i2c should be enabled to use the latter i2c
> 3) The usi node is used i2c(SDA/SCL) and i2c(SDA/SCL)
>  -> usi_i2c should be enabled to use the latter i2c
> 
> By default, all USIs are initially set to uart mode by below setting.
> samsung,mode = <USI_V2_UART>;
> You can change it either USI_V2_SPI or USI_V2_I2C.
> 
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  .../boot/dts/exynos/exynosautov9-usi.dtsi     | 1127 +++++++++++++++++
>  1 file changed, 1127 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/exynos/exynosautov9-usi.dtsi

Put all this directly in exynosautov9.dtsi, because this is not a
re-usable piece among different DTSI.

> 
> diff --git a/arch/arm64/boot/dts/exynos/exynosautov9-usi.dtsi b/arch/arm64/boot/dts/exynos/exynosautov9-usi.dtsi
> new file mode 100644
> index 000000000000..0e4c6332770b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/exynos/exynosautov9-usi.dtsi
> @@ -0,0 +1,1127 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Samsung's ExynosAutov9 SoC USI device tree source
> + *
> + * Copyright (c) 2022 Samsung Electronics Co., Ltd.
> + *
> + * Samsung's ExynosAutov9 SoC USI(Universal Serial Interface, uart/spi/i2c)
> + * are listed as device tree nodes in this file.
> + */
> +
> +/* PERIC0 USIs */
> +&soc {
> +	syscon_peric0: syscon@10220000 {
> +		compatible = "samsung,exynosautov9-sysreg", "syscon";
> +		reg = <0x10220000 0x2000>;
> +	};
> +
> +	usi_0: usi@103000c0 {
> +		compatible = "samsung,exynos850-usi";

We should start adding dedicated compatible, so:
"samsung,exynosautov9-usi", "samsung,exynos850-usi"

> +		reg = <0x103000c0 0x20>;
> +		samsung,sysreg = <&syscon_peric0 0x1000>;
> +		samsung,mode = <USI_V2_UART>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +		clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PCLK_0>,
> +			 <&cmu_peric0 CLK_GOUT_PERIC0_IPCLK_0>;
> +		clock-names = "pclk", "ipclk";
> +		status = "disabled";
> +
> +		/* USI: UART */

Skip the comments, they are obvious from device node name. Long time ago
I was not advocating this, but I see it's benefits - much easier to
introduce changes to DTS or binding in case of some differences.

> +		serial_0: serial@10300000 {
> +			compatible = "samsung,exynos850-uart";

Here as well.

> +			reg = <0x10300000 0xc0>;
> +			interrupts = <GIC_SPI 345 IRQ_TYPE_LEVEL_HIGH>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&uart0_bus>;
> +			clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PCLK_0>,
> +				 <&cmu_peric0 CLK_GOUT_PERIC0_IPCLK_0>;
> +			clock-names = "uart", "clk_uart_baud0";
> +			samsung,uart-fifosize = <256>;
> +			status = "disabled";
> +		};
> +
> +		/* USI: SPI */
> +		spi_0: spi@10300000 {
> +			compatible = "samsung,exynosautov9-spi";
> +			reg = <0x10300000 0x30>;
> +			interrupts = <GIC_SPI 345 IRQ_TYPE_LEVEL_HIGH>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&spi0_bus &spi0_cs_func>;
> +			clocks = <&cmu_peric0 CLK_GOUT_PERIC0_IPCLK_0>,
> +				 <&cmu_peric0 CLK_DOUT_PERIC0_USI00_USI>,
> +				 <&cmu_peric0 CLK_GOUT_PERIC0_PCLK_0>;
> +			clock-names = "spi", "spi_busclk0", "spi_ioclk";
> +			samsung,spi-src-clk = <0>;
> +			dmas = <&pdma0 1 &pdma0 0>;

These are two separate phandles <>, not one.

> +			dma-names = "tx", "rx";
> +			num-cs = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		/* USI: I2C */
> +		hsi2c_0: hsi2c@10300000 {
> +			compatible = "samsung,exynosautov9-hsi2c";
> +			reg = <0x10300000 0xc0>;
> +			interrupts = <GIC_SPI 345 IRQ_TYPE_LEVEL_HIGH>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&hsi2c0_bus>;
> +			clocks = <&cmu_peric0 CLK_GOUT_PERIC0_IPCLK_0>,
> +				 <&cmu_peric0 CLK_GOUT_PERIC0_PCLK_0>;
> +			clock-names = "hsi2c", "hsi2c_pclk";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +	};




Best regards,
Krzysztof