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 <[email protected]> >>>>> Signed-off-by: Tobias Jakobi <[email protected]> >>>>> --- >>>>> 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@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@VDDQ_EMMC_1.8V: set 1800000 uV; enabling > LDO22@VDDQ_EMMC_2.8V: set 2800000 uV; enabling > LDO21@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 <[email protected]> >> --- >> 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, _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
