linux-kernel - Re: [PATCH] arm64: dts: rockchip: use cs-gpios for spi1 on ringneck

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] [day] [month] [year] [list]

Message-ID: <1925003.tdWV9SEqCh@phil>
Date: Mon, 23 Jun 2025 11:49:26 +0200
From: Heiko Stuebner <heiko@...ech.de>
To: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Quentin Schulz <quentin.schulz@...rry.de>,
 Jakob Unterwurzacher <jakob.unterwurzacher@...rry.de>,
 Lukasz Czechowski <lukasz.czechowski@...umatec.com>,
 Dragan Simic <dsimic@...jaro.org>, Diederik de Haas <didi.debian@...ow.org>,
 Farouk Bouabid <farouk.bouabid@...rry.de>, Johan Jonker <jbx6244@...il.com>,
 Jakob Unterwurzacher <jakobunt@...il.com>
Cc: stable@...r.kernel.org, Heiko Stuebner <heiko.stuebner@...rry.de>,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: dts: rockchip: use cs-gpios for spi1 on ringneck

Hi Jakob,

Am Freitag, 20. Juni 2025, 13:35:46 Mitteleuropäische Sommerzeit schrieb Jakob Unterwurzacher:
> From: Jakob Unterwurzacher <jakob.unterwurzacher@...rry.de>
> 
> Hardware CS has a very slow rise time of about 6us,
> causing transmission errors when CS does not reach
> high between transaction.
> 
> It looks like it's not driven actively when transitioning
> from low to high but switched to input, so only the CPU
> pull-up pulls it high, slowly. Transitions from high to low
> are fast. On the oscilloscope, CS looks like an irregular sawtooth
> pattern like this:
>                          _____
>               ^         /     |
>       ^      /|        /      |
>      /|     / |       /       |
>     / |    /  |      /        |
> ___/  |___/   |_____/         |___
> 
> With cs-gpios we have a CS rise time of about 20ns, as it should be,
> and CS looks rectangular.
> 
> This fixes the data errors when running a flashcp loop against a
> m25p40 spi flash.
> 
> With the Rockchip 6.1 kernel we see the same slow rise time, but
> for some reason CS is always high for long enough to reach a solid
> high.
> 
> The RK3399 and RK3588 SoCs use the same SPI driver, so we also
> checked our "Puma" (RK3399) and "Tiger" (RK3588) boards.
> They do not have this problem. Hardware CS rise time is good.
> 
> Fixes: c484cf93f61b ("arm64: dts: rockchip: add PX30-µQ7 (Ringneck) SoM with Haikou baseboard")
> Cc: stable@...r.kernel.org
> Reviewed-by: Quentin Schulz <quentin.schulz@...rry.de>
> Signed-off-by: Jakob Unterwurzacher <jakob.unterwurzacher@...rry.de>
> ---
>  .../boot/dts/rockchip/px30-ringneck.dtsi      | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> index ab232e5c7ad6..dcc62dd9b894 100644
> --- a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> @@ -379,6 +379,18 @@ pmic_int: pmic-int {
>  				<0 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>;
>  		};
>  	};
> +
> +	spi1 {
> +		spi1_csn0_gpio: spi1-csn0-gpio {
> +			rockchip,pins =
> +				<3 RK_PB1 RK_FUNC_GPIO &pcfg_pull_up_4ma>;
> +		};
> +
> +		spi1_csn1_gpio: spi1-csn1-gpio {

naming the node -gpio trigger the bot, I guess a better name would
be something with -pin at the end instead.

> +&spi1 {
> +	/*
> +	 * Hardware CS has a very slow rise time of about 6us,
> +	 * causing transmission errors.
> +	 * With cs-gpios we have a rise time of about 20ns.
> +	 */
> +	cs-gpios = <&gpio3 RK_PB1 GPIO_ACTIVE_LOW>, <&gpio3 RK_PB2 GPIO_ACTIVE_LOW>;

please also provide a
	pinctrl-names = "default"
here.
It feels more futur proof when overriding the pinctrl-0 entry
to also state we only expect that one.

> +	pinctrl-0 = <&spi1_clk &spi1_csn0_gpio &spi1_csn1_gpio &spi1_miso &spi1_mosi>;
> +};
> +
>  &tsadc {
>  	status = "okay";
>  };
>