On 06/21/2016 05:13 PM, Shawn Lin wrote:
> On 2016/6/21 13:35, Jaehoon Chung wrote:
>> If there ins no vqmmc, voltage should not be changed.
>> Then it nees to maintain the previous status for USH_REG register.
>> To use the standard spec, it should be returned error when voltage
>> can't switch.
>>
>> Note:
>> Dwmmc controller will not make any exception case.(like exynos.)
>> If card isn't working well after appling this patch, it needs to check
>> the regulator side.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
>> ---
>> Changelog V2:
>> -Fix the typo
>>
>> drivers/mmc/host/dw_mmc.c | 40 ++++++++++++++++++++++------------------
>> 1 file changed, 22 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index ec3f0a8..56c48fb 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1404,33 +1404,37 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>> struct dw_mci_slot *slot = mmc_priv(mmc);
>> struct dw_mci *host = slot->host;
>> const struct dw_mci_drv_data *drv_data = host->drv_data;
>> - u32 uhs;
>> + u32 uhs = 0;
>> u32 v18 = SDMMC_UHS_18V << slot->id;
>> int ret;
>>
>> if (drv_data && drv_data->switch_voltage)
>> return drv_data->switch_voltage(mmc, ios);
>>
>> - /*
>> - * Program the voltage. Note that some instances of dw_mmc may use
>> - * the UHS_REG for this. For other instances (like exynos) the UHS_REG
>> - * does no harm but you need to set the regulator directly. Try both.
>> - */
>> + if (IS_ERR(mmc->supply.vqmmc)) {
>> + dev_dbg(&mmc->class_dev,
>> + "vqmmc not available.(Skip the switching voltage)\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = mmc_regulator_set_vqmmc(mmc, ios);
>> + if (ret) {
>> + dev_err(&mmc->class_dev,
>> + "Regulator set error %d - %s V\n",
>> + ret, uhs & v18 ? "1.8" : "3.3");
>> + return ret;
>> + }
>> +
>> uhs = mci_readl(host, UHS_REG);
>> - if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
>
> uhs is still zero now, so maybe we can remove this switch branch and
> only need a simple checking like this:
>
> if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180)
> uhs |= v18;
It's not enough to set the correct value.
So i will maintain this patch..
In future, i will add the MMC_SIGNAL_VOLTAGE_120 for USH_REG_EXT register.
At that time, switch statement should be better than "if" statement.. :)
Anyway, I applied this patch on my repository. If you have any objection, i will accept your opinion. :)
Best Regards,
Jaehoon Chung
>
> otherwise, it looks good to me.
>
>> + switch (ios->signal_voltage) {
>> + case MMC_SIGNAL_VOLTAGE_330:
>> uhs &= ~v18;
>> - else
>> + break;
>> + case MMC_SIGNAL_VOLTAGE_180:
>> uhs |= v18;
>> -
>> - if (!IS_ERR(mmc->supply.vqmmc)) {
>> - ret = mmc_regulator_set_vqmmc(mmc, ios);
>> -
>> - if (ret) {
>> - dev_dbg(&mmc->class_dev,
>> - "Regulator set error %d - %s V\n",
>> - ret, uhs & v18 ? "1.8" : "3.3");
>> - return ret;
>> - }
>> + break;
>> + default:
>> + uhs &= ~v18;
>> }
>> mci_writel(host, UHS_REG, uhs);
>>
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
