diff mbox

[U-Boot,3/4] mmc: fsl_esdhc: Implement card-detect hook.

Message ID 1323073424-16656-3-git-send-email-thierry.reding@avionic-design.de
State Accepted
Commit d48d2e21d4cec0f1ae70287a6d32413d3262675c
Delegated to: Andy Fleming
Headers show

Commit Message

Thierry Reding Dec. 5, 2011, 8:23 a.m. UTC
This card-detect hook probably doesn't work. Perhaps somebody with more
knowledge about the hardware can comment on this. I think that perhaps
even the complete code from esdhc_init() could go into the getcd()
function instead or mmc_getcd() needs to be called at some later time
after mmc_init(), which, however, would require many other drivers to
change.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
 drivers/mmc/fsl_esdhc.c |   29 ++++++++++++-----------------
 1 files changed, 12 insertions(+), 17 deletions(-)

Comments

Stefano Babic Dec. 5, 2011, 10:47 a.m. UTC | #1
On 05/12/2011 09:23, Thierry Reding wrote:
> This card-detect hook probably doesn't work. Perhaps somebody with more
> knowledge about the hardware can comment on this. I think that perhaps
> even the complete code from esdhc_init() could go into the getcd()

The reason was only that the SDHC controller returns (up now) if the car
is inserted or not. However, there is no hardware related reason to make
this function called by the esdhc_init().

> function instead or mmc_getcd() needs to be called at some later time
> after mmc_init(), which, however, would require many other drivers to
> change.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
>  drivers/mmc/fsl_esdhc.c |   29 ++++++++++++-----------------
>  1 files changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index f719afd..b46bf9f 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -412,7 +412,6 @@ static int esdhc_init(struct mmc *mmc)
>  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
>  	struct fsl_esdhc *regs = (struct fsl_esdhc *)cfg->esdhc_base;
>  	int timeout = 1000;
> -	int ret = 0;
>  
>  	/* Reset the entire host controller */
>  	esdhc_write32(&regs->sysctl, SYSCTL_RSTA);
> @@ -439,24 +438,19 @@ static int esdhc_init(struct mmc *mmc)
>  	/* Set timout to the maximum value */
>  	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
>  
> -	/* Check if there is a callback for detecting the card */
> -	ret = board_mmc_getcd(mmc);
> -	if (ret < 0) {
> -		timeout = 1000;
> -		while (!(esdhc_read32(&regs->prsstat) & PRSSTAT_CINS) &&
> -				--timeout)
> -			udelay(1000);
> +	return 0;
> +}

This is wrong, we have on fsl_esdhc.c at least two cases:
- the Card Detect is executed directly by the controller reading the
prsstat register - this happens for most (or all) PowerPC SOC haveing a
ESDHC controller.

- The Card Detect is done via GPIOs, as most of MX5 boards are doing
now, and as you implemented for Tegra (next patch).

You drop the way via GPIOs, breaking several boards.

Best regards,
Stefano Babic
Thierry Reding Dec. 5, 2011, 12:49 p.m. UTC | #2
* Stefano Babic wrote:
> On 05/12/2011 09:23, Thierry Reding wrote:
> > This card-detect hook probably doesn't work. Perhaps somebody with more
> > knowledge about the hardware can comment on this. I think that perhaps
> > even the complete code from esdhc_init() could go into the getcd()
> 
> The reason was only that the SDHC controller returns (up now) if the car
> is inserted or not. However, there is no hardware related reason to make
> this function called by the esdhc_init().

Does that mean that the prsstat register work even if the controller hasn't
been initialized? If so, then the hook should work as is.

> > function instead or mmc_getcd() needs to be called at some later time
> > after mmc_init(), which, however, would require many other drivers to
> > change.
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > ---
> >  drivers/mmc/fsl_esdhc.c |   29 ++++++++++++-----------------
> >  1 files changed, 12 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> > index f719afd..b46bf9f 100644
> > --- a/drivers/mmc/fsl_esdhc.c
> > +++ b/drivers/mmc/fsl_esdhc.c
> > @@ -412,7 +412,6 @@ static int esdhc_init(struct mmc *mmc)
> >  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> >  	struct fsl_esdhc *regs = (struct fsl_esdhc *)cfg->esdhc_base;
> >  	int timeout = 1000;
> > -	int ret = 0;
> >  
> >  	/* Reset the entire host controller */
> >  	esdhc_write32(&regs->sysctl, SYSCTL_RSTA);
> > @@ -439,24 +438,19 @@ static int esdhc_init(struct mmc *mmc)
> >  	/* Set timout to the maximum value */
> >  	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
> >  
> > -	/* Check if there is a callback for detecting the card */
> > -	ret = board_mmc_getcd(mmc);
> > -	if (ret < 0) {
> > -		timeout = 1000;
> > -		while (!(esdhc_read32(&regs->prsstat) & PRSSTAT_CINS) &&
> > -				--timeout)
> > -			udelay(1000);
> > +	return 0;
> > +}
> 
> This is wrong, we have on fsl_esdhc.c at least two cases:
> - the Card Detect is executed directly by the controller reading the
> prsstat register - this happens for most (or all) PowerPC SOC haveing a
> ESDHC controller.
> 
> - The Card Detect is done via GPIOs, as most of MX5 boards are doing
> now, and as you implemented for Tegra (next patch).
> 
> You drop the way via GPIOs, breaking several boards.

No, patch 2 already moved board_mmc_getcd() to mmc_getcd() and calls
mmc_getcd() before mmc_init() is even called. So all boards should work as
before. This really only removes the duplicate check. Perhaps I should
mention that in the commit message.

Thierry
diff mbox

Patch

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index f719afd..b46bf9f 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -412,7 +412,6 @@  static int esdhc_init(struct mmc *mmc)
 	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
 	struct fsl_esdhc *regs = (struct fsl_esdhc *)cfg->esdhc_base;
 	int timeout = 1000;
-	int ret = 0;
 
 	/* Reset the entire host controller */
 	esdhc_write32(&regs->sysctl, SYSCTL_RSTA);
@@ -439,24 +438,19 @@  static int esdhc_init(struct mmc *mmc)
 	/* Set timout to the maximum value */
 	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
 
-	/* Check if there is a callback for detecting the card */
-	ret = board_mmc_getcd(mmc);
-	if (ret < 0) {
-		timeout = 1000;
-		while (!(esdhc_read32(&regs->prsstat) & PRSSTAT_CINS) &&
-				--timeout)
-			udelay(1000);
+	return 0;
+}
 
-		if (timeout <= 0)
-			ret = NO_CARD_ERR;
-	} else {
-		if (ret == 0)
-			ret = NO_CARD_ERR;
-		else
-			ret = 0;
-	}
+static int esdhc_getcd(struct mmc *mmc)
+{
+	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
+	struct fsl_esdhc *regs = (struct fsl_esdhc *)cfg->esdhc_base;
+	int timeout = 1000;
+
+	while (!(esdhc_read32(&regs->prsstat) & PRSSTAT_CINS) && --timeout)
+		udelay(1000);
 
-	return ret;
+	return timeout > 0;
 }
 
 static void esdhc_reset(struct fsl_esdhc *regs)
@@ -494,6 +488,7 @@  int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg)
 	mmc->send_cmd = esdhc_send_cmd;
 	mmc->set_ios = esdhc_set_ios;
 	mmc->init = esdhc_init;
+	mmc->getcd = esdhc_getcd;
 
 	voltage_caps = 0;
 	caps = regs->hostcapblt;