diff mbox series

[v2] mmc: block: avoid multiblock reads for the last sector in SPI mode

Message ID 20181008150730.8638-1-peron.clem@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2] mmc: block: avoid multiblock reads for the last sector in SPI mode | expand

Commit Message

Clément Péron Oct. 8, 2018, 3:07 p.m. UTC 
From: Chris Boot <bootc@bootc.net>

On some SD cards over SPI, reading with the multiblock read command the last
sector will leave the card in a bad state.

Remove last sectors from the multiblock reading cmd.

Signed-off-by: Chris Boot <bootc@bootc.net>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---

 v2:
        - Change commit log
        - Indent properly

 drivers/mmc/core/block.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Avri Altman Oct. 9, 2018, 7:18 a.m. UTC | #1 
> @@ -1370,6 +1370,16 @@ static void mmc_blk_data_prep(struct mmc_queue
> *mq, struct mmc_queue_req *mqrq,
>  		brq->data.blocks = card->host->max_blk_count;
> 
>  	if (brq->data.blocks > 1) {
> +		/*
> +		 * Some SD cards in SPI mode return a CRC error or even lock
> up
> +		 * completely when trying to read the last block using a
> +		 * multiblock read command.
> +		 */
> +		if (mmc_host_is_spi(card->host) && (rq_data_dir(req) ==
> READ) &&
> +		    (blk_rq_pos(req) + blk_rq_sectors(req) ==
> +		     get_capacity(md->disk)))
> +			brq->data.blocks--;
> +
data.blocks is being assigned about 4 times in this function.
Maybe do all that in a small wrapper, which might do that more effectively,
And document its logic?

Thanks,
Avri
Ulf Hansson Oct. 9, 2018, 7:30 a.m. UTC | #2 
On 9 October 2018 at 09:18, Avri Altman <Avri.Altman@wdc.com> wrote:
>> @@ -1370,6 +1370,16 @@ static void mmc_blk_data_prep(struct mmc_queue
>> *mq, struct mmc_queue_req *mqrq,
>>               brq->data.blocks = card->host->max_blk_count;
>>
>>       if (brq->data.blocks > 1) {
>> +             /*
>> +              * Some SD cards in SPI mode return a CRC error or even lock
>> up
>> +              * completely when trying to read the last block using a
>> +              * multiblock read command.
>> +              */
>> +             if (mmc_host_is_spi(card->host) && (rq_data_dir(req) ==
>> READ) &&
>> +                 (blk_rq_pos(req) + blk_rq_sectors(req) ==
>> +                  get_capacity(md->disk)))
>> +                     brq->data.blocks--;
>> +
> data.blocks is being assigned about 4 times in this function.
> Maybe do all that in a small wrapper, which might do that more effectively,
> And document its logic?

That seems to makes sense, however as a fix this is better (less
changes). Let's do the cleanup on top.

I have applied this for fixes and added a stable tag, thanks!

Kind regards
Uffe
diff mbox series

Patch

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index a0b9102c4c6e..e201ccb3fda4 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1370,6 +1370,16 @@  static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
 		brq->data.blocks = card->host->max_blk_count;
 
 	if (brq->data.blocks > 1) {
+		/*
+		 * Some SD cards in SPI mode return a CRC error or even lock up
+		 * completely when trying to read the last block using a
+		 * multiblock read command.
+		 */
+		if (mmc_host_is_spi(card->host) && (rq_data_dir(req) == READ) &&
+		    (blk_rq_pos(req) + blk_rq_sectors(req) ==
+		     get_capacity(md->disk)))
+			brq->data.blocks--;
+
 		/*
 		 * After a read error, we redo the request one sector
 		 * at a time in order to accurately determine which