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

2011-12-05 Thread Thierry Reding
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.red...@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 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 

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

2011-12-05 Thread Marek Vasut
 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.red...@avionic-design.de

A silly question -- why do we need this change ? Can you explain it in the 
changelog of V2 too?

M
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2011-12-05 Thread Thierry Reding
* Marek Vasut 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.red...@avionic-design.de
 
 A silly question -- why do we need this change ? Can you explain it in the 
 changelog of V2 too?

It's the first step in implementing card-detection. I discussed this with
Andy and he came up with the idea that board_mmc_getcd() should really have
had the mmc parameter as first argument in the first place instead of the cd
parameter. Furthermore, the cd parameter is used inconsistently in individual
implementations. After some discussion we came to the conclusion that the cd
parameter wasn't required at all and the same information can be represented
in the return value. The whole discussion is in this thread:

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

It's not really a necessary change, but it makes board_mmc_getcd() much more
consistent with other MMC-related functions.

Perhaps this last sentence would be a good explanation to put in the v2
commit message?

Thierry


pgpFJthpXEohi.pgp
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2011-12-05 Thread Stefano Babic
On 05/12/2011 11:00, Thierry Reding wrote:
 * Marek Vasut 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.red...@avionic-design.de
 
 A silly question -- why do we need this change ? Can you explain
 it in the changelog of V2 too?
 
 It's the first step in implementing card-detection. I discussed
 this with Andy and he came up with the idea that board_mmc_getcd()
 should really have had the mmc parameter as first argument in the
 first place instead of the cd parameter.

Ok, I get it now.

 Furthermore, the cd parameter is used inconsistently in individual 
 implementations. After some discussion we came to the conclusion
 that the cd parameter wasn't required at all and the same
 information can be represented in the return value. The whole
 discussion is in this thread:
 
 http://lists.denx.de/pipermail/u-boot/2011-November/110180.html
 
 It's not really a necessary change, but it makes board_mmc_getcd()
 much more consistent with other MMC-related functions.
 
 Perhaps this last sentence would be a good explanation to put in
 the v2 commit message?

Ok, thanks - this explains much better which is your intention for the
patchset. It is also not bad to add a reference to the above thread.

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: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2011-12-05 Thread Marek Vasut
 On 05/12/2011 11:00, Thierry Reding wrote:
  * Marek Vasut 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.red...@avionic-design.de
  
  A silly question -- why do we need this change ? Can you explain
  it in the changelog of V2 too?
  
  It's the first step in implementing card-detection. I discussed
  this with Andy and he came up with the idea that board_mmc_getcd()
  should really have had the mmc parameter as first argument in the
  first place instead of the cd parameter.
 
 Ok, I get it now.
 
  Furthermore, the cd parameter is used inconsistently in individual
  implementations. After some discussion we came to the conclusion
  that the cd parameter wasn't required at all and the same
  information can be represented in the return value. The whole
  discussion is in this thread:
  
  http://lists.denx.de/pipermail/u-boot/2011-November/110180.html
  
  It's not really a necessary change, but it makes board_mmc_getcd()
  much more consistent with other MMC-related functions.
  
  Perhaps this last sentence would be a good explanation to put in
  the v2 commit message?
 
 Ok, thanks - this explains much better which is your intention for the
 patchset. It is also not bad to add a reference to the above thread.
 
 Best regards,
 Stefano Babic

Yep, and when/if submitting V2, also make a cover letter (while also adding the 
explanation to this patch's commit message).

Thanks!
M
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2011-12-05 Thread Thierry Reding
* Stefano Babic wrote:
 On 05/12/2011 09:23, Thierry Reding wrote:
[...]
  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 ?

The reason is that the fsl_esdhc driver actually calls the cd parameter
absent, so the logic is inverted in that driver. This change merely brings
the board implementation in line with the new API. Perhaps it would help if
the meaning of the return value should be documented explicitly in mmc.h?

Thierry


pgpKmjBqcKAlu.pgp
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot