linux-kernel - Re: [PATCH 1/2] mmc: core: use card pointer as the first parameter of execute

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD=FV=VQg+sQUpgbKw7Zz=-f+jPM=6O-+3gUpohWhj8JeSPu6Q@mail.gmail.com>
Date:	Wed, 28 Jan 2015 16:55:28 -0800
From:	Doug Anderson <dianders@...omium.org>
To:	Ulf Hansson <ulf.hansson@...aro.org>
Cc:	Addy Ke <addy.ke@...k-chips.com>, Rob Herring <robh+dt@...nel.org>,
	Paweł Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Randy Dunlap <rdunlap@...radead.org>,
	"tgih.jun@...sung.com" <tgih.jun@...sung.com>,
	Jaehoon Chung <jh80.chung@...sung.com>,
	Chris Ball <chris@...ntf.net>,
	Dinh Nguyen <dinguyen@...era.com>,
	Heiko Stübner <heiko@...ech.de>,
	Olof Johansson <olof@...om.net>,
	Sonny Rao <sonnyrao@...omium.org>,
	Alexandru Stan <amstan@...omium.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-mmc <linux-mmc@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	"zhenfu.fang" <zhenfu.fang@...k-chips.com>,
	Eddie Cai <cf@...k-chips.com>, lintao <lintao@...k-chips.com>,
	chenfen <chenfen@...k-chips.com>, zyf <zyf@...k-chips.com>,
	Jianqun Xu <xjq@...k-chips.com>,
	Tao Huang <huangtao@...k-chips.com>,
	Chris Zhong <zyw@...k-chips.com>,
	姚智情 <yzq@...k-chips.com>,
	Han Jiang <hj@...k-chips.com>,
	Kever Yang <kever.yang@...k-chips.com>,
	zhangqing <zhangqing@...k-chips.com>,
	Lin Huang <hl@...k-chips.com>
Subject: Re: [PATCH 1/2] mmc: core: use card pointer as the first parameter of execute_tuning()

Ulf,

On Tue, Jan 27, 2015 at 7:18 AM, Ulf Hansson <ulf.hansson@...aro.org> wrote:
>> I asked Addy to post upstream against mmc_send_tuning(), but I guess
>> he didn't (he posted against Alex's NAKed patch instead).
>>
>> ...when I talked to him about it, Addy was asserting that when tuning
>> fails it is important (at least on dw_mmc on rk3288) that we wait for
>> the card to stop being busy and that the way to detect was using
>> mmc_send_status().
>
> So, could that be due to the internal logic of the error handling in
> dw_mmc driver? Or you think this is a generic issue?
>
> According to the specifications (eMMC and SD) both states that the
> tuning command has an R1 response. So, there shouldn't be any busy
> signalling involved - at least according to spec.

I did a bit of digging into this issue myself.  What I found was that
a "response CRC" and "end of transfer".  This was why I posted up
<https://patchwork.kernel.org/patch/5623071/>.  From that patch:

> Specifically it looks like in certain error conditions (I saw this
> with Response CRC errors) that data keeps showing up in the FIFO even
> after the error is reported and the CD (command done) bit is set.  If
> we don't wait for this data to finish transferring then it confuses
> the next transaction.  In the specific failure case I ran into I found
> that I could monitor the data_state_mc_busy bit and wait for it to
> clear, but in other failure cases this bit was stuck at busy when we
> saw an error.  Hence a generic big delay seems like the only option.

...Addy instead fixed the problem using mmc_send_status() to try to
detect when the transfer was all done and it apparently worked, but it
seemed odd to me.  My MMC "expertise" pretty much ends with looking
for simple logic errors in the MMC driver, so my hope was that one of
you guys would know this better...


>> That would mean that against upstream you'd need to change
>> mmc_send_tuning() to take in the card as well (or move the "host->card
>> = card" assignment to before UHS init, which seems less desirable?)
>>
>> What do you think about that?  Is there a better solution?
>
> Why do we need to change mmc_send_tuning()? I thought the issue was
> that mmc_send_status(), which currently takes "card" as a parameter.

Well, if mmc_send_tuning() needed to call mmc_send_status() then
mmc_send_tuning() would need the card parameter, right?


Doug
--
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/