linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5
@ 2012-05-17  9:40 Seungwon Jeon
  2012-05-20 11:31 ` merez
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Seungwon Jeon @ 2012-05-17  9:40 UTC (permalink / raw)
  To: linux-mmc; +Cc: 'Chris Ball', merez, linux-kernel

This patch adds packed command feature of eMMC4.5.
The maximum number for packing read(or write) is offered
and exception event relevant to packed command which is
used for error handling is enabled. If host wants to use
this feature, MMC_CAP2_PACKED_CMD should be set.

Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
 drivers/mmc/core/mmc.c   |   24 ++++++++++++++++++++++++
 include/linux/mmc/card.h |    3 +++
 include/linux/mmc/host.h |    4 ++++
 include/linux/mmc/mmc.h  |   15 +++++++++++++++
 4 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 2f0e11c..8310ce8 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -516,6 +516,11 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		} else {
 			card->ext_csd.data_tag_unit_size = 0;
 		}
+
+		card->ext_csd.max_packed_writes =
+			ext_csd[EXT_CSD_MAX_PACKED_WRITES];
+		card->ext_csd.max_packed_reads =
+			ext_csd[EXT_CSD_MAX_PACKED_READS];
 	}
 
 out:
@@ -1244,6 +1249,25 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 		}
 	}
 
+	if ((host->caps2 & MMC_CAP2_PACKED_CMD) &&
+			(card->ext_csd.max_packed_writes > 0) &&
+			(card->ext_csd.max_packed_reads > 0)) {
+		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+				EXT_CSD_EXP_EVENTS_CTRL,
+				EXT_CSD_PACKED_EVENT_EN,
+				card->ext_csd.generic_cmd6_time);
+		if (err && err != -EBADMSG)
+			goto free_card;
+		if (err) {
+			pr_warning("%s: Enabling packed event failed\n",
+					mmc_hostname(card->host));
+			card->ext_csd.packed_event_en = 0;
+			err = 0;
+		} else {
+			card->ext_csd.packed_event_en = 1;
+		}
+	}
+
 	if (!oldcard)
 		host->card = card;
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index d76513b..4aeb4e9 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -53,6 +53,9 @@ struct mmc_ext_csd {
 	u8			part_config;
 	u8			cache_ctrl;
 	u8			rst_n_function;
+	u8			max_packed_writes;
+	u8			max_packed_reads;
+	u8			packed_event_en;
 	unsigned int		part_time;		/* Units: ms */
 	unsigned int		sa_timeout;		/* Units: 100ns */
 	unsigned int		generic_cmd6_time;	/* Units: 10ms */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 0707d22..9d0d946 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -238,6 +238,10 @@ struct mmc_host {
 #define MMC_CAP2_BROKEN_VOLTAGE	(1 << 7)	/* Use the broken voltage */
 #define MMC_CAP2_DETECT_ON_ERR	(1 << 8)	/* On I/O err check card removal */
 #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
+#define MMC_CAP2_PACKED_RD	(1 << 10)	/* Allow packed read */
+#define MMC_CAP2_PACKED_WR	(1 << 11)	/* Allow packed write */
+#define MMC_CAP2_PACKED_CMD	(MMC_CAP2_PACKED_RD | \
+				 MMC_CAP2_PACKED_WR) /* Allow packed commands */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 	unsigned int        power_notify_type;
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index d425cab..254901a 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -139,6 +139,7 @@ static inline bool mmc_op_multi(u32 opcode)
 #define R1_CURRENT_STATE(x)	((x & 0x00001E00) >> 9)	/* sx, b (4 bits) */
 #define R1_READY_FOR_DATA	(1 << 8)	/* sx, a */
 #define R1_SWITCH_ERROR		(1 << 7)	/* sx, c */
+#define R1_EXP_EVENT		(1 << 6)	/* sr, a */
 #define R1_APP_CMD		(1 << 5)	/* sr, c */
 
 #define R1_STATE_IDLE	0
@@ -274,6 +275,10 @@ struct _mmc_csd {
 #define EXT_CSD_FLUSH_CACHE		32      /* W */
 #define EXT_CSD_CACHE_CTRL		33      /* R/W */
 #define EXT_CSD_POWER_OFF_NOTIFICATION	34	/* R/W */
+#define EXT_CSD_PACKED_FAILURE_INDEX	35	/* RO */
+#define EXT_CSD_PACKED_CMD_STATUS	36	/* RO */
+#define EXT_CSD_EXP_EVENTS_STATUS	54	/* RO, 2 bytes */
+#define EXT_CSD_EXP_EVENTS_CTRL	56	/* R/W, 2 bytes */
 #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
 #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
@@ -318,6 +323,8 @@ struct _mmc_csd {
 #define EXT_CSD_CACHE_SIZE		249	/* RO, 4 bytes */
 #define EXT_CSD_TAG_UNIT_SIZE		498	/* RO */
 #define EXT_CSD_DATA_TAG_SUPPORT	499	/* RO */
+#define EXT_CSD_MAX_PACKED_WRITES	500	/* RO */
+#define EXT_CSD_MAX_PACKED_READS	501	/* RO */
 #define EXT_CSD_HPI_FEATURES		503	/* RO */
 
 /*
@@ -377,6 +384,14 @@ struct _mmc_csd {
 #define EXT_CSD_PWR_CL_4BIT_MASK	0x0F	/* 8 bit PWR CLS */
 #define EXT_CSD_PWR_CL_8BIT_SHIFT	4
 #define EXT_CSD_PWR_CL_4BIT_SHIFT	0
+
+#define EXT_CSD_PACKED_EVENT_EN	(1 << 3)
+
+#define EXT_CSD_PACKED_FAILURE	(1 << 3)
+
+#define EXT_CSD_PACKED_GENERIC_ERROR	(1 << 0)
+#define EXT_CSD_PACKED_INDEXED_ERROR	(1 << 1)
+
 /*
  * MMC_SWITCH access modes
  */
-- 
1.7.0.4



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

* Re: [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5
  2012-05-17  9:40 [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5 Seungwon Jeon
@ 2012-05-20 11:31 ` merez
  2012-05-21 11:30 ` Subhash Jadavani
  2012-05-29 11:37 ` merez
  2 siblings, 0 replies; 8+ messages in thread
From: merez @ 2012-05-20 11:31 UTC (permalink / raw)
  To: Seungwon Jeon; +Cc: linux-mmc, 'Chris Ball', merez, linux-kernel

Looks good to me.

Thanks,
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

> This patch adds packed command feature of eMMC4.5.
> The maximum number for packing read(or write) is offered
> and exception event relevant to packed command which is
> used for error handling is enabled. If host wants to use
> this feature, MMC_CAP2_PACKED_CMD should be set.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
>  drivers/mmc/core/mmc.c   |   24 ++++++++++++++++++++++++
>  include/linux/mmc/card.h |    3 +++
>  include/linux/mmc/host.h |    4 ++++
>  include/linux/mmc/mmc.h  |   15 +++++++++++++++
>  4 files changed, 46 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 2f0e11c..8310ce8 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -516,6 +516,11 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8
> *ext_csd)
>  		} else {
>  			card->ext_csd.data_tag_unit_size = 0;
>  		}
> +
> +		card->ext_csd.max_packed_writes =
> +			ext_csd[EXT_CSD_MAX_PACKED_WRITES];
> +		card->ext_csd.max_packed_reads =
> +			ext_csd[EXT_CSD_MAX_PACKED_READS];
>  	}
>
>  out:
> @@ -1244,6 +1249,25 @@ static int mmc_init_card(struct mmc_host *host, u32
> ocr,
>  		}
>  	}
>
> +	if ((host->caps2 & MMC_CAP2_PACKED_CMD) &&
> +			(card->ext_csd.max_packed_writes > 0) &&
> +			(card->ext_csd.max_packed_reads > 0)) {
> +		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +				EXT_CSD_EXP_EVENTS_CTRL,
> +				EXT_CSD_PACKED_EVENT_EN,
> +				card->ext_csd.generic_cmd6_time);
> +		if (err && err != -EBADMSG)
> +			goto free_card;
> +		if (err) {
> +			pr_warning("%s: Enabling packed event failed\n",
> +					mmc_hostname(card->host));
> +			card->ext_csd.packed_event_en = 0;
> +			err = 0;
> +		} else {
> +			card->ext_csd.packed_event_en = 1;
> +		}
> +	}
> +
>  	if (!oldcard)
>  		host->card = card;
>
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index d76513b..4aeb4e9 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -53,6 +53,9 @@ struct mmc_ext_csd {
>  	u8			part_config;
>  	u8			cache_ctrl;
>  	u8			rst_n_function;
> +	u8			max_packed_writes;
> +	u8			max_packed_reads;
> +	u8			packed_event_en;
>  	unsigned int		part_time;		/* Units: ms */
>  	unsigned int		sa_timeout;		/* Units: 100ns */
>  	unsigned int		generic_cmd6_time;	/* Units: 10ms */
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 0707d22..9d0d946 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -238,6 +238,10 @@ struct mmc_host {
>  #define MMC_CAP2_BROKEN_VOLTAGE	(1 << 7)	/* Use the broken voltage */
>  #define MMC_CAP2_DETECT_ON_ERR	(1 << 8)	/* On I/O err check card removal
> */
>  #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
> +#define MMC_CAP2_PACKED_RD	(1 << 10)	/* Allow packed read */
> +#define MMC_CAP2_PACKED_WR	(1 << 11)	/* Allow packed write */
> +#define MMC_CAP2_PACKED_CMD	(MMC_CAP2_PACKED_RD | \
> +				 MMC_CAP2_PACKED_WR) /* Allow packed commands */
>
>  	mmc_pm_flag_t		pm_caps;	/* supported pm features */
>  	unsigned int        power_notify_type;
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index d425cab..254901a 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -139,6 +139,7 @@ static inline bool mmc_op_multi(u32 opcode)
>  #define R1_CURRENT_STATE(x)	((x & 0x00001E00) >> 9)	/* sx, b (4 bits) */
>  #define R1_READY_FOR_DATA	(1 << 8)	/* sx, a */
>  #define R1_SWITCH_ERROR		(1 << 7)	/* sx, c */
> +#define R1_EXP_EVENT		(1 << 6)	/* sr, a */
>  #define R1_APP_CMD		(1 << 5)	/* sr, c */
>
>  #define R1_STATE_IDLE	0
> @@ -274,6 +275,10 @@ struct _mmc_csd {
>  #define EXT_CSD_FLUSH_CACHE		32      /* W */
>  #define EXT_CSD_CACHE_CTRL		33      /* R/W */
>  #define EXT_CSD_POWER_OFF_NOTIFICATION	34	/* R/W */
> +#define EXT_CSD_PACKED_FAILURE_INDEX	35	/* RO */
> +#define EXT_CSD_PACKED_CMD_STATUS	36	/* RO */
> +#define EXT_CSD_EXP_EVENTS_STATUS	54	/* RO, 2 bytes */
> +#define EXT_CSD_EXP_EVENTS_CTRL	56	/* R/W, 2 bytes */
>  #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
>  #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
>  #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
> @@ -318,6 +323,8 @@ struct _mmc_csd {
>  #define EXT_CSD_CACHE_SIZE		249	/* RO, 4 bytes */
>  #define EXT_CSD_TAG_UNIT_SIZE		498	/* RO */
>  #define EXT_CSD_DATA_TAG_SUPPORT	499	/* RO */
> +#define EXT_CSD_MAX_PACKED_WRITES	500	/* RO */
> +#define EXT_CSD_MAX_PACKED_READS	501	/* RO */
>  #define EXT_CSD_HPI_FEATURES		503	/* RO */
>
>  /*
> @@ -377,6 +384,14 @@ struct _mmc_csd {
>  #define EXT_CSD_PWR_CL_4BIT_MASK	0x0F	/* 8 bit PWR CLS */
>  #define EXT_CSD_PWR_CL_8BIT_SHIFT	4
>  #define EXT_CSD_PWR_CL_4BIT_SHIFT	0
> +
> +#define EXT_CSD_PACKED_EVENT_EN	(1 << 3)
> +
> +#define EXT_CSD_PACKED_FAILURE	(1 << 3)
> +
> +#define EXT_CSD_PACKED_GENERIC_ERROR	(1 << 0)
> +#define EXT_CSD_PACKED_INDEXED_ERROR	(1 << 1)
> +
>  /*
>   * MMC_SWITCH access modes
>   */
> --
> 1.7.0.4
>
>
>



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

* Re: [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5
  2012-05-17  9:40 [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5 Seungwon Jeon
  2012-05-20 11:31 ` merez
@ 2012-05-21 11:30 ` Subhash Jadavani
  2012-05-22  3:35   ` Seungwon Jeon
  2012-05-29 11:37 ` merez
  2 siblings, 1 reply; 8+ messages in thread
From: Subhash Jadavani @ 2012-05-21 11:30 UTC (permalink / raw)
  To: Seungwon Jeon; +Cc: linux-mmc, 'Chris Ball', merez, linux-kernel


Hi Seungwon,

Sorry for commenting on this late. I have one comment below. Please 
check if it makes sense or not.

On 5/17/2012 3:10 PM, Seungwon Jeon wrote:
> This patch adds packed command feature of eMMC4.5.
> The maximum number for packing read(or write) is offered
> and exception event relevant to packed command which is
> used for error handling is enabled. If host wants to use
> this feature, MMC_CAP2_PACKED_CMD should be set.
>
> Signed-off-by: Seungwon Jeon<tgih.jun@samsung.com>
> ---
>   drivers/mmc/core/mmc.c   |   24 ++++++++++++++++++++++++
>   include/linux/mmc/card.h |    3 +++
>   include/linux/mmc/host.h |    4 ++++
>   include/linux/mmc/mmc.h  |   15 +++++++++++++++
>   4 files changed, 46 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 2f0e11c..8310ce8 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -516,6 +516,11 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>   		} else {
>   			card->ext_csd.data_tag_unit_size = 0;
>   		}
> +
> +		card->ext_csd.max_packed_writes =
> +			ext_csd[EXT_CSD_MAX_PACKED_WRITES];
> +		card->ext_csd.max_packed_reads =
> +			ext_csd[EXT_CSD_MAX_PACKED_READS];
>   	}
>
>   out:
> @@ -1244,6 +1249,25 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>   		}
>   	}
>
> +	if ((host->caps2&  MMC_CAP2_PACKED_CMD)&&
> +			(card->ext_csd.max_packed_writes>  0)&&
> +			(card->ext_csd.max_packed_reads>  0)) {
why both max_packed_reads and max_packed_writes need to be non-zero to 
enable the write packing event? What if write packing is not supported 
by card and only read packing is supported? Shouldn't we still make the 
read packing work?

I would suggest to change it like this:
     if (  (host->caps2 & MMC_CAP2_PACKED_WR && 
card->ext_csd.max_packed_writes > 0) ||
            (host->caps2 & MMC_CAP2_PACKED_RD && 
card->ext_csd.max_packed_reads > 0)
> +		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +				EXT_CSD_EXP_EVENTS_CTRL,
> +				EXT_CSD_PACKED_EVENT_EN,
> +				card->ext_csd.generic_cmd6_time);
> +		if (err&&  err != -EBADMSG)
> +			goto free_card;
> +		if (err) {
> +			pr_warning("%s: Enabling packed event failed\n",
> +					mmc_hostname(card->host));
> +			card->ext_csd.packed_event_en = 0;
> +			err = 0;
> +		} else {
> +			card->ext_csd.packed_event_en = 1;
> +		}
> +	}
> +
>   	if (!oldcard)
>   		host->card = card;
>
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index d76513b..4aeb4e9 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -53,6 +53,9 @@ struct mmc_ext_csd {
>   	u8			part_config;
>   	u8			cache_ctrl;
>   	u8			rst_n_function;
> +	u8			max_packed_writes;
> +	u8			max_packed_reads;
> +	u8			packed_event_en;
>   	unsigned int		part_time;		/* Units: ms */
>   	unsigned int		sa_timeout;		/* Units: 100ns */
>   	unsigned int		generic_cmd6_time;	/* Units: 10ms */
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 0707d22..9d0d946 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -238,6 +238,10 @@ struct mmc_host {
>   #define MMC_CAP2_BROKEN_VOLTAGE	(1<<  7)	/* Use the broken voltage */
>   #define MMC_CAP2_DETECT_ON_ERR	(1<<  8)	/* On I/O err check card removal */
>   #define MMC_CAP2_HC_ERASE_SZ	(1<<  9)	/* High-capacity erase size */
> +#define MMC_CAP2_PACKED_RD	(1<<  10)	/* Allow packed read */
> +#define MMC_CAP2_PACKED_WR	(1<<  11)	/* Allow packed write */
> +#define MMC_CAP2_PACKED_CMD	(MMC_CAP2_PACKED_RD | \
> +				 MMC_CAP2_PACKED_WR) /* Allow packed commands */
>
>   	mmc_pm_flag_t		pm_caps;	/* supported pm features */
>   	unsigned int        power_notify_type;
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index d425cab..254901a 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -139,6 +139,7 @@ static inline bool mmc_op_multi(u32 opcode)
>   #define R1_CURRENT_STATE(x)	((x&  0x00001E00)>>  9)	/* sx, b (4 bits) */
>   #define R1_READY_FOR_DATA	(1<<  8)	/* sx, a */
>   #define R1_SWITCH_ERROR		(1<<  7)	/* sx, c */
> +#define R1_EXP_EVENT		(1<<  6)	/* sr, a */
>   #define R1_APP_CMD		(1<<  5)	/* sr, c */
>
>   #define R1_STATE_IDLE	0
> @@ -274,6 +275,10 @@ struct _mmc_csd {
>   #define EXT_CSD_FLUSH_CACHE		32      /* W */
>   #define EXT_CSD_CACHE_CTRL		33      /* R/W */
>   #define EXT_CSD_POWER_OFF_NOTIFICATION	34	/* R/W */
> +#define EXT_CSD_PACKED_FAILURE_INDEX	35	/* RO */
> +#define EXT_CSD_PACKED_CMD_STATUS	36	/* RO */
> +#define EXT_CSD_EXP_EVENTS_STATUS	54	/* RO, 2 bytes */
> +#define EXT_CSD_EXP_EVENTS_CTRL	56	/* R/W, 2 bytes */
>   #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
>   #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
>   #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
> @@ -318,6 +323,8 @@ struct _mmc_csd {
>   #define EXT_CSD_CACHE_SIZE		249	/* RO, 4 bytes */
>   #define EXT_CSD_TAG_UNIT_SIZE		498	/* RO */
>   #define EXT_CSD_DATA_TAG_SUPPORT	499	/* RO */
> +#define EXT_CSD_MAX_PACKED_WRITES	500	/* RO */
> +#define EXT_CSD_MAX_PACKED_READS	501	/* RO */
>   #define EXT_CSD_HPI_FEATURES		503	/* RO */
>
>   /*
> @@ -377,6 +384,14 @@ struct _mmc_csd {
>   #define EXT_CSD_PWR_CL_4BIT_MASK	0x0F	/* 8 bit PWR CLS */
>   #define EXT_CSD_PWR_CL_8BIT_SHIFT	4
>   #define EXT_CSD_PWR_CL_4BIT_SHIFT	0
> +
> +#define EXT_CSD_PACKED_EVENT_EN	(1<<  3)
> +
> +#define EXT_CSD_PACKED_FAILURE	(1<<  3)
> +
> +#define EXT_CSD_PACKED_GENERIC_ERROR	(1<<  0)
> +#define EXT_CSD_PACKED_INDEXED_ERROR	(1<<  1)
> +
>   /*
>    * MMC_SWITCH access modes
>    */


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

* RE: [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5
  2012-05-21 11:30 ` Subhash Jadavani
@ 2012-05-22  3:35   ` Seungwon Jeon
  0 siblings, 0 replies; 8+ messages in thread
From: Seungwon Jeon @ 2012-05-22  3:35 UTC (permalink / raw)
  To: 'Subhash Jadavani'
  Cc: linux-mmc, 'Chris Ball', merez, linux-kernel

Subhash Jadavani <subhashj@codeaurora.org> wrote:
> Hi Seungwon,
> 
> Sorry for commenting on this late. I have one comment below. Please
> check if it makes sense or not.
> 
> On 5/17/2012 3:10 PM, Seungwon Jeon wrote:
> > This patch adds packed command feature of eMMC4.5.
> > The maximum number for packing read(or write) is offered
> > and exception event relevant to packed command which is
> > used for error handling is enabled. If host wants to use
> > this feature, MMC_CAP2_PACKED_CMD should be set.
> >
> > Signed-off-by: Seungwon Jeon<tgih.jun@samsung.com>
> > ---
> >   drivers/mmc/core/mmc.c   |   24 ++++++++++++++++++++++++
> >   include/linux/mmc/card.h |    3 +++
> >   include/linux/mmc/host.h |    4 ++++
> >   include/linux/mmc/mmc.h  |   15 +++++++++++++++
> >   4 files changed, 46 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index 2f0e11c..8310ce8 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -516,6 +516,11 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
> >   		} else {
> >   			card->ext_csd.data_tag_unit_size = 0;
> >   		}
> > +
> > +		card->ext_csd.max_packed_writes =
> > +			ext_csd[EXT_CSD_MAX_PACKED_WRITES];
> > +		card->ext_csd.max_packed_reads =
> > +			ext_csd[EXT_CSD_MAX_PACKED_READS];
> >   	}
> >
> >   out:
> > @@ -1244,6 +1249,25 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> >   		}
> >   	}
> >
> > +	if ((host->caps2&  MMC_CAP2_PACKED_CMD)&&
> > +			(card->ext_csd.max_packed_writes>  0)&&
> > +			(card->ext_csd.max_packed_reads>  0)) {
> why both max_packed_reads and max_packed_writes need to be non-zero to
> enable the write packing event? What if write packing is not supported
> by card and only read packing is supported? Shouldn't we still make the
> read packing work?
I have not experienced that case. But it seems like possible actually.
Spec doesn't mention two should be satisfied for packed command.
I'll apply your comment.

Best regards,
Seungwon Jeon
> 
> I would suggest to change it like this:
>      if (  (host->caps2 & MMC_CAP2_PACKED_WR &&
> card->ext_csd.max_packed_writes > 0) ||
>             (host->caps2 & MMC_CAP2_PACKED_RD &&
> card->ext_csd.max_packed_reads > 0)
> > +		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> > +				EXT_CSD_EXP_EVENTS_CTRL,
> > +				EXT_CSD_PACKED_EVENT_EN,
> > +				card->ext_csd.generic_cmd6_time);
> > +		if (err&&  err != -EBADMSG)
> > +			goto free_card;
> > +		if (err) {
> > +			pr_warning("%s: Enabling packed event failed\n",
> > +					mmc_hostname(card->host));
> > +			card->ext_csd.packed_event_en = 0;
> > +			err = 0;
> > +		} else {
> > +			card->ext_csd.packed_event_en = 1;
> > +		}
> > +	}
> > +
> >   	if (!oldcard)
> >   		host->card = card;
> >
> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> > index d76513b..4aeb4e9 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -53,6 +53,9 @@ struct mmc_ext_csd {
> >   	u8			part_config;
> >   	u8			cache_ctrl;
> >   	u8			rst_n_function;
> > +	u8			max_packed_writes;
> > +	u8			max_packed_reads;
> > +	u8			packed_event_en;
> >   	unsigned int		part_time;		/* Units: ms */
> >   	unsigned int		sa_timeout;		/* Units: 100ns */
> >   	unsigned int		generic_cmd6_time;	/* Units: 10ms */
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > index 0707d22..9d0d946 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -238,6 +238,10 @@ struct mmc_host {
> >   #define MMC_CAP2_BROKEN_VOLTAGE	(1<<  7)	/* Use the broken voltage */
> >   #define MMC_CAP2_DETECT_ON_ERR	(1<<  8)	/* On I/O err check card removal */
> >   #define MMC_CAP2_HC_ERASE_SZ	(1<<  9)	/* High-capacity erase size */
> > +#define MMC_CAP2_PACKED_RD	(1<<  10)	/* Allow packed read */
> > +#define MMC_CAP2_PACKED_WR	(1<<  11)	/* Allow packed write */
> > +#define MMC_CAP2_PACKED_CMD	(MMC_CAP2_PACKED_RD | \
> > +				 MMC_CAP2_PACKED_WR) /* Allow packed commands */
> >
> >   	mmc_pm_flag_t		pm_caps;	/* supported pm features */
> >   	unsigned int        power_notify_type;
> > diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> > index d425cab..254901a 100644
> > --- a/include/linux/mmc/mmc.h
> > +++ b/include/linux/mmc/mmc.h
> > @@ -139,6 +139,7 @@ static inline bool mmc_op_multi(u32 opcode)
> >   #define R1_CURRENT_STATE(x)	((x&  0x00001E00)>>  9)	/* sx, b (4 bits) */
> >   #define R1_READY_FOR_DATA	(1<<  8)	/* sx, a */
> >   #define R1_SWITCH_ERROR		(1<<  7)	/* sx, c */
> > +#define R1_EXP_EVENT		(1<<  6)	/* sr, a */
> >   #define R1_APP_CMD		(1<<  5)	/* sr, c */
> >
> >   #define R1_STATE_IDLE	0
> > @@ -274,6 +275,10 @@ struct _mmc_csd {
> >   #define EXT_CSD_FLUSH_CACHE		32      /* W */
> >   #define EXT_CSD_CACHE_CTRL		33      /* R/W */
> >   #define EXT_CSD_POWER_OFF_NOTIFICATION	34	/* R/W */
> > +#define EXT_CSD_PACKED_FAILURE_INDEX	35	/* RO */
> > +#define EXT_CSD_PACKED_CMD_STATUS	36	/* RO */
> > +#define EXT_CSD_EXP_EVENTS_STATUS	54	/* RO, 2 bytes */
> > +#define EXT_CSD_EXP_EVENTS_CTRL	56	/* R/W, 2 bytes */
> >   #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
> >   #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
> >   #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
> > @@ -318,6 +323,8 @@ struct _mmc_csd {
> >   #define EXT_CSD_CACHE_SIZE		249	/* RO, 4 bytes */
> >   #define EXT_CSD_TAG_UNIT_SIZE		498	/* RO */
> >   #define EXT_CSD_DATA_TAG_SUPPORT	499	/* RO */
> > +#define EXT_CSD_MAX_PACKED_WRITES	500	/* RO */
> > +#define EXT_CSD_MAX_PACKED_READS	501	/* RO */
> >   #define EXT_CSD_HPI_FEATURES		503	/* RO */
> >
> >   /*
> > @@ -377,6 +384,14 @@ struct _mmc_csd {
> >   #define EXT_CSD_PWR_CL_4BIT_MASK	0x0F	/* 8 bit PWR CLS */
> >   #define EXT_CSD_PWR_CL_8BIT_SHIFT	4
> >   #define EXT_CSD_PWR_CL_4BIT_SHIFT	0
> > +
> > +#define EXT_CSD_PACKED_EVENT_EN	(1<<  3)
> > +
> > +#define EXT_CSD_PACKED_FAILURE	(1<<  3)
> > +
> > +#define EXT_CSD_PACKED_GENERIC_ERROR	(1<<  0)
> > +#define EXT_CSD_PACKED_INDEXED_ERROR	(1<<  1)
> > +
> >   /*
> >    * MMC_SWITCH access modes
> >    */
> 
> --
> 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] 8+ messages in thread

* Re: [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5
  2012-05-17  9:40 [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5 Seungwon Jeon
  2012-05-20 11:31 ` merez
  2012-05-21 11:30 ` Subhash Jadavani
@ 2012-05-29 11:37 ` merez
  2012-05-29 12:45   ` Subhash Jadavani
  2012-05-30  0:20   ` Seungwon Jeon
  2 siblings, 2 replies; 8+ messages in thread
From: merez @ 2012-05-29 11:37 UTC (permalink / raw)
  To: Seungwon Jeon; +Cc: linux-mmc, 'Chris Ball', merez, linux-kernel

> @@ -1244,6 +1249,25 @@ static int mmc_init_card(struct mmc_host *host, u32
> ocr,
>  		}
>  	}
>
> +	if ((host->caps2 & MMC_CAP2_PACKED_CMD) &&
> +			(card->ext_csd.max_packed_writes > 0) &&
> +			(card->ext_csd.max_packed_reads > 0)) {
> +		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +				EXT_CSD_EXP_EVENTS_CTRL,
> +				EXT_CSD_PACKED_EVENT_EN,
> +				card->ext_csd.generic_cmd6_time);
> +		if (err && err != -EBADMSG)
> +			goto free_card;
> +		if (err) {
> +			pr_warning("%s: Enabling packed event failed\n",
> +					mmc_hostname(card->host));
> +			card->ext_csd.packed_event_en = 0;
> +			err = 0;
> +		} else {
> +			card->ext_csd.packed_event_en = 1;
> +		}
> +	}
> +
The above shoud not be performed in case of resume. Therefore it needs to
be done only if (!oldcard)

Thanks,
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum


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

* RE: [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5
  2012-05-29 11:37 ` merez
@ 2012-05-29 12:45   ` Subhash Jadavani
  2012-05-31 19:19     ` merez
  2012-05-30  0:20   ` Seungwon Jeon
  1 sibling, 1 reply; 8+ messages in thread
From: Subhash Jadavani @ 2012-05-29 12:45 UTC (permalink / raw)
  To: merez, 'Seungwon Jeon'
  Cc: linux-mmc, 'Chris Ball', linux-kernel



> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of merez@codeaurora.org
> Sent: Tuesday, May 29, 2012 5:08 PM
> To: Seungwon Jeon
> Cc: linux-mmc@vger.kernel.org; 'Chris Ball'; merez@codeaurora.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v6 1/3] mmc: core: Add packed command feature of
> eMMC4.5
> 
> > @@ -1244,6 +1249,25 @@ static int mmc_init_card(struct mmc_host *host,
> > u32 ocr,
> >  		}
> >  	}
> >
> > +	if ((host->caps2 & MMC_CAP2_PACKED_CMD) &&
> > +			(card->ext_csd.max_packed_writes > 0) &&
> > +			(card->ext_csd.max_packed_reads > 0)) {
> > +		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> > +				EXT_CSD_EXP_EVENTS_CTRL,
> > +				EXT_CSD_PACKED_EVENT_EN,
> > +				card->ext_csd.generic_cmd6_time);
> > +		if (err && err != -EBADMSG)
> > +			goto free_card;
> > +		if (err) {
> > +			pr_warning("%s: Enabling packed event failed\n",
> > +					mmc_hostname(card->host));
> > +			card->ext_csd.packed_event_en = 0;
> > +			err = 0;
> > +		} else {
> > +			card->ext_csd.packed_event_en = 1;
> > +		}
> > +	}
> > +
> The above shoud not be performed in case of resume. Therefore it needs to
be
> done only if (!oldcard)

Maya,
What if eMMC power (VCC and VCCQ) was removed during suspend? Then in that
case during resume, we have to send this mmc_switch() command again. So this
operation should not be under "if (!oldcard)" check.

Regards,
Subhash


> 
> Thanks,
> Maya Erez
> Consultant for Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> 
> --
> 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] 8+ messages in thread

* RE: [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5
  2012-05-29 11:37 ` merez
  2012-05-29 12:45   ` Subhash Jadavani
@ 2012-05-30  0:20   ` Seungwon Jeon
  1 sibling, 0 replies; 8+ messages in thread
From: Seungwon Jeon @ 2012-05-30  0:20 UTC (permalink / raw)
  To: merez; +Cc: linux-mmc, 'Chris Ball', linux-kernel

Maya Erez <merez@codeaurora.org> wrote:
> > @@ -1244,6 +1249,25 @@ static int mmc_init_card(struct mmc_host *host, u32
> > ocr,
> >  		}
> >  	}
> >
> > +	if ((host->caps2 & MMC_CAP2_PACKED_CMD) &&
> > +			(card->ext_csd.max_packed_writes > 0) &&
> > +			(card->ext_csd.max_packed_reads > 0)) {
> > +		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> > +				EXT_CSD_EXP_EVENTS_CTRL,
> > +				EXT_CSD_PACKED_EVENT_EN,
> > +				card->ext_csd.generic_cmd6_time);
> > +		if (err && err != -EBADMSG)
> > +			goto free_card;
> > +		if (err) {
> > +			pr_warning("%s: Enabling packed event failed\n",
> > +					mmc_hostname(card->host));
> > +			card->ext_csd.packed_event_en = 0;
> > +			err = 0;
> > +		} else {
> > +			card->ext_csd.packed_event_en = 1;
> > +		}
> > +	}
> > +
> The above shoud not be performed in case of resume. Therefore it needs to
> be done only if (!oldcard)
Could you explain the reason for this?

Thanks,
Seungwon Jeon
> 
> Thanks,
> Maya Erez
> Consultant for Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> 
> --
> 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] 8+ messages in thread

* RE: [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5
  2012-05-29 12:45   ` Subhash Jadavani
@ 2012-05-31 19:19     ` merez
  0 siblings, 0 replies; 8+ messages in thread
From: merez @ 2012-05-31 19:19 UTC (permalink / raw)
  To: Subhash Jadavani
  Cc: merez, 'Seungwon Jeon', linux-mmc, 'Chris Ball',
	linux-kernel


>
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>> owner@vger.kernel.org] On Behalf Of merez@codeaurora.org
>> Sent: Tuesday, May 29, 2012 5:08 PM
>> To: Seungwon Jeon
>> Cc: linux-mmc@vger.kernel.org; 'Chris Ball'; merez@codeaurora.org;
>> linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH v6 1/3] mmc: core: Add packed command feature of
>> eMMC4.5
>>
>> > @@ -1244,6 +1249,25 @@ static int mmc_init_card(struct mmc_host *host,
>> > u32 ocr,
>> >  		}
>> >  	}
>> >
>> > +	if ((host->caps2 & MMC_CAP2_PACKED_CMD) &&
>> > +			(card->ext_csd.max_packed_writes > 0) &&
>> > +			(card->ext_csd.max_packed_reads > 0)) {
>> > +		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> > +				EXT_CSD_EXP_EVENTS_CTRL,
>> > +				EXT_CSD_PACKED_EVENT_EN,
>> > +				card->ext_csd.generic_cmd6_time);
>> > +		if (err && err != -EBADMSG)
>> > +			goto free_card;
>> > +		if (err) {
>> > +			pr_warning("%s: Enabling packed event failed\n",
>> > +					mmc_hostname(card->host));
>> > +			card->ext_csd.packed_event_en = 0;
>> > +			err = 0;
>> > +		} else {
>> > +			card->ext_csd.packed_event_en = 1;
>> > +		}
>> > +	}
>> > +
>> The above shoud not be performed in case of resume. Therefore it needs
>> to
> be
>> done only if (!oldcard)
>
> Maya,
> What if eMMC power (VCC and VCCQ) was removed during suspend? Then in that
> case during resume, we have to send this mmc_switch() command again. So
> this
> operation should not be under "if (!oldcard)" check.
>
> Regards,
> Subhash
Thanks for the correction, my mistake.

Thanks,
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum


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

end of thread, other threads:[~2012-05-31 19:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-17  9:40 [PATCH v6 1/3] mmc: core: Add packed command feature of eMMC4.5 Seungwon Jeon
2012-05-20 11:31 ` merez
2012-05-21 11:30 ` Subhash Jadavani
2012-05-22  3:35   ` Seungwon Jeon
2012-05-29 11:37 ` merez
2012-05-29 12:45   ` Subhash Jadavani
2012-05-31 19:19     ` merez
2012-05-30  0:20   ` Seungwon Jeon

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).