2GB MMC/SD cards

2GB MMC/SD cards

[Pierre Ossman]

Hi Russell!

Not sure when you'll be back from your trip, but I'll leave this hanging
in your inbox until then. :)

I've been getting several complaints about the issue with sector sizes
and large MMC/SD cards. I seem to recall we discussed this earlier, but
I cannot find those mail and I don't remember our conclusions.

I do, however, have the following in both the SD and MMC card specs I
have (both sandisk though):

WRITE_BL_PARTIAL — defines whether partial block sizes can be used
                   in block write commands.

Table 3-25
  WRITE_BL_PARTIAL                                  Definition
            0       Only the WRITE_BL_LEN block size, and its partial
                    derivatives in
                    resolution of units of 512 blocks, can be used for
                    block oriented data
                    write.
            1       Smaller blocks can be used as well. The minimum
                    block size is one
                    byte.

So perhaps we should remove all the funky logic that's in mmc_block.c
right now and just always select a block size of 512 bytes? People have
been reporting that their Palms, cameras and USB readers will not accept
anything else.

Rgds
Pierre

Re: 2GB MMC/SD cards

[Russell King]

On Mon, May 29, 2006 at 04:00:26PM +0200, Pierre Ossman wrote:
> I do, however, have the following in both the SD and MMC card specs I
> have (both sandisk though):
> 
> WRITE_BL_PARTIAL ? defines whether partial block sizes can be used
>                    in block write commands.
> 
> Table 3-25
>   WRITE_BL_PARTIAL                                  Definition
>             0       Only the WRITE_BL_LEN block size, and its partial
>                     derivatives in
>                     resolution of units of 512 blocks, can be used for
>                     block oriented data
>                     write.
>             1       Smaller blocks can be used as well. The minimum
>                     block size is one
>                     byte.
> 
> So perhaps we should remove all the funky logic that's in mmc_block.c
> right now and just always select a block size of 512 bytes?

The specs I have say the following about WRITE_BL_PARTIAL:

Samsung (csd v1.2 mmc v4.1):
0: means that only the WRITE_BL_LEN block size can be used for block oriented data write.
1: means that smaller blocks can be used as well. The minimum block size is one byte.

Sandisk (csd v1.1 mmc v1.4):
0: means that only the WRITE_BL_LEN block size can be used for block oriented data write.
1: means that smaller blocks can be used as well. The minimum block size is one byte.

The wording is identical from these two differing manufacturers, so I
suspect it comes from the original spec.

> People have been reporting that their Palms, cameras and USB readers
> will not accept anything else.

I am not aware of any bug reports in this area, so I can't comment.  In
fact, I see very few reports of MMC problems at all, so I just assume
that it merely works.  Unless folk report bugs to me...

I don't know what to do about this since I don't have any cards and
I've not seen any bug reports to investigate what's going on.  So I'm
just going to say "the code as it stands is correct as to my best
knowledge, please provide details of it's failings."

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

Re: 2GB MMC/SD cards

[Russell King]

On Sat, Jun 03, 2006 at 03:15:48PM +0100, Russell King wrote:
> I don't know what to do about this since I don't have any cards and
> I've not seen any bug reports to investigate what's going on.  So I'm
> just going to say "the code as it stands is correct as to my best
> knowledge, please provide details of it's failings."

It would be nice to get the results from the following patch from people
with failing cards.  Obviously they need to enable the debugging message
to see it.

diff --git a/drivers/mmc/mmc_block.c b/drivers/mmc/mmc_block.c
--- a/drivers/mmc/mmc_block.c
+++ b/drivers/mmc/mmc_block.c
@@ -44,6 +44,7 @@
 #define MMC_SHIFT	3
 
 static int major;
+static int debug;
 
 /*
  * There is one mmc_blk_data per slot.
@@ -324,6 +325,19 @@ static struct mmc_blk_data *mmc_blk_allo
 	 */
 	md->read_only = mmc_blk_readonly(card);
 
