On 26/02/2019 16:43, Doug Anderson wrote:
Hi,
On Tue, Feb 26, 2019 at 6:46 AM Robin Murphy <robin.murphy@xxxxxxx> wrote:
On 25/02/2019 22:20, Heiko Stübner wrote:
Am Montag, 25. Februar 2019, 22:18:28 CET schrieb Doug Anderson:
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...
Right, forcing it at the regulator end is somewhat of a blunt instrument
when faced with all possible subtleties (but it is what all the vendor
kernels seem to do). My next thought would be some
"cd-pulled-up-by-vqmmc" quirk where the only difference is that at the
point we know we *don't* have a card present and go to release vqmmc,
the quirk turns it back on (or skips turning it off at all). However it
seems like that that's almost exactly what was proposed last time - I
hadn't seen that thread, so I'll take some time to digest it fully at
some point.
I don't remember exactly where we left off last time, but in any case
it would probably be worth re-evaluating any conclusions made 4 years
ago, especially given that there's definitely more than one board in
the same position now.
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.
It's not so much power that bothers me here as the general "doing more
work" impact of polling - specifically I'm recalling the time we
discovered that AMD Overdrive boards gained something ridiculous like 5%
performance uplift in hackbench just from having a card in the MMC slot.
Admittedly dw_mmc doesn't seem to be *that* bad - a quick play with and
without "broken-cd" on my little rk3328 suggests that any difference is
probably way down in the noise, so maybe it's OK.
What would be really fun, though, would be to take advantage of the fact
that CD is only half-broken - it should still detect a card by virtue of
the pin being properly pulled to ground, it just can't reliably detect
not-a-card. AFAICS we could have a quirk to handle phantom insertions,
which double-checks CD after powering up all the regulators to see if it
stayed low (hmm, I would hope card-detect-delay might do that anyway...)
and ignores the event without error if it didn't. Even if we still use a
timer to delay unmasking the interrupt for rate-limiting, that ought to
be a fair bit lighter-weight than the rigmarole of trying to initiate
communication with a possible card and waiting for it to time out.
I don't _think_ that would work, but I could be wrong. When you stop
powering the IO rails then that stops powering the logic in the SoC.
I don't think you can reliably detect interrupts in this case.
Hmm, it certainly works on RK3288, but in general you do have a point
there. As I understand the typical case, the external regulator only
powers the I/O pads, while the controller block itself is at the mercy
of SoC-internal power domains. On 3288 there's so much other gubbins in
PD_PERI that the controller itself will effectively never get powered
down, thus the interrupt logic keeps ticking. Comparing RK3399, though,
the controller's on its own in the finer-grained PD_SD, which I can see
getting aggressively auto-suspended by runtime PM...
...and I think you've just solved the puzzle of why my new board's card
detect works as a GPIO but not as the dedicated function (despite having
its I/O pad over in the always-on PMU domain away from regulator
problems). Thanks! :D
IMO the ideal case would be to power on the rails periodically and
then check the Card Detect. That would be better than trying to talk
to a card that doesn't exist.
Yes, that sounds like the most useful compromise - a (possibly
dwmmc-specific) property which says that the internal CD *interrupt* is
unreliable, but the status bit itself will still be accurate after a
proper powerup sequence. Then the existing polling machinery could
probably grow an intermediate level wherein it checks ->get_cd() first
and only bothers calling ->alive() if it looks like a card's actually
turned up.
Robin.
