Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

Doug Anderson
2014-08-25T15:06:52Z

Jaehoon,

On Mon, Aug 25, 2014 at 5:32 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 08/22/2014 10:47 PM, Yuvaraj Kumar C D wrote:
>> This patch makes use of mmc_regulator_get_supply() to handle
>> the vmmc and vqmmc regulators.Also it moves the code handling
>> the these regulators to dw_mci_set_ios().It turned on the vmmc
>> and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
>> during MMC_POWER_OFF.
>>
>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>> ---
>> changes from v1:
>> 1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
>> 2.Turned on vmmc and vqmmc during MMC_POWER_UP.
>> 3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
>> added during the initial version of this patch.
>> 4. Added error message, if it failed to turn on regulator's.
>>
>> drivers/mmc/host/dw_mmc.c | 72 +++++++++++++++++++++-----------------------
>> include/linux/mmc/dw_mmc.h | 2 +-
>> 2 files changed, 36 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 7f227e9..aadb0d6 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>> struct dw_mci_slot *slot = mmc_priv(mmc);
>> const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
>> u32 regs;
>> + int ret;
>>
>> switch (ios->bus_width) {
>> case MMC_BUS_WIDTH_4:
>> @@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>
>> switch (ios->power_mode) {
>> case MMC_POWER_UP:
>> + if (!IS_ERR(mmc->supply.vmmc)) {
>> + ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
>> + ios->vdd);
>> + if (ret) {
>> + dev_err(slot->host->dev,
>> + "failed to enable vmmc regulator\n");
>> + /*return, if failed turn on vmmc*/
>> + return;
>> + }
>> + }
>> + if (!IS_ERR(mmc->supply.vqmmc) && !slot->host->vqmmc_enabled) {
>
> Can't use the regulator_is_enabled() instead of "slot->host->vqmmc_enabled"?

I think we mentioned this before, but regulator_is_enabled() won't do
what you want with shared regulators since they are refcounted. You
need to keep track of whether you've enabled the regulator yourself.

-Doug