* mmci: U300 "sync with blockend" broken for multi-block? @ 2011-01-01 11:05 Rabin Vincent 2011-01-01 12:10 ` Russell King - ARM Linux 2011-01-05 16:02 ` Linus Walleij 0 siblings, 2 replies; 10+ messages in thread From: Rabin Vincent @ 2011-01-01 11:05 UTC (permalink / raw) To: linux-arm-kernel In MMCI, there is some code to sync between the MCI_DATABLOCKEND and the MCI_DATAEND for U300 variants, which ensures that the transfer terminates only when both MCI_DATABLOCKEND and MCI_DATAEND occurs: * In the U300, the IRQs can arrive out-of-order, * e.g. MCI_DATABLOCKEND sometimes arrives after MCI_DATAEND, * so for this case we use the flags "blockend" and * "dataend" to make sure both IRQs have arrived before * concluding the transaction. It seems to me that this code won't work correctly for multi-block transfers, because there MCI_DATABLOCKEND will hit for the earlier blocks and the blockend flag will be set, and if on the last block the MCI_DATABLOCKEND hits after the MCI_DATAEND, this synching code won't do what it's trying to do and will instead just terminate the transfer after MCI_DATAEND. ^ permalink raw reply [flat|nested] 10+ messages in thread
* mmci: U300 "sync with blockend" broken for multi-block? 2011-01-01 11:05 mmci: U300 "sync with blockend" broken for multi-block? Rabin Vincent @ 2011-01-01 12:10 ` Russell King - ARM Linux 2011-01-05 16:15 ` Linus Walleij 2011-01-05 16:02 ` Linus Walleij 1 sibling, 1 reply; 10+ messages in thread From: Russell King - ARM Linux @ 2011-01-01 12:10 UTC (permalink / raw) To: linux-arm-kernel On Sat, Jan 01, 2011 at 04:35:14PM +0530, Rabin Vincent wrote: > In MMCI, there is some code to sync between the MCI_DATABLOCKEND and the > MCI_DATAEND for U300 variants, which ensures that the transfer > terminates only when both MCI_DATABLOCKEND and MCI_DATAEND occurs: > > * In the U300, the IRQs can arrive out-of-order, > * e.g. MCI_DATABLOCKEND sometimes arrives after MCI_DATAEND, > * so for this case we use the flags "blockend" and > * "dataend" to make sure both IRQs have arrived before > * concluding the transaction. > > It seems to me that this code won't work correctly for multi-block > transfers, because there MCI_DATABLOCKEND will hit for the earlier > blocks and the blockend flag will be set, and if on the last block the > MCI_DATABLOCKEND hits after the MCI_DATAEND, this synching code won't do > what it's trying to do and will instead just terminate the transfer > after MCI_DATAEND. It would be good to characterize what's actually going on with U300 some more, especially the timing between these signals and the FIFO interrupts, rather than just stating that they occur "out of order". Is the data block end interrupt being triggered when you've read the required data from the FIFO, and the data end interrupt triggered when the card has transferred the required amount of data (iow, data into the FIFO)? Once they have been properly characterized, then it may be possible to come up with an alternative solution. At the moment, it's very had to guess what's going on from the descriptions given. ^ permalink raw reply [flat|nested] 10+ messages in thread
* mmci: U300 "sync with blockend" broken for multi-block? 2011-01-01 12:10 ` Russell King - ARM Linux @ 2011-01-05 16:15 ` Linus Walleij 2011-01-05 16:43 ` Russell King - ARM Linux 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2011-01-05 16:15 UTC (permalink / raw) To: linux-arm-kernel 2011/1/1 Russell King - ARM Linux <linux@arm.linux.org.uk>: > It would be good to characterize what's actually going on with U300 some > more, especially the timing between these signals and the FIFO interrupts, > rather than just stating that they occur "out of order". I will try to document more closely. OTOMH it was like for reads they would come in one order first one then another and for writes the other way around. That was why the older quirk for U300 was working, wiring the DATAEND high, though it was no good in modeling what was actually happening. > Is the data block end interrupt being triggered when you've read the > required data from the FIFO, and the data end interrupt triggered when > the card has transferred the required amount of data (iow, data into > the FIFO)? > > Once they have been properly characterized, then it may be possible to > come up with an alternative solution. ?At the moment, it's very had to > guess what's going on from the descriptions given. Ulf, do you know the details of what is happening here? I think you have the most up-to-date knowledge. I've been trying to determine this a number of times, empirically mostly, probably failing to understand the most important variables. The current solution is as far as I've been able to model what's actually happening. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* mmci: U300 "sync with blockend" broken for multi-block? 2011-01-05 16:15 ` Linus Walleij @ 2011-01-05 16:43 ` Russell King - ARM Linux 2011-01-14 20:13 ` Linus Walleij 0 siblings, 1 reply; 10+ messages in thread From: Russell King - ARM Linux @ 2011-01-05 16:43 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jan 05, 2011 at 05:15:04PM +0100, Linus Walleij wrote: > 2011/1/1 Russell King - ARM Linux <linux@arm.linux.org.uk>: > > > It would be good to characterize what's actually going on with U300 some > > more, especially the timing between these signals and the FIFO interrupts, > > rather than just stating that they occur "out of order". > > I will try to document more closely. OTOMH it was like > for reads they would come in one order first one then > another and for writes the other way around. That was > why the older quirk for U300 was working, wiring the > DATAEND high, though it was no good in modeling > what was actually happening. Any chance of pr_debug'ing the complete status register each time you service an interrupt? You'll probably need to set the kernel log buffer fairly large to ensure that you capture everything. I wouldn't recommend dumping the messages through the serial port directly as that'd put far too much latency on the servicing of them, but using dmesg after the accesses to retrieve them. (IOW, don't pass 'debug' to the kernel...) ^ permalink raw reply [flat|nested] 10+ messages in thread
* mmci: U300 "sync with blockend" broken for multi-block? 2011-01-05 16:43 ` Russell King - ARM Linux @ 2011-01-14 20:13 ` Linus Walleij 2011-01-14 22:44 ` Russell King - ARM Linux ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Linus Walleij @ 2011-01-14 20:13 UTC (permalink / raw) To: linux-arm-kernel 2011/1/5 Russell King - ARM Linux <linux@arm.linux.org.uk>: > Any chance of pr_debug'ing the complete status register each time you > service an interrupt? ?You'll probably need to set the kernel log > buffer fairly large to ensure that you capture everything. I did this test now. Typical read/write test: dd if=/dev/mmcblk0 of=/dev/null bs=16384 count=16 of=/dev/null dd if=/dev/zero of=/dev/mmcblk0 bs=16384 count=16 The MCI_DATABLOCKENDMASK is set in both modes so it should appear after every block (512 bytes) no matter whether you're using DMA or not. In this case you would expect 0x80 x MCI_DATABLOCKEND followed by MCI_DATAEND, repeated 16 times. But this is what happens: TEST WITH PIO ON U8500 READ: mmci-pl18x sdi2: START DATA: blksz 0200 blks 0018 flags 00000200 mmci-pl18x sdi2: MCI_DATABLOCKEND && MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0028 flags 00000200 mmci-pl18x sdi2: MCI_DATABLOCKEND && MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATABLOCKEND && MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATABLOCKEND && MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATABLOCKEND && MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATABLOCKEND && MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATABLOCKEND && MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATABLOCKEND && MCI_DATAEND So MCI_DATABLOCKEND only appear if MCI_DATAEND appear at the same time or soon thereafter. TEST WITH PIO ON U8500 WRITE: mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000100 mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND mmci-pl18x sdi2: MCI_DATABLOCKEND && MCI_DATAEND Note: too few MCI_DATABLOCKEND, 32 of them, while you're expecting 128 of them. This would lead you to think that maybe you get MCI_DATABLOCKEND for every fourth block, which has some kind of twisted logic to it. But in reality this varies: there are sometimes things like this for a very large number of blocks: mmci-pl18x sdi4: START DATA: blksz 0200 blks 01e2 flags 00000100 mmci-pl18x sdi4: MCI_DATABLOCKEND mmci-pl18x sdi4: MCI_DATABLOCKEND mmci-pl18x sdi4: MCI_DATABLOCKEND mmci-pl18x sdi4: MCI_DATABLOCKEND mmci-pl18x sdi4: MCI_DATABLOCKEND && MCI_DATAEND Just 4 MCI_DATABLOCKEND for 0x1e2 blocks! Totally unpredictable. TEST WITH DMA ON U8500 READ: mmci-pl18x sdi2: START DATA: blksz 0200 blks 0008 flags 00000200 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0038 flags 00000200 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000200 mmci-pl18x sdi2: MCI_DATAEND TEST WITH DMA ON U8500 WRITE: mmci-pl18x sdi2: START DATA: blksz 0200 blks 0018 flags 00000100 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000100 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000100 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0080 flags 00000100 mmci-pl18x sdi2: MCI_DATAEND mmci-pl18x sdi2: START DATA: blksz 0200 blks 0068 flags 00000100 mmci-pl18x sdi2: MCI_DATAEND No sign of MCI_DATABLOCKEND when using DMA. It's not there. TEST WITH PIO ON U300 READ: [ 699.219323] mmci-pl18x mmci: START DATA: blksz 0200 blks 0080 flags 00000200 [ 699.226623] mmci-pl18x mmci: MCI_DATABLOCKEND [ 699.231085] mmci-pl18x mmci: MCI_DATABLOCKEND (.. 0x80 in total ...) [ 699.236598] mmci-pl18x mmci: MCI_DATABLOCKEND [ 699.759572] mmci-pl18x mmci: MCI_DATABLOCKEND && MCI_DATAEND (repeat x16) WRITE looks the same, just flags=00000100 instead So yes, MCI_DATABLOCKEND indeed does work if you're not using DMA. Haven't been able to provoke READ nor WRITE to get the flags after each other, they always seem to appear simultaneously. What it problematic with the reads *and* writes is that sometimes there us a MCI_DATABLOCKEND missing, so the blocks simply don't add up! This is the real bug that the sync code was trying to solve, and not in a very good way. TEST WITH DMA ON U300 READ: [ 43.402718] mmci-pl18x mmci: START DATA: blksz 0200 blks 0040 flags 00000200 [ 43.414096] mmci-pl18x mmci: MCI_DATAEND [ 43.424922] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000200 [ 43.439449] mmci-pl18x mmci: MCI_DATAEND [ 43.449258] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000200 [ 43.463692] mmci-pl18x mmci: MCI_DATAEND [ 43.474446] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000200 [ 43.488953] mmci-pl18x mmci: MCI_DATAEND [ 43.498793] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000200 [ 43.513341] mmci-pl18x mmci: MCI_DATAEND [ 43.524605] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000200 [ 43.539141] mmci-pl18x mmci: MCI_DATAEND [ 43.547262] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000200 [ 43.561702] mmci-pl18x mmci: MCI_DATAEND [ 43.569592] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000200 [ 43.584066] mmci-pl18x mmci: MCI_DATAEND [ 43.591893] mmci-pl18x mmci: START DATA: blksz 0200 blks 0038 flags 00000200 [ 43.602723] mmci-pl18x mmci: MCI_DATAEND TEST WITH DMA ON U300 WRITE: [ 40.266289] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000100 [ 40.301415] mmci-pl18x mmci: MCI_DATAEND [ 40.310951] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000100 [ 40.325644] mmci-pl18x mmci: MCI_DATAEND [ 40.334980] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000100 [ 40.349552] mmci-pl18x mmci: MCI_DATAEND [ 40.358561] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000100 [ 40.373245] mmci-pl18x mmci: MCI_DATAEND [ 40.382234] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000100 [ 40.396811] mmci-pl18x mmci: MCI_DATAEND [ 40.406151] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000100 [ 40.420838] mmci-pl18x mmci: MCI_DATAEND [ 40.430276] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000100 [ 40.444852] mmci-pl18x mmci: MCI_DATAEND [ 40.453939] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000100 [ 40.468638] mmci-pl18x mmci: MCI_DATAEND [ 40.477697] mmci-pl18x mmci: START DATA: blksz 0200 blks 0078 flags 00000100 [ 40.492269] mmci-pl18x mmci: MCI_DATAEND [ 40.501770] mmci-pl18x mmci: START DATA: blksz 0200 blks 003b flags 00000100 [ 40.512910] mmci-pl18x mmci: MCI_DATAEND [ 40.522388] mmci-pl18x mmci: START DATA: blksz 0200 blks 0001 flags 00000100 [ 40.529674] mmci-pl18x mmci: MCI_DATAEND [ 40.537511] mmci-pl18x mmci: START DATA: blksz 0200 blks 0001 flags 00000100 [ 40.544790] mmci-pl18x mmci: MCI_DATAEND [ 40.552528] mmci-pl18x mmci: START DATA: blksz 0200 blks 0001 flags 00000100 [ 40.559804] mmci-pl18x mmci: MCI_DATAEND These tests were done using a file rather than dd but the pattern is the same as on the U8500: you only get MCI_DATAEND when using DMA on U300. So in conclusion: - The sync code (to make sure both MCI_DATAEND and MCI_DATABLOCKEND both arrived) is not necessary. - Both U300 and Ux500 (and presumably also their sibling Nomadik) should be marked as .broken_blockend = true, it is simply broken, for PIO as well as for DMA. So we should fix a patch that simplifies the code accordingly: remove the sync and drop the .broken_blockend_dma flag for U300, it's simply just broken, all agree? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* mmci: U300 "sync with blockend" broken for multi-block? 2011-01-14 20:13 ` Linus Walleij @ 2011-01-14 22:44 ` Russell King - ARM Linux 2011-01-16 21:11 ` Linus Walleij 2011-01-16 21:16 ` Linus Walleij 2011-01-16 22:09 ` Linus Walleij 2 siblings, 1 reply; 10+ messages in thread From: Russell King - ARM Linux @ 2011-01-14 22:44 UTC (permalink / raw) To: linux-arm-kernel On Fri, Jan 14, 2011 at 09:13:05PM +0100, Linus Walleij wrote: > 2011/1/5 Russell King - ARM Linux <linux@arm.linux.org.uk>: > > > Any chance of pr_debug'ing the complete status register each time you > > service an interrupt? ?You'll probably need to set the kernel log > > buffer fairly large to ensure that you capture everything. > > I did this test now. Just to complete the picture, can I see the patch between the version which produced this and mainline? ^ permalink raw reply [flat|nested] 10+ messages in thread
* mmci: U300 "sync with blockend" broken for multi-block? 2011-01-14 22:44 ` Russell King - ARM Linux @ 2011-01-16 21:11 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2011-01-16 21:11 UTC (permalink / raw) To: linux-arm-kernel 2011/1/14 Russell King - ARM Linux <linux@arm.linux.org.uk>: > On Fri, Jan 14, 2011 at 09:13:05PM +0100, Linus Walleij wrote: >> 2011/1/5 Russell King - ARM Linux <linux@arm.linux.org.uk>: >> >> > Any chance of pr_debug'ing the complete status register each time you >> > service an interrupt? ?You'll probably need to set the kernel log >> > buffer fairly large to ensure that you capture everything. >> >> I did this test now. > > Just to complete the picture, can I see the patch between the version > which produced this and mainline? Sure, but I patched on top of the pending DMA-patches, since I needed to test the workings in DMA mode. Here is that patch, if you want the entire DMA patch series too I can resend them: diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index af0cae9..8ae32d9 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -81,7 +81,7 @@ static struct variant_data variant_u300 = { .fifohalfsize = 8 * 4, .clkreg_enable = 1 << 13, /* HWFCEN */ .datalength_bits = 16, - .broken_blockend = true, + .broken_blockend = false, .sdio = true, }; @@ -414,7 +414,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) void __iomem *base; int blksz_bits; - dev_dbg(mmc_dev(host->mmc), "blksz %04x blks %04x flags %08x\n", + dev_info(mmc_dev(host->mmc), "\n"); + dev_info(mmc_dev(host->mmc), "START DATA: blksz %04x blks %04x flags %08x\n", data->blksz, data->blocks, data->flags); host->data = data; @@ -480,10 +481,10 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) writel(datactrl, base + MMCIDATACTRL); irqmask0 = readl(base + MMCIMASK0); - if (variant->broken_blockend) - irqmask0 &= ~MCI_DATABLOCKENDMASK; - else - irqmask0 |= MCI_DATABLOCKENDMASK; + // if (variant->broken_blockend) + // irqmask0 &= ~MCI_DATABLOCKENDMASK; + // else + irqmask0 |= MCI_DATABLOCKENDMASK; irqmask0 &= ~MCI_DATAENDMASK; writel(irqmask0, base + MMCIMASK0); mmci_set_mask1(host, irqmask1); @@ -523,6 +524,16 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data, { struct variant_data *variant = host->variant; + if ((status & MCI_DATABLOCKEND) && + (status & MCI_DATAEND)) { + dev_info(mmc_dev(host->mmc), "MCI_DATABLOCKEND && MCI_DATAEND\n"); + dev_info(mmc_dev(host->mmc), "xfered according to no of blockends = 0x%08x, expected: 0x%08x\n", host->data_xfered + data->blksz, data->blksz * data->blocks); + } else if (status & MCI_DATABLOCKEND) { + dev_info(mmc_dev(host->mmc), "MCI_DATABLOCKEND\n"); + } else if (status & MCI_DATAEND) { + dev_info(mmc_dev(host->mmc), "MCI_DATAEND\n"); + } + /* First check for errors */ if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_TXUNDERRUN|MCI_RXOVERRUN)) { dev_dbg(mmc_dev(host->mmc), "MCI ERROR IRQ (status %08x)\n", status); @@ -589,8 +600,10 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data, */ if (!variant->broken_blockend && !host->dma_on_current_xfer) host->data_xfered += data->blksz; - if (host->data_xfered == data->blksz * data->blocks) + if (host->data_xfered == data->blksz * data->blocks) { host->last_blockend = true; + dev_info(mmc_dev(host->mmc), "THIS IS THE LAST MCI_DATABLOCKEND\n"); + } } if (status & MCI_DATAEND) Yours, Linus Walleij ^ permalink raw reply related [flat|nested] 10+ messages in thread
* mmci: U300 "sync with blockend" broken for multi-block? 2011-01-14 20:13 ` Linus Walleij 2011-01-14 22:44 ` Russell King - ARM Linux @ 2011-01-16 21:16 ` Linus Walleij 2011-01-16 22:09 ` Linus Walleij 2 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2011-01-16 21:16 UTC (permalink / raw) To: linux-arm-kernel 2011/1/14 Linus Walleij <linus.ml.walleij@gmail.com>: > What it problematic with the reads *and* writes is that > sometimes there us a MCI_DATABLOCKEND missing, so the > blocks simply don't add up! This is the real bug that > the sync code was trying to solve, and not in a very > good way. Here is a follow-up explanation as to why the code as it is today actually works: the sync code has a bug. It is intended to check whether the *last* blockend interrupt *and* the dataend interrupt has occurred. However it actually checks whether *any* blockend interrupt and the datend interrupt has occured. Thus it works, with the side effect of ACK:ing multiblock transfers on U300 while the host->data_xfered value is lower that what was requested - the blocks have been read but not all are accounted for, because there are blockend IRQs missing. I'll follow up with a patch fixing the *real* issue, in an atleast a little more elegant way :-/ Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* mmci: U300 "sync with blockend" broken for multi-block? 2011-01-14 20:13 ` Linus Walleij 2011-01-14 22:44 ` Russell King - ARM Linux 2011-01-16 21:16 ` Linus Walleij @ 2011-01-16 22:09 ` Linus Walleij 2 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2011-01-16 22:09 UTC (permalink / raw) To: linux-arm-kernel ...and to round off a theory of why the U300 and Ux500 is missing interrupts: Maybe the IP-block does not really handle the case of a block interrupt not being ACK:ed before the next block is ready. So it will just fire another "1" flag, which gets ACK:ed at the end of the interrupt handler with the IRQ currently being processed. So the interrupt handler will unknowingly consume interrupts for other blocks. The right way would be to either: - Queue block ACKs so that they are ACK:ed one at the time if the hardware reads ahead. (Which requires a quite deep queue on large writes.) - Hold back further block reading from the card until the IRQ has been ACK:ed. (Which is not good for speed.) So to avoid both it is indeed a logical thing to remove the block interrupt altogether and just wait for the last one to avoid trouble, it's just not documented and HW-implemented as such. Now I'll send that patch. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* mmci: U300 "sync with blockend" broken for multi-block? 2011-01-01 11:05 mmci: U300 "sync with blockend" broken for multi-block? Rabin Vincent 2011-01-01 12:10 ` Russell King - ARM Linux @ 2011-01-05 16:02 ` Linus Walleij 1 sibling, 0 replies; 10+ messages in thread From: Linus Walleij @ 2011-01-05 16:02 UTC (permalink / raw) To: linux-arm-kernel 2011/1/1 Rabin Vincent <rabin@rab.in>: > In MMCI, there is some code to sync between the MCI_DATABLOCKEND and the > MCI_DATAEND for U300 variants, which ensures that the transfer > terminates only when both MCI_DATABLOCKEND and MCI_DATAEND occurs: > > ? ? ? ? * In the U300, the IRQs can arrive out-of-order, > ? ? ? ? * e.g. MCI_DATABLOCKEND sometimes arrives after MCI_DATAEND, > ? ? ? ? * so for this case we use the flags "blockend" and > ? ? ? ? * "dataend" to make sure both IRQs have arrived before > ? ? ? ? * concluding the transaction. > > It seems to me that this code won't work correctly for multi-block > transfers, because there MCI_DATABLOCKEND will hit for the earlier > blocks and the blockend flag will be set, and if on the last block the > MCI_DATABLOCKEND hits after the MCI_DATAEND, this synching code won't do > what it's trying to do and will instead just terminate the transfer > after MCI_DATAEND. Yes Ulf Hansson has spotted this problem... Actually, there is this patch in the public ST-Ericsson git: http://git.linaro.org/gitweb?p=bsp/st-ericsson/linux-2.6.35-ux500.git;a=commitdiff;h=2f0534d9527540a0a28e124f4e827f146f3bc128 The intention is to send out patches ASAP right now the vacations have been holding things back a bit. (Else I would have done it myself.) Ulf would you like to submit this patch to the maillist? Acked-by: Linus Walleij <linus.walleij@stericsson.com> for what's in the public git. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-01-16 22:09 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-01-01 11:05 mmci: U300 "sync with blockend" broken for multi-block? Rabin Vincent 2011-01-01 12:10 ` Russell King - ARM Linux 2011-01-05 16:15 ` Linus Walleij 2011-01-05 16:43 ` Russell King - ARM Linux 2011-01-14 20:13 ` Linus Walleij 2011-01-14 22:44 ` Russell King - ARM Linux 2011-01-16 21:11 ` Linus Walleij 2011-01-16 21:16 ` Linus Walleij 2011-01-16 22:09 ` Linus Walleij 2011-01-05 16:02 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).