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

2011-06-21 Thread Per Forlin
On 20 June 2011 17:17, Kishore Kadiyala kishorek.kadiy...@gmail.com wrote:
 On Mon, Jun 20, 2011 at 2:47 AM, Per Forlin per.for...@linaro.org 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.for...@linaro.org
 ---
  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;

 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-06-21 Thread Per Forlin
On 21 June 2011 08:40, Per Forlin per.for...@linaro.org wrote:
 On 20 June 2011 17:17, Kishore Kadiyala kishorek.kadiy...@gmail.com wrote:
 On Mon, Jun 20, 2011 at 2:47 AM, Per Forlin per.for...@linaro.org 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.for...@linaro.org
 ---
  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 

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

2011-06-21 Thread Per Forlin
On 19 June 2011 23:17, Per Forlin per.for...@linaro.org 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.for...@linaro.org
 ---
  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);
 +               if (!areq)
 +                       goto out;

 -               mmc_queue_bounce_post(mq-mqrq_cur);
 -               status = mmc_blk_err_check(brq, req, card, md);
 +               mq_rq = container_of(areq, struct mmc_queue_req, mmc_active);
 +               brq = mq_rq-brq;
 +               req = mq_rq-req;
 +               mmc_queue_bounce_post(mq_rq);

                switch (status) {
 -               case MMC_BLK_CMD_ERR:
 -                       goto cmd_err;
 +               case MMC_BLK_SUCCESS:
 +               case MMC_BLK_PARTIAL:
 +                       /*
 +                        * A block was successfully transferred.
 +                        */
 +                       spin_lock_irq(md-lock);
 +                       ret = __blk_end_request(req, 0,
 +                                               brq-data.bytes_xfered);
 +                       spin_unlock_irq(md-lock);
 +                       if (status == MMC_BLK_SUCCESS  ret) {
 +                               /* If this happen it is a bug */
 +                               printk(KERN_ERR %s BUG rq_tot %d d_xfer 
 %d\n,
 +           

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

2011-06-21 Thread Kishore Kadiyala
Hi Per,

snip

 --- 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,

snip

 -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;
Can initialize here
enum mmc_blk_status = MMC_BLK_SUCCESS

 +       struct mmc_queue_req *mq_rq;
 +       struct request *req;
 +       struct mmc_async_req *areq;
 +
 +       if (!rqc  !mq-mqrq_prev-req)
 +               goto out;

snip

I meant doing initialization in block.c itself and not core.c as above.

 The intention is to make this function available for SDIO as well.

Totally agree, having the API access to SDIO

 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;


The above change reduced the code size but still since 'error' is
pointer to the 'status' which is uninitialized,
so if(error) will be always true.
If you want to update *error in success/failure case [based on 'err'
]then can remove the if(error) check
else to update the error case only can look at the way proposed in my
previous mail.

Regards,
Kishore
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-06-21 Thread Per Forlin
On 20 June 2011 17:17, Kishore Kadiyala kishorek.kadiy...@gmail.com wrote:
 On Mon, Jun 20, 2011 at 2:47 AM, Per Forlin per.for...@linaro.org 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.for...@linaro.org
 ---
  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.
 With this struct mmc_async_req *mmc_start_req in your first patch
 if (error)
        *error = err;
 return data;
 condition which always passes.
It's valid to pass in NULL as status.
Do you suggest to make the status pointer mandatory?


 You can have
 enum mmc_blk_status status = MMC_BLK_SUCCESS;

status will be set to MMC_BLK_SUCCESS for the first iteration of the
do-while loop.
It may look like:
MMC_BLK_RETRY
MMC_BLK_PARTIAL
MMC_BLK_PARTIAL
MMC_BLK_PARTIAL
MMC_BLK_SUCCESS

This means mmc_start_req needs to return MMC_BLK_SUCCESS.

Regards,
Per
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-06-20 Thread Kishore Kadiyala
On Mon, Jun 20, 2011 at 2:47 AM, Per Forlin per.for...@linaro.org 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.for...@linaro.org
 ---
  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.
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;

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
}

Regards,
Kishore
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html