lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 29 Oct 2014 11:33:57 +0200
From:	Adrian Hunter <adrian.hunter@...el.com>
To:	À¯ÇÑ°æ <hankyung.yu@....com>,
	'Chanho Min' <chanho.min@....com>
CC:	'Chris Ball' <chris@...ntf.net>,
	'Ulf Hansson' <ulf.hansson@...aro.org>,
	'Seungwon Jeon' <tgih.jun@...sung.com>,
	'Jaehoon Chung' <jh80.chung@...sung.com>,
	linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
	'HyoJun Im' <hyojun.im@....com>, gunho.lee@....com
Subject: Re: [PATCH] mmc:core: fix hs400 timing selection

On 29/10/14 01:30, À¯ÇÑ°æ wrote:
> Hi I'm Hankyung Yu
> 
> I will answer instead Chanho Min
> 
> HS200 mode right thing to support less than 52Mhz
> 
> However CLK <-> DATA delay timing is dependent on the clock.
> 
> So only lower clock without adjusting the timing and mode of a control h/w
> ever the problem may occur.
> 
> So change the operating mode and to lower the clock.

I meant something different.

With your patch I find that __mmc_switch() -> send_status() fails.

So I suggest something like this:

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 92247c2..195d48a 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1045,21 +1045,28 @@ static int mmc_select_hs400(struct mmc_card *card)
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
 			   card->ext_csd.generic_cmd6_time,
-			   true, true, true);
+			   true, false, false);
+	if (!err) {
+		u32 status;
+
+		/*
+		 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
+		 * be set to a value not greater than 52MHz after the HS_TIMING
+		 * field is set to 0x1.
+		*/
+		mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
+		mmc_set_bus_speed(card);
+
+		err = __mmc_send_status(card, &status, false, 1);
+		if (!err)
+			err = mmc_check_switch_status(card->host, status);
+	}
 	if (err) {
 		pr_err("%s: switch to high-speed from hs200 failed, err:%d\n",
 			mmc_hostname(host), err);
 		return err;
 	}
 
-	/*
-	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
-	 * be set to a value not greater than 52MHz after the HS_TIMING
-	 * field is set to 0x1.
-	 */
-	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
-	mmc_set_bus_speed(card);
-
 	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			 EXT_CSD_BUS_WIDTH,
 			 EXT_CSD_DDR_BUS_WIDTH_8,
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 23aa3a3..f917527 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -23,15 +23,12 @@
 
 #define MMC_OPS_TIMEOUT_MS	(10 * 60 * 1000) /* 10 minute timeout */
 
-static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
-				    bool ignore_crc)
+int __mmc_send_status(struct mmc_card *card, u32 *status, bool ignore_crc,
+		      int retries)
 {
 	int err;
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!card);
-	BUG_ON(!card->host);
-
 	cmd.opcode = MMC_SEND_STATUS;
 	if (!mmc_host_is_spi(card->host))
 		cmd.arg = card->rca << 16;
@@ -39,7 +36,7 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
 	if (ignore_crc)
 		cmd.flags &= ~MMC_RSP_CRC;
 
-	err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
+	err = mmc_wait_for_cmd(card->host, &cmd, retries);
 	if (err)
 		return err;
 
@@ -54,7 +51,7 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
 
 int mmc_send_status(struct mmc_card *card, u32 *status)
 {
-	return __mmc_send_status(card, status, false);
+	return __mmc_send_status(card, status, false, MMC_CMD_RETRIES);
 }
 
 static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
@@ -419,6 +416,21 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc)
 	return err;
 }
 
