[PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc
@ 2021-11-12 19:15 Sean Anderson
  2021-11-12 19:15 ` [PATCH v2 01/11] mmc: fsl_esdhc_imx: make BLK as hard requirement of DM_MMC Sean Anderson
                   ` (10 more replies)
  0 siblings, 11 replies; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

This series ports some of the patches from fsl_esdhc to fsl_esdhc_imx.
Because these drivers share a common lineage, many of these patches
apply with minor changes. For each one, I have noted the originating
commit in the style of linux stable backports.

In fa33d20749 ("mmc: split fsl_esdhc driver for i.MX"), Yangbo says

> For the two series processors, the eSDHCs are becoming more and more
> different

However, these drivers are still extremely similar; the differences
between them are not major. NXP has not done a good job of porting
patches which apply to both drivers. This causes the fsl_esdhc_imx
driver to rot, as the fsl_esdhc gets more general fixes.  For this
reason, I think that the fsl_esdhc_imx driver should be removed unless
NXP can commit to creating series like this which port patches which
apply to both drivers.

Changes in v2:
- Use a switch statement instead of ifs for max_bus_width
- Only default to 8 bit width when max_bus_width is not set

Sean Anderson (11):
  mmc: fsl_esdhc_imx: make BLK as hard requirement of DM_MMC
  mmc: fsl_esdhc_imx: remove redundant DM_MMC checking
  mmc: fsl_esdhc_imx: fix voltage validation
  mmc: fsl_esdhc_imx: clean up bus width configuration code
  mmc: fsl_esdhc_imx: drop redundant code for non-removable feature
  mmc: fsl_esdhc_imx: fix mmc->clock with actual clock
  mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers
  mmc: fsl_esdhc_imx: use dma-mapping API
  mmc: fsl_esdhc_imx: simplify esdhc_setup_data()
  mmc: fsl_esdhc_imx: replace most #ifdefs by IS_ENABLED()
  mmc: fsl_esdhc_imx: set sysctl register for clock initialization

 drivers/mmc/Kconfig         |   2 +
 drivers/mmc/fsl_esdhc_imx.c | 487 ++++++++++++++----------------------
 include/fsl_esdhc_imx.h     |  14 +-
 3 files changed, 194 insertions(+), 309 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 01/11] mmc: fsl_esdhc_imx: make BLK as hard requirement of DM_MMC
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:35   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 02/11] mmc: fsl_esdhc_imx: remove redundant DM_MMC checking Sean Anderson
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit 41dec2fe99512e941261594f522b2e7d485c314b ]

U-boot prefers DM_MMC + BLK for MMC. Now eSDHC driver has already
support it, so let's force to use it.

- Drop non-BLK support for DM_MMC introduced by below patch.
  66fa035 mmc: fsl_esdhc: fix probe issue without CONFIG_BLK enabled

- Support only DM_MMC + BLK (assuming BLK is always enabled for DM_MMC).

- Use DM_MMC instead of BLK for conditional compile.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/Kconfig         |  2 ++
 drivers/mmc/fsl_esdhc_imx.c | 33 +--------------------------------
 2 files changed, 3 insertions(+), 32 deletions(-)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 1569e8c44a..313244682a 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -790,6 +790,7 @@ endif
 
 config FSL_ESDHC
 	bool "Freescale/NXP eSDHC controller support"
+	depends on BLK
 	help
 	  This selects support for the eSDHC (Enhanced Secure Digital Host
 	  Controller) found on numerous Freescale/NXP SoCs.
@@ -826,6 +827,7 @@ config FSL_ESDHC_VS33_NOT_SUPPORT
 
 config FSL_ESDHC_IMX
 	bool "Freescale/NXP i.MX eSDHC controller support"
+	depends on BLK
 	help
 	  This selects support for the i.MX eSDHC (Enhanced Secure Digital Host
 	  Controller) found on numerous Freescale/NXP SoCs.
diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index aabf39535f..0a7f7e61cb 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -39,10 +39,6 @@
 #include <dm/ofnode.h>
 #include <linux/iopoll.h>
 
-#if !CONFIG_IS_ENABLED(BLK)
-#include "mmc_private.h"
-#endif
-
 #ifndef ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
 #ifdef CONFIG_FSL_USDHC
 #define ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE	1
@@ -58,7 +54,6 @@ DECLARE_GLOBAL_DATA_PTR;
 				IRQSTATEN_DEBE | IRQSTATEN_BRR | IRQSTATEN_BWR | \
 				IRQSTATEN_DINT)
 #define MAX_TUNING_LOOP 40
-#define ESDHC_DRIVER_STAGE_VALUE 0xffffffff
 
 struct fsl_esdhc {
 	uint    dsaddr;		/* SDMA system address register */
@@ -157,7 +152,7 @@ struct fsl_esdhc_priv {
 	unsigned int clock;
 	unsigned int mode;
 	unsigned int bus_width;
-#if !CONFIG_IS_ENABLED(BLK)
+#if !CONFIG_IS_ENABLED(DM_MMC)
 	struct mmc *mmc;
 #endif
 	struct udevice *dev;
@@ -1506,9 +1501,6 @@ static int fsl_esdhc_probe(struct udevice *dev)
 	struct esdhc_soc_data *data =
 		(struct esdhc_soc_data *)dev_get_driver_data(dev);
 	struct mmc *mmc;
-#if !CONFIG_IS_ENABLED(BLK)
-	struct blk_desc *bdesc;
-#endif
 	int ret;
 
 #if CONFIG_IS_ENABLED(OF_PLATDATA)
@@ -1607,25 +1599,6 @@ static int fsl_esdhc_probe(struct udevice *dev)
 	mmc = &plat->mmc;
 	mmc->cfg = &plat->cfg;
 	mmc->dev = dev;
-#if !CONFIG_IS_ENABLED(BLK)
-	mmc->priv = priv;
-
-	/* Setup dsr related values */
-	mmc->dsr_imp = 0;
-	mmc->dsr = ESDHC_DRIVER_STAGE_VALUE;
-	/* Setup the universal parts of the block interface just once */
-	bdesc = mmc_get_blk_desc(mmc);
-	bdesc->if_type = IF_TYPE_MMC;
-	bdesc->removable = 1;
-	bdesc->devnum = mmc_get_next_devnum();
-	bdesc->block_read = mmc_bread;
-	bdesc->block_write = mmc_bwrite;
-	bdesc->block_erase = mmc_berase;
-
-	/* setup initial part type */
-	bdesc->part_type = mmc->cfg->part_type;
-	mmc_list_add(mmc);
-#endif
 
 	upriv->mmc = mmc;
 
@@ -1730,14 +1703,12 @@ static const struct udevice_id fsl_esdhc_ids[] = {
 	{ /* sentinel */ }
 };
 
-#if CONFIG_IS_ENABLED(BLK)
 static int fsl_esdhc_bind(struct udevice *dev)
 {
 	struct fsl_esdhc_plat *plat = dev_get_plat(dev);
 
 	return mmc_bind(dev, &plat->mmc, &plat->cfg);
 }
-#endif
 
 U_BOOT_DRIVER(fsl_esdhc) = {
 	.name	= "fsl_esdhc",
@@ -1745,9 +1716,7 @@ U_BOOT_DRIVER(fsl_esdhc) = {
 	.of_match = fsl_esdhc_ids,
 	.of_to_plat = fsl_esdhc_of_to_plat,
 	.ops	= &fsl_esdhc_ops,
-#if CONFIG_IS_ENABLED(BLK)
 	.bind	= fsl_esdhc_bind,
-#endif
 	.probe	= fsl_esdhc_probe,
 	.plat_auto	= sizeof(struct fsl_esdhc_plat),
 	.priv_auto	= sizeof(struct fsl_esdhc_priv),
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 02/11] mmc: fsl_esdhc_imx: remove redundant DM_MMC checking
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
  2021-11-12 19:15 ` [PATCH v2 01/11] mmc: fsl_esdhc_imx: make BLK as hard requirement of DM_MMC Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:35   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 03/11] mmc: fsl_esdhc_imx: fix voltage validation Sean Anderson
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit 2913926f3b3dec282f8773e3c02377c9600d8267 ]

Remove redundant DM_MMC checking which is already in DM_MMC conditional
compile block.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 0a7f7e61cb..40886f37aa 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -1605,7 +1605,6 @@ static int fsl_esdhc_probe(struct udevice *dev)
 	return esdhc_init_common(priv, mmc);
 }
 
-#if CONFIG_IS_ENABLED(DM_MMC)
 static int fsl_esdhc_get_cd(struct udevice *dev)
 {
 	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
@@ -1671,7 +1670,6 @@ static const struct dm_mmc_ops fsl_esdhc_ops = {
 #endif
 	.wait_dat0 = fsl_esdhc_wait_dat0,
 };
-#endif
 
 static struct esdhc_soc_data usdhc_imx7d_data = {
 	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 03/11] mmc: fsl_esdhc_imx: fix voltage validation
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
  2021-11-12 19:15 ` [PATCH v2 01/11] mmc: fsl_esdhc_imx: make BLK as hard requirement of DM_MMC Sean Anderson
  2021-11-12 19:15 ` [PATCH v2 02/11] mmc: fsl_esdhc_imx: remove redundant DM_MMC checking Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:36   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 04/11] mmc: fsl_esdhc_imx: clean up bus width configuration code Sean Anderson
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit 5b05fc0310cd933acf76ee661577c6b07a95e684 ]

Voltage validation should be done by CMD8. Current comparison between
mmc_cfg voltages and host voltage capabilities is meaningless.
So drop current comparison and let voltage validation is through CMD8.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 35 +++++++++++++----------------------
 include/fsl_esdhc_imx.h     | 12 ++++++------
 2 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 40886f37aa..b91dda27f9 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -1164,7 +1164,7 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 {
 	struct mmc_config *cfg;
 	struct fsl_esdhc *regs;
-	u32 caps, voltage_caps;
+	u32 caps;
 	int ret;
 
 	if (!priv)
@@ -1203,9 +1203,7 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 	memset(cfg, '\0', sizeof(*cfg));
 #endif
 
-	voltage_caps = 0;
 	caps = esdhc_read32(&regs->hostcapblt);
-
 #ifdef CONFIG_MCF5441x
 	/*
 	 * MCF5441x RM declares in more points that sdhc clock speed must
@@ -1216,31 +1214,24 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 #endif
 
 #ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC135
-	caps = caps & ~(ESDHC_HOSTCAPBLT_SRS |
-			ESDHC_HOSTCAPBLT_VS18 | ESDHC_HOSTCAPBLT_VS30);
+	caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
 #endif
 
-	if (caps & ESDHC_HOSTCAPBLT_VS18)
-		voltage_caps |= MMC_VDD_165_195;
-	if (caps & ESDHC_HOSTCAPBLT_VS30)
-		voltage_caps |= MMC_VDD_29_30 | MMC_VDD_30_31;
-	if (caps & ESDHC_HOSTCAPBLT_VS33)
-		voltage_caps |= MMC_VDD_32_33 | MMC_VDD_33_34;
+#ifdef CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33
+	caps |= HOSTCAPBLT_VS33;
+#endif
+
+	if (caps & HOSTCAPBLT_VS18)
+		cfg->voltages |= MMC_VDD_165_195;
+	if (caps & HOSTCAPBLT_VS30)
+		cfg->voltages |= MMC_VDD_29_30 | MMC_VDD_30_31;
+	if (caps & HOSTCAPBLT_VS33)
+		cfg->voltages |= MMC_VDD_32_33 | MMC_VDD_33_34;
 
 	cfg->name = "FSL_SDHC";
 #if !CONFIG_IS_ENABLED(DM_MMC)
 	cfg->ops = &esdhc_ops;
 #endif
-#ifdef CONFIG_SYS_SD_VOLTAGE
-	cfg->voltages = CONFIG_SYS_SD_VOLTAGE;
-#else
-	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
-#endif
-	if ((cfg->voltages & voltage_caps) == 0) {
-		printf("voltage not supported by controller\n");
-		return -1;
-	}
-
 	if (priv->bus_width == 8)
 		cfg->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
 	else if (priv->bus_width == 4)
@@ -1258,7 +1249,7 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 			cfg->host_caps &= ~MMC_MODE_4BIT;
 	}
 
-	if (caps & ESDHC_HOSTCAPBLT_HSS)
+	if (caps & HOSTCAPBLT_HSS)
 		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
 
 #ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
index 45ed635a77..1529b8bba3 100644
--- a/include/fsl_esdhc_imx.h
+++ b/include/fsl_esdhc_imx.h
@@ -164,12 +164,12 @@
 #define BLKATTR_SIZE(x)	(x & 0x1fff)
 #define MAX_BLK_CNT	0x7fff	/* so malloc will have enough room with 32M */
 
-#define ESDHC_HOSTCAPBLT_VS18	0x04000000
-#define ESDHC_HOSTCAPBLT_VS30	0x02000000
-#define ESDHC_HOSTCAPBLT_VS33	0x01000000
-#define ESDHC_HOSTCAPBLT_SRS	0x00800000
-#define ESDHC_HOSTCAPBLT_DMAS	0x00400000
-#define ESDHC_HOSTCAPBLT_HSS	0x00200000
+#define HOSTCAPBLT_VS18	0x04000000
+#define HOSTCAPBLT_VS30	0x02000000
+#define HOSTCAPBLT_VS33	0x01000000
+#define HOSTCAPBLT_SRS	0x00800000
+#define HOSTCAPBLT_DMAS	0x00400000
+#define HOSTCAPBLT_HSS	0x00200000
 
 #define ESDHC_VENDORSPEC_VSELECT 0x00000002 /* Use 1.8V */
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 04/11] mmc: fsl_esdhc_imx: clean up bus width configuration code
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
                   ` (2 preceding siblings ...)
  2021-11-12 19:15 ` [PATCH v2 03/11] mmc: fsl_esdhc_imx: fix voltage validation Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:36   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 05/11] mmc: fsl_esdhc_imx: drop redundant code for non-removable feature Sean Anderson
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit 07bae1de382723b94244096953b05225572728cd ]

This patch is to clean up bus width setting code.

- For DM_MMC, remove getting "bus-width" from device tree.
  This has been done in mmc_of_parse().

- For non-DM_MMC, move bus width configuration from fsl_esdhc_init()
  to fsl_esdhc_initialize() which is non-DM_MMC specific.
  And fix up bus width configuration to support only 1-bit, 4-bit,
  or 8-bit. Keep using 8-bit if it's not set because many platforms
  use driver without providing max bus width.

- Remove bus_width member from fsl_esdhc_priv structure.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
[ converted if statement to switch ]
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

Changes in v2:
- Use a switch statement instead of ifs for max_bus_width
- Only default to 8 bit width when max_bus_width is not set

 drivers/mmc/fsl_esdhc_imx.c | 83 ++++++++++++-------------------------
 1 file changed, 26 insertions(+), 57 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index b91dda27f9..b604729750 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -126,7 +126,6 @@ struct esdhc_soc_data {
  *
  * @esdhc_regs: registers of the sdhc controller
  * @sdhc_clk: Current clk of the sdhc controller
- * @bus_width: bus width, 1bit, 4bit or 8bit
  * @cfg: mmc config
  * @mmc: mmc
  * Following is used when Driver Model is enabled for MMC
@@ -151,7 +150,6 @@ struct fsl_esdhc_priv {
 	struct clk per_clk;
 	unsigned int clock;
 	unsigned int mode;
-	unsigned int bus_width;
 #if !CONFIG_IS_ENABLED(DM_MMC)
 	struct mmc *mmc;
 #endif
@@ -1232,31 +1230,13 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 #if !CONFIG_IS_ENABLED(DM_MMC)
 	cfg->ops = &esdhc_ops;
 #endif
-	if (priv->bus_width == 8)
-		cfg->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
-	else if (priv->bus_width == 4)
-		cfg->host_caps = MMC_MODE_4BIT;
-
-	cfg->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
 #ifdef CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE
 	cfg->host_caps |= MMC_MODE_DDR_52MHz;
 #endif
 
-	if (priv->bus_width > 0) {
-		if (priv->bus_width < 8)
-			cfg->host_caps &= ~MMC_MODE_8BIT;
-		if (priv->bus_width < 4)
-			cfg->host_caps &= ~MMC_MODE_4BIT;
-	}
-
 	if (caps & HOSTCAPBLT_HSS)
 		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
 
-#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
-	if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
-		cfg->host_caps &= ~MMC_MODE_8BIT;
-#endif
-
 	cfg->host_caps |= priv->caps;
 
 	cfg->f_min = 400000;
@@ -1294,25 +1274,11 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 }
 
 #if !CONFIG_IS_ENABLED(DM_MMC)
-static int fsl_esdhc_cfg_to_priv(struct fsl_esdhc_cfg *cfg,
-				 struct fsl_esdhc_priv *priv)
-{
-	if (!cfg || !priv)
-		return -EINVAL;
-
-	priv->esdhc_regs = (struct fsl_esdhc *)(unsigned long)(cfg->esdhc_base);
-	priv->bus_width = cfg->max_bus_width;
-	priv->sdhc_clk = cfg->sdhc_clk;
-	priv->wp_enable  = cfg->wp_enable;
-	priv->vs18_enable  = cfg->vs18_enable;
-
-	return 0;
-};
-
 int fsl_esdhc_initialize(struct bd_info *bis, struct fsl_esdhc_cfg *cfg)
 {
 	struct fsl_esdhc_plat *plat;
 	struct fsl_esdhc_priv *priv;
+	struct mmc_config *mmc_cfg;
 	struct mmc *mmc;
 	int ret;
 
@@ -1328,14 +1294,33 @@ int fsl_esdhc_initialize(struct bd_info *bis, struct fsl_esdhc_cfg *cfg)
 		return -ENOMEM;
 	}
 
-	ret = fsl_esdhc_cfg_to_priv(cfg, priv);
-	if (ret) {
-		debug("%s xlate failure\n", __func__);
-		free(plat);
-		free(priv);
-		return ret;
+	priv->esdhc_regs = (struct fsl_esdhc *)(unsigned long)(cfg->esdhc_base);
+	priv->sdhc_clk = cfg->sdhc_clk;
+	priv->wp_enable  = cfg->wp_enable;
+
+	mmc_cfg = &plat->cfg;
+
+	switch (cfg->max_bus_width) {
+	case 0: /* Not set in config; assume everything is supported */
+	case 8:
+		mmc_cfg->host_caps |= MMC_MODE_8BIT;
+		fallthrough;
+	case 4:
+		mmc_cfg->host_caps |= MMC_MODE_4BIT;
+		fallthrough;
+	case 1:
+		mmc_cfg->host_caps |= MMC_MODE_1BIT;
+		break;
+	default:
+		printf("invalid max bus width %u\n", cfg->max_bus_width);
+		return -EINVAL;
 	}
 
+#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
+	if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
+		mmc_cfg->host_caps &= ~MMC_MODE_8BIT;
+#endif
+
 	ret = fsl_esdhc_init(priv, plat);
 	if (ret) {
 		debug("%s init failure\n", __func__);
@@ -1416,14 +1401,6 @@ static int fsl_esdhc_of_to_plat(struct udevice *dev)
 	priv->dev = dev;
 	priv->mode = -1;
 
-	val = dev_read_u32_default(dev, "bus-width", -1);
-	if (val == 8)
-		priv->bus_width = 8;
-	else if (val == 4)
-		priv->bus_width = 4;
-	else
-		priv->bus_width = 1;
-
 	val = fdtdec_get_int(fdt, node, "fsl,tuning-step", 1);
 	priv->tuning_step = val;
 	val = fdtdec_get_int(fdt, node, "fsl,tuning-start-tap",
@@ -1496,16 +1473,8 @@ static int fsl_esdhc_probe(struct udevice *dev)
 
 #if CONFIG_IS_ENABLED(OF_PLATDATA)
 	struct dtd_fsl_esdhc *dtplat = &plat->dtplat;
-	unsigned int val;
 
 	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;
 
 	if (dtplat->non_removable)
 		priv->non_removable = 1;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 05/11] mmc: fsl_esdhc_imx: drop redundant code for non-removable feature
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
                   ` (3 preceding siblings ...)
  2021-11-12 19:15 ` [PATCH v2 04/11] mmc: fsl_esdhc_imx: clean up bus width configuration code Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:37   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 06/11] mmc: fsl_esdhc_imx: fix mmc->clock with actual clock Sean Anderson
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit commit 08197cb8dff7cd097ab07a325093043c39d19bbd ]

Drop redundant code for non-removable feature. "non-removable" property
has been read in mmc_of_parse().

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index b604729750..b2844d0a7b 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -130,7 +130,6 @@ struct esdhc_soc_data {
  * @mmc: mmc
  * Following is used when Driver Model is enabled for MMC
  * @dev: pointer for the device
- * @non_removable: 0: removable; 1: non-removable
  * @broken_cd: 0: use GPIO for card detect; 1: Do not use GPIO for card detect
  * @wp_enable: 1: enable checking wp; 0: no check
  * @vs18_enable: 1: use 1.8V voltage; 0: use 3.3V
@@ -154,7 +153,6 @@ struct fsl_esdhc_priv {
 	struct mmc *mmc;
 #endif
 	struct udevice *dev;
-	int non_removable;
 	int broken_cd;
 	int wp_enable;
 	int vs18_enable;
@@ -1083,9 +1081,6 @@ static int esdhc_getcd_common(struct fsl_esdhc_priv *priv)
 #endif
 
 #if CONFIG_IS_ENABLED(DM_MMC)
-	if (priv->non_removable)
-		return 1;
-
 	if (priv->broken_cd)
 		return 1;
 #if CONFIG_IS_ENABLED(DM_GPIO)
@@ -1415,25 +1410,18 @@ static int fsl_esdhc_of_to_plat(struct udevice *dev)
 	if (dev_read_bool(dev, "broken-cd"))
 		priv->broken_cd = 1;
 
-	if (dev_read_bool(dev, "non-removable")) {
-		priv->non_removable = 1;
-	 } else {
-		priv->non_removable = 0;
-#if CONFIG_IS_ENABLED(DM_GPIO)
-		gpio_request_by_name(dev, "cd-gpios", 0, &priv->cd_gpio,
-				     GPIOD_IS_IN);
-#endif
-	}
-
 	if (dev_read_prop(dev, "fsl,wp-controller", NULL)) {
 		priv->wp_enable = 1;
 	} else {
 		priv->wp_enable = 0;
+	}
+
 #if CONFIG_IS_ENABLED(DM_GPIO)
-		gpio_request_by_name(dev, "wp-gpios", 0, &priv->wp_gpio,
-				   GPIOD_IS_IN);
+	gpio_request_by_name(dev, "cd-gpios", 0, &priv->cd_gpio,
+			     GPIOD_IS_IN);
+	gpio_request_by_name(dev, "wp-gpios", 0, &priv->wp_gpio,
+			     GPIOD_IS_IN);
 #endif
-	}
 
 	priv->vs18_enable = 0;
 
@@ -1567,8 +1555,12 @@ static int fsl_esdhc_probe(struct udevice *dev)
 
 static int fsl_esdhc_get_cd(struct udevice *dev)
 {
+	struct fsl_esdhc_plat *plat = dev_get_plat(dev);
 	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
 
+	if (plat->cfg.host_caps & MMC_CAP_NONREMOVABLE)
+		return 1;
+
 	return esdhc_getcd_common(priv);
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 06/11] mmc: fsl_esdhc_imx: fix mmc->clock with actual clock
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
                   ` (4 preceding siblings ...)
  2021-11-12 19:15 ` [PATCH v2 05/11] mmc: fsl_esdhc_imx: drop redundant code for non-removable feature Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:37   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers Sean Anderson
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit 30f6444d024a74ee48aa6969c1531aecd3c59deb ]

Fix mmc->clock with actual clock which is divided by the
controller, and record it with priv->clock.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index b2844d0a7b..95cde9e410 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -665,6 +665,7 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
 #endif
 
+	mmc->clock = sdhc_clk / pre_div / div;
 	priv->clock = clock;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
                   ` (5 preceding siblings ...)
  2021-11-12 19:15 ` [PATCH v2 06/11] mmc: fsl_esdhc_imx: fix mmc->clock with actual clock Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:39   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 08/11] mmc: fsl_esdhc_imx: use dma-mapping API Sean Anderson
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit da86e8cfcb03ed5c1d8e0718bc8bc8583e60ced8 ]

SDMA can only do DMA with 32 bit addresses. This is true for all
architectures (just doesn't apply to 32 bit ones). Simplify the code and
remove unnecessary CONFIG_FSL_LAYERSCAPE.

Also make the error message more concise.

Signed-off-by: Michael Walle <michael@walle.cc>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 33 ++++++---------------------------
 1 file changed, 6 insertions(+), 27 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 95cde9e410..3588056759 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -282,10 +282,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 {
 	int timeout;
 	struct fsl_esdhc *regs = priv->esdhc_regs;
-#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
-	defined(CONFIG_IMX8ULP)
 	dma_addr_t addr;
-#endif
 	uint wml_value;
 
 	wml_value = data->blocksize/4;
@@ -296,16 +293,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 
 		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
 #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
-	defined(CONFIG_IMX8ULP)
 		addr = virt_to_phys((void *)(data->dest));
 		if (upper_32_bits(addr))
-			printf("Error found for upper 32 bits\n");
-		else
-			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
-#else
-		esdhc_write32(&regs->dsaddr, (u32)data->dest);
-#endif
+			printf("Cannot use 64 bit addresses with SDMA\n");
+		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
 #endif
 	} else {
 #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
@@ -334,16 +325,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
 					wml_value << 16);
 #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
-		defined(CONFIG_IMX8ULP)
 		addr = virt_to_phys((void *)(data->src));
 		if (upper_32_bits(addr))
-			printf("Error found for upper 32 bits\n");
-		else
-			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
-#else
-		esdhc_write32(&regs->dsaddr, (u32)data->src);
-#endif
+			printf("Cannot use 64 bit addresses with SDMA\n");
+		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
 #endif
 	}
 
@@ -400,18 +385,12 @@ static void check_and_invalidate_dcache_range
 	unsigned end = 0;
 	unsigned size = roundup(ARCH_DMA_MINALIGN,
 				data->blocks*data->blocksize);
-#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
-	defined(CONFIG_IMX8ULP)
 	dma_addr_t addr;
 
 	addr = virt_to_phys((void *)(data->dest));
 	if (upper_32_bits(addr))
-		printf("Error found for upper 32 bits\n");
-	else
-		start = lower_32_bits(addr);
-#else
-	start = (unsigned)data->dest;
-#endif
+		printf("Cannot use 64 bit addresses with SDMA\n");
+	start = lower_32_bits(addr);
 	end = start + size;
 	invalidate_dcache_range(start, end);
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 08/11] mmc: fsl_esdhc_imx: use dma-mapping API
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
                   ` (6 preceding siblings ...)
  2021-11-12 19:15 ` [PATCH v2 07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:40   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 09/11] mmc: fsl_esdhc_imx: simplify esdhc_setup_data() Sean Anderson
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit b1ba1460a445bcc67972a617625d0349e4f22b31 ]

Use the dma_{map,unmap}_single() calls. These will take care of the
flushing and invalidation of caches.

Signed-off-by: Michael Walle <michael@walle.cc>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 50 +++++++++++--------------------------
 1 file changed, 15 insertions(+), 35 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 3588056759..afc8259323 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -38,6 +38,7 @@
 #include <mapmem.h>
 #include <dm/ofnode.h>
 #include <linux/iopoll.h>
+#include <linux/dma-mapping.h>
 
 #ifndef ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
 #ifdef CONFIG_FSL_USDHC
@@ -171,6 +172,7 @@ struct fsl_esdhc_priv {
 	struct gpio_desc cd_gpio;
 	struct gpio_desc wp_gpio;
 #endif
+	dma_addr_t dma_addr;
 };
 
 /* Return the XFERTYP flags for a given command and data packet */
@@ -281,8 +283,8 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 			    struct mmc_data *data)
 {
 	int timeout;
+	uint trans_bytes = data->blocksize * data->blocks;
 	struct fsl_esdhc *regs = priv->esdhc_regs;
-	dma_addr_t addr;
 	uint wml_value;
 
 	wml_value = data->blocksize/4;
@@ -293,17 +295,13 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 
 		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
 #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-		addr = virt_to_phys((void *)(data->dest));
-		if (upper_32_bits(addr))
+		priv->dma_addr = dma_map_single(data->dest, trans_bytes,
+						mmc_get_dma_dir(data));
+		if (upper_32_bits(priv->dma_addr))
 			printf("Cannot use 64 bit addresses with SDMA\n");
-		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
+		esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
 #endif
 	} else {
-#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-		flush_dcache_range((ulong)data->src,
-				   (ulong)data->src+data->blocks
-					 *data->blocksize);
-#endif
 		if (wml_value > WML_WR_WML_MAX)
 			wml_value = WML_WR_WML_MAX_VAL;
 		if (priv->wp_enable) {
@@ -325,10 +323,11 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
 					wml_value << 16);
 #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-		addr = virt_to_phys((void *)(data->src));
-		if (upper_32_bits(addr))
+		priv->dma_addr = dma_map_single((void *)data->src, trans_bytes,
+						mmc_get_dma_dir(data));
+		if (upper_32_bits(priv->dma_addr))
 			printf("Cannot use 64 bit addresses with SDMA\n");
-		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
+		esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
 #endif
 	}
 
@@ -378,23 +377,6 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 	return 0;
 }
 
-static void check_and_invalidate_dcache_range
-	(struct mmc_cmd *cmd,
-	 struct mmc_data *data) {
-	unsigned start = 0;
-	unsigned end = 0;
-	unsigned size = roundup(ARCH_DMA_MINALIGN,
-				data->blocks*data->blocksize);
-	dma_addr_t addr;
-
-	addr = virt_to_phys((void *)(data->dest));
-	if (upper_32_bits(addr))
-		printf("Cannot use 64 bit addresses with SDMA\n");
-	start = lower_32_bits(addr);
-	end = start + size;
-	invalidate_dcache_range(start, end);
-}
-
 #ifdef CONFIG_MCF5441x
 /*
  * Swaps 32-bit words to little-endian byte order.
@@ -450,9 +432,6 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 		err = esdhc_setup_data(priv, mmc, data);
 		if(err)
 			return err;
-
-		if (data->flags & MMC_DATA_READ)
-			check_and_invalidate_dcache_range(cmd, data);
 	}
 
 	/* Figure out the transfer arguments */
@@ -560,12 +539,13 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 		 * cache-fill during the DMA operations such as the
 		 * speculative pre-fetching etc.
 		 */
-		if (data->flags & MMC_DATA_READ) {
-			check_and_invalidate_dcache_range(cmd, data);
+		dma_unmap_single(priv->dma_addr,
+				 data->blocks * data->blocksize,
+				 mmc_get_dma_dir(data));
 #ifdef CONFIG_MCF5441x
+		if (data->flags & MMC_DATA_READ)
 			sd_swap_dma_buff(data);
 #endif
-		}
 #endif
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 09/11] mmc: fsl_esdhc_imx: simplify esdhc_setup_data()
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
                   ` (7 preceding siblings ...)
  2021-11-12 19:15 ` [PATCH v2 08/11] mmc: fsl_esdhc_imx: use dma-mapping API Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:41   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 10/11] mmc: fsl_esdhc_imx: replace most #ifdefs by IS_ENABLED() Sean Anderson
  2021-11-12 19:15 ` [PATCH v2 11/11] mmc: fsl_esdhc_imx: set sysctl register for clock initialization Sean Anderson
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit 7e48a028a42c111ba38a90b86e5f57dace980fa0 ]

First, we need the waterlevel setting for PIO mode only. Secondy, both DMA
setup code is identical for both directions, except for the data pointer.
Thus, unify them.

Signed-off-by: Michael Walle <michael@walle.cc>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 89 ++++++++++++++++++++++---------------
 1 file changed, 52 insertions(+), 37 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index afc8259323..aa3d0877cb 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -279,59 +279,74 @@ static void esdhc_pio_read_write(struct fsl_esdhc_priv *priv,
 }
 #endif
 
-static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
-			    struct mmc_data *data)
+#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
+static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
+					struct mmc_data *data)
 {
-	int timeout;
-	uint trans_bytes = data->blocksize * data->blocks;
 	struct fsl_esdhc *regs = priv->esdhc_regs;
-	uint wml_value;
-
-	wml_value = data->blocksize/4;
+	uint wml_value = data->blocksize / 4;
 
 	if (data->flags & MMC_DATA_READ) {
 		if (wml_value > WML_RD_WML_MAX)
 			wml_value = WML_RD_WML_MAX_VAL;
 
 		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
-#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-		priv->dma_addr = dma_map_single(data->dest, trans_bytes,
-						mmc_get_dma_dir(data));
-		if (upper_32_bits(priv->dma_addr))
-			printf("Cannot use 64 bit addresses with SDMA\n");
-		esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
-#endif
 	} else {
 		if (wml_value > WML_WR_WML_MAX)
 			wml_value = WML_WR_WML_MAX_VAL;
-		if (priv->wp_enable) {
-			if ((esdhc_read32(&regs->prsstat) &
-			    PRSSTAT_WPSPL) == 0) {
-				printf("\nThe SD card is locked. Can not write to a locked card.\n\n");
-				return -ETIMEDOUT;
-			}
-		} else {
-#if CONFIG_IS_ENABLED(DM_GPIO)
-			if (dm_gpio_is_valid(&priv->wp_gpio) &&
-			    dm_gpio_get_value(&priv->wp_gpio)) {
-				printf("\nThe SD card is locked. Can not write to a locked card.\n\n");
-				return -ETIMEDOUT;
-			}
-#endif
-		}
 
 		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
-					wml_value << 16);
-#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-		priv->dma_addr = dma_map_single((void *)data->src, trans_bytes,
-						mmc_get_dma_dir(data));
-		if (upper_32_bits(priv->dma_addr))
-			printf("Cannot use 64 bit addresses with SDMA\n");
-		esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
-#endif
+				   wml_value << 16);
 	}
+}
+#endif
 
+static void esdhc_setup_dma(struct fsl_esdhc_priv *priv, struct mmc_data *data)
+{
+	uint trans_bytes = data->blocksize * data->blocks;
+	struct fsl_esdhc *regs = priv->esdhc_regs;
+	void *buf;
+
+	if (data->flags & MMC_DATA_WRITE)
+		buf = (void *)data->src;
+	else
+		buf = data->dest;
+
+	priv->dma_addr = dma_map_single(buf, trans_bytes,
+					mmc_get_dma_dir(data));
+	if (upper_32_bits(priv->dma_addr))
+		printf("Cannot use 64 bit addresses with SDMA\n");
+	esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
 	esdhc_write32(&regs->blkattr, data->blocks << 16 | data->blocksize);
+}
+
+static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
+			    struct mmc_data *data)
+{
+	int timeout;
+	bool is_write = data->flags & MMC_DATA_WRITE;
+	struct fsl_esdhc *regs = priv->esdhc_regs;
+
+	if (is_write) {
+		if (priv->wp_enable && !(esdhc_read32(&regs->prsstat) & PRSSTAT_WPSPL)) {
+			printf("Cannot write to locked SD card.\n");
+			return -EINVAL;
+		} else {
+#if CONFIG_IS_ENABLED(DM_GPIO)
+			if (dm_gpio_is_valid(&priv->wp_gpio) &&
+			    dm_gpio_get_value(&priv->wp_gpio)) {
+				printf("Cannot write to locked SD card.\n");
+				return -EINVAL;
+			}
+#endif
+		}
+	}
+
+#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
+	esdhc_setup_watermark_level(priv, data);
+#else
+	esdhc_setup_dma(priv, data);
+#endif
 
 	/* Calculate the timeout period for data transactions */
 	/*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 10/11] mmc: fsl_esdhc_imx: replace most #ifdefs by IS_ENABLED()
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
                   ` (8 preceding siblings ...)
  2021-11-12 19:15 ` [PATCH v2 09/11] mmc: fsl_esdhc_imx: simplify esdhc_setup_data() Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  2021-11-15  8:44   ` Jaehoon Chung
  2021-11-12 19:15 ` [PATCH v2 11/11] mmc: fsl_esdhc_imx: set sysctl register for clock initialization Sean Anderson
  10 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit 52faec31827ec1a1837977e29c067424426634c5 ]

Make the code cleaner and drop the old-style #ifdef constructs where it is
possible.

Signed-off-by: Michael Walle <michael@walle.cc>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 209 +++++++++++++++++-------------------
 include/fsl_esdhc_imx.h     |   2 -
 2 files changed, 100 insertions(+), 111 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index aa3d0877cb..89572509a7 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -182,15 +182,15 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
 
 	if (data) {
 		xfertyp |= XFERTYP_DPSEL;
-#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-		xfertyp |= XFERTYP_DMAEN;
-#endif
+		if (!IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO) &&
+		    cmd->cmdidx != MMC_CMD_SEND_TUNING_BLOCK &&
+		    cmd->cmdidx != MMC_CMD_SEND_TUNING_BLOCK_HS200)
+			xfertyp |= XFERTYP_DMAEN;
 		if (data->blocks > 1) {
 			xfertyp |= XFERTYP_MSBSEL;
 			xfertyp |= XFERTYP_BCEN;
-#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
-			xfertyp |= XFERTYP_AC12EN;
-#endif
+			if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC111))
+				xfertyp |= XFERTYP_AC12EN;
 		}
 
 		if (data->flags & MMC_DATA_READ)
@@ -214,7 +214,6 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
 	return XFERTYP_CMD(cmd->cmdidx) | xfertyp;
 }
 
-#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
 /*
  * PIO Read/Write Mode reduce the performace as DMA is not used in this mode.
  */
@@ -277,9 +276,7 @@ static void esdhc_pio_read_write(struct fsl_esdhc_priv *priv,
 		}
 	}
 }
-#endif
 
-#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
 static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
 					struct mmc_data *data)
 {
@@ -299,7 +296,6 @@ static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
 				   wml_value << 16);
 	}
 }
-#endif
 
 static void esdhc_setup_dma(struct fsl_esdhc_priv *priv, struct mmc_data *data)
 {
@@ -342,11 +338,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 		}
 	}
 
-#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
-	esdhc_setup_watermark_level(priv, data);
-#else
-	esdhc_setup_dma(priv, data);
-#endif
+	if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO))
+		esdhc_setup_watermark_level(priv, data);
+	else
+		esdhc_setup_dma(priv, data);
 
 	/* Calculate the timeout period for data transactions */
 	/*
@@ -379,14 +374,13 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 	if (timeout < 0)
 		timeout = 0;
 
-#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC_A001
-	if ((timeout == 4) || (timeout == 8) || (timeout == 12))
+	if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC_A001) &&
+	    (timeout == 4 || timeout == 8 || timeout == 12))
 		timeout++;
-#endif
 
-#ifdef ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
-	timeout = 0xE;
-#endif
+	if (IS_ENABLED(ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE))
+		timeout = 0xE;
+
 	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_TIMEOUT_MASK, timeout << 16);
 
 	return 0;
@@ -409,6 +403,11 @@ static inline void sd_swap_dma_buff(struct mmc_data *data)
 		}
 	}
 }
+#else
+static inline void sd_swap_dma_buff(struct mmc_data *data)
+{
+	return;
+}
 #endif
 
 /*
@@ -425,10 +424,9 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 	struct fsl_esdhc *regs = priv->esdhc_regs;
 	unsigned long start;
 
-#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
-	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
+	if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC111) &&
+	    cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
 		return 0;
-#endif
 
 	esdhc_write32(&regs->irqstat, -1);
 
@@ -526,42 +524,40 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 
 	/* Wait until all of the blocks are transferred */
 	if (data) {
-#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
-		esdhc_pio_read_write(priv, data);
-#else
-		flags = DATA_COMPLETE;
-		if ((cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK) ||
-		    (cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK_HS200)) {
-			flags = IRQSTAT_BRR;
+		if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO)) {
+			esdhc_pio_read_write(priv, data);
+		} else {
+			flags = DATA_COMPLETE;
+			if (cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK ||
+			    cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK_HS200)
+				flags = IRQSTAT_BRR;
+
+			do {
+				irqstat = esdhc_read32(&regs->irqstat);
+
+				if (irqstat & IRQSTAT_DTOE) {
+					err = -ETIMEDOUT;
+					goto out;
+				}
+
+				if (irqstat & DATA_ERR) {
+					err = -ECOMM;
+					goto out;
+				}
+			} while ((irqstat & flags) != flags);
+
+			/*
+			 * Need invalidate the dcache here again to avoid any
+			 * cache-fill during the DMA operations such as the
+			 * speculative pre-fetching etc.
+			 */
+			dma_unmap_single(priv->dma_addr,
+					 data->blocks * data->blocksize,
+					 mmc_get_dma_dir(data));
+			if (IS_ENABLED(CONFIG_MCF5441x) &&
+			    (data->flags & MMC_DATA_READ))
+				sd_swap_dma_buff(data);
 		}
-
-		do {
-			irqstat = esdhc_read32(&regs->irqstat);
-
-			if (irqstat & IRQSTAT_DTOE) {
-				err = -ETIMEDOUT;
-				goto out;
-			}
-
-			if (irqstat & DATA_ERR) {
-				err = -ECOMM;
-				goto out;
-			}
-		} while ((irqstat & flags) != flags);
-
-		/*
-		 * Need invalidate the dcache here again to avoid any
-		 * cache-fill during the DMA operations such as the
-		 * speculative pre-fetching etc.
-		 */
-		dma_unmap_single(priv->dma_addr,
-				 data->blocks * data->blocksize,
-				 mmc_get_dma_dir(data));
-#ifdef CONFIG_MCF5441x
-		if (data->flags & MMC_DATA_READ)
-			sd_swap_dma_buff(data);
-#endif
-#endif
 	}
 
 out:
@@ -595,21 +591,22 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 	struct fsl_esdhc *regs = priv->esdhc_regs;
 	int div = 1;
 	u32 tmp;
-	int ret;
-#ifdef ARCH_MXC
-#ifdef CONFIG_MX53
-	/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
-	int pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;
-#else
-	int pre_div = 1;
-#endif
-#else
-	int pre_div = 2;
-#endif
+	int ret, pre_div;
 	int ddr_pre_div = mmc->ddr_mode ? 2 : 1;
 	int sdhc_clk = priv->sdhc_clk;
 	uint clk;
 
+	if (IS_ENABLED(ARCH_MXC)) {
+#ifdef CONFIG_MX53
+		/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
+		pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;
+#else
+		pre_div = 1;
+#endif
+	} else {
+		pre_div = 2;
+	}
+
 	while (sdhc_clk / (16 * pre_div * ddr_pre_div) > clock && pre_div < 256)
 		pre_div *= 2;
 
@@ -621,11 +618,10 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 
 	clk = (pre_div << 8) | (div << 4);
 
-#ifdef CONFIG_FSL_USDHC
-	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
-#else
-	esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
-#endif
+	if (IS_ENABLED(CONFIG_FSL_USDHC))
+		esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
+	else
+		esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
 
 	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_CLOCK_MASK, clk);
 
@@ -633,11 +629,10 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 	if (ret)
 		pr_warn("fsl_esdhc_imx: Internal clock never stabilised.\n");
 
-#ifdef CONFIG_FSL_USDHC
-	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN | VENDORSPEC_CKEN);
-#else
-	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
-#endif
+	if (IS_ENABLED(CONFIG_FSL_USDHC))
+		esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN | VENDORSPEC_CKEN);
+	else
+		esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
 
 	mmc->clock = sdhc_clk / pre_div / div;
 	priv->clock = clock;
@@ -1145,22 +1140,21 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 	if (ret)
 		return ret;
 
-#ifdef CONFIG_MCF5441x
 	/* ColdFire, using SDHC_DATA[3] for card detection */
-	esdhc_write32(&regs->proctl, PROCTL_INIT | PROCTL_D3CD);
-#endif
+	if (IS_ENABLED(CONFIG_MCF5441x))
+		esdhc_write32(&regs->proctl, PROCTL_INIT | PROCTL_D3CD);
 
-#ifndef CONFIG_FSL_USDHC
-	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
-				| SYSCTL_IPGEN | SYSCTL_CKEN);
-	/* Clearing tuning bits in case ROM has set it already */
-	esdhc_write32(&regs->mixctrl, 0);
-	esdhc_write32(&regs->autoc12err, 0);
-	esdhc_write32(&regs->clktunectrlstatus, 0);
-#else
-	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
-			VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
-#endif
+	if (IS_ENABLED(CONFIG_FSL_USDHC)) {
+		esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
+				VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
+	} else {
+		esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
+					| SYSCTL_IPGEN | SYSCTL_CKEN);
+		/* Clearing tuning bits in case ROM has set it already */
+		esdhc_write32(&regs->mixctrl, 0);
+		esdhc_write32(&regs->autoc12err, 0);
+		esdhc_write32(&regs->clktunectrlstatus, 0);
+	}
 
 	if (priv->vs18_enable)
 		esdhc_setbits32(&regs->vendorspec, ESDHC_VENDORSPEC_VSELECT);
@@ -1172,22 +1166,20 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 #endif
 
 	caps = esdhc_read32(&regs->hostcapblt);
-#ifdef CONFIG_MCF5441x
+
 	/*
 	 * MCF5441x RM declares in more points that sdhc clock speed must
 	 * never exceed 25 Mhz. From this, the HS bit needs to be disabled
 	 * from host capabilities.
 	 */
-	caps &= ~ESDHC_HOSTCAPBLT_HSS;
-#endif
+	if (IS_ENABLED(CONFIG_MCF5441x))
+		caps &= ~HOSTCAPBLT_HSS;
 
-#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC135
-	caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
-#endif
+	if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC135))
+		caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
 
