RE: [PATCH 3/4 v5] SDHCI: add sdhci_get_cd callback to detect the card

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----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


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux