diff mbox

[RFT,v2] mmc: sd: try to enable UHS mode w/o power cycle from S3

Message ID 1503978827-202915-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shawn Lin Aug. 29, 2017, 3:53 a.m. UTC
CMD0 couldn't put the card back to 3.3v if can't do power
cycle for cards. The card is already in SDR12 then, so still
try to enable UHS mode.

Pleas note that, if the platform use a hardware fixed
sd power, you should still include a regulator-fixed
regulator with always-on property there. So mmc core
could handle this as well. Otherwise you shouldn't
expect UHS to work for that case.

Just compile only, please have a look and test.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 drivers/mmc/core/card.h            | 17 +++++++++++++++++
 drivers/mmc/core/core.c            |  8 ++++++++
 drivers/mmc/core/sd.c              | 28 ++++++++++++++++++++++++++--
 drivers/mmc/core/sd.h              |  3 ++-
 drivers/mmc/core/sdio.c            |  2 +-
 drivers/regulator/core.c           | 14 ++++++++++++++
 include/linux/mmc/host.h           |  1 +
 include/linux/regulator/consumer.h |  6 ++++++
 8 files changed, 75 insertions(+), 4 deletions(-)

Comments

Zhoujie Wu Aug. 29, 2017, 10:13 p.m. UTC | #1
Hi Shawn,
thanks for your patch.

On 08/29/2017 02:56 PM, zhoujie wu wrote:
>
>
> CMD0 couldn't put the card back to 3.3v if can't do power
> cycle for cards. The card is already in SDR12 then, so still
> try to enable UHS mode.
>
> Pleas note that, if the platform use a hardware fixed
> sd power, you should still include a regulator-fixed
> regulator with always-on property there. So mmc core
> could handle this as well. Otherwise you shouldn't
> expect UHS to work for that case.
>
> Just compile only, please have a look and test.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> ---
>
>   drivers/mmc/core/card.h            | 17 +++++++++++++++++
>   drivers/mmc/core/core.c            |  8 ++++++++
>   drivers/mmc/core/sd.c              | 28 ++++++++++++++++++++++++++--
>   drivers/mmc/core/sd.h              |  3 ++-
>   drivers/mmc/core/sdio.c            |  2 +-
>   drivers/regulator/core.c           | 14 ++++++++++++++
>   include/linux/mmc/host.h           |  1 +
>   include/linux/regulator/consumer.h |  6 ++++++
>   8 files changed, 75 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
> index f06cd91..a562110 100644
> --- a/drivers/mmc/core/card.h
> +++ b/drivers/mmc/core/card.h
> @@ -218,4 +218,21 @@ static inline int mmc_card_broken_hpi(const struct
> mmc_card *c)
>          return c->quirks & MMC_QUIRK_BROKEN_HPI;
>   }
>
> +static inline bool mmc_card_support_uhs(struct mmc_card *card)
> +{
> +       if (WARN_ON(!card))
> +               return false;
> +
> +       switch (card->sd_bus_speed) {
> +       case UHS_SDR104_BUS_SPEED:
> +       case UHS_DDR50_BUS_SPEED:
> +       case UHS_SDR50_BUS_SPEED:
> +       case UHS_SDR25_BUS_SPEED:
> +       case UHS_SDR12_BUS_SPEED:
> +               return true;
> +       default:
> +               return false;
> +       }
> +}
> +
>   #endif
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 2643126..6b5c6c5 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1424,6 +1424,14 @@ int mmc_regulator_get_supply(struct mmc_host *mmc)
>   }
>   EXPORT_SYMBOL_GPL(mmc_regulator_get_supply);
>
> +bool mmc_regulator_can_power_off(struct regulator *supply)
> +{
> +       if (!IS_ERR(supply))
> +               return regulator_is_always_on(supply);
> +       return false;
> +}
> +EXPORT_SYMBOL_GPL(mmc_regulator_can_power_off);
> +
>   /*
>    * Mask off any voltages we don't support and select
>    * the lowest voltage
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index a1b0aa1..7bec61b 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -721,7 +721,8 @@ struct device_type sd_type = {
>   /*
>    * Fetch CID from card.
>    */
> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
> +                                  u32 ocr, u32 *cid, u32 *rocr)
>   {
>          int err;
>          u32 max_current;
> @@ -773,6 +774,27 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32
> *cid, u32 *rocr)
>                  return err;
>
>          /*
> +        * Per SD physical specification version 4.10, section
> +        * 3.9.4 UHS-I Bus Speed Mode Selection Sequence, once
> +        * the signal voltage is switched to 1V8, the card continues
> +        * 1.8V signaling regardless of CMD0. Power cycle resets the
> +        * signal voltage to 3.3V. As a fact that some vmmc is always
> +        * on so the card would still work with SDR12 after CMD0. So
> +        * let's attempt to enable enable UHS mode if vmmc can't be
> +        * powered off.
> +        */
> +       if (card && mmc_card_support_uhs(card) && mmc_host_uhs(host) &&
> +           !mmc_regulator_can_power_off(host->supply.vmmc) &&
> +           (ocr & SD_OCR_S18R)) {
> +                       *rocr |= SD_ROCR_S18A;
> +                       if (mmc_set_signal_voltage(host,
> MMC_SIGNAL_VOLTAGE_180)) {
> +                               retries = 0;
> +                               goto try_again;
> +                       }
> +
> +                       goto done;
> +       }
Good to have a higher level solution if the issue is common.
I did basic test with your patch(added your regulator modification you 
sent to me) and add vmmc-supply in board dts, I saw warning when boot up.
I think in below case, the card is not added yet, the card is null and 
print out the warning. I don't quite understand mmc_card_support_uhs, it 
return true for all card speed.

[    4.310003] WARNING: CPU: 1 PID: 48 at drivers/mmc/core/card.h:224 
mmc_sd_get_cid+0x270/0x298

.....

[    4.549657] [<ffffff8008757e24>] mmc_sd_get_cid+0x270/0x298

[    4.555510] [<ffffff800875847c>] mmc_sd_init_card+0x44/0x614

[    4.561456] [<ffffff8008758bd4>] mmc_attach_sd+0x7c/0x134

[    4.566951] [<ffffff8008751fb8>] mmc_rescan+0x2e4/0x35c

[    4.572451] [<ffffff80080d58d8>] process_one_work+0x12c/0x28c

[    4.578304] [<ffffff80080d5a90>] worker_thread+0x58/0x3b8

[    4.583801] [<ffffff80080db394>] kthread+0x100/0x12c

[    4.589029] [<ffffff8008082ec0>] ret_from_fork+0x10/0x50

[    4.594793] ---[ end trace 58af756d1037eb51 ]---

I did some modifications and let code enter the if condition any way, I 
added some log and you can see, rocr is updated, but when it tried to 
switch to 1.8V signal, it failed. I don't know why yet.
So the code will try again, and use high speed to init card again.

[    4.259194] before rocr = c0ff8000, ocr = 41200000

[    4.359909] after rocr = c1ff8000  -- if condition meets, updated rocr

[    4.364230] mmc0: 1.8V regulator output did not became stable  -- 
mmc_set_signal_voltage 1.8V failed.
[    4.372603] mmc0: Skipping voltage switch

