diff mbox

[U-Boot,v2,1/4] mmc: Change board_mmc_getcd() signature.

Message ID 1323414678-4291-2-git-send-email-thierry.reding@avionic-design.de
State Superseded
Headers show

Commit Message

Thierry Reding Dec. 9, 2011, 7:11 a.m. UTC
The new API no longer uses the extra cd parameter that was used to store
the card presence state. Instead, this information is returned via the
function's return value. board_mmc_getcd() returns -1 to indicate that
no card-detection mechanism is implemented; 0 indicates that no card is
present and 1 is returned if it was detected that a card is present.

The rationale for this change can be found in the following email
thread:

	http://lists.denx.de/pipermail/u-boot/2011-November/110180.html

In summary, the old API was not consistent with the rest of the MMC API
which always passes a struct mmc as the first parameter. Furthermore the
cd parameter was used to mean "card absence" in some implementations and
"card presence" in others.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
 board/efikamx/efikamx.c             |    8 +++-----
 board/emk/top9000/top9000.c         |   12 ++----------
 board/freescale/mx51evk/mx51evk.c   |    8 +++-----
 board/freescale/mx53ard/mx53ard.c   |    8 +++-----
 board/freescale/mx53evk/mx53evk.c   |    8 +++-----
 board/freescale/mx53loco/mx53loco.c |    8 +++-----
 board/freescale/mx53smd/mx53smd.c   |    6 ++----
 doc/README.atmel_mci                |   12 ++----------
 drivers/mmc/fsl_esdhc.c             |    8 +++++---
 drivers/mmc/mmc.c                   |    4 ++--
 include/mmc.h                       |    2 +-
 11 files changed, 29 insertions(+), 55 deletions(-)

Comments

Marek Vasut Dec. 9, 2011, 8:32 a.m. UTC | #1
By "signature" you mean signedness ?

> The new API no longer uses the extra cd parameter that was used to store
> the card presence state. Instead, this information is returned via the
> function's return value. board_mmc_getcd() returns -1 to indicate that
> no card-detection mechanism is implemented; 0 indicates that no card is
> present and 1 is returned if it was detected that a card is present.
> 
> The rationale for this change can be found in the following email
> thread:
> 
> 	http://lists.denx.de/pipermail/u-boot/2011-November/110180.html
> 
> In summary, the old API was not consistent with the rest of the MMC API
> which always passes a struct mmc as the first parameter. Furthermore the
> cd parameter was used to mean "card absence" in some implementations and
> "card presence" in others.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
>  board/efikamx/efikamx.c             |    8 +++-----
>  board/emk/top9000/top9000.c         |   12 ++----------
>  board/freescale/mx51evk/mx51evk.c   |    8 +++-----
>  board/freescale/mx53ard/mx53ard.c   |    8 +++-----
>  board/freescale/mx53evk/mx53evk.c   |    8 +++-----
>  board/freescale/mx53loco/mx53loco.c |    8 +++-----
>  board/freescale/mx53smd/mx53smd.c   |    6 ++----
>  doc/README.atmel_mci                |   12 ++----------
>  drivers/mmc/fsl_esdhc.c             |    8 +++++---
>  drivers/mmc/mmc.c                   |    4 ++--
>  include/mmc.h                       |    2 +-
>  11 files changed, 29 insertions(+), 55 deletions(-)
> 
> diff --git a/board/efikamx/efikamx.c b/board/efikamx/efikamx.c
> index b78bf6c..451d709 100644
> --- a/board/efikamx/efikamx.c
> +++ b/board/efikamx/efikamx.c
> @@ -309,17 +309,15 @@ static inline uint32_t efika_mmc_cd(void)
>  		return MX51_PIN_EIM_CS2;
>  }
> 
> -int board_mmc_getcd(u8 *absent, struct mmc *mmc)
> +int board_mmc_getcd(struct mmc *mmc)
>  {
>  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
>  	uint32_t cd = efika_mmc_cd();
> 
>  	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
> -		*absent = gpio_get_value(IOMUX_TO_GPIO(cd));
> -	else
> -		*absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8));
> +		return !gpio_get_value(IOMUX_TO_GPIO(cd));
> 
> -	return 0;
> +	return !gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8));

int ret;

if (cfg->...)
  ret = ...
else
  ret = ...

return ret;


>  }
> 
>  int board_mmc_init(bd_t *bis)
> diff --git a/board/emk/top9000/top9000.c b/board/emk/top9000/top9000.c
> index 61dee62..d156e32 100644
> --- a/board/emk/top9000/top9000.c
> +++ b/board/emk/top9000/top9000.c
> @@ -108,17 +108,9 @@ int board_mmc_init(bd_t *bd)
>  }
> 
>  /* this is a weak define that we are overriding */
> -int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> +int board_mmc_getcd(struct mmc *mmc)
>  {
> -	/*
> -	 * the only currently existing use of this function
> -	 * (fsl_esdhc.c) suggests this function must return
> -	 * *cs = TRUE if a card is NOT detected -> in most
> -	 * cases the value of the pin when the detect switch
> -	 * closes to GND
> -	 */
> -	*cd = at91_get_gpio_value(CONFIG_SYS_MMC_CD_PIN) ? 1 : 0;
> -	return 0;
> +	return !at91_get_gpio_value(CONFIG_SYS_MMC_CD_PIN);
>  }
> 
>  #endif
> diff --git a/board/freescale/mx51evk/mx51evk.c
> b/board/freescale/mx51evk/mx51evk.c index 37e6e4d..bc03496 100644
> --- a/board/freescale/mx51evk/mx51evk.c
> +++ b/board/freescale/mx51evk/mx51evk.c
> @@ -261,16 +261,14 @@ static void power_init(void)
>  }
> 
>  #ifdef CONFIG_FSL_ESDHC
> -int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> +int board_mmc_getcd(struct mmc *mmc)
>  {
>  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> 
>  	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
> -		*cd = gpio_get_value(0);
> -	else
> -		*cd = gpio_get_value(6);
> +		return !gpio_get_value(0);
> 
> -	return 0;
> +	return !gpio_get_value(6);

DTTO

>  }
> 
>  int board_mmc_init(bd_t *bis)
> diff --git a/board/freescale/mx53ard/mx53ard.c
> b/board/freescale/mx53ard/mx53ard.c index be32aee..786770a 100644
> --- a/board/freescale/mx53ard/mx53ard.c
> +++ b/board/freescale/mx53ard/mx53ard.c
> @@ -83,16 +83,14 @@ struct fsl_esdhc_cfg esdhc_cfg[2] = {
>  	{MMC_SDHC2_BASE_ADDR, 1 },
>  };
> 
> -int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> +int board_mmc_getcd(struct mmc *mmc)
>  {
>  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> 
>  	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
> -		*cd = gpio_get_value(1); /*GPIO1_1*/
> -	else
> -		*cd = gpio_get_value(4); /*GPIO1_4*/
> +		return !gpio_get_value(1); /*GPIO1_1*/
> 
> -	return 0;
> +	return !gpio_get_value(4); /*GPIO1_4*/

DTTO here please, also add spaces into the comment ... /* GPIO1_4 */

>  }
> 
>  int board_mmc_init(bd_t *bis)
> diff --git a/board/freescale/mx53evk/mx53evk.c
> b/board/freescale/mx53evk/mx53evk.c index 335661f..a4cd983 100644
> --- a/board/freescale/mx53evk/mx53evk.c
> +++ b/board/freescale/mx53evk/mx53evk.c
> @@ -208,16 +208,14 @@ struct fsl_esdhc_cfg esdhc_cfg[2] = {
>  	{MMC_SDHC3_BASE_ADDR, 1},
>  };
> 
> -int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> +int board_mmc_getcd(struct mmc *mmc)
>  {
>  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> 
>  	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
> -		*cd = gpio_get_value(77); /*GPIO3_13*/
> -	else
> -		*cd = gpio_get_value(75); /*GPIO3_11*/
> +		return !gpio_get_value(77); /*GPIO3_13*/
> 
> -	return 0;
> +	return !gpio_get_value(75); /*GPIO3_11*/

DTTO

>  }
> 
>  int board_mmc_init(bd_t *bis)
> diff --git a/board/freescale/mx53loco/mx53loco.c
> b/board/freescale/mx53loco/mx53loco.c index b4c7f33..a0fe5fd 100644
> --- a/board/freescale/mx53loco/mx53loco.c
> +++ b/board/freescale/mx53loco/mx53loco.c
> @@ -136,16 +136,14 @@ struct fsl_esdhc_cfg esdhc_cfg[2] = {
>  	{MMC_SDHC3_BASE_ADDR, 1},
>  };
> 
> -int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> +int board_mmc_getcd(struct mmc *mmc)
>  {
>  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> 
>  	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
> -		*cd = gpio_get_value(77); /*GPIO3_13*/
> -	else
> -		*cd = gpio_get_value(75); /*GPIO3_11*/
> +		return !gpio_get_value(77); /*GPIO3_13*/
> 
> -	return 0;
> +	return !gpio_get_value(75); /*GPIO3_11*/
>  }
> 
>  int board_mmc_init(bd_t *bis)
> diff --git a/board/freescale/mx53smd/mx53smd.c
> b/board/freescale/mx53smd/mx53smd.c index 87fa7fa..39274f9 100644
> --- a/board/freescale/mx53smd/mx53smd.c
> +++ b/board/freescale/mx53smd/mx53smd.c
> @@ -132,11 +132,9 @@ struct fsl_esdhc_cfg esdhc_cfg[1] = {
>  	{MMC_SDHC1_BASE_ADDR, 1},
>  };
> 
> -int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> +int board_mmc_getcd(struct mmc *mmc)
>  {
> -	*cd = gpio_get_value(77); /*GPIO3_13*/
> -
> -	return 0;
> +	return !gpio_get_value(77); /*GPIO3_13*/
>  }
> 
>  int board_mmc_init(bd_t *bis)
> diff --git a/doc/README.atmel_mci b/doc/README.atmel_mci
> index dee0cf0..0cbd909 100644
> --- a/doc/README.atmel_mci
> +++ b/doc/README.atmel_mci
> @@ -59,17 +59,9 @@ int board_mmc_init(bd_t *bd)
>  }
> 
>  /* this is a weak define that we are overriding */
> -int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> +int board_mmc_getcd(struct mmc *mmc)
>  {
> -	/*
> -	 * the only currently existing use of this function
> -	 * (fsl_esdhc.c) suggests this function must return
> -	 * *cs = TRUE if a card is NOT detected -> in most
> -	 * cases the value of the pin when the detect switch
> -	 * closes to GND
> -	 */
> -	*cd = at91_get_gpio_value (CONFIG_SYS_MMC_CD_PIN) ? 1 : 0;
> -	return 0;
> +	return !at91_get_gpio_value(CONFIG_SYS_MMC_CD_PIN);
>  }
> 
>  #endif
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index ec953f0..f719afd 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -413,7 +413,6 @@ static int esdhc_init(struct mmc *mmc)
>  	struct fsl_esdhc *regs = (struct fsl_esdhc *)cfg->esdhc_base;
>  	int timeout = 1000;
>  	int ret = 0;
> -	u8 card_absent;
> 
>  	/* Reset the entire host controller */
>  	esdhc_write32(&regs->sysctl, SYSCTL_RSTA);
> @@ -441,7 +440,8 @@ static int esdhc_init(struct mmc *mmc)
>  	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
> 
>  	/* Check if there is a callback for detecting the card */
> -	if (board_mmc_getcd(&card_absent, mmc)) {
> +	ret = board_mmc_getcd(mmc);
> +	if (ret < 0) {
>  		timeout = 1000;
>  		while (!(esdhc_read32(&regs->prsstat) & PRSSTAT_CINS) &&
>  				--timeout)
> @@ -450,8 +450,10 @@ static int esdhc_init(struct mmc *mmc)
>  		if (timeout <= 0)
>  			ret = NO_CARD_ERR;
>  	} else {
> -		if (card_absent)
> +		if (ret == 0)
>  			ret = NO_CARD_ERR;
> +		else
> +			ret = 0;
>  	}
> 
>  	return ret;
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 37ce6e8..936259f 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -40,11 +40,11 @@
>  static struct list_head mmc_devices;
>  static int cur_dev_num = -1;
> 
> -int __board_mmc_getcd(u8 *cd, struct mmc *mmc) {
> +int __board_mmc_getcd(struct mmc *mmc) {
>  	return -1;
>  }
> 
> -int board_mmc_getcd(u8 *cd, struct mmc *mmc)__attribute__((weak,
> +int board_mmc_getcd(struct mmc *mmc)__attribute__((weak,
>  	alias("__board_mmc_getcd")));
> 
>  int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data
> *data) diff --git a/include/mmc.h b/include/mmc.h
> index 015a7f3..a850174 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -314,7 +314,7 @@ struct mmc *find_mmc_device(int dev_num);
>  int mmc_set_dev(int dev_num);
>  void print_mmc_devices(char separator);
>  int get_mmc_num(void);
> -int board_mmc_getcd(u8 *cd, struct mmc *mmc);
> +int board_mmc_getcd(struct mmc *mmc);
>  int mmc_switch_part(int dev_num, unsigned int part_num);
> 
>  #ifdef CONFIG_GENERIC_MMC

M
Thierry Reding Dec. 9, 2011, 8:42 a.m. UTC | #2
* Marek Vasut wrote:
> By "signature" you mean signedness ?

No, I mean "signature" as synonymous to "function prototype".

[...]
> > -int board_mmc_getcd(u8 *absent, struct mmc *mmc)
> > +int board_mmc_getcd(struct mmc *mmc)
> >  {
> >  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> >  	uint32_t cd = efika_mmc_cd();
> > 
> >  	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
> > -		*absent = gpio_get_value(IOMUX_TO_GPIO(cd));
> > -	else
> > -		*absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8));
> > +		return !gpio_get_value(IOMUX_TO_GPIO(cd));
> > 
> > -	return 0;
> > +	return !gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8));
> 
> int ret;
> 
> if (cfg->...)
>   ret = ...
> else
>   ret = ...
> 
> return ret;

