diff mbox series

[v2] arm64: dts: allwinner: Revert SD card CD GPIO for Pine64-LTS

Message ID 20210414104740.31497-1-andre.przywara@arm.com (mailing list archive)
State New
Headers show
Series [v2] arm64: dts: allwinner: Revert SD card CD GPIO for Pine64-LTS | expand

Commit Message

Andre Przywara April 14, 2021, 10:47 a.m. UTC
Commit 941432d00768 ("arm64: dts: allwinner: Drop non-removable from
SoPine/LTS SD card") enabled the card detect GPIO for the SOPine module,
along the way with the Pine64-LTS, which share the same base .dtsi.

This was based on the observation that the Pine64-LTS has as "push-push"
SD card socket, and that the schematic mentions the card detect GPIO.

After having received two reports about failing SD card access with that
patch, some more research and polls on that subject revealed that there
are at least two different versions of the Pine64-LTS out there:
- On some boards (including mine) the card detect pin is "stuck" at
  high, regardless of an microSD card being inserted or not.
- On other boards the card-detect is working, but is active-high, by
  virtue of an explicit inverter circuit, as shown in the schematic.

To cover all versions of the board out there, and don't take any chances,
let's revert the introduction of the active-low CD GPIO, but let's use
the broken-cd property for the Pine64-LTS this time. That should avoid
regressions and should work for everyone, even allowing SD card changes
now.
The SOPine card detect has proven to be working, so let's keep that
GPIO in place.

Fixes: 941432d00768 ("arm64: dts: allwinner: Drop non-removable from SoPine/LTS SD card")
Reported-by: Michael Weiser <michael.weiser@gmx.de>
Reported-by: Daniel Kulesz <kuleszdl@posteo.org>
Suggested-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Weiser April 14, 2021, 6:35 p.m. UTC | #1
Hi Andre,

On Wed, Apr 14, 2021 at 11:47:40AM +0100, Andre Przywara wrote:

> Commit 941432d00768 ("arm64: dts: allwinner: Drop non-removable from
> SoPine/LTS SD card") enabled the card detect GPIO for the SOPine module,
> along the way with the Pine64-LTS, which share the same base .dtsi.

> This was based on the observation that the Pine64-LTS has as "push-push"
> SD card socket, and that the schematic mentions the card detect GPIO.

> After having received two reports about failing SD card access with that
> patch, some more research and polls on that subject revealed that there
> are at least two different versions of the Pine64-LTS out there:
> - On some boards (including mine) the card detect pin is "stuck" at
>   high, regardless of an microSD card being inserted or not.
> - On other boards the card-detect is working, but is active-high, by
>   virtue of an explicit inverter circuit, as shown in the schematic.

> To cover all versions of the board out there, and don't take any chances,
> let's revert the introduction of the active-low CD GPIO, but let's use
> the broken-cd property for the Pine64-LTS this time. That should avoid
> regressions and should work for everyone, even allowing SD card changes
> now.
> The SOPine card detect has proven to be working, so let's keep that
> GPIO in place.

I can confirm that this change works on my Pine64 LTS boards (with
working high-active card detect) when applied to today's linux-next (which
already includes your previous change to change the card detect GPIO
from low- to high-active in sun50i-a64-sopine.dtsi).

> Fixes: 941432d00768 ("arm64: dts: allwinner: Drop non-removable from SoPine/LTS SD card")
> Reported-by: Michael Weiser <michael.weiser@gmx.de>
> Reported-by: Daniel Kulesz <kuleszdl@posteo.org>
> Suggested-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Tested-by: Michael Weiser <michael.weiser@gmx.de>

Thanks!
Michael

> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
> index e79ce49e7e6a..596a25907432 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
> @@ -21,5 +21,5 @@
>  };

>  &mmc0 {
> -	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 push-push switch */
> +	broken-cd;		/* card detect is broken on *some* boards */
>  };
Andre Przywara April 21, 2021, 11:33 a.m. UTC | #2
On Wed, 14 Apr 2021 20:35:03 +0200
Michael Weiser <michael.weiser@gmx.de> wrote:

Maxime, Chen-Yu:

can you please try to push this patch into 5.12, still?
The Pine64-LTS' SD card is broken otherwise, on both versions of the
board. The incriminating patch was introduced in 5.12-rc1 (my bad!), so
it qualifies as a regression fix.

Many thanks!
Andre

> On Wed, Apr 14, 2021 at 11:47:40AM +0100, Andre Przywara wrote:
> 
> > Commit 941432d00768 ("arm64: dts: allwinner: Drop non-removable from
> > SoPine/LTS SD card") enabled the card detect GPIO for the SOPine module,
> > along the way with the Pine64-LTS, which share the same base .dtsi.  
> 
> > This was based on the observation that the Pine64-LTS has as "push-push"
> > SD card socket, and that the schematic mentions the card detect GPIO.  
> 
> > After having received two reports about failing SD card access with that
> > patch, some more research and polls on that subject revealed that there
> > are at least two different versions of the Pine64-LTS out there:
> > - On some boards (including mine) the card detect pin is "stuck" at
> >   high, regardless of an microSD card being inserted or not.
> > - On other boards the card-detect is working, but is active-high, by
> >   virtue of an explicit inverter circuit, as shown in the schematic.  
> 
> > To cover all versions of the board out there, and don't take any chances,
> > let's revert the introduction of the active-low CD GPIO, but let's use
> > the broken-cd property for the Pine64-LTS this time. That should avoid
> > regressions and should work for everyone, even allowing SD card changes
> > now.
> > The SOPine card detect has proven to be working, so let's keep that
> > GPIO in place.  
> 
> I can confirm that this change works on my Pine64 LTS boards (with
> working high-active card detect) when applied to today's linux-next (which
> already includes your previous change to change the card detect GPIO
> from low- to high-active in sun50i-a64-sopine.dtsi).
> 
> > Fixes: 941432d00768 ("arm64: dts: allwinner: Drop non-removable from SoPine/LTS SD card")
> > Reported-by: Michael Weiser <michael.weiser@gmx.de>
> > Reported-by: Daniel Kulesz <kuleszdl@posteo.org>
> > Suggested-by: Chen-Yu Tsai <wens@csie.org>
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>  
> 
> Tested-by: Michael Weiser <michael.weiser@gmx.de>
> 
> Thanks!
> Michael
> 
> > ---
> >  arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)  
> 
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
> > index e79ce49e7e6a..596a25907432 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
> > @@ -21,5 +21,5 @@
> >  };  
> 
> >  &mmc0 {
> > -	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 push-push switch */
> > +	broken-cd;		/* card detect is broken on *some* boards */
> >  };  
>
Maxime Ripard April 22, 2021, 7:46 a.m. UTC | #3
On Wed, Apr 21, 2021 at 12:33:54PM +0100, Andre Przywara wrote:
> On Wed, 14 Apr 2021 20:35:03 +0200
> Michael Weiser <michael.weiser@gmx.de> wrote:
> 
> Maxime, Chen-Yu:
> 
> can you please try to push this patch into 5.12, still?
> The Pine64-LTS' SD card is broken otherwise, on both versions of the
> board. The incriminating patch was introduced in 5.12-rc1 (my bad!), so
> it qualifies as a regression fix.

I just sent a PR for it, thanks

Maxime
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
index e79ce49e7e6a..596a25907432 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
@@ -21,5 +21,5 @@ 
 };
 
 &mmc0 {
-	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 push-push switch */
+	broken-cd;		/* card detect is broken on *some* boards */
 };