Re: [PATCH] mmc: mmci_sdmmc: fix power on issue due to pwr_reg initialization

From: Ludovic BARRE
Date: Thu Apr 23 2020 - 04:12:36 EST




Le 4/22/20 Ã 6:03 PM, Ulf Hansson a ÃcritÂ:
On Wed, 22 Apr 2020 at 15:40, Ludovic BARRE <ludovic.barre@xxxxxx> wrote:

hi Ulf

Le 4/21/20 Ã 11:38 AM, Ulf Hansson a Ãcrit :
On Tue, 21 Apr 2020 at 11:25, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:

On Mon, 20 Apr 2020 at 18:18, Ludovic Barre <ludovic.barre@xxxxxx> wrote:

This patch fix a power-on issue, and avoid to retry the power sequence.

In power off sequence: sdmmc must set pwr_reg in "power-cycle" state
(value 0x2), to prevent the card from being supplied through the signal
lines (all the lines are driven low).

In power on sequence: when the power is stable, sdmmc must set pwr_reg
in "power-off" state (value 0x0) to drive all signal to high before to
set "power-on".

Just a question to gain further understanding.

Let's assume that the controller is a power-on state, because it's
been initialized by the boot loader. When the mmc core then starts the
power-on sequence (not doing a power-off first), would $subject patch
then cause the
MMCIPOWER to remain as is, or is it going to be overwritten?

On sdmmc controller, the PWRCTRL[1:0] field of MMCIPOWER register allow
to manage sd lines and has a specific bahavior.

PWRCTRL value:
- 0x0: After reset, Reset: the SDMMC is disabled and the clock to the
Card is stopped, SDMMC_D[7:0], and SDMMC_CMD are HiZ and
SDMMC_CK is driven low.
When written 00, power-off: the SDMMC is disabled and the clock
to the card is stopped, SDMMC_D[7:0], SDMMC_CMD and SDMMC_CK
are driven high.

- 0x2: Power-cycle, the SDMMC is disabled and the clock to the card is
stopped, SDMMC_D[7:0], SDMMC_CMD and SDMMC_CK are driven low.

- 0x3: Power-on: the card is clocked, The first 74 SDMMC_CK cycles the
SDMMC is still disabled. After the 74 cycles the SDMMC is
enabled and the SDMMC_D[7:0], SDMMC_CMD and SDMMC_CK are
controlled according the SDMMC operation.
**Any further write will be ignored, PWRCTRL value
will keep 0x3**. when the SDMMC is ON (0x3) only a reset could
change pwrctrl value and the state of sdmmc lines.

So if the lines are already "ON", the power-on sequence (decribed in
commit message) not overwrite the pwctrl field and not disturb the sdmmc
lines.

Thanks for the detailed information, much appreciated!



I am a little worried that we may start to rely on boot loader
conditions, which isn't really what we want either...


We not depend of boot loader conditions.

This patch simply allows to drive high the sd lines before to set
"power-on" value (no effect if already power ON).

Yep, thanks!



To avoid writing the same value to the power register several times, this
register is cached by the pwr_reg variable. At probe pwr_reg is initialized
to 0 by kzalloc of mmc_alloc_host.

Like pwr_reg value is 0 at probing, the power on sequence fail because
the "power-off" state is not writes (value 0x0) and the lines
remain drive to low.

This patch initializes "pwr_reg" variable with power register value.
This it done in sdmmc variant init to not disturb default mmci behavior.

Signed-off-by: Ludovic Barre <ludovic.barre@xxxxxx>

Besides the comment, the code and the approach seems reasonable to me.

Another related question. I just realized why you probably haven't set
.pwrreg_nopower for the variant_stm32_sdmmc and variant_stm32_sdmmcv2.

I guess it's because you need a slightly different way to restore the
context of MMCIPOWER register at ->runtime_resume(), rather than just
re-writing it with the saved register values. Is this something that
you are looking into as well?

Yes exactly, the sequence is slightly different. I can't write 0 on
mmci_runtime_suspend, and can't just re-writing the saved register.

So, it seems like you need to use the ->set_ios() callback, to
re-configure the controller correctly.

Just tell if you need more help to make that work, otherwise I am here
to review your patches.

In regards to $subject patch, I have applied it for next, thanks!

Thanks for your review.
Have a nice day.


Kind regards
Uffe