Re: [PATCH v4] mmc: dw_mmc: force setup bus if active slots exist

[Jaehoon Chung]

On 01/09/2017 12:39 PM, Ziyuan wrote:
> 
> 
> On 01/05/2017 03:34 PM, Shawn Lin wrote:
>> On 2017/1/5 15:23, Ziyuan Xu wrote:
>>> It's necessary to setup bus if any slots are present.
>>> - update clock after ctrl reset
>>> - if the host has genpd node, we can guarantee the clock is available
>>> before starting request. Otherwies, the clock register is reset once
>>> power off the pd, and host can't output the active clock during
>>> communication.
>>>
>>> fixes: e9ed8835e990 ("mmc: dw_mmc: add runtime PM callback")
>>> Reported-by: Randy Li <randy.li@xxxxxxxxxxxxxx>
>>> Signed-off-by: Ziyuan Xu <xzy.xu@xxxxxxxxxxxxxx>
>>>
>>> ---
>>> Hi guys,
>>>
>>> I found a similar issue on rk3399 platform, which has a genpd node for
>>> SD card host. Power off-on pd will reset the registers to a default
>>> value (ie. CLKENA), so that the host can't output the active clock
>>> during communication.
>>>
>>
>> Indeed, Caesar recently introduced all the genpd for rk3399 platform,
>> so we need to restore them.
>>
>>> So we need to setup bus in rpm resume. It also wraps the update clock
>>> behaviour which I did in V3.
>>>
>>> Thanks,
>>> Ziyuan Xu
>>>
>>>
>>> Changes in v4:
>>> - update commit message
>>> - fix SD host rpm resume can't work
>>>
>>> Changes in v3:
>>> - only reset host with active slot.
>>>
>>> Changes in v2:
>>> - update the commit message
>>> - use dw_mci_reset instead of dw_mci_ctrl_reset
>>>
>>>  drivers/mmc/host/dw_mmc.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index b44306b..b6053b3 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -3354,10 +3354,10 @@ int dw_mci_runtime_resume(struct device *dev)
>>>
>>>          if (!slot)
>>>              continue;
>>> -        if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
>>> +        if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER)
>>>              dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
>>> -            dw_mci_setup_bus(slot, true);
>>> -        }
>>> +        /* Force setup bus to guarantee available clock output */
>>> +        dw_mci_setup_bus(slot, true);
>>
>> So the spamming message about
>>
>> "Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)\n"
>>
>> will always be there, right? So you could append a new patch to shut
>> up it as I think it's useless no matter for system pm or rpm to print
>> it.  How about?
> 
> Fine, it's favourable with dev_vdbg if the dw_mmc rpm is enabled.
> Hi Jaehoon,
> What's your opinion? If you think this patch and shawn's advice are acceptable, I will send the v5 patch.

I don't agreed to use dev_vdbg()..but it needs not to display at everytime.
Hmm..It needs to consider more..because we needs to see the clock value when it's initialized.

Best Regards,
Jaehoon Chung

> 
> BR
> Ziyuan Xu
> 
>>
>>>      }
>>>
>>>      /* Now that slots are all setup, we can enable card detect */
>>>
>>
>>
> 
> 
> 
> 
>