linux-kernel - Re: [PATCH] arm64: dts: exynos: gs101: 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: <073e5ef5-2a2e-4300-93d6-e25552276e13@linaro.org>
Date: Fri, 8 Mar 2024 17:45:02 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Tudor Ambarus <tudor.ambarus@...aro.org>, peter.griffin@...aro.org,
 robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org
Cc: alim.akhtar@...sung.com, linux-arm-kernel@...ts.infradead.org,
 linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, andre.draszik@...aro.org,
 willmcvicker@...gle.com, kernel-team@...roid.com
Subject: Re: [PATCH] arm64: dts: exynos: gs101: define all PERIC USI nodes

On 07/03/2024 14:59, Tudor Ambarus wrote:
> Universal Serial Interface (USI) supports three types of serial
> interface such as UART, SPI and I2C. Each protocol works independently.
> USI can be configured to work as one of these protocols. Define all the
> USI nodes from the PERIC blocks (USI0-14), in all their possible
> configurations. These blocks have the TX/RX FIFO depth of 64 bytes.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@...aro.org>
> ---
> Please note that:
> - google,gs101-spi compatible was queued through the SPI tree:
>   https://lore.kernel.org/linux-arm-kernel/170742731537.2266792.3851016361229293846.b4-ty@kernel.org/
> - SPI dma properties were marked as not requiered. Queued through the
>   SPI tree:
>   https://lore.kernel.org/linux-spi/170967132774.228925.1759895846287455970.b4-ty@kernel.org/
> 
>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 782 +++++++++++++++++++
>  1 file changed, 782 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> index ee65ed9d2cfc..d7ecfbc7e440 100644
> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> @@ -373,6 +373,398 @@ pinctrl_peric0: pinctrl@...40000 {
>  			interrupts = <GIC_SPI 625 IRQ_TYPE_LEVEL_HIGH 0>;
>  		};
>  
> +		usi1: usi@...000c0 {
> +			compatible = "google,gs101-usi",
> +				     "samsung,exynos850-usi";

This should fit within 81 characters, which is pretty close to 80, so if
there is going to be any resend/new version, please join the lines.

> +			reg = <0x109000c0 0x20>;
> +			ranges;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_0>,
> +				 <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_0>;
> +			clock-names = "pclk", "ipclk";
> +			samsung,sysreg = <&sysreg_peric0 0x1000>;
> +			status = "disabled";
> +
> +			hsi2c_1: i2c@...00000 {
> +				compatible = "google,gs101-hsi2c",
> +					     "samsung,exynosautov9-hsi2c";
> +				reg = <0x10900000 0xc0>;
> +				interrupts = <GIC_SPI 635 IRQ_TYPE_LEVEL_HIGH 0>;

I wonder why we use four cells in GIC...

> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&hsi2c1_bus>;

Please reverse two lines, first pinctrl-0 then pinctrl-names. I know we
did not follow this convention till now, but at least new code can be
correct. Also clocks should be before pinctrl, so we keep some sort of
alphabetical order.

It is anyway too late in the cycle for me to pick it up, so no need to
hurry with resend.

Best regards,
Krzysztof