On 03/05/2012 02:08 PM, Jaehoon Chung wrote:
> On 03/03/2012 12:29 AM, Per Förlin 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 much
>>> 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) failed
>>>> [ 50.761235] FAT-fs (mmcblk1p1): Directory bread(block 8255) failed
>>>> 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 removed
>>>>>
>>>>> Signed-off-by: Ulf Hansson<ulf.hansson@xxxxxxxxxxxxxx>
>>>>> ---
>>>>> 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_request *mrq)
>>>>> +static int __mmc_start_req(struct mmc_host *host, struct mmc_request *mrq)
>>>>> {
>>>>> init_completion(&mrq->completion);
>>>>> mrq->done = mmc_wait_done;
>>>>> if (mmc_card_removed(host->card)) {
>>>>> mrq->cmd->error = -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 = 0;
>>>>> + int start_err = 0;
>>>>> struct mmc_async_req *data = host->areq;
>>>>>
>>>>> /* Prepare a new request */
>>>>> @@ -351,30 +353,23 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
>>>>> if (host->areq) {
>>>>> mmc_wait_for_req_done(host, host->areq->mrq);
>>>>> err = 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 = NULL;
>>>>> - goto out;
>>>>> - }
>>>>> }
>>>>>
>>>>> - if (areq)
>>>>> - __mmc_start_req(host, areq->mrq);
>>>>> + if (!err&& areq)
>>>>> + start_err = __mmc_start_req(host, areq->mrq);
>>>>>
>>>>> if (host->areq)
>>>>> mmc_post_req(host, host->areq->mrq, 0);
>>>>>
>>>>> - host->areq = 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 = NULL;
>> There seems to be an issue when setting host->areq=NULL when __mmc_start_req fails. host->areq == NULL indicates there are no ongoing transfers.
>> host->areq is used in block.c to check if there are pending requests.
>>
>> 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);
>> }
>>
>> if (err)
>> host->areq = NULL;
>> else
>> host->areq = areq;
>> ...
>>
>> This issue will be addressed in version 2. How to resolve it is not decided yet.
If start_err is set and err didn't set, maybe should be set "host->areq = areq".
Then in block.c, should be check the "status".
But start_err didn't be assigned anywhere, how about this?
...
if (error) {
*error = err
if (start_err)
*error = start_err;
}
...
Best Regards,
Jaehoon Chung
>
> It seems to work fine. But i didn't test yet.
>
> Best Regards,
> Jaehoon Chung
>
>>
>> Feel free to comment,
>> Per
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
