[PATCH 0/6] mci: some cleanups

[PATCH 0/6] mci: some cleanups

[Sascha Hauer]

This cleans up some stuff in the mci layer and its drivers.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (6):
      mci: use struct cid
      mmc: merge block read/write functions
      mci: cleanup code around ready_for_use flag
      mci: mci_spi: remove stray return 0
      mci: mci_spi: fix warning message
      mci: mci_spi: use mci_of_parse()

 drivers/mci/mci-core.c | 488 ++++++++++++++++++++-----------------------------
 drivers/mci/mci_spi.c  |   7 +-
 include/mci.h          |  18 +-
 3 files changed, 216 insertions(+), 297 deletions(-)
---
base-commit: 2afd1a809f1a41f1dd42b95c2bc0ae74853b475b
change-id: 20250317-mci-misc-cleanup-2362d1efd1ea

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH 1/6] mci: use struct cid

[Sascha Hauer]

Linux has a struct mmc_cid where the CID data is parsed into whereas
in barebox we call the UNSTUFF_BITS macro whenever we need a field
from the CID data. Do it like Linux and parse the CID data into the
same struct. While at it convert the UNSTUFF_BITS macro into a
unstuff_bits static inline function.

This also changes some names of the mci device parameters:

- cid_mdt is now split up into year and month and becomes cid_year and
  cid_month
- cid_prv is now split up into hardware revision and firmware revision
  and becomes cid_hwrev and cid_fwrev

This is done as Linux has this as well. It is assumed that these
variables are informational only and thus no scripts depend on the
exact names.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 340 ++++++++++++++++++++-----------------------------
 include/mci.h          |  16 ++-
 2 files changed, 154 insertions(+), 202 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 7ec2643b8d..34ea775813 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -26,19 +26,19 @@
 
 #define MAX_BUFFER_NUMBER 0xffffffff
 
-#define UNSTUFF_BITS(resp,start,size)					\
-	({								\
-		const int __size = size;				\
-		const u32 __mask = (__size < 32 ? 1 << __size : 0) - 1;	\
-		const int __off = 3 - ((start) / 32);			\
-		const int __shft = (start) & 31;			\
-		u32 __res;						\
-									\
-		__res = resp[__off] >> __shft;				\
-		if (__size + __shft > 32)				\
-			__res |= resp[__off-1] << ((32 - __shft) % 32);	\
-		__res & __mask;						\
-	})
+static inline u32 unstuff_bits(const u32 *resp, int start, int size)
+{
+	const int __size = size;
+	const u32 __mask = (__size < 32 ? 1 << __size : 0) - 1;
+	const int __off = 3 - (start / 32);
+	const int __shft = start & 31;
+	u32 __res = resp[__off] >> __shft;
+
+	if (__size + __shft > 32)
+		__res |= resp[__off - 1] << ((32 - __shft) % 32);
+
+	return __res & __mask;
+}
 
 LIST_HEAD(mci_list);
 
@@ -1202,7 +1202,7 @@ static void mci_extract_max_tran_speed_from_csd(struct mci *mci)
  */
 static void mci_extract_block_lengths_from_csd(struct mci *mci)
 {
-	mci->read_bl_len = 1 << UNSTUFF_BITS(mci->csd, 80, 4);
+	mci->read_bl_len = 1 << unstuff_bits(mci->csd, 80, 4);
 
 	/* Quoting Physical Layer Simplified Specification Version 9.10:
 	 * Note that in an SD Memory Card the WRITE_BL_LEN is always
@@ -1224,18 +1224,18 @@ static void mci_extract_block_lengths_from_csd(struct mci *mci)
 static void mci_extract_erase_group_size(struct mci *mci)
 {
 	if (!IS_ENABLED(CONFIG_MCI_ERASE) ||
-	    !(UNSTUFF_BITS(mci->csd, 84, 12) & CCC_ERASE))
+	    !(unstuff_bits(mci->csd, 84, 12) & CCC_ERASE))
 		return;
 
 
 	if (IS_SD(mci)) {
-		if (UNSTUFF_BITS(mci->csd, 126, 2) == 0) {
-			unsigned int write_blkbits = UNSTUFF_BITS(mci->csd, 22, 4);
+		if (unstuff_bits(mci->csd, 126, 2) == 0) {
+			unsigned int write_blkbits = unstuff_bits(mci->csd, 22, 4);
 
-			if (UNSTUFF_BITS(mci->csd, 46, 1)) {
+			if (unstuff_bits(mci->csd, 46, 1)) {
 				mci->erase_grp_size = 1;
 			} else if (write_blkbits >= 9) {
-				mci->erase_grp_size = UNSTUFF_BITS(mci->csd, 39, 7) + 1;
+				mci->erase_grp_size = unstuff_bits(mci->csd, 39, 7) + 1;
 				mci->erase_grp_size <<= write_blkbits - 9;
 			}
 		} else {
@@ -1279,7 +1279,7 @@ static void mci_extract_card_capacity_from_csd(struct mci *mci)
 
 	if (mci->high_capacity) {
 		if (IS_SD(mci)) {
-			csize = UNSTUFF_BITS(mci->csd, 48, 22);
+			csize = unstuff_bits(mci->csd, 48, 22);
 			mci->capacity = (1 + csize) << 10;
 		} else {
 			mci->capacity = mci->ext_csd[EXT_CSD_SEC_COUNT] << 0 |
@@ -1288,12 +1288,12 @@ static void mci_extract_card_capacity_from_csd(struct mci *mci)
 				mci->ext_csd[EXT_CSD_SEC_COUNT + 3] << 24;
 		}
 	} else {
-		cmult = UNSTUFF_BITS(mci->csd, 47, 3);
-		csize = UNSTUFF_BITS(mci->csd, 62, 12);
+		cmult = unstuff_bits(mci->csd, 47, 3);
+		csize = unstuff_bits(mci->csd, 62, 12);
 		mci->capacity = (csize + 1) << (cmult + 2);
 	}
 
-	mci->capacity *= 1 << UNSTUFF_BITS(mci->csd, 80, 4);
+	mci->capacity *= 1 << unstuff_bits(mci->csd, 80, 4);
 	dev_dbg(&mci->dev, "Capacity: %u MiB\n", (unsigned)(mci->capacity >> 20));
 }
 
@@ -1303,7 +1303,7 @@ static void mci_extract_card_capacity_from_csd(struct mci *mci)
  */
 static void mci_extract_card_dsr_imp_from_csd(struct mci *mci)
 {
-	mci->dsr_imp = UNSTUFF_BITS(mci->csd, 76, 1);
+	mci->dsr_imp = unstuff_bits(mci->csd, 76, 1);
 }
 
 static int mmc_compare_ext_csds(struct mci *mci, enum mci_bus_width bus_width)
@@ -1806,10 +1806,10 @@ static int mci_startup(struct mci *mci)
 		return err;
 	}
 
-	memcpy(mci->cid, cmd.response, 16);
+	memcpy(mci->raw_cid, cmd.response, 16);
 
 	dev_dbg(&mci->dev, "Card's identification data is: %08X-%08X-%08X-%08X\n",
-		mci->cid[0], mci->cid[1], mci->cid[2], mci->cid[3]);
+		mci->raw_cid[0], mci->raw_cid[1], mci->raw_cid[2], mci->raw_cid[3]);
 
 	/*
 	 * For MMC cards, set the Relative Address.
@@ -2259,156 +2259,6 @@ static int mci_sd_read(struct block_device *blk, void *buffer, sector_t block,
 
 /* ------------------ attach to the device API --------------------------- */
 
-/**
- * Extract the Manufacturer ID from the CID
- * @param mci Instance data
- *
- * The 'MID' is encoded in bit 127:120 in the CID
- */
-static unsigned extract_mid(struct mci *mci)
-{
-	if (!IS_SD(mci) && mci->version <= MMC_VERSION_1_4)
-		return UNSTUFF_BITS(mci->cid, 104, 24);
-	else
-		return UNSTUFF_BITS(mci->cid, 120, 8);
-}
-
-/**
- * Extract the CBX from the CID
- * @param mci Instance data
- *
- * The 'CBX' is encoded in bit 113:112 in the CID and only present in MMC cards
- */
-static unsigned extract_cbx(struct mci *mci)
-{
-	return UNSTUFF_BITS(mci->cid, 112, 2);
-}
-
-/**
- * Extract the OEM/Application ID from the CID
- * @param mci Instance data
- *
- * The 'OID' is encoded in bit 119:104 in the CID for SD cards and 111:104 for
- * MMC cards
- */
-static void extract_oid(struct mci *mci, char oid[static 5])
-{
-	if (IS_SD(mci)) {
-		// SD cards have a 2 character long OEM ID
-		snprintf(oid, 5, "%c%c", UNSTUFF_BITS(mci->cid, 112, 8), UNSTUFF_BITS(mci->cid, 104, 8));
-	} else {
-		// MMC cards have a 8-bit binary number as OEM ID
-		snprintf(oid, 5, "0x%02X", UNSTUFF_BITS(mci->cid, 104, 8));
-	}
-}
-
-/**
- * Extract the product name from the CID
- * @param mci Instance data
- *
- * The 'PNM' is encoded in bit 103:64 in the CID for SD cards and 103:56 for
- * MMC cards
- */
-static void extract_pnm(struct mci *mci, char pnm[static 7])
-{
-	pnm[0] = UNSTUFF_BITS(mci->cid, 96, 8);
-	pnm[1] = UNSTUFF_BITS(mci->cid, 88, 8);
-	pnm[2] = UNSTUFF_BITS(mci->cid, 80, 8);
-	pnm[3] = UNSTUFF_BITS(mci->cid, 72, 8);
-	pnm[4] = UNSTUFF_BITS(mci->cid, 64, 8);
-
-	if (IS_SD(mci)) {
-		// SD cards have a 5 character long product name
-		pnm[5] = '\0';
-	} else {
-		// MMC cards have a 6 character long product name
-		pnm[5] = UNSTUFF_BITS(mci->cid, 56, 8);
-		pnm[6] = '\0';
-	}
-}
-
-/**
- * Extract the product revision from the CID
- * @param mci Instance data
- *
- * The 'PRV' is encoded in bit 63:56 in the CID for SD cards and 55:48 for MMC cards
- */
-static void extract_prv(struct mci *mci, char prv[static 8])
-{
-	unsigned prv_bcd = IS_SD(mci) ? UNSTUFF_BITS(mci->cid, 56, 8) : UNSTUFF_BITS(mci->cid, 48, 8);
-
-	snprintf(prv, 8,"%u.%u", prv_bcd >> 4, prv_bcd & 0xf);
-}
-
-/**
- * Extract the product serial number from the CID
- * @param mci Instance data
- *
- * The 'PSN' is encoded in bit 55:24 in the CID for SD cards and 47:16 for MMC cards
- */
-static unsigned extract_psn(struct mci *mci)
-{
-	if (IS_SD(mci)) {
-		return UNSTUFF_BITS(mci->csd, 24, 32);
-	} else {
-		if (mci->version > MMC_VERSION_1_4)
-			return UNSTUFF_BITS(mci->cid, 16, 32);
-		else
-			return UNSTUFF_BITS(mci->cid, 16, 24);
-	}
-
-}
-
-/**
- * Extract the month of the manufacturing date from the CID
- * @param mci Instance data
- *
- * The 'MDT' is encoded in bit 19:8 in the CID, month in 11:8
- */
-static unsigned extract_mdt_month(struct mci *mci)
-{
-	if (IS_SD(mci))
-		return UNSTUFF_BITS(mci->cid, 8, 4);
-	else
-		return UNSTUFF_BITS(mci->cid, 12, 4);
-}
-
-/**
- * Extract the year of the manufacturing date from the CID
- * @param mci Instance data
- *
- * The 'MDT' is encoded in bit 19:8 in the CID, year in 19:12
- * An encoded 0 means the year 2000
- */
-static unsigned extract_mdt_year(struct mci *mci)
-{
-	unsigned year;
-	if (IS_SD(mci))
-		year = UNSTUFF_BITS(mci->cid, 12, 8) + 2000;
-	else if (mci->version < MMC_VERSION_4_41)
-		return UNSTUFF_BITS(mci->cid, 8, 4) + 1997;
-	else {
-		year = UNSTUFF_BITS(mci->cid, 8, 4) + 1997;
-		if (year < 2010)
-			year += 16;
-	}
-	return year;
-}
-
-/**
- * Extract the manufacturing date from the CID
- * @param mci Instance data
- *
- * The 'MDT' is encoded in bit 19:8 in the CID
- */
-static void extract_mdt(struct mci *mci, char mdt[static 8])
-{
-	unsigned month = extract_mdt_month(mci);
-	unsigned year = extract_mdt_year(mci);
-
-	snprintf(mdt, 8, "%u.%u", year, month);
-}
-
 static const char *mci_timing_tostr(unsigned timing)
 {
 	switch (timing) {
@@ -2440,6 +2290,88 @@ static void mci_print_caps(unsigned caps)
 		caps & MMC_CAP_MMC_1_2V_DDR ? "ddr-1.2v " : "");
 }
 
+/*
+ * Given the decoded CSD structure, decode the raw CID to our CID structure.
+ */
+static int mci_mmc_decode_cid(struct mci *card)
+{
+	u32 *resp = card->raw_cid;
+	u32 mmca_vsn = unstuff_bits(card->csd, 122, 4);
+
+	/*
+	 * The selection of the format here is based upon published
+	 * specs from sandisk and from what people have reported.
+	 */
+	switch (mmca_vsn) {
+	case 0: /* MMC v1.0 - v1.2 */
+	case 1: /* MMC v1.4 */
+		card->cid.manfid	= unstuff_bits(resp, 104, 24);
+		card->cid.prod_name[0]	= unstuff_bits(resp, 96, 8);
+		card->cid.prod_name[1]	= unstuff_bits(resp, 88, 8);
+		card->cid.prod_name[2]	= unstuff_bits(resp, 80, 8);
+		card->cid.prod_name[3]	= unstuff_bits(resp, 72, 8);
+		card->cid.prod_name[4]	= unstuff_bits(resp, 64, 8);
+		card->cid.prod_name[5]	= unstuff_bits(resp, 56, 8);
+		card->cid.prod_name[6]	= unstuff_bits(resp, 48, 8);
+		card->cid.hwrev		= unstuff_bits(resp, 44, 4);
+		card->cid.fwrev		= unstuff_bits(resp, 40, 4);
+		card->cid.serial	= unstuff_bits(resp, 16, 24);
+		card->cid.month		= unstuff_bits(resp, 12, 4);
+		card->cid.year		= unstuff_bits(resp, 8, 4) + 1997;
+		break;
+
+	case 2: /* MMC v2.0 - v2.2 */
+	case 3: /* MMC v3.1 - v3.3 */
+	case 4: /* MMC v4 */
+		card->cid.manfid	= unstuff_bits(resp, 120, 8);
+		card->cid.oemid		= unstuff_bits(resp, 104, 16);
+		card->cid.prod_name[0]	= unstuff_bits(resp, 96, 8);
+		card->cid.prod_name[1]	= unstuff_bits(resp, 88, 8);
+		card->cid.prod_name[2]	= unstuff_bits(resp, 80, 8);
+		card->cid.prod_name[3]	= unstuff_bits(resp, 72, 8);
+		card->cid.prod_name[4]	= unstuff_bits(resp, 64, 8);
+		card->cid.prod_name[5]	= unstuff_bits(resp, 56, 8);
+		card->cid.prv		= unstuff_bits(resp, 48, 8);
+		card->cid.serial	= unstuff_bits(resp, 16, 32);
+		card->cid.month		= unstuff_bits(resp, 12, 4);
+		card->cid.year		= unstuff_bits(resp, 8, 4) + 1997;
+		break;
+
+	default:
+		dev_err(&card->dev, "card has unknown MMCA version %d\n", mmca_vsn);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * Given the decoded CSD structure, decode the raw CID to our CID structure.
+ */
+static void mci_sd_decode_cid(struct mci *card)
+{
+	u32 *resp = card->raw_cid;
+
+	/*
+	 * SD doesn't currently have a version field so we will
+	 * have to assume we can parse this.
+	 */
+	card->cid.manfid		= unstuff_bits(resp, 120, 8);
+	card->cid.oemid			= unstuff_bits(resp, 104, 16);
+	card->cid.prod_name[0]		= unstuff_bits(resp, 96, 8);
+	card->cid.prod_name[1]		= unstuff_bits(resp, 88, 8);
+	card->cid.prod_name[2]		= unstuff_bits(resp, 80, 8);
+	card->cid.prod_name[3]		= unstuff_bits(resp, 72, 8);
+	card->cid.prod_name[4]		= unstuff_bits(resp, 64, 8);
+	card->cid.hwrev			= unstuff_bits(resp, 60, 4);
+	card->cid.fwrev			= unstuff_bits(resp, 56, 4);
+	card->cid.serial		= unstuff_bits(resp, 24, 32);
+	card->cid.year			= unstuff_bits(resp, 12, 8);
+	card->cid.month			= unstuff_bits(resp, 8, 4);
+
+	card->cid.year += 2000; /* SD cards year offset */
+}
+
 /**
  * Output some valuable information when the user runs 'devinfo' on an MCI device
  * @param mci MCI device instance
@@ -2492,38 +2424,44 @@ static void mci_info(struct device *dev)
 
 	if (mci->high_capacity)
 		printf("  High capacity card\n");
-	printf("   CID: %08X-%08X-%08X-%08X\n", mci->cid[0], mci->cid[1],
-		mci->cid[2], mci->cid[3]);
+	printf("   CID: %08X-%08X-%08X-%08X\n", mci->raw_cid[0], mci->raw_cid[1],
+		mci->raw_cid[2], mci->raw_cid[3]);
 	printf("   CSD: %08X-%08X-%08X-%08X\n", mci->csd[0], mci->csd[1],
 		mci->csd[2], mci->csd[3]);
 	printf("  Max. transfer speed: %u Hz\n", mci->tran_speed);
 	mci_print_caps(mci->card_caps);
-	printf("  Manufacturer ID: %s\n", dev_get_param(dev, "cid_mid"));
-	printf("  OEM/Application ID: %s\n", dev_get_param(dev, "cid_oid"));
-	if (!IS_SD(mci))
-		printf("  CBX: %s\n", dev_get_param(dev, "cid_cbx"));
-	printf("  Product name: '%s'\n", dev_get_param(dev, "cid_pnm"));
-	printf("  Product revision: %s\n", dev_get_param(dev, "cid_prv"));
-	printf("  Serial no: %s\n", dev_get_param(dev, "cid_psn"));
-	printf("  Manufacturing date: %s\n", dev_get_param(dev, "cid_mdt"));
+	printf("  Manufacturer ID: 0x%02x\n", mci->cid.manfid);
+	printf("  OEM/Application ID: 0x%04x\n", mci->cid.oemid);
+	printf("  Product name: '%s'\n", mci->cid.prod_name);
+	printf("  Hardware revision: 0x%02x\n", mci->cid.hwrev);
+	printf("  Firmware revision: 0x%02x\n", mci->cid.fwrev);
+	printf("  Serial no: %u\n", mci->cid.serial);
+	printf("  Manufacturing date: %u.%u\n", mci->cid.year, mci->cid.month);
 }
 
-static void mci_parse_cid(struct mci *mci) {
+static void mci_parse_cid(struct mci *mci)
+{
 	struct device *dev = &mci->dev;
-	char buffer[8];
-
-	dev_add_param_uint32_fixed(dev, "cid_mid", extract_mid(mci), "0x%02X");
-	extract_oid(mci, buffer);
-	dev_add_param_string_fixed(dev, "cid_oid", buffer);
-	if (!IS_SD(mci))
-		dev_add_param_uint32_fixed(dev, "cid_cbx", extract_cbx(mci), "%u");
-	extract_pnm(mci, buffer);
-	dev_add_param_string_fixed(dev, "cid_pnm", buffer);
-	extract_prv(mci, buffer);
-	dev_add_param_string_fixed(dev, "cid_prv", buffer);
-	dev_add_param_uint32_fixed(dev, "cid_psn", extract_psn(mci), "%0u");
-	extract_mdt(mci, buffer);
-	dev_add_param_string_fixed(dev, "cid_mdt", buffer);
+
+	if (IS_SD(mci))
+		mci_sd_decode_cid(mci);
+	else
+		mci_mmc_decode_cid(mci);
+
+	if (mci->ext_csd[EXT_CSD_REV] >= 5) {
+		/* Adjust production date as per JEDEC JESD84-B451 */
+		if (mci->cid.year < 2010)
+			mci->cid.year += 16;
+	}
+
+	dev_add_param_uint32_fixed(dev, "cid_mid", mci->cid.manfid, "0x%02X");
+	dev_add_param_uint32_fixed(dev, "cid_oid", mci->cid.oemid, "0x%04X");
+	dev_add_param_string_fixed(dev, "cid_pnm", mci->cid.prod_name);
+	dev_add_param_uint32_fixed(dev, "cid_hwrev", mci->cid.hwrev, "0x%02X");
+	dev_add_param_uint32_fixed(dev, "cid_fwrev", mci->cid.fwrev, "0x%02X");
+	dev_add_param_uint32_fixed(dev, "cid_psn", mci->cid.serial, "%0u");
+	dev_add_param_uint32_fixed(dev, "cid_year", mci->cid.year, "%0u");
+	dev_add_param_uint32_fixed(dev, "cid_month", mci->cid.month, "%0u");
 }
 
 /**
diff --git a/include/mci.h b/include/mci.h
index 1e37570274..689ca7167f 100644
--- a/include/mci.h
+++ b/include/mci.h
@@ -626,6 +626,18 @@ struct sd_ssr {
 	unsigned int erase_offset;      /* In milliseconds */
 };
 
+struct mmc_cid {
+	unsigned int		manfid;
+	char			prod_name[8];
+	unsigned char		prv;
+	unsigned int		serial;
+	unsigned short		oemid;
+	unsigned short		year;
+	unsigned char		hwrev;
+	unsigned char		fwrev;
+	unsigned char		month;
+};
+
 /** MMC/SD and interface instance information */
 struct mci {
 	struct mci_host *host;		/**< the host for this card */
@@ -635,7 +647,7 @@ struct mci {
 	unsigned ocr;		/**< card's "operation condition register" */
 	unsigned scr[2];
 	unsigned csd[4];	/**< card's "card specific data register" */
-	unsigned cid[4];	/**< card's "card identification register" */
+	unsigned raw_cid[4];	/**< card's "card identification register" */
 	unsigned short rca;	/**< relative card address */
 	u8 sdio:1;              /**< card is a SDIO card */
 	u8 high_capacity:1;	/**< high capacity card is connected (OCR -> OCR_HCS) */
@@ -663,6 +675,8 @@ struct mci {
 	struct mci_part *part_curr;
 	u8 ext_csd_part_config;
 
+	struct mmc_cid cid;
+
 	struct list_head list;     /* The list of all mci devices */
 };
 

-- 
2.39.5

[PATCH 2/6] mmc: merge block read/write functions

[Sascha Hauer]

mci_block_write() and mci_read_block() both have very similar
implementations. Merge them into a mci_do_block_op() function
and call it from the actual block read/write functions.

While at it rename mci_read_block() to mci_block_read() to get a
naming consistent to mci_block_write().

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 123 +++++++++++++++++++++++--------------------------
 1 file changed, 57 insertions(+), 66 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 34ea775813..975896d2d8 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -260,59 +260,6 @@ static int mci_poll_until_ready(struct mci *mci, int timeout_ms)
 	return 0;
 }
 
-
-/**
- * Write one or several blocks of data to the card
- * @param mci_dev MCI instance
- * @param src Where to read from to write to the card
- * @param blocknum Block number to write
- * @param blocks Block count to write
- * @return Transaction status (0 on success)
- */
-static int mci_block_write(struct mci *mci, const void *src, int blocknum,
-	int blocks)
-{
-	struct mci_cmd cmd = {};
-	struct mci_data data;
-	unsigned mmccmd;
-	int ret;
-
-	/*
-	 * Quoting eMMC Spec v5.1 (JEDEC Standard No. 84-B51):
-	 * Due to legacy reasons, a Device may still treat CMD24/25 during
-	 * prg-state (while busy is active) as a legal or illegal command.
-	 * A host should not send CMD24/25 while the Device is in the prg
-	 * state and busy is active.
-	 */
-	ret = mci_poll_until_ready(mci, 1000 /* ms */);
-	if (ret && ret != -ENOSYS)
-		return ret;
-
-	if (blocks > 1)
-		mmccmd = MMC_CMD_WRITE_MULTIPLE_BLOCK;
-	else
-		mmccmd = MMC_CMD_WRITE_SINGLE_BLOCK;
-
-	mci_setup_cmd(&cmd,
-		mmccmd,
-		mci->high_capacity != 0 ? blocknum : blocknum * mci->write_bl_len,
-		MMC_RSP_R1);
-
-	data.src = src;
-	data.blocks = blocks;
-	data.blocksize = mci->write_bl_len;
-	data.flags = MMC_DATA_WRITE;
-
-	ret = mci_send_cmd(mci, &cmd, &data);
-
-	if (ret || blocks > 1) {
-		mci_setup_cmd(&cmd, MMC_CMD_STOP_TRANSMISSION, 0, MMC_RSP_R1b);
-		mci_send_cmd(mci, &cmd, NULL);
-        }
-
-	return ret;
-}
-
 /**
  * Erase one or several blocks of data to the card
  * @param mci_dev MCI instance
@@ -364,35 +311,51 @@ static int mci_block_erase(struct mci *card, unsigned int from,
 	return -EIO;
 }
 
-/**
- * Read one or several block(s) of data from the card
- * @param mci MCI instance
- * @param dst Where to store the data read from the card
- * @param blocknum Block number to read
- * @param blocks number of blocks to read
- */
-static int mci_read_block(struct mci *mci, void *dst, int blocknum,
+static int mci_do_block_op(struct mci *mci, const void *src, void *dst, int blocknum,
 		int blocks)
 {
 	struct mci_cmd cmd = {};
 	struct mci_data data;
 	int ret;
-	unsigned mmccmd;
+	unsigned mmccmd_multi_block, mmccmd_single_block, mmccmd;
+	unsigned int flags;
+
+	if (dst) {
+		mmccmd_multi_block = MMC_CMD_READ_MULTIPLE_BLOCK;
+		mmccmd_single_block = MMC_CMD_READ_SINGLE_BLOCK;
+		flags = MMC_DATA_READ;
+	} else {
+		/*
+		 * Quoting eMMC Spec v5.1 (JEDEC Standard No. 84-B51):
+		 * Due to legacy reasons, a Device may still treat CMD24/25 during
+		 * prg-state (while busy is active) as a legal or illegal command.
+		 * A host should not send CMD24/25 while the Device is in the prg
+		 * state and busy is active.
+		 */
+		ret = mci_poll_until_ready(mci, 1000 /* ms */);
+		if (ret && ret != -ENOSYS)
+			return ret;
+
+		mmccmd_multi_block = MMC_CMD_WRITE_MULTIPLE_BLOCK;
+		mmccmd_single_block = MMC_CMD_WRITE_SINGLE_BLOCK;
+		flags = MMC_DATA_WRITE;
+	}
 
 	if (blocks > 1)
-		mmccmd = MMC_CMD_READ_MULTIPLE_BLOCK;
+		mmccmd = mmccmd_multi_block;
 	else
-		mmccmd = MMC_CMD_READ_SINGLE_BLOCK;
+		mmccmd = mmccmd_single_block;
 
 	mci_setup_cmd(&cmd,
 		mmccmd,
 		mci->high_capacity != 0 ? blocknum : blocknum * mci->read_bl_len,
 		MMC_RSP_R1);
 
+	data.src = src;
 	data.dest = dst;
 	data.blocks = blocks;
 	data.blocksize = mci->read_bl_len;
-	data.flags = MMC_DATA_READ;
+	data.flags = flags;
 
 	ret = mci_send_cmd(mci, &cmd, &data);
 
@@ -401,9 +364,37 @@ static int mci_read_block(struct mci *mci, void *dst, int blocknum,
 			      IS_SD(mci) ? MMC_RSP_R1b : MMC_RSP_R1);
 		mci_send_cmd(mci, &cmd, NULL);
 	}
+
 	return ret;
 }
 
+/**
+ * Read one or several block(s) of data from the card
+ * @param mci MCI instance
+ * @param dst Where to store the data read from the card
+ * @param blocknum Block number to read
+ * @param blocks number of blocks to read
+ */
+static int mci_block_read(struct mci *mci, void *dst, int blocknum,
+		int blocks)
+{
+	return mci_do_block_op(mci, NULL, dst, blocknum, blocks);
+}
+
+/**
+ * Write one or several blocks of data to the card
+ * @param mci_dev MCI instance
+ * @param src Where to read from to write to the card
+ * @param blocknum Block number to write
+ * @param blocks Block count to write
+ * @return Transaction status (0 on success)
+ */
+static int mci_block_write(struct mci *mci, const void *src, int blocknum,
+	int blocks)
+{
+	return mci_do_block_op(mci, src, NULL, blocknum, blocks);
+}
+
 /**
  * Reset the attached MMC/SD card
  * @param mci MCI instance
@@ -2244,7 +2235,7 @@ static int mci_sd_read(struct block_device *blk, void *buffer, sector_t block,
 
 	while (num_blocks) {
 		read_block = min(num_blocks, max_req_block);
-		rc = mci_read_block(mci, buffer, block, read_block);
+		rc = mci_block_read(mci, buffer, block, read_block);
 		if (rc != 0) {
 			dev_dbg(&mci->dev, "Reading block %llu failed with %d\n", block, rc);
 			return rc;

-- 
2.39.5

[PATCH 3/6] mci: cleanup code around ready_for_use flag

[Sascha Hauer]

struct mci::ready_for_use is a flag, thus should be boolean. Also
no need to have an extra function which returns a negative error
code when the flag is set. Just check for the flag where needed.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 29 ++++-------------------------
 include/mci.h          |  2 +-
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 975896d2d8..f29dc84f93 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -2373,7 +2373,7 @@ static void mci_info(struct device *dev)
 	struct mci_host *host = mci->host;
 	int bw;
 
-	if (mci->ready_for_use == 0) {
+	if (!mci->ready_for_use) {
 		printf(" No information available:\n  MCI card not probed yet\n");
 		return;
 	}
@@ -2455,23 +2455,6 @@ static void mci_parse_cid(struct mci *mci)
 	dev_add_param_uint32_fixed(dev, "cid_month", mci->cid.month, "%0u");
 }
 
-/**
- * Check if the MCI card is already probed
- * @param mci MCI device instance
- * @return 0 when not probed yet, -EPERM if already probed
- *
- * @a barebox cannot really cope with hot plugging. So, probing an attached
- * MCI card is a one time only job. If its already done, there is no way to
- * return.
- */
-static int mci_check_if_already_initialized(struct mci *mci)
-{
-	if (mci->ready_for_use != 0)
-		return -EPERM;
-
-	return 0;
-}
-
 static struct block_device_ops mci_ops = {
 	.read = mci_sd_read,
 	.write = IS_ENABLED(CONFIG_MCI_WRITE) ? mci_sd_write : NULL,
@@ -2716,7 +2699,7 @@ static int mci_card_probe(struct mci *mci)
 	}
 
 	dev_dbg(&mci->dev, "Card is up and running now, registering as a disk\n");
-	mci->ready_for_use = 1;	/* TODO now or later? */
+	mci->ready_for_use = true; /* TODO now or later? */
 
 	for (i = 0; i < mci->nr_parts; i++) {
 		struct mci_part *part = &mci->part[i];
@@ -2775,8 +2758,7 @@ static int mci_set_probe(struct param_d *param, void *priv)
 	if (!mci->probe)
 		return 0;
 
-	rc = mci_check_if_already_initialized(mci);
-	if (rc != 0)
+	if (mci->ready_for_use)
 		return 0;
 
 	rc = mci_card_probe(mci);
@@ -2788,10 +2770,7 @@ static int mci_set_probe(struct param_d *param, void *priv)
 
 int mci_detect_card(struct mci_host *host)
 {
-	int rc;
-
-	rc = mci_check_if_already_initialized(host->mci);
-	if (rc != 0)
+	if (host->mci->ready_for_use)
 		return 0;
 
 	return mci_card_probe(host->mci);
diff --git a/include/mci.h b/include/mci.h
index 689ca7167f..db1333999a 100644
--- a/include/mci.h
+++ b/include/mci.h
@@ -660,7 +660,7 @@ struct mci {
 	unsigned write_bl_len;
 	unsigned erase_grp_size;
 	uint64_t capacity;	/**< Card's data capacity in bytes */
-	int ready_for_use;	/** true if already probed */
+	bool ready_for_use;	/** true if already probed */
 	int dsr_imp;		/**< DSR implementation state from CSD */
 	u8 *ext_csd;
 	int probe;

-- 
2.39.5

[PATCH 4/6] mci: mci_spi: remove stray return 0

[Sascha Hauer]

The return 0 in mmc_spi_request() cannot be reached. Remove it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci_spi.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mci/mci_spi.c b/drivers/mci/mci_spi.c
index 41d8c25e27..41d788704f 100644
--- a/drivers/mci/mci_spi.c
+++ b/drivers/mci/mci_spi.c
@@ -294,9 +294,6 @@ static int mmc_spi_request(struct mci_host *mci, struct mci_cmd *cmd, struct mci
 done:
 	mmc_cs_off(host);
 	return ret;
-
-return  0;
-
 }
 
 static void mmc_spi_set_ios(struct mci_host *mci, struct mci_ios *ios)

-- 
2.39.5

[PATCH 5/6] mci: mci_spi: fix warning message

[Sascha Hauer]

The driver warns about a missing reset GPIO, but it is the card detect
GPIO that is searched for. Fix the warning message.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mci/mci_spi.c b/drivers/mci/mci_spi.c
index 41d788704f..864e15a416 100644
--- a/drivers/mci/mci_spi.c
+++ b/drivers/mci/mci_spi.c
@@ -440,7 +440,7 @@ static int spi_mci_probe(struct device *dev)
 		host->mci.devname = xstrdup(of_alias_get(np));
 		host->detect_pin = gpiod_get_optional(dev, NULL, GPIOD_IN);
 		if (IS_ERR(host->detect_pin))
-			dev_warn(dev, "Failed to get 'reset' GPIO (ignored)\n");
+			dev_warn(dev, "Failed to get card detect GPIO (ignored)\n");
 	}
 
 	mci_register(&host->mci);

-- 
2.39.5

[PATCH 6/6] mci: mci_spi: use mci_of_parse()

[Sascha Hauer]

The mci_spi driver parses the device tree alias itself. Instead call
mci_of_parse() which does the same and also parses other generic mmc
properties.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mci/mci_spi.c b/drivers/mci/mci_spi.c
index 864e15a416..dd4c9333ee 100644
--- a/drivers/mci/mci_spi.c
+++ b/drivers/mci/mci_spi.c
@@ -437,7 +437,7 @@ static int spi_mci_probe(struct device *dev)
 	host->mci.host_caps = MMC_CAP_SPI;
 
 	if (np) {
-		host->mci.devname = xstrdup(of_alias_get(np));
+		mci_of_parse(&host->mci);
 		host->detect_pin = gpiod_get_optional(dev, NULL, GPIOD_IN);
 		if (IS_ERR(host->detect_pin))
 			dev_warn(dev, "Failed to get card detect GPIO (ignored)\n");

-- 
2.39.5

Re: [PATCH 1/6] mci: use struct cid

[Alexander Shiyan]

Hello.

> Linux has a struct mmc_cid where the CID data is parsed into whereas
> in barebox we call the UNSTUFF_BITS macro whenever we need a field
> from the CID data. Do it like Linux and parse the CID data into the
> same struct. While at it convert the UNSTUFF_BITS macro into a
> unstuff_bits static inline function.
...
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 7ec2643b8d..34ea775813 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -26,19 +26,19 @@
>
>  #define MAX_BUFFER_NUMBER 0xffffffff
>
> -#define UNSTUFF_BITS(resp,start,size)                                  \
> -       ({                                                              \
> -               const int __size = size;                                \
> -               const u32 __mask = (__size < 32 ? 1 << __size : 0) - 1; \
> -               const int __off = 3 - ((start) / 32);                   \
> -               const int __shft = (start) & 31;                        \
> -               u32 __res;                                              \
> -                                                                       \
> -               __res = resp[__off] >> __shft;                          \
> -               if (__size + __shft > 32)                               \
> -                       __res |= resp[__off-1] << ((32 - __shft) % 32); \
> -               __res & __mask;                                         \
> -       })
> +static inline u32 unstuff_bits(const u32 *resp, int start, int size)
> +{
> +       const int __size = size;
> +       const u32 __mask = (__size < 32 ? 1 << __size : 0) - 1;
> +       const int __off = 3 - (start / 32);
> +       const int __shft = start & 31;
> +       u32 __res = resp[__off] >> __shft;
> +
> +       if (__size + __shft > 32)
> +               __res |= resp[__off - 1] << ((32 - __shft) % 32);
> +
> +       return __res & __mask;
> +}

For board specific code where the eMMC ID is used as a unique identifier,
it would be useful to have this function in the header.

Re: [PATCH 1/6] mci: use struct cid

[Sascha Hauer]

On Mon, Mar 17, 2025 at 12:41:22PM +0300, Alexander Shiyan wrote:
> Hello.
> 
> > Linux has a struct mmc_cid where the CID data is parsed into whereas
> > in barebox we call the UNSTUFF_BITS macro whenever we need a field
> > from the CID data. Do it like Linux and parse the CID data into the
> > same struct. While at it convert the UNSTUFF_BITS macro into a
> > unstuff_bits static inline function.
> ...
> > diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> > index 7ec2643b8d..34ea775813 100644
> > --- a/drivers/mci/mci-core.c
> > +++ b/drivers/mci/mci-core.c
> > @@ -26,19 +26,19 @@
> >
> >  #define MAX_BUFFER_NUMBER 0xffffffff
> >
> > -#define UNSTUFF_BITS(resp,start,size)                                  \
> > -       ({                                                              \
> > -               const int __size = size;                                \
> > -               const u32 __mask = (__size < 32 ? 1 << __size : 0) - 1; \
> > -               const int __off = 3 - ((start) / 32);                   \
> > -               const int __shft = (start) & 31;                        \
> > -               u32 __res;                                              \
> > -                                                                       \
> > -               __res = resp[__off] >> __shft;                          \
> > -               if (__size + __shft > 32)                               \
> > -                       __res |= resp[__off-1] << ((32 - __shft) % 32); \
> > -               __res & __mask;                                         \
> > -       })
> > +static inline u32 unstuff_bits(const u32 *resp, int start, int size)
> > +{
> > +       const int __size = size;
> > +       const u32 __mask = (__size < 32 ? 1 << __size : 0) - 1;
> > +       const int __off = 3 - (start / 32);
> > +       const int __shft = start & 31;
> > +       u32 __res = resp[__off] >> __shft;
> > +
> > +       if (__size + __shft > 32)
> > +               __res |= resp[__off - 1] << ((32 - __shft) % 32);
> > +
> > +       return __res & __mask;
> > +}
> 
> For board specific code where the eMMC ID is used as a unique identifier,
> it would be useful to have this function in the header.

Wouldn't you rather use something like getenv("mmc0.cid_psn") then?

We could also introduce some mmc helper function for convenience.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/6] mci: some cleanups

[Sascha Hauer]

On Mon, 17 Mar 2025 10:34:32 +0100, Sascha Hauer wrote:
> This cleans up some stuff in the mci layer and its drivers.
> 
> 

Applied, thanks!

[1/6] mci: use struct cid
      https://git.pengutronix.de/cgit/barebox/commit/?id=a4a78c4f321a (link may not be stable)
[2/6] mmc: merge block read/write functions
      https://git.pengutronix.de/cgit/barebox/commit/?id=38543119546f (link may not be stable)
[3/6] mci: cleanup code around ready_for_use flag
      https://git.pengutronix.de/cgit/barebox/commit/?id=8c7710fdf1c0 (link may not be stable)
[4/6] mci: mci_spi: remove stray return 0
      https://git.pengutronix.de/cgit/barebox/commit/?id=52ea3c5986cf (link may not be stable)
[5/6] mci: mci_spi: fix warning message
      https://git.pengutronix.de/cgit/barebox/commit/?id=6c0dcb67f65a (link may not be stable)
[6/6] mci: mci_spi: use mci_of_parse()
      https://git.pengutronix.de/cgit/barebox/commit/?id=82a681eb1889 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>