Re: [RESEDN PATCH v2 3/4] mmc: sdhci: Don't try to switch to unsupported voltage

2016-09-23 Thread Shawn Lin 

在 2016/9/23 15:12, Adrian Hunter 写道:

On 23/09/16 03:12, Shawn Lin wrote:

From: Ziyuan Xu 

Sdhci shouldn't switch to the unsupported voltage if claiming
that it can not support the requested voltage. Let's fix it.

Signed-off-by: Ziyuan Xu 
Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/sdhci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4805566..b1f1edd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1845,7 +1845,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,

switch (ios->signal_voltage) {
case MMC_SIGNAL_VOLTAGE_330:
-   if (!(host->flags & SDHCI_SIGNALING_330))
+   if (!(host->flags & SDHCI_SIGNALING_330) ||
+   !(host->caps & SDHCI_CAN_VDD_330))


You are mixing signal voltage and supply voltage here.  Note, I don't


Oh, I see what you mean, driver could still set 3.3 voltage supply
even they don't support 3V3 signal voltage.


believe all drivers set the capabilities correctly when they are using
regulators, but in any case this is not the place in the code to make such
assumptions.


Indeed, there is a risk of breaking some drivers if they don't set the
capabilities correctly.

So, I think we could drop this patch since patch 4 is actually what we
need in order to slove our problem. :)



Better for the driver to remove SDHCI_SIGNALING_330 from flags when it is
not supported.


return -EINVAL;
/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
ctrl &= ~SDHCI_CTRL_VDD_180;
@@ -1872,7 +1873,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,

return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_180:
-   if (!(host->flags & SDHCI_SIGNALING_180))
+   if (!(host->flags & SDHCI_SIGNALING_180) ||
+   !(host->caps & SDHCI_CAN_VDD_180))


You are mixing signal voltage and supply voltage here, and this is not
correct: not being able to do 1.8V supply does not mean you can't do 1.8V
signaling.  The SDHCI way of determining supported signal voltages is based
on supported transfer modes.

If the driver can't get the transfer modes correct (e.g. we still don't have
MMC_CAP2_3_3V_ONLY_DDR or equivalent), remove SDHCI_SIGNALING_180 from the
flags.


return -EINVAL;
if (!IS_ERR(mmc->supply.vqmmc)) {
ret = mmc_regulator_set_vqmmc(mmc, ios);









--
Best Regards
Shawn Lin

Re: [RESEDN PATCH v2 3/4] mmc: sdhci: Don't try to switch to unsupported voltage

2016-09-23 Thread Shawn Lin 

在 2016/9/23 15:12, Adrian Hunter 写道:

On 23/09/16 03:12, Shawn Lin wrote:

From: Ziyuan Xu 

Sdhci shouldn't switch to the unsupported voltage if claiming
that it can not support the requested voltage. Let's fix it.

Signed-off-by: Ziyuan Xu 
Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/sdhci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4805566..b1f1edd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1845,7 +1845,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,

switch (ios->signal_voltage) {
case MMC_SIGNAL_VOLTAGE_330:
-   if (!(host->flags & SDHCI_SIGNALING_330))
+   if (!(host->flags & SDHCI_SIGNALING_330) ||
+   !(host->caps & SDHCI_CAN_VDD_330))


You are mixing signal voltage and supply voltage here.  Note, I don't


Oh, I see what you mean, driver could still set 3.3 voltage supply
even they don't support 3V3 signal voltage.


believe all drivers set the capabilities correctly when they are using
regulators, but in any case this is not the place in the code to make such
assumptions.


Indeed, there is a risk of breaking some drivers if they don't set the
capabilities correctly.

So, I think we could drop this patch since patch 4 is actually what we
need in order to slove our problem. :)



Better for the driver to remove SDHCI_SIGNALING_330 from flags when it is
not supported.


return -EINVAL;
/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
ctrl &= ~SDHCI_CTRL_VDD_180;
@@ -1872,7 +1873,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,

return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_180:
-   if (!(host->flags & SDHCI_SIGNALING_180))
+   if (!(host->flags & SDHCI_SIGNALING_180) ||
+   !(host->caps & SDHCI_CAN_VDD_180))


You are mixing signal voltage and supply voltage here, and this is not
correct: not being able to do 1.8V supply does not mean you can't do 1.8V
signaling.  The SDHCI way of determining supported signal voltages is based
on supported transfer modes.

If the driver can't get the transfer modes correct (e.g. we still don't have
MMC_CAP2_3_3V_ONLY_DDR or equivalent), remove SDHCI_SIGNALING_180 from the
flags.


return -EINVAL;
if (!IS_ERR(mmc->supply.vqmmc)) {
ret = mmc_regulator_set_vqmmc(mmc, ios);









--
Best Regards
Shawn Lin

Re: [RESEDN PATCH v2 3/4] mmc: sdhci: Don't try to switch to unsupported voltage

2016-09-23 Thread Adrian Hunter 
On 23/09/16 03:12, Shawn Lin wrote:
> From: Ziyuan Xu 
> 
> Sdhci shouldn't switch to the unsupported voltage if claiming
> that it can not support the requested voltage. Let's fix it.
> 
> Signed-off-by: Ziyuan Xu 
> Signed-off-by: Shawn Lin 
> ---
> 
> Changes in v2: None
> 
>  drivers/mmc/host/sdhci.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 4805566..b1f1edd 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1845,7 +1845,8 @@ static int sdhci_start_signal_voltage_switch(struct 
> mmc_host *mmc,
>  
>   switch (ios->signal_voltage) {
>   case MMC_SIGNAL_VOLTAGE_330:
> - if (!(host->flags & SDHCI_SIGNALING_330))
> + if (!(host->flags & SDHCI_SIGNALING_330) ||
> + !(host->caps & SDHCI_CAN_VDD_330))

You are mixing signal voltage and supply voltage here.  Note, I don't
believe all drivers set the capabilities correctly when they are using
regulators, but in any case this is not the place in the code to make such
assumptions.

Better for the driver to remove SDHCI_SIGNALING_330 from flags when it is
not supported.

>   return -EINVAL;
>   /* Set 1.8V Signal Enable in the Host Control2 register to 0 */
>   ctrl &= ~SDHCI_CTRL_VDD_180;
> @@ -1872,7 +1873,8 @@ static int sdhci_start_signal_voltage_switch(struct 
> mmc_host *mmc,
>  
>   return -EAGAIN;
>   case MMC_SIGNAL_VOLTAGE_180:
> - if (!(host->flags & SDHCI_SIGNALING_180))
> + if (!(host->flags & SDHCI_SIGNALING_180) ||
> + !(host->caps & SDHCI_CAN_VDD_180))

You are mixing signal voltage and supply voltage here, and this is not
correct: not being able to do 1.8V supply does not mean you can't do 1.8V
signaling.  The SDHCI way of determining supported signal voltages is based
on supported transfer modes.

If the driver can't get the transfer modes correct (e.g. we still don't have
MMC_CAP2_3_3V_ONLY_DDR or equivalent), remove SDHCI_SIGNALING_180 from the
flags.

>   return -EINVAL;
>   if (!IS_ERR(mmc->supply.vqmmc)) {
>   ret = mmc_regulator_set_vqmmc(mmc, ios);
>

Re: [RESEDN PATCH v2 3/4] mmc: sdhci: Don't try to switch to unsupported voltage

2016-09-23 Thread Adrian Hunter 
On 23/09/16 03:12, Shawn Lin wrote:
> From: Ziyuan Xu 
> 
> Sdhci shouldn't switch to the unsupported voltage if claiming
> that it can not support the requested voltage. Let's fix it.
> 
> Signed-off-by: Ziyuan Xu 
> Signed-off-by: Shawn Lin 
> ---
> 
> Changes in v2: None
> 
>  drivers/mmc/host/sdhci.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 4805566..b1f1edd 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1845,7 +1845,8 @@ static int sdhci_start_signal_voltage_switch(struct 
> mmc_host *mmc,
>  
>   switch (ios->signal_voltage) {
>   case MMC_SIGNAL_VOLTAGE_330:
> - if (!(host->flags & SDHCI_SIGNALING_330))
> + if (!(host->flags & SDHCI_SIGNALING_330) ||
> + !(host->caps & SDHCI_CAN_VDD_330))

You are mixing signal voltage and supply voltage here.  Note, I don't
believe all drivers set the capabilities correctly when they are using
regulators, but in any case this is not the place in the code to make such
assumptions.

Better for the driver to remove SDHCI_SIGNALING_330 from flags when it is
not supported.

>   return -EINVAL;
>   /* Set 1.8V Signal Enable in the Host Control2 register to 0 */
>   ctrl &= ~SDHCI_CTRL_VDD_180;
> @@ -1872,7 +1873,8 @@ static int sdhci_start_signal_voltage_switch(struct 
> mmc_host *mmc,
>  
>   return -EAGAIN;
>   case MMC_SIGNAL_VOLTAGE_180:
> - if (!(host->flags & SDHCI_SIGNALING_180))
> + if (!(host->flags & SDHCI_SIGNALING_180) ||
> + !(host->caps & SDHCI_CAN_VDD_180))

You are mixing signal voltage and supply voltage here, and this is not
correct: not being able to do 1.8V supply does not mean you can't do 1.8V
signaling.  The SDHCI way of determining supported signal voltages is based
on supported transfer modes.

If the driver can't get the transfer modes correct (e.g. we still don't have
MMC_CAP2_3_3V_ONLY_DDR or equivalent), remove SDHCI_SIGNALING_180 from the
flags.

>   return -EINVAL;
>   if (!IS_ERR(mmc->supply.vqmmc)) {
>   ret = mmc_regulator_set_vqmmc(mmc, ios);
>

[RESEDN PATCH v2 3/4] mmc: sdhci: Don't try to switch to unsupported voltage

2016-09-22 Thread Shawn Lin 
From: Ziyuan Xu 

Sdhci shouldn't switch to the unsupported voltage if claiming
that it can not support the requested voltage. Let's fix it.

Signed-off-by: Ziyuan Xu 
Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/sdhci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4805566..b1f1edd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1845,7 +1845,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,
 
switch (ios->signal_voltage) {
case MMC_SIGNAL_VOLTAGE_330:
-   if (!(host->flags & SDHCI_SIGNALING_330))
+   if (!(host->flags & SDHCI_SIGNALING_330) ||
+   !(host->caps & SDHCI_CAN_VDD_330))
return -EINVAL;
/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
ctrl &= ~SDHCI_CTRL_VDD_180;
@@ -1872,7 +1873,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,
 
return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_180:
-   if (!(host->flags & SDHCI_SIGNALING_180))
+   if (!(host->flags & SDHCI_SIGNALING_180) ||
+   !(host->caps & SDHCI_CAN_VDD_180))
return -EINVAL;
if (!IS_ERR(mmc->supply.vqmmc)) {
ret = mmc_regulator_set_vqmmc(mmc, ios);
-- 
2.3.7

[RESEDN PATCH v2 3/4] mmc: sdhci: Don't try to switch to unsupported voltage

2016-09-22 Thread Shawn Lin 
From: Ziyuan Xu 

Sdhci shouldn't switch to the unsupported voltage if claiming
that it can not support the requested voltage. Let's fix it.

Signed-off-by: Ziyuan Xu 
Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/sdhci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4805566..b1f1edd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1845,7 +1845,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,
 
switch (ios->signal_voltage) {
case MMC_SIGNAL_VOLTAGE_330:
-   if (!(host->flags & SDHCI_SIGNALING_330))
+   if (!(host->flags & SDHCI_SIGNALING_330) ||
+   !(host->caps & SDHCI_CAN_VDD_330))
return -EINVAL;
/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
ctrl &= ~SDHCI_CTRL_VDD_180;
@@ -1872,7 +1873,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,
 
return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_180:
-   if (!(host->flags & SDHCI_SIGNALING_180))
+   if (!(host->flags & SDHCI_SIGNALING_180) ||
+   !(host->caps & SDHCI_CAN_VDD_180))
return -EINVAL;
if (!IS_ERR(mmc->supply.vqmmc)) {
ret = mmc_regulator_set_vqmmc(mmc, ios);
-- 
2.3.7

[PATCH v2 3/4] mmc: sdhci: Don't try to switch to unsupported voltage

2016-09-22 Thread Shawn Lin 
From: Ziyuan Xu 

Sdhci shouldn't switch to the unsupported voltage if claiming
that it can not support the requested voltage. Let's fix it.

Signed-off-by: Ziyuan Xu 
Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/sdhci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4805566..b1f1edd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1845,7 +1845,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,
 
switch (ios->signal_voltage) {
case MMC_SIGNAL_VOLTAGE_330:
-   if (!(host->flags & SDHCI_SIGNALING_330))
+   if (!(host->flags & SDHCI_SIGNALING_330) ||
+   !(host->caps & SDHCI_CAN_VDD_330))
return -EINVAL;
/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
ctrl &= ~SDHCI_CTRL_VDD_180;
@@ -1872,7 +1873,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,
 
return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_180:
-   if (!(host->flags & SDHCI_SIGNALING_180))
+   if (!(host->flags & SDHCI_SIGNALING_180) ||
+   !(host->caps & SDHCI_CAN_VDD_180))
return -EINVAL;
if (!IS_ERR(mmc->supply.vqmmc)) {
ret = mmc_regulator_set_vqmmc(mmc, ios);
-- 
2.3.7

[PATCH v2 3/4] mmc: sdhci: Don't try to switch to unsupported voltage

2016-09-22 Thread Shawn Lin 
From: Ziyuan Xu 

Sdhci shouldn't switch to the unsupported voltage if claiming
that it can not support the requested voltage. Let's fix it.

Signed-off-by: Ziyuan Xu 
Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/sdhci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4805566..b1f1edd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1845,7 +1845,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,
 
switch (ios->signal_voltage) {
case MMC_SIGNAL_VOLTAGE_330:
-   if (!(host->flags & SDHCI_SIGNALING_330))
+   if (!(host->flags & SDHCI_SIGNALING_330) ||
+   !(host->caps & SDHCI_CAN_VDD_330))
return -EINVAL;
/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
ctrl &= ~SDHCI_CTRL_VDD_180;
@@ -1872,7 +1873,8 @@ static int sdhci_start_signal_voltage_switch(struct 
mmc_host *mmc,
 
return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_180:
-   if (!(host->flags & SDHCI_SIGNALING_180))
+   if (!(host->flags & SDHCI_SIGNALING_180) ||
+   !(host->caps & SDHCI_CAN_VDD_180))
return -EINVAL;
if (!IS_ERR(mmc->supply.vqmmc)) {
ret = mmc_regulator_set_vqmmc(mmc, ios);
-- 
2.3.7