Re: [RFC PATCH v3 2/8] mmc: omap_hsmmc: handle vcc and vcc_aux independently

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Wednesday 11 December 2013 04:51 PM, Ulf Hansson wrote:
On 10 December 2013 12:48, Balaji T K <balajitk@xxxxxx> wrote:
On Tuesday 10 December 2013 04:39 PM, Ulf Hansson wrote:

On 21 November 2013 15:20, Balaji T K <balajitk@xxxxxx> wrote:

handle vcc and vcc_aux independently to reduce indent.

Signed-off-by: Balaji T K <balajitk@xxxxxx>
---
   drivers/mmc/host/omap_hsmmc.c |   54
+++++++++++++++++++----------------------
   1 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c
b/drivers/mmc/host/omap_hsmmc.c
index 1eb4350..342be25 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -286,11 +286,12 @@ static int omap_hsmmc_set_power(struct device *dev,
int slot, int power_on,
           * chips/cards need an interface voltage rail too.
           */
          if (power_on) {
-               ret = mmc_regulator_set_ocr(host->mmc, host->vcc, vdd);
+               if (host->vcc)
+                       ret = mmc_regulator_set_ocr(host->mmc, host->vcc,
vdd);
                  /* Enable interface voltage rail, if needed */
                  if (ret == 0 && host->vcc_aux) {
                          ret = regulator_enable(host->vcc_aux);
-                       if (ret < 0)
+                       if (ret < 0 && host->vcc)
                                  ret = mmc_regulator_set_ocr(host->mmc,
                                                          host->vcc, 0);
                  }
@@ -298,7 +299,7 @@ static int omap_hsmmc_set_power(struct device *dev,
int slot, int power_on,
                  /* Shut down the rail */
                  if (host->vcc_aux)
                          ret = regulator_disable(host->vcc_aux);
-               if (!ret) {
+               if (host->vcc) {
                          /* Then proceed to shut down the local regulator
*/
                          ret = mmc_regulator_set_ocr(host->mmc,
                                                  host->vcc, 0);
@@ -318,10 +319,10 @@ static int omap_hsmmc_reg_get(struct
omap_hsmmc_host *host)

          reg = devm_regulator_get(host->dev, "vmmc");
          if (IS_ERR(reg)) {
-               dev_err(host->dev, "vmmc regulator missing\n");
+               dev_err(host->dev, "unable to get vmmc regulator %ld\n",
+                       PTR_ERR(reg));
                  return PTR_ERR(reg);
          } else {
-               mmc_slot(host).set_power = omap_hsmmc_set_power;
                  host->vcc = reg;
                  ocr_value = mmc_regulator_get_ocrmask(reg);
                  if (!mmc_slot(host).ocr_mask) {
@@ -334,31 +335,26 @@ static int omap_hsmmc_reg_get(struct
omap_hsmmc_host *host)
                                  return -EINVAL;
                          }
                  }
+       }
+       mmc_slot(host).set_power = omap_hsmmc_set_power;

-               /* Allow an aux regulator */
-               reg = devm_regulator_get_optional(host->dev, "vmmc_aux");
-               host->vcc_aux = IS_ERR(reg) ? NULL : reg;
+       /* Allow an aux regulator */
+       reg = devm_regulator_get_optional(host->dev, "vmmc_aux");
+       host->vcc_aux = IS_ERR(reg) ? NULL : reg;

-               /* For eMMC do not power off when not in sleep state */
-               if (mmc_slot(host).no_regulator_off_init)
-                       return 0;
-               /*
-               * UGLY HACK:  workaround regulator framework bugs.
-               * When the bootloader leaves a supply active, it's
-               * initialized with zero usecount ... and we can't
-               * disable it without first enabling it.  Until the
-               * framework is fixed, we need a workaround like this
-               * (which is safe for MMC, but not in general).
-               */


The above problem is handled by the mmc core layer. I certainly think
you shall adopt your code to it.

Hi Ulf,

how about optional vqmmc, omap_hsmmc does call mmc_regulator_set_ocr for
vmmc
Am I missing something?

Hi Balaji,

Sorry for being to vague, let me elaborate.

1. Before omap_hsmmc_probe returns, it invokes mmc_add_host().

2. mmc_add_host() -> mmc_start_host() -> > mmc_power_up().

3. mmc_power_up() invokes the host driver's .set_ios() callback, to
makes sure boot-on regulators are kept enabled. Otherwise the
late_initcall, regulator_init_complete() will potentially cut power
for unused regulators.

This is important because there are SoCs that are only capable of
power cycling the VCC regulator but not VCCQ. According to the eMMC
spec, we must not cut VCC without sending the SLEEP cmd first.
Obviously, since we prevent VCC from being cut, we need to prevent
VCCQ from being cut as well.

Normally a .set_ios() function's invokes mmc_regulator_set_ocr() for
the VCC regulator. VCCQ ("vmmc_aux") is handled by directly using the
regulator API (I guess we could invent an API similar to
mmc_regulator_set_ocr() but for VCCQ, but that is a future task).

So, from a host driver perspective you could add a local variable to
cache the "enabled" state of the VCCQ regulator. By leaving the
default state to "disabled", you will trigger it to be enabled once
mmc_power_up() invokes your .set_ios() callback. Vice verse will then
happen when mmc_power_off gets invoked.

Finally, don't forget to enable MMC_CAP2_FULL_PWR_CYCLE, if your SoC
can cut both VCC and VCCQ for your eMMC slots. For SD cards only VCC
should be needed to enable MMC_CAP2_FULL_PWR_CYCLE.


Hi Ulf,

Thanks for the explanation. I need to check if the Work around (power ON
and then power OFF) are there for legacy reason or needed for TI SDIO
detection on soft reset, if it is latter need to figure out a way to
fix TI SDIO before removing it.

Kind regards
Ulf Hansson





Kind regards
Ulf Hansson

-               if (regulator_is_enabled(host->vcc) > 0 ||
-                   (host->vcc_aux &&
regulator_is_enabled(host->vcc_aux))) {
-                       int vdd = ffs(mmc_slot(host).ocr_mask) - 1;
+       /* For eMMC do not power off when not in sleep state */
+       if (mmc_slot(host).no_regulator_off_init)
+               return 0;
+       /*
+        * To disable boot_on regulator, enable regulator
+        * to increase usecount and then disable it.
+        */
+       if ((host->vcc && regulator_is_enabled(host->vcc) > 0) ||
+           (host->vcc_aux && regulator_is_enabled(host->vcc_aux))) {
+               int vdd = ffs(mmc_slot(host).ocr_mask) - 1;

-                       mmc_slot(host).set_power(host->dev,
host->slot_id,
-                                                1, vdd);
-                       mmc_slot(host).set_power(host->dev,
host->slot_id,
-                                                0, 0);
-               }
+               mmc_slot(host).set_power(host->dev, host->slot_id, 1,
vdd);
+               mmc_slot(host).set_power(host->dev, host->slot_id, 0, 0);
          }

          return 0;
--
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux