> -----Original Message----- > From: Philip Rakity [mailto:prakity@xxxxxxxxxxx] > Sent: Wednesday, December 14, 2011 12:52 PM > To: Huang Changming-R66093 > Cc: linux-mmc@xxxxxxxxxxxxxxx; Huang Changming-R66093; Chris Ball > Subject: Re: [PATCH 3/4 v5] SDHCI: add sdhci_get_cd callback to detect > the card > > > On Dec 13, 2011, at 6:18 PM, <r66093@xxxxxxxxxxxxx> wrote: > > > From: Jerry Huang <Chang-Ming.Huang@xxxxxxxxxxxxx> > > > > Add callback function sdhci_get_cd to detect the card. > > And one new callback added to implement the card detect in sdhci > struncture. > > If special platform has the card detect callback, it will return the > > card state, the value zero is for absent cardi and one is for present > card. > > If the controller don't support card detect, sdhci_get_cd will return - > ENOSYS. > > > > Signed-off-by: Jerry Huang <Chang-Ming.Huang@xxxxxxxxxxxxx> > > CC: Chris Ball <cjb@xxxxxxxxxx> > > --- > > changes for v2: > > - when controller don't support get_cd, return -ENOSYS > > - add new callback for sdhci to detect the card > > - add the CC > > changes for v3: > > - use pin_lock only when get_cd defined changes for v4: > > - enalbe the controller clock in platform, instead of core changes > > for v5: > > - remove the copyright > > > > drivers/mmc/host/sdhci.c | 21 ++++++++++++++++++++++ > > drivers/mmc/host/sdhci.h | 2 ++ > > 2 files changed, 23 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index > > 6d8eea3..fbe2f46 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -1518,6 +1519,26 @@ static int sdhci_get_ro(struct mmc_host *mmc) > > return ret; > > } > > > > +/* Return values for the sdjco_get_cd callback: > > + * 0 for a absent card > > + * 1 for a present card > > + * -ENOSYS when not supported (equal to NULL callback) > > + */ > > +static int sdhci_get_cd(struct mmc_host *mmc) { > > + struct sdhci_host *host = mmc_priv(mmc); > > + unsigned long flags; > > + int present = -ENOSYS; > > + > > + if (host->ops->get_cd) { > > + spin_lock_irqsave(&host->lock, flags); > > + present = host->ops->get_cd(host); > > + spin_unlock_irqrestore(&host->lock, flags); > > + } > > + > > + return present; > > +} > > In static void sdhci_request(struct mmc_host *mmc, struct mmc_request > *mrq) > > there is code below to handle broken card detect. > > 1257 > 1258 /* If polling, assume that the card is always present. */ > 1259 if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) > 1260 present = true; > 1261 else > 1262 present = sdhci_readl(host, SDHCI_PRESENT_STATE) & > 1263 SDHCI_CARD_PRESENT; > > The problem with this code is that it assumes broken card detect is > broken since the present state register cannot be read. > > would it make more sense to do something like > if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) { > if (host->ops->get_cd) > present = present = host->ops->get_cd(host); > else > present = true; > > and adjust the code in host->ops->get_cd() to not -ENOSYS. I think we should not detect the card present state in sdhci_request function, Because if we do it, this detection will be performed with any command to card, which will down the performance. And if the quirks has SDHCI_QUIRK_BROKEN_CARD_DETECTION, that means the card is always present, We don't need to detect the card state. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html