[PATCH 0/7] Preparations to support SD UHS-II cards

[PATCH 0/7] Preparations to support SD UHS-II cards

[Jason Lai]

From: Jason Lai <jason.lai@genesyslogic.com.tw>

Series [1] that has been posted by Ulf Hansson which provided some guidance
and an overall structure.

Series [2] focused on UHS-II card control side to address Ulf's intention
regarding to "modularising" sd_uhs2.c.

This series is the successor version of post [2], which adopts Ulf's
comments about series [2]:
1. Remove unnecessary debug print.
2. Rephrase description about uhs2_cmd_assemble() in sd_uhs2.c
3. Place UHS-II variables in the appropriate structure.

Kind regards
Jason Lai

[1]
https://patchwork.kernel.org/project/linux-mmc/list/?series=438509

[2]
https://patchwork.kernel.org/project/linux-mmc/list/?series=539737

Jason Lai (3):
  mmc: add UHS-II related definitions in headers
  mmc: Implement content of UHS-II card initialization functions
  mmc: core: Support UHS-II card access

Ulf Hansson (4):
  mmc: core: Cleanup printing of speed mode at card insertion
  mmc: core: Prepare to support SD UHS-II cards
  mmc: core: Announce successful insertion of an SD UHS-II card
  mmc: core: Extend support for mmc regulators with a vqmmc2

 drivers/mmc/core/Makefile    |    2 +-
 drivers/mmc/core/bus.c       |   38 +-
 drivers/mmc/core/core.c      |   43 +-
 drivers/mmc/core/core.h      |    1 +
 drivers/mmc/core/host.h      |    4 +
 drivers/mmc/core/regulator.c |   34 ++
 drivers/mmc/core/sd_uhs2.c   | 1081 ++++++++++++++++++++++++++++++++++
 drivers/mmc/core/sd_uhs2.h   |   18 +
 include/linux/mmc/card.h     |   35 ++
 include/linux/mmc/core.h     |    4 +-
 include/linux/mmc/host.h     |   52 ++
 include/linux/mmc/sd_uhs2.h  |  196 ++++++
 12 files changed, 1485 insertions(+), 23 deletions(-)
 create mode 100644 drivers/mmc/core/sd_uhs2.c
 create mode 100644 drivers/mmc/core/sd_uhs2.h
 create mode 100644 include/linux/mmc/sd_uhs2.h


------ original cover letter from Ulf's series ------
A series [1] that has been collaborative worked upon by Takahiro Akashi
(Linaro) and Ben Chuang (Genesys Logic) is targeting to add SD UHS-II
support
to the mmc subsystem.

Throughout the reviews, we realized that the changes affecting the mmc core
to
support the UHS-II interface/protocol might not be entirely straightforward
to
implement. Especially, I expressed some concerns about the code that
manages
power on/off, initialization and power management of a SD UHS-II card.

Therefore, I have posted this small series to try to help to put some of
the
foundation in the mmc core in place. Hopefully this can provide some
guidance
and an overall structure, of how I think the code could evolve.

More details are available in the commit messages and through comments in
the
code, for each path.

Kind regards
Uffe

[1]
https://lkml.org/lkml/2020/11/5/1472


Ulf Hansson (4):
  mmc: core: Cleanup printing of speed mode at card insertion
  mmc: core: Prepare to support SD UHS-II cards
  mmc: core: Announce successful insertion of an SD UHS-II card
  mmc: core: Extend support for mmc regulators with a vqmmc2

 drivers/mmc/core/Makefile    |   2 +-
 drivers/mmc/core/bus.c       |  38 +++--
 drivers/mmc/core/core.c      |  17 ++-
 drivers/mmc/core/core.h      |   1 +
 drivers/mmc/core/host.h      |   5 +
 drivers/mmc/core/regulator.c |  34 +++++
 drivers/mmc/core/sd_uhs2.c   | 289 +++++++++++++++++++++++++++++++++++
 include/linux/mmc/card.h     |   6 +
 include/linux/mmc/host.h     |  30 ++++
 9 files changed, 404 insertions(+), 18 deletions(-)
 create mode 100644 drivers/mmc/core/sd_uhs2.c

-- 
2.34.0

[PATCH 1/7] mmc: core: Cleanup printing of speed mode at card insertion

[Jason Lai]

From: Ulf Hansson <ulf.hansson@linaro.org>

The current print of the bus speed mode in mmc_add_card() has grown over
the years and is now difficult to parse. Let's clean up the code and also
take the opportunity to properly announce "DDR" for eMMCs as
"high speed DDR", which is according to the eMMC spec.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/bus.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 4383c262b..27ba3c749 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -311,6 +311,7 @@ int mmc_add_card(struct mmc_card *card)
 {
 	int ret;
 	const char *type;
+	const char *speed_mode = "";
 	const char *uhs_bus_speed_mode = "";
 	static const char *const uhs_speeds[] = {
 		[UHS_SDR12_BUS_SPEED] = "SDR12 ",
@@ -349,27 +350,30 @@ int mmc_add_card(struct mmc_card *card)
 		break;
 	}
 
+	if (mmc_card_hs(card))
+		speed_mode = "high speed ";
+	else if (mmc_card_uhs(card))
+		speed_mode = "ultra high speed ";
+	else if	(mmc_card_ddr52(card))
+		speed_mode = "high speed DDR ";
+	else if (mmc_card_hs200(card))
+		speed_mode = "HS200 ";
+	else if (mmc_card_hs400es(card))
+		speed_mode = "HS400 Enhanced strobe ";
+	else if (mmc_card_hs400(card))
+		speed_mode = "HS400 ";
+
 	if (mmc_card_uhs(card) &&
 		(card->sd_bus_speed < ARRAY_SIZE(uhs_speeds)))
 		uhs_bus_speed_mode = uhs_speeds[card->sd_bus_speed];
 
-	if (mmc_host_is_spi(card->host)) {
-		pr_info("%s: new %s%s%s card on SPI\n",
-			mmc_hostname(card->host),
-			mmc_card_hs(card) ? "high speed " : "",
-			mmc_card_ddr52(card) ? "DDR " : "",
-			type);
-	} else {
-		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 " : "",
+	if (mmc_host_is_spi(card->host))
+		pr_info("%s: new %s%s card on SPI\n",
+			mmc_hostname(card->host), speed_mode, type);
+	else
+		pr_info("%s: new %s%s%s card at address %04x\n",
+			mmc_hostname(card->host), speed_mode,
 			uhs_bus_speed_mode, type, card->rca);
-	}
 
 #ifdef CONFIG_DEBUG_FS
 	mmc_add_card_debugfs(card);
-- 
2.34.0

[PATCH 2/7] mmc: core: Prepare to support SD UHS-II cards

[Jason Lai]

From: Ulf Hansson <ulf.hansson@linaro.org>

The SD UHS-II interface was introduced to the SD spec v4.00 several years
ago. The interface is fundamentally different from an electrical and a
protocol point of view, comparing to the legacy SD interface.

However, the legacy SD protocol is supported through a specific transport
layer (SD-TRAN) defined in the UHS-II addendum of the spec. This allows the
SD card to be managed in a very similar way as a legacy SD card, hence a
lot of code can be re-used to support these new types of cards through the
mmc subsystem.

Moreover, an SD card that supports the UHS-II interface shall also be
backwards compatible with the legacy SD interface, which allows a UHS-II
card to be inserted into a legacy slot. As a matter of fact, this is
already supported by mmc subsystem as of today.

To prepare to add support for UHS-II, this change puts the basic foundation
in the mmc core in place, allowing it to be more easily reviewed before
subsequent changes implements the actual support.

Basically, the approach here adds a new UHS-II bus_ops type and adds a
separate initialization path for the UHS-II card. The intent is to avoid us
from sprinkling the legacy initialization path, but also to simplify
implementation of the UHS-II specific bits.

At this point, there is only one new host ops added to manage the various
ios settings needed for UHS-II. Additional host ops that are needed, are
being added from subsequent changes.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/Makefile  |   2 +-
 drivers/mmc/core/core.c    |  17 ++-
 drivers/mmc/core/core.h    |   1 +
 drivers/mmc/core/sd_uhs2.c | 292 +++++++++++++++++++++++++++++++++++++
 include/linux/mmc/card.h   |   7 +
 include/linux/mmc/host.h   |  23 +++
 6 files changed, 340 insertions(+), 2 deletions(-)
 create mode 100644 drivers/mmc/core/sd_uhs2.c

diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 6a907736c..15b067e8b 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_MMC)		+= mmc_core.o
 mmc_core-y			:= core.o bus.o host.o \
 				   mmc.o mmc_ops.o sd.o sd_ops.o \
 				   sdio.o sdio_ops.o sdio_bus.o \
-				   sdio_cis.o sdio_io.o sdio_irq.o \
+				   sdio_cis.o sdio_io.o sdio_irq.o sd_uhs2.o\
 				   slot-gpio.o regulator.o
 mmc_core-$(CONFIG_OF)		+= pwrseq.o
 obj-$(CONFIG_PWRSEQ_SIMPLE)	+= pwrseq_simple.o
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 95fedcf56..19b409179 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2194,6 +2194,18 @@ void mmc_rescan(struct work_struct *work)
 		goto out;
 	}
 
