Re: [linux-sunxi] [PATCH] arm64: dts: allwinner: Revert SD card CD GPIO for Pine64-LTS

[Andre Przywara <andre.przywara@arm.com>]

On Mon, 12 Apr 2021 19:41:25 +0200
Michael Weiser <michael.weiser@gmx.de> wrote:

Hi Michael,

> Hi Andre, ChenYu,
> 
> On Mon, Apr 12, 2021 at 05:45:58PM +0100, Andre Przywara wrote:
> > > > 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..843338e19694 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 */
> > > > +       non-removable;          /* card detect is broken on some boards */    
> > > 
> > > So a revert is good, but has anyone tried using the "broken-cd" instead?  
> > Ha, that's a good idea, I totally forgot about this property!  
> 
> > > That way, at least on Linux, the mmc core resorts to polling for a card.
> > > At least this way the card is still removable.  
> > Yes indeed, I tested it on my "stuck at 1" Pine64-LTS, and it works like
> > a charm!  
> 
> > Daniel, Michael, can you test this on your boards? So removing the
> > cd-gpios property, and adding "broken-cd;" instead?  
> 
> Yes, it works fine. What flummoxed me at first was that obviously I also
> have to disable the ACTIVE_LOW definition in sun50i-a64-sopine.dtsi
> after having added and disabled an ACTIVE_HIGH definition in
> sun50i-a64-pine64-lts.dts.

Why? From my experiments it should not matter, the actual card presence
is typically detected via the SD bus anyway (if I understand the code
correctly). broken-cd just prevents installation of an interrupt
handler, so it's less efficient and prevents wakeup on card detect,
AFAICS.
But also: ACTIVE_HIGH *is* the right polarity, even for the
Pine64-LTS. At least that's what an earlier report from Daniel
suggested:
=> gpio input pf6
card inserted: value is 1
card removed: value is 0
So for your particular board (version) you could actually remove the
whole &mmc0 node override, use the same node as the SOPine (working
active-high CD) and it should work.

> BTW: My boards have a marking "PINE64-R18-V1_1" and below it
> "2017-08-03" on their upper side. On the back it says on one sticker
> "Model:PineA64 2GB LTS" and on another "2O1-PINE64R18-00" and
> "PINE64-R18-V1.1 2G". Is CD being stuck at 1 a bug of revision 1.0
> perhaps?  Is there a way to detect this difference in software and add
> some sort of quirk handler for it?

As Jernej mentioned, this would be U-Boot's task, but I don't see a
good reason for it. Firstly, you would need to find a good automatic
way of determining the board revision, which I doubt there is. And
secondly, I don't see the benefit: It works quite nicely with
broken-cd: card removals and insertions are detected automatically,
it's just not as efficient (interrupt-driven) as it could be.
Or do you see any problems with broken-cd?

Cheers,
Andre