[U-Boot] [PATCH 0/4] Fix operation on Odroid devices

Hello,
currently operation on Exynos4412-based Odroid devices is broken.
The bootloader stops with this message: Card did not respond to voltage select! *** Warning - MMC init failed, using default environment
This series fixes error handling in the s5p sdhci driver and the cause of the issue, which is an inverted card detection check.
Thanks goes to Marek Vasut and Sjoerd Simons who helped me on IRC to get this triaged.
With best wishes, Tobias
Tobias Jakobi (4): exynos: Properly initialize host_caps in s5p_sdhci_core_init() exynos: Fix passing of errors in exynos_mmc_init() exynos: be more verbose in process_nodes() exynos: fix and cleanup do_sdhci_init()
drivers/mmc/s5p_sdhci.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)

The sdhci_host struct is allocated in s5p_sdhci_init() but the fields are not initialized.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de --- drivers/mmc/s5p_sdhci.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 4db51d6..e9c43a9 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -76,6 +76,7 @@ static int s5p_sdhci_core_init(struct sdhci_host *host) host->set_control_reg = &s5p_sdhci_set_control_reg; host->set_clock = set_mmc_clk;
+ host->host_caps = 0; if (host->bus_width == 8) host->host_caps |= MMC_MODE_8BIT;

Hi Tobias,
The sdhci_host struct is allocated in s5p_sdhci_init() but the fields are not initialized.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 4db51d6..e9c43a9 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -76,6 +76,7 @@ static int s5p_sdhci_core_init(struct sdhci_host *host) host->set_control_reg = &s5p_sdhci_set_control_reg; host->set_clock = set_mmc_clk;
- host->host_caps = 0; if (host->bus_width == 8) host->host_caps |= MMC_MODE_8BIT;
Acked-by: Lukasz Majewski l.majewski@samsung.com

exynos_mmc_init() always returns zero, so for the caller it looks like it never fails.
Correct this by returning the error code of process_nodes(). For process_nodes() do something similar and return early when do_sdhci_init() fails.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de --- drivers/mmc/s5p_sdhci.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index e9c43a9..bc2102a 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -187,7 +187,11 @@ static int process_nodes(const void *blob, int node_list[], int count) printf("%s: failed to decode dev %d\n", __func__, i); return -1; } - do_sdhci_init(host); + + if (do_sdhci_init(host)) { + printf("%s: failed to initialize dev %d\n", __func__, i); + return -2; + } } return 0; } @@ -201,8 +205,6 @@ int exynos_mmc_init(const void *blob) COMPAT_SAMSUNG_EXYNOS_MMC, node_list, SDHCI_MAX_HOSTS);
- process_nodes(blob, node_list, count); - - return 0; + return process_nodes(blob, node_list, count); } #endif

Hi Tobias,
exynos_mmc_init() always returns zero, so for the caller it looks like it never fails.
Correct this by returning the error code of process_nodes(). For process_nodes() do something similar and return early when do_sdhci_init() fails.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index e9c43a9..bc2102a 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -187,7 +187,11 @@ static int process_nodes(const void *blob, int node_list[], int count) printf("%s: failed to decode dev %d\n", __func__, i); return -1; }
do_sdhci_init(host);
if (do_sdhci_init(host)) {
printf("%s: failed to initialize dev %d\n",
__func__, i);
return -2;
IMHO, it would be better to write this code as follows:
ret = do_sdhci_init(host); if (ret) {
printf(); return ret; } In the above code you read the exact return code from do_sdhci_init() and then you pass it to upper layer.
Returning only -2 is far less informational.
} return 0;}
} @@ -201,8 +205,6 @@ int exynos_mmc_init(const void *blob) COMPAT_SAMSUNG_EXYNOS_MMC, node_list, SDHCI_MAX_HOSTS);
- process_nodes(blob, node_list, count);
- return 0;
- return process_nodes(blob, node_list, count);
} #endif

Hi.
On 09/23/2015 06:39 PM, Lukasz Majewski wrote:
Hi Tobias,
exynos_mmc_init() always returns zero, so for the caller it looks like it never fails.
Correct this by returning the error code of process_nodes(). For process_nodes() do something similar and return early when do_sdhci_init() fails.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index e9c43a9..bc2102a 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -187,7 +187,11 @@ static int process_nodes(const void *blob, int node_list[], int count) printf("%s: failed to decode dev %d\n", __func__, i); return -1; }
do_sdhci_init(host);
if (do_sdhci_init(host)) {
printf("%s: failed to initialize dev %d\n",
__func__, i);
return -2;
IMHO, it would be better to write this code as follows:
ret = do_sdhci_init(host); if (ret) {
printf(); return ret; }
I think it should be replaced to "continue;", not "return ret;" If returned the fail, then next host can't initialize.(if there is next host..) So maybe, it didn't use "return ret".
Best Regards, Jaehoon Chung
In the above code you read the exact return code from do_sdhci_init() and then you pass it to upper layer.
Returning only -2 is far less informational.
} return 0;}
} @@ -201,8 +205,6 @@ int exynos_mmc_init(const void *blob) COMPAT_SAMSUNG_EXYNOS_MMC, node_list, SDHCI_MAX_HOSTS);
- process_nodes(blob, node_list, count);
- return 0;
- return process_nodes(blob, node_list, count);
} #endif

On 23/09/15 18:54, Jaehoon Chung wrote:
Hi.
On 09/23/2015 06:39 PM, Lukasz Majewski wrote:
Hi Tobias,
exynos_mmc_init() always returns zero, so for the caller it looks like it never fails.
Correct this by returning the error code of process_nodes(). For process_nodes() do something similar and return early when do_sdhci_init() fails.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index e9c43a9..bc2102a 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -187,7 +187,11 @@ static int process_nodes(const void *blob, int node_list[], int count) printf("%s: failed to decode dev %d\n", __func__, i); return -1; }
do_sdhci_init(host);
if (do_sdhci_init(host)) {
printf("%s: failed to initialize dev %d\n",
__func__, i);
return -2;
IMHO, it would be better to write this code as follows:
ret = do_sdhci_init(host); if (ret) {
printf(); return ret; }
I think it should be replaced to "continue;", not "return ret;" If returned the fail, then next host can't initialize.(if there is next host..) So maybe, it didn't use "return ret".
Right. If succeeded to initialize at least one host, should not be returned an error.
Thanks, Minkyu Kang.

Hi,
On 09/21/2015 08:18 AM, Tobias Jakobi wrote:
Hello,
currently operation on Exynos4412-based Odroid devices is broken.
The bootloader stops with this message: Card did not respond to voltage select! *** Warning - MMC init failed, using default environment
Which board did you use? and Which cad is failed? SD or eMMC? Your [PATCH 4/4] is strange..so i want to get more information.
If you can share more information, i can explain more exactly about your problem.
Best Regards, Jaehoon Chung
This series fixes error handling in the s5p sdhci driver and the cause of the issue, which is an inverted card detection check.
Thanks goes to Marek Vasut and Sjoerd Simons who helped me on IRC to get this triaged.
With best wishes, Tobias
Tobias Jakobi (4): exynos: Properly initialize host_caps in s5p_sdhci_core_init() exynos: Fix passing of errors in exynos_mmc_init() exynos: be more verbose in process_nodes() exynos: fix and cleanup do_sdhci_init()
drivers/mmc/s5p_sdhci.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)

Hello,
Jaehoon Chung wrote:
Hi,
On 09/21/2015 08:18 AM, Tobias Jakobi wrote:
Hello,
currently operation on Exynos4412-based Odroid devices is broken.
The bootloader stops with this message: Card did not respond to voltage select! *** Warning - MMC init failed, using default environment
Which board did you use? and Which cad is failed? SD or eMMC?
This is an Odroid-X2. I'm booting from SD with a Sandisk Extreme. I don't have any eMMC here.
Actually it was some other person who encountered this issue with his Odroid-X2 and booting from SD. After he made me aware of it, I also updated my u-boot (I was then using a version from May 2015, which works perfectly btw) and encountered the very same issue as him.
Your [PATCH 4/4] is strange..so i want to get more information.
For me it restores proper operation of the bootloader to the board.
With best wishes, Tobias
If you can share more information, i can explain more exactly about your problem.
Best Regards, Jaehoon Chung
This series fixes error handling in the s5p sdhci driver and the cause of the issue, which is an inverted card detection check.
Thanks goes to Marek Vasut and Sjoerd Simons who helped me on IRC to get this triaged.
With best wishes, Tobias
Tobias Jakobi (4): exynos: Properly initialize host_caps in s5p_sdhci_core_init() exynos: Fix passing of errors in exynos_mmc_init() exynos: be more verbose in process_nodes() exynos: fix and cleanup do_sdhci_init()
drivers/mmc/s5p_sdhci.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)

Dear, Tobias.
On 09/21/2015 06:34 PM, Tobias Jakobi wrote:
Hello,
Jaehoon Chung wrote:
Hi,
On 09/21/2015 08:18 AM, Tobias Jakobi wrote:
Hello,
currently operation on Exynos4412-based Odroid devices is broken.
The bootloader stops with this message: Card did not respond to voltage select! *** Warning - MMC init failed, using default environment
Which board did you use? and Which cad is failed? SD or eMMC?
This is an Odroid-X2. I'm booting from SD with a Sandisk Extreme. I don't have any eMMC here.
But if your patch is applied, other exynos4412 and exynos using s5p_sdhci don't work fine for SD-card. This case is the specific X2 board. I think schematic is wrong. (I have checked the Odrodd_X-base_Rev_06.pdf)
Commonly, SD-detect pin is supplied to always-on power. But odroid-x/u board used the same supplier with I/O line. It's wrong.
This is H/W mis-designed problem, not code problem. Well, I will reproduce your problem with X2 board. And if reproduced your problem, I will find other solution, not this. how about?
Actually it was some other person who encountered this issue with his Odroid-X2 and booting from SD. After he made me aware of it, I also updated my u-boot (I was then using a version from May 2015, which works perfectly btw) and encountered the very same issue as him.
Need to consider other SoC. You seems to consider only Odroid based-on exyno4412. Did you test other exynos4412 board?
Best Regards, Jaehoon Chung
Your [PATCH 4/4] is strange..so i want to get more information.
For me it restores proper operation of the bootloader to the board.
With best wishes, Tobias
If you can share more information, i can explain more exactly about your problem.
Best Regards, Jaehoon Chung
This series fixes error handling in the s5p sdhci driver and the cause of the issue, which is an inverted card detection check.
Thanks goes to Marek Vasut and Sjoerd Simons who helped me on IRC to get this triaged.
With best wishes, Tobias
Tobias Jakobi (4): exynos: Properly initialize host_caps in s5p_sdhci_core_init() exynos: Fix passing of errors in exynos_mmc_init() exynos: be more verbose in process_nodes() exynos: fix and cleanup do_sdhci_init()
drivers/mmc/s5p_sdhci.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)

Hello Jaehoon,
Jaehoon Chung wrote:
Dear, Tobias.
On 09/21/2015 06:34 PM, Tobias Jakobi wrote:
Hello,
Jaehoon Chung wrote:
Hi,
On 09/21/2015 08:18 AM, Tobias Jakobi wrote:
Hello,
currently operation on Exynos4412-based Odroid devices is broken.
The bootloader stops with this message: Card did not respond to voltage select! *** Warning - MMC init failed, using default environment
Which board did you use? and Which cad is failed? SD or eMMC?
This is an Odroid-X2. I'm booting from SD with a Sandisk Extreme. I don't have any eMMC here.
But if your patch is applied, other exynos4412 and exynos using s5p_sdhci don't work fine for SD-card. This case is the specific X2 board. I think schematic is wrong. (I have checked the Odrodd_X-base_Rev_06.pdf)
Commonly, SD-detect pin is supplied to always-on power. But odroid-x/u board used the same supplier with I/O line. It's wrong.
I wouldn't know, I don't even know how to properly read these schematics.
This is H/W mis-designed problem, not code problem. Well, I will reproduce your problem with X2 board. And if reproduced your problem, I will find other solution, not this. how about?
Sure, as long as this fixes the issue. Let me know if I can test anything.
Actually it was some other person who encountered this issue with his Odroid-X2 and booting from SD. After he made me aware of it, I also updated my u-boot (I was then using a version from May 2015, which works perfectly btw) and encountered the very same issue as him.
Need to consider other SoC. You seems to consider only Odroid based-on exyno4412. Did you test other exynos4412 board?
I have only one ARM board here, which is the Odroid-X2. Nothing else.
With best wishes, Tobias
Best Regards, Jaehoon Chung
Your [PATCH 4/4] is strange..so i want to get more information.
For me it restores proper operation of the bootloader to the board.
With best wishes, Tobias
If you can share more information, i can explain more exactly about your problem.
Best Regards, Jaehoon Chung
This series fixes error handling in the s5p sdhci driver and the cause of the issue, which is an inverted card detection check.
Thanks goes to Marek Vasut and Sjoerd Simons who helped me on IRC to get this triaged.
With best wishes, Tobias
Tobias Jakobi (4): exynos: Properly initialize host_caps in s5p_sdhci_core_init() exynos: Fix passing of errors in exynos_mmc_init() exynos: be more verbose in process_nodes() exynos: fix and cleanup do_sdhci_init()
drivers/mmc/s5p_sdhci.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)

Hello Tobias,
On 09/21/2015 01:18 AM, Tobias Jakobi wrote:
Hello,
currently operation on Exynos4412-based Odroid devices is broken.
The bootloader stops with this message: Card did not respond to voltage select! *** Warning - MMC init failed, using default environment
This series fixes error handling in the s5p sdhci driver and the cause of the issue, which is an inverted card detection check.
Thanks goes to Marek Vasut and Sjoerd Simons who helped me on IRC to get this triaged.
With best wishes, Tobias
Tobias Jakobi (4): exynos: Properly initialize host_caps in s5p_sdhci_core_init() exynos: Fix passing of errors in exynos_mmc_init() exynos: be more verbose in process_nodes() exynos: fix and cleanup do_sdhci_init()
drivers/mmc/s5p_sdhci.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)
NAK for the series, since the real problem is in device tree parsing, not in s5p_sdhci.c. I will send patches in a moment.
Best regards,
participants (5)
-
Jaehoon Chung
-
Lukasz Majewski
-
Minkyu Kang
-
Przemyslaw Marczak
-
Tobias Jakobi