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

[Denzeel Oliva <wachiturroxd150@gmail.com>]

On Thu, Feb 13, 2025 at 07:38:35AM +0000, Tudor Ambarus wrote:
> > +		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.

Not all Exynos SoCs will follow the same order

> <&cmu_peric0 CLK_GOUT_PERIC0_TOP0_IPCLK_4>;

Is

GATE(CLK_GOUT_PERIC0_TOP0_IPCLK_4, "gout_peric0_top0_ipclk_4",
     "dout_peric0_uart_dbg",
     CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_4,
     21, 0, 0), [Mainline CLK]

You can find it in the cmucal-node.c driver downstream of the kernel. [0]

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

Because it is simply displayed in the exynos9830-usi.dtsi [1]

> > +		};
> > +
> > +		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?

As shown in the node it only uses i2c which
corresponds to the exynos9830-usi.dts. [2]

> > +		};
> > +
> 
> 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?

Here :) [3]

> Cheers,
> ta

[0] https://github.com/ExtremeXT/android_kernel_samsung_exynos990/blob/69515fbb7a4395898c05a8624f76a12afbac11c5/drivers/soc/samsung/cal-if/exynos9830/cmucal-node.c#L2719
[1] https://github.com/pascua28/android_kernel_samsung_s20fe/blob/3be539e9cd22b89ba3cc8282945a0c46ff27341d/arch/arm64/boot/dts/exynos/exynos9830-usi.dtsi#L1954
[2] https://github.com/pascua28/android_kernel_samsung_s20fe/blob/3be539e9cd22b89ba3cc8282945a0c46ff27341d/arch/arm64/boot/dts/exynos/exynos9830-usi.dtsi#L170
[3] https://github.com/pascua28/android_kernel_samsung_s20fe/blob/3be539e9cd22b89ba3cc8282945a0c46ff27341d/arch/arm64/boot/dts/exynos/exynos9830-usi.dtsi#L1638