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

Reply via email to