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

[Kever Yang]

Hi Heiko,

On 2024/2/1 16:41, Heiko Stübner wrote:
> 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.

We use this feature in vendor kernel for many boards.

For mainline support, there are 15 rk3588/rk3588 boards available, and 
10 of them

enable sdmmc node in dts, and 4 boards define "cd-gpios" while the 
hardware do use GPIO0A4,

and 1 board(rk3588-jaguar) using "broken-cd", and the other 5 boards are 
using default "&sdmmc_det"

  with the same hardware design.

If the hardware is using GPIO0A4(SDMMC_DET function IO) for sdmmc 
detect, then no need to define "cd-gpios";

if the hardware is not using GPIO0A4 for sdmmc detect, then the 
"cd-gpios" or "broken-cd" is needed.

So this patch is to sync up to use the "&sdmmc_det" when the IO is using 
the one has SDMMC_DET function.

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

I know you have disable the force_jtag by default, and I didn't want to 
change this.

As you have said we may need to enable it for debug, we suppose to only 
have to revert

the disable force_jtag patch and then it works without affect the 
default sdmmc function.

The sdmmc function is broken if we enable force_jtag for debug, and this 
patch can fix it.

> (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.

You are right, "cd-gpios" and "broken-cd" are available for boards have 
different design,

and this patch is for the boards with SDMMC_DET(GPIO0A4) as sdmmc 
detect, they should go to

the default "&sdmmc_det" in sdmmc node.


Thanks,

- Kever

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