RE: [PATCH 4/5] mmc: cmdq: support for command queue enabled host

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

 



Hi Asutosh,

        Thanks for your comments. Could you check my comments please?

-----Original Message-----
From: Asutosh Das [mailto:asutoshd@xxxxxxxxxxxxxx]
Sent: 2014年12月9日 22:46
To: Ziji Hu
Cc: linux-arm-msm@xxxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx
Subject: Re: [PATCH 4/5] mmc: cmdq: support for command queue enabled host

Hi Ziji,
Thanks for your comments.

On 12/10/2014 4:37 AM, Ziji Hu wrote:
> Hi Asutosh,
>
>           Could you check me comments please?
>
>      Please correct me if I misunderstand you.
>
>      Thank you.
>
> +irqreturn_t cmdq_irq(struct mmc_host *mmc, u32 intmask)
>
> +{
>
> +       u32 status;
>
> +       unsigned long tag = 0, comp_status;
>
> +       struct cmdq_host *cq_host = (struct cmdq_host
> *)mmc_cmdq_private(mmc);
>
> +
>
> +       spin_lock(&cq_host->cmdq_lock);
>
> +
>
> +       status = cmdq_readl(cq_host, CQIS);
>
> +       cmdq_writel(cq_host, status, CQIS);
>
> +
>
> +       if (status & CQIS_HAC) {
>
> +               /* halt is completed, wakeup waiting thread */
>
> +               complete(&cq_host->halt_comp);
>
> +       } else if (status & CQIS_TCC) {
>
> +               /* read QCTCN and complete the request */
>
> +               comp_status = cmdq_readl(cq_host, CQTCN);
>
> +               if (!comp_status) {
>
> +                       pr_err("%s: bogus comp-stat\n", __func__);
>
> +                       cmdq_dumpregs(cq_host);
>
> +                       WARN_ON(1);
>
> +               }
>
> +               for_each_set_bit(tag, &comp_status,
> + cq_host->num_slots) {
>
> +                       /* complete the corresponding mrq */
>
> +                       cmdq_finish_data(mmc, tag);
>
> According to eMMC 5.1 spec: CQE shall set bit n of this register (at
> the same time it clears bit n of CQTDBR) when a task execution is completed
> (with success or error).
>
>       Assume an error and an completion both occur at the same time,
> then two bits of CQTCN register will be set. One bit presents the completion. The
> other indicates the error slot.
>
>      Based on your current implementation, host will handle the error
> with cmdq_finish_data. Later, mrq->data->error and mrq->cmd->error are used to
>
Agree. I'm planning to change it to:
1. read CQIS
2. first check for errors on all the completed tasks, mark success/failure appropriately to the respective mrq 3. invoke cmdq_finish_data on all the completed tasks.

Thoughts ?

[Ziji]: The above change seems to be a reasonable one. And I think the error interrupt registers in legacy eMMC should be also checked, according to spec.

> check the error status. However, there is no cmdq source code to set
> those two
I'm working on the error handling part now. Will post the patch when done.
>
> error flag. They are supposed to be setup in legacy eMMC irq handling,
> which is replace by your cmdq irq handling. Thus actually host will receive the
> error request with no error flag. As a result, host will treat the error request as
> a successful completion.
>
>      Thus there will no error handling. Or the error handling will be
> executed after the error request is finished as a successful one.
>
> +                       /* complete DCMD on tag 31 */
>
> +               }
>
> +               cmdq_writel(cq_host, comp_status, CQTCN);
>
> +       } else if (status & CQIS_RED) {
>
> +               /* task response has an error */
>
> +               pr_err("%s: RED error %d !!!\n", mmc_hostname(mmc),
> + status);
>
> +               cmdq_dumpregs(cq_host);
>
> +               BUG_ON(1);
>
>           Thank you.
>
> Best regards,
>
> Hu Ziji
>


--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
?韬{.n?????%??檩??w?{.n???{饼??&??骅w*jg????????G??⒏⒎?:+v????????????"??????




[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