That'll require an extra variable and will actually be longer. I don't see
any advantage in converting it.

> DTTO here please, also add spaces into the comment ... /* GPIO1_4 */

I was going to keep that as it was, but I guess since I'm already changing
the line I can as well clean it up.

Thierry
Marek Vasut Dec. 9, 2011, 9:12 a.m. UTC | #3
> * Marek Vasut wrote:
> > By "signature" you mean signedness ?
> 
> No, I mean "signature" as synonymous to "function prototype".

Please say so then, it was slightly confusing.
> 
> [...]
> 
> > > -int board_mmc_getcd(u8 *absent, struct mmc *mmc)
> > > +int board_mmc_getcd(struct mmc *mmc)
> > > 
> > >  {
> > >  
> > >  	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> > >  	uint32_t cd = efika_mmc_cd();
> > >  	
> > >  	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
> > > 
> > > -		*absent = gpio_get_value(IOMUX_TO_GPIO(cd));
> > > -	else
> > > -		*absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8));
> > > +		return !gpio_get_value(IOMUX_TO_GPIO(cd));
> > > 
> > > -	return 0;
> > > +	return !gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8));
> > 
> > int ret;
> > 
> > if (cfg->...)
> > 
> >   ret = ...
> > 
> > else
> > 
> >   ret = ...
> > 
> > return ret;
> 
> That'll require an extra variable and will actually be longer. I don't see
> any advantage in converting it.

