Skip to contentSkip to conversation

Support for read-only MMC cards

PATCH In review Pierre Ossman 2 participants 7 messages Jun 4, 2005 MMC, SD and SDIO

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Support for read-only MMC cards

If the card does not support the write commands then only allow read-only access.

drivers/mmc/mmc_block.c renamed from drivers/mmc/mmc_block.c.orig +8 -1
@@ -95,6 +95,10 @@ static int mmc_blk_open(struct inode *in
 		if (md->usage == 2)
 			check_disk_change(inode->i_bdev);
 		ret = 0;
+
+		if ((filp->f_mode & FMODE_WRITE) &&
+			!(md->queue.card->csd.cmdclass & CCC_BLOCK_WRITE))
+			ret = -EROFS;
 	}
 
 	return ret;
@@ -403,9 +407,12 @@ static int mmc_blk_probe(struct mmc_card
 	if (err)
 		goto out;
 
-	printk(KERN_INFO "%s: %s %s %dKiB\n",
+	printk(KERN_INFO "%s: %s %s %dKiB",
 		md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
 		(card->csd.capacity << card->csd.read_blkbits) / 1024);
+	if (!(card->csd.cmdclass & CCC_BLOCK_WRITE))
+		printk(" (ro)");
+	printk("\n");
 
 	mmc_set_drvdata(card, md);
 	add_disk(md->disk);
#1 lore raw
Quoted 4 lines · Pierre Ossman

If the card does not support the write commands then only allow read-only access.

drivers/mmc/mmc_block.c static int mmc_blk_probe(struct mmc_card · line 415

Quoted 10 lines
drivers/mmc/mmc_block.c renamed from drivers/mmc/mmc_block.c.orig +4 -1
@@ -403,9 +407,12 @@ static int mmc_blk_probe(struct mmc_card
 	if (err)
 		goto out;
 
-	printk(KERN_INFO "%s: %s %s %dKiB\n",
+	printk(KERN_INFO "%s: %s %s %dKiB",
 		md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
 		(card->csd.capacity << card->csd.read_blkbits) / 1024);
+	if (!())
+		printk(" (ro)");
+	printk("\n");

I'd prefer this to be:

	printk(KERN_INFO "%s: %s %s %dKiB%s\n",
 		md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
 		(card->csd.capacity << card->csd.read_blkbits) / 1024,
		card->csd.cmdclass & CCC_BLOCK_WRITE ? "" : " (ro)");
Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core
#2 lore raw
Quoted 10 lines · Russell King

I'd prefer this to be:

	printk(KERN_INFO "%s: %s %s %dKiB%s\n",
		md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
		(card->csd.capacity << card->csd.read_blkbits) / 1024,
		card->csd.cmdclass & CCC_BLOCK_WRITE ? "" : " (ro)");

I'd rather not since the SD patches have a similar test but on a different condition. It makes the [SD] patch a bit easier to follow when trying to determine what it does. Since it doesn't seem to be getting included anytime soon I'm trying to do what I can to keep it maintainable.

Rgds
Pierre

#3 lore raw
Quoted 11 lines · Pierre Ossman
Quoted 9 lines · Russell King

I'd prefer this to be:

	printk(KERN_INFO "%s: %s %s %dKiB%s\n",
		md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
		(card->csd.capacity << card->csd.read_blkbits) / 1024,
		card->csd.cmdclass & CCC_BLOCK_WRITE ? "" : " (ro)");
Quoted 2 lines

I'd rather not since the SD patches have a similar test but on a different condition.

That's just a case of adding another %s and ?:. I'd really prefer it to be done this way.

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core
#4 lore raw
Quoted 17 lines · Russell King
Quoted 14 lines · Pierre Ossman
Quoted 11 lines · Russell King

I'd prefer this to be:

	printk(KERN_INFO "%s: %s %s %dKiB%s\n",
		md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
		(card->csd.capacity << card->csd.read_blkbits) / 1024,
		card->csd.cmdclass & CCC_BLOCK_WRITE ? "" : " (ro)");
Quoted 10 lines
Quoted 4 lines

I'd rather not since the SD patches have a similar test but on a different condition.

That's just a case of adding another %s and ?:. I'd really prefer it to be done this way.

Well that would turn out something like:

    printk(KERN_INFO "%s: %s %s %dKiB%s\n",
        md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
        (card->csd.capacity << card->csd.read_blkbits) / 1024,
        (!(card->csd.cmdclass & CCC_BLOCK_WRITE) ||

mmc_card_readonly(card)) ? " (ro)" : "");

which is a bit messy IMHO.

#5 lore raw
Quoted 9 lines · Pierre Ossman

Well that would turn out something like:

    printk(KERN_INFO "%s: %s %s %dKiB%s\n",
        md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
        (card->csd.capacity << card->csd.read_blkbits) / 1024,
        (!(card->csd.cmdclass & CCC_BLOCK_WRITE) ||

mmc_card_readonly(card)) ? " (ro)" : "");

which is a bit messy IMHO.

Can't we set mmc_card_readonly status if !CCC_BLOCK_WRITE ? Would that not simplify the code in the open method as well?

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core
#6 lore raw
Quoted 5 lines · Russell King

Can't we set mmc_card_readonly status if !CCC_BLOCK_WRITE ? Would that not simplify the code in the open method as well?

Well, that depends on what we want mmc_card_readonly to signify. My thought was that it meant the entire card was read only (as should be the case with the external ro-switch on SD cards). This might not be the same thing as that the card lacks block writes. E.g. there are cards with combined I/O and storage functionality.

For most cases the lack of CCC_BLOCK_WRITE will mean read-only for everything that the card can do. But I'm not convinced that's the case for all cards. The fact that there are command classes for streamed writes should indicate possible corner cases.