| Message ID | 20220926094906.14537-3-pshete@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/4] mmc: sdhci-tegra: Separate Tegra194 and Tegra234 SoC data | expand |
On 26/09/22 12:49, Prathamesh Shete wrote: > In case of error condition to avoid system crash > Tegra SDMMC controller requires CMD and DAT resets > issued together. SDHCI controller FSM goes into > bad state due to rapid SD card hot-plug event. > Issuing reset on the CMD FSM before DATA FSM results > in kernel panic, hence add support to issue CMD and > DAT resets together. > This is applicable to Tegra186 and later chips. > > Signed-off-by: Aniruddha TVS Rao <anrao@nvidia.com> > Signed-off-by: Prathamesh Shete <pshete@nvidia.com> > --- > drivers/mmc/host/sdhci-tegra.c | 3 ++- > drivers/mmc/host/sdhci.c | 20 +++++++++++++++----- > drivers/mmc/host/sdhci.h | 2 ++ > 3 files changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c > index 4d32b5bfc424..58449e010a9b 100644 > --- a/drivers/mmc/host/sdhci-tegra.c > +++ b/drivers/mmc/host/sdhci-tegra.c > @@ -1532,7 +1532,8 @@ static const struct sdhci_pltfm_data sdhci_tegra186_pdata = { > SDHCI_QUIRK_NO_HISPD_BIT | > SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC | > SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN, > - .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN, > + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN | > + SDHCI_QUIRK2_ISSUE_CMD_DAT_RESET_TOGETHER, > .ops = &tegra186_sdhci_ops, > }; > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 7689ffec5ad1..2f4a0e84fee8 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -3060,12 +3060,22 @@ static bool sdhci_request_done(struct sdhci_host *host) > host->ops->set_clock(host, host->clock); > > /* > - * Spec says we should do both at the same time, but Ricoh > - * controllers do not like that. > + * While the specification says we should do both at the > + * same time, Ricoh controllers (and potentially others) do not > + * like that. On the other hand, some controllers (such as those > + * found on Tegra186 and later) rely on both being reset at the > + * same time. Use a quirk for the latter category since most > + * controllers seem to work fine with DAT and CMD getting reset > + * at the same time. > */ > - sdhci_do_reset(host, SDHCI_RESET_CMD); > - sdhci_do_reset(host, SDHCI_RESET_DATA); > - > + if (host->quirks2 & > + SDHCI_QUIRK2_ISSUE_CMD_DAT_RESET_TOGETHER) { > + sdhci_do_reset(host, SDHCI_RESET_CMD | > + SDHCI_RESET_DATA); > + } else { > + sdhci_do_reset(host, SDHCI_RESET_CMD); > + sdhci_do_reset(host, SDHCI_RESET_DATA); > + } There is a bit a tidy up of SDHCI resets here: https://lore.kernel.org/linux-mmc/20220926192022.85660-1-adrian.hunter@intel.com/ Would you mind implementing the quirk on top of that patch set. The quirk should go somewhere in the new sdhci_reset_for_reason() function, which should make it's use more consistent.
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index 4d32b5bfc424..58449e010a9b 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -1532,7 +1532,8 @@ static const struct sdhci_pltfm_data sdhci_tegra186_pdata = { SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC | SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN, - .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN, + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN | + SDHCI_QUIRK2_ISSUE_CMD_DAT_RESET_TOGETHER, .ops = &tegra186_sdhci_ops, }; diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 7689ffec5ad1..2f4a0e84fee8 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -3060,12 +3060,22 @@ static bool sdhci_request_done(struct sdhci_host *host) host->ops->set_clock(host, host->clock); /* - * Spec says we should do both at the same time, but Ricoh - * controllers do not like that. + * While the specification says we should do both at the + * same time, Ricoh controllers (and potentially others) do not + * like that. On the other hand, some controllers (such as those + * found on Tegra186 and later) rely on both being reset at the + * same time. Use a quirk for the latter category since most + * controllers seem to work fine with DAT and CMD getting reset + * at the same time. */ - sdhci_do_reset(host, SDHCI_RESET_CMD); - sdhci_do_reset(host, SDHCI_RESET_DATA); - + if (host->quirks2 & + SDHCI_QUIRK2_ISSUE_CMD_DAT_RESET_TOGETHER) { + sdhci_do_reset(host, SDHCI_RESET_CMD | + SDHCI_RESET_DATA); + } else { + sdhci_do_reset(host, SDHCI_RESET_CMD); + sdhci_do_reset(host, SDHCI_RESET_DATA); + } host->pending_reset = false; } diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 95a08f09df30..8045308f7859 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -480,6 +480,8 @@ struct sdhci_host { * block count. */ #define SDHCI_QUIRK2_USE_32BIT_BLK_CNT (1<<18) +/* Issue CMD and DATA reset together */ +#define SDHCI_QUIRK2_ISSUE_CMD_DAT_RESET_TOGETHER (1<<19) int irq; /* Device IRQ */ void __iomem *ioaddr; /* Mapped address */