It's slightly more readable IMO, but that's a matter of personal taste so I'd 
like others to comment on this one.

> 
> > DTTO here please, also add spaces into the comment ... /* GPIO1_4 */
> 
> I was going to keep that as it was, but I guess since I'm already changing
> the line I can as well clean it up.
> 
> Thierry

Thanks!

M
diff mbox

Patch

diff --git a/board/efikamx/efikamx.c b/board/efikamx/efikamx.c
index b78bf6c..451d709 100644
--- a/board/efikamx/efikamx.c
+++ b/board/efikamx/efikamx.c
@@ -309,17 +309,15 @@  static inline uint32_t efika_mmc_cd(void)
 		return MX51_PIN_EIM_CS2;
 }
 
-int board_mmc_getcd(u8 *absent, struct mmc *mmc)
+int board_mmc_getcd(struct mmc *mmc)
 {
 	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
 	uint32_t cd = efika_mmc_cd();
 
 	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
-		*absent = gpio_get_value(IOMUX_TO_GPIO(cd));
-	else
-		*absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8));
+		return !gpio_get_value(IOMUX_TO_GPIO(cd));
 
-	return 0;
+	return !gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8));
 }
 
 int board_mmc_init(bd_t *bis)
diff --git a/board/emk/top9000/top9000.c b/board/emk/top9000/top9000.c
index 61dee62..d156e32 100644
--- a/board/emk/top9000/top9000.c
+++ b/board/emk/top9000/top9000.c
@@ -108,17 +108,9 @@  int board_mmc_init(bd_t *bd)
 }
 
 /* this is a weak define that we are overriding */
-int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+int board_mmc_getcd(struct mmc *mmc)
 {
-	/*
-	 * the only currently existing use of this function
-	 * (fsl_esdhc.c) suggests this function must return
-	 * *cs = TRUE if a card is NOT detected -> in most
-	 * cases the value of the pin when the detect switch
-	 * closes to GND
-	 */
-	*cd = at91_get_gpio_value(CONFIG_SYS_MMC_CD_PIN) ? 1 : 0;
-	return 0;
+	return !at91_get_gpio_value(CONFIG_SYS_MMC_CD_PIN);
 }
 
 #endif
diff --git a/board/freescale/mx51evk/mx51evk.c b/board/freescale/mx51evk/mx51evk.c
index 37e6e4d..bc03496 100644
--- a/board/freescale/mx51evk/mx51evk.c
+++ b/board/freescale/mx51evk/mx51evk.c
@@ -261,16 +261,14 @@  static void power_init(void)
 }
 
 #ifdef CONFIG_FSL_ESDHC
-int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+int board_mmc_getcd(struct mmc *mmc)
 {
 	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
 
 	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
-		*cd = gpio_get_value(0);
-	else
-		*cd = gpio_get_value(6);
+		return !gpio_get_value(0);
 
-	return 0;
+	return !gpio_get_value(6);
 }
 
 int board_mmc_init(bd_t *bis)
diff --git a/board/freescale/mx53ard/mx53ard.c b/board/freescale/mx53ard/mx53ard.c
index be32aee..786770a 100644
--- a/board/freescale/mx53ard/mx53ard.c
+++ b/board/freescale/mx53ard/mx53ard.c
@@ -83,16 +83,14 @@  struct fsl_esdhc_cfg esdhc_cfg[2] = {
 	{MMC_SDHC2_BASE_ADDR, 1 },
 };
 
-int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+int board_mmc_getcd(struct mmc *mmc)
 {
 	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
 
 	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
-		*cd = gpio_get_value(1); /*GPIO1_1*/
-	else
-		*cd = gpio_get_value(4); /*GPIO1_4*/
+		return !gpio_get_value(1); /*GPIO1_1*/
 
-	return 0;
+	return !gpio_get_value(4); /*GPIO1_4*/
 }
 
 int board_mmc_init(bd_t *bis)
diff --git a/board/freescale/mx53evk/mx53evk.c b/board/freescale/mx53evk/mx53evk.c
index 335661f..a4cd983 100644
--- a/board/freescale/mx53evk/mx53evk.c
+++ b/board/freescale/mx53evk/mx53evk.c
@@ -208,16 +208,14 @@  struct fsl_esdhc_cfg esdhc_cfg[2] = {
 	{MMC_SDHC3_BASE_ADDR, 1},
 };
 
-int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+int board_mmc_getcd(struct mmc *mmc)
 {
 	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
 
 	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
-		*cd = gpio_get_value(77); /*GPIO3_13*/
-	else
-		*cd = gpio_get_value(75); /*GPIO3_11*/
+		return !gpio_get_value(77); /*GPIO3_13*/
 
-	return 0;
+	return !gpio_get_value(75); /*GPIO3_11*/
 }
 
 int board_mmc_init(bd_t *bis)
diff --git a/board/freescale/mx53loco/mx53loco.c b/board/freescale/mx53loco/mx53loco.c
index b4c7f33..a0fe5fd 100644
--- a/board/freescale/mx53loco/mx53loco.c
+++ b/board/freescale/mx53loco/mx53loco.c
@@ -136,16 +136,14 @@  struct fsl_esdhc_cfg esdhc_cfg[2] = {
 	{MMC_SDHC3_BASE_ADDR, 1},
 };
 
-int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+int board_mmc_getcd(struct mmc *mmc)
 {
 	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
 
 	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
-		*cd = gpio_get_value(77); /*GPIO3_13*/
-	else
-		*cd = gpio_get_value(75); /*GPIO3_11*/
+		return !gpio_get_value(77); /*GPIO3_13*/
 
-	return 0;
+	return !gpio_get_value(75); /*GPIO3_11*/
 }
 
 int board_mmc_init(bd_t *bis)
diff --git a/board/freescale/mx53smd/mx53smd.c b/board/freescale/mx53smd/mx53smd.c
index 87fa7fa..39274f9 100644
--- a/board/freescale/mx53smd/mx53smd.c
+++ b/board/freescale/mx53smd/mx53smd.c
@@ -132,11 +132,9 @@  struct fsl_esdhc_cfg esdhc_cfg[1] = {
 	{MMC_SDHC1_BASE_ADDR, 1},
 };
 
-int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+int board_mmc_getcd(struct mmc *mmc)
 {
-	*cd = gpio_get_value(77); /*GPIO3_13*/
-
-	return 0;
+	return !gpio_get_value(77); /*GPIO3_13*/
 }
 
 int board_mmc_init(bd_t *bis)
diff --git a/doc/README.atmel_mci b/doc/README.atmel_mci
index dee0cf0..0cbd909 100644
--- a/doc/README.atmel_mci
+++ b/doc/README.atmel_mci
@@ -59,17 +59,9 @@  int board_mmc_init(bd_t *bd)
 }
 
 /* this is a weak define that we are overriding */
-int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+int board_mmc_getcd(struct mmc *mmc)
 {
-	/*
-	 * the only currently existing use of this function
-	 * (fsl_esdhc.c) suggests this function must return
-	 * *cs = TRUE if a card is NOT detected -> in most
-	 * cases the value of the pin when the detect switch
-	 * closes to GND
-	 */
-	*cd = at91_get_gpio_value (CONFIG_SYS_MMC_CD_PIN) ? 1 : 0;
-	return 0;
+	return !at91_get_gpio_value(CONFIG_SYS_MMC_CD_PIN);
 }
 
 #endif
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index ec953f0..f719afd 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -413,7 +413,6 @@  static int esdhc_init(struct mmc *mmc)
 	struct fsl_esdhc *regs = (struct fsl_esdhc *)cfg->esdhc_base;
 	int timeout = 1000;
 	int ret = 0;
-	u8 card_absent;
 
 	/* Reset the entire host controller */
 	esdhc_write32(&regs->sysctl, SYSCTL_RSTA);
@@ -441,7 +440,8 @@  static int esdhc_init(struct mmc *mmc)
 	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
 
 	/* Check if there is a callback for detecting the card */
-	if (board_mmc_getcd(&card_absent, mmc)) {
+	ret = board_mmc_getcd(mmc);
+	if (ret < 0) {
 		timeout = 1000;
 		while (!(esdhc_read32(&regs->prsstat) & PRSSTAT_CINS) &&
 				--timeout)
@@ -450,8 +450,10 @@  static int esdhc_init(struct mmc *mmc)
 		if (timeout <= 0)
 			ret = NO_CARD_ERR;
 	} else {
-		if (card_absent)
+		if (ret == 0)
 			ret = NO_CARD_ERR;
+		else
+			ret = 0;
 	}
 
 	return ret;
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 37ce6e8..936259f 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -40,11 +40,11 @@ 
 static struct list_head mmc_devices;
 static int cur_dev_num = -1;
 
-int __board_mmc_getcd(u8 *cd, struct mmc *mmc) {
+int __board_mmc_getcd(struct mmc *mmc) {
 	return -1;
 }
 
-int board_mmc_getcd(u8 *cd, struct mmc *mmc)__attribute__((weak,
+int board_mmc_getcd(struct mmc *mmc)__attribute__((weak,
 	alias("__board_mmc_getcd")));
 
 int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
diff --git a/include/mmc.h b/include/mmc.h
index 015a7f3..a850174 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -314,7 +314,7 @@  struct mmc *find_mmc_device(int dev_num);
 int mmc_set_dev(int dev_num);
 void print_mmc_devices(char separator);
 int get_mmc_num(void);
-int board_mmc_getcd(u8 *cd, struct mmc *mmc);
+int board_mmc_getcd(struct mmc *mmc);
 int mmc_switch_part(int dev_num, unsigned int part_num);
 
 #ifdef CONFIG_GENERIC_MMC