linux-kernel - Re: [PATCH 2/2] mmc: dw_mmc: add resets support to dw

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

Date:	Tue, 29 Mar 2016 16:23:12 +0800
From:	zhangfei <zhangfei.gao@...aro.org>
To:	Shawn Lin <shawn.lin@...nel-upstream.org>,
	Guodong Xu <guodong.xu@...aro.org>,
	Shawn Lin <shawn.lin@...k-chips.com>
CC:	robh+dt@...nel.org,
	Paweł Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	ijc+devicetree@...lion.org.uk, Kumar Gala <galak@...eaurora.org>,
	jh80.chung@...sung.com, Ulf Hansson <ulf.hansson@...aro.org>,
	devicetree@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-mmc@...r.kernel.org,
	Xinwei Kong <kong.kongxinwei@...ilicon.com>
Subject: Re: [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mmc



On 03/29/2016 10:22 AM, Shawn Lin wrote:

>>
>>
>>
>>         +               else if (IS_ERR(host->pdata)) {
>>                                  dev_err(host->dev, "platform data not
>>         available\n");
>>                                  return -EINVAL;
>>                          }
>>         @@ -3012,6 +3022,9 @@ int dw_mci_probe(struct dw_mci *host)
>>                          }
>>                  }
>>
>>         +       if (host->pdata->rstc != NULL)
>>         +               reset_control_deassert(host->pdata->rstc);
>>         +
>>
>>
>>     sorry, I can't follow your intention here. Shouldn't it be something
>>     like "assert mmc -> may need delay -> deassert mmc". As your current
>>     code, nothing happend right?
Should be abstracted in reset driver.

>>
>>
>> The chip exits from bootloader with this bit asserted. And when entering
>> kernel, we only need to deassert.
>>
>> In my current code, the driver deassert mmc in _probe(), and assert mmc
>> in _remove().
>
> I catch your point. From the previous discussion, we add it to make sure
> dw_mmc in good state after leaving bootloader to kernel. But My real
> question is that you can assert it in  bootloader, so you can also
> dessert it in bootloaer to make sure dw_mmc work fine when probing
> in kernel. In that way, we don't need this patch?

uefi does not have exit point, and kernel may not assume mmc controller 
state is always correct when boot.
If Uefi need copy Image from mmc, mmc controller is in working state.
When jump to kernel, uefi mmc driver can not recover itself.
If kernel assume mmc controller state is clean, mmc will be in abnormal 
state.
Some controller will clear itself when set clock, however, hip660 does 
not, it need special register to access.


>
> More to think, Is it ok to match the behaviour of bootloader stage?
> My bootloader doesn't assert the reset pin of dw_mmc, so it seams if
> I want to fix you issue on kernel stage, I need a new round of
> assert->delay->deassert.

The process like delay (if required) should be abstracted in reset driver.
reset framework just export reset_control_assert/reset_control_deassert API.
Unfortunately not find clear description in Documentation/.
Suppose deassert is like start, while assert is like stop.

drivers/reset/core.c
reset_control_deassert - deasserts the reset line
reset_control_assert - asserts the reset line

More example:
drivers/mmc/host/sdhci-st.c
drivers/mmc/host/sunxi-mmc.c
drivers/usb/host/ohci-platform.c
drivers/i2c/busses/i2c-mv64xxx.c

Thanks