linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings
@ 2019-10-03 21:50 Soeren Moch
  2019-10-03 21:50 ` [PATCH 2/3] arm64: dts: rockchip: fix RockPro64 sdhci settings Soeren Moch
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Soeren Moch @ 2019-10-03 21:50 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-rockchip, Soeren Moch, linux-kernel, linux-arm-kernel

The RockPro64 schematic [1] page 18 states a min voltage of 0.8V and a
max voltage of 1.4V for the VDD_LOG pwm regulator. However, there is an
additional note that the pwm parameter needs to be modified.
From the schematics a voltage range of 0.8V to 1.7V can be calculated.
Additional voltage measurements on the board show that this fix indeed
leads to the correct voltage, while without this fix the voltage was set
too high.

[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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
index 0401d4ec1f45..845eb070b5b0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -173,7 +173,7 @@
 		regulator-always-on;
 		regulator-boot-on;
 		regulator-min-microvolt = <800000>;
-		regulator-max-microvolt = <1400000>;
+		regulator-max-microvolt = <1700000>;
 		vin-supply = <&vcc5v0_sys>;
 	};
 };
--
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/3] arm64: dts: rockchip: fix RockPro64 sdhci settings
  2019-10-03 21:50 [PATCH 1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings Soeren Moch
@ 2019-10-03 21:50 ` Soeren Moch
  2019-10-04 20:05   ` Heiko Stuebner
  2019-10-03 21:50 ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings Soeren Moch
  2019-10-04 20:04 ` [PATCH 1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings Heiko Stuebner
  2 siblings, 1 reply; 23+ messages in thread
From: Soeren Moch @ 2019-10-03 21:50 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-rockchip, Soeren Moch, linux-kernel, linux-arm-kernel

The RockPro64 schematics [1], [2] show that the rk3399 EMMC_STRB pin is
connected to the RESET pin instead of the DATA_STROBE pin of the eMMC module.
So the data strobe cannot be used for its intended purpose on this board,
and so the HS400 eMMC mode is not functional. Limit the controller to HS200.

[1] http://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf
[2] http://files.pine64.org/doc/rock64/PINE64_eMMC_Module_20170719.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 845eb070b5b0..2e44dae4865a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -636,8 +636,7 @@

 &sdhci {
 	bus-width = <8>;
-	mmc-hs400-1_8v;
-	mmc-hs400-enhanced-strobe;
+	mmc-hs200-1_8v;
 	non-removable;
 	status = "okay";
 };
--
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings
  2019-10-03 21:50 [PATCH 1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings Soeren Moch
  2019-10-03 21:50 ` [PATCH 2/3] arm64: dts: rockchip: fix RockPro64 sdhci settings Soeren Moch
@ 2019-10-03 21:50 ` Soeren Moch
  2019-10-04  0:01   ` Robin Murphy
  2019-10-04 20:04 ` [PATCH 1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings Heiko Stuebner
  2 siblings, 1 reply; 23+ messages in thread
From: Soeren Moch @ 2019-10-03 21:50 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-rockchip, Soeren Moch, linux-kernel, linux-arm-kernel

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

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings
  2019-10-03 21:50 ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings Soeren Moch
@ 2019-10-04  0:01   ` Robin Murphy
  2019-10-04  0:53     ` Soeren Moch
  0 siblings, 1 reply; 23+ messages in thread
From: Robin Murphy @ 2019-10-04  0:01 UTC (permalink / raw)
  To: Soeren Moch, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel

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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings
  2019-10-04  0:01   ` Robin Murphy
@ 2019-10-04  0:53     ` Soeren Moch
  2019-10-04  2:13       ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】 Shawn Lin
  0 siblings, 1 reply; 23+ messages in thread
From: Soeren Moch @ 2019-10-04  0:53 UTC (permalink / raw)
  To: Robin Murphy, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel



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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04  0:53     ` Soeren Moch
@ 2019-10-04  2:13       ` Shawn Lin
  2019-10-04  3:39         ` Soeren Moch
  0 siblings, 1 reply; 23+ messages in thread
From: Shawn Lin @ 2019-10-04  2:13 UTC (permalink / raw)
  To: Soeren Moch, Robin Murphy, Heiko Stuebner
  Cc: linux-rockchip, shawn.lin, linux-kernel, linux-arm-kernel

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
> 


-- 
Best Regards
Shawn Lin



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04  2:13       ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】 Shawn Lin
@ 2019-10-04  3:39         ` Soeren Moch
  2019-10-04 14:20           ` Robin Murphy
  0 siblings, 1 reply; 23+ messages in thread
From: Soeren Moch @ 2019-10-04  3:39 UTC (permalink / raw)
  To: Shawn Lin, Robin Murphy, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel



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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04  3:39         ` Soeren Moch
@ 2019-10-04 14:20           ` Robin Murphy
  2019-10-04 14:42             ` Jonas Karlman
                               ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Robin Murphy @ 2019-10-04 14:20 UTC (permalink / raw)
  To: Soeren Moch, Shawn Lin, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04 14:20           ` Robin Murphy
@ 2019-10-04 14:42             ` Jonas Karlman
  2019-10-04 15:33             ` Shawn Lin
  2019-10-04 16:39             ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】 Soeren Moch
  2 siblings, 0 replies; 23+ messages in thread
From: Jonas Karlman @ 2019-10-04 14:42 UTC (permalink / raw)
  To: Robin Murphy, Soeren Moch, Shawn Lin, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04 14:20           ` Robin Murphy
  2019-10-04 14:42             ` Jonas Karlman
@ 2019-10-04 15:33             ` Shawn Lin
  2019-10-04 17:24               ` Sören Moch
  2019-10-04 16:39             ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】 Soeren Moch
  2 siblings, 1 reply; 23+ messages in thread
From: Shawn Lin @ 2019-10-04 15:33 UTC (permalink / raw)
  To: Robin Murphy, Soeren Moch, Heiko Stuebner
  Cc: linux-rockchip, shawn.lin, linux-kernel, linux-arm-kernel

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


-- 
Best Regards
Shawn Lin



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04 14:20           ` Robin Murphy
  2019-10-04 14:42             ` Jonas Karlman
  2019-10-04 15:33             ` Shawn Lin
@ 2019-10-04 16:39             ` Soeren Moch
  2 siblings, 0 replies; 23+ messages in thread
From: Soeren Moch @ 2019-10-04 16:39 UTC (permalink / raw)
  To: Robin Murphy, Shawn Lin, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel



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.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04 15:33             ` Shawn Lin
@ 2019-10-04 17:24               ` Sören Moch
  2019-10-04 20:15                 ` Soeren Moch
  2019-10-11  8:22                 ` Jonas Karlman
  0 siblings, 2 replies; 23+ messages in thread
From: Sören Moch @ 2019-10-04 17:24 UTC (permalink / raw)
  To: Shawn Lin, Robin Murphy, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel



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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings
  2019-10-03 21:50 [PATCH 1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings Soeren Moch
  2019-10-03 21:50 ` [PATCH 2/3] arm64: dts: rockchip: fix RockPro64 sdhci settings Soeren Moch
  2019-10-03 21:50 ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings Soeren Moch
@ 2019-10-04 20:04 ` Heiko Stuebner
  2 siblings, 0 replies; 23+ messages in thread
From: Heiko Stuebner @ 2019-10-04 20:04 UTC (permalink / raw)
  To: Soeren Moch; +Cc: linux-rockchip, linux-kernel, linux-arm-kernel

Am Donnerstag, 3. Oktober 2019, 23:50:34 CEST schrieb Soeren Moch:
> The RockPro64 schematic [1] page 18 states a min voltage of 0.8V and a
> max voltage of 1.4V for the VDD_LOG pwm regulator. However, there is an
> additional note that the pwm parameter needs to be modified.
> From the schematics a voltage range of 0.8V to 1.7V can be calculated.
> Additional voltage measurements on the board show that this fix indeed
> leads to the correct voltage, while without this fix the voltage was set
> too high.
> 
> [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>

applied as fix  for 5.4

Thanks for going that extra mile with all the calculations and measurements
Heiko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] arm64: dts: rockchip: fix RockPro64 sdhci settings
  2019-10-03 21:50 ` [PATCH 2/3] arm64: dts: rockchip: fix RockPro64 sdhci settings Soeren Moch
@ 2019-10-04 20:05   ` Heiko Stuebner
  0 siblings, 0 replies; 23+ messages in thread
From: Heiko Stuebner @ 2019-10-04 20:05 UTC (permalink / raw)
  To: Soeren Moch; +Cc: linux-rockchip, linux-kernel, linux-arm-kernel

Am Donnerstag, 3. Oktober 2019, 23:50:35 CEST schrieb Soeren Moch:
> The RockPro64 schematics [1], [2] show that the rk3399 EMMC_STRB pin is
> connected to the RESET pin instead of the DATA_STROBE pin of the eMMC module.
> So the data strobe cannot be used for its intended purpose on this board,
> and so the HS400 eMMC mode is not functional. Limit the controller to HS200.
> 
> [1] http://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf
> [2] http://files.pine64.org/doc/rock64/PINE64_eMMC_Module_20170719.pdf
> 
> Fixes: e4f3fb490967 ("arm64: dts: rockchip: add initial dts support for Rockpro64")
> Signed-off-by: Soeren Moch <smoch@web.de>

applied as fix for 5.4

Thanks
Heiko





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04 17:24               ` Sören Moch
@ 2019-10-04 20:15                 ` Soeren Moch
  2019-10-04 20:18                   ` Heiko Stuebner
  2019-10-11  8:22                 ` Jonas Karlman
  1 sibling, 1 reply; 23+ messages in thread
From: Soeren Moch @ 2019-10-04 20:15 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-rockchip, Shawn Lin, Robin Murphy, linux-kernel, linux-arm-kernel

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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04 20:15                 ` Soeren Moch
@ 2019-10-04 20:18                   ` Heiko Stuebner
  2019-10-04 20:39                     ` Sören Moch
  0 siblings, 1 reply; 23+ messages in thread
From: Heiko Stuebner @ 2019-10-04 20:18 UTC (permalink / raw)
  To: Soeren Moch
  Cc: linux-rockchip, Shawn Lin, Robin Murphy, linux-kernel, linux-arm-kernel

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




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04 20:18                   ` Heiko Stuebner
@ 2019-10-04 20:39                     ` Sören Moch
  0 siblings, 0 replies; 23+ messages in thread
From: Sören Moch @ 2019-10-04 20:39 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-rockchip, Shawn Lin, Robin Murphy, linux-kernel, linux-arm-kernel



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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】
  2019-10-04 17:24               ` Sören Moch
  2019-10-04 20:15                 ` Soeren Moch
@ 2019-10-11  8:22                 ` Jonas Karlman
  2019-10-11 11:40                   ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings Soeren Moch
  1 sibling, 1 reply; 23+ messages in thread
From: Jonas Karlman @ 2019-10-11  8:22 UTC (permalink / raw)
  To: Sören Moch, Shawn Lin, Robin Murphy, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel

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.
>>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings
  2019-10-11  8:22                 ` Jonas Karlman
@ 2019-10-11 11:40                   ` Soeren Moch
  2019-10-11 13:00                     ` Robin Murphy
  0 siblings, 1 reply; 23+ messages in thread
From: Soeren Moch @ 2019-10-11 11:40 UTC (permalink / raw)
  To: Jonas Karlman, Shawn Lin, Robin Murphy, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel



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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings
  2019-10-11 11:40                   ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings Soeren Moch
@ 2019-10-11 13:00                     ` Robin Murphy
  2019-10-11 14:16                       ` Soeren Moch
  0 siblings, 1 reply; 23+ messages in thread
From: Robin Murphy @ 2019-10-11 13:00 UTC (permalink / raw)
  To: Soeren Moch, Jonas Karlman, Shawn Lin, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings
  2019-10-11 13:00                     ` Robin Murphy
@ 2019-10-11 14:16                       ` Soeren Moch
  2019-10-12  4:09                         ` Shawn Lin
  0 siblings, 1 reply; 23+ messages in thread
From: Soeren Moch @ 2019-10-11 14:16 UTC (permalink / raw)
  To: Robin Murphy, Jonas Karlman, Shawn Lin, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel

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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings
  2019-10-11 14:16                       ` Soeren Moch
@ 2019-10-12  4:09                         ` Shawn Lin
  2019-10-12 12:45                           ` Soeren Moch
  0 siblings, 1 reply; 23+ messages in thread
From: Shawn Lin @ 2019-10-12  4:09 UTC (permalink / raw)
  To: Soeren Moch, Robin Murphy, Jonas Karlman, Heiko Stuebner
  Cc: linux-rockchip, shawn.lin, linux-kernel, linux-arm-kernel


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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings
  2019-10-12  4:09                         ` Shawn Lin
@ 2019-10-12 12:45                           ` Soeren Moch
  0 siblings, 0 replies; 23+ messages in thread
From: Soeren Moch @ 2019-10-12 12:45 UTC (permalink / raw)
  To: Shawn Lin, Robin Murphy, Jonas Karlman, Heiko Stuebner
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2019-10-12 12:55 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-03 21:50 [PATCH 1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings Soeren Moch
2019-10-03 21:50 ` [PATCH 2/3] arm64: dts: rockchip: fix RockPro64 sdhci settings Soeren Moch
2019-10-04 20:05   ` Heiko Stuebner
2019-10-03 21:50 ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings Soeren Moch
2019-10-04  0:01   ` Robin Murphy
2019-10-04  0:53     ` Soeren Moch
2019-10-04  2:13       ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】 Shawn Lin
2019-10-04  3:39         ` Soeren Moch
2019-10-04 14:20           ` Robin Murphy
2019-10-04 14:42             ` Jonas Karlman
2019-10-04 15:33             ` Shawn Lin
2019-10-04 17:24               ` Sören Moch
2019-10-04 20:15                 ` Soeren Moch
2019-10-04 20:18                   ` Heiko Stuebner
2019-10-04 20:39                     ` Sören Moch
2019-10-11  8:22                 ` Jonas Karlman
2019-10-11 11:40                   ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings Soeren Moch
2019-10-11 13:00                     ` Robin Murphy
2019-10-11 14:16                       ` Soeren Moch
2019-10-12  4:09                         ` Shawn Lin
2019-10-12 12:45                           ` Soeren Moch
2019-10-04 16:39             ` [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings【请注意,邮件由linux-rockchip-bounces+shawn.lin=rock-chips.com@lists.infradead.org代发】 Soeren Moch
2019-10-04 20:04 ` [PATCH 1/3] arm64: dts: rockchip: fix RockPro64 vdd-log regulator settings Heiko Stuebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).