[PATCH] Force MMC/SD to 512 byte block sizes

[PATCH] Force MMC/SD to 512 byte block sizes

[Jordan Crouse]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

This patch is not specific to the AU1200 SD driver, but thats what
we used to debug and verify this, so thats why it is applied against
the linux-mips tree.   Pierre, I'm sending this to you too, because I thought
you may be interested.

Large SD cards (>=2GB) report a physical block size greater then 512 bytes
(2GB reports 1024, and 4GB reports 2048).  However, a sample of different 
brands of USB attached SD readers have shown that the logical block size 
is still forced to 512 bytes.

The original mmc_block code was setting the block size to whatever the
card was reporting, thereby causing much pain and suffering when using
a card initialized elsewhere (bad partition tables, invalid FAT tables, etc).

This patch forces the block size to be 512 bytes, and adjusts the 
capacity accordingly.   With this you should be able to happily use very
large cards interchangeably between platforms.  At least, it has worked for
us.

Comments welcome,
Jordan

-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

[-- Attachment #2: mmc_fix.patch --]
[-- Type: text/plain, Size: 1899 bytes --]

PATCH: Force MMC/SD to 512 byte block sizes.

Angry customers and investigation into USB attached MMC/SD
readers have lead us to believe that the SD device block size should
be fixed at 512 bytes regardless of the block size reported by the card.
This comes into play with >2G cards.  After applying this fix, filesystems
written with a USB card reader can be read by the MMC subsystem and 
vice-versa.

Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
---

 drivers/mmc/mmc_block.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/mmc_block.c b/drivers/mmc/mmc_block.c
index d91fcf7..96fa121 100644
--- a/drivers/mmc/mmc_block.c
+++ b/drivers/mmc/mmc_block.c
@@ -303,6 +303,7 @@ static struct mmc_blk_data *mmc_blk_allo
 {
 	struct mmc_blk_data *md;
 	int devidx, ret;
+	unsigned long cap;
 
 	devidx = find_first_zero_bit(dev_use, MMC_NUM_MINORS);
 	if (devidx >= MMC_NUM_MINORS)
@@ -356,10 +357,19 @@ static struct mmc_blk_data *mmc_blk_allo
 		sprintf(md->disk->disk_name, "mmcblk%d", devidx);
 		sprintf(md->disk->devfs_name, "mmc/blk%d", devidx);
 
-		md->block_bits = card->csd.read_blkbits;
+		if (card->csd.read_blkbits > 9) {
+			md->block_bits = 9;
+
+			cap = card->csd.capacity <<
+				(card->csd.read_blkbits - 9);
+		}
+		else {
+			md->block_bits = card->csd.read_blkbits;
+			cap = card->csd.capacity;
+		}
 
 		blk_queue_hardsect_size(md->queue.queue, 1 << md->block_bits);
-		set_capacity(md->disk, card->csd.capacity);
+		set_capacity(md->disk, cap);
 	}
  out:
 	return md;
@@ -373,7 +383,7 @@ mmc_blk_set_blksize(struct mmc_blk_data 
 
 	mmc_card_claim_host(card);
 	cmd.opcode = MMC_SET_BLOCKLEN;
-	cmd.arg = 1 << card->csd.read_blkbits;
+	cmd.arg = 1 << ((card->csd.read_blkbits > 9) ? 9 : card->csd.read_blkbits);
 	cmd.flags = MMC_RSP_R1;
 	err = mmc_wait_for_cmd(card->host, &cmd, 5);
 	mmc_card_release_host(card);

Re: [PATCH] Force MMC/SD to 512 byte block sizes

[Russell King]

On Fri, Jan 06, 2006 at 09:44:06AM -0700, Jordan Crouse wrote:
> This patch is not specific to the AU1200 SD driver, but thats what
> we used to debug and verify this, so thats why it is applied against
> the linux-mips tree.   Pierre, I'm sending this to you too, because I thought
> you may be interested.

NACK.  Please wait until the next round of patches get merged and then
revalidate this.

It's obviously wrong in the case of cards which do not support partial
block writes, and it does nothing to detect this (apart from violating
their advertised capabilities.)

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: Force MMC/SD to 512 byte block sizes

[Jordan Crouse]

On 06/01/06 16:59 +0000, Russell King wrote:
> On Fri, Jan 06, 2006 at 09:44:06AM -0700, Jordan Crouse wrote:
> > This patch is not specific to the AU1200 SD driver, but thats what
> > we used to debug and verify this, so thats why it is applied against
> > the linux-mips tree.   Pierre, I'm sending this to you too, because I thought
> > you may be interested.
> 
> NACK.  Please wait until the next round of patches get merged and then
> revalidate this.

Fair enough - I am only getting what Ralf and Linus put in their git tree -
I probably should have checked your tree before assuming the code was
current.

> It's obviously wrong in the case of cards which do not support partial
> block writes, and it does nothing to detect this (apart from violating
> their advertised capabilities.)

Ok - I can understand that - its unlikely that such cards would have a 
physical block size > 512 bytes, but its always better to be safe then sorry.
Thats a fairly straight forward fix, and I'll do it while waiting for your
code to bubble through.

Regards,
Jordan

-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

Re: Force MMC/SD to 512 byte block sizes

[Jordan Crouse]

On 06/01/06 16:59 +0000, Russell King wrote:
> On Fri, Jan 06, 2006 at 09:44:06AM -0700, Jordan Crouse wrote:
> > This patch is not specific to the AU1200 SD driver, but thats what
> > we used to debug and verify this, so thats why it is applied against
> > the linux-mips tree.   Pierre, I'm sending this to you too, because I thought
> > you may be interested.
> 
> NACK.  Please wait until the next round of patches get merged and then
> revalidate this.

Ok - I saw your fixes.  Looks good to me.

I'm still sticking to the (assumption|hypothesis|foolish fantasy) that
even cards that do not support partial block writes will still support
512 byte writes - mainly because I just don't see those ASICs in the 
el-Cheapo card readers being capable of doing the advanced buffering
to convert 512 <-> 1024 or 2048, and hopefully one of these days I'll
find a card to prove that.  Until then, I don't have a leg to stand on,
so I'll quietly hang back.

Jordan

-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

Re: [PATCH] Force MMC/SD to 512 byte block sizes

[Pierre Ossman]

Jordan Crouse wrote:
> This patch is not specific to the AU1200 SD driver, but thats what
> we used to debug and verify this, so thats why it is applied against
> the linux-mips tree.   Pierre, I'm sending this to you too, because I thought
> you may be interested.

Much appreciated. :)

> 
> Large SD cards (>=2GB) report a physical block size greater then 512 bytes
> (2GB reports 1024, and 4GB reports 2048).  However, a sample of different 
> brands of USB attached SD readers have shown that the logical block size 
> is still forced to 512 bytes.
> 
> The original mmc_block code was setting the block size to whatever the
> card was reporting, thereby causing much pain and suffering when using
> a card initialized elsewhere (bad partition tables, invalid FAT tables, etc).
> 
> This patch forces the block size to be 512 bytes, and adjusts the 
> capacity accordingly.   With this you should be able to happily use very
> large cards interchangeably between platforms.  At least, it has worked for
> us.
> 
> 
> @@ -373,7 +383,7 @@ mmc_blk_set_blksize(struct mmc_blk_data 
>  
>  	mmc_card_claim_host(card);
>  	cmd.opcode = MMC_SET_BLOCKLEN;
> -	cmd.arg = 1 << card->csd.read_blkbits;
> +	cmd.arg = 1 << ((card->csd.read_blkbits > 9) ? 9 : card->csd.read_blkbits);
>  	cmd.flags = MMC_RSP_R1;
>  	err = mmc_wait_for_cmd(card->host, &cmd, 5);
>  	mmc_card_release_host(card);

This will not work. Some cards do not accept block sizes larger than the
one they've specified.

This issue has been discussed on the arm kernel ml and Russell has begun
producing patches to resolve the issue.

To solve the issue you're seeing we should lie to the block layer, not
the card. Which will cause problems when the block layer issues request
that cannot be mapped to a integer number of card blocks.

The issue is more complex than your patch suggests and I do not know
enough about the block layer to propose a way out.

Rgds
Pierre

Re: Force MMC/SD to 512 byte block sizes

[Jordan Crouse]

On 06/01/06 18:04 +0100, Pierre Ossman wrote:
> This will not work. Some cards do not accept block sizes larger than the
> one they've specified.
>
> This issue has been discussed on the arm kernel ml and Russell has begun
> producing patches to resolve the issue.

Which just goes to prove that if I'm going to do SD development, I need
to start watching the right mailing lists.

I'll defer to the previous work on this - I do know that this has
worked for pretty much every 2G and 4G card we could get our hands on,
so that was enough for me to throw it at you guys and see what sticks.

Jordan

-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>