[U-Boot] [PATCH RFC]: mmc: Add multi-block support

[U-Boot] [PATCH RFC]: mmc: Add multi-block support

[Steve Sakoman]

From: Alagu Sankar <alagusankar@embwise.com>

This patch adds multi-block read support for generic MMC. It also modifies
existing multi-block write to limit the maximum number of blocks per transfer.
A new member is added in the mmc structure for the host controller to specify
the maximum number of blocks it supports.

Signed-off-by: Alagu Sankar <alagusankar@embwise.com>
Acked-by: Steve Sakoman <steve.sakoman@linaro.org>
Tested-by: Steve Sakoman <steve.sakoman@linaro.org>
---

This is a re-submission of Alagu's patch, modified only to remove the CONFIG
option as requested in the earlier discussion of the patch:

http://www.mail-archive.com/u-boot at lists.denx.de/msg32319.html

I tested the patch on OMAP3 and OMAP4 and found no issues.  Sadly I did
not see a performance improvement.

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index c543d83..55975bb 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -77,26 +77,16 @@ struct mmc *find_mmc_device(int dev_num)
 	return NULL;
 }
 
-static ulong
-mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
+static int mmc_write_blocks(struct mmc *mmc, const char *src, uint start,
+		uint blkcnt)
 {
 	struct mmc_cmd cmd;
 	struct mmc_data data;
 	int err;
-	int stoperr = 0;
-	struct mmc *mmc = find_mmc_device(dev_num);
 	int blklen;
 
-	if (!mmc)
-		return -1;
-
 	blklen = mmc->write_bl_len;
 
-	if ((start + blkcnt) > mmc->block_dev.lba) {
-		printf("MMC: block number 0x%lx exceeds max(0x%lx)",
-			start + blkcnt, mmc->block_dev.lba);
-		return 0;
-	}
 	err = mmc_set_blocklen(mmc, mmc->write_bl_len);
 
 	if (err) {
@@ -134,18 +124,46 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
 		cmd.cmdarg = 0;
 		cmd.resp_type = MMC_RSP_R1b;
 		cmd.flags = 0;
-		stoperr = mmc_send_cmd(mmc, &cmd, NULL);
+		err = mmc_send_cmd(mmc, &cmd, NULL);
+	}
+
+	return err;
+}
+
+static ulong
+mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void *src)
+{
+	int err;
+	int i;
+	struct mmc *mmc = find_mmc_device(dev_num);
+	uint b_max = mmc->b_max;
+
+	if (!mmc)
+		return 0;
+
+	for (i = blkcnt; i > 0; i -= b_max) {
+		uint blocks = (i > b_max) ? b_max : i;
+
+		err = mmc_write_blocks(mmc, src, start, blocks);
+		if (err)
+			return blkcnt - i;
+		start += blocks;
+		src += (mmc->write_bl_len * blocks);
 	}
 
 	return blkcnt;
 }
 
-int mmc_read_block(struct mmc *mmc, void *dst, uint blocknum)
+int mmc_read_blocks(struct mmc *mmc, void *dst, uint blocknum, uint blkcnt)
 {
+	int err;
 	struct mmc_cmd cmd;
 	struct mmc_data data;
 
-	cmd.cmdidx = MMC_CMD_READ_SINGLE_BLOCK;
+	if (blkcnt > 1)
+		cmd.cmdidx = MMC_CMD_READ_MULTIPLE_BLOCK;
+	else
+		cmd.cmdidx = MMC_CMD_READ_SINGLE_BLOCK;
 
 	if (mmc->high_capacity)
 		cmd.cmdarg = blocknum;
@@ -156,62 +174,22 @@ int mmc_read_block(struct mmc *mmc, void *dst, uint blocknum)
 	cmd.flags = 0;
 
 	data.dest = dst;
-	data.blocks = 1;
+	data.blocks = blkcnt;
 	data.blocksize = mmc->read_bl_len;
 	data.flags = MMC_DATA_READ;
 
-	return mmc_send_cmd(mmc, &cmd, &data);
-}
-
-int mmc_read(struct mmc *mmc, u64 src, uchar *dst, int size)
-{
-	char *buffer;
-	int i;
-	int blklen = mmc->read_bl_len;
-	int startblock = lldiv(src, mmc->read_bl_len);
-	int endblock = lldiv(src + size - 1, mmc->read_bl_len);
-	int err = 0;
-
-	/* Make a buffer big enough to hold all the blocks we might read */
-	buffer = malloc(blklen);
-
-	if (!buffer) {
-		printf("Could not allocate buffer for MMC read!\n");
-		return -1;
-	}
-
-	/* We always do full block reads from the card */
-	err = mmc_set_blocklen(mmc, mmc->read_bl_len);
-
+	err = mmc_send_cmd(mmc, &cmd, &data);
 	if (err)
-		goto free_buffer;
-
-	for (i = startblock; i <= endblock; i++) {
-		int segment_size;
-		int offset;
-
-		err = mmc_read_block(mmc, buffer, i);
-
-		if (err)
-			goto free_buffer;
-
-		/*
-		 * The first block may not be aligned, so we
-		 * copy from the desired point in the block
-		 */
-		offset = (src & (blklen - 1));
-		segment_size = MIN(blklen - offset, size);
-
-		memcpy(dst, buffer + offset, segment_size);
+		return err;
 
-		dst += segment_size;
-		src += segment_size;
-		size -= segment_size;
+	if (blkcnt > 1) {
+		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
+		cmd.cmdarg = 0;
+		cmd.resp_type = MMC_RSP_R1b;
+		cmd.flags = 0;
+		err = mmc_send_cmd(mmc, &cmd, NULL);
 	}
 
-free_buffer:
-	free(buffer);
-
 	return err;
 }
 
@@ -220,29 +198,26 @@ static ulong mmc_bread(int dev_num, ulong start, lbaint_t blkcnt, void *dst)
 	int err;
 	int i;
 	struct mmc *mmc = find_mmc_device(dev_num);
+	uint b_max = mmc->b_max;
 
 	if (!mmc)
 		return 0;
 
-	if ((start + blkcnt) > mmc->block_dev.lba) {
-		printf("MMC: block number 0x%lx exceeds max(0x%lx)",
-			start + blkcnt, mmc->block_dev.lba);
-		return 0;
-	}
 	/* We always do full block reads from the card */
 	err = mmc_set_blocklen(mmc, mmc->read_bl_len);
-
-	if (err) {
+	if (err)
 		return 0;
-	}
 
-	for (i = start; i < start + blkcnt; i++, dst += mmc->read_bl_len) {
-		err = mmc_read_block(mmc, dst, i);
+	for (i = blkcnt; i > 0; i -= b_max) {
+		uint blocks = (i > b_max) ? b_max : i;
 
+		err = mmc_read_blocks(mmc, dst, start, blocks);
 		if (err) {
 			printf("block read failed: %d\n", err);
-			return i - start;
+			return blkcnt - i;
 		}
+		start += blocks;
+		dst += (mmc->read_bl_len * blocks);
 	}
 
 	return blkcnt;
@@ -872,6 +847,9 @@ int mmc_register(struct mmc *mmc)
 	mmc->block_dev.block_read = mmc_bread;
 	mmc->block_dev.block_write = mmc_bwrite;
 
+	if (mmc->b_max == 0)
+		mmc->b_max = 1;
+
 	INIT_LIST_HEAD (&mmc->link);
 
 	list_add_tail (&mmc->link, &mmc_devices);
diff --git a/include/mmc.h b/include/mmc.h
index 9f94f42..245e83a 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -274,6 +274,7 @@ struct mmc {
 			struct mmc_cmd *cmd, struct mmc_data *data);
 	void (*set_ios)(struct mmc *mmc);
 	int (*init)(struct mmc *mmc);
+	uint b_max;
 };
 
 int mmc_register(struct mmc *mmc);

[U-Boot] [PATCH RFC]: mmc: Add multi-block support

[Reinhard Meyer]

Dear Steve Sakoman,
> From: Alagu Sankar<alagusankar@embwise.com>
>
> This patch adds multi-block read support for generic MMC. It also modifies
> existing multi-block write to limit the maximum number of blocks per transfer.
> A new member is added in the mmc structure for the host controller to specify
> the maximum number of blocks it supports.
>
> Signed-off-by: Alagu Sankar<alagusankar@embwise.com>
> Acked-by: Steve Sakoman<steve.sakoman@linaro.org>
> Tested-by: Steve Sakoman<steve.sakoman@linaro.org>

> +static ulong
> +mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void *src)
> +{
> +	int err;
> +	int i;
> +	struct mmc *mmc = find_mmc_device(dev_num);
> +	uint b_max = mmc->b_max;
> +
> +	if (!mmc)
> +		return 0;
> +
> +	for (i = blkcnt; i>  0; i -= b_max) {
> +		uint blocks = (i>  b_max) ? b_max : i;

I feel that all vars dealing with #blocks (b_max, i) should be lbaint_t too,
not a mixture of int, uint... (i -= b_max) would have to be rephrased, then.
That would apply to the parameters of the *_blocks functions as well...
Have a look at Lei Wen's patch of 2 days ago in this matter:
[PATCH V4] mmc: seperate block number into small parts for multi-write cmd

> +	if (mmc->b_max == 0)
> +		mmc->b_max = 1;

Currently some of the mmc low level drivers do not clear the structure,
so unfortunately this would fail. That should be corrected in the drivers,
of course.

> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -274,6 +274,7 @@ struct mmc {
>   			struct mmc_cmd *cmd, struct mmc_data *data);
>   	void (*set_ios)(struct mmc *mmc);
>   	int (*init)(struct mmc *mmc);
> +	uint b_max;

lbaint_t b_max ?

Best Regards,
Reinhard

[U-Boot] [PATCH RFC]: mmc: Add multi-block support

[Lei Wen]

Hi Steve,

On Fri, Oct 15, 2010 at 6:12 AM, Steve Sakoman <steve@sakoman.com> wrote:
> From: Alagu Sankar <alagusankar@embwise.com>
>
> This patch adds multi-block read support for generic MMC. It also modifies
> existing multi-block write to limit the maximum number of blocks per transfer.
> A new member is added in the mmc structure for the host controller to specify
> the maximum number of blocks it supports.
>
> Signed-off-by: Alagu Sankar <alagusankar@embwise.com>
> Acked-by: Steve Sakoman <steve.sakoman@linaro.org>
> Tested-by: Steve Sakoman <steve.sakoman@linaro.org>
> ---
>
> This is a re-submission of Alagu's patch, modified only to remove the CONFIG
> option as requested in the earlier discussion of the patch:
>
> http://www.mail-archive.com/u-boot at lists.denx.de/msg32319.html
>
> I tested the patch on OMAP3 and OMAP4 and found no issues. ?Sadly I did
> not see a performance improvement.
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index c543d83..55975bb 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -77,26 +77,16 @@ struct mmc *find_mmc_device(int dev_num)
> ? ? ? ?return NULL;
> ?}
>
> -static ulong
> -mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
> +static int mmc_write_blocks(struct mmc *mmc, const char *src, uint start,
> + ? ? ? ? ? ? ? uint blkcnt)
> ?{
> ? ? ? ?struct mmc_cmd cmd;
> ? ? ? ?struct mmc_data data;
> ? ? ? ?int err;
> - ? ? ? int stoperr = 0;
> - ? ? ? struct mmc *mmc = find_mmc_device(dev_num);
> ? ? ? ?int blklen;
>
> - ? ? ? if (!mmc)
> - ? ? ? ? ? ? ? return -1;
> -
> ? ? ? ?blklen = mmc->write_bl_len;
>
> - ? ? ? if ((start + blkcnt) > mmc->block_dev.lba) {
> - ? ? ? ? ? ? ? printf("MMC: block number 0x%lx exceeds max(0x%lx)",
> - ? ? ? ? ? ? ? ? ? ? ? start + blkcnt, mmc->block_dev.lba);
> - ? ? ? ? ? ? ? return 0;
> - ? ? ? }

You seems don't want this boudary check, but since it was merged as a
seperated patch
before, you should give your reason for remove this warning.

> ? ? ? ?err = mmc_set_blocklen(mmc, mmc->write_bl_len);
>
> ? ? ? ?if (err) {
> @@ -134,18 +124,46 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
> ? ? ? ? ? ? ? ?cmd.cmdarg = 0;
> ? ? ? ? ? ? ? ?cmd.resp_type = MMC_RSP_R1b;
> ? ? ? ? ? ? ? ?cmd.flags = 0;
> - ? ? ? ? ? ? ? stoperr = mmc_send_cmd(mmc, &cmd, NULL);
> + ? ? ? ? ? ? ? err = mmc_send_cmd(mmc, &cmd, NULL);
> + ? ? ? }
> +
> + ? ? ? return err;
> +}
> +
> +static ulong
> +mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void *src)
> +{
> + ? ? ? int err;
> + ? ? ? int i;
> + ? ? ? struct mmc *mmc = find_mmc_device(dev_num);
> + ? ? ? uint b_max = mmc->b_max;
> +
> + ? ? ? if (!mmc)
> + ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? for (i = blkcnt; i > 0; i -= b_max) {
> + ? ? ? ? ? ? ? uint blocks = (i > b_max) ? b_max : i;
> +
> + ? ? ? ? ? ? ? err = mmc_write_blocks(mmc, src, start, blocks);
> + ? ? ? ? ? ? ? if (err)
> + ? ? ? ? ? ? ? ? ? ? ? return blkcnt - i;
> + ? ? ? ? ? ? ? start += blocks;
> + ? ? ? ? ? ? ? src += (mmc->write_bl_len * blocks);
> ? ? ? ?}
>
> ? ? ? ?return blkcnt;
> ?}
>
> -int mmc_read_block(struct mmc *mmc, void *dst, uint blocknum)
> +int mmc_read_blocks(struct mmc *mmc, void *dst, uint blocknum, uint blkcnt)
> ?{
> + ? ? ? int err;
> ? ? ? ?struct mmc_cmd cmd;
> ? ? ? ?struct mmc_data data;
>
> - ? ? ? cmd.cmdidx = MMC_CMD_READ_SINGLE_BLOCK;
> + ? ? ? if (blkcnt > 1)
> + ? ? ? ? ? ? ? cmd.cmdidx = MMC_CMD_READ_MULTIPLE_BLOCK;
> + ? ? ? else
> + ? ? ? ? ? ? ? cmd.cmdidx = MMC_CMD_READ_SINGLE_BLOCK;
>
> ? ? ? ?if (mmc->high_capacity)
> ? ? ? ? ? ? ? ?cmd.cmdarg = blocknum;
> @@ -156,62 +174,22 @@ int mmc_read_block(struct mmc *mmc, void *dst, uint blocknum)
> ? ? ? ?cmd.flags = 0;
>
> ? ? ? ?data.dest = dst;
> - ? ? ? data.blocks = 1;
> + ? ? ? data.blocks = blkcnt;
> ? ? ? ?data.blocksize = mmc->read_bl_len;
> ? ? ? ?data.flags = MMC_DATA_READ;
>
> - ? ? ? return mmc_send_cmd(mmc, &cmd, &data);
> -}
> -
> -int mmc_read(struct mmc *mmc, u64 src, uchar *dst, int size)
> -{
> - ? ? ? char *buffer;
> - ? ? ? int i;
> - ? ? ? int blklen = mmc->read_bl_len;
> - ? ? ? int startblock = lldiv(src, mmc->read_bl_len);
> - ? ? ? int endblock = lldiv(src + size - 1, mmc->read_bl_len);
> - ? ? ? int err = 0;
> -
> - ? ? ? /* Make a buffer big enough to hold all the blocks we might read */
> - ? ? ? buffer = malloc(blklen);
> -
> - ? ? ? if (!buffer) {
> - ? ? ? ? ? ? ? printf("Could not allocate buffer for MMC read!\n");
> - ? ? ? ? ? ? ? return -1;
> - ? ? ? }
> -
> - ? ? ? /* We always do full block reads from the card */
> - ? ? ? err = mmc_set_blocklen(mmc, mmc->read_bl_len);
> -
> + ? ? ? err = mmc_send_cmd(mmc, &cmd, &data);
> ? ? ? ?if (err)
> - ? ? ? ? ? ? ? goto free_buffer;
> -
> - ? ? ? for (i = startblock; i <= endblock; i++) {
> - ? ? ? ? ? ? ? int segment_size;
> - ? ? ? ? ? ? ? int offset;
> -
> - ? ? ? ? ? ? ? err = mmc_read_block(mmc, buffer, i);
> -
> - ? ? ? ? ? ? ? if (err)
> - ? ? ? ? ? ? ? ? ? ? ? goto free_buffer;
> -
> - ? ? ? ? ? ? ? /*
> - ? ? ? ? ? ? ? ?* The first block may not be aligned, so we
> - ? ? ? ? ? ? ? ?* copy from the desired point in the block
> - ? ? ? ? ? ? ? ?*/
> - ? ? ? ? ? ? ? offset = (src & (blklen - 1));
> - ? ? ? ? ? ? ? segment_size = MIN(blklen - offset, size);
> -
> - ? ? ? ? ? ? ? memcpy(dst, buffer + offset, segment_size);
> + ? ? ? ? ? ? ? return err;
>
> - ? ? ? ? ? ? ? dst += segment_size;
> - ? ? ? ? ? ? ? src += segment_size;
> - ? ? ? ? ? ? ? size -= segment_size;
> + ? ? ? if (blkcnt > 1) {
> + ? ? ? ? ? ? ? cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> + ? ? ? ? ? ? ? cmd.cmdarg = 0;
> + ? ? ? ? ? ? ? cmd.resp_type = MMC_RSP_R1b;
> + ? ? ? ? ? ? ? cmd.flags = 0;
> + ? ? ? ? ? ? ? err = mmc_send_cmd(mmc, &cmd, NULL);
> ? ? ? ?}
>
> -free_buffer:
> - ? ? ? free(buffer);
> -
> ? ? ? ?return err;
> ?}
>
> @@ -220,29 +198,26 @@ static ulong mmc_bread(int dev_num, ulong start, lbaint_t blkcnt, void *dst)
> ? ? ? ?int err;
> ? ? ? ?int i;
> ? ? ? ?struct mmc *mmc = find_mmc_device(dev_num);
> + ? ? ? uint b_max = mmc->b_max;
>
> ? ? ? ?if (!mmc)
> ? ? ? ? ? ? ? ?return 0;
>
> - ? ? ? if ((start + blkcnt) > mmc->block_dev.lba) {
> - ? ? ? ? ? ? ? printf("MMC: block number 0x%lx exceeds max(0x%lx)",
> - ? ? ? ? ? ? ? ? ? ? ? start + blkcnt, mmc->block_dev.lba);
> - ? ? ? ? ? ? ? return 0;
> - ? ? ? }

Same comments as above.

> ? ? ? ?/* We always do full block reads from the card */
> ? ? ? ?err = mmc_set_blocklen(mmc, mmc->read_bl_len);
> -
> - ? ? ? if (err) {
> + ? ? ? if (err)
> ? ? ? ? ? ? ? ?return 0;
> - ? ? ? }
>
> - ? ? ? for (i = start; i < start + blkcnt; i++, dst += mmc->read_bl_len) {
> - ? ? ? ? ? ? ? err = mmc_read_block(mmc, dst, i);
> + ? ? ? for (i = blkcnt; i > 0; i -= b_max) {
> + ? ? ? ? ? ? ? uint blocks = (i > b_max) ? b_max : i;
>
> + ? ? ? ? ? ? ? err = mmc_read_blocks(mmc, dst, start, blocks);
> ? ? ? ? ? ? ? ?if (err) {
> ? ? ? ? ? ? ? ? ? ? ? ?printf("block read failed: %d\n", err);
> - ? ? ? ? ? ? ? ? ? ? ? return i - start;
> + ? ? ? ? ? ? ? ? ? ? ? return blkcnt - i;
> ? ? ? ? ? ? ? ?}
> + ? ? ? ? ? ? ? start += blocks;
> + ? ? ? ? ? ? ? dst += (mmc->read_bl_len * blocks);
> ? ? ? ?}
>
> ? ? ? ?return blkcnt;
> @@ -872,6 +847,9 @@ int mmc_register(struct mmc *mmc)
> ? ? ? ?mmc->block_dev.block_read = mmc_bread;
> ? ? ? ?mmc->block_dev.block_write = mmc_bwrite;
>
> + ? ? ? if (mmc->b_max == 0)
> + ? ? ? ? ? ? ? mmc->b_max = 1;
> +
> ? ? ? ?INIT_LIST_HEAD (&mmc->link);
>
> ? ? ? ?list_add_tail (&mmc->link, &mmc_devices);
> diff --git a/include/mmc.h b/include/mmc.h
> index 9f94f42..245e83a 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -274,6 +274,7 @@ struct mmc {
> ? ? ? ? ? ? ? ? ? ? ? ?struct mmc_cmd *cmd, struct mmc_data *data);
> ? ? ? ?void (*set_ios)(struct mmc *mmc);
> ? ? ? ?int (*init)(struct mmc *mmc);
> + ? ? ? uint b_max;
> ?};
>
> ?int mmc_register(struct mmc *mmc);
>
Generally, I like the idea to apply multi-read command for reading,
for it really coudl boost the performance
during booting. But considering I have post the silimar patch for only
do the change on the seperating write
command, could you rebase your multi-read patch on mine, and resend it?

Best regards,
Lei