linux-kernel - Re: [PATCH] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc node

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

Message-ID: <4514845.zXnORWrf4K@diego>
Date: Thu, 01 Feb 2024 09:41:31 +0100
From: Heiko Stübner <heiko@...ech.de>
To: Kever Yang <kever.yang@...k-chips.com>, Ondrej Jirman <megi@....cz>
Cc: linux-rockchip@...ts.infradead.org,
 Kever Yang <kever.yang@...k-chips.com>,
 Christopher Obbard <chris.obbard@...labora.com>,
 Conor Dooley <conor+dt@...nel.org>,
 Cristian Ciocaltea <cristian.ciocaltea@...labora.com>,
 Dragan Simic <dsimic@...jaro.org>, FUKAUMI Naoki <naoki@...xa.com>,
 Jagan Teki <jagan@...eble.ai>, John Clark <inindev@...il.com>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Lucas Tanure <lucas.tanure@...labora.com>, Rob Herring <robh+dt@...nel.org>,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 Shreeya Patel <shreeya.patel@...labora.com>,
 Tamás Szűcs <tszucs@...tonmail.ch>,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, quentin.schulz@...obroma-systems.com
Subject:
 Re: [PATCH] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc
 node

Hi Kever,

Am Donnerstag, 1. Februar 2024, 04:46:21 CET schrieb Kever Yang:
> The sdmmc node already have a "&sdmmc_det" for pinctrl which switch the
> GPIO0A4 to sdmmc detect function, no need to define a separate "cd-gpios".

just to make sure, did you test this on actual hardware?
Because there might be differences in behaviour.

> RK3588 has force_jtage feature which is enable JTAG function via sdmmc
> pins automatically when there is no SD card insert, this feature will
> need the GPIO0A4 works in sdmmc_det function like other mmc signal instead
> of GPIO function, or else the force_jtag can not auto be disabled when
> SD card insert.

We disable the jtag switching by default [0] ;-) .
And there are very good reasons for it too:


(1) JTAG is very much a debug feature, that the normal user will not need.
Especially not in a finished product. If a developer is debugging _that_
deep and needs jtag, they can enable it in their debug build.


(2) Randomly enabling features that may compromise security.
Why go through all the hoops of doing things like secure boot, signed
images and everything, just to have the kernel then export direct access
to the hardware on sd-card pins. If one wants to expose JTAG somewhere
this should be conscious choice and devs should not need to fork their
kernel just to shut down unwanted security-critical functionality.


(3) It affects board layouts _not following_ the standard layout.
Nobody is forcing board-designers to use Rockchip's desired pin
for card-detection. Some designer may just select a different pin
or a board could go without card-detect at all - see rk3588-jaguar.
These are both valid use-cases that need to be supported.


Heiko


[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6f6878ec6faf16a5f36761c93da6ea9cf09adb33


> ---
> 
>  arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts | 1 -
>  arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts     | 1 -
>  arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts         | 1 -
>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts        | 1 -
>  4 files changed, 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> index 3e660ff6cd5ff..1b606ea5b6cf2 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> @@ -444,7 +444,6 @@ &sdhci {
>  &sdmmc {
>  	bus-width = <4>;
>  	cap-sd-highspeed;
> -	cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>;
>  	disable-wp;
>  	max-frequency = <150000000>;
>  	no-sdio;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
> index 87a0abf95f7d4..67414d72e2b6e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
> @@ -429,7 +429,6 @@ &sdhci {
>  &sdmmc {
>  	bus-width = <4>;
>  	cap-sd-highspeed;
> -	cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>;
>  	disable-wp;
>  	max-frequency = <150000000>;
>  	no-sdio;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> index a0e303c3a1dc6..25a82008e4f76 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> @@ -371,7 +371,6 @@ &sdmmc {
>  	bus-width = <4>;
>  	cap-mmc-highspeed;
>  	cap-sd-highspeed;
> -	cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>;
>  	disable-wp;
>  	sd-uhs-sdr104;
>  	vmmc-supply = <&vcc_3v3_s3>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> index 2002fd0221fa3..00afb90d4eb10 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> @@ -366,7 +366,6 @@ &sdmmc {
>  	bus-width = <4>;
>  	cap-mmc-highspeed;
>  	cap-sd-highspeed;
> -	cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>;
>  	disable-wp;
>  	max-frequency = <150000000>;
>  	no-sdio;
>