* [U-Boot] [PATCH 3/4] exynos: be more verbose in process_nodes()
@ 2015-09-20 23:54 Tobias Jakobi
2015-09-20 23:54 ` [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init() Tobias Jakobi
2015-09-23 9:41 ` [U-Boot] [PATCH 3/4] exynos: be more verbose in process_nodes() Lukasz Majewski
0 siblings, 2 replies; 11+ messages in thread
From: Tobias Jakobi @ 2015-09-20 23:54 UTC (permalink / raw)
To: u-boot
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;
}
}
--
2.0.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()
2015-09-20 23:54 [U-Boot] [PATCH 3/4] exynos: be more verbose in process_nodes() Tobias Jakobi
@ 2015-09-20 23:54 ` Tobias Jakobi
2015-09-23 9:44 ` Lukasz Majewski
2015-09-24 15:05 ` Przemyslaw Marczak
2015-09-23 9:41 ` [U-Boot] [PATCH 3/4] exynos: be more verbose in process_nodes() Lukasz Majewski
1 sibling, 2 replies; 11+ messages in thread
From: Tobias Jakobi @ 2015-09-20 23:54 UTC (permalink / raw)
To: u-boot
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;
}
}
--
2.0.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 3/4] exynos: be more verbose in process_nodes()
2015-09-20 23:54 [U-Boot] [PATCH 3/4] exynos: be more verbose in process_nodes() Tobias Jakobi
2015-09-20 23:54 ` [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init() Tobias Jakobi
@ 2015-09-23 9:41 ` Lukasz Majewski
1 sibling, 0 replies; 11+ messages in thread
From: Lukasz Majewski @ 2015-09-23 9:41 UTC (permalink / raw)
To: u-boot
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.
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()
2015-09-20 23:54 ` [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init() Tobias Jakobi
@ 2015-09-23 9:44 ` Lukasz Majewski
2015-09-23 9:48 ` Lukasz Majewski
2015-09-24 15:05 ` Przemyslaw Marczak
1 sibling, 1 reply; 11+ messages in thread
From: Lukasz Majewski @ 2015-09-23 9:44 UTC (permalink / raw)
To: u-boot
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>
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()
2015-09-23 9:44 ` Lukasz Majewski
@ 2015-09-23 9:48 ` Lukasz Majewski
2015-09-23 9:59 ` Jaehoon Chung
0 siblings, 1 reply; 11+ messages in thread
From: Lukasz Majewski @ 2015-09-23 9:48 UTC (permalink / raw)
To: u-boot
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.
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()
2015-09-23 9:48 ` Lukasz Majewski
@ 2015-09-23 9:59 ` Jaehoon Chung
2015-09-23 10:06 ` Lukasz Majewski
2015-09-23 10:06 ` Minkyu Kang
0 siblings, 2 replies; 11+ messages in thread
From: Jaehoon Chung @ 2015-09-23 9:59 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()
2015-09-23 9:59 ` Jaehoon Chung
@ 2015-09-23 10:06 ` Lukasz Majewski
2015-09-23 10:06 ` Minkyu Kang
1 sibling, 0 replies; 11+ messages in thread
From: Lukasz Majewski @ 2015-09-23 10:06 UTC (permalink / raw)
To: u-boot
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
>
>
> >
>
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()
2015-09-23 9:59 ` Jaehoon Chung
2015-09-23 10:06 ` Lukasz Majewski
@ 2015-09-23 10:06 ` Minkyu Kang
2015-09-24 12:48 ` Tobias Jakobi
1 sibling, 1 reply; 11+ messages in thread
From: Minkyu Kang @ 2015-09-23 10:06 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()
2015-09-23 10:06 ` Minkyu Kang
@ 2015-09-24 12:48 ` Tobias Jakobi
2015-09-24 13:27 ` Lukasz Majewski
0 siblings, 1 reply; 11+ messages in thread
From: Tobias Jakobi @ 2015-09-24 12:48 UTC (permalink / raw)
To: u-boot
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.
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()
2015-09-24 12:48 ` Tobias Jakobi
@ 2015-09-24 13:27 ` Lukasz Majewski
0 siblings, 0 replies; 11+ messages in thread
From: Lukasz Majewski @ 2015-09-24 13:27 UTC (permalink / raw)
To: u-boot
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.
> >
>
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()
2015-09-20 23:54 ` [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init() Tobias Jakobi
2015-09-23 9:44 ` Lukasz Majewski
@ 2015-09-24 15:05 ` Przemyslaw Marczak
1 sibling, 0 replies; 11+ messages in thread
From: Przemyslaw Marczak @ 2015-09-24 15:05 UTC (permalink / raw)
To: u-boot
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,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-09-24 15:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-20 23:54 [U-Boot] [PATCH 3/4] exynos: be more verbose in process_nodes() Tobias Jakobi
2015-09-20 23:54 ` [U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init() Tobias Jakobi
2015-09-23 9:44 ` Lukasz Majewski
2015-09-23 9:48 ` Lukasz Majewski
2015-09-23 9:59 ` Jaehoon Chung
2015-09-23 10:06 ` Lukasz Majewski
2015-09-23 10:06 ` Minkyu Kang
2015-09-24 12:48 ` Tobias Jakobi
2015-09-24 13:27 ` Lukasz Majewski
2015-09-24 15:05 ` Przemyslaw Marczak
2015-09-23 9:41 ` [U-Boot] [PATCH 3/4] exynos: be more verbose in process_nodes() Lukasz Majewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox