| 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
| ||
|
Message-ID: <546543ec-7817-4422-8717-82aaf46f2a3b@rock-chips.com>
Date: Sun, 4 Feb 2024 17:56:34 +0800
From: Kever Yang <kever.yang@...k-chips.com>
To: Heiko Stübner <heiko@...ech.de>,
Ondrej Jirman <megi@....cz>
Cc: linux-rockchip@...ts.infradead.org,
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>,
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 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;
>>
>
Powered by blists - more mailing lists