On 20 January 2014 20:13, Olof Johansson [off-list ref] wrote:

On Mon, Jan 20, 2014 at 09:44:23AM +0100, Ulf Hansson wrote:

quoted

On 20 January 2014 04:56, Olof Johansson [off-list ref] wrote:

quoted

This patch enables support for power-on sequencing of SDIO peripherals through DT.

In general, it's quite common that wifi modules and other similar
peripherals have several signals in addition to the SDIO interface that
needs wiggling before the module will power on. It's common to have a
reference clock, one or several power rails and one or several lines
for reset/enable type functions.

The binding as written today introduces a number of reset gpios,
a regulator and a clock specifier. The code will handle up to 2 gpio
reset lines, but it's trivial to increase to more than that if needed
at some point.

Implementation-wise, the MMC core has been changed to handle this during
host power up, before the host interface is powered on. I have not yet
implemented the power-down side, I wanted people to have a chance for
reporting back w.r.t. issues (or comments on the bindings) first.

I have not tested the regulator portion, since the system and module
I'm working on doesn't need one (Samsung Chromebook with Marvell
8797-based wifi). Testing of those portions (and reporting back) would
be appreciated.

Signed-off-by: Olof Johansson <redacted>
---
 Documentation/devicetree/bindings/mmc/mmc.txt |   11 +++++++
 drivers/mmc/core/core.c                       |   42 +++++++++++++++++++++++++
 drivers/mmc/core/host.c                       |   30 +++++++++++++++++-
 include/linux/mmc/host.h                      |    5 +++
 4 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 458b57f..962e0ee 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt

@@ -5,6 +5,8 @@ these definitions.
 Interpreted by the OF core:
 - reg: Registers location and length.
 - interrupts: Interrupts used by the MMC controller.
+- clocks: Clocks needed for the host controller, if any.
+- clock-names: Goes with clocks above.

 Card detection:
 If no property below is supplied, host native card detect is used.

@@ -30,6 +32,15 @@ Optional properties:
 - cap-sdio-irq: enable SDIO IRQ signalling on this interface
 - full-pwr-cycle: full power cycle of the card is supported

+Card power and reset control:
+The following properties can be specified for cases where the MMC
+peripheral needs additional reset, regulator and clock lines. It is for
+example common for WiFi/BT adapters to have these separate from the main
+MMC bus:
+  - card-reset-gpios: Specify GPIOs for card reset (reset active low)
+  - card-external-vcc-supply: Regulator to drive (independent) card VCC
+  - clock with name "card_ext_clock": External clock provided to the card
+
 *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
 polarity properties, we have to fix the meaning of the "normal" and "inverted"
 line levels. We choose to follow the SDHCI standard, which specifies both those

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 098374b..c43e6c8 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c

@@ -13,11 +13,13 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/device.h>
 #include <linux/delay.h>
 #include <linux/pagemap.h>
 #include <linux/err.h>
+#include <linux/gpio.h>
 #include <linux/leds.h>
 #include <linux/scatterlist.h>
 #include <linux/log2.h>

@@ -1519,6 +1521,43 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type)
        mmc_host_clk_release(host);
 }

+static void mmc_card_power_up(struct mmc_host *host)
+{
+       int i;
+       struct gpio_desc **gds = host->card_reset_gpios;
+
+       for (i = 0; i < ARRAY_SIZE(host->card_reset_gpios); i++) {
+               if (gds[i]) {
+                       dev_dbg(host->parent, "Asserting reset line %d", i);
+                       gpiod_set_value(gds[i], 1);
+               }
+       }
+
+       if (host->card_regulator) {
+               dev_dbg(host->parent, "Enabling external regulator");
+               if (regulator_enable(host->card_regulator))
+                       dev_err(host->parent, "Failed to enable external regulator");
+       }
+
+       if (host->card_clk) {
+               dev_dbg(host->parent, "Enabling external clock");
+               clk_prepare_enable(host->card_clk);
+       }
+
+       /* 2ms delay to let clocks and power settle */
+       mmc_delay(20);
+
+       for (i = 0; i < ARRAY_SIZE(host->card_reset_gpios); i++) {
+               if (gds[i]) {
+                       dev_dbg(host->parent, "Deasserting reset line %d", i);
+                       gpiod_set_value(gds[i], 0);
+               }
+       }
+
+       /* 2ms delay to after reset release */
+       mmc_delay(20);
+}
+
 /*
  * Apply power to the MMC stack.  This is a two-stage process.
  * First, we enable power to the card without the clock running.

@@ -1535,6 +1574,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
        if (host->ios.power_mode == MMC_POWER_ON)
                return;

+       /* Power up the card/module first, if needed */
+       mmc_card_power_up(host);
+
        mmc_host_clk_hold(host);

        host->ios.vdd = fls(ocr) - 1;

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 49bc403..e6b850b 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c

@@ -12,14 +12,18 @@
  *  MMC host class device management
  */

+#include <linux/kernel.h>
+#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/idr.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/pagemap.h>
 #include <linux/export.h>
 #include <linux/leds.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>

@@ -312,7 +316,7 @@ int mmc_of_parse(struct mmc_host *host)
        u32 bus_width;
        bool explicit_inv_wp, gpio_inv_wp = false;
        enum of_gpio_flags flags;
-       int len, ret, gpio;
+       int i, len, ret, gpio;

        if (!host->parent || !host->parent->of_node)
                return 0;

@@ -415,6 +419,30 @@ int mmc_of_parse(struct mmc_host *host)
        if (explicit_inv_wp ^ gpio_inv_wp)
                host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;

+       /* Parse card power/reset/clock control */

I would like us to prevent to open up for confusion with the "eMMC hw
reset" when adding this. Unless we are able to combine them in some
way?

Could we maybe add some more comments about in what scenarios this DT
property would be useful?

Ok, can do. How about something like:

        /*
         * Some cards need separate power/reset/clock control from the main
         * MMC/SDIO bus. Parse the description of those controls so we can
         * power on the card before the host controller.
         */

quoted

quoted

+       if (of_find_property(np, "card-reset-gpios", NULL)) {
+               struct gpio_desc *gpd;
+               for (i = 0; i < ARRAY_SIZE(host->card_reset_gpios); i++) {
+                       gpd = devm_gpiod_get_index(host->parent, "card-reset", i);
+                       if (IS_ERR(gpd))
+                               break;
+                       gpiod_direction_output(gpd, 0);
+                       host->card_reset_gpios[i] = gpd;
+               }
+
+               gpd = devm_gpiod_get_index(host->parent, "card-reset", ARRAY_SIZE(host->card_reset_gpios));
+               if (!IS_ERR(gpd)) {
+                       dev_warn(host->parent, "More reset gpios than we can handle");
+                       gpiod_put(gpd);
+               }
+       }
+
+       host->card_clk = of_clk_get_by_name(np, "card_ext_clock");

of_clk_get_by_name relies on COMMON_CLK, is that really what you want here?

quoted

+       if (IS_ERR(host->card_clk))
+               host->card_clk = NULL;
+
+       host->card_regulator = regulator_get(host->parent, "card-external-vcc");

Is the above regulator related to host->ocr_avail mask? Could the
above regulator be replaced by vmmc?

I have to admit that I don't know MMC as well as I could, but OCR seems to be
something that's between the driver/controller/device, not related to external
power control in this case?

This is related to the power of the card, typically external
regulators controlled by the host driver.

Both the card and the host supports a voltage range. This range is
exactly what the OCR mask describes. At initialization of the card,
the host ocr is validated against the card ocr.

quoted

At the moment host drivers uses mmc_regulator_get_supply(), which
fetches regulators called "vmmc" and "vqmmc". It is also common to
have these defined in DT like "vmmc-supply". This has not been
properly documented for most host cases, and we should fix that. I
also think it would make sense to include these in the documentation
for the common mmc bindings, instead of host specific bindings.

Hm, I had been of the impression that the vmmc stuff is to control
power/voltage on the signal lines, not for external card power. Still, even in
that case there's need for the reset line handling and clock control.

vmmc: the power to the card.
vqmmc: the I/O voltage levels (for the signal lines).

Regarding reset, I agree, those seems to be needed.

Regarding clock control. I suppose you are referring to separate
external clocks, not affecting the SDIO/SD/MMC interface speed!?

That could make sense, but still I wonder how those shall be handled
in a fine grained power management setup. In other words, when shall
those be gated/ungated? Is the mmc core able to take the correct
decision about these?

Kind regards
Uffe

I'll take a look and see if there's a way to handle that in a properly
sequenced way and still use the same regulator.

-Olof