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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,
On Mon, Feb 25, 2019 at 1:11 PM David Summers
<beagleboard@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 25/02/2019 17:13, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, Feb 22, 2019 at 10:48 AM David Summers
> > <beagleboard@xxxxxxxxxxxxxxxxxxx> 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.
> > Yeah, this is broken on a lot of SoCs that use dw_mmc.  :(  Really the
> > card detect line needs to be on a different rail and this is why all
> > boards I've worked on recently have a the card detect going to a GPIO
> > instead of the dw_mmc CD.
> >
> > IIRC Rockchip moved the Card Detect to a different rail on newer SoCs
> > (like rk3399) but we still used a GPIO even there since we didn't like
> > the default/automatic muxing of JTAG and SD signals.
> >
> > The one board I was involved in that did it wrong (where we discovered
> > this issue) was exynos5250-snow.  You can see some discussion about
> > the issue at:
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/282474.html
> >
> > In that discussion I'm pretty sure that Ulf suggested that a better
> > way to go was to use something like "broken-cd" which I think was
> > supposed to switch us to use polling.  AKA periodically the SD card
> > would be powered on and we'd check for a card, then we'd power
> > everything off.  ...but that was never implemented for snow at least
> > so there may be something more than just adding the property.  You can
> > read through the whole thread for more details.
> >
> >
> > IIRC leaving the IO rail always on like you're proposing can also work
> > OK but there may be some corner cases, especially if you are trying to
> > reach UHS speeds and/or if the bootloader ever tries to use UHS
> > speeds.  It's almost certainly busted if the bootloader did UHS since
> > it will leave the line at ~1.8 V and the kernel will expect it to be
> > at ~3.3 V.  ...but maybe you rely on the bootloader not doing UHS and
> > maybe things are generally OK if not?  There may also be cases where
> > you can't properly power down / reset a card because the card may be
> > drawing power through the IO lines when you power off its main lines.
> > That's not good for the card and can also put it in a bad state.  I
> > haven't done all the research here so this may be a bit of FUD--it's
> > just a vague recollection from many years ago.
> >
> >
> > ...so to make a long story short, a better solution is to allow the IO
> > lines to be powered off but then poll for the card periodically.
> >
> >
> >> 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.
> > As others have said, please leave disable-wp.  There's no way for the
> > kernel to know if you have a SD or uSD slot and the only difference
> > between the two (electrically) is that there's no write protect for
> > micro SD.
> >
> >
> > Also: please CC dw_mmc people on future patches in this area.
> >
> > $ ./scripts/get_maintainer.pl -f drivers/mmc/host/dw_mmc.c
> > Jaehoon Chung <jh80.chung@xxxxxxxxxxx> (maintainer:SYNOPSYS DESIGNWARE
> > MMC/SD/SDIO DRIVER)
> > Ulf Hansson <ulf.hansson@xxxxxxxxxx> (maintainer:MULTIMEDIA CARD
> > (MMC), SECURE DIGITAL (SD) AND...)
> > linux-mmc@xxxxxxxxxxxxxxx (open list:SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER)
> >
> > -Doug
>
> I think the possible problem is that without this we were getting a lot
> of errors. Now as the errors happen when the sd io is power down, and so
> CD floats; then the IO will be powered back up gain - to access the
> card, only to find no card.

I definitely haven't thought through all the consequences of adding
polling.  ...but given that the problem is really common with SoCs
using dw_mmc it's probably worth it to figure out out something sane.
In theory you could have some code that knows that the card detect
becomes reliable once the IOs are powered on...


> So this means the power line goes up and down a lot. Now if we have
> broken-cd, and polling has to be used, doesn't this also have to power
> up the IO so it can poll, and the poll puts a bit more load on the
> processor.
>
> So question is which is better? To keep the IO powered up, or to have it
> going up and down?
>
> Anyway I'm happy with either solution. So if we can agree which is best,
> I'll do the patch for that.

I don't know which is better.  ...but I wouldn't expect that turning
on regulators and checking a GPIO ever second or so would burn much
power.

-Doug



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux