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

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

[Tobias Jakobi]

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

[U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()

[Tobias Jakobi]

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

[U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()

[Lukasz Majewski]

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

[U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()

[Lukasz Majewski]

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

[U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()

[Jaehoon Chung]

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


> 

[U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()

[Lukasz Majewski]

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

[U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()

[Minkyu Kang]

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.

[U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()

[Tobias Jakobi]

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.
> 

[U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()

[Lukasz Majewski]

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

[U-Boot] [PATCH 4/4] exynos: fix and cleanup do_sdhci_init()

[Przemyslaw Marczak]

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

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

[Lukasz Majewski]

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