Re: [PATCH 1/4] mmc: core: add support for hardware reset gpio line

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hello,

On 2015-01-29 11:56, Javier Martinez Canillas wrote:
> On Thu, Jan 29, 2015 at 10:15 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>>> Also, I wonder whether we could extend the mmc-pwrseq to cover your
>>> case? Did you consider that as an option?
>> I didn't consider mmc-pwrseq yet. For me it looked straightforward to
> I agree with Ulf that using mmc-pwrseq would be a good solution and in
> fact I think the pwrseq_simple [0] driver will fit your use case since
> it supports a reset GPIO pin which is what many WLAN chips attached to
> a SDIO interface use.
>> implement
>> it just like card detect or write-protection gpio pins. I already noticed
>> that
>> there is code for some mmc host driver, which perform mmc hardware reset. If
>> you
>> think that extending pwrseq is the better approach, I will try to update my
>> patches. This will add reboot handler to mmc-pwrseq then. The only question
>> is
> I don't think that adding a reboot handler to mmc-pwrseq is needed.
> AFAICT the call chain is:
>
> sys_reboot() -> kernel_restart() -> device_shutdown() ->
> mmc_bus_shutdown() -> _mmc_suspend() -> mmc_power_off() ->
> mmc_pwrseq_power_off() -> struct mmc_pwrseq_ops .power_off
>
> So since the pwrseq_simple already asserts the reset GPIO in
> .power_off, it should be enough to ensure the eMMC will be reset to
> its default configuration for the iROM to work properly.

I think that I had to add reset handler, because device_shutdown() was not
called if reset was triggered from magic sysrq, but I will check it again.

> It also asserts the GPIO pin in .pre_power_on and de-asserts in
> .post_power_on which should be needed as well for the other case you
> mentioned when a broken bootloader left the emmc card in some unknown
> state.
>
>> weather to use it always when mmc-pwrseq has been enabled or only if some
>> additional property like 'reset-on-reboot' has been provided.
>>
> Having a reset GPIO is optional AFAIK so I don't think there is a need
> for an additional "reset-on-reboot" propery.
>
>> Best regards
>> --
>> Marek Szyprowski, PhD
>> Samsung R&D Institute Poland
>>
> Best regards,
> Javier
>
> [0]:
> FYI, these are the relevant commits in linux-next:
> fe1922d5d4d0 mmc: pwrseq_simple: Add support for a reset GPIO pin
> ec2017f2491f mmc: pwrseq: Initial support for the simple MMC power
> sequence provider
> fe1686658f9c mmc: pwrseq: Document DT bindings for the simple MMC power sequence
> 1b745e8a4627 mmc: core: Initial support for MMC power sequences

Ok, I will check it.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland