On Wed, Sep 14, 2022 at 09:21:07PM +0300, Adrian Hunter wrote:
> On 14/09/22 15:20, Thierry Reding wrote:
> > On Wed, Sep 14, 2022 at 03:26:27PM +0530, Prathamesh Shete wrote:
> >> In case of error condition to avoid system crash
> >> Tegra SDMMC controller requires CMD and DAT resets
> >> issued together.
> >
> > It might be worth specifying exactly what "system crash" means. Does
> > this always happen (i.e. do we have a problem right now?) or are there
> > specific circumstances that cause the crash.
> >
> >> This is applicable to Tegra186 and later chips.
> >>
> >> Signed-off-by: Aniruddha TVS Rao <anrao@xxxxxxxxxx>
> >> Signed-off-by: Prathamesh Shete <pshete@xxxxxxxxxx>
> >> ---
> >> drivers/mmc/host/sdhci-tegra.c | 3 ++-
> >> drivers/mmc/host/sdhci.c | 11 ++++++++---
> >> drivers/mmc/host/sdhci.h | 2 ++
> >> 3 files changed, 12 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> >> index b66b0cc51497..7d16dc41fe91 100644
> >> --- a/drivers/mmc/host/sdhci-tegra.c
> >> +++ b/drivers/mmc/host/sdhci-tegra.c
> >> @@ -1530,7 +1530,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..289fa8ae4866 100644
> >> --- a/drivers/mmc/host/sdhci.c
> >> +++ b/drivers/mmc/host/sdhci.c
> >> @@ -3063,9 +3063,14 @@ static bool sdhci_request_done(struct sdhci_host *host)
> >> * Spec says we should do both at the same time, but Ricoh
> >> * controllers do not like that.
> >> */
> >
> > The comment above seems to indicate that the current behavior (i.e.
> > splitting the CMD and DATA resets) is actually the quirk, so I wonder if
> > this perhaps should be reversed? I suppose it could be difficult to
> > track down the exact controllers that need the separate resets, but this
> > might be worth doing. It's possible that other controllers might run
> > into the same issue that we are if they work strictly to the spec.
> >
> > Adrian, any ideas on how much of this is just cargo-culted? Do we play
> > it safe and do the "double workaround" or do we want to attempt to
> > rectify this by adding a Ricoh-specific quirk?
>
> It is a good question, but it has been that way for a very long time,
> and the spec tends to document them separately anyway, so it doesn't
> seem there is much reason to change.
Fair enough. Prathamesh, perhaps revise the comment above as part of
this patch because with the change below it now sounds a bit confusing.
Thierry
Attachment:
signature.asc
Description: PGP signature
