From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrian Hunter Subject: Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback Date: Tue, 29 Aug 2017 13:25:22 +0300 Message-ID: <0895bbd0-438d-9460-b913-37d3a6ec5a59@intel.com> References: <1503338529-21102-1-git-send-email-zjwu@marvell.com> <87val76624.fsf@free-electrons.com> <59A464BA.6030401@marvell.com> <87bmmy65je.fsf@free-electrons.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from mga07.intel.com ([134.134.136.100]:17126 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752306AbdH2Kb5 (ORCPT ); Tue, 29 Aug 2017 06:31:57 -0400 In-Reply-To: <87bmmy65je.fsf@free-electrons.com> Content-Language: en-US Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: Gregory CLEMENT , ulf.hansson@linaro.org, Zhoujie Wu Cc: linux-mmc@vger.kernel.org, zmxu@marvell.com, jszhang@marvell.com, nadavh@marvell.com, xigu@marvell.com, dingwei@marvell.com, kostap@marvell.com, hannah@marvell.com, hongd@marvell.com, dougj@marvell.com, ygao@marvell.com, liuw@marvell.com, thomas.petazzoni@free-electrons.com On 29/08/17 13:18, Gregory CLEMENT wrote: > Hi Zhoujie, > > On lun., août 28 2017, Zhoujie Wu wrote: > >> Hi Gregory, >> >> On 08/28/2017 08:54 AM, Gregory CLEMENT wrote: >>> Hi, >>> On lun., août 28 2017, Adrian Hunter >>> 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 >>>> Acked-by: Adrian Hunter >>> 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 >>> >>> 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: 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, >>>>> >> >