RFC/RFT: Powering on MMC Wifi/BT modules in MMC core

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 <[email protected]>

---
 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(-)

Documentation/devicetree/bindings/mmc/mmc.txt +11 -0

index 458b57f..962e0ee 100644
@@ -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

drivers/mmc/core/core.c +42 -0

index 098374b..c43e6c8 100644
@@ -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;

drivers/mmc/core/host.c +5 -1

index 49bc403..e6b850b 100644
@@ -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;

drivers/mmc/core/host.c +8 -0

+       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;

drivers/mmc/core/host.c +1 -0

+
        if (of_find_property(np, "cap-sd-highspeed", &len))
                host->caps |= MMC_CAP_SD_HIGHSPEED;
        if (of_find_property(np, "cap-mmc-highspeed", &len))

include/linux/mmc/host.h +5 -1

index 99f5709..6781887 100644
@@ -297,6 +297,11 @@ struct mmc_host {
        unsigned long           clkgate_delay;
 #endif
 
+       /* card specific properties to deal with power and reset */
+       struct regulator        *card_regulator; /* External VCC needed by the card */
+       struct gpio_desc        *card_reset_gpios[2]; /* External resets, active low */
+       struct clk              *card_clk;      /* External clock needed by the card */
+
        /* host specific block data */
        unsigned int            max_seg_size;   /* see blk_queue_max_segment_size */
        unsigned short          max_segs;       /* see blk_queue_max_segments */
--

1.7.10.4

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

Quoted 168 lines · Olof Johansson

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 <[email protected]>

---
 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(-)

Documentation/devicetree/bindings/mmc/mmc.txt +11 -0

index 458b57f..962e0ee 100644
@@ -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

drivers/mmc/core/core.c +42 -0

index 098374b..c43e6c8 100644
@@ -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;

drivers/mmc/core/host.c +5 -1

index 49bc403..e6b850b 100644
@@ -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;
 

drivers/mmc/core/host.c int mmc_of_parse(struct mmc_host *host) · line 422

Quoted 1 line

drivers/mmc/core/host.c +1 -0

+       /* 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?

Quoted 8 lines

drivers/mmc/core/host.c +8 -0

+       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;

drivers/mmc/core/host.c int mmc_of_parse(struct mmc_host *host) · line 440

Quoted 10 lines

drivers/mmc/core/host.c +10 -0

+               }
+
+               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?

drivers/mmc/core/host.c int mmc_of_parse(struct mmc_host *host) · line 444

Quoted 4 lines

drivers/mmc/core/host.c +4 -0

+       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?

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.

Quoted 169 lines · Ulf Hansson

Quoted 168 lines · Olof Johansson

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 <[email protected]>

---
 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(-)

Documentation/devicetree/bindings/mmc/mmc.txt +11 -0

index 458b57f..962e0ee 100644
@@ -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

drivers/mmc/core/core.c +42 -0

index 098374b..c43e6c8 100644
@@ -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;

drivers/mmc/core/host.c +5 -1

index 49bc403..e6b850b 100644
@@ -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;
 

Quoted 8 lines

drivers/mmc/core/host.c int mmc_of_parse(struct mmc_host *host) · line 422

Quoted 1 line

drivers/mmc/core/host.c +1 -0

+       /* 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 26 lines

Quoted 8 lines

drivers/mmc/core/host.c +8 -0

+       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;

drivers/mmc/core/host.c int mmc_of_parse(struct mmc_host *host) · line 440

Quoted 10 lines

drivers/mmc/core/host.c +10 -0

+               }
+
+               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 4 lines

drivers/mmc/core/host.c +4 -0

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

Quoted 2 lines

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?

Quoted 7 lines

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.

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

Quoted 6 lines

Quoted 2 lines

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 7 lines

Quoted 6 lines

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.

Quoted 3 lines

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?

Quoted 4 lines · Ulf Hansson

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?

The reference clock is in most cases I've seen 32kHz, and not something that's under fine-grained power management. So it's not used to regulate interface speed, etc.

Documentation/devicetree/bindings/mmc/mmc.txt +2 -0

index 458b57f..962e0ee 100644
@@ -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

So far so good. Now, what about this external oscillator which has its own separate power control. My immediate thought is that this can be specified via card_ext_clock - I would simply need to declare a fixed-rate clock with either a regulator (power switch) controlled via a gpio (which would probably be closer to the hardware) or a gpio as an enable... ah, that requires me to write a common clock driver for that bit since this is currently not modelled by CCF...

Quoted 7 lines · Russell King - ARM Linux

So far so good. Now, what about this external oscillator which has its own separate power control. My immediate thought is that this can be specified via card_ext_clock - I would simply need to declare a fixed-rate clock with either a regulator (power switch) controlled via a gpio (which would probably be closer to the hardware) or a gpio as an enable... ah, that requires me to write a common clock driver for that bit since this is currently not modelled by CCF...

Isn't this covered by the gpios property of fixed-clock?

We do the following to enable the 26MHz codec clock in imx51-babbage.dts via GPIO4_26:

        clk_26M: codec_clock {
            compatible = "fixed-clock";
            reg=<0>;
            #clock-cells = <0>;
            clock-frequency = <26000000>;
            gpios = <&gpio4 26 GPIO_ACTIVE_LOW>;
        };

Not as far as I can see. fixed-clock appears to have two properies:

clock-frequency
clock-output-names

and nothing else. See of_fixed_clk_setup in drivers/clk/clk-fixed-rate.c. You'll also find that the documentation in this file says this about it:

Quoted 5 lines · Russell King - ARM Linux

Not as far as I can see. fixed-clock appears to have two properies:

clock-frequency
clock-output-names

Quoted 2 lines

and nothing else. See of_fixed_clk_setup in drivers/clk/clk-fixed-rate.c. You'll also find that the documentation in this file says this about it:

Looks like Documentation/devicetree/bindings/clock/fixed-clock.txt is misleading then:

"Optional properties:
- gpios : From common gpio binding; gpio connection to clock enable pin."

From what I can see, this "gpios" property has never been supported.

So far so good. Now, what about this external oscillator which has its own separate power control. My immediate thought is that this can be specified via card_ext_clock - I would simply need to declare a fixed-rate clock with either a regulator (power switch) controlled via a gpio (which would probably be closer to the hardware) or a gpio as an enable... ah, that requires me to write a common clock driver for that bit since this is currently not modelled by CCF...

Quoted 7 lines · Russell King - ARM Linux

So far so good. Now, what about this external oscillator which has its own separate power control. My immediate thought is that this can be specified via card_ext_clock - I would simply need to declare a fixed-rate clock with either a regulator (power switch) controlled via a gpio (which would probably be closer to the hardware) or a gpio as an enable... ah, that requires me to write a common clock driver for that bit since this is currently not modelled by CCF...

Jiry Sarha posted a gpio controlled clock proposal:
http://www.spinics.net/lists/devicetree/msg16651.html

Quoted 14 lines · Fabio Estevam

Quoted 8 lines · Russell King - ARM Linux

So far so good. Now, what about this external oscillator which has its own separate power control. My immediate thought is that this can be specified via card_ext_clock - I would simply need to declare a fixed-rate
clock with either a regulator (power switch) controlled via a gpio (which would probably be closer to the hardware) or a gpio as an enable... ah, that requires me to write a common clock driver for that bit since this is currently not modelled by CCF...

Quoted 3 lines

Jiry Sarha posted a gpio controlled clock proposal:
http://www.spinics.net/lists/devicetree/msg16651.html

I have not received too much feedback to my patch yet. CCF is a bit new territory to me, but I think having a separate stackable clk-gpio would be more flexible than having the gpio property implemented in clk-fixed-rate.

Anyway, I am happy do it either way as long as I can get a gpio -controlled clock implementation into the main line.

Hi,

Quoted 1 line · Jyri Sarha

On 01/24/2014 07:35 PM, Fabio Estevam wrote:

Quoted 3 lines

I have not received too much feedback to my patch yet. CCF is a bit new territory to me, but I think having a separate stackable clk-gpio would be more flexible than having the gpio property implemented in clk-fixed-rate.

Not sure where stacking external clocks would be used, but sounds like a fixed factor clock? Maybe you should add DT support to that if that's what you need or have.

Documentation/devicetree/bindings/mmc/mmc.txt Optional properties: · line 42

Quoted 9 lines · Olof Johansson

Documentation/devicetree/bindings/mmc/mmc.txt +8 -0

+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

I wonder whether the reset line should use the generic reset controller binding rather than the gpio binding. There has been a recent discussion about a gpio-reset driver, which should generalize this at the reset level, and it would make it possible to use reset controllers that are not gpio driven. In general, any gpio can be used as a reset, but not every reset line can be a gpio. I don't know whether anyone would use an internal reset-controller for an external SDIO chip though, so maybe your approach is sufficient.

I'd expect most usage of this to be through gpios, since we're talking about external independent modules here. I would prefer not to bring in the reset controller stuff here -- it just adds another layer of abstraction that seems unnecessary.

I would prefer if the reset framework was contained to only be the on-chip SoC IP block resets, etc, as originally intended.

Documentation/devicetree/bindings/mmc/mmc.txt Optional properties: · line 40

Quoted 6 lines · Olof Johansson

Documentation/devicetree/bindings/mmc/mmc.txt +6 -0

+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)

Wouldn't it be better not to assume that the card reset is always active low and read the GPIO_ACTIVE_xxx flags instead?

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 <[email protected]>

---
 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(-)

Documentation/devicetree/bindings/mmc/mmc.txt +2 -0

index 458b57f..962e0ee 100644
@@ -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.

Documentation/devicetree/bindings/mmc/mmc.txt Optional properties: · line 40

Quoted 6 lines · Olof Johansson

Documentation/devicetree/bindings/mmc/mmc.txt +6 -0

+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)

We have GPIO_ACTIVE_LOW/HIGH. No need to hardcode this.

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 <[email protected]>

---
  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(-)

Documentation/devicetree/bindings/mmc/mmc.txt +11 -0

index 458b57f..962e0ee 100644
@@ -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

drivers/mmc/core/core.c +42 -0

index 098374b..c43e6c8 100644
@@ -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;

drivers/mmc/core/host.c +29 -1

index 49bc403..e6b850b 100644
@@ -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 */
+	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");
+	if (IS_ERR(host->card_clk))
+		host->card_clk = NULL;
+
+	host->card_regulator = regulator_get(host->parent, "card-external-vcc");
+
  	if (of_find_property(np, "cap-sd-highspeed", &len))
  		host->caps |= MMC_CAP_SD_HIGHSPEED;
  	if (of_find_property(np, "cap-mmc-highspeed", &len))

include/linux/mmc/host.h +5 -0

index 99f5709..6781887 100644
@@ -297,6 +297,11 @@ struct mmc_host {
  	unsigned long           clkgate_delay;
  #endif
 
+	/* card specific properties to deal with power and reset */
+	struct regulator	*card_regulator; /* External VCC needed by the card */
+	struct gpio_desc	*card_reset_gpios[2]; /* External resets, active low */
+	struct clk		*card_clk;	/* External clock needed by the card */
+
  	/* host specific block data */
  	unsigned int		max_seg_size;	/* see blk_queue_max_segment_size */
  	unsigned short		max_segs;	/* see blk_queue_max_segments */

What do you think?

Quoted 1 line · Tomasz Figa

What do you think?

Sure, post the patches.

Hi,

Quoted 19 lines · Olof Johansson

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.

Quoted 4 lines

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.

While I fully agree that this is an important problem that needs to be solved, I really don't think this is the right way, because:

a) power-up sequence is really specific to the MMC device and often it's not simply a matter of switching on one regulator or one clock, e.g. specific time constraints need to be met.

b) you can have WLAN chips in which SDIO is just one of the options to use as host interface, which may be also HSIC, I2C or UART. Really. See [1].

c) this is leaking device specific details to generic host code, which isn't really elegant.

Now, to make this a bit more constructive, [2] is a solution that I came up with (not perfect either), which simply adds a separate platform device for the low level part of the chip. I believe this is a better solution because:

a) you can often see such WLAN/BT combo chip as a set of separate devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC, which provides power/reset control, out of band signalling and etc. for the first two, so it isn't that bad to have a separate device node for the last one,

b) you have full freedom of defining your DT binding with whatever data you need, any number of clocks, regulators, GPIOs and even out of band interrupts (IMHO the most important one).

c) you can implement power-on, power-off sequences as needed for your particular device,

d) you have full separation of device-specific data from MMC core (or any other subsystem simply used as a way to perform I/O to the chip).

Now what's missing there is a way to signal the MMC core or any other transport that a device showed up and the controller should be woken up out of standby and scan of the bus initialized. This could be done by explicitly specifying the device as a subnode of the MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power controller of the chip or the other way around - a link to the MMC/UART/... controller from the power controller node.

Quoted 65 lines · Tomasz Figa

Hi,

Quoted 20 lines · Olof Johansson

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.

Quoted 4 lines

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.

While I fully agree that this is an important problem that needs to be solved, I really don't think this is the right way, because:

a) power-up sequence is really specific to the MMC device and often it's not simply a matter of switching on one regulator or one clock, e.g. specific time constraints need to be met.

b) you can have WLAN chips in which SDIO is just one of the options to use as host interface, which may be also HSIC, I2C or UART. Really. See [1].

c) this is leaking device specific details to generic host code, which isn't really elegant.

Now, to make this a bit more constructive, [2] is a solution that I came up with (not perfect either), which simply adds a separate platform device for the low level part of the chip. I believe this is a better solution because:

a) you can often see such WLAN/BT combo chip as a set of separate devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC, which provides power/reset control, out of band signalling and etc. for the first two, so it isn't that bad to have a separate device node for the last one,

b) you have full freedom of defining your DT binding with whatever data you need, any number of clocks, regulators, GPIOs and even out of band interrupts (IMHO the most important one).

c) you can implement power-on, power-off sequences as needed for your particular device,

d) you have full separation of device-specific data from MMC core (or any other subsystem simply used as a way to perform I/O to the chip).

Quoted 7 lines

Now what's missing there is a way to signal the MMC core or any other transport that a device showed up and the controller should be woken up out of standby and scan of the bus initialized. This could be done by explicitly specifying the device as a subnode of the MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power controller of the chip or the other way around - a link to the MMC/UART/... controller from the power controller node.

I've looked a bit around MMC core code and got some basic idea how things look. I will definitely need some guidance, or at least some opinions, from MMC guys, as some MMC core changes are unavoidable.

Now, the device-specific code is not really an issue, existing drivers usually already have their ways of powering the chips on and off, based on platform data. Everything needed here is to retrieve needed resources (GPIOs, clocks, regulators) using DT, which should be trivial.

The worse part is the interaction between MMC and power controller driver (the platform driver part of WLAN driver, if you look at brcmfmac as an example). I believe that we need following things:

a) A way to tell the MMC controller that there is no card detection mechanism available on given slot and it also should not be polling the slot to check card presence. Something like a "manual card detect" that would be triggered by another kernel entity that controls whether the MMC device is present (i.e. WLAN driver). We already have "broken-cd" property, but it only implies the former, wasting time on needless polling.

b) A mechanism to bind the power controller to used MMC slot. Something like "mmc-bus = <&mmc2>;" property in device node of the power controller and a function like of_find_mmc_controller_by_node(), which would be an MMC counterpart of I2C's of_find_i2c_adapter_by_node(). To avoid races, it should probably take a reference on MMC host that would have to be dropped explicitly whenever it is not needed anymore.

c) A method to notify the MMC subsystem that card presence has changed. We already have something like this in drivers/mmc/core/slot-gpio.c, but used for a simple GPIO-based card detection. If the main part of mmc_gpio_cd_irqt() could be turned into an exported helper, e.g. mmc_force_card_detect(host) then basically we would have everything needed.

Unfortunately, I don't have more time left for today to create patches and test them, so for now, I'd like to hear opinion of MMC maintainers about this approach. Do you find this acceptable?

By the way, it seems like slot-gpio.c could replace a lot of custom GPIO card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there any reason why it couldn't?

Best regards,
Tomasz

Quoted 88 lines · Tomasz Figa

Quoted 65 lines · Tomasz Figa

Hi,

Quoted 20 lines · Olof Johansson

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.

Quoted 4 lines

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.

While I fully agree that this is an important problem that needs to be solved, I really don't think this is the right way, because:

a) power-up sequence is really specific to the MMC device and often it's not simply a matter of switching on one regulator or one clock, e.g. specific time constraints need to be met.

b) you can have WLAN chips in which SDIO is just one of the options to use as host interface, which may be also HSIC, I2C or UART. Really. See [1].

c) this is leaking device specific details to generic host code, which isn't really elegant.

Now, to make this a bit more constructive, [2] is a solution that I came up with (not perfect either), which simply adds a separate platform device for the low level part of the chip. I believe this is a better solution because:

a) you can often see such WLAN/BT combo chip as a set of separate devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC, which provides power/reset control, out of band signalling and etc. for the first two, so it isn't that bad to have a separate device node for the last one,

b) you have full freedom of defining your DT binding with whatever data you need, any number of clocks, regulators, GPIOs and even out of band interrupts (IMHO the most important one).

c) you can implement power-on, power-off sequences as needed for your particular device,

d) you have full separation of device-specific data from MMC core (or any other subsystem simply used as a way to perform I/O to the chip).

Quoted 7 lines

Now what's missing there is a way to signal the MMC core or any other transport that a device showed up and the controller should be woken up out of standby and scan of the bus initialized. This could be done by explicitly specifying the device as a subnode of the MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power controller of the chip or the other way around - a link to the MMC/UART/... controller from the power controller node.

I've looked a bit around MMC core code and got some basic idea how things look. I will definitely need some guidance, or at least some opinions, from MMC guys, as some MMC core changes are unavoidable.

Now, the device-specific code is not really an issue, existing drivers usually already have their ways of powering the chips on and off, based on platform data. Everything needed here is to retrieve needed resources (GPIOs, clocks, regulators) using DT, which should be trivial.

The worse part is the interaction between MMC and power controller driver (the platform driver part of WLAN driver, if you look at brcmfmac as an example). I believe that we need following things:

Quoted 6 lines

a) A way to tell the MMC controller that there is no card detection mechanism available on given slot and it also should not be polling the slot to check card presence. Something like a "manual card detect" that would be triggered by another kernel entity that controls whether the MMC device is present (i.e. WLAN driver). We already have "broken-cd" property, but it only implies the former, wasting time on needless polling.

There is already a host capability that I think we could use to handle this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is "non-removable", and it may be set per host device.

Using this cap means the mmc_rescan process that runs to detect new cards, will only be executed once and during boot. So, we need to make sure all resources and powers are provided to the card at this point. Otherwise the card will not be detected.

In the SDIO case, to save power, the SDIO func driver may use runtime PM to tell the mmc core power about whether the card needs to be powered. Typically from the WLAN driver's probe() and "interface up/down" the runtime PM reference for the SDIO func device, should be adjusted with pm_runtime_get|put.

Quoted 8 lines

b) A mechanism to bind the power controller to used MMC slot. Something like "mmc-bus = <&mmc2>;" property in device node of the power controller and a function like of_find_mmc_controller_by_node(), which would be an MMC counterpart of I2C's of_find_i2c_adapter_by_node(). To avoid races, it should probably take a reference on MMC host that would have to be dropped explicitly whenever it is not needed anymore.

I suppose an "MMC slot" can be translated to "MMC host"?

What I am trying to understand is how the mmc core (or if we push it to be handled from the mmc host's .set_ios callback) shall be able to tell the power controller driver to enable/disable it's resources. Somehow we need the struct device available to handle that. Then I guess operating on it using runtime PM would be a solution that would be quite nice!?

Quoted 6 lines

c) A method to notify the MMC subsystem that card presence has changed. We already have something like this in drivers/mmc/core/slot-gpio.c, but used for a simple GPIO-based card detection. If the main part of mmc_gpio_cd_irqt() could be turned into an exported helper, e.g. mmc_force_card_detect(host) then basically we would have everything needed.

I am not sure I understand why this is needed. I think it would be more convenient to use MMC_CAP_NONREMOVABLE instead as stated earlier. But please elaborate, I might have missed something.

Quoted 8 lines

Unfortunately, I don't have more time left for today to create patches and test them, so for now, I'd like to hear opinion of MMC maintainers about this approach. Do you find this acceptable?

By the way, it seems like slot-gpio.c could replace a lot of custom GPIO card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there any reason why it couldn't?

I suppose most host driver's should convert to the slot-gpio API, it's is just a matter of someone to send the patches. :-)

Quoted 3 lines

I am not sure I understand why this is needed. I think it would be more convenient to use MMC_CAP_NONREMOVABLE instead as stated earlier. But please elaborate, I might have missed something.

See above. I'm not quite convinced that state of MMC interface should determine power state of the chip. I can easily imagine a situation where the MMC link is powered down (link power management) but the WLAN chip keeps operation. Keep in mind that those are usually complete SoCs that can keep processing network traffic autonomously and wake-up the application processor whenever anything interesting happens using extra out of bounds signalling, which might trigger re-enabling the MMC link.

Quoted 109 lines · Ulf Hansson

Quoted 96 lines · Tomasz Figa

Quoted 69 lines · Tomasz Figa

Hi,

Quoted 22 lines · Olof Johansson

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.

Quoted 4 lines

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.

While I fully agree that this is an important problem that needs to be solved, I really don't think this is the right way, because:

a) power-up sequence is really specific to the MMC device and often it's not simply a matter of switching on one regulator or one clock, e.g. specific time constraints need to be met.

b) you can have WLAN chips in which SDIO is just one of the options to use as host interface, which may be also HSIC, I2C or UART. Really. See [1].

c) this is leaking device specific details to generic host code, which isn't really elegant.

Now, to make this a bit more constructive, [2] is a solution that I came up with (not perfect either), which simply adds a separate platform device for the low level part of the chip. I believe this is a better solution because:

a) you can often see such WLAN/BT combo chip as a set of separate devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC, which provides power/reset control, out of band signalling and etc. for the first two, so it isn't that bad to have a separate device node for the last one,

b) you have full freedom of defining your DT binding with whatever data you need, any number of clocks, regulators, GPIOs and even out of band interrupts (IMHO the most important one).

c) you can implement power-on, power-off sequences as needed for your particular device,

d) you have full separation of device-specific data from MMC core (or any other subsystem simply used as a way to perform I/O to the chip).

Quoted 7 lines

Now what's missing there is a way to signal the MMC core or any other transport that a device showed up and the controller should be woken up out of standby and scan of the bus initialized. This could be done by explicitly specifying the device as a subnode of the MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power controller of the chip or the other way around - a link to the MMC/UART/... controller from the power controller node.

I've looked a bit around MMC core code and got some basic idea how things look. I will definitely need some guidance, or at least some opinions, from
MMC guys, as some MMC core changes are unavoidable.

Now, the device-specific code is not really an issue, existing drivers usually already have their ways of powering the chips on and off, based on
platform data. Everything needed here is to retrieve needed resources (GPIOs, clocks, regulators) using DT, which should be trivial.

The worse part is the interaction between MMC and power controller driver (the platform driver part of WLAN driver, if you look at brcmfmac as an example). I believe that we need following things:

Quoted 9 lines

a) A way to tell the MMC controller that there is no card detection mechanism available on given slot and it also should not be polling the slot
to check card presence. Something like a "manual card detect" that would be
triggered by another kernel entity that controls whether the MMC device is
present (i.e. WLAN driver). We already have "broken-cd" property, but it only implies the former, wasting time on needless polling.

Quoted 8 lines

There is already a host capability that I think we could use to handle this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is "non-removable", and it may be set per host device.

Using this cap means the mmc_rescan process that runs to detect new cards, will only be executed once and during boot. So, we need to make sure all resources and powers are provided to the card at this point. Otherwise the card will not be detected.

I don't quite like this requirement, especially if you consider multi-platform kernels where a lot of drivers is going to be provided as modules. WLAN drivers are especially good candidates. This means that even if the card is powered off at boot-up, if user (or init system) loads appropriate module, which powers the chip on, MMC core must be able to notice this.

Quoted 7 lines

In the SDIO case, to save power, the SDIO func driver may use runtime PM to tell the mmc core power about whether the card needs to be powered. Typically from the WLAN driver's probe() and "interface up/down" the runtime PM reference for the SDIO func device, should be adjusted with pm_runtime_get|put.

I need to think a bit more about the power management control flow here. In case of such chips I'd tend to look at MMC merely as a host interface, which as I said, might be only one of available options. I'm not sure if it should be the host interface core that decides whether the whole device should be powered off. However there might be a solution that leverages SDIO func runtime PM, which wouldn't imply such control flow. Let me reconsider this.

Quoted 14 lines

Quoted 12 lines

b) A mechanism to bind the power controller to used MMC slot. Something like
"mmc-bus = <&mmc2>;" property in device node of the power controller and a
function like of_find_mmc_controller_by_node(), which would be an MMC counterpart of I2C's of_find_i2c_adapter_by_node(). To avoid races, it should probably take a reference on MMC host that would have to be dropped
explicitly whenever it is not needed anymore.

Quoted 1 line

I suppose an "MMC slot" can be translated to "MMC host"?

Right.

Quoted 6 lines

What I am trying to understand is how the mmc core (or if we push it to be handled from the mmc host's .set_ios callback) shall be able to tell the power controller driver to enable/disable it's resources. Somehow we need the struct device available to handle that. Then I guess operating on it using runtime PM would be a solution that would be quite nice!?

As I wrote above, I'm not quite sure about this yet.

Quoted 11 lines

Quoted 9 lines

c) A method to notify the MMC subsystem that card presence has changed. We
already have something like this in drivers/mmc/core/slot-gpio.c, but used
for a simple GPIO-based card detection. If the main part of mmc_gpio_cd_irqt() could be turned into an exported helper, e.g. mmc_force_card_detect(host) then basically we would have everything needed.

Quoted 3 lines

I am not sure I understand why this is needed. I think it would be more convenient to use MMC_CAP_NONREMOVABLE instead as stated earlier. But please elaborate, I might have missed something.

See above. I'm not quite convinced that state of MMC interface should determine power state of the chip. I can easily imagine a situation where the MMC link is powered down (link power management) but the WLAN chip keeps operation. Keep in mind that those are usually complete SoCs that can keep processing network traffic autonomously and wake-up the application processor whenever anything interesting happens using extra out of bounds signalling, which might trigger re-enabling the MMC link.

Quoted 8 lines

Quoted 8 lines

Unfortunately, I don't have more time left for today to create patches and
test them, so for now, I'd like to hear opinion of MMC maintainers about this approach. Do you find this acceptable?

Quoted 8 lines

Quoted 4 lines

By the way, it seems like slot-gpio.c could replace a lot of custom GPIO card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there any
reason why it couldn't?

I suppose most host driver's should convert to the slot-gpio API, it's is just a matter of someone to send the patches. :-)

OK, great. I'll add conversion of sdhci-s3c to my queue then.

Best regards,
Tomasz

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

Quoted 10 lines · Tomasz Figa

Quoted 8 lines · Ulf Hansson

There is already a host capability that I think we could use to handle this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is "non-removable", and it may be set per host device.

Using this cap means the mmc_rescan process that runs to detect new cards, will only be executed once and during boot. So, we need to make sure all resources and powers are provided to the card at this point. Otherwise the card will not be detected.

Quoted 6 lines

I don't quite like this requirement, especially if you consider multi-platform kernels where a lot of drivers is going to be provided as modules. WLAN drivers are especially good candidates. This means that even if the card is powered off at boot-up, if user (or init system) loads appropriate module, which powers the chip on, MMC core must be able to notice this.

To be able to detect the card, the WLAN driver doesn't have to be probed, only the "power controller" driver. I suppose this is were it becomes a bit tricky.

Somehow the mmc core needs to be involved in the probe process of the power controller driver. Could perhaps the power controller bus be located in the mmc core and thus the power controller driver needs to register itself by using a new API from the mmc core? Similar how SDIO func driver's register themselves.

I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver if we allow a secondary detection path using DT rather than the probing of the SDIO bus. Essentially you'd have to list the class/vendor/device ID for each function that cannot be autodetected in DT, and have the SDIO core pretend that it found the device just by looking at the device nodes, and register the struct sdio_func so it can be bound to the driver. The driver then does all the power handling in a device specific way. At some point the hardware gets registered at the mmc host, and the sdio core connects the bus state to the already present sdio_func, possibly notifying the function driver that it has become usable.

Obviously, this can only work for CAP_NONREMOVABLE devices, but those are exactly the ones we are worried about here. The advantage is that the power sequencing for a particular device can then be in device specific code and can have an arbitrarily complex in the driver without needing the mmc code to handle all possible corner cases.

Quoted 22 lines · Ulf Hansson

Quoted 10 lines · Tomasz Figa

Quoted 8 lines · Ulf Hansson

There is already a host capability that I think we could use to handle this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is "non-removable", and it may be set per host device.

Using this cap means the mmc_rescan process that runs to detect new cards, will only be executed once and during boot. So, we need to make sure all resources and powers are provided to the card at this point. Otherwise the card will not be detected.

Quoted 6 lines

I don't quite like this requirement, especially if you consider multi-platform kernels where a lot of drivers is going to be provided as modules. WLAN drivers are especially good candidates. This means that even if the card is powered off at boot-up, if user (or init system) loads appropriate module, which powers the chip on, MMC core must be able to notice this.

To be able to detect the card, the WLAN driver doesn't have to be probed, only the "power controller" driver. I suppose this is were it becomes a bit tricky.

Quoted 5 lines

Somehow the mmc core needs to be involved in the probe process of the power controller driver. Could perhaps the power controller bus be located in the mmc core and thus the power controller driver needs to register itself by using a new API from the mmc core? Similar how SDIO func driver's register themselves.

I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver if we allow a secondary detection path using DT rather than the probing of the SDIO bus. Essentially you'd have to list the class/vendor/device ID for each function that cannot be autodetected in DT, and have the SDIO core pretend that it found the device just by looking at the device nodes, and register the struct sdio_func so it can be bound to the driver. The driver then does all the power handling in a device specific way. At some point the hardware gets registered at the mmc host, and the sdio core connects the bus state to the already present sdio_func, possibly notifying the function driver that it has become usable.

Obviously, this can only work for CAP_NONREMOVABLE devices, but those are exactly the ones we are worried about here. The advantage is that the power sequencing for a particular device can then be in device specific code and can have an arbitrarily complex in the driver without needing the mmc code to handle all possible corner cases.

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

Quoted 29 lines · Arnd Bergmann

Quoted 22 lines · Ulf Hansson

Quoted 10 lines · Tomasz Figa

Quoted 8 lines · Ulf Hansson

There is already a host capability that I think we could use to handle this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is "non-removable", and it may be set per host device.

Using this cap means the mmc_rescan process that runs to detect new cards, will only be executed once and during boot. So, we need to make sure all resources and powers are provided to the card at this point. Otherwise the card will not be detected.

Quoted 6 lines

I don't quite like this requirement, especially if you consider multi-platform kernels where a lot of drivers is going to be provided as modules. WLAN drivers are especially good candidates. This means that even if the card is powered off at boot-up, if user (or init system) loads appropriate module, which powers the chip on, MMC core must be able to notice this.

To be able to detect the card, the WLAN driver doesn't have to be probed, only the "power controller" driver. I suppose this is were it becomes a bit tricky.

Quoted 5 lines

Somehow the mmc core needs to be involved in the probe process of the power controller driver. Could perhaps the power controller bus be located in the mmc core and thus the power controller driver needs to register itself by using a new API from the mmc core? Similar how SDIO func driver's register themselves.

Quoted 12 lines

I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver if we allow a secondary detection path using DT rather than the probing of the SDIO bus. Essentially you'd have to list the class/vendor/device ID for each function that cannot be autodetected in DT, and have the SDIO core pretend that it found the device just by looking at the device nodes, and register the struct sdio_func so it can be bound to the driver. The driver then does all the power handling in a device specific way. At some point the hardware gets registered at the mmc host, and the sdio core connects the bus state to the already present sdio_func, possibly notifying the function driver that it has become usable.

It seems like an option we could try.

There are some tricky parts that needs to be taken care of by the mmc/sdio core, regarding the probe/removal and the suspend/resume sequence, but I suppose it should be possible.

A minor concern is how do we handle devices that are fully powered at boot? Unless the sdio func driver will be loaded we can't power off them, right? Do we need to cover this case, do you think?

Quoted 6 lines

Obviously, this can only work for CAP_NONREMOVABLE devices, but those are exactly the ones we are worried about here. The advantage is that the power sequencing for a particular device can then be in device specific code and can have an arbitrarily complex in the driver without needing the mmc code to handle all possible corner cases.

Agree!

I think a removable SDIO card won't l need this additional power controller mechanism.

I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver if we allow a secondary detection path using DT rather than the probing of the SDIO bus.

What if we have a platform where things subtly change, like for instance, the wiring on the SD slot to fix a problem with UHS-1 cards, which means you don't have UHS-1 support for some platforms but do for others.

What if you have a platform which uses a brcm4329 chip for Wifi, but then later in the production run switch to using a different Wifi chipset?

With this information encoded into DT, the number of DT files quickly increases, and then this presents its own problem - how do users get to know which DT file should be used for their platform when all they see externally is "a product of type A"?

Quoted 7 lines · On Thursday 13 February 2014 10:42:48 Russell King - ARM Linux wrote:

What if we have a platform where things subtly change, like for instance, the wiring on the SD slot to fix a problem with UHS-1 cards, which means you don't have UHS-1 support for some platforms but do for others.

What if you have a platform which uses a brcm4329 chip for Wifi, but then later in the production run switch to using a different Wifi chipset?

As far as I can tell, the power sequencing is normally really dependent on the device. If someone has an on-board brcm4329 that requires a specific set of clocks, resets, voltages etc to be routed to the chip and enabled in the correct order to allow probing, it seems unlikely that changing the wifi chipset to something else would keep the exact same requirements.

Quoted 9 lines · Arnd Bergmann

Quoted 7 lines · On Thursday 13 February 2014 10:42:48 Russell King - ARM Linux wrote:

What if we have a platform where things subtly change, like for instance, the wiring on the SD slot to fix a problem with UHS-1 cards, which means you don't have UHS-1 support for some platforms but do for others.

What if you have a platform which uses a brcm4329 chip for Wifi, but then later in the production run switch to using a different Wifi chipset?

Quoted 6 lines

As far as I can tell, the power sequencing is normally really dependent on the device. If someone has an on-board brcm4329 that requires a specific set of clocks, resets, voltages etc to be routed to the chip and enabled in the correct order to allow probing, it seems unlikely that changing the wifi chipset to something else would keep the exact same requirements.

That's your assertion - however, do we /know/ whether there's a situation where Olof's solution doesn't work because the sequencing is wrong?

I see nothing unreasonable about the sequence:

1. hold reset at low level
2. apply power
3. turn clock on
4. apply reset
5. release reset

The points being:
* never set a signal to a high level before power is applied, otherwise we can end up supplying power through that signal (which may cause damage.) That goes for the reset and clock.
* devices normally want clocks running to complete their reset sequencing.

This is actually the sequence which MMC/SD cards do (except without the reset) anyway - it's spec'd that on the MMC/SD bus, power will be applied and will be stable before the clock signal is applied, and then the clock signal runs for a certain number of cycles before you even start talking to the card.

That all said, we do have the problem that once we decide, we need to support it because it becomes part of DT - this is one of the things I hate about DT, it requires over-design.

Your point is "Olof's solution
may break if we have a device which requires a different sequence" which is a valid point which has to be considered from the DT perspective and addressed whether or not we actually have a device which meets that criteria. I don't see an easy solution to this.

Quoted 20 lines · On Thursday 13 February 2014 14:41:06 Russell King - ARM Linux wrote:

Quoted 9 lines · Arnd Bergmann

Quoted 7 lines · On Thursday 13 February 2014 10:42:48 Russell King - ARM Linux wrote:

What if we have a platform where things subtly change, like for instance, the wiring on the SD slot to fix a problem with UHS-1 cards, which means you don't have UHS-1 support for some platforms but do for others.

What if you have a platform which uses a brcm4329 chip for Wifi, but then later in the production run switch to using a different Wifi chipset?

Quoted 6 lines

As far as I can tell, the power sequencing is normally really dependent on the device. If someone has an on-board brcm4329 that requires a specific set of clocks, resets, voltages etc to be routed to the chip and enabled in the correct order to allow probing, it seems unlikely that changing the wifi chipset to something else would keep the exact same requirements.

That's your assertion - however, do we /know/ whether there's a situation where Olof's solution doesn't work because the sequencing is wrong?

Quoted 7 lines

I see nothing unreasonable about the sequence:

1. hold reset at low level
2. apply power
3. turn clock on
4. apply reset
5. release reset

I was thinking of cases where you may need a more complex sequence:
- wait for a device specific time between some of the steps (the cw1200 driver seems to need that, but we could probably get away with waiting long enough for everyone)
- have more than one of each, and turn them on in the right order. cw1200 seems to need two voltages, two gpio resets ("reset" and "powerup").

Again, we could specify a larger number of clocks that can be provided to the host, but if we make it a device specific property, we already know how many we need.

I can't think of anything that would require significant changes to the procedure though, just refinements as we run into problems.

Quoted 7 lines

The points being:
* never set a signal to a high level before power is applied, otherwise we can end up supplying power through that signal (which may cause damage.) That goes for the reset and clock.
* devices normally want clocks running to complete their reset sequencing.

Quoted 5 lines

This is actually the sequence which MMC/SD cards do (except without the reset) anyway - it's spec'd that on the MMC/SD bus, power will be applied and will be stable before the clock signal is applied, and then the clock signal runs for a certain number of cycles before you even start talking to the card.

It may be dangerous to refer to the spec, since we are talking specifically about devices that require something beyond what the spec says ;-) For instance in SD/MMC cards I'd assume the device clock to be derived from the bus clock. However we can expect that clock to work already (any working mmc host driver would provide that), but we may need to drive the device clock. It still sounds reasonable to assume that the sequencing is the same as for the bus clock.

Quoted 3 lines

That all said, we do have the problem that once we decide, we need to support it because it becomes part of DT - this is one of the things I hate about DT, it requires over-design.

Yes, I agree. It is a problem that we have to face all the time. We have in the past defined bindings of both types, overdesigned and not thought through enough.

Quoted 5 lines

Your point is "Olof's solution
may break if we have a device which requires a different sequence" which is a valid point which has to be considered from the DT perspective and addressed whether or not we actually have a device which meets that criteria. I don't see an easy solution to this.

I think either one will work. With Olof's suggestion that may mean we have to keep adding support for increasingly complex cases when we run into them, or it may all be easy. With my suggestion, we give more room for function drivers to mess things up, but at least we can keep the complexity in the places that need them and only need to change the core once.

Aside from the power-on problem, my suggestion would at the same time solve the second problem of having a place to stick arbitrary DT properties for the sdio function. Again looking at the cw1200 example, they may require passing an IRQ descriptor, a MAC address, the device clock rate, and two flags for things that are not detectable by looking at the device ID (whether a 5GHz antenna is connected and something about odd block size transfers). This is probably the main difference between the two approaches.

Aside from the power-on problem, my suggestion would at the same time solve the second problem of having a place to stick arbitrary DT properties for the sdio function. Again looking at the cw1200 example, they may require passing an IRQ descriptor, a MAC address, the device clock rate, and two flags for things that are not detectable by looking at the device ID (whether a 5GHz antenna is connected and something about odd block size transfers). This is probably the main difference between the two approaches.

If a wlan adapter has both SPI and SDIO front-ends, the external dependencies (reset, clock, voltage, ...) will be the same, and from the kernel perspective the main difference is that SPI cannot be probed at all, while SDIO can be probed as long as the device is powered on already.

Quoted 5 lines · Arnd Bergmann

If a wlan adapter has both SPI and SDIO front-ends, the external dependencies (reset, clock, voltage, ...) will be the same, and from the kernel perspective the main difference is that SPI cannot be probed at all, while SDIO can be probed as long as the device is powered on already.

Remember that MMC/SD/SDIO cards can be driven by either a MMC host interface, or a SPI interface. Both are probe-able.

Quoted 9 lines · On Saturday 15 February 2014 12:27:33 Russell King - ARM Linux wrote:

Quoted 5 lines · Arnd Bergmann

If a wlan adapter has both SPI and SDIO front-ends, the external dependencies (reset, clock, voltage, ...) will be the same, and from the kernel perspective the main difference is that SPI cannot be probed at all, while SDIO can be probed as long as the device is powered on already.

Remember that MMC/SD/SDIO cards can be driven by either a MMC host interface, or a SPI interface. Both are probe-able.

I knew about MMC/SD cards being required to understand simple SPI, I wasn't sure about SDIO. My understanding however is that you have to use the mmc_spi host driver to actually use MMC/SD devices as a block device, and that requires having either a DT description for the host or an spi_board_info, which I would not consider discoverable.

For spi-mode SDIO devices I'm assuming it's similar, except that you'd describe the actual SDIO device in the board info rather than create a fake SDIO controller. Still not discoverable unless I'm missing your point.

Quoted 4 lines · Arnd Bergmann

For spi-mode SDIO devices I'm assuming it's similar, except that you'd describe the actual SDIO device in the board info rather than create a fake SDIO controller. Still not discoverable unless I'm missing your point.

I'm not sure if we should assume that SPI = MMC over SPI. I believe there might be a custom protocol involved as well.

Stepping aside from SPI, I already gave an example of a WLAN chip that supports multiple control busses [1]. In addition to the commonly used SDIO, it supports USB and HSIC as well:

[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf

Moreover, some of Samsung boards use HSIC to communicate with modem chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.

So I really don't think we should be limiting this to MMC alone by any means.

Now I don't really know why we want that badly to represent low level control parts of such devices as children of control buses of their enumerable parts. Could you tell me what benefits it has to justify the added complexity of having to instantiate fake devices in respective devices, even though they can be fully detected later?

Quoted 7 lines · On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:

Quoted 4 lines · Arnd Bergmann

For spi-mode SDIO devices I'm assuming it's similar, except that you'd describe the actual SDIO device in the board info rather than create a fake SDIO controller. Still not discoverable unless I'm missing your point.

Quoted 2 lines

I'm not sure if we should assume that SPI = MMC over SPI. I believe there might be a custom protocol involved as well.

In case of SD/MMC, you essentially have three separate command sets:
SPI, MMC and SD, and each of them has multiple versions. MMC and SD compatible devices generally also support the SPI command set (IIRC it is required,

If a device supports both SDIO and SPI, I think a straightforward implementation would be to use the exact same command set, but you are right that this isn't the only possibility, and the SD/MMC shows how they can be slightly different already.

Quoted 4 lines

Stepping aside from SPI, I already gave an example of a WLAN chip that supports multiple control busses [1]. In addition to the commonly used SDIO, it supports USB and HSIC as well:

Quoted 5 lines

[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf

Moreover, some of Samsung boards use HSIC to communicate with modem chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.

Thanks, this definitely makes a good example. I see that it also supports SPI mode for SDIO as mentioned in your link.

Agreed. Putting the same chip on USB or HSIC has the exact same requirements, since we also have a discoverable bus, but actually finding the device likely involves some power-on sequencing before the bus controller can find it.

* Arnd's proposal (change bus code to probe nonstandard devices from DT if we can't easily detect them):
+ Matches what we already do for PCI (at least on powerpc) and AMBA/Primecell devices: If a device can't be probed using the standard method, we treat it as nondiscoverable and describe it using DT.
+ Devices can have arbitrary complex requirements without impacting the core, since all code is contained in the driver for the nonstandard device.
+ Properties that are required for probing and runtime configuration only have to be set once (e.g. you may need clk_get() for probing and clk_set_rate() for runtime-pm).
+ Devices that have alternative bus interfaces like 88w8797 can implement the power-on code in a central place per driver, and can reuse the code they have for nondiscoverable

buses on the buses that are normally discoverable but broken here.
- Still need to modify each subsystem to have alternate ways of probing, and match up devices later.
- Has to be implemented in each driver that needs it, making it harder to share code for drivers with the same need (e.g. every device that just needs an external reset trigger).

Quoted 8 lines · Arnd Bergmann

Quoted 7 lines · On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:

Quoted 4 lines · Arnd Bergmann

For spi-mode SDIO devices I'm assuming it's similar, except that you'd describe the actual SDIO device in the board info rather than create a fake SDIO controller. Still not discoverable unless I'm missing your point.

Quoted 7 lines

Quoted 2 lines

I'm not sure if we should assume that SPI = MMC over SPI. I believe there might be a custom protocol involved as well.

In case of SD/MMC, you essentially have three separate command sets:
SPI, MMC and SD, and each of them has multiple versions. MMC and SD compatible devices generally also support the SPI command set (IIRC it is required,

SPI support is mandatory for SDIO as well.

SDIO has CIS (remember card information structures... like PCMCIA) which identifies the various different logical functions of the device, giving class information, vendor information etc.

So... certainly the type of the attached device is discoverable even on SPI.

Quoted 4 lines

If a device supports both SDIO and SPI, I think a straightforward implementation would be to use the exact same command set, but you are right that this isn't the only possibility, and the SD/MMC shows how they can be slightly different already.

Given that the SPI mode is mandatory for SDIO cards, why would you also implement another SPI mode with different commands?

Quoted 4 lines

Quoted 4 lines

Stepping aside from SPI, I already gave an example of a WLAN chip that supports multiple control busses [1]. In addition to the commonly used SDIO, it supports USB and HSIC as well:

Quoted 8 lines

Quoted 5 lines

[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf

Moreover, some of Samsung boards use HSIC to communicate with modem chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.

Thanks, this definitely makes a good example. I see that it also supports SPI mode for SDIO as mentioned in your link.

Well, USB is another discoverable bus. As HSIC is a derivative of USB, I'd be surprised if it weren't discoverable there too.

So, out of everything identified so far, we have no undiscoverable buses.

Quoted 4 lines

Agreed. Putting the same chip on USB or HSIC has the exact same requirements, since we also have a discoverable bus, but actually finding the device likely involves some power-on sequencing before the bus controller can find it.

That's partly the nature of integrating something onto a board where you want maximal power savings. It's basically that dreaded word which software people seem to hate: "embedded".

Quoted 16 lines

* Arnd's proposal (change bus code to probe nonstandard devices from DT if we can't easily detect them):
+ Matches what we already do for PCI (at least on powerpc) and AMBA/Primecell devices: If a device can't be probed using the standard method, we treat it as nondiscoverable and describe it using DT.
+ Devices can have arbitrary complex requirements without impacting the core, since all code is contained in the driver for the nonstandard device.
+ Properties that are required for probing and runtime configuration only have to be set once (e.g. you may need clk_get() for probing and clk_set_rate() for runtime-pm).
+ Devices that have alternative bus interfaces like 88w8797 can implement the power-on code in a central place per driver, and can reuse the code they have for nondiscoverable

Quoted 8 lines

buses on the buses that are normally discoverable but broken here.
- Still need to modify each subsystem to have alternate ways of probing, and match up devices later.
- Has to be implemented in each driver that needs it, making it harder to share code for drivers with the same need (e.g. every device that just needs an external reset trigger).

- requires different DT if the chip is changed, which causes problems for users to identify which out of zillions of DT files they should use for their exact platform.

- have to work out how to match up the fake device with a probed device when it becomes available: existing SDIO drivers all assume that the card has been through a fairly complex initialisation sequence already.
- multi-function SDIO is much harder to deal with since you have mutliple drivers involved, and the SDIO device as a whole needs initialisation before anyone can drive it.
- adds complexity to the SDIO drivers; they would have to know whether they're embedded or on a plug-in card.

If you change your hardware in an incompatible way and such change can't be detected automatically (i.e. the chip is non-discoverable) then I don't know how you could let the OS know about this change in any other way than providing it with the information it needs.

Quoted 8 lines · Arnd Bergmann

Quoted 7 lines · On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:

Quoted 4 lines · Arnd Bergmann

For spi-mode SDIO devices I'm assuming it's similar, except that you'd describe the actual SDIO device in the board info rather than create a fake SDIO controller. Still not discoverable unless I'm missing your point.

Quoted 7 lines

Quoted 2 lines

I'm not sure if we should assume that SPI = MMC over SPI. I believe there might be a custom protocol involved as well.

In case of SD/MMC, you essentially have three separate command sets:
SPI, MMC and SD, and each of them has multiple versions. MMC and SD compatible devices generally also support the SPI command set (IIRC it is required,

SPI support is mandatory for SDIO as well.

SDIO has CIS (remember card information structures... like PCMCIA) which identifies the various different logical functions of the device, giving class information, vendor information etc.

So... certainly the type of the attached device is discoverable even on SPI

Quoted 4 lines

If a device supports both SDIO and SPI, I think a straightforward implementation would be to use the exact same command set, but you are right that this isn't the only possibility, and the SD/MMC shows how they can be slightly different already.

Given that the SPI mode is mandatory for SDIO cards, why would you also implement another SPI mode with different commands?

Quoted 10 lines

Quoted 9 lines

Stepping aside from SPI, I already gave an example of a WLAN chip that supports multiple control busses [1]. In addition to the commonly used SDIO, it supports USB and HSIC as well:

[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf

Moreover, some of Samsung boards use HSIC to communicate with modem chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.

Quoted 2 lines

Thanks, this definitely makes a good example. I see that it also supports SPI mode for SDIO as mentioned in your link.

Well, USB is another discoverable bus. As HSIC is a derivative of USB, I'd be surprised if it weren't discoverable there too.

So, out of everything identified so far, we have no undiscoverable buses.

- requires different DT if the chip is changed, which causes problems for users to identify which out of zillions of DT files they should use for their exact platform.

- have to work out how to match up the fake device with a probed device when it becomes available: existing SDIO drivers all assume that the card has been through a fairly complex initialisation sequence already.

- multi-function SDIO is much harder to deal with since you have mutliple drivers involved, and the SDIO device as a whole needs initialisation before anyone can drive it.

- adds complexity to the SDIO drivers; they would have to know whether they're embedded or on a plug-in card.

Quoted 2 lines · On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:

I'm not sure if we should assume that SPI = MMC over SPI. I believe there might be a custom protocol involved as well.

In case of SD/MMC, you essentially have three separate command sets:
SPI, MMC and SD, and each of them has multiple versions. MMC and SD compatible devices generally also support the SPI command set (IIRC

If a device supports both SDIO and SPI, I think a straightforward implementation would be to use the exact same command set, but you are right that this isn't the only possibility, and the SD/MMC shows how they can be slightly different already.

Quoted 3 lines

Stepping aside from SPI, I already gave an example of a WLAN chip that supports multiple control busses [1]. In addition to the commonly used SDIO, it supports USB and HSIC as well:

Quoted 5 lines

[1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf

Moreover, some of Samsung boards use HSIC to communicate with modem chips, which have exactly the same problem as we're trying to solve here
- they need to be powered on to be discovered.

Thanks, this definitely makes a good example. I see that it also supports SPI mode for SDIO as mentioned in your link.

* Olof's proposal (add properties or a child node to the host controller node with just power-on sequencing information):
+ We only need one implementation for each bus, possibly shared across buses to some degree, and can handle lots of devices without having to touch their individual drivers.

+ A logical extension of things we already do on SD cards (CD/WP GPIOs, external clocks and voltages supplied to standard compliant devices as part of the normal probing)
- The shared code may get rather complex to deal with all possible corner cases we run into over the years.
- Somewhat harder to do if you have to attach the power information to a device node for a USB hub port, rather than an SDIO controller that only has one slave device.

* Arnd's proposal (change bus code to probe nonstandard devices from DT if we can't easily detect them):

- Has to be implemented in each driver that needs it, making it harder to share code for drivers with the same need (e.g. every device that just needs an external reset trigger).

The shared of parse function fills in common options for capabilities, etc, but it needs to be called from each driver that wants to make use of it. dw_mmc was missing the call.

Signed-off-by Olof Johansson <[email protected]>

---
 drivers/mmc/host/dw_mmc.c |    2 ++
 1 file changed, 2 insertions(+)

drivers/mmc/host/dw_mmc.c +2 -0

index a776f24..7119f63 100644
@@ -2137,6 +2137,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	if (!mmc)
 		return -ENOMEM;
 
+	mmc_of_parse(mmc);
+
 	slot = mmc_priv(mmc);
 	slot->id = id;
 	slot->mmc = mmc;

This is a small series enhancing the MMC core code to power on modules before the host in cases where needed, and the corresponding DT bindings changes.

I've got some other issues to debug on the Chromebook, i.e. the interface doens't actually work. So far it seems unrelated to this patch set so it's worth posting this and get things going since others need the same functionality (i.e Cubox-i).

As mentioned in the patch in the series, I haven't implemented power-down yet, I wanted to make sure that the power-on side will be adequate for those who are looking to use it right away.

Comments/test reports/etc welcome.

Quoted 9 lines · Olof Johansson

This is a small series enhancing the MMC core code to power on modules before the host in cases where needed, and the corresponding DT bindings changes.

I've got some other issues to debug on the Chromebook, i.e. the interface doens't actually work. So far it seems unrelated to this patch set so it's worth posting this and get things going since others need the same functionality (i.e Cubox-i).

Quoted 5 lines

As mentioned in the patch in the series, I haven't implemented power-down yet, I wanted to make sure that the power-on side will be adequate for those who are looking to use it right away.

Comments/test reports/etc welcome.

So, I thought I'd give this a go on the Cubox-i4, and... it doesn't work there. It's not your patches, it's down to sdhci-esdhc-imx.c not using mmc_of_parse() at all, so those new properties have no way to be used there.

It doesn't look like it could in its current form use mmc_of_parse(), as the imx code manually parses some of the generic properties to hand them into the sdhci layer. This looks icky, and it looks like something that should be fixed - why should drivers be parsing the core attributes themselves?

Quoted 21 lines · Russell King - ARM Linux

Quoted 9 lines · Olof Johansson

This is a small series enhancing the MMC core code to power on modules before the host in cases where needed, and the corresponding DT bindings changes.

I've got some other issues to debug on the Chromebook, i.e. the interface doens't actually work. So far it seems unrelated to this patch set so it's worth posting this and get things going since others need the same functionality (i.e Cubox-i).

Quoted 5 lines

As mentioned in the patch in the series, I haven't implemented power-down yet, I wanted to make sure that the power-on side will be adequate for those who are looking to use it right away.

Comments/test reports/etc welcome.

So, I thought I'd give this a go on the Cubox-i4, and... it doesn't work there. It's not your patches, it's down to sdhci-esdhc-imx.c not using mmc_of_parse() at all, so those new properties have no way to be used there.

Quoted 5 lines

It doesn't look like it could in its current form use mmc_of_parse(), as the imx code manually parses some of the generic properties to hand them into the sdhci layer. This looks icky, and it looks like something that should be fixed - why should drivers be parsing the core attributes themselves?

Here's an illustration of why it's icky.

If we call mmc_of_parse() in the sdhci-esdhc-imx driver (which we'd need to do in order to get information on how to configure the card detection etc) then this fills in mmc->f_max.

However, the subsequent call to sdhci_add_host() computes the maximum clock from the sdhci capabilities, and then does this:

        host->max_clk *= 1000000;
        if (host->max_clk == 0 || host->quirks &
                        SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN) {
                if (!host->ops->get_max_clock) {
                        pr_err("%s: Hardware doesn't specify base clock "
                               "frequency.\n", mmc_hostname(mmc));
                        return -ENODEV;
                }
                host->max_clk = host->ops->get_max_clock(host);
        }
...
        /*
         * Set host parameters.
         */
        mmc->ops = &sdhci_ops;
        mmc->f_max = host->max_clk;

which would have the effect of overwriting a previously set f_max from the OF data.

There's also the whole "cd-gpios" thing which would need sorting out - the imx sdhci driver already parses this property itself, and sets its own internal data (so it knows whether it has to use the controller based card detect or the gpio card detect) and simply adding a call to mmc_of_parse() would result in the gpio slot stuff being setup twice.

The obvious solution here is to rewrite the sdhci initialisation such that it uses the generic infrastructure, but I don't have the motivation to do that (I've already plenty of patches to deal with that I don't need any more at the moment.)

A simpler solution would be to split mmc_of_parse() so that the new bits are a separate function, which the generic MMC core always calls for every host - taking the decision over whether this is supported completely away from hosts. I think that makes a lot of sense, especially as this has nothing to do with the facilities found on any particular host.

There's another issue here about resets. Let's take the case where the external card is powered off, but has active high resets. At the moment, the sequence is this:

power: _____/~~~~~~~~~~~~
reset: __/~~~~\__________

That's not particularly nice, as the reset signal will tend do drive power into the device before it's powered up via the clamping diodes in the case. Generally, devices are not designed to be powered in this way. However, this is a relatively minor issue though compared to this one, which is what happens if the card uses active low reset:

power: _____/~~~~~~~~~~~~
reset: ~~\_____/~~~~~~~~~

This is definitely not good, because it means that the reset is higher for longer, which may result in unacceptable dissapation in the package from those clamping diodes. What we need instead is for active low reset is:

power: _____/~~~~~~~~~~~~
reset: ________/~~~~~~~~~

So, we need the GPIO layer to tell us whether the output is active high or active low and adjust the initial setting accordingly. Basically, whenever the attached device is powered down, GPIOs to it should be held at low level or high impedance (with a pull-down to reduce the risks of ESD damage.)

I've seen designs get this wrong in the past - Intel Assabet is a good one where the UDA1341 audio codec ends up illuminating a LED by being powered not via it's supply pin, but by a CPLD output driving one of the I2S pins high. The result is that the CPLD output sources quite a bit of current into the UDA1341, which then holds other pins on the SA1110 around mid-rail, which is the /worst/ thing you can do with CMOS. Powering chips via their inputs is basically a big no-no.

So, I think something like the below is needed on top of your patches. Note that I added -EPROBE_DEFER handling too (which fixes a bug, because regulator_get() returns pointer-errors):

 drivers/mmc/core/host.c            | 90 +++++++++++++++++++++++++++-----------
 1 files changed, 65 insertions(+), 25 deletions(-)

drivers/mmc/core/host.c +65 -25

index e6b850b3241f..64942eb495b6 100644
@@ -316,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 i, len, ret, gpio;
+	int len, ret, gpio;
 
 	if (!host->parent || !host->parent->of_node)
 		return 0;

@@ -419,30 +419,6 @@ 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 */
-	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");
-	if (IS_ERR(host->card_clk))
-		host->card_clk = NULL;
-
-	host->card_regulator = regulator_get(host->parent, "card-external-vcc");
-
 	if (of_find_property(np, "cap-sd-highspeed", &len))
 		host->caps |= MMC_CAP_SD_HIGHSPEED;
 	if (of_find_property(np, "cap-mmc-highspeed", &len))

@@ -467,6 +443,66 @@ int mmc_of_parse(struct mmc_host *host)
 
 EXPORT_SYMBOL(mmc_of_parse);
 
+static int mmc_of_parse_child(struct mmc_host *host)
+{
+	struct device_node *np;
+	struct clk *clk;
+	int i;
+
+	if (!host->parent || !host->parent->of_node)
+		return 0;
+
+	np = host->parent->of_node;
+
+	host->card_regulator = regulator_get(host->parent, "card-external-vcc");
+	if (IS_ERR(host->card_regulator)) {
+		if (PTR_ERR(host->card_regulator) == -EPROBE_DEFER)
+			return PTR_ERR(host->card_regulator);
+		host->card_regulator = NULL;
+	}
+
+	/* Parse card power/reset/clock control */
+	if (of_find_property(np, "card-reset-gpios", NULL)) {
+		struct gpio_desc *gpd;
+		int level = 0;
+
+		/*
+		 * If the regulator is enabled, then we can hold the
+		 * card in reset with an active high resets.  Otherwise,
+		 * hold the resets low.
+		 */
+		if (host->card_regulator && regulator_is_enabled(host->card_regulator))
+			level = 1;
+
+		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)) {
+				if (PTR_ERR(gpd) == -EPROBE_DEFER)
+					return PTR_ERR(gpd);
+				break;
+			}
+			gpiod_direction_output(gpd, gpiod_is_active_low(gpd) | level);
+			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);
+		}
+	}
+
+	clk = of_clk_get_by_name(np, "card_ext_clock");
+	if (IS_ERR(clk)) {
+		if (PTR_ERR(clk) == -EPROBE_DEFER)
+			return PTR_ERR(clk);
+		clk = NULL;
+	}
+	host->card_clk = clk;
+
+	return 0;
+}
+
 /**
  *	mmc_alloc_host - initialise the per-host structure.
  *	@extra: sizeof private data structure

@@ -546,6 +582,10 @@ int mmc_add_host(struct mmc_host *host)
 {
 	int err;
 
+	err = mmc_of_parse_child(host);
+	if (err)
+		return err;
+
 	WARN_ON((host->caps & MMC_CAP_SDIO_IRQ) &&
 		!host->ops->enable_sdio_irq);
 

Signature · 9 lines

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel