Re: [PATCH] mmc: mmci: Support non-power-of-two block sizes for ux500v2 variant

Russell King - ARM Linux
2012-11-28T17:12:31Z

On Wed, Nov 28, 2012 at 05:55:13PM +0100, Per Forlin wrote:
> I have tried to work out an if-statement to check this properly. Here
> is my conclusion,
> This only works if sg len is 1 (in the SDIO case)
>
> if (WRITE)
> align = sg->offset & 3
> else if (READ)
> align = 0
>
> if (sg->offset & 3 && (PAGE_SIZE - (sg->offset + align) < host->size))
> error; /* cannot be handled by mmci driver */

This looks wrong. For starters, why in the write case do you _add_ the
byte offset in the word to sg->offset ? So, if we're offset by one byte,
it becomes two bytes. Makes no sense.

Secondly, (PAGE_SIZE - whatever) < host->size will be mostly always true.
What that means is the check is effectively just "sg->offset & 3".

Ah, maybe that's what you're trying to do - obfuscate the code in the hope
that I won't analyse it so you can get your check past me and then remove
all the obfuscation later... but maybe I'm just paranoid. :)

> Is this if-statement align with your view of the alignment issue ?

I've no idea why you're making it soo complicated.

Let's review. It copes fine with aligned buffers. It copes fine with
misaligned buffers that don't cross a page boundary. It also copes
fine with transfers that end with a non-word sized number of bytes
(which we don't need a check for).

So:

/* We can't cope with a misaligned transfer which crosses a page */
if (sg->offset & 3 && (sg->offset + sg->len > PAGE_SIZE ||
data->sg_len > 1))
bad;

See what I've done there? I've taken the English and converted it directly
to code. And the comment also matches the code.

And this way we continue to allow buffers that the string-IO operations
can cope with just fine, but avoid the issue of a word crossing a page
boundary.