[U-Boot] [PATCH 3/4] exynos: be more verbose in process_nodes()

In case sdhci_get_config() or do_sdhci_init() fail, show the error code that was returned.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de --- drivers/mmc/s5p_sdhci.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index bc2102a..6be3609 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -171,7 +171,7 @@ static int sdhci_get_config(const void *blob, int node, struct sdhci_host *host) static int process_nodes(const void *blob, int node_list[], int count) { struct sdhci_host *host; - int i, node; + int i, node, ret;
debug("%s: count = %d\n", __func__, count);
@@ -183,13 +183,15 @@ static int process_nodes(const void *blob, int node_list[], int count)
host = &sdhci_host[i];
- if (sdhci_get_config(blob, node, host)) { - printf("%s: failed to decode dev %d\n", __func__, i); + ret = sdhci_get_config(blob, node, host); + if (ret) { + printf("%s: failed to decode dev %d (%d)\n", __func__, i, ret); return -1; }
- if (do_sdhci_init(host)) { - printf("%s: failed to initialize dev %d\n", __func__, i); + ret = do_sdhci_init(host); + if (ret) { + printf("%s: failed to initialize dev %d (%d)\n", __func__, i, ret); return -2; } }

The CD check is currently inverted. dm_gpio_get_value() returns one when a card is detected. All other values (zero when there is no card, or negative values for the internal errors) indicate failure.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de --- drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 6be3609..bc04370 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -102,29 +102,31 @@ struct sdhci_host sdhci_host[SDHCI_MAX_HOSTS];
static int do_sdhci_init(struct sdhci_host *host) { - int dev_id, flag; - int err = 0; + int dev_id, flag, ret;
flag = host->bus_width == 8 ? PINMUX_FLAG_8BIT_MODE : PINMUX_FLAG_NONE; dev_id = host->index + PERIPH_ID_SDMMC0;
if (dm_gpio_is_valid(&host->pwr_gpio)) { dm_gpio_set_value(&host->pwr_gpio, 1); - err = exynos_pinmux_config(dev_id, flag); - if (err) { + ret = exynos_pinmux_config(dev_id, flag); + if (ret) { debug("MMC not configured\n"); - return err; + return ret; } }
if (dm_gpio_is_valid(&host->cd_gpio)) { - if (dm_gpio_get_value(&host->cd_gpio)) + ret = dm_gpio_get_value(&host->cd_gpio); + if (ret != 1) { + debug("No card detected (%d)\n", ret); return -ENODEV; + }
- err = exynos_pinmux_config(dev_id, flag); - if (err) { + ret = exynos_pinmux_config(dev_id, flag); + if (ret) { printf("external SD not configured\n"); - return err; + return ret; } }

Hi Tobias,
The CD check is currently inverted. dm_gpio_get_value() returns one when a card is detected. All other values (zero when there is no card, or negative values for the internal errors) indicate failure.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 6be3609..bc04370 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -102,29 +102,31 @@ struct sdhci_host sdhci_host[SDHCI_MAX_HOSTS];
static int do_sdhci_init(struct sdhci_host *host) {
- int dev_id, flag;
- int err = 0;
int dev_id, flag, ret;
flag = host->bus_width == 8 ? PINMUX_FLAG_8BIT_MODE :
PINMUX_FLAG_NONE; dev_id = host->index + PERIPH_ID_SDMMC0;
if (dm_gpio_is_valid(&host->pwr_gpio)) { dm_gpio_set_value(&host->pwr_gpio, 1);
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { debug("MMC not configured\n");
return err;
return ret;
} }
if (dm_gpio_is_valid(&host->cd_gpio)) {
if (dm_gpio_get_value(&host->cd_gpio))
ret = dm_gpio_get_value(&host->cd_gpio);
if (ret != 1) {
debug("No card detected (%d)\n", ret); return -ENODEV;
}
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { printf("external SD not configured\n");
return err;
} }return ret;
Reviewed-by: Lukasz Majewski l.majewski@samsung.com

Hi Lukasz,
Hi Tobias,
The CD check is currently inverted. dm_gpio_get_value() returns one when a card is detected. All other values (zero when there is no card, or negative values for the internal errors) indicate failure.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 6be3609..bc04370 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -102,29 +102,31 @@ struct sdhci_host sdhci_host[SDHCI_MAX_HOSTS];
static int do_sdhci_init(struct sdhci_host *host) {
- int dev_id, flag;
- int err = 0;
int dev_id, flag, ret;
flag = host->bus_width == 8 ? PINMUX_FLAG_8BIT_MODE :
PINMUX_FLAG_NONE; dev_id = host->index + PERIPH_ID_SDMMC0;
if (dm_gpio_is_valid(&host->pwr_gpio)) { dm_gpio_set_value(&host->pwr_gpio, 1);
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { debug("MMC not configured\n");
return err;
return ret;
} }
if (dm_gpio_is_valid(&host->cd_gpio)) {
if (dm_gpio_get_value(&host->cd_gpio))
ret = dm_gpio_get_value(&host->cd_gpio);
if (ret != 1) {
debug("No card detected (%d)\n", ret); return -ENODEV;
}
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { printf("external SD not configured\n");
return err;
} }return ret;
Reviewed-by: Lukasz Majewski l.majewski@samsung.com
Sorry, I was too fast. I've read the whole thread and I can confirm that your change would break Trats board.
I hope that we will come up with proper solution.

Hi,
On 09/23/2015 06:48 PM, Lukasz Majewski wrote:
Hi Lukasz,
Hi Tobias,
The CD check is currently inverted. dm_gpio_get_value() returns one when a card is detected. All other values (zero when there is no card, or negative values for the internal errors) indicate failure.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 6be3609..bc04370 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -102,29 +102,31 @@ struct sdhci_host sdhci_host[SDHCI_MAX_HOSTS];
static int do_sdhci_init(struct sdhci_host *host) {
- int dev_id, flag;
- int err = 0;
int dev_id, flag, ret;
flag = host->bus_width == 8 ? PINMUX_FLAG_8BIT_MODE :
PINMUX_FLAG_NONE; dev_id = host->index + PERIPH_ID_SDMMC0;
if (dm_gpio_is_valid(&host->pwr_gpio)) { dm_gpio_set_value(&host->pwr_gpio, 1);
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { debug("MMC not configured\n");
return err;
return ret;
} }
if (dm_gpio_is_valid(&host->cd_gpio)) {
if (dm_gpio_get_value(&host->cd_gpio))
ret = dm_gpio_get_value(&host->cd_gpio);
if (ret != 1) {
debug("No card detected (%d)\n", ret); return -ENODEV;
}
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { printf("external SD not configured\n");
return err;
} }return ret;
Reviewed-by: Lukasz Majewski l.majewski@samsung.com
Sorry, I was too fast. I've read the whole thread and I can confirm that your change would break Trats board. ry I hope that we will come up with proper solution.
We can use the "cd-inverted" property like linux-kernel. Then i think that all board based on exynos4412 can cover. how about?
Best Regards, Jaehoon Chung

Hi Jaehoon,
Hi,
On 09/23/2015 06:48 PM, Lukasz Majewski wrote:
Hi Lukasz,
Hi Tobias,
The CD check is currently inverted. dm_gpio_get_value() returns one when a card is detected. All other values (zero when there is no card, or negative values for the internal errors) indicate failure.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 6be3609..bc04370 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -102,29 +102,31 @@ struct sdhci_host sdhci_host[SDHCI_MAX_HOSTS]; static int do_sdhci_init(struct sdhci_host *host) {
- int dev_id, flag;
- int err = 0;
int dev_id, flag, ret;
flag = host->bus_width == 8 ? PINMUX_FLAG_8BIT_MODE :
PINMUX_FLAG_NONE; dev_id = host->index + PERIPH_ID_SDMMC0;
if (dm_gpio_is_valid(&host->pwr_gpio)) { dm_gpio_set_value(&host->pwr_gpio, 1);
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { debug("MMC not configured\n");
return err;
return ret;
} }
if (dm_gpio_is_valid(&host->cd_gpio)) {
if (dm_gpio_get_value(&host->cd_gpio))
ret = dm_gpio_get_value(&host->cd_gpio);
if (ret != 1) {
debug("No card detected (%d)\n", ret); return -ENODEV;
}
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { printf("external SD not configured\n");
return err;
} }return ret;
Reviewed-by: Lukasz Majewski l.majewski@samsung.com
Sorry, I was too fast. I've read the whole thread and I can confirm that your change would break Trats board. ry I hope that we will come up with proper solution.
We can use the "cd-inverted" property like linux-kernel. Then i think that all board based on exynos4412 can cover. how about?
I'm ok with this approach.
Best Regards, Jaehoon Chung

On 23/09/15 18:59, Jaehoon Chung wrote:
Hi,
On 09/23/2015 06:48 PM, Lukasz Majewski wrote:
Hi Lukasz,
Hi Tobias,
The CD check is currently inverted. dm_gpio_get_value() returns one when a card is detected. All other values (zero when there is no card, or negative values for the internal errors) indicate failure.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 6be3609..bc04370 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -102,29 +102,31 @@ struct sdhci_host sdhci_host[SDHCI_MAX_HOSTS];
static int do_sdhci_init(struct sdhci_host *host) {
- int dev_id, flag;
- int err = 0;
int dev_id, flag, ret;
flag = host->bus_width == 8 ? PINMUX_FLAG_8BIT_MODE :
PINMUX_FLAG_NONE; dev_id = host->index + PERIPH_ID_SDMMC0;
if (dm_gpio_is_valid(&host->pwr_gpio)) { dm_gpio_set_value(&host->pwr_gpio, 1);
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { debug("MMC not configured\n");
return err;
return ret;
} }
if (dm_gpio_is_valid(&host->cd_gpio)) {
if (dm_gpio_get_value(&host->cd_gpio))
ret = dm_gpio_get_value(&host->cd_gpio);
if (ret != 1) {
debug("No card detected (%d)\n", ret); return -ENODEV;
}
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { printf("external SD not configured\n");
return err;
} }return ret;
Reviewed-by: Lukasz Majewski l.majewski@samsung.com
Sorry, I was too fast. I've read the whole thread and I can confirm that your change would break Trats board. ry I hope that we will come up with proper solution.
We can use the "cd-inverted" property like linux-kernel. Then i think that all board based on exynos4412 can cover. how about?
I agree :)
Thanks, Minkyu Kang.

Hello,
Minkyu Kang wrote:
On 23/09/15 18:59, Jaehoon Chung wrote:
Hi,
On 09/23/2015 06:48 PM, Lukasz Majewski wrote:
Hi Lukasz,
Hi Tobias,
The CD check is currently inverted. dm_gpio_get_value() returns one when a card is detected. All other values (zero when there is no card, or negative values for the internal errors) indicate failure.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 6be3609..bc04370 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -102,29 +102,31 @@ struct sdhci_host sdhci_host[SDHCI_MAX_HOSTS];
static int do_sdhci_init(struct sdhci_host *host) {
- int dev_id, flag;
- int err = 0;
int dev_id, flag, ret;
flag = host->bus_width == 8 ? PINMUX_FLAG_8BIT_MODE :
PINMUX_FLAG_NONE; dev_id = host->index + PERIPH_ID_SDMMC0;
if (dm_gpio_is_valid(&host->pwr_gpio)) { dm_gpio_set_value(&host->pwr_gpio, 1);
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { debug("MMC not configured\n");
return err;
return ret;
} }
if (dm_gpio_is_valid(&host->cd_gpio)) {
if (dm_gpio_get_value(&host->cd_gpio))
ret = dm_gpio_get_value(&host->cd_gpio);
if (ret != 1) {
debug("No card detected (%d)\n", ret); return -ENODEV;
}
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { printf("external SD not configured\n");
return err;
} }return ret;
Reviewed-by: Lukasz Majewski l.majewski@samsung.com
Sorry, I was too fast. I've read the whole thread and I can confirm that your change would break Trats board. ry I hope that we will come up with proper solution.
We can use the "cd-inverted" property like linux-kernel. Then i think that all board based on exynos4412 can cover. how about?
I don't think this is going to work. I asked someone else to check current upstream u-boot on his Odroid-U3 and he has confirmed that it boots fine from SD card.
So this seems to be a X2 specific thing. However the 'cd-inverted' property is present in exynos4412-odroid-common.dtsi, so it applies to both U3 and X2 in the kernel.
So either the kernel is wrong here, or this is board difference which can't be derived from 'cd-inverted'.
With best wishes, Tobias
I agree :)
Thanks, Minkyu Kang.

Hi Tobias,
Hello,
Minkyu Kang wrote:
On 23/09/15 18:59, Jaehoon Chung wrote:
Hi,
On 09/23/2015 06:48 PM, Lukasz Majewski wrote:
Hi Lukasz,
Hi Tobias,
The CD check is currently inverted. dm_gpio_get_value() returns one when a card is detected. All other values (zero when there is no card, or negative values for the internal errors) indicate failure.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 6be3609..bc04370 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -102,29 +102,31 @@ struct sdhci_host sdhci_host[SDHCI_MAX_HOSTS]; static int do_sdhci_init(struct sdhci_host *host) {
- int dev_id, flag;
- int err = 0;
int dev_id, flag, ret;
flag = host->bus_width == 8 ? PINMUX_FLAG_8BIT_MODE :
PINMUX_FLAG_NONE; dev_id = host->index + PERIPH_ID_SDMMC0;
if (dm_gpio_is_valid(&host->pwr_gpio)) { dm_gpio_set_value(&host->pwr_gpio, 1);
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { debug("MMC not configured\n");
return err;
return ret;
} }
if (dm_gpio_is_valid(&host->cd_gpio)) {
if (dm_gpio_get_value(&host->cd_gpio))
ret = dm_gpio_get_value(&host->cd_gpio);
if (ret != 1) {
debug("No card detected (%d)\n", ret); return -ENODEV;
}
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { printf("external SD not configured\n");
return err;
} }return ret;
Reviewed-by: Lukasz Majewski l.majewski@samsung.com
Sorry, I was too fast. I've read the whole thread and I can confirm that your change would break Trats board. ry I hope that we will come up with proper solution.
We can use the "cd-inverted" property like linux-kernel. Then i think that all board based on exynos4412 can cover. how about?
I don't think this is going to work. I asked someone else to check current upstream u-boot on his Odroid-U3 and he has confirmed that it boots fine from SD card.
Have you tried the newest mainline? I'm struggling to debrick my Trats board after some eMMC early misconfiguration.
For now I can confirm that it works with v2015.07.
So this seems to be a X2 specific thing. However the 'cd-inverted' property is present in exynos4412-odroid-common.dtsi, so it applies to both U3 and X2 in the kernel.
So either the kernel is wrong here, or this is board difference which can't be derived from 'cd-inverted'.
With best wishes, Tobias
I agree :)
Thanks, Minkyu Kang.

Hello Tobias,
On 09/21/2015 01:54 AM, Tobias Jakobi wrote:
The CD check is currently inverted. dm_gpio_get_value() returns one when a card is detected. All other values (zero when there is no card, or negative values for the internal errors) indicate failure.
If you want revert the GPIO state, please modify the phandle in device-tree and then in the code.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 6be3609..bc04370 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -102,29 +102,31 @@ struct sdhci_host sdhci_host[SDHCI_MAX_HOSTS];
static int do_sdhci_init(struct sdhci_host *host) {
- int dev_id, flag;
- int err = 0;
int dev_id, flag, ret;
flag = host->bus_width == 8 ? PINMUX_FLAG_8BIT_MODE : PINMUX_FLAG_NONE; dev_id = host->index + PERIPH_ID_SDMMC0;
if (dm_gpio_is_valid(&host->pwr_gpio)) { dm_gpio_set_value(&host->pwr_gpio, 1);
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { debug("MMC not configured\n");
return err;
return ret;
} }
if (dm_gpio_is_valid(&host->cd_gpio)) {
if (dm_gpio_get_value(&host->cd_gpio))
Please don't revert this GPIO. This part of code was correct. There is another source of broken SD card detect. It's about device-tree parsing. I will send a patches today.
ret = dm_gpio_get_value(&host->cd_gpio);
if (ret != 1) {
debug("No card detected (%d)\n", ret); return -ENODEV;
}
err = exynos_pinmux_config(dev_id, flag);
if (err) {
ret = exynos_pinmux_config(dev_id, flag);
if (ret) { printf("external SD not configured\n");
return err;
} }return ret;
Best regards,

Hi Tobias,
ret = sdhci_get_config(blob, node, host);
if (ret) {
printf("%s: failed to decode dev %d (%d)\n",
__func__, i, ret); return -1;
My only commit here is regarding leaving return -1 as it is now. I think, that it would be better to return ret.
participants (5)
-
Jaehoon Chung
-
Lukasz Majewski
-
Minkyu Kang
-
Przemyslaw Marczak
-
Tobias Jakobi