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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z67f+lDxISVubiJJ@droid-r8s>
Date: Fri, 14 Feb 2025 06:17:30 +0000
From: Denzeel Oliva <wachiturroxd150@...il.com>
To: Tudor Ambarus <tudor.ambarus@...aro.org>
Cc: alim.akhtar@...sung.com, conor+dt@...nel.org,
	devicetree@...r.kernel.org, gregkh@...uxfoundation.org,
	jirislaby@...nel.org, krzk+dt@...nel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-samsung-soc@...r.kernel.org, linux-serial@...r.kernel.org,
	robh@...nel.org, semen.protsenko@...aro.org,
	wachiturroxd150@...il.com
Subject: Re: [PATCH v1 3/3] arm64: dts: exynos990: define all PERIC USI nodes

On Thu, Feb 13, 2025 at 07:38:35AM +0000, Tudor Ambarus wrote:
> > +		usi_uart: usi@...400c0 {
> > +			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@...40000 {
> > +				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@...500c0 {
> 
> cut
> 
> > +
> > +			hsi2c_0: i2c@...50000 {
> 
> cut
> 
> > +
> > +			spi_0: spi@...50000 {
> 
> cut
> 
> > +			serial_2: serial@...50000 {
> 
> why not serial_0 since you're in USI0.

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

> > +		};
> > +
> > +		usi_i2c_0: usi@...600c0 {
> > +			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@...60000 {
> > +				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@...e0000 {
> > +				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