linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] mmc: mmci: Support non-power-of-two block sizes for ux500v2 variant
Date: Thu, 22 Nov 2012 14:50:30 +0000	[thread overview]
Message-ID: <20121122145030.GH5764@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <CAPDyKFq4dBfP0ruxB-rQKruiXfk0iDLo5OKCkSQ4idK_gg1-Lg@mail.gmail.com>

On Thu, Nov 22, 2012 at 02:43:10PM +0100, Ulf Hansson wrote:
> The mmci_pio_read|write functions are being provided with sg element
> buffers, which may have any length and any address alignment, as you
> stated.
> 
> This example is a read:
> -sg-list has 2 elements.
> -sg-element [0], is 3 bytes long, and the address is not aligned to 4 bytes.
> -sg-element [1], is 1 bytes long and the address is aligned to 4 bytes.

Notice here you're talking about the _length_ of the buffer, not its
alignment.  So you're talking about something entirely different from
what I'm saying.

The alignment of the buffer has precisely NOTHING to do with what you're
talking about above.

If the first sg-element is three bytes long and the second is one byte
long, it makes no difference what so ever whether it's aligned to 4 bytes
or not.  Think about it.  You're going to have to read three bytes from
the first sg element whatever.  You can't read four bytes from it and
hope that the additional byte in the second sg-element was following it.

So actually, if you have sg-element[0].length=3 and sg-lement[1].length=1
you can't deal with that case if you're going to be loading 4 bytes
_irrespective_ of the alignment of sg-element[0]'s buffer.

See?  The memory buffer location has absolutely nothing to do with this
issue.

> pio_read will start by reading one word (4 bytes) from the FIFO and
> fill the sg-element [0] with 3 of those 4 bytes. Then it will return
> that 3 bytes has been read.
> The upper pio_irq loop still think there is 1 byte more to read but
> will never be able to read it. Instead the DATAEND irq will be
> triggered and the mmc request will be "successfully" ended.
> 
> The above problem, can be fixed by reading data to a local allocated
> buffer instead of directly to the sg-element buffer. Thus an extra
> memcpy will be needed. Our concern is that it will be messy when
> solving the corner cases and thus affecting the "hot path" too much
> for pio_read.
> 
> Instead we decided to figure out a way to prevent us from needing to
> take care of such pio_read situations completely. Since sg-element
> buffers can be considered to be consecutive in memory and by adding
> the 4 bytes alignment constraint of the buffer address, we believe
> this should do it. The idea is that it will then only be the _last_
> read to the FIFO which might be done as "unaligned" due to that the
> _length_ of data does not have to be 4 bytes aligned. Such read is
> already handled properly by pio_read.

... and thereby penalise all cases where (eg) you're transferring 16
bytes but are misaligned?

No, I think you've got this all wrong.

  reply	other threads:[~2012-11-22 14:50 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-12 14:02 [PATCH] mmc: mmci: Support non-power-of-two block sizes for ux500v2 variant Ulf Hansson
2012-10-12 21:22 ` Linus Walleij
2012-10-15 10:24 ` Johan Rudholm
2012-11-21 15:38 ` Russell King - ARM Linux
2012-11-21 16:13   ` Per Forlin
2012-11-21 16:50     ` Russell King - ARM Linux
2012-11-22 13:43       ` Ulf Hansson
2012-11-22 14:50         ` Russell King - ARM Linux [this message]
2012-11-22 17:28       ` Per Forlin
2012-11-22 17:37         ` Russell King - ARM Linux
2012-11-26 10:20           ` Per Förlin
2012-11-26 10:27             ` Russell King - ARM Linux
2012-11-26 10:52               ` Per Förlin
2012-11-28 16:55                 ` Per Forlin
2012-11-28 17:12                   ` Russell King - ARM Linux
2012-11-29 11:38                     ` Ulf Hansson
2012-12-21 10:36                       ` Ulf Hansson
2012-12-21 10:39                         ` Russell King - ARM Linux
2012-12-21 10:43                           ` Ulf Hansson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121122145030.GH5764@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).