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