Hi Simon,
On 8/4/20 00:14, Simon Glass wrote:
Hi Walter,
On Tue, 7 Apr 2020 at 14:05, Walter Lozano <walter.loz...@collabora.com> 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<walter.loz...@collabora.com> wrote:
Signed-off-by: Walter Lozano<walter.loz...@collabora.com>
---
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?
I'm sure I' understand the idea you try to share with me. Sorry, I
understand that one limitation of the current implementation of
OF_PLATDATA is the fact that the U_BOOT_DRIVER name should match the one
used as first entry in DT. As in particular this driver has several
compatible
{ .compatible = "fsl,imx53-esdhc", },
{ .compatible = "fsl,imx6ul-usdhc", },
{ .compatible = "fsl,imx6sx-usdhc", },
{ .compatible = "fsl,imx6sl-usdhc", },
{ .compatible = "fsl,imx6q-usdhc", },
{ .compatible = "fsl,imx7d-usdhc", .data =
(ulong)&usdhc_imx7d_data,},
{ .compatible = "fsl,imx7ulp-usdhc", },
{ .compatible = "fsl,imx8qm-usdhc", .data =
(ulong)&usdhc_imx8qm_data,},
{ .compatible = "fsl,imx8mm-usdhc", .data =
(ulong)&usdhc_imx8qm_data,},
{ .compatible = "fsl,imx8mn-usdhc", .data =
(ulong)&usdhc_imx8qm_data,},
{ .compatible = "fsl,imx8mq-usdhc", .data =
(ulong)&usdhc_imx8qm_data,},
{ .compatible = "fsl,imxrt-usdhc", },
and in order to create a general solution, we need to search for the
compatible string instead of matching for driver name.
Could you please elaborate a bit more your suggestion in order to
understand your approach.
Thanks in advance.
