> -----Original Message----- > From: Philip Rakity [mailto:prakity@xxxxxxxxxxx] > Sent: Thursday, December 15, 2011 2:42 PM > To: Huang Changming-R66093 > Cc: linux-mmc@xxxxxxxxxxxxxxx; Chris Ball > Subject: Re: [PATCH 3/4 v5] SDHCI: add sdhci_get_cd callback to detect > the card > > > On Dec 14, 2011, at 6:32 PM, Huang Changming-R66093 wrote: > > > > > > >> -----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. > > currently WE do read the present state register on every request. All > the above suggestion does is try to use your code > > maybe we want something like > >> if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) > >> present = true; > >> else if (host->ops->get_cd) > >> present = present = host->ops->get_cd(host); > >> else > >> present = sdhci_readl(host, SDHCI_PRESENT_STATE) & > >> SDHCI_CARD_PRESENT; > I am very confused, why do we read the present state register on every request? My codes are added to the function mmc_sd_detect in file core/sd.c Function mmc_rescan has detect the card present state repeatedly, so I think we don't need to detect the card state on every request. So I think the codes to detect the card present state in sdhci_request should be removed to improve the performance. -- 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
