* Re: [PATCH] mmc: block: Add single read for 4k sector cards
@ 2022-06-24 13:58 Christian Löhle
0 siblings, 0 replies; 4+ messages in thread
From: Christian Löhle @ 2022-06-24 13:58 UTC (permalink / raw)
To: ulf.hansson, linux-mmc, linux-kernel; +Cc: Avri Altman, adrian.hunter
Anyone wanting to test this and does not have a way to disturb eMMC signals,
use mmc-utils to disable 512B emulation and something along the lines of:
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 4b70cbfc6d5d..bc0b9241a00f 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -32,6 +32,8 @@
#include <linux/mmc/sd.h>
#include <linux/mmc/slot-gpio.h>
+#include <linux/sched/clock.h>
+
#define CREATE_TRACE_POINTS
#include <trace/events/mmc.h>
@@ -141,6 +143,12 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
struct mmc_command *cmd = mrq->cmd;
int err = cmd->error;
+ if (mrq->data && !mrq->data->error && mrq->data->flags & MMC_DATA_READ)
+ if ((sched_clock() % 1000) == 0) {
+ mrq->data->bytes_xfered = 0;
+ mrq->data->error = -EILSEQ;
+ }
+
/* Flag re-tuning needed on CRC errors */
if (cmd->opcode != MMC_SEND_TUNING_BLOCK &&
cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200 &&
Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mmc: block: Add single read for 4k sector cards
@ 2022-06-24 14:32 Christian Löhle
0 siblings, 0 replies; 4+ messages in thread
From: Christian Löhle @ 2022-06-24 14:32 UTC (permalink / raw)
To: Avri Altman, ulf.hansson, linux-mmc, linux-kernel; +Cc: adrian.hunter
>Hi,
>
>> Cards with 4k native sector size may only be read 4k-aligned,
>> accommodate for this in the single read recovery and use it.
>
>Fixes: 2bf22b39823c (mmc: core: add workaround for controllers with broken multiblock reads)
If a Fixes tag is appropriate it should be mmc: block: Add blk-mq support, right?
(The commit introducing the FIXME)
>>
>> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
>> ---
>> drivers/mmc/core/block.c | 26 ++++++++++++++------------
>> 1 file changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
>> index f4a1281658db..6f0b24cb0747 100644
>> --- a/drivers/mmc/core/block.c
>> +++ b/drivers/mmc/core/block.c
>> @@ -176,7 +176,7 @@ static inline int mmc_blk_part_switch(struct
>> mmc_card *card,
>> unsigned int part_type);
>> static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>> struct mmc_card *card,
>> - int disable_multi,
>> + int recovery_mode,
>> struct mmc_queue *mq);
>> static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
>>
>> @@ -1302,7 +1302,7 @@ static void mmc_blk_eval_resp_error(struct
>> mmc_blk_request *brq)
>> }
>>
>> static void mmc_blk_data_prep(struct mmc_queue *mq, struct
>> mmc_queue_req *mqrq,
>> - int disable_multi, bool *do_rel_wr_p,
>> + int recovery_mode, bool *do_rel_wr_p,
>> bool *do_data_tag_p)
>> {
>> struct mmc_blk_data *md = mq->blkdata;
>> @@ -1372,8 +1372,11 @@ static void mmc_blk_data_prep(struct
>> mmc_queue *mq, struct mmc_queue_req *mqrq,
>> * at a time in order to accurately determine which
>> * sectors can be read successfully.
>> */
>> - if (disable_multi)
>> + if (recovery_mode) {
>> brq->data.blocks = 1;
>> + if (mmc_large_sector(card))
>> + brq->data.blocks = 8;
>Maybe a ternary operator here?
>
Sure!
>> + }
>>
>> /*
>> * Some controllers have HW issues while operating
>> @@ -1590,7 +1593,7 @@ static int mmc_blk_cqe_issue_rw_rq(struct
>> mmc_queue *mq, struct request *req)
>>
>> static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>> struct mmc_card *card,
>> - int disable_multi,
>> + int recovery_mode,
>> struct mmc_queue *mq)
>> {
>> u32 readcmd, writecmd;
>> @@ -1599,7 +1602,7 @@ static void mmc_blk_rw_rq_prep(struct
>> mmc_queue_req *mqrq,
>> struct mmc_blk_data *md = mq->blkdata;
>> bool do_rel_wr, do_data_tag;
>>
>> - mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr,
>> &do_data_tag);
>> + mmc_blk_data_prep(mq, mqrq, recovery_mode, &do_rel_wr,
>> &do_data_tag);
>>
>> brq->mrq.cmd = &brq->cmd;
>>
>> @@ -1690,7 +1693,7 @@ static int mmc_blk_fix_state(struct mmc_card
>> *card, struct request *req)
>>
>> #define MMC_READ_SINGLE_RETRIES 2
>>
>> -/* Single sector read during recovery */
>> +/* Single (native) sector read during recovery */
>> static void mmc_blk_read_single(struct mmc_queue *mq, struct request
>> *req)
>> {
>> struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
>> @@ -1698,6 +1701,7 @@ static void mmc_blk_read_single(struct
>> mmc_queue *mq, struct request *req)
>> struct mmc_card *card = mq->card;
>> struct mmc_host *host = card->host;
>> blk_status_t error = BLK_STS_OK;
>> + size_t bytes_per_read = mmc_large_sector(card) ? 4069 : 512;
>Typo: 4069 -> 4096
Oh wow, thanks, not sure how I missed that, in that case it was not just sloppy testing,
but a manual rewrite from the kernel source for the board I currently have an eMMC with 4k native sector size on.
(So also sloppy testing)
>
>Thanks,
>Avri
>>
>> do {
>> u32 status;
>> @@ -1732,13 +1736,13 @@ static void mmc_blk_read_single(struct
>> mmc_queue *mq, struct request *req)
>> else
>> error = BLK_STS_OK;
>>
>> - } while (blk_update_request(req, error, 512));
>> + } while (blk_update_request(req, error, bytes_per_read));
>>
>> return;
>>
>> error_exit:
>> mrq->data->bytes_xfered = 0;
>> - blk_update_request(req, BLK_STS_IOERR, 512);
>> + blk_update_request(req, BLK_STS_IOERR, bytes_per_read);
>> /* Let it try the remaining request again */
>> if (mqrq->retries > MMC_MAX_RETRIES - 1)
>> mqrq->retries = MMC_MAX_RETRIES - 1;
>> @@ -1879,10 +1883,8 @@ static void mmc_blk_mq_rw_recovery(struct
>> mmc_queue *mq, struct request *req)
>> return;
>> }
>>
>> - /* FIXME: Missing single sector read for large sector size */
>> - if (!mmc_large_sector(card) && rq_data_dir(req) == READ &&
>> - brq->data.blocks > 1) {
>> - /* Read one sector at a time */
>> + if (rq_data_dir(req) == READ && brq->data.blocks > 1) {
>> + /* Read one (native) sector at a time */
>> mmc_blk_read_single(mq, req);
>> return;
>> }
>> --
>> 2.36.1
>> Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
>> Managing Director: Dr. Jan Peter Berns.
>> Commercial register of local courts: Freiburg HRB381782
>
>
Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] mmc: block: Add single read for 4k sector cards
2022-06-24 12:40 Christian Löhle
@ 2022-06-24 14:14 ` Avri Altman
0 siblings, 0 replies; 4+ messages in thread
From: Avri Altman @ 2022-06-24 14:14 UTC (permalink / raw)
To: Christian Löhle, ulf.hansson, linux-mmc, linux-kernel; +Cc: adrian.hunter
Hi,
> Cards with 4k native sector size may only be read 4k-aligned,
> accommodate for this in the single read recovery and use it.
Fixes: 2bf22b39823c (mmc: core: add workaround for controllers with broken multiblock reads)
>
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> ---
> drivers/mmc/core/block.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index f4a1281658db..6f0b24cb0747 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -176,7 +176,7 @@ static inline int mmc_blk_part_switch(struct
> mmc_card *card,
> unsigned int part_type);
> static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_card *card,
> - int disable_multi,
> + int recovery_mode,
> struct mmc_queue *mq);
> static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
>
> @@ -1302,7 +1302,7 @@ static void mmc_blk_eval_resp_error(struct
> mmc_blk_request *brq)
> }
>
> static void mmc_blk_data_prep(struct mmc_queue *mq, struct
> mmc_queue_req *mqrq,
> - int disable_multi, bool *do_rel_wr_p,
> + int recovery_mode, bool *do_rel_wr_p,
> bool *do_data_tag_p)
> {
> struct mmc_blk_data *md = mq->blkdata;
> @@ -1372,8 +1372,11 @@ static void mmc_blk_data_prep(struct
> mmc_queue *mq, struct mmc_queue_req *mqrq,
> * at a time in order to accurately determine which
> * sectors can be read successfully.
> */
> - if (disable_multi)
> + if (recovery_mode) {
> brq->data.blocks = 1;
> + if (mmc_large_sector(card))
> + brq->data.blocks = 8;
Maybe a ternary operator here?
> + }
>
> /*
> * Some controllers have HW issues while operating
> @@ -1590,7 +1593,7 @@ static int mmc_blk_cqe_issue_rw_rq(struct
> mmc_queue *mq, struct request *req)
>
> static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_card *card,
> - int disable_multi,
> + int recovery_mode,
> struct mmc_queue *mq)
> {
> u32 readcmd, writecmd;
> @@ -1599,7 +1602,7 @@ static void mmc_blk_rw_rq_prep(struct
> mmc_queue_req *mqrq,
> struct mmc_blk_data *md = mq->blkdata;
> bool do_rel_wr, do_data_tag;
>
> - mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr,
> &do_data_tag);
> + mmc_blk_data_prep(mq, mqrq, recovery_mode, &do_rel_wr,
> &do_data_tag);
>
> brq->mrq.cmd = &brq->cmd;
>
> @@ -1690,7 +1693,7 @@ static int mmc_blk_fix_state(struct mmc_card
> *card, struct request *req)
>
> #define MMC_READ_SINGLE_RETRIES 2
>
> -/* Single sector read during recovery */
> +/* Single (native) sector read during recovery */
> static void mmc_blk_read_single(struct mmc_queue *mq, struct request
> *req)
> {
> struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> @@ -1698,6 +1701,7 @@ static void mmc_blk_read_single(struct
> mmc_queue *mq, struct request *req)
> struct mmc_card *card = mq->card;
> struct mmc_host *host = card->host;
> blk_status_t error = BLK_STS_OK;
> + size_t bytes_per_read = mmc_large_sector(card) ? 4069 : 512;
Typo: 4069 -> 4096
Thanks,
Avri
>
> do {
> u32 status;
> @@ -1732,13 +1736,13 @@ static void mmc_blk_read_single(struct
> mmc_queue *mq, struct request *req)
> else
> error = BLK_STS_OK;
>
> - } while (blk_update_request(req, error, 512));
> + } while (blk_update_request(req, error, bytes_per_read));
>
> return;
>
> error_exit:
> mrq->data->bytes_xfered = 0;
> - blk_update_request(req, BLK_STS_IOERR, 512);
> + blk_update_request(req, BLK_STS_IOERR, bytes_per_read);
> /* Let it try the remaining request again */
> if (mqrq->retries > MMC_MAX_RETRIES - 1)
> mqrq->retries = MMC_MAX_RETRIES - 1;
> @@ -1879,10 +1883,8 @@ static void mmc_blk_mq_rw_recovery(struct
> mmc_queue *mq, struct request *req)
> return;
> }
>
> - /* FIXME: Missing single sector read for large sector size */
> - if (!mmc_large_sector(card) && rq_data_dir(req) == READ &&
> - brq->data.blocks > 1) {
> - /* Read one sector at a time */
> + if (rq_data_dir(req) == READ && brq->data.blocks > 1) {
> + /* Read one (native) sector at a time */
> mmc_blk_read_single(mq, req);
> return;
> }
> --
> 2.36.1
> Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
> Managing Director: Dr. Jan Peter Berns.
> Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] mmc: block: Add single read for 4k sector cards
@ 2022-06-24 12:40 Christian Löhle
2022-06-24 14:14 ` Avri Altman
0 siblings, 1 reply; 4+ messages in thread
From: Christian Löhle @ 2022-06-24 12:40 UTC (permalink / raw)
To: ulf.hansson, linux-mmc, linux-kernel, Christian Löhle
Cc: Avri Altman, adrian.hunter
Cards with 4k native sector size may only be read 4k-aligned,
accommodate for this in the single read recovery and use it.
Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
drivers/mmc/core/block.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index f4a1281658db..6f0b24cb0747 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -176,7 +176,7 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
unsigned int part_type);
static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
struct mmc_card *card,
- int disable_multi,
+ int recovery_mode,
struct mmc_queue *mq);
static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
@@ -1302,7 +1302,7 @@ static void mmc_blk_eval_resp_error(struct mmc_blk_request *brq)
}
static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
- int disable_multi, bool *do_rel_wr_p,
+ int recovery_mode, bool *do_rel_wr_p,
bool *do_data_tag_p)
{
struct mmc_blk_data *md = mq->blkdata;
@@ -1372,8 +1372,11 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
* at a time in order to accurately determine which
* sectors can be read successfully.
*/
- if (disable_multi)
+ if (recovery_mode) {
brq->data.blocks = 1;
+ if (mmc_large_sector(card))
+ brq->data.blocks = 8;
+ }
/*
* Some controllers have HW issues while operating
@@ -1590,7 +1593,7 @@ static int mmc_blk_cqe_issue_rw_rq(struct mmc_queue *mq, struct request *req)
static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
struct mmc_card *card,
- int disable_multi,
+ int recovery_mode,
struct mmc_queue *mq)
{
u32 readcmd, writecmd;
@@ -1599,7 +1602,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
struct mmc_blk_data *md = mq->blkdata;
bool do_rel_wr, do_data_tag;
- mmc_blk_data_prep(mq, mqrq, disable_multi, &do_rel_wr, &do_data_tag);
+ mmc_blk_data_prep(mq, mqrq, recovery_mode, &do_rel_wr, &do_data_tag);
brq->mrq.cmd = &brq->cmd;
@@ -1690,7 +1693,7 @@ static int mmc_blk_fix_state(struct mmc_card *card, struct request *req)
#define MMC_READ_SINGLE_RETRIES 2
-/* Single sector read during recovery */
+/* Single (native) sector read during recovery */
static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
{
struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
@@ -1698,6 +1701,7 @@ static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
struct mmc_card *card = mq->card;
struct mmc_host *host = card->host;
blk_status_t error = BLK_STS_OK;
+ size_t bytes_per_read = mmc_large_sector(card) ? 4069 : 512;
do {
u32 status;
@@ -1732,13 +1736,13 @@ static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
else
error = BLK_STS_OK;
- } while (blk_update_request(req, error, 512));
+ } while (blk_update_request(req, error, bytes_per_read));
return;
error_exit:
mrq->data->bytes_xfered = 0;
- blk_update_request(req, BLK_STS_IOERR, 512);
+ blk_update_request(req, BLK_STS_IOERR, bytes_per_read);
/* Let it try the remaining request again */
if (mqrq->retries > MMC_MAX_RETRIES - 1)
mqrq->retries = MMC_MAX_RETRIES - 1;
@@ -1879,10 +1883,8 @@ static void mmc_blk_mq_rw_recovery(struct mmc_queue *mq, struct request *req)
return;
}
- /* FIXME: Missing single sector read for large sector size */
- if (!mmc_large_sector(card) && rq_data_dir(req) == READ &&
- brq->data.blocks > 1) {
- /* Read one sector at a time */
+ if (rq_data_dir(req) == READ && brq->data.blocks > 1) {
+ /* Read one (native) sector at a time */
mmc_blk_read_single(mq, req);
return;
}
--
2.36.1
Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-24 14:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-24 13:58 [PATCH] mmc: block: Add single read for 4k sector cards Christian Löhle
-- strict thread matches above, loose matches on Subject: below --
2022-06-24 14:32 Christian Löhle
2022-06-24 12:40 Christian Löhle
2022-06-24 14:14 ` Avri Altman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.