[PATCH v3 0/5] Add enhanced strobe support for emmc version 5.1 or later

[PATCH v3 0/5] Add enhanced strobe support for emmc version 5.1 or later

[Shawn Lin]

Hello Ulf and Adrian,

This patch is going to support enhanced strobe function
for emmc version 5.1+ introduced by JEDEC recently.

Enchanced strobe is a optional function, so we add a new cap* for drivers to
decide whether to use it.

When introduing hs400 mode, JEDEC asks controllers to use data strobe line
to latch the data from emmc devives. But for cmd-reponse, not mentioned yet.
Since emmc version 5.1 published, JEDEC adds enhanced strobe function to deal
with cmd-response the same way as data-read. This feature is optional.

>From the spec(section 6.6.2.3), the standard scenario to select HS400 enhanced
strobe mode illustrated like this:
(1) set HS_TIMIMG (Highspeed)
(2) Host change freq to <= 52Mhz
(3) set the bus width to Enhanced strobe and DDR8Bit(CMD6), EXT_CSD[183] = 0x86
instead of 0x80
(4) set HS_TIMING to 0x3 (HS400)
(5) Host change freq to <= 200Mhz
(6) Host select HS400 enhanced strobe complete

I can't find a upstreamed controller claimed to support it, as well as the
mmc stack. But my "arasan,sdhci-5.1" actually supports this function. So I decide
to work for this part.

By looking into the SDHCI spec, I find there isn't any registers to enable the
enhanced strobe function. But from my "arasan,sdhci-5.1" databook, it describes a
register called VENDOR_REGISTER(0x78) to trigger this mode. So I guess other sdhci
variant drivers may also need s vendor specific register to deal with it.

If we are sure that our controller supports enhanced strobe mode, just add
mmc-hs400-enhanced-strobe in DT. Once emmc devices claims to support this mode,
we enable it automatically. Of course, other sdhci*/non-sdhci drivers should
implement/overwrite the prepare_enhanced_strobe by themselves. I believe all of the
platforms which need to support this mode should do some basic preparation for their
controllers. Currently I just limit the callback within ths scope of arasan,sdhci-5.1,
but I prone to believe arasan will use VENDOR_REGISTER to tirgger this mode from now on
because I do believe vendor will consider the registers' compatibility.

With this patchset applied, we can successfully run in HS400 enhanced strobe mode on
RK3399 platform with Samsung eMMC KLMBG2JENB-B041(v5.1, 16GB).

mmc1: new HS400 Enhanced strobe MMC card at address 0001
mmcblk0: mmc1:0001 AJNB4R 14.6 GiB
mmcblk0boot0: mmc1:0001 AJNB4R partition 1 4.00 MiB
mmcblk0boot1: mmc1:0001 AJNB4R partition 2 4.00 MiB
mmcblk0rpmb: mmc1:0001 AJNB4R partition 3 4.00 MiB

Also I check this patchset with removing some caps to make sure
all the init sequences for hs400/hs200/DDR52/HighSpeed is correct.


Changes in v3:
- remove helper function and shorten cap2 to MMC_CAP2_HS400_ES
- rename callback to hs400_enhanced_strobe and make it return void
- add enhanced_strobe in mmc_ios and pass it to hs400_enhanced_strobe
- disable host's enhanced strobe in mmc_set_initial_state
- remove bus checking after switching hs400es
- add new function for hs400es timing selection
- fix wrong judgement of mmc_card_hs400es

Changes in v2:
- switch to HS400ES from Highspeed mode directly

Shawn Lin (5):
  Documentation: mmc: add mmc-hs400-enhanced-strobe
  mmc: core: add mmc-hs400-enhanced-strobe support
  mmc: core: implement enhanced strobe support
  mmc: debugfs: add HS400 enhanced strobe description
  mmc: sdhci-of-arasan: implement enhanced strobe callback

 Documentation/devicetree/bindings/mmc/mmc.txt |  1 +
 drivers/mmc/core/bus.c                        |  3 +-
 drivers/mmc/core/core.c                       |  8 +++
 drivers/mmc/core/debugfs.c                    |  4 +-
 drivers/mmc/core/host.c                       |  2 +
 drivers/mmc/core/mmc.c                        | 79 ++++++++++++++++++++++++++-
 drivers/mmc/host/sdhci-of-arasan.c            | 20 +++++++
 include/linux/mmc/card.h                      |  1 +
 include/linux/mmc/host.h                      | 12 ++++
 include/linux/mmc/mmc.h                       |  3 +
 10 files changed, 129 insertions(+), 4 deletions(-)

-- 
2.3.7

[PATCH v3 1/5] Documentation: mmc: add mmc-hs400-enhanced-strobe

[Shawn Lin]

mmc-hs400-enhanced-strobe is used to claim that the
host can support hs400 mode with enhanced strobe
introduced by emmc 5.1 spec.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes in v3: None
Changes in v2: None

 Documentation/devicetree/bindings/mmc/mmc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index ed23b9b..ecc007a 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -46,6 +46,7 @@ Optional properties:
 - mmc-hs200-1_2v: eMMC HS200 mode(1.2V I/O) is supported
 - mmc-hs400-1_8v: eMMC HS400 mode(1.8V I/O) is supported
 - mmc-hs400-1_2v: eMMC HS400 mode(1.2V I/O) is supported
+- mmc-hs400-enhanced-strobe: eMMC HS400 enhanced strobe mode is supported
 - dsr: Value the card's (optional) Driver Stage Register (DSR) should be
   programmed with. Valid range: [0 .. 0xffff].
 
-- 
2.3.7

[PATCH v3 2/5] mmc: core: add mmc-hs400-enhanced-strobe support

[Shawn Lin]

This patch introduce mmc-hs400-enhanced-strobe for platforms
which want to enable enhanced strobe function from DT if the
mmc host controller claims to support enhanced strobe.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

Changes in v3:
- remove helper function and shorten cap2 to MMC_CAP2_HS400_ES
- rename callback to hs400_enhanced_strobe and make it return void
- add enhanced_strobe in mmc_ios and pass it to hs400_enhanced_strobe
- disable host's enhanced strobe in mmc_set_initial_state
- remove bus checking after switching hs400es
- add new function for hs400es timing selection
- fix wrong judgement of mmc_card_hs400es

Changes in v2:
- switch to HS400ES from Highspeed mode directly

 drivers/mmc/core/host.c  | 2 ++
 include/linux/mmc/host.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index e0a3ee1..45773e4 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -289,6 +289,8 @@ int mmc_of_parse(struct mmc_host *host)
 		host->caps2 |= MMC_CAP2_HS400_1_8V | MMC_CAP2_HS200_1_8V_SDR;
 	if (of_property_read_bool(np, "mmc-hs400-1_2v"))
 		host->caps2 |= MMC_CAP2_HS400_1_2V | MMC_CAP2_HS200_1_2V_SDR;
+	if (of_property_read_bool(np, "mmc-hs400-enhanced-strobe"))
+		host->caps2 |= MMC_CAP2_HS400_ES;
 
 	host->dsr_req = !of_property_read_u32(np, "dsr", &host->dsr);
 	if (host->dsr_req && (host->dsr & ~0xffff)) {
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 85800b4..2a06fb0 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -302,6 +302,7 @@ struct mmc_host {
 #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
 #define MMC_CAP2_NO_WRITE_PROTECT (1 << 18)	/* No physical write protect pin, assume that card is always read-write */
 #define MMC_CAP2_NO_SDIO	(1 << 19)	/* Do not send SDIO commands during initialization */
+#define MMC_CAP2_HS400_ES	(1 << 20)	/* Host supports enhanced strobe */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
-- 
2.3.7

[PATCH v3 3/5] mmc: core: implement enhanced strobe support

[Shawn Lin]

Controllers use data strobe line to latch data from devices
under hs400 mode, but not for cmd line. So since emmc 5.1, JEDEC
introduces enhanced strobe mode for latching cmd response from
emmc devices to host controllers. This new feature is optional,
so it depends both on device's cap and host's cap to decide
whether to use it or not.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v3: None
Changes in v2: None

 drivers/mmc/core/bus.c   |  3 +-
 drivers/mmc/core/core.c  |  8 +++++
 drivers/mmc/core/mmc.c   | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/mmc/card.h |  1 +
 include/linux/mmc/host.h | 11 +++++++
 include/linux/mmc/mmc.h  |  3 ++
 6 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 4bc48f1..7e94b9d 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -332,12 +332,13 @@ int mmc_add_card(struct mmc_card *card)
 			mmc_card_ddr52(card) ? "DDR " : "",
 			type);
 	} else {
-		pr_info("%s: new %s%s%s%s%s card at address %04x\n",
+		pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
 			mmc_hostname(card->host),
 			mmc_card_uhs(card) ? "ultra high speed " :
 			(mmc_card_hs(card) ? "high speed " : ""),
 			mmc_card_hs400(card) ? "HS400 " :
 			(mmc_card_hs200(card) ? "HS200 " : ""),
+			mmc_card_hs400es(card) ? "Enhanced strobe" : "",
 			mmc_card_ddr52(card) ? "DDR " : "",
 			uhs_bus_speed_mode, type, card->rca);
 	}
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 99275e4..a559501 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1127,6 +1127,14 @@ void mmc_set_initial_state(struct mmc_host *host)
 	host->ios.bus_width = MMC_BUS_WIDTH_1;
 	host->ios.timing = MMC_TIMING_LEGACY;
 	host->ios.drv_type = 0;
+	host->ios.enhanced_strobe = false;
+
+	/*
+	 * Make sure we are in non-enhanced strobe mode before we
+	 * actually enable it in ext_csd.
+	 */
+	if (host->ops->hs400_enhanced_strobe)
+		host->ops->hs400_enhanced_strobe(host, &host->ios);
 
 	mmc_set_ios(host);
 }
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f99c47e..c2d1981 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -235,6 +235,11 @@ static void mmc_select_card_type(struct mmc_card *card)
 		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
 	}
 
+	if ((caps2 & MMC_CAP2_HS400_ES) &&
+	    card->ext_csd.strobe_support &&
+	    (card_type & EXT_CSD_CARD_TYPE_HS400))
+		avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
+
 	card->ext_csd.hs_max_dtr = hs_max_dtr;
 	card->ext_csd.hs200_max_dtr = hs200_max_dtr;
 	card->mmc_avail_type = avail_type;
@@ -383,6 +388,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
 			mmc_card_set_blockaddr(card);
 	}
 
+	card->ext_csd.strobe_support = ext_csd[EXT_CSD_STROBE_SUPPORT];
 	card->ext_csd.raw_card_type = ext_csd[EXT_CSD_CARD_TYPE];
 	mmc_select_card_type(card);
 
@@ -1216,6 +1222,73 @@ out_err:
 	return err;
 }
 
+static int mmc_select_hs400es(struct mmc_card *card)
+{
+	struct mmc_host *host = card->host;
+	int err = 0;
+	u8 val;
+
+	err = mmc_select_bus_width(card);
+	if (IS_ERR_VALUE(err))
+		goto out_err;
+
+	/* Switch card to HS mode */
+	err = mmc_select_hs(card);
+	if (err) {
+		pr_err("%s: switch to high-speed failed, err:%d\n",
+			mmc_hostname(host), err);
+		goto out_err;
+	}
+
+	err = mmc_switch_status(card);
+	if (err)
+		goto out_err;
+
+	/* Switch card to DDR with strobe bit */
+	val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
+	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+			 EXT_CSD_BUS_WIDTH,
+			 val,
+			 card->ext_csd.generic_cmd6_time);
+	if (err) {
+		pr_err("%s: switch to bus width for hs400es failed, err:%d\n",
+			mmc_hostname(host), err);
+		goto out_err;
+	}
+
+	/* Switch card to HS400 */
+	val = EXT_CSD_TIMING_HS400 |
+	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+			   EXT_CSD_HS_TIMING, val,
+			   card->ext_csd.generic_cmd6_time,
+			   true, false, true);
+	if (err) {
+		pr_err("%s: switch to hs400es failed, err:%d\n",
+			mmc_hostname(host), err);
+		goto out_err;
+	}
+
+	/* Set host controller to HS400 timing and frequency */
+	mmc_set_timing(host, MMC_TIMING_MMC_HS400);
+
+	/* Controller enable enhanced strobe function */
+	host->ios.enhanced_strobe = true;
+	if (host->ops->hs400_enhanced_strobe)
+		host->ops->hs400_enhanced_strobe(host, &host->ios);
+
+	err = mmc_switch_status(card);
+	if (err)
+		goto out_err;
+
+	return 0;
+
+out_err:
+	pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
+	       __func__, err);
+	return err;
+}
+
 static void mmc_select_driver_type(struct mmc_card *card)
 {
 	int card_drv_type, drive_strength, drv_type;
@@ -1297,7 +1370,7 @@ err:
 }
 
 /*
- * Activate High Speed or HS200 mode if supported.
+ * Activate High Speed, HS200 or HS400ES mode if supported.
  */
 static int mmc_select_timing(struct mmc_card *card)
 {
@@ -1306,7 +1379,9 @@ static int mmc_select_timing(struct mmc_card *card)
 	if (!mmc_can_ext_csd(card))
 		goto bus_speed;
 
-	if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
+	if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES)
+		err = mmc_select_hs400es(card);
+	else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
 		err = mmc_select_hs200(card);
 	else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
 		err = mmc_select_hs(card);
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index eb0151b..22defc2 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -95,6 +95,7 @@ struct mmc_ext_csd {
 	u8			raw_partition_support;	/* 160 */
 	u8			raw_rpmb_size_mult;	/* 168 */
 	u8			raw_erased_mem_count;	/* 181 */
+	u8			strobe_support;		/* 184 */
 	u8			raw_ext_csd_structure;	/* 194 */
 	u8			raw_card_type;		/* 196 */
 	u8			raw_driver_strength;	/* 197 */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 2a06fb0..f98bd0e 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -19,6 +19,7 @@
 
 #include <linux/mmc/core.h>
 #include <linux/mmc/card.h>
+#include <linux/mmc/mmc.h>
 #include <linux/mmc/pm.h>
 
 struct mmc_ios {
@@ -77,6 +78,8 @@ struct mmc_ios {
 #define MMC_SET_DRIVER_TYPE_A	1
 #define MMC_SET_DRIVER_TYPE_C	2
 #define MMC_SET_DRIVER_TYPE_D	3
+
+	bool enhanced_strobe;			/* hs400es selection */
 };
 
 struct mmc_host_ops {
@@ -143,6 +146,9 @@ struct mmc_host_ops {
 
 	/* Prepare HS400 target operating frequency depending host driver */
 	int	(*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios);
+	/* Prepare enhanced strobe depending host driver */
+	void	(*hs400_enhanced_strobe)(struct mmc_host *host,
+					 struct mmc_ios *ios);
 	int	(*select_drive_strength)(struct mmc_card *card,
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
@@ -513,6 +519,11 @@ static inline bool mmc_card_hs400(struct mmc_card *card)
 	return card->host->ios.timing == MMC_TIMING_MMC_HS400;
 }
 
+static inline bool mmc_card_hs400es(struct mmc_card *card)
+{
+	return card->host->ios.enhanced_strobe == true;
+}
+
 void mmc_retune_timer_stop(struct mmc_host *host);
 
 static inline void mmc_retune_needed(struct mmc_host *host)
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 15f2c4a..c376209 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -297,6 +297,7 @@ struct _mmc_csd {
 #define EXT_CSD_PART_CONFIG		179	/* R/W */
 #define EXT_CSD_ERASED_MEM_CONT		181	/* RO */
 #define EXT_CSD_BUS_WIDTH		183	/* R/W */
+#define EXT_CSD_STROBE_SUPPORT		184	/* RO */
 #define EXT_CSD_HS_TIMING		185	/* R/W */
 #define EXT_CSD_POWER_CLASS		187	/* R/W */
 #define EXT_CSD_REV			192	/* RO */
@@ -380,12 +381,14 @@ struct _mmc_csd {
 #define EXT_CSD_CARD_TYPE_HS400_1_2V	(1<<7)	/* Card can run at 200MHz DDR, 1.2V */
 #define EXT_CSD_CARD_TYPE_HS400		(EXT_CSD_CARD_TYPE_HS400_1_8V | \
 					 EXT_CSD_CARD_TYPE_HS400_1_2V)
+#define EXT_CSD_CARD_TYPE_HS400ES	(1<<8)	/* Card can run at HS400ES */
 
 #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
 #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
 #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
 #define EXT_CSD_DDR_BUS_WIDTH_4	5	/* Card is in 4 bit DDR mode */
 #define EXT_CSD_DDR_BUS_WIDTH_8	6	/* Card is in 8 bit DDR mode */
+#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)	/* Enhanced strobe mode */
 
 #define EXT_CSD_TIMING_BC	0	/* Backwards compatility */
 #define EXT_CSD_TIMING_HS	1	/* High speed */
-- 
2.3.7

[PATCH v3 4/5] mmc: debugfs: add HS400 enhanced strobe description

[Shawn Lin]

We introduce HS400 with enhanced strobe function, so we need
to add it for debug show.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v3: None
Changes in v2: None

 drivers/mmc/core/debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 9382a57..51d7c38 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -148,7 +148,9 @@ static int mmc_ios_show(struct seq_file *s, void *data)
 		str = "mmc HS200";
 		break;
 	case MMC_TIMING_MMC_HS400:
-		str = "mmc HS400";
+		mmc_card_hs400es(host->card) ?
+			(str = "mmc HS400 enhanced strobe") :
+			(str = "mmc HS400");
 		break;
 	default:
 		str = "invalid";
-- 
2.3.7

[PATCH v3 5/5] mmc: sdhci-of-arasan: implement enhanced strobe callback

[Shawn Lin]

Currently sdhci-arasan 5.1 can support enhanced strobe function,
and we now limit it just for "arasan,sdhci-5.1". Add
mmc-hs400-enhanced-strobe in DT to enable the function if we'r sure
our controller can support it.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v3: None
Changes in v2: None

 drivers/mmc/host/sdhci-of-arasan.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index b6f4c1d..8e5cb06 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -25,7 +25,9 @@
 #include "sdhci-pltfm.h"
 
 #define SDHCI_ARASAN_CLK_CTRL_OFFSET	0x2c
+#define SDHCI_ARASAN_VENDOR_REGISTER	0x78
 
+#define VENDOR_ENHANCED_STROBE		BIT(0)
 #define CLK_CTRL_TIMEOUT_SHIFT		16
 #define CLK_CTRL_TIMEOUT_MASK		(0xf << CLK_CTRL_TIMEOUT_SHIFT)
 #define CLK_CTRL_TIMEOUT_MIN_EXP	13
@@ -79,6 +81,21 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
 	}
 }
 
+static void sdhci_arasan_hs400_enhanced_strobe(struct mmc_host *mmc,
+					struct mmc_ios *ios)
+{
+	u32 vendor;
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	vendor = readl(host->ioaddr + SDHCI_ARASAN_VENDOR_REGISTER);
+	if (ios->enhanced_strobe)
+		vendor |= VENDOR_ENHANCED_STROBE;
+	else
+		vendor &= (~VENDOR_ENHANCED_STROBE);
+
+	writel(vendor, host->ioaddr + SDHCI_ARASAN_VENDOR_REGISTER);
+}
+
 static struct sdhci_ops sdhci_arasan_ops = {
 	.set_clock = sdhci_arasan_set_clock,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
@@ -245,6 +262,9 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "phy_power_on err.\n");
 			goto err_phy_power;
 		}
+
+		host->mmc_host_ops.hs400_enhanced_strobe =
+					sdhci_arasan_hs400_enhanced_strobe;
 	}
 
 	ret = sdhci_add_host(host);
-- 
2.3.7

Re: [PATCH v3 5/5] mmc: sdhci-of-arasan: implement enhanced strobe callback

[Doug Anderson]

Hi,

On Tue, May 10, 2016 at 2:10 AM, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> Currently sdhci-arasan 5.1 can support enhanced strobe function,
> and we now limit it just for "arasan,sdhci-5.1". Add
> mmc-hs400-enhanced-strobe in DT to enable the function if we'r sure

nit:s/we'r/we're/

[ ... ]
> @@ -79,6 +81,21 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
>         }
>  }
>
> +static void sdhci_arasan_hs400_enhanced_strobe(struct mmc_host *mmc,
> +                                       struct mmc_ios *ios)
> +{
> +       u32 vendor;
> +       struct sdhci_host *host = mmc_priv(mmc);
> +
> +       vendor = readl(host->ioaddr + SDHCI_ARASAN_VENDOR_REGISTER);
> +       if (ios->enhanced_strobe)
> +               vendor |= VENDOR_ENHANCED_STROBE;
> +       else
> +               vendor &= (~VENDOR_ENHANCED_STROBE);

nit: no extra parentheses:

  vendor &= ~VENDOR_ENHANCED_STROBE;


Other than the nits this patch looks good to me.  Feel free to add my
Reviewed-by when nits fixed.

-Doug

Re: [PATCH v3 3/5] mmc: core: implement enhanced strobe support

[Doug Anderson]

Hi,

On Tue, May 10, 2016 at 2:09 AM, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> Controllers use data strobe line to latch data from devices
> under hs400 mode, but not for cmd line. So since emmc 5.1, JEDEC
> introduces enhanced strobe mode for latching cmd response from
> emmc devices to host controllers. This new feature is optional,
> so it depends both on device's cap and host's cap to decide
> whether to use it or not.

I don't totally understand it, but two boards I'm working with have
eMMC devices that claim to be eMMC 5.0 devices but also have support
for strobe.  Did people just cherry-pick this feature from eMMC 5.1 to
their eMMC 5.0 devices?


> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  drivers/mmc/core/bus.c   |  3 +-
>  drivers/mmc/core/core.c  |  8 +++++
>  drivers/mmc/core/mmc.c   | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
>  include/linux/mmc/card.h |  1 +
>  include/linux/mmc/host.h | 11 +++++++
>  include/linux/mmc/mmc.h  |  3 ++
>  6 files changed, 102 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 4bc48f1..7e94b9d 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -332,12 +332,13 @@ int mmc_add_card(struct mmc_card *card)
>                         mmc_card_ddr52(card) ? "DDR " : "",
>                         type);
>         } else {
> -               pr_info("%s: new %s%s%s%s%s card at address %04x\n",
> +               pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
>                         mmc_hostname(card->host),
>                         mmc_card_uhs(card) ? "ultra high speed " :
>                         (mmc_card_hs(card) ? "high speed " : ""),
>                         mmc_card_hs400(card) ? "HS400 " :
>                         (mmc_card_hs200(card) ? "HS200 " : ""),
> +                       mmc_card_hs400es(card) ? "Enhanced strobe" : "",

Nit: there should be a space before the quote.  Otherwise the message
runs together, like:

new HS400 Enhanced strobeMMC card at address 0001


>                         mmc_card_ddr52(card) ? "DDR " : "",
>                         uhs_bus_speed_mode, type, card->rca);
>         }
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 99275e4..a559501 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1127,6 +1127,14 @@ void mmc_set_initial_state(struct mmc_host *host)
>         host->ios.bus_width = MMC_BUS_WIDTH_1;
>         host->ios.timing = MMC_TIMING_LEGACY;
>         host->ios.drv_type = 0;
> +       host->ios.enhanced_strobe = false;
> +
> +       /*
> +        * Make sure we are in non-enhanced strobe mode before we
> +        * actually enable it in ext_csd.
> +        */
> +       if (host->ops->hs400_enhanced_strobe)
> +               host->ops->hs400_enhanced_strobe(host, &host->ios);
>
>         mmc_set_ios(host);
>  }
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f99c47e..c2d1981 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -235,6 +235,11 @@ static void mmc_select_card_type(struct mmc_card *card)
>                 avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
>         }
>
> +       if ((caps2 & MMC_CAP2_HS400_ES) &&
> +           card->ext_csd.strobe_support &&
> +           (card_type & EXT_CSD_CARD_TYPE_HS400))
> +               avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
> +
>         card->ext_csd.hs_max_dtr = hs_max_dtr;
>         card->ext_csd.hs200_max_dtr = hs200_max_dtr;
>         card->mmc_avail_type = avail_type;
> @@ -383,6 +388,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
>                         mmc_card_set_blockaddr(card);
>         }
>
> +       card->ext_csd.strobe_support = ext_csd[EXT_CSD_STROBE_SUPPORT];
>         card->ext_csd.raw_card_type = ext_csd[EXT_CSD_CARD_TYPE];
>         mmc_select_card_type(card);
>
> @@ -1216,6 +1222,73 @@ out_err:
>         return err;
>  }
>
> +static int mmc_select_hs400es(struct mmc_card *card)
> +{
> +       struct mmc_host *host = card->host;
> +       int err = 0;
> +       u8 val;
> +
> +       err = mmc_select_bus_width(card);
> +       if (IS_ERR_VALUE(err))
> +               goto out_err;
> +
> +       /* Switch card to HS mode */
> +       err = mmc_select_hs(card);
> +       if (err) {
> +               pr_err("%s: switch to high-speed failed, err:%d\n",
> +                       mmc_hostname(host), err);
> +               goto out_err;
> +       }
> +
> +       err = mmc_switch_status(card);
> +       if (err)
> +               goto out_err;
> +
> +       /* Switch card to DDR with strobe bit */
> +       val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;

In the hs400 function earlier in the file the check to confirm that
the bus width is 8 before doing this.  Should we do that here, too?

> +       err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +                        EXT_CSD_BUS_WIDTH,
> +                        val,
> +                        card->ext_csd.generic_cmd6_time);
> +       if (err) {
> +               pr_err("%s: switch to bus width for hs400es failed, err:%d\n",
> +                       mmc_hostname(host), err);
> +               goto out_err;
> +       }
> +
> +       /* Switch card to HS400 */
> +       val = EXT_CSD_TIMING_HS400 |
> +             card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
> +       err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +                          EXT_CSD_HS_TIMING, val,
> +                          card->ext_csd.generic_cmd6_time,
> +                          true, false, true);
> +       if (err) {
> +               pr_err("%s: switch to hs400es failed, err:%d\n",
> +                       mmc_hostname(host), err);
> +               goto out_err;
> +       }
> +
> +       /* Set host controller to HS400 timing and frequency */
> +       mmc_set_timing(host, MMC_TIMING_MMC_HS400);
> +
> +       /* Controller enable enhanced strobe function */
> +       host->ios.enhanced_strobe = true;
> +       if (host->ops->hs400_enhanced_strobe)
> +               host->ops->hs400_enhanced_strobe(host, &host->ios);
> +
> +       err = mmc_switch_status(card);
> +       if (err)
> +               goto out_err;
> +
> +       return 0;
> +
> +out_err:
> +       pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
> +              __func__, err);
> +       return err;
> +}
> +
>  static void mmc_select_driver_type(struct mmc_card *card)
>  {
>         int card_drv_type, drive_strength, drv_type;
> @@ -1297,7 +1370,7 @@ err:
>  }
>
>  /*
> - * Activate High Speed or HS200 mode if supported.
> + * Activate High Speed, HS200 or HS400ES mode if supported.
>   */
>  static int mmc_select_timing(struct mmc_card *card)
>  {
> @@ -1306,7 +1379,9 @@ static int mmc_select_timing(struct mmc_card *card)
>         if (!mmc_can_ext_csd(card))
>                 goto bus_speed;
>
> -       if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
> +       if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES)
> +               err = mmc_select_hs400es(card);
> +       else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>                 err = mmc_select_hs200(card);
>         else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
>                 err = mmc_select_hs(card);
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index eb0151b..22defc2 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -95,6 +95,7 @@ struct mmc_ext_csd {
>         u8                      raw_partition_support;  /* 160 */
>         u8                      raw_rpmb_size_mult;     /* 168 */
>         u8                      raw_erased_mem_count;   /* 181 */
> +       u8                      strobe_support;         /* 184 */
>         u8                      raw_ext_csd_structure;  /* 194 */
>         u8                      raw_card_type;          /* 196 */
>         u8                      raw_driver_strength;    /* 197 */
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 2a06fb0..f98bd0e 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -19,6 +19,7 @@
>
>  #include <linux/mmc/core.h>
>  #include <linux/mmc/card.h>
> +#include <linux/mmc/mmc.h>
>  #include <linux/mmc/pm.h>
>
>  struct mmc_ios {
> @@ -77,6 +78,8 @@ struct mmc_ios {
>  #define MMC_SET_DRIVER_TYPE_A  1
>  #define MMC_SET_DRIVER_TYPE_C  2
>  #define MMC_SET_DRIVER_TYPE_D  3
> +
> +       bool enhanced_strobe;                   /* hs400es selection */
>  };
>
>  struct mmc_host_ops {
> @@ -143,6 +146,9 @@ struct mmc_host_ops {
>
>         /* Prepare HS400 target operating frequency depending host driver */
>         int     (*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios);
> +       /* Prepare enhanced strobe depending host driver */
> +       void    (*hs400_enhanced_strobe)(struct mmc_host *host,
> +                                        struct mmc_ios *ios);
>         int     (*select_drive_strength)(struct mmc_card *card,
>                                          unsigned int max_dtr, int host_drv,
>                                          int card_drv, int *drv_type);
> @@ -513,6 +519,11 @@ static inline bool mmc_card_hs400(struct mmc_card *card)
>         return card->host->ios.timing == MMC_TIMING_MMC_HS400;
>  }
>
> +static inline bool mmc_card_hs400es(struct mmc_card *card)
> +{
> +       return card->host->ios.enhanced_strobe == true;

nit: never need "== true".  Just:

  return card->host->ios.enhanced_strobe;

> +}
> +
>  void mmc_retune_timer_stop(struct mmc_host *host);
>
>  static inline void mmc_retune_needed(struct mmc_host *host)
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 15f2c4a..c376209 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -297,6 +297,7 @@ struct _mmc_csd {
>  #define EXT_CSD_PART_CONFIG            179     /* R/W */
>  #define EXT_CSD_ERASED_MEM_CONT                181     /* RO */
>  #define EXT_CSD_BUS_WIDTH              183     /* R/W */
> +#define EXT_CSD_STROBE_SUPPORT         184     /* RO */
>  #define EXT_CSD_HS_TIMING              185     /* R/W */
>  #define EXT_CSD_POWER_CLASS            187     /* R/W */
>  #define EXT_CSD_REV                    192     /* RO */
> @@ -380,12 +381,14 @@ struct _mmc_csd {
>  #define EXT_CSD_CARD_TYPE_HS400_1_2V   (1<<7)  /* Card can run at 200MHz DDR, 1.2V */
>  #define EXT_CSD_CARD_TYPE_HS400                (EXT_CSD_CARD_TYPE_HS400_1_8V | \
>                                          EXT_CSD_CARD_TYPE_HS400_1_2V)
> +#define EXT_CSD_CARD_TYPE_HS400ES      (1<<8)  /* Card can run at HS400ES */
>
>  #define EXT_CSD_BUS_WIDTH_1    0       /* Card is in 1 bit mode */
>  #define EXT_CSD_BUS_WIDTH_4    1       /* Card is in 4 bit mode */
>  #define EXT_CSD_BUS_WIDTH_8    2       /* Card is in 8 bit mode */
>  #define EXT_CSD_DDR_BUS_WIDTH_4        5       /* Card is in 4 bit DDR mode */
>  #define EXT_CSD_DDR_BUS_WIDTH_8        6       /* Card is in 8 bit DDR mode */
> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)        /* Enhanced strobe mode */

nit: while your code should be OK, it seems better to be BIT(3), since
above is really:

0
BIT(0)
BIT(1)
BIT(2) | BIT(0)
BIT(2) | BIT(1)

...at least that's my understanding...

Re: [PATCH v3 3/5] mmc: core: implement enhanced strobe support

[Shawn Lin]

Hi

在 2016-5-20 8:18, Doug Anderson 写道:
> Hi,
>
> On Tue, May 10, 2016 at 2:09 AM, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>> Controllers use data strobe line to latch data from devices
>> under hs400 mode, but not for cmd line. So since emmc 5.1, JEDEC
>> introduces enhanced strobe mode for latching cmd response from
>> emmc devices to host controllers. This new feature is optional,
>> so it depends both on device's cap and host's cap to decide
>> whether to use it or not.
>
> I don't totally understand it, but two boards I'm working with have
> eMMC devices that claim to be eMMC 5.0 devices but also have support
> for strobe.  Did people just cherry-pick this feature from eMMC 5.1 to
> their eMMC 5.0 devices?

Presumably I guess your "support for strobe" means "support for
enhanced strobe", right?

hs400es is also optional for emmc 5.1, so it's impossible to find a
emmc 5.0 claiming to support hs400es.

Coud you share me the device's ID number on its package or from the
device's manual?


>
>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>>   drivers/mmc/core/bus.c   |  3 +-
>>   drivers/mmc/core/core.c  |  8 +++++
>>   drivers/mmc/core/mmc.c   | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
>>   include/linux/mmc/card.h |  1 +
>>   include/linux/mmc/host.h | 11 +++++++
>>   include/linux/mmc/mmc.h  |  3 ++
>>   6 files changed, 102 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>> index 4bc48f1..7e94b9d 100644
>> --- a/drivers/mmc/core/bus.c
>> +++ b/drivers/mmc/core/bus.c
>> @@ -332,12 +332,13 @@ int mmc_add_card(struct mmc_card *card)
>>                          mmc_card_ddr52(card) ? "DDR " : "",
>>                          type);
>>          } else {
>> -               pr_info("%s: new %s%s%s%s%s card at address %04x\n",
>> +               pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
>>                          mmc_hostname(card->host),
>>                          mmc_card_uhs(card) ? "ultra high speed " :
>>                          (mmc_card_hs(card) ? "high speed " : ""),
>>                          mmc_card_hs400(card) ? "HS400 " :
>>                          (mmc_card_hs200(card) ? "HS200 " : ""),
>> +                       mmc_card_hs400es(card) ? "Enhanced strobe" : "",
>
> Nit: there should be a space before the quote.  Otherwise the message
> runs together, like:
>
> new HS400 Enhanced strobeMMC card at address 0001

Good catch, thanks.

>
>
>>                          mmc_card_ddr52(card) ? "DDR " : "",
>>                          uhs_bus_speed_mode, type, card->rca);
>>          }
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 99275e4..a559501 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -1127,6 +1127,14 @@ void mmc_set_initial_state(struct mmc_host *host)
>>          host->ios.bus_width = MMC_BUS_WIDTH_1;
>>          host->ios.timing = MMC_TIMING_LEGACY;
>>          host->ios.drv_type = 0;
>> +       host->ios.enhanced_strobe = false;
>> +
>> +       /*
>> +        * Make sure we are in non-enhanced strobe mode before we
>> +        * actually enable it in ext_csd.
>> +        */
>> +       if (host->ops->hs400_enhanced_strobe)
>> +               host->ops->hs400_enhanced_strobe(host, &host->ios);
>>
>>          mmc_set_ios(host);
>>   }
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index f99c47e..c2d1981 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -235,6 +235,11 @@ static void mmc_select_card_type(struct mmc_card *card)
>>                  avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
>>          }
>>
>> +       if ((caps2 & MMC_CAP2_HS400_ES) &&
>> +           card->ext_csd.strobe_support &&
>> +           (card_type & EXT_CSD_CARD_TYPE_HS400))
>> +               avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
>> +
>>          card->ext_csd.hs_max_dtr = hs_max_dtr;
>>          card->ext_csd.hs200_max_dtr = hs200_max_dtr;
>>          card->mmc_avail_type = avail_type;
>> @@ -383,6 +388,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>                          mmc_card_set_blockaddr(card);
>>          }
>>
>> +       card->ext_csd.strobe_support = ext_csd[EXT_CSD_STROBE_SUPPORT];
>>          card->ext_csd.raw_card_type = ext_csd[EXT_CSD_CARD_TYPE];
>>          mmc_select_card_type(card);
>>
>> @@ -1216,6 +1222,73 @@ out_err:
>>          return err;
>>   }
>>
>> +static int mmc_select_hs400es(struct mmc_card *card)
>> +{
>> +       struct mmc_host *host = card->host;
>> +       int err = 0;
>> +       u8 val;
>> +
>> +       err = mmc_select_bus_width(card);
>> +       if (IS_ERR_VALUE(err))
>> +               goto out_err;
>> +
>> +       /* Switch card to HS mode */
>> +       err = mmc_select_hs(card);
>> +       if (err) {
>> +               pr_err("%s: switch to high-speed failed, err:%d\n",
>> +                       mmc_hostname(host), err);
>> +               goto out_err;
>> +       }
>> +
>> +       err = mmc_switch_status(card);
>> +       if (err)
>> +               goto out_err;
>> +
>> +       /* Switch card to DDR with strobe bit */
>> +       val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
>
> In the hs400 function earlier in the file the check to confirm that
> the bus width is 8 before doing this.  Should we do that here, too?

yes, will fix it. And we can just check host's cap to see
if it supports 8bit before mmc_select_bus_width because we don't
need to do anything without 8-bit support.

>
>> +       err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> +                        EXT_CSD_BUS_WIDTH,
>> +                        val,
>> +                        card->ext_csd.generic_cmd6_time);
>> +       if (err) {
>> +               pr_err("%s: switch to bus width for hs400es failed, err:%d\n",
>> +                       mmc_hostname(host), err);
>> +               goto out_err;
>> +       }
>> +
>> +       /* Switch card to HS400 */
>> +       val = EXT_CSD_TIMING_HS400 |
>> +             card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
>> +       err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> +                          EXT_CSD_HS_TIMING, val,
>> +                          card->ext_csd.generic_cmd6_time,
>> +                          true, false, true);
>> +       if (err) {
>> +               pr_err("%s: switch to hs400es failed, err:%d\n",
>> +                       mmc_hostname(host), err);
>> +               goto out_err;
>> +       }
>> +
>> +       /* Set host controller to HS400 timing and frequency */
>> +       mmc_set_timing(host, MMC_TIMING_MMC_HS400);
>> +
>> +       /* Controller enable enhanced strobe function */
>> +       host->ios.enhanced_strobe = true;
>> +       if (host->ops->hs400_enhanced_strobe)
>> +               host->ops->hs400_enhanced_strobe(host, &host->ios);
>> +
>> +       err = mmc_switch_status(card);
>> +       if (err)
>> +               goto out_err;
>> +
>> +       return 0;
>> +
>> +out_err:
>> +       pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
>> +              __func__, err);
>> +       return err;
>> +}
>> +
>>   static void mmc_select_driver_type(struct mmc_card *card)
>>   {
>>          int card_drv_type, drive_strength, drv_type;
>> @@ -1297,7 +1370,7 @@ err:
>>   }
>>
>>   /*
>> - * Activate High Speed or HS200 mode if supported.
>> + * Activate High Speed, HS200 or HS400ES mode if supported.
>>    */
>>   static int mmc_select_timing(struct mmc_card *card)
>>   {
>> @@ -1306,7 +1379,9 @@ static int mmc_select_timing(struct mmc_card *card)
>>          if (!mmc_can_ext_csd(card))
>>                  goto bus_speed;
>>
>> -       if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>> +       if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES)
>> +               err = mmc_select_hs400es(card);
>> +       else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>>                  err = mmc_select_hs200(card);
>>          else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
>>                  err = mmc_select_hs(card);
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index eb0151b..22defc2 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -95,6 +95,7 @@ struct mmc_ext_csd {
>>          u8                      raw_partition_support;  /* 160 */
>>          u8                      raw_rpmb_size_mult;     /* 168 */
>>          u8                      raw_erased_mem_count;   /* 181 */
>> +       u8                      strobe_support;         /* 184 */
>>          u8                      raw_ext_csd_structure;  /* 194 */
>>          u8                      raw_card_type;          /* 196 */
>>          u8                      raw_driver_strength;    /* 197 */
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 2a06fb0..f98bd0e 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -19,6 +19,7 @@
>>
>>   #include <linux/mmc/core.h>
>>   #include <linux/mmc/card.h>
>> +#include <linux/mmc/mmc.h>
>>   #include <linux/mmc/pm.h>
>>
>>   struct mmc_ios {
>> @@ -77,6 +78,8 @@ struct mmc_ios {
>>   #define MMC_SET_DRIVER_TYPE_A  1
>>   #define MMC_SET_DRIVER_TYPE_C  2
>>   #define MMC_SET_DRIVER_TYPE_D  3
>> +
>> +       bool enhanced_strobe;                   /* hs400es selection */
>>   };
>>
>>   struct mmc_host_ops {
>> @@ -143,6 +146,9 @@ struct mmc_host_ops {
>>
>>          /* Prepare HS400 target operating frequency depending host driver */
>>          int     (*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios);
>> +       /* Prepare enhanced strobe depending host driver */
>> +       void    (*hs400_enhanced_strobe)(struct mmc_host *host,
>> +                                        struct mmc_ios *ios);
>>          int     (*select_drive_strength)(struct mmc_card *card,
>>                                           unsigned int max_dtr, int host_drv,
>>                                           int card_drv, int *drv_type);
>> @@ -513,6 +519,11 @@ static inline bool mmc_card_hs400(struct mmc_card *card)
>>          return card->host->ios.timing == MMC_TIMING_MMC_HS400;
>>   }
>>
>> +static inline bool mmc_card_hs400es(struct mmc_card *card)
>> +{
>> +       return card->host->ios.enhanced_strobe == true;
>
> nit: never need "== true".  Just:
>
>    return card->host->ios.enhanced_strobe;
>

yes, thanks.

>> +}
>> +
>>   void mmc_retune_timer_stop(struct mmc_host *host);
>>
>>   static inline void mmc_retune_needed(struct mmc_host *host)
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 15f2c4a..c376209 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -297,6 +297,7 @@ struct _mmc_csd {
>>   #define EXT_CSD_PART_CONFIG            179     /* R/W */
>>   #define EXT_CSD_ERASED_MEM_CONT                181     /* RO */
>>   #define EXT_CSD_BUS_WIDTH              183     /* R/W */
>> +#define EXT_CSD_STROBE_SUPPORT         184     /* RO */
>>   #define EXT_CSD_HS_TIMING              185     /* R/W */
>>   #define EXT_CSD_POWER_CLASS            187     /* R/W */
>>   #define EXT_CSD_REV                    192     /* RO */
>> @@ -380,12 +381,14 @@ struct _mmc_csd {
>>   #define EXT_CSD_CARD_TYPE_HS400_1_2V   (1<<7)  /* Card can run at 200MHz DDR, 1.2V */
>>   #define EXT_CSD_CARD_TYPE_HS400                (EXT_CSD_CARD_TYPE_HS400_1_8V | \
>>                                           EXT_CSD_CARD_TYPE_HS400_1_2V)
>> +#define EXT_CSD_CARD_TYPE_HS400ES      (1<<8)  /* Card can run at HS400ES */
>>
>>   #define EXT_CSD_BUS_WIDTH_1    0       /* Card is in 1 bit mode */
>>   #define EXT_CSD_BUS_WIDTH_4    1       /* Card is in 4 bit mode */
>>   #define EXT_CSD_BUS_WIDTH_8    2       /* Card is in 8 bit mode */
>>   #define EXT_CSD_DDR_BUS_WIDTH_4        5       /* Card is in 4 bit DDR mode */
>>   #define EXT_CSD_DDR_BUS_WIDTH_8        6       /* Card is in 8 bit DDR mode */
>> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)        /* Enhanced strobe mode */
>
> nit: while your code should be OK, it seems better to be BIT(3), since
> above is really:
>

I don't unstand here. EXT_CSD_BUS_WIDTH_STROBE is the MSB of
BUS_WIDTH[183] which is a 8-bit reg.


> 0
> BIT(0)
> BIT(1)
> BIT(2) | BIT(0)
> BIT(2) | BIT(1)
>
> ...at least that's my understanding...
>
>
>

Re: [PATCH v3 5/5] mmc: sdhci-of-arasan: implement enhanced strobe callback

[Shawn Lin]

Hi

在 2016-5-20 8:11, Doug Anderson 写道:
> Hi,
>
> On Tue, May 10, 2016 at 2:10 AM, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>> Currently sdhci-arasan 5.1 can support enhanced strobe function,
>> and we now limit it just for "arasan,sdhci-5.1". Add
>> mmc-hs400-enhanced-strobe in DT to enable the function if we'r sure
>
> nit:s/we'r/we're/
>
> [ ... ]
>> @@ -79,6 +81,21 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
>>          }
>>   }
>>
>> +static void sdhci_arasan_hs400_enhanced_strobe(struct mmc_host *mmc,
>> +                                       struct mmc_ios *ios)
>> +{
>> +       u32 vendor;
>> +       struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +       vendor = readl(host->ioaddr + SDHCI_ARASAN_VENDOR_REGISTER);
>> +       if (ios->enhanced_strobe)
>> +               vendor |= VENDOR_ENHANCED_STROBE;
>> +       else
>> +               vendor &= (~VENDOR_ENHANCED_STROBE);
>
> nit: no extra parentheses:
>
>    vendor &= ~VENDOR_ENHANCED_STROBE;
>
>
> Other than the nits this patch looks good to me.  Feel free to add my
> Reviewed-by when nits fixed.

okay, will fix them.

Thanks for your review!

>
> -Doug
>
>
>

Re: [PATCH v3 3/5] mmc: core: implement enhanced strobe support

[Doug Anderson]

Shawn,

On Thu, May 19, 2016 at 6:48 PM, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>>   #define EXT_CSD_BUS_WIDTH_1    0       /* Card is in 1 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_4    1       /* Card is in 4 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_8    2       /* Card is in 8 bit mode */
>>>   #define EXT_CSD_DDR_BUS_WIDTH_4        5       /* Card is in 4 bit DDR
>>> mode */
>>>   #define EXT_CSD_DDR_BUS_WIDTH_8        6       /* Card is in 8 bit DDR
>>> mode */
>>> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)        /* Enhanced strobe mode
>>> */
>>
>>
>> nit: while your code should be OK, it seems better to be BIT(3), since
>> above is really:
>>
>
> I don't unstand here. EXT_CSD_BUS_WIDTH_STROBE is the MSB of
> BUS_WIDTH[183] which is a 8-bit reg.

I totally misunderstood.  You are correct.

-Doug

Re: [PATCH v3 3/5] mmc: core: implement enhanced strobe support

[Jaehoon Chung]

On 05/10/2016 06:09 PM, Shawn Lin wrote:
> Controllers use data strobe line to latch data from devices
> under hs400 mode, but not for cmd line. So since emmc 5.1, JEDEC
> introduces enhanced strobe mode for latching cmd response from
> emmc devices to host controllers. This new feature is optional,
> so it depends both on device's cap and host's cap to decide
> whether to use it or not.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/mmc/core/bus.c   |  3 +-
>  drivers/mmc/core/core.c  |  8 +++++
>  drivers/mmc/core/mmc.c   | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
>  include/linux/mmc/card.h |  1 +
>  include/linux/mmc/host.h | 11 +++++++
>  include/linux/mmc/mmc.h  |  3 ++
>  6 files changed, 102 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 4bc48f1..7e94b9d 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -332,12 +332,13 @@ int mmc_add_card(struct mmc_card *card)
>  			mmc_card_ddr52(card) ? "DDR " : "",
>  			type);
>  	} else {
> -		pr_info("%s: new %s%s%s%s%s card at address %04x\n",
> +		pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
>  			mmc_hostname(card->host),
>  			mmc_card_uhs(card) ? "ultra high speed " :
>  			(mmc_card_hs(card) ? "high speed " : ""),
>  			mmc_card_hs400(card) ? "HS400 " :
>  			(mmc_card_hs200(card) ? "HS200 " : ""),
> +			mmc_card_hs400es(card) ? "Enhanced strobe" : "",
>  			mmc_card_ddr52(card) ? "DDR " : "",
>  			uhs_bus_speed_mode, type, card->rca);
>  	}
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 99275e4..a559501 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1127,6 +1127,14 @@ void mmc_set_initial_state(struct mmc_host *host)
>  	host->ios.bus_width = MMC_BUS_WIDTH_1;
>  	host->ios.timing = MMC_TIMING_LEGACY;
>  	host->ios.drv_type = 0;
> +	host->ios.enhanced_strobe = false;
> +
> +	/*
> +	 * Make sure we are in non-enhanced strobe mode before we
> +	 * actually enable it in ext_csd.
> +	 */
> +	if (host->ops->hs400_enhanced_strobe)
> +		host->ops->hs400_enhanced_strobe(host, &host->ios);
>  
>  	mmc_set_ios(host);
>  }
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f99c47e..c2d1981 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -235,6 +235,11 @@ static void mmc_select_card_type(struct mmc_card *card)
>  		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
>  	}
>  
> +	if ((caps2 & MMC_CAP2_HS400_ES) &&
> +	    card->ext_csd.strobe_support &&
> +	    (card_type & EXT_CSD_CARD_TYPE_HS400))
> +		avail_type |= EXT_CSD_CARD_TYPE_HS400ES;

Does it need to check whether support HS400_1.8V/1.2V or not?
It should be more stable than now.

if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) {
	if ((cap2 & MMC_CAP2_HS400_ES) && ...
}

how about this?

Best Regards,
Jaehoon Chung

> +
>  	card->ext_csd.hs_max_dtr = hs_max_dtr;
>  	card->ext_csd.hs200_max_dtr = hs200_max_dtr;
>  	card->mmc_avail_type = avail_type;
> @@ -383,6 +388,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
>  			mmc_card_set_blockaddr(card);
>  	}
>  
> +	card->ext_csd.strobe_support = ext_csd[EXT_CSD_STROBE_SUPPORT];
>  	card->ext_csd.raw_card_type = ext_csd[EXT_CSD_CARD_TYPE];
>  	mmc_select_card_type(card);
>  
> @@ -1216,6 +1222,73 @@ out_err:
>  	return err;
>  }
>  
> +static int mmc_select_hs400es(struct mmc_card *card)
> +{
> +	struct mmc_host *host = card->host;
> +	int err = 0;
> +	u8 val;
> +
> +	err = mmc_select_bus_width(card);
> +	if (IS_ERR_VALUE(err))
> +		goto out_err;
> +
> +	/* Switch card to HS mode */
> +	err = mmc_select_hs(card);
> +	if (err) {
> +		pr_err("%s: switch to high-speed failed, err:%d\n",
> +			mmc_hostname(host), err);
> +		goto out_err;
> +	}
> +
> +	err = mmc_switch_status(card);
> +	if (err)
> +		goto out_err;
> +
> +	/* Switch card to DDR with strobe bit */
> +	val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
> +	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +			 EXT_CSD_BUS_WIDTH,
> +			 val,
> +			 card->ext_csd.generic_cmd6_time);
> +	if (err) {
> +		pr_err("%s: switch to bus width for hs400es failed, err:%d\n",
> +			mmc_hostname(host), err);
> +		goto out_err;
> +	}
> +
> +	/* Switch card to HS400 */
> +	val = EXT_CSD_TIMING_HS400 |
> +	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
> +	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +			   EXT_CSD_HS_TIMING, val,
> +			   card->ext_csd.generic_cmd6_time,
> +			   true, false, true);
> +	if (err) {
> +		pr_err("%s: switch to hs400es failed, err:%d\n",
> +			mmc_hostname(host), err);
> +		goto out_err;
> +	}
> +
> +	/* Set host controller to HS400 timing and frequency */
> +	mmc_set_timing(host, MMC_TIMING_MMC_HS400);
> +
> +	/* Controller enable enhanced strobe function */
> +	host->ios.enhanced_strobe = true;
> +	if (host->ops->hs400_enhanced_strobe)
> +		host->ops->hs400_enhanced_strobe(host, &host->ios);
> +
> +	err = mmc_switch_status(card);
> +	if (err)
> +		goto out_err;
> +
> +	return 0;
> +
> +out_err:
> +	pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
> +	       __func__, err);
> +	return err;
> +}
> +
>  static void mmc_select_driver_type(struct mmc_card *card)
>  {
>  	int card_drv_type, drive_strength, drv_type;
> @@ -1297,7 +1370,7 @@ err:
>  }
>  
>  /*
> - * Activate High Speed or HS200 mode if supported.
> + * Activate High Speed, HS200 or HS400ES mode if supported.
>   */
>  static int mmc_select_timing(struct mmc_card *card)
>  {
> @@ -1306,7 +1379,9 @@ static int mmc_select_timing(struct mmc_card *card)
>  	if (!mmc_can_ext_csd(card))
>  		goto bus_speed;
>  
> -	if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
> +	if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES)
> +		err = mmc_select_hs400es(card);
> +	else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>  		err = mmc_select_hs200(card);
>  	else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
>  		err = mmc_select_hs(card);
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index eb0151b..22defc2 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -95,6 +95,7 @@ struct mmc_ext_csd {
>  	u8			raw_partition_support;	/* 160 */
>  	u8			raw_rpmb_size_mult;	/* 168 */
>  	u8			raw_erased_mem_count;	/* 181 */
> +	u8			strobe_support;		/* 184 */
>  	u8			raw_ext_csd_structure;	/* 194 */
>  	u8			raw_card_type;		/* 196 */
>  	u8			raw_driver_strength;	/* 197 */
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 2a06fb0..f98bd0e 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -19,6 +19,7 @@
>  
>  #include <linux/mmc/core.h>
>  #include <linux/mmc/card.h>
> +#include <linux/mmc/mmc.h>
>  #include <linux/mmc/pm.h>
>  
>  struct mmc_ios {
> @@ -77,6 +78,8 @@ struct mmc_ios {
>  #define MMC_SET_DRIVER_TYPE_A	1
>  #define MMC_SET_DRIVER_TYPE_C	2
>  #define MMC_SET_DRIVER_TYPE_D	3
> +
> +	bool enhanced_strobe;			/* hs400es selection */
>  };
>  
>  struct mmc_host_ops {
> @@ -143,6 +146,9 @@ struct mmc_host_ops {
>  
>  	/* Prepare HS400 target operating frequency depending host driver */
>  	int	(*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios);
> +	/* Prepare enhanced strobe depending host driver */
> +	void	(*hs400_enhanced_strobe)(struct mmc_host *host,
> +					 struct mmc_ios *ios);
>  	int	(*select_drive_strength)(struct mmc_card *card,
>  					 unsigned int max_dtr, int host_drv,
>  					 int card_drv, int *drv_type);
> @@ -513,6 +519,11 @@ static inline bool mmc_card_hs400(struct mmc_card *card)
>  	return card->host->ios.timing == MMC_TIMING_MMC_HS400;
>  }
>  
> +static inline bool mmc_card_hs400es(struct mmc_card *card)
> +{
> +	return card->host->ios.enhanced_strobe == true;
> +}
> +
>  void mmc_retune_timer_stop(struct mmc_host *host);
>  
>  static inline void mmc_retune_needed(struct mmc_host *host)
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 15f2c4a..c376209 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -297,6 +297,7 @@ struct _mmc_csd {
>  #define EXT_CSD_PART_CONFIG		179	/* R/W */
>  #define EXT_CSD_ERASED_MEM_CONT		181	/* RO */
>  #define EXT_CSD_BUS_WIDTH		183	/* R/W */
> +#define EXT_CSD_STROBE_SUPPORT		184	/* RO */
>  #define EXT_CSD_HS_TIMING		185	/* R/W */
>  #define EXT_CSD_POWER_CLASS		187	/* R/W */
>  #define EXT_CSD_REV			192	/* RO */
> @@ -380,12 +381,14 @@ struct _mmc_csd {
>  #define EXT_CSD_CARD_TYPE_HS400_1_2V	(1<<7)	/* Card can run at 200MHz DDR, 1.2V */
>  #define EXT_CSD_CARD_TYPE_HS400		(EXT_CSD_CARD_TYPE_HS400_1_8V | \
>  					 EXT_CSD_CARD_TYPE_HS400_1_2V)
> +#define EXT_CSD_CARD_TYPE_HS400ES	(1<<8)	/* Card can run at HS400ES */
>  
>  #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
>  #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
>  #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
>  #define EXT_CSD_DDR_BUS_WIDTH_4	5	/* Card is in 4 bit DDR mode */
>  #define EXT_CSD_DDR_BUS_WIDTH_8	6	/* Card is in 8 bit DDR mode */
> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)	/* Enhanced strobe mode */
>  
>  #define EXT_CSD_TIMING_BC	0	/* Backwards compatility */
>  #define EXT_CSD_TIMING_HS	1	/* High speed */
> 

Re: [PATCH v3 3/5] mmc: core: implement enhanced strobe support

[Shawn Lin]

Hi

在 2016-5-20 10:45, Jaehoon Chung 写道:
> On 05/10/2016 06:09 PM, Shawn Lin wrote:
>> Controllers use data strobe line to latch data from devices
>> under hs400 mode, but not for cmd line. So since emmc 5.1, JEDEC
>> introduces enhanced strobe mode for latching cmd response from
>> emmc devices to host controllers. This new feature is optional,
>> so it depends both on device's cap and host's cap to decide
>> whether to use it or not.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>>   drivers/mmc/core/bus.c   |  3 +-
>>   drivers/mmc/core/core.c  |  8 +++++
>>   drivers/mmc/core/mmc.c   | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
>>   include/linux/mmc/card.h |  1 +
>>   include/linux/mmc/host.h | 11 +++++++
>>   include/linux/mmc/mmc.h  |  3 ++
>>   6 files changed, 102 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>> index 4bc48f1..7e94b9d 100644
>> --- a/drivers/mmc/core/bus.c
>> +++ b/drivers/mmc/core/bus.c
>> @@ -332,12 +332,13 @@ int mmc_add_card(struct mmc_card *card)
>>   			mmc_card_ddr52(card) ? "DDR " : "",
>>   			type);
>>   	} else {
>> -		pr_info("%s: new %s%s%s%s%s card at address %04x\n",
>> +		pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
>>   			mmc_hostname(card->host),
>>   			mmc_card_uhs(card) ? "ultra high speed " :
>>   			(mmc_card_hs(card) ? "high speed " : ""),
>>   			mmc_card_hs400(card) ? "HS400 " :
>>   			(mmc_card_hs200(card) ? "HS200 " : ""),
>> +			mmc_card_hs400es(card) ? "Enhanced strobe" : "",
>>   			mmc_card_ddr52(card) ? "DDR " : "",
>>   			uhs_bus_speed_mode, type, card->rca);
>>   	}
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 99275e4..a559501 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -1127,6 +1127,14 @@ void mmc_set_initial_state(struct mmc_host *host)
>>   	host->ios.bus_width = MMC_BUS_WIDTH_1;
>>   	host->ios.timing = MMC_TIMING_LEGACY;
>>   	host->ios.drv_type = 0;
>> +	host->ios.enhanced_strobe = false;
>> +
>> +	/*
>> +	 * Make sure we are in non-enhanced strobe mode before we
>> +	 * actually enable it in ext_csd.
>> +	 */
>> +	if (host->ops->hs400_enhanced_strobe)
>> +		host->ops->hs400_enhanced_strobe(host, &host->ios);
>>
>>   	mmc_set_ios(host);
>>   }
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index f99c47e..c2d1981 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -235,6 +235,11 @@ static void mmc_select_card_type(struct mmc_card *card)
>>   		avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
>>   	}
>>
>> +	if ((caps2 & MMC_CAP2_HS400_ES) &&
>> +	    card->ext_csd.strobe_support &&
>> +	    (card_type & EXT_CSD_CARD_TYPE_HS400))
>> +		avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
>
> Does it need to check whether support HS400_1.8V/1.2V or not?
> It should be more stable than now.
>

Ahh, right.
I need to add "avail_type & EXT_CSD_CARD_TYPE_HS400" instead of
"card_type & EXT_CSD_CARD_TYPE_HS400"... because if "avail_type &
EXT_CSD_CARD_TYPE_HS400" is true, it implies that "card_type & 
EXT_CSD_CARD_TYPE_HS400" must be true.


Thansk for catching this!


> if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) {
> 	if ((cap2 & MMC_CAP2_HS400_ES) && ...
> }
>
> how about this?
>
> Best Regards,
> Jaehoon Chung
>
>> +
>>   	card->ext_csd.hs_max_dtr = hs_max_dtr;
>>   	card->ext_csd.hs200_max_dtr = hs200_max_dtr;
>>   	card->mmc_avail_type = avail_type;
>> @@ -383,6 +388,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>   			mmc_card_set_blockaddr(card);
>>   	}
>>
>> +	card->ext_csd.strobe_support = ext_csd[EXT_CSD_STROBE_SUPPORT];
>>   	card->ext_csd.raw_card_type = ext_csd[EXT_CSD_CARD_TYPE];
>>   	mmc_select_card_type(card);
>>
>> @@ -1216,6 +1222,73 @@ out_err:
>>   	return err;
>>   }
>>
>> +static int mmc_select_hs400es(struct mmc_card *card)
>> +{
>> +	struct mmc_host *host = card->host;
>> +	int err = 0;
>> +	u8 val;
>> +
>> +	err = mmc_select_bus_width(card);
>> +	if (IS_ERR_VALUE(err))
>> +		goto out_err;
>> +
>> +	/* Switch card to HS mode */
>> +	err = mmc_select_hs(card);
>> +	if (err) {
>> +		pr_err("%s: switch to high-speed failed, err:%d\n",
>> +			mmc_hostname(host), err);
>> +		goto out_err;
>> +	}
>> +
>> +	err = mmc_switch_status(card);
>> +	if (err)
>> +		goto out_err;
>> +
>> +	/* Switch card to DDR with strobe bit */
>> +	val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
>> +	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> +			 EXT_CSD_BUS_WIDTH,
>> +			 val,
>> +			 card->ext_csd.generic_cmd6_time);
>> +	if (err) {
>> +		pr_err("%s: switch to bus width for hs400es failed, err:%d\n",
>> +			mmc_hostname(host), err);
>> +		goto out_err;
>> +	}
>> +
>> +	/* Switch card to HS400 */
>> +	val = EXT_CSD_TIMING_HS400 |
>> +	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
>> +	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> +			   EXT_CSD_HS_TIMING, val,
>> +			   card->ext_csd.generic_cmd6_time,
>> +			   true, false, true);
>> +	if (err) {
>> +		pr_err("%s: switch to hs400es failed, err:%d\n",
>> +			mmc_hostname(host), err);
>> +		goto out_err;
>> +	}
>> +
>> +	/* Set host controller to HS400 timing and frequency */
>> +	mmc_set_timing(host, MMC_TIMING_MMC_HS400);
>> +
>> +	/* Controller enable enhanced strobe function */
>> +	host->ios.enhanced_strobe = true;
>> +	if (host->ops->hs400_enhanced_strobe)
>> +		host->ops->hs400_enhanced_strobe(host, &host->ios);
>> +
>> +	err = mmc_switch_status(card);
>> +	if (err)
>> +		goto out_err;
>> +
>> +	return 0;
>> +
>> +out_err:
>> +	pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
>> +	       __func__, err);
>> +	return err;
>> +}
>> +
>>   static void mmc_select_driver_type(struct mmc_card *card)
>>   {
>>   	int card_drv_type, drive_strength, drv_type;
>> @@ -1297,7 +1370,7 @@ err:
>>   }
>>
>>   /*
>> - * Activate High Speed or HS200 mode if supported.
>> + * Activate High Speed, HS200 or HS400ES mode if supported.
>>    */
>>   static int mmc_select_timing(struct mmc_card *card)
>>   {
>> @@ -1306,7 +1379,9 @@ static int mmc_select_timing(struct mmc_card *card)
>>   	if (!mmc_can_ext_csd(card))
>>   		goto bus_speed;
>>
>> -	if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>> +	if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES)
>> +		err = mmc_select_hs400es(card);
>> +	else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>>   		err = mmc_select_hs200(card);
>>   	else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
>>   		err = mmc_select_hs(card);
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index eb0151b..22defc2 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -95,6 +95,7 @@ struct mmc_ext_csd {
>>   	u8			raw_partition_support;	/* 160 */
>>   	u8			raw_rpmb_size_mult;	/* 168 */
>>   	u8			raw_erased_mem_count;	/* 181 */
>> +	u8			strobe_support;		/* 184 */
>>   	u8			raw_ext_csd_structure;	/* 194 */
>>   	u8			raw_card_type;		/* 196 */
>>   	u8			raw_driver_strength;	/* 197 */
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 2a06fb0..f98bd0e 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -19,6 +19,7 @@
>>
>>   #include <linux/mmc/core.h>
>>   #include <linux/mmc/card.h>
>> +#include <linux/mmc/mmc.h>
>>   #include <linux/mmc/pm.h>
>>
>>   struct mmc_ios {
>> @@ -77,6 +78,8 @@ struct mmc_ios {
>>   #define MMC_SET_DRIVER_TYPE_A	1
>>   #define MMC_SET_DRIVER_TYPE_C	2
>>   #define MMC_SET_DRIVER_TYPE_D	3
>> +
>> +	bool enhanced_strobe;			/* hs400es selection */
>>   };
>>
>>   struct mmc_host_ops {
>> @@ -143,6 +146,9 @@ struct mmc_host_ops {
>>
>>   	/* Prepare HS400 target operating frequency depending host driver */
>>   	int	(*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios);
>> +	/* Prepare enhanced strobe depending host driver */
>> +	void	(*hs400_enhanced_strobe)(struct mmc_host *host,
>> +					 struct mmc_ios *ios);
>>   	int	(*select_drive_strength)(struct mmc_card *card,
>>   					 unsigned int max_dtr, int host_drv,
>>   					 int card_drv, int *drv_type);
>> @@ -513,6 +519,11 @@ static inline bool mmc_card_hs400(struct mmc_card *card)
>>   	return card->host->ios.timing == MMC_TIMING_MMC_HS400;
>>   }
>>
>> +static inline bool mmc_card_hs400es(struct mmc_card *card)
>> +{
>> +	return card->host->ios.enhanced_strobe == true;
>> +}
>> +
>>   void mmc_retune_timer_stop(struct mmc_host *host);
>>
>>   static inline void mmc_retune_needed(struct mmc_host *host)
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 15f2c4a..c376209 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -297,6 +297,7 @@ struct _mmc_csd {
>>   #define EXT_CSD_PART_CONFIG		179	/* R/W */
>>   #define EXT_CSD_ERASED_MEM_CONT		181	/* RO */
>>   #define EXT_CSD_BUS_WIDTH		183	/* R/W */
>> +#define EXT_CSD_STROBE_SUPPORT		184	/* RO */
>>   #define EXT_CSD_HS_TIMING		185	/* R/W */
>>   #define EXT_CSD_POWER_CLASS		187	/* R/W */
>>   #define EXT_CSD_REV			192	/* RO */
>> @@ -380,12 +381,14 @@ struct _mmc_csd {
>>   #define EXT_CSD_CARD_TYPE_HS400_1_2V	(1<<7)	/* Card can run at 200MHz DDR, 1.2V */
>>   #define EXT_CSD_CARD_TYPE_HS400		(EXT_CSD_CARD_TYPE_HS400_1_8V | \
>>   					 EXT_CSD_CARD_TYPE_HS400_1_2V)
>> +#define EXT_CSD_CARD_TYPE_HS400ES	(1<<8)	/* Card can run at HS400ES */
>>
>>   #define EXT_CSD_BUS_WIDTH_1	0	/* Card is in 1 bit mode */
>>   #define EXT_CSD_BUS_WIDTH_4	1	/* Card is in 4 bit mode */
>>   #define EXT_CSD_BUS_WIDTH_8	2	/* Card is in 8 bit mode */
>>   #define EXT_CSD_DDR_BUS_WIDTH_4	5	/* Card is in 4 bit DDR mode */
>>   #define EXT_CSD_DDR_BUS_WIDTH_8	6	/* Card is in 8 bit DDR mode */
>> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)	/* Enhanced strobe mode */
>>
>>   #define EXT_CSD_TIMING_BC	0	/* Backwards compatility */
>>   #define EXT_CSD_TIMING_HS	1	/* High speed */
>>
>
>
>
>

Re: [PATCH v3 3/5] mmc: core: implement enhanced strobe support

[Jaehoon Chung]

On 05/20/2016 12:15 PM, Shawn Lin wrote:
> Hi
> 
> 在 2016-5-20 10:45, Jaehoon Chung 写道:
>> On 05/10/2016 06:09 PM, Shawn Lin wrote:
>>> Controllers use data strobe line to latch data from devices
>>> under hs400 mode, but not for cmd line. So since emmc 5.1, JEDEC
>>> introduces enhanced strobe mode for latching cmd response from
>>> emmc devices to host controllers. This new feature is optional,
>>> so it depends both on device's cap and host's cap to decide
>>> whether to use it or not.
>>>
>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>> ---
>>>
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>   drivers/mmc/core/bus.c   |  3 +-
>>>   drivers/mmc/core/core.c  |  8 +++++
>>>   drivers/mmc/core/mmc.c   | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
>>>   include/linux/mmc/card.h |  1 +
>>>   include/linux/mmc/host.h | 11 +++++++
>>>   include/linux/mmc/mmc.h  |  3 ++
>>>   6 files changed, 102 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>>> index 4bc48f1..7e94b9d 100644
>>> --- a/drivers/mmc/core/bus.c
>>> +++ b/drivers/mmc/core/bus.c
>>> @@ -332,12 +332,13 @@ int mmc_add_card(struct mmc_card *card)
>>>               mmc_card_ddr52(card) ? "DDR " : "",
>>>               type);
>>>       } else {
>>> -        pr_info("%s: new %s%s%s%s%s card at address %04x\n",
>>> +        pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
>>>               mmc_hostname(card->host),
>>>               mmc_card_uhs(card) ? "ultra high speed " :
>>>               (mmc_card_hs(card) ? "high speed " : ""),
>>>               mmc_card_hs400(card) ? "HS400 " :
>>>               (mmc_card_hs200(card) ? "HS200 " : ""),
>>> +            mmc_card_hs400es(card) ? "Enhanced strobe" : "",
>>>               mmc_card_ddr52(card) ? "DDR " : "",
>>>               uhs_bus_speed_mode, type, card->rca);
>>>       }
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index 99275e4..a559501 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -1127,6 +1127,14 @@ void mmc_set_initial_state(struct mmc_host *host)
>>>       host->ios.bus_width = MMC_BUS_WIDTH_1;
>>>       host->ios.timing = MMC_TIMING_LEGACY;
>>>       host->ios.drv_type = 0;
>>> +    host->ios.enhanced_strobe = false;
>>> +
>>> +    /*
>>> +     * Make sure we are in non-enhanced strobe mode before we
>>> +     * actually enable it in ext_csd.
>>> +     */
>>> +    if (host->ops->hs400_enhanced_strobe)
>>> +        host->ops->hs400_enhanced_strobe(host, &host->ios);
>>>
>>>       mmc_set_ios(host);
>>>   }
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index f99c47e..c2d1981 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -235,6 +235,11 @@ static void mmc_select_card_type(struct mmc_card *card)
>>>           avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
>>>       }
>>>
>>> +    if ((caps2 & MMC_CAP2_HS400_ES) &&
>>> +        card->ext_csd.strobe_support &&
>>> +        (card_type & EXT_CSD_CARD_TYPE_HS400))
>>> +        avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
>>
>> Does it need to check whether support HS400_1.8V/1.2V or not?
>> It should be more stable than now.
>>
> 
> Ahh, right.
> I need to add "avail_type & EXT_CSD_CARD_TYPE_HS400" instead of
> "card_type & EXT_CSD_CARD_TYPE_HS400"... because if "avail_type &
> EXT_CSD_CARD_TYPE_HS400" is true, it implies that "card_type & EXT_CSD_CARD_TYPE_HS400" must be true.

Otherwise, you can check or add the capabilities in the parse_of_mmc().
It can choose according to your preference. :)

Anyway, i have tested your patches..If you send the patch V4, i will also test.

Best Regards,
Jaehoon Chung

> 
> 
> Thansk for catching this!
> 
> 
>> if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) {
>>     if ((cap2 & MMC_CAP2_HS400_ES) && ...
>> }
>>
>> how about this?
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> +
>>>       card->ext_csd.hs_max_dtr = hs_max_dtr;
>>>       card->ext_csd.hs200_max_dtr = hs200_max_dtr;
>>>       card->mmc_avail_type = avail_type;
>>> @@ -383,6 +388,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>               mmc_card_set_blockaddr(card);
>>>       }
>>>
>>> +    card->ext_csd.strobe_support = ext_csd[EXT_CSD_STROBE_SUPPORT];
>>>       card->ext_csd.raw_card_type = ext_csd[EXT_CSD_CARD_TYPE];
>>>       mmc_select_card_type(card);
>>>
>>> @@ -1216,6 +1222,73 @@ out_err:
>>>       return err;
>>>   }
>>>
>>> +static int mmc_select_hs400es(struct mmc_card *card)
>>> +{
>>> +    struct mmc_host *host = card->host;
>>> +    int err = 0;
>>> +    u8 val;
>>> +
>>> +    err = mmc_select_bus_width(card);
>>> +    if (IS_ERR_VALUE(err))
>>> +        goto out_err;
>>> +
>>> +    /* Switch card to HS mode */
>>> +    err = mmc_select_hs(card);
>>> +    if (err) {
>>> +        pr_err("%s: switch to high-speed failed, err:%d\n",
>>> +            mmc_hostname(host), err);
>>> +        goto out_err;
>>> +    }
>>> +
>>> +    err = mmc_switch_status(card);
>>> +    if (err)
>>> +        goto out_err;
>>> +
>>> +    /* Switch card to DDR with strobe bit */
>>> +    val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
>>> +    err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>> +             EXT_CSD_BUS_WIDTH,
>>> +             val,
>>> +             card->ext_csd.generic_cmd6_time);
>>> +    if (err) {
>>> +        pr_err("%s: switch to bus width for hs400es failed, err:%d\n",
>>> +            mmc_hostname(host), err);
>>> +        goto out_err;
>>> +    }
>>> +
>>> +    /* Switch card to HS400 */
>>> +    val = EXT_CSD_TIMING_HS400 |
>>> +          card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
>>> +    err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>> +               EXT_CSD_HS_TIMING, val,
>>> +               card->ext_csd.generic_cmd6_time,
>>> +               true, false, true);
>>> +    if (err) {
>>> +        pr_err("%s: switch to hs400es failed, err:%d\n",
>>> +            mmc_hostname(host), err);
>>> +        goto out_err;
>>> +    }
>>> +
>>> +    /* Set host controller to HS400 timing and frequency */
>>> +    mmc_set_timing(host, MMC_TIMING_MMC_HS400);
>>> +
>>> +    /* Controller enable enhanced strobe function */
>>> +    host->ios.enhanced_strobe = true;
>>> +    if (host->ops->hs400_enhanced_strobe)
>>> +        host->ops->hs400_enhanced_strobe(host, &host->ios);
>>> +
>>> +    err = mmc_switch_status(card);
>>> +    if (err)
>>> +        goto out_err;
>>> +
>>> +    return 0;
>>> +
>>> +out_err:
>>> +    pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
>>> +           __func__, err);
>>> +    return err;
>>> +}
>>> +
>>>   static void mmc_select_driver_type(struct mmc_card *card)
>>>   {
>>>       int card_drv_type, drive_strength, drv_type;
>>> @@ -1297,7 +1370,7 @@ err:
>>>   }
>>>
>>>   /*
>>> - * Activate High Speed or HS200 mode if supported.
>>> + * Activate High Speed, HS200 or HS400ES mode if supported.
>>>    */
>>>   static int mmc_select_timing(struct mmc_card *card)
>>>   {
>>> @@ -1306,7 +1379,9 @@ static int mmc_select_timing(struct mmc_card *card)
>>>       if (!mmc_can_ext_csd(card))
>>>           goto bus_speed;
>>>
>>> -    if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>>> +    if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES)
>>> +        err = mmc_select_hs400es(card);
>>> +    else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>>>           err = mmc_select_hs200(card);
>>>       else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
>>>           err = mmc_select_hs(card);
>>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>>> index eb0151b..22defc2 100644
>>> --- a/include/linux/mmc/card.h
>>> +++ b/include/linux/mmc/card.h
>>> @@ -95,6 +95,7 @@ struct mmc_ext_csd {
>>>       u8            raw_partition_support;    /* 160 */
>>>       u8            raw_rpmb_size_mult;    /* 168 */
>>>       u8            raw_erased_mem_count;    /* 181 */
>>> +    u8            strobe_support;        /* 184 */
>>>       u8            raw_ext_csd_structure;    /* 194 */
>>>       u8            raw_card_type;        /* 196 */
>>>       u8            raw_driver_strength;    /* 197 */
>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>> index 2a06fb0..f98bd0e 100644
>>> --- a/include/linux/mmc/host.h
>>> +++ b/include/linux/mmc/host.h
>>> @@ -19,6 +19,7 @@
>>>
>>>   #include <linux/mmc/core.h>
>>>   #include <linux/mmc/card.h>
>>> +#include <linux/mmc/mmc.h>
>>>   #include <linux/mmc/pm.h>
>>>
>>>   struct mmc_ios {
>>> @@ -77,6 +78,8 @@ struct mmc_ios {
>>>   #define MMC_SET_DRIVER_TYPE_A    1
>>>   #define MMC_SET_DRIVER_TYPE_C    2
>>>   #define MMC_SET_DRIVER_TYPE_D    3
>>> +
>>> +    bool enhanced_strobe;            /* hs400es selection */
>>>   };
>>>
>>>   struct mmc_host_ops {
>>> @@ -143,6 +146,9 @@ struct mmc_host_ops {
>>>
>>>       /* Prepare HS400 target operating frequency depending host driver */
>>>       int    (*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios);
>>> +    /* Prepare enhanced strobe depending host driver */
>>> +    void    (*hs400_enhanced_strobe)(struct mmc_host *host,
>>> +                     struct mmc_ios *ios);
>>>       int    (*select_drive_strength)(struct mmc_card *card,
>>>                        unsigned int max_dtr, int host_drv,
>>>                        int card_drv, int *drv_type);
>>> @@ -513,6 +519,11 @@ static inline bool mmc_card_hs400(struct mmc_card *card)
>>>       return card->host->ios.timing == MMC_TIMING_MMC_HS400;
>>>   }
>>>
>>> +static inline bool mmc_card_hs400es(struct mmc_card *card)
>>> +{
>>> +    return card->host->ios.enhanced_strobe == true;
>>> +}
>>> +
>>>   void mmc_retune_timer_stop(struct mmc_host *host);
>>>
>>>   static inline void mmc_retune_needed(struct mmc_host *host)
>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>> index 15f2c4a..c376209 100644
>>> --- a/include/linux/mmc/mmc.h
>>> +++ b/include/linux/mmc/mmc.h
>>> @@ -297,6 +297,7 @@ struct _mmc_csd {
>>>   #define EXT_CSD_PART_CONFIG        179    /* R/W */
>>>   #define EXT_CSD_ERASED_MEM_CONT        181    /* RO */
>>>   #define EXT_CSD_BUS_WIDTH        183    /* R/W */
>>> +#define EXT_CSD_STROBE_SUPPORT        184    /* RO */
>>>   #define EXT_CSD_HS_TIMING        185    /* R/W */
>>>   #define EXT_CSD_POWER_CLASS        187    /* R/W */
>>>   #define EXT_CSD_REV            192    /* RO */
>>> @@ -380,12 +381,14 @@ struct _mmc_csd {
>>>   #define EXT_CSD_CARD_TYPE_HS400_1_2V    (1<<7)    /* Card can run at 200MHz DDR, 1.2V */
>>>   #define EXT_CSD_CARD_TYPE_HS400        (EXT_CSD_CARD_TYPE_HS400_1_8V | \
>>>                        EXT_CSD_CARD_TYPE_HS400_1_2V)
>>> +#define EXT_CSD_CARD_TYPE_HS400ES    (1<<8)    /* Card can run at HS400ES */
>>>
>>>   #define EXT_CSD_BUS_WIDTH_1    0    /* Card is in 1 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_4    1    /* Card is in 4 bit mode */
>>>   #define EXT_CSD_BUS_WIDTH_8    2    /* Card is in 8 bit mode */
>>>   #define EXT_CSD_DDR_BUS_WIDTH_4    5    /* Card is in 4 bit DDR mode */
>>>   #define EXT_CSD_DDR_BUS_WIDTH_8    6    /* Card is in 8 bit DDR mode */
>>> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)    /* Enhanced strobe mode */
>>>
>>>   #define EXT_CSD_TIMING_BC    0    /* Backwards compatility */
>>>   #define EXT_CSD_TIMING_HS    1    /* High speed */
>>>
>>
>>
>>
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH v3 3/5] mmc: core: implement enhanced strobe support

[Shawn Lin]

Hi

在 2016-5-20 11:54, Jaehoon Chung 写道:
> On 05/20/2016 12:15 PM, Shawn Lin wrote:
>> Hi
>>
>> 在 2016-5-20 10:45, Jaehoon Chung 写道:
>>> On 05/10/2016 06:09 PM, Shawn Lin wrote:
>>>> Controllers use data strobe line to latch data from devices
>>>> under hs400 mode, but not for cmd line. So since emmc 5.1, JEDEC
>>>> introduces enhanced strobe mode for latching cmd response from
>>>> emmc devices to host controllers. This new feature is optional,
>>>> so it depends both on device's cap and host's cap to decide
>>>> whether to use it or not.
>>>>
>>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>> ---
>>>>
>>>> Changes in v3: None
>>>> Changes in v2: None
>>>>
>>>>    drivers/mmc/core/bus.c   |  3 +-
>>>>    drivers/mmc/core/core.c  |  8 +++++
>>>>    drivers/mmc/core/mmc.c   | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
>>>>    include/linux/mmc/card.h |  1 +
>>>>    include/linux/mmc/host.h | 11 +++++++
>>>>    include/linux/mmc/mmc.h  |  3 ++
>>>>    6 files changed, 102 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>>>> index 4bc48f1..7e94b9d 100644
>>>> --- a/drivers/mmc/core/bus.c
>>>> +++ b/drivers/mmc/core/bus.c
>>>> @@ -332,12 +332,13 @@ int mmc_add_card(struct mmc_card *card)
>>>>                mmc_card_ddr52(card) ? "DDR " : "",
>>>>                type);
>>>>        } else {
>>>> -        pr_info("%s: new %s%s%s%s%s card at address %04x\n",
>>>> +        pr_info("%s: new %s%s%s%s%s%s card at address %04x\n",
>>>>                mmc_hostname(card->host),
>>>>                mmc_card_uhs(card) ? "ultra high speed " :
>>>>                (mmc_card_hs(card) ? "high speed " : ""),
>>>>                mmc_card_hs400(card) ? "HS400 " :
>>>>                (mmc_card_hs200(card) ? "HS200 " : ""),
>>>> +            mmc_card_hs400es(card) ? "Enhanced strobe" : "",
>>>>                mmc_card_ddr52(card) ? "DDR " : "",
>>>>                uhs_bus_speed_mode, type, card->rca);
>>>>        }
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index 99275e4..a559501 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -1127,6 +1127,14 @@ void mmc_set_initial_state(struct mmc_host *host)
>>>>        host->ios.bus_width = MMC_BUS_WIDTH_1;
>>>>        host->ios.timing = MMC_TIMING_LEGACY;
>>>>        host->ios.drv_type = 0;
>>>> +    host->ios.enhanced_strobe = false;
>>>> +
>>>> +    /*
>>>> +     * Make sure we are in non-enhanced strobe mode before we
>>>> +     * actually enable it in ext_csd.
>>>> +     */
>>>> +    if (host->ops->hs400_enhanced_strobe)
>>>> +        host->ops->hs400_enhanced_strobe(host, &host->ios);
>>>>
>>>>        mmc_set_ios(host);
>>>>    }
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index f99c47e..c2d1981 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -235,6 +235,11 @@ static void mmc_select_card_type(struct mmc_card *card)
>>>>            avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V;
>>>>        }
>>>>
>>>> +    if ((caps2 & MMC_CAP2_HS400_ES) &&
>>>> +        card->ext_csd.strobe_support &&
>>>> +        (card_type & EXT_CSD_CARD_TYPE_HS400))
>>>> +        avail_type |= EXT_CSD_CARD_TYPE_HS400ES;
>>>
>>> Does it need to check whether support HS400_1.8V/1.2V or not?
>>> It should be more stable than now.
>>>
>>
>> Ahh, right.
>> I need to add "avail_type & EXT_CSD_CARD_TYPE_HS400" instead of
>> "card_type & EXT_CSD_CARD_TYPE_HS400"... because if "avail_type &
>> EXT_CSD_CARD_TYPE_HS400" is true, it implies that "card_type & EXT_CSD_CARD_TYPE_HS400" must be true.
>
> Otherwise, you can check or add the capabilities in the parse_of_mmc().
> It can choose according to your preference. :)
>
> Anyway, i have tested your patches..If you send the patch V4, i will also test.

Thanks for testing. I will cc you when pushing V4.

>
> Best Regards,
> Jaehoon Chung
>
>>
>>
>> Thansk for catching this!
>>
>>
>>> if (avail_type & (EXT_CSD_CARD_TYPE_HS400_1.8V | EXT_CSD_CARD_TYPE_HS400_1.2V)) {
>>>      if ((cap2 & MMC_CAP2_HS400_ES) && ...
>>> }
>>>
>>> how about this?
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>> +
>>>>        card->ext_csd.hs_max_dtr = hs_max_dtr;
>>>>        card->ext_csd.hs200_max_dtr = hs200_max_dtr;
>>>>        card->mmc_avail_type = avail_type;
>>>> @@ -383,6 +388,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>>                mmc_card_set_blockaddr(card);
>>>>        }
>>>>
>>>> +    card->ext_csd.strobe_support = ext_csd[EXT_CSD_STROBE_SUPPORT];
>>>>        card->ext_csd.raw_card_type = ext_csd[EXT_CSD_CARD_TYPE];
>>>>        mmc_select_card_type(card);
>>>>
>>>> @@ -1216,6 +1222,73 @@ out_err:
>>>>        return err;
>>>>    }
>>>>
>>>> +static int mmc_select_hs400es(struct mmc_card *card)
>>>> +{
>>>> +    struct mmc_host *host = card->host;
>>>> +    int err = 0;
>>>> +    u8 val;
>>>> +
>>>> +    err = mmc_select_bus_width(card);
>>>> +    if (IS_ERR_VALUE(err))
>>>> +        goto out_err;
>>>> +
>>>> +    /* Switch card to HS mode */
>>>> +    err = mmc_select_hs(card);
>>>> +    if (err) {
>>>> +        pr_err("%s: switch to high-speed failed, err:%d\n",
>>>> +            mmc_hostname(host), err);
>>>> +        goto out_err;
>>>> +    }
>>>> +
>>>> +    err = mmc_switch_status(card);
>>>> +    if (err)
>>>> +        goto out_err;
>>>> +
>>>> +    /* Switch card to DDR with strobe bit */
>>>> +    val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
>>>> +    err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>>> +             EXT_CSD_BUS_WIDTH,
>>>> +             val,
>>>> +             card->ext_csd.generic_cmd6_time);
>>>> +    if (err) {
>>>> +        pr_err("%s: switch to bus width for hs400es failed, err:%d\n",
>>>> +            mmc_hostname(host), err);
>>>> +        goto out_err;
>>>> +    }
>>>> +
>>>> +    /* Switch card to HS400 */
>>>> +    val = EXT_CSD_TIMING_HS400 |
>>>> +          card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
>>>> +    err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>>> +               EXT_CSD_HS_TIMING, val,
>>>> +               card->ext_csd.generic_cmd6_time,
>>>> +               true, false, true);
>>>> +    if (err) {
>>>> +        pr_err("%s: switch to hs400es failed, err:%d\n",
>>>> +            mmc_hostname(host), err);
>>>> +        goto out_err;
>>>> +    }
>>>> +
>>>> +    /* Set host controller to HS400 timing and frequency */
>>>> +    mmc_set_timing(host, MMC_TIMING_MMC_HS400);
>>>> +
>>>> +    /* Controller enable enhanced strobe function */
>>>> +    host->ios.enhanced_strobe = true;
>>>> +    if (host->ops->hs400_enhanced_strobe)
>>>> +        host->ops->hs400_enhanced_strobe(host, &host->ios);
>>>> +
>>>> +    err = mmc_switch_status(card);
>>>> +    if (err)
>>>> +        goto out_err;
>>>> +
>>>> +    return 0;
>>>> +
>>>> +out_err:
>>>> +    pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
>>>> +           __func__, err);
>>>> +    return err;
>>>> +}
>>>> +
>>>>    static void mmc_select_driver_type(struct mmc_card *card)
>>>>    {
>>>>        int card_drv_type, drive_strength, drv_type;
>>>> @@ -1297,7 +1370,7 @@ err:
>>>>    }
>>>>
>>>>    /*
>>>> - * Activate High Speed or HS200 mode if supported.
>>>> + * Activate High Speed, HS200 or HS400ES mode if supported.
>>>>     */
>>>>    static int mmc_select_timing(struct mmc_card *card)
>>>>    {
>>>> @@ -1306,7 +1379,9 @@ static int mmc_select_timing(struct mmc_card *card)
>>>>        if (!mmc_can_ext_csd(card))
>>>>            goto bus_speed;
>>>>
>>>> -    if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>>>> +    if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES)
>>>> +        err = mmc_select_hs400es(card);
>>>> +    else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200)
>>>>            err = mmc_select_hs200(card);
>>>>        else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS)
>>>>            err = mmc_select_hs(card);
>>>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>>>> index eb0151b..22defc2 100644
>>>> --- a/include/linux/mmc/card.h
>>>> +++ b/include/linux/mmc/card.h
>>>> @@ -95,6 +95,7 @@ struct mmc_ext_csd {
>>>>        u8            raw_partition_support;    /* 160 */
>>>>        u8            raw_rpmb_size_mult;    /* 168 */
>>>>        u8            raw_erased_mem_count;    /* 181 */
>>>> +    u8            strobe_support;        /* 184 */
>>>>        u8            raw_ext_csd_structure;    /* 194 */
>>>>        u8            raw_card_type;        /* 196 */
>>>>        u8            raw_driver_strength;    /* 197 */
>>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>> index 2a06fb0..f98bd0e 100644
>>>> --- a/include/linux/mmc/host.h
>>>> +++ b/include/linux/mmc/host.h
>>>> @@ -19,6 +19,7 @@
>>>>
>>>>    #include <linux/mmc/core.h>
>>>>    #include <linux/mmc/card.h>
>>>> +#include <linux/mmc/mmc.h>
>>>>    #include <linux/mmc/pm.h>
>>>>
>>>>    struct mmc_ios {
>>>> @@ -77,6 +78,8 @@ struct mmc_ios {
>>>>    #define MMC_SET_DRIVER_TYPE_A    1
>>>>    #define MMC_SET_DRIVER_TYPE_C    2
>>>>    #define MMC_SET_DRIVER_TYPE_D    3
>>>> +
>>>> +    bool enhanced_strobe;            /* hs400es selection */
>>>>    };
>>>>
>>>>    struct mmc_host_ops {
>>>> @@ -143,6 +146,9 @@ struct mmc_host_ops {
>>>>
>>>>        /* Prepare HS400 target operating frequency depending host driver */
>>>>        int    (*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios);
>>>> +    /* Prepare enhanced strobe depending host driver */
>>>> +    void    (*hs400_enhanced_strobe)(struct mmc_host *host,
>>>> +                     struct mmc_ios *ios);
>>>>        int    (*select_drive_strength)(struct mmc_card *card,
>>>>                         unsigned int max_dtr, int host_drv,
>>>>                         int card_drv, int *drv_type);
>>>> @@ -513,6 +519,11 @@ static inline bool mmc_card_hs400(struct mmc_card *card)
>>>>        return card->host->ios.timing == MMC_TIMING_MMC_HS400;
>>>>    }
>>>>
>>>> +static inline bool mmc_card_hs400es(struct mmc_card *card)
>>>> +{
>>>> +    return card->host->ios.enhanced_strobe == true;
>>>> +}
>>>> +
>>>>    void mmc_retune_timer_stop(struct mmc_host *host);
>>>>
>>>>    static inline void mmc_retune_needed(struct mmc_host *host)
>>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>>> index 15f2c4a..c376209 100644
>>>> --- a/include/linux/mmc/mmc.h
>>>> +++ b/include/linux/mmc/mmc.h
>>>> @@ -297,6 +297,7 @@ struct _mmc_csd {
>>>>    #define EXT_CSD_PART_CONFIG        179    /* R/W */
>>>>    #define EXT_CSD_ERASED_MEM_CONT        181    /* RO */
>>>>    #define EXT_CSD_BUS_WIDTH        183    /* R/W */
>>>> +#define EXT_CSD_STROBE_SUPPORT        184    /* RO */
>>>>    #define EXT_CSD_HS_TIMING        185    /* R/W */
>>>>    #define EXT_CSD_POWER_CLASS        187    /* R/W */
>>>>    #define EXT_CSD_REV            192    /* RO */
>>>> @@ -380,12 +381,14 @@ struct _mmc_csd {
>>>>    #define EXT_CSD_CARD_TYPE_HS400_1_2V    (1<<7)    /* Card can run at 200MHz DDR, 1.2V */
>>>>    #define EXT_CSD_CARD_TYPE_HS400        (EXT_CSD_CARD_TYPE_HS400_1_8V | \
>>>>                         EXT_CSD_CARD_TYPE_HS400_1_2V)
>>>> +#define EXT_CSD_CARD_TYPE_HS400ES    (1<<8)    /* Card can run at HS400ES */
>>>>
>>>>    #define EXT_CSD_BUS_WIDTH_1    0    /* Card is in 1 bit mode */
>>>>    #define EXT_CSD_BUS_WIDTH_4    1    /* Card is in 4 bit mode */
>>>>    #define EXT_CSD_BUS_WIDTH_8    2    /* Card is in 8 bit mode */
>>>>    #define EXT_CSD_DDR_BUS_WIDTH_4    5    /* Card is in 4 bit DDR mode */
>>>>    #define EXT_CSD_DDR_BUS_WIDTH_8    6    /* Card is in 8 bit DDR mode */
>>>> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7)    /* Enhanced strobe mode */
>>>>
>>>>    #define EXT_CSD_TIMING_BC    0    /* Backwards compatility */
>>>>    #define EXT_CSD_TIMING_HS    1    /* High speed */
>>>>
>>>
>>>
>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>
>
>
>