Thread (35 messages) 35 messages, 6 authors, 2014-07-22
STALE4362d

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

From: dianders@chromium.org (Doug Anderson)
Date: 2014-06-30 15:06:46
Also in: linux-mmc, linux-samsung-soc

Jaehoon,

On Mon, Jun 30, 2014 at 5:13 AM, Jaehoon Chung [off-list ref] wrote:
On 06/27/2014 01:18 AM, Doug Anderson wrote:
quoted
Yuvaraj,

On Thu, Jun 26, 2014 at 4:21 AM, Yuvaraj Kumar [off-list ref] wrote:
quoted
Doug

On Tue, Jun 24, 2014 at 11:30 PM, Doug Anderson [off-list ref] wrote:
quoted
Yuvaraj,

On Mon, Jun 23, 2014 at 3:45 AM, Yuvaraj Kumar C D [off-list ref] wrote:
quoted
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 <redacted>
---
 drivers/mmc/host/dw_mmc.c |   71 ++++++++++++++++++++++-----------------------
 drivers/mmc/host/dw_mmc.h |    2 ++
 2 files changed, 36 insertions(+), 37 deletions(-)
Perhaps you could CC me on the whole series for the next version since
I was involved in privately reviewing previous versions?
It was just accidental missing you in the CC .Surely i will add you in
CC for next versions.
quoted
Overall caveat for my review is that I'm nowhere near an SD/MMC expert.

quoted
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 1ac227c..f5cabce 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -937,6 +937,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:
@@ -975,16 +976,41 @@ 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)) &&
+                               !test_bit(DW_MMC_CARD_POWERED, &slot->flags)) {
+                       ret = regulator_enable(mmc->supply.vmmc);
+                       if (!ret)
+                               set_bit(DW_MMC_CARD_POWERED, &slot->flags);
+               }
As per below, I'm not sure why you'd want to turn on vqmmc and vmmc at
different times.
As you can see people's have different opinion on this.When i had a
look at the other drivers in the subsystem which does in the same flow
as above.However i will change in the next version.
Given my self proclaimed lack of SD/MMC knowledge, if others have a
good reason for doing them separate then you should do it that way.
So far I haven't heard that reason but I certainly could be wrong.
At first time, i had believed nothing problem that it turns on vmmc and vqmmc at different time.
It could have the potential problem.
OK.  As I said I'm nowhere near an expert on SD/MMC, so if there's
something out there that says that turning vmmc on before vqmmc is the
right thing to do then I guess that's the answer.  I would still be
very curious to get more details on what the potential problem is.
Could you provide a reference to documentation that says vmmc should
be on before vqmmc or describe a situation where it's important?

Thanks!

-Doug
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help