From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrian Hunter Subject: Re: [PATCH 07/23] mmc: sdhci: check SDHCI_QUIRK2_NO_1_8_V when do voltage switch Date: Wed, 27 Apr 2016 23:27:07 +0300 Message-ID: <5721209B.7000707@intel.com> References: <1460741387-23815-1-git-send-email-aisheng.dong@nxp.com> <1460741387-23815-8-git-send-email-aisheng.dong@nxp.com> <571A1970.7010800@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com ([134.134.136.20]:30722 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752362AbcD0U1M (ORCPT ); Wed, 27 Apr 2016 16:27:12 -0400 In-Reply-To: Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: Dong Aisheng Cc: Dong Aisheng , "linux-mmc@vger.kernel.org" , Ulf Hansson , Chris Ball , Shawn Guo , "linux-arm-kernel@lists.infradead.org" , haibo.chen@nxp.com On 24/04/2016 12:56 p.m., Dong Aisheng wrote: > On Fri, Apr 22, 2016 at 8:30 PM, Adrian Hunter wrote: >> On 15/04/16 20:29, Dong Aisheng wrote: >>> Currently when card type supports EXT_CSD_CARD_TYPE_DDR_1_8V >>> which means it can work on DDR mode with either 3.3v IO or 1.8v >>> IO voltage. MMC core will first try 1.8v then 3.3v if host claims >>> MMC_CAP_1_8V_DDR support. >>> However the host driver voltage switch code does not check NO_1_8_V >>> quirk which may set a wrong 1.8v and causes the card fixed to 3.3v >>> VIO un-work. >>> >>> Checking 1.8V quirk before setting it to avoid such issue. >> >> We need to look forward to when SDHCI_QUIRK2_NO_1_8_V doesn't exist. >> > > Yes, i did try to clean up SDHCI_QUIRK2_NO_1_8_V. > However, it seems not that simply. > Currently we may still need it before we got a better way. > > My point is whether we should stop fixing the exist issue in SDHCI driver > just caused by we want to clean up it later? If you want a fix for stable kernels, then I guess it is OK. In that case it is better to have a fix that applies cleanly to older kernels i.e. not on top of re-factoring > >> There are two possibilities: >> >> 1. Add MMC_CAP_3_3V_DDR and support to core and use that instead of >> MMC_CAP_1_8V_DDR. You'll need Ulf's feedback on that. >> > > Yes, we can do it. > But the point is for DDR50/SD3.0/SDIO3.0/HS200/HS400 cards we have > the same situation and still need QUIRK2_NO_1_8_V. > > And i somehow a bit wonder whether we should retrieve the speed mode > support from device tree since it's actually controller capability. > IO range capability is another thing. > > Probably we may start another topic to discuss it specificly. > >> 2. Replace ->start_signal_voltage_switch() i.e. >> >> host->mmc_host_ops.start_signal_voltage_switch = >> esdhci_start_signal_voltage_switch; >> >> You will also need to change all calls to >> sdhci_start_signal_voltage_switch() with >> host->mmc->ops->start_signal_voltage_switch(), but that needs to be done anyway. >> > > esdhc can fully use the common sdhci_start_signal_voltag_switch. > Since it's already support QUIRK_NO_1_8_V, we don't want to > invent imx voltage swith currently, but fix and use it first. > > Regards > Dong Aisheng > >>> >>> CC: stable >>> Signed-off-by: Dong Aisheng >>> --- >>> drivers/mmc/host/sdhci.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> index 2338aab..96ccb15 100644 >>> --- a/drivers/mmc/host/sdhci.c >>> +++ b/drivers/mmc/host/sdhci.c >>> @@ -1683,6 +1683,8 @@ static int sdhci_do_signal_voltage_switch(struct sdhci_host *host, >>> >>> return -EAGAIN; >>> case MMC_SIGNAL_VOLTAGE_180: >>> + if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) >>> + return -EINVAL; >>> /* >>> * Enable 1.8V Signal Enable in the Host Control2 >>> * register >>> >>