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

Ulf Hansson
2015-01-30T11:33:35Z

On 30 January 2015 at 11:37, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 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.
>
>
> Ok, I've checked mmc-prwseq and mmc-pwrseq-simple. I also checked the
> hardware and it mmc-pwrseq-simple cannot be used directly.
>
> Although the signal is called RSTN (on Odroid U3 schema), the eMMC card
> gets resetted not on low line level, but during the rising edge. This RSTN
> line is also pulled up by the external resistor. However, the strangest
> thing is the fact that the default SoC configuration (which is applied
> during hw reset) for this GPIO line is input, pulled-down. The SoC
> internal pull-down is stronger than the external pull up, so in the end,
> during the SoC reboot the RSTN signal is set to zero. Later bootloader
> disables the internal pull-down.
>
> To sum up - to perform proper reboot on Odroid U3/XU3, one need to set
> RSTN to zero, wait a while and the set it back to 1.
>
> To achieve this with mmc-pwrseq-simple, I would need to modify the power_off
> callback to toggle reset line to zero and back to one. This however might
> not be desired to other sd/mmc cards used with mmc-pwrseq-simple.
>
> I can also provide separate mmc-pwrsrq-odroid driver, which will be very
> similar to mmc-pwrseq-simple.
>
> Ulf - which approach would you prefer?

To me this seems like a quite generic eMMC hw-reset thing, thus we
should maybe add a new pwrseq "driver" for it.

In principle you need to toogle a GPIO in the ->pre_power_on()
callback, right? And you are also proposing to register a restart
handler from the ->alloc() callback!?

I suppose this should work nicely.

Kind regards
Uffe

>
>
>>
>>> 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.
>>
>> 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.
>
>
> emergency_restart() doesn't call device_shutdown(), so I think it still
> makes
> sense to add real reset handler to mmc-pwrseq to ensure that power_off will
> be called even during emergency_restart().
>
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>