latest MMC-over-SPI support

Same deal as before ... refreshed against the latest git-mmc.patch, four patches:

- headers
- block
- core
- mmc_spi host

Differences from the very last patches sent are primarily that this one includes the lock/unlock changes again (since it's against git-mmc not against kernel.org), and code for the SDIO stuff is there (ditto). But also, a few more of Pierre's comments have kicked in.

I'm hoping this version gets merged into git-mmc. :)

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Teach the MMC/SD/SDIO system headers that some hosts use SPI mode

- New host capabilities and status bits
* MMC_CAP_SPI, with mmc_host_is_spi() test
* mmc_host.use_spi_crc flag

- SPI-specific declarations:
* Response types, MMC_RSP_SPI_R*
* Two SPI-only commands
* Status bits used native to SPI: R1_SPI_*, R2_SPI_*

- Fix a few (unrelated) whitespace bugs in the headers.

- Reorder a few mmc_host fields, removing several bytes of padding

None of these changes affect current code.

---
Against: git-mmc.patch from 2.6.21-rc1-mm2

 include/linux/mmc/core.h |   26 ++++++++++++++++++++++++--
 include/linux/mmc/host.h |   16 +++++++++++-----
 include/linux/mmc/mmc.h  |   47 +++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 76 insertions(+), 13 deletions(-)

@@ -92,6 +92,7 @@ struct mmc_host {
 #define MMC_CAP_MMC_HIGHSPEED	(1 << 2)	/* Can do MMC high-speed timing */
 #define MMC_CAP_SD_HIGHSPEED	(1 << 3)	/* Can do SD high-speed timing */
 #define MMC_CAP_SDIO_IRQ	(1 << 4)	/* Can signal pending SDIO IRQs */
+#define MMC_CAP_SPI		(1 << 5)	/* Talks only SPI protocols */
 
 	/* host specific block data */
 	unsigned int		max_seg_size;	/* see blk_queue_max_segment_size */

@@ -108,6 +109,14 @@ struct mmc_host {
 	struct mmc_ios		ios;		/* current io bus settings */
 	u32			ocr;		/* the current OCR setting */
 
+	/* group bitfields together to minimize padding */
+	unsigned int		use_spi_crc:1;
+	unsigned int		claimed:1;	/* host exclusively claimed */
+	unsigned int		bus_dead:1;	/* bus has been released */
+#ifdef CONFIG_MMC_DEBUG
+	unsigned int		removed:1;	/* host is being removed */
+#endif
+
 	unsigned int		mode;		/* current card mode of host */
 #define MMC_MODE_MMC		0
 #define MMC_MODE_SD		1

@@ -115,16 +124,11 @@ struct mmc_host {
 	struct mmc_card		*card;		/* device attached to this host */
 
 	wait_queue_head_t	wq;
-	unsigned int		claimed:1;	/* host exclusively claimed */
 
 	struct delayed_work	detect;
-#ifdef CONFIG_MMC_DEBUG
-	unsigned int		removed:1;	/* host is being removed */
-#endif
 
 	const struct mmc_bus_ops *bus_ops;	/* current bus driver */
 	unsigned int		bus_refs;	/* reference counter */
-	unsigned int		bus_dead:1;	/* bus has been released */
 
 	unsigned int		sdio_irqs;
 	struct task_struct	*sdio_irq_thread;

@@ -143,6 +147,8 @@ static inline void *mmc_priv(struct mmc_
 	return (void *)host->private;
 }
 
+#define mmc_host_is_spi(host)	((host)->caps & MMC_CAP_SPI)
+
 #define mmc_dev(x)	((x)->parent)
 #define mmc_classdev(x)	(&(x)->class_dev)
 #define mmc_hostname(x)	((x)->class_dev.bus_id)

@@ -25,14 +25,20 @@ struct mmc_command {
 #define MMC_RSP_CRC	(1 << 2)		/* expect valid crc */
 #define MMC_RSP_BUSY	(1 << 3)		/* card may send busy */
 #define MMC_RSP_OPCODE	(1 << 4)		/* response contains opcode */
-#define MMC_CMD_MASK	(3 << 5)		/* command type */
+
+#define MMC_CMD_MASK	(3 << 5)		/* non-SPI command type */
 #define MMC_CMD_AC	(0 << 5)
 #define MMC_CMD_ADTC	(1 << 5)
 #define MMC_CMD_BC	(2 << 5)
 #define MMC_CMD_BCR	(3 << 5)
 
+#define MMC_RSP_SPI_S1	(1 << 7)		/* one status byte */
+#define MMC_RSP_SPI_S2	(1 << 8)		/* second byte */
+#define MMC_RSP_SPI_B4	(1 << 9)		/* four data bytes */
+#define MMC_RSP_SPI_BUSY (1 << 10)		/* card may send busy */
+
 /*
- * These are the response types, and correspond to valid bit
+ * These are the native response types, and correspond to valid bit
  * patterns of the above flags.  One additional valid pattern
  * is all zeros, which means we don't expect a response.
  */

@@ -49,6 +55,22 @@ struct mmc_command {
 #define mmc_resp_type(cmd)	((cmd)->flags & (MMC_RSP_PRESENT|MMC_RSP_136|MMC_RSP_CRC|MMC_RSP_BUSY|MMC_RSP_OPCODE))
 
 /*
+ * These are the SPI response types for MMC, SD, and SDIO cards.
+ * Commands return R1, with maybe more info.  Zero is an error type;
+ * callers must always provide the appropriate MMC_RSP_SPI_Rx flags.
+ */
+#define MMC_RSP_SPI_R1	(MMC_RSP_SPI_S1)
+#define MMC_RSP_SPI_R1B	(MMC_RSP_SPI_S1|MMC_RSP_SPI_BUSY)
+#define MMC_RSP_SPI_R2	(MMC_RSP_SPI_S1|MMC_RSP_SPI_S2)
+#define MMC_RSP_SPI_R3	(MMC_RSP_SPI_S1|MMC_RSP_SPI_B4)
+#define MMC_RSP_SPI_R4	(MMC_RSP_SPI_S1|MMC_RSP_SPI_B4)
+#define MMC_RSP_SPI_R5	(MMC_RSP_SPI_S1|MMC_RSP_SPI_S2)
+#define MMC_RSP_SPI_R7	(MMC_RSP_SPI_S1|MMC_RSP_SPI_B4)
+
+#define mmc_spi_resp_type(cmd)	((cmd)->flags & \
+		(MMC_RSP_SPI_S1|MMC_RSP_SPI_BUSY|MMC_RSP_SPI_S2|MMC_RSP_SPI_B4))
+
+/*
  * These are the command types.
  */
 #define mmc_cmd_type(cmd)	((cmd)->flags & MMC_CMD_MASK)

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Teaching the MMC/SD block card driver about SPI.

- Provide the SPI response type flags with each request issued.

- Understand that multiblock SPI writes don't use STOP_TRANSMISSION.

- Correct check for APP_CMD failure.

---
Against: git-mmc.patch from 2.6.21-rc1-mm2

 drivers/mmc/card/block.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

@@ -151,17 +151,19 @@ static u32 mmc_sd_num_wr_blocks(struct m
 
 	cmd.opcode = MMC_APP_CMD;
 	cmd.arg = card->rca << 16;
-	cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
 
 	err = mmc_wait_for_cmd(card->host, &cmd, 0);
-	if (err || !(cmd.resp[0] & R1_APP_CMD))
+	if (err)
+		return (u32)-1;
+	if (!mmc_host_is_spi(card->host) && !(cmd.resp[0] & R1_APP_CMD))
 		return (u32)-1;
 
 	memset(&cmd, 0, sizeof(struct mmc_command));
 
 	cmd.opcode = SD_APP_SEND_NUM_WR_BLKS;
 	cmd.arg = 0;
-	cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
 
 	memset(&data, 0, sizeof(struct mmc_data));
 

@@ -220,11 +222,11 @@ static int mmc_blk_issue_rq(struct mmc_q
 		brq.cmd.arg = req->sector;
 		if (!mmc_card_blockaddr(card))
 			brq.cmd.arg <<= 9;
-		brq.cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+		brq.cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
 		brq.data.blksz = 1 << md->block_bits;
 		brq.stop.opcode = MMC_STOP_TRANSMISSION;
 		brq.stop.arg = 0;
-		brq.stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
+		brq.stop.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
 		brq.data.blocks = req->nr_sectors >> (md->block_bits - 9);
 		if (brq.data.blocks > card->host->max_blk_count)
 			brq.data.blocks = card->host->max_blk_count;

@@ -241,7 +243,12 @@ static int mmc_blk_issue_rq(struct mmc_q
 			brq.data.blocks = 1;
 
 		if (brq.data.blocks > 1) {
-			brq.mrq.stop = &brq.stop;
+			/* SPI multiblock writes terminate using a special
+			 * token, not a STOP_TRANSMISSION request.
+			 */
+			if (!mmc_host_is_spi(card->host)
+					|| rq_data_dir(req) == READ)
+				brq.mrq.stop = &brq.stop;
 			readcmd = MMC_READ_MULTIPLE_BLOCK;
 			writecmd = MMC_WRITE_MULTIPLE_BLOCK;
 		} else {

@@ -301,7 +308,7 @@ static int mmc_blk_issue_rq(struct mmc_q
 			goto cmd_err;
 		}
 
-		if (rq_data_dir(req) != READ) {
+		if (!mmc_host_is_spi(card->host) && rq_data_dir(req) != READ) {
 			do {
 				int err;
 

@@ -509,7 +516,7 @@ mmc_blk_set_blksize(struct mmc_blk_data 
 	mmc_claim_host(card->host);
 	cmd.opcode = MMC_SET_BLOCKLEN;
 	cmd.arg = 1 << md->block_bits;
-	cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
 	err = mmc_wait_for_cmd(card->host, &cmd, 5);
 	mmc_release_host(card->host);
 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Teach the MMC/SD/SDIO core about using SPI mode.

- Use mmc_host_is_spi() so enumeration works through SPI signaling and protocols, not just the native versions.

- Provide the SPI response type flags with each request issued, including requests from the new lock/unlock code.

- Understand that cmd->resp[0] and mmc_get_status() results for SPI return different values than for "native" MMC/SD protocol; this affects resetting, checking card lock status, and some others.

- Understand that some commands act a bit differently ... notably:
* OP_COND command doesn't return the OCR
* APP_CMD status doesn't have an R1_APP_CMD analogue

Those changes required some new and updated primitives:

- Provide utilities to access two SPI-only requests, and one request that wasn't previously needed:
* mmc_spi_read_ocr() ... SPI only
* mmc_spi_set_crc() ... SPI only (override by module parm)
* mmc_send_cid() ... for use without broadcast mode

- Updated internal routines:
* Previous mmc_send_csd() modified into mmc_send_cxd_native(); it uses native "R2" responses, which include 16 bytes of data.
* Previous mmc_send_ext_csd() becomes new mmc_send_cxd_data() helper for command-and-data access
* Bugfix to that mmc_send_cxd_data() code: dma-to-stack is unsafe/nonportable, so kmalloc a bounce buffer instead.

- Modified mmc_send_ext_csd() now uses mmc_send_cxd_data() helper

- Modified mmc_send_csd(), and new mmc_spi_send_cid(), routines use those helper routines based on whether they're native or SPI

The newest categories of cards supported by the MMC stack aren't expected to work yet with SPI: MMC or SD cards with over 4GB data, and SDIO. All those cards support SPI mode, so eventually they should work too.

---
Against: git-mmc.patch from 2.6.21-rc1-mm2. Changes since last version:
restore the lock/unlock hooks; SDIO is coded-to-spec. So it's bigger.

 drivers/mmc/core/core.c     |   39 +++++++--
 drivers/mmc/core/core.h     |    2 
 drivers/mmc/core/mmc.c      |   43 ++++++----
 drivers/mmc/core/mmc_ops.c  |  185 ++++++++++++++++++++++++++++++++++----------
 drivers/mmc/core/mmc_ops.h  |    3 
 drivers/mmc/core/sd.c       |   39 ++++++---
 drivers/mmc/core/sd_ops.c   |   38 ++++++---
 drivers/mmc/core/sdio_ops.c |   49 +++++++----
 8 files changed, 301 insertions(+), 97 deletions(-)

@@ -25,6 +25,9 @@ int mmc_send_ext_csd(struct mmc_card *ca
 int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value);
 int mmc_send_status(struct mmc_card *card, u32 *status);
 int mmc_lock_unlock(struct mmc_card *card, struct key *key, int mode);
+int mmc_send_cid(struct mmc_host *host, u32 *cid);
+int mmc_spi_read_ocr(struct mmc_host *host, u32 *ocrp);
+int mmc_spi_set_crc(struct mmc_host *host, int use_crc);
 
 #endif
 

@@ -43,6 +43,14 @@ extern int mmc_attach_sdio(struct mmc_ho
 static struct workqueue_struct *workqueue;
 
 /*
+ * Enabling software CRCs on the data blocks can be a significant (30%)
+ * performance cost, and for other reasons may not always be desired.
+ * So we allow it it to be disabled.
+ */
+int use_spi_crc = 1;
+module_param(use_spi_crc, bool, 0);
+
+/*
  * Internal function. Schedule delayed work in the MMC work queue.
  */
 static int mmc_schedule_delayed_work(struct delayed_work *work,

@@ -72,6 +80,11 @@ void mmc_request_done(struct mmc_host *h
 	struct mmc_command *cmd = mrq->cmd;
 	int err = cmd->error;
 
+	if (err && cmd->retries && mmc_host_is_spi(host)) {
+		if (cmd->resp[0] & R1_SPI_ILLEGAL_COMMAND)
+			cmd->retries = 0;
+	}
+
 	if (err && cmd->retries) {
 		pr_debug("%s: req failed (CMD%u): %d, retrying...\n",
 			mmc_hostname(host), cmd->opcode, err);

@@ -445,8 +458,13 @@ static void mmc_power_up(struct mmc_host
 	int bit = fls(host->ocr_avail) - 1;
 
 	host->ios.vdd = bit;
-	host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
-	host->ios.chip_select = MMC_CS_DONTCARE;
+	if (mmc_host_is_spi(host)) {
+		host->ios.chip_select = MMC_CS_HIGH;
+		host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;
+	} else {
+		host->ios.chip_select = MMC_CS_DONTCARE;
+		host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
+	}
 	host->ios.power_mode = MMC_POWER_UP;
 	host->ios.bus_width = MMC_BUS_WIDTH_1;
 	host->ios.timing = MMC_TIMING_LEGACY;

@@ -465,8 +483,10 @@ static void mmc_power_off(struct mmc_hos
 {
 	host->ios.clock = 0;
 	host->ios.vdd = 0;
-	host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
-	host->ios.chip_select = MMC_CS_DONTCARE;
+	if (!mmc_host_is_spi(host)) {
+		host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
+		host->ios.chip_select = MMC_CS_DONTCARE;
+	}
 	host->ios.power_mode = MMC_POWER_OFF;
 	host->ios.bus_width = MMC_BUS_WIDTH_1;
 	host->ios.timing = MMC_TIMING_LEGACY;

@@ -624,17 +644,24 @@ void mmc_rescan(struct work_struct *work
 		 */
 		err = mmc_send_app_op_cond(host, 0, &ocr);
 		if (!err) {
-			if (mmc_attach_sd(host, ocr))
+			if (mmc_host_is_spi(host))
+				err = mmc_spi_read_ocr(host, &ocr);
+			if (err || mmc_attach_sd(host, ocr))
 				mmc_power_off(host);
 			return;
 		}
 
 		/*
 		 * ...and finally MMC.
+		 *
+		 * REVISIT we currently expect that MMC4 cards (4+ GB)
+		 * will fail on SPI hosts.
 		 */
 		err = mmc_send_op_cond(host, 0, &ocr);
 		if (!err) {
-			if (mmc_attach_mmc(host, ocr))
+			if (mmc_host_is_spi(host))
+				err = mmc_spi_read_ocr(host, &ocr);
+			if (err || mmc_attach_mmc(host, ocr))
 				mmc_power_off(host);
 			return;
 		}

@@ -327,9 +327,21 @@ static int mmc_sd_init_card(struct mmc_h
 		goto err;
 
 	/*
+	 * For SPI, enable CRC as appropriate.
+	 */
+	if (mmc_host_is_spi(host)) {
+		err = mmc_spi_set_crc(host, use_spi_crc);
+		if (err)
+			goto err;
+	}
+
+	/*
 	 * Fetch CID from card.
 	 */
-	err = mmc_all_send_cid(host, cid);
+	if (mmc_host_is_spi(host))
+		err = mmc_send_cid(host, cid);
+	else
+		err = mmc_all_send_cid(host, cid);
 	if (err)
 		goto err;
 

@@ -355,13 +367,15 @@ static int mmc_sd_init_card(struct mmc_h
 	}
 
 	/*
-	 * Set card RCA.
+	 * For native busses:  get card RCA and quit open drain mode.
 	 */
-	err = mmc_send_relative_addr(host, &card->rca);
-	if (err)
-		goto free_card;
+	if (!mmc_host_is_spi(host)) {
+		err = mmc_send_relative_addr(host, &card->rca);
+		if (err)
+			goto free_card;
 
-	mmc_set_bus_mode(host, MMC_BUSMODE_PUSHPULL);
+		mmc_set_bus_mode(host, MMC_BUSMODE_PUSHPULL);
+	}
 
 	/*
 	 * Check if card is locked.

@@ -369,7 +383,7 @@ static int mmc_sd_init_card(struct mmc_h
 	err = mmc_send_status(card, &status);
 	if (err)
 		goto free_card;
-	if (status & R1_CARD_IS_LOCKED)
+	if (mmc_status_card_is_locked(host, status))
 		mmc_card_set_locked(card);
 
 	if (!oldcard) {

@@ -390,9 +404,11 @@ static int mmc_sd_init_card(struct mmc_h
 	/*
 	 * Select card, as all following commands rely on that.
 	 */
-	err = mmc_select_card(card);
-	if (err)
-		goto free_card;
+	if (!mmc_host_is_spi(host)) {
+		err = mmc_select_card(card);
+		if (err)
+			goto free_card;
+	}
 
 	if (!oldcard) {
 		/*

@@ -582,7 +598,8 @@ static void mmc_sd_suspend(struct mmc_ho
 	BUG_ON(!host->card);
 
 	mmc_claim_host(host);
-	mmc_deselect_cards(host);
+	if (!mmc_host_is_spi(host))
+		mmc_deselect_cards(host);
 	host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_LOCKED);
 	mmc_release_host(host);
 }

@@ -33,10 +33,10 @@ static int mmc_app_cmd(struct mmc_host *
 
 	if (card) {
 		cmd.arg = card->rca << 16;
-		cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
+		cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
 	} else {
 		cmd.arg = 0;
-		cmd.flags = MMC_RSP_R1 | MMC_CMD_BCR;
+		cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_BCR;
 	}
 
 	err = mmc_wait_for_cmd(host, &cmd, 0);

@@ -44,7 +44,7 @@ static int mmc_app_cmd(struct mmc_host *
 		return err;
 
 	/* Check that card supported application commands */
-	if (!(cmd.resp[0] & R1_APP_CMD))
+	if (!mmc_host_is_spi(host) && !(cmd.resp[0] & R1_APP_CMD))
 		return -EOPNOTSUPP;
 
 	return 0;

@@ -83,8 +83,14 @@ int mmc_wait_for_app_cmd(struct mmc_host
 		memset(&mrq, 0, sizeof(struct mmc_request));
 
 		err = mmc_app_cmd(host, card);
-		if (err)
+		if (err) {
+			/* no point in retrying; no APP commands allowed */
+			if (mmc_host_is_spi(host)) {
+				if (cmd->resp[0] & R1_SPI_ILLEGAL_COMMAND)
+					break;
+			}
 			continue;
+		}
 
 		memset(&mrq, 0, sizeof(struct mmc_request));
 

@@ -99,6 +105,12 @@ int mmc_wait_for_app_cmd(struct mmc_host
 		err = cmd->error;
 		if (!cmd->error)
 			break;
+
+		/* no point in retrying illegal APP commands */
+		if (mmc_host_is_spi(host)) {
+			if (cmd->resp[0] & R1_SPI_ILLEGAL_COMMAND)
+				break;
+		}
 	}
 
 	return err;

@@ -148,14 +160,18 @@ int mmc_send_app_op_cond(struct mmc_host
 
 	cmd.opcode = SD_APP_OP_COND;
 	cmd.arg = ocr;
-	cmd.flags = MMC_RSP_R3 | MMC_CMD_BCR;
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R3 | MMC_CMD_BCR;
 
 	for (i = 100; i; i--) {
 		err = mmc_wait_for_app_cmd(host, NULL, &cmd, MMC_CMD_RETRIES);
 		if (err)
 			break;
 
-		if (cmd.resp[0] & MMC_CARD_BUSY || ocr == 0)
+		if (mmc_host_is_spi(host)) {
+			/* wait until reset completes */
+			if (!(cmd.resp[0] & R1_SPI_IDLE))
+				break;
+		} else if ((cmd.resp[0] & MMC_CARD_BUSY) || ocr == 0)
 			break;
 
 		err = -ETIMEDOUT;

@@ -182,7 +198,7 @@ int mmc_send_if_cond(struct mmc_host *ho
 	 */
 	cmd.opcode = SD_SEND_IF_COND;
 	cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | test_pattern;
-	cmd.flags = MMC_RSP_R7 | MMC_CMD_BCR;
+	cmd.flags = MMC_RSP_SPI_R7 | MMC_RSP_R7 | MMC_CMD_BCR;
 
 	err = mmc_wait_for_cmd(host, &cmd, 0);
 	if (err)

@@ -229,6 +245,8 @@ int mmc_app_send_scr(struct mmc_card *ca
 	BUG_ON(!card->host);
 	BUG_ON(!scr);
 
+	/* NOTE: caller guarantees scr is heap-allocated */
+
 	err = mmc_app_cmd(card->host, card);
 	if (err)
 		return err;

@@ -242,7 +260,7 @@ int mmc_app_send_scr(struct mmc_card *ca
 
 	cmd.opcode = SD_APP_SEND_SCR;
 	cmd.arg = 0;
-	cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
 
 	data.blksz = 8;
 	data.blocks = 1;

@@ -278,6 +296,8 @@ int mmc_sd_switch(struct mmc_card *card,
 	BUG_ON(!card);
 	BUG_ON(!card->host);
 
+	/* NOTE: caller guarantees resp is heap-allocated */
+
 	mode = !!mode;
 	value &= 0xF;
 

@@ -292,7 +312,7 @@ int mmc_sd_switch(struct mmc_card *card,
 	cmd.arg = mode << 31 | 0x00FFFFFF;
 	cmd.arg &= ~(0xF << (group * 4));
 	cmd.arg |= value << (group * 4);
-	cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
 
 	data.blksz = 64;
 	data.blocks = 1;

@@ -48,5 +48,7 @@ void mmc_rescan(struct work_struct *work
 void mmc_start_host(struct mmc_host *host);
 void mmc_stop_host(struct mmc_host *host);
 
+extern int use_spi_crc;
+
 #endif
 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Either they're already handled correctly in the mmc_spi host code (and don't need duplicate handling here), or they need attention from someone with all the relevant hardware in hand...

If you insist ... appended.

=========
Tweak the (untested) SDIO-over-SPI code paths.

---
 drivers/mmc/core/sdio_ops.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

@@ -79,8 +79,7 @@ int mmc_io_rw_direct(struct mmc_card *ca
 		return err;
 
 	if (mmc_host_is_spi(card->host)) {
-		if (out)
-			*out = (cmd.resp[0] >> 8) & 0xFF;
+		/* host driver already reported errors */
 	} else {
 		if (cmd.resp[0] & R5_ERROR)
 			return -EIO;

@@ -88,8 +87,12 @@ int mmc_io_rw_direct(struct mmc_card *ca
 			return -EINVAL;
 		if (cmd.resp[0] & R5_OUT_OF_RANGE)
 			return -ERANGE;
+	}
 
-		if (out)
+	if (out) {
+		if (mmc_host_is_spi(card->host))
+			*out = (cmd.resp[0] >> 8) & 0xFF;
+		else
 			*out = cmd.resp[0] & 0xFF;
 	}
 

@@ -140,7 +143,9 @@ int mmc_io_rw_extended(struct mmc_card *
 	if (data.error)
 		return data.error;
 
-	if (!mmc_host_is_spi(card->host)) {
+	if (mmc_host_is_spi(card->host)) {
+		/* host driver already reported errors */
+	} else {
 		if (cmd.resp[0] & R5_ERROR)
 			return -EIO;
 		if (cmd.resp[0] & R5_FUNCTION_NUMBER)

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

I'll put it on my list. The hardware is in the mail (kudos to Atmel), so I'll hopefully be running sdio cards of SPI in a short while.

Muchos gracias.

Btw, the first batch is already pushed to Andrew. So it will be in his next -mm.

Rgds

     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

This wasn't merged correctly into 2.6.23-rc2-mm2's version of git-mmc, and that goof guarantees oopsing while enumerating at least SD-cards.

When the merge botch is fixed, that version of git-mmc.patch passes sanity testing with MMC-over-SPI.

The line above was merged as "mmc_wait_for_req(card->host, &mrq);" ...

Note the preceding "if (card) ...". When "card" is null, as it usually is during SPI enumeration, this always oopses.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

I can only agree, ooops :)

Rgds

     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Hi David and all,

I need the following patch on my (little endian) System to successfully read the csd/cid structure.

Sascha

CSD/CID is transfered in big endian format. Convert it to cpu endianess.

---
 drivers/mmc/core/mmc_ops.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux-2.6-arm/drivers/mmc/core/mmc_ops.c
===================================================================
@@ -207,6 +207,7 @@ mmc_send_cxd_data(struct mmc_host *host,
 	struct mmc_data data;
 	struct scatterlist sg;
 	void *data_buf;
+	int i;
 
 	/* dma onto stack is unsafe/nonportable, but callers here
 	 * normally provide temporary on-stack buffers.

@@ -246,7 +247,11 @@ mmc_send_cxd_data(struct mmc_host *host,
 
 	mmc_wait_for_req(host, &mrq);
 
-	memcpy(buf, data_buf, len);
+	for (i = 0; i < (len >> 2); i++) {
+		u32 *buf32 = buf;
+		buf32[i] = be32_to_cpu(*((u32 *)data_buf + i));
+	}
+
 	kfree(data_buf);
 
 	if (cmd.error != MMC_ERR_NONE)

Pengutronix - Linux Solutions for Science and Industry
Entwicklungszentrum Nord     http://www.pengutronix.de

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

The resp field should already be in host order, so there must be a bug in the mmc/spi host controller driver.

Rgds

     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

I wouldn't think it could be in host order ... what would have changed it from wire-order, if not for a patch like Sascha sent?

This sounds like exactly the failure I'd expect from bugs caused by omitted byteswapping, since I did all my recent work on a big-endian machine.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

I can confirm, that the byte-swapping patch from Sascha is needed on my little endian mips testbed too.

I can also see, that in earlier versions of mmc-spi, the byte swapping was present in there together with routine to read csd/cid. Now it is handled in mmc core, without byte swapping, so it does not work on little endian systems...

I've tested mmc-spi from linux-2.6.23-rc8-mm2, plus:
- added byte-swapping patch from Sascha
- added buffer length patch from Sascha as improved by David

This resulted in nearly working mmc-spi, except the problem with the last sector(s).
Tried to apply the fix to force single block read for last sector as suggested by Sascha, but it fails not with the last sector, but several of them at the end, with following error:
mmcblk0: error -22 sending stop command

Changing the patch to use single block reads for any of 8 sectors at the end of sd card, it works perfectly.

In addition to that, it seems, that retries are not enabled for sector access in mmc core, so I needed to add set of brq.cmd.retries into mmc_blk_issue_rq() and also checking of mrq->data->error and mrq->stop->error with call to host->ops->request() into mmc_request_done().

Otherwise, block transfers are never retried. And there are cases, that errors are caused by too long wires and high frequencies in my environment, so retries are sometimes very useful.

Finally I have tested also the suspend/resume callbacks of mmc core. As I am using sd card in embedded environment (assuming that sd card will never be removed), so MMC_UNSAFE_RESUME is good to me.

However, it seems to me, that the
- if (!blk_queue_plugged(q))

                        req = elv_next_request(q);

in mmc_queue_thread() is too kind to accept new requests, even when the queue is stopped.

This seems to be much better for faster suspending:
+ if (!blk_queue_plugged(q) && !blk_queue_stopped(q))

                        req = elv_next_request(q);

so that new requests are not accepted if queue is stopped or plugged.

See the complete patches bellow, please.

Thanks and best regards,
Jan

diff -urNp linux-2.6-orig/drivers/mmc/card/block.c linux-2.6/drivers/mmc/card/block.c
@@ -244,6 +244,16 @@ static int mmc_blk_issue_rq(struct mmc_q
 		    !mmc_card_sd(card))
 			brq.data.blocks = 1;
 
+		/* Some SD cards in SPI mode return a crc error when trying
+		 * to read the last block using a multiread command.
+		 */
+		if (mmc_host_is_spi(card->host)
+				&& brq.data.blocks > 1
+				&& rq_data_dir(req) == READ
+				&& req->sector + req->nr_sectors >=
+					get_capacity(md->disk)-8)
+			brq.data.blocks = 1;
+
 		if (brq.data.blocks > 1) {
 			/* SPI multiblock writes terminate using a special
 			 * token, not a STOP_TRANSMISSION request.

diff -urNp linux-2.6-orig/drivers/mmc/card/block.c linux-2.6/drivers/mmc/card/block.c
@@ -224,6 +224,7 @@ static int mmc_blk_issue_rq(struct mmc_q
 		if (!mmc_card_blockaddr(card))
 			brq.cmd.arg <<= 9;
 		brq.cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
+		brq.cmd.retries = 8;
 		brq.data.blksz = 1 << md->block_bits;
 		brq.stop.opcode = MMC_STOP_TRANSMISSION;
 		brq.stop.arg = 0;

diff -urNp linux-2.6-orig/drivers/mmc/core/core.c linux-2.6/drivers/mmc/core/core.c
@@ -102,6 +102,13 @@ void mmc_request_done(struct mmc_host *h
 			pr_debug("%s:     %d bytes transferred: %d\n",
 				mmc_hostname(host),
 				mrq->data->bytes_xfered, mrq->data->error);
+			if (mrq->data->error && cmd->retries) {
+				pr_debug("  ...DATA error: retrying...\n");
+				cmd->retries--;
+				cmd->error = 0;
+				mrq->data->error = 0;
+				host->ops->request(host, mrq);
+			}
 		}
 
 		if (mrq->stop) {

@@ -110,6 +117,13 @@ void mmc_request_done(struct mmc_host *h
 				mrq->stop->error,
 				mrq->stop->resp[0], mrq->stop->resp[1],
 				mrq->stop->resp[2], mrq->stop->resp[3]);
+			if (mrq->stop->error && cmd->retries) {
+				pr_debug("  ...STOP error: retrying...\n");
+				cmd->retries--;
+				cmd->error = 0;
+				mrq->stop->error = 0;
+				host->ops->request(host, mrq);
+			}
 		}
 
 		if (mrq->done)

diff -urNp linux-2.6-orig/drivers/mmc/card/queue.c linux-2.6/drivers/mmc/card/queue.c
@@ -53,7 +53,7 @@ static int mmc_queue_thread(void *d)
 
 		spin_lock_irq(q->queue_lock);
 		set_current_state(TASK_INTERRUPTIBLE);
-		if (!blk_queue_plugged(q))
+		if (!blk_queue_plugged(q) && !blk_queue_stopped(q))
 			req = elv_next_request(q);
 		mq->req = req;
 		spin_unlock_irq(q->queue_lock);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

_______________________________________________
spi-devel-general mailing list spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Odd. Could you point to the byte swapping in the earlier version?

This is extremely weird. How common is this? (i.e. how many cards have you tested and how many fail).

Also, do you have some easy test case so that I can check with my stash?

Then you need to fix your board. Retrying write with a translation layer in the middle (the FTL on the card) can cause all kinds of weird results. So it's up to upper layers how to handle failures.

I'd rather not muck about with working code unless there is an actual gain. Have you observed a problem caused by the queue flush?

Rgds

     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

It was present in mmc-spi update patch posted here for example:
http://sourceforge.net/mailarchive/message.php?msg_id=200706042025.18252.david-b%40pacbell.net
Check the mmc_spi_read_cXd(), there are be32_to_cpus() calls present.

well, it is the same for all 3 out of 3 different SD cards I have here:
mmcblk0: mmc0:0000 SD 999936KiB (myFlash Turbo SD 150x 1GB A-Data)
mmcblk0: mmc0:0000 SMI-S 126464KiB (X4store?)
mmcblk0: mmc0:0000 AF SD 249856KiB (X4store?)

I used just the dd command to read last 256 sectors (i.e.128KiB). With patch from Sascha, it reads only 248 sectors.
Using my modification, it reads full 256 sectors at the end of sd card.

The board is ok, it is due to that the sd card is on external board, connected wia cable, so it might happen rarely using 8.1MHz spi clock, that there is an error in the transfer. As said, it works in 99.9%. But when there is an error, retry is useful. The number of retries of 8 was a bit exaggerated, just one retry suffices for the setup I have here.
What do you mean by FTL on the card?

Without the !blk_queue_stopped(q) check, the suspend waits until dd command finishes it's job.
If the check is there, it's possible to suspend when dd command (or any other file copy command) is running and properly resume after.

Best regards,
Jan

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Ah. I see what you mean now. The problem is in the data transfer, not a response field. So Sacha's patch was almost correct. Could you test if the included patch solves your issue?

The wear leveling and other voodoo that translates block commands to NAND storage. If we send a write multiple times and the card fails it partially, then we have no idea what it has actually committed to NAND. Hence we play it safe and let upper layers handle the policy of retrying.

Not quite. It should just wait for the data dd has currently queued up (which should be 1 bs plus possible read-ahead).

OTOH, letting them finish reduces the number of dirty pages that might have to be committed to disk for a hibernation.

Is this really a problem that plagues you, or just something you noticed in the code?

Rgds

     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

index bf4bc6a..8b20e55 100644
@@ -267,15 +267,26 @@ mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host,
 
 int mmc_send_csd(struct mmc_card *card, u32 *csd)
 {
+	int ret, i;
+
 	if (!mmc_host_is_spi(card->host))
 		return mmc_send_cxd_native(card->host, card->rca << 16,
 				csd, MMC_SEND_CSD);
 
-	return mmc_send_cxd_data(card, card->host, MMC_SEND_CSD, csd, 16);
+	ret = mmc_send_cxd_data(card, card->host, MMC_SEND_CSD, csd, 16);
+	if (ret)
+		return ret;
+
+	for (i = 0;i < 4;i++)
+		csd[i] = be32_to_cpu(csd[i]);
+
+	return 0;
 }
 
 int mmc_send_cid(struct mmc_host *host, u32 *cid)
 {
+	int ret, i;
+
 	if (!mmc_host_is_spi(host)) {
 		if (!host->card)
 			return -EINVAL;

@@ -283,7 +294,14 @@ int mmc_send_cid(struct mmc_host *host, u32 *cid)
 				cid, MMC_SEND_CID);
 	}
 
-	return mmc_send_cxd_data(NULL, host, MMC_SEND_CID, cid, 16);
+	ret = mmc_send_cxd_data(NULL, host, MMC_SEND_CID, cid, 16);
+	if (ret)
+		return ret;
+
+	for (i = 0;i < 4;i++)
+		cid[i] = be32_to_cpu(cid[i]);
+
+	return 0;
 }
 
 int mmc_send_ext_csd(struct mmc_card *card, u8 *ext_csd)

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

_______________________________________________
spi-devel-general mailing list spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

With your patch it works as good as with the one from Sascha.

That's true.

It is that we need to suspend as soon as possible because of power loss. Waiting to flush all the caches does not help, because there will not be enough time with backup power. So it's better not to start any new block requests to safe as much backup power as possible and to avoid interrupted block transfer.

Best regards,
Jan

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Great. I'll send it to Linus then.

(Just in case it wasn't apparent, my patch was different from Sascha's as his flipped some stuff that shouldn't)

Ah, that is a very good point. Could I bother you with having a look at how other parts of the kernel does things?

Rgds

     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

That'll be interesting to know ... but I think the "panic suspend" should probably have different code paths than "normal suspend".

In the normal case, there's no power shortage *just now* and the safest shutdown sequence is appropriate. Notably, all removable media should have I/O be fully flushed, and the filesystem made consistent. And on resume, the filesystem should NOT be assumed to be in the same state it was on suspend.

In the panic case, like this, power may not be available, and the prayer is that some can be found before too long.

is handled all that well. If anything, there's an assumption that suspend is never done in a panic ... but that there's no need to make the filesystem fully consistent/clean since media aren't removable.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

I completely agree.

The thing with having a look at other parts of the kernel in this matter is that we do not use the latest kernel because of corporate policy unfortunately. So I need to backport useful things to 2.6.17 kernel:-(

Best regards,
Jan

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

This is the latest version of the MMC-over-SPI support. It works on 2.6.23-rc2 plus git-mmc (from rc1-mm2), along with the preceding patches which teach the rest of the MMC stack about SPI.

The main issue of note is that sometimes cards need to be power cycled to recover after certain faults. Also, it may sometimes be necessary to disable CRCs. ("modprobe mmc_core use_spi_crc=n")

---
 drivers/mmc/host/Kconfig    |   13 
 drivers/mmc/host/Makefile   |    1 
 drivers/mmc/host/mmc_spi.c  | 1398 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/mmc_spi.h |   33 +
 4 files changed, 1445 insertions(+)

@@ -100,3 +100,16 @@ config MMC_TIFM_SD
           To compile this driver as a module, choose M here: the
 	  module will be called tifm_sd.
 
+config MMC_SPI
+	tristate "MMC/SD over SPI (EXPERIMENTAL)"
+	depends on MMC && SPI_MASTER && !HIGHMEM && EXPERIMENTAL
+	select CRC7
+	select CRC_ITU_T
+	help
+	  Some systems accss MMC/SD cards using a SPI controller instead of
+	  using a "native" MMC/SD controller.  This has a disadvantage of
+	  being relatively high overhead, but a compensating advantage of
+	  working on many systems without dedicated MMC/SD controllers.
+
+	  If unsure, or if your system has no SPI master driver, say N.
+

@@ -15,4 +15,5 @@ obj-$(CONFIG_MMC_AU1X)		+= au1xmmc.o
 obj-$(CONFIG_MMC_OMAP)		+= omap.o
 obj-$(CONFIG_MMC_AT91)		+= at91_mci.o
 obj-$(CONFIG_MMC_TIFM_SD)	+= tifm_sd.o
+obj-$(CONFIG_MMC_SPI)		+= mmc_spi.o
 

@@ -0,0 +1,33 @@
+#ifndef __LINUX_SPI_MMC_SPI_H
+#define __LINUX_SPI_MMC_SPI_H
+
+struct device;
+struct mmc_host;
+
+/* Put this in platform_data of a device being used to manage an MMC/SD
+ * card slot.  (Modeled after PXA mmc glue; see that for usage examples.)
+ *
+ * REVISIT This is not a spi-specific notion.  Any card slot should be
+ * able to handle it.  If the MMC core doesn't adopt this kind of notion,
+ * switch the "struct device *" parameters over to "struct spi_device *".
+ */
+struct mmc_spi_platform_data {
+	/* driver activation and (optional) card detect irq hookup */
+	int (*init)(struct device *,
+		irqreturn_t (*)(int, void *),
+		void *);
+	void (*exit)(struct device *, void *);
+
+	/* sense switch on sd cards */
+	int (*get_ro)(struct device *);
+
+	/* how long to debounce card detect, in msecs */
+	u16 detect_delay;
+
+	/* power management */
+	u16 powerup_msecs;		/* delay of up to 250 msec */
+	u32 ocr_mask;			/* available voltages */
+	void (*setpower)(struct device *, unsigned int maskval);
+};
+
+#endif /* __LINUX_SPI_MMC_SPI_H */

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Hi,

On Wed, Aug 08, 2007 at 09:12:54AM -0700, David Brownell wrote:
+

---
 drivers/mmc/host/mmc_spi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6-arm/drivers/mmc/host/mmc_spi.c
===================================================================
@@ -886,7 +886,7 @@ mmc_spi_data_do(struct mmc_spi_host *hos
 		memset(scratch->status + 1, 0xff, statlen - 1);
 		host->early_status.tx_buf = host->early_status.rx_buf;
 		host->early_status.tx_dma = host->early_status.rx_dma;
-		host->early_status.len = 1 + statlen;
+		host->early_status.len = statlen;
 
 		if (host->dma_dev)
 			dma_sync_single_for_device(host->dma_dev,

Pengutronix - Linux Solutions for Science and Industry
Entwicklungszentrum Nord     http://www.pengutronix.de

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Right -- ACK. Although the code could be shrunk a smidgeon (and IMO made clearer) by doing the memset of the full buffer, then just overwriting the first byte with that token.

That would be an error on the READ path, yes? Because the SPI protocols only use a STOP command on multiblock reads.

You're sure? On my systems, error 2 == ENOENT, which is only reported when an MMC or SD card fails CID fetch...

Feeling brave, aren't you? :)

Haven't noticed such a beast myself. You can disable CRCs, that could help rule out CRCs as the cause (or finger them a bit more clearly).

- Dave

Could I get that patch instead? Pretty please? :)

Rgds

     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

======= CUT HERE
Small bugfix for an issue noted by Sascha Hauer <[email protected]>, whereby a buffer length was the passed with the wrong size. This also
includes a minor cleanup, to eliminate the error-prone +1/-1 stuff.

---
 drivers/mmc/host/mmc_spi.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

@@ -882,11 +882,12 @@ mmc_spi_data_do(struct mmc_spi_host *hos
 		list_add(&host->early_status.transfer_list,
 				&host->m.transfers);
 
+		memset(scratch->status, 0xff, statlen);
 		scratch->status[0] = SPI_TOKEN_STOP_TRAN;
-		memset(scratch->status + 1, 0xff, statlen - 1);
+
 		host->early_status.tx_buf = host->early_status.rx_buf;
 		host->early_status.tx_dma = host->early_status.rx_dma;
-		host->early_status.len = 1 + statlen;
+		host->early_status.len = statlen;
 
 		if (host->dma_dev)
 			dma_sync_single_for_device(host->dma_dev,

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

No, it's MMC_ERR_BADCRC. I don't use the patches from -mm at the moment, I noticed that they report errno instead of MMC_ERR_*.

The problem with the crc is reproducible. I watched this on the logic analyzer, it's the response from the card to a SPI_TOKEN_STOP_TRAN transfer. It occurs on the very last block on an SD card:

mmc0: new SD card at address 0000
mmcblk0: mmc0:0000 S016B 14560KiB
mmcblk0: p1

root@netx:~> cat /dev/mmcblk0p1 > /dev/null
mmcblk0: error 2 sending stop command
end_request: I/O error, dev mmcblk0, sector 29112 Buffer I/O error on device mmcblk0p1, logical block 3637
mmcblk0: error 5 sending read/write command
end_request: I/O error, dev mmcblk0, sector 29112 Buffer I/O error on device mmcblk0p1, logical block 3637
mmcblk0: error 5 sending read/write command
end_request: I/O error, dev mmcblk0, sector 29112 Buffer I/O error on device mmcblk0p1, logical block 3637
cat: read error: Input/output error root@netx:~>

14560 * 1024 - 29112 * 512 = 4096

I tested this with six SD cards. They all behave like this except one 512MB Kingston card which works. A MMC card I tested works too. Maybe we have to use a single block transfer on the last sector?

Sure, we do quality software here ;)

Sascha

Pengutronix - Linux Solutions for Science and Industry
Entwicklungszentrum Nord     http://www.pengutronix.de

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

You should use the current patchset. I think the rc3-mm1 git-mmc.patch includes the fix for that merge botch I noted, and I *know* it also includes a few notable fixes that earlier code didn't. (None that would seem to relate to this issue though.)

ISTR seeing some comment in a document pointing out that there can be some odd faults reported when reading that block. Maybe you've found a case where the mmc_spi code needs updating.

Could be. See what the current "Simplified SD" spec says ... I do recall language about reading that sector, but it didn't seem to matter for any of the cards I have for testing. So I probably didn't pay enough attention to those words.

I can only applaud! :)

None of the other protocol level SPI drivers seem to be quite as demanding of the underlying controller drivers, so in that sense it's a good testbed. On the other hand, the mmc-over-SPI code is a bit young, and various integration and corner-case issues are still to be expected.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Will do.

Unfortunately I did not find anything about that in the spec. If no other idea comes up I will try what happens when I read the last block using a single block read next week. The easiest way would be to fix it in the mmc block driver. That should not be too much overhead for 'real' SD cards, or is it?

btw what is the reason why mmc_spi requires an unshared bus? Even the SD spec talks about connecting multiple cards to one spi bus. Is it because of the CS high phase during initialization?

Sascha

Pengutronix - Linux Solutions for Science and Industry
Entwicklungszentrum Nord     http://www.pengutronix.de

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Did you verify that even with CRCs disabled, a problem appears?

I'm sure I saw that, but have no idea what document it was in. If not the "Simplified" spec, then a vendor's card spec ... either Sandisk (MMC, SD), Samsung, Hitachi, whatever.

I'd not think so. That's more Pierre's call though.

There are a bunch of chipselect games MMC plays. Notice how most of the commands require the card to drive MISO for at least eight clocks after they're deselected, too ... that means having the SPI host drive the clock during that period too, also shifting out ones on MOSI.

I'm remembering that one of the issues was the handling of BUSY status, which is a bit awkward but still remains one of the few places where it would make sense to drop the chipselect and access another device. But it still has those silly rules about "working" after chip deselect...

The canonical trouble spot is probably just reading a block. That's built out of any number of transactions, and it's not possible to know in advance how many it will be. So the host has to issue one, see if the card's got data ready; if not, repeat until it is; then finally process the actual data. All *without* dropping chipselect. And the same issue comes up with both single and multiblock reads.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Hi David,

Yes, this does not change anything.

Still did not find something about it. I looked in the simplified spec and in vendor specs from Toshiba and Sandisk.

I tried using a single block read for the last sector and this works for me. I would be glad if a third person confirms that he actually needs this patch as I'm still not sure whether theres a bug in my driver.

Here's the patch:

Some SD cards in SPI mode return a crc error when trying to read the last block using a multiple block read. Use a single block read instead.

---
 drivers/mmc/card/block.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-2.6-arm/drivers/mmc/card/block.c
===================================================================
@@ -244,6 +244,16 @@ static int mmc_blk_issue_rq(struct mmc_q
 		    !mmc_card_sd(card))
 			brq.data.blocks = 1;
 
+		/* Some SD cards in SPI mode return a crc error when trying
+		 * to read the last block using a multiread command.
+		 */
+		if (mmc_host_is_spi(card->host)
+				&& brq.data.blocks > 1
+				&& rq_data_dir(req) == READ
+				&& req->sector + req->nr_sectors ==
+					get_capacity(md->disk))
+			brq.data.blocks--;
+
 		if (brq.data.blocks > 1) {
 			brq.data.flags |= MMC_DATA_MULTI;
 			/* SPI multiblock writes terminate using a special

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

I suspect this is a case where a fault code gets overloaded. There aren't many codes defined, after all!

I'm still sure I saw it. Maybe I'll try searching for that text myself, it's surely in one of half a dozen huge documents I have sitting around in PDF form. ;)

I think there may be a regression in this area. When debugging this updated code, I was able to "dd" an entire SanDisk SD card without any errors. But when I tried this a short while ago, I saw the kind of error you report. I don't believe the SanDisk card suddenly broke.

Your patch prevented that problem from appearing ...

Now, taking only a very brief look at this, I wonder if maybe this isn't partly a symptom of an off-by-one error of some kind, with the current MMC stack asking for too much data. It must be requesting two blocks, since subtracting one turns it into a single block read ... but for some reason I count a multiblock read of eight blocks, indicating sg_len is eight not two.

Anyway, here's some debug output. It's the tail end of two DD sessions, one with a generic kernel and one with your patch. Only the latter one worked.

- Dave

==== #1

This is generic 2.6.24-rc2 (in terms of MMC patches), with mmc_spi debugging enabled

The card is a 128 MB SanDisk SD card. The command is:

# dd bs=4k if=/dev/mmcblk0 of=/dev/null

Gets a CRC error (-84, -EILSEQ) from R1_SPI_COM_CRC status (0x08) when issuing CMD12 (MMC_STOP_TRANSMISSION).

mmc_spi spi1.0:   mmc_spi: CMD18, resp R1
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:   mmc_spi: CMD12, resp R1B
mmc_spi spi1.0:   mmc_spi: CMD18, resp R1
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:   mmc_spi: CMD12, resp R1B
mmc_spi spi1.0:   ... CMD12 response SPI_R1B: resp 0008 00000000
mmcblk0: error -84 sending stop command
end_request: I/O error, dev mmcblk0, sector 246008
Buffer I/O error on device mmcblk0, logical block 30751
mmc_spi spi1.0:   mmc_spi: CMD18, resp R1
mmc_spi spi1.0:   ... CMD18 response SPI_R1: resp 0004 00000000
mmcblk0: error -22 sending read/write command
end_request: I/O error, dev mmcblk0, sector 246008

Buffer I/O error on device mmcblk0, logical block 30751

... and the DD command failed.

==== #2
This is generic 2.6.24-rc2 (in terms of MMC patches), with mmc_spi debugging enabled, plus the patch from Sascha Hauer.

The card is a 128 MB SanDisk SD card. The command is:

# dd bs=4k if=/dev/mmcblk0 of=/dev/null

No problems ... although note that there was only *ONE* block read at the end, where the failing case tried reading eight.

mmc_spi spi1.0:   mmc_spi: CMD18, resp R1
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:   mmc_spi: CMD12, resp R1B
mmc_spi spi1.0:   mmc_spi: CMD18, resp R1
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes
mmc_spi spi1.0:   mmc_spi: CMD12, resp R1B
mmc_spi spi1.0:   mmc_spi: CMD17, resp R1
mmc_spi spi1.0:     mmc_spi: read block, 512 bytes

... and the DD command succeeded.

Looks to me like an issue higher up in the stack than your driver.

(further attributions lost)

That kind of rung a bell, and I just had to dive into my virtual pile of MMC/SPI PDFs. At time of this writing, the document is still downloadable from www.sandisk.com. The site changed since I last was there, but once I just clicked my way to it:
<http://www.sandisk.com/Assets/File/OEM/Manuals/ProdManRS-MMCv1.3.pdf>.

In "Chapter 5 SPI Mode", section "5.14 Read Ahead in Multiple Block Read Operation" on page 74, it says: "In Multiple Block read operations, in order to improve read performance, the card may fetch data from the memory array, ahead of the host. In this case, when the host is reading the last addresses of the memory, the card attempts to fetch data beyond the last physical memory address and generates an OUT_OF_RANGE error. Therefore, even if the host times the Stop Transmission command to stop the card immediately after the last byte of data was read, the card may already have generated the error, which will show in the response to the Stop Transmission command. The host should ignore this error."

HTH.

brgds, H-P
PS. FWIW, I've not been near MMC/SPI programming since January.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

This seems to be what I was thinking about, thanks.

Yes, but it doesn't quite explain the specific failure mode being observed ... which is a CRC error on the STOP command, rather than an OUT_OF_RANGE token replacing start-of-data.

It's a small puzzle...

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Works great here. Thanks!

Anton Vorontsov
email: cbou-JGs/[email protected]
backup email: [email protected]
irc://irc.freenode.net/bd2

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

The emperor is pleased. Code merged.

Your great leader

     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/