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?

+U_BOOT_DRIVER(fsl_usdhc) = {
+       .name   = "fsl_imx6q_usdhc",
+       .id     = UCLASS_MMC,
+       .ops    = &fsl_esdhc_ops,
+#if CONFIG_IS_ENABLED(BLK)
+       .bind   = fsl_esdhc_bind,
+#endif
+       .probe  = fsl_esdhc_probe,
+       .platdata_auto_alloc_size = sizeof(struct fsl_esdhc_plat),
+       .priv_auto_alloc_size = sizeof(struct fsl_esdhc_priv),
+};
+#endif
  #endif
--
2.20.1

Regards,
Simon

Reply via email to