Hi Walter, On Tue, 7 Apr 2020 at 14:05, Walter Lozano <[email protected]> wrote: > > Hi Simon, > > Thank you for taking the time to review this series. > > On 6/4/20 00:42, Simon Glass wrote: > > Hi Walter, > > > > On Sun, 29 Mar 2020 at 21:32, Walter Lozano<[email protected]> > > wrote: > >> Signed-off-by: Walter Lozano<[email protected]> > >> --- > >> drivers/mmc/fsl_esdhc_imx.c | 46 +++++++++++++++++++++++++++++++++---- > >> 1 file changed, 42 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c > >> index 4900498e9b..761a4b46e9 100644 > >> --- a/drivers/mmc/fsl_esdhc_imx.c > >> +++ b/drivers/mmc/fsl_esdhc_imx.c > >> @@ -29,6 +29,8 @@ > >> #include <dm.h> > >> #include <asm-generic/gpio.h> > >> #include <dm/pinctrl.h> > >> +#include <dt-structs.h> > >> +#include <mapmem.h> > >> > >> #if !CONFIG_IS_ENABLED(BLK) > >> #include "mmc_private.h" > >> @@ -98,6 +100,11 @@ struct fsl_esdhc { > >> }; > >> > >> struct fsl_esdhc_plat { > >> +#if CONFIG_IS_ENABLED(OF_PLATDATA) > >> + /* Put this first since driver model will copy the data here */ > >> + struct dtd_fsl_imx6q_usdhc dtplat; > >> +#endif > >> + > >> struct mmc_config cfg; > >> struct mmc mmc; > >> }; > >> @@ -1377,14 +1384,18 @@ static int fsl_esdhc_probe(struct udevice *dev) > >> struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); > >> struct fsl_esdhc_plat *plat = dev_get_platdata(dev); > >> struct fsl_esdhc_priv *priv = dev_get_priv(dev); > >> +#if !CONFIG_IS_ENABLED(OF_PLATDATA) > >> const void *fdt = gd->fdt_blob; > >> int node = dev_of_offset(dev); > >> + fdt_addr_t addr; > >> +#else > >> + struct dtd_fsl_imx6q_usdhc *dtplat = &plat->dtplat; > >> +#endif > >> struct esdhc_soc_data *data = > >> (struct esdhc_soc_data *)dev_get_driver_data(dev); > >> #if CONFIG_IS_ENABLED(DM_REGULATOR) > >> struct udevice *vqmmc_dev; > >> #endif > >> - fdt_addr_t addr; > >> unsigned int val; > >> struct mmc *mmc; > >> #if !CONFIG_IS_ENABLED(BLK) > >> @@ -1392,14 +1403,23 @@ static int fsl_esdhc_probe(struct udevice *dev) > >> #endif > >> int ret; > >> > >> +#if CONFIG_IS_ENABLED(OF_PLATDATA) > >> + priv->esdhc_regs = map_sysmem(dtplat->reg[0], dtplat->reg[1]); > >> + val = plat->dtplat.bus_width; > >> + if (val == 8) > >> + priv->bus_width = 8; > >> + else if (val == 4) > >> + priv->bus_width = 4; > >> + else > >> + priv->bus_width = 1; > >> + priv->non_removable = 1; > >> +#else > >> addr = dev_read_addr(dev); > >> if (addr == FDT_ADDR_T_NONE) > >> return -EINVAL; > >> priv->esdhc_regs = (struct fsl_esdhc *)addr; > >> priv->dev = dev; > >> priv->mode = -1; > >> - if (data) > >> - priv->flags = data->flags; > >> > >> val = dev_read_u32_default(dev, "bus-width", -1); > >> if (val == 8) > >> @@ -1462,7 +1482,9 @@ static int fsl_esdhc_probe(struct udevice *dev) > >> priv->vs18_enable = 1; > >> } > >> #endif > >> - > >> +#endif > >> + if (data) > >> + priv->flags = data->flags; > >> /* > >> * TODO: > >> * Because lack of clk driver, if SDHC clk is not enabled, > >> @@ -1513,9 +1535,11 @@ static int fsl_esdhc_probe(struct udevice *dev) > >> return ret; > >> } > >> > >> +#if !CONFIG_IS_ENABLED(OF_PLATDATA) > > Maybe can use if() for this one? > > Thank you for the suggestion > > >> ret = mmc_of_parse(dev, &plat->cfg); > >> if (ret) > >> return ret; > >> +#endif > >> > >> mmc = &plat->mmc; > >> mmc->cfg = &plat->cfg; > >> @@ -1648,4 +1672,18 @@ U_BOOT_DRIVER(fsl_esdhc) = { > >> .platdata_auto_alloc_size = sizeof(struct fsl_esdhc_plat), > >> .priv_auto_alloc_size = sizeof(struct fsl_esdhc_priv), > >> }; > >> + > >> +#if CONFIG_IS_ENABLED(OF_PLATDATA) > > Don't you already have a U_BOOT_DRIVER declaration? > > > > You may need to change the name of your existing driver though (see > > of-platdata docs). > > > > So if it is because of that, please add a comment. > > I have my doubts regarding this issue. As I see, this driver is used by > many different DT with different compatible strings, so I'm thinking in > trying to find a more generic approach. Would it be useful to have a > more elaborated solution searching for the compatible string when > matching drivers with device?
Yes I think so. Actually searching for a string is not great anyway. I wonder if we can use the linker-list idea in the previous email somehow? Regards, SImon
