2008-06-01 14:43:25

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 4/7] mmc: at91_mci: add multiwrite switch

On Fri, May 30, 2008 at 02:08:56PM +0200, Nicolas Ferre wrote:
> at91_mci is capable of multiwrite. Enable it before it disappears.

This doesn't look safe. MULTIWRITE is not about whether you can do
multi-writes, but whether you can properly report how many bytes were
successfully written to the card _and_ the card acknowledged as having
been successfully written.

It looks to me as if the AT91 MMC driver reports no bytes, or all
bytes if the FIFO reports that it became empty, which presumably it
may do if you're unable to keep the FIFO full?

(Dropped Hans-JA?rgen from the CC list since my mailer seems to corrupt
the address.)

If Pierre wants to remove the MULTIWRITE flag, I'd like to hear his
solution for the pxamci driver, where the only way to ascertain how
many bytes were transmitted may be to walk the SG list comparing the
DMA pointer with what was in the hardware DMA engine at the time.
Maybe.


2008-06-09 10:43:33

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH 4/7] mmc: at91_mci: add multiwrite switch

On Sun, 1 Jun 2008 15:42:52 +0100
Russell King - ARM Linux <[email protected]> wrote:

> If Pierre wants to remove the MULTIWRITE flag, I'd like to hear his
> solution for the pxamci driver, where the only way to ascertain how
> many bytes were transmitted may be to walk the SG list comparing the
> DMA pointer with what was in the hardware DMA engine at the time.
> Maybe.

You set bytes_xfered to 0. As mentioned in my previous mail, I had a
chat with Jens about this and upper layers can only expect to get the
lower bound in how many bytes were written. Other hardware/drivers
already behaves like this so there is no point in crippling the MMC
layer in an effort to give nicer guarantees.

This is why I asked people to audit their drivers to make sure it's
the lower bound that's returned, but I've not received much in the way
of replies.

Rgds
Pierre


Attachments:
signature.asc (197.00 B)

2008-06-09 13:09:18

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 4/7] mmc: at91_mci: add multiwrite switch

On Mon, Jun 09, 2008 at 12:42:49PM +0200, Pierre Ossman wrote:
> On Sun, 1 Jun 2008 15:42:52 +0100
> Russell King - ARM Linux <[email protected]> wrote:
>
> > If Pierre wants to remove the MULTIWRITE flag, I'd like to hear his
> > solution for the pxamci driver, where the only way to ascertain how
> > many bytes were transmitted may be to walk the SG list comparing the
> > DMA pointer with what was in the hardware DMA engine at the time.
> > Maybe.
>
> You set bytes_xfered to 0. As mentioned in my previous mail, I had a
> chat with Jens about this and upper layers can only expect to get the
> lower bound in how many bytes were written. Other hardware/drivers
> already behaves like this so there is no point in crippling the MMC
> layer in an effort to give nicer guarantees.
>
> This is why I asked people to audit their drivers to make sure it's
> the lower bound that's returned, but I've not received much in the way
> of replies.

You won't do from me concerning pxamci - I don't look after it anymore
after one of my MMC cards got eaten by the misbehaving bugger. I don't
think anyone else is looking after it, so that means the driver's
effectively unmaintained.

2008-06-09 13:43:30

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH 4/7] mmc: at91_mci: add multiwrite switch

On Mon, 9 Jun 2008 14:08:31 +0100
Russell King - ARM Linux <[email protected]> wrote:

> On Mon, Jun 09, 2008 at 12:42:49PM +0200, Pierre Ossman wrote:
> >
> > This is why I asked people to audit their drivers to make sure it's
> > the lower bound that's returned, but I've not received much in the way
> > of replies.
>
> You won't do from me concerning pxamci - I don't look after it anymore
> after one of my MMC cards got eaten by the misbehaving bugger. I don't
> think anyone else is looking after it, so that means the driver's
> effectively unmaintained.

pxamxi is marked as orphaned in MAINTAINERS, so I didn't expect an
answer about that one from anyone. :)

You're still listed for the primecell driver though. What's the status
on that one?

Rgds
Pierre


Attachments:
signature.asc (197.00 B)

2008-06-09 13:54:28

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 4/7] mmc: at91_mci: add multiwrite switch

On Mon, Jun 09, 2008 at 03:42:47PM +0200, Pierre Ossman wrote:
> You're still listed for the primecell driver though. What's the status
> on that one?

It's always reported the number of bytes transferred to/from the card
(which I think includes the wait for the busy signal to deassert.)

Probably the best thing is to mark MMCI as orphaned as well - for much
the same reason. I've only one remaining MMC card which I use for
other purposes. I'm not willing to sacrifice that one for further
testing, neither am I willing to buy another card to just be used for
testing purposes.

2008-06-09 13:59:15

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH 4/7] mmc: at91_mci: add multiwrite switch

On Mon, 9 Jun 2008 14:53:57 +0100
Russell King - ARM Linux <[email protected]> wrote:

> On Mon, Jun 09, 2008 at 03:42:47PM +0200, Pierre Ossman wrote:
> > You're still listed for the primecell driver though. What's the status
> > on that one?
>
> It's always reported the number of bytes transferred to/from the card
> (which I think includes the wait for the busy signal to deassert.)
>

If only all hardware was like that...

> Probably the best thing is to mark MMCI as orphaned as well - for much
> the same reason. I've only one remaining MMC card which I use for
> other purposes. I'm not willing to sacrifice that one for further
> testing, neither am I willing to buy another card to just be used for
> testing purposes.

Fair enough. Could you send a patch, preferably with some relevant
list(s) cc:d in case someone else wants to take over the work?

Rgds
Pierre


Attachments:
signature.asc (197.00 B)