mmc_spi lowlevel onetime retry?
In short:
1) im about to make the mmc_spi driver work on Blackfin uClinux platforms via the Blackfin SPI master driver.
2) will then apply and evaluate some new ideas to fix concurrent access on the SPI bus.
Longer:
I made another mmc/sd driver a few years ago so i have had this battle before vs the odd mmc/sd spi protocol implementation and shaky spi master drivers. Progress and modifications so far, all to mmc_spi.c host driver:
1) Disabled spi DMA(could be dma_map_page not working correctly vs bfin-uclinux, will look into this deeper soon)
2) Disabled spi crc checks, CRC is one off(0xabcd vs 0xabce ie.) on many read blocks. Data seem correct anyways, checked by md5 on files etc.
3) Ignored transmission data-response byte (allways 0xff for me) but data get to the card anyways. md5 verified.
4) Implement one-time low-level retry. It increase stability immensley. I can format, copy files and do any heavy action vs the card. Without it, no go. I implemented it like this:
in mmc_data_do()
retried_once = 0;
retry:
if (direction == DMA_TO_DEVICE)
status = mmc_spi_writeblock(host, t);
else
status = mmc_spi_readblock(host, t);
/* one low-level retry */
if (status < 0 && !retried_once) {
retried_once = 1;
goto retry;
}I know i did the same to my old spi_mmc driver to improve stability. However, i always got the feeling this was not the right way to do it. Books and others told med to end_request with a status != 0 and let block layer resubmit the request(eventually) for me to retry. Whats the correct policy for this? Whitout my retry-fix i cant write more than a couple of 100 sectors until i get a sector error like:
end_request: I/O error, dev mmcblk0, sector 160
Why is this sector not resumited to lower levels to try again? Since the sector number that failed was different allways a retry would probably work, its a communication error not an actual bad sector.
Without fix, the sector fail ratio is somewhere around 1:100 to 1:1000, wich is not very good, my fix is hiding a common low level SPI error. But even with a perfect SPI driver i would really like to have a retry policy somewhere when doing data transfers, in this case it actually squares the fail ratio to make it 1:10000 to 1:Mil. Where should that policy be implemented? I never got a good answer to this the last time i asked around.
Thanks.
/Hans
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
Quoted 7 lines · Hans
In short:
1) im about to make the
mmc_spidriver work on Blackfin uClinux platforms via the Blackfin SPI master driver.2) will then apply and evaluate some new ideas to fix concurrent access on the SPI bus.
Good on both of those. :)
Quoted 9 lines
Longer:
I made another mmc/sd driver a few years ago so i have had this battle before vs the odd mmc/sd spi protocol implementation and shaky spi master drivers. Progress and modifications so far, all to
mmc_spi.chost driver:1) Disabled spi
DMA(could be dma_map_page not working correctly vs bfin-uclinux, will look into this deeper soon)
DMA issues sound likely to be platform-specific, yes.
Quoted 2 lines
2) Disabled spi crc checks, CRC is one off(0xabcd vs
0xabceie.) on many read blocks. Data seem correct anyways, checked by md5 on files etc.
Strange.
Quoted 2 lines
3) Ignored transmission data-response byte (allways
0xfffor me) but data get to the card anyways. md5 verified.
Sounds like another platform-specific issue. Unless maybe you've verified that issue by looking at the SPI data that gets transferred? Not every MMC card seems to conform very closely to the MMC specs, but that would be a new and very surprising failure mode...
Quoted 19 lines
4) Implement one-time low-level retry. It increase stability immensley. I can format, copy files and do any heavy action vs the card. Without it, no go. I implemented it like this:
in
mmc_data_do()retried_once = 0; retry: if (direction == DMA_TO_DEVICE) status = mmc_spi_writeblock(host, t); else status = mmc_spi_readblock(host, t); /* one low-level retry */ if (status < 0 && !retried_once) { retried_once = 1; goto retry; }
Quoted 5 lines
I know i did the same to my old
spi_mmcdriver to improve stability. However, i always got the feeling this was not the right way to do it. Books and others told med toend_requestwith a status != 0 and let block layer resubmit the request(eventually) for me to retry. Whats the correct policy for this?
Low level drivers should as a rule never retry unless they're explicitly told to do so. The current MMC stack doesn't have a way to give such instructions, but it *should* have ways to retry itself.
Quoted 8 lines
Whitout my retry-fix i cant write more than a couple of 100 sectors until i get a sector error like:
end_request: I/O error, dev mmcblk0, sector 160Why is this sector not resumited to lower levels to try again? Since the sector number that failed was different allways a retry would probably work, its a communication error not an actual bad sector.
I don't quite follow what you're saying. It sounds like you may be suggesting that there are two problems in the way the MMC layer handles faults: (a) it doesn't retry, in at least some cases where that could recover from transient faults;
(b) sometimes the fault is reported against the wrong sector. That latter sounds sort of like a buggy diagnostic...
Quoted 7 lines
Without fix, the sector fail ratio is somewhere around 1:100 to 1:1000, wich is not very good, my fix is hiding a common low level SPI error. But even with a perfect SPI driver i would really like to have a retry policy somewhere when doing data transfers, in this case it actually squares the fail ratio to make it 1:10000 to 1:Mil. Where should that policy be implemented? I never got a good answer to this the last time i asked around.
Since as far as I know the MMC drivers are not expected to retry commands, I'd expect the upper MMC/SD/SDIO code to do be issuing any retries.
In general, the lowlevel code isn't expected to understand whether it's even safe to retry like that. It's supposed to understand protocol patterns, not command semantics ... and there's no reason to believe that a block write must always be idempotent.
- Dave
Quoted 8 lines
Thanks.
/Hans
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Quoted 8 lines · On Tue, 15 Apr 2008 11:20:27 -0700 David Brownell…
Quoted 6 lines · Hans
I know i did the same to my old
spi_mmcdriver to improve stability. However, i always got the feeling this was not the right way to do it. Books and others told med toend_requestwith a status != 0 and let block layer resubmit the request(eventually) for me to retry. Whats the correct policy for this?
Quoted 5 lines
Low level drivers should as a rule never retry unless they're explicitly told to do so. The current MMC stack doesn't have a way to give such instructions, but it *should* have ways to retry itself.
It has, but see below...
Quoted 11 lines
Since as far as I know the MMC drivers are not expected to retry commands, I'd expect the upper MMC/SD/SDIO code to do be issuing any retries.
In general, the lowlevel code isn't expected to understand whether it's even safe to retry like that. It's supposed to understand protocol patterns, not command semantics ... and there's no reason to believe that a block write must always be idempotent.
The problem is that the mmc_block driver doesn't know what's safe either. We could add retries for reads as that should always be safe. But we can't do it for writes as we're supposed to at least give the guarantee that if given n blocks, we will write 0 to n blocks in a continuous manner. Retrying a write might end up with a random assortments of blocks on the media.
Signature · 11 lines
-- Pierre Ossman Linux kernel, MMC maintainer http://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone