diff mbox series

[RESEND,v7,05/11] rockchip: dts: rk3399: nanopi4: Use CD pin as RK_FUNC_1

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

Commit Message

Jagan Teki May 8, 2019, 5:41 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.

Add these changes in -u-boot.dtsi to make Linux sync easy for future
changes.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
 arch/arm/dts/rk3399-nanopi4-u-boot.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 arch/arm/dts/rk3399-nanopi4-u-boot.dtsi

Comments

Robin Murphy May 8, 2019, 1:52 p.m. UTC | #1 
On 08/05/2019 06:41, 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.

AFAICS this should also be true of RockPro64 and (at least with the 
Linux DT) Firefly - those aren't grabbing &sdmmc_cd by default either. I 
imagine that U-Boot might also see similar problems on Gru, where the 
card detect signal is on a completely different GPIO.

I'd note that in Linux, only rk3399-evb is actually *using* &sdmmc_cd - 
despite the fact that they claim it, nearly all the other boards also 
have "cd-gpios" and thus end up overriding the dedicated function with 
an implicit GPIO configuration anyway. Sapphire is the odd one out in 
using "broken-cd" as the less-efficient way of mitigating the runtime PM 
issue.

> Add these changes in -u-boot.dtsi to make Linux sync easy for future
> changes.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>   arch/arm/dts/rk3399-nanopi4-u-boot.dtsi | 9 +++++++++
>   1 file changed, 9 insertions(+)
>   create mode 100644 arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
> 
> diff --git a/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi b/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
> new file mode 100644
> index 0000000000..20db99c0b8
> --- /dev/null
> +++ b/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Jagan Teki <jagan@amarulasolutions.com>
> + */
> +
> +&sdmmc {
> +	pinctrl-names = "default";

That's already set in the base DTSI, so doesn't really need to be 
shadowed here.

> +	pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc_cd>;
> +};
> 

I suppose you could also delete the "cd-gpios" property to make it 
really clear what this override is for (and save a few bytes if it's 
going to be ignored anyway).

Robin.
Jagan Teki May 8, 2019, 2:59 p.m. UTC | #2 
On Wed, May 8, 2019 at 7:22 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 08/05/2019 06:41, 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.
>
> AFAICS this should also be true of RockPro64 and (at least with the
> Linux DT) Firefly - those aren't grabbing &sdmmc_cd by default either. I
> imagine that U-Boot might also see similar problems on Gru, where the
> card detect signal is on a completely different GPIO.

But RockPro64 is not using sdmmc_cd and it is able to detect the card
w/o pin but it has cd-gpio.

>
> I'd note that in Linux, only rk3399-evb is actually *using* &sdmmc_cd -
> despite the fact that they claim it, nearly all the other boards also
> have "cd-gpios" and thus end up overriding the dedicated function with
> an implicit GPIO configuration anyway. Sapphire is the odd one out in
> using "broken-cd" as the less-efficient way of mitigating the runtime PM
> issue.
>
> > Add these changes in -u-boot.dtsi to make Linux sync easy for future
> > changes.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> > ---
> >   arch/arm/dts/rk3399-nanopi4-u-boot.dtsi | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >   create mode 100644 arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
> >
> > diff --git a/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi b/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
> > new file mode 100644
> > index 0000000000..20db99c0b8
> > --- /dev/null
> > +++ b/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
> > @@ -0,0 +1,9 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2019 Jagan Teki <jagan@amarulasolutions.com>
> > + */
> > +
> > +&sdmmc {
> > +     pinctrl-names = "default";
>
> That's already set in the base DTSI, so doesn't really need to be
> shadowed here.
>
> > +     pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc_cd>;
> > +};
> >
>
> I suppose you could also delete the "cd-gpios" property to make it
> really clear what this override is for (and save a few bytes if it's
> going to be ignored anyway).

Why so? few boards like OrangePI do use both sdmmc_cd along with cd-gpio.

On the other-hand, how about doing this change in Linux?
diff mbox series

Patch

diff --git a/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi b/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
new file mode 100644
index 0000000000..20db99c0b8
--- /dev/null
+++ b/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
@@ -0,0 +1,9 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Jagan Teki <jagan@amarulasolutions.com>
+ */
+
+&sdmmc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc_cd>;
+};