+int mmc_check_switch_status(struct mmc_host *host, u32 status)
+{
+	if (mmc_host_is_spi(host)) {
+		if (status & R1_SPI_ILLEGAL_COMMAND)
+			return -EBADMSG;
+	} else {
+		if (status & 0xFDFFA000)
+			pr_warn("%s: unexpected status %#x after switch\n",
+				mmc_hostname(host), status);
+		if (status & R1_SWITCH_ERROR)
+			return -EBADMSG;
+	}
+	return 0;
+}
+
 /**
  *	__mmc_switch - modify EXT_CSD register
  *	@card: the MMC card associated with the data transfer
@@ -497,7 +509,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 	timeout = jiffies + msecs_to_jiffies(timeout_ms);
 	do {
 		if (send_status) {
-			err = __mmc_send_status(card, &status, ignore_crc);
+			err = __mmc_send_status(card, &status, ignore_crc, 1);
 			if (err)
 				return err;
 		}
@@ -524,18 +536,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		}
 	} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
 
-	if (mmc_host_is_spi(host)) {
-		if (status & R1_SPI_ILLEGAL_COMMAND)
-			return -EBADMSG;
-	} else {
-		if (status & 0xFDFFA000)
-			pr_warn("%s: unexpected status %#x after switch\n",
-				mmc_hostname(host), status);
-		if (status & R1_SWITCH_ERROR)
-			return -EBADMSG;
-	}
-
-	return 0;
+	return mmc_check_switch_status(host, status);
 }
 EXPORT_SYMBOL_GPL(__mmc_switch);
 
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 6f4b00e..a59319c 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -20,7 +20,10 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr);
 int mmc_all_send_cid(struct mmc_host *host, u32 *cid);
 int mmc_set_relative_addr(struct mmc_card *card);
 int mmc_send_csd(struct mmc_card *card, u32 *csd);
+int __mmc_send_status(struct mmc_card *card, u32 *status, bool ignore_crc,
+		      int retries);
 int mmc_send_status(struct mmc_card *card, u32 *status);
+int mmc_check_switch_status(struct mmc_host *host, u32 status);
 int mmc_send_cid(struct mmc_host *host, u32 *cid);
 int mmc_spi_read_ocr(struct mmc_host *host, int highcap, u32 *ocrp);
 int mmc_spi_set_crc(struct mmc_host *host, int use_crc);



> 
> 
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@...el.com] 
> Sent: Tuesday, October 28, 2014 10:24 PM
> To: Chanho Min
> Cc: Chris Ball; Ulf Hansson; Seungwon Jeon; Jaehoon Chung; linux-
> mmc@...r.kernel.org; linux-kernel@...r.kernel.org; HyoJun Im; gunho.lee@....
> com; Hankyung Yu
> Subject: Re: [PATCH] mmc:core: fix hs400 timing selection
> 
> On 22/10/14 05:55, Chanho Min wrote:
>> According to JEDEC v5.01 spec (6.6.5), In order to switch to HS400 
>> mode, host should perform the following steps.
>>
>>  1. HS200 mode selection completed
>>  2. Set HS_TIMING to 0x01(High Speed)
>>  3. Host changes frequency to =< 52MHz  4. Set the bus width to DDR 
>> 8bit (CMD6)  5. Host may read Driver Strength (CMD8)  6. Set HS_TIMING 
>> to 0x03 (HS400)
>>
>> In current implementation, the order of 2 and 3 is reversed.
> 
> But HS200 mode supports running at speeds less than 52 MHz whereas High
> Speed mode does not support running at speeds greater than
> 52 MHz.
> 
> So the switch command might succeed, but the subsequent send_status command
> (see __mmc_switch) could be expected to fail unless the frequency is
> changed first.
> 
>> The HS_TIMING field should be set to 0x1 before the clock frequency is 
>> set to a value not greater than 52 MHz. Otherwise, Initialization of 
>> timing can be failed. Also, the host contoller's UHS timing mode 
>> should be set to DDR50 after the bus width is set to DDR 8bit.
>>
>> Signed-off-by: Hankyung Yu <hankyung.yu@....com>
>> Signed-off-by: Chanho Min <chanho.min@....com>
>> ---
>>  drivers/mmc/core/mmc.c |   13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
>> a301a78..52f78e0 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1061,9 +1061,6 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  	 * Before switching to dual data rate operation for HS400,
>>  	 * it is required to convert from HS200 mode to HS mode.
>>  	 */
>> -	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> -	mmc_set_bus_speed(card);
>> -
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
>>  			   card->ext_csd.generic_cmd6_time, @@ -1074,6
> +1071,14 @@ static 
>> int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	/*
>> +	 * According to JEDEC v5.01 spec (6.6.5), Clock frequency should
>> +	 * be set to a value not greater than 52MHz after the HS_TIMING
>> +	 * field is set to 0x1.
>> +	 */
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
>> +	mmc_set_bus_speed(card);
>> +
>>  	err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			 EXT_CSD_BUS_WIDTH,
>>  			 EXT_CSD_DDR_BUS_WIDTH_8,
>> @@ -1084,6 +1089,8 @@ static int mmc_select_hs400(struct mmc_card *card)
>>  		return err;
>>  	}
>>  
>> +	mmc_set_timing(card->host, MMC_TIMING_MMC_DDR52);
>> +
>>  	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  			   EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS400,
>>  			   card->ext_csd.generic_cmd6_time,
>>
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists