diff mbox series

mmc: block: Use .card_busy() to detect busy state in card_busy_detect

Message ID 1623057495-63363-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive)
State New, archived
Headers show
Series mmc: block: Use .card_busy() to detect busy state in card_busy_detect | expand

Commit Message

Shawn Lin June 7, 2021, 9:18 a.m. UTC 
No need to send CMD13 if host driver supports .card_busy().

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/block.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

kernel test robot June 7, 2021, 1:28 p.m. UTC | #1 
Hi Shawn,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.13-rc5 next-20210607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Shawn-Lin/mmc-block-Use-card_busy-to-detect-busy-state-in-card_busy_detect/20210607-172028
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 614124bea77e452aa6df7a8714e8bc820b489922
config: alpha-randconfig-r034-20210607 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/2224db607cd2136af3a045ca8b2b8442705f8752
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Shawn-Lin/mmc-block-Use-card_busy-to-detect-busy-state-in-card_busy_detect/20210607-172028
        git checkout 2224db607cd2136af3a045ca8b2b8442705f8752
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mmc/core/block.c: In function 'card_busy_detect':
>> drivers/mmc/core/block.c:451:1: error: label at end of compound statement
     451 | cb:
         | ^~


vim +451 drivers/mmc/core/block.c

   412	
   413	static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
   414				    u32 *resp_errs)
   415	{
   416		unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
   417		int err = 0;
   418		u32 status;
   419		bool busy;
   420	
   421		do {
   422			bool done = time_after(jiffies, timeout);
   423	
   424			if (card->host->ops->card_busy) {
   425				busy = card->host->ops->card_busy(card->host);
   426				status = busy ?	0 : R1_READY_FOR_DATA | R1_STATE_TRAN << 9;
   427				goto cb;
   428			}
   429	
   430			err = __mmc_send_status(card, &status, 5);
   431			if (err) {
   432				dev_err(mmc_dev(card->host),
   433					"error %d requesting status\n", err);
   434				return err;
   435			}
   436	
   437			/* Accumulate any response error bits seen */
   438			if (resp_errs)
   439				*resp_errs |= status;
   440	
   441			/*
   442			 * Timeout if the device never becomes ready for data and never
   443			 * leaves the program state.
   444			 */
   445			if (done) {
   446				dev_err(mmc_dev(card->host),
   447					"Card stuck in wrong state! %s status: %#x\n",
   448					 __func__, status);
   449				return -ETIMEDOUT;
   450			}
 > 451	cb:
   452		} while (!mmc_ready_for_data(status));
   453	
   454		return err;
   455	}
   456	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot June 7, 2021, 2:03 p.m. UTC | #2 
Hi Shawn,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.13-rc5 next-20210607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Shawn-Lin/mmc-block-Use-card_busy-to-detect-busy-state-in-card_busy_detect/20210607-172028
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 614124bea77e452aa6df7a8714e8bc820b489922
config: x86_64-randconfig-a002-20210607 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project ae973380c5f6be77ce395022be40350942260be9)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/2224db607cd2136af3a045ca8b2b8442705f8752
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Shawn-Lin/mmc-block-Use-card_busy-to-detect-busy-state-in-card_busy_detect/20210607-172028
        git checkout 2224db607cd2136af3a045ca8b2b8442705f8752
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/mmc/core/block.c:452:2: error: expected statement
           } while (!mmc_ready_for_data(status));
           ^
   1 error generated.


vim +452 drivers/mmc/core/block.c

