[U-Boot] [PATCH v3 4/4] exynos: more debug and cleanup in do_sdhci_init()
Przemyslaw Marczak
p.marczak at samsung.com
Wed Oct 28 14:46:43 CET 2015
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>;
}
> 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.
>
> 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.
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.
>
> 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,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
More information about the U-Boot
mailing list