[U-Boot] [PATCH 1/9] wandboard: Return error if the SDHC port index is invalid
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/wandboard/wandboard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c index bb98352..70e070c 100644 --- a/board/wandboard/wandboard.c +++ b/board/wandboard/wandboard.c @@ -164,7 +164,7 @@ int board_mmc_init(bd_t *bis) printf("Warning: you configured more USDHC controllers" "(%d) then supported by the board (%d)\n", index + 1, CONFIG_SYS_FSL_USDHC_NUM); - return status; + return -EINVAL; }
status |= fsl_esdhc_initialize(bis, &usdhc_cfg[index]);
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx53loco/mx53loco.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx53loco/mx53loco.c b/board/freescale/mx53loco/mx53loco.c index 10e9d36..f1b2c40 100644 --- a/board/freescale/mx53loco/mx53loco.c +++ b/board/freescale/mx53loco/mx53loco.c @@ -204,7 +204,7 @@ int board_mmc_init(bd_t *bis) printf("Warning: you configured more ESDHC controller" "(%d) as supported by the board(2)\n", CONFIG_SYS_FSL_ESDHC_NUM); - return status; + return -EINVAL; } status |= fsl_esdhc_initialize(bis, &esdhc_cfg[index]); }
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx51evk/mx51evk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx51evk/mx51evk.c b/board/freescale/mx51evk/mx51evk.c index 369da6d..80749eb 100644 --- a/board/freescale/mx51evk/mx51evk.c +++ b/board/freescale/mx51evk/mx51evk.c @@ -356,7 +356,7 @@ int board_mmc_init(bd_t *bis) printf("Warning: you configured more ESDHC controller" "(%d) as supported by the board(2)\n", CONFIG_SYS_FSL_ESDHC_NUM); - return status; + return -EINVAL; } status |= fsl_esdhc_initialize(bis, &esdhc_cfg[index]); }
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx53ard/mx53ard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx53ard/mx53ard.c b/board/freescale/mx53ard/mx53ard.c index e2dbf63..1813bf5 100644 --- a/board/freescale/mx53ard/mx53ard.c +++ b/board/freescale/mx53ard/mx53ard.c @@ -201,7 +201,7 @@ int board_mmc_init(bd_t *bis) printf("Warning: you configured more ESDHC controller" "(%d) as supported by the board(2)\n", CONFIG_SYS_FSL_ESDHC_NUM); - return status; + return -EINVAL; } status |= fsl_esdhc_initialize(bis, &esdhc_cfg[index]); }
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx53evk/mx53evk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx53evk/mx53evk.c b/board/freescale/mx53evk/mx53evk.c index 727ad65..6e9f62c 100644 --- a/board/freescale/mx53evk/mx53evk.c +++ b/board/freescale/mx53evk/mx53evk.c @@ -230,7 +230,7 @@ int board_mmc_init(bd_t *bis) printf("Warning: you configured more ESDHC controller" "(%d) as supported by the board(2)\n", CONFIG_SYS_FSL_ESDHC_NUM); - return status; + return -EINVAL; } status |= fsl_esdhc_initialize(bis, &esdhc_cfg[index]); }
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx53smd/mx53smd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx53smd/mx53smd.c b/board/freescale/mx53smd/mx53smd.c index d04f44f..a6a6bbe 100644 --- a/board/freescale/mx53smd/mx53smd.c +++ b/board/freescale/mx53smd/mx53smd.c @@ -137,7 +137,7 @@ int board_mmc_init(bd_t *bis) printf("Warning: you configured more ESDHC controller" "(%d) as supported by the board(1)\n", CONFIG_SYS_FSL_ESDHC_NUM); - return status; + return -EINVAL; } status |= fsl_esdhc_initialize(bis, &esdhc_cfg[index]); }
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx6qarm2/mx6qarm2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx6qarm2/mx6qarm2.c b/board/freescale/mx6qarm2/mx6qarm2.c index e336746..e3e0b76 100644 --- a/board/freescale/mx6qarm2/mx6qarm2.c +++ b/board/freescale/mx6qarm2/mx6qarm2.c @@ -156,7 +156,7 @@ int board_mmc_init(bd_t *bis) printf("Warning: you configured more USDHC controllers" "(%d) then supported by the board (%d)\n", index + 1, CONFIG_SYS_FSL_USDHC_NUM); - return status; + return -EINVAL; }
status |= fsl_esdhc_initialize(bis, &usdhc_cfg[index]);
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx6qsabrelite/mx6qsabrelite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index 8ce054e..8a2e4b3 100644 --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c @@ -287,7 +287,7 @@ int board_mmc_init(bd_t *bis) printf("Warning: you configured more USDHC controllers" "(%d) then supported by the board (%d)\n", index + 1, CONFIG_SYS_FSL_USDHC_NUM); - return status; + return -EINVAL; }
status |= fsl_esdhc_initialize(bis, &usdhc_cfg[index]);
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx6qsabresd/mx6qsabresd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx6qsabresd/mx6qsabresd.c b/board/freescale/mx6qsabresd/mx6qsabresd.c index 2529826..094382b 100644 --- a/board/freescale/mx6qsabresd/mx6qsabresd.c +++ b/board/freescale/mx6qsabresd/mx6qsabresd.c @@ -198,7 +198,7 @@ int board_mmc_init(bd_t *bis) printf("Warning: you configured more USDHC controllers" "(%d) then supported by the board (%d)\n", i + 1, CONFIG_SYS_FSL_USDHC_NUM); - return status; + return -EINVAL; }
status |= fsl_esdhc_initialize(bis, &usdhc_cfg[i]);
On Tue, May 21, 2013 at 2:32 PM, Fabio Estevam fabio.estevam@freescale.comwrote:
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
When I looked at this code I didn't change it because it raises a warning. So it returns the status if any failed.
On Tue, May 21, 2013 at 3:26 PM, Otavio Salvador otavio@ossystems.com.br wrote:
When I looked at this code I didn't change it because it raises a warning. So it returns the status if any failed.
Which warning are you talking about? Build warning or run-time warning?
I did not see any of these here.
status is zero at that point of code, so we should not 'return 0' on failure.
On Tue, May 21, 2013 at 3:39 PM, Fabio Estevam festevam@gmail.com wrote:
On Tue, May 21, 2013 at 3:26 PM, Otavio Salvador otavio@ossystems.com.br wrote:
When I looked at this code I didn't change it because it raises a
warning.
So it returns the status if any failed.
Which warning are you talking about? Build warning or run-time warning?
I did not see any of these here.
status is zero at that point of code, so we should not 'return 0' on failure.
In the loop; it does:
status |= fsl...
so it gets the output of it. In case any fails it won't be 0.
On Tue, May 21, 2013 at 3:44 PM, Otavio Salvador otavio@ossystems.com.br wrote:
In the loop; it does:
status |= fsl...
so it gets the output of it. In case any fails it won't be 0.
It's good practice to return immediately when an error condition happens.
On Tue, May 21, 2013 at 3:49 PM, Fabio Estevam festevam@gmail.com wrote:
On Tue, May 21, 2013 at 3:44 PM, Otavio Salvador otavio@ossystems.com.br wrote:
In the loop; it does:
status |= fsl...
so it gets the output of it. In case any fails it won't be 0.
It's good practice to return immediately when an error condition happens.
Well the idea of the code was to return in case anything goes wrong. So it outputs a 'WARN'. If you wish to change this, please change to ERROR.
Dear Fabio Estevam,
In message 1369157542-25734-1-git-send-email-fabio.estevam@freescale.com you wrote:
When the SDHC port number index is invalid we should return an error code immediately.
Currently we return 'status', which has a value of zero, causing board_mmc_init() to incorrectly return sucess.
Fix this by returning -EINVAL instead.
Can we _please_ remove all this code? A _runtime_ check for a _build_ _time_ _detectable_ situation makes no sense to me.
For such a misconfiguration, the build should fail.
Handling this at runtime is the wrong approach.
This comment applies for the whole series.
Actually - are you not surprised that you have to fix the same issue for all boards? This is a clear indication of duplicated code that needs to be factored out.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Tue, May 21, 2013 at 6:24 PM, Wolfgang Denk wd@denx.de wrote:
Can we _please_ remove all this code? A _runtime_ check for a _build_ _time_ _detectable_ situation makes no sense to me.
For such a misconfiguration, the build should fail.
Handling this at runtime is the wrong approach.
This comment applies for the whole series.
For the wandboard file, would the code bellow be better?
for (index = 0; index < CONFIG_SYS_FSL_USDHC_NUM; ++index) { switch (index) { case 0: imx_iomux_v3_setup_multiple_pads( usdhc3_pads, ARRAY_SIZE(usdhc3_pads)); usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); usdhc_cfg[0].max_bus_width = 4; gpio_direction_input(USDHC3_CD_GPIO); break; case 1: imx_iomux_v3_setup_multiple_pads( usdhc1_pads, ARRAY_SIZE(usdhc1_pads)); usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK); usdhc_cfg[1].max_bus_width = 4; gpio_direction_input(USDHC1_CD_GPIO); break; }
status = fsl_esdhc_initialize(bis, &usdhc_cfg[index]); if (status) return status; }
return 0;
Actually - are you not surprised that you have to fix the same issue for all boards? This is a clear indication of duplicated code that needs to be factored out.
I will try factor out.
Regards,
Fabio Estevam
participants (4)
-
Fabio Estevam -
Fabio Estevam -
Otavio Salvador -
Wolfgang Denk