mmci: U300 "sync with blockend" broken for multi-block?

mmci: U300 "sync with blockend" broken for multi-block?

[Rabin Vincent]

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.

mmci: U300 "sync with blockend" broken for multi-block?

[Russell King - ARM Linux]

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.

mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

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

mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

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

mmci: U300 "sync with blockend" broken for multi-block?

[Russell King - ARM Linux]

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...)

Re: mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

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

mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

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

Re: mmci: U300 "sync with blockend" broken for multi-block?

[Russell King - ARM Linux]

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?

mmci: U300 "sync with blockend" broken for multi-block?

[Russell King - ARM Linux]

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?

Re: mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

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

mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

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

Re: mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

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

mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

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

Re: mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

...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

mmci: U300 "sync with blockend" broken for multi-block?

[Linus Walleij]

...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