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.
> + 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.
> + }
> + 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.
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html