[    4.403225] before rocr = c0ff8000, ocr = 40200000   -- tried again 
with no SD_OCR_S18R .
> +       /*
>           * In case CCS and S18A in the response is set, start Signal Voltage
>           * Switch procedure. SPI mode doesn't support CMD11.
>           */
> @@ -788,6 +810,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32
> *cid, u32 *rocr)
>                  }
>          }
>
> +done:
>          err = mmc_send_cid(host, cid);
>          return err;
>   }
> @@ -924,7 +947,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32
> ocr,
>
>          WARN_ON(!host->claimed);
>
> -       err = mmc_sd_get_cid(host, ocr, cid, &rocr);
> +       err = mmc_sd_get_cid(host, oldcard, ocr, cid, &rocr);
>          if (err)
>                  return err;
>
> @@ -995,6 +1018,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32
> ocr,
>                  if (err)
>                          goto free_card;
>          } else {
> +
>                  /*
>                   * Attempt to change to high-speed (if supported)
>                   */
> diff --git a/drivers/mmc/core/sd.h b/drivers/mmc/core/sd.h
> index 1ada980..9cd2482 100644
> --- a/drivers/mmc/core/sd.h
> +++ b/drivers/mmc/core/sd.h
> @@ -8,7 +8,8 @@
>   struct mmc_host;
>   struct mmc_card;
>
> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr);
> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
> +                  u32 ocr, u32 *cid, u32 *rocr);
>   int mmc_sd_get_csd(struct mmc_host *host, struct mmc_card *card);
>   void mmc_decode_cid(struct mmc_card *card);
>   int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index cc43687..08e82fe 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -608,7 +608,7 @@ static int mmc_sdio_init_card(struct mmc_host *host,
> u32 ocr,
>          }
>
>          if ((rocr & R4_MEMORY_PRESENT) &&
> -           mmc_sd_get_cid(host, ocr & rocr, card->raw_cid, NULL) == 0) {
> +           mmc_sd_get_cid(host, NULL, ocr & rocr, card->raw_cid, NULL) ==
> 0) {
>                  card->type = MMC_TYPE_SD_COMBO;
>
>                  if (oldcard && (oldcard->type != MMC_TYPE_SD_COMBO ||
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index e567fa5..edea93b0 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -2522,6 +2522,20 @@ int regulator_is_enabled(struct regulator *regulator)
>   EXPORT_SYMBOL_GPL(regulator_is_enabled);
>
>   /**
> + * regulator_is_always_on - is the regulator output always enabled
> + * @regulator: regulator source
> + *
> + * Returns positive if the regulator driver backing the source/client
> + * has requested that the device be always on, zero if it isn't.
> + *
> + */
> +inline int regulator_is_always_on(struct regulator *regulator)
> +{
> +       return (regulator->always_on) ? 1 : 0;
> +}
> +EXPORT_SYMBOL_GPL(regulator_is_always_on);
> +
> +/**
>    * regulator_count_voltages - count regulator_list_voltage() selectors
>    * @regulator: regulator source
>    *
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index ebd1ceb..2f1206b 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -462,6 +462,7 @@ static inline int mmc_regulator_set_vqmmc(struct
> mmc_host *mmc,
>
>   u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
>   int mmc_regulator_get_supply(struct mmc_host *mmc);
> +bool mmc_regulator_can_power_off(struct regulator *supply);
>
>   static inline int mmc_card_is_removable(struct mmc_host *host)
>   {
> diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/
> consumer.h
> index df176d7..46126e3 100644
> --- a/include/linux/regulator/consumer.h
> +++ b/include/linux/regulator/consumer.h
> @@ -226,6 +226,7 @@ void devm_regulator_bulk_unregister_supply_alias(struct
> device *dev,
>   int regulator_disable(struct regulator *regulator);
>   int regulator_force_disable(struct regulator *regulator);
>   int regulator_is_enabled(struct regulator *regulator);
> +int regulator_is_always_on(struct regulator *regulator);
>   int regulator_disable_deferred(struct regulator *regulator, int ms);
>
>   int __must_check regulator_bulk_get(struct device *dev, int num_consumers,
> @@ -418,6 +419,11 @@ static inline int regulator_is_enabled(struct
> regulator *regulator)
>          return 1;
>   }
>
> +static inline int regulator_is_always_on(struct regulator *regulator)
> +{
> +       return 0;
> +}
> +
>   static inline int regulator_bulk_get(struct device *dev,
>                                       int num_consumers,
>                                       struct regulator_bulk_data *consumers)
> --
> 1.9.1
>
>
> --
> 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
Shawn Lin Aug. 30, 2017, 12:14 a.m. UTC | #2
On 2017/8/30 6:13, Zhoujie Wu wrote:
> Hi Shawn,
> thanks for your patch.
> 
> On 08/29/2017 02:56 PM, zhoujie wu wrote:
>>
>>
>> CMD0 couldn't put the card back to 3.3v if can't do power
>> cycle for cards. The card is already in SDR12 then, so still
>> try to enable UHS mode.
>>
>> Pleas note that, if the platform use a hardware fixed
>> sd power, you should still include a regulator-fixed
>> regulator with always-on property there. So mmc core
>> could handle this as well. Otherwise you shouldn't
>> expect UHS to work for that case.
>>
>> Just compile only, please have a look and test.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>>   drivers/mmc/core/card.h            | 17 +++++++++++++++++
>>   drivers/mmc/core/core.c            |  8 ++++++++
>>   drivers/mmc/core/sd.c              | 28 ++++++++++++++++++++++++++--
>>   drivers/mmc/core/sd.h              |  3 ++-
>>   drivers/mmc/core/sdio.c            |  2 +-
>>   drivers/regulator/core.c           | 14 ++++++++++++++
>>   include/linux/mmc/host.h           |  1 +
>>   include/linux/regulator/consumer.h |  6 ++++++
>>   8 files changed, 75 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
>> index f06cd91..a562110 100644
>> --- a/drivers/mmc/core/card.h
>> +++ b/drivers/mmc/core/card.h
>> @@ -218,4 +218,21 @@ static inline int mmc_card_broken_hpi(const struct
>> mmc_card *c)
>>          return c->quirks & MMC_QUIRK_BROKEN_HPI;
>>   }
>>
>> +static inline bool mmc_card_support_uhs(struct mmc_card *card)
>> +{
>> +       if (WARN_ON(!card))
>> +               return false;
>> +
>> +       switch (card->sd_bus_speed) {
>> +       case UHS_SDR104_BUS_SPEED:
>> +       case UHS_DDR50_BUS_SPEED:
>> +       case UHS_SDR50_BUS_SPEED:
>> +       case UHS_SDR25_BUS_SPEED:
>> +       case UHS_SDR12_BUS_SPEED:
>> +               return true;
>> +       default:
>> +               return false;
>> +       }
>> +}
>> +
>>   #endif
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 2643126..6b5c6c5 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -1424,6 +1424,14 @@ int mmc_regulator_get_supply(struct mmc_host *mmc)
>>   }
>>   EXPORT_SYMBOL_GPL(mmc_regulator_get_supply);
>>
>> +bool mmc_regulator_can_power_off(struct regulator *supply)
>> +{
>> +       if (!IS_ERR(supply))
>> +               return regulator_is_always_on(supply);
>> +       return false;
>> +}
>> +EXPORT_SYMBOL_GPL(mmc_regulator_can_power_off);
>> +
>>   /*
>>    * Mask off any voltages we don't support and select
>>    * the lowest voltage
>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>> index a1b0aa1..7bec61b 100644
>> --- a/drivers/mmc/core/sd.c
>> +++ b/drivers/mmc/core/sd.c
>> @@ -721,7 +721,8 @@ struct device_type sd_type = {
>>   /*
>>    * Fetch CID from card.
>>    */
>> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
>> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
>> +                                  u32 ocr, u32 *cid, u32 *rocr)
>>   {
>>          int err;
>>          u32 max_current;
>> @@ -773,6 +774,27 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 
>> ocr, u32
>> *cid, u32 *rocr)
>>                  return err;
>>
>>          /*
>> +        * Per SD physical specification version 4.10, section
>> +        * 3.9.4 UHS-I Bus Speed Mode Selection Sequence, once
>> +        * the signal voltage is switched to 1V8, the card continues
>> +        * 1.8V signaling regardless of CMD0. Power cycle resets the
>> +        * signal voltage to 3.3V. As a fact that some vmmc is always
>> +        * on so the card would still work with SDR12 after CMD0. So
>> +        * let's attempt to enable enable UHS mode if vmmc can't be
>> +        * powered off.
>> +        */
>> +       if (card && mmc_card_support_uhs(card) && mmc_host_uhs(host) &&
>> +           !mmc_regulator_can_power_off(host->supply.vmmc) &&
>> +           (ocr & SD_OCR_S18R)) {
>> +                       *rocr |= SD_ROCR_S18A;
>> +                       if (mmc_set_signal_voltage(host,
>> MMC_SIGNAL_VOLTAGE_180)) {
>> +                               retries = 0;
>> +                               goto try_again;
>> +                       }
>> +
>> +                       goto done;
>> +       }
> Good to have a higher level solution if the issue is common.
> I did basic test with your patch(added your regulator modification you 
> sent to me) and add vmmc-supply in board dts, I saw warning when boot up.
> I think in below case, the card is not added yet, the card is null and 
> print out the warning. I don't quite understand mmc_card_support_uhs, it 
> return true for all card speed.
> 
> [    4.310003] WARNING: CPU: 1 PID: 48 at drivers/mmc/core/card.h:224 
> mmc_sd_get_cid+0x270/0x298

it's just for debug, and won't be a big deal.

And I find mmc_host_uhs here is redundant as oldcard's
card->sd_bus_speed is already the result of checking uhs cap for both of
controller and card. So mmc_card_support_uhs return true means both
of your controller and the cards support UHS mode, didn't they?

> 
> .....
> 
> [    4.549657] [<ffffff8008757e24>] mmc_sd_get_cid+0x270/0x298
> 
> [    4.555510] [<ffffff800875847c>] mmc_sd_init_card+0x44/0x614
> 
> [    4.561456] [<ffffff8008758bd4>] mmc_attach_sd+0x7c/0x134
> 
> [    4.566951] [<ffffff8008751fb8>] mmc_rescan+0x2e4/0x35c
> 
> [    4.572451] [<ffffff80080d58d8>] process_one_work+0x12c/0x28c
> 
> [    4.578304] [<ffffff80080d5a90>] worker_thread+0x58/0x3b8
> 
> [    4.583801] [<ffffff80080db394>] kthread+0x100/0x12c
> 
> [    4.589029] [<ffffff8008082ec0>] ret_from_fork+0x10/0x50
> 
> [    4.594793] ---[ end trace 58af756d1037eb51 ]---
> 
> I did some modifications and let code enter the if condition any way, I 
> added some log and you can see, rocr is updated, but when it tried to 
> switch to 1.8V signal, it failed. I don't know why yet.

Ok, sounds the key point is to see why it fails to switch to 1.8v signal

> So the code will try again, and use high speed to init card again.

Thanks for testing this, but I haven' had enough time to put my hand
on it. But I will be back and check it out by myself.. Anyway you may
still be able to do more debugging if this approach looks good to you:)

> 
> [    4.259194] before rocr = c0ff8000, ocr = 41200000
> 
> [    4.359909] after rocr = c1ff8000  -- if condition meets, updated rocr
> 
> [    4.364230] mmc0: 1.8V regulator output did not became stable  -- 
> mmc_set_signal_voltage 1.8V failed.
> [    4.372603] mmc0: Skipping voltage switch
> 
> [    4.403225] before rocr = c0ff8000, ocr = 40200000   -- tried again 
> with no SD_OCR_S18R .
>> +       /*
>>           * In case CCS and S18A in the response is set, start Signal 
>> Voltage
>>           * Switch procedure. SPI mode doesn't support CMD11.
>>           */
>> @@ -788,6 +810,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, 
>> u32
>> *cid, u32 *rocr)
>>                  }
>>          }
>>
>> +done:
>>          err = mmc_send_cid(host, cid);
>>          return err;
>>   }
>> @@ -924,7 +947,7 @@ static int mmc_sd_init_card(struct mmc_host *host, 
>> u32
>> ocr,
>>
>>          WARN_ON(!host->claimed);
>>
>> -       err = mmc_sd_get_cid(host, ocr, cid, &rocr);
>> +       err = mmc_sd_get_cid(host, oldcard, ocr, cid, &rocr);
>>          if (err)
>>                  return err;
>>
>> @@ -995,6 +1018,7 @@ static int mmc_sd_init_card(struct mmc_host 
>> *host, u32
>> ocr,
>>                  if (err)
>>                          goto free_card;
>>          } else {
>> +
>>                  /*
>>                   * Attempt to change to high-speed (if supported)
>>                   */
>> diff --git a/drivers/mmc/core/sd.h b/drivers/mmc/core/sd.h
>> index 1ada980..9cd2482 100644
>> --- a/drivers/mmc/core/sd.h
>> +++ b/drivers/mmc/core/sd.h
>> @@ -8,7 +8,8 @@
>>   struct mmc_host;
>>   struct mmc_card;
>>
>> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr);
>> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
>> +                  u32 ocr, u32 *cid, u32 *rocr);
>>   int mmc_sd_get_csd(struct mmc_host *host, struct mmc_card *card);
>>   void mmc_decode_cid(struct mmc_card *card);
>>   int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>> index cc43687..08e82fe 100644
>> --- a/drivers/mmc/core/sdio.c
>> +++ b/drivers/mmc/core/sdio.c
>> @@ -608,7 +608,7 @@ static int mmc_sdio_init_card(struct mmc_host *host,
>> u32 ocr,
>>          }
>>
>>          if ((rocr & R4_MEMORY_PRESENT) &&
>> -           mmc_sd_get_cid(host, ocr & rocr, card->raw_cid, NULL) == 0) {
>> +           mmc_sd_get_cid(host, NULL, ocr & rocr, card->raw_cid, 
>> NULL) ==
>> 0) {
>>                  card->type = MMC_TYPE_SD_COMBO;
>>
>>                  if (oldcard && (oldcard->type != MMC_TYPE_SD_COMBO ||
>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>> index e567fa5..edea93b0 100644
>> --- a/drivers/regulator/core.c
>> +++ b/drivers/regulator/core.c
>> @@ -2522,6 +2522,20 @@ int regulator_is_enabled(struct regulator 
>> *regulator)
>>   EXPORT_SYMBOL_GPL(regulator_is_enabled);
>>
>>   /**
>> + * regulator_is_always_on - is the regulator output always enabled
>> + * @regulator: regulator source
>> + *
>> + * Returns positive if the regulator driver backing the source/client
>> + * has requested that the device be always on, zero if it isn't.
>> + *
>> + */
>> +inline int regulator_is_always_on(struct regulator *regulator)
>> +{
>> +       return (regulator->always_on) ? 1 : 0;
>> +}
>> +EXPORT_SYMBOL_GPL(regulator_is_always_on);
>> +
>> +/**
>>    * regulator_count_voltages - count regulator_list_voltage() selectors
>>    * @regulator: regulator source
>>    *
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index ebd1ceb..2f1206b 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -462,6 +462,7 @@ static inline int mmc_regulator_set_vqmmc(struct
>> mmc_host *mmc,
>>
>>   u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
>>   int mmc_regulator_get_supply(struct mmc_host *mmc);
>> +bool mmc_regulator_can_power_off(struct regulator *supply);
>>
>>   static inline int mmc_card_is_removable(struct mmc_host *host)
>>   {
>> diff --git a/include/linux/regulator/consumer.h 
>> b/include/linux/regulator/
>> consumer.h
>> index df176d7..46126e3 100644
>> --- a/include/linux/regulator/consumer.h
>> +++ b/include/linux/regulator/consumer.h
>> @@ -226,6 +226,7 @@ void 
>> devm_regulator_bulk_unregister_supply_alias(struct
>> device *dev,
>>   int regulator_disable(struct regulator *regulator);
>>   int regulator_force_disable(struct regulator *regulator);
>>   int regulator_is_enabled(struct regulator *regulator);
>> +int regulator_is_always_on(struct regulator *regulator);
>>   int regulator_disable_deferred(struct regulator *regulator, int ms);
>>
>>   int __must_check regulator_bulk_get(struct device *dev, int 
>> num_consumers,
>> @@ -418,6 +419,11 @@ static inline int regulator_is_enabled(struct
>> regulator *regulator)
>>          return 1;
>>   }
>>
>> +static inline int regulator_is_always_on(struct regulator *regulator)
>> +{
>> +       return 0;
>> +}
>> +
>>   static inline int regulator_bulk_get(struct device *dev,
>>                                       int num_consumers,
>>                                       struct regulator_bulk_data 
>> *consumers)
>> -- 
>> 1.9.1
>>
>>
>> -- 
>> 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
Zhoujie Wu Aug. 30, 2017, 10:32 p.m. UTC | #3
On 08/29/2017 05:14 PM, Shawn Lin wrote:
> On 2017/8/30 6:13, Zhoujie Wu wrote:
>> Hi Shawn,
>> thanks for your patch.
>>
>> On 08/29/2017 02:56 PM, zhoujie wu wrote:
>>>
>>>
>>> CMD0 couldn't put the card back to 3.3v if can't do power
>>> cycle for cards. The card is already in SDR12 then, so still
>>> try to enable UHS mode.
>>>
>>> Pleas note that, if the platform use a hardware fixed
>>> sd power, you should still include a regulator-fixed
>>> regulator with always-on property there. So mmc core
>>> could handle this as well. Otherwise you shouldn't
>>> expect UHS to work for that case.
>>>
>>> Just compile only, please have a look and test.
>>>
>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>
>>> ---
>>>
>>>   drivers/mmc/core/card.h            | 17 +++++++++++++++++
>>>   drivers/mmc/core/core.c            |  8 ++++++++
>>>   drivers/mmc/core/sd.c              | 28 ++++++++++++++++++++++++++--
>>>   drivers/mmc/core/sd.h              |  3 ++-
>>>   drivers/mmc/core/sdio.c            |  2 +-
>>>   drivers/regulator/core.c           | 14 ++++++++++++++
>>>   include/linux/mmc/host.h           |  1 +
>>>   include/linux/regulator/consumer.h |  6 ++++++
>>>   8 files changed, 75 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
>>> index f06cd91..a562110 100644
>>> --- a/drivers/mmc/core/card.h
>>> +++ b/drivers/mmc/core/card.h
>>> @@ -218,4 +218,21 @@ static inline int mmc_card_broken_hpi(const struct
>>> mmc_card *c)
>>>          return c->quirks & MMC_QUIRK_BROKEN_HPI;
>>>   }
>>>
>>> +static inline bool mmc_card_support_uhs(struct mmc_card *card)
>>> +{
>>> +       if (WARN_ON(!card))
>>> +               return false;
>>> +
>>> +       switch (card->sd_bus_speed) {
>>> +       case UHS_SDR104_BUS_SPEED:
>>> +       case UHS_DDR50_BUS_SPEED:
>>> +       case UHS_SDR50_BUS_SPEED:
>>> +       case UHS_SDR25_BUS_SPEED:
>>> +       case UHS_SDR12_BUS_SPEED:
>>> +               return true;
>>> +       default:
>>> +               return false;
>>> +       }
>>> +}
>>> +
>>>   #endif
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index 2643126..6b5c6c5 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -1424,6 +1424,14 @@ int mmc_regulator_get_supply(struct mmc_host 
>>> *mmc)
>>>   }
>>>   EXPORT_SYMBOL_GPL(mmc_regulator_get_supply);
>>>
>>> +bool mmc_regulator_can_power_off(struct regulator *supply)
>>> +{
>>> +       if (!IS_ERR(supply))
>>> +               return regulator_is_always_on(supply);
>>> +       return false;
>>> +}
>>> +EXPORT_SYMBOL_GPL(mmc_regulator_can_power_off);
>>> +
>>>   /*
>>>    * Mask off any voltages we don't support and select
>>>    * the lowest voltage
>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>> index a1b0aa1..7bec61b 100644
>>> --- a/drivers/mmc/core/sd.c
>>> +++ b/drivers/mmc/core/sd.c
>>> @@ -721,7 +721,8 @@ struct device_type sd_type = {
>>>   /*
>>>    * Fetch CID from card.
>>>    */
>>> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 
>>> *rocr)
>>> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
>>> +                                  u32 ocr, u32 *cid, u32 *rocr)
>>>   {
>>>          int err;
>>>          u32 max_current;
>>> @@ -773,6 +774,27 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 
>>> ocr, u32
>>> *cid, u32 *rocr)
>>>                  return err;
>>>
>>>          /*
>>> +        * Per SD physical specification version 4.10, section
>>> +        * 3.9.4 UHS-I Bus Speed Mode Selection Sequence, once
>>> +        * the signal voltage is switched to 1V8, the card continues
>>> +        * 1.8V signaling regardless of CMD0. Power cycle resets the
>>> +        * signal voltage to 3.3V. As a fact that some vmmc is always
>>> +        * on so the card would still work with SDR12 after CMD0. So
>>> +        * let's attempt to enable enable UHS mode if vmmc can't be
>>> +        * powered off.
>>> +        */
>>> +       if (card && mmc_card_support_uhs(card) && mmc_host_uhs(host) &&
>>> +           !mmc_regulator_can_power_off(host->supply.vmmc) &&
>>> +           (ocr & SD_OCR_S18R)) {
>>> +                       *rocr |= SD_ROCR_S18A;
>>> +                       if (mmc_set_signal_voltage(host,
>>> MMC_SIGNAL_VOLTAGE_180)) {
>>> +                               retries = 0;
>>> +                               goto try_again;
>>> +                       }
>>> +
>>> +                       goto done;
>>> +       }
>> Good to have a higher level solution if the issue is common.
>> I did basic test with your patch(added your regulator modification 
>> you sent to me) and add vmmc-supply in board dts, I saw warning when 
>> boot up.
>> I think in below case, the card is not added yet, the card is null 
>> and print out the warning. I don't quite understand 
>> mmc_card_support_uhs, it return true for all card speed.
>>
>> [    4.310003] WARNING: CPU: 1 PID: 48 at drivers/mmc/core/card.h:224 
>> mmc_sd_get_cid+0x270/0x298
>
> it's just for debug, and won't be a big deal.
>
> And I find mmc_host_uhs here is redundant as oldcard's
> card->sd_bus_speed is already the result of checking uhs cap for both of
> controller and card. So mmc_card_support_uhs return true means both
> of your controller and the cards support UHS mode, didn't they?
>
>>
>> .....
>>
>> [    4.549657] [<ffffff8008757e24>] mmc_sd_get_cid+0x270/0x298
>>
>> [    4.555510] [<ffffff800875847c>] mmc_sd_init_card+0x44/0x614
>>
>> [    4.561456] [<ffffff8008758bd4>] mmc_attach_sd+0x7c/0x134
>>
>> [    4.566951] [<ffffff8008751fb8>] mmc_rescan+0x2e4/0x35c
>>
>> [    4.572451] [<ffffff80080d58d8>] process_one_work+0x12c/0x28c
>>
>> [    4.578304] [<ffffff80080d5a90>] worker_thread+0x58/0x3b8
>>
>> [    4.583801] [<ffffff80080db394>] kthread+0x100/0x12c
>>
>> [    4.589029] [<ffffff8008082ec0>] ret_from_fork+0x10/0x50
>>
>> [    4.594793] ---[ end trace 58af756d1037eb51 ]---
>>
>> I did some modifications and let code enter the if condition any way, 
>> I added some log and you can see, rocr is updated, but when it tried 
>> to switch to 1.8V signal, it failed. I don't know why yet.
>
> Ok, sounds the key point is to see why it fails to switch to 1.8v signal
>
>> So the code will try again, and use high speed to init card again.
>
> Thanks for testing this, but I haven' had enough time to put my hand
> on it. But I will be back and check it out by myself.. Anyway you may
> still be able to do more debugging if this approach looks good to you:)

The solution is ok for me, I will check more details later. thanks.
>
>>
>> [    4.259194] before rocr = c0ff8000, ocr = 41200000
>>
>> [    4.359909] after rocr = c1ff8000  -- if condition meets, updated 
>> rocr
>>
>> [    4.364230] mmc0: 1.8V regulator output did not became stable  -- 
>> mmc_set_signal_voltage 1.8V failed.
>> [    4.372603] mmc0: Skipping voltage switch
>>
>> [    4.403225] before rocr = c0ff8000, ocr = 40200000   -- tried 
>> again with no SD_OCR_S18R .
>>> +       /*
>>>           * In case CCS and S18A in the response is set, start 
>>> Signal Voltage
>>>           * Switch procedure. SPI mode doesn't support CMD11.
>>>           */
>>> @@ -788,6 +810,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 
>>> ocr, u32
>>> *cid, u32 *rocr)
>>>                  }
>>>          }
>>>
>>> +done:
>>>          err = mmc_send_cid(host, cid);
>>>          return err;
>>>   }
>>> @@ -924,7 +947,7 @@ static int mmc_sd_init_card(struct mmc_host 
>>> *host, u32
>>> ocr,
>>>
>>>          WARN_ON(!host->claimed);
>>>
>>> -       err = mmc_sd_get_cid(host, ocr, cid, &rocr);
>>> +       err = mmc_sd_get_cid(host, oldcard, ocr, cid, &rocr);
>>>          if (err)
>>>                  return err;
>>>
>>> @@ -995,6 +1018,7 @@ static int mmc_sd_init_card(struct mmc_host 
>>> *host, u32
>>> ocr,
>>>                  if (err)
>>>                          goto free_card;
>>>          } else {
>>> +
>>>                  /*
>>>                   * Attempt to change to high-speed (if supported)
>>>                   */
>>> diff --git a/drivers/mmc/core/sd.h b/drivers/mmc/core/sd.h
>>> index 1ada980..9cd2482 100644
>>> --- a/drivers/mmc/core/sd.h
>>> +++ b/drivers/mmc/core/sd.h
>>> @@ -8,7 +8,8 @@
>>>   struct mmc_host;
>>>   struct mmc_card;
>>>
>>> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 
>>> *rocr);
>>> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
>>> +                  u32 ocr, u32 *cid, u32 *rocr);
>>>   int mmc_sd_get_csd(struct mmc_host *host, struct mmc_card *card);
>>>   void mmc_decode_cid(struct mmc_card *card);
>>>   int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>> index cc43687..08e82fe 100644
>>> --- a/drivers/mmc/core/sdio.c
>>> +++ b/drivers/mmc/core/sdio.c
>>> @@ -608,7 +608,7 @@ static int mmc_sdio_init_card(struct mmc_host 
>>> *host,
>>> u32 ocr,
>>>          }
>>>
>>>          if ((rocr & R4_MEMORY_PRESENT) &&
>>> -           mmc_sd_get_cid(host, ocr & rocr, card->raw_cid, NULL) == 
>>> 0) {
>>> +           mmc_sd_get_cid(host, NULL, ocr & rocr, card->raw_cid, 
>>> NULL) ==
>>> 0) {
>>>                  card->type = MMC_TYPE_SD_COMBO;
>>>
>>>                  if (oldcard && (oldcard->type != MMC_TYPE_SD_COMBO ||
>>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>>> index e567fa5..edea93b0 100644
>>> --- a/drivers/regulator/core.c
>>> +++ b/drivers/regulator/core.c
>>> @@ -2522,6 +2522,20 @@ int regulator_is_enabled(struct regulator 
>>> *regulator)
>>>   EXPORT_SYMBOL_GPL(regulator_is_enabled);
>>>
>>>   /**
>>> + * regulator_is_always_on - is the regulator output always enabled
>>> + * @regulator: regulator source
>>> + *
>>> + * Returns positive if the regulator driver backing the source/client
>>> + * has requested that the device be always on, zero if it isn't.
>>> + *
>>> + */
>>> +inline int regulator_is_always_on(struct regulator *regulator)
>>> +{
>>> +       return (regulator->always_on) ? 1 : 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(regulator_is_always_on);
>>> +
>>> +/**
>>>    * regulator_count_voltages - count regulator_list_voltage() 
>>> selectors
>>>    * @regulator: regulator source
>>>    *
>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>> index ebd1ceb..2f1206b 100644
>>> --- a/include/linux/mmc/host.h
>>> +++ b/include/linux/mmc/host.h
>>> @@ -462,6 +462,7 @@ static inline int mmc_regulator_set_vqmmc(struct
>>> mmc_host *mmc,
>>>
>>>   u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
>>>   int mmc_regulator_get_supply(struct mmc_host *mmc);
>>> +bool mmc_regulator_can_power_off(struct regulator *supply);
>>>
>>>   static inline int mmc_card_is_removable(struct mmc_host *host)
>>>   {
>>> diff --git a/include/linux/regulator/consumer.h 
>>> b/include/linux/regulator/
>>> consumer.h
>>> index df176d7..46126e3 100644
>>> --- a/include/linux/regulator/consumer.h
>>> +++ b/include/linux/regulator/consumer.h
>>> @@ -226,6 +226,7 @@ void 
>>> devm_regulator_bulk_unregister_supply_alias(struct
>>> device *dev,
>>>   int regulator_disable(struct regulator *regulator);
>>>   int regulator_force_disable(struct regulator *regulator);
>>>   int regulator_is_enabled(struct regulator *regulator);
>>> +int regulator_is_always_on(struct regulator *regulator);
>>>   int regulator_disable_deferred(struct regulator *regulator, int ms);
>>>
>>>   int __must_check regulator_bulk_get(struct device *dev, int 
>>> num_consumers,
>>> @@ -418,6 +419,11 @@ static inline int regulator_is_enabled(struct
>>> regulator *regulator)
>>>          return 1;
>>>   }
>>>
>>> +static inline int regulator_is_always_on(struct regulator *regulator)
>>> +{
>>> +       return 0;
>>> +}
>>> +
>>>   static inline int regulator_bulk_get(struct device *dev,
>>>                                       int num_consumers,
>>>                                       struct regulator_bulk_data 
>>> *consumers)
>>> -- 
>>> 1.9.1
>>>
>>>
>>> -- 
>>> 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
Zhoujie Wu Sept. 1, 2017, 10:57 p.m. UTC | #4
Hi Shawn,

On 08/29/2017 05:14 PM, Shawn Lin wrote:
> On 2017/8/30 6:13, Zhoujie Wu wrote:
>> Hi Shawn,
>> thanks for your patch.
>>
>> On 08/29/2017 02:56 PM, zhoujie wu wrote:
>>>
>>>
>>> CMD0 couldn't put the card back to 3.3v if can't do power
>>> cycle for cards. The card is already in SDR12 then, so still
>>> try to enable UHS mode.
>>>
>>> Pleas note that, if the platform use a hardware fixed
>>> sd power, you should still include a regulator-fixed
>>> regulator with always-on property there. So mmc core
>>> could handle this as well. Otherwise you shouldn't
>>> expect UHS to work for that case.
>>>
>>> Just compile only, please have a look and test.
>>>
>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>
>>> ---
>>>
>>>   drivers/mmc/core/card.h            | 17 +++++++++++++++++
>>>   drivers/mmc/core/core.c            |  8 ++++++++
>>>   drivers/mmc/core/sd.c              | 28 ++++++++++++++++++++++++++--
>>>   drivers/mmc/core/sd.h              |  3 ++-
>>>   drivers/mmc/core/sdio.c            |  2 +-
>>>   drivers/regulator/core.c           | 14 ++++++++++++++
>>>   include/linux/mmc/host.h           |  1 +
>>>   include/linux/regulator/consumer.h |  6 ++++++
>>>   8 files changed, 75 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
>>> index f06cd91..a562110 100644
>>> --- a/drivers/mmc/core/card.h
>>> +++ b/drivers/mmc/core/card.h
>>> @@ -218,4 +218,21 @@ static inline int mmc_card_broken_hpi(const struct
>>> mmc_card *c)
>>>          return c->quirks & MMC_QUIRK_BROKEN_HPI;
>>>   }
>>>
>>> +static inline bool mmc_card_support_uhs(struct mmc_card *card)
>>> +{
>>> +       if (WARN_ON(!card))
>>> +               return false;
>>> +
>>> +       switch (card->sd_bus_speed) {
>>> +       case UHS_SDR104_BUS_SPEED:
>>> +       case UHS_DDR50_BUS_SPEED:
>>> +       case UHS_SDR50_BUS_SPEED:
>>> +       case UHS_SDR25_BUS_SPEED:
>>> +       case UHS_SDR12_BUS_SPEED:
>>> +               return true;
>>> +       default:
>>> +               return false;
>>> +       }
>>> +}
>>> +
>>>   #endif
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index 2643126..6b5c6c5 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -1424,6 +1424,14 @@ int mmc_regulator_get_supply(struct mmc_host 
>>> *mmc)
>>>   }
>>>   EXPORT_SYMBOL_GPL(mmc_regulator_get_supply);
>>>
>>> +bool mmc_regulator_can_power_off(struct regulator *supply)
>>> +{
>>> +       if (!IS_ERR(supply))
>>> +               return regulator_is_always_on(supply);
>>> +       return false;
>>> +}
>>> +EXPORT_SYMBOL_GPL(mmc_regulator_can_power_off);
>>> +
>>>   /*
>>>    * Mask off any voltages we don't support and select
>>>    * the lowest voltage
>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>> index a1b0aa1..7bec61b 100644
>>> --- a/drivers/mmc/core/sd.c
>>> +++ b/drivers/mmc/core/sd.c
>>> @@ -721,7 +721,8 @@ struct device_type sd_type = {
>>>   /*
>>>    * Fetch CID from card.
>>>    */
>>> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 
>>> *rocr)
>>> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
>>> +                                  u32 ocr, u32 *cid, u32 *rocr)
>>>   {
>>>          int err;
>>>          u32 max_current;
>>> @@ -773,6 +774,27 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 
>>> ocr, u32
>>> *cid, u32 *rocr)
>>>                  return err;
>>>
>>>          /*
>>> +        * Per SD physical specification version 4.10, section
>>> +        * 3.9.4 UHS-I Bus Speed Mode Selection Sequence, once
>>> +        * the signal voltage is switched to 1V8, the card continues
>>> +        * 1.8V signaling regardless of CMD0. Power cycle resets the
>>> +        * signal voltage to 3.3V. As a fact that some vmmc is always
>>> +        * on so the card would still work with SDR12 after CMD0. So
>>> +        * let's attempt to enable enable UHS mode if vmmc can't be
>>> +        * powered off.
>>> +        */
>>> +       if (card && mmc_card_support_uhs(card) && mmc_host_uhs(host) &&
>>> +           !mmc_regulator_can_power_off(host->supply.vmmc) &&
>>> +           (ocr & SD_OCR_S18R)) {
>>> +                       *rocr |= SD_ROCR_S18A;
>>> +                       if (mmc_set_signal_voltage(host,
>>> MMC_SIGNAL_VOLTAGE_180)) {
>>> +                               retries = 0;
>>> +                               goto try_again;
>>> +                       }
>>> +
>>> +                       goto done;
>>> +       }
>> Good to have a higher level solution if the issue is common.
>> I did basic test with your patch(added your regulator modification 
>> you sent to me) and add vmmc-supply in board dts, I saw warning when 
>> boot up.
>> I think in below case, the card is not added yet, the card is null 
>> and print out the warning. I don't quite understand 
>> mmc_card_support_uhs, it return true for all card speed.
>>
>> [    4.310003] WARNING: CPU: 1 PID: 48 at drivers/mmc/core/card.h:224 
>> mmc_sd_get_cid+0x270/0x298
>
> it's just for debug, and won't be a big deal.
>
> And I find mmc_host_uhs here is redundant as oldcard's
> card->sd_bus_speed is already the result of checking uhs cap for both of
> controller and card. So mmc_card_support_uhs return true means both
> of your controller and the cards support UHS mode, didn't they?
>
>>
>> .....
>>
>> [    4.549657] [<ffffff8008757e24>] mmc_sd_get_cid+0x270/0x298
>>
>> [    4.555510] [<ffffff800875847c>] mmc_sd_init_card+0x44/0x614
>>
>> [    4.561456] [<ffffff8008758bd4>] mmc_attach_sd+0x7c/0x134
>>
>> [    4.566951] [<ffffff8008751fb8>] mmc_rescan+0x2e4/0x35c
>>
>> [    4.572451] [<ffffff80080d58d8>] process_one_work+0x12c/0x28c
>>
>> [    4.578304] [<ffffff80080d5a90>] worker_thread+0x58/0x3b8
>>
>> [    4.583801] [<ffffff80080db394>] kthread+0x100/0x12c
>>
>> [    4.589029] [<ffffff8008082ec0>] ret_from_fork+0x10/0x50
>>
>> [    4.594793] ---[ end trace 58af756d1037eb51 ]---
>>
>> I did some modifications and let code enter the if condition any way, 
>> I added some log and you can see, rocr is updated, but when it tried 
>> to switch to 1.8V signal, it failed. I don't know why yet.
>
> Ok, sounds the key point is to see why it fails to switch to 1.8v signal
>
>> So the code will try again, and use high speed to init card again.
>
> Thanks for testing this, but I haven' had enough time to put my hand
> on it. But I will be back and check it out by myself.. Anyway you may
> still be able to do more debugging if this approach looks good to you:)

I have checked why 1.8V signal switch failed here, as SD spec mentioned 
it needs to gate the clock for a while when switching the voltage, I 
added some logic to gate the clock before switch signal,
then re-enable the clock after that, just like what set_uhs_voltage does 
and it could work now.
and I adjust the if condition logic as below, hacked 
mmc_card_support_uhs now always return true.
This can be a fix, but to be a generic solution,  I am wondering from sd 
card perspective, except for no power cycle, if there is any other 
reason that may cause SD_OCR_S18A not set for uhs card?

+       if (mmc_card_support_uhs(card)
+           !mmc_regulator_can_power_off(host->supply.vmmc) &&
+           !(*rocr & SD_OCR_S18A))
>
>>
>> [    4.259194] before rocr = c0ff8000, ocr = 41200000
>>
>> [    4.359909] after rocr = c1ff8000  -- if condition meets, updated 
>> rocr
>>
>> [    4.364230] mmc0: 1.8V regulator output did not became stable  -- 
>> mmc_set_signal_voltage 1.8V failed.
>> [    4.372603] mmc0: Skipping voltage switch
>>
>> [    4.403225] before rocr = c0ff8000, ocr = 40200000   -- tried 
>> again with no SD_OCR_S18R .
>>> +       /*
>>>           * In case CCS and S18A in the response is set, start 
>>> Signal Voltage
>>>           * Switch procedure. SPI mode doesn't support CMD11.
>>>           */
>>> @@ -788,6 +810,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 
>>> ocr, u32
>>> *cid, u32 *rocr)
>>>                  }
>>>          }
>>>
>>> +done:
>>>          err = mmc_send_cid(host, cid);
>>>          return err;
>>>   }
>>> @@ -924,7 +947,7 @@ static int mmc_sd_init_card(struct mmc_host 
>>> *host, u32
>>> ocr,
>>>
>>>          WARN_ON(!host->claimed);
>>>
>>> -       err = mmc_sd_get_cid(host, ocr, cid, &rocr);
>>> +       err = mmc_sd_get_cid(host, oldcard, ocr, cid, &rocr);
>>>          if (err)
>>>                  return err;
>>>
>>> @@ -995,6 +1018,7 @@ static int mmc_sd_init_card(struct mmc_host 
>>> *host, u32
>>> ocr,
>>>                  if (err)
>>>                          goto free_card;
>>>          } else {
>>> +
>>>                  /*
>>>                   * Attempt to change to high-speed (if supported)
>>>                   */
>>> diff --git a/drivers/mmc/core/sd.h b/drivers/mmc/core/sd.h
>>> index 1ada980..9cd2482 100644
>>> --- a/drivers/mmc/core/sd.h
>>> +++ b/drivers/mmc/core/sd.h
>>> @@ -8,7 +8,8 @@
>>>   struct mmc_host;
>>>   struct mmc_card;
>>>
>>> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 
>>> *rocr);
>>> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
>>> +                  u32 ocr, u32 *cid, u32 *rocr);
>>>   int mmc_sd_get_csd(struct mmc_host *host, struct mmc_card *card);
>>>   void mmc_decode_cid(struct mmc_card *card);
>>>   int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>> index cc43687..08e82fe 100644
>>> --- a/drivers/mmc/core/sdio.c
>>> +++ b/drivers/mmc/core/sdio.c
>>> @@ -608,7 +608,7 @@ static int mmc_sdio_init_card(struct mmc_host 
>>> *host,
>>> u32 ocr,
>>>          }
>>>
>>>          if ((rocr & R4_MEMORY_PRESENT) &&
>>> -           mmc_sd_get_cid(host, ocr & rocr, card->raw_cid, NULL) == 
>>> 0) {
>>> +           mmc_sd_get_cid(host, NULL, ocr & rocr, card->raw_cid, 
>>> NULL) ==
>>> 0) {
>>>                  card->type = MMC_TYPE_SD_COMBO;
>>>
>>>                  if (oldcard && (oldcard->type != MMC_TYPE_SD_COMBO ||
>>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>>> index e567fa5..edea93b0 100644
>>> --- a/drivers/regulator/core.c
>>> +++ b/drivers/regulator/core.c
>>> @@ -2522,6 +2522,20 @@ int regulator_is_enabled(struct regulator 
>>> *regulator)
>>>   EXPORT_SYMBOL_GPL(regulator_is_enabled);
>>>
>>>   /**
>>> + * regulator_is_always_on - is the regulator output always enabled
>>> + * @regulator: regulator source
>>> + *
>>> + * Returns positive if the regulator driver backing the source/client
>>> + * has requested that the device be always on, zero if it isn't.
>>> + *
>>> + */
>>> +inline int regulator_is_always_on(struct regulator *regulator)
>>> +{
>>> +       return (regulator->always_on) ? 1 : 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(regulator_is_always_on);
>>> +
>>> +/**
>>>    * regulator_count_voltages - count regulator_list_voltage() 
>>> selectors
>>>    * @regulator: regulator source
>>>    *
>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>> index ebd1ceb..2f1206b 100644
>>> --- a/include/linux/mmc/host.h
>>> +++ b/include/linux/mmc/host.h
>>> @@ -462,6 +462,7 @@ static inline int mmc_regulator_set_vqmmc(struct
>>> mmc_host *mmc,
>>>
>>>   u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
>>>   int mmc_regulator_get_supply(struct mmc_host *mmc);
>>> +bool mmc_regulator_can_power_off(struct regulator *supply);
>>>
>>>   static inline int mmc_card_is_removable(struct mmc_host *host)
>>>   {
>>> diff --git a/include/linux/regulator/consumer.h 
>>> b/include/linux/regulator/
>>> consumer.h
>>> index df176d7..46126e3 100644
>>> --- a/include/linux/regulator/consumer.h
>>> +++ b/include/linux/regulator/consumer.h
>>> @@ -226,6 +226,7 @@ void 
>>> devm_regulator_bulk_unregister_supply_alias(struct
>>> device *dev,
>>>   int regulator_disable(struct regulator *regulator);
>>>   int regulator_force_disable(struct regulator *regulator);
>>>   int regulator_is_enabled(struct regulator *regulator);
>>> +int regulator_is_always_on(struct regulator *regulator);
>>>   int regulator_disable_deferred(struct regulator *regulator, int ms);
>>>
>>>   int __must_check regulator_bulk_get(struct device *dev, int 
>>> num_consumers,
>>> @@ -418,6 +419,11 @@ static inline int regulator_is_enabled(struct
>>> regulator *regulator)
>>>          return 1;
>>>   }
>>>
>>> +static inline int regulator_is_always_on(struct regulator *regulator)
>>> +{
>>> +       return 0;
>>> +}
>>> +
>>>   static inline int regulator_bulk_get(struct device *dev,
>>>                                       int num_consumers,
>>>                                       struct regulator_bulk_data 
>>> *consumers)
>>> -- 
>>> 1.9.1
>>>
>>>
>>> -- 
>>> 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
Shawn Lin Sept. 4, 2017, 1:06 a.m. UTC | #5
On 2017/9/2 6:57, Zhoujie Wu wrote:
> Hi Shawn,
> 
> On 08/29/2017 05:14 PM, Shawn Lin wrote:
>> On 2017/8/30 6:13, Zhoujie Wu wrote:
>>> Hi Shawn,
>>> thanks for your patch.
>>>
>>> On 08/29/2017 02:56 PM, zhoujie wu wrote:
>>>>
>>>>
>>>> CMD0 couldn't put the card back to 3.3v if can't do power
>>>> cycle for cards. The card is already in SDR12 then, so still
>>>> try to enable UHS mode.
>>>>
>>>> Pleas note that, if the platform use a hardware fixed
>>>> sd power, you should still include a regulator-fixed
>>>> regulator with always-on property there. So mmc core
>>>> could handle this as well. Otherwise you shouldn't
>>>> expect UHS to work for that case.
>>>>
>>>> Just compile only, please have a look and test.
>>>>
>>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>>
>>>> ---
>>>>
>>>>   drivers/mmc/core/card.h            | 17 +++++++++++++++++
>>>>   drivers/mmc/core/core.c            |  8 ++++++++
>>>>   drivers/mmc/core/sd.c              | 28 ++++++++++++++++++++++++++--
>>>>   drivers/mmc/core/sd.h              |  3 ++-
>>>>   drivers/mmc/core/sdio.c            |  2 +-
>>>>   drivers/regulator/core.c           | 14 ++++++++++++++
>>>>   include/linux/mmc/host.h           |  1 +
>>>>   include/linux/regulator/consumer.h |  6 ++++++
>>>>   8 files changed, 75 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
>>>> index f06cd91..a562110 100644
>>>> --- a/drivers/mmc/core/card.h
>>>> +++ b/drivers/mmc/core/card.h
>>>> @@ -218,4 +218,21 @@ static inline int mmc_card_broken_hpi(const struct
>>>> mmc_card *c)
>>>>          return c->quirks & MMC_QUIRK_BROKEN_HPI;
>>>>   }
>>>>
>>>> +static inline bool mmc_card_support_uhs(struct mmc_card *card)
>>>> +{
>>>> +       if (WARN_ON(!card))
>>>> +               return false;
>>>> +
>>>> +       switch (card->sd_bus_speed) {
>>>> +       case UHS_SDR104_BUS_SPEED:
>>>> +       case UHS_DDR50_BUS_SPEED:
>>>> +       case UHS_SDR50_BUS_SPEED:
>>>> +       case UHS_SDR25_BUS_SPEED:
>>>> +       case UHS_SDR12_BUS_SPEED:
>>>> +               return true;
>>>> +       default:
>>>> +               return false;
>>>> +       }
>>>> +}
>>>> +
>>>>   #endif
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index 2643126..6b5c6c5 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -1424,6 +1424,14 @@ int mmc_regulator_get_supply(struct mmc_host 
>>>> *mmc)
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(mmc_regulator_get_supply);
>>>>
>>>> +bool mmc_regulator_can_power_off(struct regulator *supply)
>>>> +{
>>>> +       if (!IS_ERR(supply))
>>>> +               return regulator_is_always_on(supply);
>>>> +       return false;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(mmc_regulator_can_power_off);
>>>> +
>>>>   /*
>>>>    * Mask off any voltages we don't support and select
>>>>    * the lowest voltage
>>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>>> index a1b0aa1..7bec61b 100644
>>>> --- a/drivers/mmc/core/sd.c
>>>> +++ b/drivers/mmc/core/sd.c
>>>> @@ -721,7 +721,8 @@ struct device_type sd_type = {
>>>>   /*
>>>>    * Fetch CID from card.
>>>>    */
>>>> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 
>>>> *rocr)
>>>> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
>>>> +                                  u32 ocr, u32 *cid, u32 *rocr)
>>>>   {
>>>>          int err;
>>>>          u32 max_current;
>>>> @@ -773,6 +774,27 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 
>>>> ocr, u32
>>>> *cid, u32 *rocr)
>>>>                  return err;
>>>>
>>>>          /*
>>>> +        * Per SD physical specification version 4.10, section
>>>> +        * 3.9.4 UHS-I Bus Speed Mode Selection Sequence, once
>>>> +        * the signal voltage is switched to 1V8, the card continues
>>>> +        * 1.8V signaling regardless of CMD0. Power cycle resets the
>>>> +        * signal voltage to 3.3V. As a fact that some vmmc is always
>>>> +        * on so the card would still work with SDR12 after CMD0. So
>>>> +        * let's attempt to enable enable UHS mode if vmmc can't be
>>>> +        * powered off.
>>>> +        */
>>>> +       if (card && mmc_card_support_uhs(card) && mmc_host_uhs(host) &&
>>>> +           !mmc_regulator_can_power_off(host->supply.vmmc) &&
>>>> +           (ocr & SD_OCR_S18R)) {
>>>> +                       *rocr |= SD_ROCR_S18A;
>>>> +                       if (mmc_set_signal_voltage(host,
>>>> MMC_SIGNAL_VOLTAGE_180)) {
>>>> +                               retries = 0;
>>>> +                               goto try_again;
>>>> +                       }
>>>> +
>>>> +                       goto done;
>>>> +       }
>>> Good to have a higher level solution if the issue is common.
>>> I did basic test with your patch(added your regulator modification 
>>> you sent to me) and add vmmc-supply in board dts, I saw warning when 
>>> boot up.
>>> I think in below case, the card is not added yet, the card is null 
>>> and print out the warning. I don't quite understand 
>>> mmc_card_support_uhs, it return true for all card speed.
>>>
>>> [    4.310003] WARNING: CPU: 1 PID: 48 at drivers/mmc/core/card.h:224 
>>> mmc_sd_get_cid+0x270/0x298
>>
>> it's just for debug, and won't be a big deal.
>>
>> And I find mmc_host_uhs here is redundant as oldcard's
>> card->sd_bus_speed is already the result of checking uhs cap for both of
>> controller and card. So mmc_card_support_uhs return true means both
>> of your controller and the cards support UHS mode, didn't they?
>>
>>>
>>> .....
>>>
>>> [    4.549657] [<ffffff8008757e24>] mmc_sd_get_cid+0x270/0x298
>>>
>>> [    4.555510] [<ffffff800875847c>] mmc_sd_init_card+0x44/0x614
>>>
>>> [    4.561456] [<ffffff8008758bd4>] mmc_attach_sd+0x7c/0x134
>>>
>>> [    4.566951] [<ffffff8008751fb8>] mmc_rescan+0x2e4/0x35c
>>>
>>> [    4.572451] [<ffffff80080d58d8>] process_one_work+0x12c/0x28c
>>>
>>> [    4.578304] [<ffffff80080d5a90>] worker_thread+0x58/0x3b8
>>>
>>> [    4.583801] [<ffffff80080db394>] kthread+0x100/0x12c
>>>
>>> [    4.589029] [<ffffff8008082ec0>] ret_from_fork+0x10/0x50
>>>
>>> [    4.594793] ---[ end trace 58af756d1037eb51 ]---
>>>
>>> I did some modifications and let code enter the if condition any way, 
>>> I added some log and you can see, rocr is updated, but when it tried 
>>> to switch to 1.8V signal, it failed. I don't know why yet.
>>
>> Ok, sounds the key point is to see why it fails to switch to 1.8v signal
>>
>>> So the code will try again, and use high speed to init card again.
>>
>> Thanks for testing this, but I haven' had enough time to put my hand
>> on it. But I will be back and check it out by myself.. Anyway you may
>> still be able to do more debugging if this approach looks good to you:)
> 
> I have checked why 1.8V signal switch failed here, as SD spec mentioned 
> it needs to gate the clock for a while when switching the voltage, I 
> added some logic to gate the clock before switch signal,
> then re-enable the clock after that, just like what set_uhs_voltage does 
> and it could work now.

Good to know that.

> and I adjust the if condition logic as below, hacked 
> mmc_card_support_uhs now always return true.
> This can be a fix, but to be a generic solution,  I am wondering from sd 
> card perspective, except for no power cycle, if there is any other 
> reason that may cause SD_OCR_S18A not set for uhs card?

I'm not aware of another reason except for that UHS-II card always
response with S18A=0, but it isn't supportted yet.

Would you mind submitting proper patch(es) for this solution?

> 
> +       if (mmc_card_support_uhs(card)
> +           !mmc_regulator_can_power_off(host->supply.vmmc) &&
> +           !(*rocr & SD_OCR_S18A))
>>
>>>
>>> [    4.259194] before rocr = c0ff8000, ocr = 41200000
>>>
>>> [    4.359909] after rocr = c1ff8000  -- if condition meets, updated 
>>> rocr
>>>
>>> [    4.364230] mmc0: 1.8V regulator output did not became stable  -- 
>>> mmc_set_signal_voltage 1.8V failed.
>>> [    4.372603] mmc0: Skipping voltage switch
>>>
>>> [    4.403225] before rocr = c0ff8000, ocr = 40200000   -- tried 
>>> again with no SD_OCR_S18R .
>>>> +       /*
>>>>           * In case CCS and S18A in the response is set, start 
>>>> Signal Voltage
>>>>           * Switch procedure. SPI mode doesn't support CMD11.
>>>>           */
>>>> @@ -788,6 +810,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 
>>>> ocr, u32
>>>> *cid, u32 *rocr)
>>>>                  }
>>>>          }
>>>>
>>>> +done:
>>>>          err = mmc_send_cid(host, cid);
>>>>          return err;
>>>>   }
>>>> @@ -924,7 +947,7 @@ static int mmc_sd_init_card(struct mmc_host 
>>>> *host, u32
>>>> ocr,
>>>>
>>>>          WARN_ON(!host->claimed);
>>>>
>>>> -       err = mmc_sd_get_cid(host, ocr, cid, &rocr);
>>>> +       err = mmc_sd_get_cid(host, oldcard, ocr, cid, &rocr);
>>>>          if (err)
>>>>                  return err;
>>>>
>>>> @@ -995,6 +1018,7 @@ static int mmc_sd_init_card(struct mmc_host 
>>>> *host, u32
>>>> ocr,
>>>>                  if (err)
>>>>                          goto free_card;
>>>>          } else {
>>>> +
>>>>                  /*
>>>>                   * Attempt to change to high-speed (if supported)
>>>>                   */
>>>> diff --git a/drivers/mmc/core/sd.h b/drivers/mmc/core/sd.h
>>>> index 1ada980..9cd2482 100644
>>>> --- a/drivers/mmc/core/sd.h
>>>> +++ b/drivers/mmc/core/sd.h
>>>> @@ -8,7 +8,8 @@
>>>>   struct mmc_host;
>>>>   struct mmc_card;
>>>>
>>>> -int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 
>>>> *rocr);
>>>> +int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
>>>> +                  u32 ocr, u32 *cid, u32 *rocr);
>>>>   int mmc_sd_get_csd(struct mmc_host *host, struct mmc_card *card);
>>>>   void mmc_decode_cid(struct mmc_card *card);
>>>>   int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>> index cc43687..08e82fe 100644
>>>> --- a/drivers/mmc/core/sdio.c
>>>> +++ b/drivers/mmc/core/sdio.c
>>>> @@ -608,7 +608,7 @@ static int mmc_sdio_init_card(struct mmc_host 
>>>> *host,
>>>> u32 ocr,
>>>>          }
>>>>
>>>>          if ((rocr & R4_MEMORY_PRESENT) &&
>>>> -           mmc_sd_get_cid(host, ocr & rocr, card->raw_cid, NULL) == 
>>>> 0) {
>>>> +           mmc_sd_get_cid(host, NULL, ocr & rocr, card->raw_cid, 
>>>> NULL) ==
>>>> 0) {
>>>>                  card->type = MMC_TYPE_SD_COMBO;
>>>>
>>>>                  if (oldcard && (oldcard->type != MMC_TYPE_SD_COMBO ||
>>>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>>>> index e567fa5..edea93b0 100644
>>>> --- a/drivers/regulator/core.c
>>>> +++ b/drivers/regulator/core.c
>>>> @@ -2522,6 +2522,20 @@ int regulator_is_enabled(struct regulator 
>>>> *regulator)
>>>>   EXPORT_SYMBOL_GPL(regulator_is_enabled);
>>>>
>>>>   /**
>>>> + * regulator_is_always_on - is the regulator output always enabled
>>>> + * @regulator: regulator source
>>>> + *
>>>> + * Returns positive if the regulator driver backing the source/client
>>>> + * has requested that the device be always on, zero if it isn't.
>>>> + *
>>>> + */
>>>> +inline int regulator_is_always_on(struct regulator *regulator)
>>>> +{
>>>> +       return (regulator->always_on) ? 1 : 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(regulator_is_always_on);
>>>> +
>>>> +/**
>>>>    * regulator_count_voltages - count regulator_list_voltage() 
>>>> selectors
>>>>    * @regulator: regulator source
>>>>    *
>>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>> index ebd1ceb..2f1206b 100644
>>>> --- a/include/linux/mmc/host.h
>>>> +++ b/include/linux/mmc/host.h
>>>> @@ -462,6 +462,7 @@ static inline int mmc_regulator_set_vqmmc(struct
>>>> mmc_host *mmc,
>>>>
>>>>   u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
>>>>   int mmc_regulator_get_supply(struct mmc_host *mmc);
>>>> +bool mmc_regulator_can_power_off(struct regulator *supply);
>>>>
>>>>   static inline int mmc_card_is_removable(struct mmc_host *host)
>>>>   {
>>>> diff --git a/include/linux/regulator/consumer.h 
>>>> b/include/linux/regulator/
>>>> consumer.h
>>>> index df176d7..46126e3 100644
>>>> --- a/include/linux/regulator/consumer.h
>>>> +++ b/include/linux/regulator/consumer.h
>>>> @@ -226,6 +226,7 @@ void 
>>>> devm_regulator_bulk_unregister_supply_alias(struct
>>>> device *dev,
>>>>   int regulator_disable(struct regulator *regulator);
>>>>   int regulator_force_disable(struct regulator *regulator);
>>>>   int regulator_is_enabled(struct regulator *regulator);
>>>> +int regulator_is_always_on(struct regulator *regulator);
>>>>   int regulator_disable_deferred(struct regulator *regulator, int ms);
>>>>
>>>>   int __must_check regulator_bulk_get(struct device *dev, int 
>>>> num_consumers,
>>>> @@ -418,6 +419,11 @@ static inline int regulator_is_enabled(struct
>>>> regulator *regulator)
>>>>          return 1;
>>>>   }
>>>>
>>>> +static inline int regulator_is_always_on(struct regulator *regulator)
>>>> +{
>>>> +       return 0;
>>>> +}
>>>> +
>>>>   static inline int regulator_bulk_get(struct device *dev,
>>>>                                       int num_consumers,
>>>>                                       struct regulator_bulk_data 
>>>> *consumers)
>>>> -- 
>>>> 1.9.1
>>>>
>>>>
>>>> -- 
>>>> 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
diff mbox

Patch

diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
index f06cd91..a562110 100644
--- a/drivers/mmc/core/card.h
+++ b/drivers/mmc/core/card.h
@@ -218,4 +218,21 @@  static inline int mmc_card_broken_hpi(const struct mmc_card *c)
 	return c->quirks & MMC_QUIRK_BROKEN_HPI;
 }
 
+static inline bool mmc_card_support_uhs(struct mmc_card *card)
+{
+	if (WARN_ON(!card))
+		return false;
+
+	switch (card->sd_bus_speed) {
+	case UHS_SDR104_BUS_SPEED:
+	case UHS_DDR50_BUS_SPEED:
+	case UHS_SDR50_BUS_SPEED:
+	case UHS_SDR25_BUS_SPEED:
+	case UHS_SDR12_BUS_SPEED:
+		return true;
+	default:
+		return false;
+	}
+}
+
 #endif
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 2643126..6b5c6c5 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1424,6 +1424,14 @@  int mmc_regulator_get_supply(struct mmc_host *mmc)
 }
 EXPORT_SYMBOL_GPL(mmc_regulator_get_supply);
 
+bool mmc_regulator_can_power_off(struct regulator *supply)
+{
+	if (!IS_ERR(supply))
+		return regulator_is_always_on(supply);
+	return false;
+}
+EXPORT_SYMBOL_GPL(mmc_regulator_can_power_off);
+
 /*
  * Mask off any voltages we don't support and select
  * the lowest voltage
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index a1b0aa1..7bec61b 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -721,7 +721,8 @@  struct device_type sd_type = {
 /*
  * Fetch CID from card.
  */
-int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
+int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
+				   u32 ocr, u32 *cid, u32 *rocr)
 {
 	int err;
 	u32 max_current;
@@ -773,6 +774,27 @@  int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
 		return err;
 
 	/*
+	 * Per SD physical specification version 4.10, section
+	 * 3.9.4 UHS-I Bus Speed Mode Selection Sequence, once
+	 * the signal voltage is switched to 1V8, the card continues
+	 * 1.8V signaling regardless of CMD0. Power cycle resets the
+	 * signal voltage to 3.3V. As a fact that some vmmc is always
+	 * on so the card would still work with SDR12 after CMD0. So
+	 * let's attempt to enable enable UHS mode if vmmc can't be
+	 * powered off.
+	 */
+	if (card && mmc_card_support_uhs(card) && mmc_host_uhs(host) &&
+	    !mmc_regulator_can_power_off(host->supply.vmmc) &&
+	    (ocr & SD_OCR_S18R)) {
+			*rocr |= SD_ROCR_S18A;
+			if (mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180)) {
+				retries = 0;
+				goto try_again;
+			}
+
+			goto done;
+	}
+	/*
 	 * In case CCS and S18A in the response is set, start Signal Voltage
 	 * Switch procedure. SPI mode doesn't support CMD11.
 	 */
@@ -788,6 +810,7 @@  int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
 		}
 	}
 
+done:
 	err = mmc_send_cid(host, cid);
 	return err;
 }
@@ -924,7 +947,7 @@  static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 
 	WARN_ON(!host->claimed);
 
-	err = mmc_sd_get_cid(host, ocr, cid, &rocr);
+	err = mmc_sd_get_cid(host, oldcard, ocr, cid, &rocr);
 	if (err)
 		return err;
 
@@ -995,6 +1018,7 @@  static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 		if (err)
 			goto free_card;
 	} else {
+
 		/*
 		 * Attempt to change to high-speed (if supported)
 		 */
diff --git a/drivers/mmc/core/sd.h b/drivers/mmc/core/sd.h
index 1ada980..9cd2482 100644
--- a/drivers/mmc/core/sd.h
+++ b/drivers/mmc/core/sd.h
@@ -8,7 +8,8 @@ 
 struct mmc_host;
 struct mmc_card;
 
-int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr);
+int mmc_sd_get_cid(struct mmc_host *host, struct mmc_card *card,
+		   u32 ocr, u32 *cid, u32 *rocr);
 int mmc_sd_get_csd(struct mmc_host *host, struct mmc_card *card);
 void mmc_decode_cid(struct mmc_card *card);
 int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index cc43687..08e82fe 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -608,7 +608,7 @@  static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 	}
 
 	if ((rocr & R4_MEMORY_PRESENT) &&
-	    mmc_sd_get_cid(host, ocr & rocr, card->raw_cid, NULL) == 0) {
+	    mmc_sd_get_cid(host, NULL, ocr & rocr, card->raw_cid, NULL) == 0) {
 		card->type = MMC_TYPE_SD_COMBO;
 
 		if (oldcard && (oldcard->type != MMC_TYPE_SD_COMBO ||
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e567fa5..edea93b0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2522,6 +2522,20 @@  int regulator_is_enabled(struct regulator *regulator)
 EXPORT_SYMBOL_GPL(regulator_is_enabled);
 
 /**
+ * regulator_is_always_on - is the regulator output always enabled
+ * @regulator: regulator source
+ *
+ * Returns positive if the regulator driver backing the source/client
+ * has requested that the device be always on, zero if it isn't.
+ *
+ */
+inline int regulator_is_always_on(struct regulator *regulator)
+{
+	return (regulator->always_on) ? 1 : 0;
+}
+EXPORT_SYMBOL_GPL(regulator_is_always_on);
+
+/**
  * regulator_count_voltages - count regulator_list_voltage() selectors
  * @regulator: regulator source
  *
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index ebd1ceb..2f1206b 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -462,6 +462,7 @@  static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
 
 u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
 int mmc_regulator_get_supply(struct mmc_host *mmc);
+bool mmc_regulator_can_power_off(struct regulator *supply);
 
 static inline int mmc_card_is_removable(struct mmc_host *host)
 {
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index df176d7..46126e3 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -226,6 +226,7 @@  void devm_regulator_bulk_unregister_supply_alias(struct device *dev,
 int regulator_disable(struct regulator *regulator);
 int regulator_force_disable(struct regulator *regulator);
 int regulator_is_enabled(struct regulator *regulator);
+int regulator_is_always_on(struct regulator *regulator);
 int regulator_disable_deferred(struct regulator *regulator, int ms);
 
 int __must_check regulator_bulk_get(struct device *dev, int num_consumers,
@@ -418,6 +419,11 @@  static inline int regulator_is_enabled(struct regulator *regulator)
 	return 1;
 }
 
+static inline int regulator_is_always_on(struct regulator *regulator)
+{
+	return 0;
+}
+
 static inline int regulator_bulk_get(struct device *dev,
 				     int num_consumers,
 				     struct regulator_bulk_data *consumers)