a5f5774c55a2e3 drivers/mmc/card/block.c Jon Hunter    2015-09-22  412  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  413  static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  414  			    u32 *resp_errs)
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  415  {
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  416  	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  417  	int err = 0;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  418  	u32 status;
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  419  	bool busy;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  420  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  421  	do {
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  422  		bool done = time_after(jiffies, timeout);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  423  
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  424  		if (card->host->ops->card_busy) {
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  425  			busy = card->host->ops->card_busy(card->host);
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  426  			status = busy ?	0 : R1_READY_FOR_DATA | R1_STATE_TRAN << 9;
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  427  			goto cb;
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  428  		}
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  429  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  430  		err = __mmc_send_status(card, &status, 5);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  431  		if (err) {
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  432  			dev_err(mmc_dev(card->host),
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  433  				"error %d requesting status\n", err);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  434  			return err;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  435  		}
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  436  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  437  		/* Accumulate any response error bits seen */
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  438  		if (resp_errs)
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  439  			*resp_errs |= status;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  440  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  441  		/*
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  442  		 * Timeout if the device never becomes ready for data and never
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  443  		 * leaves the program state.
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  444  		 */
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  445  		if (done) {
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  446  			dev_err(mmc_dev(card->host),
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  447  				"Card stuck in wrong state! %s status: %#x\n",
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  448  				 __func__, status);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  449  			return -ETIMEDOUT;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  450  		}
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  451  cb:
40c96853fef1bd drivers/mmc/core/block.c Ulf Hansson   2020-02-04 @452  	} while (!mmc_ready_for_data(status));
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  453  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  454  	return err;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  455  }
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  456  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot June 7, 2021, 2:28 p.m. UTC | #3 
Hi Shawn,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.13-rc5 next-20210607]
[cannot apply to ulf.hansson-mmc/next mmc/mmc-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Shawn-Lin/mmc-block-Use-card_busy-to-detect-busy-state-in-card_busy_detect/20210607-172028
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 614124bea77e452aa6df7a8714e8bc820b489922
config: riscv-randconfig-r002-20210607 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project ae973380c5f6be77ce395022be40350942260be9)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/2224db607cd2136af3a045ca8b2b8442705f8752
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Shawn-Lin/mmc-block-Use-card_busy-to-detect-busy-state-in-card_busy_detect/20210607-172028
        git checkout 2224db607cd2136af3a045ca8b2b8442705f8752
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/mmc/core/block.c:452:2: error: expected statement
           } while (!mmc_ready_for_data(status));
           ^
   1 error generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for LOCKDEP
   Depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
   Selected by
   - PROVE_LOCKING && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
   - LOCK_STAT && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
   - DEBUG_LOCK_ALLOC && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT


vim +452 drivers/mmc/core/block.c

a5f5774c55a2e3 drivers/mmc/card/block.c Jon Hunter    2015-09-22  412  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  413  static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  414  			    u32 *resp_errs)
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  415  {
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  416  	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  417  	int err = 0;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  418  	u32 status;
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  419  	bool busy;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  420  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  421  	do {
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  422  		bool done = time_after(jiffies, timeout);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  423  
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  424  		if (card->host->ops->card_busy) {
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  425  			busy = card->host->ops->card_busy(card->host);
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  426  			status = busy ?	0 : R1_READY_FOR_DATA | R1_STATE_TRAN << 9;
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  427  			goto cb;
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  428  		}
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  429  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  430  		err = __mmc_send_status(card, &status, 5);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  431  		if (err) {
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  432  			dev_err(mmc_dev(card->host),
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  433  				"error %d requesting status\n", err);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  434  			return err;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  435  		}
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  436  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  437  		/* Accumulate any response error bits seen */
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  438  		if (resp_errs)
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  439  			*resp_errs |= status;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  440  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  441  		/*
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  442  		 * Timeout if the device never becomes ready for data and never
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  443  		 * leaves the program state.
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  444  		 */
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  445  		if (done) {
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  446  			dev_err(mmc_dev(card->host),
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  447  				"Card stuck in wrong state! %s status: %#x\n",
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  448  				 __func__, status);
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  449  			return -ETIMEDOUT;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  450  		}
2224db607cd213 drivers/mmc/core/block.c Shawn Lin     2021-06-07  451  cb:
40c96853fef1bd drivers/mmc/core/block.c Ulf Hansson   2020-02-04 @452  	} while (!mmc_ready_for_data(status));
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  453  
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  454  	return err;
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  455  }
a0d4c7eb71dd08 drivers/mmc/core/block.c Chaotian Jing 2019-09-05  456  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Christian Loehle June 8, 2021, 6:28 a.m. UTC | #4 
Hey Shawn,
You're assuming a card not signalling busy indicates TRAN state, and set the state manually,
but a card might not be pulling DAT lines in PROG state.
I will send a patch later that reworks card_busy_detect, as there are some issues
with some command sequences in practice. I'd ask to wait with the patch until then.

Regards,
Christian


From: Shawn Lin <shawn.lin@rock-chips.com>
Sent: Monday, June 7, 2021 11:18 AM
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-mmc@vger.kernel.org <linux-mmc@vger.kernel.org>; Shawn Lin <shawn.lin@rock-chips.com>
Subject: [PATCH] mmc: block: Use .card_busy() to detect busy state in card_busy_detect 
 
No need to send CMD13 if host driver supports .card_busy().

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/block.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 88f4c215..23623a9 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -417,10 +417,17 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
         unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
         int err = 0;
         u32 status;
+       bool busy;
 
         do {
                 bool done = time_after(jiffies, timeout);
 
+               if (card->host->ops->card_busy) {
+                       busy = card->host->ops->card_busy(card->host);
+                       status = busy ? 0 : R1_READY_FOR_DATA | R1_STATE_TRAN << 9;
+                       goto cb;
+               }
+
                 err = __mmc_send_status(card, &status, 5);
                 if (err) {
                         dev_err(mmc_dev(card->host),
@@ -442,6 +449,7 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
                                  __func__, status);
                         return -ETIMEDOUT;
                 }
+cb:
         } while (!mmc_ready_for_data(status));
 
         return err;
Shawn Lin June 8, 2021, 6:58 a.m. UTC | #5 
Hi Christian

On 2021/6/8 14:28, Christian Löhle wrote:
> Hey Shawn,
> You're assuming a card not signalling busy indicates TRAN state, and set the state manually,
> but a card might not be pulling DAT lines in PROG state.

Refer to JESD84-B51 for emmc spec, section 6.5.13 clearly says that. And
SD spec V4 also has a similar statement in section 4.3.4.

So I guess if that was the case you point out, most of all operations in
mmc_ops.c would suffer from this.


> I will send a patch later that reworks card_busy_detect, as there are some issues
> with some command sequences in practice. I'd ask to wait with the patch until then.
> 
> Regards,
> Christian
> 
> 
> From: Shawn Lin <shawn.lin@rock-chips.com>
> Sent: Monday, June 7, 2021 11:18 AM
> To: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-mmc@vger.kernel.org <linux-mmc@vger.kernel.org>; Shawn Lin <shawn.lin@rock-chips.com>
> Subject: [PATCH] mmc: block: Use .card_busy() to detect busy state in card_busy_detect
>   
> No need to send CMD13 if host driver supports .card_busy().
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>   drivers/mmc/core/block.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 88f4c215..23623a9 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -417,10 +417,17 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
>           unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
>           int err = 0;
>           u32 status;
> +       bool busy;
>   
>           do {
>                   bool done = time_after(jiffies, timeout);
>   
> +               if (card->host->ops->card_busy) {
> +                       busy = card->host->ops->card_busy(card->host);
> +                       status = busy ? 0 : R1_READY_FOR_DATA | R1_STATE_TRAN << 9;
> +                       goto cb;
> +               }
> +
>                   err = __mmc_send_status(card, &status, 5);
>                   if (err) {
>                           dev_err(mmc_dev(card->host),
> @@ -442,6 +449,7 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
>                                    __func__, status);
>                           return -ETIMEDOUT;
>                   }
> +cb:
>           } while (!mmc_ready_for_data(status));
>   
>           return err;
>
Christian Loehle June 8, 2021, 7:51 a.m. UTC | #6 
From: Shawn Lin <shawn.lin@rock-chips.com>
Sent: Tuesday, June 8, 2021 8:58 AM

>>You're assuming a card not signalling busy indicates TRAN state, and 
>>set the state manually, but a card might not be pulling DAT lines in PROG state
>>
>Refer to JESD84-B51 for emmc spec, section 6.5.13 clearly says that. And
>SD spec V4 also has a similar statement in section 4.3.4.
>
>So I guess if that was the case you point out, most of all operations in
>mmc_ops.c would suffer from this.

Im not sure what youre referring to as 6.5.13 is inside of 6.5 (no further subsections)
in JESD84-B51 and 4.3.4 in the SD spec is "4.3.4. Data Write"

The patch itself is fine, i just think that card_busy_detect should not actually
be a busy detect but rather a TRAN detect. I will send a patch now, but if anyone
wants to check this out, a full SDSC card that receives CMD16->CMD42 Lock -> CMD42 Force Erase
-> CMD16 (Reset to 512) -> Read like CMD17 will hit this race condition
pretty consistently as the CMD42 Force Erase will be in PROG for a while on
most cards. If the commands are sent through ioctl in a batch that is.
If card_busy_detect is changed to card_tran_detect then using .card_busy no
longer makes sense.

Regards,
Christian

Hyperstone GmbH | Line-Eid-Strasse 3 | 78467 Konstanz
Managing Directors: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
diff mbox series

Patch

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 88f4c215..23623a9 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -417,10 +417,17 @@  static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
 	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
 	int err = 0;
 	u32 status;
+	bool busy;
 
 	do {
 		bool done = time_after(jiffies, timeout);
 
+		if (card->host->ops->card_busy) {
+			busy = card->host->ops->card_busy(card->host);
+			status = busy ?	0 : R1_READY_FOR_DATA | R1_STATE_TRAN << 9;
+			goto cb;
+		}
+
 		err = __mmc_send_status(card, &status, 5);
 		if (err) {
 			dev_err(mmc_dev(card->host),
@@ -442,6 +449,7 @@  static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
 				 __func__, status);
 			return -ETIMEDOUT;
 		}
+cb:
 	} while (!mmc_ready_for_data(status));
 
 	return err;