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. Kind regards Uffe > --- > > This patch is the proposal from: > https://patchwork.kernel.org/patch/11457987/ > > --- > drivers/mmc/host/mmci_stm32_sdmmc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c > index d33e62bd6153..14f99d8aa3f0 100644 > --- a/drivers/mmc/host/mmci_stm32_sdmmc.c > +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c > @@ -519,6 +519,7 @@ void sdmmc_variant_init(struct mmci_host *host) > struct sdmmc_dlyb *dlyb; > > host->ops = &sdmmc_variant_ops; > + host->pwr_reg = readl_relaxed(host->base + MMCIPOWER); > > base_dlyb = devm_of_iomap(mmc_dev(host->mmc), np, 1, NULL); > if (IS_ERR(base_dlyb)) > -- > 2.17.1 >
