Hi Simon
On 05/17/2017 03:38 AM, Simon Glass wrote:
Hi Patrice,
On 15 May 2017 at 03:21, Patrice CHOTARD patrice.chotard@st.com wrote:
Hi Simon
On 05/15/2017 05:02 AM, Simon Glass wrote:
Hi Patrice,
On 10 May 2017 at 10:09, patrice.chotard@st.com wrote:
From: Patrice Chotard patrice.chotard@st.com
Signed-off-by: Patrice Chotard patrice.chotard@st.com Reviewed-by: Jaehoon Chung jh80.chung@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 sjg@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:
- 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"
I am wondering if the value of the 'resets' property is 'softtreset'?
"softreset" is the value of the "reset-names" property.
resets = <&softreset STIH407_MMC1_SOFTRESET>; reset-names = "softreset";
But i am thinking to simplify this part.
Patrice
If so, you could perhaps use that value instead of the string 'softreset'.
- 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