linux-kernel - Re: [PATCH v4 2/7] mmc: mediatek: Add Mediatek MMC driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Tue, 26 May 2015 14:33:41 +0200
From:	Ulf Hansson <ulf.hansson@...aro.org>
To:	Chaotian Jing <chaotian.jing@...iatek.com>
Cc:	Rob Herring <robh+dt@...nel.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	Chris Ball <chris@...ntf.net>,
	Mark Rutland <mark.rutland@....com>,
	James Liao <jamesjj.liao@...iatek.com>,
	srv_heupstream <srv_heupstream@...iatek.com>,
	Arnd Bergmann <arnd@...db.de>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Hongzhou Yang <hongzhou.yang@...iatek.com>,
	Catalin Marinas <catalin.marinas@....com>,
	linux-mmc <linux-mmc@...r.kernel.org>,
	Will Deacon <will.deacon@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	Sascha Hauer <kernel@...gutronix.de>,
	"Joe.C" <yingjoe.chen@...iatek.com>,
	Eddie Huang <eddie.huang@...iatek.com>,
	Bin Zhang (章斌) <bin.zhang@...iatek.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v4 2/7] mmc: mediatek: Add Mediatek MMC driver

[...]

>> >> > +{
>> >> > +       unsigned long tmo = jiffies + msecs_to_jiffies(20);
>> >> > +
>> >> > +       while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY)
>> >> > +                       && time_before(jiffies, tmo))
>> >> > +               continue;
>> >> > +
>> >> > +       if (readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) {
>> >> > +               dev_err(host->dev, "CMD bus busy detected\n");
>> >> > +               host->error |= REQ_CMD_BUSY;
>> >> > +               msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
>> >> > +               return false;
>> >> > +       }
>> >> > +
>> >> > +       if (mmc_resp_type(cmd) == MMC_RSP_R1B || cmd->data) {
>> >> > +               /* R1B or with data, should check SDCBUSY */
>> >> > +               while (readl(host->base + SDC_STS) & SDC_STS_SDCBUSY)
>> >> > +                       cpu_relax();
>> >> > +       }
>> >>
>> >> MSDC seems to be handling card busy detection in HW, right?
>> >>
>> > Do not have this ability, HW only know if CMD/DAT is low, but do not
>> > have any interrupt for it,
>>
>> I see, but doesn't the above polling mean that msdc will not propagate
>> the response until the card have stopped signal busy? That's what
>> MMC_CAP_WAIT_WHILE_BUSY shall be used for.
>>
> As you see, we only check the "busy state" BEFORE issue a R1B command or
> with data command, but do not check if AFTER the request was done, that
> would do not match the "MMC_CAP_WAIT_WHILE_BUSY"(eg. CMD5 to sleep)

Okay, fair enough. Still, I don't understand why you want to do that?

> In addition, about CMD5, I find that the suspend/resume flow of EMMC is
> stranger in new kernel version, when suspend, it may issue CMD5 to enter
> sleep mode, then power off MMC, but when resume, it will
> re-initialization, So that why need do the redundant CMD5 in suspend ?

Both CMD0 and CMD5 are valid as wakeup commands.

To be able to use CMD5, the VCCQ regulator must be kept enabled in the
sleep state. That's when it becomes a bit tricky, due to the range of
different host drivers and SoCs for how VCCQ is managed.

To be safe we have chosen to use CMD0, since it works for *all* cases
no matter if VCCQ get gated or not.

Moreover, using CMD5 as the wakeup command would require added
complexity in the code dealing with suspend/resume. I don't think the
effort is worth it, at least until someone has proven that the resume
time is greatly decreased by using CMD5.

>
>> Perhaps you should remove the above polling, and rely on the MMC core
>> to poll with CMD13 instead?
> before any read/write command, core will issue CMD13 to confirm card
> status, here is just only do double confirm to avoid HW issue.

What HW issue and why do you need to double confirm? It seems strange.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/