> -----Original Message----- > From: linux-mmc-owner@xxxxxxxxxxxxxxx <linux-mmc-owner@xxxxxxxxxxxxxxx> > On Behalf Of Ulf Hansson > Sent: Wednesday, December 18, 2019 9:40 PM > To: Yinbo Zhu <yinbo.zhu@xxxxxxx>; Y.b. Lu <yangbo.lu@xxxxxxx> > Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>; Rasmus Villemoes > <linux@xxxxxxxxxxxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx; Linux Kernel > Mailing List <linux-kernel@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH] mmc: sdhci-of-esdhc: Revert "mmc: sdhci-of-esdhc: add > erratum A-009204 support" > > Yinbo, Yangbo > > On Tue, 10 Dec 2019 at 10:51, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > > > On Wed, 4 Dec 2019 at 09:54, Rasmus Villemoes > <linux@xxxxxxxxxxxxxxxxxx> wrote: > > > > > > This reverts commit 5dd195522562542bc6ebe6e7bd47890d8b7ca93c. > > > > > > First, the fix seems to be plain wrong, since the erratum suggests > > > waiting 5ms before setting setting SYSCTL[RSTD], but this msleep() > > > happens after the call of sdhci_reset() which is where that bit gets > > > set (if SDHCI_RESET_DATA is in mask). > > > > > > Second, walking the whole device tree to figure out if some node has a > > > "fsl,p2020-esdhc" compatible string is hugely expensive - about 70 to > > > 100 us on our mpc8309 board. Walking the device tree is done under a > > > raw_spin_lock, so this is obviously really bad on an -rt system, and a > > > waste of time on all. > > > > > > In fact, since esdhc_reset() seems to get called around 100 times per > > > second, that mpc8309 now spends 0.8% of its time determining that > > > it is not a p2020. Whether those 100 calls/s are normal or due to some > > > other bug or misconfiguration, regularly hitting a 100 us > > > non-preemptible window is unacceptable. > > > > > > Signed-off-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> > > > > Applied for fixes and by adding a stable tag, thanks! > > Just wanted to highlight, that $subject patch has been applied for > fixes, which means we need a new fix the errata A-009204. > > Rasmus kind of already hinted on how this could be fixed in a better > way, hope this helps. > Sorry Uffe, I missed the patch since my name wasn't in TO/CC list. I have sent out a patch for re-implementation. Thanks! > Kind regards > Uffe > > > > --- > > > > > > The errata sheet for mpc8309 also mentions A-009204, so I'm not at all > > > opposed to having a fix for that. But it needs to be done properly > > > without causing a huge performance or latency impact. We should > > > probably just add a bit to struct sdhci_esdhc which gets initialized > > > in esdhc_init. > > > > > > drivers/mmc/host/sdhci-of-esdhc.c | 3 --- > > > 1 file changed, 3 deletions(-) > > > > > > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c > b/drivers/mmc/host/sdhci-of-esdhc.c > > > index 5cca3fa4610b..7f87a90bf56a 100644 > > > --- a/drivers/mmc/host/sdhci-of-esdhc.c > > > +++ b/drivers/mmc/host/sdhci-of-esdhc.c > > > @@ -764,9 +764,6 @@ static void esdhc_reset(struct sdhci_host *host, u8 > mask) > > > sdhci_writel(host, host->ier, SDHCI_INT_ENABLE); > > > sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); > > > > > > - if (of_find_compatible_node(NULL, NULL, "fsl,p2020-esdhc")) > > > - mdelay(5); > > > - > > > if (mask & SDHCI_RESET_ALL) { > > > val = sdhci_readl(host, ESDHC_TBCTL); > > > val &= ~ESDHC_TB_EN; > > > -- > > > 2.23.0 > > >