-#ifdef CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33
-	caps |= HOSTCAPBLT_VS33;
-#endif
+	if (IS_ENABLED(CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33))
+		caps |= HOSTCAPBLT_VS33;
 
 	if (caps & HOSTCAPBLT_VS18)
 		cfg->voltages |= MMC_VDD_165_195;
@@ -1197,12 +1189,13 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 		cfg->voltages |= MMC_VDD_32_33 | MMC_VDD_33_34;
 
 	cfg->name = "FSL_SDHC";
+
 #if !CONFIG_IS_ENABLED(DM_MMC)
 	cfg->ops = &esdhc_ops;
 #endif
-#ifdef CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE
-	cfg->host_caps |= MMC_MODE_DDR_52MHz;
-#endif
+
+	if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE))
+		cfg->host_caps |= MMC_MODE_DDR_52MHz;
 
 	if (caps & HOSTCAPBLT_HSS)
 		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
@@ -1286,10 +1279,8 @@ int fsl_esdhc_initialize(struct bd_info *bis, struct fsl_esdhc_cfg *cfg)
 		return -EINVAL;
 	}
 
-#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
-	if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
+	if (IS_ENABLED(CONFIG_ESDHC_DETECT_8_BIT_QUIRK))
 		mmc_cfg->host_caps &= ~MMC_MODE_8BIT;
-#endif
 
 	ret = fsl_esdhc_init(priv, plat);
 	if (ret) {
diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
index 1529b8bba3..b958a6b3bb 100644
--- a/include/fsl_esdhc_imx.h
+++ b/include/fsl_esdhc_imx.h
@@ -24,12 +24,10 @@
 #define SYSCTL_INITA		0x08000000
 #define SYSCTL_TIMEOUT_MASK	0x000f0000
 #define SYSCTL_CLOCK_MASK	0x0000fff0
-#if !defined(CONFIG_FSL_USDHC)
 #define SYSCTL_CKEN		0x00000008
 #define SYSCTL_PEREN		0x00000004
 #define SYSCTL_HCKEN		0x00000002
 #define SYSCTL_IPGEN		0x00000001
-#endif
 #define SYSCTL_RSTA		0x01000000
 #define SYSCTL_RSTC		0x02000000
 #define SYSCTL_RSTD		0x04000000
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 11/11] mmc: fsl_esdhc_imx: set sysctl register for clock initialization
  2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
                   ` (9 preceding siblings ...)
  2021-11-12 19:15 ` [PATCH v2 10/11] mmc: fsl_esdhc_imx: replace most #ifdefs by IS_ENABLED() Sean Anderson
@ 2021-11-12 19:15 ` Sean Anderson
  10 siblings, 0 replies; 25+ messages in thread
From: Sean Anderson @ 2021-11-12 19:15 UTC (permalink / raw)
  To: u-boot, Peng Fan, Jaehoon Chung
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam, Sean Anderson

[ fsl_esdhc commit 263ddfc3454ead3a988adef39b962479adce2b28 ]

The initial clock setting should be through sysctl register only,
while the mmc_set_clock() will call mmc_set_ios() introduce other
configurations like bus width, mode, and so on.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 89572509a7..636c35bc7c 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -1022,7 +1022,7 @@ static int esdhc_init_common(struct fsl_esdhc_priv *priv, struct mmc *mmc)
 #endif
 
 	/* Set the initial clock speed */
-	mmc_set_clock(mmc, 400000, MMC_CLK_ENABLE);
+	set_sysctl(priv, mmc, 400000);
 
 	/* Disable the BRR and BWR bits in IRQSTAT */
 	esdhc_clrbits32(&regs->irqstaten, IRQSTATEN_BRR | IRQSTATEN_BWR);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 01/11] mmc: fsl_esdhc_imx: make BLK as hard requirement of DM_MMC
  2021-11-12 19:15 ` [PATCH v2 01/11] mmc: fsl_esdhc_imx: make BLK as hard requirement of DM_MMC Sean Anderson
@ 2021-11-15  8:35   ` Jaehoon Chung
  0 siblings, 0 replies; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:35 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit 41dec2fe99512e941261594f522b2e7d485c314b ]
> 
> U-boot prefers DM_MMC + BLK for MMC. Now eSDHC driver has already
> support it, so let's force to use it.
> 
> - Drop non-BLK support for DM_MMC introduced by below patch.
>   66fa035 mmc: fsl_esdhc: fix probe issue without CONFIG_BLK enabled
> 
> - Support only DM_MMC + BLK (assuming BLK is always enabled for DM_MMC).
> 
> - Use DM_MMC instead of BLK for conditional compile.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/Kconfig         |  2 ++
>  drivers/mmc/fsl_esdhc_imx.c | 33 +--------------------------------
>  2 files changed, 3 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 1569e8c44a..313244682a 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -790,6 +790,7 @@ endif
>  
>  config FSL_ESDHC
>  	bool "Freescale/NXP eSDHC controller support"
> +	depends on BLK
>  	help
>  	  This selects support for the eSDHC (Enhanced Secure Digital Host
>  	  Controller) found on numerous Freescale/NXP SoCs.
> @@ -826,6 +827,7 @@ config FSL_ESDHC_VS33_NOT_SUPPORT
>  
>  config FSL_ESDHC_IMX
>  	bool "Freescale/NXP i.MX eSDHC controller support"
> +	depends on BLK
>  	help
>  	  This selects support for the i.MX eSDHC (Enhanced Secure Digital Host
>  	  Controller) found on numerous Freescale/NXP SoCs.
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index aabf39535f..0a7f7e61cb 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -39,10 +39,6 @@
>  #include <dm/ofnode.h>
>  #include <linux/iopoll.h>
>  
> -#if !CONFIG_IS_ENABLED(BLK)
> -#include "mmc_private.h"
> -#endif
> -
>  #ifndef ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
>  #ifdef CONFIG_FSL_USDHC
>  #define ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE	1
> @@ -58,7 +54,6 @@ DECLARE_GLOBAL_DATA_PTR;
>  				IRQSTATEN_DEBE | IRQSTATEN_BRR | IRQSTATEN_BWR | \
>  				IRQSTATEN_DINT)
>  #define MAX_TUNING_LOOP 40
> -#define ESDHC_DRIVER_STAGE_VALUE 0xffffffff
>  
>  struct fsl_esdhc {
>  	uint    dsaddr;		/* SDMA system address register */
> @@ -157,7 +152,7 @@ struct fsl_esdhc_priv {
>  	unsigned int clock;
>  	unsigned int mode;
>  	unsigned int bus_width;
> -#if !CONFIG_IS_ENABLED(BLK)
> +#if !CONFIG_IS_ENABLED(DM_MMC)
>  	struct mmc *mmc;
>  #endif
>  	struct udevice *dev;
> @@ -1506,9 +1501,6 @@ static int fsl_esdhc_probe(struct udevice *dev)
>  	struct esdhc_soc_data *data =
>  		(struct esdhc_soc_data *)dev_get_driver_data(dev);
>  	struct mmc *mmc;
> -#if !CONFIG_IS_ENABLED(BLK)
> -	struct blk_desc *bdesc;
> -#endif
>  	int ret;
>  
>  #if CONFIG_IS_ENABLED(OF_PLATDATA)
> @@ -1607,25 +1599,6 @@ static int fsl_esdhc_probe(struct udevice *dev)
>  	mmc = &plat->mmc;
>  	mmc->cfg = &plat->cfg;
>  	mmc->dev = dev;
> -#if !CONFIG_IS_ENABLED(BLK)
> -	mmc->priv = priv;
> -
> -	/* Setup dsr related values */
> -	mmc->dsr_imp = 0;
> -	mmc->dsr = ESDHC_DRIVER_STAGE_VALUE;
> -	/* Setup the universal parts of the block interface just once */
> -	bdesc = mmc_get_blk_desc(mmc);
> -	bdesc->if_type = IF_TYPE_MMC;
> -	bdesc->removable = 1;
> -	bdesc->devnum = mmc_get_next_devnum();
> -	bdesc->block_read = mmc_bread;
> -	bdesc->block_write = mmc_bwrite;
> -	bdesc->block_erase = mmc_berase;
> -
> -	/* setup initial part type */
> -	bdesc->part_type = mmc->cfg->part_type;
> -	mmc_list_add(mmc);
> -#endif
>  
>  	upriv->mmc = mmc;
>  
> @@ -1730,14 +1703,12 @@ static const struct udevice_id fsl_esdhc_ids[] = {
>  	{ /* sentinel */ }
>  };
>  
> -#if CONFIG_IS_ENABLED(BLK)
>  static int fsl_esdhc_bind(struct udevice *dev)
>  {
>  	struct fsl_esdhc_plat *plat = dev_get_plat(dev);
>  
>  	return mmc_bind(dev, &plat->mmc, &plat->cfg);
>  }
> -#endif
>  
>  U_BOOT_DRIVER(fsl_esdhc) = {
>  	.name	= "fsl_esdhc",
> @@ -1745,9 +1716,7 @@ U_BOOT_DRIVER(fsl_esdhc) = {
>  	.of_match = fsl_esdhc_ids,
>  	.of_to_plat = fsl_esdhc_of_to_plat,
>  	.ops	= &fsl_esdhc_ops,
> -#if CONFIG_IS_ENABLED(BLK)
>  	.bind	= fsl_esdhc_bind,
> -#endif
>  	.probe	= fsl_esdhc_probe,
>  	.plat_auto	= sizeof(struct fsl_esdhc_plat),
>  	.priv_auto	= sizeof(struct fsl_esdhc_priv),
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 02/11] mmc: fsl_esdhc_imx: remove redundant DM_MMC checking
  2021-11-12 19:15 ` [PATCH v2 02/11] mmc: fsl_esdhc_imx: remove redundant DM_MMC checking Sean Anderson
@ 2021-11-15  8:35   ` Jaehoon Chung
  0 siblings, 0 replies; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:35 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit 2913926f3b3dec282f8773e3c02377c9600d8267 ]
> 
> Remove redundant DM_MMC checking which is already in DM_MMC conditional
> compile block.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 0a7f7e61cb..40886f37aa 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -1605,7 +1605,6 @@ static int fsl_esdhc_probe(struct udevice *dev)
>  	return esdhc_init_common(priv, mmc);
>  }
>  
> -#if CONFIG_IS_ENABLED(DM_MMC)
>  static int fsl_esdhc_get_cd(struct udevice *dev)
>  {
>  	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
> @@ -1671,7 +1670,6 @@ static const struct dm_mmc_ops fsl_esdhc_ops = {
>  #endif
>  	.wait_dat0 = fsl_esdhc_wait_dat0,
>  };
> -#endif
>  
>  static struct esdhc_soc_data usdhc_imx7d_data = {
>  	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 03/11] mmc: fsl_esdhc_imx: fix voltage validation
  2021-11-12 19:15 ` [PATCH v2 03/11] mmc: fsl_esdhc_imx: fix voltage validation Sean Anderson
@ 2021-11-15  8:36   ` Jaehoon Chung
  0 siblings, 0 replies; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:36 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit 5b05fc0310cd933acf76ee661577c6b07a95e684 ]
> 
> Voltage validation should be done by CMD8. Current comparison between
> mmc_cfg voltages and host voltage capabilities is meaningless.
> So drop current comparison and let voltage validation is through CMD8.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 35 +++++++++++++----------------------
>  include/fsl_esdhc_imx.h     | 12 ++++++------
>  2 files changed, 19 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 40886f37aa..b91dda27f9 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -1164,7 +1164,7 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>  {
>  	struct mmc_config *cfg;
>  	struct fsl_esdhc *regs;
> -	u32 caps, voltage_caps;
> +	u32 caps;
>  	int ret;
>  
>  	if (!priv)
> @@ -1203,9 +1203,7 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>  	memset(cfg, '\0', sizeof(*cfg));
>  #endif
>  
> -	voltage_caps = 0;
>  	caps = esdhc_read32(&regs->hostcapblt);
> -
>  #ifdef CONFIG_MCF5441x
>  	/*
>  	 * MCF5441x RM declares in more points that sdhc clock speed must
> @@ -1216,31 +1214,24 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>  #endif
>  
>  #ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC135
> -	caps = caps & ~(ESDHC_HOSTCAPBLT_SRS |
> -			ESDHC_HOSTCAPBLT_VS18 | ESDHC_HOSTCAPBLT_VS30);
> +	caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
>  #endif
>  
> -	if (caps & ESDHC_HOSTCAPBLT_VS18)
> -		voltage_caps |= MMC_VDD_165_195;
> -	if (caps & ESDHC_HOSTCAPBLT_VS30)
> -		voltage_caps |= MMC_VDD_29_30 | MMC_VDD_30_31;
> -	if (caps & ESDHC_HOSTCAPBLT_VS33)
> -		voltage_caps |= MMC_VDD_32_33 | MMC_VDD_33_34;
> +#ifdef CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33
> +	caps |= HOSTCAPBLT_VS33;
> +#endif
> +
> +	if (caps & HOSTCAPBLT_VS18)
> +		cfg->voltages |= MMC_VDD_165_195;
> +	if (caps & HOSTCAPBLT_VS30)
> +		cfg->voltages |= MMC_VDD_29_30 | MMC_VDD_30_31;
> +	if (caps & HOSTCAPBLT_VS33)
> +		cfg->voltages |= MMC_VDD_32_33 | MMC_VDD_33_34;
>  
>  	cfg->name = "FSL_SDHC";
>  #if !CONFIG_IS_ENABLED(DM_MMC)
>  	cfg->ops = &esdhc_ops;
>  #endif
> -#ifdef CONFIG_SYS_SD_VOLTAGE
> -	cfg->voltages = CONFIG_SYS_SD_VOLTAGE;
> -#else
> -	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
> -#endif
> -	if ((cfg->voltages & voltage_caps) == 0) {
> -		printf("voltage not supported by controller\n");
> -		return -1;
> -	}
> -
>  	if (priv->bus_width == 8)
>  		cfg->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
>  	else if (priv->bus_width == 4)
> @@ -1258,7 +1249,7 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>  			cfg->host_caps &= ~MMC_MODE_4BIT;
>  	}
>  
> -	if (caps & ESDHC_HOSTCAPBLT_HSS)
> +	if (caps & HOSTCAPBLT_HSS)
>  		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
>  
>  #ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
> diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
> index 45ed635a77..1529b8bba3 100644
> --- a/include/fsl_esdhc_imx.h
> +++ b/include/fsl_esdhc_imx.h
> @@ -164,12 +164,12 @@
>  #define BLKATTR_SIZE(x)	(x & 0x1fff)
>  #define MAX_BLK_CNT	0x7fff	/* so malloc will have enough room with 32M */
>  
> -#define ESDHC_HOSTCAPBLT_VS18	0x04000000
> -#define ESDHC_HOSTCAPBLT_VS30	0x02000000
> -#define ESDHC_HOSTCAPBLT_VS33	0x01000000
> -#define ESDHC_HOSTCAPBLT_SRS	0x00800000
> -#define ESDHC_HOSTCAPBLT_DMAS	0x00400000
> -#define ESDHC_HOSTCAPBLT_HSS	0x00200000
> +#define HOSTCAPBLT_VS18	0x04000000
> +#define HOSTCAPBLT_VS30	0x02000000
> +#define HOSTCAPBLT_VS33	0x01000000
> +#define HOSTCAPBLT_SRS	0x00800000
> +#define HOSTCAPBLT_DMAS	0x00400000
> +#define HOSTCAPBLT_HSS	0x00200000
>  
>  #define ESDHC_VENDORSPEC_VSELECT 0x00000002 /* Use 1.8V */
>  
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 04/11] mmc: fsl_esdhc_imx: clean up bus width configuration code
  2021-11-12 19:15 ` [PATCH v2 04/11] mmc: fsl_esdhc_imx: clean up bus width configuration code Sean Anderson
@ 2021-11-15  8:36   ` Jaehoon Chung
  0 siblings, 0 replies; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:36 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit 07bae1de382723b94244096953b05225572728cd ]
> 
> This patch is to clean up bus width setting code.
> 
> - For DM_MMC, remove getting "bus-width" from device tree.
>   This has been done in mmc_of_parse().
> 
> - For non-DM_MMC, move bus width configuration from fsl_esdhc_init()
>   to fsl_esdhc_initialize() which is non-DM_MMC specific.
>   And fix up bus width configuration to support only 1-bit, 4-bit,
>   or 8-bit. Keep using 8-bit if it's not set because many platforms
>   use driver without providing max bus width.
> 
> - Remove bus_width member from fsl_esdhc_priv structure.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> [ converted if statement to switch ]
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> Changes in v2:
> - Use a switch statement instead of ifs for max_bus_width
> - Only default to 8 bit width when max_bus_width is not set
> 
>  drivers/mmc/fsl_esdhc_imx.c | 83 ++++++++++++-------------------------
>  1 file changed, 26 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index b91dda27f9..b604729750 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -126,7 +126,6 @@ struct esdhc_soc_data {
>   *
>   * @esdhc_regs: registers of the sdhc controller
>   * @sdhc_clk: Current clk of the sdhc controller
> - * @bus_width: bus width, 1bit, 4bit or 8bit
>   * @cfg: mmc config
>   * @mmc: mmc
>   * Following is used when Driver Model is enabled for MMC
> @@ -151,7 +150,6 @@ struct fsl_esdhc_priv {
>  	struct clk per_clk;
>  	unsigned int clock;
>  	unsigned int mode;
> -	unsigned int bus_width;
>  #if !CONFIG_IS_ENABLED(DM_MMC)
>  	struct mmc *mmc;
>  #endif
> @@ -1232,31 +1230,13 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>  #if !CONFIG_IS_ENABLED(DM_MMC)
>  	cfg->ops = &esdhc_ops;
>  #endif
> -	if (priv->bus_width == 8)
> -		cfg->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
> -	else if (priv->bus_width == 4)
> -		cfg->host_caps = MMC_MODE_4BIT;
> -
> -	cfg->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
>  #ifdef CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE
>  	cfg->host_caps |= MMC_MODE_DDR_52MHz;
>  #endif
>  
> -	if (priv->bus_width > 0) {
> -		if (priv->bus_width < 8)
> -			cfg->host_caps &= ~MMC_MODE_8BIT;
> -		if (priv->bus_width < 4)
> -			cfg->host_caps &= ~MMC_MODE_4BIT;
> -	}
> -
>  	if (caps & HOSTCAPBLT_HSS)
>  		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
>  
> -#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
> -	if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
> -		cfg->host_caps &= ~MMC_MODE_8BIT;
> -#endif
> -
>  	cfg->host_caps |= priv->caps;
>  
>  	cfg->f_min = 400000;
> @@ -1294,25 +1274,11 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>  }
>  
>  #if !CONFIG_IS_ENABLED(DM_MMC)
> -static int fsl_esdhc_cfg_to_priv(struct fsl_esdhc_cfg *cfg,
> -				 struct fsl_esdhc_priv *priv)
> -{
> -	if (!cfg || !priv)
> -		return -EINVAL;
> -
> -	priv->esdhc_regs = (struct fsl_esdhc *)(unsigned long)(cfg->esdhc_base);
> -	priv->bus_width = cfg->max_bus_width;
> -	priv->sdhc_clk = cfg->sdhc_clk;
> -	priv->wp_enable  = cfg->wp_enable;
> -	priv->vs18_enable  = cfg->vs18_enable;
> -
> -	return 0;
> -};
> -
>  int fsl_esdhc_initialize(struct bd_info *bis, struct fsl_esdhc_cfg *cfg)
>  {
>  	struct fsl_esdhc_plat *plat;
>  	struct fsl_esdhc_priv *priv;
> +	struct mmc_config *mmc_cfg;
>  	struct mmc *mmc;
>  	int ret;
>  
> @@ -1328,14 +1294,33 @@ int fsl_esdhc_initialize(struct bd_info *bis, struct fsl_esdhc_cfg *cfg)
>  		return -ENOMEM;
>  	}
>  
> -	ret = fsl_esdhc_cfg_to_priv(cfg, priv);
> -	if (ret) {
> -		debug("%s xlate failure\n", __func__);
> -		free(plat);
> -		free(priv);
> -		return ret;
> +	priv->esdhc_regs = (struct fsl_esdhc *)(unsigned long)(cfg->esdhc_base);
> +	priv->sdhc_clk = cfg->sdhc_clk;
> +	priv->wp_enable  = cfg->wp_enable;
> +
> +	mmc_cfg = &plat->cfg;
> +
> +	switch (cfg->max_bus_width) {
> +	case 0: /* Not set in config; assume everything is supported */
> +	case 8:
> +		mmc_cfg->host_caps |= MMC_MODE_8BIT;
> +		fallthrough;
> +	case 4:
> +		mmc_cfg->host_caps |= MMC_MODE_4BIT;
> +		fallthrough;
> +	case 1:
> +		mmc_cfg->host_caps |= MMC_MODE_1BIT;
> +		break;
> +	default:
> +		printf("invalid max bus width %u\n", cfg->max_bus_width);
> +		return -EINVAL;
>  	}
>  
> +#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
> +	if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
> +		mmc_cfg->host_caps &= ~MMC_MODE_8BIT;
> +#endif
> +
>  	ret = fsl_esdhc_init(priv, plat);
>  	if (ret) {
>  		debug("%s init failure\n", __func__);
> @@ -1416,14 +1401,6 @@ static int fsl_esdhc_of_to_plat(struct udevice *dev)
>  	priv->dev = dev;
>  	priv->mode = -1;
>  
> -	val = dev_read_u32_default(dev, "bus-width", -1);
> -	if (val == 8)
> -		priv->bus_width = 8;
> -	else if (val == 4)
> -		priv->bus_width = 4;
> -	else
> -		priv->bus_width = 1;
> -
>  	val = fdtdec_get_int(fdt, node, "fsl,tuning-step", 1);
>  	priv->tuning_step = val;
>  	val = fdtdec_get_int(fdt, node, "fsl,tuning-start-tap",
> @@ -1496,16 +1473,8 @@ static int fsl_esdhc_probe(struct udevice *dev)
>  
>  #if CONFIG_IS_ENABLED(OF_PLATDATA)
>  	struct dtd_fsl_esdhc *dtplat = &plat->dtplat;
> -	unsigned int val;
>  
>  	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;
>  
>  	if (dtplat->non_removable)
>  		priv->non_removable = 1;
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 05/11] mmc: fsl_esdhc_imx: drop redundant code for non-removable feature
  2021-11-12 19:15 ` [PATCH v2 05/11] mmc: fsl_esdhc_imx: drop redundant code for non-removable feature Sean Anderson
@ 2021-11-15  8:37   ` Jaehoon Chung
  0 siblings, 0 replies; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:37 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit commit 08197cb8dff7cd097ab07a325093043c39d19bbd ]
> 
> Drop redundant code for non-removable feature. "non-removable" property
> has been read in mmc_of_parse().
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index b604729750..b2844d0a7b 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -130,7 +130,6 @@ struct esdhc_soc_data {
>   * @mmc: mmc
>   * Following is used when Driver Model is enabled for MMC
>   * @dev: pointer for the device
> - * @non_removable: 0: removable; 1: non-removable
>   * @broken_cd: 0: use GPIO for card detect; 1: Do not use GPIO for card detect
>   * @wp_enable: 1: enable checking wp; 0: no check
>   * @vs18_enable: 1: use 1.8V voltage; 0: use 3.3V
> @@ -154,7 +153,6 @@ struct fsl_esdhc_priv {
>  	struct mmc *mmc;
>  #endif
>  	struct udevice *dev;
> -	int non_removable;
>  	int broken_cd;
>  	int wp_enable;
>  	int vs18_enable;
> @@ -1083,9 +1081,6 @@ static int esdhc_getcd_common(struct fsl_esdhc_priv *priv)
>  #endif
>  
>  #if CONFIG_IS_ENABLED(DM_MMC)
> -	if (priv->non_removable)
> -		return 1;
> -
>  	if (priv->broken_cd)
>  		return 1;
>  #if CONFIG_IS_ENABLED(DM_GPIO)
> @@ -1415,25 +1410,18 @@ static int fsl_esdhc_of_to_plat(struct udevice *dev)
>  	if (dev_read_bool(dev, "broken-cd"))
>  		priv->broken_cd = 1;
>  
> -	if (dev_read_bool(dev, "non-removable")) {
> -		priv->non_removable = 1;
> -	 } else {
> -		priv->non_removable = 0;
> -#if CONFIG_IS_ENABLED(DM_GPIO)
> -		gpio_request_by_name(dev, "cd-gpios", 0, &priv->cd_gpio,
> -				     GPIOD_IS_IN);
> -#endif
> -	}
> -
>  	if (dev_read_prop(dev, "fsl,wp-controller", NULL)) {
>  		priv->wp_enable = 1;
>  	} else {
>  		priv->wp_enable = 0;
> +	}
> +
>  #if CONFIG_IS_ENABLED(DM_GPIO)
> -		gpio_request_by_name(dev, "wp-gpios", 0, &priv->wp_gpio,
> -				   GPIOD_IS_IN);
> +	gpio_request_by_name(dev, "cd-gpios", 0, &priv->cd_gpio,
> +			     GPIOD_IS_IN);
> +	gpio_request_by_name(dev, "wp-gpios", 0, &priv->wp_gpio,
> +			     GPIOD_IS_IN);
>  #endif
> -	}
>  
>  	priv->vs18_enable = 0;
>  
> @@ -1567,8 +1555,12 @@ static int fsl_esdhc_probe(struct udevice *dev)
>  
>  static int fsl_esdhc_get_cd(struct udevice *dev)
>  {
> +	struct fsl_esdhc_plat *plat = dev_get_plat(dev);
>  	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
>  
> +	if (plat->cfg.host_caps & MMC_CAP_NONREMOVABLE)
> +		return 1;
> +
>  	return esdhc_getcd_common(priv);
>  }
>  
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 06/11] mmc: fsl_esdhc_imx: fix mmc->clock with actual clock
  2021-11-12 19:15 ` [PATCH v2 06/11] mmc: fsl_esdhc_imx: fix mmc->clock with actual clock Sean Anderson
@ 2021-11-15  8:37   ` Jaehoon Chung
  0 siblings, 0 replies; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:37 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit 30f6444d024a74ee48aa6969c1531aecd3c59deb ]
> 
> Fix mmc->clock with actual clock which is divided by the
> controller, and record it with priv->clock.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index b2844d0a7b..95cde9e410 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -665,6 +665,7 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>  	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
>  #endif
>  
> +	mmc->clock = sdhc_clk / pre_div / div;
>  	priv->clock = clock;
>  }
>  
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers
  2021-11-12 19:15 ` [PATCH v2 07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers Sean Anderson
@ 2021-11-15  8:39   ` Jaehoon Chung
  2021-11-15 15:12     ` Sean Anderson
  0 siblings, 1 reply; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:39 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit da86e8cfcb03ed5c1d8e0718bc8bc8583e60ced8 ]
> 
> SDMA can only do DMA with 32 bit addresses. This is true for all
> architectures (just doesn't apply to 32 bit ones). Simplify the code and
> remove unnecessary CONFIG_FSL_LAYERSCAPE.
> 
> Also make the error message more concise.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Just add minor comments.

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 33 ++++++---------------------------
>  1 file changed, 6 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 95cde9e410..3588056759 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -282,10 +282,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  {
>  	int timeout;
>  	struct fsl_esdhc *regs = priv->esdhc_regs;
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
> -	defined(CONFIG_IMX8ULP)
>  	dma_addr_t addr;
> -#endif
>  	uint wml_value;
>  
>  	wml_value = data->blocksize/4;
> @@ -296,16 +293,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  
>  		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
> -	defined(CONFIG_IMX8ULP)
>  		addr = virt_to_phys((void *)(data->dest));
>  		if (upper_32_bits(addr))
> -			printf("Error found for upper 32 bits\n");
> -		else
> -			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
> -#else
> -		esdhc_write32(&regs->dsaddr, (u32)data->dest);
> -#endif
> +			printf("Cannot use 64 bit addresses with SDMA\n");
> +		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>  #endif
>  	} else {
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> @@ -334,16 +325,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
>  					wml_value << 16);
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
> -		defined(CONFIG_IMX8ULP)
>  		addr = virt_to_phys((void *)(data->src));
>  		if (upper_32_bits(addr))
> -			printf("Error found for upper 32 bits\n");
> -		else
> -			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
> -#else
> -		esdhc_write32(&regs->dsaddr, (u32)data->src);
> -#endif
> +			printf("Cannot use 64 bit addresses with SDMA\n");

Fix indentation?

> +		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>  #endif
>  	}
>  
> @@ -400,18 +385,12 @@ static void check_and_invalidate_dcache_range
>  	unsigned end = 0;
>  	unsigned size = roundup(ARCH_DMA_MINALIGN,
>  				data->blocks*data->blocksize);
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
> -	defined(CONFIG_IMX8ULP)
>  	dma_addr_t addr;
>  
>  	addr = virt_to_phys((void *)(data->dest));
>  	if (upper_32_bits(addr))
> -		printf("Error found for upper 32 bits\n");
> -	else
> -		start = lower_32_bits(addr);
> -#else
> -	start = (unsigned)data->dest;
> -#endif
> +		printf("Cannot use 64 bit addresses with SDMA\n");

Ditto.

> +	start = lower_32_bits(addr);
>  	end = start + size;
>  	invalidate_dcache_range(start, end);
>  }
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 08/11] mmc: fsl_esdhc_imx: use dma-mapping API
  2021-11-12 19:15 ` [PATCH v2 08/11] mmc: fsl_esdhc_imx: use dma-mapping API Sean Anderson
@ 2021-11-15  8:40   ` Jaehoon Chung
  0 siblings, 0 replies; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:40 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit b1ba1460a445bcc67972a617625d0349e4f22b31 ]
> 
> Use the dma_{map,unmap}_single() calls. These will take care of the
> flushing and invalidation of caches.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 50 +++++++++++--------------------------
>  1 file changed, 15 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 3588056759..afc8259323 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -38,6 +38,7 @@
>  #include <mapmem.h>
>  #include <dm/ofnode.h>
>  #include <linux/iopoll.h>
> +#include <linux/dma-mapping.h>
>  
>  #ifndef ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
>  #ifdef CONFIG_FSL_USDHC
> @@ -171,6 +172,7 @@ struct fsl_esdhc_priv {
>  	struct gpio_desc cd_gpio;
>  	struct gpio_desc wp_gpio;
>  #endif
> +	dma_addr_t dma_addr;
>  };
>  
>  /* Return the XFERTYP flags for a given command and data packet */
> @@ -281,8 +283,8 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  			    struct mmc_data *data)
>  {
>  	int timeout;
> +	uint trans_bytes = data->blocksize * data->blocks;
>  	struct fsl_esdhc *regs = priv->esdhc_regs;
> -	dma_addr_t addr;
>  	uint wml_value;
>  
>  	wml_value = data->blocksize/4;
> @@ -293,17 +295,13 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  
>  		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -		addr = virt_to_phys((void *)(data->dest));
> -		if (upper_32_bits(addr))
> +		priv->dma_addr = dma_map_single(data->dest, trans_bytes,
> +						mmc_get_dma_dir(data));
> +		if (upper_32_bits(priv->dma_addr))
>  			printf("Cannot use 64 bit addresses with SDMA\n");
> -		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
> +		esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
>  #endif
>  	} else {
> -#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -		flush_dcache_range((ulong)data->src,
> -				   (ulong)data->src+data->blocks
> -					 *data->blocksize);
> -#endif
>  		if (wml_value > WML_WR_WML_MAX)
>  			wml_value = WML_WR_WML_MAX_VAL;
>  		if (priv->wp_enable) {
> @@ -325,10 +323,11 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
>  					wml_value << 16);
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -		addr = virt_to_phys((void *)(data->src));
> -		if (upper_32_bits(addr))
> +		priv->dma_addr = dma_map_single((void *)data->src, trans_bytes,
> +						mmc_get_dma_dir(data));
> +		if (upper_32_bits(priv->dma_addr))
>  			printf("Cannot use 64 bit addresses with SDMA\n");
> -		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
> +		esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
>  #endif
>  	}
>  
> @@ -378,23 +377,6 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  	return 0;
>  }
>  
> -static void check_and_invalidate_dcache_range
> -	(struct mmc_cmd *cmd,
> -	 struct mmc_data *data) {
> -	unsigned start = 0;
> -	unsigned end = 0;
> -	unsigned size = roundup(ARCH_DMA_MINALIGN,
> -				data->blocks*data->blocksize);
> -	dma_addr_t addr;
> -
> -	addr = virt_to_phys((void *)(data->dest));
> -	if (upper_32_bits(addr))
> -		printf("Cannot use 64 bit addresses with SDMA\n");
> -	start = lower_32_bits(addr);
> -	end = start + size;
> -	invalidate_dcache_range(start, end);
> -}
> -
>  #ifdef CONFIG_MCF5441x
>  /*
>   * Swaps 32-bit words to little-endian byte order.
> @@ -450,9 +432,6 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  		err = esdhc_setup_data(priv, mmc, data);
>  		if(err)
>  			return err;
> -
> -		if (data->flags & MMC_DATA_READ)
> -			check_and_invalidate_dcache_range(cmd, data);
>  	}
>  
>  	/* Figure out the transfer arguments */
> @@ -560,12 +539,13 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  		 * cache-fill during the DMA operations such as the
>  		 * speculative pre-fetching etc.
>  		 */
> -		if (data->flags & MMC_DATA_READ) {
> -			check_and_invalidate_dcache_range(cmd, data);
> +		dma_unmap_single(priv->dma_addr,
> +				 data->blocks * data->blocksize,
> +				 mmc_get_dma_dir(data));
>  #ifdef CONFIG_MCF5441x
> +		if (data->flags & MMC_DATA_READ)
>  			sd_swap_dma_buff(data);
>  #endif
> -		}
>  #endif
>  	}
>  
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 09/11] mmc: fsl_esdhc_imx: simplify esdhc_setup_data()
  2021-11-12 19:15 ` [PATCH v2 09/11] mmc: fsl_esdhc_imx: simplify esdhc_setup_data() Sean Anderson
@ 2021-11-15  8:41   ` Jaehoon Chung
  0 siblings, 0 replies; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:41 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit 7e48a028a42c111ba38a90b86e5f57dace980fa0 ]
> 
> First, we need the waterlevel setting for PIO mode only. Secondy, both DMA
> setup code is identical for both directions, except for the data pointer.
> Thus, unify them.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>


Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 89 ++++++++++++++++++++++---------------
>  1 file changed, 52 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index afc8259323..aa3d0877cb 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -279,59 +279,74 @@ static void esdhc_pio_read_write(struct fsl_esdhc_priv *priv,
>  }
>  #endif
>  
> -static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
> -			    struct mmc_data *data)
> +#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> +static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
> +					struct mmc_data *data)
>  {
> -	int timeout;
> -	uint trans_bytes = data->blocksize * data->blocks;
>  	struct fsl_esdhc *regs = priv->esdhc_regs;
> -	uint wml_value;
> -
> -	wml_value = data->blocksize/4;
> +	uint wml_value = data->blocksize / 4;
>  
>  	if (data->flags & MMC_DATA_READ) {
>  		if (wml_value > WML_RD_WML_MAX)
>  			wml_value = WML_RD_WML_MAX_VAL;
>  
>  		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
> -#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -		priv->dma_addr = dma_map_single(data->dest, trans_bytes,
> -						mmc_get_dma_dir(data));
> -		if (upper_32_bits(priv->dma_addr))
> -			printf("Cannot use 64 bit addresses with SDMA\n");
> -		esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
> -#endif
>  	} else {
>  		if (wml_value > WML_WR_WML_MAX)
>  			wml_value = WML_WR_WML_MAX_VAL;
> -		if (priv->wp_enable) {
> -			if ((esdhc_read32(&regs->prsstat) &
> -			    PRSSTAT_WPSPL) == 0) {
> -				printf("\nThe SD card is locked. Can not write to a locked card.\n\n");
> -				return -ETIMEDOUT;
> -			}
> -		} else {
> -#if CONFIG_IS_ENABLED(DM_GPIO)
> -			if (dm_gpio_is_valid(&priv->wp_gpio) &&
> -			    dm_gpio_get_value(&priv->wp_gpio)) {
> -				printf("\nThe SD card is locked. Can not write to a locked card.\n\n");
> -				return -ETIMEDOUT;
> -			}
> -#endif
> -		}
>  
>  		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
> -					wml_value << 16);
> -#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -		priv->dma_addr = dma_map_single((void *)data->src, trans_bytes,
> -						mmc_get_dma_dir(data));
> -		if (upper_32_bits(priv->dma_addr))
> -			printf("Cannot use 64 bit addresses with SDMA\n");
> -		esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
> -#endif
> +				   wml_value << 16);
>  	}
> +}
> +#endif
>  
> +static void esdhc_setup_dma(struct fsl_esdhc_priv *priv, struct mmc_data *data)
> +{
> +	uint trans_bytes = data->blocksize * data->blocks;
> +	struct fsl_esdhc *regs = priv->esdhc_regs;
> +	void *buf;
> +
> +	if (data->flags & MMC_DATA_WRITE)
> +		buf = (void *)data->src;
> +	else
> +		buf = data->dest;
> +
> +	priv->dma_addr = dma_map_single(buf, trans_bytes,
> +					mmc_get_dma_dir(data));
> +	if (upper_32_bits(priv->dma_addr))
> +		printf("Cannot use 64 bit addresses with SDMA\n");
> +	esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
>  	esdhc_write32(&regs->blkattr, data->blocks << 16 | data->blocksize);
> +}
> +
> +static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
> +			    struct mmc_data *data)
> +{
> +	int timeout;
> +	bool is_write = data->flags & MMC_DATA_WRITE;
> +	struct fsl_esdhc *regs = priv->esdhc_regs;
> +
> +	if (is_write) {
> +		if (priv->wp_enable && !(esdhc_read32(&regs->prsstat) & PRSSTAT_WPSPL)) {
> +			printf("Cannot write to locked SD card.\n");
> +			return -EINVAL;
> +		} else {
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +			if (dm_gpio_is_valid(&priv->wp_gpio) &&
> +			    dm_gpio_get_value(&priv->wp_gpio)) {
> +				printf("Cannot write to locked SD card.\n");
> +				return -EINVAL;
> +			}
> +#endif
> +		}
> +	}
> +
> +#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> +	esdhc_setup_watermark_level(priv, data);
> +#else
> +	esdhc_setup_dma(priv, data);
> +#endif
>  
>  	/* Calculate the timeout period for data transactions */
>  	/*
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 10/11] mmc: fsl_esdhc_imx: replace most #ifdefs by IS_ENABLED()
  2021-11-12 19:15 ` [PATCH v2 10/11] mmc: fsl_esdhc_imx: replace most #ifdefs by IS_ENABLED() Sean Anderson
@ 2021-11-15  8:44   ` Jaehoon Chung
  2021-11-15 15:13     ` Sean Anderson
  0 siblings, 1 reply; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15  8:44 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit 52faec31827ec1a1837977e29c067424426634c5 ]
> 
> Make the code cleaner and drop the old-style #ifdef constructs where it is
> possible.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 209 +++++++++++++++++-------------------
>  include/fsl_esdhc_imx.h     |   2 -
>  2 files changed, 100 insertions(+), 111 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index aa3d0877cb..89572509a7 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -182,15 +182,15 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
>  
>  	if (data) {
>  		xfertyp |= XFERTYP_DPSEL;
> -#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -		xfertyp |= XFERTYP_DMAEN;
> -#endif
> +		if (!IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO) &&
> +		    cmd->cmdidx != MMC_CMD_SEND_TUNING_BLOCK &&
> +		    cmd->cmdidx != MMC_CMD_SEND_TUNING_BLOCK_HS200)
> +			xfertyp |= XFERTYP_DMAEN;
>  		if (data->blocks > 1) {
>  			xfertyp |= XFERTYP_MSBSEL;
>  			xfertyp |= XFERTYP_BCEN;
> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
> -			xfertyp |= XFERTYP_AC12EN;
> -#endif
> +			if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC111))
> +				xfertyp |= XFERTYP_AC12EN;
>  		}
>  
>  		if (data->flags & MMC_DATA_READ)
> @@ -214,7 +214,6 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
>  	return XFERTYP_CMD(cmd->cmdidx) | xfertyp;
>  }
>  
> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
>  /*
>   * PIO Read/Write Mode reduce the performace as DMA is not used in this mode.
>   */
> @@ -277,9 +276,7 @@ static void esdhc_pio_read_write(struct fsl_esdhc_priv *priv,
>  		}
>  	}
>  }
> -#endif
>  
> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
>  static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
>  					struct mmc_data *data)
>  {
> @@ -299,7 +296,6 @@ static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
>  				   wml_value << 16);
>  	}
>  }
> -#endif
>  
>  static void esdhc_setup_dma(struct fsl_esdhc_priv *priv, struct mmc_data *data)
>  {
> @@ -342,11 +338,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  		}
>  	}
>  
> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -	esdhc_setup_watermark_level(priv, data);
> -#else
> -	esdhc_setup_dma(priv, data);
> -#endif
> +	if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO))
> +		esdhc_setup_watermark_level(priv, data);
> +	else
> +		esdhc_setup_dma(priv, data);
>  
>  	/* Calculate the timeout period for data transactions */
>  	/*
> @@ -379,14 +374,13 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  	if (timeout < 0)
>  		timeout = 0;
>  
> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC_A001
> -	if ((timeout == 4) || (timeout == 8) || (timeout == 12))
> +	if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC_A001) &&
> +	    (timeout == 4 || timeout == 8 || timeout == 12))
>  		timeout++;
> -#endif
>  
> -#ifdef ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
> -	timeout = 0xE;
> -#endif
> +	if (IS_ENABLED(ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE))
> +		timeout = 0xE;
> +
>  	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_TIMEOUT_MASK, timeout << 16);
>  
>  	return 0;
> @@ -409,6 +403,11 @@ static inline void sd_swap_dma_buff(struct mmc_data *data)
>  		}
>  	}
>  }
> +#else
> +static inline void sd_swap_dma_buff(struct mmc_data *data)
> +{
> +	return;
> +}
>  #endif
>  
>  /*
> @@ -425,10 +424,9 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  	struct fsl_esdhc *regs = priv->esdhc_regs;
>  	unsigned long start;
>  
> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
> -	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
> +	if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC111) &&
> +	    cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
>  		return 0;
> -#endif
>  
>  	esdhc_write32(&regs->irqstat, -1);
>  
> @@ -526,42 +524,40 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  
>  	/* Wait until all of the blocks are transferred */
>  	if (data) {
> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -		esdhc_pio_read_write(priv, data);
> -#else
> -		flags = DATA_COMPLETE;
> -		if ((cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK) ||
> -		    (cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK_HS200)) {
> -			flags = IRQSTAT_BRR;
> +		if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO)) {
> +			esdhc_pio_read_write(priv, data);
> +		} else {
> +			flags = DATA_COMPLETE;
> +			if (cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK ||
> +			    cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK_HS200)
> +				flags = IRQSTAT_BRR;
> +
> +			do {
> +				irqstat = esdhc_read32(&regs->irqstat);
> +
> +				if (irqstat & IRQSTAT_DTOE) {
> +					err = -ETIMEDOUT;
> +					goto out;
> +				}
> +
> +				if (irqstat & DATA_ERR) {
> +					err = -ECOMM;
> +					goto out;
> +				}
> +			} while ((irqstat & flags) != flags);
> +
> +			/*
> +			 * Need invalidate the dcache here again to avoid any
> +			 * cache-fill during the DMA operations such as the
> +			 * speculative pre-fetching etc.
> +			 */
> +			dma_unmap_single(priv->dma_addr,
> +					 data->blocks * data->blocksize,
> +					 mmc_get_dma_dir(data));
> +			if (IS_ENABLED(CONFIG_MCF5441x) &&
> +			    (data->flags & MMC_DATA_READ))
> +				sd_swap_dma_buff(data);
>  		}
> -
> -		do {
> -			irqstat = esdhc_read32(&regs->irqstat);
> -
> -			if (irqstat & IRQSTAT_DTOE) {
> -				err = -ETIMEDOUT;
> -				goto out;
> -			}
> -
> -			if (irqstat & DATA_ERR) {
> -				err = -ECOMM;
> -				goto out;
> -			}
> -		} while ((irqstat & flags) != flags);
> -
> -		/*
> -		 * Need invalidate the dcache here again to avoid any
> -		 * cache-fill during the DMA operations such as the
> -		 * speculative pre-fetching etc.
> -		 */
> -		dma_unmap_single(priv->dma_addr,
> -				 data->blocks * data->blocksize,
> -				 mmc_get_dma_dir(data));
> -#ifdef CONFIG_MCF5441x
> -		if (data->flags & MMC_DATA_READ)
> -			sd_swap_dma_buff(data);
> -#endif
> -#endif
>  	}
>  
>  out:
> @@ -595,21 +591,22 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>  	struct fsl_esdhc *regs = priv->esdhc_regs;
>  	int div = 1;
>  	u32 tmp;
> -	int ret;
> -#ifdef ARCH_MXC
> -#ifdef CONFIG_MX53
> -	/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
> -	int pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;
> -#else
> -	int pre_div = 1;
> -#endif
> -#else
> -	int pre_div = 2;
> -#endif
> +	int ret, pre_div;
>  	int ddr_pre_div = mmc->ddr_mode ? 2 : 1;
>  	int sdhc_clk = priv->sdhc_clk;
>  	uint clk;
>  
> +	if (IS_ENABLED(ARCH_MXC)) {
> +#ifdef CONFIG_MX53

Is there any reason not to use "IS_ENABLED()" ?

Best Regards,
Jaehoon Chung

> +		/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
> +		pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;
> +#else
> +		pre_div = 1;
> +#endif
> +	} else {
> +		pre_div = 2;
> +	}
> +
>  	while (sdhc_clk / (16 * pre_div * ddr_pre_div) > clock && pre_div < 256)
>  		pre_div *= 2;
>  
> @@ -621,11 +618,10 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>  
>  	clk = (pre_div << 8) | (div << 4);
>  
> -#ifdef CONFIG_FSL_USDHC
> -	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
> -#else
> -	esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
> -#endif
> +	if (IS_ENABLED(CONFIG_FSL_USDHC))
> +		esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
> +	else
> +		esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
>  
>  	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_CLOCK_MASK, clk);
>  
> @@ -633,11 +629,10 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>  	if (ret)
>  		pr_warn("fsl_esdhc_imx: Internal clock never stabilised.\n");
>  
> -#ifdef CONFIG_FSL_USDHC
> -	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN | VENDORSPEC_CKEN);
> -#else
> -	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
> -#endif
> +	if (IS_ENABLED(CONFIG_FSL_USDHC))
> +		esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN | VENDORSPEC_CKEN);
> +	else
> +		esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
>  
>  	mmc->clock = sdhc_clk / pre_div / div;
>  	priv->clock = clock;
> @@ -1145,22 +1140,21 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>  	if (ret)
>  		return ret;
>  
> -#ifdef CONFIG_MCF5441x
>  	/* ColdFire, using SDHC_DATA[3] for card detection */
> -	esdhc_write32(&regs->proctl, PROCTL_INIT | PROCTL_D3CD);
> -#endif
> +	if (IS_ENABLED(CONFIG_MCF5441x))
> +		esdhc_write32(&regs->proctl, PROCTL_INIT | PROCTL_D3CD);
>  
> -#ifndef CONFIG_FSL_USDHC
> -	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
> -				| SYSCTL_IPGEN | SYSCTL_CKEN);
> -	/* Clearing tuning bits in case ROM has set it already */
> -	esdhc_write32(&regs->mixctrl, 0);
> -	esdhc_write32(&regs->autoc12err, 0);
> -	esdhc_write32(&regs->clktunectrlstatus, 0);
> -#else
> -	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
> -			VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
> -#endif
> +	if (IS_ENABLED(CONFIG_FSL_USDHC)) {
> +		esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
> +				VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
> +	} else {
> +		esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
> +					| SYSCTL_IPGEN | SYSCTL_CKEN);
> +		/* Clearing tuning bits in case ROM has set it already */
> +		esdhc_write32(&regs->mixctrl, 0);
> +		esdhc_write32(&regs->autoc12err, 0);
> +		esdhc_write32(&regs->clktunectrlstatus, 0);
> +	}
>  
>  	if (priv->vs18_enable)
>  		esdhc_setbits32(&regs->vendorspec, ESDHC_VENDORSPEC_VSELECT);
> @@ -1172,22 +1166,20 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>  #endif
>  
>  	caps = esdhc_read32(&regs->hostcapblt);
> -#ifdef CONFIG_MCF5441x
> +
>  	/*
>  	 * MCF5441x RM declares in more points that sdhc clock speed must
>  	 * never exceed 25 Mhz. From this, the HS bit needs to be disabled
>  	 * from host capabilities.
>  	 */
> -	caps &= ~ESDHC_HOSTCAPBLT_HSS;
> -#endif
> +	if (IS_ENABLED(CONFIG_MCF5441x))
> +		caps &= ~HOSTCAPBLT_HSS;
>  
> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC135
> -	caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
> -#endif
> +	if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC135))
> +		caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
>  
> -#ifdef CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33
> -	caps |= HOSTCAPBLT_VS33;
> -#endif
> +	if (IS_ENABLED(CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33))
> +		caps |= HOSTCAPBLT_VS33;
>  
>  	if (caps & HOSTCAPBLT_VS18)
>  		cfg->voltages |= MMC_VDD_165_195;
> @@ -1197,12 +1189,13 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>  		cfg->voltages |= MMC_VDD_32_33 | MMC_VDD_33_34;
>  
>  	cfg->name = "FSL_SDHC";
> +
>  #if !CONFIG_IS_ENABLED(DM_MMC)
>  	cfg->ops = &esdhc_ops;
>  #endif
> -#ifdef CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE
> -	cfg->host_caps |= MMC_MODE_DDR_52MHz;
> -#endif
> +
> +	if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE))
> +		cfg->host_caps |= MMC_MODE_DDR_52MHz;
>  
>  	if (caps & HOSTCAPBLT_HSS)
>  		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
> @@ -1286,10 +1279,8 @@ int fsl_esdhc_initialize(struct bd_info *bis, struct fsl_esdhc_cfg *cfg)
>  		return -EINVAL;
>  	}
>  
> -#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
> -	if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
> +	if (IS_ENABLED(CONFIG_ESDHC_DETECT_8_BIT_QUIRK))
>  		mmc_cfg->host_caps &= ~MMC_MODE_8BIT;
> -#endif
>  
>  	ret = fsl_esdhc_init(priv, plat);
>  	if (ret) {
> diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
> index 1529b8bba3..b958a6b3bb 100644
> --- a/include/fsl_esdhc_imx.h
> +++ b/include/fsl_esdhc_imx.h
> @@ -24,12 +24,10 @@
>  #define SYSCTL_INITA		0x08000000
>  #define SYSCTL_TIMEOUT_MASK	0x000f0000
>  #define SYSCTL_CLOCK_MASK	0x0000fff0
> -#if !defined(CONFIG_FSL_USDHC)
>  #define SYSCTL_CKEN		0x00000008
>  #define SYSCTL_PEREN		0x00000004
>  #define SYSCTL_HCKEN		0x00000002
>  #define SYSCTL_IPGEN		0x00000001
> -#endif
>  #define SYSCTL_RSTA		0x01000000
>  #define SYSCTL_RSTC		0x02000000
>  #define SYSCTL_RSTD		0x04000000
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers
  2021-11-15  8:39   ` Jaehoon Chung
@ 2021-11-15 15:12     ` Sean Anderson
  2021-11-15 22:18       ` Jaehoon Chung
  0 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2021-11-15 15:12 UTC (permalink / raw)
  To: Jaehoon Chung, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam



On 11/15/21 3:39 AM, Jaehoon Chung wrote:
> On 11/13/21 4:15 AM, Sean Anderson wrote:
>> [ fsl_esdhc commit da86e8cfcb03ed5c1d8e0718bc8bc8583e60ced8 ]
>> 
>> SDMA can only do DMA with 32 bit addresses. This is true for all
>> architectures (just doesn't apply to 32 bit ones). Simplify the code and
>> remove unnecessary CONFIG_FSL_LAYERSCAPE.
>> 
>> Also make the error message more concise.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> 
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> 
> Just add minor comments.
> 
>> ---
>> 
>> (no changes since v1)
>> 
>>  drivers/mmc/fsl_esdhc_imx.c | 33 ++++++---------------------------
>>  1 file changed, 6 insertions(+), 27 deletions(-)
>> 
>> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
>> index 95cde9e410..3588056759 100644
>> --- a/drivers/mmc/fsl_esdhc_imx.c
>> +++ b/drivers/mmc/fsl_esdhc_imx.c
>> @@ -282,10 +282,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  {
>>  	int timeout;
>>  	struct fsl_esdhc *regs = priv->esdhc_regs;
>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>> -	defined(CONFIG_IMX8ULP)
>>  	dma_addr_t addr;
>> -#endif
>>  	uint wml_value;
>>  
>>  	wml_value = data->blocksize/4;
>> @@ -296,16 +293,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  
>>  		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>> -	defined(CONFIG_IMX8ULP)
>>  		addr = virt_to_phys((void *)(data->dest));
>>  		if (upper_32_bits(addr))
>> -			printf("Error found for upper 32 bits\n");
>> -		else
>> -			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>> -#else
>> -		esdhc_write32(&regs->dsaddr, (u32)data->dest);
>> -#endif
>> +			printf("Cannot use 64 bit addresses with SDMA\n");
>> +		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>  #endif
>>  	} else {
>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>> @@ -334,16 +325,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
>>  					wml_value << 16);
>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>> -		defined(CONFIG_IMX8ULP)
>>  		addr = virt_to_phys((void *)(data->src));
>>  		if (upper_32_bits(addr))
>> -			printf("Error found for upper 32 bits\n");
>> -		else
>> -			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>> -#else
>> -		esdhc_write32(&regs->dsaddr, (u32)data->src);
>> -#endif
>> +			printf("Cannot use 64 bit addresses with SDMA\n");
> 
> Fix indentation?

Looks OK to me. The final code is

		if (upper_32_bits(addr))
			printf("Cannot use 64 bit addresses with SDMA\n");


> 
>> +		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>  #endif
>>  	}
>>  
>> @@ -400,18 +385,12 @@ static void check_and_invalidate_dcache_range
>>  	unsigned end = 0;
>>  	unsigned size = roundup(ARCH_DMA_MINALIGN,
>>  				data->blocks*data->blocksize);
>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>> -	defined(CONFIG_IMX8ULP)
>>  	dma_addr_t addr;
>>  
>>  	addr = virt_to_phys((void *)(data->dest));
>>  	if (upper_32_bits(addr))
>> -		printf("Error found for upper 32 bits\n");
>> -	else
>> -		start = lower_32_bits(addr);
>> -#else
>> -	start = (unsigned)data->dest;
>> -#endif
>> +		printf("Cannot use 64 bit addresses with SDMA\n");
> 
> Ditto.

	if (upper_32_bits(addr))
		printf("Cannot use 64 bit addresses with SDMA\n");

--Sean

>> +	start = lower_32_bits(addr);
>>  	end = start + size;
>>  	invalidate_dcache_range(start, end);
>>  }
>> 
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 10/11] mmc: fsl_esdhc_imx: replace most #ifdefs by IS_ENABLED()
  2021-11-15  8:44   ` Jaehoon Chung
@ 2021-11-15 15:13     ` Sean Anderson
  0 siblings, 0 replies; 25+ messages in thread
From: Sean Anderson @ 2021-11-15 15:13 UTC (permalink / raw)
  To: Jaehoon Chung, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam



On 11/15/21 3:44 AM, Jaehoon Chung wrote:
> On 11/13/21 4:15 AM, Sean Anderson wrote:
>> [ fsl_esdhc commit 52faec31827ec1a1837977e29c067424426634c5 ]
>> 
>> Make the code cleaner and drop the old-style #ifdef constructs where it is
>> possible.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> ---
>> 
>> (no changes since v1)
>> 
>>  drivers/mmc/fsl_esdhc_imx.c | 209 +++++++++++++++++-------------------
>>  include/fsl_esdhc_imx.h     |   2 -
>>  2 files changed, 100 insertions(+), 111 deletions(-)
>> 
>> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
>> index aa3d0877cb..89572509a7 100644
>> --- a/drivers/mmc/fsl_esdhc_imx.c
>> +++ b/drivers/mmc/fsl_esdhc_imx.c
>> @@ -182,15 +182,15 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
>>  
>>  	if (data) {
>>  		xfertyp |= XFERTYP_DPSEL;
>> -#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>> -		xfertyp |= XFERTYP_DMAEN;
>> -#endif
>> +		if (!IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO) &&
>> +		    cmd->cmdidx != MMC_CMD_SEND_TUNING_BLOCK &&
>> +		    cmd->cmdidx != MMC_CMD_SEND_TUNING_BLOCK_HS200)
>> +			xfertyp |= XFERTYP_DMAEN;
>>  		if (data->blocks > 1) {
>>  			xfertyp |= XFERTYP_MSBSEL;
>>  			xfertyp |= XFERTYP_BCEN;
>> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
>> -			xfertyp |= XFERTYP_AC12EN;
>> -#endif
>> +			if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC111))
>> +				xfertyp |= XFERTYP_AC12EN;
>>  		}
>>  
>>  		if (data->flags & MMC_DATA_READ)
>> @@ -214,7 +214,6 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
>>  	return XFERTYP_CMD(cmd->cmdidx) | xfertyp;
>>  }
>>  
>> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
>>  /*
>>   * PIO Read/Write Mode reduce the performace as DMA is not used in this mode.
>>   */
>> @@ -277,9 +276,7 @@ static void esdhc_pio_read_write(struct fsl_esdhc_priv *priv,
>>  		}
>>  	}
>>  }
>> -#endif
>>  
>> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
>>  static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
>>  					struct mmc_data *data)
>>  {
>> @@ -299,7 +296,6 @@ static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
>>  				   wml_value << 16);
>>  	}
>>  }
>> -#endif
>>  
>>  static void esdhc_setup_dma(struct fsl_esdhc_priv *priv, struct mmc_data *data)
>>  {
>> @@ -342,11 +338,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  		}
>>  	}
>>  
>> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
>> -	esdhc_setup_watermark_level(priv, data);
>> -#else
>> -	esdhc_setup_dma(priv, data);
>> -#endif
>> +	if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO))
>> +		esdhc_setup_watermark_level(priv, data);
>> +	else
>> +		esdhc_setup_dma(priv, data);
>>  
>>  	/* Calculate the timeout period for data transactions */
>>  	/*
>> @@ -379,14 +374,13 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  	if (timeout < 0)
>>  		timeout = 0;
>>  
>> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC_A001
>> -	if ((timeout == 4) || (timeout == 8) || (timeout == 12))
>> +	if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC_A001) &&
>> +	    (timeout == 4 || timeout == 8 || timeout == 12))
>>  		timeout++;
>> -#endif
>>  
>> -#ifdef ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
>> -	timeout = 0xE;
>> -#endif
>> +	if (IS_ENABLED(ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE))
>> +		timeout = 0xE;
>> +
>>  	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_TIMEOUT_MASK, timeout << 16);
>>  
>>  	return 0;
>> @@ -409,6 +403,11 @@ static inline void sd_swap_dma_buff(struct mmc_data *data)
>>  		}
>>  	}
>>  }
>> +#else
>> +static inline void sd_swap_dma_buff(struct mmc_data *data)
>> +{
>> +	return;
>> +}
>>  #endif
>>  
>>  /*
>> @@ -425,10 +424,9 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  	struct fsl_esdhc *regs = priv->esdhc_regs;
>>  	unsigned long start;
>>  
>> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
>> -	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
>> +	if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC111) &&
>> +	    cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
>>  		return 0;
>> -#endif
>>  
>>  	esdhc_write32(&regs->irqstat, -1);
>>  
>> @@ -526,42 +524,40 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  
>>  	/* Wait until all of the blocks are transferred */
>>  	if (data) {
>> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
>> -		esdhc_pio_read_write(priv, data);
>> -#else
>> -		flags = DATA_COMPLETE;
>> -		if ((cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK) ||
>> -		    (cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK_HS200)) {
>> -			flags = IRQSTAT_BRR;
>> +		if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO)) {
>> +			esdhc_pio_read_write(priv, data);
>> +		} else {
>> +			flags = DATA_COMPLETE;
>> +			if (cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK ||
>> +			    cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK_HS200)
>> +				flags = IRQSTAT_BRR;
>> +
>> +			do {
>> +				irqstat = esdhc_read32(&regs->irqstat);
>> +
>> +				if (irqstat & IRQSTAT_DTOE) {
>> +					err = -ETIMEDOUT;
>> +					goto out;
>> +				}
>> +
>> +				if (irqstat & DATA_ERR) {
>> +					err = -ECOMM;
>> +					goto out;
>> +				}
>> +			} while ((irqstat & flags) != flags);
>> +
>> +			/*
>> +			 * Need invalidate the dcache here again to avoid any
>> +			 * cache-fill during the DMA operations such as the
>> +			 * speculative pre-fetching etc.
>> +			 */
>> +			dma_unmap_single(priv->dma_addr,
>> +					 data->blocks * data->blocksize,
>> +					 mmc_get_dma_dir(data));
>> +			if (IS_ENABLED(CONFIG_MCF5441x) &&
>> +			    (data->flags & MMC_DATA_READ))
>> +				sd_swap_dma_buff(data);
>>  		}
>> -
>> -		do {
>> -			irqstat = esdhc_read32(&regs->irqstat);
>> -
>> -			if (irqstat & IRQSTAT_DTOE) {
>> -				err = -ETIMEDOUT;
>> -				goto out;
>> -			}
>> -
>> -			if (irqstat & DATA_ERR) {
>> -				err = -ECOMM;
>> -				goto out;
>> -			}
>> -		} while ((irqstat & flags) != flags);
>> -
>> -		/*
>> -		 * Need invalidate the dcache here again to avoid any
>> -		 * cache-fill during the DMA operations such as the
>> -		 * speculative pre-fetching etc.
>> -		 */
>> -		dma_unmap_single(priv->dma_addr,
>> -				 data->blocks * data->blocksize,
>> -				 mmc_get_dma_dir(data));
>> -#ifdef CONFIG_MCF5441x
>> -		if (data->flags & MMC_DATA_READ)
>> -			sd_swap_dma_buff(data);
>> -#endif
>> -#endif
>>  	}
>>  
>>  out:
>> @@ -595,21 +591,22 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>>  	struct fsl_esdhc *regs = priv->esdhc_regs;
>>  	int div = 1;
>>  	u32 tmp;
>> -	int ret;
>> -#ifdef ARCH_MXC
>> -#ifdef CONFIG_MX53
>> -	/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
>> -	int pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;
>> -#else
>> -	int pre_div = 1;
>> -#endif
>> -#else
>> -	int pre_div = 2;
>> -#endif
>> +	int ret, pre_div;
>>  	int ddr_pre_div = mmc->ddr_mode ? 2 : 1;
>>  	int sdhc_clk = priv->sdhc_clk;
>>  	uint clk;
>>  
>> +	if (IS_ENABLED(ARCH_MXC)) {
>> +#ifdef CONFIG_MX53
> 
> Is there any reason not to use "IS_ENABLED()" ?

No. I will fix this one up for the next revision.

--Sean

> Best Regards,
> Jaehoon Chung
> 
>> +		/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
>> +		pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;
>> +#else
>> +		pre_div = 1;
>> +#endif
>> +	} else {
>> +		pre_div = 2;
>> +	}
>> +
>>  	while (sdhc_clk / (16 * pre_div * ddr_pre_div) > clock && pre_div < 256)
>>  		pre_div *= 2;
>>  
>> @@ -621,11 +618,10 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>>  
>>  	clk = (pre_div << 8) | (div << 4);
>>  
>> -#ifdef CONFIG_FSL_USDHC
>> -	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
>> -#else
>> -	esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
>> -#endif
>> +	if (IS_ENABLED(CONFIG_FSL_USDHC))
>> +		esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
>> +	else
>> +		esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
>>  
>>  	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_CLOCK_MASK, clk);
>>  
>> @@ -633,11 +629,10 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>>  	if (ret)
>>  		pr_warn("fsl_esdhc_imx: Internal clock never stabilised.\n");
>>  
>> -#ifdef CONFIG_FSL_USDHC
>> -	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN | VENDORSPEC_CKEN);
>> -#else
>> -	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
>> -#endif
>> +	if (IS_ENABLED(CONFIG_FSL_USDHC))
>> +		esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN | VENDORSPEC_CKEN);
>> +	else
>> +		esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
>>  
>>  	mmc->clock = sdhc_clk / pre_div / div;
>>  	priv->clock = clock;
>> @@ -1145,22 +1140,21 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>>  	if (ret)
>>  		return ret;
>>  
>> -#ifdef CONFIG_MCF5441x
>>  	/* ColdFire, using SDHC_DATA[3] for card detection */
>> -	esdhc_write32(&regs->proctl, PROCTL_INIT | PROCTL_D3CD);
>> -#endif
>> +	if (IS_ENABLED(CONFIG_MCF5441x))
>> +		esdhc_write32(&regs->proctl, PROCTL_INIT | PROCTL_D3CD);
>>  
>> -#ifndef CONFIG_FSL_USDHC
>> -	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
>> -				| SYSCTL_IPGEN | SYSCTL_CKEN);
>> -	/* Clearing tuning bits in case ROM has set it already */
>> -	esdhc_write32(&regs->mixctrl, 0);
>> -	esdhc_write32(&regs->autoc12err, 0);
>> -	esdhc_write32(&regs->clktunectrlstatus, 0);
>> -#else
>> -	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
>> -			VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
>> -#endif
>> +	if (IS_ENABLED(CONFIG_FSL_USDHC)) {
>> +		esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
>> +				VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
>> +	} else {
>> +		esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
>> +					| SYSCTL_IPGEN | SYSCTL_CKEN);
>> +		/* Clearing tuning bits in case ROM has set it already */
>> +		esdhc_write32(&regs->mixctrl, 0);
>> +		esdhc_write32(&regs->autoc12err, 0);
>> +		esdhc_write32(&regs->clktunectrlstatus, 0);
>> +	}
>>  
>>  	if (priv->vs18_enable)
>>  		esdhc_setbits32(&regs->vendorspec, ESDHC_VENDORSPEC_VSELECT);
>> @@ -1172,22 +1166,20 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>>  #endif
>>  
>>  	caps = esdhc_read32(&regs->hostcapblt);
>> -#ifdef CONFIG_MCF5441x
>> +
>>  	/*
>>  	 * MCF5441x RM declares in more points that sdhc clock speed must
>>  	 * never exceed 25 Mhz. From this, the HS bit needs to be disabled
>>  	 * from host capabilities.
>>  	 */
>> -	caps &= ~ESDHC_HOSTCAPBLT_HSS;
>> -#endif
>> +	if (IS_ENABLED(CONFIG_MCF5441x))
>> +		caps &= ~HOSTCAPBLT_HSS;
>>  
>> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC135
>> -	caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
>> -#endif
>> +	if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC135))
>> +		caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
>>  
>> -#ifdef CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33
>> -	caps |= HOSTCAPBLT_VS33;
>> -#endif
>> +	if (IS_ENABLED(CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33))
>> +		caps |= HOSTCAPBLT_VS33;
>>  
>>  	if (caps & HOSTCAPBLT_VS18)
>>  		cfg->voltages |= MMC_VDD_165_195;
>> @@ -1197,12 +1189,13 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>>  		cfg->voltages |= MMC_VDD_32_33 | MMC_VDD_33_34;
>>  
>>  	cfg->name = "FSL_SDHC";
>> +
>>  #if !CONFIG_IS_ENABLED(DM_MMC)
>>  	cfg->ops = &esdhc_ops;
>>  #endif
>> -#ifdef CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE
>> -	cfg->host_caps |= MMC_MODE_DDR_52MHz;
>> -#endif
>> +
>> +	if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE))
>> +		cfg->host_caps |= MMC_MODE_DDR_52MHz;
>>  
>>  	if (caps & HOSTCAPBLT_HSS)
>>  		cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
>> @@ -1286,10 +1279,8 @@ int fsl_esdhc_initialize(struct bd_info *bis, struct fsl_esdhc_cfg *cfg)
>>  		return -EINVAL;
>>  	}
>>  
>> -#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
>> -	if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
>> +	if (IS_ENABLED(CONFIG_ESDHC_DETECT_8_BIT_QUIRK))
>>  		mmc_cfg->host_caps &= ~MMC_MODE_8BIT;
>> -#endif
>>  
>>  	ret = fsl_esdhc_init(priv, plat);
>>  	if (ret) {
>> diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
>> index 1529b8bba3..b958a6b3bb 100644
>> --- a/include/fsl_esdhc_imx.h
>> +++ b/include/fsl_esdhc_imx.h
>> @@ -24,12 +24,10 @@
>>  #define SYSCTL_INITA		0x08000000
>>  #define SYSCTL_TIMEOUT_MASK	0x000f0000
>>  #define SYSCTL_CLOCK_MASK	0x0000fff0
>> -#if !defined(CONFIG_FSL_USDHC)
>>  #define SYSCTL_CKEN		0x00000008
>>  #define SYSCTL_PEREN		0x00000004
>>  #define SYSCTL_HCKEN		0x00000002
>>  #define SYSCTL_IPGEN		0x00000001
>> -#endif
>>  #define SYSCTL_RSTA		0x01000000
>>  #define SYSCTL_RSTC		0x02000000
>>  #define SYSCTL_RSTD		0x04000000
>> 
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers
  2021-11-15 15:12     ` Sean Anderson
@ 2021-11-15 22:18       ` Jaehoon Chung
  0 siblings, 0 replies; 25+ messages in thread
From: Jaehoon Chung @ 2021-11-15 22:18 UTC (permalink / raw)
  To: Sean Anderson, u-boot, Peng Fan
  Cc: Haibo Chen, Michael Walle, Yangbo Lu, Fabio Estevam

On 11/16/21 12:12 AM, Sean Anderson wrote:
> 
> 
> On 11/15/21 3:39 AM, Jaehoon Chung wrote:
>> On 11/13/21 4:15 AM, Sean Anderson wrote:
>>> [ fsl_esdhc commit da86e8cfcb03ed5c1d8e0718bc8bc8583e60ced8 ]
>>>
>>> SDMA can only do DMA with 32 bit addresses. This is true for all
>>> architectures (just doesn't apply to 32 bit ones). Simplify the code and
>>> remove unnecessary CONFIG_FSL_LAYERSCAPE.
>>>
>>> Also make the error message more concise.
>>>
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>>
>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>>
>> Just add minor comments.
>>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>  drivers/mmc/fsl_esdhc_imx.c | 33 ++++++---------------------------
>>>  1 file changed, 6 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
>>> index 95cde9e410..3588056759 100644
>>> --- a/drivers/mmc/fsl_esdhc_imx.c
>>> +++ b/drivers/mmc/fsl_esdhc_imx.c
>>> @@ -282,10 +282,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>>  {
>>>      int timeout;
>>>      struct fsl_esdhc *regs = priv->esdhc_regs;
>>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>>> -    defined(CONFIG_IMX8ULP)
>>>      dma_addr_t addr;
>>> -#endif
>>>      uint wml_value;
>>>  
>>>      wml_value = data->blocksize/4;
>>> @@ -296,16 +293,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>>  
>>>          esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>>> -    defined(CONFIG_IMX8ULP)
>>>          addr = virt_to_phys((void *)(data->dest));
>>>          if (upper_32_bits(addr))
>>> -            printf("Error found for upper 32 bits\n");
>>> -        else
>>> -            esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>> -#else
>>> -        esdhc_write32(&regs->dsaddr, (u32)data->dest);
>>> -#endif
>>> +            printf("Cannot use 64 bit addresses with SDMA\n");
>>> +        esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>>  #endif
>>>      } else {
>>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>>> @@ -334,16 +325,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>>          esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
>>>                      wml_value << 16);
>>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>>> -        defined(CONFIG_IMX8ULP)
>>>          addr = virt_to_phys((void *)(data->src));
>>>          if (upper_32_bits(addr))
>>> -            printf("Error found for upper 32 bits\n");
>>> -        else
>>> -            esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>> -#else
>>> -        esdhc_write32(&regs->dsaddr, (u32)data->src);
>>> -#endif
>>> +            printf("Cannot use 64 bit addresses with SDMA\n");
>>
>> Fix indentation?
> 
> Looks OK to me. The final code is

It's my mis-reading. Thanks

Best Regards,
Jaehoon Chung

> 
>         if (upper_32_bits(addr))
>             printf("Cannot use 64 bit addresses with SDMA\n");
> 
> 
>>
>>> +        esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>>  #endif
>>>      }
>>>  
>>> @@ -400,18 +385,12 @@ static void check_and_invalidate_dcache_range
>>>      unsigned end = 0;
>>>      unsigned size = roundup(ARCH_DMA_MINALIGN,
>>>                  data->blocks*data->blocksize);
>>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>>> -    defined(CONFIG_IMX8ULP)
>>>      dma_addr_t addr;
>>>  
>>>      addr = virt_to_phys((void *)(data->dest));
>>>      if (upper_32_bits(addr))
>>> -        printf("Error found for upper 32 bits\n");
>>> -    else
>>> -        start = lower_32_bits(addr);
>>> -#else
>>> -    start = (unsigned)data->dest;
>>> -#endif
>>> +        printf("Cannot use 64 bit addresses with SDMA\n");
>>
>> Ditto.
> 
>     if (upper_32_bits(addr))
>         printf("Cannot use 64 bit addresses with SDMA\n");
> 
> --Sean
> 
>>> +    start = lower_32_bits(addr);
>>>      end = start + size;
>>>      invalidate_dcache_range(start, end);
>>>  }
>>>
>>
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2021-11-15 22:18 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-12 19:15 [PATCH v2 00/11] fsl_esdhc_imx: port several patches from fsl_esdhc Sean Anderson
2021-11-12 19:15 ` [PATCH v2 01/11] mmc: fsl_esdhc_imx: make BLK as hard requirement of DM_MMC Sean Anderson
2021-11-15  8:35   ` Jaehoon Chung
2021-11-12 19:15 ` [PATCH v2 02/11] mmc: fsl_esdhc_imx: remove redundant DM_MMC checking Sean Anderson
2021-11-15  8:35   ` Jaehoon Chung
2021-11-12 19:15 ` [PATCH v2 03/11] mmc: fsl_esdhc_imx: fix voltage validation Sean Anderson
2021-11-15  8:36   ` Jaehoon Chung
2021-11-12 19:15 ` [PATCH v2 04/11] mmc: fsl_esdhc_imx: clean up bus width configuration code Sean Anderson
2021-11-15  8:36   ` Jaehoon Chung
2021-11-12 19:15 ` [PATCH v2 05/11] mmc: fsl_esdhc_imx: drop redundant code for non-removable feature Sean Anderson
2021-11-15  8:37   ` Jaehoon Chung
2021-11-12 19:15 ` [PATCH v2 06/11] mmc: fsl_esdhc_imx: fix mmc->clock with actual clock Sean Anderson
2021-11-15  8:37   ` Jaehoon Chung
2021-11-12 19:15 ` [PATCH v2 07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers Sean Anderson
2021-11-15  8:39   ` Jaehoon Chung
2021-11-15 15:12     ` Sean Anderson
2021-11-15 22:18       ` Jaehoon Chung
2021-11-12 19:15 ` [PATCH v2 08/11] mmc: fsl_esdhc_imx: use dma-mapping API Sean Anderson
2021-11-15  8:40   ` Jaehoon Chung
2021-11-12 19:15 ` [PATCH v2 09/11] mmc: fsl_esdhc_imx: simplify esdhc_setup_data() Sean Anderson
2021-11-15  8:41   ` Jaehoon Chung
2021-11-12 19:15 ` [PATCH v2 10/11] mmc: fsl_esdhc_imx: replace most #ifdefs by IS_ENABLED() Sean Anderson
2021-11-15  8:44   ` Jaehoon Chung
2021-11-15 15:13     ` Sean Anderson
2021-11-12 19:15 ` [PATCH v2 11/11] mmc: fsl_esdhc_imx: set sysctl register for clock initialization Sean Anderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.