[3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings

Message ID	20191003215036.15023-3-smoch@web.de (mailing list archive)
State	New, archived
Headers	show Return-Path: <SRS0=YgdS=X4=lists.infradead.org=linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@kernel.org> DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E6DA6222C8 From: Soeren Moch <smoch@web.de> To: Heiko Stuebner <heiko@sntech.de> Subject: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings Date: Thu, 3 Oct 2019 23:50:36 +0200 Message-Id: <20191003215036.15023-3-smoch@web.de> In-Reply-To: <20191003215036.15023-1-smoch@web.de> References: <20191003215036.15023-1-smoch@web.de> summary: Content analysis details: (-0.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at https://www.dnswl.org/, low trust [212.227.17.12 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (smoch[at]web.de) -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid Precedence: list Cc: linux-rockchip@lists.infradead.org, Soeren Moch <smoch@web.de>, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-rockchip" <linux-rockchip-bounces@lists.infradead.org> Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org
Series	[1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings \| expand [1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings [2/3] arm64: dts: rockchip: fix RockPro64 sdhci settings [3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings

Soeren Moch Oct. 3, 2019, 9:50 p.m. UTC

According to the RockPro64 schematic [1] the rk3399 sdmmc controller is
connected to a microSD (TF card) slot, which cannot be switched to 1.8V.
So also configure the vcc_sdio regulator, which drives the i/o voltage
of the sdmmc controller, accordingly.

While at it, also remove the cap-mmc-highspeed property of the sdmmc
controller, since no mmc card can be connected here.

[1] http://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf

Fixes: e4f3fb490967 ("arm64: dts: rockchip: add initial dts support for Rockpro64")
Signed-off-by: Soeren Moch <smoch@web.de>
---
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: linux-rockchip@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--
2.17.1

Robin Murphy Oct. 4, 2019, 12:01 a.m. UTC | #1

On 2019-10-03 10:50 pm, Soeren Moch wrote:
> According to the RockPro64 schematic [1] the rk3399 sdmmc controller is
> connected to a microSD (TF card) slot, which cannot be switched to 1.8V.

Really? AFAICS the SDMMC0 wiring looks pretty much identical to the 
NanoPC-T4 schematic (it's the same reference design, after all), and I 
know that board can happily drive a UHS-I microSD card with 1.8v I/Os, 
because mine's doing so right now.

Robin.

> So also configure the vcc_sdio regulator, which drives the i/o voltage
> of the sdmmc controller, accordingly.
> 
> While at it, also remove the cap-mmc-highspeed property of the sdmmc
> controller, since no mmc card can be connected here.
> 
> [1] http://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf
> 
> Fixes: e4f3fb490967 ("arm64: dts: rockchip: add initial dts support for Rockpro64")
> Signed-off-by: Soeren Moch <smoch@web.de>
> ---
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: linux-rockchip@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>   arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> index 2e44dae4865a..084f1d994a50 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> @@ -353,7 +353,7 @@
>   				regulator-name = "vcc_sdio";
>   				regulator-always-on;
>   				regulator-boot-on;
> -				regulator-min-microvolt = <1800000>;
> +				regulator-min-microvolt = <3000000>;
>   				regulator-max-microvolt = <3000000>;
>   				regulator-state-mem {
>   					regulator-on-in-suspend;
> @@ -624,7 +624,6 @@
> 
>   &sdmmc {
>   	bus-width = <4>;
> -	cap-mmc-highspeed;
>   	cap-sd-highspeed;
>   	cd-gpios = <&gpio0 7 GPIO_ACTIVE_LOW>;
>   	disable-wp;
> --
> 2.17.1
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>

Soeren Moch Oct. 4, 2019, 12:53 a.m. UTC | #2

On 04.10.19 02:01, Robin Murphy wrote:
> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>> According to the RockPro64 schematic [1] the rk3399 sdmmc controller is
>> connected to a microSD (TF card) slot, which cannot be switched to 1.8V.
>
> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
> NanoPC-T4 schematic (it's the same reference design, after all), and I
> know that board can happily drive a UHS-I microSD card with 1.8v I/Os,
> because mine's doing so right now.
>
> Robin.
OK, the RockPro64 does not allow a card reset (power cycle) since
VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
SDMMC0_PWH_H signal is not connected. So the card fails to identify
itself after suspend or reboot when switched to 1.8V operation.

Regards,
Soeren
>
>> So also configure the vcc_sdio regulator, which drives the i/o voltage
>> of the sdmmc controller, accordingly.
>>
>> While at it, also remove the cap-mmc-highspeed property of the sdmmc
>> controller, since no mmc card can be connected here.
>>
>> [1] http://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf
>>
>> Fixes: e4f3fb490967 ("arm64: dts: rockchip: add initial dts support
>> for Rockpro64")
>> Signed-off-by: Soeren Moch <smoch@web.de>
>> ---
>> Cc: Heiko Stuebner <heiko@sntech.de>
>> Cc: linux-rockchip@lists.infradead.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>   arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>> b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>> index 2e44dae4865a..084f1d994a50 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>> @@ -353,7 +353,7 @@
>>                   regulator-name = "vcc_sdio";
>>                   regulator-always-on;
>>                   regulator-boot-on;
>> -                regulator-min-microvolt = <1800000>;
>> +                regulator-min-microvolt = <3000000>;
>>                   regulator-max-microvolt = <3000000>;
>>                   regulator-state-mem {
>>                       regulator-on-in-suspend;
>> @@ -624,7 +624,6 @@
>>
>>   &sdmmc {
>>       bus-width = <4>;
>> -    cap-mmc-highspeed;
>>       cap-sd-highspeed;
>>       cd-gpios = <&gpio0 7 GPIO_ACTIVE_LOW>;
>>       disable-wp;
>> -- 
>> 2.17.1
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>

Shawn Lin Oct. 4, 2019, 2:13 a.m. UTC | #3

On 2019/10/4 8:53, Soeren Moch wrote:
> 
> 
> On 04.10.19 02:01, Robin Murphy wrote:
>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>> According to the RockPro64 schematic [1] the rk3399 sdmmc controller is
>>> connected to a microSD (TF card) slot, which cannot be switched to 1.8V.
>>
>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>> NanoPC-T4 schematic (it's the same reference design, after all), and I
>> know that board can happily drive a UHS-I microSD card with 1.8v I/Os,
>> because mine's doing so right now.
>>
>> Robin.
> OK, the RockPro64 does not allow a card reset (power cycle) since
> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
> SDMMC0_PWH_H signal is not connected. So the card fails to identify
> itself after suspend or reboot when switched to 1.8V operation.
> 

I believe we addressed this issue long time ago, please check:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb

> Regards,
> Soeren
>>
>>> So also configure the vcc_sdio regulator, which drives the i/o voltage
>>> of the sdmmc controller, accordingly.
>>>
>>> While at it, also remove the cap-mmc-highspeed property of the sdmmc
>>> controller, since no mmc card can be connected here.
>>>
>>> [1] http://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf
>>>
>>> Fixes: e4f3fb490967 ("arm64: dts: rockchip: add initial dts support
>>> for Rockpro64")
>>> Signed-off-by: Soeren Moch <smoch@web.de>
>>> ---
>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>> Cc: linux-rockchip@lists.infradead.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> ---
>>>    arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts | 3 +--
>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>> b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>> index 2e44dae4865a..084f1d994a50 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>> @@ -353,7 +353,7 @@
>>>                    regulator-name = "vcc_sdio";
>>>                    regulator-always-on;
>>>                    regulator-boot-on;
>>> -                regulator-min-microvolt = <1800000>;
>>> +                regulator-min-microvolt = <3000000>;
>>>                    regulator-max-microvolt = <3000000>;
>>>                    regulator-state-mem {
>>>                        regulator-on-in-suspend;
>>> @@ -624,7 +624,6 @@
>>>
>>>    &sdmmc {
>>>        bus-width = <4>;
>>> -    cap-mmc-highspeed;
>>>        cap-sd-highspeed;
>>>        cd-gpios = <&gpio0 7 GPIO_ACTIVE_LOW>;
>>>        disable-wp;
>>> -- 
>>> 2.17.1
>>>
>>>
>>> _______________________________________________
>>> Linux-rockchip mailing list
>>> Linux-rockchip@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>>
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>

Soeren Moch Oct. 4, 2019, 3:39 a.m. UTC | #4

On 04.10.19 04:13, Shawn Lin wrote:
> On 2019/10/4 8:53, Soeren Moch wrote:
>>
>>
>> On 04.10.19 02:01, Robin Murphy wrote:
>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>> controller is
>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>> 1.8V.
>>>
>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>> NanoPC-T4 schematic (it's the same reference design, after all), and I
>>> know that board can happily drive a UHS-I microSD card with 1.8v I/Os,
>>> because mine's doing so right now.
>>>
>>> Robin.
>> OK, the RockPro64 does not allow a card reset (power cycle) since
>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>> itself after suspend or reboot when switched to 1.8V operation.
>>
>
> I believe we addressed this issue long time ago, please check:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>
Thanks for the pointer.
In this case I guess I should use following patch instead:

--- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
+++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
@@ -619,6 +619,8 @@
     max-frequency = <150000000>;
     pinctrl-names = "default";
     pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
+    sd-uhs-sdr104;
+    vqmmc-supply = <&vcc_sdio>;
     status = "okay";
 };
 
When I do so, the sd card is detected as SDR104, but a reboot hangs:

Boot1: 2018-06-26, version: 1.14
CPUId = 0x0
ChipType = 0x10, 286
Spi_ChipId = c84018
no find rkpartition
SpiBootInit:ffffffff
mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
mmc: ERROR: Card did not respond to voltage select!
emmc reinit
mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
mmc: ERROR: Card did not respond to voltage select!
emmc reinit
mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
mmc: ERROR: Card did not respond to voltage select!
SdmmcInit=2 1
mmc0:cmd5,32
mmc0:cmd7,32
mmc0:cmd5,32
mmc0:cmd7,32
mmc0:cmd5,32
mmc0:cmd7,32
SdmmcInit=0 1

So I guess I should use a different miniloader for this reboot to work!?
Or what else could be wrong here?

Thanks,
Soeren

>> Regards,
>> Soeren
>>>
>>>> So also configure the vcc_sdio regulator, which drives the i/o voltage
>>>> of the sdmmc controller, accordingly.
>>>>
>>>> While at it, also remove the cap-mmc-highspeed property of the sdmmc
>>>> controller, since no mmc card can be connected here.
>>>>
>>>> [1] http://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf
>>>>
>>>> Fixes: e4f3fb490967 ("arm64: dts: rockchip: add initial dts support
>>>> for Rockpro64")
>>>> Signed-off-by: Soeren Moch <smoch@web.de>
>>>> ---
>>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>>> Cc: linux-rockchip@lists.infradead.org
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> ---
>>>>    arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts | 3 +--
>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>> b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>> index 2e44dae4865a..084f1d994a50 100644
>>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>> @@ -353,7 +353,7 @@
>>>>                    regulator-name = "vcc_sdio";
>>>>                    regulator-always-on;
>>>>                    regulator-boot-on;
>>>> -                regulator-min-microvolt = <1800000>;
>>>> +                regulator-min-microvolt = <3000000>;
>>>>                    regulator-max-microvolt = <3000000>;
>>>>                    regulator-state-mem {
>>>>                        regulator-on-in-suspend;
>>>> @@ -624,7 +624,6 @@
>>>>
>>>>    &sdmmc {
>>>>        bus-width = <4>;
>>>> -    cap-mmc-highspeed;
>>>>        cap-sd-highspeed;
>>>>        cd-gpios = <&gpio0 7 GPIO_ACTIVE_LOW>;
>>>>        disable-wp;
>>>> -- 
>>>> 2.17.1
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-rockchip mailing list
>>>> Linux-rockchip@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>
>
>

Robin Murphy Oct. 4, 2019, 2:20 p.m. UTC | #5

On 04/10/2019 04:39, Soeren Moch wrote:
> 
> 
> On 04.10.19 04:13, Shawn Lin wrote:
>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>
>>>
>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>> controller is
>>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>>> 1.8V.
>>>>
>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>>> NanoPC-T4 schematic (it's the same reference design, after all), and I
>>>> know that board can happily drive a UHS-I microSD card with 1.8v I/Os,
>>>> because mine's doing so right now.
>>>>
>>>> Robin.
>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>>> itself after suspend or reboot when switched to 1.8V operation.

Ah, thanks for clarifying - I did overlook the subtlety that U12 and 
friends have "NC" as alternative part numbers, even though they aren't 
actually marked as DNP. So it's still not so much "cannot be switched" 
as "switching can lead to other problems".

>>
>> I believe we addressed this issue long time ago, please check:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>
> Thanks for the pointer.
> In this case I guess I should use following patch instead:
> 
> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
> @@ -619,6 +619,8 @@
>       max-frequency = <150000000>;
>       pinctrl-names = "default";
>       pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
> +    sd-uhs-sdr104;
> +    vqmmc-supply = <&vcc_sdio>;
>       status = "okay";
>   };
>   
> When I do so, the sd card is detected as SDR104, but a reboot hangs:
> 
> Boot1: 2018-06-26, version: 1.14
> CPUId = 0x0
> ChipType = 0x10, 286
> Spi_ChipId = c84018
> no find rkpartition
> SpiBootInit:ffffffff
> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
> mmc: ERROR: Card did not respond to voltage select!
> emmc reinit
> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
> mmc: ERROR: Card did not respond to voltage select!
> emmc reinit
> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
> mmc: ERROR: Card did not respond to voltage select!
> SdmmcInit=2 1
> mmc0:cmd5,32
> mmc0:cmd7,32
> mmc0:cmd5,32
> mmc0:cmd7,32
> mmc0:cmd5,32
> mmc0:cmd7,32
> SdmmcInit=0 1
> 
> So I guess I should use a different miniloader for this reboot to work!?
> Or what else could be wrong here?

Hmm, I guess this is "the Tinkerboard problem" again - the patch above 
would be OK if we could get as far as the kernel, but can't help if the 
offending card is itself the boot medium. There was a proposal here:

https://patchwork.kernel.org/patch/10817217/

although I'm not sure what if any progress has been made since then.

Robin.

Jonas Karlman Oct. 4, 2019, 2:42 p.m. UTC | #6

On 2019-10-04 16:20, Robin Murphy wrote:
> On 04/10/2019 04:39, Soeren Moch wrote:
>>
>> On 04.10.19 04:13, Shawn Lin wrote:
>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>
>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>> controller is
>>>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>>>> 1.8V.
>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>>>> NanoPC-T4 schematic (it's the same reference design, after all), and I
>>>>> know that board can happily drive a UHS-I microSD card with 1.8v I/Os,
>>>>> because mine's doing so right now.
>>>>>
>>>>> Robin.
>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>>>> itself after suspend or reboot when switched to 1.8V operation.
> Ah, thanks for clarifying - I did overlook the subtlety that U12 and 
> friends have "NC" as alternative part numbers, even though they aren't 
> actually marked as DNP. So it's still not so much "cannot be switched" 
> as "switching can lead to other problems".
>
>>> I believe we addressed this issue long time ago, please check:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>
>> Thanks for the pointer.
>> In this case I guess I should use following patch instead:
>>
>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>> @@ -619,6 +619,8 @@
>>       max-frequency = <150000000>;
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>> +    sd-uhs-sdr104;
>> +    vqmmc-supply = <&vcc_sdio>;
>>       status = "okay";
>>   };
>>   
>> When I do so, the sd card is detected as SDR104, but a reboot hangs:
>>
>> Boot1: 2018-06-26, version: 1.14
>> CPUId = 0x0
>> ChipType = 0x10, 286
>> Spi_ChipId = c84018
>> no find rkpartition
>> SpiBootInit:ffffffff
>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>> mmc: ERROR: Card did not respond to voltage select!
>> emmc reinit
>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>> mmc: ERROR: Card did not respond to voltage select!
>> emmc reinit
>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>> mmc: ERROR: Card did not respond to voltage select!
>> SdmmcInit=2 1
>> mmc0:cmd5,32
>> mmc0:cmd7,32
>> mmc0:cmd5,32
>> mmc0:cmd7,32
>> mmc0:cmd5,32
>> mmc0:cmd7,32
>> SdmmcInit=0 1
>>
>> So I guess I should use a different miniloader for this reboot to work!?
>> Or what else could be wrong here?
> Hmm, I guess this is "the Tinkerboard problem" again - the patch above 
> would be OK if we could get as far as the kernel, but can't help if the 
> offending card is itself the boot medium. There was a proposal here:
>
> https://patchwork.kernel.org/patch/10817217/
>
> although I'm not sure what if any progress has been made since then.

Correct, I never got around to complete testing on other devices.
Maybe someone more familiar with mmc subsystem can pick that up if it now turns out other devices show the same/similar issue?

fwiw the "mmc: core: set initial signal voltage on power off" rfc/patch have been used in LibreELEC rockchip builds for well over a year now.

Regards,
Jonas

>
> Robin.
>

Shawn Lin Oct. 4, 2019, 3:33 p.m. UTC | #7

On 2019/10/4 22:20, Robin Murphy wrote:
> On 04/10/2019 04:39, Soeren Moch wrote:
>>
>>
>> On 04.10.19 04:13, Shawn Lin wrote:
>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>
>>>>
>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>> controller is
>>>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>>>> 1.8V.
>>>>>
>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>>>> NanoPC-T4 schematic (it's the same reference design, after all), and I
>>>>> know that board can happily drive a UHS-I microSD card with 1.8v I/Os,
>>>>> because mine's doing so right now.
>>>>>
>>>>> Robin.
>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>>>> itself after suspend or reboot when switched to 1.8V operation.
> 
> Ah, thanks for clarifying - I did overlook the subtlety that U12 and 
> friends have "NC" as alternative part numbers, even though they aren't 
> actually marked as DNP. So it's still not so much "cannot be switched" 
> as "switching can lead to other problems".
> 
>>>
>>> I believe we addressed this issue long time ago, please check:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb 
>>>
>>>
>> Thanks for the pointer.
>> In this case I guess I should use following patch instead:
>>
>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>> @@ -619,6 +619,8 @@
>>       max-frequency = <150000000>;
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>> +    sd-uhs-sdr104;
>> +    vqmmc-supply = <&vcc_sdio>;
>>       status = "okay";
>>   };
>> When I do so, the sd card is detected as SDR104, but a reboot hangs:
>>
>> Boot1: 2018-06-26, version: 1.14
>> CPUId = 0x0
>> ChipType = 0x10, 286
>> Spi_ChipId = c84018
>> no find rkpartition
>> SpiBootInit:ffffffff
>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>> mmc: ERROR: Card did not respond to voltage select!
>> emmc reinit
>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>> mmc: ERROR: Card did not respond to voltage select!
>> emmc reinit
>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>> mmc: ERROR: Card did not respond to voltage select!
>> SdmmcInit=2 1
>> mmc0:cmd5,32
>> mmc0:cmd7,32
>> mmc0:cmd5,32
>> mmc0:cmd7,32
>> mmc0:cmd5,32
>> mmc0:cmd7,32
>> SdmmcInit=0 1
>>
>> So I guess I should use a different miniloader for this reboot to work!?
>> Or what else could be wrong here?
> 
> Hmm, I guess this is "the Tinkerboard problem" again - the patch above 
> would be OK if we could get as far as the kernel, but can't help if the 

I didn't realize that SD was used as boot medium for RockPro64, but I
did patch the vendor tree to solve the issue for Tinkerboard, see
https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0

My initial plan was to patching upstream kernel by adding ->shutdown,but
never finish it.

> offending card is itself the boot medium. There was a proposal here:
> 
> https://patchwork.kernel.org/patch/10817217/

This RFC also looks good to me, but seems it needs volunteers
to push it again.

> 
> although I'm not sure what if any progress has been made since then.
> 
> Robin.
> 
> 
>

Soeren Moch Oct. 4, 2019, 4:39 p.m. UTC | #8

On 04.10.19 16:20, Robin Murphy wrote:
> On 04/10/2019 04:39, Soeren Moch wrote:
>>
>>
>> On 04.10.19 04:13, Shawn Lin wrote:
>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>
>>>>
>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>> controller is
>>>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>>>> 1.8V.
>>>>>
>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>>>> NanoPC-T4 schematic (it's the same reference design, after all),
>>>>> and I
>>>>> know that board can happily drive a UHS-I microSD card with 1.8v
>>>>> I/Os,
>>>>> because mine's doing so right now.
>>>>>
>>>>> Robin.
>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>>>> itself after suspend or reboot when switched to 1.8V operation.
>
> Ah, thanks for clarifying - I did overlook the subtlety that U12 and
> friends have "NC" as alternative part numbers, even though they aren't
> actually marked as DNP. So it's still not so much "cannot be switched"
> as "switching can lead to other problems".
Agreed. I should have been more precise about this in the commit message.

Soeren
>
>>>
>>> I believe we addressed this issue long time ago, please check:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>
>>>
>> Thanks for the pointer.
>> In this case I guess I should use following patch instead:
>>
>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>> @@ -619,6 +619,8 @@
>>       max-frequency = <150000000>;
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>> +    sd-uhs-sdr104;
>> +    vqmmc-supply = <&vcc_sdio>;
>>       status = "okay";
>>   };
>>   When I do so, the sd card is detected as SDR104, but a reboot hangs:
>>
>> Boot1: 2018-06-26, version: 1.14
>> CPUId = 0x0
>> ChipType = 0x10, 286
>> Spi_ChipId = c84018
>> no find rkpartition
>> SpiBootInit:ffffffff
>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>> mmc: ERROR: Card did not respond to voltage select!
>> emmc reinit
>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>> mmc: ERROR: Card did not respond to voltage select!
>> emmc reinit
>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>> mmc: ERROR: Card did not respond to voltage select!
>> SdmmcInit=2 1
>> mmc0:cmd5,32
>> mmc0:cmd7,32
>> mmc0:cmd5,32
>> mmc0:cmd7,32
>> mmc0:cmd5,32
>> mmc0:cmd7,32
>> SdmmcInit=0 1
>>
>> So I guess I should use a different miniloader for this reboot to work!?
>> Or what else could be wrong here?
>
> Hmm, I guess this is "the Tinkerboard problem" again - the patch above
> would be OK if we could get as far as the kernel, but can't help if
> the offending card is itself the boot medium. There was a proposal here:
>
> https://patchwork.kernel.org/patch/10817217/
>
> although I'm not sure what if any progress has been made since then.
>
> Robin.

Soeren Moch Oct. 4, 2019, 5:24 p.m. UTC | #9

On 04.10.19 17:33, Shawn Lin wrote:
> On 2019/10/4 22:20, Robin Murphy wrote:
>> On 04/10/2019 04:39, Soeren Moch wrote:
>>>
>>>
>>> On 04.10.19 04:13, Shawn Lin wrote:
>>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>>
>>>>>
>>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>>> controller is
>>>>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>>>>> 1.8V.
>>>>>>
>>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>>>>> NanoPC-T4 schematic (it's the same reference design, after all),
>>>>>> and I
>>>>>> know that board can happily drive a UHS-I microSD card with 1.8v
>>>>>> I/Os,
>>>>>> because mine's doing so right now.
>>>>>>
>>>>>> Robin.
>>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>>>>> itself after suspend or reboot when switched to 1.8V operation.
>>
>> Ah, thanks for clarifying - I did overlook the subtlety that U12 and
>> friends have "NC" as alternative part numbers, even though they
>> aren't actually marked as DNP. So it's still not so much "cannot be
>> switched" as "switching can lead to other problems".
>>
>>>>
>>>> I believe we addressed this issue long time ago, please check:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>>
>>>>
>>> Thanks for the pointer.
>>> In this case I guess I should use following patch instead:
>>>
>>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>>> @@ -619,6 +619,8 @@
>>>       max-frequency = <150000000>;
>>>       pinctrl-names = "default";
>>>       pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>>> +    sd-uhs-sdr104;
>>> +    vqmmc-supply = <&vcc_sdio>;
>>>       status = "okay";
>>>   };
>>> When I do so, the sd card is detected as SDR104, but a reboot hangs:
>>>
>>> Boot1: 2018-06-26, version: 1.14
>>> CPUId = 0x0
>>> ChipType = 0x10, 286
>>> Spi_ChipId = c84018
>>> no find rkpartition
>>> SpiBootInit:ffffffff
>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>> mmc: ERROR: Card did not respond to voltage select!
>>> emmc reinit
>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>> mmc: ERROR: Card did not respond to voltage select!
>>> emmc reinit
>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>> mmc: ERROR: Card did not respond to voltage select!
>>> SdmmcInit=2 1
>>> mmc0:cmd5,32
>>> mmc0:cmd7,32
>>> mmc0:cmd5,32
>>> mmc0:cmd7,32
>>> mmc0:cmd5,32
>>> mmc0:cmd7,32
>>> SdmmcInit=0 1
>>>
>>> So I guess I should use a different miniloader for this reboot to
>>> work!?
>>> Or what else could be wrong here?
>>
>> Hmm, I guess this is "the Tinkerboard problem" again - the patch
>> above would be OK if we could get as far as the kernel, but can't
>> help if the 
>
> I didn't realize that SD was used as boot medium for RockPro64, but I
> did patch the vendor tree to solve the issue for Tinkerboard, see
> https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0
>
>
> My initial plan was to patching upstream kernel by adding ->shutdown,but
> never finish it.
>
>> offending card is itself the boot medium. There was a proposal here:
>>
>> https://patchwork.kernel.org/patch/10817217/
>
> This RFC also looks good to me, but seems it needs volunteers
> to push it again.
Oh, I think this is a totally wrong way.

While this might work for some cards, setting the controller's i/o
voltage to 3.3V while leaving the card at 1.8V configuration is totally
against the specification, can lead to all kinds of strange behaviour
and even cause hardware damage. It also would actively defend the
purpose of the above mentioned patch (6a11fc4) where the kernel guesses
the i/o voltage from the card configuration and switches the controller
accordingly. We would end up with a 1.8V card and controller
configuration and a regulator voltage of 3.3V. This would only work with
good luck. Even if the kernel driver would switch the regulator back to
1.8V in this case, the voltage mismatch remains in the bootloader when
this card contains the boot image.

The only sane way I see to handle this is implementing the same
workaround (mode guessing) also in the bootloader (rockchip miniloader
and u-boot SPL since both bootloader chains are supported for this board).

Or maybe I miss something?

Soeren


>
>>
>> although I'm not sure what if any progress has been made since then.
>>
>> Robin.
>>
>>
>>
>
>

Soeren Moch Oct. 4, 2019, 8:15 p.m. UTC | #10

Heiko,

since you started to apply the first 2 Patches of this series (thanks
for that!), now after all the discussions here (and the heads-up for the
implemented mode detection) I think we should leave the vcc_sdio
regulator settings unmodified.

It still could make sense to remove the cap-mmc-highspeed property. Are
you OK with a V2 patch for that?

Thanks,
Soeren


On 04.10.19 19:24, Sören Moch wrote:
>
> On 04.10.19 17:33, Shawn Lin wrote:
>> On 2019/10/4 22:20, Robin Murphy wrote:
>>> On 04/10/2019 04:39, Soeren Moch wrote:
>>>>
>>>> On 04.10.19 04:13, Shawn Lin wrote:
>>>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>>>
>>>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>>>> controller is
>>>>>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>>>>>> 1.8V.
>>>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>>>>>> NanoPC-T4 schematic (it's the same reference design, after all),
>>>>>>> and I
>>>>>>> know that board can happily drive a UHS-I microSD card with 1.8v
>>>>>>> I/Os,
>>>>>>> because mine's doing so right now.
>>>>>>>
>>>>>>> Robin.
>>>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>>>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>>>>>> itself after suspend or reboot when switched to 1.8V operation.
>>> Ah, thanks for clarifying - I did overlook the subtlety that U12 and
>>> friends have "NC" as alternative part numbers, even though they
>>> aren't actually marked as DNP. So it's still not so much "cannot be
>>> switched" as "switching can lead to other problems".
>>>
>>>>> I believe we addressed this issue long time ago, please check:
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>>>
>>>>>
>>>> Thanks for the pointer.
>>>> In this case I guess I should use following patch instead:
>>>>
>>>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>>>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>>>> @@ -619,6 +619,8 @@
>>>>       max-frequency = <150000000>;
>>>>       pinctrl-names = "default";
>>>>       pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>>>> +    sd-uhs-sdr104;
>>>> +    vqmmc-supply = <&vcc_sdio>;
>>>>       status = "okay";
>>>>   };
>>>> When I do so, the sd card is detected as SDR104, but a reboot hangs:
>>>>
>>>> Boot1: 2018-06-26, version: 1.14
>>>> CPUId = 0x0
>>>> ChipType = 0x10, 286
>>>> Spi_ChipId = c84018
>>>> no find rkpartition
>>>> SpiBootInit:ffffffff
>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>> mmc: ERROR: Card did not respond to voltage select!
>>>> emmc reinit
>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>> mmc: ERROR: Card did not respond to voltage select!
>>>> emmc reinit
>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>> mmc: ERROR: Card did not respond to voltage select!
>>>> SdmmcInit=2 1
>>>> mmc0:cmd5,32
>>>> mmc0:cmd7,32
>>>> mmc0:cmd5,32
>>>> mmc0:cmd7,32
>>>> mmc0:cmd5,32
>>>> mmc0:cmd7,32
>>>> SdmmcInit=0 1
>>>>
>>>> So I guess I should use a different miniloader for this reboot to
>>>> work!?
>>>> Or what else could be wrong here?
>>> Hmm, I guess this is "the Tinkerboard problem" again - the patch
>>> above would be OK if we could get as far as the kernel, but can't
>>> help if the 
>> I didn't realize that SD was used as boot medium for RockPro64, but I
>> did patch the vendor tree to solve the issue for Tinkerboard, see
>> https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0
>>
>>
>> My initial plan was to patching upstream kernel by adding ->shutdown,but
>> never finish it.
>>
>>> offending card is itself the boot medium. There was a proposal here:
>>>
>>> https://patchwork.kernel.org/patch/10817217/
>> This RFC also looks good to me, but seems it needs volunteers
>> to push it again.
> Oh, I think this is a totally wrong way.
>
> While this might work for some cards, setting the controller's i/o
> voltage to 3.3V while leaving the card at 1.8V configuration is totally
> against the specification, can lead to all kinds of strange behaviour
> and even cause hardware damage. It also would actively defend the
> purpose of the above mentioned patch (6a11fc4) where the kernel guesses
> the i/o voltage from the card configuration and switches the controller
> accordingly. We would end up with a 1.8V card and controller
> configuration and a regulator voltage of 3.3V. This would only work with
> good luck. Even if the kernel driver would switch the regulator back to
> 1.8V in this case, the voltage mismatch remains in the bootloader when
> this card contains the boot image.
>
> The only sane way I see to handle this is implementing the same
> workaround (mode guessing) also in the bootloader (rockchip miniloader
> and u-boot SPL since both bootloader chains are supported for this board).
>
> Or maybe I miss something?
>
> Soeren
>
>
>>> although I'm not sure what if any progress has been made since then.
>>>
>>> Robin.
>>>
>>>
>>>
>>
>

Heiko Stübner Oct. 4, 2019, 8:18 p.m. UTC | #11

Hi Sören,

Am Freitag, 4. Oktober 2019, 22:15:45 CEST schrieb Soeren Moch:
> Heiko,
> 
> since you started to apply the first 2 Patches of this series (thanks
> for that!), now after all the discussions here (and the heads-up for the
> implemented mode detection) I think we should leave the vcc_sdio
> regulator settings unmodified.

I was composing a mail about me holding off on this patch due to the
ongoing discussion when your mail came ;-)

> It still could make sense to remove the cap-mmc-highspeed property. Are
> you OK with a V2 patch for that?

Sure, go ahead.

Heiko

Soeren Moch Oct. 4, 2019, 8:39 p.m. UTC | #12

On 04.10.19 22:18, Heiko Stuebner wrote:
> Hi Sören,
>
> Am Freitag, 4. Oktober 2019, 22:15:45 CEST schrieb Soeren Moch:
>> Heiko,
>>
>> since you started to apply the first 2 Patches of this series (thanks
>> for that!), now after all the discussions here (and the heads-up for the
>> implemented mode detection) I think we should leave the vcc_sdio
>> regulator settings unmodified.
> I was composing a mail about me holding off on this patch due to the
> ongoing discussion when your mail came ;-)
>
>> It still could make sense to remove the cap-mmc-highspeed property. Are
>> you OK with a V2 patch for that?
> Sure, go ahead.
>
OK, I sent a V2 patch for that.

Maybe someone can come up with a real solution for the 1.8V/3.3V
problem, which would allow to enable the uhs-sdr104 mode by default.
For now I think we have to live with sd-highspeed to be able to
(re-)boot from SD.

Thanks,
Soeren

Jonas Karlman Oct. 11, 2019, 8:22 a.m. UTC | #13

On 2019-10-04 19:24, Sören Moch wrote:
>
> On 04.10.19 17:33, Shawn Lin wrote:
>> On 2019/10/4 22:20, Robin Murphy wrote:
>>> On 04/10/2019 04:39, Soeren Moch wrote:
>>>>
>>>> On 04.10.19 04:13, Shawn Lin wrote:
>>>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>>>
>>>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>>>> controller is
>>>>>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>>>>>> 1.8V.
>>>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>>>>>> NanoPC-T4 schematic (it's the same reference design, after all),
>>>>>>> and I
>>>>>>> know that board can happily drive a UHS-I microSD card with 1.8v
>>>>>>> I/Os,
>>>>>>> because mine's doing so right now.
>>>>>>>
>>>>>>> Robin.
>>>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>>>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>>>>>> itself after suspend or reboot when switched to 1.8V operation.
>>> Ah, thanks for clarifying - I did overlook the subtlety that U12 and
>>> friends have "NC" as alternative part numbers, even though they
>>> aren't actually marked as DNP. So it's still not so much "cannot be
>>> switched" as "switching can lead to other problems".
>>>
>>>>> I believe we addressed this issue long time ago, please check:
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>>>
>>>>>
>>>> Thanks for the pointer.
>>>> In this case I guess I should use following patch instead:
>>>>
>>>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>>>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>>>> @@ -619,6 +619,8 @@
>>>>       max-frequency = <150000000>;
>>>>       pinctrl-names = "default";
>>>>       pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>>>> +    sd-uhs-sdr104;
>>>> +    vqmmc-supply = <&vcc_sdio>;
>>>>       status = "okay";
>>>>   };
>>>> When I do so, the sd card is detected as SDR104, but a reboot hangs:
>>>>
>>>> Boot1: 2018-06-26, version: 1.14
>>>> CPUId = 0x0
>>>> ChipType = 0x10, 286
>>>> Spi_ChipId = c84018
>>>> no find rkpartition
>>>> SpiBootInit:ffffffff
>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>> mmc: ERROR: Card did not respond to voltage select!
>>>> emmc reinit
>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>> mmc: ERROR: Card did not respond to voltage select!
>>>> emmc reinit
>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>> mmc: ERROR: Card did not respond to voltage select!
>>>> SdmmcInit=2 1
>>>> mmc0:cmd5,32
>>>> mmc0:cmd7,32
>>>> mmc0:cmd5,32
>>>> mmc0:cmd7,32
>>>> mmc0:cmd5,32
>>>> mmc0:cmd7,32
>>>> SdmmcInit=0 1
>>>>
>>>> So I guess I should use a different miniloader for this reboot to
>>>> work!?
>>>> Or what else could be wrong here?
>>> Hmm, I guess this is "the Tinkerboard problem" again - the patch
>>> above would be OK if we could get as far as the kernel, but can't
>>> help if the 
>> I didn't realize that SD was used as boot medium for RockPro64, but I
>> did patch the vendor tree to solve the issue for Tinkerboard, see
>> https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0
>>
>>
>> My initial plan was to patching upstream kernel by adding ->shutdown,but
>> never finish it.
>>
>>> offending card is itself the boot medium. There was a proposal here:
>>>
>>> https://patchwork.kernel.org/patch/10817217/
>> This RFC also looks good to me, but seems it needs volunteers
>> to push it again.
> Oh, I think this is a totally wrong way.
>
> While this might work for some cards, setting the controller's i/o
> voltage to 3.3V while leaving the card at 1.8V configuration is totally
> against the specification, can lead to all kinds of strange behaviour
> and even cause hardware damage. It also would actively defend the
> purpose of the above mentioned patch (6a11fc4) where the kernel guesses
> the i/o voltage from the card configuration and switches the controller
> accordingly. We would end up with a 1.8V card and controller
> configuration and a regulator voltage of 3.3V. This would only work with
> good luck. Even if the kernel driver would switch the regulator back to
> 1.8V in this case, the voltage mismatch remains in the bootloader when
> this card contains the boot image.
>
> The only sane way I see to handle this is implementing the same
> workaround (mode guessing) also in the bootloader (rockchip miniloader
> and u-boot SPL since both bootloader chains are supported for this board).
>
> Or maybe I miss something?

Thanks for your input, I have made a new series [1] with a similar approach but is limited to dw_mmc-rockchip
and only changes the regulator at power_off after the regulator has been disabled (the vqmmc regulator in affected devices is always-on).

To my knowledge the problem is not with the rockchip miniloader or u-boot SPL, it is the initial boot rom that tries to load
the miniloader/SPL from a SD-card, so nothing that can be updated.

I have observed this issue on the following devices, and the patches at [1] makes it possible to reboot from SD-card after UHS has been enabled.
- Asus Tinker Board (RK3288)
- Rockchip Sapphire Board (RK3399)
- Radxa Rock Pi 4 (RK3399)
- Pine64 RockPro64 (RK3399)
All of the above seem to use the RK808 regulator for sd io voltage.

The ROC-RK3328-CC did not have this issue and seem to automatically reset to 3.3v.

[1] https://github.com/Kwiboo/linux-rockchip/compare/next-20191011...next-20191011-mmc

Regards,
Jonas

>
> Soeren
>
>
>>> although I'm not sure what if any progress has been made since then.
>>>
>>> Robin.
>>>

Soeren Moch Oct. 11, 2019, 11:40 a.m. UTC | #14

On 11.10.19 10:22, Jonas Karlman wrote:
> On 2019-10-04 19:24, Sören Moch wrote:
>> On 04.10.19 17:33, Shawn Lin wrote:
>>> On 2019/10/4 22:20, Robin Murphy wrote:
>>>> On 04/10/2019 04:39, Soeren Moch wrote:
>>>>> On 04.10.19 04:13, Shawn Lin wrote:
>>>>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>>>>> controller is
>>>>>>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>>>>>>> 1.8V.
>>>>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>>>>>>> NanoPC-T4 schematic (it's the same reference design, after all),
>>>>>>>> and I
>>>>>>>> know that board can happily drive a UHS-I microSD card with 1.8v
>>>>>>>> I/Os,
>>>>>>>> because mine's doing so right now.
>>>>>>>>
>>>>>>>> Robin.
>>>>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>>>>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>>>>>>> itself after suspend or reboot when switched to 1.8V operation.
>>>> Ah, thanks for clarifying - I did overlook the subtlety that U12 and
>>>> friends have "NC" as alternative part numbers, even though they
>>>> aren't actually marked as DNP. So it's still not so much "cannot be
>>>> switched" as "switching can lead to other problems".
>>>>
>>>>>> I believe we addressed this issue long time ago, please check:
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>>>>
>>>>>>
>>>>> Thanks for the pointer.
>>>>> In this case I guess I should use following patch instead:
>>>>>
>>>>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>>>>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>>>>> @@ -619,6 +619,8 @@
>>>>>       max-frequency = <150000000>;
>>>>>       pinctrl-names = "default";
>>>>>       pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>>>>> +    sd-uhs-sdr104;
>>>>> +    vqmmc-supply = <&vcc_sdio>;
>>>>>       status = "okay";
>>>>>   };
>>>>> When I do so, the sd card is detected as SDR104, but a reboot hangs:
>>>>>
>>>>> Boot1: 2018-06-26, version: 1.14
>>>>> CPUId = 0x0
>>>>> ChipType = 0x10, 286
>>>>> Spi_ChipId = c84018
>>>>> no find rkpartition
>>>>> SpiBootInit:ffffffff
>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>> emmc reinit
>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>> emmc reinit
>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>> SdmmcInit=2 1
>>>>> mmc0:cmd5,32
>>>>> mmc0:cmd7,32
>>>>> mmc0:cmd5,32
>>>>> mmc0:cmd7,32
>>>>> mmc0:cmd5,32
>>>>> mmc0:cmd7,32
>>>>> SdmmcInit=0 1
>>>>>
>>>>> So I guess I should use a different miniloader for this reboot to
>>>>> work!?
>>>>> Or what else could be wrong here?
>>>> Hmm, I guess this is "the Tinkerboard problem" again - the patch
>>>> above would be OK if we could get as far as the kernel, but can't
>>>> help if the 
>>> I didn't realize that SD was used as boot medium for RockPro64, but I
>>> did patch the vendor tree to solve the issue for Tinkerboard, see
>>> https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0
>>>
>>>
>>> My initial plan was to patching upstream kernel by adding ->shutdown,but
>>> never finish it.
>>>
>>>> offending card is itself the boot medium. There was a proposal here:
>>>>
>>>> https://patchwork.kernel.org/patch/10817217/
>>> This RFC also looks good to me, but seems it needs volunteers
>>> to push it again.
>> Oh, I think this is a totally wrong way.
>>
>> While this might work for some cards, setting the controller's i/o
>> voltage to 3.3V while leaving the card at 1.8V configuration is totally
>> against the specification, can lead to all kinds of strange behaviour
>> and even cause hardware damage. It also would actively defend the
>> purpose of the above mentioned patch (6a11fc4) where the kernel guesses
>> the i/o voltage from the card configuration and switches the controller
>> accordingly. We would end up with a 1.8V card and controller
>> configuration and a regulator voltage of 3.3V. This would only work with
>> good luck. Even if the kernel driver would switch the regulator back to
>> 1.8V in this case, the voltage mismatch remains in the bootloader when
>> this card contains the boot image.
>>
>> The only sane way I see to handle this is implementing the same
>> workaround (mode guessing) also in the bootloader (rockchip miniloader
>> and u-boot SPL since both bootloader chains are supported for this board).
>>
>> Or maybe I miss something?
> Thanks for your input, I have made a new series [1] with a similar approach but is limited to dw_mmc-rockchip
> and only changes the regulator at power_off after the regulator has been disabled (the vqmmc regulator in affected devices is always-on).
Thanks for your work on this. Unfortunately I still think that this is
the wrong approach.
I see several problems in your patch series:
- You introduced GPIO0_PA1 as regulator enable for RockPro64.
Unfortunately Pine64 decided to disable this regulator on Board Version
2.1 (real product version), see above. I have no idea why they did this.
- You changed the i/o voltage from 3.0V to 3.3V. This is not allowed on
RK3399 I/O bank F.

Changing the i/o voltage to 3.0V/3.3V while the sd card is configured
for 1.8V is against the specification and dangerous. While experimenting
with different images (ayufan, armbian) for my newly bought RockPro64 I
killed a SD card (32GB Samsung UHS-I). I cannot reconstruct the exact
circumstances, but I'm pretty sure this happened due to the voltage
mismatch. Of course I'm not keen to experiment further with this and
kill more sd cards. This is not just an theoretical issue.
> To my knowledge the problem is not with the rockchip miniloader or u-boot SPL, it is the initial boot rom that tries to load
> the miniloader/SPL from a SD-card, so nothing that can be updated.
What I observed on my RockPro64:
The ROM bootloader always was able to load the next stage, maybe due to
the low initial clock of 400kHz? Also the ROM bootloader cannot change
voltage regulator settings. So if the i/o voltage still is at 1.8V and
matching the sd card setting, there is no problem for the ROM bootloader.
So I think the normal reboot handling should be:
If the sd card can be switched off (preferred solution), do so and reset
the controller i/o voltage to 3.0V/3.3V.
If the sd card can not be switched off, make sure to leave the
controller i/o voltage at the current setting. Make sure in later boot
stages to not change the controller i/o voltage to 3.0V/3.3V when the
card is configured for 1.8V. According to the patch mentioned above this
behaviour already is implemented in linux, we also need this for the
bootloaders.

Regards,
Soeren
>
> I have observed this issue on the following devices, and the patches at [1] makes it possible to reboot from SD-card after UHS has been enabled.
> - Asus Tinker Board (RK3288)
> - Rockchip Sapphire Board (RK3399)
> - Radxa Rock Pi 4 (RK3399)
> - Pine64 RockPro64 (RK3399)
> All of the above seem to use the RK808 regulator for sd io voltage.
>
> The ROC-RK3328-CC did not have this issue and seem to automatically reset to 3.3v.
>
> [1] https://github.com/Kwiboo/linux-rockchip/compare/next-20191011...next-20191011-mmc
>
> Regards,
> Jonas
>
>> Soeren
>>
>>
>>>> although I'm not sure what if any progress has been made since then.
>>>>
>>>> Robin.
>>>>

Robin Murphy Oct. 11, 2019, 1 p.m. UTC | #15

On 11/10/2019 12:40, Soeren Moch wrote:
> 
> 
> On 11.10.19 10:22, Jonas Karlman wrote:
>> On 2019-10-04 19:24, Sören Moch wrote:
>>> On 04.10.19 17:33, Shawn Lin wrote:
>>>> On 2019/10/4 22:20, Robin Murphy wrote:
>>>>> On 04/10/2019 04:39, Soeren Moch wrote:
>>>>>> On 04.10.19 04:13, Shawn Lin wrote:
>>>>>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>>>>>> controller is
>>>>>>>>>> connected to a microSD (TF card) slot, which cannot be switched to
>>>>>>>>>> 1.8V.
>>>>>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical to the
>>>>>>>>> NanoPC-T4 schematic (it's the same reference design, after all),
>>>>>>>>> and I
>>>>>>>>> know that board can happily drive a UHS-I microSD card with 1.8v
>>>>>>>>> I/Os,
>>>>>>>>> because mine's doing so right now.
>>>>>>>>>
>>>>>>>>> Robin.
>>>>>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>>>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>>>>>> SDMMC0_PWH_H signal is not connected. So the card fails to identify
>>>>>>>> itself after suspend or reboot when switched to 1.8V operation.
>>>>> Ah, thanks for clarifying - I did overlook the subtlety that U12 and
>>>>> friends have "NC" as alternative part numbers, even though they
>>>>> aren't actually marked as DNP. So it's still not so much "cannot be
>>>>> switched" as "switching can lead to other problems".
>>>>>
>>>>>>> I believe we addressed this issue long time ago, please check:
>>>>>>>
>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>>>>>
>>>>>>>
>>>>>> Thanks for the pointer.
>>>>>> In this case I guess I should use following patch instead:
>>>>>>
>>>>>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>>>>>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>>>>>> @@ -619,6 +619,8 @@
>>>>>>        max-frequency = <150000000>;
>>>>>>        pinctrl-names = "default";
>>>>>>        pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>>>>>> +    sd-uhs-sdr104;
>>>>>> +    vqmmc-supply = <&vcc_sdio>;
>>>>>>        status = "okay";
>>>>>>    };
>>>>>> When I do so, the sd card is detected as SDR104, but a reboot hangs:
>>>>>>
>>>>>> Boot1: 2018-06-26, version: 1.14
>>>>>> CPUId = 0x0
>>>>>> ChipType = 0x10, 286
>>>>>> Spi_ChipId = c84018
>>>>>> no find rkpartition
>>>>>> SpiBootInit:ffffffff
>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>> emmc reinit
>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>> emmc reinit
>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>> SdmmcInit=2 1
>>>>>> mmc0:cmd5,32
>>>>>> mmc0:cmd7,32
>>>>>> mmc0:cmd5,32
>>>>>> mmc0:cmd7,32
>>>>>> mmc0:cmd5,32
>>>>>> mmc0:cmd7,32
>>>>>> SdmmcInit=0 1
>>>>>>
>>>>>> So I guess I should use a different miniloader for this reboot to
>>>>>> work!?
>>>>>> Or what else could be wrong here?
>>>>> Hmm, I guess this is "the Tinkerboard problem" again - the patch
>>>>> above would be OK if we could get as far as the kernel, but can't
>>>>> help if the
>>>> I didn't realize that SD was used as boot medium for RockPro64, but I
>>>> did patch the vendor tree to solve the issue for Tinkerboard, see
>>>> https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0
>>>>
>>>>
>>>> My initial plan was to patching upstream kernel by adding ->shutdown,but
>>>> never finish it.
>>>>
>>>>> offending card is itself the boot medium. There was a proposal here:
>>>>>
>>>>> https://patchwork.kernel.org/patch/10817217/
>>>> This RFC also looks good to me, but seems it needs volunteers
>>>> to push it again.
>>> Oh, I think this is a totally wrong way.
>>>
>>> While this might work for some cards, setting the controller's i/o
>>> voltage to 3.3V while leaving the card at 1.8V configuration is totally
>>> against the specification, can lead to all kinds of strange behaviour
>>> and even cause hardware damage. It also would actively defend the
>>> purpose of the above mentioned patch (6a11fc4) where the kernel guesses
>>> the i/o voltage from the card configuration and switches the controller
>>> accordingly. We would end up with a 1.8V card and controller
>>> configuration and a regulator voltage of 3.3V. This would only work with
>>> good luck. Even if the kernel driver would switch the regulator back to
>>> 1.8V in this case, the voltage mismatch remains in the bootloader when
>>> this card contains the boot image.
>>>
>>> The only sane way I see to handle this is implementing the same
>>> workaround (mode guessing) also in the bootloader (rockchip miniloader
>>> and u-boot SPL since both bootloader chains are supported for this board).
>>>
>>> Or maybe I miss something?
>> Thanks for your input, I have made a new series [1] with a similar approach but is limited to dw_mmc-rockchip
>> and only changes the regulator at power_off after the regulator has been disabled (the vqmmc regulator in affected devices is always-on).
> Thanks for your work on this. Unfortunately I still think that this is
> the wrong approach.
> I see several problems in your patch series:
> - You introduced GPIO0_PA1 as regulator enable for RockPro64.
> Unfortunately Pine64 decided to disable this regulator on Board Version
> 2.1 (real product version), see above. I have no idea why they did this.
> - You changed the i/o voltage from 3.0V to 3.3V. This is not allowed on
> RK3399 I/O bank F.
> 
> Changing the i/o voltage to 3.0V/3.3V while the sd card is configured
> for 1.8V is against the specification and dangerous. While experimenting
> with different images (ayufan, armbian) for my newly bought RockPro64 I
> killed a SD card (32GB Samsung UHS-I). I cannot reconstruct the exact
> circumstances, but I'm pretty sure this happened due to the voltage
> mismatch. Of course I'm not keen to experiment further with this and
> kill more sd cards. This is not just an theoretical issue.
>> To my knowledge the problem is not with the rockchip miniloader or u-boot SPL, it is the initial boot rom that tries to load
>> the miniloader/SPL from a SD-card, so nothing that can be updated.
> What I observed on my RockPro64:
> The ROM bootloader always was able to load the next stage, maybe due to
> the low initial clock of 400kHz? Also the ROM bootloader cannot change
> voltage regulator settings. So if the i/o voltage still is at 1.8V and
> matching the sd card setting, there is no problem for the ROM bootloader.

Hmm, that makes me wonder if the problem might be not so much that the 
level of SDMMC0_VDD itself stays at 1.8V, but that at some point after 
the bootrom the GRF_IO_VSEL bit gets reset so the controller just stops 
being able to read anything as logic-high.

Robin.

> So I think the normal reboot handling should be:
> If the sd card can be switched off (preferred solution), do so and reset
> the controller i/o voltage to 3.0V/3.3V.
> If the sd card can not be switched off, make sure to leave the
> controller i/o voltage at the current setting. Make sure in later boot
> stages to not change the controller i/o voltage to 3.0V/3.3V when the
> card is configured for 1.8V. According to the patch mentioned above this
> behaviour already is implemented in linux, we also need this for the
> bootloaders.
> 
> Regards,
> Soeren
>>
>> I have observed this issue on the following devices, and the patches at [1] makes it possible to reboot from SD-card after UHS has been enabled.
>> - Asus Tinker Board (RK3288)
>> - Rockchip Sapphire Board (RK3399)
>> - Radxa Rock Pi 4 (RK3399)
>> - Pine64 RockPro64 (RK3399)
>> All of the above seem to use the RK808 regulator for sd io voltage.
>>
>> The ROC-RK3328-CC did not have this issue and seem to automatically reset to 3.3v.
>>
>> [1] https://github.com/Kwiboo/linux-rockchip/compare/next-20191011...next-20191011-mmc
>>
>> Regards,
>> Jonas
>>
>>> Soeren
>>>
>>>
>>>>> although I'm not sure what if any progress has been made since then.
>>>>>
>>>>> Robin.
>>>>>
> 
>

Soeren Moch Oct. 11, 2019, 2:16 p.m. UTC | #16

On 11.10.19 15:00, Robin Murphy wrote:
> On 11/10/2019 12:40, Soeren Moch wrote:
>> On 11.10.19 10:22, Jonas Karlman wrote:
>>> On 2019-10-04 19:24, Sören Moch wrote:
>>>> On 04.10.19 17:33, Shawn Lin wrote:
>>>>> On 2019/10/4 22:20, Robin Murphy wrote:
>>>>>> On 04/10/2019 04:39, Soeren Moch wrote:
>>>>>>> On 04.10.19 04:13, Shawn Lin wrote:
>>>>>>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>>>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>>>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>>>>>>> controller is
>>>>>>>>>>> connected to a microSD (TF card) slot, which cannot be
>>>>>>>>>>> switched to
>>>>>>>>>>> 1.8V.
>>>>>>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical
>>>>>>>>>> to the
>>>>>>>>>> NanoPC-T4 schematic (it's the same reference design, after all),
>>>>>>>>>> and I
>>>>>>>>>> know that board can happily drive a UHS-I microSD card with 1.8v
>>>>>>>>>> I/Os,
>>>>>>>>>> because mine's doing so right now.
>>>>>>>>>>
>>>>>>>>>> Robin.
>>>>>>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>>>>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>>>>>>> SDMMC0_PWH_H signal is not connected. So the card fails to
>>>>>>>>> identify
>>>>>>>>> itself after suspend or reboot when switched to 1.8V operation.
>>>>>> Ah, thanks for clarifying - I did overlook the subtlety that U12 and
>>>>>> friends have "NC" as alternative part numbers, even though they
>>>>>> aren't actually marked as DNP. So it's still not so much "cannot be
>>>>>> switched" as "switching can lead to other problems".
>>>>>>
>>>>>>>> I believe we addressed this issue long time ago, please check:
>>>>>>>>
>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> Thanks for the pointer.
>>>>>>> In this case I guess I should use following patch instead:
>>>>>>>
>>>>>>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>>>>>>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>>>>>>> @@ -619,6 +619,8 @@
>>>>>>>        max-frequency = <150000000>;
>>>>>>>        pinctrl-names = "default";
>>>>>>>        pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>>>>>>> +    sd-uhs-sdr104;
>>>>>>> +    vqmmc-supply = <&vcc_sdio>;
>>>>>>>        status = "okay";
>>>>>>>    };
>>>>>>> When I do so, the sd card is detected as SDR104, but a reboot
>>>>>>> hangs:
>>>>>>>
>>>>>>> Boot1: 2018-06-26, version: 1.14
>>>>>>> CPUId = 0x0
>>>>>>> ChipType = 0x10, 286
>>>>>>> Spi_ChipId = c84018
>>>>>>> no find rkpartition
>>>>>>> SpiBootInit:ffffffff
>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>> emmc reinit
>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>> emmc reinit
>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>> SdmmcInit=2 1
>>>>>>> mmc0:cmd5,32
>>>>>>> mmc0:cmd7,32
>>>>>>> mmc0:cmd5,32
>>>>>>> mmc0:cmd7,32
>>>>>>> mmc0:cmd5,32
>>>>>>> mmc0:cmd7,32
>>>>>>> SdmmcInit=0 1
>>>>>>>
>>>>>>> So I guess I should use a different miniloader for this reboot to
>>>>>>> work!?
>>>>>>> Or what else could be wrong here?
>>>>>> Hmm, I guess this is "the Tinkerboard problem" again - the patch
>>>>>> above would be OK if we could get as far as the kernel, but can't
>>>>>> help if the
>>>>> I didn't realize that SD was used as boot medium for RockPro64, but I
>>>>> did patch the vendor tree to solve the issue for Tinkerboard, see
>>>>> https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0
>>>>>
>>>>>
>>>>>
>>>>> My initial plan was to patching upstream kernel by adding
>>>>> ->shutdown,but
>>>>> never finish it.
>>>>>
>>>>>> offending card is itself the boot medium. There was a proposal here:
>>>>>>
>>>>>> https://patchwork.kernel.org/patch/10817217/
>>>>> This RFC also looks good to me, but seems it needs volunteers
>>>>> to push it again.
>>>> Oh, I think this is a totally wrong way.
>>>>
>>>> While this might work for some cards, setting the controller's i/o
>>>> voltage to 3.3V while leaving the card at 1.8V configuration is
>>>> totally
>>>> against the specification, can lead to all kinds of strange behaviour
>>>> and even cause hardware damage. It also would actively defend the
>>>> purpose of the above mentioned patch (6a11fc4) where the kernel
>>>> guesses
>>>> the i/o voltage from the card configuration and switches the
>>>> controller
>>>> accordingly. We would end up with a 1.8V card and controller
>>>> configuration and a regulator voltage of 3.3V. This would only work
>>>> with
>>>> good luck. Even if the kernel driver would switch the regulator
>>>> back to
>>>> 1.8V in this case, the voltage mismatch remains in the bootloader when
>>>> this card contains the boot image.
>>>>
>>>> The only sane way I see to handle this is implementing the same
>>>> workaround (mode guessing) also in the bootloader (rockchip miniloader
>>>> and u-boot SPL since both bootloader chains are supported for this
>>>> board).
>>>>
>>>> Or maybe I miss something?
>>> Thanks for your input, I have made a new series [1] with a similar
>>> approach but is limited to dw_mmc-rockchip
>>> and only changes the regulator at power_off after the regulator has
>>> been disabled (the vqmmc regulator in affected devices is always-on).
>> Thanks for your work on this. Unfortunately I still think that this is
>> the wrong approach.
>> I see several problems in your patch series:
>> - You introduced GPIO0_PA1 as regulator enable for RockPro64.
>> Unfortunately Pine64 decided to disable this regulator on Board Version
>> 2.1 (real product version), see above. I have no idea why they did this.
>> - You changed the i/o voltage from 3.0V to 3.3V. This is not allowed on
>> RK3399 I/O bank F.
>>
>> Changing the i/o voltage to 3.0V/3.3V while the sd card is configured
>> for 1.8V is against the specification and dangerous. While experimenting
>> with different images (ayufan, armbian) for my newly bought RockPro64 I
>> killed a SD card (32GB Samsung UHS-I). I cannot reconstruct the exact
>> circumstances, but I'm pretty sure this happened due to the voltage
>> mismatch. Of course I'm not keen to experiment further with this and
>> kill more sd cards. This is not just an theoretical issue.
>>> To my knowledge the problem is not with the rockchip miniloader or
>>> u-boot SPL, it is the initial boot rom that tries to load
>>> the miniloader/SPL from a SD-card, so nothing that can be updated.
>> What I observed on my RockPro64:
>> The ROM bootloader always was able to load the next stage, maybe due to
>> the low initial clock of 400kHz? Also the ROM bootloader cannot change
>> voltage regulator settings. So if the i/o voltage still is at 1.8V and
>> matching the sd card setting, there is no problem for the ROM
>> bootloader.
>
> Hmm, that makes me wonder if the problem might be not so much that the
> level of SDMMC0_VDD itself stays at 1.8V, but that at some point after
> the bootrom the GRF_IO_VSEL bit gets reset so the controller just
> stops being able to read anything as logic-high.
Would be great if someone from Rockchip could give some insights whether
and when during reboot GRF_IO_VSEL is reset (ATF before reboot, some SoC
soft-reset, ROM bootloader, rkminiloader, something different), Shawn?
Or gets this VSEL changed only when switching the voltage regulator (via
io_domains,sdmmc-supply)?

Soeren
>
> Robin.
>
>> So I think the normal reboot handling should be:
>> If the sd card can be switched off (preferred solution), do so and reset
>> the controller i/o voltage to 3.0V/3.3V.
>> If the sd card can not be switched off, make sure to leave the
>> controller i/o voltage at the current setting. Make sure in later boot
>> stages to not change the controller i/o voltage to 3.0V/3.3V when the
>> card is configured for 1.8V. According to the patch mentioned above this
>> behaviour already is implemented in linux, we also need this for the
>> bootloaders.
>>
>> Regards,
>> Soeren
>>>
>>> I have observed this issue on the following devices, and the patches
>>> at [1] makes it possible to reboot from SD-card after UHS has been
>>> enabled.
>>> - Asus Tinker Board (RK3288)
>>> - Rockchip Sapphire Board (RK3399)
>>> - Radxa Rock Pi 4 (RK3399)
>>> - Pine64 RockPro64 (RK3399)
>>> All of the above seem to use the RK808 regulator for sd io voltage.
>>>
>>> The ROC-RK3328-CC did not have this issue and seem to automatically
>>> reset to 3.3v.
>>>
>>> [1]
>>> https://github.com/Kwiboo/linux-rockchip/compare/next-20191011...next-20191011-mmc
>>>
>>> Regards,
>>> Jonas
>>>
>>>> Soeren
>>>>
>>>>
>>>>>> although I'm not sure what if any progress has been made since then.
>>>>>>
>>>>>> Robin.
>>>>>>
>>
>>

Shawn Lin Oct. 12, 2019, 4:09 a.m. UTC | #17

On 2019/10/11 22:16, Soeren Moch wrote:
> On 11.10.19 15:00, Robin Murphy wrote:
>> On 11/10/2019 12:40, Soeren Moch wrote:
>>> On 11.10.19 10:22, Jonas Karlman wrote:
>>>> On 2019-10-04 19:24, Sören Moch wrote:
>>>>> On 04.10.19 17:33, Shawn Lin wrote:
>>>>>> On 2019/10/4 22:20, Robin Murphy wrote:
>>>>>>> On 04/10/2019 04:39, Soeren Moch wrote:
>>>>>>>> On 04.10.19 04:13, Shawn Lin wrote:
>>>>>>>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>>>>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>>>>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>>>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>>>>>>>> controller is
>>>>>>>>>>>> connected to a microSD (TF card) slot, which cannot be
>>>>>>>>>>>> switched to
>>>>>>>>>>>> 1.8V.
>>>>>>>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical
>>>>>>>>>>> to the
>>>>>>>>>>> NanoPC-T4 schematic (it's the same reference design, after all),
>>>>>>>>>>> and I
>>>>>>>>>>> know that board can happily drive a UHS-I microSD card with 1.8v
>>>>>>>>>>> I/Os,
>>>>>>>>>>> because mine's doing so right now.
>>>>>>>>>>>
>>>>>>>>>>> Robin.
>>>>>>>>>> OK, the RockPro64 does not allow a card reset (power cycle) since
>>>>>>>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>>>>>>>> SDMMC0_PWH_H signal is not connected. So the card fails to
>>>>>>>>>> identify
>>>>>>>>>> itself after suspend or reboot when switched to 1.8V operation.
>>>>>>> Ah, thanks for clarifying - I did overlook the subtlety that U12 and
>>>>>>> friends have "NC" as alternative part numbers, even though they
>>>>>>> aren't actually marked as DNP. So it's still not so much "cannot be
>>>>>>> switched" as "switching can lead to other problems".
>>>>>>>
>>>>>>>>> I believe we addressed this issue long time ago, please check:
>>>>>>>>>
>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> Thanks for the pointer.
>>>>>>>> In this case I guess I should use following patch instead:
>>>>>>>>
>>>>>>>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799 +0200
>>>>>>>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>>>>>>>> @@ -619,6 +619,8 @@
>>>>>>>>         max-frequency = <150000000>;
>>>>>>>>         pinctrl-names = "default";
>>>>>>>>         pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>>>>>>>> +    sd-uhs-sdr104;
>>>>>>>> +    vqmmc-supply = <&vcc_sdio>;
>>>>>>>>         status = "okay";
>>>>>>>>     };
>>>>>>>> When I do so, the sd card is detected as SDR104, but a reboot
>>>>>>>> hangs:
>>>>>>>>
>>>>>>>> Boot1: 2018-06-26, version: 1.14
>>>>>>>> CPUId = 0x0
>>>>>>>> ChipType = 0x10, 286
>>>>>>>> Spi_ChipId = c84018
>>>>>>>> no find rkpartition
>>>>>>>> SpiBootInit:ffffffff
>>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>>> emmc reinit
>>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>>> emmc reinit
>>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>>> SdmmcInit=2 1
>>>>>>>> mmc0:cmd5,32
>>>>>>>> mmc0:cmd7,32
>>>>>>>> mmc0:cmd5,32
>>>>>>>> mmc0:cmd7,32
>>>>>>>> mmc0:cmd5,32
>>>>>>>> mmc0:cmd7,32
>>>>>>>> SdmmcInit=0 1
>>>>>>>>
>>>>>>>> So I guess I should use a different miniloader for this reboot to
>>>>>>>> work!?
>>>>>>>> Or what else could be wrong here?
>>>>>>> Hmm, I guess this is "the Tinkerboard problem" again - the patch
>>>>>>> above would be OK if we could get as far as the kernel, but can't
>>>>>>> help if the
>>>>>> I didn't realize that SD was used as boot medium for RockPro64, but I
>>>>>> did patch the vendor tree to solve the issue for Tinkerboard, see
>>>>>> https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0
>>>>>>
>>>>>>
>>>>>>
>>>>>> My initial plan was to patching upstream kernel by adding
>>>>>> ->shutdown,but
>>>>>> never finish it.
>>>>>>
>>>>>>> offending card is itself the boot medium. There was a proposal here:
>>>>>>>
>>>>>>> https://patchwork.kernel.org/patch/10817217/
>>>>>> This RFC also looks good to me, but seems it needs volunteers
>>>>>> to push it again.
>>>>> Oh, I think this is a totally wrong way.
>>>>>
>>>>> While this might work for some cards, setting the controller's i/o
>>>>> voltage to 3.3V while leaving the card at 1.8V configuration is
>>>>> totally
>>>>> against the specification, can lead to all kinds of strange behaviour
>>>>> and even cause hardware damage. It also would actively defend the
>>>>> purpose of the above mentioned patch (6a11fc4) where the kernel
>>>>> guesses
>>>>> the i/o voltage from the card configuration and switches the
>>>>> controller
>>>>> accordingly. We would end up with a 1.8V card and controller
>>>>> configuration and a regulator voltage of 3.3V. This would only work
>>>>> with
>>>>> good luck. Even if the kernel driver would switch the regulator
>>>>> back to
>>>>> 1.8V in this case, the voltage mismatch remains in the bootloader when
>>>>> this card contains the boot image.
>>>>>
>>>>> The only sane way I see to handle this is implementing the same
>>>>> workaround (mode guessing) also in the bootloader (rockchip miniloader
>>>>> and u-boot SPL since both bootloader chains are supported for this
>>>>> board).
>>>>>
>>>>> Or maybe I miss something?
>>>> Thanks for your input, I have made a new series [1] with a similar
>>>> approach but is limited to dw_mmc-rockchip
>>>> and only changes the regulator at power_off after the regulator has
>>>> been disabled (the vqmmc regulator in affected devices is always-on).
>>> Thanks for your work on this. Unfortunately I still think that this is
>>> the wrong approach.
>>> I see several problems in your patch series:
>>> - You introduced GPIO0_PA1 as regulator enable for RockPro64.
>>> Unfortunately Pine64 decided to disable this regulator on Board Version
>>> 2.1 (real product version), see above. I have no idea why they did this.
>>> - You changed the i/o voltage from 3.0V to 3.3V. This is not allowed on
>>> RK3399 I/O bank F.
>>>
>>> Changing the i/o voltage to 3.0V/3.3V while the sd card is configured
>>> for 1.8V is against the specification and dangerous. While experimenting
>>> with different images (ayufan, armbian) for my newly bought RockPro64 I
>>> killed a SD card (32GB Samsung UHS-I). I cannot reconstruct the exact
>>> circumstances, but I'm pretty sure this happened due to the voltage
>>> mismatch. Of course I'm not keen to experiment further with this and
>>> kill more sd cards. This is not just an theoretical issue.
>>>> To my knowledge the problem is not with the rockchip miniloader or
>>>> u-boot SPL, it is the initial boot rom that tries to load
>>>> the miniloader/SPL from a SD-card, so nothing that can be updated.
>>> What I observed on my RockPro64:
>>> The ROM bootloader always was able to load the next stage, maybe due to
>>> the low initial clock of 400kHz? Also the ROM bootloader cannot change
>>> voltage regulator settings. So if the i/o voltage still is at 1.8V and
>>> matching the sd card setting, there is no problem for the ROM
>>> bootloader.
>>
>> Hmm, that makes me wonder if the problem might be not so much that the
>> level of SDMMC0_VDD itself stays at 1.8V, but that at some point after
>> the bootrom the GRF_IO_VSEL bit gets reset so the controller just
>> stops being able to read anything as logic-high.
> Would be great if someone from Rockchip could give some insights whether
> and when during reboot GRF_IO_VSEL is reset (ATF before reboot, some SoC
> soft-reset, ROM bootloader, rkminiloader, something different), Shawn?

ROM code and miniloader never touch this GRF_IO_VSEL for sdmmc, and only
kernel did since now it support UHS mode. After reboot, the value should
depends on the (value-in-kernel && does-reboot-level-hold-this-reg?)

Different SoCs+PMICs excute differents reboot level policy.


> Or gets this VSEL changed only when switching the voltage regulator (via
> io_domains,sdmmc-supply)?
> 
> Soeren
>>
>> Robin.
>>
>>> So I think the normal reboot handling should be:
>>> If the sd card can be switched off (preferred solution), do so and reset
>>> the controller i/o voltage to 3.0V/3.3V.
>>> If the sd card can not be switched off, make sure to leave the
>>> controller i/o voltage at the current setting. Make sure in later boot
>>> stages to not change the controller i/o voltage to 3.0V/3.3V when the
>>> card is configured for 1.8V. According to the patch mentioned above this
>>> behaviour already is implemented in linux, we also need this for the
>>> bootloaders.
>>>
>>> Regards,
>>> Soeren
>>>>
>>>> I have observed this issue on the following devices, and the patches
>>>> at [1] makes it possible to reboot from SD-card after UHS has been
>>>> enabled.
>>>> - Asus Tinker Board (RK3288)
>>>> - Rockchip Sapphire Board (RK3399)
>>>> - Radxa Rock Pi 4 (RK3399)
>>>> - Pine64 RockPro64 (RK3399)
>>>> All of the above seem to use the RK808 regulator for sd io voltage.
>>>>
>>>> The ROC-RK3328-CC did not have this issue and seem to automatically
>>>> reset to 3.3v.
>>>>
>>>> [1]
>>>> https://github.com/Kwiboo/linux-rockchip/compare/next-20191011...next-20191011-mmc
>>>>
>>>> Regards,
>>>> Jonas
>>>>
>>>>> Soeren
>>>>>
>>>>>
>>>>>>> although I'm not sure what if any progress has been made since then.
>>>>>>>
>>>>>>> Robin.
>>>>>>>
>>>
>>>
> 
> 
> 
> 
>

Soeren Moch Oct. 12, 2019, 12:45 p.m. UTC | #18

On 12.10.19 06:09, Shawn Lin wrote:
> On 2019/10/11 22:16, Soeren Moch wrote:
>> On 11.10.19 15:00, Robin Murphy wrote:
>>> On 11/10/2019 12:40, Soeren Moch wrote:
>>>> On 11.10.19 10:22, Jonas Karlman wrote:
>>>>> On 2019-10-04 19:24, Sören Moch wrote:
>>>>>> On 04.10.19 17:33, Shawn Lin wrote:
>>>>>>> On 2019/10/4 22:20, Robin Murphy wrote:
>>>>>>>> On 04/10/2019 04:39, Soeren Moch wrote:
>>>>>>>>> On 04.10.19 04:13, Shawn Lin wrote:
>>>>>>>>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>>>>>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>>>>>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>>>>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>>>>>>>>> controller is
>>>>>>>>>>>>> connected to a microSD (TF card) slot, which cannot be
>>>>>>>>>>>>> switched to
>>>>>>>>>>>>> 1.8V.
>>>>>>>>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical
>>>>>>>>>>>> to the
>>>>>>>>>>>> NanoPC-T4 schematic (it's the same reference design, after
>>>>>>>>>>>> all),
>>>>>>>>>>>> and I
>>>>>>>>>>>> know that board can happily drive a UHS-I microSD card with
>>>>>>>>>>>> 1.8v
>>>>>>>>>>>> I/Os,
>>>>>>>>>>>> because mine's doing so right now.
>>>>>>>>>>>>
>>>>>>>>>>>> Robin.
>>>>>>>>>>> OK, the RockPro64 does not allow a card reset (power cycle)
>>>>>>>>>>> since
>>>>>>>>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>>>>>>>>> SDMMC0_PWH_H signal is not connected. So the card fails to
>>>>>>>>>>> identify
>>>>>>>>>>> itself after suspend or reboot when switched to 1.8V operation.
>>>>>>>> Ah, thanks for clarifying - I did overlook the subtlety that
>>>>>>>> U12 and
>>>>>>>> friends have "NC" as alternative part numbers, even though they
>>>>>>>> aren't actually marked as DNP. So it's still not so much
>>>>>>>> "cannot be
>>>>>>>> switched" as "switching can lead to other problems".
>>>>>>>>
>>>>>>>>>> I believe we addressed this issue long time ago, please check:
>>>>>>>>>>
>>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> Thanks for the pointer.
>>>>>>>>> In this case I guess I should use following patch instead:
>>>>>>>>>
>>>>>>>>> --- rk3399-rockpro64.dts.bak    2019-10-03 22:14:00.067745799
>>>>>>>>> +0200
>>>>>>>>> +++ rk3399-rockpro64.dts    2019-10-04 00:02:50.047892366 +0200
>>>>>>>>> @@ -619,6 +619,8 @@
>>>>>>>>>         max-frequency = <150000000>;
>>>>>>>>>         pinctrl-names = "default";
>>>>>>>>>         pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>>>>>>>>> +    sd-uhs-sdr104;
>>>>>>>>> +    vqmmc-supply = <&vcc_sdio>;
>>>>>>>>>         status = "okay";
>>>>>>>>>     };
>>>>>>>>> When I do so, the sd card is detected as SDR104, but a reboot
>>>>>>>>> hangs:
>>>>>>>>>
>>>>>>>>> Boot1: 2018-06-26, version: 1.14
>>>>>>>>> CPUId = 0x0
>>>>>>>>> ChipType = 0x10, 286
>>>>>>>>> Spi_ChipId = c84018
>>>>>>>>> no find rkpartition
>>>>>>>>> SpiBootInit:ffffffff
>>>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>>>> emmc reinit
>>>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>>>> emmc reinit
>>>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>>>> SdmmcInit=2 1
>>>>>>>>> mmc0:cmd5,32
>>>>>>>>> mmc0:cmd7,32
>>>>>>>>> mmc0:cmd5,32
>>>>>>>>> mmc0:cmd7,32
>>>>>>>>> mmc0:cmd5,32
>>>>>>>>> mmc0:cmd7,32
>>>>>>>>> SdmmcInit=0 1
>>>>>>>>>
>>>>>>>>> So I guess I should use a different miniloader for this reboot to
>>>>>>>>> work!?
>>>>>>>>> Or what else could be wrong here?
>>>>>>>> Hmm, I guess this is "the Tinkerboard problem" again - the patch
>>>>>>>> above would be OK if we could get as far as the kernel, but can't
>>>>>>>> help if the
>>>>>>> I didn't realize that SD was used as boot medium for RockPro64,
>>>>>>> but I
>>>>>>> did patch the vendor tree to solve the issue for Tinkerboard, see
>>>>>>> https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> My initial plan was to patching upstream kernel by adding
>>>>>>> ->shutdown,but
>>>>>>> never finish it.
>>>>>>>
>>>>>>>> offending card is itself the boot medium. There was a proposal
>>>>>>>> here:
>>>>>>>>
>>>>>>>> https://patchwork.kernel.org/patch/10817217/
>>>>>>> This RFC also looks good to me, but seems it needs volunteers
>>>>>>> to push it again.
>>>>>> Oh, I think this is a totally wrong way.
>>>>>>
>>>>>> While this might work for some cards, setting the controller's i/o
>>>>>> voltage to 3.3V while leaving the card at 1.8V configuration is
>>>>>> totally
>>>>>> against the specification, can lead to all kinds of strange
>>>>>> behaviour
>>>>>> and even cause hardware damage. It also would actively defend the
>>>>>> purpose of the above mentioned patch (6a11fc4) where the kernel
>>>>>> guesses
>>>>>> the i/o voltage from the card configuration and switches the
>>>>>> controller
>>>>>> accordingly. We would end up with a 1.8V card and controller
>>>>>> configuration and a regulator voltage of 3.3V. This would only work
>>>>>> with
>>>>>> good luck. Even if the kernel driver would switch the regulator
>>>>>> back to
>>>>>> 1.8V in this case, the voltage mismatch remains in the bootloader
>>>>>> when
>>>>>> this card contains the boot image.
>>>>>>
>>>>>> The only sane way I see to handle this is implementing the same
>>>>>> workaround (mode guessing) also in the bootloader (rockchip
>>>>>> miniloader
>>>>>> and u-boot SPL since both bootloader chains are supported for this
>>>>>> board).
>>>>>>
>>>>>> Or maybe I miss something?
>>>>> Thanks for your input, I have made a new series [1] with a similar
>>>>> approach but is limited to dw_mmc-rockchip
>>>>> and only changes the regulator at power_off after the regulator has
>>>>> been disabled (the vqmmc regulator in affected devices is always-on).
>>>> Thanks for your work on this. Unfortunately I still think that this is
>>>> the wrong approach.
>>>> I see several problems in your patch series:
>>>> - You introduced GPIO0_PA1 as regulator enable for RockPro64.
>>>> Unfortunately Pine64 decided to disable this regulator on Board
>>>> Version
>>>> 2.1 (real product version), see above. I have no idea why they did
>>>> this.
>>>> - You changed the i/o voltage from 3.0V to 3.3V. This is not
>>>> allowed on
>>>> RK3399 I/O bank F.
>>>>
>>>> Changing the i/o voltage to 3.0V/3.3V while the sd card is configured
>>>> for 1.8V is against the specification and dangerous. While
>>>> experimenting
>>>> with different images (ayufan, armbian) for my newly bought
>>>> RockPro64 I
>>>> killed a SD card (32GB Samsung UHS-I). I cannot reconstruct the exact
>>>> circumstances, but I'm pretty sure this happened due to the voltage
>>>> mismatch. Of course I'm not keen to experiment further with this and
>>>> kill more sd cards. This is not just an theoretical issue.
>>>>> To my knowledge the problem is not with the rockchip miniloader or
>>>>> u-boot SPL, it is the initial boot rom that tries to load
>>>>> the miniloader/SPL from a SD-card, so nothing that can be updated.
>>>> What I observed on my RockPro64:
>>>> The ROM bootloader always was able to load the next stage, maybe
>>>> due to
>>>> the low initial clock of 400kHz? Also the ROM bootloader cannot change
>>>> voltage regulator settings. So if the i/o voltage still is at 1.8V and
>>>> matching the sd card setting, there is no problem for the ROM
>>>> bootloader.
>>>
>>> Hmm, that makes me wonder if the problem might be not so much that the
>>> level of SDMMC0_VDD itself stays at 1.8V, but that at some point after
>>> the bootrom the GRF_IO_VSEL bit gets reset so the controller just
>>> stops being able to read anything as logic-high.
>> Would be great if someone from Rockchip could give some insights whether
>> and when during reboot GRF_IO_VSEL is reset (ATF before reboot, some SoC
>> soft-reset, ROM bootloader, rkminiloader, something different), Shawn?
>
> ROM code and miniloader never touch this GRF_IO_VSEL for sdmmc, and only
> kernel did since now it support UHS mode. After reboot, the value should
> depends on the (value-in-kernel && does-reboot-level-hold-this-reg?)
>
> Different SoCs+PMICs excute differents reboot level policy.
OK, thanks. RockPro64 uses RK3399 and RK808D.
Is there such a soft-reset for GRF_IO_VSEL in RK3399?


Let's have another detailed look at the reboot process on my RockPro64
with SD UHS enabled:

[   84.675548] reboot: Restarting system
DDR Version 1.23 20190709
[...]
change freq to 856MHz 1,0
ch 0 ddrconfig = 0x101, ddrsize = 0x2020
ch 1 ddrconfig = 0x101, ddrsize = 0x2020
pmugrf_os_reg[2] = 0x3AA1FAA1, stride = 0xD
OUT
Boot1: 2018-06-26, version: 1.14
CPUId = 0x0
ChipType = 0x10, 287
Spi_ChipId = c84018
no find rkpartition
SpiBootInit:ffffffff
mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
mmc: ERROR: Card did not respond to voltage select!
emmc reinit
mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
mmc: ERROR: Card did not respond to voltage select!
emmc reinit
mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
mmc: ERROR: Card did not respond to voltage select!
SdmmcInit=2 1
mmc0:cmd5,32
mmc0:cmd16,32
mmc0:cmd5,32
mmc0:cmd16,32
mmc0:cmd5,32                                                                                                                       

mmc0:cmd16,32
SdmmcInit=0 1
powerOn 489837
Usb re Boot. 6489831
Usb re Boot. 12489837
Usb re Boot. 18489840
SoftReset
DDR Version 1.23 20190709
[boot loop]

What we can see here:
The ROM bootloader is able to access the SD card after reboot, it loads
and starts the DDR config firmware without any problem. The DDR firmware
runs through and loads and starts the miniloader without any problem.
The miniloader then tries to find RKTRUST on SPI (empty on my board),
eMMC (not populated) and SD card and fails to access the SD card
properly, if this card is configured for UHS mode. (With SD card at HS
not UHS configuration the miniloader just loads and starts ATF, TEE and
u-boot without any problem.)

So in principle the UHS configured SD card can be accessed after reboot
without any problem. At least ROM bootloader and DDR firmware do so. The
miniloader seems to do something differently and therefore fails.
Shawn, any idea what's going on there?

Soeren


>
>> Or gets this VSEL changed only when switching the voltage regulator (via
>> io_domains,sdmmc-supply)?
>>
>> Soeren
>>>
>>> Robin.
>>>
>>>> So I think the normal reboot handling should be:
>>>> If the sd card can be switched off (preferred solution), do so and
>>>> reset
>>>> the controller i/o voltage to 3.0V/3.3V.
>>>> If the sd card can not be switched off, make sure to leave the
>>>> controller i/o voltage at the current setting. Make sure in later boot
>>>> stages to not change the controller i/o voltage to 3.0V/3.3V when the
>>>> card is configured for 1.8V. According to the patch mentioned above
>>>> this
>>>> behaviour already is implemented in linux, we also need this for the
>>>> bootloaders.
>>>>
>>>> Regards,
>>>> Soeren
>>>>>
>>>>> I have observed this issue on the following devices, and the patches
>>>>> at [1] makes it possible to reboot from SD-card after UHS has been
>>>>> enabled.
>>>>> - Asus Tinker Board (RK3288)
>>>>> - Rockchip Sapphire Board (RK3399)
>>>>> - Radxa Rock Pi 4 (RK3399)
>>>>> - Pine64 RockPro64 (RK3399)
>>>>> All of the above seem to use the RK808 regulator for sd io voltage.
>>>>>
>>>>> The ROC-RK3328-CC did not have this issue and seem to automatically
>>>>> reset to 3.3v.
>>>>>
>>>>> [1]
>>>>> https://github.com/Kwiboo/linux-rockchip/compare/next-20191011...next-20191011-mmc
>>>>>
>>>>>
>>>>> Regards,
>>>>> Jonas
>>>>>
>>>>>> Soeren
>>>>>>
>>>>>>
>>>>>>>> although I'm not sure what if any progress has been made since
>>>>>>>> then.
>>>>>>>>
>>>>>>>> Robin.
>>>>>>>>
>>>>
>>>>
>>
>>
>>
>>
>>
>
>

[3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings

Commit Message

Comments

Patch