[PATCH] mmc: sdhci: Rework SDHCI_QUIRK_BROKEN_R1B

[Jaehoon Chung]

> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Jaehoon Chung
> Sent: Wednesday, November 1, 2023 10:09 AM
> To: 'Henrik Grimler' <henrik at grimler.se>; 'Sean Anderson' <sean.anderson at seco.com>
> Cc: u-boot at lists.denx.de; 'Rayagonda Kokatanur' <rayagonda.kokatanur at broadcom.com>; 'Bharat Kumar
> Reddy Gooty' <bharat.gooty at broadcom.com>
> Subject: RE: [PATCH] mmc: sdhci: Rework SDHCI_QUIRK_BROKEN_R1B
> 
> Hi,
> 
> > -----Original Message-----
> > From: Henrik Grimler <henrik at grimler.se>
> > Sent: Sunday, October 29, 2023 5:31 AM
> > To: Sean Anderson <sean.anderson at seco.com>
> > Cc: u-boot at lists.denx.de; Jaehoon Chung <jh80.chung at samsung.com>; Rayagonda Kokatanur
> > <rayagonda.kokatanur at broadcom.com>; Bharat Kumar Reddy Gooty <bharat.gooty at broadcom.com>
> > Subject: Re: [PATCH] mmc: sdhci: Rework SDHCI_QUIRK_BROKEN_R1B
> >
> > Hi Sean,
> >
> > Tested on odroid-u2 (exynos4412-odroid) on top of 2024.01-rc1, still
> > works fine, so feel free to add:
> >
> > Tested-by: Henrik Grimler <henrik at grimler.se>
> >
> > Best regards,
> > Henrik Grimler
> >
> > On Fri, Oct 27, 2023 at 04:57:03PM -0400, Sean Anderson wrote:
> > > As noted in commit 3a6383207be ("mmc: sdhci: add the quirk for broken
> > > r1b response"), some MMC controllers don't always set the transfer
> > > complete bit with R1b responses.
> > >
> > > According to the SD Host Controller Simplified Specification v4.20,
> > >
> > > > In the case of a command pairing with response-with-busy[, Transfer
> > > > Complete] is set when busy is de-asserted. Refer to DAT Line Active
> > > > and Command Inhibit (DAT) in the Present State register.
> > >
> > > By polling the DAT Line Active bit in the present state register, we can
> > > detect when we are no longer busy, without waiting for a long timeout.
> > > This results in much faster reads/writes on buggy controllers.
> > >
> > > Signed-off-by: Sean Anderson <sean.anderson at seco.com>
> 
> Reviewed-by: Jaehoon Chung <jh80.chung at samsung.com>

Applied to u-boot-mmc/master. Thanks!

Best Regards,
Jaehoon Chung

> 
> Best Regards,
> Jaehoon Chung
> 
> > > ---
> > > I've CC'd a few people who've added this quirk to controllers recently.
> > > Please let me know if your hardware still works. It's possible that my
> > > hardware is buggy in a different way.
> > >
> > >  drivers/mmc/sdhci.c | 19 ++++++++++++-------
> > >  include/sdhci.h     |  1 +
> > >  2 files changed, 13 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > > index fc9c6c37996..0178ed8a11e 100644
> > > --- a/drivers/mmc/sdhci.c
> > > +++ b/drivers/mmc/sdhci.c
> > > @@ -306,14 +306,19 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
> > >  		if (stat & SDHCI_INT_ERROR)
> > >  			break;
> > >
> > > -		if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {
> > > -			if (host->quirks & SDHCI_QUIRK_BROKEN_R1B) {
> > > +		if (host->quirks & SDHCI_QUIRK_BROKEN_R1B &&
> > > +		    cmd->resp_type & MMC_RSP_BUSY && !data) {
> > > +			unsigned int state =
> > > +				sdhci_readl(host, SDHCI_PRESENT_STATE);
> > > +
> > > +			if (!(state & SDHCI_DAT_ACTIVE))
> > >  				return 0;
> > > -			} else {
> > > -				printf("%s: Timeout for status update!\n",
> > > -				       __func__);
> > > -				return -ETIMEDOUT;
> > > -			}
> > > +		}
> > > +
> > > +		if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {
> > > +			printf("%s: Timeout for status update: %08x %08x\n",
> > > +			       __func__, stat, mask);
> > > +			return -ETIMEDOUT;
> > >  		}
> > >  	} while ((stat & mask) != mask);
> > >
> > > diff --git a/include/sdhci.h b/include/sdhci.h
> > > index 70fefca2a97..a1b74e3bd79 100644
> > > --- a/include/sdhci.h
> > > +++ b/include/sdhci.h
> > > @@ -57,6 +57,7 @@
> > >  #define SDHCI_PRESENT_STATE	0x24
> > >  #define  SDHCI_CMD_INHIBIT	BIT(0)
> > >  #define  SDHCI_DATA_INHIBIT	BIT(1)
> > > +#define  SDHCI_DAT_ACTIVE	BIT(2)
> > >  #define  SDHCI_DOING_WRITE	BIT(8)
> > >  #define  SDHCI_DOING_READ	BIT(9)
> > >  #define  SDHCI_SPACE_AVAILABLE	BIT(10)
> > > --
> > > 2.35.1.1320.gc452695387.dirty
> > >