diff mbox

[1/2,MMC-4.5] Disable emulation

Message ID 1337185615-17202-1-git-send-email-saugata.das@stericsson.com
State New
Headers show

Commit Message

Saugata Das May 16, 2012, 4:26 p.m. UTC
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(-)

Comments

Namjae Jeon May 17, 2012, 1:05 a.m. UTC | #1
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
>
Saugata Das May 17, 2012, 3:12 a.m. UTC | #2
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
>>
Namjae Jeon May 17, 2012, 3:34 a.m. UTC | #3
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
>>>
>
Saugata Das May 17, 2012, 4:47 a.m. UTC | #4
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
>>>>
>>
Saugata Das May 17, 2012, 5:05 a.m. UTC | #5
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
>>>>>
>>>
Arnd Bergmann May 17, 2012, 7:07 a.m. UTC | #6
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
diff mbox

Patch

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: