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

["Chanho Park" <chanho61.park@samsung.com>]

> 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.

Okay. I'll move them in the exynosautov9.dtsi. I thought they're too long to put in the exynosautov9.dtsi and I also found a similar case such as exynos5433-bus.dtsi and exynos5433-tmu.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"

So, I need to add the compatible to the Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml, right?

- samsung,exynos850-usi   # for USIv2 (Exynos850, ExynosAutoV9)

To be>
- samsung,exynos850-usi
- samsung,exynosautov9-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.

I'll drop them.

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

I'll add "samsung,exynosautov9-uart" to the yaml file.

> 
> > +			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.

I'll separate it.

Best Regards,
Chanho Park