Linux MultiMedia Card development
 help / color / mirror / Atom feed
* [PATCH v6 0/9] Add SDUC Support
@ 2024-09-04 14:52 Avri Altman
  2024-09-04 14:52 ` [PATCH v6 1/9] mmc: sd: SDUC Support Recognition Avri Altman
                   ` (9 more replies)
  0 siblings, 10 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

Ultra Capacity SD cards (SDUC) was already introduced in SD7.0.  Those
cards support capacity larger than 2TB and up to including 128TB. Thus,
the address range of the card expands beyond the 32-bit command
argument. To that end, a new command - CMD22 is defined, to carry the
extra 6-bit upper part of the 38-bit block address that enable access to
128TB memory space.

SDUC capacity is agnostic to the interface mode: UHS-I and UHS-II – Same
as SDXC.

The spec defines several extensions/modifications to the current SDXC
cards, which we address in patches 1 - 10.  Otherwise requirements are
out-of-scope of this change.  Specifically, CMDQ (CMD44+CMD45), and
Extension for Video Speed Class (CMD20).

First publication of SDUC was in [1].  This series was developed and
tested separately from [1] and does not borrow from it.

[1] https://lwn.net/Articles/982566/

---
Changes in v6:
 - Remove Ricky's tested-by tag - the series has changed greatly
 - Call mmc_send_ext_addr from mmc_start_request (Adrian)

Changes in v5:
 - leave out the mask in mmc_send_ext_addr (Adrian)
 - leave out close-ended SDUC support
 - remove 500msec write delay as there is no busy indication (Adrian)
 - disable mmc-test for SDUC
 - move enabling SDUC to the last patch (Adrian)

Changes in v4:
 - Squash patches 1 & 2 (Ulf)
 - Amend SD_OCR_2T to SD_OCR_CCS in mmc_sd_get_cid (Ulf)
 - Use card state instead of caps2 (Ricky & Ulf)
 - Switch patches 5 & 6 (Ulf)

Changes in v3:
 - Some more kernel test robot fixes
 - Fix a typo in a commit log (Ricky WU)
 - Fix ACMD22 returned value
 - Add 'Tested-by' tag for the whole series (Ricky WU)

Changes in v2:
 - Attend kernel test robot warnings

---

Avri Altman (9):
  mmc: sd: SDUC Support Recognition
  mmc: sd: Add Extension memory addressing
  mmc: core: Add open-ended Ext memory addressing
  mmc: core: Don't use close-ended rw for SDUC
  mmc: core: Allow mmc erase to carry large addresses
  mmc: core: Add Ext memory addressing for erase
  mmc: core: Adjust ACMD22 to SDUC
  mmc: core: Disable SDUC for mmc_test
  mmc: core: Enable SDUC

 drivers/mmc/core/block.c    | 43 +++++++++++++++++++++++-------
 drivers/mmc/core/bus.c      |  4 ++-
 drivers/mmc/core/card.h     |  3 +++
 drivers/mmc/core/core.c     | 52 +++++++++++++++++++++++++------------
 drivers/mmc/core/core.h     | 16 +++++++++---
 drivers/mmc/core/mmc_test.c |  7 +++++
 drivers/mmc/core/sd.c       | 36 ++++++++++++++++---------
 drivers/mmc/core/sd.h       |  2 +-
 drivers/mmc/core/sd_ops.c   | 16 ++++++++++++
 drivers/mmc/core/sd_ops.h   |  1 +
 drivers/mmc/core/sdio.c     |  2 +-
 include/linux/mmc/card.h    |  2 +-
 include/linux/mmc/core.h    |  5 ++++
 include/linux/mmc/sd.h      |  4 +++
 14 files changed, 146 insertions(+), 47 deletions(-)

-- 
2.25.1


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

* [PATCH v6 1/9] mmc: sd: SDUC Support Recognition
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
@ 2024-09-04 14:52 ` Avri Altman
  2024-09-06  6:39   ` Adrian Hunter
  2024-09-04 14:52 ` [PATCH v6 2/9] mmc: sd: Add Extension memory addressing Avri Altman
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

Ultra Capacity SD cards (SDUC) was already introduced in SD7.0.  Those
cards support capacity larger than 2TB and up to including 128TB.

ACMD41 was extended to support the host-card handshake during
initialization.  The card expects that the HCS & HO2T bits to be set in
the command argument, and sets the applicable bits in the R3 returned
response.  On the contrary, if a SDUC card is inserted to a
non-supporting host, it will never respond to this ACMD41 until
eventually, the host will timed out and give up.

Also, add SD CSD version 3.0 - designated for SDUC, and properly parse
the csd register as the c_size field got expanded to 28 bits.

Do not enable SDUC for now - leave it to the last patch in the series.

Tested-by: Ricky WU <ricky_wu@realtek.com>
Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/bus.c   |  4 +++-
 drivers/mmc/core/card.h  |  3 +++
 drivers/mmc/core/sd.c    | 33 +++++++++++++++++++++------------
 drivers/mmc/core/sd.h    |  2 +-
 drivers/mmc/core/sdio.c  |  2 +-
 include/linux/mmc/card.h |  2 +-
 include/linux/mmc/sd.h   |  1 +
 7 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 0ddaee0eae54..30763b342bd3 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -321,7 +321,9 @@ int mmc_add_card(struct mmc_card *card)
 	case MMC_TYPE_SD:
 		type = "SD";
 		if (mmc_card_blockaddr(card)) {
-			if (mmc_card_ext_capacity(card))
+			if (mmc_card_ult_capacity(card))
+				type = "SDUC";
+			else if (mmc_card_ext_capacity(card))
 				type = "SDXC";
 			else
 				type = "SDHC";
diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
index b7754a1b8d97..64dcb463a4f4 100644
--- a/drivers/mmc/core/card.h
+++ b/drivers/mmc/core/card.h
@@ -23,6 +23,7 @@
 #define MMC_CARD_SDXC		(1<<3)		/* card is SDXC */
 #define MMC_CARD_REMOVED	(1<<4)		/* card has been removed */
 #define MMC_STATE_SUSPENDED	(1<<5)		/* card is suspended */
+#define MMC_CARD_SDUC		(1<<6)		/* card is SDUC */
 
 #define mmc_card_present(c)	((c)->state & MMC_STATE_PRESENT)
 #define mmc_card_readonly(c)	((c)->state & MMC_STATE_READONLY)
@@ -30,11 +31,13 @@
 #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
 #define mmc_card_removed(c)	((c) && ((c)->state & MMC_CARD_REMOVED))
 #define mmc_card_suspended(c)	((c)->state & MMC_STATE_SUSPENDED)
+#define mmc_card_ult_capacity(c) ((c)->state & MMC_CARD_SDUC)
 
 #define mmc_card_set_present(c)	((c)->state |= MMC_STATE_PRESENT)
 #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
 #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR)
 #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
+#define mmc_card_set_ult_capacity(c) ((c)->state |= MMC_CARD_SDUC)
 #define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED)
 #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED)
 #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED)
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index ee37ad14e79e..eb9990d9db56 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -114,7 +114,7 @@ void mmc_decode_cid(struct mmc_card *card)
 /*
  * Given a 128-bit response, decode to our card CSD structure.
  */
-static int mmc_decode_csd(struct mmc_card *card)
+static int mmc_decode_csd(struct mmc_card *card, bool is_sduc)
 {
 	struct mmc_csd *csd = &card->csd;
 	unsigned int e, m, csd_struct;
@@ -158,9 +158,10 @@ static int mmc_decode_csd(struct mmc_card *card)
 			mmc_card_set_readonly(card);
 		break;
 	case 1:
+	case 2:
 		/*
-		 * This is a block-addressed SDHC or SDXC card. Most
-		 * interesting fields are unused and have fixed
+		 * This is a block-addressed SDHC, SDXC or SDUC card.
+		 * Most interesting fields are unused and have fixed
 		 * values. To avoid getting tripped by buggy cards,
 		 * we assume those fixed values ourselves.
 		 */
@@ -173,14 +174,19 @@ static int mmc_decode_csd(struct mmc_card *card)
 		e = UNSTUFF_BITS(resp, 96, 3);
 		csd->max_dtr	  = tran_exp[e] * tran_mant[m];
 		csd->cmdclass	  = UNSTUFF_BITS(resp, 84, 12);
-		csd->c_size	  = UNSTUFF_BITS(resp, 48, 22);
 
-		/* SDXC cards have a minimum C_SIZE of 0x00FFFF */
-		if (csd->c_size >= 0xFFFF)
+		if (csd_struct == 1)
+			m = UNSTUFF_BITS(resp, 48, 22);
+		else
+			m = UNSTUFF_BITS(resp, 48, 28);
+		csd->c_size = m;
+
+		if (csd->c_size >= 0x400000 && is_sduc)
+			mmc_card_set_ult_capacity(card);
+		else if (csd->c_size >= 0xFFFF)
 			mmc_card_set_ext_capacity(card);
 
-		m = UNSTUFF_BITS(resp, 48, 22);
-		csd->capacity     = (1 + m) << 10;
+		csd->capacity     = (1 + (typeof(sector_t))m) << 10;
 
 		csd->read_blkbits = 9;
 		csd->read_partial = 0;
@@ -841,8 +847,11 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
 	 * block-addressed SDHC cards.
 	 */
 	err = mmc_send_if_cond(host, ocr);
-	if (!err)
+	if (!err) {
 		ocr |= SD_OCR_CCS;
+		/* Set HO2T as well - SDUC card won't respond otherwise */
+		ocr |= SD_OCR_2T;
+	}
 
 	/*
 	 * If the host supports one of UHS-I modes, request the card
@@ -887,7 +896,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
 	return err;
 }
 
-int mmc_sd_get_csd(struct mmc_card *card)
+int mmc_sd_get_csd(struct mmc_card *card, bool is_sduc)
 {
 	int err;
 
@@ -898,7 +907,7 @@ int mmc_sd_get_csd(struct mmc_card *card)
 	if (err)
 		return err;
 
-	err = mmc_decode_csd(card);
+	err = mmc_decode_csd(card, is_sduc);
 	if (err)
 		return err;
 
@@ -1453,7 +1462,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 	}
 
 	if (!oldcard) {
-		err = mmc_sd_get_csd(card);
+		err = mmc_sd_get_csd(card, false);
 		if (err)
 			goto free_card;
 
diff --git a/drivers/mmc/core/sd.h b/drivers/mmc/core/sd.h
index fe6dd46927a4..7e8beface2ca 100644
--- a/drivers/mmc/core/sd.h
+++ b/drivers/mmc/core/sd.h
@@ -10,7 +10,7 @@ struct mmc_host;
 struct mmc_card;
 
 int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr);
-int mmc_sd_get_csd(struct mmc_card *card);
+int mmc_sd_get_csd(struct mmc_card *card, bool is_sduc);
 void mmc_decode_cid(struct mmc_card *card);
 int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
 	bool reinit);
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 4fb247fde5c0..9566837c9848 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -769,7 +769,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 	 * Read CSD, before selecting the card
 	 */
 	if (!oldcard && mmc_card_sd_combo(card)) {
-		err = mmc_sd_get_csd(card);
+		err = mmc_sd_get_csd(card, false);
 		if (err)
 			goto remove;
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index f34407cc2788..f39bce322365 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -35,7 +35,7 @@ struct mmc_csd {
 	unsigned int		wp_grp_size;
 	unsigned int		read_blkbits;
 	unsigned int		write_blkbits;
-	unsigned int		capacity;
+	sector_t		capacity;
 	unsigned int		read_partial:1,
 				read_misalign:1,
 				write_partial:1,
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index 6727576a8755..865cc0ca8543 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -36,6 +36,7 @@
 /* OCR bit definitions */
 #define SD_OCR_S18R		(1 << 24)    /* 1.8V switching request */
 #define SD_ROCR_S18A		SD_OCR_S18R  /* 1.8V switching accepted by card */
+#define SD_OCR_2T		(1 << 27)    /* HO2T/CO2T - SDUC support */
 #define SD_OCR_XPC		(1 << 28)    /* SDXC power control */
 #define SD_OCR_CCS		(1 << 30)    /* Card Capacity Status */
 
-- 
2.25.1


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

* [PATCH v6 2/9] mmc: sd: Add Extension memory addressing
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
  2024-09-04 14:52 ` [PATCH v6 1/9] mmc: sd: SDUC Support Recognition Avri Altman
@ 2024-09-04 14:52 ` Avri Altman
  2024-09-04 14:52 ` [PATCH v6 3/9] mmc: core: Add open-ended Ext " Avri Altman
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

SDUC memory addressing spans beyond 2TB and up to 128TB.  Therefore, 38
bits are required to access the entire memory space of all sectors.
Those extra 6 bits are to be carried by CMD22 prior of sending
read/write/erase commands: CMD17, CMD18, CMD24, CMD25, CMD32, and CMD33.

CMD22 will carry the higher order 6 bits, and must precedes any of the
above commands even if it targets sector < 2TB.

No error related to address or length is indicated in CMD22 but rather
in the read/write command itself.

Tested-by: Ricky WU <ricky_wu@realtek.com>
Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/sd_ops.c | 16 ++++++++++++++++
 drivers/mmc/core/sd_ops.h |  1 +
 include/linux/mmc/sd.h    |  3 +++
 3 files changed, 20 insertions(+)

diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
index 8b9b34286ef3..4397e5e06dd8 100644
--- a/drivers/mmc/core/sd_ops.c
+++ b/drivers/mmc/core/sd_ops.c
@@ -16,6 +16,7 @@
 #include <linux/mmc/sd.h>
 
 #include "core.h"
+#include "card.h"
 #include "sd_ops.h"
 #include "mmc_ops.h"
 
@@ -188,6 +189,21 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
 	return 0;
 }
 
+int mmc_send_ext_addr(struct mmc_host *host, u32 addr)
+{
+	struct mmc_command cmd = {
+		.opcode = SD_ADDR_EXT,
+		.arg = addr,
+		.flags = MMC_RSP_R1 | MMC_CMD_AC,
+	};
+
+	if (!mmc_card_ult_capacity(host->card))
+		return 0;
+
+	return mmc_wait_for_cmd(host, &cmd, 0);
+}
+EXPORT_SYMBOL_GPL(mmc_send_ext_addr);
+
 static int __mmc_send_if_cond(struct mmc_host *host, u32 ocr, u8 pcie_bits,
 			      u32 *resp)
 {
diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
index 7667fc223b74..fd3f10b9cf86 100644
--- a/drivers/mmc/core/sd_ops.h
+++ b/drivers/mmc/core/sd_ops.h
@@ -21,6 +21,7 @@ int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
 int mmc_app_send_scr(struct mmc_card *card);
 int mmc_app_sd_status(struct mmc_card *card, void *ssr);
 int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card);
+int mmc_send_ext_addr(struct mmc_host *host, u32 addr);
 
 #endif
 
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index 865cc0ca8543..af5fc70e09a2 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -15,6 +15,9 @@
 #define SD_SEND_IF_COND           8   /* bcr  [11:0] See below   R7  */
 #define SD_SWITCH_VOLTAGE         11  /* ac                      R1  */
 
+/* Class 2 */
+#define SD_ADDR_EXT		 22   /* ac   [5:0]              R1  */
+
   /* class 10 */
 #define SD_SWITCH                 6   /* adtc [31:0] See below   R1  */
 
-- 
2.25.1


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

* [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
  2024-09-04 14:52 ` [PATCH v6 1/9] mmc: sd: SDUC Support Recognition Avri Altman
  2024-09-04 14:52 ` [PATCH v6 2/9] mmc: sd: Add Extension memory addressing Avri Altman
@ 2024-09-04 14:52 ` Avri Altman
  2024-09-04 22:13   ` Christian Loehle
  2024-09-06  8:02   ` Adrian Hunter
  2024-09-04 14:52 ` [PATCH v6 4/9] mmc: core: Don't use close-ended rw for SDUC Avri Altman
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

For open-ended read/write - just send CMD22 before issuing the command.
While at it, make sure that the rw command arg is properly casting the
lower 32 bits, as it can be larger now.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/block.c | 6 +++++-
 drivers/mmc/core/core.c  | 3 +++
 include/linux/mmc/core.h | 5 +++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index cc7318089cf2..54469261bc25 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -226,6 +226,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
 static int mmc_spi_err_check(struct mmc_card *card);
 static int mmc_blk_busy_cb(void *cb_data, bool *busy);
+static int mmc_blk_wait_for_idle(struct mmc_queue *mq, struct mmc_host *host);
 
 static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
 {
@@ -1710,7 +1711,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 
 	brq->mrq.cmd = &brq->cmd;
 
-	brq->cmd.arg = blk_rq_pos(req);
+	brq->cmd.arg = blk_rq_pos(req) & 0xFFFFFFFF;
 	if (!mmc_card_blockaddr(card))
 		brq->cmd.arg <<= 9;
 	brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
@@ -1758,6 +1759,9 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 			(do_data_tag ? (1 << 29) : 0);
 		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
 		brq->mrq.sbc = &brq->sbc;
+	} else if (mmc_card_ult_capacity(card)) {
+		brq->cmd.ext_addr = (blk_rq_pos(req) >> 32) & 0x3F;
+		brq->cmd.has_ext_addr = 1;
 	}
 }
 
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d6c819dd68ed..a0b2999684b3 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -336,6 +336,9 @@ int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 {
 	int err;
 
+	if (mrq->cmd && mrq->cmd->has_ext_addr)
+		mmc_send_ext_addr(host, mrq->cmd->ext_addr);
+
 	init_completion(&mrq->cmd_completion);
 
 	mmc_retune_hold(host);
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f0ac2e469b32..41c21c216584 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -76,6 +76,11 @@ struct mmc_command {
  */
 #define mmc_cmd_type(cmd)	((cmd)->flags & MMC_CMD_MASK)
 
+	/* for SDUC */
+	u8 has_ext_addr;
+	u8 ext_addr;
+	u16 reserved;
+
 	unsigned int		retries;	/* max number of retries */
 	int			error;		/* command error */
 
-- 
2.25.1


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

* [PATCH v6 4/9] mmc: core: Don't use close-ended rw for SDUC
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
                   ` (2 preceding siblings ...)
  2024-09-04 14:52 ` [PATCH v6 3/9] mmc: core: Add open-ended Ext " Avri Altman
@ 2024-09-04 14:52 ` Avri Altman
  2024-09-04 14:52 ` [PATCH v6 5/9] mmc: core: Allow mmc erase to carry large addresses Avri Altman
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

The SDUC spec expects CMD22 to get squeezed between CMD23 and the
read/write command, e.g. CMD23->CMD22->CMD18 and CMD23->CMD22->CMD25.
At this early stage of adoption, we want to avoid an amid stream of
fixes & quirks of bogus hw, that tends to apply extra logic specifically
around auto-cmd12 & auto-cmd23.

Let's leave close-ended out for now, and re-consider this should those
cards become ubiquitous, if any.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 54469261bc25..35e82b0f924b 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2551,7 +2551,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	if (mmc_host_cmd23(card->host)) {
 		if ((mmc_card_mmc(card) &&
 		     card->csd.mmca_vsn >= CSD_SPEC_VER_3) ||
-		    (mmc_card_sd(card) &&
+		    (mmc_card_sd(card) && !mmc_card_ult_capacity(card) &&
 		     card->scr.cmds & SD_SCR_CMD23_SUPPORT))
 			md->flags |= MMC_BLK_CMD23;
 	}
-- 
2.25.1


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

* [PATCH v6 5/9] mmc: core: Allow mmc erase to carry large addresses
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
                   ` (3 preceding siblings ...)
  2024-09-04 14:52 ` [PATCH v6 4/9] mmc: core: Don't use close-ended rw for SDUC Avri Altman
@ 2024-09-04 14:52 ` Avri Altman
  2024-09-04 14:52 ` [PATCH v6 6/9] mmc: core: Add Ext memory addressing for erase Avri Altman
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

Preparing for SDUC, Allow the erase address to be larger beyond a 32 bit
address.

Tested-by: Ricky WU <ricky_wu@realtek.com>
Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/block.c |  6 ++++--
 drivers/mmc/core/core.c  | 33 ++++++++++++++++++---------------
 drivers/mmc/core/core.h  | 16 ++++++++++++----
 3 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 35e82b0f924b..50d37c4f5a50 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1200,7 +1200,8 @@ static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct request *req,
 {
 	struct mmc_blk_data *md = mq->blkdata;
 	struct mmc_card *card = md->queue.card;
-	unsigned int from, nr;
+	unsigned int nr;
+	sector_t from;
 	int err = 0;
 	blk_status_t status = BLK_STS_OK;
 
@@ -1255,7 +1256,8 @@ static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
 {
 	struct mmc_blk_data *md = mq->blkdata;
 	struct mmc_card *card = md->queue.card;
-	unsigned int from, nr, arg;
+	unsigned int nr, arg;
+	sector_t from;
 	int err = 0, type = MMC_BLK_SECDISCARD;
 	blk_status_t status = BLK_STS_OK;
 
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a0b2999684b3..06f63fbaadfb 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1601,8 +1601,8 @@ static unsigned int mmc_erase_timeout(struct mmc_card *card,
 		return mmc_mmc_erase_timeout(card, arg, qty);
 }
 
-static int mmc_do_erase(struct mmc_card *card, unsigned int from,
-			unsigned int to, unsigned int arg)
+static int mmc_do_erase(struct mmc_card *card, sector_t from,
+			sector_t to, unsigned int arg)
 {
 	struct mmc_command cmd = {};
 	unsigned int qty = 0, busy_timeout = 0;
@@ -1633,8 +1633,8 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from,
 	else if (mmc_card_sd(card))
 		qty += to - from + 1;
 	else
-		qty += ((to / card->erase_size) -
-			(from / card->erase_size)) + 1;
+		qty += (mmc_sector_div(to, card->erase_size) -
+			mmc_sector_div(from, card->erase_size)) + 1;
 
 	if (!mmc_card_blockaddr(card)) {
 		from <<= 9;
@@ -1703,18 +1703,19 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from,
 }
 
 static unsigned int mmc_align_erase_size(struct mmc_card *card,
-					 unsigned int *from,
-					 unsigned int *to,
+					 sector_t *from,
+					 sector_t *to,
 					 unsigned int nr)
 {
-	unsigned int from_new = *from, nr_new = nr, rem;
+	sector_t from_new = *from;
+	unsigned int nr_new = nr, rem;
 
 	/*
 	 * When the 'card->erase_size' is power of 2, we can use round_up/down()
 	 * to align the erase size efficiently.
 	 */
 	if (is_power_of_2(card->erase_size)) {
-		unsigned int temp = from_new;
+		sector_t temp = from_new;
 
 		from_new = round_up(temp, card->erase_size);
 		rem = from_new - temp;
@@ -1726,7 +1727,7 @@ static unsigned int mmc_align_erase_size(struct mmc_card *card,
 
 		nr_new = round_down(nr_new, card->erase_size);
 	} else {
-		rem = from_new % card->erase_size;
+		rem = mmc_sector_mod(from_new, card->erase_size);
 		if (rem) {
 			rem = card->erase_size - rem;
 			from_new += rem;
@@ -1759,10 +1760,12 @@ static unsigned int mmc_align_erase_size(struct mmc_card *card,
  *
  * Caller must claim host before calling this function.
  */
-int mmc_erase(struct mmc_card *card, unsigned int from, unsigned int nr,
+int mmc_erase(struct mmc_card *card, sector_t from, unsigned int nr,
 	      unsigned int arg)
 {
-	unsigned int rem, to = from + nr;
+	unsigned int rem;
+	sector_t to = from + nr;
+
 	int err;
 
 	if (!(card->csd.cmdclass & CCC_ERASE))
@@ -1783,7 +1786,7 @@ int mmc_erase(struct mmc_card *card, unsigned int from, unsigned int nr,
 		return -EOPNOTSUPP;
 
 	if (arg == MMC_SECURE_ERASE_ARG) {
-		if (from % card->erase_size || nr % card->erase_size)
+		if (mmc_sector_mod(from, card->erase_size) || nr % card->erase_size)
 			return -EINVAL;
 	}
 
@@ -1807,7 +1810,7 @@ int mmc_erase(struct mmc_card *card, unsigned int from, unsigned int nr,
 	 * and call mmc_do_erase() twice if necessary. This special case is
 	 * identified by the card->eg_boundary flag.
 	 */
-	rem = card->erase_size - (from % card->erase_size);
+	rem = card->erase_size - mmc_sector_mod(from, card->erase_size);
 	if ((arg & MMC_TRIM_OR_DISCARD_ARGS) && card->eg_boundary && nr > rem) {
 		err = mmc_do_erase(card, from, from + rem - 1, arg);
 		from += rem;
@@ -1866,12 +1869,12 @@ int mmc_can_secure_erase_trim(struct mmc_card *card)
 }
 EXPORT_SYMBOL(mmc_can_secure_erase_trim);
 
-int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
+int mmc_erase_group_aligned(struct mmc_card *card, sector_t from,
 			    unsigned int nr)
 {
 	if (!card->erase_size)
 		return 0;
-	if (from % card->erase_size || nr % card->erase_size)
+	if (mmc_sector_mod(from, card->erase_size) || nr % card->erase_size)
 		return 0;
 	return 1;
 }
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 37091a6589ed..fc9d5e9620b1 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -116,15 +116,13 @@ bool mmc_is_req_done(struct mmc_host *host, struct mmc_request *mrq);
 
 int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq);
 
-int mmc_erase(struct mmc_card *card, unsigned int from, unsigned int nr,
-		unsigned int arg);
+int mmc_erase(struct mmc_card *card, sector_t from, unsigned int nr, unsigned int arg);
 int mmc_can_erase(struct mmc_card *card);
 int mmc_can_trim(struct mmc_card *card);
 int mmc_can_discard(struct mmc_card *card);
 int mmc_can_sanitize(struct mmc_card *card);
 int mmc_can_secure_erase_trim(struct mmc_card *card);
-int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
-			unsigned int nr);
+int mmc_erase_group_aligned(struct mmc_card *card, sector_t from, unsigned int nr);
 unsigned int mmc_calc_max_discard(struct mmc_card *card);
 
 int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
@@ -199,4 +197,14 @@ static inline int mmc_flush_cache(struct mmc_host *host)
 	return 0;
 }
 
+static inline unsigned int mmc_sector_div(sector_t dividend, u32 divisor)
+{
+	return div_u64(dividend, divisor);
+}
+
+static inline unsigned int mmc_sector_mod(sector_t dividend, u32 divisor)
+{
+	return sector_div(dividend, divisor);
+}
+
 #endif
-- 
2.25.1


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

* [PATCH v6 6/9] mmc: core: Add Ext memory addressing for erase
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
                   ` (4 preceding siblings ...)
  2024-09-04 14:52 ` [PATCH v6 5/9] mmc: core: Allow mmc erase to carry large addresses Avri Altman
@ 2024-09-04 14:52 ` Avri Altman
  2024-09-06 10:06   ` Adrian Hunter
  2024-09-04 14:52 ` [PATCH v6 7/9] mmc: core: Adjust ACMD22 to SDUC Avri Altman
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

CMD22 shall precede CMD32 and CMD33 to configure 38-bit erase start
address and 38 bit erase stop address.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 06f63fbaadfb..8d9f2c44d2a2 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1645,8 +1645,14 @@ static int mmc_do_erase(struct mmc_card *card, sector_t from,
 		cmd.opcode = SD_ERASE_WR_BLK_START;
 	else
 		cmd.opcode = MMC_ERASE_GROUP_START;
-	cmd.arg = from;
+	cmd.arg = from & 0xFFFFFFFF;
 	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
+
+	if (mmc_card_ult_capacity(card)) {
+		cmd.ext_addr = (from >> 32) & 0x3F;
+		cmd.has_ext_addr = 1;
+	}
+
 	err = mmc_wait_for_cmd(card->host, &cmd, 0);
 	if (err) {
 		pr_err("mmc_erase: group start error %d, "
@@ -1660,8 +1666,14 @@ static int mmc_do_erase(struct mmc_card *card, sector_t from,
 		cmd.opcode = SD_ERASE_WR_BLK_END;
 	else
 		cmd.opcode = MMC_ERASE_GROUP_END;
-	cmd.arg = to;
+	cmd.arg = to & 0xFFFFFFFF;
 	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
+
+	if (mmc_card_ult_capacity(card)) {
+		cmd.ext_addr = (to >> 32) & 0x3F;
+		cmd.has_ext_addr = 1;
+	}
+
 	err = mmc_wait_for_cmd(card->host, &cmd, 0);
 	if (err) {
 		pr_err("mmc_erase: group end error %d, status %#x\n",
-- 
2.25.1


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

* [PATCH v6 7/9] mmc: core: Adjust ACMD22 to SDUC
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
                   ` (5 preceding siblings ...)
  2024-09-04 14:52 ` [PATCH v6 6/9] mmc: core: Add Ext memory addressing for erase Avri Altman
@ 2024-09-04 14:52 ` Avri Altman
  2024-09-06 10:57   ` Adrian Hunter
  2024-09-04 14:52 ` [PATCH v6 8/9] mmc: core: Disable SDUC for mmc_test Avri Altman
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

ACMD22 is used to verify the previously write operation.  Normally, it
returns the number of written sectors as u32.  SDUC, however, returns it
as u64.  This is not a superfluous requirement, because SDUC writes may
exceeds 2TB.  For Linux mmc however, the previously write operation
could not be more than the block layer limits, thus we make room for a
u64 and cast the returning value to u32.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/block.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 50d37c4f5a50..f36611512a1d 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -50,6 +50,7 @@
 #include <linux/mmc/sd.h>
 
 #include <linux/uaccess.h>
+#include <asm/unaligned.h>
 
 #include "queue.h"
 #include "block.h"
@@ -994,11 +995,10 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
 	int err;
 	u32 result;
 	__be32 *blocks;
-
+	u8 resp_sz;
 	struct mmc_request mrq = {};
 	struct mmc_command cmd = {};
 	struct mmc_data data = {};
-
 	struct scatterlist sg;
 
 	err = mmc_app_cmd(card->host, card);
@@ -1009,7 +1009,14 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
 	cmd.arg = 0;
 	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
 
-	data.blksz = 4;
+	/*
+	 * Normally, ACMD22 returns the number of written sectors as u32.
+	 * SDUC, however, returns it as u64.  This is not a superfluous
+	 * requirement, because SDUC writes may exceed 2TB.
+	 */
+	resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
+
+	data.blksz = resp_sz;
 	data.blocks = 1;
 	data.flags = MMC_DATA_READ;
 	data.sg = &sg;
@@ -1019,15 +1026,25 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
 	mrq.cmd = &cmd;
 	mrq.data = &data;
 
-	blocks = kmalloc(4, GFP_KERNEL);
+	blocks = kmalloc(resp_sz, GFP_KERNEL);
 	if (!blocks)
 		return -ENOMEM;
 
-	sg_init_one(&sg, blocks, 4);
+	sg_init_one(&sg, blocks, resp_sz);
 
 	mmc_wait_for_req(card->host, &mrq);
 
-	result = ntohl(*blocks);
+	if (mmc_card_ult_capacity(card)) {
+		u64 blocks_64 = get_unaligned_be64(blocks);
+		/*
+		 * For Linux mmc however, the previously write operation could
+		 * not be more than the block layer limits, thus just make room
+		 * for a u64 and cast the response back to u32.
+		 */
+		result = blocks_64 > UINT_MAX ? UINT_MAX : (u32)blocks_64;
+	} else {
+		result = ntohl(*blocks);
+	}
 	kfree(blocks);
 
 	if (cmd.error || data.error)
-- 
2.25.1


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

* [PATCH v6 8/9] mmc: core: Disable SDUC for mmc_test
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
                   ` (6 preceding siblings ...)
  2024-09-04 14:52 ` [PATCH v6 7/9] mmc: core: Adjust ACMD22 to SDUC Avri Altman
@ 2024-09-04 14:52 ` Avri Altman
  2024-09-06 10:10   ` Adrian Hunter
  2024-09-04 14:52 ` [PATCH v6 9/9] mmc: core: Enable SDUC Avri Altman
  2024-09-06 11:10 ` [PATCH v6 0/9] Add SDUC Support Adrian Hunter
  9 siblings, 1 reply; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

Panning to ameliorate it in the very near future.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/mmc_test.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
index b7f627a9fdea..a28e1a1aded3 100644
--- a/drivers/mmc/core/mmc_test.c
+++ b/drivers/mmc/core/mmc_test.c
@@ -3218,6 +3218,13 @@ static int mmc_test_register_dbgfs_file(struct mmc_card *card)
 
 	mutex_lock(&mmc_test_lock);
 
+	if (mmc_card_ult_capacity(card)) {
+		pr_info("%s: mmc-test currently UNSUPPORTED for SDUC\n",
+			mmc_hostname(card->host));
+		ret = -EOPNOTSUPP;
+		goto err;
+	}
+
 	ret = __mmc_test_register_dbgfs_file(card, "test", S_IWUSR | S_IRUGO,
 		&mmc_test_fops_test);
 	if (ret)
-- 
2.25.1


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

* [PATCH v6 9/9] mmc: core: Enable SDUC
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
                   ` (7 preceding siblings ...)
  2024-09-04 14:52 ` [PATCH v6 8/9] mmc: core: Disable SDUC for mmc_test Avri Altman
@ 2024-09-04 14:52 ` Avri Altman
  2024-09-06 11:10 ` [PATCH v6 0/9] Add SDUC Support Adrian Hunter
  9 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-04 14:52 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin, Avri Altman

Enable SDUC if the card responded to ACMD41 with HCS & HO2T bits set.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/sd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index eb9990d9db56..4261d3ae31ef 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1462,7 +1462,10 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 	}
 
 	if (!oldcard) {
-		err = mmc_sd_get_csd(card, false);
+		u32 sduc_arg = SD_OCR_CCS | SD_OCR_2T;
+		bool is_sduc = (rocr & sduc_arg) == sduc_arg;
+
+		err = mmc_sd_get_csd(card, is_sduc);
 		if (err)
 			goto free_card;
 
-- 
2.25.1


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

* Re: [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-04 14:52 ` [PATCH v6 3/9] mmc: core: Add open-ended Ext " Avri Altman
@ 2024-09-04 22:13   ` Christian Loehle
  2024-09-05  6:12     ` Avri Altman
  2024-09-06  8:02   ` Adrian Hunter
  1 sibling, 1 reply; 28+ messages in thread
From: Christian Loehle @ 2024-09-04 22:13 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc; +Cc: Adrian Hunter, Ricky WU, Shawn Lin

On 9/4/24 15:52, Avri Altman wrote:
> For open-ended read/write - just send CMD22 before issuing the command.
> While at it, make sure that the rw command arg is properly casting the
> lower 32 bits, as it can be larger now.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/block.c | 6 +++++-
>  drivers/mmc/core/core.c  | 3 +++
>  include/linux/mmc/core.h | 5 +++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index cc7318089cf2..54469261bc25 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -226,6 +226,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
>  static int mmc_spi_err_check(struct mmc_card *card);
>  static int mmc_blk_busy_cb(void *cb_data, bool *busy);
> +static int mmc_blk_wait_for_idle(struct mmc_queue *mq, struct mmc_host *host);
>  
>  static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
>  {
> @@ -1710,7 +1711,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  
>  	brq->mrq.cmd = &brq->cmd;
>  
> -	brq->cmd.arg = blk_rq_pos(req);
> +	brq->cmd.arg = blk_rq_pos(req) & 0xFFFFFFFF;
>  	if (!mmc_card_blockaddr(card))
>  		brq->cmd.arg <<= 9;
>  	brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> @@ -1758,6 +1759,9 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  			(do_data_tag ? (1 << 29) : 0);
>  		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
>  		brq->mrq.sbc = &brq->sbc;
> +	} else if (mmc_card_ult_capacity(card)) {
> +		brq->cmd.ext_addr = (blk_rq_pos(req) >> 32) & 0x3F;
> +		brq->cmd.has_ext_addr = 1;
>  	}
>  }
>  
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index d6c819dd68ed..a0b2999684b3 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -336,6 +336,9 @@ int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>  {
>  	int err;
>  
> +	if (mrq->cmd && mrq->cmd->has_ext_addr)
> +		mmc_send_ext_addr(host, mrq->cmd->ext_addr);

We should check that this was actually successful, right?

> +
>  	init_completion(&mrq->cmd_completion);
>  
>  	mmc_retune_hold(host);
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index f0ac2e469b32..41c21c216584 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -76,6 +76,11 @@ struct mmc_command {
>   */
>  #define mmc_cmd_type(cmd)	((cmd)->flags & MMC_CMD_MASK)
>  
> +	/* for SDUC */
> +	u8 has_ext_addr;
> +	u8 ext_addr;
> +	u16 reserved;

Is there a reason for has_ext_addr being u8?
What's the reserved for?

> +
>  	unsigned int		retries;	/* max number of retries */
>  	int			error;		/* command error */
>  


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

* RE: [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-04 22:13   ` Christian Loehle
@ 2024-09-05  6:12     ` Avri Altman
  2024-09-05  7:25       ` Adrian Hunter
  2024-09-05  8:27       ` Christian Loehle
  0 siblings, 2 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-05  6:12 UTC (permalink / raw)
  To: Christian Loehle, Ulf Hansson, linux-mmc@vger.kernel.org
  Cc: Adrian Hunter, Ricky WU, Shawn Lin

Thanks for having a look.

> >
> > +     if (mrq->cmd && mrq->cmd->has_ext_addr)
> > +             mmc_send_ext_addr(host, mrq->cmd->ext_addr);
> 
> We should check that this was actually successful, right?
Actually no, as errors in CMD22 are being carried in the subsequent command.

> 
> > +
> >       init_completion(&mrq->cmd_completion);
> >
> >       mmc_retune_hold(host);
> > diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
> > f0ac2e469b32..41c21c216584 100644
> > --- a/include/linux/mmc/core.h
> > +++ b/include/linux/mmc/core.h
> > @@ -76,6 +76,11 @@ struct mmc_command {
> >   */
> >  #define mmc_cmd_type(cmd)    ((cmd)->flags & MMC_CMD_MASK)
> >
> > +     /* for SDUC */
> > +     u8 has_ext_addr;
> > +     u8 ext_addr;
> > +     u16 reserved;
> 
> Is there a reason for has_ext_addr being u8?
Theoretically a single bit suffices, and since ext_addr uses only 6 bits, I had that bit to spare in ext_addr,
but I see no reason to be cheap here - see the reserved u16.

> What's the reserved for?
Not to break the packed 4bytes alignment of mmc_command.

Thanks,
Avri
> 
> > +
> >       unsigned int            retries;        /* max number of retries */
> >       int                     error;          /* command error */
> >


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

* Re: [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-05  6:12     ` Avri Altman
@ 2024-09-05  7:25       ` Adrian Hunter
  2024-09-05  8:27       ` Christian Loehle
  1 sibling, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2024-09-05  7:25 UTC (permalink / raw)
  To: Avri Altman, Christian Loehle, Ulf Hansson,
	linux-mmc@vger.kernel.org
  Cc: Ricky WU, Shawn Lin

On 5/09/24 09:12, Avri Altman wrote:
> Thanks for having a look.
> 
>>>
>>> +     if (mrq->cmd && mrq->cmd->has_ext_addr)
>>> +             mmc_send_ext_addr(host, mrq->cmd->ext_addr);
>>
>> We should check that this was actually successful, right?
> Actually no, as errors in CMD22 are being carried in the subsequent command.
> 
>>
>>> +
>>>       init_completion(&mrq->cmd_completion);
>>>
>>>       mmc_retune_hold(host);
>>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
>>> f0ac2e469b32..41c21c216584 100644
>>> --- a/include/linux/mmc/core.h
>>> +++ b/include/linux/mmc/core.h
>>> @@ -76,6 +76,11 @@ struct mmc_command {
>>>   */
>>>  #define mmc_cmd_type(cmd)    ((cmd)->flags & MMC_CMD_MASK)
>>>
>>> +     /* for SDUC */
>>> +     u8 has_ext_addr;
>>> +     u8 ext_addr;
>>> +     u16 reserved;
>>
>> Is there a reason for has_ext_addr being u8?
> Theoretically a single bit suffices, and since ext_addr uses only 6 bits, I had that bit to spare in ext_addr,

It could be a bool instead of u8 though.

> but I see no reason to be cheap here - see the reserved u16.
> 
>> What's the reserved for?
> Not to break the packed 4bytes alignment of mmc_command.

Although it isn't "__packed" so compiler should take care of
alignment.

> 
> Thanks,
> Avri
>>
>>> +
>>>       unsigned int            retries;        /* max number of retries */
>>>       int                     error;          /* command error */
>>>
> 


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

* Re: [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-05  6:12     ` Avri Altman
  2024-09-05  7:25       ` Adrian Hunter
@ 2024-09-05  8:27       ` Christian Loehle
  2024-09-05  9:47         ` Avri Altman
  1 sibling, 1 reply; 28+ messages in thread
From: Christian Loehle @ 2024-09-05  8:27 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc@vger.kernel.org
  Cc: Adrian Hunter, Ricky WU, Shawn Lin

On 9/5/24 07:12, Avri Altman wrote:
> Thanks for having a look.
> 
>>>
>>> +     if (mrq->cmd && mrq->cmd->has_ext_addr)
>>> +             mmc_send_ext_addr(host, mrq->cmd->ext_addr);
>>
>> We should check that this was actually successful, right?
> Actually no, as errors in CMD22 are being carried in the subsequent command.

I see, sorry for the noise.

> 
>>
>>> +
>>>       init_completion(&mrq->cmd_completion);
>>>
>>>       mmc_retune_hold(host);
>>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
>>> f0ac2e469b32..41c21c216584 100644
>>> --- a/include/linux/mmc/core.h
>>> +++ b/include/linux/mmc/core.h
>>> @@ -76,6 +76,11 @@ struct mmc_command {
>>>   */
>>>  #define mmc_cmd_type(cmd)    ((cmd)->flags & MMC_CMD_MASK)
>>>
>>> +     /* for SDUC */
>>> +     u8 has_ext_addr;
>>> +     u8 ext_addr;
>>> +     u16 reserved;
>>
>> Is there a reason for has_ext_addr being u8?
> Theoretically a single bit suffices, and since ext_addr uses only 6 bits, I had that bit to spare in ext_addr,
> but I see no reason to be cheap here - see the reserved u16.
> 
>> What's the reserved for?
> Not to break the packed 4bytes alignment of mmc_command.

FWIW, that's what it looks like so I was a bit surprised:

pahole -C mmc_command vmlinux
 struct mmc_command {
	u32                        opcode;               /*     0     4 */
	u32                        arg;                  /*     4     4 */
	u32                        resp[4];              /*     8    16 */
	unsigned int               flags;                /*    24     4 */
	bool                       has_ext_addr;         /*    28     1 */
	u8                         ext_addr;             /*    29     1 */
	u16                        reserved;             /*    30     2 */
	unsigned int               retries;              /*    32     4 */
	int                        error;                /*    36     4 */
	unsigned int               busy_timeout;         /*    40     4 */

	/* XXX 4 bytes hole, try to pack */

	struct mmc_data *          data;                 /*    48     8 */
	struct mmc_request *       mrq;                  /*    56     8 */

	/* size: 64, cachelines: 1, members: 12 */
	/* sum members: 60, holes: 1, sum holes: 4 */
};

has_ext_addr also should be equivalent to:
mmc_card_ult_capacity(card) && opcode in [17,18,24,25,32,33]
but the flag is also fine.

I'm a bit hesitant to put my R-b but it all looks plausible
to me FWIW.

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

* RE: [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-05  8:27       ` Christian Loehle
@ 2024-09-05  9:47         ` Avri Altman
  0 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-05  9:47 UTC (permalink / raw)
  To: Christian Loehle, Ulf Hansson, linux-mmc@vger.kernel.org
  Cc: Adrian Hunter, Ricky WU, Shawn Lin

> >>> +     /* for SDUC */
> >>> +     u8 has_ext_addr;
> >>> +     u8 ext_addr;
> >>> +     u16 reserved;
> >>
> >> Is there a reason for has_ext_addr being u8?
> > Theoretically a single bit suffices, and since ext_addr uses only 6
> > bits, I had that bit to spare in ext_addr, but I see no reason to be cheap here -
> see the reserved u16.
> >
> >> What's the reserved for?
> > Not to break the packed 4bytes alignment of mmc_command.
> 
> FWIW, that's what it looks like so I was a bit surprised:
> 
> pahole -C mmc_command vmlinux
>  struct mmc_command {
>         u32                        opcode;               /*     0     4 */
>         u32                        arg;                  /*     4     4 */
>         u32                        resp[4];              /*     8    16 */
>         unsigned int               flags;                /*    24     4 */
>         bool                       has_ext_addr;         /*    28     1 */
>         u8                         ext_addr;             /*    29     1 */
>         u16                        reserved;             /*    30     2 */
>         unsigned int               retries;              /*    32     4 */
>         int                        error;                /*    36     4 */
>         unsigned int               busy_timeout;         /*    40     4 */
> 
>         /* XXX 4 bytes hole, try to pack */
> 
>         struct mmc_data *          data;                 /*    48     8 */
>         struct mmc_request *       mrq;                  /*    56     8 */
> 
>         /* size: 64, cachelines: 1, members: 12 */
>         /* sum members: 60, holes: 1, sum holes: 4 */ };
Moving the sduc members to the end minimizes the padding further:
$ pahole -C mmc_command vmlinux
struct mmc_command {
        u32                        opcode;               /*     0     4 */
        u32                        arg;                  /*     4     4 */
        u32                        resp[4];              /*     8    16 */
        unsigned int               flags;                /*    24     4 */
        unsigned int               retries;              /*    28     4 */
        int                        error;                /*    32     4 */
        unsigned int               busy_timeout;         /*    36     4 */
        struct mmc_data *          data;                 /*    40     8 */
        struct mmc_request *       mrq;                  /*    48     8 */
        u8                         has_ext_addr;         /*    56     1 */
        u8                         ext_addr;             /*    57     1 */
        u16                        reserved;             /*    58     2 */

        /* size: 64, cachelines: 1, members: 12 */
        /* padding: 4 */
};

I guess I can do that.

Thanks,
Avri

> 
> has_ext_addr also should be equivalent to:
> mmc_card_ult_capacity(card) && opcode in [17,18,24,25,32,33] but the flag is
> also fine.
> 
> I'm a bit hesitant to put my R-b but it all looks plausible to me FWIW.

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

* Re: [PATCH v6 1/9] mmc: sd: SDUC Support Recognition
  2024-09-04 14:52 ` [PATCH v6 1/9] mmc: sd: SDUC Support Recognition Avri Altman
@ 2024-09-06  6:39   ` Adrian Hunter
  2024-09-06  8:10     ` Avri Altman
  0 siblings, 1 reply; 28+ messages in thread
From: Adrian Hunter @ 2024-09-06  6:39 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc; +Cc: Ricky WU, Shawn Lin

On 4/09/24 17:52, Avri Altman wrote:
> Ultra Capacity SD cards (SDUC) was already introduced in SD7.0.  Those
> cards support capacity larger than 2TB and up to including 128TB.
> 
> ACMD41 was extended to support the host-card handshake during
> initialization.  The card expects that the HCS & HO2T bits to be set in
> the command argument, and sets the applicable bits in the R3 returned
> response.  On the contrary, if a SDUC card is inserted to a
> non-supporting host, it will never respond to this ACMD41 until
> eventually, the host will timed out and give up.
> 
> Also, add SD CSD version 3.0 - designated for SDUC, and properly parse
> the csd register as the c_size field got expanded to 28 bits.
> 
> Do not enable SDUC for now - leave it to the last patch in the series.
> 
> Tested-by: Ricky WU <ricky_wu@realtek.com>
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/bus.c   |  4 +++-
>  drivers/mmc/core/card.h  |  3 +++
>  drivers/mmc/core/sd.c    | 33 +++++++++++++++++++++------------
>  drivers/mmc/core/sd.h    |  2 +-
>  drivers/mmc/core/sdio.c  |  2 +-
>  include/linux/mmc/card.h |  2 +-
>  include/linux/mmc/sd.h   |  1 +
>  7 files changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 0ddaee0eae54..30763b342bd3 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -321,7 +321,9 @@ int mmc_add_card(struct mmc_card *card)
>  	case MMC_TYPE_SD:
>  		type = "SD";
>  		if (mmc_card_blockaddr(card)) {
> -			if (mmc_card_ext_capacity(card))
> +			if (mmc_card_ult_capacity(card))
> +				type = "SDUC";
> +			else if (mmc_card_ext_capacity(card))
>  				type = "SDXC";
>  			else
>  				type = "SDHC";
> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
> index b7754a1b8d97..64dcb463a4f4 100644
> --- a/drivers/mmc/core/card.h
> +++ b/drivers/mmc/core/card.h
> @@ -23,6 +23,7 @@
>  #define MMC_CARD_SDXC		(1<<3)		/* card is SDXC */
>  #define MMC_CARD_REMOVED	(1<<4)		/* card has been removed */
>  #define MMC_STATE_SUSPENDED	(1<<5)		/* card is suspended */
> +#define MMC_CARD_SDUC		(1<<6)		/* card is SDUC */
>  
>  #define mmc_card_present(c)	((c)->state & MMC_STATE_PRESENT)
>  #define mmc_card_readonly(c)	((c)->state & MMC_STATE_READONLY)
> @@ -30,11 +31,13 @@
>  #define mmc_card_ext_capacity(c) ((c)->state & MMC_CARD_SDXC)
>  #define mmc_card_removed(c)	((c) && ((c)->state & MMC_CARD_REMOVED))
>  #define mmc_card_suspended(c)	((c)->state & MMC_STATE_SUSPENDED)
> +#define mmc_card_ult_capacity(c) ((c)->state & MMC_CARD_SDUC)
>  
>  #define mmc_card_set_present(c)	((c)->state |= MMC_STATE_PRESENT)
>  #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
>  #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR)
>  #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
> +#define mmc_card_set_ult_capacity(c) ((c)->state |= MMC_CARD_SDUC)
>  #define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED)
>  #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED)
>  #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED)
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index ee37ad14e79e..eb9990d9db56 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -114,7 +114,7 @@ void mmc_decode_cid(struct mmc_card *card)
>  /*
>   * Given a 128-bit response, decode to our card CSD structure.
>   */
> -static int mmc_decode_csd(struct mmc_card *card)
> +static int mmc_decode_csd(struct mmc_card *card, bool is_sduc)
>  {
>  	struct mmc_csd *csd = &card->csd;
>  	unsigned int e, m, csd_struct;
> @@ -158,9 +158,10 @@ static int mmc_decode_csd(struct mmc_card *card)
>  			mmc_card_set_readonly(card);
>  		break;
>  	case 1:
> +	case 2:
>  		/*
> -		 * This is a block-addressed SDHC or SDXC card. Most
> -		 * interesting fields are unused and have fixed
> +		 * This is a block-addressed SDHC, SDXC or SDUC card.
> +		 * Most interesting fields are unused and have fixed
>  		 * values. To avoid getting tripped by buggy cards,
>  		 * we assume those fixed values ourselves.
>  		 */
> @@ -173,14 +174,19 @@ static int mmc_decode_csd(struct mmc_card *card)
>  		e = UNSTUFF_BITS(resp, 96, 3);
>  		csd->max_dtr	  = tran_exp[e] * tran_mant[m];
>  		csd->cmdclass	  = UNSTUFF_BITS(resp, 84, 12);
> -		csd->c_size	  = UNSTUFF_BITS(resp, 48, 22);
>  
> -		/* SDXC cards have a minimum C_SIZE of 0x00FFFF */
> -		if (csd->c_size >= 0xFFFF)
> +		if (csd_struct == 1)
> +			m = UNSTUFF_BITS(resp, 48, 22);
> +		else
> +			m = UNSTUFF_BITS(resp, 48, 28);
> +		csd->c_size = m;
> +
> +		if (csd->c_size >= 0x400000 && is_sduc)
> +			mmc_card_set_ult_capacity(card);
> +		else if (csd->c_size >= 0xFFFF)
>  			mmc_card_set_ext_capacity(card);
>  
> -		m = UNSTUFF_BITS(resp, 48, 22);
> -		csd->capacity     = (1 + m) << 10;
> +		csd->capacity     = (1 + (typeof(sector_t))m) << 10;
>  
>  		csd->read_blkbits = 9;
>  		csd->read_partial = 0;
> @@ -841,8 +847,11 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
>  	 * block-addressed SDHC cards.
>  	 */
>  	err = mmc_send_if_cond(host, ocr);
> -	if (!err)
> +	if (!err) {
>  		ocr |= SD_OCR_CCS;
> +		/* Set HO2T as well - SDUC card won't respond otherwise */
> +		ocr |= SD_OCR_2T;

Wouldn't that be better to leave for the last patch.

> +	}
>  
>  	/*
>  	 * If the host supports one of UHS-I modes, request the card
> @@ -887,7 +896,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
>  	return err;
>  }
>  
> -int mmc_sd_get_csd(struct mmc_card *card)
> +int mmc_sd_get_csd(struct mmc_card *card, bool is_sduc)
>  {
>  	int err;
>  
> @@ -898,7 +907,7 @@ int mmc_sd_get_csd(struct mmc_card *card)
>  	if (err)
>  		return err;
>  
> -	err = mmc_decode_csd(card);
> +	err = mmc_decode_csd(card, is_sduc);
>  	if (err)
>  		return err;
>  
> @@ -1453,7 +1462,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
>  	}
>  
>  	if (!oldcard) {
> -		err = mmc_sd_get_csd(card);
> +		err = mmc_sd_get_csd(card, false);
>  		if (err)
>  			goto free_card;
>  

Also need to prevent Host Software Queue from enabling
for SDUC.  Something like:

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 8d77a49357aa..769cd8b9f49c 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1578,7 +1578,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 			goto free_card;
 	}
 
-	if (host->cqe_ops && !host->cqe_enabled) {
+	if (!mmc_card_ult_capacity(card) && host->cqe_ops && !host->cqe_enabled) {
 		err = host->cqe_ops->cqe_enable(host, card);
 		if (!err) {
 			host->cqe_enabled = true;

Ideally try to get testing / feedback from HSQ users though.


> diff --git a/drivers/mmc/core/sd.h b/drivers/mmc/core/sd.h
> index fe6dd46927a4..7e8beface2ca 100644
> --- a/drivers/mmc/core/sd.h
> +++ b/drivers/mmc/core/sd.h
> @@ -10,7 +10,7 @@ struct mmc_host;
>  struct mmc_card;
>  
>  int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr);
> -int mmc_sd_get_csd(struct mmc_card *card);
> +int mmc_sd_get_csd(struct mmc_card *card, bool is_sduc);
>  void mmc_decode_cid(struct mmc_card *card);
>  int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
>  	bool reinit);
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 4fb247fde5c0..9566837c9848 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -769,7 +769,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
>  	 * Read CSD, before selecting the card
>  	 */
>  	if (!oldcard && mmc_card_sd_combo(card)) {
> -		err = mmc_sd_get_csd(card);
> +		err = mmc_sd_get_csd(card, false);
>  		if (err)
>  			goto remove;
>  
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index f34407cc2788..f39bce322365 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -35,7 +35,7 @@ struct mmc_csd {
>  	unsigned int		wp_grp_size;
>  	unsigned int		read_blkbits;
>  	unsigned int		write_blkbits;
> -	unsigned int		capacity;
> +	sector_t		capacity;
>  	unsigned int		read_partial:1,
>  				read_misalign:1,
>  				write_partial:1,
> diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
> index 6727576a8755..865cc0ca8543 100644
> --- a/include/linux/mmc/sd.h
> +++ b/include/linux/mmc/sd.h
> @@ -36,6 +36,7 @@
>  /* OCR bit definitions */
>  #define SD_OCR_S18R		(1 << 24)    /* 1.8V switching request */
>  #define SD_ROCR_S18A		SD_OCR_S18R  /* 1.8V switching accepted by card */
> +#define SD_OCR_2T		(1 << 27)    /* HO2T/CO2T - SDUC support */
>  #define SD_OCR_XPC		(1 << 28)    /* SDXC power control */
>  #define SD_OCR_CCS		(1 << 30)    /* Card Capacity Status */
>  


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

* Re: [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-04 14:52 ` [PATCH v6 3/9] mmc: core: Add open-ended Ext " Avri Altman
  2024-09-04 22:13   ` Christian Loehle
@ 2024-09-06  8:02   ` Adrian Hunter
  2024-09-06  8:20     ` Avri Altman
  1 sibling, 1 reply; 28+ messages in thread
From: Adrian Hunter @ 2024-09-06  8:02 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc; +Cc: Ricky WU, Shawn Lin

On 4/09/24 17:52, Avri Altman wrote:
> For open-ended read/write - just send CMD22 before issuing the command.
> While at it, make sure that the rw command arg is properly casting the
> lower 32 bits, as it can be larger now.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/block.c | 6 +++++-
>  drivers/mmc/core/core.c  | 3 +++
>  include/linux/mmc/core.h | 5 +++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index cc7318089cf2..54469261bc25 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -226,6 +226,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
>  static int mmc_spi_err_check(struct mmc_card *card);
>  static int mmc_blk_busy_cb(void *cb_data, bool *busy);
> +static int mmc_blk_wait_for_idle(struct mmc_queue *mq, struct mmc_host *host);

Not using mmc_blk_wait_for_idle() anymore.

>  
>  static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
>  {
> @@ -1710,7 +1711,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  
>  	brq->mrq.cmd = &brq->cmd;
>  
> -	brq->cmd.arg = blk_rq_pos(req);
> +	brq->cmd.arg = blk_rq_pos(req) & 0xFFFFFFFF;

arg is 32 bits so not needed

>  	if (!mmc_card_blockaddr(card))
>  		brq->cmd.arg <<= 9;
>  	brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> @@ -1758,6 +1759,9 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>  			(do_data_tag ? (1 << 29) : 0);
>  		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
>  		brq->mrq.sbc = &brq->sbc;
> +	} else if (mmc_card_ult_capacity(card)) {

The 'else' isn't actually needed, is it?  Might as well keep sbc
and ext_addr separate in this patch.

> +		brq->cmd.ext_addr = (blk_rq_pos(req) >> 32) & 0x3F;

'& 0x3f' should not be needed. i.e. we either validate blk_rq_pos(req)
(no point) or we assume it is valid.

> +		brq->cmd.has_ext_addr = 1;

If you switch to bool, that could use 'true' instead of '1'

>  	}
>  }
>  
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index d6c819dd68ed..a0b2999684b3 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -336,6 +336,9 @@ int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>  {
>  	int err;
>  
> +	if (mrq->cmd && mrq->cmd->has_ext_addr)
> +		mmc_send_ext_addr(host, mrq->cmd->ext_addr);
> +
>  	init_completion(&mrq->cmd_completion);
>  
>  	mmc_retune_hold(host);
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index f0ac2e469b32..41c21c216584 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -76,6 +76,11 @@ struct mmc_command {
>   */
>  #define mmc_cmd_type(cmd)	((cmd)->flags & MMC_CMD_MASK)
>  
> +	/* for SDUC */
> +	u8 has_ext_addr;
> +	u8 ext_addr;
> +	u16 reserved;
> +
>  	unsigned int		retries;	/* max number of retries */
>  	int			error;		/* command error */
>  


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

* RE: [PATCH v6 1/9] mmc: sd: SDUC Support Recognition
  2024-09-06  6:39   ` Adrian Hunter
@ 2024-09-06  8:10     ` Avri Altman
  0 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-06  8:10 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson, linux-mmc@vger.kernel.org; +Cc: Ricky WU, Shawn Lin

> > @@ -841,8 +847,11 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr,
> u32 *cid, u32 *rocr)
> >        * block-addressed SDHC cards.
> >        */
> >       err = mmc_send_if_cond(host, ocr);
> > -     if (!err)
> > +     if (!err) {
> >               ocr |= SD_OCR_CCS;
> > +             /* Set HO2T as well - SDUC card won't respond otherwise */
> > +             ocr |= SD_OCR_2T;
> 
> Wouldn't that be better to leave for the last patch.
Done.

> 
> > +     }
> >
> >       /*
> >        * If the host supports one of UHS-I modes, request the card @@
> > -887,7 +896,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32
> *cid, u32 *rocr)
> >       return err;
> >  }
> >
> > -int mmc_sd_get_csd(struct mmc_card *card)
> > +int mmc_sd_get_csd(struct mmc_card *card, bool is_sduc)
> >  {
> >       int err;
> >
> > @@ -898,7 +907,7 @@ int mmc_sd_get_csd(struct mmc_card *card)
> >       if (err)
> >               return err;
> >
> > -     err = mmc_decode_csd(card);
> > +     err = mmc_decode_csd(card, is_sduc);
> >       if (err)
> >               return err;
> >
> > @@ -1453,7 +1462,7 @@ static int mmc_sd_init_card(struct mmc_host *host,
> u32 ocr,
> >       }
> >
> >       if (!oldcard) {
> > -             err = mmc_sd_get_csd(card);
> > +             err = mmc_sd_get_csd(card, false);
> >               if (err)
> >                       goto free_card;
> >
> 
> Also need to prevent Host Software Queue from enabling for SDUC.  Something
> like:
> 
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> 8d77a49357aa..769cd8b9f49c 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1578,7 +1578,7 @@ static int mmc_sd_init_card(struct mmc_host *host,
> u32 ocr,
>                         goto free_card;
>         }
> 
> -       if (host->cqe_ops && !host->cqe_enabled) {
> +       if (!mmc_card_ult_capacity(card) && host->cqe_ops &&
> + !host->cqe_enabled) {
>                 err = host->cqe_ops->cqe_enable(host, card);
>                 if (!err) {
>                         host->cqe_enabled = true;
Yeah. I was just thinking about it the other day,
And wouldn't be able to find where we read the PERFORMANCE_ENHANCE SD_STATUS b[335:331].
Will add it.  Thanks.

Thanks,
Avri

> 
> Ideally try to get testing / feedback from HSQ users though.
> 
> 
> > diff --git a/drivers/mmc/core/sd.h b/drivers/mmc/core/sd.h index
> > fe6dd46927a4..7e8beface2ca 100644
> > --- a/drivers/mmc/core/sd.h
> > +++ b/drivers/mmc/core/sd.h
> > @@ -10,7 +10,7 @@ struct mmc_host;
> >  struct mmc_card;
> >
> >  int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32
> > *rocr); -int mmc_sd_get_csd(struct mmc_card *card);
> > +int mmc_sd_get_csd(struct mmc_card *card, bool is_sduc);
> >  void mmc_decode_cid(struct mmc_card *card);  int
> > mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
> >       bool reinit);
> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index
> > 4fb247fde5c0..9566837c9848 100644
> > --- a/drivers/mmc/core/sdio.c
> > +++ b/drivers/mmc/core/sdio.c
> > @@ -769,7 +769,7 @@ static int mmc_sdio_init_card(struct mmc_host *host,
> u32 ocr,
> >        * Read CSD, before selecting the card
> >        */
> >       if (!oldcard && mmc_card_sd_combo(card)) {
> > -             err = mmc_sd_get_csd(card);
> > +             err = mmc_sd_get_csd(card, false);
> >               if (err)
> >                       goto remove;
> >
> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index
> > f34407cc2788..f39bce322365 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -35,7 +35,7 @@ struct mmc_csd {
> >       unsigned int            wp_grp_size;
> >       unsigned int            read_blkbits;
> >       unsigned int            write_blkbits;
> > -     unsigned int            capacity;
> > +     sector_t                capacity;
> >       unsigned int            read_partial:1,
> >                               read_misalign:1,
> >                               write_partial:1, diff --git
> > a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h index
> > 6727576a8755..865cc0ca8543 100644
> > --- a/include/linux/mmc/sd.h
> > +++ b/include/linux/mmc/sd.h
> > @@ -36,6 +36,7 @@
> >  /* OCR bit definitions */
> >  #define SD_OCR_S18R          (1 << 24)    /* 1.8V switching request */
> >  #define SD_ROCR_S18A         SD_OCR_S18R  /* 1.8V switching accepted by
> card */
> > +#define SD_OCR_2T            (1 << 27)    /* HO2T/CO2T - SDUC support */
> >  #define SD_OCR_XPC           (1 << 28)    /* SDXC power control */
> >  #define SD_OCR_CCS           (1 << 30)    /* Card Capacity Status */
> >


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

* RE: [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-06  8:02   ` Adrian Hunter
@ 2024-09-06  8:20     ` Avri Altman
  2024-09-06  8:33       ` Adrian Hunter
  0 siblings, 1 reply; 28+ messages in thread
From: Avri Altman @ 2024-09-06  8:20 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson, linux-mmc@vger.kernel.org; +Cc: Ricky WU, Shawn Lin

> 
> On 4/09/24 17:52, Avri Altman wrote:
> > For open-ended read/write - just send CMD22 before issuing the command.
> > While at it, make sure that the rw command arg is properly casting the
> > lower 32 bits, as it can be larger now.
> >
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > ---
> >  drivers/mmc/core/block.c | 6 +++++-
> >  drivers/mmc/core/core.c  | 3 +++
> >  include/linux/mmc/core.h | 5 +++++
> >  3 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> > cc7318089cf2..54469261bc25 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -226,6 +226,7 @@ static void mmc_blk_rw_rq_prep(struct
> > mmc_queue_req *mqrq,  static void mmc_blk_hsq_req_done(struct
> > mmc_request *mrq);  static int mmc_spi_err_check(struct mmc_card
> > *card);  static int mmc_blk_busy_cb(void *cb_data, bool *busy);
> > +static int mmc_blk_wait_for_idle(struct mmc_queue *mq, struct
> > +mmc_host *host);
> 
> Not using mmc_blk_wait_for_idle() anymore.
Done.

> 
> >
> >  static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)  { @@
> > -1710,7 +1711,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req
> > *mqrq,
> >
> >       brq->mrq.cmd = &brq->cmd;
> >
> > -     brq->cmd.arg = blk_rq_pos(req);
> > +     brq->cmd.arg = blk_rq_pos(req) & 0xFFFFFFFF;
> 
> arg is 32 bits so not needed
Done.

> 
> >       if (!mmc_card_blockaddr(card))
> >               brq->cmd.arg <<= 9;
> >       brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> @@
> > -1758,6 +1759,9 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req
> *mqrq,
> >                       (do_data_tag ? (1 << 29) : 0);
> >               brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> >               brq->mrq.sbc = &brq->sbc;
> > +     } else if (mmc_card_ult_capacity(card)) {
> 
> The 'else' isn't actually needed, is it?  Might as well keep sbc and ext_addr
> separate in this patch.
Sorry - I don't follow what you mean.
Doesn't the else implies open-ended?

> 
> > +             brq->cmd.ext_addr = (blk_rq_pos(req) >> 32) & 0x3F;
> 
> '& 0x3f' should not be needed. i.e. we either validate blk_rq_pos(req) (no point)
> or we assume it is valid.
Done.

> 
> > +             brq->cmd.has_ext_addr = 1;
> 
> If you switch to bool, that could use 'true' instead of '1'
Done.

Thanks,
Avri

> 
> >       }
> >  }
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > d6c819dd68ed..a0b2999684b3 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -336,6 +336,9 @@ int mmc_start_request(struct mmc_host *host,
> > struct mmc_request *mrq)  {
> >       int err;
> >
> > +     if (mrq->cmd && mrq->cmd->has_ext_addr)
> > +             mmc_send_ext_addr(host, mrq->cmd->ext_addr);
> > +
> >       init_completion(&mrq->cmd_completion);
> >
> >       mmc_retune_hold(host);
> > diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
> > f0ac2e469b32..41c21c216584 100644
> > --- a/include/linux/mmc/core.h
> > +++ b/include/linux/mmc/core.h
> > @@ -76,6 +76,11 @@ struct mmc_command {
> >   */
> >  #define mmc_cmd_type(cmd)    ((cmd)->flags & MMC_CMD_MASK)
> >
> > +     /* for SDUC */
> > +     u8 has_ext_addr;
> > +     u8 ext_addr;
> > +     u16 reserved;
> > +
> >       unsigned int            retries;        /* max number of retries */
> >       int                     error;          /* command error */
> >


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

* Re: [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-06  8:20     ` Avri Altman
@ 2024-09-06  8:33       ` Adrian Hunter
  2024-09-06 10:11         ` Avri Altman
  0 siblings, 1 reply; 28+ messages in thread
From: Adrian Hunter @ 2024-09-06  8:33 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc@vger.kernel.org; +Cc: Ricky WU, Shawn Lin

On 6/09/24 11:20, Avri Altman wrote:
>>
>> On 4/09/24 17:52, Avri Altman wrote:
>>> For open-ended read/write - just send CMD22 before issuing the command.
>>> While at it, make sure that the rw command arg is properly casting the
>>> lower 32 bits, as it can be larger now.
>>>
>>> Signed-off-by: Avri Altman <avri.altman@wdc.com>
>>> ---
>>>  drivers/mmc/core/block.c | 6 +++++-
>>>  drivers/mmc/core/core.c  | 3 +++
>>>  include/linux/mmc/core.h | 5 +++++
>>>  3 files changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
>>> cc7318089cf2..54469261bc25 100644
>>> --- a/drivers/mmc/core/block.c
>>> +++ b/drivers/mmc/core/block.c
>>> @@ -226,6 +226,7 @@ static void mmc_blk_rw_rq_prep(struct
>>> mmc_queue_req *mqrq,  static void mmc_blk_hsq_req_done(struct
>>> mmc_request *mrq);  static int mmc_spi_err_check(struct mmc_card
>>> *card);  static int mmc_blk_busy_cb(void *cb_data, bool *busy);
>>> +static int mmc_blk_wait_for_idle(struct mmc_queue *mq, struct
>>> +mmc_host *host);
>>
>> Not using mmc_blk_wait_for_idle() anymore.
> Done.
> 
>>
>>>
>>>  static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)  { @@
>>> -1710,7 +1711,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req
>>> *mqrq,
>>>
>>>       brq->mrq.cmd = &brq->cmd;
>>>
>>> -     brq->cmd.arg = blk_rq_pos(req);
>>> +     brq->cmd.arg = blk_rq_pos(req) & 0xFFFFFFFF;
>>
>> arg is 32 bits so not needed
> Done.
> 
>>
>>>       if (!mmc_card_blockaddr(card))
>>>               brq->cmd.arg <<= 9;
>>>       brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>> @@
>>> -1758,6 +1759,9 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req
>> *mqrq,
>>>                       (do_data_tag ? (1 << 29) : 0);
>>>               brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
>>>               brq->mrq.sbc = &brq->sbc;
>>> +     } else if (mmc_card_ult_capacity(card)) {
>>
>> The 'else' isn't actually needed, is it?  Might as well keep sbc and ext_addr
>> separate in this patch.
> Sorry - I don't follow what you mean.
> Doesn't the else implies open-ended?

Disallowing SDUC with close-ended seems like a separate
issue.  The 'else' is not actually required, is it?

> 
>>
>>> +             brq->cmd.ext_addr = (blk_rq_pos(req) >> 32) & 0x3F;
>>
>> '& 0x3f' should not be needed. i.e. we either validate blk_rq_pos(req) (no point)
>> or we assume it is valid.
> Done.
> 
>>
>>> +             brq->cmd.has_ext_addr = 1;
>>
>> If you switch to bool, that could use 'true' instead of '1'
> Done.
> 
> Thanks,
> Avri
> 
>>
>>>       }
>>>  }
>>>
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
>>> d6c819dd68ed..a0b2999684b3 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -336,6 +336,9 @@ int mmc_start_request(struct mmc_host *host,
>>> struct mmc_request *mrq)  {
>>>       int err;
>>>
>>> +     if (mrq->cmd && mrq->cmd->has_ext_addr)
>>> +             mmc_send_ext_addr(host, mrq->cmd->ext_addr);
>>> +
>>>       init_completion(&mrq->cmd_completion);
>>>
>>>       mmc_retune_hold(host);
>>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
>>> f0ac2e469b32..41c21c216584 100644
>>> --- a/include/linux/mmc/core.h
>>> +++ b/include/linux/mmc/core.h
>>> @@ -76,6 +76,11 @@ struct mmc_command {
>>>   */
>>>  #define mmc_cmd_type(cmd)    ((cmd)->flags & MMC_CMD_MASK)
>>>
>>> +     /* for SDUC */
>>> +     u8 has_ext_addr;
>>> +     u8 ext_addr;
>>> +     u16 reserved;
>>> +
>>>       unsigned int            retries;        /* max number of retries */
>>>       int                     error;          /* command error */
>>>
> 


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

* Re: [PATCH v6 6/9] mmc: core: Add Ext memory addressing for erase
  2024-09-04 14:52 ` [PATCH v6 6/9] mmc: core: Add Ext memory addressing for erase Avri Altman
@ 2024-09-06 10:06   ` Adrian Hunter
  2024-09-06 10:14     ` Avri Altman
  0 siblings, 1 reply; 28+ messages in thread
From: Adrian Hunter @ 2024-09-06 10:06 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc; +Cc: Ricky WU, Shawn Lin

On 4/09/24 17:52, Avri Altman wrote:
> CMD22 shall precede CMD32 and CMD33 to configure 38-bit erase start
> address and 38 bit erase stop address.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/core.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 06f63fbaadfb..8d9f2c44d2a2 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1645,8 +1645,14 @@ static int mmc_do_erase(struct mmc_card *card, sector_t from,
>  		cmd.opcode = SD_ERASE_WR_BLK_START;
>  	else
>  		cmd.opcode = MMC_ERASE_GROUP_START;
> -	cmd.arg = from;
> +	cmd.arg = from & 0xFFFFFFFF;

Not needed

>  	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
> +
> +	if (mmc_card_ult_capacity(card)) {
> +		cmd.ext_addr = (from >> 32) & 0x3F;

'0x3F' not needed


> +		cmd.has_ext_addr = 1;

'true' if use bool

> +	}
> +
>  	err = mmc_wait_for_cmd(card->host, &cmd, 0);
>  	if (err) {
>  		pr_err("mmc_erase: group start error %d, "
> @@ -1660,8 +1666,14 @@ static int mmc_do_erase(struct mmc_card *card, sector_t from,
>  		cmd.opcode = SD_ERASE_WR_BLK_END;
>  	else
>  		cmd.opcode = MMC_ERASE_GROUP_END;
> -	cmd.arg = to;
> +	cmd.arg = to & 0xFFFFFFFF;

Not needed

>  	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
> +
> +	if (mmc_card_ult_capacity(card)) {
> +		cmd.ext_addr = (to >> 32) & 0x3F;

'0x3F' not needed

> +		cmd.has_ext_addr = 1;

'true' if use bool

> +	}
> +
>  	err = mmc_wait_for_cmd(card->host, &cmd, 0);
>  	if (err) {
>  		pr_err("mmc_erase: group end error %d, status %#x\n",


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

* Re: [PATCH v6 8/9] mmc: core: Disable SDUC for mmc_test
  2024-09-04 14:52 ` [PATCH v6 8/9] mmc: core: Disable SDUC for mmc_test Avri Altman
@ 2024-09-06 10:10   ` Adrian Hunter
  2024-09-06 10:15     ` Avri Altman
  0 siblings, 1 reply; 28+ messages in thread
From: Adrian Hunter @ 2024-09-06 10:10 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc; +Cc: Ricky WU, Shawn Lin

On 4/09/24 17:52, Avri Altman wrote:
> Panning to ameliorate it in the very near future.

Panning -> Planning

> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/mmc_test.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
> index b7f627a9fdea..a28e1a1aded3 100644
> --- a/drivers/mmc/core/mmc_test.c
> +++ b/drivers/mmc/core/mmc_test.c
> @@ -3218,6 +3218,13 @@ static int mmc_test_register_dbgfs_file(struct mmc_card *card)
>  
>  	mutex_lock(&mmc_test_lock);
>  
> +	if (mmc_card_ult_capacity(card)) {
> +		pr_info("%s: mmc-test currently UNSUPPORTED for SDUC\n",
> +			mmc_hostname(card->host));
> +		ret = -EOPNOTSUPP;
> +		goto err;
> +	}

Probably simpler to put the check in mmc_test_probe()

> +
>  	ret = __mmc_test_register_dbgfs_file(card, "test", S_IWUSR | S_IRUGO,
>  		&mmc_test_fops_test);
>  	if (ret)


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

* RE: [PATCH v6 3/9] mmc: core: Add open-ended Ext memory addressing
  2024-09-06  8:33       ` Adrian Hunter
@ 2024-09-06 10:11         ` Avri Altman
  0 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-06 10:11 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson, linux-mmc@vger.kernel.org; +Cc: Ricky WU, Shawn Lin

> 
> Disallowing SDUC with close-ended seems like a separate issue.  The 'else' is not
> actually required, is it?
Ahha - ok.
I need then to switch patches 3 & 4.

Thanks,
Avri

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

* RE: [PATCH v6 6/9] mmc: core: Add Ext memory addressing for erase
  2024-09-06 10:06   ` Adrian Hunter
@ 2024-09-06 10:14     ` Avri Altman
  0 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-06 10:14 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson, linux-mmc@vger.kernel.org; +Cc: Ricky WU, Shawn Lin

> On 4/09/24 17:52, Avri Altman wrote:
> > CMD22 shall precede CMD32 and CMD33 to configure 38-bit erase start
> > address and 38 bit erase stop address.
> >
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > ---
> >  drivers/mmc/core/core.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > 06f63fbaadfb..8d9f2c44d2a2 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -1645,8 +1645,14 @@ static int mmc_do_erase(struct mmc_card *card,
> sector_t from,
> >               cmd.opcode = SD_ERASE_WR_BLK_START;
> >       else
> >               cmd.opcode = MMC_ERASE_GROUP_START;
> > -     cmd.arg = from;
> > +     cmd.arg = from & 0xFFFFFFFF;
> 
> Not needed
Done.

> 
> >       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
> > +
> > +     if (mmc_card_ult_capacity(card)) {
> > +             cmd.ext_addr = (from >> 32) & 0x3F;
> 
> '0x3F' not needed
Done.

> 
> 
> > +             cmd.has_ext_addr = 1;
> 
> 'true' if use bool
Done.

> 
> > +     }
> > +
> >       err = mmc_wait_for_cmd(card->host, &cmd, 0);
> >       if (err) {
> >               pr_err("mmc_erase: group start error %d, "
> > @@ -1660,8 +1666,14 @@ static int mmc_do_erase(struct mmc_card *card,
> sector_t from,
> >               cmd.opcode = SD_ERASE_WR_BLK_END;
> >       else
> >               cmd.opcode = MMC_ERASE_GROUP_END;
> > -     cmd.arg = to;
> > +     cmd.arg = to & 0xFFFFFFFF;
> 
> Not needed
Done.

> 
> >       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
> > +
> > +     if (mmc_card_ult_capacity(card)) {
> > +             cmd.ext_addr = (to >> 32) & 0x3F;
> 
> '0x3F' not needed
Done.

> 
> > +             cmd.has_ext_addr = 1;
> 
> 'true' if use bool
Done.

Thanks,
Avri
> 
> > +     }
> > +
> >       err = mmc_wait_for_cmd(card->host, &cmd, 0);
> >       if (err) {
> >               pr_err("mmc_erase: group end error %d, status %#x\n",


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

* RE: [PATCH v6 8/9] mmc: core: Disable SDUC for mmc_test
  2024-09-06 10:10   ` Adrian Hunter
@ 2024-09-06 10:15     ` Avri Altman
  0 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-06 10:15 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson, linux-mmc@vger.kernel.org; +Cc: Ricky WU, Shawn Lin

> On 4/09/24 17:52, Avri Altman wrote:
> > Panning to ameliorate it in the very near future.
> 
> Panning -> Planning
Done.

> 
> >
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > ---
> >  drivers/mmc/core/mmc_test.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
> > index b7f627a9fdea..a28e1a1aded3 100644
> > --- a/drivers/mmc/core/mmc_test.c
> > +++ b/drivers/mmc/core/mmc_test.c
> > @@ -3218,6 +3218,13 @@ static int mmc_test_register_dbgfs_file(struct
> > mmc_card *card)
> >
> >       mutex_lock(&mmc_test_lock);
> >
> > +     if (mmc_card_ult_capacity(card)) {
> > +             pr_info("%s: mmc-test currently UNSUPPORTED for SDUC\n",
> > +                     mmc_hostname(card->host));
> > +             ret = -EOPNOTSUPP;
> > +             goto err;
> > +     }
> 
> Probably simpler to put the check in mmc_test_probe()
Done.

> 
> > +
> >       ret = __mmc_test_register_dbgfs_file(card, "test", S_IWUSR | S_IRUGO,
> >               &mmc_test_fops_test);
> >       if (ret)


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

* Re: [PATCH v6 7/9] mmc: core: Adjust ACMD22 to SDUC
  2024-09-04 14:52 ` [PATCH v6 7/9] mmc: core: Adjust ACMD22 to SDUC Avri Altman
@ 2024-09-06 10:57   ` Adrian Hunter
  2024-09-06 11:07     ` Avri Altman
  0 siblings, 1 reply; 28+ messages in thread
From: Adrian Hunter @ 2024-09-06 10:57 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc; +Cc: Ricky WU, Shawn Lin

On 4/09/24 17:52, Avri Altman wrote:
> ACMD22 is used to verify the previously write operation.  Normally, it
> returns the number of written sectors as u32.  SDUC, however, returns it
> as u64.  This is not a superfluous requirement, because SDUC writes may
> exceeds 2TB.  For Linux mmc however, the previously write operation
> could not be more than the block layer limits, thus we make room for a
> u64 and cast the returning value to u32.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/block.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 50d37c4f5a50..f36611512a1d 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -50,6 +50,7 @@
>  #include <linux/mmc/sd.h>
>  
>  #include <linux/uaccess.h>
> +#include <asm/unaligned.h>
>  
>  #include "queue.h"
>  #include "block.h"
> @@ -994,11 +995,10 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>  	int err;
>  	u32 result;
>  	__be32 *blocks;
> -
> +	u8 resp_sz;

Could do the initialization here i.e.

	u8 resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;

>  	struct mmc_request mrq = {};
>  	struct mmc_command cmd = {};
>  	struct mmc_data data = {};
> -
>  	struct scatterlist sg;
>  
>  	err = mmc_app_cmd(card->host, card);
> @@ -1009,7 +1009,14 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>  	cmd.arg = 0;
>  	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>  
> -	data.blksz = 4;
> +	/*
> +	 * Normally, ACMD22 returns the number of written sectors as u32.
> +	 * SDUC, however, returns it as u64.  This is not a superfluous
> +	 * requirement, because SDUC writes may exceed 2TB.
> +	 */
> +	resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
> +
> +	data.blksz = resp_sz;
>  	data.blocks = 1;
>  	data.flags = MMC_DATA_READ;
>  	data.sg = &sg;
> @@ -1019,15 +1026,25 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>  	mrq.cmd = &cmd;
>  	mrq.data = &data;
>  
> -	blocks = kmalloc(4, GFP_KERNEL);
> +	blocks = kmalloc(resp_sz, GFP_KERNEL);

Separate issue, but this should probably be GFP_NOIO, or
just have the buffer in mmc_host or mmc_card

>  	if (!blocks)
>  		return -ENOMEM;
>  
> -	sg_init_one(&sg, blocks, 4);
> +	sg_init_one(&sg, blocks, resp_sz);
>  
>  	mmc_wait_for_req(card->host, &mrq);
>  
> -	result = ntohl(*blocks);
> +	if (mmc_card_ult_capacity(card)) {
> +		u64 blocks_64 = get_unaligned_be64(blocks);
> +		/*
> +		 * For Linux mmc however, the previously write operation could
> +		 * not be more than the block layer limits, thus just make room
> +		 * for a u64 and cast the response back to u32.
> +		 */
> +		result = blocks_64 > UINT_MAX ? UINT_MAX : (u32)blocks_64;

Perhaps:
		result = clamp_val(get_unaligned_be64(blocks), 0, UINT_MAX);

> +	} else {
> +		result = ntohl(*blocks);
> +	}
>  	kfree(blocks);
>  
>  	if (cmd.error || data.error)


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

* RE: [PATCH v6 7/9] mmc: core: Adjust ACMD22 to SDUC
  2024-09-06 10:57   ` Adrian Hunter
@ 2024-09-06 11:07     ` Avri Altman
  0 siblings, 0 replies; 28+ messages in thread
From: Avri Altman @ 2024-09-06 11:07 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson, linux-mmc@vger.kernel.org; +Cc: Ricky WU, Shawn Lin

> On 4/09/24 17:52, Avri Altman wrote:
> > ACMD22 is used to verify the previously write operation.  Normally, it
> > returns the number of written sectors as u32.  SDUC, however, returns
> > it as u64.  This is not a superfluous requirement, because SDUC writes
> > may exceeds 2TB.  For Linux mmc however, the previously write
> > operation could not be more than the block layer limits, thus we make
> > room for a
> > u64 and cast the returning value to u32.
> >
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > ---
> >  drivers/mmc/core/block.c | 29 +++++++++++++++++++++++------
> >  1 file changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> > 50d37c4f5a50..f36611512a1d 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -50,6 +50,7 @@
> >  #include <linux/mmc/sd.h>
> >
> >  #include <linux/uaccess.h>
> > +#include <asm/unaligned.h>
> >
> >  #include "queue.h"
> >  #include "block.h"
> > @@ -994,11 +995,10 @@ static int mmc_sd_num_wr_blocks(struct mmc_card
> *card, u32 *written_blocks)
> >       int err;
> >       u32 result;
> >       __be32 *blocks;
> > -
> > +     u8 resp_sz;
> 
> Could do the initialization here i.e.
> 
>         u8 resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
Done.

> 
> >       struct mmc_request mrq = {};
> >       struct mmc_command cmd = {};
> >       struct mmc_data data = {};
> > -
> >       struct scatterlist sg;
> >
> >       err = mmc_app_cmd(card->host, card); @@ -1009,7 +1009,14 @@
> > static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32
> *written_blocks)
> >       cmd.arg = 0;
> >       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> >
> > -     data.blksz = 4;
> > +     /*
> > +      * Normally, ACMD22 returns the number of written sectors as u32.
> > +      * SDUC, however, returns it as u64.  This is not a superfluous
> > +      * requirement, because SDUC writes may exceed 2TB.
> > +      */
> > +     resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
> > +
> > +     data.blksz = resp_sz;
> >       data.blocks = 1;
> >       data.flags = MMC_DATA_READ;
> >       data.sg = &sg;
> > @@ -1019,15 +1026,25 @@ static int mmc_sd_num_wr_blocks(struct
> mmc_card *card, u32 *written_blocks)
> >       mrq.cmd = &cmd;
> >       mrq.data = &data;
> >
> > -     blocks = kmalloc(4, GFP_KERNEL);
> > +     blocks = kmalloc(resp_sz, GFP_KERNEL);
> 
> Separate issue, but this should probably be GFP_NOIO, or just have the buffer in
> mmc_host or mmc_card
OK.  Mark this to be fixed in a subsequent patch.

> 
> >       if (!blocks)
> >               return -ENOMEM;
> >
> > -     sg_init_one(&sg, blocks, 4);
> > +     sg_init_one(&sg, blocks, resp_sz);
> >
> >       mmc_wait_for_req(card->host, &mrq);
> >
> > -     result = ntohl(*blocks);
> > +     if (mmc_card_ult_capacity(card)) {
> > +             u64 blocks_64 = get_unaligned_be64(blocks);
> > +             /*
> > +              * For Linux mmc however, the previously write operation could
> > +              * not be more than the block layer limits, thus just make room
> > +              * for a u64 and cast the response back to u32.
> > +              */
> > +             result = blocks_64 > UINT_MAX ? UINT_MAX :
> > + (u32)blocks_64;
> 
> Perhaps:
>                 result = clamp_val(get_unaligned_be64(blocks), 0, UINT_MAX);
Done.

Thanks,
Avri
> 
> > +     } else {
> > +             result = ntohl(*blocks);
> > +     }
> >       kfree(blocks);
> >
> >       if (cmd.error || data.error)


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

* Re: [PATCH v6 0/9] Add SDUC Support
  2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
                   ` (8 preceding siblings ...)
  2024-09-04 14:52 ` [PATCH v6 9/9] mmc: core: Enable SDUC Avri Altman
@ 2024-09-06 11:10 ` Adrian Hunter
  9 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2024-09-06 11:10 UTC (permalink / raw)
  To: Avri Altman, Ulf Hansson, linux-mmc; +Cc: Ricky WU, Shawn Lin

On 4/09/24 17:52, Avri Altman wrote:
> Ultra Capacity SD cards (SDUC) was already introduced in SD7.0.  Those
> cards support capacity larger than 2TB and up to including 128TB. Thus,
> the address range of the card expands beyond the 32-bit command
> argument. To that end, a new command - CMD22 is defined, to carry the
> extra 6-bit upper part of the 38-bit block address that enable access to
> 128TB memory space.
> 
> SDUC capacity is agnostic to the interface mode: UHS-I and UHS-II – Same
> as SDXC.
> 
> The spec defines several extensions/modifications to the current SDXC
> cards, which we address in patches 1 - 10.  Otherwise requirements are
> out-of-scope of this change.  Specifically, CMDQ (CMD44+CMD45), and
> Extension for Video Speed Class (CMD20).
> 
> First publication of SDUC was in [1].  This series was developed and
> tested separately from [1] and does not borrow from it.
> 
> [1] https://lwn.net/Articles/982566/
> 
> ---
> Changes in v6:
>  - Remove Ricky's tested-by tag - the series has changed greatly
>  - Call mmc_send_ext_addr from mmc_start_request (Adrian)
> 
> Changes in v5:
>  - leave out the mask in mmc_send_ext_addr (Adrian)
>  - leave out close-ended SDUC support
>  - remove 500msec write delay as there is no busy indication (Adrian)
>  - disable mmc-test for SDUC
>  - move enabling SDUC to the last patch (Adrian)
> 
> Changes in v4:
>  - Squash patches 1 & 2 (Ulf)
>  - Amend SD_OCR_2T to SD_OCR_CCS in mmc_sd_get_cid (Ulf)
>  - Use card state instead of caps2 (Ricky & Ulf)
>  - Switch patches 5 & 6 (Ulf)
> 
> Changes in v3:
>  - Some more kernel test robot fixes
>  - Fix a typo in a commit log (Ricky WU)
>  - Fix ACMD22 returned value
>  - Add 'Tested-by' tag for the whole series (Ricky WU)
> 
> Changes in v2:
>  - Attend kernel test robot warnings
> 
> ---
> 
> Avri Altman (9):
>   mmc: sd: SDUC Support Recognition
>   mmc: sd: Add Extension memory addressing
>   mmc: core: Add open-ended Ext memory addressing
>   mmc: core: Don't use close-ended rw for SDUC
>   mmc: core: Allow mmc erase to carry large addresses
>   mmc: core: Add Ext memory addressing for erase
>   mmc: core: Adjust ACMD22 to SDUC
>   mmc: core: Disable SDUC for mmc_test
>   mmc: core: Enable SDUC
> 
>  drivers/mmc/core/block.c    | 43 +++++++++++++++++++++++-------
>  drivers/mmc/core/bus.c      |  4 ++-
>  drivers/mmc/core/card.h     |  3 +++
>  drivers/mmc/core/core.c     | 52 +++++++++++++++++++++++++------------
>  drivers/mmc/core/core.h     | 16 +++++++++---
>  drivers/mmc/core/mmc_test.c |  7 +++++
>  drivers/mmc/core/sd.c       | 36 ++++++++++++++++---------
>  drivers/mmc/core/sd.h       |  2 +-
>  drivers/mmc/core/sd_ops.c   | 16 ++++++++++++
>  drivers/mmc/core/sd_ops.h   |  1 +
>  drivers/mmc/core/sdio.c     |  2 +-
>  include/linux/mmc/card.h    |  2 +-
>  include/linux/mmc/core.h    |  5 ++++
>  include/linux/mmc/sd.h      |  4 +++
>  14 files changed, 146 insertions(+), 47 deletions(-)
> 

I made a few comments, but this looks good
otherwise.


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

end of thread, other threads:[~2024-09-06 11:10 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-04 14:52 [PATCH v6 0/9] Add SDUC Support Avri Altman
2024-09-04 14:52 ` [PATCH v6 1/9] mmc: sd: SDUC Support Recognition Avri Altman
2024-09-06  6:39   ` Adrian Hunter
2024-09-06  8:10     ` Avri Altman
2024-09-04 14:52 ` [PATCH v6 2/9] mmc: sd: Add Extension memory addressing Avri Altman
2024-09-04 14:52 ` [PATCH v6 3/9] mmc: core: Add open-ended Ext " Avri Altman
2024-09-04 22:13   ` Christian Loehle
2024-09-05  6:12     ` Avri Altman
2024-09-05  7:25       ` Adrian Hunter
2024-09-05  8:27       ` Christian Loehle
2024-09-05  9:47         ` Avri Altman
2024-09-06  8:02   ` Adrian Hunter
2024-09-06  8:20     ` Avri Altman
2024-09-06  8:33       ` Adrian Hunter
2024-09-06 10:11         ` Avri Altman
2024-09-04 14:52 ` [PATCH v6 4/9] mmc: core: Don't use close-ended rw for SDUC Avri Altman
2024-09-04 14:52 ` [PATCH v6 5/9] mmc: core: Allow mmc erase to carry large addresses Avri Altman
2024-09-04 14:52 ` [PATCH v6 6/9] mmc: core: Add Ext memory addressing for erase Avri Altman
2024-09-06 10:06   ` Adrian Hunter
2024-09-06 10:14     ` Avri Altman
2024-09-04 14:52 ` [PATCH v6 7/9] mmc: core: Adjust ACMD22 to SDUC Avri Altman
2024-09-06 10:57   ` Adrian Hunter
2024-09-06 11:07     ` Avri Altman
2024-09-04 14:52 ` [PATCH v6 8/9] mmc: core: Disable SDUC for mmc_test Avri Altman
2024-09-06 10:10   ` Adrian Hunter
2024-09-06 10:15     ` Avri Altman
2024-09-04 14:52 ` [PATCH v6 9/9] mmc: core: Enable SDUC Avri Altman
2024-09-06 11:10 ` [PATCH v6 0/9] Add SDUC Support Adrian Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox