lkml.org 
[lkml]   [2024]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc node
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;
>>
>

\
 
 \ /
  Last update: 2024-05-27 14:47    [W:0.069 / U:0.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site