[U-Boot] [PATCH v3 4/4] exynos: more debug and cleanup in do_sdhci_init()
Jaehoon Chung
jh80.chung at gmail.com
Wed Oct 28 15:36:47 CET 2015
Hi, Przemyslaw.
On 10/28/2015 10:46 PM, Przemyslaw Marczak wrote:
> Hello Jaehoon,
>
> On 10/28/2015 01:30 PM, Jaehoon Chung wrote:
>> Hi, Przemyslaw.
>>
>> On 10/28/2015 08:33 PM, Przemyslaw Marczak wrote:
>>> Hello Jaehoon,
>>>
>>> On 10/28/2015 07:33 AM, Jaehoon Chung wrote:
>>>> Hi, All.
>>>>
>>>> On 10/05/2015 08:47 PM, Tobias Jakobi wrote:
>>>>> Add more debug printfs in do_sdhci_init() for calls
>>>>> that can potentially fail.
>>>>>
>>>>> Acked-by: Przemyslaw Marczak <p.marczak at samsung.com>
>>>>> Signed-off-by: Tobias Jakobi <tjakobi at 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 b203bee..15ecfee 100644
>>>>> --- a/drivers/mmc/s5p_sdhci.c
>>>>> +++ b/drivers/mmc/s5p_sdhci.c
>>>>> @@ -101,29 +101,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) {
>>>>> + debug("no SD card detected (%d)\n", ret);
>>>>> return -ENODEV;
>>>>> + }
>>>>
>>>> This patch was already applied. But i didn't know why used "ret" at here.
>>>> If cd-gpio is active-high, this should be always returned "no SD card detected".
>>>> Even if commonly cd-gpio is active-low, we don't know whether cd-gpio is active-low or not.
>>>>
>>>> And dm_gpio_get_value() should be returned error for only one case.
>>>>
>>>> Best Regards,
>>>> Jaehoon Chung
>>>>
>>>
>>> Could you precise, where is the problem exactly? The active low or high can be set in device tree, so the code can be still common.
>>
>> How can active low or high be set in device tree? I can't find any information.
>>
>
> It is defined here:
> arch/arm/dts/include/dt-bindings/gpio/gpio.h
> and is used by gpio's phandle, e.g.:
> sdhci at 12530000 {
> samsung,bus-width = <4>;
> samsung,timing = <1 2 3>;
> cd-gpios = <&gpk2 2 0>;
> }
Yes, I have checked.
>
>> Anyway, I have tested on Odroid-x2 board. And i have tried to boot with SD-card.
>> In that case, host can't detect SD-card.
>
> Can you also see this error:
> -----------------------------------------------------------
> DRAM: 2 GiB
> __of_translate_address: Bad cell count for gpf0
> __of_translate_address: Bad cell count for gpj0
> __of_translate_address: Bad cell count for gpk0
> __of_translate_address: Bad cell count for gpm0
> __of_translate_address: Bad cell count for gpx0
> LDO20 at VDDQ_EMMC_1.8V: set 1800000 uV; enabling
> LDO22 at VDDQ_EMMC_2.8V: set 2800000 uV; enabling
> LDO21 at TFLASH_2.8V: set 2800000 uV; enabling
> MMC: process_nodes: failed to initialize dev 0 (-19)
> -----------------------------------------------------------
>
> This is caused by this commit:
> commit ef5cd33064f83db6f6cfe774ecdb36e32ac1d232
> dm: core: Enable optional use of fdt_translate_address()
>
> And it's not related to GPIO configuration.
>
> Right now, I'm looking on how to fix it.
Yes, i also found those message. I'm also finding solution.
>
>>
>> MMC: process_nodes: failed to initialize dev 0 (-19)
>>
>> After Enable debug message..
>>
>> no SD card detected (1)
>> process_nodes: failed to initialize dev 0 (-19)
>>
>> no SD card detected (1) -> 1 is just gpio value. it should be to 0.
>> Do you distinguish between 0 and 1? We don't know whether gpio is active-low or high.
>> I don't think that included information.
>>
>> It's just my thinking...other people should be helpful.
>>
>> And I'm making some patches...and below is one of them for detecting SD-card.
>>
>> Subject: [PATCH] mmc: s5p_sdhci: change the flags of cd-gpio to ACTIVE_LOW
>>
>> In case of exynos, "cd-gpio" is commonly used the active-low.
>> So it changed the flags of gpio from GPIOD_IS_IN to GPIOD_ACTIVE_LOW.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung at samsung.com>
>> ---
>> drivers/mmc/s5p_sdhci.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c
>> index 15ecfee..26a8fe8 100644
>> --- a/drivers/mmc/s5p_sdhci.c
>> +++ b/drivers/mmc/s5p_sdhci.c
>> @@ -164,7 +164,7 @@ static int sdhci_get_config(const void *blob, int node, struct sdhci_host *host)
>> gpio_request_by_name_nodev(blob, node, "pwr-gpios", 0, &host->pwr_gpio,
>> GPIOD_IS_OUT);
>> gpio_request_by_name_nodev(blob, node, "cd-gpios", 0, &host->cd_gpio,
>> - GPIOD_IS_IN);
>> + GPIOD_ACTIVE_LOW);
>
> This change, is a bad idea.
Yep, it's not good idea.
I'm not sure, cd-gpio pin has to set GPIOD_IS_IN. I can't find the any information about gpio direction.
Tomorrow, when i go to the office, i will check.
>
> We uses this flag to set the direction (input/output), and also the flag GPIOD_ACTIVE_LOW is checked for output only in this function.
>
> Your commit fixes the SD detection issue by a mistake.
>
> I will send the fix in a moment, then you will see what is wrong.
Well, i didn't minutely see u-boot code during long time.
So i may miss something..I will check more for u-boot code.
Thanks for pointing out. :)
Best Regards,
Jaehoon Chung
>
>>
>> return 0;
>> }
>> @@ -187,7 +187,8 @@ static int process_nodes(const void *blob, int node_list[], int count)
>>
>> ret = sdhci_get_config(blob, node, host);
>> if (ret) {
>> - printf("%s: failed to decode dev %d (%d)\n", __func__, i, ret);
>> + printf("%s: failed to decode dev %d (%d)\n",
>> + __func__, i, ret);
>> failed++;
>> continue;
>> }
>> --
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>
>>>
>>> And the ret value can inform, if card is not detected or the error is in GPIO subsystem(ret < 0). So where is the problem?
>>>
>>>>>
>>>>> - 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,
>>
>>
>
> Best regards,
More information about the U-Boot
mailing list