+	/*
+	 * Ideally we should favor initialization of legacy SD cards and defer
+	 * UHS-II enumeration. However, it seems like cards doesn't reliably
+	 * announce their support for UHS-II in the response to the ACMD41,
+	 * while initializing the legacy SD interface. Therefore, let's start
+	 * with UHS-II for now.
+	 */
+	if (!mmc_attach_sd_uhs2(host)) {
+		mmc_release_host(host);
+		goto out;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(freqs); i++) {
 		unsigned int freq = freqs[i];
 		if (freq > host->f_max) {
@@ -2215,10 +2227,13 @@ void mmc_rescan(struct work_struct *work)
 
 void mmc_start_host(struct mmc_host *host)
 {
+	bool power_up = !(host->caps2 &
+			 (MMC_CAP2_NO_PRESCAN_POWERUP | MMC_CAP2_SD_UHS2));
+
 	host->f_init = max(min(freqs[0], host->f_max), host->f_min);
 	host->rescan_disable = 0;
 
-	if (!(host->caps2 & MMC_CAP2_NO_PRESCAN_POWERUP)) {
+	if (power_up) {
 		mmc_claim_host(host);
 		mmc_power_up(host, host->ocr_avail);
 		mmc_release_host(host);
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 0c4de2030..5fb89afff 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -80,6 +80,7 @@ int mmc_detect_card_removed(struct mmc_host *host);
 int mmc_attach_mmc(struct mmc_host *host);
 int mmc_attach_sd(struct mmc_host *host);
 int mmc_attach_sdio(struct mmc_host *host);
+int mmc_attach_sd_uhs2(struct mmc_host *host);
 
 /* Module parameters */
 extern bool use_spi_crc;
diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c
new file mode 100644
index 000000000..24aa51a6d
--- /dev/null
+++ b/drivers/mmc/core/sd_uhs2.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 Linaro Ltd
+ *
+ * Author: Ulf Hansson <ulf.hansson@linaro.org>
+ *
+ * Support for SD UHS-II cards
+ */
+#include <linux/err.h>
+
+#include <linux/mmc/host.h>
+#include <linux/mmc/card.h>
+
+#include "core.h"
+#include "bus.h"
+#include "sd.h"
+#include "mmc_ops.h"
+
+static const unsigned int sd_uhs2_freqs[] = { 52000000, 26000000 };
+
+static int sd_uhs2_set_ios(struct mmc_host *host)
+{
+	struct mmc_ios *ios = &host->ios;
+
+	return host->ops->uhs2_set_ios(host, ios);
+}
+
+static int sd_uhs2_power_up(struct mmc_host *host)
+{
+	host->ios.vdd = fls(host->ocr_avail) - 1;
+	host->ios.clock = host->f_init;
+	host->ios.timing = MMC_TIMING_SD_UHS2;
+	host->ios.power_mode = MMC_POWER_UP;
+
+	return sd_uhs2_set_ios(host);
+}
+
+static void sd_uhs2_power_off(struct mmc_host *host)
+{
+	host->ios.vdd = 0;
+	host->ios.clock = 0;
+	host->ios.timing = MMC_TIMING_LEGACY;
+	host->ios.power_mode = MMC_POWER_OFF;
+
+	sd_uhs2_set_ios(host);
+}
+
+/*
+ * Run the phy initialization sequence, which mainly relies on the UHS-II host
+ * to check that we reach the expected electrical state, between the host and
+ * the card.
+ */
+static int sd_uhs2_phy_init(struct mmc_host *host)
+{
+	return 0;
+}
+
+/*
+ * Do the early initialization of the card, by sending the device init
+roadcast
+ * command and wait for the process to be completed.
+ */
+static int sd_uhs2_dev_init(struct mmc_host *host)
+{
+	return 0;
+}
+
+/*
+ * Run the enumeration process by sending the enumerate command to the card.
+ * Note that, we currently support only the point to point connection, which
+ * means only one card can be attached per host/slot.
+ */
+static int sd_uhs2_enum(struct mmc_host *host, u32 *node_id)
+{
+	return 0;
+}
+
+/*
+ * Read the UHS-II configuration registers (CFG_REG) of the card, by sending it
+ * commands and by parsing the responses. Store a copy of the relevant data in
+ * card->uhs2_config.
+ */
+static int sd_uhs2_config_read(struct mmc_host *host, struct mmc_card *card)
+{
+	return 0;
+}
+
+/*
+ * Based on the card's and host's UHS-II capabilities, let's update the
+ * configuration of the card and the host. This may also include to move to a
+ * greater speed range/mode. Depending on the updated configuration, we may
+eed
+ * to do a soft reset of the card via sending it a GO_DORMANT_STATE command.
+ *
+ * In the final step, let's check if the card signals "config completion",
+hich
+ * indicates that the card has moved from config state into active state.
+ */
+static int sd_uhs2_config_write(struct mmc_host *host, struct mmc_card *card)
+{
+	return 0;
+}
+
+/*
+ * Initialize the UHS-II card through the SD-TRAN transport layer. This enables
+ * commands/requests to be backwards compatible through the legacy SD protocol.
+ * UHS-II cards has a specific power limit specified for VDD1/VDD2, that should
+ * be set through a legacy CMD6. Note that, the power limit that becomes set,
+ * survives a soft reset through the GO_DORMANT_STATE command.
+ */
+static int sd_uhs2_legacy_init(struct mmc_host *host, struct mmc_card *card)
+{
+	return 0;
+}
+
+/*
+ * Allocate the data structure for the mmc_card and run the UHS-II specific
+ * initialization sequence.
+ */
+static int sd_uhs2_init_card(struct mmc_host *host)
+{
+	struct mmc_card *card;
+	u32 node_id;
+	int err;
+
+	err = sd_uhs2_dev_init(host);
+	if (err)
+		return err;
+
+	err = sd_uhs2_enum(host, &node_id);
+	if (err)
+		return err;
+
+	card = mmc_alloc_card(host, &sd_type);
+	if (IS_ERR(card))
+		return PTR_ERR(card);
+
+	card->uhs2_config.node_id = node_id;
+	card->type = MMC_TYPE_SD;
+
+	err = sd_uhs2_config_read(host, card);
+	if (err)
+		goto err;
+
+	err = sd_uhs2_config_write(host, card);
+	if (err)
+		goto err;
+
+	err = sd_uhs2_legacy_init(host, card);
+	if (err)
+		goto err;
+
+	host->card = card;
+	return 0;
+
+err:
+	mmc_remove_card(card);
+	return err;
+}
+
+static void sd_uhs2_remove(struct mmc_host *host)
+{
+	mmc_remove_card(host->card);
+	host->card = NULL;
+}
+
+static int sd_uhs2_alive(struct mmc_host *host)
+{
+	return mmc_send_status(host->card, NULL);
+}
+
+static void sd_uhs2_detect(struct mmc_host *host)
+{
+	int err;
+
+	mmc_get_card(host->card, NULL);
+	err = _mmc_detect_card_removed(host);
+	mmc_put_card(host->card, NULL);
+
+	if (err) {
+		sd_uhs2_remove(host);
+
+		mmc_claim_host(host);
+		mmc_detach_bus(host);
+		sd_uhs2_power_off(host);
+		mmc_release_host(host);
+	}
+}
+
+static int sd_uhs2_suspend(struct mmc_host *host)
+{
+	return 0;
+}
+
+static int sd_uhs2_resume(struct mmc_host *host)
+{
+	return 0;
+}
+
+static int sd_uhs2_runtime_suspend(struct mmc_host *host)
+{
+	return 0;
+}
+
+static int sd_uhs2_runtime_resume(struct mmc_host *host)
+{
+	return 0;
+}
+
+static int sd_uhs2_shutdown(struct mmc_host *host)
+{
+	return 0;
+}
+
+static int sd_uhs2_hw_reset(struct mmc_host *host)
+{
+	return 0;
+}
+
+static const struct mmc_bus_ops sd_uhs2_ops = {
+	.remove = sd_uhs2_remove,
+	.alive = sd_uhs2_alive,
+	.detect = sd_uhs2_detect,
+	.suspend = sd_uhs2_suspend,
+	.resume = sd_uhs2_resume,
+	.runtime_suspend = sd_uhs2_runtime_suspend,
+	.runtime_resume = sd_uhs2_runtime_resume,
+	.shutdown = sd_uhs2_shutdown,
+	.hw_reset = sd_uhs2_hw_reset,
+};
+
+static int sd_uhs2_attach(struct mmc_host *host)
+{
+	int err;
+
+	err = sd_uhs2_power_up(host);
+	if (err)
+		goto err;
+
+	err = sd_uhs2_phy_init(host);
+	if (err)
+		goto err;
+
+	err = sd_uhs2_init_card(host);
+	if (err)
+		goto err;
+
+	mmc_attach_bus(host, &sd_uhs2_ops);
+
+	mmc_release_host(host);
+
+	err = mmc_add_card(host->card);
+	if (err)
+		goto remove_card;
+
+	mmc_claim_host(host);
+	return 0;
+
+remove_card:
+	mmc_remove_card(host->card);
+	host->card = NULL;
+	mmc_claim_host(host);
+	mmc_detach_bus(host);
+err:
+	sd_uhs2_power_off(host);
+	return err;
+}
+
+int mmc_attach_sd_uhs2(struct mmc_host *host)
+{
+	int i, err = 0;
+
+	if (!(host->caps2 & MMC_CAP2_SD_UHS2))
+		return -EOPNOTSUPP;
+
+	/* Turn off the legacy SD interface before trying with UHS-II. */
+	mmc_power_off(host);
+
+	/*
+	 * Start UHS-II initialization at 52MHz and possibly make a retry at
+	 * 26MHz according to the spec. It's required that the host driver
+	 * validates ios->clock, to set a rate within the correct range.
+	 */
+	for (i = 0; i < ARRAY_SIZE(sd_uhs2_freqs); i++) {
+		host->f_init = sd_uhs2_freqs[i];
+		err = sd_uhs2_attach(host);
+		if (!err)
+			break;
+	}
+
+	return err;
+}
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 74e6c0624..82b07eac1 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -210,6 +210,11 @@ struct sd_ext_reg {
 #define SD_EXT_PERF_CMD_QUEUE	(1<<4)
 };
 
+struct sd_uhs2_config {
+	u32			node_id;
+	/* TODO: Extend with more register configs. */
+};
+
 struct sdio_cccr {
 	unsigned int		sdio_vsn;
 	unsigned int		sd_vsn;
@@ -314,6 +319,8 @@ struct mmc_card {
 	struct sd_ext_reg	ext_power;	/* SD extension reg for PM */
 	struct sd_ext_reg	ext_perf;	/* SD extension reg for PERF */
 
+	struct sd_uhs2_config	uhs2_config;	/* SD UHS-II config */
+
 	unsigned int		sdio_funcs;	/* number of SDIO functions */
 	atomic_t		sdio_funcs_probed; /* number of probed SDIO funcs */
 	struct sdio_cccr	cccr;		/* common card info */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 0abd47e9e..91065ab7f 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -63,6 +63,10 @@ struct mmc_ios {
 #define MMC_TIMING_MMC_HS400	10
 #define MMC_TIMING_SD_EXP	11
 #define MMC_TIMING_SD_EXP_1_2V	12
+#define MMC_TIMING_SD_UHS2	13
+
+	unsigned char	vqmmc2_voltage;
+#define MMC_VQMMC2_VOLTAGE_180	0
 
 	unsigned char	signal_voltage;		/* signalling voltage (1.8V or 3.3V) */
 
@@ -91,6 +95,10 @@ struct mmc_clk_phase_map {
 	struct mmc_clk_phase phase[MMC_NUM_CLK_PHASES];
 };
 
+struct sd_uhs2_caps {
+	/* TODO: Add UHS-II capabilities for the host. */
+};
+
 struct mmc_host;
 
 struct mmc_host_ops {
@@ -126,6 +134,18 @@ struct mmc_host_ops {
 	 */
 	void	(*set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 
+	/*
+	 * The uhs2_set_ios callback is mandatory to implement for hosts that
+	 * supports the SD UHS-II interface (MMC_CAP2_SD_UHS2), while the
+	 * callback is unused for the other cases. Note that, the struct
+	 * mmc_ios is being re-used for this as well.
+	 *
+	 * Expected return values for the uhs2_set_ios callback are a negative
+	 * errno in case of a failure or zero for success.
+	 */
+	int	(*uhs2_set_ios)(struct mmc_host *host, struct mmc_ios *ios);
+
+
 	/*
 	 * Return values for the get_ro callback should be:
 	 *   0 for a read/write card
@@ -374,6 +394,7 @@ struct mmc_host {
 				 MMC_CAP2_HS200_1_2V_SDR)
 #define MMC_CAP2_SD_EXP		(1 << 7)	/* SD express via PCIe */
 #define MMC_CAP2_SD_EXP_1_2V	(1 << 8)	/* SD express 1.2V */
+#define MMC_CAP2_SD_UHS2	(1 << 9)	/* SD UHS-II support */
 #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */
 #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */
 #define MMC_CAP2_NO_PRESCAN_POWERUP (1 << 14)	/* Don't power up before scan */
@@ -399,6 +420,8 @@ struct mmc_host {
 #define MMC_CAP2_CRYPTO		0
 #endif
 
+	struct sd_uhs2_caps	uhs2_caps;	/* SD UHS-II capabilities */
+
 	int			fixed_drv_type;	/* fixed driver type for non-removable media */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
-- 
2.34.0

[PATCH 3/7] mmc: core: Announce successful insertion of an SD UHS-II card

[Jason Lai]

From: Ulf Hansson <ulf.hansson@linaro.org>

To inform the users about SD UHS-II cards, let's extend the print at card
insertion with a "UHS-II" substring. Within this change, it seems
reasonable to convert from using "ultra high speed" into "UHS-I speed", for
the UHS-I type, as it should makes it more clear.

Note that, the new print for UHS-II cards doesn't include the actual
selected speed mode. Instead, this is going to be added from subsequent
change.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/bus.c  | 4 +++-
 drivers/mmc/core/host.h | 4 ++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 27ba3c749..8fd46b5f5 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -353,7 +353,9 @@ int mmc_add_card(struct mmc_card *card)
 	if (mmc_card_hs(card))
 		speed_mode = "high speed ";
 	else if (mmc_card_uhs(card))
-		speed_mode = "ultra high speed ";
+		speed_mode = "UHS-I speed ";
+	else if (mmc_card_uhs2(card))
+		speed_mode = "UHS-II speed ";
 	else if	(mmc_card_ddr52(card))
 		speed_mode = "high speed DDR ";
 	else if (mmc_card_hs200(card))
diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h
index ba407617e..c243d3096 100644
--- a/drivers/mmc/core/host.h
+++ b/drivers/mmc/core/host.h
@@ -83,5 +83,9 @@ static inline bool mmc_card_sd_express(struct mmc_host *host)
 		host->ios.timing == MMC_TIMING_SD_EXP_1_2V;
 }
 
+static inline bool mmc_card_uhs2(struct mmc_card *card)
+{
+	return card->host->ios.timing == MMC_TIMING_SD_UHS2;
+}
 #endif
 
-- 
2.34.0

[PATCH 4/7] mmc: core: Extend support for mmc regulators with a vqmmc2

[Jason Lai]

From: Ulf Hansson <ulf.hansson@linaro.org>

To allow an additional external regulator to be controlled by an mmc host
driver, let's add support for a vqmmc2 regulator to the mmc core.

For an SD UHS-II interface the vqmmc2 regulator may correspond to the so
called vdd2 supply, as described by the SD spec. Initially, only 1.8V is
needed, hence limit the new helper function, mmc_regulator_set_vqmmc2() to
this too.

Note that, to allow for flexibility mmc host drivers need to manage the
enable/disable of the vqmmc2 regulator themselves, while the regulator is
looked up through the common mmc_regulator_get_supply().

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Jason Lai <jasonlai.genesyslogic@gmail.com>
---
 drivers/mmc/core/regulator.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h     |  8 ++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/mmc/core/regulator.c b/drivers/mmc/core/regulator.c
index 609201a46..3c1896827 100644
--- a/drivers/mmc/core/regulator.c
+++ b/drivers/mmc/core/regulator.c
@@ -223,6 +223,33 @@ int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios)
 }
 EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
 
+/**
+ * mmc_regulator_set_vqmmc2 - Set vqmmc2 as per the ios->vqmmc2_voltage
+ * @mmc: The mmc host to regulate
+ * @ios: The io bus settings
+ *
+ * Sets a new voltage level for the vqmmc2 regulator, which may correspond to
+ * the vdd2 regulator for an SD UHS-II interface. This function is expected to
+ * be called by mmc host drivers.
+ *
+ * Returns a negative error code on failure, zero if the voltage level was
+ * changed successfully or a positive value if the level didn't need to change.
+ */
+int mmc_regulator_set_vqmmc2(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	if (IS_ERR(mmc->supply.vqmmc2))
+		return -EINVAL;
+
+	switch (ios->vqmmc2_voltage) {
+	case MMC_VQMMC2_VOLTAGE_180:
+		return mmc_regulator_set_voltage_if_supported(
+			mmc->supply.vqmmc2, 1700000, 1800000, 1950000);
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc2);
+
 #else
 
 static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
@@ -249,6 +276,7 @@ int mmc_regulator_get_supply(struct mmc_host *mmc)
 
 	mmc->supply.vmmc = devm_regulator_get_optional(dev, "vmmc");
 	mmc->supply.vqmmc = devm_regulator_get_optional(dev, "vqmmc");
+	mmc->supply.vqmmc2 = devm_regulator_get_optional(dev, "vqmmc2");
 
 	if (IS_ERR(mmc->supply.vmmc)) {
 		if (PTR_ERR(mmc->supply.vmmc) == -EPROBE_DEFER)
@@ -268,6 +296,12 @@ int mmc_regulator_get_supply(struct mmc_host *mmc)
 		dev_dbg(dev, "No vqmmc regulator found\n");
 	}
 
+	if (IS_ERR(mmc->supply.vqmmc2)) {
+		if (PTR_ERR(mmc->supply.vqmmc2) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_dbg(dev, "No vqmmc2 regulator found\n");
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mmc_regulator_get_supply);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 91065ab7f..69f8c8a8f 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -303,6 +303,7 @@ struct mmc_pwrseq;
 struct mmc_supply {
 	struct regulator *vmmc;		/* Card power supply */
 	struct regulator *vqmmc;	/* Optional Vccq supply */
+	struct regulator *vqmmc2;	/* Optional supply for phy */
 };
 
 struct mmc_ctx {
@@ -580,6 +581,7 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc,
 			struct regulator *supply,
 			unsigned short vdd_bit);
 int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios);
+int mmc_regulator_set_vqmmc2(struct mmc_host *mmc, struct mmc_ios *ios);
 #else
 static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
 				 struct regulator *supply,
@@ -593,6 +595,12 @@ static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
 {
 	return -EINVAL;
 }
+
+static inline int mmc_regulator_set_vqmmc2(struct mmc_host *mmc,
+					   struct mmc_ios *ios)
+{
+	return -EINVAL;
+}
 #endif
 
 int mmc_regulator_get_supply(struct mmc_host *mmc);
-- 
2.34.0

[PATCH 5/7] mmc: add UHS-II related definitions in headers

[Jason Lai]

From: Jason Lai <jason.lai@genesyslogic.com.tw>

All LINK layer messages, registers and SD-TRAN command packet described in
'Part 1 UHS-II Addendum Ver 1.01' are defined in include/linux/mmc/sd_uhs2.h

drivers/mmc/core/sd_uhs2.h contains exported function prototype.

Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
---
 drivers/mmc/core/sd_uhs2.h  |  18 ++++
 include/linux/mmc/card.h    |  30 +++++-
 include/linux/mmc/core.h    |   4 +-
 include/linux/mmc/host.h    |  27 ++++-
 include/linux/mmc/sd_uhs2.h | 196 ++++++++++++++++++++++++++++++++++++
 5 files changed, 268 insertions(+), 7 deletions(-)
 create mode 100644 drivers/mmc/core/sd_uhs2.h
 create mode 100644 include/linux/mmc/sd_uhs2.h

diff --git a/drivers/mmc/core/sd_uhs2.h b/drivers/mmc/core/sd_uhs2.h
new file mode 100644
index 000000000..5bb5dc1d1
--- /dev/null
+++ b/drivers/mmc/core/sd_uhs2.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Header file for UHS-II packets, Host Controller registers and I/O
+ * accessors.
+ *
+ *  Copyright (C) 2014 Intel Corp, All Rights Reserved.
+ */
+#ifndef MMC_UHS2_H
+#define MMC_UHS2_H
+
+#include <linux/mmc/core.h>
+#include <linux/mmc/host.h>
+
+#define UHS2_PHY_INIT_ERR	1
+
+int sd_uhs2_prepare_cmd(struct mmc_host *host, struct mmc_request *mrq);
+
+#endif /* MMC_UHS2_H */
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 82b07eac1..4b2fda2e6 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -211,8 +211,36 @@ struct sd_ext_reg {
 };
 
 struct sd_uhs2_config {
-	u32			node_id;
+	u32	node_id;
 	/* TODO: Extend with more register configs. */
+	u32	dap;
+	u32	gap;
+	u32	n_fcu;
+	u32	maxblk_len;
+	u8	n_lanes;
+	u8	dadr_len;
+	u8	app_type;
+	u8	phy_minor_rev;
+	u8	phy_major_rev;
+	u8	can_hibernate;
+	u8	n_lss_sync;
+	u8	n_lss_dir;
+	u8	link_minor_rev;
+	u8	link_major_rev;
+	u8	dev_type;
+	u8	n_data_gap;
+
+	u32	n_fcu_set;
+	u32	maxblk_len_set;
+	u8	n_lanes_set;
+	u8	speed_range_set;
+	u8	n_lss_sync_set;
+	u8	n_lss_dir_set;
+	u8	n_data_gap_set;
+	u8	pwrctrl_mode_set;
+	u8	max_retry_set;
+
+	u8	cfg_complete;
 };
 
 struct sdio_cccr {
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index ab19245e9..8ac4b0b52 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -1,7 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
-/*
- *  linux/include/linux/mmc/core.h
- */
+
 #ifndef LINUX_MMC_CORE_H
 #define LINUX_MMC_CORE_H
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 69f8c8a8f..ad6cccf67 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -96,7 +96,29 @@ struct mmc_clk_phase_map {
 };
 
 struct sd_uhs2_caps {
-	/* TODO: Add UHS-II capabilities for the host. */
+	u32	dap;
+	u32	gap;
+	u32	maxblk_len;
+	u32	n_fcu;
+	u8	n_lanes;
+	u8	addr64;
+	u8	card_type;
+	u8	phy_rev;
+	u8	speed_range;
+	u8	can_hibernate;
+	u8	n_lss_sync;
+	u8	n_lss_dir;
+	u8	link_rev;
+	u8	host_type;
+	u8	n_data_gap;
+
+	u32	maxblk_len_set;
+	u32	n_fcu_set;
+	u8	n_lanes_set;
+	u8	n_lss_sync_set;
+	u8	n_lss_dir_set;
+	u8	n_data_gap_set;
+	u8	max_retry_set;
 };
 
 struct mmc_host;
@@ -145,7 +167,6 @@ struct mmc_host_ops {
 	 */
 	int	(*uhs2_set_ios)(struct mmc_host *host, struct mmc_ios *ios);
 
-
 	/*
 	 * Return values for the get_ro callback should be:
 	 *   0 for a read/write card
@@ -421,7 +442,7 @@ struct mmc_host {
 #define MMC_CAP2_CRYPTO		0
 #endif
 
-	struct sd_uhs2_caps	uhs2_caps;	/* SD UHS-II capabilities */
+	struct sd_uhs2_caps	uhs2_caps;	/* SD UHS-II host capabilities */
 
 	int			fixed_drv_type;	/* fixed driver type for non-removable media */
 
diff --git a/include/linux/mmc/sd_uhs2.h b/include/linux/mmc/sd_uhs2.h
new file mode 100644
index 000000000..5d12fb9d0
--- /dev/null
+++ b/include/linux/mmc/sd_uhs2.h
@@ -0,0 +1,196 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Header file for UHS-II packets, Host Controller registers and I/O
+ * accessors.
+ *
+ *  Copyright (C) 2014 Intel Corp, All Rights Reserved.
+ */
+#ifndef LINUX_MMC_UHS2_H
+#define LINUX_MMC_UHS2_H
+
+struct mmc_request;
+
+/* LINK Layer definition */
+/* UHS2 Header */
+#define UHS2_NATIVE_PACKET_POS	7
+#define UHS2_NATIVE_PACKET	(1 << UHS2_NATIVE_PACKET_POS)
+
+#define UHS2_PACKET_TYPE_POS	4
+#define UHS2_PACKET_TYPE_CCMD	(0 << UHS2_PACKET_TYPE_POS)
+#define UHS2_PACKET_TYPE_DCMD	(1 << UHS2_PACKET_TYPE_POS)
+#define UHS2_PACKET_TYPE_RES	(2 << UHS2_PACKET_TYPE_POS)
+#define UHS2_PACKET_TYPE_DATA	(3 << UHS2_PACKET_TYPE_POS)
+#define UHS2_PACKET_TYPE_MSG	(7 << UHS2_PACKET_TYPE_POS)
+
+#define UHS2_DEST_ID_MASK	0x0F
+#define UHS2_DEST_ID		0x1
+
+#define UHS2_SRC_ID_POS		12
+#define UHS2_SRC_ID_MASK	0xF000
+
+#define UHS2_TRANS_ID_POS	8
+#define UHS2_TRANS_ID_MASK	0x0700
+
+/* UHS2 MSG */
+#define UHS2_MSG_CTG_POS	5
+#define UHS2_MSG_CTG_LMSG	0x00
+#define UHS2_MSG_CTG_INT	0x60
+#define UHS2_MSG_CTG_AMSG	0x80
+
+#define UHS2_MSG_CTG_FCREQ	0x00
+#define UHS2_MSG_CTG_FCRDY	0x01
+#define UHS2_MSG_CTG_STAT	0x02
+
+#define UHS2_MSG_CODE_POS			8
+#define UHS2_MSG_CODE_FC_UNRECOVER_ERR		0x8
+#define UHS2_MSG_CODE_STAT_UNRECOVER_ERR	0x8
+#define UHS2_MSG_CODE_STAT_RECOVER_ERR		0x1
+
+/* TRANS Layer definition */
+
+/* Native packets*/
+#define UHS2_NATIVE_CMD_RW_POS	7
+#define UHS2_NATIVE_CMD_WRITE	(1 << UHS2_NATIVE_CMD_RW_POS)
+#define UHS2_NATIVE_CMD_READ	(0 << UHS2_NATIVE_CMD_RW_POS)
+
+#define UHS2_NATIVE_CMD_PLEN_POS	4
+#define UHS2_NATIVE_CMD_PLEN_4B		(1 << UHS2_NATIVE_CMD_PLEN_POS)
+#define UHS2_NATIVE_CMD_PLEN_8B		(2 << UHS2_NATIVE_CMD_PLEN_POS)
+#define UHS2_NATIVE_CMD_PLEN_16B	(3 << UHS2_NATIVE_CMD_PLEN_POS)
+
+#define UHS2_NATIVE_CCMD_GET_MIOADR_MASK	0xF00
+#define UHS2_NATIVE_CCMD_MIOADR_MASK		0x0F
+
+#define UHS2_NATIVE_CCMD_LIOADR_POS		8
+#define UHS2_NATIVE_CCMD_GET_LIOADR_MASK	0x0FF
+
+#define UHS2_DCMD_DM_POS	6
+#define UHS2_DCMD_2L_HD_MODE	(1 << UHS2_DCMD_DM_POS)
+#define UHS2_DCMD_LM_POS	5
+#define UHS2_DCMD_LM_TLEN_EXIST	(1 << UHS2_DCMD_LM_POS)
+#define UHS2_DCMD_TLUM_POS	4
+#define UHS2_DCMD_TLUM_BYTE_MODE	(1 << UHS2_DCMD_TLUM_POS)
+#define UHS2_NATIVE_DCMD_DAM_POS	3
+#define UHS2_NATIVE_DCMD_DAM_IO		(1 << UHS2_NATIVE_DCMD_DAM_POS)
+
+#define UHS2_RES_NACK_POS	7
+#define UHS2_RES_NACK_MASK	(0x1 << UHS2_RES_NACK_POS)
+
+#define UHS2_RES_ECODE_POS	4
+#define UHS2_RES_ECODE_MASK	0x7
+#define UHS2_RES_ECODE_COND	1
+#define UHS2_RES_ECODE_ARG	2
+#define UHS2_RES_ECODE_GEN	3
+
+/* IOADR of device registers */
+#define UHS2_IOADR_GENERIC_CAPS		0x00
+#define UHS2_IOADR_PHY_CAPS		0x02
+#define UHS2_IOADR_LINK_CAPS		0x04
+#define UHS2_IOADR_RSV_CAPS		0x06
+#define UHS2_IOADR_GENERIC_SETTINGS	0x08
+#define UHS2_IOADR_PHY_SETTINGS		0x0A
+#define UHS2_IOADR_LINK_SETTINGS	0x0C
+#define UHS2_IOADR_PRESET		0x40
+
+/* SD application packets */
+#define UHS2_SD_CMD_INDEX_POS		8
+
+#define UHS2_SD_CMD_APP_POS		14
+#define UHS2_SD_CMD_APP			(1 << UHS2_SD_CMD_APP_POS)
+
+struct uhs2_command {
+	u16	header;
+	u16	arg;
+	u32	*payload;
+	u32	payload_len;
+	u32	packet_len;
+};
+
+enum uhs2_act {
+	SET_CONFIG,
+	ENABLE_INT,
+	DISABLE_INT,
+	SET_SPEED_B,
+	CHECK_DORMANT,
+	UHS2_SW_RESET,
+};
+
+/* UHS-II Device Registers */
+#define UHS2_DEV_CONFIG_REG	0x000
+
+/* General Caps and Settings registers */
+#define  UHS2_DEV_CONFIG_GEN_CAPS	(UHS2_DEV_CONFIG_REG + 0x000)
+#define   UHS2_DEV_CONFIG_N_LANES_POS	8
+#define   UHS2_DEV_CONFIG_N_LANES_MASK	0x3F
+#define   UHS2_DEV_CONFIG_2L_HD_FD	0x1
+#define   UHS2_DEV_CONFIG_2D1U_FD	0x2
+#define   UHS2_DEV_CONFIG_1D2U_FD	0x4
+#define   UHS2_DEV_CONFIG_2D2U_FD	0x8
+#define   UHS2_DEV_CONFIG_DADR_POS	14
+#define   UHS2_DEV_CONFIG_DADR_MASK	0x1
+#define   UHS2_DEV_CONFIG_APP_POS	16
+#define   UHS2_DEV_CONFIG_APP_MASK	0xFF
+#define   UHS2_DEV_CONFIG_APP_SD_MEM	0x1
+
+#define  UHS2_DEV_CONFIG_GEN_SET	(UHS2_DEV_CONFIG_REG + 0x008)
+#define   UHS2_DEV_CONFIG_GEN_SET_N_LANES_POS	8
+#define   UHS2_DEV_CONFIG_GEN_SET_2L_FD_HD	0x0
+#define   UHS2_DEV_CONFIG_GEN_SET_2D1U_FD	0x2
+#define   UHS2_DEV_CONFIG_GEN_SET_1D2U_FD	0x3
+#define   UHS2_DEV_CONFIG_GEN_SET_2D2U_FD	0x4
+#define   UHS2_DEV_CONFIG_GEN_SET_CFG_COMPLETE	(0x1 << 31)
+
+/* PHY Caps and Settings registers */
+#define  UHS2_DEV_CONFIG_PHY_CAPS	(UHS2_DEV_CONFIG_REG + 0x002)
+#define   UHS2_DEV_CONFIG_PHY_MINOR_MASK	0xF
+#define   UHS2_DEV_CONFIG_PHY_MAJOR_POS		4
+#define   UHS2_DEV_CONFIG_PHY_MAJOR_MASK	0x3
+#define   UHS2_DEV_CONFIG_CAN_HIBER_POS		15
+#define   UHS2_DEV_CONFIG_CAN_HIBER_MASK	0x1
+#define  UHS2_DEV_CONFIG_PHY_CAPS1	(UHS2_DEV_CONFIG_REG + 0x003)
+#define   UHS2_DEV_CONFIG_N_LSS_SYN_MASK	0xF
+#define   UHS2_DEV_CONFIG_N_LSS_DIR_POS		4
+#define   UHS2_DEV_CONFIG_N_LSS_DIR_MASK	0xF
+
+#define  UHS2_DEV_CONFIG_PHY_SET	(UHS2_DEV_CONFIG_REG + 0x00A)
+#define   UHS2_DEV_CONFIG_PHY_SET_SPEED_POS	6
+#define   UHS2_DEV_CONFIG_PHY_SET_SPEED_A	0x0
+#define   UHS2_DEV_CONFIG_PHY_SET_SPEED_B	0x1
+
+/* LINK-TRAN Caps and Settings registers */
+#define  UHS2_DEV_CONFIG_LINK_TRAN_CAPS	(UHS2_DEV_CONFIG_REG + 0x004)
+#define   UHS2_DEV_CONFIG_LT_MINOR_MASK		0xF
+#define   UHS2_DEV_CONFIG_LT_MAJOR_POS		4
+#define   UHS2_DEV_CONFIG_LT_MAJOR_MASK		0x3
+#define   UHS2_DEV_CONFIG_N_FCU_POS		8
+#define   UHS2_DEV_CONFIG_N_FCU_MASK		0xFF
+#define   UHS2_DEV_CONFIG_DEV_TYPE_POS		16
+#define   UHS2_DEV_CONFIG_DEV_TYPE_MASK		0x7
+#define   UHS2_DEV_CONFIG_MAX_BLK_LEN_POS	20
+#define   UHS2_DEV_CONFIG_MAX_BLK_LEN_MASK	0xFFF
+#define  UHS2_DEV_CONFIG_LINK_TRAN_CAPS1	(UHS2_DEV_CONFIG_REG + 0x005)
+#define   UHS2_DEV_CONFIG_N_DATA_GAP_MASK	0xFF
+
+#define  UHS2_DEV_CONFIG_LINK_TRAN_SET	(UHS2_DEV_CONFIG_REG + 0x00C)
+#define   UHS2_DEV_CONFIG_LT_SET_MAX_BLK_LEN	0x200
+#define   UHS2_DEV_CONFIG_LT_SET_MAX_RETRY_POS	16
+
+/* Preset register */
+#define  UHS2_DEV_CONFIG_PRESET		(UHS2_DEV_CONFIG_REG + 0x040)
+
+#define UHS2_DEV_INT_REG	0x100
+
+#define UHS2_DEV_STATUS_REG	0x180
+
+#define UHS2_DEV_CMD_REG	0x200
+#define  UHS2_DEV_CMD_FULL_RESET	(UHS2_DEV_CMD_REG + 0x000)
+#define  UHS2_DEV_CMD_GO_DORMANT_STATE	(UHS2_DEV_CMD_REG + 0x001)
+#define   UHS2_DEV_CMD_DORMANT_HIBER	(0x1 << 7)
+#define  UHS2_DEV_CMD_DEVICE_INIT	(UHS2_DEV_CMD_REG + 0x002)
+#define  UHS2_DEV_CMD_ENUMERATE		(UHS2_DEV_CMD_REG + 0x003)
+#define  UHS2_DEV_CMD_TRANS_ABORT	(UHS2_DEV_CMD_REG + 0x004)
+
+#define UHS2_RCLK_MAX	52000000
+#define UHS2_RCLK_MIN	26000000
+
+#endif /* LINUX_MMC_UHS2_H */
-- 
2.34.0

Re: [PATCH 5/7] mmc: add UHS-II related definitions in headers

[Ulf Hansson]

On Fri, 3 Dec 2021 at 11:51, Jason Lai <jasonlai.genesyslogic@gmail.com> wrote:
>
> From: Jason Lai <jason.lai@genesyslogic.com.tw>
>
> All LINK layer messages, registers and SD-TRAN command packet described in
> 'Part 1 UHS-II Addendum Ver 1.01' are defined in include/linux/mmc/sd_uhs2.h
>
> drivers/mmc/core/sd_uhs2.h contains exported function prototype.
>
> Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
> ---
>  drivers/mmc/core/sd_uhs2.h  |  18 ++++
>  include/linux/mmc/card.h    |  30 +++++-
>  include/linux/mmc/core.h    |   4 +-
>  include/linux/mmc/host.h    |  27 ++++-
>  include/linux/mmc/sd_uhs2.h | 196 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 268 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/mmc/core/sd_uhs2.h
>  create mode 100644 include/linux/mmc/sd_uhs2.h
>
> diff --git a/drivers/mmc/core/sd_uhs2.h b/drivers/mmc/core/sd_uhs2.h
> new file mode 100644
> index 000000000..5bb5dc1d1
> --- /dev/null
> +++ b/drivers/mmc/core/sd_uhs2.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Header file for UHS-II packets, Host Controller registers and I/O
> + * accessors.
> + *
> + *  Copyright (C) 2014 Intel Corp, All Rights Reserved.
> + */
> +#ifndef MMC_UHS2_H
> +#define MMC_UHS2_H
> +
> +#include <linux/mmc/core.h>
> +#include <linux/mmc/host.h>

This is wrong, as these includes should instead be added into those
c-files that need them. Please drop this.

I noticed that you use struct mmc_host *host and struct mmc_request
*mrq, below. That can be done by forward declaring them, like this:
struct mmc_host;
struct mmc_request;

> +
> +#define UHS2_PHY_INIT_ERR      1

Please use common error codes, so drop this.

> +
> +int sd_uhs2_prepare_cmd(struct mmc_host *host, struct mmc_request *mrq);
> +
> +#endif /* MMC_UHS2_H */
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 82b07eac1..4b2fda2e6 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -211,8 +211,36 @@ struct sd_ext_reg {
>  };
>
>  struct sd_uhs2_config {
> -       u32                     node_id;
> +       u32     node_id;

Please make the above change part of the patch that introduced "node_id".

>         /* TODO: Extend with more register configs. */

Looks like $subject patch is adding the register configs, so I assume
it would make sense to drop the TODO comment above, right?

> +       u32     dap;
> +       u32     gap;
> +       u32     n_fcu;
> +       u32     maxblk_len;
> +       u8      n_lanes;
> +       u8      dadr_len;
> +       u8      app_type;
> +       u8      phy_minor_rev;
> +       u8      phy_major_rev;
> +       u8      can_hibernate;
> +       u8      n_lss_sync;
> +       u8      n_lss_dir;
> +       u8      link_minor_rev;
> +       u8      link_major_rev;
> +       u8      dev_type;
> +       u8      n_data_gap;
> +
> +       u32     n_fcu_set;
> +       u32     maxblk_len_set;
> +       u8      n_lanes_set;
> +       u8      speed_range_set;
> +       u8      n_lss_sync_set;
> +       u8      n_lss_dir_set;
> +       u8      n_data_gap_set;
> +       u8      pwrctrl_mode_set;
> +       u8      max_retry_set;
> +
> +       u8      cfg_complete;
>  };
>
>  struct sdio_cccr {
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index ab19245e9..8ac4b0b52 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -1,7 +1,5 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - *  linux/include/linux/mmc/core.h
> - */

It's okay to remove these lines. However, it should be a separate
patch - and please keep it outside of the UHS-II series, as it doesn't
belong here.

> +
>  #ifndef LINUX_MMC_CORE_H
>  #define LINUX_MMC_CORE_H
>
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 69f8c8a8f..ad6cccf67 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -96,7 +96,29 @@ struct mmc_clk_phase_map {
>  };
>
>  struct sd_uhs2_caps {
> -       /* TODO: Add UHS-II capabilities for the host. */
> +       u32     dap;
> +       u32     gap;
> +       u32     maxblk_len;
> +       u32     n_fcu;
> +       u8      n_lanes;
> +       u8      addr64;
> +       u8      card_type;
> +       u8      phy_rev;
> +       u8      speed_range;
> +       u8      can_hibernate;
> +       u8      n_lss_sync;
> +       u8      n_lss_dir;
> +       u8      link_rev;
> +       u8      host_type;
> +       u8      n_data_gap;
> +
> +       u32     maxblk_len_set;
> +       u32     n_fcu_set;
> +       u8      n_lanes_set;
> +       u8      n_lss_sync_set;
> +       u8      n_lss_dir_set;
> +       u8      n_data_gap_set;
> +       u8      max_retry_set;
>  };
>
>  struct mmc_host;
> @@ -145,7 +167,6 @@ struct mmc_host_ops {
>          */
>         int     (*uhs2_set_ios)(struct mmc_host *host, struct mmc_ios *ios);
>
> -

White space, please drop this change.

If this makes the code better, please change this in the patch that
introduced the code earlier in the series.

>         /*
>          * Return values for the get_ro callback should be:
>          *   0 for a read/write card
> @@ -421,7 +442,7 @@ struct mmc_host {
>  #define MMC_CAP2_CRYPTO                0
>  #endif
>
> -       struct sd_uhs2_caps     uhs2_caps;      /* SD UHS-II capabilities */
> +       struct sd_uhs2_caps     uhs2_caps;      /* SD UHS-II host capabilities */

If you prefer "host capabilities" over plain "capabilities", that's fine by me.

However, please make this change as part of the patch that introduced
the code, earlier in the series.

>
>         int                     fixed_drv_type; /* fixed driver type for non-removable media */
>
> diff --git a/include/linux/mmc/sd_uhs2.h b/include/linux/mmc/sd_uhs2.h
> new file mode 100644
> index 000000000..5d12fb9d0
> --- /dev/null
> +++ b/include/linux/mmc/sd_uhs2.h
> @@ -0,0 +1,196 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Header file for UHS-II packets, Host Controller registers and I/O
> + * accessors.
> + *
> + *  Copyright (C) 2014 Intel Corp, All Rights Reserved.
> + */
> +#ifndef LINUX_MMC_UHS2_H
> +#define LINUX_MMC_UHS2_H
> +
> +struct mmc_request;

Doesn't look like this is needed, please drop it.

> +
> +/* LINK Layer definition */
> +/* UHS2 Header */
> +#define UHS2_NATIVE_PACKET_POS 7
> +#define UHS2_NATIVE_PACKET     (1 << UHS2_NATIVE_PACKET_POS)
> +
> +#define UHS2_PACKET_TYPE_POS   4
> +#define UHS2_PACKET_TYPE_CCMD  (0 << UHS2_PACKET_TYPE_POS)
> +#define UHS2_PACKET_TYPE_DCMD  (1 << UHS2_PACKET_TYPE_POS)
> +#define UHS2_PACKET_TYPE_RES   (2 << UHS2_PACKET_TYPE_POS)
> +#define UHS2_PACKET_TYPE_DATA  (3 << UHS2_PACKET_TYPE_POS)
> +#define UHS2_PACKET_TYPE_MSG   (7 << UHS2_PACKET_TYPE_POS)
> +
> +#define UHS2_DEST_ID_MASK      0x0F
> +#define UHS2_DEST_ID           0x1
> +
> +#define UHS2_SRC_ID_POS                12
> +#define UHS2_SRC_ID_MASK       0xF000
> +
> +#define UHS2_TRANS_ID_POS      8
> +#define UHS2_TRANS_ID_MASK     0x0700
> +
> +/* UHS2 MSG */
> +#define UHS2_MSG_CTG_POS       5
> +#define UHS2_MSG_CTG_LMSG      0x00
> +#define UHS2_MSG_CTG_INT       0x60
> +#define UHS2_MSG_CTG_AMSG      0x80
> +
> +#define UHS2_MSG_CTG_FCREQ     0x00
> +#define UHS2_MSG_CTG_FCRDY     0x01
> +#define UHS2_MSG_CTG_STAT      0x02
> +
> +#define UHS2_MSG_CODE_POS                      8
> +#define UHS2_MSG_CODE_FC_UNRECOVER_ERR         0x8
> +#define UHS2_MSG_CODE_STAT_UNRECOVER_ERR       0x8
> +#define UHS2_MSG_CODE_STAT_RECOVER_ERR         0x1
> +
> +/* TRANS Layer definition */
> +
> +/* Native packets*/
> +#define UHS2_NATIVE_CMD_RW_POS 7
> +#define UHS2_NATIVE_CMD_WRITE  (1 << UHS2_NATIVE_CMD_RW_POS)
> +#define UHS2_NATIVE_CMD_READ   (0 << UHS2_NATIVE_CMD_RW_POS)
> +
> +#define UHS2_NATIVE_CMD_PLEN_POS       4
> +#define UHS2_NATIVE_CMD_PLEN_4B                (1 << UHS2_NATIVE_CMD_PLEN_POS)
> +#define UHS2_NATIVE_CMD_PLEN_8B                (2 << UHS2_NATIVE_CMD_PLEN_POS)
> +#define UHS2_NATIVE_CMD_PLEN_16B       (3 << UHS2_NATIVE_CMD_PLEN_POS)
> +
> +#define UHS2_NATIVE_CCMD_GET_MIOADR_MASK       0xF00
> +#define UHS2_NATIVE_CCMD_MIOADR_MASK           0x0F
> +
> +#define UHS2_NATIVE_CCMD_LIOADR_POS            8
> +#define UHS2_NATIVE_CCMD_GET_LIOADR_MASK       0x0FF
> +
> +#define UHS2_DCMD_DM_POS       6
> +#define UHS2_DCMD_2L_HD_MODE   (1 << UHS2_DCMD_DM_POS)
> +#define UHS2_DCMD_LM_POS       5
> +#define UHS2_DCMD_LM_TLEN_EXIST        (1 << UHS2_DCMD_LM_POS)
> +#define UHS2_DCMD_TLUM_POS     4
> +#define UHS2_DCMD_TLUM_BYTE_MODE       (1 << UHS2_DCMD_TLUM_POS)
> +#define UHS2_NATIVE_DCMD_DAM_POS       3
> +#define UHS2_NATIVE_DCMD_DAM_IO                (1 << UHS2_NATIVE_DCMD_DAM_POS)
> +
> +#define UHS2_RES_NACK_POS      7
> +#define UHS2_RES_NACK_MASK     (0x1 << UHS2_RES_NACK_POS)
> +
> +#define UHS2_RES_ECODE_POS     4
> +#define UHS2_RES_ECODE_MASK    0x7
> +#define UHS2_RES_ECODE_COND    1
> +#define UHS2_RES_ECODE_ARG     2
> +#define UHS2_RES_ECODE_GEN     3
> +
> +/* IOADR of device registers */
> +#define UHS2_IOADR_GENERIC_CAPS                0x00
> +#define UHS2_IOADR_PHY_CAPS            0x02
> +#define UHS2_IOADR_LINK_CAPS           0x04
> +#define UHS2_IOADR_RSV_CAPS            0x06
> +#define UHS2_IOADR_GENERIC_SETTINGS    0x08
> +#define UHS2_IOADR_PHY_SETTINGS                0x0A
> +#define UHS2_IOADR_LINK_SETTINGS       0x0C
> +#define UHS2_IOADR_PRESET              0x40
> +
> +/* SD application packets */
> +#define UHS2_SD_CMD_INDEX_POS          8
> +
> +#define UHS2_SD_CMD_APP_POS            14
> +#define UHS2_SD_CMD_APP                        (1 << UHS2_SD_CMD_APP_POS)
> +
> +struct uhs2_command {
> +       u16     header;
> +       u16     arg;
> +       u32     *payload;
> +       u32     payload_len;
> +       u32     packet_len;
> +};
> +
> +enum uhs2_act {

Perhaps uhs2_action is more clear?

> +       SET_CONFIG,
> +       ENABLE_INT,
> +       DISABLE_INT,
> +       SET_SPEED_B,
> +       CHECK_DORMANT,
> +       UHS2_SW_RESET,
> +};
> +
> +/* UHS-II Device Registers */
> +#define UHS2_DEV_CONFIG_REG    0x000
> +
> +/* General Caps and Settings registers */
> +#define  UHS2_DEV_CONFIG_GEN_CAPS      (UHS2_DEV_CONFIG_REG + 0x000)
> +#define   UHS2_DEV_CONFIG_N_LANES_POS  8
> +#define   UHS2_DEV_CONFIG_N_LANES_MASK 0x3F
> +#define   UHS2_DEV_CONFIG_2L_HD_FD     0x1
> +#define   UHS2_DEV_CONFIG_2D1U_FD      0x2
> +#define   UHS2_DEV_CONFIG_1D2U_FD      0x4
> +#define   UHS2_DEV_CONFIG_2D2U_FD      0x8
> +#define   UHS2_DEV_CONFIG_DADR_POS     14
> +#define   UHS2_DEV_CONFIG_DADR_MASK    0x1
> +#define   UHS2_DEV_CONFIG_APP_POS      16
> +#define   UHS2_DEV_CONFIG_APP_MASK     0xFF
> +#define   UHS2_DEV_CONFIG_APP_SD_MEM   0x1
> +
> +#define  UHS2_DEV_CONFIG_GEN_SET       (UHS2_DEV_CONFIG_REG + 0x008)
> +#define   UHS2_DEV_CONFIG_GEN_SET_N_LANES_POS  8
> +#define   UHS2_DEV_CONFIG_GEN_SET_2L_FD_HD     0x0
> +#define   UHS2_DEV_CONFIG_GEN_SET_2D1U_FD      0x2
> +#define   UHS2_DEV_CONFIG_GEN_SET_1D2U_FD      0x3
> +#define   UHS2_DEV_CONFIG_GEN_SET_2D2U_FD      0x4
> +#define   UHS2_DEV_CONFIG_GEN_SET_CFG_COMPLETE (0x1 << 31)
> +
> +/* PHY Caps and Settings registers */
> +#define  UHS2_DEV_CONFIG_PHY_CAPS      (UHS2_DEV_CONFIG_REG + 0x002)
> +#define   UHS2_DEV_CONFIG_PHY_MINOR_MASK       0xF
> +#define   UHS2_DEV_CONFIG_PHY_MAJOR_POS                4
> +#define   UHS2_DEV_CONFIG_PHY_MAJOR_MASK       0x3
> +#define   UHS2_DEV_CONFIG_CAN_HIBER_POS                15
> +#define   UHS2_DEV_CONFIG_CAN_HIBER_MASK       0x1
> +#define  UHS2_DEV_CONFIG_PHY_CAPS1     (UHS2_DEV_CONFIG_REG + 0x003)
> +#define   UHS2_DEV_CONFIG_N_LSS_SYN_MASK       0xF
> +#define   UHS2_DEV_CONFIG_N_LSS_DIR_POS                4
> +#define   UHS2_DEV_CONFIG_N_LSS_DIR_MASK       0xF
> +
> +#define  UHS2_DEV_CONFIG_PHY_SET       (UHS2_DEV_CONFIG_REG + 0x00A)
> +#define   UHS2_DEV_CONFIG_PHY_SET_SPEED_POS    6
> +#define   UHS2_DEV_CONFIG_PHY_SET_SPEED_A      0x0
> +#define   UHS2_DEV_CONFIG_PHY_SET_SPEED_B      0x1
> +
> +/* LINK-TRAN Caps and Settings registers */
> +#define  UHS2_DEV_CONFIG_LINK_TRAN_CAPS        (UHS2_DEV_CONFIG_REG + 0x004)
> +#define   UHS2_DEV_CONFIG_LT_MINOR_MASK                0xF
> +#define   UHS2_DEV_CONFIG_LT_MAJOR_POS         4
> +#define   UHS2_DEV_CONFIG_LT_MAJOR_MASK                0x3
> +#define   UHS2_DEV_CONFIG_N_FCU_POS            8
> +#define   UHS2_DEV_CONFIG_N_FCU_MASK           0xFF
> +#define   UHS2_DEV_CONFIG_DEV_TYPE_POS         16
> +#define   UHS2_DEV_CONFIG_DEV_TYPE_MASK                0x7
> +#define   UHS2_DEV_CONFIG_MAX_BLK_LEN_POS      20
> +#define   UHS2_DEV_CONFIG_MAX_BLK_LEN_MASK     0xFFF
> +#define  UHS2_DEV_CONFIG_LINK_TRAN_CAPS1       (UHS2_DEV_CONFIG_REG + 0x005)
> +#define   UHS2_DEV_CONFIG_N_DATA_GAP_MASK      0xFF
> +
> +#define  UHS2_DEV_CONFIG_LINK_TRAN_SET (UHS2_DEV_CONFIG_REG + 0x00C)
> +#define   UHS2_DEV_CONFIG_LT_SET_MAX_BLK_LEN   0x200
> +#define   UHS2_DEV_CONFIG_LT_SET_MAX_RETRY_POS 16
> +
> +/* Preset register */
> +#define  UHS2_DEV_CONFIG_PRESET                (UHS2_DEV_CONFIG_REG + 0x040)
> +
> +#define UHS2_DEV_INT_REG       0x100
> +
> +#define UHS2_DEV_STATUS_REG    0x180
> +
> +#define UHS2_DEV_CMD_REG       0x200
> +#define  UHS2_DEV_CMD_FULL_RESET       (UHS2_DEV_CMD_REG + 0x000)
> +#define  UHS2_DEV_CMD_GO_DORMANT_STATE (UHS2_DEV_CMD_REG + 0x001)
> +#define   UHS2_DEV_CMD_DORMANT_HIBER   (0x1 << 7)
> +#define  UHS2_DEV_CMD_DEVICE_INIT      (UHS2_DEV_CMD_REG + 0x002)
> +#define  UHS2_DEV_CMD_ENUMERATE                (UHS2_DEV_CMD_REG + 0x003)
> +#define  UHS2_DEV_CMD_TRANS_ABORT      (UHS2_DEV_CMD_REG + 0x004)
> +
> +#define UHS2_RCLK_MAX  52000000
> +#define UHS2_RCLK_MIN  26000000
> +
> +#endif /* LINUX_MMC_UHS2_H */

Kind regards
Uffe

Re: [PATCH 5/7] mmc: add UHS-II related definitions in headers

[Lai Jason]

On Tue, Dec 14, 2021 at 9:37 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Fri, 3 Dec 2021 at 11:51, Jason Lai <jasonlai.genesyslogic@gmail.com> wrote:
> >
> > From: Jason Lai <jason.lai@genesyslogic.com.tw>
> >
> > All LINK layer messages, registers and SD-TRAN command packet described in
> > 'Part 1 UHS-II Addendum Ver 1.01' are defined in include/linux/mmc/sd_uhs2.h
> >
> > drivers/mmc/core/sd_uhs2.h contains exported function prototype.
> >
> > Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
> > ---
> >  drivers/mmc/core/sd_uhs2.h  |  18 ++++
> >  include/linux/mmc/card.h    |  30 +++++-
> >  include/linux/mmc/core.h    |   4 +-
> >  include/linux/mmc/host.h    |  27 ++++-
> >  include/linux/mmc/sd_uhs2.h | 196 ++++++++++++++++++++++++++++++++++++
> >  5 files changed, 268 insertions(+), 7 deletions(-)
> >  create mode 100644 drivers/mmc/core/sd_uhs2.h
> >  create mode 100644 include/linux/mmc/sd_uhs2.h
> >
> > diff --git a/drivers/mmc/core/sd_uhs2.h b/drivers/mmc/core/sd_uhs2.h
> > new file mode 100644
> > index 000000000..5bb5dc1d1
> > --- /dev/null
> > +++ b/drivers/mmc/core/sd_uhs2.h
> > @@ -0,0 +1,18 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Header file for UHS-II packets, Host Controller registers and I/O
> > + * accessors.
> > + *
> > + *  Copyright (C) 2014 Intel Corp, All Rights Reserved.
> > + */
> > +#ifndef MMC_UHS2_H
> > +#define MMC_UHS2_H
> > +
> > +#include <linux/mmc/core.h>
> > +#include <linux/mmc/host.h>
>
> This is wrong, as these includes should instead be added into those
> c-files that need them. Please drop this.

Okay. I will remove them in the next version.

> I noticed that you use struct mmc_host *host and struct mmc_request
> *mrq, below. That can be done by forward declaring them, like this:
> struct mmc_host;
> struct mmc_request;
>
> > +
> > +#define UHS2_PHY_INIT_ERR      1
>
> Please use common error codes, so drop this.

Okay. I will remove it in the next version.

> > +
> > +int sd_uhs2_prepare_cmd(struct mmc_host *host, struct mmc_request *mrq);
> > +
> > +#endif /* MMC_UHS2_H */
> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> > index 82b07eac1..4b2fda2e6 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -211,8 +211,36 @@ struct sd_ext_reg {
> >  };
> >
> >  struct sd_uhs2_config {
> > -       u32                     node_id;
> > +       u32     node_id;
>
> Please make the above change part of the patch that introduced "node_id".

I just aligned this variable with others. Is there anything that needs
to be changed?

> >         /* TODO: Extend with more register configs. */
>
> Looks like $subject patch is adding the register configs, so I assume
> it would make sense to drop the TODO comment above, right?

Okay.  It will be removed in the next version.

> > +       u32     dap;
> > +       u32     gap;
> > +       u32     n_fcu;
> > +       u32     maxblk_len;
> > +       u8      n_lanes;
> > +       u8      dadr_len;
> > +       u8      app_type;
> > +       u8      phy_minor_rev;
> > +       u8      phy_major_rev;
> > +       u8      can_hibernate;
> > +       u8      n_lss_sync;
> > +       u8      n_lss_dir;
> > +       u8      link_minor_rev;
> > +       u8      link_major_rev;
> > +       u8      dev_type;
> > +       u8      n_data_gap;
> > +
> > +       u32     n_fcu_set;
> > +       u32     maxblk_len_set;
> > +       u8      n_lanes_set;
> > +       u8      speed_range_set;
> > +       u8      n_lss_sync_set;
> > +       u8      n_lss_dir_set;
> > +       u8      n_data_gap_set;
> > +       u8      pwrctrl_mode_set;
> > +       u8      max_retry_set;
> > +
> > +       u8      cfg_complete;
> >  };
> >
> >  struct sdio_cccr {
> > diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> > index ab19245e9..8ac4b0b52 100644
> > --- a/include/linux/mmc/core.h
> > +++ b/include/linux/mmc/core.h
> > @@ -1,7 +1,5 @@
> >  /* SPDX-License-Identifier: GPL-2.0-only */
> > -/*
> > - *  linux/include/linux/mmc/core.h
> > - */
>
> It's okay to remove these lines. However, it should be a separate
> patch - and please keep it outside of the UHS-II series, as it doesn't
> belong here.

Okay. I will put them back in the next version.
Should I create a patch for this rescovery?

> > +
> >  #ifndef LINUX_MMC_CORE_H
> >  #define LINUX_MMC_CORE_H
> >
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > index 69f8c8a8f..ad6cccf67 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -96,7 +96,29 @@ struct mmc_clk_phase_map {
> >  };
> >
> >  struct sd_uhs2_caps {
> > -       /* TODO: Add UHS-II capabilities for the host. */
> > +       u32     dap;
> > +       u32     gap;
> > +       u32     maxblk_len;
> > +       u32     n_fcu;
> > +       u8      n_lanes;
> > +       u8      addr64;
> > +       u8      card_type;
> > +       u8      phy_rev;
> > +       u8      speed_range;
> > +       u8      can_hibernate;
> > +       u8      n_lss_sync;
> > +       u8      n_lss_dir;
> > +       u8      link_rev;
> > +       u8      host_type;
> > +       u8      n_data_gap;
> > +
> > +       u32     maxblk_len_set;
> > +       u32     n_fcu_set;
> > +       u8      n_lanes_set;
> > +       u8      n_lss_sync_set;
> > +       u8      n_lss_dir_set;
> > +       u8      n_data_gap_set;
> > +       u8      max_retry_set;
> >  };
> >
> >  struct mmc_host;
> > @@ -145,7 +167,6 @@ struct mmc_host_ops {
> >          */
> >         int     (*uhs2_set_ios)(struct mmc_host *host, struct mmc_ios *ios);
> >
> > -
>
> White space, please drop this change.

Okay. I will drop it in the next version.

> If this makes the code better, please change this in the patch that
> introduced the code earlier in the series.
>
> >         /*
> >          * Return values for the get_ro callback should be:
> >          *   0 for a read/write card
> > @@ -421,7 +442,7 @@ struct mmc_host {
> >  #define MMC_CAP2_CRYPTO                0
> >  #endif
> >
> > -       struct sd_uhs2_caps     uhs2_caps;      /* SD UHS-II capabilities */
> > +       struct sd_uhs2_caps     uhs2_caps;      /* SD UHS-II host capabilities */
>
> If you prefer "host capabilities" over plain "capabilities", that's fine by me.
>
> However, please make this change as part of the patch that introduced
> the code, earlier in the series.

I did not change the variable name, I modified the comment just to
remind me what
the capability is used for. (host side or device side)

Shall I do anything related to this comment change?
If I revert to the old comment in the next version, should I create a
separate patch for it?

> >
> >         int                     fixed_drv_type; /* fixed driver type for non-removable media */
> >
> > diff --git a/include/linux/mmc/sd_uhs2.h b/include/linux/mmc/sd_uhs2.h
> > new file mode 100644
> > index 000000000..5d12fb9d0
> > --- /dev/null
> > +++ b/include/linux/mmc/sd_uhs2.h
> > @@ -0,0 +1,196 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Header file for UHS-II packets, Host Controller registers and I/O
> > + * accessors.
> > + *
> > + *  Copyright (C) 2014 Intel Corp, All Rights Reserved.
> > + */
> > +#ifndef LINUX_MMC_UHS2_H
> > +#define LINUX_MMC_UHS2_H
> > +
> > +struct mmc_request;
>
> Doesn't look like this is needed, please drop it.

Okay. I will drop them in the next version.

> > +
> > +/* LINK Layer definition */
> > +/* UHS2 Header */
> > +#define UHS2_NATIVE_PACKET_POS 7
> > +#define UHS2_NATIVE_PACKET     (1 << UHS2_NATIVE_PACKET_POS)
> > +
> > +#define UHS2_PACKET_TYPE_POS   4
> > +#define UHS2_PACKET_TYPE_CCMD  (0 << UHS2_PACKET_TYPE_POS)
> > +#define UHS2_PACKET_TYPE_DCMD  (1 << UHS2_PACKET_TYPE_POS)
> > +#define UHS2_PACKET_TYPE_RES   (2 << UHS2_PACKET_TYPE_POS)
> > +#define UHS2_PACKET_TYPE_DATA  (3 << UHS2_PACKET_TYPE_POS)
> > +#define UHS2_PACKET_TYPE_MSG   (7 << UHS2_PACKET_TYPE_POS)
> > +
> > +#define UHS2_DEST_ID_MASK      0x0F
> > +#define UHS2_DEST_ID           0x1
> > +
> > +#define UHS2_SRC_ID_POS                12
> > +#define UHS2_SRC_ID_MASK       0xF000
> > +
> > +#define UHS2_TRANS_ID_POS      8
> > +#define UHS2_TRANS_ID_MASK     0x0700
> > +
> > +/* UHS2 MSG */
> > +#define UHS2_MSG_CTG_POS       5
> > +#define UHS2_MSG_CTG_LMSG      0x00
> > +#define UHS2_MSG_CTG_INT       0x60
> > +#define UHS2_MSG_CTG_AMSG      0x80
> > +
> > +#define UHS2_MSG_CTG_FCREQ     0x00
> > +#define UHS2_MSG_CTG_FCRDY     0x01
> > +#define UHS2_MSG_CTG_STAT      0x02
> > +
> > +#define UHS2_MSG_CODE_POS                      8
> > +#define UHS2_MSG_CODE_FC_UNRECOVER_ERR         0x8
> > +#define UHS2_MSG_CODE_STAT_UNRECOVER_ERR       0x8
> > +#define UHS2_MSG_CODE_STAT_RECOVER_ERR         0x1
> > +
> > +/* TRANS Layer definition */
> > +
> > +/* Native packets*/
> > +#define UHS2_NATIVE_CMD_RW_POS 7
> > +#define UHS2_NATIVE_CMD_WRITE  (1 << UHS2_NATIVE_CMD_RW_POS)
> > +#define UHS2_NATIVE_CMD_READ   (0 << UHS2_NATIVE_CMD_RW_POS)
> > +
> > +#define UHS2_NATIVE_CMD_PLEN_POS       4
> > +#define UHS2_NATIVE_CMD_PLEN_4B                (1 << UHS2_NATIVE_CMD_PLEN_POS)
> > +#define UHS2_NATIVE_CMD_PLEN_8B                (2 << UHS2_NATIVE_CMD_PLEN_POS)
> > +#define UHS2_NATIVE_CMD_PLEN_16B       (3 << UHS2_NATIVE_CMD_PLEN_POS)
> > +
> > +#define UHS2_NATIVE_CCMD_GET_MIOADR_MASK       0xF00
> > +#define UHS2_NATIVE_CCMD_MIOADR_MASK           0x0F
> > +
> > +#define UHS2_NATIVE_CCMD_LIOADR_POS            8
> > +#define UHS2_NATIVE_CCMD_GET_LIOADR_MASK       0x0FF
> > +
> > +#define UHS2_DCMD_DM_POS       6
> > +#define UHS2_DCMD_2L_HD_MODE   (1 << UHS2_DCMD_DM_POS)
> > +#define UHS2_DCMD_LM_POS       5
> > +#define UHS2_DCMD_LM_TLEN_EXIST        (1 << UHS2_DCMD_LM_POS)
> > +#define UHS2_DCMD_TLUM_POS     4
> > +#define UHS2_DCMD_TLUM_BYTE_MODE       (1 << UHS2_DCMD_TLUM_POS)
> > +#define UHS2_NATIVE_DCMD_DAM_POS       3
> > +#define UHS2_NATIVE_DCMD_DAM_IO                (1 << UHS2_NATIVE_DCMD_DAM_POS)
> > +
> > +#define UHS2_RES_NACK_POS      7
> > +#define UHS2_RES_NACK_MASK     (0x1 << UHS2_RES_NACK_POS)
> > +
> > +#define UHS2_RES_ECODE_POS     4
> > +#define UHS2_RES_ECODE_MASK    0x7
> > +#define UHS2_RES_ECODE_COND    1
> > +#define UHS2_RES_ECODE_ARG     2
> > +#define UHS2_RES_ECODE_GEN     3
> > +
> > +/* IOADR of device registers */
> > +#define UHS2_IOADR_GENERIC_CAPS                0x00
> > +#define UHS2_IOADR_PHY_CAPS            0x02
> > +#define UHS2_IOADR_LINK_CAPS           0x04
> > +#define UHS2_IOADR_RSV_CAPS            0x06
> > +#define UHS2_IOADR_GENERIC_SETTINGS    0x08
> > +#define UHS2_IOADR_PHY_SETTINGS                0x0A
> > +#define UHS2_IOADR_LINK_SETTINGS       0x0C
> > +#define UHS2_IOADR_PRESET              0x40
> > +
> > +/* SD application packets */
> > +#define UHS2_SD_CMD_INDEX_POS          8
> > +
> > +#define UHS2_SD_CMD_APP_POS            14
> > +#define UHS2_SD_CMD_APP                        (1 << UHS2_SD_CMD_APP_POS)
> > +
> > +struct uhs2_command {
> > +       u16     header;
> > +       u16     arg;
> > +       u32     *payload;
> > +       u32     payload_len;
> > +       u32     packet_len;
> > +};
> > +
> > +enum uhs2_act {
>
> Perhaps uhs2_action is more clear?

Okay. I will change its name in the next version.

kind regards,
Jason Lai

> > +       SET_CONFIG,
> > +       ENABLE_INT,
> > +       DISABLE_INT,
> > +       SET_SPEED_B,
> > +       CHECK_DORMANT,
> > +       UHS2_SW_RESET,
> > +};
> > +
> > +/* UHS-II Device Registers */
> > +#define UHS2_DEV_CONFIG_REG    0x000
> > +
> > +/* General Caps and Settings registers */
> > +#define  UHS2_DEV_CONFIG_GEN_CAPS      (UHS2_DEV_CONFIG_REG + 0x000)
> > +#define   UHS2_DEV_CONFIG_N_LANES_POS  8
> > +#define   UHS2_DEV_CONFIG_N_LANES_MASK 0x3F
> > +#define   UHS2_DEV_CONFIG_2L_HD_FD     0x1
> > +#define   UHS2_DEV_CONFIG_2D1U_FD      0x2
> > +#define   UHS2_DEV_CONFIG_1D2U_FD      0x4
> > +#define   UHS2_DEV_CONFIG_2D2U_FD      0x8
> > +#define   UHS2_DEV_CONFIG_DADR_POS     14
> > +#define   UHS2_DEV_CONFIG_DADR_MASK    0x1
> > +#define   UHS2_DEV_CONFIG_APP_POS      16
> > +#define   UHS2_DEV_CONFIG_APP_MASK     0xFF
> > +#define   UHS2_DEV_CONFIG_APP_SD_MEM   0x1
> > +
> > +#define  UHS2_DEV_CONFIG_GEN_SET       (UHS2_DEV_CONFIG_REG + 0x008)
> > +#define   UHS2_DEV_CONFIG_GEN_SET_N_LANES_POS  8
> > +#define   UHS2_DEV_CONFIG_GEN_SET_2L_FD_HD     0x0
> > +#define   UHS2_DEV_CONFIG_GEN_SET_2D1U_FD      0x2
> > +#define   UHS2_DEV_CONFIG_GEN_SET_1D2U_FD      0x3
> > +#define   UHS2_DEV_CONFIG_GEN_SET_2D2U_FD      0x4
> > +#define   UHS2_DEV_CONFIG_GEN_SET_CFG_COMPLETE (0x1 << 31)
> > +
> > +/* PHY Caps and Settings registers */
> > +#define  UHS2_DEV_CONFIG_PHY_CAPS      (UHS2_DEV_CONFIG_REG + 0x002)
> > +#define   UHS2_DEV_CONFIG_PHY_MINOR_MASK       0xF
> > +#define   UHS2_DEV_CONFIG_PHY_MAJOR_POS                4
> > +#define   UHS2_DEV_CONFIG_PHY_MAJOR_MASK       0x3
> > +#define   UHS2_DEV_CONFIG_CAN_HIBER_POS                15
> > +#define   UHS2_DEV_CONFIG_CAN_HIBER_MASK       0x1
> > +#define  UHS2_DEV_CONFIG_PHY_CAPS1     (UHS2_DEV_CONFIG_REG + 0x003)
> > +#define   UHS2_DEV_CONFIG_N_LSS_SYN_MASK       0xF
> > +#define   UHS2_DEV_CONFIG_N_LSS_DIR_POS                4
> > +#define   UHS2_DEV_CONFIG_N_LSS_DIR_MASK       0xF
> > +
> > +#define  UHS2_DEV_CONFIG_PHY_SET       (UHS2_DEV_CONFIG_REG + 0x00A)
> > +#define   UHS2_DEV_CONFIG_PHY_SET_SPEED_POS    6
> > +#define   UHS2_DEV_CONFIG_PHY_SET_SPEED_A      0x0
> > +#define   UHS2_DEV_CONFIG_PHY_SET_SPEED_B      0x1
> > +
> > +/* LINK-TRAN Caps and Settings registers */
> > +#define  UHS2_DEV_CONFIG_LINK_TRAN_CAPS        (UHS2_DEV_CONFIG_REG + 0x004)
> > +#define   UHS2_DEV_CONFIG_LT_MINOR_MASK                0xF
> > +#define   UHS2_DEV_CONFIG_LT_MAJOR_POS         4
> > +#define   UHS2_DEV_CONFIG_LT_MAJOR_MASK                0x3
> > +#define   UHS2_DEV_CONFIG_N_FCU_POS            8
> > +#define   UHS2_DEV_CONFIG_N_FCU_MASK           0xFF
> > +#define   UHS2_DEV_CONFIG_DEV_TYPE_POS         16
> > +#define   UHS2_DEV_CONFIG_DEV_TYPE_MASK                0x7
> > +#define   UHS2_DEV_CONFIG_MAX_BLK_LEN_POS      20
> > +#define   UHS2_DEV_CONFIG_MAX_BLK_LEN_MASK     0xFFF
> > +#define  UHS2_DEV_CONFIG_LINK_TRAN_CAPS1       (UHS2_DEV_CONFIG_REG + 0x005)
> > +#define   UHS2_DEV_CONFIG_N_DATA_GAP_MASK      0xFF
> > +
> > +#define  UHS2_DEV_CONFIG_LINK_TRAN_SET (UHS2_DEV_CONFIG_REG + 0x00C)
> > +#define   UHS2_DEV_CONFIG_LT_SET_MAX_BLK_LEN   0x200
> > +#define   UHS2_DEV_CONFIG_LT_SET_MAX_RETRY_POS 16
> > +
> > +/* Preset register */
> > +#define  UHS2_DEV_CONFIG_PRESET                (UHS2_DEV_CONFIG_REG + 0x040)
> > +
> > +#define UHS2_DEV_INT_REG       0x100


> > +
> > +#define UHS2_DEV_STATUS_REG    0x180
> > +
> > +#define UHS2_DEV_CMD_REG       0x200
> > +#define  UHS2_DEV_CMD_FULL_RESET       (UHS2_DEV_CMD_REG + 0x000)
> > +#define  UHS2_DEV_CMD_GO_DORMANT_STATE (UHS2_DEV_CMD_REG + 0x001)
> > +#define   UHS2_DEV_CMD_DORMANT_HIBER   (0x1 << 7)
> > +#define  UHS2_DEV_CMD_DEVICE_INIT      (UHS2_DEV_CMD_REG + 0x002)
> > +#define  UHS2_DEV_CMD_ENUMERATE                (UHS2_DEV_CMD_REG + 0x003)
> > +#define  UHS2_DEV_CMD_TRANS_ABORT      (UHS2_DEV_CMD_REG + 0x004)
> > +
> > +#define UHS2_RCLK_MAX  52000000
> > +#define UHS2_RCLK_MIN  26000000
> > +
> > +#endif /* LINUX_MMC_UHS2_H */
>
> Kind regards
> Uffe

Re: [PATCH 5/7] mmc: add UHS-II related definitions in headers

[Ulf Hansson]

[...]

> > > diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> > > index ab19245e9..8ac4b0b52 100644
> > > --- a/include/linux/mmc/core.h
> > > +++ b/include/linux/mmc/core.h
> > > @@ -1,7 +1,5 @@
> > >  /* SPDX-License-Identifier: GPL-2.0-only */
> > > -/*
> > > - *  linux/include/linux/mmc/core.h
> > > - */
> >
> > It's okay to remove these lines. However, it should be a separate
> > patch - and please keep it outside of the UHS-II series, as it doesn't
> > belong here.
>
> Okay. I will put them back in the next version.

Yes, please.

> Should I create a patch for this rescovery?

If you want to remove these lines, that's fine by me. Although, then
send a separate patch for it.

[...]

> > > @@ -421,7 +442,7 @@ struct mmc_host {
> > >  #define MMC_CAP2_CRYPTO                0
> > >  #endif
> > >
> > > -       struct sd_uhs2_caps     uhs2_caps;      /* SD UHS-II capabilities */
> > > +       struct sd_uhs2_caps     uhs2_caps;      /* SD UHS-II host capabilities */
> >
> > If you prefer "host capabilities" over plain "capabilities", that's fine by me.
> >
> > However, please make this change as part of the patch that introduced
> > the code, earlier in the series.
>
> I did not change the variable name, I modified the comment just to
> remind me what
> the capability is used for. (host side or device side)
>
> Shall I do anything related to this comment change?
> If I revert to the old comment in the next version, should I create a
> separate patch for it?

I think it would be best to amend my original patch, to fix my mistakes.

[...]

Kind regards
Uffe

Re: [PATCH 5/7] mmc: add UHS-II related definitions in headers

[Lai Jason]

On Thu, Jan 6, 2022 at 4:37 PM Lai Jason
<jasonlai.genesyslogic@gmail.com> wrote:
>
> On Tue, Dec 14, 2021 at 9:37 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Fri, 3 Dec 2021 at 11:51, Jason Lai <jasonlai.genesyslogic@gmail.com> wrote:
> > >
> > > From: Jason Lai <jason.lai@genesyslogic.com.tw>
> > >
> > > All LINK layer messages, registers and SD-TRAN command packet described in
> > > 'Part 1 UHS-II Addendum Ver 1.01' are defined in include/linux/mmc/sd_uhs2.h
> > >
> > > drivers/mmc/core/sd_uhs2.h contains exported function prototype.
> > >
> > > Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
> > > ---
> > >  drivers/mmc/core/sd_uhs2.h  |  18 ++++
> > >  include/linux/mmc/card.h    |  30 +++++-
> > >  include/linux/mmc/core.h    |   4 +-
> > >  include/linux/mmc/host.h    |  27 ++++-
> > >  include/linux/mmc/sd_uhs2.h | 196 ++++++++++++++++++++++++++++++++++++
> > >  5 files changed, 268 insertions(+), 7 deletions(-)
> > >  create mode 100644 drivers/mmc/core/sd_uhs2.h
> > >  create mode 100644 include/linux/mmc/sd_uhs2.h
> > >

[...]

> > > diff --git a/include/linux/mmc/sd_uhs2.h b/include/linux/mmc/sd_uhs2.h
> > > new file mode 100644
> > > index 000000000..5d12fb9d0
> > > --- /dev/null
> > > +++ b/include/linux/mmc/sd_uhs2.h
> > > @@ -0,0 +1,196 @@

[...]

> > > +enum uhs2_act {
> > > +        SET_CONFIG,
> > > +        ENABLE_INT,
> > > +        DISABLE_INT,
> > > +        SET_SPEED_B,
> > > +        CHECK_DORMANT,
> > > +        UHS2_SW_RESET,
> > > +};
> >
> > Perhaps uhs2_action is more clear?

In order to integrate all UHS2 host callback functions into a single function:
uhs2_host_operation(host, uhs2_action). I add 5 actions to
uhs2_action{}:
        enum uhs2_action {
                SET_CONFIG,
                ENABLE_INT,
                DISABLE_INT,
                SET_SPEED_B,
                CHECK_DORMANT,
                SW_RESET,
                SET_REGISTER,    // callback function: uhs2_set_reg(host, act)
                DETECT_INIT,     // callback function: uhs2_detect_init(host)
                DISABLE_CLK,     // callback function: uhs2_disable_clk(host)
                ENABLE_CLK,      // callback function: uhs2_enable_clk(host)
                POST_ATTACH_SD   // callback function: uhs2_post_attach_sd(host)
        };
Do you prefer to add prefix "UHS2_" to each action in uhs2_action?

 kind regards,
 Jason Lai
>

[...]

> >
> > Kind regards
> > Uffe

[PATCH 6/7] mmc: Implement content of UHS-II card initialization functions

[Jason Lai]

From: Jason Lai <jason.lai@genesyslogic.com.tw>

UHS-II card initialization flow is divided into 2 categories: PHY & Card.
Part 1 - PHY Initialization:
         Every host controller may need their own avtivation operation
         to establish LINK between controller and card. So we add a new
         member function(uhs2_detect_init) in struct mmc_host_ops for host
         controller use.
Part 2 - Card Initialization:
         This part can be divided into 6 substeps.
         1. Send UHS-II CCMD DEVICE_INIT to card.
         2. Send UHS-II CCMD ENUMERATE to card.
         3. Send UHS-II Native Read CCMD to obtain capabilities in CFG_REG
            of card.
         4. Host compares capabilities of host controller and  card,
            then write the negotiated values to Setting field in CFG_REG
            of card through UHS-II Native Write CCMD.
         5. Switch host controller's clock to Range B if it is supported
            by both host controller and card.
         6. Execute legacy SD initialization flow.
Part 3 - Provide a function to tranaform legacy SD command packet into
         UHS-II SD-TRAN DCMD packet.

Most of the code added above came from Intel's original patch[1].

[1]
https://patchwork.kernel.org/project/linux-mmc/patch/1419672479-30852-2-
git-send-email-yi.y.sun@intel.com/

Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
---
 drivers/mmc/core/sd_uhs2.c | 831 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 810 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c
index 24aa51a6d..d13283d54 100644
--- a/drivers/mmc/core/sd_uhs2.c
+++ b/drivers/mmc/core/sd_uhs2.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2021 Linaro Ltd
  *
  * Author: Ulf Hansson <ulf.hansson@linaro.org>
+ * Author: Jason Lai <jason.lai@genesyslogic.com.tw>
  *
  * Support for SD UHS-II cards
  */
@@ -10,29 +11,31 @@
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
+#include <linux/mmc/sd_uhs2.h>
 
 #include "core.h"
 #include "bus.h"
 #include "sd.h"
 #include "mmc_ops.h"
+#include "sd_uhs2.h"
 
 static const unsigned int sd_uhs2_freqs[] = { 52000000, 26000000 };
 
-static int sd_uhs2_set_ios(struct mmc_host *host)
+static void sd_uhs2_set_ios(struct mmc_host *host)
 {
 	struct mmc_ios *ios = &host->ios;
 
-	return host->ops->uhs2_set_ios(host, ios);
+	host->ops->set_ios(host, ios);
 }
 
-static int sd_uhs2_power_up(struct mmc_host *host)
+static void sd_uhs2_power_up(struct mmc_host *host)
 {
 	host->ios.vdd = fls(host->ocr_avail) - 1;
 	host->ios.clock = host->f_init;
 	host->ios.timing = MMC_TIMING_SD_UHS2;
 	host->ios.power_mode = MMC_POWER_UP;
 
-	return sd_uhs2_set_ios(host);
+	sd_uhs2_set_ios(host);
 }
 
 static void sd_uhs2_power_off(struct mmc_host *host)
@@ -45,6 +48,50 @@ static void sd_uhs2_power_off(struct mmc_host *host)
 	sd_uhs2_set_ios(host);
 }
 
+/**
+ * sd_uhs2_cmd_assemble() - build up UHS-II command packet which is embeded in
+ *                          mmc_command structure
+ * @cmd:	MMC command to executed
+ * @uhs2_cmd:	UHS2 command corresponded to MMC command
+ * @header:	Header field of UHS-II command cxpacket
+ * @arg:	Argument field of UHS-II command packet
+ * @payload:	Payload field of UHS-II command packet
+ * @plen:	Payload length
+ * @resp:	Response buffer is allocated by caller and it is used to keep
+ *              the response of CM-TRAN command. For SD-TRAN command, uhs2_resp
+ *              should be null and SD-TRAN command response should be stored in
+ *              resp of mmc_command.
+ * @resp_len:	Response buffer length
+ *
+ * Different from legacy SD command, there defined several types of packets
+ * in UHS-II, which include CM-TRAN and SD-TRAN. These packets are then
+ * transformed to differential signals and transmitted/received between
+ * transceiver and receiver.
+ *
+ * The UHS-II packet structure(uhs2_cmd) are added in structure mmc_command
+ * and be filled according to the inputed parameters provided by caller.
+ *
+ * For mmc requests, call this function before issuing command to SD card.
+ * Host controller specific request function will send packet in uhs2_cmd
+ * to UHS-II SD card.
+ *
+ */
+static void sd_uhs2_cmd_assemble(struct mmc_command *cmd,
+			      struct uhs2_command *uhs2_cmd,
+			      u16 header, u16 arg,
+			      u32 *payload, u8 plen, u8 *resp, u8 resp_len)
+{
+	uhs2_cmd->header = header;
+	uhs2_cmd->arg = arg;
+	uhs2_cmd->payload = payload;
+	uhs2_cmd->payload_len = plen * sizeof(u32);
+	uhs2_cmd->packet_len = uhs2_cmd->payload_len + 4;
+
+	cmd->uhs2_cmd = uhs2_cmd;
+	cmd->uhs2_resp = resp;
+	cmd->uhs2_resp_len = resp_len;
+}
+
 /*
  * Run the phy initialization sequence, which mainly relies on the UHS-II host
  * to check that we reach the expected electrical state, between the host and
@@ -52,16 +99,98 @@ static void sd_uhs2_power_off(struct mmc_host *host)
  */
 static int sd_uhs2_phy_init(struct mmc_host *host)
 {
-	return 0;
+	int err = 0;
+
+	/*
+	 * Every host controller can assign its own function which is used
+	 * to initialize PHY.
+	 */
+	err = host->ops->uhs2_detect_init(host)
+	if (err) {
+		pr_err("%s: fail to detect UHS2!\n", mmc_hostname(host));
+	}
+
+	return err;
 }
 
 /*
- * Do the early initialization of the card, by sending the device init
-roadcast
+ * Do the early initialization of the card, by sending the device init broadcast
  * command and wait for the process to be completed.
  */
 static int sd_uhs2_dev_init(struct mmc_host *host)
 {
+	struct mmc_command cmd = {0};
+	struct uhs2_command uhs2_cmd = {};
+	u32 cnt;
+	u32 dap, gap, resp_gap;
+	u16 header = 0, arg = 0;
+	u32 payload[1];
+	u8 plen = 1;
+	u8 gd = 0, cf = 1;
+	u8 resp[6] = {0};
+	u8 resp_len = 6;
+	int err;
+
+	dap = host->uhs2_caps.dap;
+	gap = host->uhs2_caps.gap;
+
+	header = UHS2_NATIVE_PACKET | UHS2_PACKET_TYPE_CCMD;
+	arg = ((UHS2_DEV_CMD_DEVICE_INIT & 0xFF) << 8) |
+	       UHS2_NATIVE_CMD_WRITE |
+	       UHS2_NATIVE_CMD_PLEN_4B |
+	       (UHS2_DEV_CMD_DEVICE_INIT >> 8);
+
+	/*
+	 * Refer to UHS-II Addendum Version 1.02 section 6.3.1.
+	 * Max. time from DEVICE_INIT CCMD EOP reception on Device
+	 * Rx to its SOP transmission on Device Tx(Tfwd_init_cmd) is
+	 * 1 second.
+	 */
+	cmd.busy_timeout = 1000;
+
+	/*
+	 * Refer to UHS-II Addendum Version 1.02 section 6.2.6.3.
+	 * When the number of the DEVICE_INIT commands is reach to
+	 * 30 tiems, Host shall stop issuing DEVICE_INIT command
+	 * and regard it as an error.
+	 */
+	for (cnt = 0; cnt < 30; cnt++) {
+		payload[0] = ((dap & 0xF) << 12) |
+			     (cf << 11)         |
+			     ((gd & 0xF) << 4)  |
+			     (gap & 0xF);
+
+		sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg,
+				  payload, plen, resp, resp_len);
+
+		err = mmc_wait_for_cmd(host, &cmd, 0);
+
+		if (err) {
+			pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+			       mmc_hostname(host), __func__, err);
+			return -EIO;
+		}
+
+		if (resp[3] != (UHS2_DEV_CMD_DEVICE_INIT & 0xFF)) {
+			pr_err("%s: DEVICE_INIT response is wrong!\n",
+			       mmc_hostname(host));
+			return -EIO;
+		}
+
+		if (resp[5] & 0x8) {
+			host->group_desc = gd;
+			break;
+		}
+		resp_gap = resp[4] & 0x0F;
+		if (gap == resp_gap)
+			gd++;
+	}
+	if (cnt == 30) {
+		pr_err("%s: DEVICE_INIT fail, already 30 times!\n",
+		       mmc_hostname(host));
+		return -EIO;
+	}
+
 	return 0;
 }
 
@@ -72,32 +201,519 @@ static int sd_uhs2_dev_init(struct mmc_host *host)
  */
 static int sd_uhs2_enum(struct mmc_host *host, u32 *node_id)
 {
+	struct mmc_command cmd = {0};
+	struct uhs2_command uhs2_cmd = {};
+	u16 header = 0, arg = 0;
+	u32 payload[1];
+	u8 plen = 1;
+	u8 id_f = 0xF, id_l = 0x0;
+	u8 resp[8] = {0};
+	u8 resp_len = 8;
+	int err;
+
+	header = UHS2_NATIVE_PACKET | UHS2_PACKET_TYPE_CCMD;
+	arg = ((UHS2_DEV_CMD_ENUMERATE & 0xFF) << 8) |
+	       UHS2_NATIVE_CMD_WRITE |
+	       UHS2_NATIVE_CMD_PLEN_4B |
+	       (UHS2_DEV_CMD_ENUMERATE >> 8);
+
+	payload[0] = (id_f << 4) | id_l;
+
+	sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
+			  resp, resp_len);
+
+	err = mmc_wait_for_cmd(host, &cmd, 0);
+	if (err) {
+		pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+		       mmc_hostname(host), __func__, err);
+		return -EIO;
+	}
+
+	if (resp[3] != (UHS2_DEV_CMD_ENUMERATE & 0xFF)) {
+		pr_err("%s: ENUMERATE response is wrong!\n",
+		       mmc_hostname(host));
+		return -EIO;
+	}
+
+	id_f = (resp[4] >> 4) & 0xF;
+	id_l = resp[4] & 0xF;
+	*node_id = id_f;
+
 	return 0;
 }
 
 /*
  * Read the UHS-II configuration registers (CFG_REG) of the card, by sending it
  * commands and by parsing the responses. Store a copy of the relevant data in
- * card->uhs2_config.
+ * host->uhs2_card_prop.
  */
 static int sd_uhs2_config_read(struct mmc_host *host, struct mmc_card *card)
 {
+	struct mmc_command cmd = {0};
+	struct uhs2_command uhs2_cmd = {};
+	u16 header = 0, arg = 0;
+	u32 cap;
+	int err;
+
+	header = UHS2_NATIVE_PACKET |
+		 UHS2_PACKET_TYPE_CCMD |
+		 card->uhs2_config.node_id;
+	arg = ((UHS2_DEV_CONFIG_GEN_CAPS & 0xFF) << 8) |
+	       UHS2_NATIVE_CMD_READ |
+	       UHS2_NATIVE_CMD_PLEN_4B |
+	       (UHS2_DEV_CONFIG_GEN_CAPS >> 8);
+
+	/* There is no payload because per spec, there should be
+	 * no payload field for read CCMD.
+	 * Plen is set in arg. Per spec, plen for read CCMD
+	 * represents the len of read data which is assigned in payload
+	 * of following RES (p136).
+	 */
+	sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, NULL, 0, NULL, 0);
+
+	err = mmc_wait_for_cmd(host, &cmd, 0);
+	if (err) {
+		pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+		       mmc_hostname(host), __func__, err);
+		return err;
+	}
+
+	cap = cmd.resp[0];
+	card->uhs2_config.n_lanes =
+				(cap >> UHS2_DEV_CONFIG_N_LANES_POS) &
+				UHS2_DEV_CONFIG_N_LANES_MASK;
+	card->uhs2_config.dadr_len =
+				(cap >> UHS2_DEV_CONFIG_DADR_POS) &
+				UHS2_DEV_CONFIG_DADR_MASK;
+	card->uhs2_config.app_type =
+				(cap >> UHS2_DEV_CONFIG_APP_POS) &
+				UHS2_DEV_CONFIG_APP_MASK;
+
+	arg = ((UHS2_DEV_CONFIG_PHY_CAPS & 0xFF) << 8) |
+	       UHS2_NATIVE_CMD_READ |
+	       UHS2_NATIVE_CMD_PLEN_8B |
+	      (UHS2_DEV_CONFIG_PHY_CAPS >> 8);
+
+	sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, NULL, 0, NULL, 0);
+
+	err = mmc_wait_for_cmd(host, &cmd, 0);
+	if (err) {
+		pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+		       mmc_hostname(host), __func__, err);
+		return -EIO;
+	}
+
+	cap = cmd.resp[0];
+	card->uhs2_config.phy_minor_rev =
+				cap & UHS2_DEV_CONFIG_PHY_MINOR_MASK;
+	card->uhs2_config.phy_major_rev =
+				(cap >> UHS2_DEV_CONFIG_PHY_MAJOR_POS) &
+				 UHS2_DEV_CONFIG_PHY_MAJOR_MASK;
+	card->uhs2_config.can_hibernate =
+				(cap >> UHS2_DEV_CONFIG_CAN_HIBER_POS) &
+				 UHS2_DEV_CONFIG_CAN_HIBER_MASK;
+
+	cap = cmd.resp[1];
+	card->uhs2_config.n_lss_sync =
+				cap & UHS2_DEV_CONFIG_N_LSS_SYN_MASK;
+	card->uhs2_config.n_lss_dir =
+				(cap >> UHS2_DEV_CONFIG_N_LSS_DIR_POS) &
+				UHS2_DEV_CONFIG_N_LSS_DIR_MASK;
+	if (card->uhs2_config.n_lss_sync == 0)
+		card->uhs2_config.n_lss_sync = 16 << 2;
+	else
+		card->uhs2_config.n_lss_sync <<= 2;
+
+	if (card->uhs2_config.n_lss_dir == 0)
+		card->uhs2_config.n_lss_dir = 16 << 3;
+	else
+		card->uhs2_config.n_lss_dir <<= 3;
+
+	arg = ((UHS2_DEV_CONFIG_LINK_TRAN_CAPS & 0xFF) << 8) |
+		UHS2_NATIVE_CMD_READ |
+		UHS2_NATIVE_CMD_PLEN_8B |
+		(UHS2_DEV_CONFIG_LINK_TRAN_CAPS >> 8);
+
+	sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, NULL, 0, NULL, 0);
+
+	err = mmc_wait_for_cmd(host, &cmd, 0);
+	if (err) {
+		pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+		       mmc_hostname(host), __func__, err);
+		return -EIO;
+	}
+
+	cap = cmd.resp[0];
+	card->uhs2_config.link_minor_rev =
+				cap & UHS2_DEV_CONFIG_LT_MINOR_MASK;
+	card->uhs2_config.link_major_rev =
+				(cap >> UHS2_DEV_CONFIG_LT_MAJOR_POS) &
+				UHS2_DEV_CONFIG_LT_MAJOR_MASK;
+	card->uhs2_config.n_fcu =
+				(cap >> UHS2_DEV_CONFIG_N_FCU_POS) &
+				UHS2_DEV_CONFIG_N_FCU_MASK;
+	card->uhs2_config.dev_type =
+				(cap >>UHS2_DEV_CONFIG_DEV_TYPE_POS) &
+				UHS2_DEV_CONFIG_DEV_TYPE_MASK;
+	card->uhs2_config.maxblk_len =
+				(cap >>UHS2_DEV_CONFIG_MAX_BLK_LEN_POS) &
+				UHS2_DEV_CONFIG_MAX_BLK_LEN_MASK;
+
+	cap = cmd.resp[1];
+	card->uhs2_config.n_data_gap =
+				cap & UHS2_DEV_CONFIG_N_DATA_GAP_MASK;
+	if (card->uhs2_config.n_fcu == 0)
+		card->uhs2_config.n_fcu = 256;
+
 	return 0;
 }
 
-/*
+/**
  * Based on the card's and host's UHS-II capabilities, let's update the
  * configuration of the card and the host. This may also include to move to a
  * greater speed range/mode. Depending on the updated configuration, we may
-eed
- * to do a soft reset of the card via sending it a GO_DORMANT_STATE command.
+ * need to do a soft reset of the card via sending it a GO_DORMANT_STATE
+ * command.
  *
  * In the final step, let's check if the card signals "config completion",
-hich
- * indicates that the card has moved from config state into active state.
+ * which indicates that the card has moved from config state into active state.
  */
 static int sd_uhs2_config_write(struct mmc_host *host, struct mmc_card *card)
 {
+	struct mmc_command cmd = {0};
+	struct uhs2_command uhs2_cmd = {};
+	u16 header = 0, arg = 0;
+	u32 payload[2];
+	u8 nMinDataGap;
+	u8 plen;
+	u8 try;
+	int err;
+	u8 resp[5] = {0};
+	u8 resp_len = 5;
+
+	header = UHS2_NATIVE_PACKET |
+		 UHS2_PACKET_TYPE_CCMD | card->uhs2_config.node_id;
+	arg = ((UHS2_DEV_CONFIG_GEN_SET & 0xFF) << 8) |
+	       UHS2_NATIVE_CMD_WRITE |
+	       UHS2_NATIVE_CMD_PLEN_8B |
+	       (UHS2_DEV_CONFIG_GEN_SET >> 8);
+
+	if (card->uhs2_config.n_lanes == UHS2_DEV_CONFIG_2L_HD_FD &&
+	    host->uhs2_caps.n_lanes == UHS2_DEV_CONFIG_2L_HD_FD) {
+		/* Support HD */
+		host->flags |= MMC_UHS2_2L_HD;
+		nMinDataGap = 1;
+	} else {
+		/* Only support 2L-FD so far */
+		host->flags &= ~MMC_UHS2_2L_HD;
+		nMinDataGap = 3;
+	}
+
+	/*
+	 * Most UHS-II cards only support FD and 2L-HD mode. Other lane numbers
+	 * defined in UHS-II addendem Ver1.01 are optional.
+	 */
+	host->uhs2_caps.n_lanes_set = UHS2_DEV_CONFIG_GEN_SET_2L_FD_HD;
+	card->uhs2_config.n_lanes_set = UHS2_DEV_CONFIG_GEN_SET_2L_FD_HD;
+
+	plen = 2;
+	payload[0] = card->uhs2_config.n_lanes_set <<
+		     UHS2_DEV_CONFIG_N_LANES_POS;
+	payload[1] = 0;
+	payload[0] = cpu_to_be32(payload[0]);
+	payload[1] = cpu_to_be32(payload[1]);
+
+	/*
+	 * There is no payload because per spec, there should be
+	 * no payload field for read CCMD.
+	 * Plen is set in arg. Per spec, plen for read CCMD
+	 * represents the len of read data which is assigned in payload
+	 * of following RES (p136).
+	 */
+	sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
+			  NULL, 0);
+
+	err = mmc_wait_for_cmd(host, &cmd, 0);
+	if (err) {
+		pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+		       mmc_hostname(host), __func__, err);
+		return -EIO;
+	}
+
+	arg = ((UHS2_DEV_CONFIG_PHY_SET & 0xFF) << 8) |
+	       UHS2_NATIVE_CMD_WRITE |
+	       UHS2_NATIVE_CMD_PLEN_8B |
+	       (UHS2_DEV_CONFIG_PHY_SET >> 8);
+
+	for (try = 0; try < 2; try++) {
+		plen = 2;
+
+		if (host->uhs2_caps.speed_range ==
+			UHS2_DEV_CONFIG_PHY_SET_SPEED_B) {
+			host->flags |= MMC_UHS2_SPEED_B;
+			card->uhs2_config.speed_range_set =
+					UHS2_DEV_CONFIG_PHY_SET_SPEED_B;
+		} else {
+			card->uhs2_config.speed_range_set =
+					UHS2_DEV_CONFIG_PHY_SET_SPEED_A;
+			host->flags &= ~MMC_UHS2_SPEED_B;
+		}
+
+		payload[0] = card->uhs2_config.speed_range_set <<
+					UHS2_DEV_CONFIG_PHY_SET_SPEED_POS;
+
+		card->uhs2_config.n_lss_sync_set =
+			(min(card->uhs2_config.n_lss_sync,
+			     host->uhs2_caps.n_lss_sync) >> 2) &
+			UHS2_DEV_CONFIG_N_LSS_SYN_MASK;
+		host->uhs2_caps.n_lss_sync_set =
+			card->uhs2_config.n_lss_sync_set;
+
+		if (try == 0) {
+			host->uhs2_caps.n_lss_dir_set =
+				(card->uhs2_config.n_lss_dir >> 3) &
+				UHS2_DEV_CONFIG_N_LSS_DIR_MASK;
+			card->uhs2_config.n_lss_dir_set =
+				((host->uhs2_caps.n_lss_dir >> 3) + 1) &
+				UHS2_DEV_CONFIG_N_LSS_DIR_MASK;
+		} else {
+			card->uhs2_config.n_lss_dir_set =
+				(max(card->uhs2_config.n_lss_dir,
+				     host->uhs2_caps.n_lss_dir) >> 3) &
+				UHS2_DEV_CONFIG_N_LSS_DIR_MASK;
+			host->uhs2_caps.n_lss_dir_set =
+				card->uhs2_config.n_lss_dir_set;
+		}
+
+		payload[1] = (card->uhs2_config.n_lss_dir_set <<
+					UHS2_DEV_CONFIG_N_LSS_DIR_POS) |
+			     card->uhs2_config.n_lss_sync_set;
+		payload[0] = cpu_to_be32(payload[0]);
+		payload[1] = cpu_to_be32(payload[1]);
+
+		resp_len = 4;
+		memset(resp, 0, sizeof(resp));
+
+		sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg,
+				  payload, plen, resp, resp_len);
+
+		err = mmc_wait_for_cmd(host, &cmd, 0);
+		if (err) {
+			pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+			       mmc_hostname(host), __func__, err);
+			return -EIO;
+		}
+
+		if (!(resp[2] & 0x80))
+			break;
+	}
+
+	arg = ((UHS2_DEV_CONFIG_LINK_TRAN_SET & 0xFF) << 8) |
+		UHS2_NATIVE_CMD_WRITE |
+		UHS2_NATIVE_CMD_PLEN_8B |
+		(UHS2_DEV_CONFIG_LINK_TRAN_SET >> 8);
+
+	plen = 2;
+
+	if (card->uhs2_config.app_type == UHS2_DEV_CONFIG_APP_SD_MEM)
+		card->uhs2_config.maxblk_len_set =
+			UHS2_DEV_CONFIG_LT_SET_MAX_BLK_LEN;
+	else
+		card->uhs2_config.maxblk_len_set =
+			min(card->uhs2_config.maxblk_len,
+			    host->uhs2_caps.maxblk_len);
+	host->uhs2_caps.maxblk_len_set =
+			card->uhs2_config.maxblk_len_set;
+
+	card->uhs2_config.n_fcu_set =
+			min(card->uhs2_config.n_fcu,
+			    host->uhs2_caps.n_fcu);
+	host->uhs2_caps.n_fcu_set = card->uhs2_config.n_fcu_set;
+
+	card->uhs2_config.n_data_gap_set =
+			max(nMinDataGap, card->uhs2_config.n_data_gap);
+	host->uhs2_caps.n_data_gap_set =
+			card->uhs2_config.n_data_gap_set;
+
+	host->uhs2_caps.max_retry_set = 3;
+	card->uhs2_config.max_retry_set = host->uhs2_caps.max_retry_set;
+
+	payload[0] = (card->uhs2_config.maxblk_len_set <<
+					UHS2_DEV_CONFIG_MAX_BLK_LEN_POS) |
+		     (card->uhs2_config.max_retry_set <<
+					UHS2_DEV_CONFIG_LT_SET_MAX_RETRY_POS) |
+		     (card->uhs2_config.n_fcu_set <<
+					UHS2_DEV_CONFIG_N_FCU_POS);
+	payload[1] = card->uhs2_config.n_data_gap_set;
+	payload[0] = cpu_to_be32(payload[0]);
+	payload[1] = cpu_to_be32(payload[1]);
+
+	sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
+			  NULL, 0);
+
+	err = mmc_wait_for_cmd(host, &cmd, 0);
+	if (err) {
+		pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+			mmc_hostname(host), __func__, err);
+		return -EIO;
+	}
+
+	arg = ((UHS2_DEV_CONFIG_GEN_SET & 0xFF) << 8) |
+	       UHS2_NATIVE_CMD_WRITE |
+	       UHS2_NATIVE_CMD_PLEN_8B |
+	       (UHS2_DEV_CONFIG_GEN_SET >> 8);
+
+	plen = 2;
+	payload[0] = 0;
+	payload[1] = UHS2_DEV_CONFIG_GEN_SET_CFG_COMPLETE;
+	payload[0] = cpu_to_be32(payload[0]);
+	payload[1] = cpu_to_be32(payload[1]);
+
+	resp_len = 5;
+	memset(resp, 0, sizeof(resp));
+	sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
+			  resp, resp_len);
+
+	err = mmc_wait_for_cmd(host, &cmd, 0);
+	if (err) {
+		pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+		       mmc_hostname(host), __func__, err);
+		return -EIO;
+	}
+
+	/* Set host Config Setting registers */
+	if (!host->ops->uhs2_set_reg ||
+	    host->ops->uhs2_set_reg(host, SET_CONFIG)) {
+		pr_err("%s: %s: UHS2 SET_CONFIG fail!\n",
+			mmc_hostname(host), __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int sd_uhs2_go_dormant(struct mmc_host *host, bool hibernate, u32 node_id)
+{
+	struct mmc_command cmd = {0};
+	struct uhs2_command uhs2_cmd = {};
+	u16 header = 0, arg = 0;
+	u32 payload[1];
+	u8 plen = 1;
+	int err;
+
+	/* Disable Normal INT */
+	if (!host->ops->uhs2_set_reg ||
+	    host->ops->uhs2_set_reg(host, DISABLE_INT)) {
+		pr_err("%s: %s: UHS2 DISABLE_INT fail!\n",
+		       mmc_hostname(host), __func__);
+		return -EIO;
+	}
+
+	header = UHS2_NATIVE_PACKET | UHS2_PACKET_TYPE_CCMD | node_id;
+
+	arg = ((UHS2_DEV_CMD_GO_DORMANT_STATE & 0xFF) << 8) |
+		UHS2_NATIVE_CMD_WRITE |
+		UHS2_NATIVE_CMD_PLEN_4B |
+		(UHS2_DEV_CMD_GO_DORMANT_STATE >> 8);
+
+	if (hibernate)
+		payload[0] = UHS2_DEV_CMD_DORMANT_HIBER;
+
+	sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
+			  NULL, 0);
+
+	err = mmc_wait_for_cmd(host, &cmd, 0);
+	if (err) {
+		pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+		       mmc_hostname(host), __func__, err);
+		return -EIO;
+	}
+
+	/* Check Dormant State in Present */
+	if (!host->ops->uhs2_set_reg ||
+	    host->ops->uhs2_set_reg(host, CHECK_DORMANT)) {
+		pr_err("%s: %s: UHS2 GO_DORMANT_STATE fail!\n",
+		       mmc_hostname(host), __func__);
+		return -EIO;
+	}
+
+	if (host->ops->uhs2_disable_clk)
+		host->ops->uhs2_disable_clk(host);
+
+	return 0;
+}
+
+static int sd_uhs2_change_speed(struct mmc_host *host, u32 node_id)
+{
+	struct mmc_command cmd = {0};
+	struct uhs2_command uhs2_cmd = {};
+	u16 header = 0, arg = 0;
+	int err;
+	int timeout = 100;
+
+	/* Change Speed Range at controller side. */
+	if (!host->ops->uhs2_set_reg ||
+	    host->ops->uhs2_set_reg(host, SET_SPEED_B)) {
+		pr_err("%s: %s: UHS2 SET_SPEED fail!\n",
+		       mmc_hostname(host), __func__);
+		return -EIO;
+	}
+
+	err = sd_uhs2_go_dormant(host, false, node_id);
+	if (err) {
+		pr_err("%s: %s: UHS2 GO_DORMANT_STATE fail, err= 0x%x!\n",
+		       mmc_hostname(host), __func__, err);
+		return -EIO;
+	}
+
+	/* restore sd clock */
+	mmc_delay(5);
+	if (host->ops->uhs2_enable_clk)
+		host->ops->uhs2_enable_clk(host);
+
+	/* Enable Normal INT */
+	if (!host->ops->uhs2_set_reg ||
+	    host->ops->uhs2_set_reg(host, ENABLE_INT)) {
+		pr_err("%s: %s: UHS2 ENABLE_INT fail!\n",
+		       mmc_hostname(host), __func__);
+		return -EIO;
+	}
+	
+	/*
+	 * According to UHS-II Addendum Version 1.01, chapter 6.2.3, wait card
+	 * switch to Active State
+	 */
+	header = UHS2_NATIVE_PACKET | UHS2_PACKET_TYPE_CCMD |
+		 host->card->uhs2_config.node_id;
+	arg = ((UHS2_DEV_CONFIG_GEN_SET & 0xFF) << 8) |
+		UHS2_NATIVE_CMD_READ |
+		UHS2_NATIVE_CMD_PLEN_8B |
+		(UHS2_DEV_CONFIG_GEN_SET >> 8);
+	do {
+		sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd,
+				  header, arg,
+				  NULL, 0,
+				  NULL, 0);
+		err = mmc_wait_for_cmd(host, &cmd, 0);
+		if (err) {
+			pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
+			       mmc_hostname(host), __func__, err);
+			return -EIO;
+		}
+
+		if (cmd.resp[1] & UHS2_DEV_CONFIG_GEN_SET_CFG_COMPLETE)
+			break;
+
+		timeout--;
+		if (timeout == 0) {
+			pr_err("%s: %s: Not switch to Active in 100 ms\n",
+			       mmc_hostname(host->mmc), __func__);
+			return -EIO;
+		}
+
+		mmc_delay(1);
+	} while (1);
+
 	return 0;
 }
 
@@ -108,8 +724,70 @@ static int sd_uhs2_config_write(struct mmc_host *host, struct mmc_card *card)
  * be set through a legacy CMD6. Note that, the power limit that becomes set,
  * survives a soft reset through the GO_DORMANT_STATE command.
  */
-static int sd_uhs2_legacy_init(struct mmc_host *host, struct mmc_card *card)
+static int sd_uhs2_legacy_init(struct mmc_host *host)
 {
+	int err;
+	u32 ocr, rocr;
+
+	WARN_ON(!host->claimed);
+
+	/* Send CMD0 to reset SD card */
+	mmc_go_idle(host);
+
+	/* Send CMD8 to communicate SD interface operation condition */
+	err = mmc_send_if_cond(host, host->ocr_avail);
+	if (err) {
+		pr_err("%s: %s: SEND_IF_COND fail!\n",
+		       mmc_hostname(host), __func__);
+		return err;
+	}
+
+	/*
+	 * Send host capacity support information and activate card's
+	 * initialization process.
+	 */
+	err = mmc_send_app_op_cond(host, 0, &ocr);
+	if (err) {
+		pr_err("%s: %s: SD_SEND_OP_COND fail!\n",
+		       mmc_hostname(host), __func__);
+		return err;
+	}
+
+	if (host->ocr_avail_sd)
+		host->ocr_avail = host->ocr_avail_sd;
+
+	/*
+	 * Some SD cards claims an out of spec VDD voltage range. Let's treat
+	 * these bits as being in-valid and especially also bit7.
+	 */
+	ocr &= ~0x7FFF;
+	rocr = mmc_select_voltage(host, ocr);
+
+	/*
+	 * Some cards have zero value of rocr in UHS-II mode. Assign host's
+	 * ocr value to rocr.
+	 */
+	if (!rocr) {
+		if (host->ocr_avail) {
+			rocr = host->ocr_avail;
+		} else {
+			pr_err("%s: %s: there is no valid OCR.\n",
+			       mmc_hostname(host), __func__);
+			return -EINVAL;
+		}
+	}
+
+	/*
+	 * Get CID, send relative address, get CSD are the same as legacy
+	 * sd, call mmc_sd_init_card() in sd.c directly
+	 */
+	err = mmc_sd_init_card(host, rocr, NULL);
+	if (err) {
+		pr_err("%s: %s: mmc_sd_init_card() fail!\n",
+		       mmc_hostname(host), __func__);
+		return err;
+	}
+
 	return 0;
 }
 
@@ -124,12 +802,16 @@ static int sd_uhs2_init_card(struct mmc_host *host)
 	int err;
 
 	err = sd_uhs2_dev_init(host);
-	if (err)
+	if (err) {
+		pr_err("%s: UHS2 DEVICE_INIT fail!\n", mmc_hostname(host));
 		return err;
+	}
 
 	err = sd_uhs2_enum(host, &node_id);
-	if (err)
+	if (err) {
+		pr_err("%s: UHS2 ENUMERATE fail!\n", mmc_hostname(host));
 		return err;
+	}
 
 	card = mmc_alloc_card(host, &sd_type);
 	if (IS_ERR(card))
@@ -142,15 +824,26 @@ static int sd_uhs2_init_card(struct mmc_host *host)
 	if (err)
 		goto err;
 
+	/* Change to Speed Range B if it is supported */
+	if (host->flags & MMC_UHS2_SPEED_B) {
+		err = sd_uhs2_change_speed(host, node_id);
+		if (err) {
+			pr_err("%s: %s: UHS2 sd_uhs2_change_speed() fail!\n",
+				mmc_hostname(host), __func__);
+			return err;
+		}
+	}
+
 	err = sd_uhs2_config_write(host, card);
 	if (err)
 		goto err;
 
-	err = sd_uhs2_legacy_init(host, card);
+	host->card = card;
+
+	err = sd_uhs2_legacy_init(host);
 	if (err)
 		goto err;
 
-	host->card = card;
 	return 0;
 
 err:
@@ -217,6 +910,78 @@ static int sd_uhs2_hw_reset(struct mmc_host *host)
 	return 0;
 }
 
+/*
+ * sd_uhs2_prepare_cmd - prepare for SD command packet
+ * @host:	MMC host
+ * @mrq:	MMC request
+ *
+ * Initialize and fill in a header and a payload of SD command packet.
+ * The caller should allocate uhs2_command in host->cmd->uhs2_cmd in
+ * advance.
+ *
+ * Return:	0 on success, non-zero error on failure
+ */
+int sd_uhs2_prepare_cmd(struct mmc_host *host, struct mmc_request *mrq)
+{
+	struct mmc_command *cmd;
+	struct uhs2_command *uhs2_cmd;
+	u16 header = 0, arg = 0;
+	u32 *payload;
+	u8 plen = 0;
+
+	cmd = mrq->cmd;
+	header = host->card->uhs2_config.node_id;
+	if ((cmd->flags & MMC_CMD_MASK) == MMC_CMD_ADTC)
+		header |= UHS2_PACKET_TYPE_DCMD;
+	else
+		header |= UHS2_PACKET_TYPE_CCMD;
+
+	arg = cmd->opcode << UHS2_SD_CMD_INDEX_POS;
+	if (host->flags & MMC_UHS2_APP_CMD) {
+		arg |= UHS2_SD_CMD_APP;
+		host->flags &= ~MMC_UHS2_APP_CMD;
+	}
+
+	uhs2_cmd = cmd->uhs2_cmd;
+	payload = uhs2_cmd->payload;
+	plen = 2; /* at the maximum */
+
+	if ((cmd->flags & MMC_CMD_MASK) == MMC_CMD_ADTC &&
+	    !cmd->uhs2_tmode0_flag) {
+		if (host->flags & MMC_UHS2_2L_HD)
+			arg |= UHS2_DCMD_2L_HD_MODE;
+
+		arg |= UHS2_DCMD_LM_TLEN_EXIST;
+
+		if (cmd->data->blocks == 1 &&
+		    cmd->data->blksz != 512 &&
+		    cmd->opcode != MMC_READ_SINGLE_BLOCK &&
+		    cmd->opcode != MMC_WRITE_BLOCK) {
+			arg |= UHS2_DCMD_TLUM_BYTE_MODE;
+			payload[1] = cpu_to_be32(cmd->data->blksz);
+		} else {
+			payload[1] = cpu_to_be32(cmd->data->blocks);
+		}
+
+		if (cmd->opcode == SD_IO_RW_EXTENDED) {
+			arg &= ~(UHS2_DCMD_LM_TLEN_EXIST |
+				UHS2_DCMD_TLUM_BYTE_MODE |
+				UHS2_NATIVE_DCMD_DAM_IO);
+			payload[1] = 0;
+			plen = 1;
+		}
+	} else {
+		plen = 1;
+	}
+
+	payload[0] = cpu_to_be32(cmd->arg);
+
+	sd_uhs2_cmd_assemble(cmd, uhs2_cmd, header, arg, payload, plen, NULL, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sd_uhs2_prepare_cmd);
+
 static const struct mmc_bus_ops sd_uhs2_ops = {
 	.remove = sd_uhs2_remove,
 	.alive = sd_uhs2_alive,
@@ -254,18 +1019,34 @@ static int sd_uhs2_attach(struct mmc_host *host)
 		goto remove_card;
 
 	mmc_claim_host(host);
+
+	if (host->ops->uhs2_post_attach_sd)
+		host->ops->uhs2_post_attach_sd(host);
+
 	return 0;
 
 remove_card:
-	mmc_remove_card(host->card);
-	host->card = NULL;
+	sd_uhs2_remove(host);
 	mmc_claim_host(host);
-	mmc_detach_bus(host);
+
 err:
+	mmc_detach_bus(host);
 	sd_uhs2_power_off(host);
+	if (host->flags & MMC_UHS2_INITIALIZED)
+		host->flags &= ~MMC_UHS2_INITIALIZED;
+	host->flags &= ~MMC_UHS2_SUPPORT;
 	return err;
 }
 
+/**
+ * mmc_attach_sd_uhs2 - select UHS2 interface
+ * @host: MMC host
+ *
+ * Try to select UHS2 interface and initialize the bus for a given
+ * frequency, @freq.
+ *
+ * Return:	0 on success, non-zero error on failure
+ */
 int mmc_attach_sd_uhs2(struct mmc_host *host)
 {
 	int i, err = 0;
@@ -276,6 +1057,8 @@ int mmc_attach_sd_uhs2(struct mmc_host *host)
 	/* Turn off the legacy SD interface before trying with UHS-II. */
 	mmc_power_off(host);
 
+	host->flags |= MMC_UHS2_SUPPORT;
+
 	/*
 	 * Start UHS-II initialization at 52MHz and possibly make a retry at
 	 * 26MHz according to the spec. It's required that the host driver
@@ -283,9 +1066,15 @@ int mmc_attach_sd_uhs2(struct mmc_host *host)
 	 */
 	for (i = 0; i < ARRAY_SIZE(sd_uhs2_freqs); i++) {
 		host->f_init = sd_uhs2_freqs[i];
+		pr_info("%s: %s: trying to init UHS-II card at %u Hz\n",
+			 mmc_hostname(host), __func__, host->f_init);
+
 		err = sd_uhs2_attach(host);
 		if (!err)
 			break;
+		else
+			pr_err("%s: %s: init card at %u Hz failed\n",
+			       mmc_hostname(host), __func__, host->f_init);
 	}
 
 	return err;
-- 
2.34.0

Re: [PATCH 6/7] mmc: Implement content of UHS-II card initialization functions

[Ulf Hansson]

On Fri, 3 Dec 2021 at 11:51, Jason Lai <jasonlai.genesyslogic@gmail.com> wrote:
>
> From: Jason Lai <jason.lai@genesyslogic.com.tw>
>
> UHS-II card initialization flow is divided into 2 categories: PHY & Card.
> Part 1 - PHY Initialization:
>          Every host controller may need their own avtivation operation
>          to establish LINK between controller and card. So we add a new
>          member function(uhs2_detect_init) in struct mmc_host_ops for host
>          controller use.
> Part 2 - Card Initialization:
>          This part can be divided into 6 substeps.
>          1. Send UHS-II CCMD DEVICE_INIT to card.
>          2. Send UHS-II CCMD ENUMERATE to card.
>          3. Send UHS-II Native Read CCMD to obtain capabilities in CFG_REG
>             of card.
>          4. Host compares capabilities of host controller and  card,
>             then write the negotiated values to Setting field in CFG_REG
>             of card through UHS-II Native Write CCMD.
>          5. Switch host controller's clock to Range B if it is supported
>             by both host controller and card.
>          6. Execute legacy SD initialization flow.
> Part 3 - Provide a function to tranaform legacy SD command packet into
>          UHS-II SD-TRAN DCMD packet.
>
> Most of the code added above came from Intel's original patch[1].
>
> [1]
> https://patchwork.kernel.org/project/linux-mmc/patch/1419672479-30852-2-
> git-send-email-yi.y.sun@intel.com/
>
> Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
> ---
>  drivers/mmc/core/sd_uhs2.c | 831 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 810 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c
> index 24aa51a6d..d13283d54 100644
> --- a/drivers/mmc/core/sd_uhs2.c
> +++ b/drivers/mmc/core/sd_uhs2.c
> @@ -3,6 +3,7 @@
>   * Copyright (C) 2021 Linaro Ltd
>   *
>   * Author: Ulf Hansson <ulf.hansson@linaro.org>
> + * Author: Jason Lai <jason.lai@genesyslogic.com.tw>
>   *
>   * Support for SD UHS-II cards
>   */
> @@ -10,29 +11,31 @@
>
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/card.h>
> +#include <linux/mmc/sd_uhs2.h>
>
>  #include "core.h"
>  #include "bus.h"
>  #include "sd.h"
>  #include "mmc_ops.h"
> +#include "sd_uhs2.h"
>
>  static const unsigned int sd_uhs2_freqs[] = { 52000000, 26000000 };
>
> -static int sd_uhs2_set_ios(struct mmc_host *host)
> +static void sd_uhs2_set_ios(struct mmc_host *host)

Please don't change the function to return void. It's better to allow
the callback to return an error code, even if that might not be of
benefit for your particular host variant.

>  {
>         struct mmc_ios *ios = &host->ios;
>
> -       return host->ops->uhs2_set_ios(host, ios);
> +       host->ops->set_ios(host, ios);
>  }
>
> -static int sd_uhs2_power_up(struct mmc_host *host)
> +static void sd_uhs2_power_up(struct mmc_host *host)
>  {

Ditto.

>         host->ios.vdd = fls(host->ocr_avail) - 1;
>         host->ios.clock = host->f_init;
>         host->ios.timing = MMC_TIMING_SD_UHS2;
>         host->ios.power_mode = MMC_POWER_UP;
>
> -       return sd_uhs2_set_ios(host);
> +       sd_uhs2_set_ios(host);
>  }
>
>  static void sd_uhs2_power_off(struct mmc_host *host)
> @@ -45,6 +48,50 @@ static void sd_uhs2_power_off(struct mmc_host *host)
>         sd_uhs2_set_ios(host);
>  }
>
> +/**
> + * sd_uhs2_cmd_assemble() - build up UHS-II command packet which is embeded in
> + *                          mmc_command structure
> + * @cmd:       MMC command to executed
> + * @uhs2_cmd:  UHS2 command corresponded to MMC command
> + * @header:    Header field of UHS-II command cxpacket
> + * @arg:       Argument field of UHS-II command packet
> + * @payload:   Payload field of UHS-II command packet
> + * @plen:      Payload length
> + * @resp:      Response buffer is allocated by caller and it is used to keep
> + *              the response of CM-TRAN command. For SD-TRAN command, uhs2_resp
> + *              should be null and SD-TRAN command response should be stored in
> + *              resp of mmc_command.
> + * @resp_len:  Response buffer length
> + *
> + * Different from legacy SD command, there defined several types of packets
> + * in UHS-II, which include CM-TRAN and SD-TRAN. These packets are then
> + * transformed to differential signals and transmitted/received between
> + * transceiver and receiver.
> + *
> + * The UHS-II packet structure(uhs2_cmd) are added in structure mmc_command
> + * and be filled according to the inputed parameters provided by caller.
> + *
> + * For mmc requests, call this function before issuing command to SD card.
> + * Host controller specific request function will send packet in uhs2_cmd
> + * to UHS-II SD card.
> + *

I think we discussed the above functions description earlier. The
above description is just way too overwhelming. What the function does
is that fills out the uhs2_cmd data structure, that's it.

As this is a static function, I wouldn't mind that you simply drop the
entire function description above. Or try to make the description
short and clear.

> + */
> +static void sd_uhs2_cmd_assemble(struct mmc_command *cmd,
> +                             struct uhs2_command *uhs2_cmd,
> +                             u16 header, u16 arg,
> +                             u32 *payload, u8 plen, u8 *resp, u8 resp_len)
> +{
> +       uhs2_cmd->header = header;
> +       uhs2_cmd->arg = arg;
> +       uhs2_cmd->payload = payload;
> +       uhs2_cmd->payload_len = plen * sizeof(u32);
> +       uhs2_cmd->packet_len = uhs2_cmd->payload_len + 4;
> +
> +       cmd->uhs2_cmd = uhs2_cmd;
> +       cmd->uhs2_resp = resp;
> +       cmd->uhs2_resp_len = resp_len;
> +}
> +
>  /*
>   * Run the phy initialization sequence, which mainly relies on the UHS-II host
>   * to check that we reach the expected electrical state, between the host and
> @@ -52,16 +99,98 @@ static void sd_uhs2_power_off(struct mmc_host *host)
>   */
>  static int sd_uhs2_phy_init(struct mmc_host *host)
>  {
> -       return 0;
> +       int err = 0;
> +
> +       /*
> +        * Every host controller can assign its own function which is used
> +        * to initialize PHY.
> +        */
> +       err = host->ops->uhs2_detect_init(host)

It looks like you didn't declare this callback as part of the struct
mmc_host_ops, so this does not even compile. Please fix that.

I also suggest moving the above comment into the header file along
with its declaration, as it's usually where we keep this information.
Moreover, to make it clear what the callback is intended for, I
suggest renaming it to "uhs2_phy_init".

> +       if (err) {
> +               pr_err("%s: fail to detect UHS2!\n", mmc_hostname(host));

Maybe "failed to initial phy for UHS-II", is better?

> +       }
> +
> +       return err;
>  }
>
>  /*
> - * Do the early initialization of the card, by sending the device init
> -roadcast
> + * Do the early initialization of the card, by sending the device init broadcast

Looks like you are correcting a spelling mistake, which is good.
However, please make this a part of the patch that introduced this,
earlier in the series.

>   * command and wait for the process to be completed.
>   */
>  static int sd_uhs2_dev_init(struct mmc_host *host)
>  {
> +       struct mmc_command cmd = {0};
> +       struct uhs2_command uhs2_cmd = {};
> +       u32 cnt;
> +       u32 dap, gap, resp_gap;
> +       u16 header = 0, arg = 0;
> +       u32 payload[1];
> +       u8 plen = 1;
> +       u8 gd = 0, cf = 1;
> +       u8 resp[6] = {0};
> +       u8 resp_len = 6;
> +       int err;
> +
> +       dap = host->uhs2_caps.dap;
> +       gap = host->uhs2_caps.gap;
> +
> +       header = UHS2_NATIVE_PACKET | UHS2_PACKET_TYPE_CCMD;
> +       arg = ((UHS2_DEV_CMD_DEVICE_INIT & 0xFF) << 8) |
> +              UHS2_NATIVE_CMD_WRITE |
> +              UHS2_NATIVE_CMD_PLEN_4B |
> +              (UHS2_DEV_CMD_DEVICE_INIT >> 8);
> +
> +       /*
> +        * Refer to UHS-II Addendum Version 1.02 section 6.3.1.
> +        * Max. time from DEVICE_INIT CCMD EOP reception on Device
> +        * Rx to its SOP transmission on Device Tx(Tfwd_init_cmd) is
> +        * 1 second.
> +        */
> +       cmd.busy_timeout = 1000;
> +
> +       /*
> +        * Refer to UHS-II Addendum Version 1.02 section 6.2.6.3.
> +        * When the number of the DEVICE_INIT commands is reach to
> +        * 30 tiems, Host shall stop issuing DEVICE_INIT command
> +        * and regard it as an error.
> +        */
> +       for (cnt = 0; cnt < 30; cnt++) {
> +               payload[0] = ((dap & 0xF) << 12) |
> +                            (cf << 11)         |
> +                            ((gd & 0xF) << 4)  |
> +                            (gap & 0xF);
> +
> +               sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg,
> +                                 payload, plen, resp, resp_len);
> +
> +               err = mmc_wait_for_cmd(host, &cmd, 0);
> +
> +               if (err) {
> +                       pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                              mmc_hostname(host), __func__, err);
> +                       return -EIO;
> +               }
> +
> +               if (resp[3] != (UHS2_DEV_CMD_DEVICE_INIT & 0xFF)) {
> +                       pr_err("%s: DEVICE_INIT response is wrong!\n",
> +                              mmc_hostname(host));
> +                       return -EIO;
> +               }
> +
> +               if (resp[5] & 0x8) {
> +                       host->group_desc = gd;
> +                       break;
> +               }
> +               resp_gap = resp[4] & 0x0F;
> +               if (gap == resp_gap)
> +                       gd++;
> +       }
> +       if (cnt == 30) {
> +               pr_err("%s: DEVICE_INIT fail, already 30 times!\n",
> +                      mmc_hostname(host));
> +               return -EIO;
> +       }
> +
>         return 0;
>  }
>
> @@ -72,32 +201,519 @@ static int sd_uhs2_dev_init(struct mmc_host *host)
>   */
>  static int sd_uhs2_enum(struct mmc_host *host, u32 *node_id)
>  {
> +       struct mmc_command cmd = {0};
> +       struct uhs2_command uhs2_cmd = {};
> +       u16 header = 0, arg = 0;
> +       u32 payload[1];
> +       u8 plen = 1;
> +       u8 id_f = 0xF, id_l = 0x0;
> +       u8 resp[8] = {0};
> +       u8 resp_len = 8;
> +       int err;
> +
> +       header = UHS2_NATIVE_PACKET | UHS2_PACKET_TYPE_CCMD;
> +       arg = ((UHS2_DEV_CMD_ENUMERATE & 0xFF) << 8) |
> +              UHS2_NATIVE_CMD_WRITE |
> +              UHS2_NATIVE_CMD_PLEN_4B |
> +              (UHS2_DEV_CMD_ENUMERATE >> 8);
> +
> +       payload[0] = (id_f << 4) | id_l;
> +
> +       sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
> +                         resp, resp_len);
> +
> +       err = mmc_wait_for_cmd(host, &cmd, 0);
> +       if (err) {
> +               pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                      mmc_hostname(host), __func__, err);
> +               return -EIO;
> +       }
> +
> +       if (resp[3] != (UHS2_DEV_CMD_ENUMERATE & 0xFF)) {
> +               pr_err("%s: ENUMERATE response is wrong!\n",
> +                      mmc_hostname(host));
> +               return -EIO;
> +       }
> +
> +       id_f = (resp[4] >> 4) & 0xF;
> +       id_l = resp[4] & 0xF;
> +       *node_id = id_f;
> +
>         return 0;
>  }
>
>  /*
>   * Read the UHS-II configuration registers (CFG_REG) of the card, by sending it
>   * commands and by parsing the responses. Store a copy of the relevant data in
> - * card->uhs2_config.
> + * host->uhs2_card_prop.

This isn't correct. The code below is storing the configuration in
card->uhs2_config, which is the correct thing to do.

>   */
>  static int sd_uhs2_config_read(struct mmc_host *host, struct mmc_card *card)
>  {
> +       struct mmc_command cmd = {0};
> +       struct uhs2_command uhs2_cmd = {};
> +       u16 header = 0, arg = 0;
> +       u32 cap;
> +       int err;
> +
> +       header = UHS2_NATIVE_PACKET |
> +                UHS2_PACKET_TYPE_CCMD |
> +                card->uhs2_config.node_id;
> +       arg = ((UHS2_DEV_CONFIG_GEN_CAPS & 0xFF) << 8) |
> +              UHS2_NATIVE_CMD_READ |
> +              UHS2_NATIVE_CMD_PLEN_4B |
> +              (UHS2_DEV_CONFIG_GEN_CAPS >> 8);
> +
> +       /* There is no payload because per spec, there should be
> +        * no payload field for read CCMD.
> +        * Plen is set in arg. Per spec, plen for read CCMD
> +        * represents the len of read data which is assigned in payload
> +        * of following RES (p136).
> +        */
> +       sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, NULL, 0, NULL, 0);
> +
> +       err = mmc_wait_for_cmd(host, &cmd, 0);
> +       if (err) {
> +               pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                      mmc_hostname(host), __func__, err);
> +               return err;
> +       }
> +
> +       cap = cmd.resp[0];
> +       card->uhs2_config.n_lanes =
> +                               (cap >> UHS2_DEV_CONFIG_N_LANES_POS) &
> +                               UHS2_DEV_CONFIG_N_LANES_MASK;
> +       card->uhs2_config.dadr_len =
> +                               (cap >> UHS2_DEV_CONFIG_DADR_POS) &
> +                               UHS2_DEV_CONFIG_DADR_MASK;
> +       card->uhs2_config.app_type =
> +                               (cap >> UHS2_DEV_CONFIG_APP_POS) &
> +                               UHS2_DEV_CONFIG_APP_MASK;
> +
> +       arg = ((UHS2_DEV_CONFIG_PHY_CAPS & 0xFF) << 8) |
> +              UHS2_NATIVE_CMD_READ |
> +              UHS2_NATIVE_CMD_PLEN_8B |
> +             (UHS2_DEV_CONFIG_PHY_CAPS >> 8);
> +
> +       sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, NULL, 0, NULL, 0);
> +
> +       err = mmc_wait_for_cmd(host, &cmd, 0);
> +       if (err) {
> +               pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                      mmc_hostname(host), __func__, err);
> +               return -EIO;
> +       }
> +
> +       cap = cmd.resp[0];
> +       card->uhs2_config.phy_minor_rev =
> +                               cap & UHS2_DEV_CONFIG_PHY_MINOR_MASK;
> +       card->uhs2_config.phy_major_rev =
> +                               (cap >> UHS2_DEV_CONFIG_PHY_MAJOR_POS) &
> +                                UHS2_DEV_CONFIG_PHY_MAJOR_MASK;
> +       card->uhs2_config.can_hibernate =
> +                               (cap >> UHS2_DEV_CONFIG_CAN_HIBER_POS) &
> +                                UHS2_DEV_CONFIG_CAN_HIBER_MASK;
> +
> +       cap = cmd.resp[1];
> +       card->uhs2_config.n_lss_sync =
> +                               cap & UHS2_DEV_CONFIG_N_LSS_SYN_MASK;
> +       card->uhs2_config.n_lss_dir =
> +                               (cap >> UHS2_DEV_CONFIG_N_LSS_DIR_POS) &
> +                               UHS2_DEV_CONFIG_N_LSS_DIR_MASK;
> +       if (card->uhs2_config.n_lss_sync == 0)
> +               card->uhs2_config.n_lss_sync = 16 << 2;
> +       else
> +               card->uhs2_config.n_lss_sync <<= 2;
> +
> +       if (card->uhs2_config.n_lss_dir == 0)
> +               card->uhs2_config.n_lss_dir = 16 << 3;
> +       else
> +               card->uhs2_config.n_lss_dir <<= 3;
> +
> +       arg = ((UHS2_DEV_CONFIG_LINK_TRAN_CAPS & 0xFF) << 8) |
> +               UHS2_NATIVE_CMD_READ |
> +               UHS2_NATIVE_CMD_PLEN_8B |
> +               (UHS2_DEV_CONFIG_LINK_TRAN_CAPS >> 8);
> +
> +       sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, NULL, 0, NULL, 0);
> +
> +       err = mmc_wait_for_cmd(host, &cmd, 0);
> +       if (err) {
> +               pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                      mmc_hostname(host), __func__, err);
> +               return -EIO;
> +       }
> +
> +       cap = cmd.resp[0];
> +       card->uhs2_config.link_minor_rev =
> +                               cap & UHS2_DEV_CONFIG_LT_MINOR_MASK;
> +       card->uhs2_config.link_major_rev =
> +                               (cap >> UHS2_DEV_CONFIG_LT_MAJOR_POS) &
> +                               UHS2_DEV_CONFIG_LT_MAJOR_MASK;
> +       card->uhs2_config.n_fcu =
> +                               (cap >> UHS2_DEV_CONFIG_N_FCU_POS) &
> +                               UHS2_DEV_CONFIG_N_FCU_MASK;
> +       card->uhs2_config.dev_type =
> +                               (cap >>UHS2_DEV_CONFIG_DEV_TYPE_POS) &
> +                               UHS2_DEV_CONFIG_DEV_TYPE_MASK;
> +       card->uhs2_config.maxblk_len =
> +                               (cap >>UHS2_DEV_CONFIG_MAX_BLK_LEN_POS) &
> +                               UHS2_DEV_CONFIG_MAX_BLK_LEN_MASK;
> +
> +       cap = cmd.resp[1];
> +       card->uhs2_config.n_data_gap =
> +                               cap & UHS2_DEV_CONFIG_N_DATA_GAP_MASK;
> +       if (card->uhs2_config.n_fcu == 0)
> +               card->uhs2_config.n_fcu = 256;
> +
>         return 0;
>  }
>
> -/*
> +/**
>   * Based on the card's and host's UHS-II capabilities, let's update the
>   * configuration of the card and the host. This may also include to move to a
>   * greater speed range/mode. Depending on the updated configuration, we may
> -eed
> - * to do a soft reset of the card via sending it a GO_DORMANT_STATE command.
> + * need to do a soft reset of the card via sending it a GO_DORMANT_STATE
> + * command.
>   *
>   * In the final step, let's check if the card signals "config completion",
> -hich
> - * indicates that the card has moved from config state into active state.
> + * which indicates that the card has moved from config state into active state.

As I said earlier, please fix spelling mistakes as part of the
patch(es) that introduced them.

>   */
>  static int sd_uhs2_config_write(struct mmc_host *host, struct mmc_card *card)
>  {
> +       struct mmc_command cmd = {0};
> +       struct uhs2_command uhs2_cmd = {};
> +       u16 header = 0, arg = 0;
> +       u32 payload[2];
> +       u8 nMinDataGap;
> +       u8 plen;
> +       u8 try;
> +       int err;
> +       u8 resp[5] = {0};
> +       u8 resp_len = 5;
> +
> +       header = UHS2_NATIVE_PACKET |
> +                UHS2_PACKET_TYPE_CCMD | card->uhs2_config.node_id;
> +       arg = ((UHS2_DEV_CONFIG_GEN_SET & 0xFF) << 8) |
> +              UHS2_NATIVE_CMD_WRITE |
> +              UHS2_NATIVE_CMD_PLEN_8B |
> +              (UHS2_DEV_CONFIG_GEN_SET >> 8);
> +
> +       if (card->uhs2_config.n_lanes == UHS2_DEV_CONFIG_2L_HD_FD &&
> +           host->uhs2_caps.n_lanes == UHS2_DEV_CONFIG_2L_HD_FD) {
> +               /* Support HD */
> +               host->flags |= MMC_UHS2_2L_HD;
> +               nMinDataGap = 1;
> +       } else {
> +               /* Only support 2L-FD so far */
> +               host->flags &= ~MMC_UHS2_2L_HD;
> +               nMinDataGap = 3;
> +       }
> +
> +       /*
> +        * Most UHS-II cards only support FD and 2L-HD mode. Other lane numbers
> +        * defined in UHS-II addendem Ver1.01 are optional.
> +        */
> +       host->uhs2_caps.n_lanes_set = UHS2_DEV_CONFIG_GEN_SET_2L_FD_HD;
> +       card->uhs2_config.n_lanes_set = UHS2_DEV_CONFIG_GEN_SET_2L_FD_HD;
> +
> +       plen = 2;
> +       payload[0] = card->uhs2_config.n_lanes_set <<
> +                    UHS2_DEV_CONFIG_N_LANES_POS;
> +       payload[1] = 0;
> +       payload[0] = cpu_to_be32(payload[0]);
> +       payload[1] = cpu_to_be32(payload[1]);
> +
> +       /*
> +        * There is no payload because per spec, there should be
> +        * no payload field for read CCMD.
> +        * Plen is set in arg. Per spec, plen for read CCMD
> +        * represents the len of read data which is assigned in payload
> +        * of following RES (p136).
> +        */
> +       sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
> +                         NULL, 0);
> +
> +       err = mmc_wait_for_cmd(host, &cmd, 0);
> +       if (err) {
> +               pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                      mmc_hostname(host), __func__, err);
> +               return -EIO;
> +       }
> +
> +       arg = ((UHS2_DEV_CONFIG_PHY_SET & 0xFF) << 8) |
> +              UHS2_NATIVE_CMD_WRITE |
> +              UHS2_NATIVE_CMD_PLEN_8B |
> +              (UHS2_DEV_CONFIG_PHY_SET >> 8);
> +
> +       for (try = 0; try < 2; try++) {

This loop looks a bit odd. Perhaps you can explain why we need this
with a comment in the code?

Optionally, we could potentially also try to move some part of it into
a separate helper function and then just call that from here instead?
In this way, we may be able to avoid the loop altogether and thus also
the code should become more readable. What do you think?

> +               plen = 2;
> +
> +               if (host->uhs2_caps.speed_range ==
> +                       UHS2_DEV_CONFIG_PHY_SET_SPEED_B) {
> +                       host->flags |= MMC_UHS2_SPEED_B;
> +                       card->uhs2_config.speed_range_set =
> +                                       UHS2_DEV_CONFIG_PHY_SET_SPEED_B;
> +               } else {
> +                       card->uhs2_config.speed_range_set =
> +                                       UHS2_DEV_CONFIG_PHY_SET_SPEED_A;
> +                       host->flags &= ~MMC_UHS2_SPEED_B;
> +               }
> +
> +               payload[0] = card->uhs2_config.speed_range_set <<
> +                                       UHS2_DEV_CONFIG_PHY_SET_SPEED_POS;
> +
> +               card->uhs2_config.n_lss_sync_set =
> +                       (min(card->uhs2_config.n_lss_sync,
> +                            host->uhs2_caps.n_lss_sync) >> 2) &
> +                       UHS2_DEV_CONFIG_N_LSS_SYN_MASK;
> +               host->uhs2_caps.n_lss_sync_set =
> +                       card->uhs2_config.n_lss_sync_set;
> +
> +               if (try == 0) {
> +                       host->uhs2_caps.n_lss_dir_set =
> +                               (card->uhs2_config.n_lss_dir >> 3) &
> +                               UHS2_DEV_CONFIG_N_LSS_DIR_MASK;
> +                       card->uhs2_config.n_lss_dir_set =
> +                               ((host->uhs2_caps.n_lss_dir >> 3) + 1) &
> +                               UHS2_DEV_CONFIG_N_LSS_DIR_MASK;
> +               } else {
> +                       card->uhs2_config.n_lss_dir_set =
> +                               (max(card->uhs2_config.n_lss_dir,
> +                                    host->uhs2_caps.n_lss_dir) >> 3) &
> +                               UHS2_DEV_CONFIG_N_LSS_DIR_MASK;
> +                       host->uhs2_caps.n_lss_dir_set =
> +                               card->uhs2_config.n_lss_dir_set;
> +               }
> +
> +               payload[1] = (card->uhs2_config.n_lss_dir_set <<
> +                                       UHS2_DEV_CONFIG_N_LSS_DIR_POS) |
> +                            card->uhs2_config.n_lss_sync_set;
> +               payload[0] = cpu_to_be32(payload[0]);
> +               payload[1] = cpu_to_be32(payload[1]);
> +
> +               resp_len = 4;
> +               memset(resp, 0, sizeof(resp));
> +
> +               sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg,
> +                                 payload, plen, resp, resp_len);
> +
> +               err = mmc_wait_for_cmd(host, &cmd, 0);
> +               if (err) {
> +                       pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                              mmc_hostname(host), __func__, err);
> +                       return -EIO;
> +               }
> +
> +               if (!(resp[2] & 0x80))
> +                       break;
> +       }
> +
> +       arg = ((UHS2_DEV_CONFIG_LINK_TRAN_SET & 0xFF) << 8) |
> +               UHS2_NATIVE_CMD_WRITE |
> +               UHS2_NATIVE_CMD_PLEN_8B |
> +               (UHS2_DEV_CONFIG_LINK_TRAN_SET >> 8);
> +
> +       plen = 2;
> +
> +       if (card->uhs2_config.app_type == UHS2_DEV_CONFIG_APP_SD_MEM)
> +               card->uhs2_config.maxblk_len_set =
> +                       UHS2_DEV_CONFIG_LT_SET_MAX_BLK_LEN;
> +       else
> +               card->uhs2_config.maxblk_len_set =
> +                       min(card->uhs2_config.maxblk_len,
> +                           host->uhs2_caps.maxblk_len);
> +       host->uhs2_caps.maxblk_len_set =
> +                       card->uhs2_config.maxblk_len_set;
> +
> +       card->uhs2_config.n_fcu_set =
> +                       min(card->uhs2_config.n_fcu,
> +                           host->uhs2_caps.n_fcu);
> +       host->uhs2_caps.n_fcu_set = card->uhs2_config.n_fcu_set;
> +
> +       card->uhs2_config.n_data_gap_set =
> +                       max(nMinDataGap, card->uhs2_config.n_data_gap);
> +       host->uhs2_caps.n_data_gap_set =
> +                       card->uhs2_config.n_data_gap_set;
> +
> +       host->uhs2_caps.max_retry_set = 3;
> +       card->uhs2_config.max_retry_set = host->uhs2_caps.max_retry_set;
> +
> +       payload[0] = (card->uhs2_config.maxblk_len_set <<
> +                                       UHS2_DEV_CONFIG_MAX_BLK_LEN_POS) |
> +                    (card->uhs2_config.max_retry_set <<
> +                                       UHS2_DEV_CONFIG_LT_SET_MAX_RETRY_POS) |
> +                    (card->uhs2_config.n_fcu_set <<
> +                                       UHS2_DEV_CONFIG_N_FCU_POS);
> +       payload[1] = card->uhs2_config.n_data_gap_set;
> +       payload[0] = cpu_to_be32(payload[0]);
> +       payload[1] = cpu_to_be32(payload[1]);
> +
> +       sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
> +                         NULL, 0);
> +
> +       err = mmc_wait_for_cmd(host, &cmd, 0);
> +       if (err) {
> +               pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                       mmc_hostname(host), __func__, err);
> +               return -EIO;
> +       }
> +
> +       arg = ((UHS2_DEV_CONFIG_GEN_SET & 0xFF) << 8) |
> +              UHS2_NATIVE_CMD_WRITE |
> +              UHS2_NATIVE_CMD_PLEN_8B |
> +              (UHS2_DEV_CONFIG_GEN_SET >> 8);
> +
> +       plen = 2;
> +       payload[0] = 0;
> +       payload[1] = UHS2_DEV_CONFIG_GEN_SET_CFG_COMPLETE;
> +       payload[0] = cpu_to_be32(payload[0]);
> +       payload[1] = cpu_to_be32(payload[1]);
> +
> +       resp_len = 5;
> +       memset(resp, 0, sizeof(resp));
> +       sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
> +                         resp, resp_len);
> +
> +       err = mmc_wait_for_cmd(host, &cmd, 0);
> +       if (err) {
> +               pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                      mmc_hostname(host), __func__, err);
> +               return -EIO;
> +       }
> +
> +       /* Set host Config Setting registers */
> +       if (!host->ops->uhs2_set_reg ||

Again, I couldn't find this callback being declared, so I guess this
doesn't compile. Please fix that.

I agree that the callback should not be optional. However, I think
this is far too late in the initialization process to check it. This
can be checked already when adding the host in mmc_add_host(), in case
the host supports UHS-II, for example.

Moreover, I am questioning the name of the callback. It seems like we
are going to use it for various UHS-II operations. The name
"uhs2_set_reg" doesn't really explain this. Maybe "uhs2_do_action", or
if you can come up with something even better, that would be great.

> +           host->ops->uhs2_set_reg(host, SET_CONFIG)) {
> +               pr_err("%s: %s: UHS2 SET_CONFIG fail!\n",
> +                       mmc_hostname(host), __func__);
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
> +static int sd_uhs2_go_dormant(struct mmc_host *host, bool hibernate, u32 node_id)
> +{
> +       struct mmc_command cmd = {0};
> +       struct uhs2_command uhs2_cmd = {};
> +       u16 header = 0, arg = 0;
> +       u32 payload[1];
> +       u8 plen = 1;
> +       int err;
> +
> +       /* Disable Normal INT */
> +       if (!host->ops->uhs2_set_reg ||
> +           host->ops->uhs2_set_reg(host, DISABLE_INT)) {
> +               pr_err("%s: %s: UHS2 DISABLE_INT fail!\n",
> +                      mmc_hostname(host), __func__);
> +               return -EIO;
> +       }
> +
> +       header = UHS2_NATIVE_PACKET | UHS2_PACKET_TYPE_CCMD | node_id;
> +
> +       arg = ((UHS2_DEV_CMD_GO_DORMANT_STATE & 0xFF) << 8) |
> +               UHS2_NATIVE_CMD_WRITE |
> +               UHS2_NATIVE_CMD_PLEN_4B |
> +               (UHS2_DEV_CMD_GO_DORMANT_STATE >> 8);
> +
> +       if (hibernate)
> +               payload[0] = UHS2_DEV_CMD_DORMANT_HIBER;
> +
> +       sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
> +                         NULL, 0);
> +
> +       err = mmc_wait_for_cmd(host, &cmd, 0);
> +       if (err) {
> +               pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                      mmc_hostname(host), __func__, err);
> +               return -EIO;
> +       }
> +
> +       /* Check Dormant State in Present */
> +       if (!host->ops->uhs2_set_reg ||
> +           host->ops->uhs2_set_reg(host, CHECK_DORMANT)) {
> +               pr_err("%s: %s: UHS2 GO_DORMANT_STATE fail!\n",
> +                      mmc_hostname(host), __func__);
> +               return -EIO;
> +       }
> +
> +       if (host->ops->uhs2_disable_clk)

Again, another callback that lacks declaration.

> +               host->ops->uhs2_disable_clk(host);

In fact, rather than adding another callback, we could make use of the
"->uhs2_set_reg" callback?

Or, another option would be to use the ->uhs2_set_ios(). That would be
similar to what we do for legacy SD cards with the ->set_ios()
callback.

For example, we can do like this:
host->ios.power_mode = MMC_POWER_ON;
host->ios.clock = 0;
host->ops->uhs2_set_ios(host, &host->ios);

What do you think? Maybe we need a separate host->ios_uhs2 for this too?

> +
> +       return 0;
> +}
> +
> +static int sd_uhs2_change_speed(struct mmc_host *host, u32 node_id)
> +{
> +       struct mmc_command cmd = {0};
> +       struct uhs2_command uhs2_cmd = {};
> +       u16 header = 0, arg = 0;
> +       int err;
> +       int timeout = 100;
> +
> +       /* Change Speed Range at controller side. */
> +       if (!host->ops->uhs2_set_reg ||
> +           host->ops->uhs2_set_reg(host, SET_SPEED_B)) {
> +               pr_err("%s: %s: UHS2 SET_SPEED fail!\n",
> +                      mmc_hostname(host), __func__);
> +               return -EIO;
> +       }
> +
> +       err = sd_uhs2_go_dormant(host, false, node_id);
> +       if (err) {
> +               pr_err("%s: %s: UHS2 GO_DORMANT_STATE fail, err= 0x%x!\n",
> +                      mmc_hostname(host), __func__, err);
> +               return -EIO;
> +       }
> +
> +       /* restore sd clock */
> +       mmc_delay(5);
> +       if (host->ops->uhs2_enable_clk)
> +               host->ops->uhs2_enable_clk(host);

Ditto.

> +
> +       /* Enable Normal INT */
> +       if (!host->ops->uhs2_set_reg ||
> +           host->ops->uhs2_set_reg(host, ENABLE_INT)) {
> +               pr_err("%s: %s: UHS2 ENABLE_INT fail!\n",
> +                      mmc_hostname(host), __func__);
> +               return -EIO;
> +       }
> +
> +       /*
> +        * According to UHS-II Addendum Version 1.01, chapter 6.2.3, wait card
> +        * switch to Active State
> +        */
> +       header = UHS2_NATIVE_PACKET | UHS2_PACKET_TYPE_CCMD |
> +                host->card->uhs2_config.node_id;

Didn't you pass node_id as an in-parameter? Why can't you use that?

> +       arg = ((UHS2_DEV_CONFIG_GEN_SET & 0xFF) << 8) |
> +               UHS2_NATIVE_CMD_READ |
> +               UHS2_NATIVE_CMD_PLEN_8B |
> +               (UHS2_DEV_CONFIG_GEN_SET >> 8);
> +       do {
> +               sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd,
> +                                 header, arg,
> +                                 NULL, 0,
> +                                 NULL, 0);
> +               err = mmc_wait_for_cmd(host, &cmd, 0);
> +               if (err) {
> +                       pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> +                              mmc_hostname(host), __func__, err);
> +                       return -EIO;
> +               }
> +
> +               if (cmd.resp[1] & UHS2_DEV_CONFIG_GEN_SET_CFG_COMPLETE)
> +                       break;
> +
> +               timeout--;
> +               if (timeout == 0) {
> +                       pr_err("%s: %s: Not switch to Active in 100 ms\n",
> +                              mmc_hostname(host->mmc), __func__);
> +                       return -EIO;
> +               }
> +
> +               mmc_delay(1);
> +       } while (1);
> +
>         return 0;
>  }
>
> @@ -108,8 +724,70 @@ static int sd_uhs2_config_write(struct mmc_host *host, struct mmc_card *card)
>   * be set through a legacy CMD6. Note that, the power limit that becomes set,
>   * survives a soft reset through the GO_DORMANT_STATE command.
>   */
> -static int sd_uhs2_legacy_init(struct mmc_host *host, struct mmc_card *card)
> +static int sd_uhs2_legacy_init(struct mmc_host *host)

Please make this change from the patch that introduced this code.

>  {
> +       int err;
> +       u32 ocr, rocr;
> +
> +       WARN_ON(!host->claimed);
> +
> +       /* Send CMD0 to reset SD card */
> +       mmc_go_idle(host);
> +
> +       /* Send CMD8 to communicate SD interface operation condition */
> +       err = mmc_send_if_cond(host, host->ocr_avail);
> +       if (err) {
> +               pr_err("%s: %s: SEND_IF_COND fail!\n",
> +                      mmc_hostname(host), __func__);
> +               return err;
> +       }
> +
> +       /*
> +        * Send host capacity support information and activate card's
> +        * initialization process.
> +        */
> +       err = mmc_send_app_op_cond(host, 0, &ocr);
> +       if (err) {
> +               pr_err("%s: %s: SD_SEND_OP_COND fail!\n",
> +                      mmc_hostname(host), __func__);
> +               return err;
> +       }
> +
> +       if (host->ocr_avail_sd)
> +               host->ocr_avail = host->ocr_avail_sd;

Please drop this part. I don't want to encourage host's to use
ocr_avail_sd (for reasons that we can discuss another time).

If we need something specific for UHS-II, then it's better to add that.

> +
> +       /*
> +        * Some SD cards claims an out of spec VDD voltage range. Let's treat
> +        * these bits as being in-valid and especially also bit7.
> +        */
> +       ocr &= ~0x7FFF;
> +       rocr = mmc_select_voltage(host, ocr);
> +
> +       /*
> +        * Some cards have zero value of rocr in UHS-II mode. Assign host's
> +        * ocr value to rocr.
> +        */
> +       if (!rocr) {
> +               if (host->ocr_avail) {
> +                       rocr = host->ocr_avail;
> +               } else {
> +                       pr_err("%s: %s: there is no valid OCR.\n",
> +                              mmc_hostname(host), __func__);
> +                       return -EINVAL;
> +               }
> +       }
> +
> +       /*
> +        * Get CID, send relative address, get CSD are the same as legacy
> +        * sd, call mmc_sd_init_card() in sd.c directly
> +        */
> +       err = mmc_sd_init_card(host, rocr, NULL);
> +       if (err) {
> +               pr_err("%s: %s: mmc_sd_init_card() fail!\n",
> +                      mmc_hostname(host), __func__);
> +               return err;
> +       }

So this means we will start using the host callbacks for the legacy SD
interface, like ->set_ios() for example.

In other words, data in host->ios.* seems to be used, both for UHS-II
and legacy SD. That can be messy for host drivers to manage, I think.
Maybe we should add a separate host->ios_uhs2.*, that we use solely
for UHS2 instead?

There is also another problem here. mmc_sd_init_card() will allocate a
new struct mmc_card and assign it to host->card, if it succeeds with
initialization.

By looking at the changes below, it looks like you will assign
host->card, with the struct mmc_card that we have allocated for UHS-II
in sd_uhs2_init_card(), prior to calling sd_uhs2_legacy_init(). Not
only will this lead to a memory leak, but more importantly we will
lose all the information that we have parsed/stored from the card for
UHS-II, right?

Don't we need to keep the UHS-II information of the card around? Or is
it okay to throw it away after initialization?

What am I missing?

> +
>         return 0;
>  }
>
> @@ -124,12 +802,16 @@ static int sd_uhs2_init_card(struct mmc_host *host)
>         int err;
>
>         err = sd_uhs2_dev_init(host);
> -       if (err)
> +       if (err) {
> +               pr_err("%s: UHS2 DEVICE_INIT fail!\n", mmc_hostname(host));
>                 return err;
> +       }
>
>         err = sd_uhs2_enum(host, &node_id);
> -       if (err)
> +       if (err) {
> +               pr_err("%s: UHS2 ENUMERATE fail!\n", mmc_hostname(host));
>                 return err;
> +       }
>
>         card = mmc_alloc_card(host, &sd_type);
>         if (IS_ERR(card))
> @@ -142,15 +824,26 @@ static int sd_uhs2_init_card(struct mmc_host *host)
>         if (err)
>                 goto err;
>
> +       /* Change to Speed Range B if it is supported */
> +       if (host->flags & MMC_UHS2_SPEED_B) {

I noticed that you have added the "host->flags", to keep track of
UHS-II states and some UHS-II capabilities of the host. I would rather
see that UHS-II host capabilities should be part of the
host->uhs2_caps.*.

When it comes to keeping track of UHS-II states, like when we want to
move to a new speed mode for example, that may be better to keep as a
part of a new host->ios_uhs2.*, as I kind of indicated above.

> +               err = sd_uhs2_change_speed(host, node_id);
> +               if (err) {
> +                       pr_err("%s: %s: UHS2 sd_uhs2_change_speed() fail!\n",
> +                               mmc_hostname(host), __func__);
> +                       return err;
> +               }
> +       }
> +
>         err = sd_uhs2_config_write(host, card);
>         if (err)
>                 goto err;
>
> -       err = sd_uhs2_legacy_init(host, card);
> +       host->card = card;
> +
> +       err = sd_uhs2_legacy_init(host);
>         if (err)
>                 goto err;
>
> -       host->card = card;

This is the part that will lead to the memory leak of a struct mmc_card.

>         return 0;
>
>  err:
> @@ -217,6 +910,78 @@ static int sd_uhs2_hw_reset(struct mmc_host *host)
>         return 0;
>  }

[...]

I have stopped the review at this point, let's see if we can conclude
on the way forward around my comments on the parts above, to start
with.

Kind regards
Uffe

Re: [PATCH 6/7] mmc: Implement content of UHS-II card initialization functions

[Lai Jason]

On Wed, Dec 15, 2021 at 4:29 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
>> On Fri, 3 Dec 2021 at 11:51, Jason Lai <jasonlai.genesyslogic@gmail.com> wrote:
> >
> > From: Jason Lai <jason.lai@genesyslogic.com.tw>
> >
> > UHS-II card initialization flow is divided into 2 categories: PHY & Card.
> > Part 1 - PHY Initialization:
> >          Every host controller may need their own avtivation operation
> >          to establish LINK between controller and card. So we add a new
> >          member function(uhs2_detect_init) in struct mmc_host_ops for host
> >          controller use.
> > Part 2 - Card Initialization:
> >          This part can be divided into 6 substeps.
> >          1. Send UHS-II CCMD DEVICE_INIT to card.
> >          2. Send UHS-II CCMD ENUMERATE to card.
> >          3. Send UHS-II Native Read CCMD to obtain capabilities in CFG_REG
> >             of card.
> >          4. Host compares capabilities of host controller and  card,
> >             then write the negotiated values to Setting field in CFG_REG
> >             of card through UHS-II Native Write CCMD.
> >          5. Switch host controller's clock to Range B if it is supported
> >             by both host controller and card.
> >          6. Execute legacy SD initialization flow.
> > Part 3 - Provide a function to tranaform legacy SD command packet into
> >          UHS-II SD-TRAN DCMD packet.
> >
> > Most of the code added above came from Intel's original patch[1].
> >
> > [1]
> > https://patchwork.kernel.org/project/linux-mmc/patch/1419672479-30852-2-
> > git-send-email-yi.y.sun@intel.com/
> >
> > Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
> > ---
> >  drivers/mmc/core/sd_uhs2.c | 831 ++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 810 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c
> > index 24aa51a6d..d13283d54 100644
> > --- a/drivers/mmc/core/sd_uhs2.c
> > +++ b/drivers/mmc/core/sd_uhs2.c
> > @@ -3,6 +3,7 @@
> >
> >  static const unsigned int sd_uhs2_freqs[] = { 52000000, 26000000 };
> >
> > -static int sd_uhs2_set_ios(struct mmc_host *host)
> > +static void sd_uhs2_set_ios(struct mmc_host *host)
>
> Please don't change the function to return void. It's better to allow
> the callback to return an error code, even if that might not be of
> benefit for your particular host variant.

I have a question about the return type of this function. In
sd_uhs2_set_ios(), we only called
host->ops->set_ios() and the return type of that function was VOID. So
is there a reason
that this function must has a non-void return type?

> >  {
> >         struct mmc_ios *ios = &host->ios;
> >
> > -       return host->ops->uhs2_set_ios(host, ios);
> > +       host->ops->set_ios(host, ios);
> >  }
> >
> > -static int sd_uhs2_power_up(struct mmc_host *host)
> > +static void sd_uhs2_power_up(struct mmc_host *host)
> >  {
>
> Ditto.
>
> >         host->ios.vdd = fls(host->ocr_avail) - 1;
> >         host->ios.clock = host->f_init;
> >         host->ios.timing = MMC_TIMING_SD_UHS2;
> >         host->ios.power_mode = MMC_POWER_UP;
> >
> > -       return sd_uhs2_set_ios(host);
> > +       sd_uhs2_set_ios(host);
> >  }
> >
> >  static void sd_uhs2_power_off(struct mmc_host *host)
> > @@ -45,6 +48,50 @@ static void sd_uhs2_power_off(struct mmc_host *host)
> >         sd_uhs2_set_ios(host);
> >  }
> >
> > +/**
> > + * sd_uhs2_cmd_assemble() - build up UHS-II command packet which is embeded in
> > + *                          mmc_command structure
> > + * @cmd:       MMC command to executed
> > + * @uhs2_cmd:  UHS2 command corresponded to MMC command
> > + * @header:    Header field of UHS-II command cxpacket
> > + * @arg:       Argument field of UHS-II command packet
> > + * @payload:   Payload field of UHS-II command packet
> > + * @plen:      Payload length
> > + * @resp:      Response buffer is allocated by caller and it is used to keep
> > + *              the response of CM-TRAN command. For SD-TRAN command, uhs2_resp
> > + *              should be null and SD-TRAN command response should be stored in
> > + *              resp of mmc_command.
> > + * @resp_len:  Response buffer length
> > + *
> > + * Different from legacy SD command, there defined several types of packets
> > + * in UHS-II, which include CM-TRAN and SD-TRAN. These packets are then
> > + * transformed to differential signals and transmitted/received between
> > + * transceiver and receiver.
> > + *
> > + * The UHS-II packet structure(uhs2_cmd) are added in structure mmc_command
> > + * and be filled according to the inputed parameters provided by caller.
> > + *
> > + * For mmc requests, call this function before issuing command to SD card.
> > + * Host controller specific request function will send packet in uhs2_cmd
> > + * to UHS-II SD card.
> > + *
>
> I think we discussed the above functions description earlier. The
> above description is just way too overwhelming. What the function does
> is that fills out the uhs2_cmd data structure, that's it.
>
> As this is a static function, I wouldn't mind that you simply drop the
> entire function description above. Or try to make the description
> short and clear.

I will try to do that.

> > + */
> > +static void sd_uhs2_cmd_assemble(struct mmc_command *cmd,
> > +                             struct uhs2_command *uhs2_cmd,
> > +                             u16 header, u16 arg,
> > +                             u32 *payload, u8 plen, u8 *resp, u8 resp_len)
> > +{
> > +       uhs2_cmd->header = header;
> > +       uhs2_cmd->arg = arg;
> > +       uhs2_cmd->payload = payload;
> > +       uhs2_cmd->payload_len = plen * sizeof(u32);
> > +       uhs2_cmd->packet_len = uhs2_cmd->payload_len + 4;
> > +
> > +       cmd->uhs2_cmd = uhs2_cmd;
> > +       cmd->uhs2_resp = resp;
> > +       cmd->uhs2_resp_len = resp_len;
> > +}
> > +
> >  /*
> >   * Run the phy initialization sequence, which mainly relies on the UHS-II host
> >   * to check that we reach the expected electrical state, between the host and
> > @@ -52,16 +99,98 @@ static void sd_uhs2_power_off(struct mmc_host *host)
> >   */
> >  static int sd_uhs2_phy_init(struct mmc_host *host)
> >  {
> > -       return 0;
> > +       int err = 0;
> > +
> > +       /*
> > +        * Every host controller can assign its own function which is used
> > +        * to initialize PHY.
> > +        */
> > +       err = host->ops->uhs2_detect_init(host)
>
> It looks like you didn't declare this callback as part of the struct
> mmc_host_ops, so this does not even compile. Please fix that.
>
> I also suggest moving the above comment into the header file along
> with its declaration, as it's usually where we keep this information.
>
> Moreover, to make it clear what the callback is intended for, I
> suggest renaming it to "uhs2_phy_init".

There are 5 callback functions for UHS-II:
    int   uhs2_detect_init(struct mmc_host *host);
    int   uhs2_set_reg(struct mmc_host *host, enum uhs2_action act);
    void uhs2_disable_clk(struct mmc_host *host);
    void uhs2_enable_clk(struct mmc_host *host);
    void uhs2_post_attach_sd(struct mmc_host *host);
According to your suggestion, I plan to integrate them into a single function,
named "uhs2_do_action", since they all realize the specified function by
setting the controller.

> > +       if (err) {
> > +               pr_err("%s: fail to detect UHS2!\n", mmc_hostname(host));
>
> Maybe "failed to initial phy for UHS-II", is better?

Okay. The string will be modified in the next version.

> > +       }
> > +
> > +       return err;
> >  }
> >
> >  /*
> > - * Do the early initialization of the card, by sending the device init
> > -roadcast
> > + * Do the early initialization of the card, by sending the device init broadcast
>
> Looks like you are correcting a spelling mistake, which is good.
> However, please make this a part of the patch that introduced this,
> earlier in the series.

Do you mean to create a separate patch which only contains typo correction?
If yes, should I collect such changes in previously submitted files
and put them to
a separate patch file  in the next version, even though they are the
same between
previous and next version? Or just apply this rule to later versions?

> >  /*
> >   * Read the UHS-II configuration registers (CFG_REG) of the card, by sending it
> >   * commands and by parsing the responses. Store a copy of the relevant data in
> > - * card->uhs2_config.
> > + * host->uhs2_card_prop.
>
> This isn't correct. The code below is storing the configuration in
> card->uhs2_config, which is the correct thing to do.

Okay. I will rephrase the description in the next version.

> >
> > -/*
> > +/**
> >   * Based on the card's and host's UHS-II capabilities, let's update the
> >   * configuration of the card and the host. This may also include to move to a
> >   * greater speed range/mode. Depending on the updated configuration, we may
> > -eed
> > - * to do a soft reset of the card via sending it a GO_DORMANT_STATE command.
> > + * need to do a soft reset of the card via sending it a GO_DORMANT_STATE
> > + * command.
> >   *
> >   * In the final step, let's check if the card signals "config completion",
> > -hich
> > - * indicates that the card has moved from config state into active state.
> > + * which indicates that the card has moved from config state into active state.
>
> As I said earlier, please fix spelling mistakes as part of the
> patch(es) that introduced them.
>
> >   */
> >  static int sd_uhs2_config_write(struct mmc_host *host, struct mmc_card *card)
> >  {
> > +       struct mmc_command cmd = {0};
> > +       struct uhs2_command uhs2_cmd = {};
> > +       u16 header = 0, arg = 0;
> > +       u32 payload[2];
> > +       u8 nMinDataGap;
> > +       u8 plen;
> > +       u8 try;
> > +       int err;
> > +       u8 resp[5] = {0};
> > +       u8 resp_len = 5;
> > +
> > +       header = UHS2_NATIVE_PACKET |
> > +                UHS2_PACKET_TYPE_CCMD | card->uhs2_config.node_id;
> > +       arg = ((UHS2_DEV_CONFIG_GEN_SET & 0xFF) << 8) |
> > +              UHS2_NATIVE_CMD_WRITE |
> > +              UHS2_NATIVE_CMD_PLEN_8B |
> > +              (UHS2_DEV_CONFIG_GEN_SET >> 8);
> > +
> > +       if (card->uhs2_config.n_lanes == UHS2_DEV_CONFIG_2L_HD_FD &&
> > +           host->uhs2_caps.n_lanes == UHS2_DEV_CONFIG_2L_HD_FD) {
> > +               /* Support HD */
> > +               host->flags |= MMC_UHS2_2L_HD;
> > +               nMinDataGap = 1;
> > +       } else {
> > +               /* Only support 2L-FD so far */
> > +               host->flags &= ~MMC_UHS2_2L_HD;
> > +               nMinDataGap = 3;
> > +       }
> > +
> > +       /*
> > +        * Most UHS-II cards only support FD and 2L-HD mode. Other lane numbers
> > +        * defined in UHS-II addendem Ver1.01 are optional.
> > +        */
> > +       host->uhs2_caps.n_lanes_set = UHS2_DEV_CONFIG_GEN_SET_2L_FD_HD;
> > +       card->uhs2_config.n_lanes_set = UHS2_DEV_CONFIG_GEN_SET_2L_FD_HD;
> > +
> > +       plen = 2;
> > +       payload[0] = card->uhs2_config.n_lanes_set <<
> > +                    UHS2_DEV_CONFIG_N_LANES_POS;
> > +       payload[1] = 0;
> > +       payload[0] = cpu_to_be32(payload[0]);
> > +       payload[1] = cpu_to_be32(payload[1]);
> > +
> > +       /*
> > +        * There is no payload because per spec, there should be
> > +        * no payload field for read CCMD.
> > +        * Plen is set in arg. Per spec, plen for read CCMD
> > +        * represents the len of read data which is assigned in payload
> > +        * of following RES (p136).
> > +        */
> > +       sd_uhs2_cmd_assemble(&cmd, &uhs2_cmd, header, arg, payload, plen,
> > +                         NULL, 0);
> > +
> > +       err = mmc_wait_for_cmd(host, &cmd, 0);
> > +       if (err) {
> > +               pr_err("%s: %s: UHS2 CMD send fail, err= 0x%x!\n",
> > +                      mmc_hostname(host), __func__, err);
> > +               return -EIO;
> > +       }
> > +
> > +       arg = ((UHS2_DEV_CONFIG_PHY_SET & 0xFF) << 8) |
> > +              UHS2_NATIVE_CMD_WRITE |
> > +              UHS2_NATIVE_CMD_PLEN_8B |
> > +              (UHS2_DEV_CONFIG_PHY_SET >> 8);
> > +
> > +       for (try = 0; try < 2; try++) {
>
> This loop looks a bit odd. Perhaps you can explain why we need this
> with a comment in the code?
>
> Optionally, we could potentially also try to move some part of it into
> a separate helper function and then just call that from here instead?
> In this way, we may be able to avoid the loop altogether and thus also
> the code should become more readable. What do you think?

Agree. I will try to do it in the next version.

> > +               if (!(resp[2] & 0x80))
> > +                       break;
> > +       }
> > +
> > +       /* Set host Config Setting registers */
> > +       if (!host->ops->uhs2_set_reg ||
>
> Again, I couldn't find this callback being declared, so I guess this
> doesn't compile. Please fix that.
>
> I agree that the callback should not be optional. However, I think
> this is far too late in the initialization process to check it. This
> can be checked already when adding the host in mmc_add_host(), in case
> the host supports UHS-II, for example.
>
> Moreover, I am questioning the name of the callback. It seems like we
> are going to use it for various UHS-II operations. The name
> "uhs2_set_reg" doesn't really explain this. Maybe "uhs2_do_action", or
> if you can come up with something even better, that would be great.

Agree. As mentioned above, I plan to use uhs2_do_action to  replace 5 UHS-II
callback functions. And I will check (host->ops->uhs2_do_action) at
the beginning
of the initialization process.

> > +           host->ops->uhs2_set_reg(host, SET_CONFIG)) {
> > +               pr_err("%s: %s: UHS2 SET_CONFIG fail!\n",
> > +                       mmc_hostname(host), __func__);
> > +               return -EIO;
> > +       }
> > +
> > +static int sd_uhs2_go_dormant(struct mmc_host *host, bool hibernate, u32 node_id)
> > +{
> > +
> > +       if (host->ops->uhs2_disable_clk)
>
> Again, another callback that lacks declaration.
>
> > +               host->ops->uhs2_disable_clk(host);
>
> In fact, rather than adding another callback, we could make use of the
> "->uhs2_set_reg" callback?
>
> Or, another option would be to use the ->uhs2_set_ios(). That would be
> similar to what we do for legacy SD cards with the ->set_ios()
> callback.
>
> For example, we can do like this:
> host->ios.power_mode = MMC_POWER_ON;
> host->ios.clock = 0;
> host->ops->uhs2_set_ios(host, &host->ios);
>
> What do you think? Maybe we need a separate host->ios_uhs2 for this too?

As I mentioned above, I intend to use integrated "uhs2_do_action()".
What do you think?

> > +
> > +static int sd_uhs2_change_speed(struct mmc_host *host, u32 node_id)
> > +{
> > +
> > +       /* restore sd clock */
> > +       mmc_delay(5);
> > +       if (host->ops->uhs2_enable_clk)
> > +               host->ops->uhs2_enable_clk(host);
>
> Ditto.
>
> > +
> > +       /*
> > +        * According to UHS-II Addendum Version 1.01, chapter 6.2.3, wait card
> > +        * switch to Active State
> > +        */
> > +       header = UHS2_NATIVE_PACKET | UHS2_PACKET_TYPE_CCMD |
> > +                host->card->uhs2_config.node_id;
>
> Didn't you pass node_id as an in-parameter? Why can't you use that?

Oops! I missed it accidentally in the previous version. I will update
it in the next version.

> > +       arg = ((UHS2_DEV_CONFIG_GEN_SET & 0xFF) << 8) |
> > +               UHS2_NATIVE_CMD_READ |
> > +               UHS2_NATIVE_CMD_PLEN_8B |
> > +               (UHS2_DEV_CONFIG_GEN_SET >> 8);
> >
> >
> > @@ -108,8 +724,70 @@ static int sd_uhs2_config_write(struct mmc_host *host, struct mmc_card *card)
> >   * be set through a legacy CMD6. Note that, the power limit that becomes set,
> >   * survives a soft reset through the GO_DORMANT_STATE command.
> >   */
> > -static int sd_uhs2_legacy_init(struct mmc_host *host, struct mmc_card *card)
> > +static int sd_uhs2_legacy_init(struct mmc_host *host)
>
> Please make this change from the patch that introduced this code.

Okay.

> >  {
> > +       int err;
> > +       u32 ocr, rocr;
> > +
> > +       WARN_ON(!host->claimed);
> > +
> > +       /* Send CMD0 to reset SD card */
> > +       mmc_go_idle(host);
> > +
> > +       /* Send CMD8 to communicate SD interface operation condition */
> > +       err = mmc_send_if_cond(host, host->ocr_avail);
> > +       if (err) {
> > +               pr_err("%s: %s: SEND_IF_COND fail!\n",
> > +                      mmc_hostname(host), __func__);
> > +               return err;
> > +       }
> > +
> > +       /*
> > +        * Send host capacity support information and activate card's
> > +        * initialization process.
> > +        */
> > +       err = mmc_send_app_op_cond(host, 0, &ocr);
> > +       if (err) {
> > +               pr_err("%s: %s: SD_SEND_OP_COND fail!\n",
> > +                      mmc_hostname(host), __func__);
> > +               return err;
> > +       }
> > +
> > +       if (host->ocr_avail_sd)
> > +               host->ocr_avail = host->ocr_avail_sd;
>
> Please drop this part. I don't want to encourage host's to use
> ocr_avail_sd (for reasons that we can discuss another time).

Okay.

> If we need something specific for UHS-II, then it's better to add that.
>
> > +
> > +       /*
> > +        * Some SD cards claims an out of spec VDD voltage range. Let's treat
> > +        * these bits as being in-valid and especially also bit7.
> > +        */
> > +       ocr &= ~0x7FFF;
> > +       rocr = mmc_select_voltage(host, ocr);
> > +
> > +       /*
> > +        * Some cards have zero value of rocr in UHS-II mode. Assign host's
> > +        * ocr value to rocr.
> > +        */
> > +       if (!rocr) {
> > +               if (host->ocr_avail) {
> > +                       rocr = host->ocr_avail;
> > +               } else {
> > +                       pr_err("%s: %s: there is no valid OCR.\n",
> > +                              mmc_hostname(host), __func__);
> > +                       return -EINVAL;
> > +               }
> > +       }
> > +
> > +       /*
> > +        * Get CID, send relative address, get CSD are the same as legacy
> > +        * sd, call mmc_sd_init_card() in sd.c directly
> > +        */
> > +       err = mmc_sd_init_card(host, rocr, NULL);
> > +       if (err) {
> > +               pr_err("%s: %s: mmc_sd_init_card() fail!\n",
> > +                      mmc_hostname(host), __func__);
> > +               return err;
> > +       }
>
> So this means we will start using the host callbacks for the legacy SD
> interface, like ->set_ios() for example.

I will remove this callback and reorganize codes in this portion.

> In other words, data in host->ios.* seems to be used, both for UHS-II
> and legacy SD. That can be messy for host drivers to manage, I think.
> Maybe we should add a separate host->ios_uhs2.*, that we use solely
> for UHS2 instead?

This topic can be discussed.
The ios structure can continue to be used after adding new definitions to
certain fields, for example, adding MMC_TIMING_UHS2 for ios.timing.

> There is also another problem here. mmc_sd_init_card() will allocate a
> new struct mmc_card and assign it to host->card, if it succeeds with
> initialization.
>
> By looking at the changes below, it looks like you will assign
> host->card, with the struct mmc_card that we have allocated for UHS-II
> in sd_uhs2_init_card(), prior to calling sd_uhs2_legacy_init(). Not
> only will this lead to a memory leak, but more importantly we will
> lose all the information that we have parsed/stored from the card for
> UHS-II, right?
>
> Don't we need to keep the UHS-II information of the card around? Or is
> it okay to throw it away after initialization?
>
> What am I missing?

You are right. I will reorganize this portion.

> > +
> >         return 0;
> >  }
> >
> > @@ -124,12 +802,16 @@ static int sd_uhs2_init_card(struct mmc_host *host)
> >         int err;
> >
> >         err = sd_uhs2_dev_init(host);
> > -       if (err)
> > +       if (err) {
> > +               pr_err("%s: UHS2 DEVICE_INIT fail!\n", mmc_hostname(host));
> >                 return err;
> > +       }
> >
> >         err = sd_uhs2_enum(host, &node_id);
> > -       if (err)
> > +       if (err) {
> > +               pr_err("%s: UHS2 ENUMERATE fail!\n", mmc_hostname(host));
> >                 return err;
> > +       }
> >
> >         card = mmc_alloc_card(host, &sd_type);
> >         if (IS_ERR(card))
> > @@ -142,15 +824,26 @@ static int sd_uhs2_init_card(struct mmc_host *host)
> >         if (err)
> >                 goto err;
> >
> > +       /* Change to Speed Range B if it is supported */
> > +       if (host->flags & MMC_UHS2_SPEED_B) {
>
> I noticed that you have added the "host->flags", to keep track of
> UHS-II states and some UHS-II capabilities of the host. I would rather
> see that UHS-II host capabilities should be part of the
> host->uhs2_caps.*.

Agree. I will move the "flags" field to struct sd_uhs2_caps{} in the
next version.

> When it comes to keeping track of UHS-II states, like when we want to
> move to a new speed mode for example, that may be better to keep as a
> part of a new host->ios_uhs2.*, as I kind of indicated above.

UHS-II speed mode is only set at initialization phase and it is not
changed before hardware reset.
Hence, I think it's not necessary to place sd_uhs2_change_speed() into
host->ios_uhs2.*.
Maybe we can discuss host->ios_uhs2.* after you check my next submission.

> > +               err = sd_uhs2_change_speed(host, node_id);
> > +               if (err) {
> > +                       pr_err("%s: %s: UHS2 sd_uhs2_change_speed() fail!\n",
> > +                               mmc_hostname(host), __func__);
> > +                       return err;
> > +               }
> > +       }
> > +
> >         err = sd_uhs2_config_write(host, card);
> >         if (err)
> >                 goto err;
> >
> > -       err = sd_uhs2_legacy_init(host, card);
> > +       host->card = card;
> > +
> > +       err = sd_uhs2_legacy_init(host);
> >         if (err)
> >                 goto err;
> >
> > -       host->card = card;
>
> This is the part that will lead to the memory leak of a struct mmc_card.

I will reorganize this portion to fix this memory leak.

> >         return 0;
> >
> >  err:
> > @@ -217,6 +910,78 @@ static int sd_uhs2_hw_reset(struct mmc_host *host)
> >         return 0;
> >  }
>
> [...]
>
> I have stopped the review at this point, let's see if we can conclude
> on the way forward around my comments on the parts above, to start
> with.
>
> Kind regards
> Uffe

I will start preparing for the next version submission after receiving
your response to my question.
Thank you for your patience.

kind regards
Jason Lai

Re: [PATCH 6/7] mmc: Implement content of UHS-II card initialization functions

[Ulf Hansson]

On Thu, 6 Jan 2022 at 11:31, Lai Jason <jasonlai.genesyslogic@gmail.com> wrote:
>
> On Wed, Dec 15, 2021 at 4:29 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> >> On Fri, 3 Dec 2021 at 11:51, Jason Lai <jasonlai.genesyslogic@gmail.com> wrote:
> > >
> > > From: Jason Lai <jason.lai@genesyslogic.com.tw>
> > >
> > > UHS-II card initialization flow is divided into 2 categories: PHY & Card.
> > > Part 1 - PHY Initialization:
> > >          Every host controller may need their own avtivation operation
> > >          to establish LINK between controller and card. So we add a new
> > >          member function(uhs2_detect_init) in struct mmc_host_ops for host
> > >          controller use.
> > > Part 2 - Card Initialization:
> > >          This part can be divided into 6 substeps.
> > >          1. Send UHS-II CCMD DEVICE_INIT to card.
> > >          2. Send UHS-II CCMD ENUMERATE to card.
> > >          3. Send UHS-II Native Read CCMD to obtain capabilities in CFG_REG
> > >             of card.
> > >          4. Host compares capabilities of host controller and  card,
> > >             then write the negotiated values to Setting field in CFG_REG
> > >             of card through UHS-II Native Write CCMD.
> > >          5. Switch host controller's clock to Range B if it is supported
> > >             by both host controller and card.
> > >          6. Execute legacy SD initialization flow.
> > > Part 3 - Provide a function to tranaform legacy SD command packet into
> > >          UHS-II SD-TRAN DCMD packet.
> > >
> > > Most of the code added above came from Intel's original patch[1].
> > >
> > > [1]
> > > https://patchwork.kernel.org/project/linux-mmc/patch/1419672479-30852-2-
> > > git-send-email-yi.y.sun@intel.com/
> > >
> > > Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
> > > ---
> > >  drivers/mmc/core/sd_uhs2.c | 831 ++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 810 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c
> > > index 24aa51a6d..d13283d54 100644
> > > --- a/drivers/mmc/core/sd_uhs2.c
> > > +++ b/drivers/mmc/core/sd_uhs2.c
> > > @@ -3,6 +3,7 @@
> > >
> > >  static const unsigned int sd_uhs2_freqs[] = { 52000000, 26000000 };
> > >
> > > -static int sd_uhs2_set_ios(struct mmc_host *host)
> > > +static void sd_uhs2_set_ios(struct mmc_host *host)
> >
> > Please don't change the function to return void. It's better to allow
> > the callback to return an error code, even if that might not be of
> > benefit for your particular host variant.
>
> I have a question about the return type of this function. In
> sd_uhs2_set_ios(), we only called
> host->ops->set_ios() and the return type of that function was VOID. So
> is there a reason
> that this function must has a non-void return type?

I think the original ->set_ios() callback should have been "int"
rather than void. It's clear that those callbacks can actually fail,
but today there is no way for them to report an error.

Therefore, please use 'int'.

[...]

> > >  static int sd_uhs2_phy_init(struct mmc_host *host)
> > >  {
> > > -       return 0;
> > > +       int err = 0;
> > > +
> > > +       /*
> > > +        * Every host controller can assign its own function which is used
> > > +        * to initialize PHY.
> > > +        */
> > > +       err = host->ops->uhs2_detect_init(host)
> >
> > It looks like you didn't declare this callback as part of the struct
> > mmc_host_ops, so this does not even compile. Please fix that.
> >
> > I also suggest moving the above comment into the header file along
> > with its declaration, as it's usually where we keep this information.
> >
> > Moreover, to make it clear what the callback is intended for, I
> > suggest renaming it to "uhs2_phy_init".
>
> There are 5 callback functions for UHS-II:
>     int   uhs2_detect_init(struct mmc_host *host);
>     int   uhs2_set_reg(struct mmc_host *host, enum uhs2_action act);
>     void uhs2_disable_clk(struct mmc_host *host);
>     void uhs2_enable_clk(struct mmc_host *host);
>     void uhs2_post_attach_sd(struct mmc_host *host);
> According to your suggestion, I plan to integrate them into a single function,
> named "uhs2_do_action", since they all realize the specified function by
> setting the controller.

I don't mind that we add a few callbacks to support UHS-II, but at the
same time it gets silly to have one for each and every operation.

That said, I think the above approach that you suggest is worth a try.
Let's see what this can bring us to.

>
> > > +       if (err) {
> > > +               pr_err("%s: fail to detect UHS2!\n", mmc_hostname(host));
> >
> > Maybe "failed to initial phy for UHS-II", is better?
>
> Okay. The string will be modified in the next version.
>
> > > +       }
> > > +
> > > +       return err;
> > >  }
> > >
> > >  /*
> > > - * Do the early initialization of the card, by sending the device init
> > > -roadcast
> > > + * Do the early initialization of the card, by sending the device init broadcast
> >
> > Looks like you are correcting a spelling mistake, which is good.
> > However, please make this a part of the patch that introduced this,
> > earlier in the series.
>
> Do you mean to create a separate patch which only contains typo correction?

I would appreciate it, if you can amend my original patch to fix my mistakes.

[...]

> > > +
> > > +static int sd_uhs2_go_dormant(struct mmc_host *host, bool hibernate, u32 node_id)
> > > +{
> > > +
> > > +       if (host->ops->uhs2_disable_clk)
> >
> > Again, another callback that lacks declaration.
> >
> > > +               host->ops->uhs2_disable_clk(host);
> >
> > In fact, rather than adding another callback, we could make use of the
> > "->uhs2_set_reg" callback?
> >
> > Or, another option would be to use the ->uhs2_set_ios(). That would be
> > similar to what we do for legacy SD cards with the ->set_ios()
> > callback.
> >
> > For example, we can do like this:
> > host->ios.power_mode = MMC_POWER_ON;
> > host->ios.clock = 0;
> > host->ops->uhs2_set_ios(host, &host->ios);
> >
> > What do you think? Maybe we need a separate host->ios_uhs2 for this too?
>
> As I mentioned above, I intend to use integrated "uhs2_do_action()".
> What do you think?

As stated, seems reasonable, let's give it a try!

[...]

> > > +        * Get CID, send relative address, get CSD are the same as legacy
> > > +        * sd, call mmc_sd_init_card() in sd.c directly
> > > +        */
> > > +       err = mmc_sd_init_card(host, rocr, NULL);
> > > +       if (err) {
> > > +               pr_err("%s: %s: mmc_sd_init_card() fail!\n",
> > > +                      mmc_hostname(host), __func__);
> > > +               return err;
> > > +       }
> >
> > So this means we will start using the host callbacks for the legacy SD
> > interface, like ->set_ios() for example.
>
> I will remove this callback and reorganize codes in this portion.
>
> > In other words, data in host->ios.* seems to be used, both for UHS-II
> > and legacy SD. That can be messy for host drivers to manage, I think.
> > Maybe we should add a separate host->ios_uhs2.*, that we use solely
> > for UHS2 instead?
>
> This topic can be discussed.
> The ios structure can continue to be used after adding new definitions to
> certain fields, for example, adding MMC_TIMING_UHS2 for ios.timing.

I don't think it's going to play well for existing mmc host drivers to
use the current host->ios.*.

Let's try to add a new host->ios_uhs2.* and add the data we need for
UHS-II in there.

[...]

> I will start preparing for the next version submission after receiving
> your response to my question.
> Thank you for your patience.

Great!

If you need some additional guidance at this point, just ask.
Otherwise I will defer to review your next version of the series.

Kind regards
Uffe

Re: [PATCH 6/7] mmc: Implement content of UHS-II card initialization functions

[Lai Jason]

On Wed, Dec 15, 2021 at 4:29 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Fri, 3 Dec 2021 at 11:51, Jason Lai <jasonlai.genesyslogic@gmail.com> wrote:
> >
> > From: Jason Lai <jason.lai@genesyslogic.com.tw>
> >
> > UHS-II card initialization flow is divided into 2 categories: PHY & Card.
> > Part 1 - PHY Initialization:
> >          Every host controller may need their own avtivation operation
> >          to establish LINK between controller and card. So we add a new
> >          member function(uhs2_detect_init) in struct mmc_host_ops for host
> >          controller use.
> > Part 2 - Card Initialization:
> >          This part can be divided into 6 substeps.
> >          1. Send UHS-II CCMD DEVICE_INIT to card.
> >          2. Send UHS-II CCMD ENUMERATE to card.
> >          3. Send UHS-II Native Read CCMD to obtain capabilities in CFG_REG
> >             of card.
> >          4. Host compares capabilities of host controller and  card,
> >             then write the negotiated values to Setting field in CFG_REG
> >             of card through UHS-II Native Write CCMD.
> >          5. Switch host controller's clock to Range B if it is supported
> >             by both host controller and card.
> >          6. Execute legacy SD initialization flow.
> > Part 3 - Provide a function to tranaform legacy SD command packet into
> >          UHS-II SD-TRAN DCMD packet.
> >
> > Most of the code added above came from Intel's original patch[1].
> >
> > [1]
> > https://patchwork.kernel.org/project/linux-mmc/patch/1419672479-30852-2-
> > git-send-email-yi.y.sun@intel.com/
> >
> > Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
> > ---
> >  drivers/mmc/core/sd_uhs2.c | 831 ++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 810 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c
> > index 24aa51a6d..d13283d54 100644
> > --- a/drivers/mmc/core/sd_uhs2.c
> > +++ b/drivers/mmc/core/sd_uhs2.c
> > @@ -3,6 +3,7 @@

[...]

> > +/**
> > + * sd_uhs2_cmd_assemble() - build up UHS-II command packet which is embeded in
> > + *                          mmc_command structure
> > + * @cmd:       MMC command to executed
> > + * @uhs2_cmd:  UHS2 command corresponded to MMC command
> > + * @header:    Header field of UHS-II command cxpacket
> > + * @arg:       Argument field of UHS-II command packet
> > + * @payload:   Payload field of UHS-II command packet
> > + * @plen:      Payload length
> > + * @resp:      Response buffer is allocated by caller and it is used to keep
> > + *              the response of CM-TRAN command. For SD-TRAN command, uhs2_resp
> > + *              should be null and SD-TRAN command response should be stored in
> > + *              resp of mmc_command.
> > + * @resp_len:  Response buffer length
> > + *
> > + * Different from legacy SD command, there defined several types of packets
> > + * in UHS-II, which include CM-TRAN and SD-TRAN. These packets are then
> > + * transformed to differential signals and transmitted/received between
> > + * transceiver and receiver.
> > + *
> > + * The UHS-II packet structure(uhs2_cmd) are added in structure mmc_command
> > + * and be filled according to the inputed parameters provided by caller.
> > + *
> > + * For mmc requests, call this function before issuing command to SD card.
> > + * Host controller specific request function will send packet in uhs2_cmd
> > + * to UHS-II SD card.
> > + *
>
> I think we discussed the above functions description earlier. The
> above description is just way too overwhelming. What the function does
> is that fills out the uhs2_cmd data structure, that's it.
>
> As this is a static function, I wouldn't mind that you simply drop the
> entire function description above. Or try to make the description
> short and clear.

I try to make the function description more briefly:
/**
 * sd_uhs2_cmd_assemble() - build up UHS-II command packet which is embeded in
 *                          mmc_command structure
 * @cmd: MMC command to executed
 * @uhs2_cmd: UHS2 command corresponded to MMC command
 * @header: Header field of UHS-II command cxpacket
 * @arg: Argument field of UHS-II command packet
 * @payload: Payload field of UHS-II command packet
 * @plen: Payload length
 * @resp: Response buffer is allocated by caller and it is used to keep
 *              the response of CM-TRAN command. For SD-TRAN command, uhs2_resp
 *              should be null and SD-TRAN command response should be stored in
 *              resp of mmc_command.
 * @resp_len: Response buffer length
 *
 * The uhs2_command structure contains message packets which are transmited/
 * received on UHS-II bus. This function fills in the contents of uhs2_command
 * structure and embededs UHS2 command into mmc_command structure, which is used
 * in legacy SD operation functions.
 *
 */
What do you think?

kind regards,
Jason Lai

> [...]
>
> I have stopped the review at this point, let's see if we can conclude
> on the way forward around my comments on the parts above, to start
> with.
>
> Kind regards
> Uffe

[PATCH 7/7] mmc: core: Support UHS-II card access

[Jason Lai]

From: Jason Lai <jason.lai@genesyslogic.com.tw>

Embed the UHS-II access function into the MMC request processing flow.

Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
---
 drivers/mmc/core/core.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 19b409179..d4d873971 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -31,6 +31,7 @@
 #include <linux/mmc/mmc.h>
 #include <linux/mmc/sd.h>
 #include <linux/mmc/slot-gpio.h>
+#include <linux/mmc/sd_uhs2.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/mmc.h>
@@ -42,6 +43,7 @@
 #include "host.h"
 #include "sdio_bus.h"
 #include "pwrseq.h"
+#include "sd_uhs2.h"
 
 #include "mmc_ops.h"
 #include "sd_ops.h"
@@ -335,6 +337,8 @@ static int mmc_mrq_prep(struct mmc_host *host, struct mmc_request *mrq)
 
 int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 {
+	struct uhs2_command uhs2_cmd;
+	u32 payload[4]; /* for maximum size */
 	int err;
 
 	init_completion(&mrq->cmd_completion);
@@ -352,6 +356,13 @@ int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 	if (err)
 		return err;
 
+	if (host->flags & MMC_UHS2_SUPPORT &&
+	    host->flags & MMC_UHS2_INITIALIZED) {
+		uhs2_cmd.payload = payload;
+		mrq->cmd->uhs2_cmd = &uhs2_cmd;
+		sd_uhs2_prepare_cmd(host, mrq);
+	}
+
 	led_trigger_event(host->led, LED_FULL);
 	__mmc_start_request(host, mrq);
 
@@ -431,6 +442,8 @@ EXPORT_SYMBOL(mmc_wait_for_req_done);
  */
 int mmc_cqe_start_req(struct mmc_host *host, struct mmc_request *mrq)
 {
+	struct uhs2_command uhs2_cmd;
+	u32 payload[4]; /* for maximum size */
 	int err;
 
 	/*
@@ -451,6 +464,13 @@ int mmc_cqe_start_req(struct mmc_host *host, struct mmc_request *mrq)
 	if (err)
 		goto out_err;
 
+	if (host->flags & MMC_UHS2_SUPPORT &&
+	    host->flags & MMC_UHS2_INITIALIZED) {
+		uhs2_cmd.payload = payload;
+		mrq->cmd->uhs2_cmd = &uhs2_cmd;
+		sd_uhs2_prepare_cmd(host, mrq);
+	}
+
 	err = host->cqe_ops->cqe_request(host, mrq);
 	if (err)
 		goto out_err;
@@ -899,8 +919,10 @@ static inline void mmc_set_ios(struct mmc_host *host)
  */
 void mmc_set_chip_select(struct mmc_host *host, int mode)
 {
-	host->ios.chip_select = mode;
-	mmc_set_ios(host);
+	if (!(host->flags & MMC_UHS2_INITIALIZED)) {
+		host->ios.chip_select = mode;
+		mmc_set_ios(host);
+	}
 }
 
 /*
-- 
2.34.0