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

[Heiko Stübner]

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;
>