[PATCH 07/23] mmc: sdhci: check SDHCI_QUIRK2_NO_1_8_V when do voltage switch

[Adrian Hunter]

On 24/04/2016 12:56 p.m., Dong Aisheng wrote:
> On Fri, Apr 22, 2016 at 8:30 PM, Adrian Hunter <adrian.hunter at intel.com> 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 <stable at vger.kernel.org>
>>> Signed-off-by: Dong Aisheng <aisheng.dong at nxp.com>
>>> ---
>>>   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
>>>
>>