From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaehoon Chung Subject: Re: [PATCH] mmc: core: Clean up after mmc_pre_req if card was removed Date: Mon, 05 Mar 2012 14:08:19 +0900 Message-ID: <4F544A43.8020601@samsung.com> References: <1330616671-11952-1-git-send-email-ulf.hansson@stericsson.com> <4F508493.2000107@samsung.com> <4F508A24.50707@stericsson.com> <4F50E766.9030802@stericsson.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mailout1.samsung.com ([203.254.224.24]:29696 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750845Ab2CEFI2 (ORCPT ); Mon, 5 Mar 2012 00:08:28 -0500 Received: from epcpsbgm2.samsung.com (mailout1.samsung.com [203.254.224.24]) by mailout1.samsung.com (Oracle Communications Messaging Exchange Server 7u4-19.01 64bit (built Sep 7 2010)) with ESMTP id <0M0E000HXBLLCAG0@mailout1.samsung.com> for linux-mmc@vger.kernel.org; Mon, 05 Mar 2012 14:08:26 +0900 (KST) Received: from [165.213.219.108] by mmp2.samsung.com (Oracle Communications Messaging Exchange Server 7u4-19.01 64bit (built Sep 7 2010)) with ESMTPA id <0M0E003K3BM1IXX0@mmp2.samsung.com> for linux-mmc@vger.kernel.org; Mon, 05 Mar 2012 14:08:26 +0900 (KST) In-reply-to: <4F50E766.9030802@stericsson.com> Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: =?ISO-8859-1?Q?Per_F=F6rlin?= Cc: Ulf HANSSON , Jaehoon Chung , "linux-mmc@vger.kernel.org" , Chris Ball , Johan RUDHOLM , Lee Jones On 03/03/2012 12:29 AM, Per F=F6rlin wrote: > On 03/02/2012 09:51 AM, Ulf HANSSON wrote: >> Hi Jaehoon, >> >> I did not know this. Which host driver are you using? I would very m= uch=20 >> appreciate of you could debug and share some result. >> >> Thanks! >> >> BR >> Ulf Hansson >> >> On 03/02/2012 09:28 AM, Jaehoon Chung wrote: >>> Hi Ulf. >>> >>> I tested with this patch. >>> But in my environment, this patch didn't work fine before. >>> 1) When remove/insert, didn't entered the suspend. >>> 2) When removed during something write, >>> [ 50.755067] FAT-fs (mmcblk1p1): Directory bread(block 8254) fail= ed >>> [ 50.761235] FAT-fs (mmcblk1p1): Directory bread(block 8255) fail= ed >>> then at next-time, didn't detect sd-card. >>> >>> Did you know this? >>> If you want more information, i will debug, and share the result. >>> >>> Best Regards, >>> Jaehoon Chung >>> >>> On 03/02/2012 12:44 AM, Ulf Hansson wrote: >>> >>>> Make sure mmc_start_req cancel the prepared job, if the request >>>> was prevented to be started due to the card has been removed. >>>> >>>> This bug was introduced in commit: >>>> mmc: allow upper layers to know immediately if card has been remov= ed >>>> >>>> Signed-off-by: Ulf Hansson >>>> --- >>>> drivers/mmc/core/core.c | 35 +++++++++++++++------------------= -- >>>> 1 files changed, 15 insertions(+), 20 deletions(-) >>>> >>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >>>> index 0b317f0..9e562ab 100644 >>>> --- a/drivers/mmc/core/core.c >>>> +++ b/drivers/mmc/core/core.c >>>> @@ -249,16 +249,17 @@ static void mmc_wait_done(struct mmc_request= *mrq) >>>> complete(&mrq->completion); >>>> } >>>> >>>> -static void __mmc_start_req(struct mmc_host *host, struct mmc_req= uest *mrq) >>>> +static int __mmc_start_req(struct mmc_host *host, struct mmc_requ= est *mrq) >>>> { >>>> init_completion(&mrq->completion); >>>> mrq->done =3D mmc_wait_done; >>>> if (mmc_card_removed(host->card)) { >>>> mrq->cmd->error =3D -ENOMEDIUM; >>>> complete(&mrq->completion); >>>> - return; >>>> + return -ENOMEDIUM; >>>> } >>>> mmc_start_request(host, mrq); >>>> + return 0; >>>> } >>>> >>>> static void mmc_wait_for_req_done(struct mmc_host *host, >>>> @@ -342,6 +343,7 @@ struct mmc_async_req *mmc_start_req(struct mmc= _host *host, >>>> struct mmc_async_req *areq, int *error) >>>> { >>>> int err =3D 0; >>>> + int start_err =3D 0; >>>> struct mmc_async_req *data =3D host->areq; >>>> >>>> /* Prepare a new request */ >>>> @@ -351,30 +353,23 @@ struct mmc_async_req *mmc_start_req(struct m= mc_host *host, >>>> if (host->areq) { >>>> mmc_wait_for_req_done(host, host->areq->mrq); >>>> err =3D host->areq->err_check(host->card, host->areq); >>>> - if (err) { >>>> - /* post process the completed failed request */ >>>> - mmc_post_req(host, host->areq->mrq, 0); >>>> - if (areq) >>>> - /* >>>> - * Cancel the new prepared request, because >>>> - * it can't run until the failed >>>> - * request has been properly handled. >>>> - */ >>>> - mmc_post_req(host, areq->mrq, -EINVAL); >>>> - >>>> - host->areq =3D NULL; >>>> - goto out; >>>> - } >>>> } >>>> >>>> - if (areq) >>>> - __mmc_start_req(host, areq->mrq); >>>> + if (!err&& areq) >>>> + start_err =3D __mmc_start_req(host, areq->mrq); >>>> >>>> if (host->areq) >>>> mmc_post_req(host, host->areq->mrq, 0); >>>> >>>> - host->areq =3D areq; >>>> - out: >>>> + if (err || start_err) { >>>> + if (areq) >>>> + /* The prepared request was not started, cancel it. */ >>>> + mmc_post_req(host, areq->mrq, -EINVAL); >>>> + host->areq =3D NULL; > There seems to be an issue when setting host->areq=3DNULL when __mmc_= start_req fails. host->areq =3D=3D NULL indicates there are no ongoing = transfers. > host->areq is used in block.c to check if there are pending requests.= =20 >=20 > This seem to work: > ... > if (err || start_err) { > if (areq) > /* The prepared request was not started, cancel it. */ > mmc_post_req(host, areq->mrq, -EINVAL); > } >=20 > if (err) > host->areq =3D NULL; > else > host->areq =3D areq; > ... >=20 > This issue will be addressed in version 2. How to resolve it is not d= ecided yet.=20 It seems to work fine. But i didn't test yet. Best Regards, Jaehoon Chung >=20 > Feel free to comment, > Per > -- > 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 >=20