[PATCH 1/2] [MMC-4.5] Disable emulation

[PATCH 1/2] [MMC-4.5] Disable emulation

[Saugata Das]

From: Saugata Das <saugata.das@linaro.org>

This patch adds the support for large sector size of 4KB by disabling emulation.
This patch passes eMMC DATA_SECTOR_SIZE as the logical block size during
mmc_blk_alloc_req.

In order to use this patch for 4KB sector size, ensure that USE_NATIVE_SECTOR
is enabled, partition table is 4KB sector size aligned and file system block
size is 4KB.

Signed-off-by: Saugata Das <saugata.das@linaro.org>
---
 drivers/mmc/card/block.c |   16 ++++++++++++++--
 drivers/mmc/core/mmc.c   |    2 ++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 91cda75..b4d0eb1 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1284,7 +1284,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 	int ret = 1, disable_multi = 0, retry = 0, type;
 	enum mmc_blk_status status;
 	struct mmc_queue_req *mq_rq;
-	struct request *req;
+	struct request *req = rqc;
 	struct mmc_async_req *areq;
 
 	if (!rqc && !mq->mqrq_prev->req)
@@ -1292,6 +1292,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 
 	do {
 		if (rqc) {
+
+			/*
+			  * When 4KB native sector is enabled, single block
+			  * read or write is not allowed
+			  */
+			if ((brq->data.blocks == 1) &&
+				(card->ext_csd.data_sector_size == 4096))
+				goto cmd_abort;
 			mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
 			areq = &mq->mqrq_cur->mmc_active;
 		} else
@@ -1539,7 +1547,11 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
 		 "mmcblk%d%s", md->name_idx, subname ? subname : "");
 
-	blk_queue_logical_block_size(md->queue.queue, 512);
+	if (mmc_card_mmc(card))
+		blk_queue_logical_block_size(md->queue.queue,
+			card->ext_csd.data_sector_size);
+	else
+		blk_queue_logical_block_size(md->queue.queue, 512);
 	set_capacity(md->disk, size);
 
 	if (mmc_host_cmd23(card->host)) {
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 7268c26..11444c6 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -516,6 +516,8 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		} else {
 			card->ext_csd.data_tag_unit_size = 0;
 		}
+	} else {
+		card->ext_csd.data_sector_size = 512;
 	}
 
 out:
-- 
1.7.4.3

[PATCH 2/2] [MMC-4.5] [MMC UTIL] Disable emulation

[Saugata Das]

From: Saugata Das <saugata.das@linaro.org>

In this patch, we add utility to disable emulation mode in the eMMC-4.5.
This is done to increase the data sector size to 4KB.

Signed-off-by: Saugata Das <saugata.das@linaro.org>

changes in v2:
	Review rework based on comments from Subhash
---
 mmc.c      |    6 ++++++
 mmc.h      |   10 ++++++++++
 mmc_cmds.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 mmc_cmds.h |    1 +
 4 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/mmc.c b/mmc.c
index c27fc24..ebc7ab2 100644
--- a/mmc.c
+++ b/mmc.c
@@ -65,6 +65,12 @@ static struct Command commands[] = {
 		"Set the eMMC writeprotect status of <device>.",
 	  NULL
 	},
+	{ do_disable_512B_emulation, -1,
+	  "disable 512B emulation", "<device>\n"
+		"Set the eMMC data sector size to 4KB by disabling emulation <device>.",
+	  NULL
+	},
+
 	{ 0, 0, 0, 0 }
 };
 
diff --git a/mmc.h b/mmc.h
index 3af36f1..34c31d3 100644
--- a/mmc.h
+++ b/mmc.h
@@ -35,6 +35,16 @@
 #define EXT_CSD_PART_SWITCH_TIME	199
 #define EXT_CSD_BOOT_CFG		179
 #define EXT_CSD_BOOT_WP			173
+#define EXT_CSD_WR_REL_PARAM	166
+#define EXT_CSD_NATIVE_SECTOR_SIZE	63 /* R */
+#define EXT_CSD_USE_NATIVE_SECTOR	62 /* R/W */
+#define EXT_CSD_DATA_SECTOR_SIZE	61 /* R */
+
+/*
+ * WR_REL_PARAM field definitions
+ */
+#define HS_CTRL_REL	(1<<0)
+#define EN_REL_WR	(1<<2)
 
 /*
  * EXT_CSD field definitions
diff --git a/mmc_cmds.c b/mmc_cmds.c
index 4562cef..f0e64c9 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -168,6 +168,57 @@ int do_writeprotect_set(int nargs, char **argv)
 	return ret;
 }
 
+
+int do_disable_512B_emulation(int nargs, char **argv)
+{
+	__u8 ext_csd[512], native_sector_size, data_sector_size, wr_rel_param;
+	int fd, ret;
+	char *device;
+
+	CHECK(nargs != 2, "Usage: mmc </path/to/mmcblkX>\n", exit(1));
+
+	device = argv[1];
+
+	fd = open(device, O_RDWR);
+	if (fd < 0) {
+		perror("open");
+		exit(1);
+	}
+
+	ret = read_extcsd(fd, ext_csd);
+	if (ret) {
+		fprintf(stderr, "Could not read EXT_CSD from %s\n", device);
+		exit(1);
+	}
+
+	wr_rel_param = ext_csd[EXT_CSD_WR_REL_PARAM];
+	native_sector_size = ext_csd[EXT_CSD_NATIVE_SECTOR_SIZE];
+	data_sector_size = ext_csd[EXT_CSD_DATA_SECTOR_SIZE];
+
+	if (native_sector_size &&
+		!data_sector_size &&
+		(wr_rel_param & EN_REL_WR)) {
+		ret = write_extcsd_value(fd, EXT_CSD_USE_NATIVE_SECTOR, 1);
+
+		if (ret) {
+			fprintf(stderr, "Could not write 0x%02x to "
+					"EXT_CSD[%d] in %s\n",
+					1, EXT_CSD_BOOT_WP, device);
+			exit(1);
+		}
+		printf("MMC disable 512B emulation successful\n"
+				"Now reset the device to switch to 4KB "
+				"native sector mode\n");
+	} else if (native_sector_size && data_sector_size) {
+		printf("MMC already disabled 512B emulation mode\n");
+	} else {
+		printf("MMC does not support disabling 512B emulation mode\n");
+	}
+
+	return ret;
+}
+
+
 int do_read_extcsd(int nargs, char **argv)
 {
 	__u8 ext_csd[512], ext_csd_rev, reg;
diff --git a/mmc_cmds.h b/mmc_cmds.h
index 66e9acb..56f46d7 100644
--- a/mmc_cmds.h
+++ b/mmc_cmds.h
@@ -19,3 +19,4 @@ int do_read_extcsd(int nargs, char **argv);
 int do_write_extcsd(int nargs, char **argv);
 int do_writeprotect_get(int nargs, char **argv);
 int do_writeprotect_set(int nargs, char **argv);
+int do_disable_512B_emulation(int nargs, char **argv);
-- 
1.7.4.3

RE: [PATCH 2/2] [MMC-4.5] [MMC UTIL] Disable emulation

[Subhash Jadavani]

Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>

Sougata,
There are additional blank lines before and after the
do_disable_512B_emulation() function. You may want to remove them.

Regards,
Subhash

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Saugata Das
> Sent: Wednesday, May 16, 2012 9:57 PM
> To: linux-mmc@vger.kernel.org; venkat@linaro.org
> Cc: patches@linaro.org; saugata.das@linaro.org; arnd.bergmann@linaro.org;
> lporzio@micron.com
> Subject: [PATCH 2/2] [MMC-4.5] [MMC UTIL] Disable emulation
> 
> From: Saugata Das <saugata.das@linaro.org>
> 
> In this patch, we add utility to disable emulation mode in the eMMC-4.5.
> This is done to increase the data sector size to 4KB.
> 
> Signed-off-by: Saugata Das <saugata.das@linaro.org>
> 
> changes in v2:
> 	Review rework based on comments from Subhash
> ---
>  mmc.c      |    6 ++++++
>  mmc.h      |   10 ++++++++++
>  mmc_cmds.c |   51
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  mmc_cmds.h |    1 +
>  4 files changed, 68 insertions(+), 0 deletions(-)
> 
> diff --git a/mmc.c b/mmc.c
> index c27fc24..ebc7ab2 100644
> --- a/mmc.c
> +++ b/mmc.c
> @@ -65,6 +65,12 @@ static struct Command commands[] = {
>  		"Set the eMMC writeprotect status of <device>.",
>  	  NULL
>  	},
> +	{ do_disable_512B_emulation, -1,
> +	  "disable 512B emulation", "<device>\n"
> +		"Set the eMMC data sector size to 4KB by disabling emulation
> <device>.",
> +	  NULL
> +	},
> +
>  	{ 0, 0, 0, 0 }
>  };
> 
> diff --git a/mmc.h b/mmc.h
> index 3af36f1..34c31d3 100644
> --- a/mmc.h
> +++ b/mmc.h
> @@ -35,6 +35,16 @@
>  #define EXT_CSD_PART_SWITCH_TIME	199
>  #define EXT_CSD_BOOT_CFG		179
>  #define EXT_CSD_BOOT_WP			173
> +#define EXT_CSD_WR_REL_PARAM	166
> +#define EXT_CSD_NATIVE_SECTOR_SIZE	63 /* R */
> +#define EXT_CSD_USE_NATIVE_SECTOR	62 /* R/W */
> +#define EXT_CSD_DATA_SECTOR_SIZE	61 /* R */
> +
> +/*
> + * WR_REL_PARAM field definitions
> + */
> +#define HS_CTRL_REL	(1<<0)
> +#define EN_REL_WR	(1<<2)
> 
>  /*
>   * EXT_CSD field definitions
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 4562cef..f0e64c9 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -168,6 +168,57 @@ int do_writeprotect_set(int nargs, char **argv)
>  	return ret;
>  }
> 
> +
> +int do_disable_512B_emulation(int nargs, char **argv) {
> +	__u8 ext_csd[512], native_sector_size, data_sector_size,
> wr_rel_param;
> +	int fd, ret;
> +	char *device;
> +
> +	CHECK(nargs != 2, "Usage: mmc </path/to/mmcblkX>\n", exit(1));
> +
> +	device = argv[1];
> +
> +	fd = open(device, O_RDWR);
> +	if (fd < 0) {
> +		perror("open");
> +		exit(1);
> +	}
> +
> +	ret = read_extcsd(fd, ext_csd);
> +	if (ret) {
> +		fprintf(stderr, "Could not read EXT_CSD from %s\n", device);
> +		exit(1);
> +	}
> +
> +	wr_rel_param = ext_csd[EXT_CSD_WR_REL_PARAM];
> +	native_sector_size = ext_csd[EXT_CSD_NATIVE_SECTOR_SIZE];
> +	data_sector_size = ext_csd[EXT_CSD_DATA_SECTOR_SIZE];
> +
> +	if (native_sector_size &&
> +		!data_sector_size &&
> +		(wr_rel_param & EN_REL_WR)) {
> +		ret = write_extcsd_value(fd,
> EXT_CSD_USE_NATIVE_SECTOR, 1);
> +
> +		if (ret) {
> +			fprintf(stderr, "Could not write 0x%02x to "
> +					"EXT_CSD[%d] in %s\n",
> +					1, EXT_CSD_BOOT_WP, device);
> +			exit(1);
> +		}
> +		printf("MMC disable 512B emulation successful\n"
> +				"Now reset the device to switch to 4KB "
> +				"native sector mode\n");
> +	} else if (native_sector_size && data_sector_size) {
> +		printf("MMC already disabled 512B emulation mode\n");
> +	} else {
> +		printf("MMC does not support disabling 512B emulation
> mode\n");
> +	}
> +
> +	return ret;
> +}
> +
> +
>  int do_read_extcsd(int nargs, char **argv)  {
>  	__u8 ext_csd[512], ext_csd_rev, reg;
> diff --git a/mmc_cmds.h b/mmc_cmds.h
> index 66e9acb..56f46d7 100644
> --- a/mmc_cmds.h
> +++ b/mmc_cmds.h
> @@ -19,3 +19,4 @@ int do_read_extcsd(int nargs, char **argv);  int
> do_write_extcsd(int nargs, char **argv);  int do_writeprotect_get(int
nargs,
> char **argv);  int do_writeprotect_set(int nargs, char **argv);
> +int do_disable_512B_emulation(int nargs, char **argv);
> --
> 1.7.4.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] [MMC-4.5] [MMC UTIL] Disable emulation

[Saugata Das]

On 17 May 2012 13:18, Subhash Jadavani <subhashj@codeaurora.org> wrote:
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
>
> Sougata,
> There are additional blank lines before and after the
> do_disable_512B_emulation() function. You may want to remove them.

I have done that in the new version. Please check.


>
> Regards,
> Subhash
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>> owner@vger.kernel.org] On Behalf Of Saugata Das
>> Sent: Wednesday, May 16, 2012 9:57 PM
>> To: linux-mmc@vger.kernel.org; venkat@linaro.org
>> Cc: patches@linaro.org; saugata.das@linaro.org; arnd.bergmann@linaro.org;
>> lporzio@micron.com
>> Subject: [PATCH 2/2] [MMC-4.5] [MMC UTIL] Disable emulation
>>
>> From: Saugata Das <saugata.das@linaro.org>
>>
>> In this patch, we add utility to disable emulation mode in the eMMC-4.5.
>> This is done to increase the data sector size to 4KB.
>>
>> Signed-off-by: Saugata Das <saugata.das@linaro.org>
>>
>> changes in v2:
>>       Review rework based on comments from Subhash
>> ---
>>  mmc.c      |    6 ++++++
>>  mmc.h      |   10 ++++++++++
>>  mmc_cmds.c |   51
>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  mmc_cmds.h |    1 +
>>  4 files changed, 68 insertions(+), 0 deletions(-)
>>
>> diff --git a/mmc.c b/mmc.c
>> index c27fc24..ebc7ab2 100644
>> --- a/mmc.c
>> +++ b/mmc.c
>> @@ -65,6 +65,12 @@ static struct Command commands[] = {
>>               "Set the eMMC writeprotect status of <device>.",
>>         NULL
>>       },
>> +     { do_disable_512B_emulation, -1,
>> +       "disable 512B emulation", "<device>\n"
>> +             "Set the eMMC data sector size to 4KB by disabling emulation
>> <device>.",
>> +       NULL
>> +     },
>> +
>>       { 0, 0, 0, 0 }
>>  };
>>
>> diff --git a/mmc.h b/mmc.h
>> index 3af36f1..34c31d3 100644
>> --- a/mmc.h
>> +++ b/mmc.h
>> @@ -35,6 +35,16 @@
>>  #define EXT_CSD_PART_SWITCH_TIME     199
>>  #define EXT_CSD_BOOT_CFG             179
>>  #define EXT_CSD_BOOT_WP                      173
>> +#define EXT_CSD_WR_REL_PARAM 166
>> +#define EXT_CSD_NATIVE_SECTOR_SIZE   63 /* R */
>> +#define EXT_CSD_USE_NATIVE_SECTOR    62 /* R/W */
>> +#define EXT_CSD_DATA_SECTOR_SIZE     61 /* R */
>> +
>> +/*
>> + * WR_REL_PARAM field definitions
>> + */
>> +#define HS_CTRL_REL  (1<<0)
>> +#define EN_REL_WR    (1<<2)
>>
>>  /*
>>   * EXT_CSD field definitions
>> diff --git a/mmc_cmds.c b/mmc_cmds.c
>> index 4562cef..f0e64c9 100644
>> --- a/mmc_cmds.c
>> +++ b/mmc_cmds.c
>> @@ -168,6 +168,57 @@ int do_writeprotect_set(int nargs, char **argv)
>>       return ret;
>>  }
>>
>> +
>> +int do_disable_512B_emulation(int nargs, char **argv) {
>> +     __u8 ext_csd[512], native_sector_size, data_sector_size,
>> wr_rel_param;
>> +     int fd, ret;
>> +     char *device;
>> +
>> +     CHECK(nargs != 2, "Usage: mmc </path/to/mmcblkX>\n", exit(1));
>> +
>> +     device = argv[1];
>> +
>> +     fd = open(device, O_RDWR);
>> +     if (fd < 0) {
>> +             perror("open");
>> +             exit(1);
>> +     }
>> +
>> +     ret = read_extcsd(fd, ext_csd);
>> +     if (ret) {
>> +             fprintf(stderr, "Could not read EXT_CSD from %s\n", device);
>> +             exit(1);
>> +     }
>> +
>> +     wr_rel_param = ext_csd[EXT_CSD_WR_REL_PARAM];
>> +     native_sector_size = ext_csd[EXT_CSD_NATIVE_SECTOR_SIZE];
>> +     data_sector_size = ext_csd[EXT_CSD_DATA_SECTOR_SIZE];
>> +
>> +     if (native_sector_size &&
>> +             !data_sector_size &&
>> +             (wr_rel_param & EN_REL_WR)) {
>> +             ret = write_extcsd_value(fd,
>> EXT_CSD_USE_NATIVE_SECTOR, 1);
>> +
>> +             if (ret) {
>> +                     fprintf(stderr, "Could not write 0x%02x to "
>> +                                     "EXT_CSD[%d] in %s\n",
>> +                                     1, EXT_CSD_BOOT_WP, device);
>> +                     exit(1);
>> +             }
>> +             printf("MMC disable 512B emulation successful\n"
>> +                             "Now reset the device to switch to 4KB "
>> +                             "native sector mode\n");
>> +     } else if (native_sector_size && data_sector_size) {
>> +             printf("MMC already disabled 512B emulation mode\n");
>> +     } else {
>> +             printf("MMC does not support disabling 512B emulation
>> mode\n");
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +
>>  int do_read_extcsd(int nargs, char **argv)  {
>>       __u8 ext_csd[512], ext_csd_rev, reg;
>> diff --git a/mmc_cmds.h b/mmc_cmds.h
>> index 66e9acb..56f46d7 100644
>> --- a/mmc_cmds.h
>> +++ b/mmc_cmds.h
>> @@ -19,3 +19,4 @@ int do_read_extcsd(int nargs, char **argv);  int
>> do_write_extcsd(int nargs, char **argv);  int do_writeprotect_get(int
> nargs,
>> char **argv);  int do_writeprotect_set(int nargs, char **argv);
>> +int do_disable_512B_emulation(int nargs, char **argv);
>> --
>> 1.7.4.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the
>> body of a message to majordomo@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 1/2] [MMC-4.5] Disable emulation

[Namjae Jeon]

2012/5/17, Saugata Das <saugata.das@stericsson.com>:
> From: Saugata Das <saugata.das@linaro.org>
>
> This patch adds the support for large sector size of 4KB by disabling
> emulation.
> This patch passes eMMC DATA_SECTOR_SIZE as the logical block size during
> mmc_blk_alloc_req.
>
> In order to use this patch for 4KB sector size, ensure that
> USE_NATIVE_SECTOR
> is enabled, partition table is 4KB sector size aligned and file system
> block
> size is 4KB.
If you change native 4K from 512B emulation, filesystem sector size is
also 4KB align as well block size.
because default block size of most of filesystem is 4KB, but
filesystem sector size is 512B,
So filesystem corruption can be occured on native 4K device.
>
> Signed-off-by: Saugata Das <saugata.das@linaro.org>
> ---
>  drivers/mmc/card/block.c |   16 ++++++++++++++--
>  drivers/mmc/core/mmc.c   |    2 ++
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 91cda75..b4d0eb1 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1284,7 +1284,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
> struct request *rqc)
>  	int ret = 1, disable_multi = 0, retry = 0, type;
>  	enum mmc_blk_status status;
>  	struct mmc_queue_req *mq_rq;
> -	struct request *req;
> +	struct request *req = rqc;
>  	struct mmc_async_req *areq;
>
>  	if (!rqc && !mq->mqrq_prev->req)
> @@ -1292,6 +1292,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
> struct request *rqc)
>
>  	do {
>  		if (rqc) {
> +
> +			/*
> +			  * When 4KB native sector is enabled, single block
> +			  * read or write is not allowed
> +			  */
> +			if ((brq->data.blocks == 1) &&
> +				(card->ext_csd.data_sector_size == 4096))
> +				goto cmd_abort;
                                               Why did you add this
code in any case ?

>  			mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>  			areq = &mq->mqrq_cur->mmc_active;
>  		} else
> @@ -1539,7 +1547,11 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct
> mmc_card *card,
>  	snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
>  		 "mmcblk%d%s", md->name_idx, subname ? subname : "");
>
> -	blk_queue_logical_block_size(md->queue.queue, 512);
> +	if (mmc_card_mmc(card))
> +		blk_queue_logical_block_size(md->queue.queue,
> +			card->ext_csd.data_sector_size);
> +	else
> +		blk_queue_logical_block_size(md->queue.queue, 512);
>  	set_capacity(md->disk, size);
>
>  	if (mmc_host_cmd23(card->host)) {
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 7268c26..11444c6 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -516,6 +516,8 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8
> *ext_csd)
>  		} else {
>  			card->ext_csd.data_tag_unit_size = 0;
>  		}
> +	} else {
> +		card->ext_csd.data_sector_size = 512;
>  	}
>
>  out:
> --
> 1.7.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 1/2] [MMC-4.5] Disable emulation

[Saugata Das]

On 17 May 2012 06:35, Namjae Jeon <linkinjeon@gmail.com> wrote:
> 2012/5/17, Saugata Das <saugata.das@stericsson.com>:
>> From: Saugata Das <saugata.das@linaro.org>
>>
>> This patch adds the support for large sector size of 4KB by disabling
>> emulation.
>> This patch passes eMMC DATA_SECTOR_SIZE as the logical block size during
>> mmc_blk_alloc_req.
>>
>> In order to use this patch for 4KB sector size, ensure that
>> USE_NATIVE_SECTOR
>> is enabled, partition table is 4KB sector size aligned and file system
>> block
>> size is 4KB.
> If you change native 4K from 512B emulation, filesystem sector size is
> also 4KB align as well block size.
> because default block size of most of filesystem is 4KB, but
> filesystem sector size is 512B,
> So filesystem corruption can be occured on native 4K device.
>>

I have tested ext4 with 4KB block size and I did not see an instance
where it requested 512B.
For sure, if someone is using file system with 512B sector on eMMC
then the USE_NATIVE_SECTOR should not be enabled.

The idea behind the patch is, MMC driver should notify the sector size
information in a generic manner to file system. So, we depend here on
ext_csd.data_sector_size, which should work in both 512B and 4KB case.


>> Signed-off-by: Saugata Das <saugata.das@linaro.org>
>> ---
>>  drivers/mmc/card/block.c |   16 ++++++++++++++--
>>  drivers/mmc/core/mmc.c   |    2 ++
>>  2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index 91cda75..b4d0eb1 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1284,7 +1284,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
>> struct request *rqc)
>>       int ret = 1, disable_multi = 0, retry = 0, type;
>>       enum mmc_blk_status status;
>>       struct mmc_queue_req *mq_rq;
>> -     struct request *req;
>> +     struct request *req = rqc;
>>       struct mmc_async_req *areq;
>>
>>       if (!rqc && !mq->mqrq_prev->req)
>> @@ -1292,6 +1292,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
>> struct request *rqc)
>>
>>       do {
>>               if (rqc) {
>> +
>> +                     /*
>> +                       * When 4KB native sector is enabled, single block
>> +                       * read or write is not allowed
>> +                       */
>> +                     if ((brq->data.blocks == 1) &&
>> +                             (card->ext_csd.data_sector_size == 4096))
>> +                             goto cmd_abort;
>                                               Why did you add this
> code in any case ?
>

This was pointed during the previous review that with 4KB sector mode,
512B transfer is not allowed. So, this check has been added here.

>>                       mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>>                       areq = &mq->mqrq_cur->mmc_active;
>>               } else
>> @@ -1539,7 +1547,11 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct
>> mmc_card *card,
>>       snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
>>                "mmcblk%d%s", md->name_idx, subname ? subname : "");
>>
>> -     blk_queue_logical_block_size(md->queue.queue, 512);
>> +     if (mmc_card_mmc(card))
>> +             blk_queue_logical_block_size(md->queue.queue,
>> +                     card->ext_csd.data_sector_size);
>> +     else
>> +             blk_queue_logical_block_size(md->queue.queue, 512);
>>       set_capacity(md->disk, size);
>>
>>       if (mmc_host_cmd23(card->host)) {
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 7268c26..11444c6 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -516,6 +516,8 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8
>> *ext_csd)
>>               } else {
>>                       card->ext_csd.data_tag_unit_size = 0;
>>               }
>> +     } else {
>> +             card->ext_csd.data_sector_size = 512;
>>       }
>>
>>  out:
>> --
>> 1.7.4.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

Re: [PATCH 1/2] [MMC-4.5] Disable emulation

[Namjae Jeon]

2012/5/17, Saugata Das <saugata.das@linaro.org>:
> On 17 May 2012 06:35, Namjae Jeon <linkinjeon@gmail.com> wrote:
>> 2012/5/17, Saugata Das <saugata.das@stericsson.com>:
>>> From: Saugata Das <saugata.das@linaro.org>
>>>
>>> This patch adds the support for large sector size of 4KB by disabling
>>> emulation.
>>> This patch passes eMMC DATA_SECTOR_SIZE as the logical block size during
>>> mmc_blk_alloc_req.
>>>
>>> In order to use this patch for 4KB sector size, ensure that
>>> USE_NATIVE_SECTOR
>>> is enabled, partition table is 4KB sector size aligned and file system
>>> block
>>> size is 4KB.
>> If you change native 4K from 512B emulation, filesystem sector size is
>> also 4KB align as well block size.
>> because default block size of most of filesystem is 4KB, but
>> filesystem sector size is 512B,
>> So filesystem corruption can be occured on native 4K device.
>>>
>
> I have tested ext4 with 4KB block size and I did not see an instance
> where it requested 512B.
-> yes, you can not see it, because when formating parittion, sector
size is set to 4K by I/O topology. As you know, several filesystems
can be used on eMMC not only Ext4. You should mention filesystem
sector size align in changelog.
> For sure, if someone is using file system with 512B sector on eMMC
> then the USE_NATIVE_SECTOR should not be enabled.
-> it will be the reason about below my question.
>
> The idea behind the patch is, MMC driver should notify the sector size
> information in a generic manner to file system. So, we depend here on
> ext_csd.data_sector_size, which should work in both 512B and 4KB case.
>
>
>>> Signed-off-by: Saugata Das <saugata.das@linaro.org>
>>> ---
>>>  drivers/mmc/card/block.c |   16 ++++++++++++++--
>>>  drivers/mmc/core/mmc.c   |    2 ++
>>>  2 files changed, 16 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>> index 91cda75..b4d0eb1 100644
>>> --- a/drivers/mmc/card/block.c
>>> +++ b/drivers/mmc/card/block.c
>>> @@ -1284,7 +1284,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>>> *mq,
>>> struct request *rqc)
>>>       int ret = 1, disable_multi = 0, retry = 0, type;
>>>       enum mmc_blk_status status;
>>>       struct mmc_queue_req *mq_rq;
>>> -     struct request *req;
>>> +     struct request *req = rqc;
>>>       struct mmc_async_req *areq;
>>>
>>>       if (!rqc && !mq->mqrq_prev->req)
>>> @@ -1292,6 +1292,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>>> *mq,
>>> struct request *rqc)
>>>
>>>       do {
>>>               if (rqc) {
>>> +
>>> +                     /*
>>> +                       * When 4KB native sector is enabled, single
>>> block
>>> +                       * read or write is not allowed
>>> +                       */
>>> +                     if ((brq->data.blocks == 1) &&
>>> +                             (card->ext_csd.data_sector_size == 4096))
>>> +                             goto cmd_abort;
>>                                               Why did you add this
>> code in any case ?
>>
>
> This was pointed during the previous review that with 4KB sector mode,
> 512B transfer is not allowed. So, this check has been added here.
-> I think that you should add error log to inform user error case.
>
>>>                       mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>>>                       areq = &mq->mqrq_cur->mmc_active;
>>>               } else
>>> @@ -1539,7 +1547,11 @@ static struct mmc_blk_data
>>> *mmc_blk_alloc_req(struct
>>> mmc_card *card,
>>>       snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
>>>                "mmcblk%d%s", md->name_idx, subname ? subname : "");
>>>
>>> -     blk_queue_logical_block_size(md->queue.queue, 512);
>>> +     if (mmc_card_mmc(card))
>>> +             blk_queue_logical_block_size(md->queue.queue,
>>> +                     card->ext_csd.data_sector_size);
>>> +     else
>>> +             blk_queue_logical_block_size(md->queue.queue, 512);
>>>       set_capacity(md->disk, size);
>>>
>>>       if (mmc_host_cmd23(card->host)) {
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index 7268c26..11444c6 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -516,6 +516,8 @@ static int mmc_read_ext_csd(struct mmc_card *card,
>>> u8
>>> *ext_csd)
>>>               } else {
>>>                       card->ext_csd.data_tag_unit_size = 0;
>>>               }
>>> +     } else {
>>> +             card->ext_csd.data_sector_size = 512;
>>>       }
>>>
>>>  out:
>>> --
>>> 1.7.4.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>

Re: [PATCH 1/2] [MMC-4.5] Disable emulation

[Saugata Das]

On 17 May 2012 09:04, Namjae Jeon <linkinjeon@gmail.com> wrote:
> 2012/5/17, Saugata Das <saugata.das@linaro.org>:
>> On 17 May 2012 06:35, Namjae Jeon <linkinjeon@gmail.com> wrote:
>>> 2012/5/17, Saugata Das <saugata.das@stericsson.com>:
>>>> From: Saugata Das <saugata.das@linaro.org>
>>>>
>>>> This patch adds the support for large sector size of 4KB by disabling
>>>> emulation.
>>>> This patch passes eMMC DATA_SECTOR_SIZE as the logical block size during
>>>> mmc_blk_alloc_req.
>>>>
>>>> In order to use this patch for 4KB sector size, ensure that
>>>> USE_NATIVE_SECTOR
>>>> is enabled, partition table is 4KB sector size aligned and file system
>>>> block
>>>> size is 4KB.
>>> If you change native 4K from 512B emulation, filesystem sector size is
>>> also 4KB align as well block size.
>>> because default block size of most of filesystem is 4KB, but
>>> filesystem sector size is 512B,
>>> So filesystem corruption can be occured on native 4K device.
>>>>
>>
>> I have tested ext4 with 4KB block size and I did not see an instance
>> where it requested 512B.
> -> yes, you can not see it, because when formating parittion, sector
> size is set to 4K by I/O topology. As you know, several filesystems
> can be used on eMMC not only Ext4. You should mention filesystem
> sector size align in changelog.

I will add the remark that file system block and sector size should be
4KB multiple.

>> For sure, if someone is using file system with 512B sector on eMMC
>> then the USE_NATIVE_SECTOR should not be enabled.
> -> it will be the reason about below my question.
>>
>> The idea behind the patch is, MMC driver should notify the sector size
>> information in a generic manner to file system. So, we depend here on
>> ext_csd.data_sector_size, which should work in both 512B and 4KB case.
>>
>>
>>>> Signed-off-by: Saugata Das <saugata.das@linaro.org>
>>>> ---
>>>>  drivers/mmc/card/block.c |   16 ++++++++++++++--
>>>>  drivers/mmc/core/mmc.c   |    2 ++
>>>>  2 files changed, 16 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>> index 91cda75..b4d0eb1 100644
>>>> --- a/drivers/mmc/card/block.c
>>>> +++ b/drivers/mmc/card/block.c
>>>> @@ -1284,7 +1284,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>>>> *mq,
>>>> struct request *rqc)
>>>>       int ret = 1, disable_multi = 0, retry = 0, type;
>>>>       enum mmc_blk_status status;
>>>>       struct mmc_queue_req *mq_rq;
>>>> -     struct request *req;
>>>> +     struct request *req = rqc;
>>>>       struct mmc_async_req *areq;
>>>>
>>>>       if (!rqc && !mq->mqrq_prev->req)
>>>> @@ -1292,6 +1292,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>>>> *mq,
>>>> struct request *rqc)
>>>>
>>>>       do {
>>>>               if (rqc) {
>>>> +
>>>> +                     /*
>>>> +                       * When 4KB native sector is enabled, single
>>>> block
>>>> +                       * read or write is not allowed
>>>> +                       */
>>>> +                     if ((brq->data.blocks == 1) &&
>>>> +                             (card->ext_csd.data_sector_size == 4096))
>>>> +                             goto cmd_abort;
>>>                                               Why did you add this
>>> code in any case ?
>>>
>>
>> This was pointed during the previous review that with 4KB sector mode,
>> 512B transfer is not allowed. So, this check has been added here.
> -> I think that you should add error log to inform user error case.

I will add pr_err here. I will also modify the condition
(brq->data.blocks == 1)  to
(brq->data.blocks % 4)


>>
>>>>                       mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>>>>                       areq = &mq->mqrq_cur->mmc_active;
>>>>               } else
>>>> @@ -1539,7 +1547,11 @@ static struct mmc_blk_data
>>>> *mmc_blk_alloc_req(struct
>>>> mmc_card *card,
>>>>       snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
>>>>                "mmcblk%d%s", md->name_idx, subname ? subname : "");
>>>>
>>>> -     blk_queue_logical_block_size(md->queue.queue, 512);
>>>> +     if (mmc_card_mmc(card))
>>>> +             blk_queue_logical_block_size(md->queue.queue,
>>>> +                     card->ext_csd.data_sector_size);
>>>> +     else
>>>> +             blk_queue_logical_block_size(md->queue.queue, 512);
>>>>       set_capacity(md->disk, size);
>>>>
>>>>       if (mmc_host_cmd23(card->host)) {
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index 7268c26..11444c6 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -516,6 +516,8 @@ static int mmc_read_ext_csd(struct mmc_card *card,
>>>> u8
>>>> *ext_csd)
>>>>               } else {
>>>>                       card->ext_csd.data_tag_unit_size = 0;
>>>>               }
>>>> +     } else {
>>>> +             card->ext_csd.data_sector_size = 512;
>>>>       }
>>>>
>>>>  out:
>>>> --
>>>> 1.7.4.3
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>

Re: [PATCH 1/2] [MMC-4.5] Disable emulation

[Saugata Das]

On 17 May 2012 10:17, Saugata Das <saugata.das@linaro.org> wrote:
> On 17 May 2012 09:04, Namjae Jeon <linkinjeon@gmail.com> wrote:
>> 2012/5/17, Saugata Das <saugata.das@linaro.org>:
>>> On 17 May 2012 06:35, Namjae Jeon <linkinjeon@gmail.com> wrote:
>>>> 2012/5/17, Saugata Das <saugata.das@stericsson.com>:
>>>>> From: Saugata Das <saugata.das@linaro.org>
>>>>>
>>>>> This patch adds the support for large sector size of 4KB by disabling
>>>>> emulation.
>>>>> This patch passes eMMC DATA_SECTOR_SIZE as the logical block size during
>>>>> mmc_blk_alloc_req.
>>>>>
>>>>> In order to use this patch for 4KB sector size, ensure that
>>>>> USE_NATIVE_SECTOR
>>>>> is enabled, partition table is 4KB sector size aligned and file system
>>>>> block
>>>>> size is 4KB.
>>>> If you change native 4K from 512B emulation, filesystem sector size is
>>>> also 4KB align as well block size.
>>>> because default block size of most of filesystem is 4KB, but
>>>> filesystem sector size is 512B,
>>>> So filesystem corruption can be occured on native 4K device.
>>>>>
>>>
>>> I have tested ext4 with 4KB block size and I did not see an instance
>>> where it requested 512B.
>> -> yes, you can not see it, because when formating parittion, sector
>> size is set to 4K by I/O topology. As you know, several filesystems
>> can be used on eMMC not only Ext4. You should mention filesystem
>> sector size align in changelog.
>
> I will add the remark that file system block and sector size should be
> 4KB multiple.
>
>>> For sure, if someone is using file system with 512B sector on eMMC
>>> then the USE_NATIVE_SECTOR should not be enabled.
>> -> it will be the reason about below my question.
>>>
>>> The idea behind the patch is, MMC driver should notify the sector size
>>> information in a generic manner to file system. So, we depend here on
>>> ext_csd.data_sector_size, which should work in both 512B and 4KB case.
>>>
>>>
>>>>> Signed-off-by: Saugata Das <saugata.das@linaro.org>
>>>>> ---
>>>>>  drivers/mmc/card/block.c |   16 ++++++++++++++--
>>>>>  drivers/mmc/core/mmc.c   |    2 ++
>>>>>  2 files changed, 16 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>> index 91cda75..b4d0eb1 100644
>>>>> --- a/drivers/mmc/card/block.c
>>>>> +++ b/drivers/mmc/card/block.c
>>>>> @@ -1284,7 +1284,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>>>>> *mq,
>>>>> struct request *rqc)
>>>>>       int ret = 1, disable_multi = 0, retry = 0, type;
>>>>>       enum mmc_blk_status status;
>>>>>       struct mmc_queue_req *mq_rq;
>>>>> -     struct request *req;
>>>>> +     struct request *req = rqc;
>>>>>       struct mmc_async_req *areq;
>>>>>
>>>>>       if (!rqc && !mq->mqrq_prev->req)
>>>>> @@ -1292,6 +1292,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>>>>> *mq,
>>>>> struct request *rqc)
>>>>>
>>>>>       do {
>>>>>               if (rqc) {
>>>>> +
>>>>> +                     /*
>>>>> +                       * When 4KB native sector is enabled, single
>>>>> block
>>>>> +                       * read or write is not allowed
>>>>> +                       */
>>>>> +                     if ((brq->data.blocks == 1) &&
>>>>> +                             (card->ext_csd.data_sector_size == 4096))
>>>>> +                             goto cmd_abort;
>>>>                                               Why did you add this
>>>> code in any case ?
>>>>
>>>
>>> This was pointed during the previous review that with 4KB sector mode,
>>> 512B transfer is not allowed. So, this check has been added here.
>> -> I think that you should add error log to inform user error case.
>
> I will add pr_err here. I will also modify the condition
> (brq->data.blocks == 1)  to
> (brq->data.blocks % 4)
>

Oops. It will be (brq->data.blocks % 8)

>
>>>
>>>>>                       mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>>>>>                       areq = &mq->mqrq_cur->mmc_active;
>>>>>               } else
>>>>> @@ -1539,7 +1547,11 @@ static struct mmc_blk_data
>>>>> *mmc_blk_alloc_req(struct
>>>>> mmc_card *card,
>>>>>       snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
>>>>>                "mmcblk%d%s", md->name_idx, subname ? subname : "");
>>>>>
>>>>> -     blk_queue_logical_block_size(md->queue.queue, 512);
>>>>> +     if (mmc_card_mmc(card))
>>>>> +             blk_queue_logical_block_size(md->queue.queue,
>>>>> +                     card->ext_csd.data_sector_size);
>>>>> +     else
>>>>> +             blk_queue_logical_block_size(md->queue.queue, 512);
>>>>>       set_capacity(md->disk, size);
>>>>>
>>>>>       if (mmc_host_cmd23(card->host)) {
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index 7268c26..11444c6 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -516,6 +516,8 @@ static int mmc_read_ext_csd(struct mmc_card *card,
>>>>> u8
>>>>> *ext_csd)
>>>>>               } else {
>>>>>                       card->ext_csd.data_tag_unit_size = 0;
>>>>>               }
>>>>> +     } else {
>>>>> +             card->ext_csd.data_sector_size = 512;
>>>>>       }
>>>>>
>>>>>  out:
>>>>> --
>>>>> 1.7.4.3
>>>>>
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>
>>>

Re: [PATCH 1/2] [MMC-4.5] Disable emulation

[Arnd Bergmann]

On Thursday 17 May 2012, Namjae Jeon wrote:
> 2012/5/17, Saugata Das <saugata.das@linaro.org>:
> > On 17 May 2012 06:35, Namjae Jeon <linkinjeon@gmail.com> wrote:
> >> 2012/5/17, Saugata Das <saugata.das@stericsson.com>:
> >> If you change native 4K from 512B emulation, filesystem sector size is
> >> also 4KB align as well block size.
> >> because default block size of most of filesystem is 4KB, but
> >> filesystem sector size is 512B,
> >> So filesystem corruption can be occured on native 4K device.
> >>>
> >
> > I have tested ext4 with 4KB block size and I did not see an instance
> > where it requested 512B.
> -> yes, you can not see it, because when formating parittion, sector
> size is set to 4K by I/O topology. As you know, several filesystems
> can be used on eMMC not only Ext4. You should mention filesystem
> sector size align in changelog.

It's important to note here that if someone tries to use a block size
of less than 4KB on eMMC, or has a misaligned partition, that is already
a mistake and causes increased wear and reduced performance. Forcing 4KB
alignment is defintely a good idea and I would recommend setting the
'disable emulation' flag on all hardware that allows it just for this
reason.

Also note that hard drives and SSDs are also moving to 4KB hard sector
size and other block devices (DVD-RAM, DASD, ...) have only supported
larger sectors since the start and can be used with most file systems.

	Arnd