linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/6] mmc: Field Firmware Update
@ 2015-11-13 14:56 Holger Schurig
  2015-11-13 14:56 ` [PATCH 1/6] mmc: move mmc_prepare_mrq() to core.c Holger Schurig
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Holger Schurig @ 2015-11-13 14:56 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, linux-kernel, avi.shchislowski, alex.lemberg
  Cc: Holger Schurig

There have been some attempts to add FFU (field firmware update).  The last
AFAIK in Nov 2014, http://www.spinics.net/lists/linux-mmc/msg29324.html

But it seems that the committers weren't persistent enought.

I took the liberty to take Avi's patch and make it hopefully
maintainer-review friendly.

The first 5 patches just move functions out of mmc_test.c into core.c. Those
functions will later be used by both mmc_test.c and mmc_ffu.c.  Contrary to
Avi's patch I didn't add static helper functions to mmc_test.c, e.g. 
there's no mmc_test_prepare_mrq() that calls mmc_prepare_mrq().  It's
simpler to call mmc_prepare_mrq() directly.  It's just one more dereference
from *mmc_card to *mmc_test_card anyway.

The patch [6/6] is http://www.spinics.net/lists/linux-mmc/msg29326.html, but
with less checkpatch warnings.  And it doesn't use mmc_send_ext_csd()
anymore, which has been deleted since November.

I'm sending this patch as RFC now. It compiles (for me). But I get the
firmware update file from Kingston only next Tuesday.  That means that so
far I haven't been testing it. It won't do anything without the proper
user-space command in mmc-utils anyway :-)

Comments welcome (I intent to get this patch into the kernel)

The patch is against Linux GIT (v4.3-11748-g46d862b).

Holger

 drivers/mmc/card/Kconfig    |  11 +
 drivers/mmc/card/Makefile   |   1 +
 drivers/mmc/card/block.c    |   5 +
 drivers/mmc/card/mmc_ffu.c  | 489 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/card/mmc_test.c | 235 +++++----------------
 drivers/mmc/core/core.c     | 134 ++++++++++++
 drivers/mmc/core/mmc_ops.c  |   4 +-
 include/linux/mmc/card.h    |   1 +
 include/linux/mmc/core.h    |  41 ++++
 include/linux/mmc/mmc.h     |   6 +
 10 files changed, 739 insertions(+), 188 deletions(-)
 create mode 100644 drivers/mmc/card/mmc_ffu.c

-- 
2.1.4


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

* [PATCH 1/6] mmc: move mmc_prepare_mrq() to core.c
  2015-11-13 14:56 [RFC 0/6] mmc: Field Firmware Update Holger Schurig
@ 2015-11-13 14:56 ` Holger Schurig
  2015-11-13 14:56 ` [PATCH 2/6] mmc: move mmc_wait_busy() to core Holger Schurig
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Holger Schurig @ 2015-11-13 14:56 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, linux-kernel, avi.shchislowski, alex.lemberg
  Cc: Holger Schurig

Currently this function is used inside the mmc test driver. But it is also
usable in the (upcoming) firmware update patch. So move this function out
of mmc_test.c into core.c.

Signed-off-by: Holger Schurig <holgerschurig@gmail.com>
---
 drivers/mmc/card/mmc_test.c | 48 ++++-----------------------------------------
 drivers/mmc/core/core.c     | 41 ++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/core.h    |  4 ++++
 3 files changed, 49 insertions(+), 44 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 7fc9174..69b6c45 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -184,46 +184,6 @@ static int mmc_test_set_blksize(struct mmc_test_card *test, unsigned size)
 	return mmc_set_blocklen(test->card, size);
 }
 
-/*
- * Fill in the mmc_request structure given a set of transfer parameters.
- */
-static void mmc_test_prepare_mrq(struct mmc_test_card *test,
-	struct mmc_request *mrq, struct scatterlist *sg, unsigned sg_len,
-	unsigned dev_addr, unsigned blocks, unsigned blksz, int write)
-{
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data || !mrq->stop);
-
-	if (blocks > 1) {
-		mrq->cmd->opcode = write ?
-			MMC_WRITE_MULTIPLE_BLOCK : MMC_READ_MULTIPLE_BLOCK;
-	} else {
-		mrq->cmd->opcode = write ?
-			MMC_WRITE_BLOCK : MMC_READ_SINGLE_BLOCK;
-	}
-
-	mrq->cmd->arg = dev_addr;
-	if (!mmc_card_blockaddr(test->card))
-		mrq->cmd->arg <<= 9;
-
-	mrq->cmd->flags = MMC_RSP_R1 | MMC_CMD_ADTC;
-
-	if (blocks == 1)
-		mrq->stop = NULL;
-	else {
-		mrq->stop->opcode = MMC_STOP_TRANSMISSION;
-		mrq->stop->arg = 0;
-		mrq->stop->flags = MMC_RSP_R1B | MMC_CMD_AC;
-	}
-
-	mrq->data->blksz = blksz;
-	mrq->data->blocks = blocks;
-	mrq->data->flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
-	mrq->data->sg = sg;
-	mrq->data->sg_len = sg_len;
-
-	mmc_set_data_timeout(mrq->data, test->card);
-}
-
 static int mmc_test_busy(struct mmc_command *cmd)
 {
 	return !(cmd->resp[0] & R1_READY_FOR_DATA) ||
@@ -281,7 +241,7 @@ static int mmc_test_buffer_transfer(struct mmc_test_card *test,
 
 	sg_init_one(&sg, buffer, blksz);
 
-	mmc_test_prepare_mrq(test, &mrq, &sg, 1, addr, 1, blksz, write);
+	mmc_prepare_mrq(test->card, &mrq, &sg, 1, addr, 1, blksz, write);
 
 	mmc_wait_for_req(test->card->host, &mrq);
 
@@ -805,7 +765,7 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
 	other_areq->err_check = mmc_test_check_result_async;
 
 	for (i = 0; i < count; i++) {
-		mmc_test_prepare_mrq(test, cur_areq->mrq, sg, sg_len, dev_addr,
+		mmc_prepare_mrq(test->card, cur_areq->mrq, sg, sg_len, dev_addr,
 				     blocks, blksz, write);
 		done_areq = mmc_start_req(test->card->host, cur_areq, &ret);
 
@@ -847,7 +807,7 @@ static int mmc_test_simple_transfer(struct mmc_test_card *test,
 	mrq.data = &data;
 	mrq.stop = &stop;
 
-	mmc_test_prepare_mrq(test, &mrq, sg, sg_len, dev_addr,
+	mmc_prepare_mrq(test->card, &mrq, sg, sg_len, dev_addr,
 		blocks, blksz, write);
 
 	mmc_wait_for_req(test->card->host, &mrq);
@@ -876,7 +836,7 @@ static int mmc_test_broken_transfer(struct mmc_test_card *test,
 
 	sg_init_one(&sg, test->buffer, blocks * blksz);
 
-	mmc_test_prepare_mrq(test, &mrq, &sg, 1, 0, blocks, blksz, write);
+	mmc_prepare_mrq(test->card, &mrq, &sg, 1, 0, blocks, blksz, write);
 	mmc_test_prepare_broken_mrq(test, &mrq, write);
 
 	mmc_wait_for_req(test->card->host, &mrq);
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5ae89e4..2b5e398 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2815,6 +2815,47 @@ int mmc_pm_notify(struct notifier_block *notify_block,
 }
 #endif
 
+/*
+ * Fill in the mmc_request structure given a set of transfer parameters.
+ */
+void mmc_prepare_mrq(struct mmc_card *card,
+	struct mmc_request *mrq, struct scatterlist *sg, unsigned sg_len,
+	unsigned dev_addr, unsigned blocks, unsigned blksz, int write)
+{
+	BUG_ON(!mrq || !mrq->cmd || !mrq->data || !mrq->stop);
+
+	if (blocks > 1) {
+		mrq->cmd->opcode = write ?
+			MMC_WRITE_MULTIPLE_BLOCK : MMC_READ_MULTIPLE_BLOCK;
+	} else {
+		mrq->cmd->opcode = write ?
+			MMC_WRITE_BLOCK : MMC_READ_SINGLE_BLOCK;
+	}
+
+	mrq->cmd->arg = dev_addr;
+	if (!mmc_card_blockaddr(card))
+		mrq->cmd->arg <<= 9;
+
+	mrq->cmd->flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+
+	if (blocks == 1)
+		mrq->stop = NULL;
+	else {
+		mrq->stop->opcode = MMC_STOP_TRANSMISSION;
+		mrq->stop->arg = 0;
+		mrq->stop->flags = MMC_RSP_R1B | MMC_CMD_AC;
+	}
+
+	mrq->data->blksz = blksz;
+	mrq->data->blocks = blocks;
+	mrq->data->flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
+	mrq->data->sg = sg;
+	mrq->data->sg_len = sg_len;
+
+	mmc_set_data_timeout(mrq->data, card);
+}
+EXPORT_SYMBOL(mmc_prepare_mrq);
+
 /**
  * mmc_init_context_info() - init synchronization context
  * @host: mmc host
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 37967b6..f8db960 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -196,6 +196,10 @@ extern int mmc_flush_cache(struct mmc_card *);
 
 extern int mmc_detect_card_removed(struct mmc_host *host);
 
+extern void mmc_prepare_mrq(struct mmc_card *card,
+	struct mmc_request *mrq, struct scatterlist *sg, unsigned sg_len,
+	unsigned dev_addr, unsigned blocks, unsigned blksz, int write);
+
 /**
  *	mmc_claim_host - exclusively claim a host
  *	@host: mmc host to claim
-- 
2.1.4


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

* [PATCH 2/6] mmc: move mmc_wait_busy() to core
  2015-11-13 14:56 [RFC 0/6] mmc: Field Firmware Update Holger Schurig
  2015-11-13 14:56 ` [PATCH 1/6] mmc: move mmc_prepare_mrq() to core.c Holger Schurig
@ 2015-11-13 14:56 ` Holger Schurig
  2015-11-13 14:56 ` [PATCH 3/6] mmc: move mmc_check_result() to core.c Holger Schurig
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Holger Schurig @ 2015-11-13 14:56 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, linux-kernel, avi.shchislowski, alex.lemberg
  Cc: Holger Schurig

Currently this function is used inside the mmc test driver. But it is also
usable in the (upcoming) firmware update patch. So move this function out
of mmc_test.c into core.c.

Signed-off-by: Holger Schurig <holgerschurig@gmail.com>
---
 drivers/mmc/card/mmc_test.c | 46 ++++-----------------------------------------
 drivers/mmc/core/core.c     | 38 +++++++++++++++++++++++++++++++++++++
 include/linux/mmc/core.h    |  1 +
 3 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 69b6c45..d1c2d22 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -184,44 +184,6 @@ static int mmc_test_set_blksize(struct mmc_test_card *test, unsigned size)
 	return mmc_set_blocklen(test->card, size);
 }
 
-static int mmc_test_busy(struct mmc_command *cmd)
-{
-	return !(cmd->resp[0] & R1_READY_FOR_DATA) ||
-		(R1_CURRENT_STATE(cmd->resp[0]) == R1_STATE_PRG);
-}
-
-/*
- * Wait for the card to finish the busy state
- */
-static int mmc_test_wait_busy(struct mmc_test_card *test)
-{
-	int ret, busy;
-	struct mmc_command cmd = {0};
-
-	busy = 0;
-	do {
-		memset(&cmd, 0, sizeof(struct mmc_command));
-
-		cmd.opcode = MMC_SEND_STATUS;
-		cmd.arg = test->card->rca << 16;
-		cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
-
-		ret = mmc_wait_for_cmd(test->card->host, &cmd, 0);
-		if (ret)
-			break;
-
-		if (!busy && mmc_test_busy(&cmd)) {
-			busy = 1;
-			if (test->card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
-				pr_info("%s: Warning: Host did not "
-					"wait for busy state to end.\n",
-					mmc_hostname(test->card->host));
-		}
-	} while (mmc_test_busy(&cmd));
-
-	return ret;
-}
-
 /*
  * Transfer a single sector of kernel addressable data
  */
@@ -250,7 +212,7 @@ static int mmc_test_buffer_transfer(struct mmc_test_card *test,
 	if (data.error)
 		return data.error;
 
-	return mmc_test_wait_busy(test);
+	return mmc_wait_busy(test->card);
 }
 
 static void mmc_test_free_mem(struct mmc_test_mem *mem)
@@ -675,7 +637,7 @@ static int mmc_test_check_result_async(struct mmc_card *card,
 	struct mmc_test_async_req *test_async =
 		container_of(areq, struct mmc_test_async_req, areq);
 
-	mmc_test_wait_busy(test_async->test);
+	mmc_wait_busy(test_async->test->card);
 
 	return mmc_test_check_result(test_async->test, areq->mrq);
 }
@@ -812,7 +774,7 @@ static int mmc_test_simple_transfer(struct mmc_test_card *test,
 
 	mmc_wait_for_req(test->card->host, &mrq);
 
-	mmc_test_wait_busy(test);
+	mmc_wait_busy(test->card);
 
 	return mmc_test_check_result(test, &mrq);
 }
@@ -841,7 +803,7 @@ static int mmc_test_broken_transfer(struct mmc_test_card *test,
 
 	mmc_wait_for_req(test->card->host, &mrq);
 
-	mmc_test_wait_busy(test);
+	mmc_wait_busy(test->card);
 
 	return mmc_test_check_broken_result(test, &mrq);
 }
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 2b5e398..df5a61c 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2856,6 +2856,44 @@ void mmc_prepare_mrq(struct mmc_card *card,
 }
 EXPORT_SYMBOL(mmc_prepare_mrq);
 
+static int mmc_test_busy(struct mmc_command *cmd)
+{
+	return !(cmd->resp[0] & R1_READY_FOR_DATA) ||
+		(R1_CURRENT_STATE(cmd->resp[0]) == R1_STATE_PRG);
+}
+
+/*
+ * Wait for the card to finish the busy state
+ */
+int mmc_wait_busy(struct mmc_card *card)
+{
+	int ret, busy;
+	struct mmc_command cmd = {0};
+
+	busy = 0;
+	do {
+		memset(&cmd, 0, sizeof(struct mmc_command));
+
+		cmd.opcode = MMC_SEND_STATUS;
+		cmd.arg = card->rca << 16;
+		cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
+
+		ret = mmc_wait_for_cmd(card->host, &cmd, 0);
+		if (ret)
+			break;
+
+		if (!busy && mmc_test_busy(&cmd)) {
+			busy = 1;
+			if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
+				pr_info("%s: Warning: Host did not wait for busy state to end.\n",
+					mmc_hostname(card->host));
+		}
+	} while (mmc_test_busy(&cmd));
+
+	return ret;
+}
+EXPORT_SYMBOL(mmc_wait_busy);
+
 /**
  * mmc_init_context_info() - init synchronization context
  * @host: mmc host
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f8db960..50e37e1 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -199,6 +199,7 @@ extern int mmc_detect_card_removed(struct mmc_host *host);
 extern void mmc_prepare_mrq(struct mmc_card *card,
 	struct mmc_request *mrq, struct scatterlist *sg, unsigned sg_len,
 	unsigned dev_addr, unsigned blocks, unsigned blksz, int write);
+extern int mmc_wait_busy(struct mmc_card *card);
 
 /**
  *	mmc_claim_host - exclusively claim a host
-- 
2.1.4


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

* [PATCH 3/6] mmc: move mmc_check_result() to core.c
  2015-11-13 14:56 [RFC 0/6] mmc: Field Firmware Update Holger Schurig
  2015-11-13 14:56 ` [PATCH 1/6] mmc: move mmc_prepare_mrq() to core.c Holger Schurig
  2015-11-13 14:56 ` [PATCH 2/6] mmc: move mmc_wait_busy() to core Holger Schurig
@ 2015-11-13 14:56 ` Holger Schurig
  2015-11-13 14:56 ` [PATCH 4/6] mmc: move mmc_simple_transfer() " Holger Schurig
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Holger Schurig @ 2015-11-13 14:56 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, linux-kernel, avi.shchislowski, alex.lemberg
  Cc: Holger Schurig

Currently this function is used inside the mmc test driver. But it is also
usable in the (upcoming) firmware update patch. So move this function out
of mmc_test.c into core.c.

This also adds global MMC_RESULT_ variables that show if some condition
stems from the host controller or the card/chip.  This is helpful to
the source in printk.

Signed-off-by: Holger Schurig <holgerschurig@gmail.com>
---
 drivers/mmc/card/mmc_test.c | 109 +++++++++++++++-----------------------------
 drivers/mmc/core/core.c     |  28 ++++++++++++
 include/linux/mmc/core.h    |   8 ++++
 3 files changed, 74 insertions(+), 71 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index d1c2d22..1d9d997 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -24,11 +24,6 @@
 #include <linux/seq_file.h>
 #include <linux/module.h>
 
-#define RESULT_OK		0
-#define RESULT_FAIL		1
-#define RESULT_UNSUP_HOST	2
-#define RESULT_UNSUP_CARD	3
-
 #define BUFFER_ORDER		2
 #define BUFFER_SIZE		(PAGE_SIZE << BUFFER_ORDER)
 
@@ -603,34 +598,6 @@ static void mmc_test_prepare_broken_mrq(struct mmc_test_card *test,
 	}
 }
 
-/*
- * Checks that a normal transfer didn't have any errors
- */
-static int mmc_test_check_result(struct mmc_test_card *test,
-				 struct mmc_request *mrq)
-{
-	int ret;
-
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data);
-
-	ret = 0;
-
-	if (!ret && mrq->cmd->error)
-		ret = mrq->cmd->error;
-	if (!ret && mrq->data->error)
-		ret = mrq->data->error;
-	if (!ret && mrq->stop && mrq->stop->error)
-		ret = mrq->stop->error;
-	if (!ret && mrq->data->bytes_xfered !=
-		mrq->data->blocks * mrq->data->blksz)
-		ret = RESULT_FAIL;
-
-	if (ret == -EINVAL)
-		ret = RESULT_UNSUP_HOST;
-
-	return ret;
-}
-
 static int mmc_test_check_result_async(struct mmc_card *card,
 				       struct mmc_async_req *areq)
 {
@@ -639,7 +606,7 @@ static int mmc_test_check_result_async(struct mmc_card *card,
 
 	mmc_wait_busy(test_async->test->card);
 
-	return mmc_test_check_result(test_async->test, areq->mrq);
+	return mmc_check_result(areq->mrq);
 }
 
 /*
@@ -657,21 +624,21 @@ static int mmc_test_check_broken_result(struct mmc_test_card *test,
 	if (!ret && mrq->cmd->error)
 		ret = mrq->cmd->error;
 	if (!ret && mrq->data->error == 0)
-		ret = RESULT_FAIL;
+		ret = MMC_RESULT_FAIL;
 	if (!ret && mrq->data->error != -ETIMEDOUT)
 		ret = mrq->data->error;
 	if (!ret && mrq->stop && mrq->stop->error)
 		ret = mrq->stop->error;
 	if (mrq->data->blocks > 1) {
 		if (!ret && mrq->data->bytes_xfered > mrq->data->blksz)
-			ret = RESULT_FAIL;
+			ret = MMC_RESULT_FAIL;
 	} else {
 		if (!ret && mrq->data->bytes_xfered > 0)
-			ret = RESULT_FAIL;
+			ret = MMC_RESULT_FAIL;
 	}
 
 	if (ret == -EINVAL)
-		ret = RESULT_UNSUP_HOST;
+		ret = MMC_RESULT_UNSUP_HOST;
 
 	return ret;
 }
@@ -776,7 +743,7 @@ static int mmc_test_simple_transfer(struct mmc_test_card *test,
 
 	mmc_wait_busy(test->card);
 
-	return mmc_test_check_result(test, &mrq);
+	return mmc_check_result(&mrq);
 }
 
 /*
@@ -865,12 +832,12 @@ static int mmc_test_transfer(struct mmc_test_card *test,
 
 		for (i = 0;i < blocks * blksz;i++) {
 			if (test->buffer[i] != (u8)i)
-				return RESULT_FAIL;
+				return MMC_RESULT_FAIL;
 		}
 
 		for (;i < sectors * 512;i++) {
 			if (test->buffer[i] != 0xDF)
-				return RESULT_FAIL;
+				return MMC_RESULT_FAIL;
 		}
 	} else {
 		local_irq_save(flags);
@@ -878,7 +845,7 @@ static int mmc_test_transfer(struct mmc_test_card *test,
 		local_irq_restore(flags);
 		for (i = 0;i < blocks * blksz;i++) {
 			if (test->scratch[i] != (u8)i)
-				return RESULT_FAIL;
+				return MMC_RESULT_FAIL;
 		}
 	}
 
@@ -949,7 +916,7 @@ static int mmc_test_multi_write(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (test->card->host->max_blk_count == 1)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	size = PAGE_SIZE * 2;
 	size = min(size, test->card->host->max_req_size);
@@ -957,7 +924,7 @@ static int mmc_test_multi_write(struct mmc_test_card *test)
 	size = min(size, test->card->host->max_blk_count * 512);
 
 	if (size < 1024)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	sg_init_one(&sg, test->buffer, size);
 
@@ -970,7 +937,7 @@ static int mmc_test_multi_read(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (test->card->host->max_blk_count == 1)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	size = PAGE_SIZE * 2;
 	size = min(size, test->card->host->max_req_size);
@@ -978,7 +945,7 @@ static int mmc_test_multi_read(struct mmc_test_card *test)
 	size = min(size, test->card->host->max_blk_count * 512);
 
 	if (size < 1024)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	sg_init_one(&sg, test->buffer, size);
 
@@ -991,7 +958,7 @@ static int mmc_test_pow2_write(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (!test->card->csd.write_partial)
-		return RESULT_UNSUP_CARD;
+		return MMC_RESULT_UNSUP_CARD;
 
 	for (i = 1; i < 512;i <<= 1) {
 		sg_init_one(&sg, test->buffer, i);
@@ -1009,7 +976,7 @@ static int mmc_test_pow2_read(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (!test->card->csd.read_partial)
-		return RESULT_UNSUP_CARD;
+		return MMC_RESULT_UNSUP_CARD;
 
 	for (i = 1; i < 512;i <<= 1) {
 		sg_init_one(&sg, test->buffer, i);
@@ -1027,7 +994,7 @@ static int mmc_test_weird_write(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (!test->card->csd.write_partial)
-		return RESULT_UNSUP_CARD;
+		return MMC_RESULT_UNSUP_CARD;
 
 	for (i = 3; i < 512;i += 7) {
 		sg_init_one(&sg, test->buffer, i);
@@ -1045,7 +1012,7 @@ static int mmc_test_weird_read(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (!test->card->csd.read_partial)
-		return RESULT_UNSUP_CARD;
+		return MMC_RESULT_UNSUP_CARD;
 
 	for (i = 3; i < 512;i += 7) {
 		sg_init_one(&sg, test->buffer, i);
@@ -1094,7 +1061,7 @@ static int mmc_test_align_multi_write(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (test->card->host->max_blk_count == 1)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	size = PAGE_SIZE * 2;
 	size = min(size, test->card->host->max_req_size);
@@ -1102,7 +1069,7 @@ static int mmc_test_align_multi_write(struct mmc_test_card *test)
 	size = min(size, test->card->host->max_blk_count * 512);
 
 	if (size < 1024)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	for (i = 1; i < TEST_ALIGN_END; i++) {
 		sg_init_one(&sg, test->buffer + i, size);
@@ -1121,7 +1088,7 @@ static int mmc_test_align_multi_read(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (test->card->host->max_blk_count == 1)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	size = PAGE_SIZE * 2;
 	size = min(size, test->card->host->max_req_size);
@@ -1129,7 +1096,7 @@ static int mmc_test_align_multi_read(struct mmc_test_card *test)
 	size = min(size, test->card->host->max_blk_count * 512);
 
 	if (size < 1024)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	for (i = 1; i < TEST_ALIGN_END; i++) {
 		sg_init_one(&sg, test->buffer + i, size);
@@ -1168,7 +1135,7 @@ static int mmc_test_multi_xfersize_write(struct mmc_test_card *test)
 	int ret;
 
 	if (test->card->host->max_blk_count == 1)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	ret = mmc_test_set_blksize(test, 512);
 	if (ret)
@@ -1182,7 +1149,7 @@ static int mmc_test_multi_xfersize_read(struct mmc_test_card *test)
 	int ret;
 
 	if (test->card->host->max_blk_count == 1)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	ret = mmc_test_set_blksize(test, 512);
 	if (ret)
@@ -1219,7 +1186,7 @@ static int mmc_test_multi_write_high(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (test->card->host->max_blk_count == 1)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	size = PAGE_SIZE * 2;
 	size = min(size, test->card->host->max_req_size);
@@ -1227,7 +1194,7 @@ static int mmc_test_multi_write_high(struct mmc_test_card *test)
 	size = min(size, test->card->host->max_blk_count * 512);
 
 	if (size < 1024)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	sg_init_table(&sg, 1);
 	sg_set_page(&sg, test->highmem, size, 0);
@@ -1241,7 +1208,7 @@ static int mmc_test_multi_read_high(struct mmc_test_card *test)
 	struct scatterlist sg;
 
 	if (test->card->host->max_blk_count == 1)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	size = PAGE_SIZE * 2;
 	size = min(size, test->card->host->max_req_size);
@@ -1249,7 +1216,7 @@ static int mmc_test_multi_read_high(struct mmc_test_card *test)
 	size = min(size, test->card->host->max_blk_count * 512);
 
 	if (size < 1024)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	sg_init_table(&sg, 1);
 	sg_set_page(&sg, test->highmem, size, 0);
@@ -1615,10 +1582,10 @@ static int mmc_test_profile_trim_perf(struct mmc_test_card *test)
 	int ret;
 
 	if (!mmc_can_trim(test->card))
-		return RESULT_UNSUP_CARD;
+		return MMC_RESULT_UNSUP_CARD;
 
 	if (!mmc_can_erase(test->card))
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	for (sz = 512; sz < t->max_sz; sz <<= 1) {
 		dev_addr = t->dev_addr + (sz >> 9);
@@ -1732,10 +1699,10 @@ static int mmc_test_profile_seq_trim_perf(struct mmc_test_card *test)
 	int ret;
 
 	if (!mmc_can_trim(test->card))
-		return RESULT_UNSUP_CARD;
+		return MMC_RESULT_UNSUP_CARD;
 
 	if (!mmc_can_erase(test->card))
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
 	for (sz = 512; sz <= t->max_sz; sz <<= 1) {
 		ret = mmc_test_area_erase(test);
@@ -2193,11 +2160,11 @@ static int mmc_test_reset(struct mmc_test_card *test)
 
 	err = mmc_hw_reset(host);
 	if (!err)
-		return RESULT_OK;
+		return MMC_RESULT_OK;
 	else if (err == -EOPNOTSUPP)
-		return RESULT_UNSUP_HOST;
+		return MMC_RESULT_UNSUP_HOST;
 
-	return RESULT_FAIL;
+	return MMC_RESULT_FAIL;
 }
 
 static const struct mmc_test_case mmc_test_cases[] = {
@@ -2584,20 +2551,20 @@ static void mmc_test_run(struct mmc_test_card *test, int testcase)
 
 		ret = mmc_test_cases[i].run(test);
 		switch (ret) {
-		case RESULT_OK:
+		case MMC_RESULT_OK:
 			pr_info("%s: Result: OK\n",
 				mmc_hostname(test->card->host));
 			break;
-		case RESULT_FAIL:
+		case MMC_RESULT_FAIL:
 			pr_info("%s: Result: FAILED\n",
 				mmc_hostname(test->card->host));
 			break;
-		case RESULT_UNSUP_HOST:
+		case MMC_RESULT_UNSUP_HOST:
 			pr_info("%s: Result: UNSUPPORTED "
 				"(by host)\n",
 				mmc_hostname(test->card->host));
 			break;
-		case RESULT_UNSUP_CARD:
+		case MMC_RESULT_UNSUP_CARD:
 			pr_info("%s: Result: UNSUPPORTED "
 				"(by card)\n",
 				mmc_hostname(test->card->host));
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index df5a61c..3254bce 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2894,6 +2894,34 @@ int mmc_wait_busy(struct mmc_card *card)
 }
 EXPORT_SYMBOL(mmc_wait_busy);
 
+/*
+ * Checks that a normal transfer didn't have any errors
+ */
+int mmc_check_result(struct mmc_request *mrq)
+{
+	int ret;
+
+	BUG_ON(!mrq || !mrq->cmd || !mrq->data);
+
+	ret = 0;
+
+	if (!ret && mrq->cmd->error)
+		ret = mrq->cmd->error;
+	if (!ret && mrq->data->error)
+		ret = mrq->data->error;
+	if (!ret && mrq->stop && mrq->stop->error)
+		ret = mrq->stop->error;
+	if (!ret && mrq->data->bytes_xfered !=
+		mrq->data->blocks * mrq->data->blksz)
+		ret = MMC_RESULT_FAIL;
+
+	if (ret == -EINVAL)
+		ret = MMC_RESULT_UNSUP_HOST;
+
+	return ret;
+}
+EXPORT_SYMBOL(mmc_check_result);
+
 /**
  * mmc_init_context_info() - init synchronization context
  * @host: mmc host
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 50e37e1..a5ad2d8 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -81,6 +81,13 @@ struct mmc_command {
 	unsigned int		retries;	/* max number of retries */
 	int			error;		/* command error */
 
+/* for mmc_check_result() */
+#define MMC_RESULT_OK		0
+#define MMC_RESULT_FAIL		1
+#define MMC_RESULT_UNSUP_HOST	2
+#define MMC_RESULT_UNSUP_CARD	3
+
+
 /*
  * Standard errno values are used for errors, but some have specific
  * meaning in the MMC layer:
@@ -200,6 +207,7 @@ extern void mmc_prepare_mrq(struct mmc_card *card,
 	struct mmc_request *mrq, struct scatterlist *sg, unsigned sg_len,
 	unsigned dev_addr, unsigned blocks, unsigned blksz, int write);
 extern int mmc_wait_busy(struct mmc_card *card);
+extern int mmc_check_result(struct mmc_request *mrq);
 
 /**
  *	mmc_claim_host - exclusively claim a host
-- 
2.1.4


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

* [PATCH 4/6] mmc: move mmc_simple_transfer() to core.c
  2015-11-13 14:56 [RFC 0/6] mmc: Field Firmware Update Holger Schurig
                   ` (2 preceding siblings ...)
  2015-11-13 14:56 ` [PATCH 3/6] mmc: move mmc_check_result() to core.c Holger Schurig
@ 2015-11-13 14:56 ` Holger Schurig
  2015-11-13 14:56 ` [PATCH 5/6] mmc: move mmc_send_cxd_data() " Holger Schurig
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Holger Schurig @ 2015-11-13 14:56 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, linux-kernel, avi.shchislowski, alex.lemberg
  Cc: Holger Schurig

Currently this function is used inside the mmc test driver. But it is also
usable in the (upcoming) firmware update patch. So move this function out
of mmc_test.c into core.c.

Signed-off-by: Holger Schurig <holgerschurig@gmail.com>
---
 drivers/mmc/card/mmc_test.c | 38 ++++++--------------------------------
 drivers/mmc/core/core.c     | 27 +++++++++++++++++++++++++++
 include/linux/mmc/core.h    |  3 +++
 3 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 1d9d997..94298fa 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -721,32 +721,6 @@ err:
 }
 
 /*
- * Tests a basic transfer with certain parameters
- */
-static int mmc_test_simple_transfer(struct mmc_test_card *test,
-	struct scatterlist *sg, unsigned sg_len, unsigned dev_addr,
-	unsigned blocks, unsigned blksz, int write)
-{
-	struct mmc_request mrq = {0};
-	struct mmc_command cmd = {0};
-	struct mmc_command stop = {0};
-	struct mmc_data data = {0};
-
-	mrq.cmd = &cmd;
-	mrq.data = &data;
-	mrq.stop = &stop;
-
-	mmc_prepare_mrq(test->card, &mrq, sg, sg_len, dev_addr,
-		blocks, blksz, write);
-
-	mmc_wait_for_req(test->card->host, &mrq);
-
-	mmc_wait_busy(test->card);
-
-	return mmc_check_result(&mrq);
-}
-
-/*
  * Tests a transfer where the card will fail completely or partly
  */
 static int mmc_test_broken_transfer(struct mmc_test_card *test,
@@ -801,7 +775,7 @@ static int mmc_test_transfer(struct mmc_test_card *test,
 	if (ret)
 		return ret;
 
-	ret = mmc_test_simple_transfer(test, sg, sg_len, dev_addr,
+	ret = mmc_simple_transfer(test->card, sg, sg_len, dev_addr,
 		blocks, blksz, write);
 	if (ret)
 		return ret;
@@ -875,7 +849,7 @@ static int mmc_test_basic_write(struct mmc_test_card *test)
 
 	sg_init_one(&sg, test->buffer, 512);
 
-	return mmc_test_simple_transfer(test, &sg, 1, 0, 1, 512, 1);
+	return mmc_simple_transfer(test->card, &sg, 1, 0, 1, 512, 1);
 }
 
 static int mmc_test_basic_read(struct mmc_test_card *test)
@@ -889,7 +863,7 @@ static int mmc_test_basic_read(struct mmc_test_card *test)
 
 	sg_init_one(&sg, test->buffer, 512);
 
-	return mmc_test_simple_transfer(test, &sg, 1, 0, 1, 512, 0);
+	return mmc_simple_transfer(test->card, &sg, 1, 0, 1, 512, 0);
 }
 
 static int mmc_test_verify_write(struct mmc_test_card *test)
@@ -898,7 +872,7 @@ static int mmc_test_verify_write(struct mmc_test_card *test)
 
 	sg_init_one(&sg, test->buffer, 512);
 
-	return mmc_test_transfer(test, &sg, 1, 0, 1, 512, 1);
+	return mmc_simple_transfer(test->card, &sg, 1, 0, 1, 512, 1);
 }
 
 static int mmc_test_verify_read(struct mmc_test_card *test)
@@ -907,7 +881,7 @@ static int mmc_test_verify_read(struct mmc_test_card *test)
 
 	sg_init_one(&sg, test->buffer, 512);
 
-	return mmc_test_transfer(test, &sg, 1, 0, 1, 512, 0);
+	return mmc_simple_transfer(test->card, &sg, 1, 0, 1, 512, 0);
 }
 
 static int mmc_test_multi_write(struct mmc_test_card *test)
@@ -1268,7 +1242,7 @@ static int mmc_test_area_transfer(struct mmc_test_card *test,
 {
 	struct mmc_test_area *t = &test->area;
 
-	return mmc_test_simple_transfer(test, t->sg, t->sg_len, dev_addr,
+	return mmc_simple_transfer(test->card, t->sg, t->sg_len, dev_addr,
 					t->blocks, 512, write);
 }
 
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 3254bce..fbc59ad 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2922,6 +2922,33 @@ int mmc_check_result(struct mmc_request *mrq)
 }
 EXPORT_SYMBOL(mmc_check_result);
 
+/*
+ * Run a basic transfer
+ */
+int mmc_simple_transfer(struct mmc_card *card,
+	struct scatterlist *sg, unsigned sg_len, unsigned dev_addr,
+	unsigned blocks, unsigned blksz, int write)
+{
+	struct mmc_request mrq = {0};
+	struct mmc_command cmd = {0};
+	struct mmc_command stop = {0};
+	struct mmc_data data = {0};
+
+	mrq.cmd = &cmd;
+	mrq.data = &data;
+	mrq.stop = &stop;
+
+	mmc_prepare_mrq(card, &mrq, sg, sg_len, dev_addr,
+		blocks, blksz, write);
+
+	mmc_wait_for_req(card->host, &mrq);
+
+	mmc_wait_busy(card);
+
+	return mmc_check_result(&mrq);
+}
+EXPORT_SYMBOL(mmc_simple_transfer);
+
 /**
  * mmc_init_context_info() - init synchronization context
  * @host: mmc host
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index a5ad2d8..774846f 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -208,6 +208,9 @@ extern void mmc_prepare_mrq(struct mmc_card *card,
 	unsigned dev_addr, unsigned blocks, unsigned blksz, int write);
 extern int mmc_wait_busy(struct mmc_card *card);
 extern int mmc_check_result(struct mmc_request *mrq);
+extern int mmc_simple_transfer(struct mmc_card *test,
+	struct scatterlist *sg, unsigned sg_len, unsigned dev_addr,
+	unsigned blocks, unsigned blksz, int write);
 
 /**
  *	mmc_claim_host - exclusively claim a host
-- 
2.1.4


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

* [PATCH 5/6] mmc: move mmc_send_cxd_data() to core.c
  2015-11-13 14:56 [RFC 0/6] mmc: Field Firmware Update Holger Schurig
                   ` (3 preceding siblings ...)
  2015-11-13 14:56 ` [PATCH 4/6] mmc: move mmc_simple_transfer() " Holger Schurig
@ 2015-11-13 14:56 ` Holger Schurig
  2015-11-13 14:56 ` [PATCH 6/6] mmc: eMMC Field Firmware Update support Holger Schurig
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Holger Schurig @ 2015-11-13 14:56 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, linux-kernel, avi.shchislowski, alex.lemberg
  Cc: Holger Schurig

This function can be used to send ext_csd data towards the chip, which is
needed in the (upcoming) firmware update patch.

Signed-off-by: Holger Schurig <holgerschurig@gmail.com>
---
 drivers/mmc/core/mmc_ops.c | 4 ++--
 include/linux/mmc/core.h   | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 1f44426..d1d6abc 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -287,8 +287,7 @@ mmc_send_cxd_native(struct mmc_host *host, u32 arg, u32 *cxd, int opcode)
  * NOTE: void *buf, caller for the buf is required to use DMA-capable
  * buffer or on-stack buffer (with some overhead in callee).
  */
-static int
-mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host,
+int mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host,
 		u32 opcode, void *buf, unsigned len)
 {
 	struct mmc_request mrq = {NULL};
@@ -336,6 +335,7 @@ mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host,
 
 	return 0;
 }
+EXPORT_SYMBOL(mmc_send_cxd_data);
 
 int mmc_send_csd(struct mmc_card *card, u32 *csd)
 {
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 774846f..b0e0f15 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -162,6 +162,9 @@ extern void mmc_start_bkops(struct mmc_card *card, bool from_exception);
 extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
 extern int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
 extern int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
+extern int mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host,
+		u32 opcode, void *buf, unsigned len);
+
 
 #define MMC_ERASE_ARG		0x00000000
 #define MMC_SECURE_ERASE_ARG	0x80000000
-- 
2.1.4


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

* [PATCH 6/6] mmc: eMMC Field Firmware Update support
  2015-11-13 14:56 [RFC 0/6] mmc: Field Firmware Update Holger Schurig
                   ` (4 preceding siblings ...)
  2015-11-13 14:56 ` [PATCH 5/6] mmc: move mmc_send_cxd_data() " Holger Schurig
@ 2015-11-13 14:56 ` Holger Schurig
  2015-11-20 14:20 ` [RFC 0/6] mmc: Field Firmware Update Alan Cooper
  2015-11-23 11:40 ` Avi Shchislowski
  7 siblings, 0 replies; 21+ messages in thread
From: Holger Schurig @ 2015-11-13 14:56 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc, linux-kernel, avi.shchislowski, alex.lemberg
  Cc: Holger Schurig

The Field Firmware Update (FFU) feature is in the eMMC 5.0 spec, see
http://www.jedec.org/standards-documents/technology-focus-areas/flash-memory-ssds-ufs-emmc/e-mmc)

This adds a new ioctl MMC_FFU_INVOKE to transfer the new Firmware data from
user space (via udev firmware request) to the eMMC device and install the
new firmware.

This solution allows to:
- complete FFU as an atomic operation, without being interrupted by other IO
  requests (theoretically the firmware update could be done completely from
  userspace, just not atomic)
- not limited in firmware data size because it's using multiple write operations
- support of both EXT_CSD_MODE_OPERATION_CODES modes

Almost completely taken from Avi Shchislowsk/Alex Lemberg patch
"[PATCH 3/3]mmc: Support-FFU-for-eMMC-v5.0".

Signed-off-by: Holger Schurig <holgerschurig@gmail.com>
---
 drivers/mmc/card/Kconfig   |  11 +
 drivers/mmc/card/Makefile  |   1 +
 drivers/mmc/card/block.c   |   5 +
 drivers/mmc/card/mmc_ffu.c | 487 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/card.h   |   1 +
 include/linux/mmc/core.h   |  22 ++
 include/linux/mmc/mmc.h    |   6 +
 7 files changed, 533 insertions(+)
 create mode 100644 drivers/mmc/card/mmc_ffu.c

diff --git a/drivers/mmc/card/Kconfig b/drivers/mmc/card/Kconfig
index 5562308..b37937b 100644
--- a/drivers/mmc/card/Kconfig
+++ b/drivers/mmc/card/Kconfig
@@ -57,6 +57,17 @@ config SDIO_UART
 	  SDIO function driver for SDIO cards that implements the UART
 	  class, as well as the GPS class which appears like a UART.
 
+config MMC_FFU
+	bool "Field Firmware Update support"
+	depends on MMC != n
+	help
+	  Some eMMC 5.0 devices allow to update their firmware "in the
+	  field". This option enables support for this.
+
+	  If this is compiled in, you can use the mmc utility to request
+	  that the kernel loads the firmware via udev and writes it
+	  to the eMMC.
+
 config MMC_TEST
 	tristate "MMC host test driver"
 	help
diff --git a/drivers/mmc/card/Makefile b/drivers/mmc/card/Makefile
index c73b406..99a01e8 100644
--- a/drivers/mmc/card/Makefile
+++ b/drivers/mmc/card/Makefile
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_MMC_BLOCK)		+= mmc_block.o
 mmc_block-objs			:= block.o queue.o
+obj-$(CONFIG_MMC_FFU)		+= mmc_ffu.o
 obj-$(CONFIG_MMC_TEST)		+= mmc_test.o
 
 obj-$(CONFIG_SDIO_UART)		+= sdio_uart.o
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 23b6c8e..a49cfea 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -486,6 +486,11 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 	cmd.arg = idata->ic.arg;
 	cmd.flags = idata->ic.flags;
 
+	if (idata->ic.opcode == MMC_FFU_INVOKE_OP) {
+		err = mmc_ffu_invoke(card, idata->buf);
+		goto cmd_done;
+	}
+
 	if (idata->buf_bytes) {
 		data.sg = &sg;
 		data.sg_len = 1;
diff --git a/drivers/mmc/card/mmc_ffu.c b/drivers/mmc/card/mmc_ffu.c
new file mode 100644
index 0000000..8501460
--- /dev/null
+++ b/drivers/mmc/card/mmc_ffu.c
@@ -0,0 +1,487 @@
+/*
+ *  Copyright 2007-2008 Pierre Ossman
+ *
+ *  Modified by SanDisk Corp., Copyright (c) 2014 SanDisk Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program includes bug.h, card.h, host.h, mmc.h, scatterlist.h,
+ * slab.h, ffu.h & swap.h header files
+ * The original, unmodified version of this program - the mmc_test.c
+ * file - is obtained under the GPL v2.0 license that is available via
+ * http://www.gnu.org/licenses/,
+ * or http://www.opensource.org/licenses/gpl-2.0.php
+ */
+
+#include <linux/bug.h>
+#include <linux/errno.h>
+#include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/swap.h>
+#include <linux/firmware.h>
+
+/**
+ * struct mmc_ffu_pages - pages allocated by 'alloc_pages()'.
+ * @page: first page in the allocation
+ * @order: order of the number of pages allocated
+ */
+struct mmc_ffu_pages {
+	struct page *page;
+	unsigned int order;
+};
+
+/**
+ * struct mmc_ffu_mem - allocated memory.
+ * @arr: array of allocations
+ * @cnt: number of allocations
+ */
+struct mmc_ffu_mem {
+	struct mmc_ffu_pages *arr;
+	unsigned int cnt;
+};
+
+struct mmc_ffu_area {
+	unsigned long max_sz;
+	unsigned int max_tfr;
+	unsigned int max_segs;
+	unsigned int max_seg_sz;
+	unsigned int blocks;
+	unsigned int sg_len;
+	struct mmc_ffu_mem mem;
+	struct sg_table sgtable;
+};
+
+/*
+ * Map memory into a scatterlist.
+ */
+static unsigned int mmc_ffu_map_sg(struct mmc_ffu_mem *mem, int size,
+				struct scatterlist *sglist)
+{
+	struct scatterlist *sg = sglist;
+	unsigned int i;
+	unsigned long sz = size;
+	unsigned int sctr_len = 0;
+	unsigned long len;
+
+	for (i = 0; i < mem->cnt && sz; i++, sz -= len) {
+		len = PAGE_SIZE << mem->arr[i].order;
+
+		if (len > sz) {
+			len = sz;
+			sz = 0;
+		}
+
+		sg_set_page(sg, mem->arr[i].page, len, 0);
+		sg = sg_next(sg);
+		sctr_len++;
+	}
+
+	return sctr_len;
+}
+
+static void mmc_ffu_free_mem(struct mmc_ffu_mem *mem)
+{
+	if (!mem)
+		return;
+
+	while (mem->cnt--)
+		__free_pages(mem->arr[mem->cnt].page, mem->arr[mem->cnt].order);
+
+	kfree(mem->arr);
+}
+
+/*
+ * Cleanup struct mmc_ffu_area.
+ */
+static int mmc_ffu_area_cleanup(struct mmc_ffu_area *area)
+{
+	sg_free_table(&area->sgtable);
+	mmc_ffu_free_mem(&area->mem);
+	return 0;
+}
+
+/*
+ * Allocate a lot of memory, preferably max_sz but at least min_sz. In case
+ * there isn't much memory do not exceed 1/16th total low mem pages. Also do
+ * not exceed a maximum number of segments and try not to make segments much
+ * bigger than maximum segment size.
+ */
+static int mmc_ffu_alloc_mem(struct mmc_ffu_area *area, unsigned long min_sz)
+{
+	unsigned long max_page_cnt = DIV_ROUND_UP(area->max_tfr, PAGE_SIZE);
+	unsigned long min_page_cnt = DIV_ROUND_UP(min_sz, PAGE_SIZE);
+	unsigned long max_seg_page_cnt =
+		DIV_ROUND_UP(area->max_seg_sz, PAGE_SIZE);
+	unsigned long page_cnt = 0;
+	/* divide to not allocate unnecessary memory */
+	unsigned long limit = nr_free_buffer_pages() >> 4;
+
+	gfp_t flags = GFP_KERNEL | GFP_DMA | __GFP_NOWARN | __GFP_NORETRY;
+
+	if (max_page_cnt > limit) {
+		max_page_cnt = limit;
+		area->max_tfr = max_page_cnt * PAGE_SIZE;
+	}
+
+	if (min_page_cnt > max_page_cnt)
+		min_page_cnt = max_page_cnt;
+
+	if (area->max_segs * max_seg_page_cnt > max_page_cnt)
+		area->max_segs = DIV_ROUND_UP(max_page_cnt, max_seg_page_cnt);
+
+	area->mem.arr = kcalloc(area->max_segs, sizeof(struct mmc_ffu_pages),
+				GFP_KERNEL);
+	area->mem.cnt = 0;
+	if (!area->mem.arr)
+		goto out_free;
+
+	while (max_page_cnt) {
+		struct page *page;
+		unsigned int order;
+
+		order = get_order(max_seg_page_cnt << PAGE_SHIFT);
+
+		do {
+			page = alloc_pages(flags, order);
+		} while (!page && order--);
+
+		if (!page)
+			goto out_free;
+
+		area->mem.arr[area->mem.cnt].page = page;
+		area->mem.arr[area->mem.cnt].order = order;
+		area->mem.cnt++;
+		page_cnt += 1UL << order;
+		if (max_page_cnt <= (1UL << order))
+			break;
+		max_page_cnt -= 1UL << order;
+	}
+
+	if (page_cnt < min_page_cnt)
+		goto out_free;
+
+	return 0;
+
+out_free:
+	mmc_ffu_free_mem(&area->mem);
+	return -ENOMEM;
+}
+
+/*
+ * Initialize an area for data transfers.
+ * Copy the data to the allocated pages.
+ */
+static int mmc_ffu_area_init(struct mmc_ffu_area *area, struct mmc_card *card,
+			const u8 *data)
+{
+	int ret;
+	int i;
+	unsigned int length = 0, page_length;
+
+	ret = mmc_ffu_alloc_mem(area, 1);
+	for (i = 0; i < area->mem.cnt; i++) {
+		if (length > area->max_tfr) {
+			ret = -EINVAL;
+			goto out_free;
+		}
+		page_length = PAGE_SIZE << area->mem.arr[i].order;
+		memcpy(page_address(area->mem.arr[i].page), data + length,
+			min(area->max_tfr - length, page_length));
+		length += page_length;
+	}
+
+	ret = sg_alloc_table(&area->sgtable, area->mem.cnt, GFP_KERNEL);
+	if (ret)
+		goto out_free;
+
+	area->sg_len = mmc_ffu_map_sg(&area->mem, area->max_tfr,
+				area->sgtable.sgl);
+
+
+	return 0;
+
+out_free:
+	mmc_ffu_free_mem(&area->mem);
+	return ret;
+}
+
+static int mmc_ffu_write(struct mmc_card *card, const u8 *src, u32 arg,
+			int size)
+{
+	int rc;
+	struct mmc_ffu_area area = {0};
+	int block_size = card->ext_csd.data_sector_size;
+
+	area.max_segs = card->host->max_segs;
+	area.max_seg_sz = card->host->max_seg_size & ~(block_size - 1);
+
+	do {
+		area.max_tfr = size;
+		if (area.max_tfr >> 9 > card->host->max_blk_count)
+			area.max_tfr = card->host->max_blk_count << 9;
+		if (area.max_tfr > card->host->max_req_size)
+			area.max_tfr = card->host->max_req_size;
+		if (DIV_ROUND_UP(area.max_tfr, area.max_seg_sz) > area.max_segs)
+			area.max_tfr = area.max_segs * area.max_seg_sz;
+
+		rc = mmc_ffu_area_init(&area, card, src);
+		if (rc != 0)
+			goto exit;
+
+		rc = mmc_simple_transfer(card, area.sgtable.sgl, area.sg_len,
+			arg, area.max_tfr / block_size, block_size, 1);
+		mmc_ffu_area_cleanup(&area);
+		if (rc != 0) {
+			pr_err("%s mmc_ffu_simple_transfer %d\n", __func__, rc);
+			goto exit;
+		}
+		src += area.max_tfr;
+		size -= area.max_tfr;
+
+	} while (size > 0);
+
+exit:
+	return rc;
+}
+
+/*
+ * Flush all scheduled work from the MMC work queue.
+ * and initialize the MMC device
+ */
+static int mmc_ffu_restart(struct mmc_card *card)
+{
+	struct mmc_host *host = card->host;
+	int err = 0;
+
+	err = mmc_power_save_host(host);
+	if (err) {
+		pr_warn("%s: going to sleep failed (%d)!!!\n",
+			__func__, err);
+		goto exit;
+	}
+
+	err = mmc_power_restore_host(host);
+
+exit:
+
+	return err;
+}
+
+static int mmc_ffu_switch_mode(struct mmc_card *card, int mode)
+{
+	int err = 0;
+	int offset;
+
+	switch (mode) {
+	case MMC_FFU_MODE_SET:
+	case MMC_FFU_MODE_NORMAL:
+		offset = EXT_CSD_MODE_CONFIG;
+		break;
+	case MMC_FFU_INSTALL_SET:
+		offset = EXT_CSD_MODE_OPERATION_CODES;
+		mode = 0x1;
+		break;
+	default:
+		err = -EINVAL;
+		break;
+	}
+
+	if (err == 0) {
+		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+				offset, mode,
+				card->ext_csd.generic_cmd6_time);
+	}
+
+	return err;
+}
+
+static int mmc_ffu_install(struct mmc_card *card, u8 *ext_csd)
+{
+	int err;
+	u32 timeout;
+
+	/* check mode operation */
+	if (!card->ext_csd.ffu_mode_op) {
+		/* host switch back to work in normal MMC Read/Write commands */
+		err = mmc_ffu_switch_mode(card, MMC_FFU_MODE_NORMAL);
+		if (err) {
+			pr_err("FFU: %s: switch to normal mode error %d:\n",
+				mmc_hostname(card->host), err);
+			return err;
+		}
+
+		/* restart the eMMC */
+		err = mmc_ffu_restart(card);
+		if (err) {
+			pr_err("FFU: %s: install error %d:\n",
+				mmc_hostname(card->host), err);
+			return err;
+		}
+	} else {
+		timeout = ext_csd[EXT_CSD_OPERATION_CODE_TIMEOUT];
+		if (timeout == 0 || timeout > 0x17) {
+			timeout = 0x17;
+			pr_warn("FFU: %s: operation timeout out of range, using max timeout.\n",
+				mmc_hostname(card->host));
+		}
+
+		/* timeout is at millisecond resolution */
+		timeout = DIV_ROUND_UP((100 * (1 << timeout)), 1000);
+
+		/* set ext_csd to install mode */
+		err = mmc_ffu_switch_mode(card, MMC_FFU_INSTALL_SET);
+		if (err) {
+			pr_err("FFU: %s: error %d setting install mode\n",
+				mmc_hostname(card->host), err);
+			return err;
+		}
+	}
+
+	/* read ext_csd */
+	err =  mmc_send_cxd_data(card, card->host, MMC_SEND_EXT_CSD,
+				ext_csd, 512);
+	if (err) {
+		pr_err("FFU: %s: error %d sending ext_csd\n",
+			mmc_hostname(card->host), err);
+		return err;
+	}
+
+	/* return status */
+	err = ext_csd[EXT_CSD_FFU_STATUS];
+	if (err) {
+		pr_err("FFU: %s: error %d FFU install:\n",
+			mmc_hostname(card->host), err);
+		return  -EINVAL;
+	}
+
+	return 0;
+}
+
+int mmc_ffu_invoke(struct mmc_card *card, const char *name)
+{
+	u8 ext_csd[512];
+	int err;
+	u32 arg;
+	u32 fw_prog_bytes;
+	const struct firmware *fw;
+	int block_size = card->ext_csd.data_sector_size;
+
+	/* Check if FFU is supported */
+	if (!card->ext_csd.ffu_capable) {
+		pr_err("FFU: %s: error FFU is not supported %d rev %d\n",
+			mmc_hostname(card->host), card->ext_csd.ffu_capable,
+			card->ext_csd.rev);
+		return -EOPNOTSUPP;
+	}
+
+	if (strlen(name) > 512) {
+		pr_err("FFU: %s: %.20s is not a valid argument\n",
+			mmc_hostname(card->host), name);
+		return -EINVAL;
+	}
+
+	/* setup FW data buffer */
+	err = request_firmware(&fw, name, &card->dev);
+	if (err) {
+		pr_err("FFU: %s: Firmware request failed %d\n",
+			mmc_hostname(card->host), err);
+		return err;
+	}
+	if ((fw->size % block_size)) {
+		pr_warn("FFU: %s: Warning %zd firmware data size not aligned!\n",
+			mmc_hostname(card->host), fw->size);
+	}
+
+	mmc_get_card(card);
+
+	/* trigger flushing*/
+	err = mmc_flush_cache(card);
+	if (err) {
+		pr_err("FFU: %s: error %d flushing data\n",
+			mmc_hostname(card->host), err);
+		goto exit;
+	}
+
+	/* Read the EXT_CSD */
+	err = mmc_send_cxd_data(card, card->host, MMC_SEND_EXT_CSD,
+				ext_csd, 512);
+	if (err) {
+		pr_err("FFU: %s: error %d sending ext_csd\n",
+			mmc_hostname(card->host), err);
+		goto exit;
+	}
+
+	/* set CMD ARG */
+	arg = ext_csd[EXT_CSD_FFU_ARG] |
+		ext_csd[EXT_CSD_FFU_ARG + 1] << 8 |
+		ext_csd[EXT_CSD_FFU_ARG + 2] << 16 |
+		ext_csd[EXT_CSD_FFU_ARG + 3] << 24;
+
+	/* set device to FFU mode */
+	err = mmc_ffu_switch_mode(card, MMC_FFU_MODE_SET);
+	if (err) {
+		pr_err("FFU: %s: error %d FFU is not supported\n",
+			mmc_hostname(card->host), err);
+		goto exit;
+	}
+
+	err = mmc_ffu_write(card, fw->data, arg, fw->size);
+	if (err) {
+		pr_err("FFU: %s: write error %d\n",
+			mmc_hostname(card->host), err);
+		goto exit;
+	}
+	/* payload  will be checked only in op_mode supported */
+	if (card->ext_csd.ffu_mode_op) {
+		/* Read the EXT_CSD */
+		err = mmc_send_cxd_data(card, card->host, MMC_SEND_EXT_CSD,
+					ext_csd, 512);
+		if (err) {
+			pr_err("FFU: %s: error %d sending ext_csd\n",
+				mmc_hostname(card->host), err);
+			goto exit;
+		}
+
+		/* check that the eMMC has received the payload */
+		fw_prog_bytes = ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG] |
+			ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG + 1] << 8 |
+			ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG + 2] << 16 |
+			ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG + 3] << 24;
+
+		/*
+		 * convert sectors to bytes: multiply by -512B or 4KB as
+		 * required by the card
+		 */
+		fw_prog_bytes *=
+			block_size << (ext_csd[EXT_CSD_DATA_SECTOR_SIZE] * 3);
+		if (fw_prog_bytes != fw->size) {
+			err = -EINVAL;
+			pr_err("FFU: programmed sectors incorrect %d %zd\n",
+				fw_prog_bytes, fw->size);
+			goto exit;
+		}
+	}
+
+	err = mmc_ffu_install(card, ext_csd);
+	if (err) {
+		pr_err("FFU: %s: error firmware install %d\n",
+			mmc_hostname(card->host), err);
+		goto exit;
+	}
+
+exit:
+	if (err != 0) {
+		/* switch back to normal MMC read/write commands */
+		mmc_ffu_switch_mode(card, MMC_FFU_MODE_NORMAL);
+	}
+	release_firmware(fw);
+	mmc_put_card(card);
+	return err;
+}
+EXPORT_SYMBOL(mmc_ffu_invoke);
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index eb0151b..5b7e236 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -89,6 +89,7 @@ struct mmc_ext_csd {
 	unsigned int		boot_ro_lock;		/* ro lock support */
 	bool			boot_ro_lockable;
 	bool			ffu_capable;	/* Firmware upgrade support */
+	bool			ffu_mode_op;	/* FFU mode operation */
 #define MMC_FIRMWARE_LEN 8
 	u8			fwrev[MMC_FIRMWARE_LEN];  /* FW version */
 	u8			raw_exception_status;	/* 54 */
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index b0e0f15..27cbe75 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -230,4 +230,26 @@ struct device_node;
 extern u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
 extern int mmc_of_parse_voltage(struct device_node *np, u32 *mask);
 
+/*
+ * eMMC5.0 Field Firmware Update (FFU) opcodes
+*/
+#define MMC_FFU_INVOKE_OP 302
+
+#define MMC_FFU_MODE_SET 0x1
+#define MMC_FFU_MODE_NORMAL 0x0
+#define MMC_FFU_INSTALL_SET 0x2
+
+#ifdef CONFIG_MMC_FFU
+#define MMC_FFU_FEATURES 0x1
+#define FFU_FEATURES(ffu_features) (ffu_features & MMC_FFU_FEATURES)
+
+int mmc_ffu_invoke(struct mmc_card *card, const char *name);
+
+#else
+static inline int mmc_ffu_invoke(struct mmc_card *card, const char *name)
+{
+	return -ENOSYS;
+}
+#endif
+
 #endif /* LINUX_MMC_CORE_H */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 15f2c4a..bb40ebd 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -272,6 +272,9 @@ struct _mmc_csd {
  * EXT_CSD fields
  */
 
+#define EXT_CSD_FFU_STATUS		26	/* R */
+#define EXT_CSD_MODE_OPERATION_CODES	29	/* W */
+#define EXT_CSD_MODE_CONFIG		30	/* R/W */
 #define EXT_CSD_FLUSH_CACHE		32      /* W */
 #define EXT_CSD_CACHE_CTRL		33      /* R/W */
 #define EXT_CSD_POWER_OFF_NOTIFICATION	34	/* R/W */
@@ -330,6 +333,9 @@ struct _mmc_csd {
 #define EXT_CSD_CACHE_SIZE		249	/* RO, 4 bytes */
 #define EXT_CSD_PWR_CL_DDR_200_360	253	/* RO */
 #define EXT_CSD_FIRMWARE_VERSION	254	/* RO, 8 bytes */
+#define EXT_CSD_NUM_OF_FW_SEC_PROG	302	/* RO, 4 bytes */
+#define EXT_CSD_FFU_ARG			487	/* RO, 4 bytes */
+#define EXT_CSD_OPERATION_CODE_TIMEOUT	491	/* RO */
 #define EXT_CSD_SUPPORTED_MODE		493	/* RO */
 #define EXT_CSD_TAG_UNIT_SIZE		498	/* RO */
 #define EXT_CSD_DATA_TAG_SUPPORT	499	/* RO */
-- 
2.1.4


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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2015-11-13 14:56 [RFC 0/6] mmc: Field Firmware Update Holger Schurig
                   ` (5 preceding siblings ...)
  2015-11-13 14:56 ` [PATCH 6/6] mmc: eMMC Field Firmware Update support Holger Schurig
@ 2015-11-20 14:20 ` Alan Cooper
  2015-11-23 11:40 ` Avi Shchislowski
  7 siblings, 0 replies; 21+ messages in thread
From: Alan Cooper @ 2015-11-20 14:20 UTC (permalink / raw)
  To: Holger Schurig
  Cc: Ulf Hansson, linux-mmc, linux-kernel, avi.shchislowski, alex.lemberg

On Fri, Nov 13, 2015 at 9:56 AM, Holger Schurig <holgerschurig@gmail.com> wrote:
> There have been some attempts to add FFU (field firmware update).  The last
> AFAIK in Nov 2014, http://www.spinics.net/lists/linux-mmc/msg29324.html
>
> But it seems that the committers weren't persistent enought.
>
> I took the liberty to take Avi's patch and make it hopefully
> maintainer-review friendly.
>
> The first 5 patches just move functions out of mmc_test.c into core.c. Those
> functions will later be used by both mmc_test.c and mmc_ffu.c.  Contrary to
> Avi's patch I didn't add static helper functions to mmc_test.c, e.g.
> there's no mmc_test_prepare_mrq() that calls mmc_prepare_mrq().  It's
> simpler to call mmc_prepare_mrq() directly.  It's just one more dereference
> from *mmc_card to *mmc_test_card anyway.
>
> The patch [6/6] is http://www.spinics.net/lists/linux-mmc/msg29326.html, but
> with less checkpatch warnings.  And it doesn't use mmc_send_ext_csd()
> anymore, which has been deleted since November.
>
> I'm sending this patch as RFC now. It compiles (for me). But I get the
> firmware update file from Kingston only next Tuesday.  That means that so
> far I haven't been testing it. It won't do anything without the proper
> user-space command in mmc-utils anyway :-)
>
> Comments welcome (I intent to get this patch into the kernel)
>
> The patch is against Linux GIT (v4.3-11748-g46d862b).
>
> Holger
>
>  drivers/mmc/card/Kconfig    |  11 +
>  drivers/mmc/card/Makefile   |   1 +
>  drivers/mmc/card/block.c    |   5 +
>  drivers/mmc/card/mmc_ffu.c  | 489 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mmc/card/mmc_test.c | 235 +++++----------------
>  drivers/mmc/core/core.c     | 134 ++++++++++++
>  drivers/mmc/core/mmc_ops.c  |   4 +-
>  include/linux/mmc/card.h    |   1 +
>  include/linux/mmc/core.h    |  41 ++++
>  include/linux/mmc/mmc.h     |   6 +
>  10 files changed, 739 insertions(+), 188 deletions(-)
>  create mode 100644 drivers/mmc/card/mmc_ffu.c

The newly added ioctl MMC_IOC_MULTI_CMD allows user space to send
multiple commands
atomically, so mmu-utils may be a better place for this functionality.

Al

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

* RE: [RFC 0/6] mmc: Field Firmware Update
  2015-11-13 14:56 [RFC 0/6] mmc: Field Firmware Update Holger Schurig
                   ` (6 preceding siblings ...)
  2015-11-20 14:20 ` [RFC 0/6] mmc: Field Firmware Update Alan Cooper
@ 2015-11-23 11:40 ` Avi Shchislowski
       [not found]   ` <CAOpc7mHbM8EYYNRKR+pKnzc0wf1DR1ojX-iFuDWDYyquC3EVZg@mail.gmail.com>
  7 siblings, 1 reply; 21+ messages in thread
From: Avi Shchislowski @ 2015-11-23 11:40 UTC (permalink / raw)
  To: Holger Schurig, Ulf Hansson, linux-mmc, linux-kernel, Alex Lemberg

Hi Holger,

Thank you for resubmitting the FFU patches again.
We did couple of fixes since our last submission (in [RFC 6/6]), so we will post them soon.
Also, we would like to leave the original "signed-off-by" names in your FFU patch commits.
Could you please add the original:
Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
Signed-off-by: Alex Lemberg <Alex.Lemberg@sandisk.com>

Thanks,
Avi & Alex

> -----Original Message-----
> From: Holger Schurig [mailto:holgerschurig@gmail.com]
> Sent: Friday, November 13, 2015 4:56 PM
> To: Ulf Hansson; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Avi
> Shchislowski; Alex Lemberg
> Cc: Holger Schurig
> Subject: [RFC 0/6] mmc: Field Firmware Update
> 
> There have been some attempts to add FFU (field firmware update).  The last
> AFAIK in Nov 2014, http://www.spinics.net/lists/linux-mmc/msg29324.html
> 
> But it seems that the committers weren't persistent enought.
> 
> I took the liberty to take Avi's patch and make it hopefully maintainer-review
> friendly.
> 
> The first 5 patches just move functions out of mmc_test.c into core.c. Those
> functions will later be used by both mmc_test.c and mmc_ffu.c.  Contrary to
> Avi's patch I didn't add static helper functions to mmc_test.c, e.g.
> there's no mmc_test_prepare_mrq() that calls mmc_prepare_mrq().  It's
> simpler to call mmc_prepare_mrq() directly.  It's just one more dereference
> from *mmc_card to *mmc_test_card anyway.
> 
> The patch [6/6] is http://www.spinics.net/lists/linux-mmc/msg29326.html, but
> with less checkpatch warnings.  And it doesn't use mmc_send_ext_csd()
> anymore, which has been deleted since November.
> 
> I'm sending this patch as RFC now. It compiles (for me). But I get the firmware
> update file from Kingston only next Tuesday.  That means that so far I haven't
> been testing it. It won't do anything without the proper user-space command in
> mmc-utils anyway :-)
> 
> Comments welcome (I intent to get this patch into the kernel)
> 
> The patch is against Linux GIT (v4.3-11748-g46d862b).
> 
> Holger
> 
>  drivers/mmc/card/Kconfig    |  11 +
>  drivers/mmc/card/Makefile   |   1 +
>  drivers/mmc/card/block.c    |   5 +
>  drivers/mmc/card/mmc_ffu.c  | 489
> ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mmc/card/mmc_test.c | 235 +++++----------------
>  drivers/mmc/core/core.c     | 134 ++++++++++++
>  drivers/mmc/core/mmc_ops.c  |   4 +-
>  include/linux/mmc/card.h    |   1 +
>  include/linux/mmc/core.h    |  41 ++++
>  include/linux/mmc/mmc.h     |   6 +
>  10 files changed, 739 insertions(+), 188 deletions(-)  create mode 100644
> drivers/mmc/card/mmc_ffu.c
> 
> --
> 2.1.4


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

* Re: [RFC 0/6] mmc: Field Firmware Update
       [not found]   ` <CAOpc7mHbM8EYYNRKR+pKnzc0wf1DR1ojX-iFuDWDYyquC3EVZg@mail.gmail.com>
@ 2015-12-15 15:35     ` Ulf Hansson
  2015-12-22  8:15       ` Holger Schurig
  0 siblings, 1 reply; 21+ messages in thread
From: Ulf Hansson @ 2015-12-15 15:35 UTC (permalink / raw)
  To: Holger Schurig
  Cc: Avi Shchislowski, linux-mmc, linux-kernel, Alex Lemberg,
	Chris Ball, Baolin Wang

+Chris, Baolin

On 25 November 2015 at 09:56, Holger Schurig <holgerschurig@gmail.com> wrote:
> We had to modify the last patch a bit and was able to update a Kingston
> device from Firmware 0xba to 0xbd. But when doing this in user-space only is
> now the way to go, I should probably not submit it anymore?
>
> BTW, for this Kingston "EMMC16G-WL110-EBAU" the argument in the ext_csd is
> bogus. You need to use the "ffu_arg" hack from the mmc-utils patch and use
> argument 0x0000ffff. Sigh. They can't even stick to the standard.
>
>> Could you please add the original:
>
> Sorry, yes I could. But I won't, because if this can now be done reliably
> from user-space, there is no need to do this in kernel. So I guess the
> chances that a kernel solution get's applies is next to zero.

Hi Holger,

Sorry for the delay.

My advise right now is to try this out via the mmc ioctl in userspace,
yes. Although, if you encounter any issues with that approach that it
might not be reliable, I am open to look into the in-kernel solution
again.

Regarding mmc-utils as where I recommend you to implement this, I have
been thinking of moving this tool into the tools directory in the
kernel.

Baolin Wang, at Linaro is currently looking into this to see if it's
feasible. Ideas/comments are usual greatly appreciated.

Kind regards
Uffe

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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2015-12-15 15:35     ` Ulf Hansson
@ 2015-12-22  8:15       ` Holger Schurig
  2015-12-22  8:55         ` Ulf Hansson
  2015-12-28 14:12         ` Alex Lemberg
  0 siblings, 2 replies; 21+ messages in thread
From: Holger Schurig @ 2015-12-22  8:15 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Avi Shchislowski, linux-mmc, linux-kernel, Alex Lemberg,
	Chris Ball, Baolin Wang


> Sorry for the delay.

No problem, I was busy with many other projects as well.

> My advise right now is to try this out via the mmc ioctl in userspace,
> yes. Although, if you encounter any issues with that approach that it
> might not be reliable, I am open to look into the in-kernel solution
> again.

I managed to update my Kingston eMMCs with a slighly modified patch to
what I submitted. I however didn't bother to submit this, as I saw no
chance of getting it applied.

Also I once asked in the mailing list if there is some user-space
example of how to use the multi-block feature that is supposed to enable
this, but I haven't gotten an answer.

> Regarding mmc-utils as where I recommend you to implement this, I have
> been thinking of moving this tool into the tools directory in the
> kernel.

Sounds good to me.



Remotely related:

Do you know that some google people made their own version of mmc-utils
for ChromeOS? And the don't seem to give much effort in unification of
them?  As if the world wouldn't know how hard it can be to re-unite
things again, Google should know from their custom kernels they use on
Android ...  Sigh.


So you can

* git clone git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc-utils.git
* git clone https://chromium.googlesource.com/chromiumos/third_party/mmc-utils

currently.



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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2015-12-22  8:15       ` Holger Schurig
@ 2015-12-22  8:55         ` Ulf Hansson
  2015-12-22  8:57           ` Ulf Hansson
  2015-12-28 14:12         ` Alex Lemberg
  1 sibling, 1 reply; 21+ messages in thread
From: Ulf Hansson @ 2015-12-22  8:55 UTC (permalink / raw)
  To: Holger Schurig
  Cc: Avi Shchislowski, linux-mmc, linux-kernel, Alex Lemberg,
	Chris Ball, Baolin Wang, Gwendal Grignou, Grant Grundler,
	Seshagiri Holi, Jon Hunter

+ Gwendal, Grant, Olof, Seshagiri, Jon

On 22 December 2015 at 09:15, Holger Schurig <holgerschurig@gmail.com> wrote:
>
>> Sorry for the delay.
>
> No problem, I was busy with many other projects as well.
>
>> My advise right now is to try this out via the mmc ioctl in userspace,
>> yes. Although, if you encounter any issues with that approach that it
>> might not be reliable, I am open to look into the in-kernel solution
>> again.
>
> I managed to update my Kingston eMMCs with a slighly modified patch to
> what I submitted. I however didn't bother to submit this, as I saw no
> chance of getting it applied.
>
> Also I once asked in the mailing list if there is some user-space
> example of how to use the multi-block feature that is supposed to enable
> this, but I haven't gotten an answer.
>
>> Regarding mmc-utils as where I recommend you to implement this, I have
>> been thinking of moving this tool into the tools directory in the
>> kernel.
>
> Sounds good to me.
>
>
>
> Remotely related:
>
> Do you know that some google people made their own version of mmc-utils
> for ChromeOS? And the don't seem to give much effort in unification of
> them?  As if the world wouldn't know how hard it can be to re-unite
> things again, Google should know from their custom kernels they use on
> Android ...  Sigh.
>
>
> So you can
>
> * git clone git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc-utils.git
> * git clone https://chromium.googlesource.com/chromiumos/third_party/mmc-utils

Thanks for sharing the information. Some Chromium folkz has certainly
been helpful in upstreaming activities, but you are right that there
is more to be done for mmc utils.

Perhaps by moving the code into the kernel tools dir and finding
someone who actively wants to maintain it, will enable improved
upstream activities!?

Kind regards
Uffe

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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2015-12-22  8:55         ` Ulf Hansson
@ 2015-12-22  8:57           ` Ulf Hansson
  2015-12-22 18:44             ` Olof Johansson
  0 siblings, 1 reply; 21+ messages in thread
From: Ulf Hansson @ 2015-12-22  8:57 UTC (permalink / raw)
  To: Holger Schurig
  Cc: Avi Shchislowski, linux-mmc, linux-kernel, Alex Lemberg,
	Chris Ball, Baolin Wang, Gwendal Grignou, Grant Grundler,
	Seshagiri Holi, Jon Hunter, Olof Johansson

+ Olof (forgot to add him last time)

On 22 December 2015 at 09:55, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> + Gwendal, Grant, Olof, Seshagiri, Jon
>
> On 22 December 2015 at 09:15, Holger Schurig <holgerschurig@gmail.com> wrote:
>>
>>> Sorry for the delay.
>>
>> No problem, I was busy with many other projects as well.
>>
>>> My advise right now is to try this out via the mmc ioctl in userspace,
>>> yes. Although, if you encounter any issues with that approach that it
>>> might not be reliable, I am open to look into the in-kernel solution
>>> again.
>>
>> I managed to update my Kingston eMMCs with a slighly modified patch to
>> what I submitted. I however didn't bother to submit this, as I saw no
>> chance of getting it applied.
>>
>> Also I once asked in the mailing list if there is some user-space
>> example of how to use the multi-block feature that is supposed to enable
>> this, but I haven't gotten an answer.
>>
>>> Regarding mmc-utils as where I recommend you to implement this, I have
>>> been thinking of moving this tool into the tools directory in the
>>> kernel.
>>
>> Sounds good to me.
>>
>>
>>
>> Remotely related:
>>
>> Do you know that some google people made their own version of mmc-utils
>> for ChromeOS? And the don't seem to give much effort in unification of
>> them?  As if the world wouldn't know how hard it can be to re-unite
>> things again, Google should know from their custom kernels they use on
>> Android ...  Sigh.
>>
>>
>> So you can
>>
>> * git clone git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc-utils.git
>> * git clone https://chromium.googlesource.com/chromiumos/third_party/mmc-utils
>
> Thanks for sharing the information. Some Chromium folkz has certainly
> been helpful in upstreaming activities, but you are right that there
> is more to be done for mmc utils.
>
> Perhaps by moving the code into the kernel tools dir and finding
> someone who actively wants to maintain it, will enable improved
> upstream activities!?
>
> Kind regards
> Uffe

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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2015-12-22  8:57           ` Ulf Hansson
@ 2015-12-22 18:44             ` Olof Johansson
       [not found]               ` <CAG=SpYGf7czPwLrJVrq49n_Lhph33fmyKq-Nkm6=c0fB1Gn9_w@mail.gmail.com>
  0 siblings, 1 reply; 21+ messages in thread
From: Olof Johansson @ 2015-12-22 18:44 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Holger Schurig, Avi Shchislowski, linux-mmc, linux-kernel,
	Alex Lemberg, Chris Ball, Baolin Wang, Gwendal Grignou,
	Grant Grundler, Seshagiri Holi, Jon Hunter

[sigh, in cleartext this time.]


2015-12-22 0:57 GMT-08:00 Ulf Hansson <ulf.hansson@linaro.org>:
> + Olof (forgot to add him last time)
>
> On 22 December 2015 at 09:55, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> + Gwendal, Grant, Olof, Seshagiri, Jon
>>
>> On 22 December 2015 at 09:15, Holger Schurig <holgerschurig@gmail.com> wrote:
>>>
>>>> Sorry for the delay.
>>>
>>> No problem, I was busy with many other projects as well.
>>>
>>>> My advise right now is to try this out via the mmc ioctl in userspace,
>>>> yes. Although, if you encounter any issues with that approach that it
>>>> might not be reliable, I am open to look into the in-kernel solution
>>>> again.
>>>
>>> I managed to update my Kingston eMMCs with a slighly modified patch to
>>> what I submitted. I however didn't bother to submit this, as I saw no
>>> chance of getting it applied.
>>>
>>> Also I once asked in the mailing list if there is some user-space
>>> example of how to use the multi-block feature that is supposed to enable
>>> this, but I haven't gotten an answer.
>>>
>>>> Regarding mmc-utils as where I recommend you to implement this, I have
>>>> been thinking of moving this tool into the tools directory in the
>>>> kernel.
>>>
>>> Sounds good to me.
>>>
>>>
>>>
>>> Remotely related:
>>>
>>> Do you know that some google people made their own version of mmc-utils
>>> for ChromeOS?

"Make their own version" makes it sound like we've rewritten it. We
haven't. We've applied some patches locally over the last two years
based on work we've done for our products. It's not a deep fork.

We'd be happy to upstream them if there's interest from other users
and maintainers.

>> Perhaps by moving the code into the kernel tools dir and finding
>> someone who actively wants to maintain it, will enable improved
>> upstream activities!?


Hmm. If there's a well-defined userspace interface there's little
reason to bundle it directly, and I think that's the case here.

Chris has been busy with other stuff the last couple of years, is
there a lot of mmc-utils code that's out there that hasn't been picked
up?


-Olof

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

* RE: [RFC 0/6] mmc: Field Firmware Update
  2015-12-22  8:15       ` Holger Schurig
  2015-12-22  8:55         ` Ulf Hansson
@ 2015-12-28 14:12         ` Alex Lemberg
  2016-01-14 13:16           ` Ulf Hansson
  1 sibling, 1 reply; 21+ messages in thread
From: Alex Lemberg @ 2015-12-28 14:12 UTC (permalink / raw)
  To: Holger Schurig, Ulf Hansson
  Cc: Avi Shchislowski, linux-mmc, linux-kernel, Chris Ball, Baolin Wang

Hi Ulf,

We succeeded to run FFU via new mmc multi-command ioctl without any code modification, 
but only by using Single Sector commands (CMD24).

>From running the FFU and from code review, we see two minor issues in this way of running FFU:
1. There is no support for Multiple Block write commands (CMD25) in existing IOCTL implementation - 
seems like there is no polling for the card status on data transfer completion.
(The kernel FFU implementation supports FFU using Multiple Block Write commands).
2. As you probably remember, there are two ways to install the new FW in the end of FFU process - 
In case MODE_OPERATION_CODES field is not supported by the device, the host sets to NORMAL state 
and initiates a CMD0/HW_Reset/Power cycle to install the new firmware.
This sequence cannot be done via multi-command ioctl, and requires manual reset of the device/platform.
(The kernel FFU implementation supports both FW install methods).

For running FFU via new mmc multi-command ioctl, we have modified mmc-utils and add new functionality for FFU.
Please let us know if you want us to submit the patch for mmc-utils FFU functionality via multi-command ioctl.

Best regards,
Alex

> -----Original Message-----
> From: Holger Schurig [mailto:holgerschurig@gmail.com]
> Sent: Tuesday, December 22, 2015 10:15 AM
> To: Ulf Hansson
> Cc: Avi Shchislowski; linux-mmc@vger.kernel.org; linux-
> kernel@vger.kernel.org; Alex Lemberg; Chris Ball; Baolin Wang
> Subject: Re: [RFC 0/6] mmc: Field Firmware Update
> 
> 
> > Sorry for the delay.
> 
> No problem, I was busy with many other projects as well.
> 
> > My advise right now is to try this out via the mmc ioctl in userspace,
> > yes. Although, if you encounter any issues with that approach that it
> > might not be reliable, I am open to look into the in-kernel solution
> > again.
> 
> I managed to update my Kingston eMMCs with a slighly modified patch to
> what I submitted. I however didn't bother to submit this, as I saw no chance
> of getting it applied.
> 
> Also I once asked in the mailing list if there is some user-space example of
> how to use the multi-block feature that is supposed to enable this, but I
> haven't gotten an answer.
> 
> > Regarding mmc-utils as where I recommend you to implement this, I have
> > been thinking of moving this tool into the tools directory in the
> > kernel.
> 
> Sounds good to me.
> 
> 
> 
> Remotely related:
> 
> Do you know that some google people made their own version of mmc-utils
> for ChromeOS? And the don't seem to give much effort in unification of
> them?  As if the world wouldn't know how hard it can be to re-unite things
> again, Google should know from their custom kernels they use on Android ...
> Sigh.
> 
> 
> So you can
> 
> * git clone git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc-utils.git
> * git clone
> https://chromium.googlesource.com/chromiumos/third_party/mmc-utils
> 
> currently.
> 


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

* Re: [RFC 0/6] mmc: Field Firmware Update
       [not found]               ` <CAG=SpYGf7czPwLrJVrq49n_Lhph33fmyKq-Nkm6=c0fB1Gn9_w@mail.gmail.com>
@ 2016-01-14 12:50                 ` Ulf Hansson
  0 siblings, 0 replies; 21+ messages in thread
From: Ulf Hansson @ 2016-01-14 12:50 UTC (permalink / raw)
  To: Chris Ball
  Cc: Holger Schurig, Olof Johansson, Avi Shchislowski, linux-mmc,
	linux-kernel, Alex Lemberg, Baolin Wang, Gwendal Grignou,
	Grant Grundler, Seshagiri Holi, Jon Hunter

On 22 December 2015 at 19:51, Chris Ball <chris@printf.net> wrote:
> Hi,
>
>> Chris has been busy with other stuff the last couple of years, is there a
>> lot of mmc-utils code that's out there that hasn't been picked up?

I don't really know.

Although, I wanted to make sure that the tool still was still being
maintained as otherwise people sooner or later will not care to send
patches.

>
> I've been applying mmc-utils patches as I get them -- last push was five
> weeks ago.  So I don't have a backlog, but if people are sending mmc-utils
> patches to lists rather than to me directly, I'm unlikely to see them there.
>

I haven't bothered much at looking at mmc utils patches, although I
have noticed some being posted without any response or being picked
up.
I browsed the history of these patches and noticed that it's exactly
as you say. These haven't been sent directly you or they were sent to
an incorrect email address.

I apologize if I was giving the wrong impression that you didn't
maintain mmc utils very well, you certainly are!

Perhaps we should add some information about it in the mmc kernel doc,
to make it more visible.

Kind regards
Uffe

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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2015-12-28 14:12         ` Alex Lemberg
@ 2016-01-14 13:16           ` Ulf Hansson
  2016-04-02  0:23             ` Gwendal Grignou
  0 siblings, 1 reply; 21+ messages in thread
From: Ulf Hansson @ 2016-01-14 13:16 UTC (permalink / raw)
  To: Alex Lemberg
  Cc: Holger Schurig, Avi Shchislowski, linux-mmc, linux-kernel,
	Chris Ball, Baolin Wang

On 28 December 2015 at 15:12, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote:
> Hi Ulf,
>
> We succeeded to run FFU via new mmc multi-command ioctl without any code modification,
> but only by using Single Sector commands (CMD24).
>
> From running the FFU and from code review, we see two minor issues in this way of running FFU:
> 1. There is no support for Multiple Block write commands (CMD25) in existing IOCTL implementation -

That's right. But I guess we cope without the multiple block support!?

Although, I wonder how hard it would be to add it...

> seems like there is no polling for the card status on data transfer completion.

We should fix that!

In the rpmb case, we check the status so we can probably trigger that
code to run for CMD24/25 as well.

> (The kernel FFU implementation supports FFU using Multiple Block Write commands).
> 2. As you probably remember, there are two ways to install the new FW in the end of FFU process -
> In case MODE_OPERATION_CODES field is not supported by the device, the host sets to NORMAL state

Before starting the update, you can find out which mode that is
supported and take relevant actions, right?

> and initiates a CMD0/HW_Reset/Power cycle to install the new firmware.

Yes, but that's fragile - as discussed earlier.

What we really need to do is to also remove the "card" device from the
system, as otherwise we may have invalid data in its member variables
and who knows what issues that can cause to upper levels.

> This sequence cannot be done via multi-command ioctl, and requires manual reset of the device/platform.

Yepp, it seems so at least for now. Perhaps we can think of a way to
improve this?

> (The kernel FFU implementation supports both FW install methods).
>
> For running FFU via new mmc multi-command ioctl, we have modified mmc-utils and add new functionality for FFU.
> Please let us know if you want us to submit the patch for mmc-utils FFU functionality via multi-command ioctl.

Yes please. Don't forget to send this to Chris as well!

[...]

Kind regards
Uffe

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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2016-01-14 13:16           ` Ulf Hansson
@ 2016-04-02  0:23             ` Gwendal Grignou
  2016-04-04 11:50               ` Ulf Hansson
  0 siblings, 1 reply; 21+ messages in thread
From: Gwendal Grignou @ 2016-04-02  0:23 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Alex Lemberg, Holger Schurig, Avi Shchislowski, linux-mmc,
	linux-kernel, Chris Ball, Baolin Wang

On Thu, Jan 14, 2016 at 5:16 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 28 December 2015 at 15:12, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote:
>> Hi Ulf,
>>
>> We succeeded to run FFU via new mmc multi-command ioctl without any code modification,
>> but only by using Single Sector commands (CMD24).
>>
>> From running the FFU and from code review, we see two minor issues in this way of running FFU:
>> 1. There is no support for Multiple Block write commands (CMD25) in existing IOCTL implementation -
>
> That's right. But I guess we cope without the multiple block support!?
>
> Although, I wonder how hard it would be to add it...
>
>> seems like there is no polling for the card status on data transfer completion.
>
> We should fix that!
>
> In the rpmb case, we check the status so we can probably trigger that
> code to run for CMD24/25 as well.
>
>> (The kernel FFU implementation supports FFU using Multiple Block Write commands).
>> 2. As you probably remember, there are two ways to install the new FW in the end of FFU process -
>> In case MODE_OPERATION_CODES field is not supported by the device, the host sets to NORMAL state
>
> Before starting the update, you can find out which mode that is
> supported and take relevant actions, right?
>
>> and initiates a CMD0/HW_Reset/Power cycle to install the new firmware.
>
> Yes, but that's fragile - as discussed earlier.
>
> What we really need to do is to also remove the "card" device from the
> system, as otherwise we may have invalid data in its member variables
> and who knows what issues that can cause to upper levels.
>
>> This sequence cannot be done via multi-command ioctl, and requires manual reset of the device/platform.
>
> Yepp, it seems so at least for now. Perhaps we can think of a way to
> improve this?
>
>> (The kernel FFU implementation supports both FW install methods).
>>
>> For running FFU via new mmc multi-command ioctl, we have modified mmc-utils and add new functionality for FFU.
>> Please let us know if you want us to submit the patch for mmc-utils FFU functionality via multi-command ioctl.
>
> Yes please. Don't forget to send this to Chris as well!
I am arriving after the battle, but I have finally rebased the eMMC
FFU kernel ffu code to 4.x. It is based on what Avi and Alex have
written.
As stated earlier, the advantage over using MMC_MUTLI_CMD is we can
force a reset and rescan of the card without asking the user to reboot
their machine.
Also, by only sending a firmware name over the ioctl, we can use Kees'
work for firmware validation (https://lwn.net/Articles/605432/).
To prevent downloading firmware from unknown source, we would reject
some commands (like SWITCH with FFU_MODE) in the kernel
MMC_IOC/MULTI_CMD ioctl handler.

Gwendal.
>
> [...]
>
> Kind regards
> Uffe
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2016-04-02  0:23             ` Gwendal Grignou
@ 2016-04-04 11:50               ` Ulf Hansson
  2016-04-13 22:33                 ` Gwendal Grignou
  0 siblings, 1 reply; 21+ messages in thread
From: Ulf Hansson @ 2016-04-04 11:50 UTC (permalink / raw)
  To: Gwendal Grignou
  Cc: Alex Lemberg, Holger Schurig, Avi Shchislowski, linux-mmc,
	linux-kernel, Chris Ball, Baolin Wang

On 2 April 2016 at 02:23, Gwendal Grignou <gwendal@chromium.org> wrote:
> On Thu, Jan 14, 2016 at 5:16 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 28 December 2015 at 15:12, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote:
>>> Hi Ulf,
>>>
>>> We succeeded to run FFU via new mmc multi-command ioctl without any code modification,
>>> but only by using Single Sector commands (CMD24).
>>>
>>> From running the FFU and from code review, we see two minor issues in this way of running FFU:
>>> 1. There is no support for Multiple Block write commands (CMD25) in existing IOCTL implementation -
>>
>> That's right. But I guess we cope without the multiple block support!?
>>
>> Although, I wonder how hard it would be to add it...
>>
>>> seems like there is no polling for the card status on data transfer completion.
>>
>> We should fix that!
>>
>> In the rpmb case, we check the status so we can probably trigger that
>> code to run for CMD24/25 as well.
>>
>>> (The kernel FFU implementation supports FFU using Multiple Block Write commands).
>>> 2. As you probably remember, there are two ways to install the new FW in the end of FFU process -
>>> In case MODE_OPERATION_CODES field is not supported by the device, the host sets to NORMAL state
>>
>> Before starting the update, you can find out which mode that is
>> supported and take relevant actions, right?
>>
>>> and initiates a CMD0/HW_Reset/Power cycle to install the new firmware.
>>
>> Yes, but that's fragile - as discussed earlier.
>>
>> What we really need to do is to also remove the "card" device from the
>> system, as otherwise we may have invalid data in its member variables
>> and who knows what issues that can cause to upper levels.
>>
>>> This sequence cannot be done via multi-command ioctl, and requires manual reset of the device/platform.
>>
>> Yepp, it seems so at least for now. Perhaps we can think of a way to
>> improve this?
>>
>>> (The kernel FFU implementation supports both FW install methods).
>>>
>>> For running FFU via new mmc multi-command ioctl, we have modified mmc-utils and add new functionality for FFU.
>>> Please let us know if you want us to submit the patch for mmc-utils FFU functionality via multi-command ioctl.
>>
>> Yes please. Don't forget to send this to Chris as well!
> I am arriving after the battle, but I have finally rebased the eMMC
> FFU kernel ffu code to 4.x. It is based on what Avi and Alex have
> written.
> As stated earlier, the advantage over using MMC_MUTLI_CMD is we can
> force a reset and rescan of the card without asking the user to reboot
> their machine.

No matter what, I think the problem is how you would *safely* deal
with the reset. Especially in the case when the eMMC already has an
mounted file system on it.

Just doing something that *might* work, isn't good enough to me.

> Also, by only sending a firmware name over the ioctl, we can use Kees'
> work for firmware validation (https://lwn.net/Articles/605432/).

The request_firmware() interface would indeed be good to use. Although
unless we can figure out a way on how to safely deal with reset, we
will have to live without request_firmware().

> To prevent downloading firmware from unknown source, we would reject
> some commands (like SWITCH with FFU_MODE) in the kernel
> MMC_IOC/MULTI_CMD ioctl handler.

I don't follow, can you elaborate on this please.

Kind regards
Uffe

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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2016-04-04 11:50               ` Ulf Hansson
@ 2016-04-13 22:33                 ` Gwendal Grignou
  2016-04-14  8:35                   ` Ulf Hansson
  0 siblings, 1 reply; 21+ messages in thread
From: Gwendal Grignou @ 2016-04-13 22:33 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Gwendal Grignou, Alex Lemberg, Holger Schurig, Avi Shchislowski,
	linux-mmc, linux-kernel, Chris Ball, Baolin Wang

On Mon, Apr 4, 2016 at 4:50 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On 2 April 2016 at 02:23, Gwendal Grignou <gwendal@chromium.org> wrote:
> > On Thu, Jan 14, 2016 at 5:16 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >> On 28 December 2015 at 15:12, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote:
>
> > I am arriving after the battle, but I have finally rebased the eMMC
> > FFU kernel ffu code to 4.x. It is based on what Avi and Alex have
> > written.
> > As stated earlier, the advantage over using MMC_MUTLI_CMD is we can
> > force a reset and rescan of the card without asking the user to reboot
> > their machine.
>
> No matter what, I think the problem is how you would *safely* deal
> with the reset. Especially in the case when the eMMC already has an
> mounted file system on it.

Assuming the firmware is not wiping data or resizing the available
space, the data in the flash is readable after the upgrade.
For the host point of view, a firmware update and a reset is
equivalent to a reset, that could happen during error recovery.
The only change are in the cid/csd//extcsd registers the firmware may
have updated.
The stack has to assume these registers are not constant and can
change after reset.

When looking into the mmc stack, AFAICT, the code that needs to get
device specifics always rely on fields that are re-generated by
mmc_card_inif() (card->ext_csd, output of mmc_decode_csd()/cid(() and
so on).
>
>
> Just doing something that *might* work, isn't good enough to me.
>
> > Also, by only sending a firmware name over the ioctl, we can use Kees'
> > work for firmware validation (https://lwn.net/Articles/605432/).
>
> The request_firmware() interface would indeed be good to use. Although
> unless we can figure out a way on how to safely deal with reset, we
> will have to live without request_firmware().
>
> > To prevent downloading firmware from unknown source, we would reject
> > some commands (like SWITCH with FFU_MODE) in the kernel
> > MMC_IOC/MULTI_CMD ioctl handler.
>
> I don't follow, can you elaborate on this please.

Today, an attacker with root access could break the chain of trust by
writing a firmware in the eMMC that corrupts data on the fly and
return infected code to the host after verification.
One way is to use firmware signed by the manufacturer, a stronger
approach is to enforce that the firmware is part of the root
partition.
To prevent a bad firmware from being downloaded, we have to make sure
downloading firmware using raw single or multi commands ioctls does
not work.

Gwendal.
>
>
> Kind regards
> Uffe

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

* Re: [RFC 0/6] mmc: Field Firmware Update
  2016-04-13 22:33                 ` Gwendal Grignou
@ 2016-04-14  8:35                   ` Ulf Hansson
  0 siblings, 0 replies; 21+ messages in thread
From: Ulf Hansson @ 2016-04-14  8:35 UTC (permalink / raw)
  To: Gwendal Grignou
  Cc: Alex Lemberg, Holger Schurig, Avi Shchislowski, linux-mmc,
	linux-kernel, Chris Ball, Baolin Wang

On 14 April 2016 at 00:33, Gwendal Grignou <gwendal@chromium.org> wrote:
> On Mon, Apr 4, 2016 at 4:50 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> On 2 April 2016 at 02:23, Gwendal Grignou <gwendal@chromium.org> wrote:
>> > On Thu, Jan 14, 2016 at 5:16 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> >> On 28 December 2015 at 15:12, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote:
>>
>> > I am arriving after the battle, but I have finally rebased the eMMC
>> > FFU kernel ffu code to 4.x. It is based on what Avi and Alex have
>> > written.
>> > As stated earlier, the advantage over using MMC_MUTLI_CMD is we can
>> > force a reset and rescan of the card without asking the user to reboot
>> > their machine.
>>
>> No matter what, I think the problem is how you would *safely* deal
>> with the reset. Especially in the case when the eMMC already has an
>> mounted file system on it.
>
> Assuming the firmware is not wiping data or resizing the available
> space, the data in the flash is readable after the upgrade.

This is exactly my point. You can no* assume anything about the card
after a firmware upgrade.

> For the host point of view, a firmware update and a reset is
> equivalent to a reset, that could happen during error recovery.
> The only change are in the cid/csd//extcsd registers the firmware may
> have updated.

Is that defined by the spec and are all eMMC vendors conforming to
your above statement?

> The stack has to assume these registers are not constant and can
> change after reset.
>
> When looking into the mmc stack, AFAICT, the code that needs to get
> device specifics always rely on fields that are re-generated by
> mmc_card_inif() (card->ext_csd, output of mmc_decode_csd()/cid(() and
> so on).
>>
>>
>> Just doing something that *might* work, isn't good enough to me.
>>
>> > Also, by only sending a firmware name over the ioctl, we can use Kees'
>> > work for firmware validation (https://lwn.net/Articles/605432/).
>>
>> The request_firmware() interface would indeed be good to use. Although
>> unless we can figure out a way on how to safely deal with reset, we
>> will have to live without request_firmware().
>>
>> > To prevent downloading firmware from unknown source, we would reject
>> > some commands (like SWITCH with FFU_MODE) in the kernel
>> > MMC_IOC/MULTI_CMD ioctl handler.
>>
>> I don't follow, can you elaborate on this please.
>
> Today, an attacker with root access could break the chain of trust by
> writing a firmware in the eMMC that corrupts data on the fly and
> return infected code to the host after verification.
> One way is to use firmware signed by the manufacturer, a stronger
> approach is to enforce that the firmware is part of the root
> partition.
> To prevent a bad firmware from being downloaded, we have to make sure
> downloading firmware using raw single or multi commands ioctls does
> not work.

I clearly see the benefit of using request_firmware() and I open to
adopt an in-kernel FFU solution that uses it, as long as a safe reset
can be managed.

However, whether it's more safe to hackers has nothing to do with it.
If a hacker becomes root on a device they can do all kind of magic
things, for example replacing a firmware in rootfs or sending ioctl
commands to a device node that has root permissions. To achieve
security, verification of a signatures are needed and currently the
request_firmware() API doesn't support this and nor does the eMMC
device itself (at least to my knowledge).

Regarding the safe reset, the only way I see how to deal with this, is
to force a reboot and prevent serving new read/write request after a
firmware upgrade. Although, perhaps you can think of something more
clever.

Kind regards
Uffe

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

end of thread, other threads:[~2016-04-14  8:35 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-13 14:56 [RFC 0/6] mmc: Field Firmware Update Holger Schurig
2015-11-13 14:56 ` [PATCH 1/6] mmc: move mmc_prepare_mrq() to core.c Holger Schurig
2015-11-13 14:56 ` [PATCH 2/6] mmc: move mmc_wait_busy() to core Holger Schurig
2015-11-13 14:56 ` [PATCH 3/6] mmc: move mmc_check_result() to core.c Holger Schurig
2015-11-13 14:56 ` [PATCH 4/6] mmc: move mmc_simple_transfer() " Holger Schurig
2015-11-13 14:56 ` [PATCH 5/6] mmc: move mmc_send_cxd_data() " Holger Schurig
2015-11-13 14:56 ` [PATCH 6/6] mmc: eMMC Field Firmware Update support Holger Schurig
2015-11-20 14:20 ` [RFC 0/6] mmc: Field Firmware Update Alan Cooper
2015-11-23 11:40 ` Avi Shchislowski
     [not found]   ` <CAOpc7mHbM8EYYNRKR+pKnzc0wf1DR1ojX-iFuDWDYyquC3EVZg@mail.gmail.com>
2015-12-15 15:35     ` Ulf Hansson
2015-12-22  8:15       ` Holger Schurig
2015-12-22  8:55         ` Ulf Hansson
2015-12-22  8:57           ` Ulf Hansson
2015-12-22 18:44             ` Olof Johansson
     [not found]               ` <CAG=SpYGf7czPwLrJVrq49n_Lhph33fmyKq-Nkm6=c0fB1Gn9_w@mail.gmail.com>
2016-01-14 12:50                 ` Ulf Hansson
2015-12-28 14:12         ` Alex Lemberg
2016-01-14 13:16           ` Ulf Hansson
2016-04-02  0:23             ` Gwendal Grignou
2016-04-04 11:50               ` Ulf Hansson
2016-04-13 22:33                 ` Gwendal Grignou
2016-04-14  8:35                   ` Ulf Hansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).