[PATCH v2 00/10] Add Support for eMMC boot in AM65x and J721e

[PATCH v2 00/10] Add Support for eMMC boot in AM65x and J721e

[Faiz Abbas]

The following patches add support for eMMC boot in TI's Am65x and J721e
devices.

v2:
1. Reordered the patches according to Lokesh's preference
2. Fixed patch 2 breaking platforms where DM_MMC is not enabled.

Faiz Abbas (10):
  mmc: Add a saved_clock member
  mmc: Add init() API
  mmc: Merge SD_LEGACY and MMC_LEGACY bus modes
  mmc: sdhci: Expose sdhci_init() as non-static
  mmc: am654_sdhci: Update output tap delay writes
  mmc: am654_sdhci: Implement workaround for card detect
  spl: mmc: Fix spl_mmc_get_uboot_raw_sector() implementation
  arm: K3: sysfw-loader: Add a config_pm_pre_callback()
  configs: am65x_evm: Add CONFIG_SUPPORT_EMMC_BOOT
  configs: j721e_evm: Add Support for eMMC boot

 arch/arm/dts/k3-am65-main.dtsi               |  12 ++-
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi |  11 +-
 arch/arm/dts/k3-j721e-main.dtsi              |  15 ++-
 arch/arm/mach-imx/imx8/image.c               |   3 +-
 arch/arm/mach-k3/am6_init.c                  |  33 +++++-
 arch/arm/mach-k3/include/mach/sysfw-loader.h |   2 +-
 arch/arm/mach-k3/j721e_init.c                |  33 +++++-
 arch/arm/mach-k3/sysfw-loader.c              |   6 +-
 common/spl/spl_mmc.c                         |  11 +-
 configs/am65x_evm_a53_defconfig              |   1 +
 configs/am65x_evm_r5_defconfig               |   1 +
 configs/j721e_evm_a72_defconfig              |   3 +
 configs/j721e_evm_r5_defconfig               |   3 +
 drivers/mmc/am654_sdhci.c                    | 105 ++++++++++++++++---
 drivers/mmc/fsl_esdhc_imx.c                  |   1 -
 drivers/mmc/mmc.c                            |  31 +++---
 drivers/mmc/omap_hsmmc.c                     |   1 -
 drivers/mmc/sdhci.c                          |   2 +-
 drivers/mmc/zynq_sdhci.c                     |   1 -
 include/configs/am65x_evm.h                  |   2 -
 include/mmc.h                                |   9 +-
 include/sdhci.h                              |   1 +
 22 files changed, 235 insertions(+), 52 deletions(-)

-- 
2.19.2

[PATCH v2 01/10] mmc: Add a saved_clock member

[Faiz Abbas]

Add a saved_clock member to struct mmc to store the previous clock speed
in the clock needs to be stopped for some time.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 include/mmc.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/mmc.h b/include/mmc.h
index b5cb514f57..2f21dbf1b7 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -602,6 +602,7 @@ struct mmc {
 	bool clk_disable; /* true if the clock can be turned off */
 	uint bus_width;
 	uint clock;
+	uint saved_clock;
 	enum mmc_voltage signal_voltage;
 	uint card_caps;
 	uint host_caps;
-- 
2.19.2

[PATCH v2 02/10] mmc: Add init() API

[Faiz Abbas]

Add an init() API for platform specific init() operations.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/mmc/mmc.c | 13 ++++++-------
 include/mmc.h     |  7 +++++++
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index d43983d4a6..50df8c8626 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -2787,14 +2787,13 @@ int mmc_get_op_cond(struct mmc *mmc)
 	}
 	if (err)
 		return err;
-
 #if CONFIG_IS_ENABLED(DM_MMC)
-	/* The device has already been probed ready for use */
-#else
-	/* made sure it's not NULL earlier */
-	err = mmc->cfg->ops->init(mmc);
-	if (err)
-		return err;
+	struct dm_mmc_ops *ops = mmc_get_ops(mmc->dev);
+	if (ops->init) {
+		err = ops->init(mmc->dev);
+		if (err)
+			return err;
+	}
 #endif
 	mmc->ddr_mode = 0;
 
diff --git a/include/mmc.h b/include/mmc.h
index 2f21dbf1b7..6aef125f25 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -406,6 +406,13 @@ struct mmc;
 
 #if CONFIG_IS_ENABLED(DM_MMC)
 struct dm_mmc_ops {
+	/**
+	 * init() - platform specific initialization for the host controller
+	 *
+	 * @dev:	Device to init
+	 * @return 0 if Ok, -ve if error
+	 */
+	int (*init)(struct udevice *dev);
 	/**
 	 * send_cmd() - Send a command to the MMC device
 	 *
-- 
2.19.2

[PATCH v2 03/10] mmc: Merge SD_LEGACY and MMC_LEGACY bus modes

[Faiz Abbas]

MMC_LEGACY & SD_LEGACY are not differentiated timings in the spec and
don't have any meaningful differences. Therefore, get rid of all
references to SD_LEGACY and use MMC_LEGACY to mean both of them.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/mmc/fsl_esdhc_imx.c |  1 -
 drivers/mmc/mmc.c           | 18 ++++++++----------
 drivers/mmc/omap_hsmmc.c    |  1 -
 drivers/mmc/zynq_sdhci.c    |  1 -
 include/mmc.h               |  1 -
 5 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 462ad2878a..6555639fdf 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -741,7 +741,6 @@ static int esdhc_set_timing(struct mmc *mmc)
 
 	switch (mmc->selected_mode) {
 	case MMC_LEGACY:
-	case SD_LEGACY:
 		esdhc_reset_tuning(mmc);
 		writel(mixctrl, &regs->mixctrl);
 		break;
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 50df8c8626..c49e892e73 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -136,7 +136,6 @@ const char *mmc_mode_name(enum bus_mode mode)
 {
 	static const char *const names[] = {
 	      [MMC_LEGACY]	= "MMC legacy",
-	      [SD_LEGACY]	= "SD Legacy",
 	      [MMC_HS]		= "MMC High Speed (26MHz)",
 	      [SD_HS]		= "SD High Speed (50MHz)",
 	      [UHS_SDR12]	= "UHS SDR12 (25MHz)",
@@ -162,7 +161,6 @@ static uint mmc_mode2freq(struct mmc *mmc, enum bus_mode mode)
 {
 	static const int freqs[] = {
 	      [MMC_LEGACY]	= 25000000,
-	      [SD_LEGACY]	= 25000000,
 	      [MMC_HS]		= 26000000,
 	      [SD_HS]		= 50000000,
 	      [MMC_HS_52]	= 52000000,
@@ -1241,7 +1239,7 @@ static int sd_get_capabilities(struct mmc *mmc)
 	u32 sd3_bus_mode;
 #endif
 
-	mmc->card_caps = MMC_MODE_1BIT | MMC_CAP(SD_LEGACY);
+	mmc->card_caps = MMC_MODE_1BIT | MMC_CAP(MMC_LEGACY);
 
 	if (mmc_host_is_spi(mmc))
 		return 0;
@@ -1354,7 +1352,7 @@ static int sd_set_card_speed(struct mmc *mmc, enum bus_mode mode)
 		return 0;
 
 	switch (mode) {
-	case SD_LEGACY:
+	case MMC_LEGACY:
 		speed = UHS_SDR12_BUS_SPEED;
 		break;
 	case SD_HS:
@@ -1697,7 +1695,7 @@ static const struct mode_width_tuning sd_modes_by_pref[] = {
 	},
 #endif
 	{
-		.mode = SD_LEGACY,
+		.mode = MMC_LEGACY,
 		.widths = MMC_MODE_4BIT | MMC_MODE_1BIT,
 	}
 };
@@ -1727,7 +1725,7 @@ static int sd_select_mode_and_width(struct mmc *mmc, uint card_caps)
 
 	if (mmc_host_is_spi(mmc)) {
 		mmc_set_bus_width(mmc, 1);
-		mmc_select_mode(mmc, SD_LEGACY);
+		mmc_select_mode(mmc, MMC_LEGACY);
 		mmc_set_clock(mmc, mmc->tran_speed, MMC_CLK_ENABLE);
 		return 0;
 	}
@@ -1786,7 +1784,7 @@ static int sd_select_mode_and_width(struct mmc *mmc, uint card_caps)
 
 error:
 				/* revert to a safer bus speed */
-				mmc_select_mode(mmc, SD_LEGACY);
+				mmc_select_mode(mmc, MMC_LEGACY);
 				mmc_set_clock(mmc, mmc->tran_speed,
 						MMC_CLK_ENABLE);
 			}
@@ -2563,7 +2561,7 @@ static int mmc_startup(struct mmc *mmc)
 
 #if CONFIG_IS_ENABLED(MMC_TINY)
 	mmc_set_clock(mmc, mmc->legacy_speed, false);
-	mmc_select_mode(mmc, IS_SD(mmc) ? SD_LEGACY : MMC_LEGACY);
+	mmc_select_mode(mmc, MMC_LEGACY);
 	mmc_set_bus_width(mmc, 1);
 #else
 	if (IS_SD(mmc)) {
@@ -2844,8 +2842,8 @@ int mmc_start_init(struct mmc *mmc)
 	 * all hosts are capable of 1 bit bus-width and able to use the legacy
 	 * timings.
 	 */
-	mmc->host_caps = mmc->cfg->host_caps | MMC_CAP(SD_LEGACY) |
-			 MMC_CAP(MMC_LEGACY) | MMC_MODE_1BIT;
+	mmc->host_caps = mmc->cfg->host_caps | MMC_CAP(MMC_LEGACY) |
+			 MMC_MODE_1BIT;
 
 #if !defined(CONFIG_MMC_BROKEN_CD)
 	no_card = mmc_getcd(mmc) == 0;
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index 5d0cfb2ebd..5e79fa517d 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -390,7 +390,6 @@ static void omap_hsmmc_set_timing(struct mmc *mmc)
 		break;
 	case MMC_LEGACY:
 	case MMC_HS:
-	case SD_LEGACY:
 	case UHS_SDR12:
 		val |= AC12_UHSMC_SDR12;
 		break;
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index 529eec9c45..fc638649bd 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -35,7 +35,6 @@ struct arasan_sdhci_priv {
 
 static const u8 mode2timing[] = {
 	[MMC_LEGACY] = UHS_SDR12_BUS_SPEED,
-	[SD_LEGACY] = UHS_SDR12_BUS_SPEED,
 	[MMC_HS] = HIGH_SPEED_BUS_SPEED,
 	[SD_HS] = HIGH_SPEED_BUS_SPEED,
 	[MMC_HS_52] = HIGH_SPEED_BUS_SPEED,
diff --git a/include/mmc.h b/include/mmc.h
index 6aef125f25..a96833c1ca 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -539,7 +539,6 @@ struct sd_ssr {
 
 enum bus_mode {
 	MMC_LEGACY,
-	SD_LEGACY,
 	MMC_HS,
 	SD_HS,
 	MMC_HS_52,
-- 
2.19.2

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Faiz Abbas]

Expose sdhci_init() as non-static.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/mmc/sdhci.c | 2 +-
 include/sdhci.h     | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 01fa5a9d4d..4fce85a0ea 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -618,7 +618,7 @@ static int sdhci_set_ios(struct mmc *mmc)
 	return 0;
 }
 
-static int sdhci_init(struct mmc *mmc)
+int sdhci_init(struct mmc *mmc)
 {
 	struct sdhci_host *host = mmc->priv;
 #if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(DM_GPIO)
diff --git a/include/sdhci.h b/include/sdhci.h
index 01addb7a60..0830e05d53 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -487,6 +487,7 @@ void sdhci_set_uhs_timing(struct sdhci_host *host);
 #ifdef CONFIG_DM_MMC
 /* Export the operations to drivers */
 int sdhci_probe(struct udevice *dev);
+int sdhci_init(struct mmc *mmc);
 int sdhci_set_clock(struct mmc *mmc, unsigned int clock);
 extern const struct dm_mmc_ops sdhci_ops;
 #else
-- 
2.19.2

[PATCH v2 05/10] mmc: am654_sdhci: Update output tap delay writes

[Faiz Abbas]

With the latest RIOT, there is a different otap delay value for each
speed mode. Add a new binding with every supported speed mode. Also
disable a given speed mode in the host caps if its corresponding
otap-del-sel is not present.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 arch/arm/dts/k3-am65-main.dtsi               | 12 +++-
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi | 11 ++-
 arch/arm/dts/k3-j721e-main.dtsi              | 15 ++++-
 drivers/mmc/am654_sdhci.c                    | 70 +++++++++++++++++---
 4 files changed, 95 insertions(+), 13 deletions(-)

diff --git a/arch/arm/dts/k3-am65-main.dtsi b/arch/arm/dts/k3-am65-main.dtsi
index ab40dafceb..028f57379b 100644
--- a/arch/arm/dts/k3-am65-main.dtsi
+++ b/arch/arm/dts/k3-am65-main.dtsi
@@ -98,7 +98,17 @@
 		interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
 		mmc-ddr-1_8v;
 		mmc-hs200-1_8v;
-		ti,otap-del-sel = <0x2>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-mmc-hs = <0x0>;
+		ti,otap-del-sel-sd-hs = <0x0>;
+		ti,otap-del-sel-sdr12 = <0x0>;
+		ti,otap-del-sel-sdr25 = <0x0>;
+		ti,otap-del-sel-sdr50 = <0x8>;
+		ti,otap-del-sel-sdr104 = <0x5>;
+		ti,otap-del-sel-ddr50 = <0x5>;
+		ti,otap-del-sel-ddr52 = <0x5>;
+		ti,otap-del-sel-hs200 = <0x5>;
+		ti,otap-del-sel-hs400 = <0x0>;
 		ti,trm-icp = <0x8>;
 		dma-coherent;
 	};
diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
index a349edcfa5..54ecb3d444 100644
--- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
@@ -29,7 +29,16 @@
 		clock-names = "clk_ahb", "clk_xin";
 		power-domains = <&k3_pds 48 TI_SCI_PD_EXCLUSIVE>;
 		max-frequency = <25000000>;
-		ti,otap-del-sel = <0x2>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-mmc-hs = <0x0>;
+		ti,otap-del-sel-sd-hs = <0x0>;
+		ti,otap-del-sel-sdr12 = <0x0>;
+		ti,otap-del-sel-sdr25 = <0x0>;
+		ti,otap-del-sel-sdr50 = <0x8>;
+		ti,otap-del-sel-sdr104 = <0x7>;
+		ti,otap-del-sel-ddr50 = <0x4>;
+		ti,otap-del-sel-ddr52 = <0x4>;
+		ti,otap-del-sel-hs200 = <0x7>;
 		ti,trm-icp = <0x8>;
 	};
 
diff --git a/arch/arm/dts/k3-j721e-main.dtsi b/arch/arm/dts/k3-j721e-main.dtsi
index 5083a0c3ae..86304015c4 100644
--- a/arch/arm/dts/k3-j721e-main.dtsi
+++ b/arch/arm/dts/k3-j721e-main.dtsi
@@ -210,9 +210,14 @@
 		assigned-clocks = <&k3_clks 91 1>;
 		assigned-clock-parents = <&k3_clks 91 2>;
 		bus-width = <8>;
-		ti,otap-del-sel = <0x2>;
 		ti,trm-icp = <0x8>;
 		dma-coherent;
+		mmc-ddr-1_8v;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-mmc-hs = <0x0>;
+		ti,otap-del-sel-ddr52 = <0x5>;
+		ti,otap-del-sel-hs200 = <0x6>;
+		ti,otap-del-sel-hs400 = <0x0>;
 	};
 
 	main_sdhci1: sdhci at 4fb0000 {
@@ -224,7 +229,13 @@
 		clocks = <&k3_clks 92 0>, <&k3_clks 92 5>;
 		assigned-clocks = <&k3_clks 92 0>;
 		assigned-clock-parents = <&k3_clks 92 1>;
-		ti,otap-del-sel = <0x2>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-sd-hs = <0xf>;
+		ti,otap-del-sel-sdr12 = <0xf>;
+		ti,otap-del-sel-sdr25 = <0xf>;
+		ti,otap-del-sel-sdr50 = <0xc>;
+		ti,otap-del-sel-sdr104 = <0x5>;
+		ti,otap-del-sel-ddr50 = <0xc>;
 		ti,trm-icp = <0x8>;
 		dma-coherent;
 	};
diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
index 41a90834ff..ff0a81eaab 100644
--- a/drivers/mmc/am654_sdhci.c
+++ b/drivers/mmc/am654_sdhci.c
@@ -72,7 +72,7 @@ struct am654_sdhci_plat {
 	struct mmc mmc;
 	struct regmap *base;
 	bool non_removable;
-	u32 otap_del_sel;
+	u32 otap_del_sel[11];
 	u32 trm_icp;
 	u32 drv_strength;
 	u32 strb_sel;
@@ -84,6 +84,25 @@ struct am654_sdhci_plat {
 	bool dll_on;
 };
 
+struct timing_data {
+	const char *binding;
+	u32 capability;
+};
+
+static const struct timing_data td[] = {
+	[MMC_LEGACY] = {"ti,otap-del-sel-legacy", 0},
+	[MMC_HS] = {"ti,otap-del-sel-mmc-hs", MMC_CAP(MMC_HS)},
+	[SD_HS]  = {"ti,otap-del-sel-sd-hs", MMC_CAP(SD_HS)},
+	[UHS_SDR12] = {"ti,otap-del-sel-sdr12", MMC_CAP(UHS_SDR12)},
+	[UHS_SDR25] = {"ti,otap-del-sel-sdr25", MMC_CAP(UHS_SDR25)},
+	[UHS_SDR50] = {"ti,otap-del-sel-sdr50", MMC_CAP(UHS_SDR50)},
+	[UHS_SDR104] = {"ti,otap-del-sel-sdr104", MMC_CAP(UHS_SDR104)},
+	[UHS_DDR50] = {"ti,otap-del-sel-ddr50", MMC_CAP(UHS_DDR50)},
+	[MMC_DDR_52] = {"ti,otap-del-sel-ddr52", MMC_CAP(MMC_DDR_52)},
+	[MMC_HS_200] = {"ti,otap-del-sel-hs200", MMC_CAP(MMC_HS_200)},
+	[MMC_HS_400] = {"ti,otap-del-sel-hs400", MMC_CAP(MMC_HS_400)},
+};
+
 struct am654_driver_data {
 	const struct sdhci_ops *ops;
 	u32 flags;
@@ -110,6 +129,7 @@ static int am654_sdhci_set_ios_post(struct sdhci_host *host)
 	struct am654_sdhci_plat *plat = dev_get_platdata(dev);
 	unsigned int speed = host->mmc->clock;
 	int sel50, sel100, freqsel;
+	u32 otap_del_sel;
 	u32 mask, val;
 	int ret;
 
@@ -130,9 +150,10 @@ static int am654_sdhci_set_ios_post(struct sdhci_host *host)
 
 	/* switch phy back on */
 	if (speed > AM654_SDHCI_MIN_FREQ) {
+		otap_del_sel = plat->otap_del_sel[host->mmc->selected_mode];
 		mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
 		val = (1 << OTAPDLYENA_SHIFT) |
-		      (plat->otap_del_sel << OTAPDLYSEL_SHIFT);
+		      (otap_del_sel << OTAPDLYSEL_SHIFT);
 
 		/* Write to STRBSEL for HS400 speed mode */
 		if (host->mmc->selected_mode == MMC_HS_400) {
@@ -214,11 +235,11 @@ static int j721e_4bit_sdhci_set_ios_post(struct sdhci_host *host)
 {
 	struct udevice *dev = host->mmc->dev;
 	struct am654_sdhci_plat *plat = dev_get_platdata(dev);
-	u32 mask, val;
+	u32 otap_del_sel, mask, val;
 
+	otap_del_sel = plat->otap_del_sel[host->mmc->selected_mode];
 	mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
-	val = (1 << OTAPDLYENA_SHIFT) |
-	      (plat->otap_del_sel << OTAPDLYSEL_SHIFT);
+	val = (1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT);
 	regmap_update_bits(plat->base, PHY_CTRL4, mask, val);
 
 	return 0;
@@ -279,6 +300,37 @@ int am654_sdhci_init(struct am654_sdhci_plat *plat)
 	return 0;
 }
 
+static int sdhci_am654_get_otap_delay(struct udevice *dev,
+				      struct mmc_config *cfg)
+{
+	struct am654_sdhci_plat *plat = dev_get_platdata(dev);
+	int ret;
+	int i;
+
+	/* ti,otap-del-sel-legacy is mandatory */
+	ret = dev_read_u32(dev, "ti,otap-del-sel-legacy",
+			   &plat->otap_del_sel[0]);
+	if (ret)
+		return ret;
+	/*
+	 * Remove the corresponding capability if an otap-del-sel
+	 * value is not found
+	 */
+	for (i = MMC_HS; i <= MMC_HS_400; i++) {
+		ret = dev_read_u32(dev, td[i].binding, &plat->otap_del_sel[i]);
+		if (ret) {
+			dev_dbg(dev, "Couldn't find %s\n", td[i].binding);
+			/*
+			 * Remove the corresponding capability
+			 * if an otap-del-sel value is not found
+			 */
+			cfg->host_caps &= ~td[i].capability;
+		}
+	}
+
+	return 0;
+}
+
 static int am654_sdhci_probe(struct udevice *dev)
 {
 	struct am654_driver_data *drv_data =
@@ -311,6 +363,10 @@ static int am654_sdhci_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
+	ret = sdhci_am654_get_otap_delay(dev, cfg);
+	if (ret)
+		return ret;
+
 	host->ops = drv_data->ops;
 	host->mmc->priv = host;
 	upriv->mmc = host->mmc;
@@ -334,10 +390,6 @@ static int am654_sdhci_ofdata_to_platdata(struct udevice *dev)
 	host->ioaddr = (void *)dev_read_addr(dev);
 	plat->non_removable = dev_read_bool(dev, "non-removable");
 
-	ret = dev_read_u32(dev, "ti,otap-del-sel", &plat->otap_del_sel);
-	if (ret)
-		return ret;
-
 	if (plat->flags & DLL_PRESENT) {
 		ret = dev_read_u32(dev, "ti,trm-icp", &plat->trm_icp);
 		if (ret)
-- 
2.19.2

[PATCH v2 06/10] mmc: am654_sdhci: Implement workaround for card detect

[Faiz Abbas]

The 4 bit MMC controllers have an internal debounce for the SDCD line
with a debounce delay of 1 second. Therefore, after clocks to the IP are
enabled, software has to wait for this time before it can power on the
controller.

Add an init() callback which polls on sdcd for a maximum of 2 seconds
before switching on power to the controller or (in the case of no card)
returning a ENOMEDIUM. This pushes the 1 second wait time to when the
card is actually needed rather than at every probe() making sure that
users who don't insert an SD card in the slot don't have to wait such a
long time.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/mmc/am654_sdhci.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
index ff0a81eaab..ccae3fea31 100644
--- a/drivers/mmc/am654_sdhci.c
+++ b/drivers/mmc/am654_sdhci.c
@@ -254,7 +254,7 @@ const struct am654_driver_data j721e_4bit_drv_data = {
 	.flags = IOMUX_PRESENT,
 };
 
-int am654_sdhci_init(struct am654_sdhci_plat *plat)
+int am654_sdhci_init_phy(struct am654_sdhci_plat *plat)
 {
 	u32 ctl_cfg_2 = 0;
 	u32 mask, val;
@@ -331,8 +331,37 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
 	return 0;
 }
 
+#define MAX_SDCD_DEBOUNCE_TIME 2000
+int am654_sdhci_init(struct udevice *dev)
+{
+	struct am654_sdhci_plat *plat = dev_get_platdata(dev);
+	struct mmc *mmc = mmc_get_mmc_dev(dev);
+	struct sdhci_host *host = mmc->priv;
+	unsigned long start;
+	int val;
+
+	/*
+	 * The controller takes about 1 second to debounce the card detect line
+	 * and doesn't let us power on until that time is up. Instead of waiting
+	 * for 1 second at every stage, poll on the CARD_PRESENT bit upto a
+	 * maximum of 2 seconds to be safe..
+	 */
+	start = get_timer(0);
+	do {
+		if (get_timer(start) > MAX_SDCD_DEBOUNCE_TIME)
+			return -ENOMEDIUM;
+
+		val = mmc_getcd(host->mmc);
+	} while (!val);
+
+	am654_sdhci_init_phy(plat);
+
+	return sdhci_init(mmc);
+}
+
 static int am654_sdhci_probe(struct udevice *dev)
 {
+	struct dm_mmc_ops *ops = mmc_get_ops(dev);
 	struct am654_driver_data *drv_data =
 			(struct am654_driver_data *)dev_get_driver_data(dev);
 	struct am654_sdhci_plat *plat = dev_get_platdata(dev);
@@ -373,9 +402,9 @@ static int am654_sdhci_probe(struct udevice *dev)
 
 	regmap_init_mem_index(dev_ofnode(dev), &plat->base, 1);
 
-	am654_sdhci_init(plat);
+	ops->init = am654_sdhci_init;
 
-	return sdhci_probe(dev);
+	return 0;
 }
 
 static int am654_sdhci_ofdata_to_platdata(struct udevice *dev)
-- 
2.19.2

[PATCH v2 07/10] spl: mmc: Fix spl_mmc_get_uboot_raw_sector() implementation

[Faiz Abbas]

The call to spl_mmc_get_uboot_raw_sector() completely ignores and
overwrites the raw_sect value passed from the caller of spl_mmc_load().

Fix this by passing raw_sect to the function and returning the same
value in the default case.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 arch/arm/mach-imx/imx8/image.c |  3 ++-
 common/spl/spl_mmc.c           | 11 ++++-------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-imx/imx8/image.c b/arch/arm/mach-imx/imx8/image.c
index 58a29e8a03..08b0050f57 100644
--- a/arch/arm/mach-imx/imx8/image.c
+++ b/arch/arm/mach-imx/imx8/image.c
@@ -196,7 +196,8 @@ unsigned long spl_spi_get_uboot_offs(struct spi_flash *flash)
 #endif
 
 #ifdef CONFIG_SPL_MMC_SUPPORT
-unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc)
+unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
+					   unsigned long raw_sect)
 {
 	int end;
 
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index 3e6a17c110..a2ea363e96 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -317,13 +317,10 @@ int spl_boot_partition(const u32 boot_device)
 }
 #endif
 
-unsigned long __weak spl_mmc_get_uboot_raw_sector(struct mmc *mmc)
+unsigned long __weak spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
+						  unsigned long raw_sect)
 {
-#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
-	return CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR;
-#else
-	return 0;
-#endif
+	return raw_sect;
 }
 
 int spl_mmc_load(struct spl_image_info *spl_image,
@@ -392,7 +389,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
 				return err;
 		}
 
-		raw_sect = spl_mmc_get_uboot_raw_sector(mmc);
+		raw_sect = spl_mmc_get_uboot_raw_sector(mmc, raw_sect);
 
 #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
 		err = mmc_load_image_raw_partition(spl_image, mmc, raw_part,
-- 
2.19.2

[PATCH v2 08/10] arm: K3: sysfw-loader: Add a config_pm_pre_callback()

[Faiz Abbas]

System firmware does not guarantee that clocks going out of the device
will be stable during power management configuration. There are some
DCRC errors when SPL tries to get the next stage during eMMC boot after
sysfw pm configuration.

Therefore add a config_pm_pre_callback() to switch off the eMMC clock
before power management and restart it after it is done.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 arch/arm/mach-k3/am6_init.c                  | 33 +++++++++++++++++++-
 arch/arm/mach-k3/include/mach/sysfw-loader.h |  2 +-
 arch/arm/mach-k3/j721e_init.c                | 33 +++++++++++++++++++-
 arch/arm/mach-k3/sysfw-loader.c              |  6 +++-
 4 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c
index 8d107b870b..9d3c62b3f4 100644
--- a/arch/arm/mach-k3/am6_init.c
+++ b/arch/arm/mach-k3/am6_init.c
@@ -17,6 +17,7 @@
 #include <dm/uclass-internal.h>
 #include <dm/pinctrl.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
+#include <mmc.h>
 
 #ifdef CONFIG_SPL_BUILD
 #ifdef CONFIG_K3_LOAD_SYSFW
@@ -86,6 +87,33 @@ static void store_boot_index_from_rom(void)
 	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
 }
 
+#if defined(CONFIG_K3_LOAD_SYSFW)
+void k3_mmc_stop_clock(void)
+{
+	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
+		struct mmc *mmc = find_mmc_device(0);
+
+		if (!mmc)
+			return;
+
+		mmc->saved_clock = mmc->clock;
+		mmc_set_clock(mmc, 0, true);
+	}
+}
+
+void k3_mmc_restart_clock(void)
+{
+	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
+		struct mmc *mmc = find_mmc_device(0);
+
+		if (!mmc)
+			return;
+
+		mmc_set_clock(mmc, mmc->saved_clock, false);
+	}
+}
+#endif
+
 void board_init_f(ulong dummy)
 {
 #if defined(CONFIG_K3_LOAD_SYSFW) || defined(CONFIG_K3_AM654_DDRSS)
@@ -128,7 +156,10 @@ void board_init_f(ulong dummy)
 	 * callback hook, effectively switching on (or over) the console
 	 * output.
 	 */
-	k3_sysfw_loader(preloader_console_init);
+	k3_sysfw_loader(k3_mmc_stop_clock, k3_mmc_restart_clock);
+
+	/* Prepare console output */
+	preloader_console_init();
 
 	/* Disable ROM configured firewalls right after loading sysfw */
 #ifdef CONFIG_TI_SECURE_DEVICE
diff --git a/arch/arm/mach-k3/include/mach/sysfw-loader.h b/arch/arm/mach-k3/include/mach/sysfw-loader.h
index 36eb265348..6f5612b4fd 100644
--- a/arch/arm/mach-k3/include/mach/sysfw-loader.h
+++ b/arch/arm/mach-k3/include/mach/sysfw-loader.h
@@ -7,6 +7,6 @@
 #ifndef _SYSFW_LOADER_H_
 #define _SYSFW_LOADER_H_
 
-void k3_sysfw_loader(void (*config_pm_done_callback)(void));
+void k3_sysfw_loader(void (*config_pm_pre_callback)(void), void (*config_pm_done_callback)(void));
 
 #endif
diff --git a/arch/arm/mach-k3/j721e_init.c b/arch/arm/mach-k3/j721e_init.c
index f7f7398081..0994522f6c 100644
--- a/arch/arm/mach-k3/j721e_init.c
+++ b/arch/arm/mach-k3/j721e_init.c
@@ -18,6 +18,7 @@
 #include <dm.h>
 #include <dm/uclass-internal.h>
 #include <dm/pinctrl.h>
+#include <mmc.h>
 
 #ifdef CONFIG_SPL_BUILD
 #ifdef CONFIG_K3_LOAD_SYSFW
@@ -100,6 +101,33 @@ static void ctrl_mmr_unlock(void)
 	mmr_unlock(CTRL_MMR0_BASE, 7);
 }
 
+#if defined(CONFIG_K3_LOAD_SYSFW)
+void k3_mmc_stop_clock(void)
+{
+	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
+		struct mmc *mmc = find_mmc_device(0);
+
+		if (!mmc)
+			return;
+
+		mmc->saved_clock = mmc->clock;
+		mmc_set_clock(mmc, 0, true);
+	}
+}
+
+void k3_mmc_restart_clock(void)
+{
+	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
+		struct mmc *mmc = find_mmc_device(0);
+
+		if (!mmc)
+			return;
+
+		mmc_set_clock(mmc, mmc->saved_clock, false);
+	}
+}
+#endif
+
 /*
  * This uninitialized global variable would normal end up in the .bss section,
  * but the .bss is cleared between writing and reading this variable, so move
@@ -154,7 +182,10 @@ void board_init_f(ulong dummy)
 	 * callback hook, effectively switching on (or over) the console
 	 * output.
 	 */
-	k3_sysfw_loader(preloader_console_init);
+	k3_sysfw_loader(k3_mmc_stop_clock, k3_mmc_restart_clock);
+
+	/* Prepare console output */
+	preloader_console_init();
 
 	/* Disable ROM configured firewalls right after loading sysfw */
 #ifdef CONFIG_TI_SECURE_DEVICE
diff --git a/arch/arm/mach-k3/sysfw-loader.c b/arch/arm/mach-k3/sysfw-loader.c
index 5903bbe12a..8386499ed6 100644
--- a/arch/arm/mach-k3/sysfw-loader.c
+++ b/arch/arm/mach-k3/sysfw-loader.c
@@ -172,7 +172,8 @@ static void k3_sysfw_configure_using_fit(void *fit,
 		      ret);
 }
 
-void k3_sysfw_loader(void (*config_pm_done_callback)(void))
+void k3_sysfw_loader(void (*config_pm_pre_callback) (void),
+		     void (*config_pm_done_callback)(void))
 {
 	struct spl_image_info spl_image = { 0 };
 	struct spl_boot_device bootdev = { 0 };
@@ -264,6 +265,9 @@ void k3_sysfw_loader(void (*config_pm_done_callback)(void))
 	/* Parse and apply the different SYSFW configuration fragments */
 	k3_sysfw_configure_using_fit(sysfw_load_address, ti_sci);
 
+	if (config_pm_pre_callback)
+		config_pm_pre_callback();
+
 	/*
 	 * Now that all clocks and PM aspects are setup, invoke a user-
 	 * provided callback function. Usually this callback would be used
-- 
2.19.2

[PATCH v2 09/10] configs: am65x_evm: Add CONFIG_SUPPORT_EMMC_BOOT

[Faiz Abbas]

With CONFIG_SUPPORT_EMMC_BOOT moved to Kconfig, move it to defconfig
files.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 configs/am65x_evm_a53_defconfig | 1 +
 configs/am65x_evm_r5_defconfig  | 1 +
 include/configs/am65x_evm.h     | 2 --
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/am65x_evm_a53_defconfig b/configs/am65x_evm_a53_defconfig
index 079cd912ba..099a9b46cc 100644
--- a/configs/am65x_evm_a53_defconfig
+++ b/configs/am65x_evm_a53_defconfig
@@ -74,6 +74,7 @@ CONFIG_DM_KEYBOARD=y
 CONFIG_DM_MAILBOX=y
 CONFIG_K3_SEC_PROXY=y
 CONFIG_DM_MMC=y
+CONFIG_SUPPORT_EMMC_BOOT=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ADMA=y
 CONFIG_SPL_MMC_SDHCI_ADMA=y
diff --git a/configs/am65x_evm_r5_defconfig b/configs/am65x_evm_r5_defconfig
index 69055d5536..b475aceefe 100644
--- a/configs/am65x_evm_r5_defconfig
+++ b/configs/am65x_evm_r5_defconfig
@@ -73,6 +73,7 @@ CONFIG_K3_SEC_PROXY=y
 CONFIG_MISC=y
 CONFIG_K3_AVS0=y
 CONFIG_DM_MMC=y
+CONFIG_SUPPORT_EMMC_BOOT=y
 CONFIG_MMC_SDHCI=y
 CONFIG_SPL_MMC_SDHCI_ADMA=y
 CONFIG_MMC_SDHCI_AM654=y
diff --git a/include/configs/am65x_evm.h b/include/configs/am65x_evm.h
index 7d7f86a059..23ee2254ed 100644
--- a/include/configs/am65x_evm.h
+++ b/include/configs/am65x_evm.h
@@ -127,8 +127,6 @@
 #define CONFIG_SYS_MMC_ENV_PART	1
 #endif
 
-#define CONFIG_SUPPORT_EMMC_BOOT
-
 /* Now for the remaining common defines */
 #include <configs/ti_armv7_common.h>
 
-- 
2.19.2

[PATCH v2 10/10] configs: j721e_evm: Add Support for eMMC boot

[Faiz Abbas]

Enable configs to support eMMC boot support.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 configs/j721e_evm_a72_defconfig | 3 +++
 configs/j721e_evm_r5_defconfig  | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/configs/j721e_evm_a72_defconfig b/configs/j721e_evm_a72_defconfig
index d5e54a228d..8320fa8c5b 100644
--- a/configs/j721e_evm_a72_defconfig
+++ b/configs/j721e_evm_a72_defconfig
@@ -26,6 +26,8 @@ CONFIG_SPL_BOARD_INIT=y
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_SEPARATE_BSS=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
 CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_I2C_SUPPORT=y
 CONFIG_SPL_DM_MAILBOX=y
@@ -90,6 +92,7 @@ CONFIG_SYS_I2C_OMAP24XX=y
 CONFIG_DM_MAILBOX=y
 CONFIG_K3_SEC_PROXY=y
 CONFIG_DM_MMC=y
+CONFIG_SUPPORT_EMMC_BOOT=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ADMA=y
 CONFIG_SPL_MMC_SDHCI_ADMA=y
diff --git a/configs/j721e_evm_r5_defconfig b/configs/j721e_evm_r5_defconfig
index a90ab62195..ea726b4c7d 100644
--- a/configs/j721e_evm_r5_defconfig
+++ b/configs/j721e_evm_r5_defconfig
@@ -25,6 +25,8 @@ CONFIG_USE_BOOTCOMMAND=y
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_SEPARATE_BSS=y
 CONFIG_SPL_EARLY_BSS=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x400
 CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_I2C_SUPPORT=y
 CONFIG_SPL_DM_MAILBOX=y
@@ -72,6 +74,7 @@ CONFIG_MISC=y
 CONFIG_FS_LOADER=y
 CONFIG_K3_AVS0=y
 CONFIG_DM_MMC=y
+CONFIG_SUPPORT_EMMC_BOOT=y
 CONFIG_MMC_SDHCI=y
 CONFIG_SPL_MMC_SDHCI_ADMA=y
 CONFIG_MMC_SDHCI_AM654=y
-- 
2.19.2

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Jaehoon Chung]

On 1/24/20 8:52 PM, Faiz Abbas wrote:
> Expose sdhci_init() as non-static.

Does it need to change to non-static?

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  drivers/mmc/sdhci.c | 2 +-
>  include/sdhci.h     | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 01fa5a9d4d..4fce85a0ea 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -618,7 +618,7 @@ static int sdhci_set_ios(struct mmc *mmc)
>  	return 0;
>  }
>  
> -static int sdhci_init(struct mmc *mmc)
> +int sdhci_init(struct mmc *mmc)
>  {
>  	struct sdhci_host *host = mmc->priv;
>  #if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(DM_GPIO)
> diff --git a/include/sdhci.h b/include/sdhci.h
> index 01addb7a60..0830e05d53 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -487,6 +487,7 @@ void sdhci_set_uhs_timing(struct sdhci_host *host);
>  #ifdef CONFIG_DM_MMC
>  /* Export the operations to drivers */
>  int sdhci_probe(struct udevice *dev);
> +int sdhci_init(struct mmc *mmc);
>  int sdhci_set_clock(struct mmc *mmc, unsigned int clock);
>  extern const struct dm_mmc_ops sdhci_ops;
>  #else
> 

[PATCH v2 01/10] mmc: Add a saved_clock member

[Jaehoon Chung]

On 1/24/20 8:52 PM, Faiz Abbas wrote:
> Add a saved_clock member to struct mmc to store the previous clock speed
> in the clock needs to be stopped for some time.

I think that it doesn't need to add saved_clock for mmc member.
This is for only yours. Does it impossible to add saved_clock in platdata?

And i'm not sure but mmc->tran_speed should be kept previous speed.
I will check it 

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  include/mmc.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/mmc.h b/include/mmc.h
> index b5cb514f57..2f21dbf1b7 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -602,6 +602,7 @@ struct mmc {
>  	bool clk_disable; /* true if the clock can be turned off */
>  	uint bus_width;
>  	uint clock;
> +	uint saved_clock;
>  	enum mmc_voltage signal_voltage;
>  	uint card_caps;
>  	uint host_caps;
> 

[PATCH v2 08/10] arm: K3: sysfw-loader: Add a config_pm_pre_callback()

[Jaehoon Chung]

On 1/24/20 8:52 PM, Faiz Abbas wrote:
> System firmware does not guarantee that clocks going out of the device
> will be stable during power management configuration. There are some
> DCRC errors when SPL tries to get the next stage during eMMC boot after
> sysfw pm configuration.
> 
> Therefore add a config_pm_pre_callback() to switch off the eMMC clock
> before power management and restart it after it is done.
> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  arch/arm/mach-k3/am6_init.c                  | 33 +++++++++++++++++++-
>  arch/arm/mach-k3/include/mach/sysfw-loader.h |  2 +-
>  arch/arm/mach-k3/j721e_init.c                | 33 +++++++++++++++++++-
>  arch/arm/mach-k3/sysfw-loader.c              |  6 +++-
>  4 files changed, 70 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c
> index 8d107b870b..9d3c62b3f4 100644
> --- a/arch/arm/mach-k3/am6_init.c
> +++ b/arch/arm/mach-k3/am6_init.c
> @@ -17,6 +17,7 @@
>  #include <dm/uclass-internal.h>
>  #include <dm/pinctrl.h>
>  #include <linux/soc/ti/ti_sci_protocol.h>
> +#include <mmc.h>
>  
>  #ifdef CONFIG_SPL_BUILD
>  #ifdef CONFIG_K3_LOAD_SYSFW
> @@ -86,6 +87,33 @@ static void store_boot_index_from_rom(void)
>  	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
>  }
>  
> +#if defined(CONFIG_K3_LOAD_SYSFW)
> +void k3_mmc_stop_clock(void)
> +{
> +	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
> +		struct mmc *mmc = find_mmc_device(0);
> +
> +		if (!mmc)
> +			return;
> +
> +		mmc->saved_clock = mmc->clock;
> +		mmc_set_clock(mmc, 0, true);
> +	}
> +}
> +
> +void k3_mmc_restart_clock(void)
> +{
> +	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
> +		struct mmc *mmc = find_mmc_device(0);
> +
> +		if (!mmc)
> +			return;
> +
> +		mmc_set_clock(mmc, mmc->saved_clock, false);
> +	}
> +}
> +#endif
> +
>  void board_init_f(ulong dummy)
>  {
>  #if defined(CONFIG_K3_LOAD_SYSFW) || defined(CONFIG_K3_AM654_DDRSS)
> @@ -128,7 +156,10 @@ void board_init_f(ulong dummy)
>  	 * callback hook, effectively switching on (or over) the console
>  	 * output.
>  	 */
> -	k3_sysfw_loader(preloader_console_init);
> +	k3_sysfw_loader(k3_mmc_stop_clock, k3_mmc_restart_clock);
> +
> +	/* Prepare console output */
> +	preloader_console_init();
>  
>  	/* Disable ROM configured firewalls right after loading sysfw */
>  #ifdef CONFIG_TI_SECURE_DEVICE
> diff --git a/arch/arm/mach-k3/include/mach/sysfw-loader.h b/arch/arm/mach-k3/include/mach/sysfw-loader.h
> index 36eb265348..6f5612b4fd 100644
> --- a/arch/arm/mach-k3/include/mach/sysfw-loader.h
> +++ b/arch/arm/mach-k3/include/mach/sysfw-loader.h
> @@ -7,6 +7,6 @@
>  #ifndef _SYSFW_LOADER_H_
>  #define _SYSFW_LOADER_H_
>  
> -void k3_sysfw_loader(void (*config_pm_done_callback)(void));
> +void k3_sysfw_loader(void (*config_pm_pre_callback)(void), void (*config_pm_done_callback)(void));
>  
>  #endif
> diff --git a/arch/arm/mach-k3/j721e_init.c b/arch/arm/mach-k3/j721e_init.c
> index f7f7398081..0994522f6c 100644
> --- a/arch/arm/mach-k3/j721e_init.c
> +++ b/arch/arm/mach-k3/j721e_init.c
> @@ -18,6 +18,7 @@
>  #include <dm.h>
>  #include <dm/uclass-internal.h>
>  #include <dm/pinctrl.h>
> +#include <mmc.h>
>  
>  #ifdef CONFIG_SPL_BUILD
>  #ifdef CONFIG_K3_LOAD_SYSFW
> @@ -100,6 +101,33 @@ static void ctrl_mmr_unlock(void)
>  	mmr_unlock(CTRL_MMR0_BASE, 7);
>  }
>  
> +#if defined(CONFIG_K3_LOAD_SYSFW)
> +void k3_mmc_stop_clock(void)
> +{
> +	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
> +		struct mmc *mmc = find_mmc_device(0);
> +
> +		if (!mmc)
> +			return;
> +
> +		mmc->saved_clock = mmc->clock;
> +		mmc_set_clock(mmc, 0, true);

Use MMC_CLK_DISABLE instead of true.

> +	}
> +}
> +
> +void k3_mmc_restart_clock(void)
> +{
> +	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
> +		struct mmc *mmc = find_mmc_device(0);
> +
> +		if (!mmc)
> +			return;
> +
> +		mmc_set_clock(mmc, mmc->saved_clock, false);

ditto.

> +	}
> +}
> +#endif
> +
>  /*
>   * This uninitialized global variable would normal end up in the .bss section,
>   * but the .bss is cleared between writing and reading this variable, so move
> @@ -154,7 +182,10 @@ void board_init_f(ulong dummy)
>  	 * callback hook, effectively switching on (or over) the console
>  	 * output.
>  	 */
> -	k3_sysfw_loader(preloader_console_init);
> +	k3_sysfw_loader(k3_mmc_stop_clock, k3_mmc_restart_clock);
> +
> +	/* Prepare console output */
> +	preloader_console_init();
>  
>  	/* Disable ROM configured firewalls right after loading sysfw */
>  #ifdef CONFIG_TI_SECURE_DEVICE
> diff --git a/arch/arm/mach-k3/sysfw-loader.c b/arch/arm/mach-k3/sysfw-loader.c
> index 5903bbe12a..8386499ed6 100644
> --- a/arch/arm/mach-k3/sysfw-loader.c
> +++ b/arch/arm/mach-k3/sysfw-loader.c
> @@ -172,7 +172,8 @@ static void k3_sysfw_configure_using_fit(void *fit,
>  		      ret);
>  }
>  
> -void k3_sysfw_loader(void (*config_pm_done_callback)(void))
> +void k3_sysfw_loader(void (*config_pm_pre_callback) (void),
> +		     void (*config_pm_done_callback)(void))
>  {
>  	struct spl_image_info spl_image = { 0 };
>  	struct spl_boot_device bootdev = { 0 };
> @@ -264,6 +265,9 @@ void k3_sysfw_loader(void (*config_pm_done_callback)(void))
>  	/* Parse and apply the different SYSFW configuration fragments */
>  	k3_sysfw_configure_using_fit(sysfw_load_address, ti_sci);
>  
> +	if (config_pm_pre_callback)
> +		config_pm_pre_callback();
> +
>  	/*
>  	 * Now that all clocks and PM aspects are setup, invoke a user-
>  	 * provided callback function. Usually this callback would be used
> 

[PATCH v2 02/10] mmc: Add init() API

[Simon Goldschmidt]

On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
>
> Add an init() API for platform specific init() operations.

Could you describe why this cannot be done in the probe callback? It's not
easily visible as the function you changed (mmc_get_op_cond) doesn't even have
a comment to describe what it does...

In general, I think commit messages could be more detailed than one line. If
only to make it easier in the future to recap why things have been done.

>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  drivers/mmc/mmc.c | 13 ++++++-------
>  include/mmc.h     |  7 +++++++
>  2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index d43983d4a6..50df8c8626 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -2787,14 +2787,13 @@ int mmc_get_op_cond(struct mmc *mmc)
>         }
>         if (err)
>                 return err;
> -
>  #if CONFIG_IS_ENABLED(DM_MMC)
> -       /* The device has already been probed ready for use */
> -#else
> -       /* made sure it's not NULL earlier */
> -       err = mmc->cfg->ops->init(mmc);
> -       if (err)
> -               return err;

You're removing the init code for non-DM MMC here and did not describe it in
the commit message. Is this change intended at all?

Regards,
Simon

> +       struct dm_mmc_ops *ops = mmc_get_ops(mmc->dev);
> +       if (ops->init) {
> +               err = ops->init(mmc->dev);
> +               if (err)
> +                       return err;
> +       }
>  #endif
>         mmc->ddr_mode = 0;
>
> diff --git a/include/mmc.h b/include/mmc.h
> index 2f21dbf1b7..6aef125f25 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -406,6 +406,13 @@ struct mmc;
>
>  #if CONFIG_IS_ENABLED(DM_MMC)
>  struct dm_mmc_ops {
> +       /**
> +        * init() - platform specific initialization for the host controller
> +        *
> +        * @dev:        Device to init
> +        * @return 0 if Ok, -ve if error
> +        */
> +       int (*init)(struct udevice *dev);
>         /**
>          * send_cmd() - Send a command to the MMC device
>          *
> --
> 2.19.2
>

[PATCH v2 02/10] mmc: Add init() API

[Simon Goldschmidt]

Forgot to ask: why isn't the subsystem maintainer on CC?
If you would use patman to send patches or at least the get_maintainer script,
he would have been...

Regards,
Simon

On Wed, Jan 29, 2020 at 9:03 AM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
> >
> > Add an init() API for platform specific init() operations.
>
> Could you describe why this cannot be done in the probe callback? It's not
> easily visible as the function you changed (mmc_get_op_cond) doesn't even have
> a comment to describe what it does...
>
> In general, I think commit messages could be more detailed than one line. If
> only to make it easier in the future to recap why things have been done.
>
> >
> > Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> > ---
> >  drivers/mmc/mmc.c | 13 ++++++-------
> >  include/mmc.h     |  7 +++++++
> >  2 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > index d43983d4a6..50df8c8626 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -2787,14 +2787,13 @@ int mmc_get_op_cond(struct mmc *mmc)
> >         }
> >         if (err)
> >                 return err;
> > -
> >  #if CONFIG_IS_ENABLED(DM_MMC)
> > -       /* The device has already been probed ready for use */
> > -#else
> > -       /* made sure it's not NULL earlier */
> > -       err = mmc->cfg->ops->init(mmc);
> > -       if (err)
> > -               return err;
>
> You're removing the init code for non-DM MMC here and did not describe it in
> the commit message. Is this change intended at all?
>
> Regards,
> Simon
>
> > +       struct dm_mmc_ops *ops = mmc_get_ops(mmc->dev);
> > +       if (ops->init) {
> > +               err = ops->init(mmc->dev);
> > +               if (err)
> > +                       return err;
> > +       }
> >  #endif
> >         mmc->ddr_mode = 0;
> >
> > diff --git a/include/mmc.h b/include/mmc.h
> > index 2f21dbf1b7..6aef125f25 100644
> > --- a/include/mmc.h
> > +++ b/include/mmc.h
> > @@ -406,6 +406,13 @@ struct mmc;
> >
> >  #if CONFIG_IS_ENABLED(DM_MMC)
> >  struct dm_mmc_ops {
> > +       /**
> > +        * init() - platform specific initialization for the host controller
> > +        *
> > +        * @dev:        Device to init
> > +        * @return 0 if Ok, -ve if error
> > +        */
> > +       int (*init)(struct udevice *dev);
> >         /**
> >          * send_cmd() - Send a command to the MMC device
> >          *
> > --
> > 2.19.2
> >

[PATCH v2 08/10] arm: K3: sysfw-loader: Add a config_pm_pre_callback()

[Faiz Abbas]

Hi,

On 29/01/20 5:00 am, Jaehoon Chung wrote:
> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>> System firmware does not guarantee that clocks going out of the device
>> will be stable during power management configuration. There are some
>> DCRC errors when SPL tries to get the next stage during eMMC boot after
>> sysfw pm configuration.
>>
>> Therefore add a config_pm_pre_callback() to switch off the eMMC clock
>> before power management and restart it after it is done.
>>
>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  
>> +#if defined(CONFIG_K3_LOAD_SYSFW)
>> +void k3_mmc_stop_clock(void)
>> +{
>> +	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
>> +		struct mmc *mmc = find_mmc_device(0);
>> +
>> +		if (!mmc)
>> +			return;
>> +
>> +		mmc->saved_clock = mmc->clock;
>> +		mmc_set_clock(mmc, 0, true);
> 
> Use MMC_CLK_DISABLE instead of true.
Ok.
> 
>> +	}
>> +}
>> +
>> +void k3_mmc_restart_clock(void)
>> +{
>> +	if (spl_boot_device() == BOOT_DEVICE_MMC1) {
>> +		struct mmc *mmc = find_mmc_device(0);
>> +
>> +		if (!mmc)
>> +			return;
>> +
>> +		mmc_set_clock(mmc, mmc->saved_clock, false);
> 
> ditto.

Ok.

Thanks,
Faiz

[PATCH v2 02/10] mmc: Add init() API

[Faiz Abbas]

Hi Simon,

On 29/01/20 1:33 pm, Simon Goldschmidt wrote:
> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
>>
>> Add an init() API for platform specific init() operations.
> 
> Could you describe why this cannot be done in the probe callback? It's not
> easily visible as the function you changed (mmc_get_op_cond) doesn't even have
> a comment to describe what it does...

The reason is detailed in 06/10 patch description. probe() is always
called for all MMC instances. I only want to switch on power (by calling
sdhci_init()) and suffer the 1 second wait time when there is actually a
card in the slot and user wants to access it.
> 
> In general, I think commit messages could be more detailed than one line. If
> only to make it easier in the future to recap why things have been done.
> 

You're right. I will add a more detailed patch description in v2.

Thanks,
Faiz

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Simon Goldschmidt]

On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
> On 1/24/20 8:52 PM, Faiz Abbas wrote:
> > Expose sdhci_init() as non-static.
>
> Does it need to change to non-static?

And even if it needs to, can we have a reason *why* in the commit message?

Thanks,
Simon

>
> Best Regards,
> Jaehoon Chung
>
> >
> > Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> > ---
> >  drivers/mmc/sdhci.c | 2 +-
> >  include/sdhci.h     | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > index 01fa5a9d4d..4fce85a0ea 100644
> > --- a/drivers/mmc/sdhci.c
> > +++ b/drivers/mmc/sdhci.c
> > @@ -618,7 +618,7 @@ static int sdhci_set_ios(struct mmc *mmc)
> >       return 0;
> >  }
> >
> > -static int sdhci_init(struct mmc *mmc)
> > +int sdhci_init(struct mmc *mmc)
> >  {
> >       struct sdhci_host *host = mmc->priv;
> >  #if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(DM_GPIO)
> > diff --git a/include/sdhci.h b/include/sdhci.h
> > index 01addb7a60..0830e05d53 100644
> > --- a/include/sdhci.h
> > +++ b/include/sdhci.h
> > @@ -487,6 +487,7 @@ void sdhci_set_uhs_timing(struct sdhci_host *host);
> >  #ifdef CONFIG_DM_MMC
> >  /* Export the operations to drivers */
> >  int sdhci_probe(struct udevice *dev);
> > +int sdhci_init(struct mmc *mmc);
> >  int sdhci_set_clock(struct mmc *mmc, unsigned int clock);
> >  extern const struct dm_mmc_ops sdhci_ops;
> >  #else
> >
>

[PATCH v2 06/10] mmc: am654_sdhci: Implement workaround for card detect

[Simon Goldschmidt]

On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
>
> The 4 bit MMC controllers have an internal debounce for the SDCD line
> with a debounce delay of 1 second. Therefore, after clocks to the IP are
> enabled, software has to wait for this time before it can power on the
> controller.
>
> Add an init() callback which polls on sdcd for a maximum of 2 seconds
> before switching on power to the controller or (in the case of no card)
> returning a ENOMEDIUM. This pushes the 1 second wait time to when the
> card is actually needed rather than at every probe() making sure that
> users who don't insert an SD card in the slot don't have to wait such a
> long time.
>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  drivers/mmc/am654_sdhci.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
> index ff0a81eaab..ccae3fea31 100644
> --- a/drivers/mmc/am654_sdhci.c
> +++ b/drivers/mmc/am654_sdhci.c
> @@ -254,7 +254,7 @@ const struct am654_driver_data j721e_4bit_drv_data = {
>         .flags = IOMUX_PRESENT,
>  };
>
> -int am654_sdhci_init(struct am654_sdhci_plat *plat)
> +int am654_sdhci_init_phy(struct am654_sdhci_plat *plat)
>  {
>         u32 ctl_cfg_2 = 0;
>         u32 mask, val;
> @@ -331,8 +331,37 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
>         return 0;
>  }
>
> +#define MAX_SDCD_DEBOUNCE_TIME 2000
> +int am654_sdhci_init(struct udevice *dev)
> +{
> +       struct am654_sdhci_plat *plat = dev_get_platdata(dev);
> +       struct mmc *mmc = mmc_get_mmc_dev(dev);
> +       struct sdhci_host *host = mmc->priv;
> +       unsigned long start;
> +       int val;
> +
> +       /*
> +        * The controller takes about 1 second to debounce the card detect line
> +        * and doesn't let us power on until that time is up. Instead of waiting
> +        * for 1 second at every stage, poll on the CARD_PRESENT bit upto a
> +        * maximum of 2 seconds to be safe..
> +        */
> +       start = get_timer(0);
> +       do {
> +               if (get_timer(start) > MAX_SDCD_DEBOUNCE_TIME)
> +                       return -ENOMEDIUM;
> +
> +               val = mmc_getcd(host->mmc);
> +       } while (!val);
> +
> +       am654_sdhci_init_phy(plat);
> +
> +       return sdhci_init(mmc);
> +}
> +
>  static int am654_sdhci_probe(struct udevice *dev)
>  {
> +       struct dm_mmc_ops *ops = mmc_get_ops(dev);
>         struct am654_driver_data *drv_data =
>                         (struct am654_driver_data *)dev_get_driver_data(dev);
>         struct am654_sdhci_plat *plat = dev_get_platdata(dev);
> @@ -373,9 +402,9 @@ static int am654_sdhci_probe(struct udevice *dev)
>
>         regmap_init_mem_index(dev_ofnode(dev), &plat->base, 1);
>
> -       am654_sdhci_init(plat);
> +       ops->init = am654_sdhci_init;

Is this a valid approach? I mean, many drivers create their 'ops' const like
this:
   static const struct ram_ops altera_gen5_sdram_ops (...)

That would mean you write to a const region. I know the U-Boot sources make
this easy for you by providing the ops non-const via mmc_get_ops, but I still
think this is not good.

Regards,
Simon

>
> -       return sdhci_probe(dev);
> +       return 0;
>  }
>
>  static int am654_sdhci_ofdata_to_platdata(struct udevice *dev)
> --
> 2.19.2
>

[PATCH v2 02/10] mmc: Add init() API

[Faiz Abbas]

Hi,

+Lokesh, Tom

On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
> Forgot to ask: why isn't the subsystem maintainer on CC?
> If you would use patman to send patches or at least the get_maintainer script,
> he would have been...
> 

I did use get_maintainer for my send-email CC list but everyone other
than Michal seems to have been dropped. Here is an excerpt from the
email header I received:

From: Faiz Abbas <faiz_abbas@ti.com>
To: <u-boot@lists.denx.de>
CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>,
	<peng.fan@nxp.com>, <faiz_abbas@ti.com>
Subject: [PATCH v2 02/10] mmc: Add init() API
Date: Fri, 24 Jan 2020 17:22:44 +0530


But in the patchworks and in your reply, only Michal is remaining:
https://patchwork.ozlabs.org/patch/1228781/

Michal,

What do you see in your message header? Does it have other people copied?

Thanks,
Faiz

[PATCH v2 02/10] mmc: Add init() API

[Michal Simek]

On 30. 01. 20 16:03, Faiz Abbas wrote:
> Hi,
> 
> +Lokesh, Tom
> 
> On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
>> Forgot to ask: why isn't the subsystem maintainer on CC?
>> If you would use patman to send patches or at least the get_maintainer script,
>> he would have been...
>>
> 
> I did use get_maintainer for my send-email CC list but everyone other
> than Michal seems to have been dropped. Here is an excerpt from the
> email header I received:
> 
> From: Faiz Abbas <faiz_abbas@ti.com>
> To: <u-boot@lists.denx.de>
> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>,
> 	<peng.fan@nxp.com>, <faiz_abbas@ti.com>
> Subject: [PATCH v2 02/10] mmc: Add init() API
> Date: Fri, 24 Jan 2020 17:22:44 +0530
> 
> 
> But in the patchworks and in your reply, only Michal is remaining:
> https://patchwork.ozlabs.org/patch/1228781/
> 
> Michal,
> 
> What do you see in your message header? Does it have other people copied?

[u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c
Peng Fan <peng.fan@nxp.com> (maintainer:MMC)
u-boot at lists.denx.de (open list)

I see Peng there.

Thanks,
Michal

[PATCH v2 02/10] mmc: Add init() API

[Simon Goldschmidt]

Faiz Abbas <faiz_abbas@ti.com> schrieb am Do., 30. Jan. 2020, 16:01:

> Hi,
>
> +Lokesh, Tom
>
> On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
> > Forgot to ask: why isn't the subsystem maintainer on CC?
> > If you would use patman to send patches or at least the get_maintainer
> script,
> > he would have been...
> >
>
> I did use get_maintainer for my send-email CC list but everyone other
> than Michal seems to have been dropped. Here is an excerpt from the
> email header I received:
>
> From: Faiz Abbas <faiz_abbas@ti.com>
> To: <u-boot@lists.denx.de>
> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>,
>         <peng.fan@nxp.com>, <faiz_abbas@ti.com>
> Subject: [PATCH v2 02/10] mmc: Add init() API
> Date: Fri, 24 Jan 2020 17:22:44 +0530
>
>
> But in the patchworks and in your reply, only Michal is remaining:
> https://patchwork.ozlabs.org/patch/1228781/


I hit reply all in the gmail web interface. My header only shows Michal.

Regards,
Simon


>
> Michal,
>
> What do you see in your message header? Does it have other people copied?


> Thanks,
> Faiz
>

[PATCH v2 02/10] mmc: Add init() API

[Faiz Abbas]

Hi Michal,

On 30/01/20 8:37 pm, Michal Simek wrote:
> On 30. 01. 20 16:03, Faiz Abbas wrote:
>> Hi,
>>
>> +Lokesh, Tom
>>
>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
>>> Forgot to ask: why isn't the subsystem maintainer on CC?
>>> If you would use patman to send patches or at least the get_maintainer script,
>>> he would have been...
>>>
>>
>> I did use get_maintainer for my send-email CC list but everyone other
>> than Michal seems to have been dropped. Here is an excerpt from the
>> email header I received:
>>
>> From: Faiz Abbas <faiz_abbas@ti.com>
>> To: <u-boot@lists.denx.de>
>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>,
>> 	<peng.fan@nxp.com>, <faiz_abbas@ti.com>
>> Subject: [PATCH v2 02/10] mmc: Add init() API
>> Date: Fri, 24 Jan 2020 17:22:44 +0530
>>
>>
>> But in the patchworks and in your reply, only Michal is remaining:
>> https://patchwork.ozlabs.org/patch/1228781/
>>
>> Michal,
>>
>> What do you see in your message header? Does it have other people copied?
> 
> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c
> Peng Fan <peng.fan@nxp.com> (maintainer:MMC)
> u-boot at lists.denx.de (open list)
> 
> I see Peng there.
> 

You misunderstood. I am not asking if you see Peng in the get_maintainer
output. Do you see him CC'd in the original patch email?

Thanks,
Faiz

[PATCH v2 02/10] mmc: Add init() API

[Michal Simek]

On 30. 01. 20 16:14, Faiz Abbas wrote:
> Hi Michal,
> 
> On 30/01/20 8:37 pm, Michal Simek wrote:
>> On 30. 01. 20 16:03, Faiz Abbas wrote:
>>> Hi,
>>>
>>> +Lokesh, Tom
>>>
>>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
>>>> Forgot to ask: why isn't the subsystem maintainer on CC?
>>>> If you would use patman to send patches or at least the get_maintainer script,
>>>> he would have been...
>>>>
>>>
>>> I did use get_maintainer for my send-email CC list but everyone other
>>> than Michal seems to have been dropped. Here is an excerpt from the
>>> email header I received:
>>>
>>> From: Faiz Abbas <faiz_abbas@ti.com>
>>> To: <u-boot@lists.denx.de>
>>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>,
>>> 	<peng.fan@nxp.com>, <faiz_abbas@ti.com>
>>> Subject: [PATCH v2 02/10] mmc: Add init() API
>>> Date: Fri, 24 Jan 2020 17:22:44 +0530
>>>
>>>
>>> But in the patchworks and in your reply, only Michal is remaining:
>>> https://patchwork.ozlabs.org/patch/1228781/
>>>
>>> Michal,
>>>
>>> What do you see in your message header? Does it have other people copied?
>>
>> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c
>> Peng Fan <peng.fan@nxp.com> (maintainer:MMC)
>> u-boot at lists.denx.de (open list)
>>
>> I see Peng there.
>>
> 
> You misunderstood. I am not asking if you see Peng in the get_maintainer
> output. Do you see him CC'd in the original patch email?

Nope. I can't see him there.

M

[PATCH v2 02/10] mmc: Add init() API

[Tom Rini]

On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote:
> On 30. 01. 20 16:14, Faiz Abbas wrote:
> > Hi Michal,
> > 
> > On 30/01/20 8:37 pm, Michal Simek wrote:
> >> On 30. 01. 20 16:03, Faiz Abbas wrote:
> >>> Hi,
> >>>
> >>> +Lokesh, Tom
> >>>
> >>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
> >>>> Forgot to ask: why isn't the subsystem maintainer on CC?
> >>>> If you would use patman to send patches or at least the get_maintainer script,
> >>>> he would have been...
> >>>>
> >>>
> >>> I did use get_maintainer for my send-email CC list but everyone other
> >>> than Michal seems to have been dropped. Here is an excerpt from the
> >>> email header I received:
> >>>
> >>> From: Faiz Abbas <faiz_abbas@ti.com>
> >>> To: <u-boot@lists.denx.de>
> >>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>,
> >>> 	<peng.fan@nxp.com>, <faiz_abbas@ti.com>
> >>> Subject: [PATCH v2 02/10] mmc: Add init() API
> >>> Date: Fri, 24 Jan 2020 17:22:44 +0530
> >>>
> >>>
> >>> But in the patchworks and in your reply, only Michal is remaining:
> >>> https://patchwork.ozlabs.org/patch/1228781/
> >>>
> >>> Michal,
> >>>
> >>> What do you see in your message header? Does it have other people copied?
> >>
> >> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c
> >> Peng Fan <peng.fan@nxp.com> (maintainer:MMC)
> >> u-boot at lists.denx.de (open list)
> >>
> >> I see Peng there.
> >>
> > 
> > You misunderstood. I am not asking if you see Peng in the get_maintainer
> > output. Do you see him CC'd in the original patch email?
> 
> Nope. I can't see him there.

Wolfgang, is there some mailman setting that needs tweaking or looking
at here?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200130/bbe1d7b4/attachment.sig>

[PATCH v2 02/10] mmc: Add init() API

[Simon Goldschmidt]

Tom Rini <trini@konsulko.com> schrieb am Do., 30. Jan. 2020, 16:32:

> On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote:
> > On 30. 01. 20 16:14, Faiz Abbas wrote:
> > > Hi Michal,
> > >
> > > On 30/01/20 8:37 pm, Michal Simek wrote:
> > >> On 30. 01. 20 16:03, Faiz Abbas wrote:
> > >>> Hi,
> > >>>
> > >>> +Lokesh, Tom
> > >>>
> > >>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
> > >>>> Forgot to ask: why isn't the subsystem maintainer on CC?
> > >>>> If you would use patman to send patches or at least the
> get_maintainer script,
> > >>>> he would have been...
> > >>>>
> > >>>
> > >>> I did use get_maintainer for my send-email CC list but everyone other
> > >>> than Michal seems to have been dropped. Here is an excerpt from the
> > >>> email header I received:
> > >>>
> > >>> From: Faiz Abbas <faiz_abbas@ti.com>
> > >>> To: <u-boot@lists.denx.de>
> > >>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <
> lokeshvutla at ti.com>,
> > >>>   <peng.fan@nxp.com>, <faiz_abbas@ti.com>
> > >>> Subject: [PATCH v2 02/10] mmc: Add init() API
> > >>> Date: Fri, 24 Jan 2020 17:22:44 +0530
> > >>>
> > >>>
> > >>> But in the patchworks and in your reply, only Michal is remaining:
> > >>> https://patchwork.ozlabs.org/patch/1228781/
> > >>>
> > >>> Michal,
> > >>>
> > >>> What do you see in your message header? Does it have other people
> copied?
> > >>
> > >> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c
> > >> Peng Fan <peng.fan@nxp.com> (maintainer:MMC)
> > >> u-boot at lists.denx.de (open list)
> > >>
> > >> I see Peng there.
> > >>
> > >
> > > You misunderstood. I am not asking if you see Peng in the
> get_maintainer
> > > output. Do you see him CC'd in the original patch email?
> >
> > Nope. I can't see him there.
>
> Wolfgang, is there some mailman setting that needs tweaking or looking
> at here?  Thanks!
>

Can this be a mailman issue? If Michal was CCed, is mailman involved in the
way to him? I would have thought that mail got delivered directly.

Regards,
Simon

[PATCH v2 02/10] mmc: Add init() API

[Tom Rini]

On Thu, Jan 30, 2020 at 04:35:40PM +0100, Simon Goldschmidt wrote:
> Tom Rini <trini@konsulko.com> schrieb am Do., 30. Jan. 2020, 16:32:
> 
> > On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote:
> > > On 30. 01. 20 16:14, Faiz Abbas wrote:
> > > > Hi Michal,
> > > >
> > > > On 30/01/20 8:37 pm, Michal Simek wrote:
> > > >> On 30. 01. 20 16:03, Faiz Abbas wrote:
> > > >>> Hi,
> > > >>>
> > > >>> +Lokesh, Tom
> > > >>>
> > > >>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
> > > >>>> Forgot to ask: why isn't the subsystem maintainer on CC?
> > > >>>> If you would use patman to send patches or at least the
> > get_maintainer script,
> > > >>>> he would have been...
> > > >>>>
> > > >>>
> > > >>> I did use get_maintainer for my send-email CC list but everyone other
> > > >>> than Michal seems to have been dropped. Here is an excerpt from the
> > > >>> email header I received:
> > > >>>
> > > >>> From: Faiz Abbas <faiz_abbas@ti.com>
> > > >>> To: <u-boot@lists.denx.de>
> > > >>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <
> > lokeshvutla at ti.com>,
> > > >>>   <peng.fan@nxp.com>, <faiz_abbas@ti.com>
> > > >>> Subject: [PATCH v2 02/10] mmc: Add init() API
> > > >>> Date: Fri, 24 Jan 2020 17:22:44 +0530
> > > >>>
> > > >>>
> > > >>> But in the patchworks and in your reply, only Michal is remaining:
> > > >>> https://patchwork.ozlabs.org/patch/1228781/
> > > >>>
> > > >>> Michal,
> > > >>>
> > > >>> What do you see in your message header? Does it have other people
> > copied?
> > > >>
> > > >> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c
> > > >> Peng Fan <peng.fan@nxp.com> (maintainer:MMC)
> > > >> u-boot at lists.denx.de (open list)
> > > >>
> > > >> I see Peng there.
> > > >>
> > > >
> > > > You misunderstood. I am not asking if you see Peng in the
> > get_maintainer
> > > > output. Do you see him CC'd in the original patch email?
> > >
> > > Nope. I can't see him there.
> >
> > Wolfgang, is there some mailman setting that needs tweaking or looking
> > at here?  Thanks!
> >
> 
> Can this be a mailman issue? If Michal was CCed, is mailman involved in the
> way to him? I would have thought that mail got delivered directly.

I was thinking about the setting on if you get your own messages / ones
you're on CC to or not, and if that was the copy say in Michal's inbox
or U-Boot folder.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200130/dafbdd9c/attachment.sig>

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Jaehoon Chung]

Hi Simon,

On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>
>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>>> Expose sdhci_init() as non-static.
>>
>> Does it need to change to non-static?
> 
> And even if it needs to, can we have a reason *why* in the commit message?

When i read entire your series, it seems that doesn't need to change to non-static.
All of change that touched MMC code are only for your board.
I don't know Peng's opinion, but it's not my prefer.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Simon
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> ---
>>>  drivers/mmc/sdhci.c | 2 +-
>>>  include/sdhci.h     | 1 +
>>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>>> index 01fa5a9d4d..4fce85a0ea 100644
>>> --- a/drivers/mmc/sdhci.c
>>> +++ b/drivers/mmc/sdhci.c
>>> @@ -618,7 +618,7 @@ static int sdhci_set_ios(struct mmc *mmc)
>>>       return 0;
>>>  }
>>>
>>> -static int sdhci_init(struct mmc *mmc)
>>> +int sdhci_init(struct mmc *mmc)
>>>  {
>>>       struct sdhci_host *host = mmc->priv;
>>>  #if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(DM_GPIO)
>>> diff --git a/include/sdhci.h b/include/sdhci.h
>>> index 01addb7a60..0830e05d53 100644
>>> --- a/include/sdhci.h
>>> +++ b/include/sdhci.h
>>> @@ -487,6 +487,7 @@ void sdhci_set_uhs_timing(struct sdhci_host *host);
>>>  #ifdef CONFIG_DM_MMC
>>>  /* Export the operations to drivers */
>>>  int sdhci_probe(struct udevice *dev);
>>> +int sdhci_init(struct mmc *mmc);
>>>  int sdhci_set_clock(struct mmc *mmc, unsigned int clock);
>>>  extern const struct dm_mmc_ops sdhci_ops;
>>>  #else
>>>
>>
> 
> 

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Simon Goldschmidt]

Am 30.01.2020 um 23:21 schrieb Jaehoon Chung:
> Hi Simon,
> 
> On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
>> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>
>>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>>>> Expose sdhci_init() as non-static.
>>>
>>> Does it need to change to non-static?
>>
>> And even if it needs to, can we have a reason *why* in the commit message?
> 
> When i read entire your series, it seems that doesn't need to change to non-static.
> All of change that touched MMC code are only for your board.
> I don't know Peng's opinion, but it's not my prefer.

+1!

We need to keep the core code clean of such hacks in order to keep the 
size small for constrained targets!

Regards,
Simon

> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Thanks,
>> Simon
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>
>>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>>> ---
>>>>   drivers/mmc/sdhci.c | 2 +-
>>>>   include/sdhci.h     | 1 +
>>>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>>>> index 01fa5a9d4d..4fce85a0ea 100644
>>>> --- a/drivers/mmc/sdhci.c
>>>> +++ b/drivers/mmc/sdhci.c
>>>> @@ -618,7 +618,7 @@ static int sdhci_set_ios(struct mmc *mmc)
>>>>        return 0;
>>>>   }
>>>>
>>>> -static int sdhci_init(struct mmc *mmc)
>>>> +int sdhci_init(struct mmc *mmc)
>>>>   {
>>>>        struct sdhci_host *host = mmc->priv;
>>>>   #if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(DM_GPIO)
>>>> diff --git a/include/sdhci.h b/include/sdhci.h
>>>> index 01addb7a60..0830e05d53 100644
>>>> --- a/include/sdhci.h
>>>> +++ b/include/sdhci.h
>>>> @@ -487,6 +487,7 @@ void sdhci_set_uhs_timing(struct sdhci_host *host);
>>>>   #ifdef CONFIG_DM_MMC
>>>>   /* Export the operations to drivers */
>>>>   int sdhci_probe(struct udevice *dev);
>>>> +int sdhci_init(struct mmc *mmc);
>>>>   int sdhci_set_clock(struct mmc *mmc, unsigned int clock);
>>>>   extern const struct dm_mmc_ops sdhci_ops;
>>>>   #else
>>>>
>>>
>>
>>
> 

[PATCH v2 02/10] mmc: Add init() API

[Wolfgang Denk]

Dear Tom,

In message <20200130153216.GX13379@bill-the-cat> you wrote:
> 
> > > You misunderstood. I am not asking if you see Peng in the get_maintainer
> > > output. Do you see him CC'd in the original patch email?
> >
> > Nope. I can't see him there.
>
> Wolfgang, is there some mailman setting that needs tweaking or looking
> at here?  Thanks!

I'm not sure.  I cannot see any trae of the Cc: s at list.denx.de,
as this is handled by the poster's MTA.  I can only see the posting
as it arrived at the list server, and this has just

	From: Faiz Abbas <faiz_abbas@ti.com>
	To: <u-boot@lists.denx.de>
	Cc: michal.simek at xilinx.com

It would be interesting to check the MTA los on the sender side if
the Cc:s were actually sent...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Ill-chosen abstraction is particularly evident in the design  of  the
ADA  runtime  system.  The  interface to the ADA runtime system is so
opaque that it is impossible to model  or  predict  its  performance,
making it effectively useless for real-time systems.
                              - Marc D.  Donner and David H. Jameson.

[PATCH v2 02/10] mmc: Add init() API

[Wolfgang Denk]

Dear Simon,

In message <CAAh8qswkmJhpgSZx4KcGiXrEugS47M+_VB7Nkcs27kpDLpVcAQ@mail.gmail.com> you wrote:
>
> Can this be a mailman issue? If Michal was CCed, is mailman involved in the
> way to him? I would have thought that mail got delivered directly.

Correct, it's the sender's MTA who handles the action transmission.
The question here is if there was actually more than the single
address in the Cc: header when the message to the list got sent.
For example, it would be interesting to know if any of the people
who were supposed to be on the Cc: list (but now don't show up)
got a mesage directly, or only through the mailing list.



Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"355/113 -- Not the famous irrational number PI,  but  an  incredible
simulation!"

[PATCH v2 02/10] mmc: Add init() API

[Wolfgang Denk]

Dear Tom,

In message <20200130153827.GY13379@bill-the-cat> you wrote:
> 
> > way to him? I would have thought that mail got delivered directly.
>
> I was thinking about the setting on if you get your own messages / ones
> you're on CC to or not, and if that was the copy say in Michal's inbox
> or U-Boot folder.

Yes, but thisis a secondaryquestion here.  The more interesting one
(especially for patchwork etc.) is where the Cc: header lost the
additional addresses - was it at the sender's MUA, the sender's MTA
or on lists.denx.de ; I don't really trust mailman if it's getting
dark outside, but I have no good way to check this.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The Buddha, the Godhead, resides quite as comfortably in the circuits
of a digital computer or the gears of a cycle transmission as he does
at the top of a mountain or in the petals of a flower.
            - R.  Pirsig, "Zen and the Art of Motorcycle Maintenance"

[PATCH v2 02/10] mmc: Add init() API

[Peng Fan]

> Subject: Re: [PATCH v2 02/10] mmc: Add init() API
> 
> Hi Simon,
> 
> On 29/01/20 1:33 pm, Simon Goldschmidt wrote:
> > On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
> >>
> >> Add an init() API for platform specific init() operations.
> >
> > Could you describe why this cannot be done in the probe callback? It's
> > not easily visible as the function you changed (mmc_get_op_cond)
> > doesn't even have a comment to describe what it does...
> 
> The reason is detailed in 06/10 patch description. probe() is always called for
> all MMC instances. I only want to switch on power (by calling
> sdhci_init()) and suffer the 1 second wait time when there is actually a card in
> the slot and user wants to access it.
> >
> > In general, I think commit messages could be more detailed than one
> > line. If only to make it easier in the future to recap why things have been
> done.
> >
> 
> You're right. I will add a more detailed patch description in v2.

This patch is v2. Please v3.

Could a quirk be added in probe and if card not there, just bypass?
Or you want quick boot to avoid probe all the controllers?

Regards,
Peng.

> 
> Thanks,
> Faiz

[PATCH v2 02/10] mmc: Add init() API

[Sekhar Nori]

Michal,

On 30/01/20 9:08 PM, Tom Rini wrote:
> On Thu, Jan 30, 2020 at 04:35:40PM +0100, Simon Goldschmidt wrote:
>> Tom Rini <trini@konsulko.com> schrieb am Do., 30. Jan. 2020, 16:32:
>>
>>> On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote:
>>>> On 30. 01. 20 16:14, Faiz Abbas wrote:
>>>>> Hi Michal,
>>>>>
>>>>> On 30/01/20 8:37 pm, Michal Simek wrote:
>>>>>> On 30. 01. 20 16:03, Faiz Abbas wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> +Lokesh, Tom
>>>>>>>
>>>>>>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
>>>>>>>> Forgot to ask: why isn't the subsystem maintainer on CC?
>>>>>>>> If you would use patman to send patches or at least the
>>> get_maintainer script,
>>>>>>>> he would have been...
>>>>>>>>
>>>>>>>
>>>>>>> I did use get_maintainer for my send-email CC list but everyone other
>>>>>>> than Michal seems to have been dropped. Here is an excerpt from the
>>>>>>> email header I received:
>>>>>>>
>>>>>>> From: Faiz Abbas <faiz_abbas@ti.com>
>>>>>>> To: <u-boot@lists.denx.de>
>>>>>>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <
>>> lokeshvutla at ti.com>,
>>>>>>>   <peng.fan@nxp.com>, <faiz_abbas@ti.com>
>>>>>>> Subject: [PATCH v2 02/10] mmc: Add init() API
>>>>>>> Date: Fri, 24 Jan 2020 17:22:44 +0530
>>>>>>>
>>>>>>>
>>>>>>> But in the patchworks and in your reply, only Michal is remaining:
>>>>>>> https://patchwork.ozlabs.org/patch/1228781/
>>>>>>>
>>>>>>> Michal,
>>>>>>>
>>>>>>> What do you see in your message header? Does it have other people
>>> copied?
>>>>>>
>>>>>> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c
>>>>>> Peng Fan <peng.fan@nxp.com> (maintainer:MMC)
>>>>>> u-boot at lists.denx.de (open list)
>>>>>>
>>>>>> I see Peng there.
>>>>>>
>>>>>
>>>>> You misunderstood. I am not asking if you see Peng in the
>>> get_maintainer
>>>>> output. Do you see him CC'd in the original patch email?
>>>>
>>>> Nope. I can't see him there.
>>>
>>> Wolfgang, is there some mailman setting that needs tweaking or looking
>>> at here?  Thanks!
>>>
>>
>> Can this be a mailman issue? If Michal was CCed, is mailman involved in the
>> way to him? I would have thought that mail got delivered directly.
> 
> I was thinking about the setting on if you get your own messages / ones
> you're on CC to or not, and if that was the copy say in Michal's inbox
> or U-Boot folder.

Can you confirm how you received the message, directly or through the list?

The TI e-mail team says the CCs were not removed prior to leaving TI.

Thanks,
Sekhar

[PATCH v2 02/10] mmc: Add init() API

[Michal Simek]

On 03. 02. 20 7:04, Sekhar Nori wrote:
> Michal,
> 
> On 30/01/20 9:08 PM, Tom Rini wrote:
>> On Thu, Jan 30, 2020 at 04:35:40PM +0100, Simon Goldschmidt wrote:
>>> Tom Rini <trini@konsulko.com> schrieb am Do., 30. Jan. 2020, 16:32:
>>>
>>>> On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote:
>>>>> On 30. 01. 20 16:14, Faiz Abbas wrote:
>>>>>> Hi Michal,
>>>>>>
>>>>>> On 30/01/20 8:37 pm, Michal Simek wrote:
>>>>>>> On 30. 01. 20 16:03, Faiz Abbas wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> +Lokesh, Tom
>>>>>>>>
>>>>>>>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote:
>>>>>>>>> Forgot to ask: why isn't the subsystem maintainer on CC?
>>>>>>>>> If you would use patman to send patches or at least the
>>>> get_maintainer script,
>>>>>>>>> he would have been...
>>>>>>>>>
>>>>>>>>
>>>>>>>> I did use get_maintainer for my send-email CC list but everyone other
>>>>>>>> than Michal seems to have been dropped. Here is an excerpt from the
>>>>>>>> email header I received:
>>>>>>>>
>>>>>>>> From: Faiz Abbas <faiz_abbas@ti.com>
>>>>>>>> To: <u-boot@lists.denx.de>
>>>>>>>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <
>>>> lokeshvutla at ti.com>,
>>>>>>>>   <peng.fan@nxp.com>, <faiz_abbas@ti.com>
>>>>>>>> Subject: [PATCH v2 02/10] mmc: Add init() API
>>>>>>>> Date: Fri, 24 Jan 2020 17:22:44 +0530
>>>>>>>>
>>>>>>>>
>>>>>>>> But in the patchworks and in your reply, only Michal is remaining:
>>>>>>>> https://patchwork.ozlabs.org/patch/1228781/
>>>>>>>>
>>>>>>>> Michal,
>>>>>>>>
>>>>>>>> What do you see in your message header? Does it have other people
>>>> copied?
>>>>>>>
>>>>>>> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c
>>>>>>> Peng Fan <peng.fan@nxp.com> (maintainer:MMC)
>>>>>>> u-boot at lists.denx.de (open list)
>>>>>>>
>>>>>>> I see Peng there.
>>>>>>>
>>>>>>
>>>>>> You misunderstood. I am not asking if you see Peng in the
>>>> get_maintainer
>>>>>> output. Do you see him CC'd in the original patch email?
>>>>>
>>>>> Nope. I can't see him there.
>>>>
>>>> Wolfgang, is there some mailman setting that needs tweaking or looking
>>>> at here?  Thanks!
>>>>
>>>
>>> Can this be a mailman issue? If Michal was CCed, is mailman involved in the
>>> way to him? I would have thought that mail got delivered directly.
>>
>> I was thinking about the setting on if you get your own messages / ones
>> you're on CC to or not, and if that was the copy say in Michal's inbox
>> or U-Boot folder.
> 
> Can you confirm how you received the message, directly or through the list?
> 
> The TI e-mail team says the CCs were not removed prior to leaving TI.

It looks like it went through list.

M

[PATCH v2 02/10] mmc: Add init() API

[Faiz Abbas]

Hi Peng,

On 01/02/20 6:43 pm, Peng Fan wrote:
>> Subject: Re: [PATCH v2 02/10] mmc: Add init() API
>>
>> Hi Simon,
>>
>> On 29/01/20 1:33 pm, Simon Goldschmidt wrote:
>>> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
>>>>
>>>> Add an init() API for platform specific init() operations.
>>>
>>> Could you describe why this cannot be done in the probe callback? It's
>>> not easily visible as the function you changed (mmc_get_op_cond)
>>> doesn't even have a comment to describe what it does...
>>
>> The reason is detailed in 06/10 patch description. probe() is always called for
>> all MMC instances. I only want to switch on power (by calling
>> sdhci_init()) and suffer the 1 second wait time when there is actually a card in
>> the slot and user wants to access it.
>>>
>>> In general, I think commit messages could be more detailed than one
>>> line. If only to make it easier in the future to recap why things have been
>> done.
>>>
>>
>> You're right. I will add a more detailed patch description in v2.
> 
> This patch is v2. Please v3.
> 
> Could a quirk be added in probe and if card not there, just bypass?
> Or you want quick boot to avoid probe all the controllers?
> 

The issue is that we need to wait for 1 second to detect the card
itself. I want to move that delay out from probe().

BTW did you receive this mail directly or through the list? I had CC'd
you but its not there in the patchwork headers.

Thanks,
Faiz

[PATCH v2 02/10] mmc: Add init() API

[Peng Fan]

> Subject: Re: [PATCH v2 02/10] mmc: Add init() API
> 
> Hi Peng,
> 
> On 01/02/20 6:43 pm, Peng Fan wrote:
> >> Subject: Re: [PATCH v2 02/10] mmc: Add init() API
> >>
> >> Hi Simon,
> >>
> >> On 29/01/20 1:33 pm, Simon Goldschmidt wrote:
> >>> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
> >>>>
> >>>> Add an init() API for platform specific init() operations.
> >>>
> >>> Could you describe why this cannot be done in the probe callback?
> >>> It's not easily visible as the function you changed
> >>> (mmc_get_op_cond) doesn't even have a comment to describe what it
> does...
> >>
> >> The reason is detailed in 06/10 patch description. probe() is always
> >> called for all MMC instances. I only want to switch on power (by
> >> calling
> >> sdhci_init()) and suffer the 1 second wait time when there is
> >> actually a card in the slot and user wants to access it.
> >>>
> >>> In general, I think commit messages could be more detailed than one
> >>> line. If only to make it easier in the future to recap why things
> >>> have been
> >> done.
> >>>
> >>
> >> You're right. I will add a more detailed patch description in v2.
> >
> > This patch is v2. Please v3.
> >
> > Could a quirk be added in probe and if card not there, just bypass?
> > Or you want quick boot to avoid probe all the controllers?
> >
> 
> The issue is that we need to wait for 1 second to detect the card itself. I want
> to move that delay out from probe().

Understand. Then I am ok for patch.

Please add more info in commit log in new version patchset.

> 
> BTW did you receive this mail directly or through the list? I had CC'd you but
> its not there in the patchwork headers.

I am in the cc list of your first mail, but not from Simon's reply mail.

Thanks,
Peng.

> 
> Thanks,
> Faiz

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Faiz Abbas]

Hi,

On 31/01/20 3:55 am, Simon Goldschmidt wrote:
> Am 30.01.2020 um 23:21 schrieb Jaehoon Chung:
>> Hi Simon,
>>
>> On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
>>> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung
>>> <jh80.chung@samsung.com> wrote:
>>>>
>>>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>>>>> Expose sdhci_init() as non-static.
>>>>
>>>> Does it need to change to non-static?
>>>
>>> And even if it needs to, can we have a reason *why* in the commit
>>> message?
>>
>> When i read entire your series, it seems that doesn't need to change
>> to non-static.
>> All of change that touched MMC code are only for your board.
>> I don't know Peng's opinion, but it's not my prefer.
> 
> +1!
> 
> We need to keep the core code clean of such hacks in order to keep the
> size small for constrained targets!
> 

Peng can you comment on this?

Jaehoon, I am not sure what you mean by "it doesn't need to change to
non-static". How would I call the sdhci_init() function from my platform
driver otherwise?

Thanks,
Faiz

[PATCH v2 02/10] mmc: Add init() API

[Faiz Abbas]

Hi Tom, Wolfgang,

On 03/02/20 5:34 pm, Peng Fan wrote:
>> Subject: Re: [PATCH v2 02/10] mmc: Add init() API
>>
>> Hi Peng,
>>
>> On 01/02/20 6:43 pm, Peng Fan wrote:
>>>> Subject: Re: [PATCH v2 02/10] mmc: Add init() API
>>>>
>>>> Hi Simon,
>>>>
>>>> On 29/01/20 1:33 pm, Simon Goldschmidt wrote:
>>>>> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
>>>>>>
>>>>>> Add an init() API for platform specific init() operations.
>>>>>
>>>>> Could you describe why this cannot be done in the probe callback?
>>>>> It's not easily visible as the function you changed
>>>>> (mmc_get_op_cond) doesn't even have a comment to describe what it
>> does...
>>>>
>>>> The reason is detailed in 06/10 patch description. probe() is always
>>>> called for all MMC instances. I only want to switch on power (by
>>>> calling
>>>> sdhci_init()) and suffer the 1 second wait time when there is
>>>> actually a card in the slot and user wants to access it.
>>>>>
>>>>> In general, I think commit messages could be more detailed than one
>>>>> line. If only to make it easier in the future to recap why things
>>>>> have been
>>>> done.
>>>>>
>>>>
>>>> You're right. I will add a more detailed patch description in v2.
>>>
>>> This patch is v2. Please v3.
>>>
>>> Could a quirk be added in probe and if card not there, just bypass?
>>> Or you want quick boot to avoid probe all the controllers?
>>>
>>
>> The issue is that we need to wait for 1 second to detect the card itself. I want
>> to move that delay out from probe().
> 
> Understand. Then I am ok for patch.
> 
> Please add more info in commit log in new version patchset.
> 
>>
>> BTW did you receive this mail directly or through the list? I had CC'd you but
>> its not there in the patchwork headers.
> 
> I am in the cc list of your first mail, but not from Simon's reply mail.
> 

So Peng got the email but the list is dropping CCs after it gets them.
How do I avoid this in the future? Should I always add maintainers in To?

Thanks,
Faiz

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Peng Fan]

> Subject: Re: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static
> 
> Hi,
> 
> On 31/01/20 3:55 am, Simon Goldschmidt wrote:
> > Am 30.01.2020 um 23:21 schrieb Jaehoon Chung:
> >> Hi Simon,
> >>
> >> On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
> >>> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung
> >>> <jh80.chung@samsung.com> wrote:
> >>>>
> >>>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
> >>>>> Expose sdhci_init() as non-static.
> >>>>
> >>>> Does it need to change to non-static?
> >>>
> >>> And even if it needs to, can we have a reason *why* in the commit
> >>> message?
> >>
> >> When i read entire your series, it seems that doesn't need to change
> >> to non-static.
> >> All of change that touched MMC code are only for your board.
> >> I don't know Peng's opinion, but it's not my prefer.
> >
> > +1!
> >
> > We need to keep the core code clean of such hacks in order to keep the
> > size small for constrained targets!
> >
> 
> Peng can you comment on this?

Just one more question, does kernel has same issue, how resolved?
Could U-Boot follow similar approach?

Actually I am fine with platform specific approach , considering
your platform issue.

But since Simon and Jaehoon has concerns, not sure whether
Simon and Jaehoon have good ideas.

Regards,
Peng.

> 
> Jaehoon, I am not sure what you mean by "it doesn't need to change to
> non-static". How would I call the sdhci_init() function from my platform driver
> otherwise?
> 
> Thanks,
> Faiz

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Faiz Abbas]

Hi Peng,

On 05/02/20 12:58 pm, Peng Fan wrote:
>> Subject: Re: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static
>>
>> Hi,
>>
>> On 31/01/20 3:55 am, Simon Goldschmidt wrote:
>>> Am 30.01.2020 um 23:21 schrieb Jaehoon Chung:
>>>> Hi Simon,
>>>>
>>>> On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
>>>>> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung
>>>>> <jh80.chung@samsung.com> wrote:
>>>>>>
>>>>>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>>>>>>> Expose sdhci_init() as non-static.
>>>>>>
>>>>>> Does it need to change to non-static?
>>>>>
>>>>> And even if it needs to, can we have a reason *why* in the commit
>>>>> message?
>>>>
>>>> When i read entire your series, it seems that doesn't need to change
>>>> to non-static.
>>>> All of change that touched MMC code are only for your board.
>>>> I don't know Peng's opinion, but it's not my prefer.
>>>
>>> +1!
>>>
>>> We need to keep the core code clean of such hacks in order to keep the
>>> size small for constrained targets!
>>>
>>
>> Peng can you comment on this?
> 
> Just one more question, does kernel has same issue, how resolved?
> Could U-Boot follow similar approach?

Kernel has interrupts enabled. So software just has to wait for the card
detect interrupt to arrive rather than poll on anything.

> 
> Actually I am fine with platform specific approach , considering
> your platform issue.
> 
> But since Simon and Jaehoon has concerns, not sure whether
> Simon and Jaehoon have good ideas.
> 

Ok. Lets see if they have some better ideas.

Thanks,
Faiz

[PATCH v2 02/10] mmc: Add init() API

[Wolfgang Denk]

Dear Faiz,

In message <04e0144c-cad3-d242-0393-ba33afa3dd7a@ti.com> you wrote:
>
> > I am in the cc list of your first mail, but not from Simon's reply mail.
>
> So Peng got the email but the list is dropping CCs after it gets them.
> How do I avoid this in the future? Should I always add maintainers in To?

We are investigating this.  I _think_ (but this needs to be
verified, I just had a short look) that Mailman might try to bee too
clever; my speculation is that it might remove addresses from the
Cc: list wich have the "nodupes" option set in their profile.
Maybe mailman "thinks" that this is what it should do - otherwise
the recipient would receive dupes at least for a reply-to-all, one
trough the mailing list and the other through the Cc:

But as mentioned, this needs to be investigated.

I can see this ancient bug report [1], where a reply reads:

        In any case, this behavior is intentional by design. Cc:
        recipients who are list members with their 'avoid duplicates'
        option set are removed from the Cc: list to keep that list
        from growing excessively in long threads with many
        'reply-all' replies.

[1] https://bugs.launchpad.net/mailman/+bug/1216960


So apparently a solution/workaoround could be to globally remove the
"nodupes" option for all subscribers, but I'm not sure if this is
what we should do.


I feel the key problem here is that we expect something from the
mailing list that it has not been designed for - the differentiation
between "this is just some random posting" and "this is a posting
which is specifically addressed to you".  this may or may not work,
and it depends on several factors - how the mailing list tool works,
and how the recipient filters his incoming e-mail.

But I don't have any clever solution either.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A Sense of humor can help you overlook the unattractive, tolerate the
unpleasant,  cope  with  the  unexpected  and   smile   through   the
unbearable.                                           - Moshe Waldoks

[PATCH v2 06/10] mmc: am654_sdhci: Implement workaround for card detect

[Faiz Abbas]

Simon,

On 29/01/20 7:48 pm, Simon Goldschmidt wrote:
> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
>>
>> The 4 bit MMC controllers have an internal debounce for the SDCD line
>> with a debounce delay of 1 second. Therefore, after clocks to the IP are
>> enabled, software has to wait for this time before it can power on the
>> controller.
>>
>> Add an init() callback which polls on sdcd for a maximum of 2 seconds
>> before switching on power to the controller or (in the case of no card)
>> returning a ENOMEDIUM. This pushes the 1 second wait time to when the
>> card is actually needed rather than at every probe() making sure that
>> users who don't insert an SD card in the slot don't have to wait such a
>> long time.
>>
>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  drivers/mmc/am654_sdhci.c | 35 ++++++++++++++++++++++++++++++++---
>>  1 file changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
>> index ff0a81eaab..ccae3fea31 100644
>> --- a/drivers/mmc/am654_sdhci.c
>> +++ b/drivers/mmc/am654_sdhci.c
>> @@ -254,7 +254,7 @@ const struct am654_driver_data j721e_4bit_drv_data = {
>>         .flags = IOMUX_PRESENT,
>>  };
>>
>> -int am654_sdhci_init(struct am654_sdhci_plat *plat)
>> +int am654_sdhci_init_phy(struct am654_sdhci_plat *plat)
>>  {
>>         u32 ctl_cfg_2 = 0;
>>         u32 mask, val;
>> @@ -331,8 +331,37 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
>>         return 0;
>>  }
>>
>> +#define MAX_SDCD_DEBOUNCE_TIME 2000
>> +int am654_sdhci_init(struct udevice *dev)
>> +{
>> +       struct am654_sdhci_plat *plat = dev_get_platdata(dev);
>> +       struct mmc *mmc = mmc_get_mmc_dev(dev);
>> +       struct sdhci_host *host = mmc->priv;
>> +       unsigned long start;
>> +       int val;
>> +
>> +       /*
>> +        * The controller takes about 1 second to debounce the card detect line
>> +        * and doesn't let us power on until that time is up. Instead of waiting
>> +        * for 1 second at every stage, poll on the CARD_PRESENT bit upto a
>> +        * maximum of 2 seconds to be safe..
>> +        */
>> +       start = get_timer(0);
>> +       do {
>> +               if (get_timer(start) > MAX_SDCD_DEBOUNCE_TIME)
>> +                       return -ENOMEDIUM;
>> +
>> +               val = mmc_getcd(host->mmc);
>> +       } while (!val);
>> +
>> +       am654_sdhci_init_phy(plat);
>> +
>> +       return sdhci_init(mmc);
>> +}
>> +
>>  static int am654_sdhci_probe(struct udevice *dev)
>>  {
>> +       struct dm_mmc_ops *ops = mmc_get_ops(dev);
>>         struct am654_driver_data *drv_data =
>>                         (struct am654_driver_data *)dev_get_driver_data(dev);
>>         struct am654_sdhci_plat *plat = dev_get_platdata(dev);
>> @@ -373,9 +402,9 @@ static int am654_sdhci_probe(struct udevice *dev)
>>
>>         regmap_init_mem_index(dev_ofnode(dev), &plat->base, 1);
>>
>> -       am654_sdhci_init(plat);
>> +       ops->init = am654_sdhci_init;
> 
> Is this a valid approach? I mean, many drivers create their 'ops' const like
> this:
>    static const struct ram_ops altera_gen5_sdram_ops (...)
> 
> That would mean you write to a const region. I know the U-Boot sources make
> this easy for you by providing the ops non-const via mmc_get_ops, but I still
> think this is not good.
> 

Sorry I missed this earlier.

I do see other drivers following this approach (see
drivers/mmc/sdhci-cadence.c). The issue is that I then have to export
every API in drivers/mmc/sdhci.c:694

const struct dm_mmc_ops sdhci_ops = {
        .send_cmd       = sdhci_send_command,
        .set_ios        = sdhci_set_ios,
        .get_cd         = sdhci_get_cd,
#ifdef MMC_SUPPORTS_TUNING
        .execute_tuning = sdhci_execute_tuning,
#endif
};

and create a duplicate structure in my platform driver with an extra .init

OR

I have to create one more wrapper sdhci_pltaform_init() API in the sdhci
driver that just calls a platform init function inside it. So its

include/sdhci.h:

struct sdhci_ops {
..
+	int (*platform_init)()

and then drivers/mmc/sdhci.c:


+dm_sdhci_platform_init()
+{
+...
+	host->ops->platform_init();
+}

const struct dm_mmc_ops sdhci_ops  = {
...
+	.init		= dm_sdhci_platform_init,
};


Both of these approaches are too much boiler plate code for nothing so
its just simpler to use my approach itself.

Anyways, please comment here or in my next version if you have a better
idea.

Thanks,
Faiz

[PATCH v2 06/10] mmc: am654_sdhci: Implement workaround for card detect

[Simon Goldschmidt]

+Simon Glass for the xxx_get_ops() functions in DM

On Mon, Feb 10, 2020 at 10:46 AM Faiz Abbas <faiz_abbas@ti.com> wrote:
>
> Simon,
>
> On 29/01/20 7:48 pm, Simon Goldschmidt wrote:
> > On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
> >>
> >> The 4 bit MMC controllers have an internal debounce for the SDCD line
> >> with a debounce delay of 1 second. Therefore, after clocks to the IP are
> >> enabled, software has to wait for this time before it can power on the
> >> controller.
> >>
> >> Add an init() callback which polls on sdcd for a maximum of 2 seconds
> >> before switching on power to the controller or (in the case of no card)
> >> returning a ENOMEDIUM. This pushes the 1 second wait time to when the
> >> card is actually needed rather than at every probe() making sure that
> >> users who don't insert an SD card in the slot don't have to wait such a
> >> long time.
> >>
> >> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> >> ---
> >>  drivers/mmc/am654_sdhci.c | 35 ++++++++++++++++++++++++++++++++---
> >>  1 file changed, 32 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
> >> index ff0a81eaab..ccae3fea31 100644
> >> --- a/drivers/mmc/am654_sdhci.c
> >> +++ b/drivers/mmc/am654_sdhci.c
> >> @@ -254,7 +254,7 @@ const struct am654_driver_data j721e_4bit_drv_data = {
> >>         .flags = IOMUX_PRESENT,
> >>  };
> >>
> >> -int am654_sdhci_init(struct am654_sdhci_plat *plat)
> >> +int am654_sdhci_init_phy(struct am654_sdhci_plat *plat)
> >>  {
> >>         u32 ctl_cfg_2 = 0;
> >>         u32 mask, val;
> >> @@ -331,8 +331,37 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
> >>         return 0;
> >>  }
> >>
> >> +#define MAX_SDCD_DEBOUNCE_TIME 2000
> >> +int am654_sdhci_init(struct udevice *dev)
> >> +{
> >> +       struct am654_sdhci_plat *plat = dev_get_platdata(dev);
> >> +       struct mmc *mmc = mmc_get_mmc_dev(dev);
> >> +       struct sdhci_host *host = mmc->priv;
> >> +       unsigned long start;
> >> +       int val;
> >> +
> >> +       /*
> >> +        * The controller takes about 1 second to debounce the card detect line
> >> +        * and doesn't let us power on until that time is up. Instead of waiting
> >> +        * for 1 second at every stage, poll on the CARD_PRESENT bit upto a
> >> +        * maximum of 2 seconds to be safe..
> >> +        */
> >> +       start = get_timer(0);
> >> +       do {
> >> +               if (get_timer(start) > MAX_SDCD_DEBOUNCE_TIME)
> >> +                       return -ENOMEDIUM;
> >> +
> >> +               val = mmc_getcd(host->mmc);
> >> +       } while (!val);
> >> +
> >> +       am654_sdhci_init_phy(plat);
> >> +
> >> +       return sdhci_init(mmc);
> >> +}
> >> +
> >>  static int am654_sdhci_probe(struct udevice *dev)
> >>  {
> >> +       struct dm_mmc_ops *ops = mmc_get_ops(dev);
> >>         struct am654_driver_data *drv_data =
> >>                         (struct am654_driver_data *)dev_get_driver_data(dev);
> >>         struct am654_sdhci_plat *plat = dev_get_platdata(dev);
> >> @@ -373,9 +402,9 @@ static int am654_sdhci_probe(struct udevice *dev)
> >>
> >>         regmap_init_mem_index(dev_ofnode(dev), &plat->base, 1);
> >>
> >> -       am654_sdhci_init(plat);
> >> +       ops->init = am654_sdhci_init;
> >
> > Is this a valid approach? I mean, many drivers create their 'ops' const like
> > this:
> >    static const struct ram_ops altera_gen5_sdram_ops (...)
> >
> > That would mean you write to a const region. I know the U-Boot sources make
> > this easy for you by providing the ops non-const via mmc_get_ops, but I still
> > think this is not good.
> >
>
> Sorry I missed this earlier.
>
> I do see other drivers following this approach (see
> drivers/mmc/sdhci-cadence.c). The issue is that I then have to export
> every API in drivers/mmc/sdhci.c:694
>
> const struct dm_mmc_ops sdhci_ops = {
>         .send_cmd       = sdhci_send_command,
>         .set_ios        = sdhci_set_ios,
>         .get_cd         = sdhci_get_cd,
> #ifdef MMC_SUPPORTS_TUNING
>         .execute_tuning = sdhci_execute_tuning,
> #endif
> };
>
> and create a duplicate structure in my platform driver with an extra .init
>
> OR
>
> I have to create one more wrapper sdhci_pltaform_init() API in the sdhci
> driver that just calls a platform init function inside it. So its
>
> include/sdhci.h:
>
> struct sdhci_ops {
> ..
> +       int (*platform_init)()
>
> and then drivers/mmc/sdhci.c:
>
>
> +dm_sdhci_platform_init()
> +{
> +...
> +       host->ops->platform_init();
> +}
>
> const struct dm_mmc_ops sdhci_ops  = {
> ...
> +       .init           = dm_sdhci_platform_init,
> };
>
>
> Both of these approaches are too much boiler plate code for nothing so
> its just simpler to use my approach itself.
>
> Anyways, please comment here or in my next version if you have a better
> idea.

Honestly, I don't know the mmc code well enough to give you an alternative.

However, I do know that most drivers define 'ops' as const, so to prevent
writing to const memory, mmc_get_ops() and other similar accessors should return
a const pointer. I know that they don't currently, but strictly speaking, this
is a bug.

Your adding this code here makes it harder to fix that bug and change
mmc_get_ops() to return a const pointer.

Regards,
Simon

[PATCH v2 02/10] mmc: Add init() API

[Tom Rini]

On Sun, Feb 09, 2020 at 02:38:15PM +0100, Wolfgang Denk wrote:
> Dear Faiz,
> 
> In message <04e0144c-cad3-d242-0393-ba33afa3dd7a@ti.com> you wrote:
> >
> > > I am in the cc list of your first mail, but not from Simon's reply mail.
> >
> > So Peng got the email but the list is dropping CCs after it gets them.
> > How do I avoid this in the future? Should I always add maintainers in To?
> 
> We are investigating this.  I _think_ (but this needs to be
> verified, I just had a short look) that Mailman might try to bee too
> clever; my speculation is that it might remove addresses from the
> Cc: list wich have the "nodupes" option set in their profile.
> Maybe mailman "thinks" that this is what it should do - otherwise
> the recipient would receive dupes at least for a reply-to-all, one
> trough the mailing list and the other through the Cc:
> 
> But as mentioned, this needs to be investigated.
> 
> I can see this ancient bug report [1], where a reply reads:
> 
>         In any case, this behavior is intentional by design. Cc:
>         recipients who are list members with their 'avoid duplicates'
>         option set are removed from the Cc: list to keep that list
>         from growing excessively in long threads with many
>         'reply-all' replies.
> 
> [1] https://bugs.launchpad.net/mailman/+bug/1216960
> 
> 
> So apparently a solution/workaoround could be to globally remove the
> "nodupes" option for all subscribers, but I'm not sure if this is
> what we should do.
> 
> 
> I feel the key problem here is that we expect something from the
> mailing list that it has not been designed for - the differentiation
> between "this is just some random posting" and "this is a posting
> which is specifically addressed to you".  this may or may not work,
> and it depends on several factors - how the mailing list tool works,
> and how the recipient filters his incoming e-mail.
> 
> But I don't have any clever solution either.

I'm a little wary of changing the global setting for everyone as well.
Perhaps we just need to note the problem happened for now and see if it
really happens again in the future, such that we need to consider such a
heavy-handed work-around.  Thanks for looking into this more!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200210/62ecf3c3/attachment.sig>

[PATCH v2 06/10] mmc: am654_sdhci: Implement workaround for card detect

[Simon Glass]

Hi,

On Mon, 10 Feb 2020 at 04:26, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> +Simon Glass for the xxx_get_ops() functions in DM
>
> On Mon, Feb 10, 2020 at 10:46 AM Faiz Abbas <faiz_abbas@ti.com> wrote:
> >
> > Simon,
> >
> > On 29/01/20 7:48 pm, Simon Goldschmidt wrote:
> > > On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote:
> > >>
> > >> The 4 bit MMC controllers have an internal debounce for the SDCD line
> > >> with a debounce delay of 1 second. Therefore, after clocks to the IP are
> > >> enabled, software has to wait for this time before it can power on the
> > >> controller.
> > >>
> > >> Add an init() callback which polls on sdcd for a maximum of 2 seconds
> > >> before switching on power to the controller or (in the case of no card)
> > >> returning a ENOMEDIUM. This pushes the 1 second wait time to when the
> > >> card is actually needed rather than at every probe() making sure that
> > >> users who don't insert an SD card in the slot don't have to wait such a
> > >> long time.
> > >>
> > >> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> > >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> > >> ---
> > >>  drivers/mmc/am654_sdhci.c | 35 ++++++++++++++++++++++++++++++++---
> > >>  1 file changed, 32 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
> > >> index ff0a81eaab..ccae3fea31 100644
> > >> --- a/drivers/mmc/am654_sdhci.c
> > >> +++ b/drivers/mmc/am654_sdhci.c
> > >> @@ -254,7 +254,7 @@ const struct am654_driver_data j721e_4bit_drv_data = {
> > >>         .flags = IOMUX_PRESENT,
> > >>  };
> > >>
> > >> -int am654_sdhci_init(struct am654_sdhci_plat *plat)
> > >> +int am654_sdhci_init_phy(struct am654_sdhci_plat *plat)
> > >>  {
> > >>         u32 ctl_cfg_2 = 0;
> > >>         u32 mask, val;
> > >> @@ -331,8 +331,37 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
> > >>         return 0;
> > >>  }
> > >>
> > >> +#define MAX_SDCD_DEBOUNCE_TIME 2000
> > >> +int am654_sdhci_init(struct udevice *dev)
> > >> +{
> > >> +       struct am654_sdhci_plat *plat = dev_get_platdata(dev);
> > >> +       struct mmc *mmc = mmc_get_mmc_dev(dev);
> > >> +       struct sdhci_host *host = mmc->priv;
> > >> +       unsigned long start;
> > >> +       int val;
> > >> +
> > >> +       /*
> > >> +        * The controller takes about 1 second to debounce the card detect line
> > >> +        * and doesn't let us power on until that time is up. Instead of waiting
> > >> +        * for 1 second at every stage, poll on the CARD_PRESENT bit upto a
> > >> +        * maximum of 2 seconds to be safe..
> > >> +        */
> > >> +       start = get_timer(0);
> > >> +       do {
> > >> +               if (get_timer(start) > MAX_SDCD_DEBOUNCE_TIME)
> > >> +                       return -ENOMEDIUM;
> > >> +
> > >> +               val = mmc_getcd(host->mmc);
> > >> +       } while (!val);
> > >> +
> > >> +       am654_sdhci_init_phy(plat);
> > >> +
> > >> +       return sdhci_init(mmc);
> > >> +}
> > >> +
> > >>  static int am654_sdhci_probe(struct udevice *dev)
> > >>  {
> > >> +       struct dm_mmc_ops *ops = mmc_get_ops(dev);
> > >>         struct am654_driver_data *drv_data =
> > >>                         (struct am654_driver_data *)dev_get_driver_data(dev);
> > >>         struct am654_sdhci_plat *plat = dev_get_platdata(dev);
> > >> @@ -373,9 +402,9 @@ static int am654_sdhci_probe(struct udevice *dev)
> > >>
> > >>         regmap_init_mem_index(dev_ofnode(dev), &plat->base, 1);
> > >>
> > >> -       am654_sdhci_init(plat);
> > >> +       ops->init = am654_sdhci_init;
> > >
> > > Is this a valid approach? I mean, many drivers create their 'ops' const like
> > > this:
> > >    static const struct ram_ops altera_gen5_sdram_ops (...)
> > >
> > > That would mean you write to a const region. I know the U-Boot sources make
> > > this easy for you by providing the ops non-const via mmc_get_ops, but I still
> > > think this is not good.
> > >
> >
> > Sorry I missed this earlier.
> >
> > I do see other drivers following this approach (see
> > drivers/mmc/sdhci-cadence.c). The issue is that I then have to export
> > every API in drivers/mmc/sdhci.c:694
> >
> > const struct dm_mmc_ops sdhci_ops = {
> >         .send_cmd       = sdhci_send_command,
> >         .set_ios        = sdhci_set_ios,
> >         .get_cd         = sdhci_get_cd,
> > #ifdef MMC_SUPPORTS_TUNING
> >         .execute_tuning = sdhci_execute_tuning,
> > #endif
> > };
> >
> > and create a duplicate structure in my platform driver with an extra .init
> >
> > OR
> >
> > I have to create one more wrapper sdhci_pltaform_init() API in the sdhci
> > driver that just calls a platform init function inside it. So its
> >
> > include/sdhci.h:
> >
> > struct sdhci_ops {
> > ..
> > +       int (*platform_init)()
> >
> > and then drivers/mmc/sdhci.c:
> >
> >
> > +dm_sdhci_platform_init()
> > +{
> > +...
> > +       host->ops->platform_init();
> > +}
> >
> > const struct dm_mmc_ops sdhci_ops  = {
> > ...
> > +       .init           = dm_sdhci_platform_init,
> > };
> >
> >
> > Both of these approaches are too much boiler plate code for nothing so
> > its just simpler to use my approach itself.
> >
> > Anyways, please comment here or in my next version if you have a better
> > idea.
>
> Honestly, I don't know the mmc code well enough to give you an alternative.
>
> However, I do know that most drivers define 'ops' as const, so to prevent
> writing to const memory, mmc_get_ops() and other similar accessors should return
> a const pointer. I know that they don't currently, but strictly speaking, this
> is a bug.
>
> Your adding this code here makes it harder to fix that bug and change
> mmc_get_ops() to return a const pointer.

I really don't see why we want mmc_get_ops(). Can we just add the
operation to the existing MMC operations struct? What is the
disadvantage there?

In general with DM we should avoid callbacks and weak functions since
they bypass DM.

Regards,
Simon

[PATCH v2 02/10] mmc: Add init() API

[Wolfgang Denk]

Dear Tom,

In message <20200210142800.GJ13379@bill-the-cat> you wrote:
> 
> I'm a little wary of changing the global setting for everyone as well.
> Perhaps we just need to note the problem happened for now and see if it
> really happens again in the future, such that we need to consider such a
> heavy-handed work-around.  Thanks for looking into this more!

We will continue to look into this a bit deeper; I will report back.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Do not follow where the path may lead....go instead where there is no
path and leave a trail.

[PATCH v2 02/10] mmc: Add init() API

[Wolfgang Denk]

Dear Tom,

In message <20200211135633.8B4D02403FE@gemini.denx.de> I wrote:
>
> > I'm a little wary of changing the global setting for everyone as well.
> > Perhaps we just need to note the problem happened for now and see if it
> > really happens again in the future, such that we need to consider such a
> > heavy-handed work-around.  Thanks for looking into this more!
>
> We will continue to look into this a bit deeper; I will report back.

Running some more tests confirms that

1) mailman indeed removes entries from the Cc: address list, if
   these refer to subscribers _and_ these have the "nodupes" flag
   set;

2) unsetting the "nodupes" option makes the problem go away.


The statement in [1] suggests that this is considered "intentional
by design", though I cannot follow the argument that otherwise Cc:
lists would grow in long threads.  Apparently there are many
different opinions, see for example [3].

The code snippet given in the original bug description [2] is still
about the same in the version of mailman which we are running
(v2.1.26) so it should be easy to remove the responsible "else"
branch...


So an easy work around for the problem is to clear the "nodupes"
setting in your subscription - alternatively we can try and patch
mailman to behave like we want it.


What do you think should we do?


[1] https://bugs.launchpad.net/mailman/+bug/1216960/comments/1
[2] https://bugs.launchpad.net/mailman/+bug/1216960
[3] https://bugs.launchpad.net/mailman/+bug/266682


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"All my life I wanted to be someone; I guess I should have been  more
specific."                                              - Jane Wagner

[PATCH v2 02/10] mmc: Add init() API

[Wolfgang Denk]

Dear Tom,

In message <20200211150800.D895B2403FE@gemini.denx.de> I wrote:
>
> So an easy work around for the problem is to clear the "nodupes"
> setting in your subscription - alternatively we can try and patch
> mailman to behave like we want it.

There is even a clean approach upstream [1]:


[1] https://bazaar.launchpad.net/~mailman-coders/mailman/2.1/revision/1825

Apparently the first RC where this commit is included is mailman
2.1.30rc1; we expect it will take ~ 1 month for v2.1.30 to get
released, 2 months max.  If it's OK we will jsut wait for this
release, then upgrade, and configure with this option set:

1446 # The process which avoids sending a list copy of a message to a member who
1447 # is also directly addressed in To: or Cc: can drop the address from Cc: to
1448 # avoid growing a long Cc: list in long threads.  This can be undesirable as
1449 # it can break DKIM signatures and possibly cause confusion.  To avoid changes
1450 # to Cc: headers, set the list's drop_cc to No.
1451 DEFAULT_DROP_CC = Yes


Agreed?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"If a computer can't directly address all the RAM you can  use,  it's
just a toy."         - anonymous comp.sys.amiga posting, non-sequitir

[PATCH v2 02/10] mmc: Add init() API

[Tom Rini]

On Tue, Feb 11, 2020 at 05:25:39PM +0100, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20200211150800.D895B2403FE@gemini.denx.de> I wrote:
> >
> > So an easy work around for the problem is to clear the "nodupes"
> > setting in your subscription - alternatively we can try and patch
> > mailman to behave like we want it.
> 
> There is even a clean approach upstream [1]:
> 
> 
> [1] https://bazaar.launchpad.net/~mailman-coders/mailman/2.1/revision/1825
> 
> Apparently the first RC where this commit is included is mailman
> 2.1.30rc1; we expect it will take ~ 1 month for v2.1.30 to get
> released, 2 months max.  If it's OK we will jsut wait for this
> release, then upgrade, and configure with this option set:
> 
> 1446 # The process which avoids sending a list copy of a message to a member who
> 1447 # is also directly addressed in To: or Cc: can drop the address from Cc: to
> 1448 # avoid growing a long Cc: list in long threads.  This can be undesirable as
> 1449 # it can break DKIM signatures and possibly cause confusion.  To avoid changes
> 1450 # to Cc: headers, set the list's drop_cc to No.
> 1451 DEFAULT_DROP_CC = Yes
> 
> 
> Agreed?

Agreed, thanks again!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200211/e25a853d/attachment.sig>

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Jaehoon Chung]

Hi,

On 2/5/20 4:33 PM, Faiz Abbas wrote:
> Hi Peng,
> 
> On 05/02/20 12:58 pm, Peng Fan wrote:
>>> Subject: Re: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static
>>>
>>> Hi,
>>>
>>> On 31/01/20 3:55 am, Simon Goldschmidt wrote:
>>>> Am 30.01.2020 um 23:21 schrieb Jaehoon Chung:
>>>>> Hi Simon,
>>>>>
>>>>> On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
>>>>>> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung
>>>>>> <jh80.chung@samsung.com> wrote:
>>>>>>>
>>>>>>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>>>>>>>> Expose sdhci_init() as non-static.
>>>>>>>
>>>>>>> Does it need to change to non-static?
>>>>>>
>>>>>> And even if it needs to, can we have a reason *why* in the commit
>>>>>> message?
>>>>>
>>>>> When i read entire your series, it seems that doesn't need to change
>>>>> to non-static.
>>>>> All of change that touched MMC code are only for your board.
>>>>> I don't know Peng's opinion, but it's not my prefer.
>>>>
>>>> +1!
>>>>
>>>> We need to keep the core code clean of such hacks in order to keep the
>>>> size small for constrained targets!
>>>>
>>>
>>> Peng can you comment on this?
>>
>> Just one more question, does kernel has same issue, how resolved?
>> Could U-Boot follow similar approach?
> 
> Kernel has interrupts enabled. So software just has to wait for the card
> detect interrupt to arrive rather than poll on anything.
> 
>>
>> Actually I am fine with platform specific approach , considering
>> your platform issue.
>>
>> But since Simon and Jaehoon has concerns, not sure whether
>> Simon and Jaehoon have good ideas.
>>
> 
> Ok. Lets see if they have some better ideas.

Well, Your code needs to call am654_sdhci_init() before sdhci_init(), right?

ops->init() -> am654_sdhci_init -> sdhci_init().
If am654_sdhci_init is called for preset, how about adding pre_init() or other ops callback.
(pre_init is just example.)

ops->pre_init = am654_sdhci_init; or ops->preset = am654_sdhci_preset;

In mmc.

ops->preset or ops->pre_init  -> ops->init 

How about adding plotform specific init callback? Then someone can also use it for platform specific things in future.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Faiz
> 
> 
> 

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Faiz Abbas]

Jaehoon,

On 17/02/20 5:47 pm, Jaehoon Chung wrote:
> Hi,
> 
> On 2/5/20 4:33 PM, Faiz Abbas wrote:
>> Hi Peng,
>>
>> On 05/02/20 12:58 pm, Peng Fan wrote:
>>>> Subject: Re: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static
>>>>
>>>> Hi,
>>>>
>>>> On 31/01/20 3:55 am, Simon Goldschmidt wrote:
>>>>> Am 30.01.2020 um 23:21 schrieb Jaehoon Chung:
>>>>>> Hi Simon,
>>>>>>
>>>>>> On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
>>>>>>> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung
>>>>>>> <jh80.chung@samsung.com> wrote:
>>>>>>>>
>>>>>>>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>>>>>>>>> Expose sdhci_init() as non-static.
>>>>>>>>
>>>>>>>> Does it need to change to non-static?
>>>>>>>
>>>>>>> And even if it needs to, can we have a reason *why* in the commit
>>>>>>> message?
>>>>>>
>>>>>> When i read entire your series, it seems that doesn't need to change
>>>>>> to non-static.
>>>>>> All of change that touched MMC code are only for your board.
>>>>>> I don't know Peng's opinion, but it's not my prefer.
>>>>>
>>>>> +1!
>>>>>
>>>>> We need to keep the core code clean of such hacks in order to keep the
>>>>> size small for constrained targets!
>>>>>
>>>>
>>>> Peng can you comment on this?
>>>
>>> Just one more question, does kernel has same issue, how resolved?
>>> Could U-Boot follow similar approach?
>>
>> Kernel has interrupts enabled. So software just has to wait for the card
>> detect interrupt to arrive rather than poll on anything.
>>
>>>
>>> Actually I am fine with platform specific approach , considering
>>> your platform issue.
>>>
>>> But since Simon and Jaehoon has concerns, not sure whether
>>> Simon and Jaehoon have good ideas.
>>>
>>
>> Ok. Lets see if they have some better ideas.
> 
> Well, Your code needs to call am654_sdhci_init() before sdhci_init(), right?
> 
> ops->init() -> am654_sdhci_init -> sdhci_init().
> If am654_sdhci_init is called for preset, how about adding pre_init() or other ops callback.
> (pre_init is just example.)
> 
> ops->pre_init = am654_sdhci_init; or ops->preset = am654_sdhci_preset;
> 
> In mmc.
> 
> ops->preset or ops->pre_init  -> ops->init 
> 
> How about adding plotform specific init callback? Then someone can also use it for platform specific things in future.
> 

So we basically want an init() callback even in sdhci.c.

I have to create one more wrapper sdhci_pltaform_init() API in the sdhci
driver that just calls a platform init function inside it. So its

include/sdhci.h:

struct sdhci_ops {
..
+       int (*platform_init)()

and then drivers/mmc/sdhci.c:


+sdhci_platform_init()
+{
+...
+       host->ops->platform_init();
+}

const struct dm_mmc_ops sdhci_ops  = {
...
+       .init           = sdhci_platform_init,
};

Right?

Thanks,
Faiz

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Jaehoon Chung]

Hi Faiz,

On 2/17/20 9:42 PM, Faiz Abbas wrote:
> Jaehoon,
> 
> On 17/02/20 5:47 pm, Jaehoon Chung wrote:
>> Hi,
>>
>> On 2/5/20 4:33 PM, Faiz Abbas wrote:
>>> Hi Peng,
>>>
>>> On 05/02/20 12:58 pm, Peng Fan wrote:
>>>>> Subject: Re: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 31/01/20 3:55 am, Simon Goldschmidt wrote:
>>>>>> Am 30.01.2020 um 23:21 schrieb Jaehoon Chung:
>>>>>>> Hi Simon,
>>>>>>>
>>>>>>> On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
>>>>>>>> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung
>>>>>>>> <jh80.chung@samsung.com> wrote:
>>>>>>>>>
>>>>>>>>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>>>>>>>>>> Expose sdhci_init() as non-static.
>>>>>>>>>
>>>>>>>>> Does it need to change to non-static?
>>>>>>>>
>>>>>>>> And even if it needs to, can we have a reason *why* in the commit
>>>>>>>> message?
>>>>>>>
>>>>>>> When i read entire your series, it seems that doesn't need to change
>>>>>>> to non-static.
>>>>>>> All of change that touched MMC code are only for your board.
>>>>>>> I don't know Peng's opinion, but it's not my prefer.
>>>>>>
>>>>>> +1!
>>>>>>
>>>>>> We need to keep the core code clean of such hacks in order to keep the
>>>>>> size small for constrained targets!
>>>>>>
>>>>>
>>>>> Peng can you comment on this?
>>>>
>>>> Just one more question, does kernel has same issue, how resolved?
>>>> Could U-Boot follow similar approach?
>>>
>>> Kernel has interrupts enabled. So software just has to wait for the card
>>> detect interrupt to arrive rather than poll on anything.
>>>
>>>>
>>>> Actually I am fine with platform specific approach , considering
>>>> your platform issue.
>>>>
>>>> But since Simon and Jaehoon has concerns, not sure whether
>>>> Simon and Jaehoon have good ideas.
>>>>
>>>
>>> Ok. Lets see if they have some better ideas.
>>
>> Well, Your code needs to call am654_sdhci_init() before sdhci_init(), right?
>>
>> ops->init() -> am654_sdhci_init -> sdhci_init().
>> If am654_sdhci_init is called for preset, how about adding pre_init() or other ops callback.
>> (pre_init is just example.)
>>
>> ops->pre_init = am654_sdhci_init; or ops->preset = am654_sdhci_preset;
>>
>> In mmc.
>>
>> ops->preset or ops->pre_init  -> ops->init 
>>
>> How about adding plotform specific init callback? Then someone can also use it for platform specific things in future.
>>
> 
> So we basically want an init() callback even in sdhci.c.
> 
> I have to create one more wrapper sdhci_pltaform_init() API in the sdhci
> driver that just calls a platform init function inside it. So its

Yes, I checked wrong sequence. Sorry.

When i have checked, some functions are needs.

> 
> include/sdhci.h:
> 
> struct sdhci_ops {
> ..
> +       int (*platform_init)()
> 
> and then drivers/mmc/sdhci.c:
> 
> 
> +sdhci_platform_init()
> +{
> +...
> +       host->ops->platform_init();
> +}
> 
> const struct dm_mmc_ops sdhci_ops  = {
> ...
> +       .init           = sdhci_platform_init,
> };
> 
> Right?

Right. but not init. Just platform_init?

Well, if you want, i will make patch about callback function.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Faiz
> 
> 

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Jaehoon Chung]

Hi Faiz,

On 2/18/20 7:35 AM, Jaehoon Chung wrote:
> Hi Faiz,
> 
> On 2/17/20 9:42 PM, Faiz Abbas wrote:
>> Jaehoon,
>>
>> On 17/02/20 5:47 pm, Jaehoon Chung wrote:
>>> Hi,
>>>
>>> On 2/5/20 4:33 PM, Faiz Abbas wrote:
>>>> Hi Peng,
>>>>
>>>> On 05/02/20 12:58 pm, Peng Fan wrote:
>>>>>> Subject: Re: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 31/01/20 3:55 am, Simon Goldschmidt wrote:
>>>>>>> Am 30.01.2020 um 23:21 schrieb Jaehoon Chung:
>>>>>>>> Hi Simon,
>>>>>>>>
>>>>>>>> On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
>>>>>>>>> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung
>>>>>>>>> <jh80.chung@samsung.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>>>>>>>>>>> Expose sdhci_init() as non-static.
>>>>>>>>>>
>>>>>>>>>> Does it need to change to non-static?
>>>>>>>>>
>>>>>>>>> And even if it needs to, can we have a reason *why* in the commit
>>>>>>>>> message?
>>>>>>>>
>>>>>>>> When i read entire your series, it seems that doesn't need to change
>>>>>>>> to non-static.
>>>>>>>> All of change that touched MMC code are only for your board.
>>>>>>>> I don't know Peng's opinion, but it's not my prefer.
>>>>>>>
>>>>>>> +1!
>>>>>>>
>>>>>>> We need to keep the core code clean of such hacks in order to keep the
>>>>>>> size small for constrained targets!
>>>>>>>
>>>>>>
>>>>>> Peng can you comment on this?
>>>>>
>>>>> Just one more question, does kernel has same issue, how resolved?
>>>>> Could U-Boot follow similar approach?
>>>>
>>>> Kernel has interrupts enabled. So software just has to wait for the card
>>>> detect interrupt to arrive rather than poll on anything.
>>>>
>>>>>
>>>>> Actually I am fine with platform specific approach , considering
>>>>> your platform issue.
>>>>>
>>>>> But since Simon and Jaehoon has concerns, not sure whether
>>>>> Simon and Jaehoon have good ideas.
>>>>>
>>>>
>>>> Ok. Lets see if they have some better ideas.
>>>
>>> Well, Your code needs to call am654_sdhci_init() before sdhci_init(), right?
>>>
>>> ops->init() -> am654_sdhci_init -> sdhci_init().
>>> If am654_sdhci_init is called for preset, how about adding pre_init() or other ops callback.
>>> (pre_init is just example.)
>>>
>>> ops->pre_init = am654_sdhci_init; or ops->preset = am654_sdhci_preset;
>>>
>>> In mmc.
>>>
>>> ops->preset or ops->pre_init  -> ops->init 
>>>
>>> How about adding plotform specific init callback? Then someone can also use it for platform specific things in future.
>>>
>>
>> So we basically want an init() callback even in sdhci.c.
>>
>> I have to create one more wrapper sdhci_pltaform_init() API in the sdhci
>> driver that just calls a platform init function inside it. So its
> 
> Yes, I checked wrong sequence. Sorry.
> 
> When i have checked, some functions are needs.
> 
>>
>> include/sdhci.h:
>>
>> struct sdhci_ops {
>> ..
>> +       int (*platform_init)()
>>
>> and then drivers/mmc/sdhci.c:
>>
>>
>> +sdhci_platform_init()
>> +{
>> +...
>> +       host->ops->platform_init();
>> +}
>>
>> const struct dm_mmc_ops sdhci_ops  = {
>> ...
>> +       .init           = sdhci_platform_init,
>> };
>>
>> Right?
> 
> Right. but not init. Just platform_init?
> 
> Well, if you want, i will make patch about callback function.

How about below codes? Then you can just add am654_sdhci_deferred_probe { ... return sdhci_probe() .. }


diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 0b90a97650..c75892a72c 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -138,6 +138,21 @@ int mmc_host_power_cycle(struct mmc *mmc)
 	return dm_mmc_host_power_cycle(mmc->dev);
 }
 
+int dm_mmc_deferred_probe(struct udevice *dev)
+{
+	struct dm_mmc_ops *ops = mmc_get_ops(dev);
+
+	if (ops->deferred_probe)
+		return ops->deferred_probe(dev);
+
+	return 0;
+}
+
+int mmc_deferred_probe(struct mmc *mmc)
+{
+	return dm_mmc_deferred_probe(mmc->dev);
+}
+
 int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
 {
 	int val;
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index d43983d4a6..9eae538af4 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -2790,6 +2790,7 @@ int mmc_get_op_cond(struct mmc *mmc)
 
 #if CONFIG_IS_ENABLED(DM_MMC)
 	/* The device has already been probed ready for use */
+	mmc_deferred_probe(mmc);
 #else
 	/* made sure it's not NULL earlier */
 	err = mmc->cfg->ops->init(mmc);
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 01fa5a9d4d..9ff37b888b 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -661,6 +661,20 @@ int sdhci_probe(struct udevice *dev)
 	return sdhci_init(mmc);
 }
 
+static int sdhci_deferred_probe(struct udevice *dev)
+{
+	int err;
+	struct mmc *mmc = mmc_get_mmc_dev(dev);
+	struct sdhci_host *host = mmc->priv;
+
+	if (host->ops && host->ops->deferred_probe) {
+		err = host->ops->deferred_probe(host);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
 static int sdhci_get_cd(struct udevice *dev)
 {
 	struct mmc *mmc = mmc_get_mmc_dev(dev);
@@ -695,6 +709,7 @@ const struct dm_mmc_ops sdhci_ops = {
 	.send_cmd	= sdhci_send_command,
 	.set_ios	= sdhci_set_ios,
 	.get_cd		= sdhci_get_cd,
+	.deferred_probe	= sdhci_deferred_probe,
 #ifdef MMC_SUPPORTS_TUNING
 	.execute_tuning	= sdhci_execute_tuning,
 #endif
diff --git a/include/mmc.h b/include/mmc.h
index b5cb514f57..b362b7f4c7 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -477,6 +477,8 @@ struct dm_mmc_ops {
 	 * @return 0 if not present, 1 if present, -ve on error
 	 */
 	int (*host_power_cycle)(struct udevice *dev);
+
+	int (*deferred_probe)(struct udevice *dev);
 };
 
 #define mmc_get_ops(dev)        ((struct dm_mmc_ops *)(dev)->driver->ops)
@@ -489,6 +491,7 @@ int dm_mmc_get_wp(struct udevice *dev);
 int dm_mmc_execute_tuning(struct udevice *dev, uint opcode);
 int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us);
 int dm_mmc_host_power_cycle(struct udevice *dev);
+int dm_mmc_deferred_probe(struct udevice *dev);
 
 /* Transition functions for compatibility */
 int mmc_set_ios(struct mmc *mmc);
@@ -498,6 +501,7 @@ int mmc_execute_tuning(struct mmc *mmc, uint opcode);
 int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us);
 int mmc_set_enhanced_strobe(struct mmc *mmc);
 int mmc_host_power_cycle(struct mmc *mmc);
+int mmc_deferred_probe(struct mmc *mmc);
 
 #else
 struct mmc_ops {
diff --git a/include/sdhci.h b/include/sdhci.h
index 01addb7a60..1276f43935 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -267,6 +267,7 @@ struct sdhci_ops {
 	void	(*set_clock)(struct sdhci_host *host, u32 div);
 	int (*platform_execute_tuning)(struct mmc *host, u8 opcode);
 	void (*set_delay)(struct sdhci_host *host);
+	int	(*deferred_probe)(struct sdhci_host *host);
 };
 
 #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)

> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Thanks,
>> Faiz
>>
>>
> 
> 
> 

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Faiz Abbas]

Jaehoon,

On 18/02/20 4:54 am, Jaehoon Chung wrote:
> Hi Faiz,
> 
> On 2/18/20 7:35 AM, Jaehoon Chung wrote:
>> Hi Faiz,
>>
>> On 2/17/20 9:42 PM, Faiz Abbas wrote:
>>> Jaehoon,
>>>
>>> On 17/02/20 5:47 pm, Jaehoon Chung wrote:
>>>> Hi,
>>>>
>>>> On 2/5/20 4:33 PM, Faiz Abbas wrote:
>>>>> Hi Peng,
>>>>>
>>>>> On 05/02/20 12:58 pm, Peng Fan wrote:
>>>>>>> Subject: Re: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static
>>>>>>>
...
>>
>> Well, if you want, i will make patch about callback function.
> 
> How about below codes? Then you can just add am654_sdhci_deferred_probe { ... return sdhci_probe() .. }
> 
> 
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 0b90a97650..c75892a72c 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -138,6 +138,21 @@ int mmc_host_power_cycle(struct mmc *mmc)
>  	return dm_mmc_host_power_cycle(mmc->dev);
>  }
>  
> +int dm_mmc_deferred_probe(struct udevice *dev)
> +{
> +	struct dm_mmc_ops *ops = mmc_get_ops(dev);
> +
> +	if (ops->deferred_probe)
> +		return ops->deferred_probe(dev);
> +
> +	return 0;
> +}
> +
> +int mmc_deferred_probe(struct mmc *mmc)
> +{
> +	return dm_mmc_deferred_probe(mmc->dev);
> +}
> +
>  int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
>  {
>  	int val;
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index d43983d4a6..9eae538af4 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -2790,6 +2790,7 @@ int mmc_get_op_cond(struct mmc *mmc)
>  
>  #if CONFIG_IS_ENABLED(DM_MMC)
>  	/* The device has already been probed ready for use */
> +	mmc_deferred_probe(mmc);
>  #else
>  	/* made sure it's not NULL earlier */
>  	err = mmc->cfg->ops->init(mmc);
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 01fa5a9d4d..9ff37b888b 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -661,6 +661,20 @@ int sdhci_probe(struct udevice *dev)
>  	return sdhci_init(mmc);
>  }
>  
> +static int sdhci_deferred_probe(struct udevice *dev)
> +{
> +	int err;
> +	struct mmc *mmc = mmc_get_mmc_dev(dev);
> +	struct sdhci_host *host = mmc->priv;
> +
> +	if (host->ops && host->ops->deferred_probe) {
> +		err = host->ops->deferred_probe(host);
> +		if (err)
> +			return err;
> +	}
> +	return 0;
> +}
> +
>  static int sdhci_get_cd(struct udevice *dev)
>  {
>  	struct mmc *mmc = mmc_get_mmc_dev(dev);
> @@ -695,6 +709,7 @@ const struct dm_mmc_ops sdhci_ops = {
>  	.send_cmd	= sdhci_send_command,
>  	.set_ios	= sdhci_set_ios,
>  	.get_cd		= sdhci_get_cd,
> +	.deferred_probe	= sdhci_deferred_probe,
>  #ifdef MMC_SUPPORTS_TUNING
>  	.execute_tuning	= sdhci_execute_tuning,
>  #endif
> diff --git a/include/mmc.h b/include/mmc.h
> index b5cb514f57..b362b7f4c7 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -477,6 +477,8 @@ struct dm_mmc_ops {
>  	 * @return 0 if not present, 1 if present, -ve on error
>  	 */
>  	int (*host_power_cycle)(struct udevice *dev);
> +
> +	int (*deferred_probe)(struct udevice *dev);
>  };
>  
>  #define mmc_get_ops(dev)        ((struct dm_mmc_ops *)(dev)->driver->ops)
> @@ -489,6 +491,7 @@ int dm_mmc_get_wp(struct udevice *dev);
>  int dm_mmc_execute_tuning(struct udevice *dev, uint opcode);
>  int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us);
>  int dm_mmc_host_power_cycle(struct udevice *dev);
> +int dm_mmc_deferred_probe(struct udevice *dev);
>  
>  /* Transition functions for compatibility */
>  int mmc_set_ios(struct mmc *mmc);
> @@ -498,6 +501,7 @@ int mmc_execute_tuning(struct mmc *mmc, uint opcode);
>  int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us);
>  int mmc_set_enhanced_strobe(struct mmc *mmc);
>  int mmc_host_power_cycle(struct mmc *mmc);
> +int mmc_deferred_probe(struct mmc *mmc);
>  
>  #else
>  struct mmc_ops {
> diff --git a/include/sdhci.h b/include/sdhci.h
> index 01addb7a60..1276f43935 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -267,6 +267,7 @@ struct sdhci_ops {
>  	void	(*set_clock)(struct sdhci_host *host, u32 div);
>  	int (*platform_execute_tuning)(struct mmc *host, u8 opcode);
>  	void (*set_delay)(struct sdhci_host *host);
> +	int	(*deferred_probe)(struct sdhci_host *host);
>  };
>  
>  #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> 
>>

This does look like the cleanest solution. Will add in a v3.

Thanks,
Faiz

[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Jaehoon Chung]

On 2/18/20 4:51 PM, Faiz Abbas wrote:
> Jaehoon,
> 
> On 18/02/20 4:54 am, Jaehoon Chung wrote:
>> Hi Faiz,
>>
>> On 2/18/20 7:35 AM, Jaehoon Chung wrote:
>>> Hi Faiz,
>>>
>>> On 2/17/20 9:42 PM, Faiz Abbas wrote:
>>>> Jaehoon,
>>>>
>>>> On 17/02/20 5:47 pm, Jaehoon Chung wrote:
>>>>> Hi,
>>>>>
>>>>> On 2/5/20 4:33 PM, Faiz Abbas wrote:
>>>>>> Hi Peng,
>>>>>>
>>>>>> On 05/02/20 12:58 pm, Peng Fan wrote:
>>>>>>>> Subject: Re: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static
>>>>>>>>
> ...
>>>
>>> Well, if you want, i will make patch about callback function.
>>
>> How about below codes? Then you can just add am654_sdhci_deferred_probe { ... return sdhci_probe() .. }
>>
>>
>> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
>> index 0b90a97650..c75892a72c 100644
>> --- a/drivers/mmc/mmc-uclass.c
>> +++ b/drivers/mmc/mmc-uclass.c
>> @@ -138,6 +138,21 @@ int mmc_host_power_cycle(struct mmc *mmc)
>>  	return dm_mmc_host_power_cycle(mmc->dev);
>>  }
>>  
>> +int dm_mmc_deferred_probe(struct udevice *dev)
>> +{
>> +	struct dm_mmc_ops *ops = mmc_get_ops(dev);
>> +
>> +	if (ops->deferred_probe)
>> +		return ops->deferred_probe(dev);
>> +
>> +	return 0;
>> +}
>> +
>> +int mmc_deferred_probe(struct mmc *mmc)
>> +{
>> +	return dm_mmc_deferred_probe(mmc->dev);
>> +}
>> +
>>  int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
>>  {
>>  	int val;
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index d43983d4a6..9eae538af4 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -2790,6 +2790,7 @@ int mmc_get_op_cond(struct mmc *mmc)
>>  
>>  #if CONFIG_IS_ENABLED(DM_MMC)
>>  	/* The device has already been probed ready for use */
>> +	mmc_deferred_probe(mmc);
>>  #else
>>  	/* made sure it's not NULL earlier */
>>  	err = mmc->cfg->ops->init(mmc);
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 01fa5a9d4d..9ff37b888b 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -661,6 +661,20 @@ int sdhci_probe(struct udevice *dev)
>>  	return sdhci_init(mmc);
>>  }
>>  
>> +static int sdhci_deferred_probe(struct udevice *dev)
>> +{
>> +	int err;
>> +	struct mmc *mmc = mmc_get_mmc_dev(dev);
>> +	struct sdhci_host *host = mmc->priv;
>> +
>> +	if (host->ops && host->ops->deferred_probe) {
>> +		err = host->ops->deferred_probe(host);
>> +		if (err)
>> +			return err;
>> +	}
>> +	return 0;
>> +}
>> +
>>  static int sdhci_get_cd(struct udevice *dev)
>>  {
>>  	struct mmc *mmc = mmc_get_mmc_dev(dev);
>> @@ -695,6 +709,7 @@ const struct dm_mmc_ops sdhci_ops = {
>>  	.send_cmd	= sdhci_send_command,
>>  	.set_ios	= sdhci_set_ios,
>>  	.get_cd		= sdhci_get_cd,
>> +	.deferred_probe	= sdhci_deferred_probe,
>>  #ifdef MMC_SUPPORTS_TUNING
>>  	.execute_tuning	= sdhci_execute_tuning,
>>  #endif
>> diff --git a/include/mmc.h b/include/mmc.h
>> index b5cb514f57..b362b7f4c7 100644
>> --- a/include/mmc.h
>> +++ b/include/mmc.h
>> @@ -477,6 +477,8 @@ struct dm_mmc_ops {
>>  	 * @return 0 if not present, 1 if present, -ve on error
>>  	 */
>>  	int (*host_power_cycle)(struct udevice *dev);
>> +
>> +	int (*deferred_probe)(struct udevice *dev);
>>  };
>>  
>>  #define mmc_get_ops(dev)        ((struct dm_mmc_ops *)(dev)->driver->ops)
>> @@ -489,6 +491,7 @@ int dm_mmc_get_wp(struct udevice *dev);
>>  int dm_mmc_execute_tuning(struct udevice *dev, uint opcode);
>>  int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us);
>>  int dm_mmc_host_power_cycle(struct udevice *dev);
>> +int dm_mmc_deferred_probe(struct udevice *dev);
>>  
>>  /* Transition functions for compatibility */
>>  int mmc_set_ios(struct mmc *mmc);
>> @@ -498,6 +501,7 @@ int mmc_execute_tuning(struct mmc *mmc, uint opcode);
>>  int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us);
>>  int mmc_set_enhanced_strobe(struct mmc *mmc);
>>  int mmc_host_power_cycle(struct mmc *mmc);
>> +int mmc_deferred_probe(struct mmc *mmc);
>>  
>>  #else
>>  struct mmc_ops {
>> diff --git a/include/sdhci.h b/include/sdhci.h
>> index 01addb7a60..1276f43935 100644
>> --- a/include/sdhci.h
>> +++ b/include/sdhci.h
>> @@ -267,6 +267,7 @@ struct sdhci_ops {
>>  	void	(*set_clock)(struct sdhci_host *host, u32 div);
>>  	int (*platform_execute_tuning)(struct mmc *host, u8 opcode);
>>  	void (*set_delay)(struct sdhci_host *host);
>> +	int	(*deferred_probe)(struct sdhci_host *host);
>>  };
>>  
>>  #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
>>
>>>
> 
> This does look like the cleanest solution. Will add in a v3.
Above code is for just concept. Maybe you need to add/modify code.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Faiz
> 
> 
>