Subject: Re: [PATCH] powerpc/esdhc: enable the card insert/remove interrupt

From: Anton Vorontsov <[email-masked]>
Date: Thu, 25 Oct 2012 03:30:38 -0700

On Thu, Oct 25, 2012 at 10:05:44AM +0000, Huang Changming-R66093 wrote:
> Hi, Anton.
> Could you have any comment about it?
> If not, I will resend this patch with v2.

Technically, the patch looks fine.

But again, as far as I recall, the card detection logic was broken on the
SOC level (it's actually very hard to break it on the board level -- it
would either work or not, but in the eSDHC case it was just unreliable,
again, if I recall everything correctly -- I might be wrong).

Of course you know the hardware much better, so your words weight more, so
you don't need my ack on the patch. :)

Although there's a second issue: for P4080DS and mpc837x boards you still
have the same problem with polling method, right? It is causing
performance drops/freezes every like 100 ms, and that's why you want to
avoid the polling.

So, you've "fixed" some boards, but left others to suffer. Ideally, the
best fix would be to also make the card polling cheap.

Anyways, using (d) clause of the "Reviewer's statement of oversight", I
can easily give this:

	Reviewed-by: Anton Vorontsov <[email-masked]>

:)

Thanks!

[...]
> > > IIRC, the card detection is broken SOC-revision-wise, not board-wise.
> > > So the change seems wrong.
> > >
> > > Also, take a look at this:
> > >
> > > 	http://lkml.org/lkml/2010/7/14/127
> > >
> > > I started the work but never finished, unfortunately it caused some
> > > regressions for non-FSL hardware...
> > >
> > > >  drivers/mmc/host/sdhci-of-esdhc.c |    7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > > > b/drivers/mmc/host/sdhci-of-esdhc.c
> > > > index ffc1226..5dc362f 100644
> > > > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > > > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > > > @@ -196,6 +196,11 @@ static void esdhc_of_detect_limitation(struct
> > > platform_device *pdev,
> > > >  	if (vvn == VENDOR_V_22)
> > > >  		pdata->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;
> > > >
> > > > +	/* P4080DS and MPC837XMDS board don't support interrupt mode */
> > > > +	if (of_machine_is_compatible("fsl,mpc837xmds") ||
> > > > +	    of_machine_is_compatible("fsl,P4080DS"))
> > > > +		pdata->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> > > > +
> > > >  	iounmap(ioaddr);
> > > >  end:
> > > >  	return;
> > > > @@ -223,7 +228,7 @@ static struct sdhci_pltfm_data sdhci_esdhc_pdata
> > > > =
> > > {
> > > >  	 * card detection could be handled via GPIO
> > > >  	 * eSDHC cannot support End Attribute in NOP ADMA descriptor
> > > >  	 */
> > > > -	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION
> > > > +	.quirks = ESDHC_DEFAULT_QUIRKS
> > > >  		| SDHCI_QUIRK_NO_CARD_NO_RESET
> > > >  		| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> > > >  	.ops = &sdhci_esdhc_ops,
> > > > --
> > > > 1.7.9.5