Re: [PATCH 1/3 v2] ARM: dts: rk3288-tinker.dtsi: Fix SD card detection

[David Summers <beagleboard-8lkWs+DogrrqRUWtDFdff/XRex20P6io@public.gmane.org>]

On 24/02/2019 19:56, Jonas Karlman wrote:
> On 2019-02-24 20:35, David Summers wrote:
>> On 24/02/2019 19:26, Jonas Karlman wrote:
>>> On 2019-02-22 19:47, David Summers wrote:
>>>> The Problem:
>>>>
>>>> On ASUS Tinker Board S, when booting from the eMMC, and there is no
>>>> card sd slot, then there are constant errors.
>>>>
>>>> Cause:
>>>>
>>>> Thanks must go to Robin Murphy @ ARM for idenifying the problem. The
>>>> rk808 on the Tinker Board and Tinker Board S has many regulators, one
>>>> vccio_sd powers the IO for the sd card. Unfortunatly this is also used
>>>> in the card detect. Now when no card is install, the regulator is
>>>> powered down. This means that the card detect floats, and this means
>>>> random card detection.
>>>>
>>>> The Solution:
>>>>
>>>> Make sure that the sd IO is always powered, this means card detection
>>>> is always active, which is what should be done on a board with an sd
>>>> slot, which both the Tinker Board and Tinker Board S are. Hence change
>>>> is made to the .dtsi which takes effect on all Tinker Boards as
>>>> required.
>>>>
>>>> The change also adds "regulator-boot-on" the Tinker Board boot from
>>>> uboot, and the sd card is always one option. Hence the IO must be
>>>> powered in uboot, and so setting this flag.
>>>>
>>>> Also removed is "disable-wp" the micro sd card which are used have no
>>>> write  protection, so the concept doesn't mean anything, and the
>>>> Tinker Boards work without this. Hence it is removed to simply.
>>>>
>>>> This change came from ArchLinux Arm, but we note it is the fix also
>>>> used by Armbian:
>>>>
>>>> https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMiouyouyou%2FRockMyy%2Fblob%2Fmaster%2Fpatches%2Fkernel%2Fv5.0%2FDTS%2F0016-ARM-DTSI-rk3288-tinker-Setting-up-the-SD-regulators.patch&data=02%7C01%7C%7Cf5937082939e4163c84b08d69a8f3b61%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636866337295767294&sdata=tK8QE3bsG9LW%2FJcvFzLKa8%2BPj5u%2F8exbEn8m2vqKly0%3D&reserved=0
>>>>
>>>> Signed-off-by: David Summers <beagleboard-8lkWs+DogrrqRUWtDFdff/XRex20P6io@public.gmane.org>
>>>> ---
>>>>    arch/arm/boot/dts/rk3288-tinker.dtsi | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi
>>>> index aa107ee41b8b..6b7e55085b0c 100644
>>>> --- a/arch/arm/boot/dts/rk3288-tinker.dtsi
>>>> +++ b/arch/arm/boot/dts/rk3288-tinker.dtsi
>>>> @@ -254,6 +254,8 @@
>>>>    			};
>>>>    
>>>>    			vccio_sd: LDO_REG5 {
>>>> +				regulator-always-on;
>>>> +				regulator-boot-on;
>>>>    				regulator-min-microvolt = <1800000>;
>>>>    				regulator-max-microvolt = <3300000>;
>>>>    				regulator-name = "vccio_sd";
>>>> @@ -431,7 +433,6 @@
>>>>    	cap-mmc-highspeed;
>>>>    	cap-sd-highspeed;
>>>>    	card-detect-delay = <200>;
>>>> -	disable-wp;			/* wp not hooked up */
>>> I think disable-wp correctly describes that wp is not expected to work and should not be removed.
>>>
>>>   From comment in mmc_sd_get_ro():
>>> "Some systems don't feature a write-protect pin and don't need one.
>>> E.g. because they only have micro-SD card slot. For those systems
>>> assume that the SD card is always read-write."
>>>
>>> Without disable-wp core will call dw_mci_get_ro() to get wp status.
>>>
>>>
>>> This patch also fixes reboot when booting from sd-card and having emmc zeroed out.
>>>
>>> Tested-by: Jonas Karlman <jonas-uIzNG4q0ceqzQB+pC5nmwQ@public.gmane.org>
>>>
>>> Regards,
>>> Jonas
>>>
>>>>    	pinctrl-names = "default";
>>>>    	pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd &sdmmc_bus4>;
>>>>    	status = "okay";
>> Hi Jonas,
>>
>> Thanks for testing - did you get an error with "disable-wp" ?
>>
>> We tested this with @TheSaint On ArchLinux Arm, and it worked!
>>
>> With the "regulator-boot-on" and on reboot, it should keep power on the
>> sd card - so reboot should work.
> I tested both with and without "disable-wp" removed and both works without error.
> SD-card reboots now works as it should thanks to "regulator-always-on" and "regulator-boot-on", I only tested using both.
>
> On a side note I have also been testing [1] to make reboot with UHS signal voltage work and will send out a RFC v2
> after I have concluded testing on my other devices supporting UHS signal voltage.
>
> [1] https://github.com/Kwiboo/linux-rockchip/compare/patch-rk-5.x-tinker-uhs%5E%5E%5E...patch-rk-5.x-tinker-uhs
>
> Regards,
> Jonas

Thanks.

Glad your are testing - and I hope the uhs tests work.

Alas for us to test uhs - and our user on Arch isn't so experienced. I 
could push him to try different speeds - but to be honest I expect it to 
be beyond his comfort zone.

But its good to here you confirm "disable-wp" works. To my mind it says 
we are sure that  all three changes in the patch are are correct.

Hopefully this will be straight forward to Heiko to accept. Its the 
simple patch of the 3 to accept ;)

Regards,

David.

>> Regards,
>>
>> David.
>>
>>