Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <5e5f2e45d0a14a55a8b7a9357846114b@hyperstone.com> <7c4757cc707740e580c61c39f963a04d@hyperstone.com>
In-Reply-To: <7c4757cc707740e580c61c39f963a04d@hyperstone.com>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Fri, 4 Feb 2022 10:47:47 +0100
Message-ID: <CAPDyKFr0YXCwL-8F9M7mkpNzSQpzw6gNUq2zaiJEXj1jNxUbrg@mail.gmail.com>
Subject: Re: [PATCH] mmc: block: fix read single on recovery logic
To:     =?UTF-8?Q?Christian_L=C3=B6hle?= <CLoehle@hyperstone.com>
Cc:     "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Avri Altman <avri.altman@wdc.com>,
        Adrian Hunter <adrian.hunter@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

+ Adrian

On Thu, 3 Feb 2022 at 11:09, Christian L=C3=B6hle <CLoehle@hyperstone.com> =
wrote:
>
> So could anyone take a long at this so far?
>

Thanks for pinging. Apologize for the delay, it's on top of my "to-review" =
list.

I have added Adrian too, who knows this code very well too.

Kind regards
Uffe

>
>
> From: Christian L=C3=B6hle
> Sent: Wednesday, January 5, 2022 5:43 PM
> To: ulf.hansson@linaro.org; Christian L=C3=B6hle; linux-mmc@vger.kernel.o=
rg; linux-kernel@vger.kernel.org
> Cc: Avri Altman
> Subject: [PATCH] mmc: block: fix read single on recovery logic
>
> On reads with MMC_READ_MULTIPLE_BLOCK that fail,
> the recovery handler will use MMC_READ_SINGLE_BLOCK for
> each of the blocks, up to MMC_READ_SINGLE_RETRIES times each.
> The logic for this is fixed to never report unsuccessful reads
> as success to the block layer.
>
> On command error with retries remaining, blk_update_request was
> called with whatever value error was set last to.
> In case it was last set to BLK_STS_OK (default), the read will be
> reported as success, even though there was no data read from the device.
> This could happen on a CRC mismatch for the response,
> a card rejecting the command (e.g. again due to a CRC mismatch).
> In case it was last set to BLK_STS_IOERR, the error is reported correctly=
,
> but no retries will be attempted.
>
> The patch now will count both command and data errors as retries and
> send BLK_STS_IOERR if there are no retries remaining,
> or BLK_STS_OK if the single read was successful in the meantime.
>
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> ---
>  drivers/mmc/core/block.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 90e1bcd03b46..d7d880ce0f8a 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -1682,31 +1682,31 @@ static void mmc_blk_read_single(struct mmc_queue =
*mq, struct request *req)
>          struct mmc_card *card =3D mq->card;
>          struct mmc_host *host =3D card->host;
>          blk_status_t error =3D BLK_STS_OK;
> -       int retries =3D 0;
>
>          do {
>                  u32 status;
>                  int err;
> +               int retries =3D 0;
>
> -               mmc_blk_rw_rq_prep(mqrq, card, 1, mq);
> +               while (retries++ < MMC_READ_SINGLE_RETRIES) {
> +                       mmc_blk_rw_rq_prep(mqrq, card, 1, mq);
>
> -               mmc_wait_for_req(host, mrq);
> +                       mmc_wait_for_req(host, mrq);
>
> -               err =3D mmc_send_status(card, &status);
> -               if (err)
> -                       goto error_exit;
> -
> -               if (!mmc_host_is_spi(host) &&
> -                   !mmc_ready_for_data(status)) {
> -                       err =3D mmc_blk_fix_state(card, req);
> +                       err =3D mmc_send_status(card, &status);
>                          if (err)
>                                  goto error_exit;
> -               }
>
> -               if (mrq->cmd->error && retries++ < MMC_READ_SINGLE_RETRIE=
S)
> -                       continue;
> +                       if (!mmc_host_is_spi(host) &&
> +                           !mmc_ready_for_data(status)) {
> +                               err =3D mmc_blk_fix_state(card, req);
> +                               if (err)
> +                                       goto error_exit;
> +                       }
>
> -               retries =3D 0;
> +                       if (!mrq->cmd->error && !mrq->data->error)
> +                               break;
> +               }
>
>                  if (mrq->cmd->error ||
>                      mrq->data->error ||
> --
> 2.34.1
>     =3D
> Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
> Managing Director: Dr. Jan Peter Berns.
> Commercial register of local courts: Freiburg HRB381782
>