public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status
@ 2016-08-11 11:00 ` Peng Fan
  2016-08-11 11:00   ` [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0 Peng Fan
                     ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Peng Fan @ 2016-08-11 11:00 UTC (permalink / raw)
  To: u-boot

Add function to read SD_STATUS information.
According to the information, get erase_timeout/erase_size/erase_offset.
Add a structure sd_ssr to include the erase related information.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Eric Nelson <eric@nelint.com>
Cc: Stephen Warren <swarren@nvidia.com>
---
 drivers/mmc/mmc.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/mmc.h     |  8 +++++++
 2 files changed, 78 insertions(+)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 3daa748..efe517a 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -21,6 +21,13 @@
 #include <div64.h>
 #include "mmc_private.h"
 
+static const unsigned int sd_au_size[] = {
+	0,		SZ_16K / 512,		SZ_32K / 512,	SZ_64K / 512,
+	SZ_128K / 512,	SZ_256K / 512,		SZ_512K / 512,	SZ_1M / 512,
+	SZ_2M / 512,	SZ_4M / 512,		SZ_8M / 512,	(SZ_8M + SZ_4M) / 512,
+	SZ_16M / 512,	(SZ_16M + SZ_8M) / 512,	SZ_32M / 512,	SZ_64M / 512,
+};
+
 #ifndef CONFIG_DM_MMC_OPS
 __weak int board_mmc_getwp(struct mmc *mmc)
 {
@@ -942,6 +949,65 @@ retry_scr:
 	return 0;
 }
 
+static int sd_read_ssr(struct mmc *mmc)
+{
+	int err, i;
+	struct mmc_cmd cmd;
+	ALLOC_CACHE_ALIGN_BUFFER(uint, ssr, 16);
+	struct mmc_data data;
+	int timeout;
+	unsigned int au, eo, et, es;
+
+	cmd.cmdidx = MMC_CMD_APP_CMD;
+	cmd.resp_type = MMC_RSP_R1;
+	cmd.cmdarg = mmc->rca << 16;
+
+	err = mmc_send_cmd(mmc, &cmd, NULL);
+	if (err)
+		return err;
+
+	cmd.cmdidx = SD_CMD_APP_SD_STATUS;
+	cmd.resp_type = MMC_RSP_R1;
+	cmd.cmdarg = 0;
+
+	timeout = 3;
+
+retry_ssr:
+	data.dest = (char *)ssr;
+	data.blocksize = 64;
+	data.blocks = 1;
+	data.flags = MMC_DATA_READ;
+
+	err = mmc_send_cmd(mmc, &cmd, &data);
+	if (err) {
+		if (timeout--)
+			goto retry_ssr;
+
+		return err;
+	}
+
+	for (i = 0; i < 16; i++)
+		ssr[i] = be32_to_cpu(ssr[i]);
+
+	au = (ssr[2] >> 12) & 0xF;
+	if ((au <= 9) || (mmc->version == SD_VERSION_3)) {
+		mmc->ssr.au = sd_au_size[au];
+		es = (ssr[3] >> 24) & 0xFF;
+		es |= (ssr[2] & 0xFF) << 8;
+		et = (ssr[3] >> 18) & 0x3F;
+		if (es && et) {
+			eo = (ssr[3] >> 16) & 0x3;
+			mmc->ssr.erase_timeout = (et * 1000) / es;
+			mmc->ssr.erase_offset = eo * 1000;
+		}
+	} else {
+		printf("Invalid Allocation Unit Size.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /* frequency bases */
 /* divided by 10 to be nice to platforms without floating point */
 static const int fbase[] = {
@@ -1347,6 +1413,10 @@ static int mmc_startup(struct mmc *mmc)
 			mmc_set_bus_width(mmc, 4);
 		}
 
+		err = sd_read_ssr(mmc);
+		if (err)
+			return err;
+
 		if (mmc->card_caps & MMC_MODE_HS)
 			mmc->tran_speed = 50000000;
 		else
diff --git a/include/mmc.h b/include/mmc.h
index aa6d5d1..f09c36f 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -102,6 +102,7 @@
 #define SD_CMD_SWITCH_UHS18V		11
 
 #define SD_CMD_APP_SET_BUS_WIDTH	6
+#define SD_CMD_APP_SD_STATUS		13
 #define SD_CMD_ERASE_WR_BLK_START	32
 #define SD_CMD_ERASE_WR_BLK_END		33
 #define SD_CMD_APP_SEND_OP_COND		41
@@ -392,6 +393,12 @@ struct mmc_config {
 	unsigned char part_type;
 };
 
+struct sd_ssr {
+	unsigned int au;		/* In sectors */
+	unsigned int erase_timeout;	/* In milliseconds */
+	unsigned int erase_offset;	/* In milliseconds */
+};
+
 /*
  * With CONFIG_DM_MMC enabled, struct mmc can be accessed from the MMC device
  * with mmc_get_mmc_dev().
@@ -426,6 +433,7 @@ struct mmc {
 	uint write_bl_len;
 	uint erase_grp_size;	/* in 512-byte sectors */
 	uint hc_wp_grp_size;	/* in 512-byte sectors */
+	struct sd_ssr	ssr;
 	u64 capacity;
 	u64 capacity_user;
 	u64 capacity_boot;
-- 
2.6.2

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

* [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0
  2016-08-11 11:00 ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Peng Fan
@ 2016-08-11 11:00   ` Peng Fan
  2016-08-12 17:20     ` Simon Glass
  2016-08-11 11:00   ` [U-Boot] [PATCH 3/5] mmc: sd: add erase timeout support Peng Fan
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2016-08-11 11:00 UTC (permalink / raw)
  To: u-boot

Using {0} to initialize mmc_cmd, before filling the structure.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Eric Nelson <eric@nelint.com>
Cc: Stephen Warren <swarren@nvidia.com>
---
 drivers/mmc/mmc.c       | 28 ++++++++++++++--------------
 drivers/mmc/mmc_write.c |  4 ++--
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index efe517a..21bd0dc 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -139,7 +139,7 @@ int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 
 int mmc_send_status(struct mmc *mmc, int timeout)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	int err, retries = 5;
 
 	cmd.cmdidx = MMC_CMD_SEND_STATUS;
@@ -183,7 +183,7 @@ int mmc_send_status(struct mmc *mmc, int timeout)
 
 int mmc_set_blocklen(struct mmc *mmc, int len)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 
 	if (mmc->ddr_mode)
 		return 0;
@@ -198,7 +198,7 @@ int mmc_set_blocklen(struct mmc *mmc, int len)
 static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
 			   lbaint_t blkcnt)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	struct mmc_data data;
 
 	if (blkcnt > 1)
@@ -291,7 +291,7 @@ ulong mmc_bread(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt,
 
 static int mmc_go_idle(struct mmc *mmc)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	int err;
 
 	udelay(1000);
@@ -314,7 +314,7 @@ static int sd_send_op_cond(struct mmc *mmc)
 {
 	int timeout = 1000;
 	int err;
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 
 	while (1) {
 		cmd.cmdidx = MMC_CMD_APP_CMD;
@@ -380,7 +380,7 @@ static int sd_send_op_cond(struct mmc *mmc)
 
 static int mmc_send_op_cond_iter(struct mmc *mmc, int use_arg)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	int err;
 
 	cmd.cmdidx = MMC_CMD_SEND_OP_COND;
@@ -422,7 +422,7 @@ static int mmc_send_op_cond(struct mmc *mmc)
 
 static int mmc_complete_op_cond(struct mmc *mmc)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	int timeout = 1000;
 	uint start;
 	int err;
@@ -466,7 +466,7 @@ static int mmc_complete_op_cond(struct mmc *mmc)
 
 static int mmc_send_ext_csd(struct mmc *mmc, u8 *ext_csd)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	struct mmc_data data;
 	int err;
 
@@ -487,7 +487,7 @@ static int mmc_send_ext_csd(struct mmc *mmc, u8 *ext_csd)
 
 int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	int timeout = 1000;
 	int ret;
 
@@ -820,7 +820,7 @@ int mmc_getcd(struct mmc *mmc)
 
 static int sd_switch(struct mmc *mmc, int mode, int group, u8 value, u8 *resp)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	struct mmc_data data;
 
 	/* Switch the frequency */
@@ -842,7 +842,7 @@ static int sd_switch(struct mmc *mmc, int mode, int group, u8 value, u8 *resp)
 static int sd_change_freq(struct mmc *mmc)
 {
 	int err;
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	ALLOC_CACHE_ALIGN_BUFFER(uint, scr, 2);
 	ALLOC_CACHE_ALIGN_BUFFER(uint, switch_status, 16);
 	struct mmc_data data;
@@ -952,7 +952,7 @@ retry_scr:
 static int sd_read_ssr(struct mmc *mmc)
 {
 	int err, i;
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	ALLOC_CACHE_ALIGN_BUFFER(uint, ssr, 16);
 	struct mmc_data data;
 	int timeout;
@@ -1072,7 +1072,7 @@ static int mmc_startup(struct mmc *mmc)
 	int err, i;
 	uint mult, freq;
 	u64 cmult, csize, capacity;
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN);
 	ALLOC_CACHE_ALIGN_BUFFER(u8, test_csd, MMC_MAX_BLOCK_LEN);
 	int timeout = 1000;
@@ -1555,7 +1555,7 @@ static int mmc_startup(struct mmc *mmc)
 
 static int mmc_send_if_cond(struct mmc *mmc)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	int err;
 
 	cmd.cmdidx = SD_CMD_SEND_IF_COND;
diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
index 0f8b5c7..4149f4a 100644
--- a/drivers/mmc/mmc_write.c
+++ b/drivers/mmc/mmc_write.c
@@ -17,7 +17,7 @@
 
 static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	ulong end;
 	int err, start_cmd, end_cmd;
 
@@ -119,7 +119,7 @@ unsigned long mmc_berase(struct blk_desc *block_dev, lbaint_t start,
 static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
 		lbaint_t blkcnt, const void *src)
 {
-	struct mmc_cmd cmd;
+	struct mmc_cmd cmd = {0};
 	struct mmc_data data;
 	int timeout = 1000;
 
-- 
2.6.2

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

* [U-Boot] [PATCH 3/5] mmc: sd: add erase timeout support
  2016-08-11 11:00 ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Peng Fan
  2016-08-11 11:00   ` [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0 Peng Fan
@ 2016-08-11 11:00   ` Peng Fan
  2016-08-11 11:00   ` [U-Boot] [PATCH 4/5] mmc: esdhc: change timeout value Peng Fan
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Peng Fan @ 2016-08-11 11:00 UTC (permalink / raw)
  To: u-boot

Add timeout in mmc_cmd, we can use this in driver code.
Add mmc_sd_erase_timeout, this function is modified from linux kernel.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Eric Nelson <eric@nelint.com>
Cc: Stephen Warren <swarren@nvidia.com>
---
 drivers/mmc/mmc_write.c | 29 +++++++++++++++++++++++++++++
 include/mmc.h           |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
index 4149f4a..3589f8e 100644
--- a/drivers/mmc/mmc_write.c
+++ b/drivers/mmc/mmc_write.c
@@ -15,6 +15,33 @@
 #include <linux/math64.h>
 #include "mmc_private.h"
 
+/*
+ * Modified from from Linux kernel mmc_sd_erase_timeout.
+ */
+static unsigned int mmc_sd_erase_timeout(struct mmc *mmc,
+					 unsigned int nr)
+{
+	unsigned int erase_timeout;
+
+	if (mmc->ssr.erase_timeout) {
+		/* Erase timeout specified in SD Status Register (SSR) */
+		erase_timeout = mmc->ssr.erase_timeout * nr +
+				mmc->ssr.erase_offset;
+	} else {
+		/*
+		 * Erase timeout not specified in SD Status Register (SSR) so
+		 * use 250ms per write block.
+		 */
+		erase_timeout = 250 * nr;
+	}
+
+	/* Must not be less than 1 second */
+	if (erase_timeout < 1000)
+		erase_timeout = 1000;
+
+	return erase_timeout;
+}
+
 static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
 {
 	struct mmc_cmd cmd = {0};
@@ -54,6 +81,8 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
 	cmd.cmdidx = MMC_CMD_ERASE;
 	cmd.cmdarg = MMC_ERASE_ARG;
 	cmd.resp_type = MMC_RSP_R1b;
+	if (IS_SD(mmc))
+		cmd.timeout = mmc_sd_erase_timeout(mmc, blkcnt);
 
 	err = mmc_send_cmd(mmc, &cmd, NULL);
 	if (err)
diff --git a/include/mmc.h b/include/mmc.h
index f09c36f..c72495c 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -303,6 +303,7 @@ struct mmc_cmd {
 	uint resp_type;
 	uint cmdarg;
 	uint response[4];
+	int timeout;
 };
 
 struct mmc_data {
-- 
2.6.2

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

* [U-Boot] [PATCH 4/5] mmc: esdhc: change timeout value
  2016-08-11 11:00 ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Peng Fan
  2016-08-11 11:00   ` [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0 Peng Fan
  2016-08-11 11:00   ` [U-Boot] [PATCH 3/5] mmc: sd: add erase timeout support Peng Fan
@ 2016-08-11 11:00   ` Peng Fan
  2016-08-11 11:00   ` [U-Boot] [PATCH 5/5] mmc: sd: optimize erase Peng Fan
  2016-08-12  2:08   ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Jaehoon Chung
  4 siblings, 0 replies; 15+ messages in thread
From: Peng Fan @ 2016-08-11 11:00 UTC (permalink / raw)
  To: u-boot

Change timeout according to the timeout value in mmc_cmd->timeout.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/fsl_esdhc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index 9796d39..eca2f31 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -350,6 +350,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 	int	err = 0;
 	uint	xfertyp;
 	uint	irqstat;
+	int	timeout = cmd->timeout;
 	struct fsl_esdhc_priv *priv = mmc->priv;
 	struct fsl_esdhc *regs = priv->esdhc_regs;
 
@@ -431,7 +432,8 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 
 	/* Workaround for ESDHC errata ENGcm03648 */
 	if (!data && (cmd->resp_type & MMC_RSP_BUSY)) {
-		int timeout = 6000;
+		if (timeout < 6000)
+			timeout = 6000;
 
 		/* Poll on DATA0 line for cmd with busy signal for 600 ms */
 		while (timeout > 0 && !(esdhc_read32(&regs->prsstat) &
-- 
2.6.2

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

* [U-Boot] [PATCH 5/5] mmc: sd: optimize erase
  2016-08-11 11:00 ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Peng Fan
                     ` (2 preceding siblings ...)
  2016-08-11 11:00   ` [U-Boot] [PATCH 4/5] mmc: esdhc: change timeout value Peng Fan
@ 2016-08-11 11:00   ` Peng Fan
  2016-08-12  0:53     ` Fabio Estevam
  2016-08-12  2:08   ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Jaehoon Chung
  4 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2016-08-11 11:00 UTC (permalink / raw)
  To: u-boot

To SD, there is no erase group, then the value erase_grp_size
will be default 1. When erasing SD blocks, the blocks will be
erased one by one, which is time consuming.

use AU_SIZE as a group to speed up the erasing.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Eric Nelson <eric@nelint.com>
Cc: Stephen Warren <swarren@nvidia.com>
---
 drivers/mmc/mmc_write.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
index 3589f8e..6221b4a 100644
--- a/drivers/mmc/mmc_write.c
+++ b/drivers/mmc/mmc_write.c
@@ -129,8 +129,13 @@ unsigned long mmc_berase(struct blk_desc *block_dev, lbaint_t start,
 		       & ~(mmc->erase_grp_size - 1)) - 1);
 
 	while (blk < blkcnt) {
-		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
-			mmc->erase_grp_size : (blkcnt - blk);
+		if (IS_SD(mmc) && mmc->ssr.au) {
+			blk_r = ((blkcnt - blk) > mmc->ssr.au) ?
+				mmc->ssr.au : (blkcnt - blk);
+		} else {
+			blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
+				mmc->erase_grp_size : (blkcnt - blk);
+		}
 		err = mmc_erase_t(mmc, start + blk, blk_r);
 		if (err)
 			break;
-- 
2.6.2

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

* [U-Boot] [PATCH 5/5] mmc: sd: optimize erase
  2016-08-11 11:00   ` [U-Boot] [PATCH 5/5] mmc: sd: optimize erase Peng Fan
@ 2016-08-12  0:53     ` Fabio Estevam
  2016-08-12  3:02       ` Peng Fan
  0 siblings, 1 reply; 15+ messages in thread
From: Fabio Estevam @ 2016-08-12  0:53 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On Thu, Aug 11, 2016 at 8:00 AM, Peng Fan <van.freenix@gmail.com> wrote:
> To SD, there is no erase group, then the value erase_grp_size
> will be default 1. When erasing SD blocks, the blocks will be
> erased one by one, which is time consuming.
>
> use AU_SIZE as a group to speed up the erasing.

Just curious: what was the measured increase in performace for the
erase operation with your series?

Thanks

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

* [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status
  2016-08-11 11:00 ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Peng Fan
                     ` (3 preceding siblings ...)
  2016-08-11 11:00   ` [U-Boot] [PATCH 5/5] mmc: sd: optimize erase Peng Fan
@ 2016-08-12  2:08   ` Jaehoon Chung
  2016-08-12  2:55     ` Peng Fan
  4 siblings, 1 reply; 15+ messages in thread
From: Jaehoon Chung @ 2016-08-12  2:08 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On 08/11/2016 08:00 PM, Peng Fan wrote:
> Add function to read SD_STATUS information.
> According to the information, get erase_timeout/erase_size/erase_offset.
> Add a structure sd_ssr to include the erase related information.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Eric Nelson <eric@nelint.com>
> Cc: Stephen Warren <swarren@nvidia.com>
> ---
>  drivers/mmc/mmc.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/mmc.h     |  8 +++++++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 3daa748..efe517a 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -21,6 +21,13 @@
>  #include <div64.h>
>  #include "mmc_private.h"
>  
> +static const unsigned int sd_au_size[] = {
> +	0,		SZ_16K / 512,		SZ_32K / 512,	SZ_64K / 512,
> +	SZ_128K / 512,	SZ_256K / 512,		SZ_512K / 512,	SZ_1M / 512,
> +	SZ_2M / 512,	SZ_4M / 512,		SZ_8M / 512,	(SZ_8M + SZ_4M) / 512,
> +	SZ_16M / 512,	(SZ_16M + SZ_8M) / 512,	SZ_32M / 512,	SZ_64M / 512,
> +};

WARNING: line over 80 characters
#37: FILE: drivers/mmc/mmc.c:27:
+       SZ_2M / 512,    SZ_4M / 512,            SZ_8M / 512,    (SZ_8M + SZ_4M) / 512,

> +
>  #ifndef CONFIG_DM_MMC_OPS
>  __weak int board_mmc_getwp(struct mmc *mmc)
>  {
> @@ -942,6 +949,65 @@ retry_scr:
>  	return 0;
>  }
>  
> +static int sd_read_ssr(struct mmc *mmc)
> +{
> +	int err, i;
> +	struct mmc_cmd cmd;
> +	ALLOC_CACHE_ALIGN_BUFFER(uint, ssr, 16);
> +	struct mmc_data data;
> +	int timeout;
> +	unsigned int au, eo, et, es;
> +
> +	cmd.cmdidx = MMC_CMD_APP_CMD;
> +	cmd.resp_type = MMC_RSP_R1;
> +	cmd.cmdarg = mmc->rca << 16;
> +
> +	err = mmc_send_cmd(mmc, &cmd, NULL);
> +	if (err)
> +		return err;
> +
> +	cmd.cmdidx = SD_CMD_APP_SD_STATUS;
> +	cmd.resp_type = MMC_RSP_R1;
> +	cmd.cmdarg = 0;
> +
> +	timeout = 3;

Don't need to assign at here.

> +
> +retry_ssr:
> +	data.dest = (char *)ssr;
> +	data.blocksize = 64;
> +	data.blocks = 1;
> +	data.flags = MMC_DATA_READ;
> +
> +	err = mmc_send_cmd(mmc, &cmd, &data);
> +	if (err) {
> +		if (timeout--)
> +			goto retry_ssr;
> +
> +		return err;
> +	}
> +
> +	for (i = 0; i < 16; i++)
> +		ssr[i] = be32_to_cpu(ssr[i]);
> +
> +	au = (ssr[2] >> 12) & 0xF;
> +	if ((au <= 9) || (mmc->version == SD_VERSION_3)) {
> +		mmc->ssr.au = sd_au_size[au];
> +		es = (ssr[3] >> 24) & 0xFF;
> +		es |= (ssr[2] & 0xFF) << 8;
> +		et = (ssr[3] >> 18) & 0x3F;
> +		if (es && et) {
> +			eo = (ssr[3] >> 16) & 0x3;
> +			mmc->ssr.erase_timeout = (et * 1000) / es;
> +			mmc->ssr.erase_offset = eo * 1000;
> +		}
> +	} else {
> +		printf("Invalid Allocation Unit Size.\n");
> +		return -EINVAL;

If AU size can't read, then your patch can't also initialize the SD-card.
AU-size is critical things enough to go to non-initialize??

> +	}
> +
> +	return 0;
> +}
> +
>  /* frequency bases */
>  /* divided by 10 to be nice to platforms without floating point */
>  static const int fbase[] = {
> @@ -1347,6 +1413,10 @@ static int mmc_startup(struct mmc *mmc)
>  			mmc_set_bus_width(mmc, 4);
>  		}
>  
> +		err = sd_read_ssr(mmc);
> +		if (err)
> +			return err;
> +
>  		if (mmc->card_caps & MMC_MODE_HS)
>  			mmc->tran_speed = 50000000;
>  		else
> diff --git a/include/mmc.h b/include/mmc.h
> index aa6d5d1..f09c36f 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -102,6 +102,7 @@
>  #define SD_CMD_SWITCH_UHS18V		11
>  
>  #define SD_CMD_APP_SET_BUS_WIDTH	6
> +#define SD_CMD_APP_SD_STATUS		13
>  #define SD_CMD_ERASE_WR_BLK_START	32
>  #define SD_CMD_ERASE_WR_BLK_END		33
>  #define SD_CMD_APP_SEND_OP_COND		41
> @@ -392,6 +393,12 @@ struct mmc_config {
>  	unsigned char part_type;
>  };
>  
> +struct sd_ssr {
> +	unsigned int au;		/* In sectors */
> +	unsigned int erase_timeout;	/* In milliseconds */
> +	unsigned int erase_offset;	/* In milliseconds */
> +};
> +
>  /*
>   * With CONFIG_DM_MMC enabled, struct mmc can be accessed from the MMC device
>   * with mmc_get_mmc_dev().
> @@ -426,6 +433,7 @@ struct mmc {
>  	uint write_bl_len;
>  	uint erase_grp_size;	/* in 512-byte sectors */
>  	uint hc_wp_grp_size;	/* in 512-byte sectors */
> +	struct sd_ssr	ssr;

Add the comment what is SSR.

Best Regards,
Jaehoon Chung

>  	u64 capacity;
>  	u64 capacity_user;
>  	u64 capacity_boot;
> 

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

* [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status
  2016-08-12  2:08   ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Jaehoon Chung
@ 2016-08-12  2:55     ` Peng Fan
  2016-08-12  3:51       ` Jaehoon Chung
  0 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2016-08-12  2:55 UTC (permalink / raw)
  To: u-boot

Hi Jaehoon,

On Fri, Aug 12, 2016 at 11:08:28AM +0900, Jaehoon Chung wrote:
>Hi Peng,
>
>On 08/11/2016 08:00 PM, Peng Fan wrote:
>> Add function to read SD_STATUS information.
>> According to the information, get erase_timeout/erase_size/erase_offset.
>> Add a structure sd_ssr to include the erase related information.
>> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>> Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
>> Cc: Kever Yang <kever.yang@rock-chips.com>
>> Cc: Eric Nelson <eric@nelint.com>
>> Cc: Stephen Warren <swarren@nvidia.com>
>> ---
>>  drivers/mmc/mmc.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/mmc.h     |  8 +++++++
>>  2 files changed, 78 insertions(+)
>> 
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index 3daa748..efe517a 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -21,6 +21,13 @@
>>  #include <div64.h>
>>  #include "mmc_private.h"
>>  
>> +static const unsigned int sd_au_size[] = {
>> +	0,		SZ_16K / 512,		SZ_32K / 512,	SZ_64K / 512,
>> +	SZ_128K / 512,	SZ_256K / 512,		SZ_512K / 512,	SZ_1M / 512,
>> +	SZ_2M / 512,	SZ_4M / 512,		SZ_8M / 512,	(SZ_8M + SZ_4M) / 512,
>> +	SZ_16M / 512,	(SZ_16M + SZ_8M) / 512,	SZ_32M / 512,	SZ_64M / 512,
>> +};
>
>WARNING: line over 80 characters
>#37: FILE: drivers/mmc/mmc.c:27:
>+       SZ_2M / 512,    SZ_4M / 512,            SZ_8M / 512,    (SZ_8M + SZ_4M) / 512,

Will fix this in V2.

>
>> +
>>  #ifndef CONFIG_DM_MMC_OPS
>>  __weak int board_mmc_getwp(struct mmc *mmc)
>>  {
>> @@ -942,6 +949,65 @@ retry_scr:
>>  	return 0;
>>  }
>>  
>> +static int sd_read_ssr(struct mmc *mmc)
>> +{
>> +	int err, i;
>> +	struct mmc_cmd cmd;
>> +	ALLOC_CACHE_ALIGN_BUFFER(uint, ssr, 16);
>> +	struct mmc_data data;
>> +	int timeout;
>> +	unsigned int au, eo, et, es;
>> +
>> +	cmd.cmdidx = MMC_CMD_APP_CMD;
>> +	cmd.resp_type = MMC_RSP_R1;
>> +	cmd.cmdarg = mmc->rca << 16;
>> +
>> +	err = mmc_send_cmd(mmc, &cmd, NULL);
>> +	if (err)
>> +		return err;
>> +
>> +	cmd.cmdidx = SD_CMD_APP_SD_STATUS;
>> +	cmd.resp_type = MMC_RSP_R1;
>> +	cmd.cmdarg = 0;
>> +
>> +	timeout = 3;
>
>Don't need to assign at here.

I just follow retry_scr here. You mean there is no need to try more times?

>
>> +
>> +retry_ssr:
>> +	data.dest = (char *)ssr;
>> +	data.blocksize = 64;
>> +	data.blocks = 1;
>> +	data.flags = MMC_DATA_READ;
>> +
>> +	err = mmc_send_cmd(mmc, &cmd, &data);
>> +	if (err) {
>> +		if (timeout--)
>> +			goto retry_ssr;
>> +
>> +		return err;
>> +	}
>> +
>> +	for (i = 0; i < 16; i++)
>> +		ssr[i] = be32_to_cpu(ssr[i]);
>> +
>> +	au = (ssr[2] >> 12) & 0xF;
>> +	if ((au <= 9) || (mmc->version == SD_VERSION_3)) {
>> +		mmc->ssr.au = sd_au_size[au];
>> +		es = (ssr[3] >> 24) & 0xFF;
>> +		es |= (ssr[2] & 0xFF) << 8;
>> +		et = (ssr[3] >> 18) & 0x3F;
>> +		if (es && et) {
>> +			eo = (ssr[3] >> 16) & 0x3;
>> +			mmc->ssr.erase_timeout = (et * 1000) / es;
>> +			mmc->ssr.erase_offset = eo * 1000;
>> +		}
>> +	} else {
>> +		printf("Invalid Allocation Unit Size.\n");
>> +		return -EINVAL;
>
>If AU size can't read, then your patch can't also initialize the SD-card.
>AU-size is critical things enough to go to non-initialize??

Not a must. Will use debug here and discard `return -EINVAL`.

>
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  /* frequency bases */
>>  /* divided by 10 to be nice to platforms without floating point */
>>  static const int fbase[] = {
>> @@ -1347,6 +1413,10 @@ static int mmc_startup(struct mmc *mmc)
>>  			mmc_set_bus_width(mmc, 4);
>>  		}
>>  
>> +		err = sd_read_ssr(mmc);
>> +		if (err)
>> +			return err;
>> +
>>  		if (mmc->card_caps & MMC_MODE_HS)
>>  			mmc->tran_speed = 50000000;
>>  		else
>> diff --git a/include/mmc.h b/include/mmc.h
>> index aa6d5d1..f09c36f 100644
>> --- a/include/mmc.h
>> +++ b/include/mmc.h
>> @@ -102,6 +102,7 @@
>>  #define SD_CMD_SWITCH_UHS18V		11
>>  
>>  #define SD_CMD_APP_SET_BUS_WIDTH	6
>> +#define SD_CMD_APP_SD_STATUS		13
>>  #define SD_CMD_ERASE_WR_BLK_START	32
>>  #define SD_CMD_ERASE_WR_BLK_END		33
>>  #define SD_CMD_APP_SEND_OP_COND		41
>> @@ -392,6 +393,12 @@ struct mmc_config {
>>  	unsigned char part_type;
>>  };
>>  
>> +struct sd_ssr {
>> +	unsigned int au;		/* In sectors */
>> +	unsigned int erase_timeout;	/* In milliseconds */
>> +	unsigned int erase_offset;	/* In milliseconds */
>> +};
>> +
>>  /*
>>   * With CONFIG_DM_MMC enabled, struct mmc can be accessed from the MMC device
>>   * with mmc_get_mmc_dev().
>> @@ -426,6 +433,7 @@ struct mmc {
>>  	uint write_bl_len;
>>  	uint erase_grp_size;	/* in 512-byte sectors */
>>  	uint hc_wp_grp_size;	/* in 512-byte sectors */
>> +	struct sd_ssr	ssr;
>
>Add the comment what is SSR.

SD Status register. Will add in V2.

Thanks,
Peng.

>
>Best Regards,
>Jaehoon Chung
>
>>  	u64 capacity;
>>  	u64 capacity_user;
>>  	u64 capacity_boot;
>> 
>

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

* [U-Boot] [PATCH 5/5] mmc: sd: optimize erase
  2016-08-12  0:53     ` Fabio Estevam
@ 2016-08-12  3:02       ` Peng Fan
  2016-08-12  3:12         ` Fabio Estevam
  0 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2016-08-12  3:02 UTC (permalink / raw)
  To: u-boot

Hi Fabio,
On Thu, Aug 11, 2016 at 09:53:31PM -0300, Fabio Estevam wrote:
>Hi Peng,
>
>On Thu, Aug 11, 2016 at 8:00 AM, Peng Fan <van.freenix@gmail.com> wrote:
>> To SD, there is no erase group, then the value erase_grp_size
>> will be default 1. When erasing SD blocks, the blocks will be
>> erased one by one, which is time consuming.
>>
>> use AU_SIZE as a group to speed up the erasing.
>
>Just curious: what was the measured increase in performace for the
>erase operation with your series?

Erasing 4MB.
Before applying this patch set
=> time mmc erase 0x100000 0x2000

MMC erase: dev # 0, block # 1048576, count 8192 ... 8192 blocks erased: OK

time: 44.856 seconds

After applying the patch set:
=> time mmc erase 0x100000 0x2000

MMC erase: dev # 0, block # 1048576, count 8192 ... 8192 blocks erased: OK

time: 0.335 seconds

Will add the test reults in V2 commit log.

Thanks,
Peng.
>
>Thanks

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

* [U-Boot] [PATCH 5/5] mmc: sd: optimize erase
  2016-08-12  3:02       ` Peng Fan
@ 2016-08-12  3:12         ` Fabio Estevam
  0 siblings, 0 replies; 15+ messages in thread
From: Fabio Estevam @ 2016-08-12  3:12 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On Fri, Aug 12, 2016 at 12:02 AM, Peng Fan <van.freenix@gmail.com> wrote:

>>Just curious: what was the measured increase in performace for the
>>erase operation with your series?
>
> Erasing 4MB.
> Before applying this patch set
> => time mmc erase 0x100000 0x2000
>
> MMC erase: dev # 0, block # 1048576, count 8192 ... 8192 blocks erased: OK
>
> time: 44.856 seconds
>
> After applying the patch set:
> => time mmc erase 0x100000 0x2000
>
> MMC erase: dev # 0, block # 1048576, count 8192 ... 8192 blocks erased: OK
>
> time: 0.335 seconds
>
> Will add the test reults in V2 commit log.

That's a really nice improvement! Good job!

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

* [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status
  2016-08-12  2:55     ` Peng Fan
@ 2016-08-12  3:51       ` Jaehoon Chung
  0 siblings, 0 replies; 15+ messages in thread
From: Jaehoon Chung @ 2016-08-12  3:51 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On 08/12/2016 11:55 AM, Peng Fan wrote:
> Hi Jaehoon,
> 
> On Fri, Aug 12, 2016 at 11:08:28AM +0900, Jaehoon Chung wrote:
>> Hi Peng,
>>
>> On 08/11/2016 08:00 PM, Peng Fan wrote:
>>> Add function to read SD_STATUS information.
>>> According to the information, get erase_timeout/erase_size/erase_offset.
>>> Add a structure sd_ssr to include the erase related information.
>>>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>> Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
>>> Cc: Kever Yang <kever.yang@rock-chips.com>
>>> Cc: Eric Nelson <eric@nelint.com>
>>> Cc: Stephen Warren <swarren@nvidia.com>
>>> ---
>>>  drivers/mmc/mmc.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  include/mmc.h     |  8 +++++++
>>>  2 files changed, 78 insertions(+)
>>>
>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>> index 3daa748..efe517a 100644
>>> --- a/drivers/mmc/mmc.c
>>> +++ b/drivers/mmc/mmc.c
>>> @@ -21,6 +21,13 @@
>>>  #include <div64.h>
>>>  #include "mmc_private.h"
>>>  
>>> +static const unsigned int sd_au_size[] = {
>>> +	0,		SZ_16K / 512,		SZ_32K / 512,	SZ_64K / 512,
>>> +	SZ_128K / 512,	SZ_256K / 512,		SZ_512K / 512,	SZ_1M / 512,
>>> +	SZ_2M / 512,	SZ_4M / 512,		SZ_8M / 512,	(SZ_8M + SZ_4M) / 512,
>>> +	SZ_16M / 512,	(SZ_16M + SZ_8M) / 512,	SZ_32M / 512,	SZ_64M / 512,
>>> +};
>>
>> WARNING: line over 80 characters
>> #37: FILE: drivers/mmc/mmc.c:27:
>> +       SZ_2M / 512,    SZ_4M / 512,            SZ_8M / 512,    (SZ_8M + SZ_4M) / 512,
> 
> Will fix this in V2.
> 
>>
>>> +
>>>  #ifndef CONFIG_DM_MMC_OPS
>>>  __weak int board_mmc_getwp(struct mmc *mmc)
>>>  {
>>> @@ -942,6 +949,65 @@ retry_scr:
>>>  	return 0;
>>>  }
>>>  
>>> +static int sd_read_ssr(struct mmc *mmc)
>>> +{
>>> +	int err, i;
>>> +	struct mmc_cmd cmd;
>>> +	ALLOC_CACHE_ALIGN_BUFFER(uint, ssr, 16);
>>> +	struct mmc_data data;
>>> +	int timeout;
>>> +	unsigned int au, eo, et, es;
>>> +
>>> +	cmd.cmdidx = MMC_CMD_APP_CMD;
>>> +	cmd.resp_type = MMC_RSP_R1;
>>> +	cmd.cmdarg = mmc->rca << 16;
>>> +
>>> +	err = mmc_send_cmd(mmc, &cmd, NULL);
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	cmd.cmdidx = SD_CMD_APP_SD_STATUS;
>>> +	cmd.resp_type = MMC_RSP_R1;
>>> +	cmd.cmdarg = 0;
>>> +
>>> +	timeout = 3;
>>
>> Don't need to assign at here.
> 
> I just follow retry_scr here. You mean there is no need to try more times?

No, my meaning is that just assigned to 3 when timeout is defined.

just likes "int timeout = 3".

Best Regards,
Jaehoon Chung

> 
>>
>>> +
>>> +retry_ssr:
>>> +	data.dest = (char *)ssr;
>>> +	data.blocksize = 64;
>>> +	data.blocks = 1;
>>> +	data.flags = MMC_DATA_READ;
>>> +
>>> +	err = mmc_send_cmd(mmc, &cmd, &data);
>>> +	if (err) {
>>> +		if (timeout--)
>>> +			goto retry_ssr;
>>> +
>>> +		return err;
>>> +	}
>>> +
>>> +	for (i = 0; i < 16; i++)
>>> +		ssr[i] = be32_to_cpu(ssr[i]);
>>> +
>>> +	au = (ssr[2] >> 12) & 0xF;
>>> +	if ((au <= 9) || (mmc->version == SD_VERSION_3)) {
>>> +		mmc->ssr.au = sd_au_size[au];
>>> +		es = (ssr[3] >> 24) & 0xFF;
>>> +		es |= (ssr[2] & 0xFF) << 8;
>>> +		et = (ssr[3] >> 18) & 0x3F;
>>> +		if (es && et) {
>>> +			eo = (ssr[3] >> 16) & 0x3;
>>> +			mmc->ssr.erase_timeout = (et * 1000) / es;
>>> +			mmc->ssr.erase_offset = eo * 1000;
>>> +		}
>>> +	} else {
>>> +		printf("Invalid Allocation Unit Size.\n");
>>> +		return -EINVAL;
>>
>> If AU size can't read, then your patch can't also initialize the SD-card.
>> AU-size is critical things enough to go to non-initialize??
> 
> Not a must. Will use debug here and discard `return -EINVAL`.
> 
>>
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  /* frequency bases */
>>>  /* divided by 10 to be nice to platforms without floating point */
>>>  static const int fbase[] = {
>>> @@ -1347,6 +1413,10 @@ static int mmc_startup(struct mmc *mmc)
>>>  			mmc_set_bus_width(mmc, 4);
>>>  		}
>>>  
>>> +		err = sd_read_ssr(mmc);
>>> +		if (err)
>>> +			return err;
>>> +
>>>  		if (mmc->card_caps & MMC_MODE_HS)
>>>  			mmc->tran_speed = 50000000;
>>>  		else
>>> diff --git a/include/mmc.h b/include/mmc.h
>>> index aa6d5d1..f09c36f 100644
>>> --- a/include/mmc.h
>>> +++ b/include/mmc.h
>>> @@ -102,6 +102,7 @@
>>>  #define SD_CMD_SWITCH_UHS18V		11
>>>  
>>>  #define SD_CMD_APP_SET_BUS_WIDTH	6
>>> +#define SD_CMD_APP_SD_STATUS		13
>>>  #define SD_CMD_ERASE_WR_BLK_START	32
>>>  #define SD_CMD_ERASE_WR_BLK_END		33
>>>  #define SD_CMD_APP_SEND_OP_COND		41
>>> @@ -392,6 +393,12 @@ struct mmc_config {
>>>  	unsigned char part_type;
>>>  };
>>>  
>>> +struct sd_ssr {
>>> +	unsigned int au;		/* In sectors */
>>> +	unsigned int erase_timeout;	/* In milliseconds */
>>> +	unsigned int erase_offset;	/* In milliseconds */
>>> +};
>>> +
>>>  /*
>>>   * With CONFIG_DM_MMC enabled, struct mmc can be accessed from the MMC device
>>>   * with mmc_get_mmc_dev().
>>> @@ -426,6 +433,7 @@ struct mmc {
>>>  	uint write_bl_len;
>>>  	uint erase_grp_size;	/* in 512-byte sectors */
>>>  	uint hc_wp_grp_size;	/* in 512-byte sectors */
>>> +	struct sd_ssr	ssr;
>>
>> Add the comment what is SSR.
> 
> SD Status register. Will add in V2.
> 
> Thanks,
> Peng.
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>  	u64 capacity;
>>>  	u64 capacity_user;
>>>  	u64 capacity_boot;
>>>
>>
> 
> 
> 

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

* [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0
  2016-08-11 11:00   ` [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0 Peng Fan
@ 2016-08-12 17:20     ` Simon Glass
  2016-08-13  1:41       ` Peng Fan
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2016-08-12 17:20 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On 11 August 2016 at 05:00, Peng Fan <van.freenix@gmail.com> wrote:
> Using {0} to initialize mmc_cmd, before filling the structure.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Eric Nelson <eric@nelint.com>
> Cc: Stephen Warren <swarren@nvidia.com>
> ---
>  drivers/mmc/mmc.c       | 28 ++++++++++++++--------------
>  drivers/mmc/mmc_write.c |  4 ++--
>  2 files changed, 16 insertions(+), 16 deletions(-)

Why is this needed? Does it affect code size?

Regards,
Simon

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

* [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0
  2016-08-12 17:20     ` Simon Glass
@ 2016-08-13  1:41       ` Peng Fan
  2016-08-18  3:45         ` Simon Glass
  0 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2016-08-13  1:41 UTC (permalink / raw)
  To: u-boot

Hi Simon,
On Fri, Aug 12, 2016 at 11:20:25AM -0600, Simon Glass wrote:
>Hi Peng,
>
>On 11 August 2016 at 05:00, Peng Fan <van.freenix@gmail.com> wrote:
>> Using {0} to initialize mmc_cmd, before filling the structure.
>>
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>> Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
>> Cc: Kever Yang <kever.yang@rock-chips.com>
>> Cc: Eric Nelson <eric@nelint.com>
>> Cc: Stephen Warren <swarren@nvidia.com>
>> ---
>>  drivers/mmc/mmc.c       | 28 ++++++++++++++--------------
>>  drivers/mmc/mmc_write.c |  4 ++--
>>  2 files changed, 16 insertions(+), 16 deletions(-)
>
>Why is this needed? Does it affect code size?

I add a timeout entry in mmc_cmd, but I do not want to specify a value for timeout
for each mmc_cmd. So I use {0}.

Then to those who want use timeout, a value can be assigned to timeout, just like
I added in the patchset for mmc erase.

Thanks,
Peng.

>
>Regards,
>Simon

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

* [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0
  2016-08-13  1:41       ` Peng Fan
@ 2016-08-18  3:45         ` Simon Glass
  2016-08-18  6:21           ` Peng Fan
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2016-08-18  3:45 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On 12 August 2016 at 19:41, Peng Fan <van.freenix@gmail.com> wrote:
> Hi Simon,
> On Fri, Aug 12, 2016 at 11:20:25AM -0600, Simon Glass wrote:
>>Hi Peng,
>>
>>On 11 August 2016 at 05:00, Peng Fan <van.freenix@gmail.com> wrote:
>>> Using {0} to initialize mmc_cmd, before filling the structure.
>>>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>> Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
>>> Cc: Kever Yang <kever.yang@rock-chips.com>
>>> Cc: Eric Nelson <eric@nelint.com>
>>> Cc: Stephen Warren <swarren@nvidia.com>
>>> ---
>>>  drivers/mmc/mmc.c       | 28 ++++++++++++++--------------
>>>  drivers/mmc/mmc_write.c |  4 ++--
>>>  2 files changed, 16 insertions(+), 16 deletions(-)
>>
>>Why is this needed? Does it affect code size?
>
> I add a timeout entry in mmc_cmd, but I do not want to specify a value for timeout
> for each mmc_cmd. So I use {0}.
>
> Then to those who want use timeout, a value can be assigned to timeout, just like
> I added in the patchset for mmc erase.

Instead of that, can you assign the value to 0, or whatever you want?
You might want to have a helper like mmc_init_cmd() to zero things
out.

Regards,
Simon

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

* [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0
  2016-08-18  3:45         ` Simon Glass
@ 2016-08-18  6:21           ` Peng Fan
  0 siblings, 0 replies; 15+ messages in thread
From: Peng Fan @ 2016-08-18  6:21 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Wed, Aug 17, 2016 at 09:45:35PM -0600, Simon Glass wrote:
>Hi Peng,
>
>On 12 August 2016 at 19:41, Peng Fan <van.freenix@gmail.com> wrote:
>> Hi Simon,
>> On Fri, Aug 12, 2016 at 11:20:25AM -0600, Simon Glass wrote:
>>>Hi Peng,
>>>
>>>On 11 August 2016 at 05:00, Peng Fan <van.freenix@gmail.com> wrote:
>>>> Using {0} to initialize mmc_cmd, before filling the structure.
>>>>
>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>>> Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
>>>> Cc: Kever Yang <kever.yang@rock-chips.com>
>>>> Cc: Eric Nelson <eric@nelint.com>
>>>> Cc: Stephen Warren <swarren@nvidia.com>
>>>> ---
>>>>  drivers/mmc/mmc.c       | 28 ++++++++++++++--------------
>>>>  drivers/mmc/mmc_write.c |  4 ++--
>>>>  2 files changed, 16 insertions(+), 16 deletions(-)
>>>
>>>Why is this needed? Does it affect code size?
>>
>> I add a timeout entry in mmc_cmd, but I do not want to specify a value for timeout
>> for each mmc_cmd. So I use {0}.
>>
>> Then to those who want use timeout, a value can be assigned to timeout, just like
>> I added in the patchset for mmc erase.
>
>Instead of that, can you assign the value to 0, or whatever you want?
>You might want to have a helper like mmc_init_cmd() to zero things
>out.

Ok. Will follow your suggestion in V3.

Thanks,
Peng.

>
>Regards,
>Simon

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

end of thread, other threads:[~2016-08-18  6:21 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20160811110023epcas1p4289773001396502dbc59a4638887cae5@epcas1p4.samsung.com>
2016-08-11 11:00 ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Peng Fan
2016-08-11 11:00   ` [U-Boot] [PATCH 2/5] mmc: initialize mmc_cmd with 0 Peng Fan
2016-08-12 17:20     ` Simon Glass
2016-08-13  1:41       ` Peng Fan
2016-08-18  3:45         ` Simon Glass
2016-08-18  6:21           ` Peng Fan
2016-08-11 11:00   ` [U-Boot] [PATCH 3/5] mmc: sd: add erase timeout support Peng Fan
2016-08-11 11:00   ` [U-Boot] [PATCH 4/5] mmc: esdhc: change timeout value Peng Fan
2016-08-11 11:00   ` [U-Boot] [PATCH 5/5] mmc: sd: optimize erase Peng Fan
2016-08-12  0:53     ` Fabio Estevam
2016-08-12  3:02       ` Peng Fan
2016-08-12  3:12         ` Fabio Estevam
2016-08-12  2:08   ` [U-Boot] [PATCH 1/5] mmc: sd: extracting erase timeout information from sd status Jaehoon Chung
2016-08-12  2:55     ` Peng Fan
2016-08-12  3:51       ` Jaehoon Chung

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