Hi Simon On 05/15/2017 05:02 AM, Simon Glass wrote: > Hi Patrice, > > On 10 May 2017 at 10:09, <patrice.chot...@st.com> wrote: >> From: Patrice Chotard <patrice.chot...@st.com> >> >> Signed-off-by: Patrice Chotard <patrice.chot...@st.com> >> Reviewed-by: Jaehoon Chung <jh80.ch...@samsung.com> >> --- >> >> v5: _ none >> v4: _ none >> v3: _ none >> v2: _ none >> >> >> drivers/mmc/sti_sdhci.c | 31 ++++++++++++++++++++++--------- >> 1 file changed, 22 insertions(+), 9 deletions(-) >> > > Reviewed-by: Simon Glass <s...@chromium.org> > > This is fine as is. But please see questions below. > >> diff --git a/drivers/mmc/sti_sdhci.c b/drivers/mmc/sti_sdhci.c >> index d6c4d67..8b1b2c0 100644 >> --- a/drivers/mmc/sti_sdhci.c >> +++ b/drivers/mmc/sti_sdhci.c >> @@ -8,6 +8,7 @@ >> #include <common.h> >> #include <dm.h> >> #include <mmc.h> >> +#include <reset-uclass.h> >> #include <sdhci.h> >> #include <asm/arch/sdhci.h> >> >> @@ -16,6 +17,7 @@ DECLARE_GLOBAL_DATA_PTR; >> struct sti_sdhci_plat { >> struct mmc_config cfg; >> struct mmc mmc; >> + struct reset_ctl reset; >> int instance; >> }; >> >> @@ -37,17 +39,19 @@ struct sti_sdhci_plat { >> * W/o these settings the SDHCI could configure and use the embedded >> controller >> * with limited features. >> */ >> -static void sti_mmc_core_config(struct udevice *dev) >> +static int sti_mmc_core_config(struct udevice *dev) >> { >> struct sti_sdhci_plat *plat = dev_get_platdata(dev); >> struct sdhci_host *host = dev_get_priv(dev); >> - unsigned long *sysconf; >> + int ret; >> >> /* only MMC1 has a reset line */ >> if (plat->instance) { >> - sysconf = (unsigned long *)(STIH410_SYSCONF5_BASE + >> - ST_MMC_CCONFIG_REG_5); >> - generic_set_bit(SYSCONF_MMC1_ENABLE_BIT, sysconf); >> + ret = reset_deassert(&plat->reset); >> + if (ret < 0) { >> + error("MMC1 deassert failed: %d", ret); >> + return ret; >> + } >> } >> >> writel(STI_FLASHSS_MMC_CORE_CONFIG_1, >> @@ -66,6 +70,8 @@ static void sti_mmc_core_config(struct udevice *dev) >> } >> writel(STI_FLASHSS_MMC_CORE_CONFIG4, >> host->ioaddr + FLASHSS_MMC_CORE_CONFIG_4); >> + >> + return 0; >> } >> >> static int sti_sdhci_probe(struct udevice *dev) >> @@ -80,13 +86,20 @@ static int sti_sdhci_probe(struct udevice *dev) >> * MMC0 is wired to the SD slot, >> * MMC1 is wired on the high speed connector >> */ >> - >> - if (fdt_getprop(gd->fdt_blob, dev_of_offset(dev), "resets", NULL)) >> + if (fdt_getprop(gd->fdt_blob, dev_of_offset(dev), "resets", NULL)) { >> plat->instance = 1; >> - else >> + ret = reset_get_by_name(dev, "softreset", &plat->reset); > > Two questions: > > 1. The name "softreset" is in the "resets" property, isn't it? If so, > can you use it instead of hard-coding "softreset" here?
Sorry, i didn't understand what you mean by "can you use it instead of hard-coding "softreset" here" > 2. Can you not call reset_get_by_name() and deal with the error return > (-ENOENT I think) if there is no such reset? > >> + if (ret) { >> + error("can't get reset for %s (%d)", dev->name, ret); >> + return ret; >> + } >> + } else { >> plat->instance = 0; >> + } >> >> - sti_mmc_core_config(dev); >> + ret = sti_mmc_core_config(dev); >> + if (ret) >> + return ret; >> >> host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD | >> SDHCI_QUIRK_32BIT_DMA_ADDR | >> -- >> 1.9.1 >> > > Regards, > Simon > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot