Re: [PATCH v2] mmc: sdhci: Fix HISPD bit handling

From: Marc Zyngier <maz-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Peng Fan <peng.fan-3arQi8VN3Tc@public.gmane.org>,
	u-boot-0aAXYlwwYIKGBzrmiIFOJg@public.gmane.org,
	linux-amarula-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org,
	Kever Yang <kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Jagan Teki
	<jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>,
	sunil-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org,
	Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
Subject: Re: [PATCH v2] mmc: sdhci: Fix HISPD bit handling
Date: Wed, 10 Jun 2020 11:37:52 +0100	[thread overview]
Message-ID: <20200610113752.65d6f537@why> (raw)
In-Reply-To: <b46fe992-89d4-4a5f-fdb3-e2ec53de2b11-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On Wed, 10 Jun 2020 12:45:33 +0900
Jaehoon Chung <jh80.chung@samsung.com> wrote:

> On 6/9/20 11:38 PM, Robin Murphy wrote:
> > On 2020-06-09 15:01, Jagan Teki wrote:  
> >> SDHCI HISPD bits need to be configured based on desired mmc
> >> timings mode and some HISPD quirks.
> >>
> >> So, handle the HISPD bit based on the mmc computed selected
> >> mode(timing parameter) rather than fixed mmc card clock
> >> frequency.
> >>
> >> Linux handle the HISPD similar like this in below commit,
> >>
> >> commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
> >>
> >> This eventually fixed the mmc write issue observed in
> >> rk3399 sdhci controller.
> >>
> >> Bug log for refernece,  
> >> => gpt write mmc 0 $partitions  
> >> Writing GPT: mmc write failed
> >> ** Can't write to device 0 **
> >> ** Can't write to device 0 **
> >> error!
> >>
> >> Cc: Kever Yang <kever.yang@rock-chips.com>
> >> Cc: Peng Fan <peng.fan@nxp.com>
> >> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >> ---
> >> Changes for v2:
> >> - collect Jaehoon R-b
> >>
> >>   drivers/mmc/sdhci.c | 23 +++++++++++++++--------
> >>   1 file changed, 15 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> >> index 92cc8434af..280b8c88eb 100644
> >> --- a/drivers/mmc/sdhci.c
> >> +++ b/drivers/mmc/sdhci.c
> >> @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
> >>               ctrl &= ~SDHCI_CTRL_4BITBUS;
> >>       }
> >>   -    if (mmc->clock > 26000000)
> >> -        ctrl |= SDHCI_CTRL_HISPD;
> >> -    else
> >> -        ctrl &= ~SDHCI_CTRL_HISPD;
> >> -
> >> -    if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> >> -        (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
> >> -        ctrl &= ~SDHCI_CTRL_HISPD;
> >> +    if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||  
> > 
> > Should that be "&&" rather than "||"? Otherwise this will always
> > evaluate to true unless *both* quirks are set, which isn't
> > equivalent to the check being removed above.  
> 
> 
> You're right.

It'd be great if you could respin this patch quickly and get it merged,
as it just helped me getting my NanoPC-T4 up and running.

FWIW:

Tested-by: Marc Zyngier <maz@kernel.org>

	M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip