* Re: [PATCH] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc node [not found] <20240201034621.1970279-1-kever.yang@rock-chips.com> @ 2024-02-01 8:41 ` Heiko Stübner 2024-02-04 9:56 ` Kever Yang 2024-02-13 19:16 ` Heiko Stuebner 1 sibling, 1 reply; 3+ messages in thread From: Heiko Stübner @ 2024-02-01 8:41 UTC (permalink / raw) To: Kever Yang, Ondrej Jirman Cc: linux-rockchip, Kever Yang, Christopher Obbard, Conor Dooley, Cristian Ciocaltea, Dragan Simic, FUKAUMI Naoki, Jagan Teki, John Clark, Krzysztof Kozlowski, Lucas Tanure, Rob Herring, Sebastian Reichel, Shreeya Patel, Tamás Szűcs, devicetree, linux-arm-kernel, linux-kernel, quentin.schulz 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; > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc node 2024-02-01 8:41 ` [PATCH] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc node Heiko Stübner @ 2024-02-04 9:56 ` Kever Yang 0 siblings, 0 replies; 3+ messages in thread From: Kever Yang @ 2024-02-04 9:56 UTC (permalink / raw) To: Heiko Stübner, Ondrej Jirman Cc: linux-rockchip, Christopher Obbard, Conor Dooley, Cristian Ciocaltea, Dragan Simic, FUKAUMI Naoki, Jagan Teki, John Clark, Krzysztof Kozlowski, Rob Herring, Sebastian Reichel, Shreeya Patel, Tamás Szűcs, devicetree, linux-arm-kernel, linux-kernel, quentin.schulz 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; >> > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc node [not found] <20240201034621.1970279-1-kever.yang@rock-chips.com> 2024-02-01 8:41 ` [PATCH] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc node Heiko Stübner @ 2024-02-13 19:16 ` Heiko Stuebner 1 sibling, 0 replies; 3+ messages in thread From: Heiko Stuebner @ 2024-02-13 19:16 UTC (permalink / raw) To: Kever Yang Cc: Heiko Stuebner, Conor Dooley, FUKAUMI Naoki, Krzysztof Kozlowski, Tamás Szűcs, linux-rockchip, Lucas Tanure, Shreeya Patel, Christopher Obbard, linux-kernel, Ondrej Jirman, Rob Herring, Sebastian Reichel, linux-arm-kernel, devicetree, Jagan Teki, Dragan Simic, Cristian Ciocaltea, John Clark On Thu, 1 Feb 2024 11:46:21 +0800, Kever Yang wrote: > 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". > > 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. > > [...] Applied, thanks! [1/1] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc node commit: d859ad305ed19d9a77d8c8ecd22459b73da36ba6 Best regards, -- Heiko Stuebner <heiko@sntech.de> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-13 19:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240201034621.1970279-1-kever.yang@rock-chips.com>
2024-02-01 8:41 ` [PATCH] arm64: dts: rockchip: rk3588: remove redundant cd-gpios in sdmmc node Heiko Stübner
2024-02-04 9:56 ` Kever Yang
2024-02-13 19:16 ` Heiko Stuebner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).