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

[Stefano Babic <sbabic@denx.de>]

On 05/12/2011 09:23, Thierry Reding wrote:
> 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.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>

Hi Thierry,

it is not clear to me what you think with the new API, or better what
you think as new.

As I can see, in the current U-Boot TOT there is still :

drivers/mmc/mmc.c:int __board_mmc_getcd(u8 *cd, struct mmc *mmc)

It seems there is not (yet) a new API...

If it is true, which is the reason to change it ?

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

It seems to me you are inverting the logic. In you commit message, "1"
means that a card is detected, exactly as it is done now. Am I wrong ?

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

Ditto. gpio_get_value(0) returns not-zero if the pin is on logical level
high, telling that a card was inserted.

> 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")));

Why is it this better as before ?


Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================