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: Wed, 28 Nov 2012 17:12:21 +0000	[thread overview]
Message-ID: <20121128171221.GF19440@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <CAFEEs1nZboGyAfpYQ2hUvp366FiS0=8u5zL6Vr1RfXac_Ls_dw@mail.gmail.com>

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.

  reply	other threads:[~2012-11-28 17:12 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
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 [this message]
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=20121128171221.GF19440@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).