+	if (debug)
+		printk(KERN_INFO "%s: sd%u mmca%u cap0x%x rd:b%u:m%u:p%u wr:b%u:m%u:p%u\n",
+			mmc_card_id(card),
+			mmc_card_sd(card),
+			card->csd.mmca_vsn,
+			card->csd.capacity,
+			card->csd.read_blkbits,
+			card->csd.read_misalign,
+			card->csd.read_partial,
+			card->csd.write_blkbits,
+			card->csd.write_misalign,
+			card->csd.write_partial);
+
 	/*
 	 * Figure out a workable block size.  MMC cards have:
 	 *  - two block sizes, one for read and one for write.
@@ -575,5 +589,8 @@ module_exit(mmc_blk_exit);
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Multimedia Card (MMC) block device driver");
 
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "enable debugging messages");
+
 module_param(major, int, 0444);
 MODULE_PARM_DESC(major, "specify the major device number for MMC block driver");

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

Re: 2GB MMC/SD cards

[Matt Reimer]

On 6/3/06, Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> On Mon, May 29, 2006 at 04:00:26PM +0200, Pierre Ossman wrote:
> > People have been reporting that their Palms, cameras and USB readers
> > will not accept anything else.
>
> I am not aware of any bug reports in this area, so I can't comment.  In
> fact, I see very few reports of MMC problems at all, so I just assume
> that it merely works.  Unless folk report bugs to me...
>
> I don't know what to do about this since I don't have any cards and
> I've not seen any bug reports to investigate what's going on.  So I'm
> just going to say "the code as it stands is correct as to my best
> knowledge, please provide details of it's failings."

I suspect that a lot of these readers are broken, assuming 512 byte blocks.

Matt

Re: 2GB MMC/SD cards

[Pierre Ossman]

Russell King wrote:
> The specs I have say the following about WRITE_BL_PARTIAL:
>
> Samsung (csd v1.2 mmc v4.1):
> 0: means that only the WRITE_BL_LEN block size can be used for block oriented data write.
> 1: means that smaller blocks can be used as well. The minimum block size is one byte.
>
> Sandisk (csd v1.1 mmc v1.4):
> 0: means that only the WRITE_BL_LEN block size can be used for block oriented data write.
> 1: means that smaller blocks can be used as well. The minimum block size is one byte.
>
> The wording is identical from these two differing manufacturers, so I
> suspect it comes from the original spec.
>
>   

A sponsored copy of the official spec. is long overdue...

> I am not aware of any bug reports in this area, so I can't comment.  In
> fact, I see very few reports of MMC problems at all, so I just assume
> that it merely works.  Unless folk report bugs to me...
>   

For some reason, I get lots. :)
It's probably because I am the maintainer for the only two drivers for
hardware that is found in laptops. More average Joes in that market.

Like you, I unfortunately haven't got any of these cards for myself. But
I've gotten reports of both that "other" readers use 512 bytes[1],
regardless of WRITE_BL_LEN, and that cards that have WRITE_BL_LEN of
1024 and _not_ partial, still work just fine with 512 bytes[2].

[1] http://list.hades.drzeus.cx/pipermail/wbsd-devel/2006-May/000485.html
[2] http://list.drzeus.cx/pipermail/sdhci-devel/2006-May/000826.html

> I don't know what to do about this since I don't have any cards and
> I've not seen any bug reports to investigate what's going on.  So I'm
> just going to say "the code as it stands is correct as to my best
> knowledge, please provide details of it's failings."
>
>   

I'll point people to your patch then and ask them to have you as a cc.

Rgds
Pierre

Re: 2GB MMC/SD cards

[Pierre Ossman]

Matt Reimer wrote:
>
> I suspect that a lot of these readers are broken, assuming 512 byte
> blocks.
>

That's what I thought until I looked closer at the Sandisk specs. Until
we can see what the official specs say, we won't really know what the
correct behaviour is. The Nokia boys working on the 770 have a copy.
Perhaps someone here knows how to get in touch with one of them that can
have a look?

Rgds
Pierre

Re: 2GB MMC/SD cards

[Jordan Crouse]

On 03/06/06 23:13 +0200, "Pierre Ossman" wrote:
> Matt Reimer wrote:
> 
> That's what I thought until I looked closer at the Sandisk specs. Until
> we can see what the official specs say, we won't really know what the
> correct behaviour is. The Nokia boys working on the 770 have a copy.
> Perhaps someone here knows how to get in touch with one of them that can
> have a look?

I'm not 100% sure what the community stance is on using the simplified specs.
I do believe the answer lies definitively within, but I'll refrain from
quoting it to avoid any legal complaints.  For the longest time, my gut
feeling has been that 512 byte writes were always accepted - but since all of
our 2G and 4G cards support WRITE_BL_PARTIAL, we haven't had a chance to 
prove the argument one way or the other.

We first heard about very large card issues from one of our customers, and
we haven't heard any more problems since we gave them a patch to force the
sector size on all SD/MMC cards to 512 bytes.  Thats just anecdotal evidence,
but it is food for thought.

Jordan

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

Re: 2GB MMC/SD cards

[Pierre Ossman]

Pierre Ossman wrote:
> Matt Reimer wrote:
>> I suspect that a lot of these readers are broken, assuming 512 byte
>> blocks.
>>
> 
> That's what I thought until I looked closer at the Sandisk specs. Until
> we can see what the official specs say, we won't really know what the
> correct behaviour is. The Nokia boys working on the 770 have a copy.
> Perhaps someone here knows how to get in touch with one of them that can
> have a look?
> 

With the help of Khasim Syed, who happens to have access to the MMC
spec, we now know what's in the spec. Unfortunately, what's in there is
the same text that's in Russell's card specs, which state that only
WRITE_BL_LEN is supported.

However! In the simplified SD physical spec (which you can find on the
SDA home page), we can find:

4.3.2 2GByte Card
To make 2GByte card, the Maximum Block Length (READ_BL_LEN=WRITE_BL_LEN)
shall be set to
1024 bytes. But Block Length set by CMD16 shall be up to 512 bytes to
keep consistency with 512
bytes Maximum Block Length cards (Less than and equal 2GByte cards).

It even has an example with WRITE_BL_PARTIAL=0, still setting block
length to 512. Now I know this is SD and not MMC, but the debug dump
from Serge earlier seems to suggest that MMC cards follow the same rule.

Even though this breaks the MMC spec, my vote is for 512 bytes at all
times. If the entire industry decided to violate the spec, I don't see
much gain in being stubborn here.

Rgds
Pierre

Re: 2GB MMC/SD cards

[Richard Purdie]

On Mon, 2006-06-05 at 16:29 -0600, Jordan Crouse wrote:
> I'm not 100% sure what the community stance is on using the simplified specs.
> I do believe the answer lies definitively within, but I'll refrain from
> quoting it to avoid any legal complaints.  For the longest time, my gut
> feeling has been that 512 byte writes were always accepted - but since all of
> our 2G and 4G cards support WRITE_BL_PARTIAL, we haven't had a chance to 
> prove the argument one way or the other.
> 
> We first heard about very large card issues from one of our customers, and
> we haven't heard any more problems since we gave them a patch to force the
> sector size on all SD/MMC cards to 512 bytes.  Thats just anecdotal evidence,
> but it is food for thought.

For what its worth this is the feeling I get from the users of the
Zaurus kernels as well. For the PXA driver, there is the added
complication that it doesn't support block sizes > 1023 but I've had no
complaints since I limited it to 512.

Richard

Re: 2GB MMC/SD cards

[Pierre Ossman]

Pierre Ossman wrote:
> Pierre Ossman wrote:
>> Matt Reimer wrote:
>>> I suspect that a lot of these readers are broken, assuming 512 byte
>>> blocks.
>>>
>> That's what I thought until I looked closer at the Sandisk specs. Until
>> we can see what the official specs say, we won't really know what the
>> correct behaviour is. The Nokia boys working on the 770 have a copy.
>> Perhaps someone here knows how to get in touch with one of them that can
>> have a look?
>>
> 
> With the help of Khasim Syed, who happens to have access to the MMC
> spec, we now know what's in the spec. Unfortunately, what's in there is
> the same text that's in Russell's card specs, which state that only
> WRITE_BL_LEN is supported.
> 

New information. Version 4.2 of the MMC spec changes the wording to this:

* WRITE_BL_LEN
Block length for write operations. See READ_BL_LEN for field coding.
Note that the support for 512B write access is mandatory for all cards.

Similar wording for READ_BL_LEN, but that isn't of interest to us.

Rgds
Pierre

Re: 2GB MMC/SD cards

[Russell King]

On Wed, Jun 07, 2006 at 11:08:36AM +0200, Pierre Ossman wrote:
> New information. Version 4.2 of the MMC spec changes the wording to this:
> 
> * WRITE_BL_LEN
> Block length for write operations. See READ_BL_LEN for field coding.
> Note that the support for 512B write access is mandatory for all cards.
> 
> Similar wording for READ_BL_LEN, but that isn't of interest to us.

I wonder if all 2GB cards are >= v4.2 of the spec?  If so, we could
do what would appear correct to both the spec and reality, and select
512 byte blocksizes irrespective if they conform to v4.2 or later.

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

Re: 2GB MMC/SD cards

[Pierre Ossman]

Russell King wrote:
> I wonder if all 2GB cards are >= v4.2 of the spec?  If so, we could
> do what would appear correct to both the spec and reality, and select
> 512 byte blocksizes irrespective if they conform to v4.2 or later.
>
>   

This might only be a clarification as earlier specs apparently states
that all cards must have the block size set to 512 bytes at init.

As people seem to have been running their own hacks without any
problems, we probably should just change it and see if there's any
fallout. Live dangerous for a bit. ;)

Rgds
Pierre

Re: 2GB MMC/SD cards

[Pierre Ossman]

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

Suggested patch included.

[-- Attachment #2: mmc-512-sector.patch --]
[-- Type: text/x-patch, Size: 2547 bytes --]

[MMC] Always use a sector size of 512 bytes

Both MMC and SD specifications specify (although a bit unclearly in the MMC
case) that a sector size of 512 bytes must always be supported by the card.

Cards can report larger "native" size than this, and cards >= 2 GB even
must do so. Most other readers use 512 bytes even for these cards. We should
do the same to be compatible.

Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
---

 drivers/mmc/mmc_block.c |   49 ++++-------------------------------------------
 1 files changed, 4 insertions(+), 45 deletions(-)

diff --git a/drivers/mmc/mmc_block.c b/drivers/mmc/mmc_block.c
index 587458b..96049e2 100644
--- a/drivers/mmc/mmc_block.c
+++ b/drivers/mmc/mmc_block.c
@@ -325,52 +325,11 @@ static struct mmc_blk_data *mmc_blk_allo
 	md->read_only = mmc_blk_readonly(card);
 
 	/*
-	 * Figure out a workable block size.  MMC cards have:
-	 *  - two block sizes, one for read and one for write.
-	 *  - may support partial reads and/or writes
-	 *    (allows block sizes smaller than specified)
+	 * Both SD and MMC specifications state (although a bit
+	 * unclearly in the MMC case) that a block size of 512
+	 * bytes must always be supported by the card.
 	 */
-	md->block_bits = card->csd.read_blkbits;
-	if (card->csd.write_blkbits != card->csd.read_blkbits) {
-		if (card->csd.write_blkbits < card->csd.read_blkbits &&
-		    card->csd.read_partial) {
-			/*
-			 * write block size is smaller than read block
-			 * size, but we support partial reads, so choose
-			 * the smaller write block size.
-			 */
-			md->block_bits = card->csd.write_blkbits;
-		} else if (card->csd.write_blkbits > card->csd.read_blkbits &&
-			   card->csd.write_partial) {
-			/*
-			 * read block size is smaller than write block
-			 * size, but we support partial writes.  Use read
-			 * block size.
-			 */
-		} else {
-			/*
-			 * We don't support this configuration for writes.
-			 */
-			printk(KERN_ERR "%s: unable to select block size for "
-				"writing (rb%u wb%u rp%u wp%u)\n",
-				mmc_card_id(card),
-				1 << card->csd.read_blkbits,
-				1 << card->csd.write_blkbits,
-				card->csd.read_partial,
-				card->csd.write_partial);
-			md->read_only = 1;
-		}
-	}
-
-	/*
-	 * Refuse to allow block sizes smaller than 512 bytes.
-	 */
-	if (md->block_bits < 9) {
-		printk(KERN_ERR "%s: unable to support block size %u\n",
-			mmc_card_id(card), 1 << md->block_bits);
-		ret = -EINVAL;
-		goto err_kfree;
-	}
+	md->block_bits = 9;
 
 	md->disk = alloc_disk(1 << MMC_SHIFT);
 	if (md->disk == NULL) {

Re: 2GB MMC/SD cards

[Marcin Juszkiewicz]

I can do some testing on 2GB card in Zaurus C-760 tomorrow
(2006.06.22) during local meeting (someone will have one). Does
something other then 'card is working' need to be tested?

Re: 2GB MMC/SD cards

[Daniel Drake]

Hi Pierre,

Pierre Ossman wrote:
> Suggested patch included.

What's the status on this patch? A Gentoo user at 
http://bugs.gentoo.org/142172 reports that it is required for him to be 
able to access his card, so it definitely works in some form.

> 
> [MMC] Always use a sector size of 512 bytes
> 
> Both MMC and SD specifications specify (although a bit unclearly in the MMC
> case) that a sector size of 512 bytes must always be supported by the card.
> 
> Cards can report larger "native" size than this, and cards >= 2 GB even
> must do so. Most other readers use 512 bytes even for these cards. We should
> do the same to be compatible.
> 
> Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
> ---
> 
>  drivers/mmc/mmc_block.c |   49 ++++-------------------------------------------
>  1 files changed, 4 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/mmc/mmc_block.c b/drivers/mmc/mmc_block.c
> index 587458b..96049e2 100644
> --- a/drivers/mmc/mmc_block.c
> +++ b/drivers/mmc/mmc_block.c
> @@ -325,52 +325,11 @@ static struct mmc_blk_data *mmc_blk_allo
>  	md->read_only = mmc_blk_readonly(card);
>  
>  	/*
> -	 * Figure out a workable block size.  MMC cards have:
> -	 *  - two block sizes, one for read and one for write.
> -	 *  - may support partial reads and/or writes
> -	 *    (allows block sizes smaller than specified)
> +	 * Both SD and MMC specifications state (although a bit
> +	 * unclearly in the MMC case) that a block size of 512
> +	 * bytes must always be supported by the card.
>  	 */
> -	md->block_bits = card->csd.read_blkbits;
> -	if (card->csd.write_blkbits != card->csd.read_blkbits) {
> -		if (card->csd.write_blkbits < card->csd.read_blkbits &&
> -		    card->csd.read_partial) {
> -			/*
> -			 * write block size is smaller than read block
> -			 * size, but we support partial reads, so choose
> -			 * the smaller write block size.
> -			 */
> -			md->block_bits = card->csd.write_blkbits;
> -		} else if (card->csd.write_blkbits > card->csd.read_blkbits &&
> -			   card->csd.write_partial) {
> -			/*
> -			 * read block size is smaller than write block
> -			 * size, but we support partial writes.  Use read
> -			 * block size.
> -			 */
> -		} else {
> -			/*
> -			 * We don't support this configuration for writes.
> -			 */
> -			printk(KERN_ERR "%s: unable to select block size for "
> -				"writing (rb%u wb%u rp%u wp%u)\n",
> -				mmc_card_id(card),
> -				1 << card->csd.read_blkbits,
> -				1 << card->csd.write_blkbits,
> -				card->csd.read_partial,
> -				card->csd.write_partial);
> -			md->read_only = 1;
> -		}
> -	}
> -
> -	/*
> -	 * Refuse to allow block sizes smaller than 512 bytes.
> -	 */
> -	if (md->block_bits < 9) {
> -		printk(KERN_ERR "%s: unable to support block size %u\n",
> -			mmc_card_id(card), 1 << md->block_bits);
> -		ret = -EINVAL;
> -		goto err_kfree;
> -	}
> +	md->block_bits = 9;
>  
>  	md->disk = alloc_disk(1 << MMC_SHIFT);
>  	if (md->disk == NULL) {

Re: 2GB MMC/SD cards

[Juha Yrjola]

Daniel Drake wrote:
> Hi Pierre,
> 
> Pierre Ossman wrote:
>> Suggested patch included.
> 
> What's the status on this patch? A Gentoo user at 
> http://bugs.gentoo.org/142172 reports that it is required for him to be 
> able to access his card, so it definitely works in some form.

I have to pitch in here.  This patch is required for some cards to 
operate reliably on the Nokia 770, and we've done quite a bit of 
interoperability testing already.

Pierre, could you submit it to RMK's patch tracking system?

Cheers,
Juha

Re: 2GB MMC/SD cards

[Jeff Chua]

On 8/22/06, Juha Yrjola <juha.yrjola@solidboot.com> wrote:

> > What's the status on this patch? A Gentoo user at
> > http://bugs.gentoo.org/142172 reports that it is required for him to be
> > able to access his card, so it definitely works in some form.
>
> I have to pitch in here.  This patch is required for some cards to
> operate reliably on the Nokia 770, and we've done quite a bit of
> interoperability testing already.
>
> Pierre, could you submit it to RMK's patch tracking system?

I've tested with 2GB and 4GB SD for 2.6.17 thru 2.6.18-rc4, and this
patch works. Would very much like to see this in the mainstream
2.6.18.

Thanks,
Jeff.

Re: 2GB MMC/SD cards

[Pierre Ossman]

Juha Yrjola wrote:
>
> I have to pitch in here.  This patch is required for some cards to
> operate reliably on the Nokia 770, and we've done quite a bit of
> interoperability testing already.
>
> Pierre, could you submit it to RMK's patch tracking system?
>

It's a bit up in the air at the moment, but my ambition is to have it in
before 2.6.18 is out.

Rgds
Pierre

Re: 2GB MMC/SD cards

[Russell King]

On Tue, Aug 22, 2006 at 06:19:31PM +0300, Juha Yrjola wrote:
> Daniel Drake wrote:
> >Hi Pierre,
> >
> >Pierre Ossman wrote:
> >>Suggested patch included.
> >
> >What's the status on this patch? A Gentoo user at 
> >http://bugs.gentoo.org/142172 reports that it is required for him to be 
> >able to access his card, so it definitely works in some form.
> 
> I have to pitch in here.  This patch is required for some cards to 
> operate reliably on the Nokia 770, and we've done quite a bit of 
> interoperability testing already.
> 
> Pierre, could you submit it to RMK's patch tracking system?

Absolutely not.

MMC is a little up in the air at the moment while I decide whether I want
to continue the cherade of being the maintainer of it.  It is a cherade
because the person doing 99% of the work is Pierre, and for some strange
reason, he's the one who gets all the bug reports.

This makes it extremely difficult for me to ascertain whether any patch is
correct or not - all I have to go by is the documentation, and as far as
I can see, the majority of the documentation I have says this patch is
wrong.

So I'm considering handing maintainership over to Pierre.  If I don't
have the support of the community, which is being voiced pretty loudly
by its actions, this move makes sense.

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