| Message ID | 1444045673-31770-4-git-send-email-tjakobi@math.uni-bielefeld.de |
|---|---|
| State | Accepted |
| Delegated to: | Minkyu Kang |
| Headers | show |
On 05/10/15 20:47, Tobias Jakobi wrote: > Add more debug printfs in do_sdhci_init() for calls > that can potentially fail. > > Acked-by: Przemyslaw Marczak <p.marczak@samsung.com> > Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> > --- > drivers/mmc/s5p_sdhci.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > applied to u-boot-samsung. Thanks, Minkyu Kang.
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@samsung.com> > 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 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 > > - 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; > } > } > >
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@samsung.com> >> 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 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. 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,
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; + } - 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; } }