Re: [PATCH 3/5] mmc: card: Add eMMC command queuing support in mmc block layer

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

 



Hi Ziji
On 12/9/2014 12:33 AM, Ziji Hu wrote:
Hi Asutosh,

         May I ask whether you have tested the code on hardware platform?
I have done some basic testing on an emulation platform that using an earlier kernel version.
         If you have not tested it yet, do you have a schedule for testing?

-----Original Message-----
From: Asutosh Das [mailto:das.asutosh@xxxxxxxxx]
Sent: 2014年12月7日 21:59
To: Ziji Hu
Cc: linux-arm-msm@xxxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx
Subject: Re: [PATCH 3/5] mmc: card: Add eMMC command queuing support in mmc block layer

Hi Ziji
Thanks for your comments.

I'm adding linux-mmc to this as well.

On 8 December 2014 at 07:47, Hu Ziji <huziji@xxxxxxxxxxx> wrote:

Asutosh Das <asutoshd <at> codeaurora.org> writes:

+static inline bool mmc_cmdq_should_pull_reqs(struct mmc_host *host,
+                                     struct mmc_cmdq_context_info
+*ctx) {
+     spin_lock_bh(&ctx->cmdq_ctx_lock);
+     if (ctx->active_dcmd || ctx->rpmb_in_wait) {
+             if ((ctx->curr_state != CMDQ_STATE_HALT) ||
+                     (ctx->curr_state != CMDQ_STATE_ERR)) {

                         Because CMDQ_STATE_HALT == 0 and
CMDQ_STATE_ERR == 1, it is impossible that condition ((ctx->curr_state
== CMDQ_STATE_HALT) && (ctx-
curr_state != CMDQ_STATE_ERR)) is satisfied. Thus the above
if-condition will
always be true.


Agree.


+                     pr_debug("%s: %s: skip pulling reqs: dcmd: %d rpmb: %d state:
%d\n",
+                              mmc_hostname(host), __func__, ctx->active_dcmd,
+                              ctx->rpmb_in_wait, ctx->curr_state);
+                     spin_unlock_bh(&ctx->cmdq_ctx_lock);
+                     return false;
+             }
+     } else {
+             spin_unlock_bh(&ctx->cmdq_ctx_lock);
+             return true;
+     }
+}

+/**
+ *   mmc_cmdq_halt - halt/un-halt the command queue engine
+ *    <at> host: host instance
+ *    <at> halt: true - halt, un-halt otherwise
+ *
+ *   Host halts the command queue engine. It should complete
+ *   the ongoing transfer and release the SD bus.
+ *   All legacy SD commands can be sent upon successful
+ *   completion of this function.
+ *   Returns 0 on success, negative otherwise
+ */
+int mmc_cmdq_halt(struct mmc_host *host, bool halt) {
+     int err = 0;
+
+     if ((halt && (host->cmdq_ctx.curr_state & CMDQ_STATE_HALT)) ||
+         (!halt && !(host->cmdq_ctx.curr_state & CMDQ_STATE_HALT)))
+             return 1;
+
+     if (host->cmdq_ops->halt) {
+             err = host->cmdq_ops->halt(host, halt);
+             if (!err && halt)
+                     host->cmdq_ctx.curr_state |= CMDQ_STATE_HALT;
+             else if (!err && !halt)
+                     host->cmdq_ctx.curr_state &= ~CMDQ_STATE_HALT;

                 Since CMDQ_STATE_HALE == 0, ORed with CMDQ_STATE_HALT
and ANDed with ~CMDQ_STATE_HALT will neither modify the value of
curr_state. Thus CMDQ_STATE_HALT as 0 cannot indicate the halt state of cmd queue.

Agree.



+     }
+     return 0;
+}
+EXPORT_SYMBOL(mmc_cmdq_halt);

+enum cmdq_states {
+     CMDQ_STATE_HALT,
+     CMDQ_STATE_ERR,
+};

     According to above definition, CMDQ_STATE_HALT is 0x0 and
CMDQ_STATE_ERR is 0x1.


I will address these comments in the next patch.



Hi Asutosh,

     I think that the definition of CMDQ_STATE_HALT might fail to
indicate halt status correctly.
     Could you check my comments please?
     If I misunderstand your source code, please correct me.

Best regards,
Ziji

--
To unsubscribe from this list: send the line "unsubscribe
linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




--
Thanks & Regards
~/Asutosh Das
N嫥叉靣笡y氊b瞂千v豝?)藓{.n?+壏{睔g"炟^n噐■?侂h櫒璀?&Ⅷ瓽珴閔?(殠娸"濟?m??飦赇z罐枈帼f"穐殘坢ml==



--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux