Arnd Bergmann wrote:
> This is an experiment on how an SD/MMC card could be used in the MTD layer.
> I don't currently have a system set up to test this, so this driver is
> completely _untested_ and therefore you should consider it _broken_.
>
> You can get similar functionality by using the mmc_block driver together
> with block2mtd, so you may wonder what the point of another driver is.
> IMHO, there are two separate advantages from using a special driver:
>
> * better use of low-level interfaces: the MTD driver can detect the
> erase block size of the card and erase sectors in advance instead of
> blocking in the write path. The MTD file systems also expect the
> underlying interface to be synchronous, so there is little point
> in using extra kernel threads to operate on the card in the background.
>
I'm a complete MTD noob, but what uses does the MTD layer have besides JFFS2.
If it's none, than this advantage isn't that big of a deal.
> * It becomes possible to use MMC cards with jffs2 even with CONFIG_BLOCK
> disabled, which can save a significant amount of kernel memory on
> small machines that have an MMC slot but no other block device.
>
>From what I've heard, JFFS2 is close to unusuable on the sizes of modern
>SD/MMC cards. So I'd like to see some more use cases before I'm ready to let
>this in.
> I still want to be sure that I'm on the right track with this driver
> and did not make a conceptual mistake.
>
I can comment it from a MMC perspective, but the MTD stuff I will have to
assume is correct.
> @@ -616,6 +616,8 @@ static void mmc_decode_csd(struct mmc_ca
> csd->r2w_factor = UNSTUFF_BITS(resp, 26, 3);
> csd->write_blkbits = UNSTUFF_BITS(resp, 22, 4);
> csd->write_partial = UNSTUFF_BITS(resp, 21, 1);
> + csd->erase_blksize = (UNSTUFF_BITS(resp, 37, 5) + 1) *
> + (UNSTUFF_BITS(resp, 42, 5) + 1);
> } else {
> /*
> * We only understand CSD structure v1.1 and v1.2.
NAK. SD uses another format for erase blocks. See the simplified physical spec.
> +/*
> + * transfer a block to/from the card. The block needs to be aligned
> + * to mtd->writesize. If we want to implement an mtd_writev method,
> + * this needs to use stream operations with an appropriate stop
> + * command as well.
> + */
> +static int mmc_mtd_transfer_low(struct mmc_card *card, loff_t off, size_t
> len,
> + size_t *retlen, u_char *buf, int write)
> +{
> + struct scatterlist sg;
> + struct mmc_data data = {
> + .blksz = 1 << card->csd.read_blkbits,
> + .blocks = len >> card->csd.read_blkbits,
First of all, you cannot assume that read_blkbits is a valid block size when
doing writes.
Secondly, the cards default in a block size of 512 bytes, so you need to tell
the card your desired block size during probe.
> + .flags = write ? MMC_DATA_WRITE : MMC_DATA_READ,
> + .sg = &sg,
> + .sg_len = 1,
> + };
> + struct mmc_command cmd = {
> + .arg = off,
> + .data = &data,
> + .flags = MMC_RSP_R1 | MMC_CMD_ADTC,
> + .opcode = write ? MMC_WRITE_BLOCK : MMC_READ_SINGLE_BLOCK,
You set .blocks above, so I have to assume it can be more than 1. So you need
to change the opcodes accordingly.
> + };
> + struct mmc_request mrq = {
> + .cmd = &cmd,
> + .data = &data,
> + };
And it also means you need a stop command.
> +
> + /* copied from the block driver, don't understand why this is needed */
Now this gives me a bad feeling. Have you read any spec about the MMC protocol
or are you just winging it?
It is needed because the card goes into programming state after a write, where
it is very unresponsive to other commands.
> +
> + ret = mmc_card_claim_host(card);
> + if (ret) {
> + dev_warn(&card->dev, "%s: mmc_card_claim_host returned %d\n",
> + __FUNCTION__, ret);
> + ret = -EIO;
> + goto error;
> + }
mmc_card_claim_host() is currently very stupid in that it requires you to call
mmc_card_release_host() on error. I intend to fix that some time in the future.
> +/*
> + * Initialize an mmc card. We create a new MTD device for each
> + * MMC card we find. The operations are rather straightforward,
> + * so we don't even need our own data structure to contain the
> + * mtd_info.
> + */
> +static int mmc_mtd_probe(struct mmc_card *card)
> +{
> + struct mtd_info *mtd;
> + int ret;
> +
> + if (!(card->csd.cmdclass & CCC_ERASE))
> + return -ENODEV;
> +
You should probably check for CCC_BLOCK_READ here.
And your driver needs to check if the card support writes (both by
mmc_card_readonly() and CCC_BLOCK_WRITE).
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
