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? > > I am a little worried that we may start to rely on boot loader > conditions, which isn't really what we want either... > > > > > 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? [...] Kind regards Uffe