LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
To: Sasha Levin <sashal@kernel.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	stable@vger.kernel.org, Doug Anderson <dianders@chromium.org>,
	Enric Balletbo i Serra <enric.balletbo@collabora.com>,
	Alim Akhtar <alim.akhtar@gmail.com>,
	Jaehoon Chung <jh80.chung@samsung.com>,
	Ulf Hansson <ulf.hansson@linaro.org>
Subject: Re: [PATCH 4.4 25/31] mmc: dw_mmc: Wait for data transfer after response errors.
Date: Thu, 26 Aug 2021 20:59:53 +0900	[thread overview]
Message-ID: <CABMQnVJrxqB8koLO9-mBCZgRyQydU7x7B8aHgRPjpxw92hBWjQ@mail.gmail.com> (raw)
In-Reply-To: <20210824170743.710957-26-sashal@kernel.org>

Hi,


2021年8月25日(水) 2:39 Sasha Levin <sashal@kernel.org>:
>
> From: Doug Anderson <dianders@chromium.org>
>
> [ Upstream commit 46d179525a1f6d16957dcb4624517bc04142b3e7 ]
>
> According to the DesignWare state machine description, after we get a
> "response error" or "response CRC error" we move into data transfer
> mode. That means that we don't necessarily need to special case
> trying to deal with the failure right away. We can wait until we are
> notified that the data transfer is complete (with or without errors)
> and then we can deal with the failure.
>
> It may sound strange to defer dealing with a command that we know will
> fail anyway, but this appears to fix a bug. During tuning (CMD19) on
> a specific card on an rk3288-based system, we found that we could get
> a "response CRC error". Sending the stop command after the "response
> CRC error" would then throw the system into a confused state causing
> all future tuning phases to report failure.
>
> When in the confused state, the controller would show these (hex codes
> are interrupt status register):
>  CMD ERR: 0x00000046 (cmd=19)
>  CMD ERR: 0x0000004e (cmd=12)
>  DATA ERR: 0x00000208
>  DATA ERR: 0x0000020c
>  CMD ERR: 0x00000104 (cmd=19)
>  CMD ERR: 0x00000104 (cmd=12)
>  DATA ERR: 0x00000208
>  DATA ERR: 0x0000020c
>  ...
>  ...
>
> It is inherently difficult to deal with the complexity of trying to
> correctly send a stop command while a data transfer is taking place
> since you need to deal with different corner cases caused by the fact
> that the data transfer could complete (with errors or without errors)
> during various places in sending the stop command (dw_mci_stop_dma,
> send_stop_abort, etc)
>
> Instead of adding a bunch of extra complexity to deal with this, it
> seems much simpler to just use the more straightforward (and less
> error-prone) path of letting the data transfer finish. There
> shouldn't be any huge benefit to sending the stop command slightly
> earlier, anyway.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Cc: Alim Akhtar <alim.akhtar@gmail.com>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

This commit also requires the following modifications:
  ba2d139b02ba68: mmc: dw_mmc: Fix occasional hang after tuning on eMMC

Please apply this commit too.

Best regards,
  Nobuhiro


> ---
>  drivers/mmc/host/dw_mmc.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 581f5d0271f4..afdf539e06e9 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1744,6 +1744,33 @@ static void dw_mci_tasklet_func(unsigned long priv)
>                         }
>
>                         if (cmd->data && err) {
> +                               /*
> +                                * During UHS tuning sequence, sending the stop
> +                                * command after the response CRC error would
> +                                * throw the system into a confused state
> +                                * causing all future tuning phases to report
> +                                * failure.
> +                                *
> +                                * In such case controller will move into a data
> +                                * transfer state after a response error or
> +                                * response CRC error. Let's let that finish
> +                                * before trying to send a stop, so we'll go to
> +                                * STATE_SENDING_DATA.
> +                                *
> +                                * Although letting the data transfer take place
> +                                * will waste a bit of time (we already know
> +                                * the command was bad), it can't cause any
> +                                * errors since it's possible it would have
> +                                * taken place anyway if this tasklet got
> +                                * delayed. Allowing the transfer to take place
> +                                * avoids races and keeps things simple.
> +                                */
> +                               if ((err != -ETIMEDOUT) &&
> +                                   (cmd->opcode == MMC_SEND_TUNING_BLOCK)) {
> +                                       state = STATE_SENDING_DATA;
> +                                       continue;
> +                               }
> +
>                                 dw_mci_stop_dma(host);
>                                 send_stop_abort(host, data);
>                                 state = STATE_SENDING_STOP;
> --
> 2.30.2
>


-- 
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6

  reply	other threads:[~2021-08-26 12:00 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 17:07 [PATCH 4.4 00/31] 4.4.282-rc1 review Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 01/31] ASoC: intel: atom: Fix reference to PCM buffer address Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 02/31] i2c: dev: zero out array used for i2c reads from userspace Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 03/31] net: Fix memory leak in ieee802154_raw_deliver Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 04/31] xen/events: Fix race in set_evtchn_to_irq Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 05/31] x86/tools: Fix objdump version check again Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 06/31] PCI/MSI: Enable and mask MSI-X early Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 07/31] PCI/MSI: Do not set invalid bits in MSI mask Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 08/31] PCI/MSI: Correct misleading comments Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 09/31] PCI/MSI: Use msi_mask_irq() in pci_msi_shutdown() Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 10/31] PCI/MSI: Protect msi_desc::masked for multi-MSI Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 11/31] PCI/MSI: Mask all unused MSI-X entries Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 12/31] PCI/MSI: Enforce that MSI-X table entry is masked for update Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 13/31] PCI/MSI: Enforce MSI[X] entry updates to be visible Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 14/31] vmlinux.lds.h: Handle clang's module.{c,d}tor sections Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 15/31] KVM: nSVM: avoid picking up unsupported bits from L2 in int_ctl (CVE-2021-3653) Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 16/31] dmaengine: usb-dmac: Fix PM reference leak in usb_dmac_probe() Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 17/31] ARM: dts: am43x-epos-evm: Reduce i2c0 bus speed for tps65218 Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 18/31] dmaengine: of-dma: router_xlate to return -EPROBE_DEFER if controller is not yet available Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 19/31] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry() Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 20/31] scsi: core: Avoid printing an error if target_alloc() returns -ENXIO Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 21/31] Bluetooth: hidp: use correct wait queue when removing ctrl_wait Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 22/31] dccp: add do-while-0 stubs for dccp_pr_debug macros Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 23/31] net: 6pack: fix slab-out-of-bounds in decode_data Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 24/31] net: qlcnic: add missed unlock in qlcnic_83xx_flash_read32 Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 25/31] mmc: dw_mmc: Wait for data transfer after response errors Sasha Levin
2021-08-26 11:59   ` Nobuhiro Iwamatsu [this message]
2021-08-26 12:31     ` Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 26/31] mmc: dw_mmc: call the dw_mci_prep_stop_abort() by default Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 27/31] mmc: dw_mmc: Fix hang on data CRC error Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 28/31] ALSA: hda - fix the 'Capture Switch' value change notifications Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 29/31] ipack: tpci200: fix many double free issues in tpci200_pci_probe Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 30/31] ASoC: intel: atom: Fix breakage for PCM buffer address setup Sasha Levin
2021-08-24 17:07 ` [PATCH 4.4 31/31] Linux 4.4.282-rc1 Sasha Levin
2021-08-25  7:33 ` [PATCH 4.4 00/31] 4.4.282-rc1 review Pavel Machek
2021-08-25 20:27 ` Guenter Roeck
2021-08-25 21:18 ` Daniel Díaz
2021-08-25 22:38 ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABMQnVJrxqB8koLO9-mBCZgRyQydU7x7B8aHgRPjpxw92hBWjQ@mail.gmail.com \
    --to=iwamatsu@nigauri.org \
    --cc=alim.akhtar@gmail.com \
    --cc=dianders@chromium.org \
    --cc=enric.balletbo@collabora.com \
    --cc=jh80.chung@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=ulf.hansson@linaro.org \
    --subject='Re: [PATCH 4.4 25/31] mmc: dw_mmc: Wait for data transfer after response errors.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).