diff mbox series

[v2,08/14] rockchip: dts: rk3399: nanopi4: Use CD pin as RK_FUNC_1

Message ID 20190416105647.18437-9-jagan@amarulasolutions.com
State New
Headers
Series rockchip: Add new rk3399 boards |

Commit Message

Jagan Teki April 16, 2019, 10:56 a.m. UTC 
sdmmc cd pin is configured as RK_FUNC_GPIO which is wrong and
indeed failed to detect the sdcard on the board with below error

  Card did not respond to voltage select!

So, fix it by replacing RK_FUNC_GPIO with RK_FUNC_1 which
is already defined in rk3399.dts so make use of same like
other boards.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/dts/rk3399-nanopi4.dtsi | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Robin Murphy April 16, 2019, 11:10 a.m. UTC | #1 
On 16/04/2019 11:56, Jagan Teki wrote:
> sdmmc cd pin is configured as RK_FUNC_GPIO which is wrong and
> indeed failed to detect the sdcard on the board with below error
> 
>    Card did not respond to voltage select!
> 
> So, fix it by replacing RK_FUNC_GPIO with RK_FUNC_1 which
> is already defined in rk3399.dts so make use of same like
> other boards.

I guess the U-Boot dwmmc driver doesn't support using a GPIO? The reason 
we do this for Linux is that the dedicated function is not compatible 
with runtime power management - once we see that no card is present and 
suspend the idle controller, the CD logic is also powered off and thus 
no longer capable of generating the interrupt necessary to wake 
everything up again. The GPIO function of the same pin, however, is in 
an always-on power domain so is able to do the right thing.

So it's not "wrong" as such, but this change should be fine for U-Boot 
as long as it never turns off PD_SD itself.

Robin.

> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>   arch/arm/dts/rk3399-nanopi4.dtsi | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/arch/arm/dts/rk3399-nanopi4.dtsi b/arch/arm/dts/rk3399-nanopi4.dtsi
> index d325e11728..5dc8a8de16 100644
> --- a/arch/arm/dts/rk3399-nanopi4.dtsi
> +++ b/arch/arm/dts/rk3399-nanopi4.dtsi
> @@ -521,10 +521,6 @@
>   	};
>   
>   	sdmmc {
> -		sdmmc0_det_l: sdmmc0-det-l {
> -			rockchip,pins = <0 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>;
> -		};
> -
>   		sdmmc0_pwr_h: sdmmc0-pwr-h {
>   			rockchip,pins = <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>;
>   		};
> @@ -582,7 +578,7 @@
>   	cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>;
>   	disable-wp;
>   	pinctrl-names = "default";
> -	pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc0_det_l>;
> +	pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc_cd>;
>   	sd-uhs-sdr104;
>   	vmmc-supply = <&vcc3v0_sd>;
>   	vqmmc-supply = <&vcc_sdio>;
>
Jagan Teki April 17, 2019, 11:09 a.m. UTC | #2 
On Tue, Apr 16, 2019 at 4:40 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 16/04/2019 11:56, Jagan Teki wrote:
> > sdmmc cd pin is configured as RK_FUNC_GPIO which is wrong and
> > indeed failed to detect the sdcard on the board with below error
> >
> >    Card did not respond to voltage select!
> >
> > So, fix it by replacing RK_FUNC_GPIO with RK_FUNC_1 which
> > is already defined in rk3399.dts so make use of same like
> > other boards.
>
> I guess the U-Boot dwmmc driver doesn't support using a GPIO? The reason
> we do this for Linux is that the dedicated function is not compatible
> with runtime power management - once we see that no card is present and
> suspend the idle controller, the CD logic is also powered off and thus
> no longer capable of generating the interrupt necessary to wake
> everything up again. The GPIO function of the same pin, however, is in
> an always-on power domain so is able to do the right thing.

I can see these gpio pins were managed via
drivers/pinctrl/rockchip/pinctrl-rk3399.c and drivers/gpio/rk_gpio.c .
On the other hand other boards do use RKFUNC_1 for CD pin(even in
Linux) but what is different in nanopi4? do other boards are yet to
change?
diff mbox series

Patch

diff --git a/arch/arm/dts/rk3399-nanopi4.dtsi b/arch/arm/dts/rk3399-nanopi4.dtsi
index d325e11728..5dc8a8de16 100644
--- a/arch/arm/dts/rk3399-nanopi4.dtsi
+++ b/arch/arm/dts/rk3399-nanopi4.dtsi
@@ -521,10 +521,6 @@ 
 	};
 
 	sdmmc {
-		sdmmc0_det_l: sdmmc0-det-l {
-			rockchip,pins = <0 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>;
-		};
-
 		sdmmc0_pwr_h: sdmmc0-pwr-h {
 			rockchip,pins = <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
@@ -582,7 +578,7 @@ 
 	cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>;
 	disable-wp;
 	pinctrl-names = "default";
-	pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc0_det_l>;
+	pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc_cd>;
 	sd-uhs-sdr104;
 	vmmc-supply = <&vcc3v0_sd>;
 	vqmmc-supply = <&vcc_sdio>;