Re: Question about MMC_PM_KEEP_POWER in MMCI driver

[Yann Gautier <yann.gautier@foss.st.com>]

On 8/30/21 3:43 PM, Ulf Hansson wrote:
> On Wed, 25 Aug 2021 at 16:34, Yann Gautier <yann.gautier@foss.st.com> wrote:
>>
>> Hi,
>>

Hi Linus,

Thanks for the advice.

>> In drivers/mmc/host/mmci.c, MMC_PM_KEEP_POWER is unconditionally enabled.
>> This prevents correct low-power sequence on STM32MP157C-DK2 board which
>> embeds a Wifi chip brcm,bcm4329-fmac (this wifi part has not yet been
>> sent upstream).
> 
> Exactly why doesn't this work with the STM32MP157C-DK2 board?
> 
It is more the opposite.
We had that patch in our downstream for a while, and this was possible 
due to changes in wifi driver.
But I'll check all that precisely, and make some more tests.

>>
>> This MMC_PM_KEEP_POWER can be taken from DT with the property
>> keep-power-in-suspend. This is what is done for other MMC drivers.
> 
> The DT property is what should have been used for mmci as well.
> 
>>
>> I wonder what should be the best solution for this.
>>
>> 1) Remove MMC_PM_KEEP_POWER from the driver, and modify all SoC device
>> tree files embedding a arm,pl18x with adding keep-power-in-suspend;
>> property (except stm32mp151.dtsi file).
>> This can be easy to do (~10 files to modify). But that could be more
>> board dependent, if an SDIO chip is plugged on this MMC IP.
>> And the name keep-power-in-suspend can be misleading as it only applies
>> to SDIO.
>>
>> 2) Remove MMC_PM_KEEP_POWER from the driver, and modify board DT files
>> with the property. This could be a difficult task to find all those
>> boards. And this should be applied only for SDIO configs.
>>
>> 3) Just modify the driver to apply this capability for all MMCI chips
>> but STM32. This could be done in the dedicated file, in
>> sdmmc_variant_init() function. But some boards based on STM32MP15 chip
>> might want to keep this capability.
> 
> I would suggest option 3).
> 
> As a matter of fact, we also allow MMC_PM_KEEP_POWER to become set
> when parsing the DTB via calling mmc_of_parse(). So just changing the
> default value (don't set MMC_PM_KEEP_POWER) for the stm32 variant,
> would do the trick I think.
> 
OK, I'll do that, in mmci.c file, keeping the possibility to have the DT 
property set even for stm32 variant.

> Kind regards
> Uffe
> 
Best regards,
Yann