Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback

[Adrian Hunter <adrian.hunter@intel.com>]

On 29/08/17 13:18, Gregory CLEMENT wrote:
> Hi Zhoujie,
>  
>  On lun., août 28 2017, Zhoujie Wu <zjwu@marvell.com> wrote:
> 
>> Hi Gregory,
>>
>> On 08/28/2017 08:54 AM, Gregory CLEMENT wrote:
>>> Hi,
>>>     On lun., août 28 2017, Adrian Hunter <adrian.hunter@intel.com>
>>> wrote:
>>>
>>>> On 21/08/17 21:02, Zhoujie Wu wrote:
>>>>> Xenon sdh controller requests proper SD bus voltage select
>>>>> bits programmed even with vmmc power supply. Any reserved
>>>>> value(100b-000b) programmed in this field will lead to controller
>>>>> ignore SD bus power bit and keep its value at zero.
>>>>> Add set_power callback to handle this.
>>>>>
>>>>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>>> I tested this patch on the Armada 3720 DB board. And thanks to this patch
>>> and by adding a vmmc regulator in the device tree I did not have anymore
>>> the issue UHS card not detected on warm reset.
>>>
>>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>
>>> Actually for me this should be a fix. Indeed if I try to use a vmmc
>>> regulator without this patch then the SD card does not work at all I
>>> only have the following messages:
>>>
>>> "mmc1: Timeout waiting for hardware cmd interrupt."
>> This patch with sd vmmc power supply(must be claimed as AON, or card
>> detection can't work)enabled can solve the issue for warm reset. This
>> patch is must for xenon controller once vmmc power supply enabled,
>> since the HW will ignore any reserved value for sd bus voltage select
>> field and ignore the pwr_en in this case.
> 
> That's why I would like that this patch was tagged as a fix.
> 
> Adrian, Ulf, do you think it could be applied on the v4.13-rc or at
> least applied with the following tags ?
> 
> Fixes: 3a3748dba881(" mmc: sdhci-xenon: Add Marvell Xenon SDHC core
> functionality")
> Cc: <stable@vger.kernel.org>

I have no objections.

> 
> Thanks,
> 
> Gregory
> 
>> But recently we met another issue after resume, it is similar as warm
>> reset. When resume back, SD card(UHS) is re-initialized and it is
>> recognized as HS again. It needs power-cycle before initialization to
>> solve the resume issue. But vmmc is claimed as AON, it won't be shut
>> down in standby, so vmmc and this patch won't solve resume issue. I
>> have no better idea to solve both warm reset and resume issue, I added
>> a dedicated pwrseq for a3700 sd card, it will do power cycle during
>> pre and post pwrseq, but keep power when power off the card.
>> Do you have any other better suggestion?
>>>
>>> Thanks,
>>>
>>> Gregory
>>>
>>>
>>>>> ---
>>>>> Updated according to Jisheng Zhang's comment.
>>>>>   drivers/mmc/host/sdhci-xenon.c | 19 +++++++++++++++++++
>>>>>   1 file changed, 19 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
>>>>> index edd4d915..a4be2fd 100644
>>>>> --- a/drivers/mmc/host/sdhci-xenon.c
>>>>> +++ b/drivers/mmc/host/sdhci-xenon.c
>>>>> @@ -210,8 +210,27 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,
>>>>>   	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>>>   }
>>>>>   +static void xenon_set_power(struct sdhci_host *host, unsigned
>>>>> char mode,
>>>>> +		unsigned short vdd)
>>>>> +{
>>>>> +	struct mmc_host *mmc = host->mmc;
>>>>> +	u8 pwr = host->pwr;
>>>>> +
>>>>> +	sdhci_set_power_noreg(host, mode, vdd);
>>>>> +
>>>>> +	if (host->pwr == pwr)
>>>>> +		return;
>>>>> +
>>>>> +	if (host->pwr == 0)
>>>>> +		vdd = 0;
>>>>> +
>>>>> +	if (!IS_ERR(mmc->supply.vmmc))
>>>>> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>>>> +}
>>>>> +
>>>>>   static const struct sdhci_ops sdhci_xenon_ops = {
>>>>>   	.set_clock		= sdhci_set_clock,
>>>>> +	.set_power		= xenon_set_power,
>>>>>   	.set_bus_width		= sdhci_set_bus_width,
>>>>>   	.reset			= xenon_reset,
>>>>>   	.set_uhs_signaling	= xenon_set_uhs_signaling,
>>>>>
>>
>