Re: [PATCH v6 11/11] mmc: add handling for two parallel block requests in issue_rw_rq

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21 June 2011 08:40, Per Forlin <per.forlin@xxxxxxxxxx> wrote:
> On 20 June 2011 17:17, Kishore Kadiyala <kishorek.kadiyala@xxxxxxxxx> wrote:
>> On Mon, Jun 20, 2011 at 2:47 AM, Per Forlin <per.forlin@xxxxxxxxxx> wrote:
>>> Change mmc_blk_issue_rw_rq() to become asynchronous.
>>> The execution flow looks like this:
>>> The mmc-queue calls issue_rw_rq(), which sends the request
>>> to the host and returns back to the mmc-queue. The mmc-queue calls
>>> issue_rw_rq() again with a new request. This new request is prepared,
>>> in isuue_rw_rq(), then it waits for the active request to complete before
>>> pushing it to the host. When to mmc-queue is empty it will call
>>> isuue_rw_rq() with req=NULL to finish off the active request
>>> without starting a new request.
>>>
>>> Signed-off-by: Per Forlin <per.forlin@xxxxxxxxxx>
>>> ---
>>>  drivers/mmc/card/block.c |  121 +++++++++++++++++++++++++++++++++-------------
>>>  drivers/mmc/card/queue.c |   17 +++++--
>>>  drivers/mmc/card/queue.h |    1 +
>>>  3 files changed, 101 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>> index 6a84a75..66db77a 100644
>>> --- a/drivers/mmc/card/block.c
>>> +++ b/drivers/mmc/card/block.c
>>> @@ -108,6 +108,7 @@ static DEFINE_MUTEX(open_lock);
>>>
>>>  enum mmc_blk_status {
>>>        MMC_BLK_SUCCESS = 0,
>>> +       MMC_BLK_PARTIAL,
>>>        MMC_BLK_RETRY,
>>>        MMC_BLK_DATA_ERR,
>>>        MMC_BLK_CMD_ERR,
>>> @@ -668,14 +669,16 @@ static inline void mmc_apply_rel_rw(struct mmc_blk_request *brq,
>>>        }
>>>  }
>>>
>>> -static enum mmc_blk_status mmc_blk_err_check(struct mmc_blk_request *brq,
>>> -                                            struct request *req,
>>> -                                            struct mmc_card *card,
>>> -                                            struct mmc_blk_data *md)
>>> +static int mmc_blk_err_check(struct mmc_card *card,
>>> +                            struct mmc_async_req *areq)
>>>  {
>>>        struct mmc_command cmd;
>>>        u32 status = 0;
>>>        enum mmc_blk_status ret = MMC_BLK_SUCCESS;
>>> +       struct mmc_queue_req *mq_mrq = container_of(areq, struct mmc_queue_req,
>>> +                                                   mmc_active);
>>> +       struct mmc_blk_request *brq = &mq_mrq->brq;
>>> +       struct request *req = mq_mrq->req;
>>>
>>>        /*
>>>         * Check for errors here, but don't jump to cmd_err
>>> @@ -770,7 +773,11 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_blk_request *brq,
>>>                else
>>>                        ret = MMC_BLK_DATA_ERR;
>>>        }
>>> -out:
>>> +
>>> +       if (ret == MMC_BLK_SUCCESS &&
>>> +           blk_rq_bytes(req) != brq->data.bytes_xfered)
>>> +               ret = MMC_BLK_PARTIAL;
>>> + out:
>>>        return ret;
>>>  }
>>>
>>> @@ -901,27 +908,59 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>>>                brq->data.sg_len = i;
>>>        }
>>>
>>> +       mqrq->mmc_active.mrq = &brq->mrq;
>>> +       mqrq->mmc_active.err_check = mmc_blk_err_check;
>>> +
>>>        mmc_queue_bounce_pre(mqrq);
>>>  }
>>>
>>> -static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
>>> +static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>>>  {
>>>        struct mmc_blk_data *md = mq->data;
>>>        struct mmc_card *card = md->queue.card;
>>> -       struct mmc_blk_request *brq = &mq->mqrq_cur->brq;
>>> -       int ret = 1, disable_multi = 0;
>>> +       struct mmc_blk_request *brq;
>>> +       int ret = 1;
>>> +       int disable_multi = 0;
>>>        enum mmc_blk_status status;
>>> +       struct mmc_queue_req *mq_rq;
>>> +       struct request *req;
>>> +       struct mmc_async_req *areq;
>>> +
>>> +       if (!rqc && !mq->mqrq_prev->req)
>>> +               goto out;
>>>
>>>        do {
>>> -               mmc_blk_rw_rq_prep(mq->mqrq_cur, card, disable_multi, mq);
>>> -               mmc_wait_for_req(card->host, &brq->mrq);
>>> +               if (rqc) {
>>> +                       mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>>> +                       areq = &mq->mqrq_cur->mmc_active;
>>> +               } else
>>> +                       areq = NULL;
>>> +               areq = mmc_start_req(card->host, areq, (int *) &status);
>>
>> I think 'status' is used uninitialized.
> status is an out parameter. From that perspective it is always initialised.
> I should update the doc description of mmc_start_req to clarify this.
>
>> With this struct mmc_async_req *mmc_start_req in your first patch
>> if (error)
>>        *error = err;
>> return data;
>> condition which always passes.
>>
>> You can have
>> enum mmc_blk_status status = MMC_BLK_SUCCESS;
In core.c there is no access to the type "enum mmc_blk_status status",
this type is block.c specific.
The intention is to make this function available for SDIO as well.
"int err = 0;" is set at the top of mmc_start_req(). Default err condition is 0.

What do you think about the following changes?

  *     @areq: async request to start
- *     @error: non zero in case of error
+ *     @error: out parameter returns 0 for success, otherwise non zero
  *
  *     Start a new MMC custom command request for a host.
  *     If there is on ongoing async request wait for completion
@@ -334,9 +334,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
                                mmc_post_req(host, areq->mrq, -EINVAL);

                        host->areq = NULL;
-                       if (error)
-                               *error = err;
-                       return data;
+                       goto out;
                }
        }

@@ -347,6 +345,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
                mmc_post_req(host, host->areq->mrq, 0);

        host->areq = areq;
+ out:
        if (error)
                *error = err;
        return data;

>>
>> struct mmc_async_req *mmc_start_req  {
>> err = host->areq->err_check(host->card, host->areq);
>>                if (err) {
>>                             ...
>>                             ...
>>                             *error = err;
>>                }
>>
>> no need to update * error here in success case
>> return data
>> }
> I agree, makes the code more clear.
>
> Thanks,
> 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


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux