All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Update phy configuration for AM65x
@ 2020-01-08 15:09 Faiz Abbas
  2020-01-08 15:09 ` [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding Faiz Abbas
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Faiz Abbas @ 2020-01-08 15:09 UTC (permalink / raw)
  To: linux-kernel, linux-mmc, devicetree
  Cc: ulf.hansson, adrian.hunter, faiz_abbas, robh+dt

The following patches update phy configurations for AM65x as given in
the latest data manual.

The patches depend on my fixes series posted just before this:
https://patchwork.kernel.org/project/linux-mmc/list/?series=225425

Device tree patch updating the actual otap values will be posted
separately.

Tested with Am65x-evm and J721e-evm.

Faiz Abbas (3):
  dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
  mmc: sdhci_am654: Update OTAPDLY writes
  mmc: sdhci_am654: Enable DLL only for some speed modes

 .../devicetree/bindings/mmc/sdhci-am654.txt   |  21 +-
 drivers/mmc/host/sdhci_am654.c                | 247 ++++++++++++------
 include/linux/mmc/host.h                      |   2 +
 3 files changed, 192 insertions(+), 78 deletions(-)

-- 
2.19.2


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

* [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
  2020-01-08 15:09 [PATCH 0/3] Update phy configuration for AM65x Faiz Abbas
@ 2020-01-08 15:09 ` Faiz Abbas
  2020-01-15  1:50   ` Rob Herring
  2020-01-08 15:09 ` [PATCH 2/3] mmc: sdhci_am654: Update OTAPDLY writes Faiz Abbas
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Faiz Abbas @ 2020-01-08 15:09 UTC (permalink / raw)
  To: linux-kernel, linux-mmc, devicetree
  Cc: ulf.hansson, adrian.hunter, faiz_abbas, robh+dt

According to latest AM65x Data Manual[1], a different output tap delay
value is recommended for all speed modes. Therefore, replace the
ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
mode.

[1] http://www.ti.com/lit/gpn/am6526

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 .../devicetree/bindings/mmc/sdhci-am654.txt   | 21 +++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
index 50e87df47971..c6ccecb9ae5a 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
@@ -18,7 +18,20 @@ Required Properties:
 	- clocks: Handles to the clock inputs.
 	- clock-names: Tuple including "clk_xin" and "clk_ahb"
 	- interrupts: Interrupt specifiers
-	- ti,otap-del-sel: Output Tap Delay select
+	Output tap delay for each speed mode:
+	- ti,otap-del-sel-legacy
+	- ti,otap-del-sel-mmc-hs
+	- ti,otap-del-sel-sd-hs
+	- ti,otap-del-sel-sdr12
+	- ti,otap-del-sel-sdr25
+	- ti,otap-del-sel-sdr50
+	- ti,otap-del-sel-sdr104
+	- ti,otap-del-sel-ddr50
+	- ti,otap-del-sel-ddr52
+	- ti,otap-del-sel-hs200
+	- ti,otap-del-sel-hs400
+	  These bindings must be provided otherwise the driver will disable the
+	  corresponding speed mode (i.e. all nodes must provide at least -legacy)
 
 Optional Properties (Required for ti,am654-sdhci-5.1 and ti,j721e-sdhci-8bit):
 	- ti,trm-icp: DLL trim select
@@ -38,6 +51,10 @@ Example:
 		interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
 		sdhci-caps-mask = <0x80000007 0x0>;
 		mmc-ddr-1_8v;
-		ti,otap-del-sel = <0x2>;
+		ti,otap-del-sel-legacy = <0x0>;
+		ti,otap-del-sel-mmc-hs = <0x0>;
+		ti,otap-del-sel-ddr52 = <0x5>;
+		ti,otap-del-sel-hs200 = <0x5>;
+		ti,otap-del-sel-hs400 = <0x0>;
 		ti,trm-icp = <0x8>;
 	};
-- 
2.19.2


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

* [PATCH 2/3] mmc: sdhci_am654: Update OTAPDLY writes
  2020-01-08 15:09 [PATCH 0/3] Update phy configuration for AM65x Faiz Abbas
  2020-01-08 15:09 ` [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding Faiz Abbas
@ 2020-01-08 15:09 ` Faiz Abbas
  2020-01-20 12:31   ` Adrian Hunter
  2020-01-08 15:09 ` [PATCH 3/3] mmc: sdhci_am654: Enable DLL only for some speed modes Faiz Abbas
  2020-03-02 19:11 ` [PATCH 0/3] Update phy configuration for AM65x Faiz Abbas
  3 siblings, 1 reply; 14+ messages in thread
From: Faiz Abbas @ 2020-01-08 15:09 UTC (permalink / raw)
  To: linux-kernel, linux-mmc, devicetree
  Cc: ulf.hansson, adrian.hunter, faiz_abbas, robh+dt

According to the latest AM65x Data Manual[1], a different output tap
delay value is optimum for a given speed mode. Therefore, deprecate the
ti,otap-del-sel binding and introduce a new binding for each of the
possible MMC/SD speed modes. If the legacy mode is not found, fall back
to old binding to maintain dts compatibility.

[1] http://www.ti.com/lit/gpn/am6526

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 drivers/mmc/host/sdhci_am654.c | 123 ++++++++++++++++++++++++++++-----
 include/linux/mmc/host.h       |   2 +
 2 files changed, 107 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index b8fe94fd9525..bb977de43f7d 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -81,7 +81,8 @@ static struct regmap_config sdhci_am654_regmap_config = {
 
 struct sdhci_am654_data {
 	struct regmap *base;
-	int otap_del_sel;
+	bool legacy_otapdly;
+	int otap_del_sel[11];
 	int trm_icp;
 	int drv_strength;
 	bool dll_on;
@@ -98,11 +99,34 @@ struct sdhci_am654_driver_data {
 #define DLL_PRESENT	(1 << 3)
 };
 
+struct timing_data {
+	const char *binding;
+	u32 capability;
+};
+
+static const struct timing_data td[] = {
+	[MMC_TIMING_LEGACY] = {"ti,otap-del-sel-legacy", 0},
+	[MMC_TIMING_MMC_HS] = {"ti,otap-del-sel-mmc-hs", MMC_CAP_MMC_HIGHSPEED},
+	[MMC_TIMING_SD_HS]  = {"ti,otap-del-sel-sd-hs", MMC_CAP_SD_HIGHSPEED},
+	[MMC_TIMING_UHS_SDR12] = {"ti,otap-del-sel-sdr12", MMC_CAP_UHS_SDR12},
+	[MMC_TIMING_UHS_SDR25] = {"ti,otap-del-sel-sdr25", MMC_CAP_UHS_SDR25},
+	[MMC_TIMING_UHS_SDR50] = {"ti,otap-del-sel-sdr50", MMC_CAP_UHS_SDR50},
+	[MMC_TIMING_UHS_SDR104] = {"ti,otap-del-sel-sdr104",
+				   MMC_CAP_UHS_SDR104},
+	[MMC_TIMING_UHS_DDR50] = {"ti,otap-del-sel-ddr50", MMC_CAP_UHS_DDR50},
+	[MMC_TIMING_MMC_DDR52] = {"ti,otap-del-sel-ddr52", MMC_CAP_DDR},
+	[MMC_TIMING_MMC_HS200] = {"ti,otap-del-sel-hs200", MMC_CAP2_HS200},
+	[MMC_TIMING_MMC_HS400] = {"ti,otap-del-sel-hs400", MMC_CAP2_HS400},
+};
+
 static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
+	unsigned char timing = host->mmc->ios.timing;
 	int sel50, sel100, freqsel;
+	u32 otap_del_sel;
+	u32 otap_del_ena;
 	u32 mask, val;
 	int ret;
 
@@ -116,22 +140,29 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
 
 	if (clock > CLOCK_TOO_SLOW_HZ) {
 		/* Setup DLL Output TAP delay */
+		if (sdhci_am654->legacy_otapdly)
+			otap_del_sel = sdhci_am654->otap_del_sel[0];
+		else
+			otap_del_sel = sdhci_am654->otap_del_sel[timing];
+
+		otap_del_ena = (timing > MMC_TIMING_UHS_SDR25) ? 1 : 0;
+
 		mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
-		val = (1 << OTAPDLYENA_SHIFT) |
-		      (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
-		regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
+		val = (otap_del_ena << OTAPDLYENA_SHIFT) |
+		      (otap_del_sel << OTAPDLYSEL_SHIFT);
+
 		/* Write to STRBSEL for HS400 speed mode */
-		if (host->mmc->ios.timing == MMC_TIMING_MMC_HS400) {
+		if (timing == MMC_TIMING_MMC_HS400) {
 			if (sdhci_am654->flags & STRBSEL_4_BIT)
-				mask = STRBSEL_4BIT_MASK;
+				mask |= STRBSEL_4BIT_MASK;
 			else
-				mask = STRBSEL_8BIT_MASK;
+				mask |= STRBSEL_8BIT_MASK;
 
-			regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask,
-					   sdhci_am654->strb_sel <<
-					   STRBSEL_SHIFT);
+			val |= sdhci_am654->strb_sel << STRBSEL_SHIFT;
 		}
 
+		regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
+
 		if (sdhci_am654->flags & FREQSEL_2_BIT) {
 			switch (clock) {
 			case 200000000:
@@ -198,11 +229,19 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
-	int val, mask;
+	unsigned char timing = host->mmc->ios.timing;
+	u32 otap_del_sel;
+	u32 mask, val;
+
+	/* Setup DLL Output TAP delay */
+	if (sdhci_am654->legacy_otapdly)
+		otap_del_sel = sdhci_am654->otap_del_sel[0];
+	else
+		otap_del_sel = sdhci_am654->otap_del_sel[timing];
 
 	mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
-	val = (1 << OTAPDLYENA_SHIFT) |
-	      (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
+	val = (0x1 << OTAPDLYENA_SHIFT) |
+	      (otap_del_sel << OTAPDLYSEL_SHIFT);
 	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
 
 	sdhci_set_clock(host, clock);
@@ -371,6 +410,55 @@ static int sdhci_am654_cqe_add_host(struct sdhci_host *host)
 	return ret;
 }
 
+static int sdhci_am654_get_otap_delay(struct sdhci_host *host,
+				      struct sdhci_am654_data *sdhci_am654)
+{
+	struct device *dev = mmc_dev(host->mmc);
+	int i;
+	int ret;
+
+	ret = device_property_read_u32(dev, td[MMC_TIMING_LEGACY].binding,
+				 &sdhci_am654->otap_del_sel[MMC_TIMING_LEGACY]);
+	if (ret) {
+		/*
+		 * ti,otap-del-sel-legacy is mandatory, look for old binding
+		 * if not found.
+		 */
+		ret = device_property_read_u32(dev, "ti,otap-del-sel",
+					       &sdhci_am654->otap_del_sel[0]);
+		if (ret) {
+			dev_err(dev, "Couldn't find otap-del-sel\n");
+
+			return ret;
+		}
+
+		dev_info(dev, "Using legacy binding ti,otap-del-sel\n");
+		sdhci_am654->legacy_otapdly = true;
+
+		return 0;
+	}
+
+	for (i = MMC_TIMING_MMC_HS; i <= MMC_TIMING_MMC_HS400; i++) {
+
+		ret = device_property_read_u32(dev, td[i].binding,
+					       &sdhci_am654->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
+			 */
+			if (i <= MMC_TIMING_MMC_DDR52)
+				host->mmc->caps &= ~td[i].capability;
+			else
+				host->mmc->caps2 &= ~td[i].capability;
+		}
+	}
+
+	return 0;
+}
+
 static int sdhci_am654_init(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -419,6 +507,10 @@ static int sdhci_am654_init(struct sdhci_host *host)
 	if (ret)
 		goto err_cleanup_host;
 
+	ret = sdhci_am654_get_otap_delay(host, sdhci_am654);
+	if (ret)
+		goto err_cleanup_host;
+
 	ret = __sdhci_add_host(host);
 	if (ret)
 		goto err_cleanup_host;
@@ -437,11 +529,6 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
 	int drv_strength;
 	int ret;
 
-	ret = device_property_read_u32(dev, "ti,otap-del-sel",
-				       &sdhci_am654->otap_del_sel);
-	if (ret)
-		return ret;
-
 	if (sdhci_am654->flags & DLL_PRESENT) {
 		ret = device_property_read_u32(dev, "ti,trm-icp",
 					       &sdhci_am654->trm_icp);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index ba703384bea0..a22a10456c62 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -322,6 +322,8 @@ struct mmc_host {
 #define MMC_CAP_3_3V_DDR	(1 << 11)	/* Host supports eMMC DDR 3.3V */
 #define MMC_CAP_1_8V_DDR	(1 << 12)	/* Host supports eMMC DDR 1.8V */
 #define MMC_CAP_1_2V_DDR	(1 << 13)	/* Host supports eMMC DDR 1.2V */
+#define MMC_CAP_DDR		(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR | \
+				 MMC_CAP_1_2V_DDR)
 #define MMC_CAP_POWER_OFF_CARD	(1 << 14)	/* Can power off after boot */
 #define MMC_CAP_BUS_WIDTH_TEST	(1 << 15)	/* CMD14/CMD19 bus width ok */
 #define MMC_CAP_UHS_SDR12	(1 << 16)	/* Host supports UHS SDR12 mode */
-- 
2.19.2


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

* [PATCH 3/3] mmc: sdhci_am654: Enable DLL only for some speed modes
  2020-01-08 15:09 [PATCH 0/3] Update phy configuration for AM65x Faiz Abbas
  2020-01-08 15:09 ` [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding Faiz Abbas
  2020-01-08 15:09 ` [PATCH 2/3] mmc: sdhci_am654: Update OTAPDLY writes Faiz Abbas
@ 2020-01-08 15:09 ` Faiz Abbas
  2020-01-20 12:32   ` Adrian Hunter
  2020-03-02 19:11 ` [PATCH 0/3] Update phy configuration for AM65x Faiz Abbas
  3 siblings, 1 reply; 14+ messages in thread
From: Faiz Abbas @ 2020-01-08 15:09 UTC (permalink / raw)
  To: linux-kernel, linux-mmc, devicetree
  Cc: ulf.hansson, adrian.hunter, faiz_abbas, robh+dt

Its recommended that DLL must only be enabled for SDR50, DDR50, DDR52,
SDR104, HS200 and HS400 speed modes. Move DLL configuration to its own
function and call it only in the above speed modes.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 drivers/mmc/host/sdhci_am654.c | 128 +++++++++++++++++----------------
 1 file changed, 68 insertions(+), 60 deletions(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index bb977de43f7d..575bbab1a6ed 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -119,16 +119,80 @@ static const struct timing_data td[] = {
 	[MMC_TIMING_MMC_HS400] = {"ti,otap-del-sel-hs400", MMC_CAP2_HS400},
 };
 
+static void sdhci_am654_setup_dll(struct sdhci_host *host, unsigned int clock)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
+	int sel50, sel100, freqsel;
+	u32 mask, val;
+	int ret;
+
+	if (sdhci_am654->flags & FREQSEL_2_BIT) {
+		switch (clock) {
+		case 200000000:
+			sel50 = 0;
+			sel100 = 0;
+			break;
+		case 100000000:
+			sel50 = 0;
+			sel100 = 1;
+			break;
+		default:
+			sel50 = 1;
+			sel100 = 0;
+		}
+
+		/* Configure PHY DLL frequency */
+		mask = SEL50_MASK | SEL100_MASK;
+		val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
+		regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask, val);
+
+	} else {
+		switch (clock) {
+		case 200000000:
+			freqsel = 0x0;
+			break;
+		default:
+			freqsel = 0x4;
+		}
+
+		regmap_update_bits(sdhci_am654->base, PHY_CTRL5, FREQSEL_MASK,
+				   freqsel << FREQSEL_SHIFT);
+	}
+	/* Configure DLL TRIM */
+	mask = DLL_TRIM_ICP_MASK;
+	val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
+
+	/* Configure DLL driver strength */
+	mask |= DR_TY_MASK;
+	val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
+	regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val);
+
+	/* Enable DLL */
+	regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK,
+			   0x1 << ENDLL_SHIFT);
+	/*
+	 * Poll for DLL ready. Use a one second timeout.
+	 * Works in all experiments done so far
+	 */
+	ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1, val,
+				       val & DLLRDY_MASK, 1000, 1000000);
+	if (ret) {
+		dev_err(mmc_dev(host->mmc), "DLL failed to relock\n");
+		return;
+	}
+
+	sdhci_am654->dll_on = true;
+}
+
 static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
 	unsigned char timing = host->mmc->ios.timing;
-	int sel50, sel100, freqsel;
 	u32 otap_del_sel;
 	u32 otap_del_ena;
 	u32 mask, val;
-	int ret;
 
 	if (sdhci_am654->dll_on) {
 		regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK, 0);
@@ -163,64 +227,8 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
 
 		regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
 
-		if (sdhci_am654->flags & FREQSEL_2_BIT) {
-			switch (clock) {
-			case 200000000:
-				sel50 = 0;
-				sel100 = 0;
-				break;
-			case 100000000:
-				sel50 = 0;
-				sel100 = 1;
-				break;
-			default:
-				sel50 = 1;
-				sel100 = 0;
-			}
-
-			/* Configure PHY DLL frequency */
-			mask = SEL50_MASK | SEL100_MASK;
-			val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
-			regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask,
-					   val);
-		} else {
-			switch (clock) {
-			case 200000000:
-				freqsel = 0x0;
-				break;
-			default:
-				freqsel = 0x4;
-			}
-
-			regmap_update_bits(sdhci_am654->base, PHY_CTRL5,
-					   FREQSEL_MASK,
-					   freqsel << FREQSEL_SHIFT);
-		}
-
-		/* Configure DLL TRIM */
-		mask = DLL_TRIM_ICP_MASK;
-		val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
-
-		/* Configure DLL driver strength */
-		mask |= DR_TY_MASK;
-		val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
-		regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val);
-		/* Enable DLL */
-		regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK,
-				   0x1 << ENDLL_SHIFT);
-		/*
-		 * Poll for DLL ready. Use a one second timeout.
-		 * Works in all experiments done so far
-		 */
-		ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1,
-					       val, val & DLLRDY_MASK, 1000,
-					       1000000);
-		if (ret) {
-			dev_err(mmc_dev(host->mmc), "DLL failed to relock\n");
-			return;
-		}
-
-		sdhci_am654->dll_on = true;
+		if (timing > MMC_TIMING_UHS_SDR25)
+			sdhci_am654_setup_dll(host, clock);
 	}
 }
 
-- 
2.19.2


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

* Re: [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
  2020-01-08 15:09 ` [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding Faiz Abbas
@ 2020-01-15  1:50   ` Rob Herring
  2020-01-20  5:30     ` Faiz Abbas
  0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2020-01-15  1:50 UTC (permalink / raw)
  To: Faiz Abbas
  Cc: linux-kernel, linux-mmc, devicetree, ulf.hansson, adrian.hunter

On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
> According to latest AM65x Data Manual[1], a different output tap delay
> value is recommended for all speed modes. Therefore, replace the
> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
> mode.
> 
> [1] http://www.ti.com/lit/gpn/am6526
> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
>  .../devicetree/bindings/mmc/sdhci-am654.txt   | 21 +++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> index 50e87df47971..c6ccecb9ae5a 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> @@ -18,7 +18,20 @@ Required Properties:
>  	- clocks: Handles to the clock inputs.
>  	- clock-names: Tuple including "clk_xin" and "clk_ahb"
>  	- interrupts: Interrupt specifiers
> -	- ti,otap-del-sel: Output Tap Delay select
> +	Output tap delay for each speed mode:
> +	- ti,otap-del-sel-legacy
> +	- ti,otap-del-sel-mmc-hs
> +	- ti,otap-del-sel-sd-hs
> +	- ti,otap-del-sel-sdr12
> +	- ti,otap-del-sel-sdr25
> +	- ti,otap-del-sel-sdr50
> +	- ti,otap-del-sel-sdr104
> +	- ti,otap-del-sel-ddr50
> +	- ti,otap-del-sel-ddr52
> +	- ti,otap-del-sel-hs200
> +	- ti,otap-del-sel-hs400
> +	  These bindings must be provided otherwise the driver will disable the
> +	  corresponding speed mode (i.e. all nodes must provide at least -legacy)

Why not just extend the existing property to be an array. We already 
have properties to enable/disable speed modes.

>  
>  Optional Properties (Required for ti,am654-sdhci-5.1 and ti,j721e-sdhci-8bit):
>  	- ti,trm-icp: DLL trim select
> @@ -38,6 +51,10 @@ Example:
>  		interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
>  		sdhci-caps-mask = <0x80000007 0x0>;
>  		mmc-ddr-1_8v;
> -		ti,otap-del-sel = <0x2>;
> +		ti,otap-del-sel-legacy = <0x0>;
> +		ti,otap-del-sel-mmc-hs = <0x0>;
> +		ti,otap-del-sel-ddr52 = <0x5>;
> +		ti,otap-del-sel-hs200 = <0x5>;
> +		ti,otap-del-sel-hs400 = <0x0>;
>  		ti,trm-icp = <0x8>;
>  	};
> -- 
> 2.19.2
> 

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

* Re: [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
  2020-01-15  1:50   ` Rob Herring
@ 2020-01-20  5:30     ` Faiz Abbas
  2020-02-07  9:37       ` Faiz Abbas
  0 siblings, 1 reply; 14+ messages in thread
From: Faiz Abbas @ 2020-01-20  5:30 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, linux-mmc, devicetree, ulf.hansson, adrian.hunter

Hi Rob,

On 15/01/20 7:20 am, Rob Herring wrote:
> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>> According to latest AM65x Data Manual[1], a different output tap delay
>> value is recommended for all speed modes. Therefore, replace the
>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>> mode.
>>
>> [1] http://www.ti.com/lit/gpn/am6526
>>
>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>> ---
>>  .../devicetree/bindings/mmc/sdhci-am654.txt   | 21 +++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>> index 50e87df47971..c6ccecb9ae5a 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>> @@ -18,7 +18,20 @@ Required Properties:
>>  	- clocks: Handles to the clock inputs.
>>  	- clock-names: Tuple including "clk_xin" and "clk_ahb"
>>  	- interrupts: Interrupt specifiers
>> -	- ti,otap-del-sel: Output Tap Delay select
>> +	Output tap delay for each speed mode:
>> +	- ti,otap-del-sel-legacy
>> +	- ti,otap-del-sel-mmc-hs
>> +	- ti,otap-del-sel-sd-hs
>> +	- ti,otap-del-sel-sdr12
>> +	- ti,otap-del-sel-sdr25
>> +	- ti,otap-del-sel-sdr50
>> +	- ti,otap-del-sel-sdr104
>> +	- ti,otap-del-sel-ddr50
>> +	- ti,otap-del-sel-ddr52
>> +	- ti,otap-del-sel-hs200
>> +	- ti,otap-del-sel-hs400
>> +	  These bindings must be provided otherwise the driver will disable the
>> +	  corresponding speed mode (i.e. all nodes must provide at least -legacy)
> 
> Why not just extend the existing property to be an array. We already 
> have properties to enable/disable speed modes.
>

Its hard to keep track of which modes have values and which don't when
you add an array. This scheme is just easier on anyone adding new values
or updating old values.

We already disable speed modes based on platform specific properties in
other drivers. In sdhci-omap.c, the driver disables the corresponding
speed mode if the corresponding pinmux and iodelay values are not populated.

Thanks,
Faiz

Thanks,
Faiz



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

* Re: [PATCH 2/3] mmc: sdhci_am654: Update OTAPDLY writes
  2020-01-08 15:09 ` [PATCH 2/3] mmc: sdhci_am654: Update OTAPDLY writes Faiz Abbas
@ 2020-01-20 12:31   ` Adrian Hunter
  0 siblings, 0 replies; 14+ messages in thread
From: Adrian Hunter @ 2020-01-20 12:31 UTC (permalink / raw)
  To: Faiz Abbas, linux-kernel, linux-mmc, devicetree; +Cc: ulf.hansson, robh+dt

On 8/01/20 5:09 pm, Faiz Abbas wrote:
> According to the latest AM65x Data Manual[1], a different output tap
> delay value is optimum for a given speed mode. Therefore, deprecate the
> ti,otap-del-sel binding and introduce a new binding for each of the
> possible MMC/SD speed modes. If the legacy mode is not found, fall back
> to old binding to maintain dts compatibility.
> 
> [1] http://www.ti.com/lit/gpn/am6526
> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci_am654.c | 123 ++++++++++++++++++++++++++++-----
>  include/linux/mmc/host.h       |   2 +
>  2 files changed, 107 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index b8fe94fd9525..bb977de43f7d 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -81,7 +81,8 @@ static struct regmap_config sdhci_am654_regmap_config = {
>  
>  struct sdhci_am654_data {
>  	struct regmap *base;
> -	int otap_del_sel;
> +	bool legacy_otapdly;
> +	int otap_del_sel[11];
>  	int trm_icp;
>  	int drv_strength;
>  	bool dll_on;
> @@ -98,11 +99,34 @@ struct sdhci_am654_driver_data {
>  #define DLL_PRESENT	(1 << 3)
>  };
>  
> +struct timing_data {
> +	const char *binding;
> +	u32 capability;
> +};
> +
> +static const struct timing_data td[] = {
> +	[MMC_TIMING_LEGACY] = {"ti,otap-del-sel-legacy", 0},
> +	[MMC_TIMING_MMC_HS] = {"ti,otap-del-sel-mmc-hs", MMC_CAP_MMC_HIGHSPEED},
> +	[MMC_TIMING_SD_HS]  = {"ti,otap-del-sel-sd-hs", MMC_CAP_SD_HIGHSPEED},
> +	[MMC_TIMING_UHS_SDR12] = {"ti,otap-del-sel-sdr12", MMC_CAP_UHS_SDR12},
> +	[MMC_TIMING_UHS_SDR25] = {"ti,otap-del-sel-sdr25", MMC_CAP_UHS_SDR25},
> +	[MMC_TIMING_UHS_SDR50] = {"ti,otap-del-sel-sdr50", MMC_CAP_UHS_SDR50},
> +	[MMC_TIMING_UHS_SDR104] = {"ti,otap-del-sel-sdr104",
> +				   MMC_CAP_UHS_SDR104},
> +	[MMC_TIMING_UHS_DDR50] = {"ti,otap-del-sel-ddr50", MMC_CAP_UHS_DDR50},
> +	[MMC_TIMING_MMC_DDR52] = {"ti,otap-del-sel-ddr52", MMC_CAP_DDR},
> +	[MMC_TIMING_MMC_HS200] = {"ti,otap-del-sel-hs200", MMC_CAP2_HS200},
> +	[MMC_TIMING_MMC_HS400] = {"ti,otap-del-sel-hs400", MMC_CAP2_HS400},
> +};
> +
>  static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> +	unsigned char timing = host->mmc->ios.timing;
>  	int sel50, sel100, freqsel;
> +	u32 otap_del_sel;
> +	u32 otap_del_ena;
>  	u32 mask, val;
>  	int ret;
>  
> @@ -116,22 +140,29 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
>  
>  	if (clock > CLOCK_TOO_SLOW_HZ) {
>  		/* Setup DLL Output TAP delay */
> +		if (sdhci_am654->legacy_otapdly)
> +			otap_del_sel = sdhci_am654->otap_del_sel[0];
> +		else
> +			otap_del_sel = sdhci_am654->otap_del_sel[timing];
> +
> +		otap_del_ena = (timing > MMC_TIMING_UHS_SDR25) ? 1 : 0;
> +
>  		mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
> -		val = (1 << OTAPDLYENA_SHIFT) |
> -		      (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
> -		regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
> +		val = (otap_del_ena << OTAPDLYENA_SHIFT) |
> +		      (otap_del_sel << OTAPDLYSEL_SHIFT);
> +
>  		/* Write to STRBSEL for HS400 speed mode */
> -		if (host->mmc->ios.timing == MMC_TIMING_MMC_HS400) {
> +		if (timing == MMC_TIMING_MMC_HS400) {
>  			if (sdhci_am654->flags & STRBSEL_4_BIT)
> -				mask = STRBSEL_4BIT_MASK;
> +				mask |= STRBSEL_4BIT_MASK;
>  			else
> -				mask = STRBSEL_8BIT_MASK;
> +				mask |= STRBSEL_8BIT_MASK;
>  
> -			regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask,
> -					   sdhci_am654->strb_sel <<
> -					   STRBSEL_SHIFT);
> +			val |= sdhci_am654->strb_sel << STRBSEL_SHIFT;
>  		}
>  
> +		regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
> +
>  		if (sdhci_am654->flags & FREQSEL_2_BIT) {
>  			switch (clock) {
>  			case 200000000:
> @@ -198,11 +229,19 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> -	int val, mask;
> +	unsigned char timing = host->mmc->ios.timing;
> +	u32 otap_del_sel;
> +	u32 mask, val;
> +
> +	/* Setup DLL Output TAP delay */
> +	if (sdhci_am654->legacy_otapdly)
> +		otap_del_sel = sdhci_am654->otap_del_sel[0];
> +	else
> +		otap_del_sel = sdhci_am654->otap_del_sel[timing];
>  
>  	mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
> -	val = (1 << OTAPDLYENA_SHIFT) |
> -	      (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
> +	val = (0x1 << OTAPDLYENA_SHIFT) |
> +	      (otap_del_sel << OTAPDLYSEL_SHIFT);
>  	regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
>  
>  	sdhci_set_clock(host, clock);
> @@ -371,6 +410,55 @@ static int sdhci_am654_cqe_add_host(struct sdhci_host *host)
>  	return ret;
>  }
>  
> +static int sdhci_am654_get_otap_delay(struct sdhci_host *host,
> +				      struct sdhci_am654_data *sdhci_am654)
> +{
> +	struct device *dev = mmc_dev(host->mmc);
> +	int i;
> +	int ret;
> +
> +	ret = device_property_read_u32(dev, td[MMC_TIMING_LEGACY].binding,
> +				 &sdhci_am654->otap_del_sel[MMC_TIMING_LEGACY]);
> +	if (ret) {
> +		/*
> +		 * ti,otap-del-sel-legacy is mandatory, look for old binding
> +		 * if not found.
> +		 */
> +		ret = device_property_read_u32(dev, "ti,otap-del-sel",
> +					       &sdhci_am654->otap_del_sel[0]);
> +		if (ret) {
> +			dev_err(dev, "Couldn't find otap-del-sel\n");
> +
> +			return ret;
> +		}
> +
> +		dev_info(dev, "Using legacy binding ti,otap-del-sel\n");
> +		sdhci_am654->legacy_otapdly = true;
> +
> +		return 0;
> +	}
> +
> +	for (i = MMC_TIMING_MMC_HS; i <= MMC_TIMING_MMC_HS400; i++) {
> +
> +		ret = device_property_read_u32(dev, td[i].binding,
> +					       &sdhci_am654->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
> +			 */
> +			if (i <= MMC_TIMING_MMC_DDR52)
> +				host->mmc->caps &= ~td[i].capability;
> +			else
> +				host->mmc->caps2 &= ~td[i].capability;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int sdhci_am654_init(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -419,6 +507,10 @@ static int sdhci_am654_init(struct sdhci_host *host)
>  	if (ret)
>  		goto err_cleanup_host;
>  
> +	ret = sdhci_am654_get_otap_delay(host, sdhci_am654);
> +	if (ret)
> +		goto err_cleanup_host;
> +
>  	ret = __sdhci_add_host(host);
>  	if (ret)
>  		goto err_cleanup_host;
> @@ -437,11 +529,6 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
>  	int drv_strength;
>  	int ret;
>  
> -	ret = device_property_read_u32(dev, "ti,otap-del-sel",
> -				       &sdhci_am654->otap_del_sel);
> -	if (ret)
> -		return ret;
> -
>  	if (sdhci_am654->flags & DLL_PRESENT) {
>  		ret = device_property_read_u32(dev, "ti,trm-icp",
>  					       &sdhci_am654->trm_icp);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index ba703384bea0..a22a10456c62 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -322,6 +322,8 @@ struct mmc_host {
>  #define MMC_CAP_3_3V_DDR	(1 << 11)	/* Host supports eMMC DDR 3.3V */
>  #define MMC_CAP_1_8V_DDR	(1 << 12)	/* Host supports eMMC DDR 1.8V */
>  #define MMC_CAP_1_2V_DDR	(1 << 13)	/* Host supports eMMC DDR 1.2V */
> +#define MMC_CAP_DDR		(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR | \
> +				 MMC_CAP_1_2V_DDR)
>  #define MMC_CAP_POWER_OFF_CARD	(1 << 14)	/* Can power off after boot */
>  #define MMC_CAP_BUS_WIDTH_TEST	(1 << 15)	/* CMD14/CMD19 bus width ok */
>  #define MMC_CAP_UHS_SDR12	(1 << 16)	/* Host supports UHS SDR12 mode */
> 


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

* Re: [PATCH 3/3] mmc: sdhci_am654: Enable DLL only for some speed modes
  2020-01-08 15:09 ` [PATCH 3/3] mmc: sdhci_am654: Enable DLL only for some speed modes Faiz Abbas
@ 2020-01-20 12:32   ` Adrian Hunter
  0 siblings, 0 replies; 14+ messages in thread
From: Adrian Hunter @ 2020-01-20 12:32 UTC (permalink / raw)
  To: Faiz Abbas, linux-kernel, linux-mmc, devicetree; +Cc: ulf.hansson, robh+dt

On 8/01/20 5:09 pm, Faiz Abbas wrote:
> Its recommended that DLL must only be enabled for SDR50, DDR50, DDR52,
> SDR104, HS200 and HS400 speed modes. Move DLL configuration to its own
> function and call it only in the above speed modes.
> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci_am654.c | 128 +++++++++++++++++----------------
>  1 file changed, 68 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index bb977de43f7d..575bbab1a6ed 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -119,16 +119,80 @@ static const struct timing_data td[] = {
>  	[MMC_TIMING_MMC_HS400] = {"ti,otap-del-sel-hs400", MMC_CAP2_HS400},
>  };
>  
> +static void sdhci_am654_setup_dll(struct sdhci_host *host, unsigned int clock)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> +	int sel50, sel100, freqsel;
> +	u32 mask, val;
> +	int ret;
> +
> +	if (sdhci_am654->flags & FREQSEL_2_BIT) {
> +		switch (clock) {
> +		case 200000000:
> +			sel50 = 0;
> +			sel100 = 0;
> +			break;
> +		case 100000000:
> +			sel50 = 0;
> +			sel100 = 1;
> +			break;
> +		default:
> +			sel50 = 1;
> +			sel100 = 0;
> +		}
> +
> +		/* Configure PHY DLL frequency */
> +		mask = SEL50_MASK | SEL100_MASK;
> +		val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
> +		regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask, val);
> +
> +	} else {
> +		switch (clock) {
> +		case 200000000:
> +			freqsel = 0x0;
> +			break;
> +		default:
> +			freqsel = 0x4;
> +		}
> +
> +		regmap_update_bits(sdhci_am654->base, PHY_CTRL5, FREQSEL_MASK,
> +				   freqsel << FREQSEL_SHIFT);
> +	}
> +	/* Configure DLL TRIM */
> +	mask = DLL_TRIM_ICP_MASK;
> +	val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
> +
> +	/* Configure DLL driver strength */
> +	mask |= DR_TY_MASK;
> +	val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
> +	regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val);
> +
> +	/* Enable DLL */
> +	regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK,
> +			   0x1 << ENDLL_SHIFT);
> +	/*
> +	 * Poll for DLL ready. Use a one second timeout.
> +	 * Works in all experiments done so far
> +	 */
> +	ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1, val,
> +				       val & DLLRDY_MASK, 1000, 1000000);
> +	if (ret) {
> +		dev_err(mmc_dev(host->mmc), "DLL failed to relock\n");
> +		return;
> +	}
> +
> +	sdhci_am654->dll_on = true;
> +}
> +
>  static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
>  	unsigned char timing = host->mmc->ios.timing;
> -	int sel50, sel100, freqsel;
>  	u32 otap_del_sel;
>  	u32 otap_del_ena;
>  	u32 mask, val;
> -	int ret;
>  
>  	if (sdhci_am654->dll_on) {
>  		regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK, 0);
> @@ -163,64 +227,8 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
>  
>  		regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
>  
> -		if (sdhci_am654->flags & FREQSEL_2_BIT) {
> -			switch (clock) {
> -			case 200000000:
> -				sel50 = 0;
> -				sel100 = 0;
> -				break;
> -			case 100000000:
> -				sel50 = 0;
> -				sel100 = 1;
> -				break;
> -			default:
> -				sel50 = 1;
> -				sel100 = 0;
> -			}
> -
> -			/* Configure PHY DLL frequency */
> -			mask = SEL50_MASK | SEL100_MASK;
> -			val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
> -			regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask,
> -					   val);
> -		} else {
> -			switch (clock) {
> -			case 200000000:
> -				freqsel = 0x0;
> -				break;
> -			default:
> -				freqsel = 0x4;
> -			}
> -
> -			regmap_update_bits(sdhci_am654->base, PHY_CTRL5,
> -					   FREQSEL_MASK,
> -					   freqsel << FREQSEL_SHIFT);
> -		}
> -
> -		/* Configure DLL TRIM */
> -		mask = DLL_TRIM_ICP_MASK;
> -		val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
> -
> -		/* Configure DLL driver strength */
> -		mask |= DR_TY_MASK;
> -		val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
> -		regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val);
> -		/* Enable DLL */
> -		regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK,
> -				   0x1 << ENDLL_SHIFT);
> -		/*
> -		 * Poll for DLL ready. Use a one second timeout.
> -		 * Works in all experiments done so far
> -		 */
> -		ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1,
> -					       val, val & DLLRDY_MASK, 1000,
> -					       1000000);
> -		if (ret) {
> -			dev_err(mmc_dev(host->mmc), "DLL failed to relock\n");
> -			return;
> -		}
> -
> -		sdhci_am654->dll_on = true;
> +		if (timing > MMC_TIMING_UHS_SDR25)
> +			sdhci_am654_setup_dll(host, clock);
>  	}
>  }
>  
> 


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

* Re: [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
  2020-01-20  5:30     ` Faiz Abbas
@ 2020-02-07  9:37       ` Faiz Abbas
  2020-02-14 10:58         ` Faiz Abbas
  0 siblings, 1 reply; 14+ messages in thread
From: Faiz Abbas @ 2020-02-07  9:37 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, linux-mmc, devicetree, ulf.hansson, adrian.hunter

Rob,

On 20/01/20 11:00 am, Faiz Abbas wrote:
> Hi Rob,
> 
> On 15/01/20 7:20 am, Rob Herring wrote:
>> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>>> According to latest AM65x Data Manual[1], a different output tap delay
>>> value is recommended for all speed modes. Therefore, replace the
>>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>>> mode.
>>>
>>> [1] http://www.ti.com/lit/gpn/am6526
>>>
>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>> ---
>>>  .../devicetree/bindings/mmc/sdhci-am654.txt   | 21 +++++++++++++++++--
>>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>> index 50e87df47971..c6ccecb9ae5a 100644
>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>> @@ -18,7 +18,20 @@ Required Properties:
>>>  	- clocks: Handles to the clock inputs.
>>>  	- clock-names: Tuple including "clk_xin" and "clk_ahb"
>>>  	- interrupts: Interrupt specifiers
>>> -	- ti,otap-del-sel: Output Tap Delay select
>>> +	Output tap delay for each speed mode:
>>> +	- ti,otap-del-sel-legacy
>>> +	- ti,otap-del-sel-mmc-hs
>>> +	- ti,otap-del-sel-sd-hs
>>> +	- ti,otap-del-sel-sdr12
>>> +	- ti,otap-del-sel-sdr25
>>> +	- ti,otap-del-sel-sdr50
>>> +	- ti,otap-del-sel-sdr104
>>> +	- ti,otap-del-sel-ddr50
>>> +	- ti,otap-del-sel-ddr52
>>> +	- ti,otap-del-sel-hs200
>>> +	- ti,otap-del-sel-hs400
>>> +	  These bindings must be provided otherwise the driver will disable the
>>> +	  corresponding speed mode (i.e. all nodes must provide at least -legacy)
>>
>> Why not just extend the existing property to be an array. We already 
>> have properties to enable/disable speed modes.
>>
> 
> Its hard to keep track of which modes have values and which don't when
> you add an array. This scheme is just easier on anyone adding new values
> or updating old values.
> 
> We already disable speed modes based on platform specific properties in
> other drivers. In sdhci-omap.c, the driver disables the corresponding
> speed mode if the corresponding pinmux and iodelay values are not populated.
> 

Do you agree on above?

Thanks,
Faiz

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

* Re: [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
  2020-02-07  9:37       ` Faiz Abbas
@ 2020-02-14 10:58         ` Faiz Abbas
  2020-02-20 11:24           ` Faiz Abbas
  0 siblings, 1 reply; 14+ messages in thread
From: Faiz Abbas @ 2020-02-14 10:58 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, linux-mmc, devicetree, ulf.hansson, adrian.hunter

Rob,

On 07/02/20 3:07 pm, Faiz Abbas wrote:
> Rob,
> 
> On 20/01/20 11:00 am, Faiz Abbas wrote:
>> Hi Rob,
>>
>> On 15/01/20 7:20 am, Rob Herring wrote:
>>> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>>>> According to latest AM65x Data Manual[1], a different output tap delay
>>>> value is recommended for all speed modes. Therefore, replace the
>>>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>>>> mode.
>>>>
>>>> [1] http://www.ti.com/lit/gpn/am6526
>>>>
>>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>>> ---
>>>>  .../devicetree/bindings/mmc/sdhci-am654.txt   | 21 +++++++++++++++++--
>>>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>> index 50e87df47971..c6ccecb9ae5a 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>> @@ -18,7 +18,20 @@ Required Properties:
>>>>  	- clocks: Handles to the clock inputs.
>>>>  	- clock-names: Tuple including "clk_xin" and "clk_ahb"
>>>>  	- interrupts: Interrupt specifiers
>>>> -	- ti,otap-del-sel: Output Tap Delay select
>>>> +	Output tap delay for each speed mode:
>>>> +	- ti,otap-del-sel-legacy
>>>> +	- ti,otap-del-sel-mmc-hs
>>>> +	- ti,otap-del-sel-sd-hs
>>>> +	- ti,otap-del-sel-sdr12
>>>> +	- ti,otap-del-sel-sdr25
>>>> +	- ti,otap-del-sel-sdr50
>>>> +	- ti,otap-del-sel-sdr104
>>>> +	- ti,otap-del-sel-ddr50
>>>> +	- ti,otap-del-sel-ddr52
>>>> +	- ti,otap-del-sel-hs200
>>>> +	- ti,otap-del-sel-hs400
>>>> +	  These bindings must be provided otherwise the driver will disable the
>>>> +	  corresponding speed mode (i.e. all nodes must provide at least -legacy)
>>>
>>> Why not just extend the existing property to be an array. We already 
>>> have properties to enable/disable speed modes.
>>>
>>
>> Its hard to keep track of which modes have values and which don't when
>> you add an array. This scheme is just easier on anyone adding new values
>> or updating old values.
>>
>> We already disable speed modes based on platform specific properties in
>> other drivers. In sdhci-omap.c, the driver disables the corresponding
>> speed mode if the corresponding pinmux and iodelay values are not populated.
>>
> 
> Do you agree on above?
> 

Gentle ping.

Thanks,
Faiz

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

* Re: [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
  2020-02-14 10:58         ` Faiz Abbas
@ 2020-02-20 11:24           ` Faiz Abbas
  0 siblings, 0 replies; 14+ messages in thread
From: Faiz Abbas @ 2020-02-20 11:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, linux-mmc, devicetree, ulf.hansson, adrian.hunter

Rob,

On 14/02/20 4:28 pm, Faiz Abbas wrote:
> Rob,
> 
> On 07/02/20 3:07 pm, Faiz Abbas wrote:
>> Rob,
>>
>> On 20/01/20 11:00 am, Faiz Abbas wrote:
>>> Hi Rob,
>>>
>>> On 15/01/20 7:20 am, Rob Herring wrote:
>>>> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>>>>> According to latest AM65x Data Manual[1], a different output tap delay
>>>>> value is recommended for all speed modes. Therefore, replace the
>>>>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>>>>> mode.
>>>>>
>>>>> [1] http://www.ti.com/lit/gpn/am6526
>>>>>
>>>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>>>> ---
>>>>>  .../devicetree/bindings/mmc/sdhci-am654.txt   | 21 +++++++++++++++++--
>>>>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> index 50e87df47971..c6ccecb9ae5a 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> @@ -18,7 +18,20 @@ Required Properties:
>>>>>  	- clocks: Handles to the clock inputs.
>>>>>  	- clock-names: Tuple including "clk_xin" and "clk_ahb"
>>>>>  	- interrupts: Interrupt specifiers
>>>>> -	- ti,otap-del-sel: Output Tap Delay select
>>>>> +	Output tap delay for each speed mode:
>>>>> +	- ti,otap-del-sel-legacy
>>>>> +	- ti,otap-del-sel-mmc-hs
>>>>> +	- ti,otap-del-sel-sd-hs
>>>>> +	- ti,otap-del-sel-sdr12
>>>>> +	- ti,otap-del-sel-sdr25
>>>>> +	- ti,otap-del-sel-sdr50
>>>>> +	- ti,otap-del-sel-sdr104
>>>>> +	- ti,otap-del-sel-ddr50
>>>>> +	- ti,otap-del-sel-ddr52
>>>>> +	- ti,otap-del-sel-hs200
>>>>> +	- ti,otap-del-sel-hs400
>>>>> +	  These bindings must be provided otherwise the driver will disable the
>>>>> +	  corresponding speed mode (i.e. all nodes must provide at least -legacy)
>>>>
>>>> Why not just extend the existing property to be an array. We already 
>>>> have properties to enable/disable speed modes.
>>>>
>>>
>>> Its hard to keep track of which modes have values and which don't when
>>> you add an array. This scheme is just easier on anyone adding new values
>>> or updating old values.
>>>
>>> We already disable speed modes based on platform specific properties in
>>> other drivers. In sdhci-omap.c, the driver disables the corresponding
>>> speed mode if the corresponding pinmux and iodelay values are not populated.
>>>
>>
>> Do you agree on above?
>>
> 
> Gentle ping.
> 

Ping.

Thanks,
Faiz

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

* Re: [PATCH 0/3] Update phy configuration for AM65x
  2020-01-08 15:09 [PATCH 0/3] Update phy configuration for AM65x Faiz Abbas
                   ` (2 preceding siblings ...)
  2020-01-08 15:09 ` [PATCH 3/3] mmc: sdhci_am654: Enable DLL only for some speed modes Faiz Abbas
@ 2020-03-02 19:11 ` Faiz Abbas
  2020-03-03 20:53   ` Ulf Hansson
  3 siblings, 1 reply; 14+ messages in thread
From: Faiz Abbas @ 2020-03-02 19:11 UTC (permalink / raw)
  To: linux-kernel, linux-mmc, devicetree; +Cc: ulf.hansson, adrian.hunter, robh+dt

Uffe,

On 08/01/20 8:39 pm, Faiz Abbas wrote:
> The following patches update phy configurations for AM65x as given in
> the latest data manual.
> 
> The patches depend on my fixes series posted just before this:
> https://patchwork.kernel.org/project/linux-mmc/list/?series=225425
> 
> Device tree patch updating the actual otap values will be posted
> separately.
> 
> Tested with Am65x-evm and J721e-evm.
> 
> Faiz Abbas (3):
>   dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
>   mmc: sdhci_am654: Update OTAPDLY writes
>   mmc: sdhci_am654: Enable DLL only for some speed modes
> 
>  .../devicetree/bindings/mmc/sdhci-am654.txt   |  21 +-
>  drivers/mmc/host/sdhci_am654.c                | 247 ++++++++++++------
>  include/linux/mmc/host.h                      |   2 +
>  3 files changed, 192 insertions(+), 78 deletions(-)
> 

Can you help merge this?

Thanks,
Faiz

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

* Re: [PATCH 0/3] Update phy configuration for AM65x
  2020-03-02 19:11 ` [PATCH 0/3] Update phy configuration for AM65x Faiz Abbas
@ 2020-03-03 20:53   ` Ulf Hansson
  2020-03-04 15:34     ` Ulf Hansson
  0 siblings, 1 reply; 14+ messages in thread
From: Ulf Hansson @ 2020-03-03 20:53 UTC (permalink / raw)
  To: Faiz Abbas
  Cc: Linux Kernel Mailing List, linux-mmc, DTML, Adrian Hunter, Rob Herring

On Mon, 2 Mar 2020 at 20:11, Faiz Abbas <faiz_abbas@ti.com> wrote:
>
> Uffe,
>
> On 08/01/20 8:39 pm, Faiz Abbas wrote:
> > The following patches update phy configurations for AM65x as given in
> > the latest data manual.
> >
> > The patches depend on my fixes series posted just before this:
> > https://patchwork.kernel.org/project/linux-mmc/list/?series=225425
> >
> > Device tree patch updating the actual otap values will be posted
> > separately.
> >
> > Tested with Am65x-evm and J721e-evm.
> >
> > Faiz Abbas (3):
> >   dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
> >   mmc: sdhci_am654: Update OTAPDLY writes
> >   mmc: sdhci_am654: Enable DLL only for some speed modes
> >
> >  .../devicetree/bindings/mmc/sdhci-am654.txt   |  21 +-
> >  drivers/mmc/host/sdhci_am654.c                | 247 ++++++++++++------
> >  include/linux/mmc/host.h                      |   2 +
> >  3 files changed, 192 insertions(+), 78 deletions(-)
> >
>
> Can you help merge this?

Apologize with the delay, still focused on fixing various regressions in v5.6.

I start catching up on my mmc backlog as of tomorrow. Thanks for pinging me.

Kind regards
Uffe

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

* Re: [PATCH 0/3] Update phy configuration for AM65x
  2020-03-03 20:53   ` Ulf Hansson
@ 2020-03-04 15:34     ` Ulf Hansson
  0 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2020-03-04 15:34 UTC (permalink / raw)
  To: Faiz Abbas
  Cc: Linux Kernel Mailing List, linux-mmc, DTML, Adrian Hunter, Rob Herring

On Tue, 3 Mar 2020 at 21:53, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Mon, 2 Mar 2020 at 20:11, Faiz Abbas <faiz_abbas@ti.com> wrote:
> >
> > Uffe,
> >
> > On 08/01/20 8:39 pm, Faiz Abbas wrote:
> > > The following patches update phy configurations for AM65x as given in
> > > the latest data manual.
> > >
> > > The patches depend on my fixes series posted just before this:
> > > https://patchwork.kernel.org/project/linux-mmc/list/?series=225425
> > >
> > > Device tree patch updating the actual otap values will be posted
> > > separately.
> > >
> > > Tested with Am65x-evm and J721e-evm.
> > >
> > > Faiz Abbas (3):
> > >   dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
> > >   mmc: sdhci_am654: Update OTAPDLY writes
> > >   mmc: sdhci_am654: Enable DLL only for some speed modes
> > >
> > >  .../devicetree/bindings/mmc/sdhci-am654.txt   |  21 +-
> > >  drivers/mmc/host/sdhci_am654.c                | 247 ++++++++++++------
> > >  include/linux/mmc/host.h                      |   2 +
> > >  3 files changed, 192 insertions(+), 78 deletions(-)
> > >
> >
> > Can you help merge this?
>
> Apologize with the delay, still focused on fixing various regressions in v5.6.
>
> I start catching up on my mmc backlog as of tomorrow. Thanks for pinging me.

Applied for next, thanks!

Kind regards
Uffe

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

end of thread, other threads:[~2020-03-04 15:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-08 15:09 [PATCH 0/3] Update phy configuration for AM65x Faiz Abbas
2020-01-08 15:09 ` [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding Faiz Abbas
2020-01-15  1:50   ` Rob Herring
2020-01-20  5:30     ` Faiz Abbas
2020-02-07  9:37       ` Faiz Abbas
2020-02-14 10:58         ` Faiz Abbas
2020-02-20 11:24           ` Faiz Abbas
2020-01-08 15:09 ` [PATCH 2/3] mmc: sdhci_am654: Update OTAPDLY writes Faiz Abbas
2020-01-20 12:31   ` Adrian Hunter
2020-01-08 15:09 ` [PATCH 3/3] mmc: sdhci_am654: Enable DLL only for some speed modes Faiz Abbas
2020-01-20 12:32   ` Adrian Hunter
2020-03-02 19:11 ` [PATCH 0/3] Update phy configuration for AM65x Faiz Abbas
2020-03-03 20:53   ` Ulf Hansson
2020-03-04 15:34     ` Ulf Hansson

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