linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed
@ 2016-05-04  7:14 Jaehoon Chung
  2016-05-04  7:14 ` [PATCH 2/2] mmc: dw_mmc: prevent to set the wrong value Jaehoon Chung
  2016-05-04 10:41 ` [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed Shawn Lin
  0 siblings, 2 replies; 8+ messages in thread
From: Jaehoon Chung @ 2016-05-04  7:14 UTC (permalink / raw)
  To: linux-mmc; +Cc: ulf.hansson, shawn.lin, Jaehoon Chung

If vqmmc is used and failed to switch voltage, then retry to switch
voltage. MMC core is providing the retrying scheame.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 9dd1bd3..28602cc 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1416,7 +1416,7 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
 			dev_dbg(&mmc->class_dev,
 					 "Regulator set error %d - %s V\n",
 					 ret, uhs & v18 ? "1.8" : "3.3");
-			return ret;
+			return -EAGAIN;
 		}
 	}
 	mci_writel(host, UHS_REG, uhs);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] mmc: dw_mmc: prevent to set the wrong value
  2016-05-04  7:14 [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed Jaehoon Chung
@ 2016-05-04  7:14 ` Jaehoon Chung
  2016-05-04 10:35   ` Shawn Lin
  2016-05-04 10:41 ` [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed Shawn Lin
  1 sibling, 1 reply; 8+ messages in thread
From: Jaehoon Chung @ 2016-05-04  7:14 UTC (permalink / raw)
  To: linux-mmc; +Cc: ulf.hansson, shawn.lin, Jaehoon Chung

If there is no vqmmc, real voltage should not change.
Then it needs to maintain the previous status for UHS_REG register.
It means that it doesn't need to set any bit.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/host/dw_mmc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 28602cc..6bd8018 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1418,7 +1418,19 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
 					 ret, uhs & v18 ? "1.8" : "3.3");
 			return -EAGAIN;
 		}
+	} else {
+		/*
+		 * If there isn't vqmmc, it should fail to switch voltage.
+		 * Then it needs to maintain the previous status.
+		 * If ios->signal_voltage is 3.3v, it means that previous
+		 * voltages was 1.8v.
+		 */
+		if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
+			uhs |= v18;
+		else
+			uhs &= ~v18;
 	}
+
 	mci_writel(host, UHS_REG, uhs);
 
 	return 0;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] mmc: dw_mmc: prevent to set the wrong value
  2016-05-04  7:14 ` [PATCH 2/2] mmc: dw_mmc: prevent to set the wrong value Jaehoon Chung
@ 2016-05-04 10:35   ` Shawn Lin
  2016-05-10  1:49     ` Jaehoon Chung
  0 siblings, 1 reply; 8+ messages in thread
From: Shawn Lin @ 2016-05-04 10:35 UTC (permalink / raw)
  To: Jaehoon Chung, linux-mmc; +Cc: shawn.lin, ulf.hansson

在 2016/5/4 15:14, Jaehoon Chung 写道:
> If there is no vqmmc, real voltage should not change.
> Then it needs to maintain the previous status for UHS_REG register.
> It means that it doesn't need to set any bit.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 28602cc..6bd8018 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1418,7 +1418,19 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>  					 ret, uhs & v18 ? "1.8" : "3.3");
>  			return -EAGAIN;
>  		}
> +	} else {
> +		/*
> +		 * If there isn't vqmmc, it should fail to switch voltage.
> +		 * Then it needs to maintain the previous status.
> +		 * If ios->signal_voltage is 3.3v, it means that previous
> +		 * voltages was 1.8v.
> +		 */
> +		if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> +			uhs |= v18;
> +		else
> +			uhs &= ~v18;

I think we can just set uhs after seting vqmmc correctly. So
how about changing like this?

         uhs = mci_readl(host, UHS_REG);

         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 ? "3.3" : "1.8");
                         return -EAGAIN;
                 }
                 if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
                         uhs &= ~v18;
                 else
                         uhs |= v18;
         }
         mci_writel(host, UHS_REG, uhs);


>  	}
> +
>  	mci_writel(host, UHS_REG, uhs);
>
>  	return 0;
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed
  2016-05-04  7:14 [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed Jaehoon Chung
  2016-05-04  7:14 ` [PATCH 2/2] mmc: dw_mmc: prevent to set the wrong value Jaehoon Chung
@ 2016-05-04 10:41 ` Shawn Lin
  2016-05-10  1:55   ` Jaehoon Chung
  1 sibling, 1 reply; 8+ messages in thread
From: Shawn Lin @ 2016-05-04 10:41 UTC (permalink / raw)
  To: Jaehoon Chung, linux-mmc; +Cc: shawn.lin, ulf.hansson

在 2016/5/4 15:14, Jaehoon Chung 写道:
> If vqmmc is used and failed to switch voltage, then retry to switch
> voltage. MMC core is providing the retrying scheame.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 9dd1bd3..28602cc 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1416,7 +1416,7 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>  			dev_dbg(&mmc->class_dev,
>  					 "Regulator set error %d - %s V\n",
>  					 ret, uhs & v18 ? "1.8" : "3.3");
> -			return ret;
> +			return -EAGAIN;
>  		}
>  	}

If not vqmmc is assigned, dw_mci_switch_voltage still returns success to
mmc core?

>  	mci_writel(host, UHS_REG, uhs);
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] mmc: dw_mmc: prevent to set the wrong value
  2016-05-04 10:35   ` Shawn Lin
@ 2016-05-10  1:49     ` Jaehoon Chung
  0 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2016-05-10  1:49 UTC (permalink / raw)
  To: Shawn Lin, linux-mmc; +Cc: ulf.hansson

Hi Shawn,

On 05/04/2016 07:35 PM, Shawn Lin wrote:
> 在 2016/5/4 15:14, Jaehoon Chung 写道:
>> If there is no vqmmc, real voltage should not change.
>> Then it needs to maintain the previous status for UHS_REG register.
>> It means that it doesn't need to set any bit.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 28602cc..6bd8018 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1418,7 +1418,19 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>>                       ret, uhs & v18 ? "1.8" : "3.3");
>>              return -EAGAIN;
>>          }
>> +    } else {
>> +        /*
>> +         * If there isn't vqmmc, it should fail to switch voltage.
>> +         * Then it needs to maintain the previous status.
>> +         * If ios->signal_voltage is 3.3v, it means that previous
>> +         * voltages was 1.8v.
>> +         */
>> +        if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
>> +            uhs |= v18;
>> +        else
>> +            uhs &= ~v18;
> 
> I think we can just set uhs after seting vqmmc correctly. So
> how about changing like this?

Yes, it's more readable than mine. Thanks.

Best Regards,
Jaehoon Chung

> 
>         uhs = mci_readl(host, UHS_REG);
> 
>         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 ? "3.3" : "1.8");
>                         return -EAGAIN;
>                 }
>                 if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
>                         uhs &= ~v18;
>                 else
>                         uhs |= v18;
>         }
>         mci_writel(host, UHS_REG, uhs);
> 
> 
>>      }
>> +
>>      mci_writel(host, UHS_REG, uhs);
>>
>>      return 0;
>>
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed
  2016-05-04 10:41 ` [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed Shawn Lin
@ 2016-05-10  1:55   ` Jaehoon Chung
  2016-05-10  9:24     ` Shawn Lin
  0 siblings, 1 reply; 8+ messages in thread
From: Jaehoon Chung @ 2016-05-10  1:55 UTC (permalink / raw)
  To: Shawn Lin, linux-mmc; +Cc: ulf.hansson

Hi Shawn,

On 05/04/2016 07:41 PM, Shawn Lin wrote:
> 在 2016/5/4 15:14, Jaehoon Chung 写道:
>> If vqmmc is used and failed to switch voltage, then retry to switch
>> voltage. MMC core is providing the retrying scheame.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 9dd1bd3..28602cc 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1416,7 +1416,7 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>>              dev_dbg(&mmc->class_dev,
>>                       "Regulator set error %d - %s V\n",
>>                       ret, uhs & v18 ? "1.8" : "3.3");
>> -            return ret;
>> +            return -EAGAIN;
>>          }
>>      }
> 
> If not vqmmc is assigned, dw_mci_switch_voltage still returns success to
> mmc core?

There is comment in dw_mci_switch_voltage().. In some SoC, it's not harmful to switch voltage.
But we can decide this condition whether try to set both or not. 

Best Regards,
Jaehoon Chung

> 
>>      mci_writel(host, UHS_REG, uhs);
>>
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed
  2016-05-10  1:55   ` Jaehoon Chung
@ 2016-05-10  9:24     ` Shawn Lin
  2016-05-11  2:51       ` Jaehoon Chung
  0 siblings, 1 reply; 8+ messages in thread
From: Shawn Lin @ 2016-05-10  9:24 UTC (permalink / raw)
  To: Jaehoon Chung, Shawn Lin, linux-mmc; +Cc: shawn.lin, ulf.hansson

On 2016/5/10 9:55, Jaehoon Chung wrote:
> Hi Shawn,
>
> On 05/04/2016 07:41 PM, Shawn Lin wrote:
>> 在 2016/5/4 15:14, Jaehoon Chung 写道:
>>> If vqmmc is used and failed to switch voltage, then retry to switch
>>> voltage. MMC core is providing the retrying scheame.
>>>
>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> ---
>>>  drivers/mmc/host/dw_mmc.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 9dd1bd3..28602cc 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -1416,7 +1416,7 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>>>              dev_dbg(&mmc->class_dev,
>>>                       "Regulator set error %d - %s V\n",
>>>                       ret, uhs & v18 ? "1.8" : "3.3");
>>> -            return ret;
>>> +            return -EAGAIN;
>>>          }
>>>      }
>>
>> If not vqmmc is assigned, dw_mci_switch_voltage still returns success to
>> mmc core?
>
> There is comment in dw_mci_switch_voltage().. In some SoC, it's not harmful to switch voltage.
> But we can decide this condition whether try to set both or not.


Because for dw_mmc-rockchip, we always need to swicth voltage by
regulator. So do you mean some exynos Socs can switch voltage just
by setting UHS_REG!?  So to make things safe, we can set UHS_REG and
switch voltage by  regulator both?



>
> Best Regards,
> Jaehoon Chung
>
>>
>>>      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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed
  2016-05-10  9:24     ` Shawn Lin
@ 2016-05-11  2:51       ` Jaehoon Chung
  0 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2016-05-11  2:51 UTC (permalink / raw)
  To: Shawn Lin, Shawn Lin, linux-mmc; +Cc: ulf.hansson

On 05/10/2016 06:24 PM, Shawn Lin wrote:
> On 2016/5/10 9:55, Jaehoon Chung wrote:
>> Hi Shawn,
>>
>> On 05/04/2016 07:41 PM, Shawn Lin wrote:
>>> 在 2016/5/4 15:14, Jaehoon Chung 写道:
>>>> If vqmmc is used and failed to switch voltage, then retry to switch
>>>> voltage. MMC core is providing the retrying scheame.
>>>>
>>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>> ---
>>>>  drivers/mmc/host/dw_mmc.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 9dd1bd3..28602cc 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -1416,7 +1416,7 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>>>>              dev_dbg(&mmc->class_dev,
>>>>                       "Regulator set error %d - %s V\n",
>>>>                       ret, uhs & v18 ? "1.8" : "3.3");
>>>> -            return ret;
>>>> +            return -EAGAIN;
>>>>          }
>>>>      }
>>>
>>> If not vqmmc is assigned, dw_mci_switch_voltage still returns success to
>>> mmc core?
>>
>> There is comment in dw_mci_switch_voltage().. In some SoC, it's not harmful to switch voltage.
>> But we can decide this condition whether try to set both or not.
> 
> 
> Because for dw_mmc-rockchip, we always need to swicth voltage by
> regulator. So do you mean some exynos Socs can switch voltage just
> by setting UHS_REG!?  So to make things safe, we can set UHS_REG and
> switch voltage by  regulator both?

Ok. I will update the patch.

Best Regards,
Jaehoon Chung

> 
> 
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>>>      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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-05-11  2:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-04  7:14 [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed Jaehoon Chung
2016-05-04  7:14 ` [PATCH 2/2] mmc: dw_mmc: prevent to set the wrong value Jaehoon Chung
2016-05-04 10:35   ` Shawn Lin
2016-05-10  1:49     ` Jaehoon Chung
2016-05-04 10:41 ` [PATCH 1/2] mmc: dw_mmc: retry to switch voltage when failed Shawn Lin
2016-05-10  1:55   ` Jaehoon Chung
2016-05-10  9:24     ` Shawn Lin
2016-05-11  2:51       ` Jaehoon Chung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).