* [PATCH 1/4] mmc: core: Initial support for MMC power sequences
2015-01-02 16:14 [PATCH 0/4] mmc: core: Add support for MMC power sequences Ulf Hansson
@ 2015-01-02 16:14 ` Ulf Hansson
2015-01-02 16:14 ` [PATCH 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence Ulf Hansson
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2015-01-02 16:14 UTC (permalink / raw)
To: linux-arm-kernel
System on chip designs may specify a specific MMC power sequence. To
successfully detect an (e)MMC/SD/SDIO card, that power sequence must
be followed while initializing the card.
To be able to handle these SOC specific power sequences, let's add a
MMC power sequence interface. It provides the following functions to
help the mmc core to deal with these power sequences.
mmc_pwrseq_alloc() - Invoked from mmc_of_parse(), to initialize data.
mmc_pwrseq_power_up()- Invoked from mmc_power_up(), to power up.
mmc_pwrseq_power_on()- Invoked from mmc_power_up(), to power on.
mmc_pwrseq_power_off()- Invoked from mmc_power_off(), to power off.
mmc_pwrseq_free() - Invoked from mmc_free_host(), to free data.
Each MMC power sequence provider will be responsible to implement a set
of callbacks. These callbacks mirrors the functions above.
This patch adds the skeleton, following patches will extend the core of
the MMC power sequence and add support for a specific simple MMC power
sequence.
Do note, since the mmc_pwrseq_alloc() is invoked from mmc_of_parse(),
host drivers needs to make use of this API to enable the support for
MMC power sequences. Moreover the MMC power sequence support depends on
CONFIG_OF.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/Makefile | 2 +-
drivers/mmc/core/core.c | 7 +++++++
drivers/mmc/core/host.c | 4 +++-
drivers/mmc/core/pwrseq.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/mmc/core/pwrseq.h | 40 +++++++++++++++++++++++++++++++++++++
include/linux/mmc/host.h | 2 ++
6 files changed, 103 insertions(+), 2 deletions(-)
create mode 100644 drivers/mmc/core/pwrseq.c
create mode 100644 drivers/mmc/core/pwrseq.h
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 38ed210..ccdd35f 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -8,5 +8,5 @@ mmc_core-y := core.o bus.o host.o \
sdio.o sdio_ops.o sdio_bus.o \
sdio_cis.o sdio_io.o sdio_irq.o \
quirks.o slot-gpio.o
-
+mmc_core-$(CONFIG_OF) += pwrseq.o
mmc_core-$(CONFIG_DEBUG_FS) += debugfs.o
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d3bfbdf..fd13649 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -40,6 +40,7 @@
#include "bus.h"
#include "host.h"
#include "sdio_bus.h"
+#include "pwrseq.h"
#include "mmc_ops.h"
#include "sd_ops.h"
@@ -1583,6 +1584,8 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
mmc_host_clk_hold(host);
+ mmc_pwrseq_power_up(host);
+
host->ios.vdd = fls(ocr) - 1;
host->ios.power_mode = MMC_POWER_UP;
/* Set initial state and call mmc_set_ios */
@@ -1602,6 +1605,8 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
*/
mmc_delay(10);
+ mmc_pwrseq_power_on(host);
+
host->ios.clock = host->f_init;
host->ios.power_mode = MMC_POWER_ON;
@@ -1623,6 +1628,8 @@ void mmc_power_off(struct mmc_host *host)
mmc_host_clk_hold(host);
+ mmc_pwrseq_power_off(host);
+
host->ios.clock = 0;
host->ios.vdd = 0;
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 0763644..8be0df7 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -30,6 +30,7 @@
#include "core.h"
#include "host.h"
#include "slot-gpio.h"
+#include "pwrseq.h"
#define cls_dev_to_mmc_host(d) container_of(d, struct mmc_host, class_dev)
@@ -448,7 +449,7 @@ int mmc_of_parse(struct mmc_host *host)
host->dsr_req = 0;
}
- return 0;
+ return mmc_pwrseq_alloc(host);
}
EXPORT_SYMBOL(mmc_of_parse);
@@ -588,6 +589,7 @@ EXPORT_SYMBOL(mmc_remove_host);
*/
void mmc_free_host(struct mmc_host *host)
{
+ mmc_pwrseq_free(host);
put_device(&host->class_dev);
}
diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
new file mode 100644
index 0000000..24f2370
--- /dev/null
+++ b/drivers/mmc/core/pwrseq.c
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd
+ *
+ * Author: Ulf Hansson <ulf.hansson@linaro.org>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ *
+ * MMC power sequence management
+ */
+#include <linux/mmc/host.h>
+
+#include "pwrseq.h"
+
+
+int mmc_pwrseq_alloc(struct mmc_host *host)
+{
+ return 0;
+}
+
+void mmc_pwrseq_power_up(struct mmc_host *host)
+{
+ struct mmc_pwrseq *pwrseq = host->pwrseq;
+
+ if (pwrseq && pwrseq->ops && pwrseq->ops->power_up)
+ pwrseq->ops->power_up(host);
+}
+
+void mmc_pwrseq_power_on(struct mmc_host *host)
+{
+ struct mmc_pwrseq *pwrseq = host->pwrseq;
+
+ if (pwrseq && pwrseq->ops && pwrseq->ops->power_on)
+ pwrseq->ops->power_on(host);
+}
+
+void mmc_pwrseq_power_off(struct mmc_host *host)
+{
+ struct mmc_pwrseq *pwrseq = host->pwrseq;
+
+ if (pwrseq && pwrseq->ops && pwrseq->ops->power_off)
+ pwrseq->ops->power_off(host);
+}
+
+void mmc_pwrseq_free(struct mmc_host *host)
+{
+ struct mmc_pwrseq *pwrseq = host->pwrseq;
+
+ if (pwrseq && pwrseq->ops && pwrseq->ops->free)
+ pwrseq->ops->free(host);
+}
diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
new file mode 100644
index 0000000..430874d
--- /dev/null
+++ b/drivers/mmc/core/pwrseq.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd
+ *
+ * Author: Ulf Hansson <ulf.hansson@linaro.org>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+#ifndef _MMC_CORE_PWRSEQ_H
+#define _MMC_CORE_PWRSEQ_H
+
+struct mmc_pwrseq_ops {
+ void (*power_up)(struct mmc_host *host);
+ void (*power_on)(struct mmc_host *host);
+ void (*power_off)(struct mmc_host *host);
+ void (*free)(struct mmc_host *host);
+};
+
+struct mmc_pwrseq {
+ struct mmc_pwrseq_ops *ops;
+};
+
+#ifdef CONFIG_OF
+
+int mmc_pwrseq_alloc(struct mmc_host *host);
+void mmc_pwrseq_power_up(struct mmc_host *host);
+void mmc_pwrseq_power_on(struct mmc_host *host);
+void mmc_pwrseq_power_off(struct mmc_host *host);
+void mmc_pwrseq_free(struct mmc_host *host);
+
+#else
+
+static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
+static inline void mmc_pwrseq_power_up(struct mmc_host *host) {}
+static inline void mmc_pwrseq_power_on(struct mmc_host *host) {}
+static inline void mmc_pwrseq_power_off(struct mmc_host *host) {}
+static inline void mmc_pwrseq_free(struct mmc_host *host) {}
+
+#endif
+
+#endif
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index b6bf718..0c8cbe5 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -195,6 +195,7 @@ struct mmc_context_info {
};
struct regulator;
+struct mmc_pwrseq;
struct mmc_supply {
struct regulator *vmmc; /* Card power supply */
@@ -206,6 +207,7 @@ struct mmc_host {
struct device class_dev;
int index;
const struct mmc_host_ops *ops;
+ struct mmc_pwrseq *pwrseq;
unsigned int f_min;
unsigned int f_max;
unsigned int f_init;
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence
2015-01-02 16:14 [PATCH 0/4] mmc: core: Add support for MMC power sequences Ulf Hansson
2015-01-02 16:14 ` [PATCH 1/4] mmc: core: Initial " Ulf Hansson
@ 2015-01-02 16:14 ` Ulf Hansson
2015-01-02 16:14 ` [PATCH 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider Ulf Hansson
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2015-01-02 16:14 UTC (permalink / raw)
To: linux-arm-kernel
The simple MMC power sequence provider, intends to supports a set of
common properties between SOC designs. It thus enables us to re-use the
same provider for several SOCs.
In this initial step, let's add the top level description of the MMC
power sequence and describe the compatible string for the simple MMC
power sequence provider.
Following patches will step by step add support for new properties to
the simple MMC power sequence provider.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
.../devicetree/bindings/mmc/mmc,pwrseq-simple.txt | 18 ++++++++++++++++++
Documentation/devicetree/bindings/mmc/mmc.txt | 5 +++++
2 files changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
diff --git a/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
new file mode 100644
index 0000000..e1b7f9c
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
@@ -0,0 +1,18 @@
+* The simple MMC power sequence provider
+
+System on chip designs may specify a specific MMC power sequence. To
+successfully detect an (e)MMC/SD/SDIO card, that power sequence must be
+maintained while initializing the card.
+
+The simple MMC power sequence provider, intends to supports a set of common
+properties between SOC designs. It thus enables us to re-use the same provider
+for several SOC designs.
+
+Required properties:
+- compatible : contains "mmc,pwrseq-simple".
+
+Example:
+
+ sdhci0_pwrseq {
+ compatible = "mmc,pwrseq-simple";
+ }
diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index bac1311..b12de1e 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -64,6 +64,10 @@ Optional SDIO properties:
- keep-power-in-suspend: Preserves card power during a suspend/resume cycle
- enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion
+Optional MMC power sequence:
+- mmc-pwrseq: phandle to the MMC power sequence node. See "mmc,pwrseq-*"
+ for documentation of MMC power sequence bindings.
+
Use of Function subnodes
------------------------
@@ -101,6 +105,7 @@ sdhci at ab000000 {
max-frequency = <50000000>;
keep-power-in-suspend;
enable-sdio-wakeup;
+ mmc-pwrseq = <&sdhci0_pwrseq>
}
Example with sdio function subnode:
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider
2015-01-02 16:14 [PATCH 0/4] mmc: core: Add support for MMC power sequences Ulf Hansson
2015-01-02 16:14 ` [PATCH 1/4] mmc: core: Initial " Ulf Hansson
2015-01-02 16:14 ` [PATCH 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence Ulf Hansson
@ 2015-01-02 16:14 ` Ulf Hansson
2015-01-02 16:14 ` [PATCH 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin Ulf Hansson
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2015-01-02 16:14 UTC (permalink / raw)
To: linux-arm-kernel
To add the core part for the MMC power sequence, let's start by adding
initial support for the simple MMC power sequence provider.
In this initial step, the MMC power sequence node are fetched and the
compatible string for the simple MMC power sequence provider are
verified.
At this point we don't parse the node for any properties, but instead
that will be handled from following patches. Since there are no
properties supported yet, let's just implement the ->alloc() and the
->free() callbacks.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/Makefile | 2 +-
drivers/mmc/core/pwrseq.c | 61 +++++++++++++++++++++++++++++++++++++++-
drivers/mmc/core/pwrseq.h | 2 ++
drivers/mmc/core/pwrseq_simple.c | 48 +++++++++++++++++++++++++++++++
4 files changed, 111 insertions(+), 2 deletions(-)
create mode 100644 drivers/mmc/core/pwrseq_simple.c
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index ccdd35f..b39cbd2 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -8,5 +8,5 @@ mmc_core-y := core.o bus.o host.o \
sdio.o sdio_ops.o sdio_bus.o \
sdio_cis.o sdio_io.o sdio_irq.o \
quirks.o slot-gpio.o
-mmc_core-$(CONFIG_OF) += pwrseq.o
+mmc_core-$(CONFIG_OF) += pwrseq.o pwrseq_simple.o
mmc_core-$(CONFIG_DEBUG_FS) += debugfs.o
diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
index 24f2370..47ffa1b 100644
--- a/drivers/mmc/core/pwrseq.c
+++ b/drivers/mmc/core/pwrseq.c
@@ -7,14 +7,73 @@
*
* MMC power sequence management
*/
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+
#include <linux/mmc/host.h>
#include "pwrseq.h"
+struct mmc_pwrseq_match {
+ const char *compatible;
+ int (*alloc)(struct mmc_host *host, struct device *dev);
+};
+
+static struct mmc_pwrseq_match pwrseq_match[] = {
+ {
+ .compatible = "mmc,pwrseq-simple",
+ .alloc = mmc_pwrseq_simple_alloc,
+ },
+};
+
+static struct mmc_pwrseq_match *mmc_pwrseq_find(struct device_node *np)
+{
+ struct mmc_pwrseq_match *match = ERR_PTR(-ENODEV);
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pwrseq_match); i++) {
+ if (of_device_is_compatible(np, pwrseq_match[i].compatible)) {
+ match = &pwrseq_match[i];
+ break;
+ }
+ }
+
+ return match;
+}
int mmc_pwrseq_alloc(struct mmc_host *host)
{
- return 0;
+ struct platform_device *pdev;
+ struct device_node *np;
+ struct mmc_pwrseq_match *match;
+ int ret = 0;
+
+ np = of_parse_phandle(host->parent->of_node, "mmc-pwrseq", 0);
+ if (!np)
+ return 0;
+
+ pdev = of_find_device_by_node(np);
+ if (!pdev) {
+ ret = -ENODEV;
+ goto err;
+ }
+
+ match = mmc_pwrseq_find(np);
+ if (IS_ERR(match)) {
+ ret = PTR_ERR(match);
+ goto err;
+ }
+
+ ret = match->alloc(host, &pdev->dev);
+ if (!ret)
+ dev_info(host->parent, "allocated mmc-pwrseq\n");
+
+err:
+ of_node_put(np);
+ return ret;
}
void mmc_pwrseq_power_up(struct mmc_host *host)
diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
index 430874d..7e20dda 100644
--- a/drivers/mmc/core/pwrseq.h
+++ b/drivers/mmc/core/pwrseq.h
@@ -27,6 +27,8 @@ void mmc_pwrseq_power_on(struct mmc_host *host);
void mmc_pwrseq_power_off(struct mmc_host *host);
void mmc_pwrseq_free(struct mmc_host *host);
+int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev);
+
#else
static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
new file mode 100644
index 0000000..7f87bc1
--- /dev/null
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd
+ *
+ * Author: Ulf Hansson <ulf.hansson@linaro.org>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ *
+ * Simple MMC power sequence management
+ */
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/err.h>
+
+#include <linux/mmc/host.h>
+
+#include "pwrseq.h"
+
+struct mmc_pwrseq_simple {
+ struct mmc_pwrseq pwrseq;
+};
+
+static void mmc_pwrseq_simple_free(struct mmc_host *host)
+{
+ struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
+ struct mmc_pwrseq_simple, pwrseq);
+
+ kfree(&pwrseq);
+ host->pwrseq = NULL;
+}
+
+static struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
+ .free = mmc_pwrseq_simple_free,
+};
+
+int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
+{
+ struct mmc_pwrseq_simple *pwrseq;
+
+ pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), GFP_KERNEL);
+ if (!pwrseq)
+ return -ENOMEM;
+
+ pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
+ host->pwrseq = &pwrseq->pwrseq;
+
+ return 0;
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
2015-01-02 16:14 [PATCH 0/4] mmc: core: Add support for MMC power sequences Ulf Hansson
` (2 preceding siblings ...)
2015-01-02 16:14 ` [PATCH 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider Ulf Hansson
@ 2015-01-02 16:14 ` Ulf Hansson
2015-01-02 18:14 ` [PATCH 0/4] mmc: core: Add support for MMC power sequences Russell King - ARM Linux
2015-01-12 14:26 ` Tomeu Vizoso
5 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2015-01-02 16:14 UTC (permalink / raw)
To: linux-arm-kernel
The need for a reset GPIO has several times been pointed out from
earlier posted patchsets. Especially some WLAN chips which are
attached to an SDIO interface may use a GPIO reset.
In this first version, one reset pin is supported. We may want to
extend the support to cover more pins, but let's leave that as a future
change. The added DT binding for the reset GPIO can easily be extended
to manage several pins.
The reset GPIO is asserted at initialization and prior we start the
power up procedure. It will then be de-asserted right after the power
has been provided for the external chip/card, from the ->power_on()
callback.
Note, the reset GPIO is optional. Thus we don't return an error even if
we can't find a GPIO pin for the consumer.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
.../devicetree/bindings/mmc/mmc,pwrseq-simple.txt | 5 +++
drivers/mmc/core/pwrseq_simple.c | 38 ++++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
index e1b7f9c..6fe0cd6 100644
--- a/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
@@ -11,8 +11,13 @@ for several SOC designs.
Required properties:
- compatible : contains "mmc,pwrseq-simple".
+Optional properties:
+- reset-gpios : contains a list of GPIO specifiers, though currently only one
+ specifier is supported.
+
Example:
sdhci0_pwrseq {
compatible = "mmc,pwrseq-simple";
+ reset-gpios = <&gpio1 12 0>;
}
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 7f87bc1..97112b1 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -11,6 +11,7 @@
#include <linux/slab.h>
#include <linux/device.h>
#include <linux/err.h>
+#include <linux/gpio/consumer.h>
#include <linux/mmc/host.h>
@@ -18,31 +19,68 @@
struct mmc_pwrseq_simple {
struct mmc_pwrseq pwrseq;
+ struct gpio_desc *reset_gpio;
};
+static void mmc_pwrseq_simple_power_up(struct mmc_host *host)
+{
+ struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
+ struct mmc_pwrseq_simple, pwrseq);
+
+ if (!IS_ERR(pwrseq->reset_gpio))
+ gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
+}
+
+static void mmc_pwrseq_simple_power_on(struct mmc_host *host)
+{
+ struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
+ struct mmc_pwrseq_simple, pwrseq);
+
+ if (!IS_ERR(pwrseq->reset_gpio))
+ gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
+}
+
static void mmc_pwrseq_simple_free(struct mmc_host *host)
{
struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
struct mmc_pwrseq_simple, pwrseq);
+ if (!IS_ERR(pwrseq->reset_gpio))
+ gpiod_put(pwrseq->reset_gpio);
+
kfree(&pwrseq);
host->pwrseq = NULL;
}
static struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
+ .power_up = mmc_pwrseq_simple_power_up,
+ .power_on = mmc_pwrseq_simple_power_on,
+ .power_off = mmc_pwrseq_simple_power_up,
.free = mmc_pwrseq_simple_free,
};
int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
{
struct mmc_pwrseq_simple *pwrseq;
+ int ret = 0;
pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), GFP_KERNEL);
if (!pwrseq)
return -ENOMEM;
+ pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_HIGH);
+ if (IS_ERR(pwrseq->reset_gpio) &&
+ PTR_ERR(pwrseq->reset_gpio) != -ENOENT &&
+ PTR_ERR(pwrseq->reset_gpio) != -ENOSYS) {
+ ret = PTR_ERR(pwrseq->reset_gpio);
+ goto free;
+ }
+
pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
host->pwrseq = &pwrseq->pwrseq;
return 0;
+free:
+ kfree(&pwrseq);
+ return ret;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 0/4] mmc: core: Add support for MMC power sequences
2015-01-02 16:14 [PATCH 0/4] mmc: core: Add support for MMC power sequences Ulf Hansson
` (3 preceding siblings ...)
2015-01-02 16:14 ` [PATCH 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin Ulf Hansson
@ 2015-01-02 18:14 ` Russell King - ARM Linux
2015-01-04 19:20 ` Hans de Goede
2015-01-12 14:29 ` Ulf Hansson
2015-01-12 14:26 ` Tomeu Vizoso
5 siblings, 2 replies; 14+ messages in thread
From: Russell King - ARM Linux @ 2015-01-02 18:14 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jan 02, 2015 at 05:14:04PM +0100, Ulf Hansson wrote:
> To be able to handle these SOC specific power sequences, we add a MMC power
> sequence interface, which helps the mmc core to deal with such.
I think this should be done differently - part of that is with hind sight
given that we now have a range of interface types.
One of my early design mistakes with MMC was to incorporate the power up
and initialisation protocol (the 74 clocks business) into the core code.
This was wrong, because it is a detail which only applies to dumb host
interfaces. More inteligent interfaces do not need that complexity as
they handle that in hardware.
Rather than MMC ending up with more layers and more infrastructure like
this - turning it more into the turd which is sdhci - I would like to see
some proper thought put into design, specifically addressing the above
design issue.
What should be done is to move away from the opaque "set_ios" method into
something more appropriate - a set of callbacks which describe what we
want to achieve.
In the case of power up, there should be a single call into the host
controller code which is responsible for initiating the power up
sequence, and waits for the power up sequence to complete. In other
words, most of mmc_power_up() should be moved to a library function,
which dumb host adapters hook into the "power up" method. More
inteligent adapters (eg, PXA, sdhci) can add their own hook which poke
the hardware and return when the hardware has completed its power up
sequence.
That avoids the "mmc_delay(10)" for inteligent adapters where these
delays are not required - which presumably also are not required if some
special power up sequence for eMMC is required.
The same thing goes for this "powerseq" stuff - all you're doing is
propagating the same design mistake (with the POWER_UP/POWER_ON etc
details) into it. That shouldn't be the case - think about the dumb
powerup method I described above vs the inteligent method - these are
two separate powerup methods, and should be implemented as such.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 0/4] mmc: core: Add support for MMC power sequences
2015-01-02 18:14 ` [PATCH 0/4] mmc: core: Add support for MMC power sequences Russell King - ARM Linux
@ 2015-01-04 19:20 ` Hans de Goede
2015-01-12 14:29 ` Ulf Hansson
1 sibling, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2015-01-04 19:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi Russell,
On 02-01-15 19:14, Russell King - ARM Linux wrote:
> On Fri, Jan 02, 2015 at 05:14:04PM +0100, Ulf Hansson wrote:
>> To be able to handle these SOC specific power sequences, we add a MMC power
>> sequence interface, which helps the mmc core to deal with such.
>
> I think this should be done differently - part of that is with hind sight
> given that we now have a range of interface types.
>
> One of my early design mistakes with MMC was to incorporate the power up
> and initialisation protocol (the 74 clocks business) into the core code.
> This was wrong, because it is a detail which only applies to dumb host
> interfaces. More inteligent interfaces do not need that complexity as
> they handle that in hardware.
>
> Rather than MMC ending up with more layers and more infrastructure like
> this - turning it more into the turd which is sdhci - I would like to see
> some proper thought put into design, specifically addressing the above
> design issue.
>
> What should be done is to move away from the opaque "set_ios" method into
> something more appropriate - a set of callbacks which describe what we
> want to achieve.
I think you misunderstand what this patch-set tries to do, this patch-set
is intended for use with on board sdio devices, which typically may need
a number of resources initialized before the can be probed over the sd
bus at all. E.g. a regulator enabled, a reset de-asserted, a clk configured
and provided, etc. So this is about sequencing of events before even
trying to talk to the device at the basic spi compatible layer.
Your ideas seem like a good idea to improve the mmc subsys, but this
is somewhat unrelated, in a way this could apply equally to say an usb
attached device, or basically any discoverable bus, where some onboard
devices may need some form of enabling (or power up) before they can be
discovered.
Which is why the original patchset Ulf references in the cover letter was
trying to be more generic, since the got nowhere Ulf now is trying with
something mmc specific, since the direct need for this functionality is
with sdio devices.
Note that this is a real world problem which has been hacked around a lot
in vendor kernels, so although getting the design right as always is
important, it is also important to at some point come up with something
workable, and if you look at previous threads on this topic, you will see
that has already been discussed a lot, and always seems to get stuck on
people trying to find the "perfect. 100% generic" solution, a typical
example of perfect being the enemy of good.
Regards,
Hans
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/4] mmc: core: Add support for MMC power sequences
2015-01-02 18:14 ` [PATCH 0/4] mmc: core: Add support for MMC power sequences Russell King - ARM Linux
2015-01-04 19:20 ` Hans de Goede
@ 2015-01-12 14:29 ` Ulf Hansson
2015-01-12 16:18 ` Russell King - ARM Linux
1 sibling, 1 reply; 14+ messages in thread
From: Ulf Hansson @ 2015-01-12 14:29 UTC (permalink / raw)
To: linux-arm-kernel
On 2 January 2015 at 19:14, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Jan 02, 2015 at 05:14:04PM +0100, Ulf Hansson wrote:
>> To be able to handle these SOC specific power sequences, we add a MMC power
>> sequence interface, which helps the mmc core to deal with such.
>
> I think this should be done differently - part of that is with hind sight
> given that we now have a range of interface types.
>
> One of my early design mistakes with MMC was to incorporate the power up
> and initialisation protocol (the 74 clocks business) into the core code.
> This was wrong, because it is a detail which only applies to dumb host
> interfaces. More inteligent interfaces do not need that complexity as
> they handle that in hardware.
>
> Rather than MMC ending up with more layers and more infrastructure like
> this - turning it more into the turd which is sdhci - I would like to see
> some proper thought put into design, specifically addressing the above
> design issue.
>
> What should be done is to move away from the opaque "set_ios" method into
> something more appropriate - a set of callbacks which describe what we
> want to achieve.
>
> In the case of power up, there should be a single call into the host
> controller code which is responsible for initiating the power up
> sequence, and waits for the power up sequence to complete. In other
> words, most of mmc_power_up() should be moved to a library function,
> which dumb host adapters hook into the "power up" method. More
> inteligent adapters (eg, PXA, sdhci) can add their own hook which poke
> the hardware and return when the hardware has completed its power up
> sequence.
>
> That avoids the "mmc_delay(10)" for inteligent adapters where these
> delays are not required - which presumably also are not required if some
> special power up sequence for eMMC is required.
>
> The same thing goes for this "powerseq" stuff - all you're doing is
> propagating the same design mistake (with the POWER_UP/POWER_ON etc
> details) into it. That shouldn't be the case - think about the dumb
> powerup method I described above vs the inteligent method - these are
> two separate powerup methods, and should be implemented as such.
Hi Russell,
Thanks for your comments!
You certainly have a point around how to improve the use the existing
->set_ios() callbacks. Though, as Hans also pointed out in his reply,
I believe this patchset is not related to that.
Regarding you concern, about me propagating the same design mistake as
for the ->set_ios() callback, I am not sure I fully understand why you
think that's the case?
I am suggesting is to add a set of mmc-pwrseq callbacks, which I think
is needed for a mmc-pwrseq provider to be able handle it's SOC
specific mmc power sequence. There are no MMC/SD/SDIO protocol
specific constraints that should be handled from them, but instead
only SOC specific power resources and sequences.
If the _simple_ mmc power sequence provider, turns out to be too
complex while trying to support several SOCs, we shall instead add yet
another provider. In this case, we should of course share as much code
as possible between the different providers.
Kind regards
Uffe
>
> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/4] mmc: core: Add support for MMC power sequences
2015-01-12 14:29 ` Ulf Hansson
@ 2015-01-12 16:18 ` Russell King - ARM Linux
2015-01-13 14:08 ` Ulf Hansson
0 siblings, 1 reply; 14+ messages in thread
From: Russell King - ARM Linux @ 2015-01-12 16:18 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 12, 2015 at 03:29:54PM +0100, Ulf Hansson wrote:
> Regarding you concern, about me propagating the same design mistake as
> for the ->set_ios() callback, I am not sure I fully understand why you
> think that's the case?
Because of this:
mmc_pwrseq_power_up()- Invoked from mmc_power_up(), to power up.
mmc_pwrseq_power_on()- Invoked from mmc_power_up(), to power on.
This re-inforces the "power up, wait, power on" _power_ _sequence_
as part of the software design.
We know that _today_ there is hardware which does not work like that.
We know that we have host adapters which ignore the "power up" part,
because they deal with all the sequencing in hardware.
I'll refer to your new infrastructure as "pwrseq" and the existing
infrastructure as "power sequence". (That alone shows what an
absurd situation this is - we have two things which have the same
name!)
Let's say we have a pwrseq handler which implements the power_up()
and power_on() callbacks. It's written for use with a controller
which implements appropriate actions on set_ios() with POWER_UP
and POWER_ON states implemented.
So, what we have is:
pwrseq power sequence host state/action
power_up no power supplied
POWER_UP host applies power to the card
power_on host is supplying power to card
POWER_ON host applies clocks to the card
Now, for a more inteligent host, where the hardware takes care of
sequencing the application of power and clocks, we have this:
pwrseq power sequence host action
power_up no power applied
POWER_UP host does nothing
power_on no power applied
POWER_ON host applies power to the card, waits,
and applies the clock
As we can see, the hardware state which the power_on() method of the
pwrseq is called is highly host dependent. In the first case, power
has already been applied. In the second case, power has not been
applied.
To have consistency, you need to /first/ solve the problem which I've
been pointing out, otherwise we're going to have these new pwrseq
callbacks being called with inconsistent, host specific power states
being applied to the card.
An alternative way to get consistency is to eliminate the pwrseq
"power_on" callback altogether; the "power_up" callback will always
be made before the host interface power sequence is started - and
that is about the only thing we can guarantee given the variability
in host interface designs.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/4] mmc: core: Add support for MMC power sequences
2015-01-12 16:18 ` Russell King - ARM Linux
@ 2015-01-13 14:08 ` Ulf Hansson
2015-01-13 14:23 ` Russell King - ARM Linux
0 siblings, 1 reply; 14+ messages in thread
From: Ulf Hansson @ 2015-01-13 14:08 UTC (permalink / raw)
To: linux-arm-kernel
On 12 January 2015 at 17:18, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 12, 2015 at 03:29:54PM +0100, Ulf Hansson wrote:
>> Regarding you concern, about me propagating the same design mistake as
>> for the ->set_ios() callback, I am not sure I fully understand why you
>> think that's the case?
>
> Because of this:
>
> mmc_pwrseq_power_up()- Invoked from mmc_power_up(), to power up.
> mmc_pwrseq_power_on()- Invoked from mmc_power_up(), to power on.
>
> This re-inforces the "power up, wait, power on" _power_ _sequence_
> as part of the software design.
>
> We know that _today_ there is hardware which does not work like that.
> We know that we have host adapters which ignore the "power up" part,
> because they deal with all the sequencing in hardware.
>
> I'll refer to your new infrastructure as "pwrseq" and the existing
> infrastructure as "power sequence". (That alone shows what an
> absurd situation this is - we have two things which have the same
> name!)
>
> Let's say we have a pwrseq handler which implements the power_up()
> and power_on() callbacks. It's written for use with a controller
> which implements appropriate actions on set_ios() with POWER_UP
> and POWER_ON states implemented.
>
> So, what we have is:
>
> pwrseq power sequence host state/action
>
> power_up no power supplied
> POWER_UP host applies power to the card
>
> power_on host is supplying power to card
> POWER_ON host applies clocks to the card
>
> Now, for a more inteligent host, where the hardware takes care of
> sequencing the application of power and clocks, we have this:
>
> pwrseq power sequence host action
>
> power_up no power applied
> POWER_UP host does nothing
>
> power_on no power applied
> POWER_ON host applies power to the card, waits,
> and applies the clock
>
> As we can see, the hardware state which the power_on() method of the
> pwrseq is called is highly host dependent. In the first case, power
> has already been applied. In the second case, power has not been
> applied.
>
> To have consistency, you need to /first/ solve the problem which I've
> been pointing out, otherwise we're going to have these new pwrseq
> callbacks being called with inconsistent, host specific power states
> being applied to the card.
Thanks a lot for elaborating! I understand your concern now.
In this context, I believe it's important to state that I think these
host driver behaves badly! _All_ host drivers shall handle their
"power_up" things while they get MMC_POWER_UP through the ->set_ios()
callback. They shouldn't be taking short-cuts and ignore the
MMC_POWER_UP state, that also includes those host drivers which are
able to handle the power sequence by help from it's controller
hardware.
I did quick research and found that the following host's ->set_ios()
callbacks need to be fixed in this regards.
au1xmmc_set_ios()
cb710_mmc_set_ios()
omap_hsmmc_set_ios()
sdricoh_set_ios()
__toshsd_set_ios()
>
> An alternative way to get consistency is to eliminate the pwrseq
> "power_on" callback altogether; the "power_up" callback will always
> be made before the host interface power sequence is started - and
> that is about the only thing we can guarantee given the variability
> in host interface designs.
Eliminating is likely not an option, since it would mean we will have
a hard time to cope with requirements for the pwrseq sequences.
An option would be to move the call to the pwrseq ->power_on()
callback to the end of mmc_power_up(). That would make it as
consistent as we can from mmc core point of view. Right?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/4] mmc: core: Add support for MMC power sequences
2015-01-13 14:08 ` Ulf Hansson
@ 2015-01-13 14:23 ` Russell King - ARM Linux
2015-01-13 14:34 ` Ulf Hansson
0 siblings, 1 reply; 14+ messages in thread
From: Russell King - ARM Linux @ 2015-01-13 14:23 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 13, 2015 at 03:08:51PM +0100, Ulf Hansson wrote:
> On 12 January 2015 at 17:18, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Jan 12, 2015 at 03:29:54PM +0100, Ulf Hansson wrote:
> >> Regarding you concern, about me propagating the same design mistake as
> >> for the ->set_ios() callback, I am not sure I fully understand why you
> >> think that's the case?
> >
> > Because of this:
> >
> > mmc_pwrseq_power_up()- Invoked from mmc_power_up(), to power up.
> > mmc_pwrseq_power_on()- Invoked from mmc_power_up(), to power on.
> >
> > This re-inforces the "power up, wait, power on" _power_ _sequence_
> > as part of the software design.
> >
> > We know that _today_ there is hardware which does not work like that.
> > We know that we have host adapters which ignore the "power up" part,
> > because they deal with all the sequencing in hardware.
> >
> > I'll refer to your new infrastructure as "pwrseq" and the existing
> > infrastructure as "power sequence". (That alone shows what an
> > absurd situation this is - we have two things which have the same
> > name!)
> >
> > Let's say we have a pwrseq handler which implements the power_up()
> > and power_on() callbacks. It's written for use with a controller
> > which implements appropriate actions on set_ios() with POWER_UP
> > and POWER_ON states implemented.
> >
> > So, what we have is:
> >
> > pwrseq power sequence host state/action
> >
> > power_up no power supplied
> > POWER_UP host applies power to the card
> >
> > power_on host is supplying power to card
> > POWER_ON host applies clocks to the card
> >
> > Now, for a more inteligent host, where the hardware takes care of
> > sequencing the application of power and clocks, we have this:
> >
> > pwrseq power sequence host action
> >
> > power_up no power applied
> > POWER_UP host does nothing
> >
> > power_on no power applied
> > POWER_ON host applies power to the card, waits,
> > and applies the clock
> >
> > As we can see, the hardware state which the power_on() method of the
> > pwrseq is called is highly host dependent. In the first case, power
> > has already been applied. In the second case, power has not been
> > applied.
> >
> > To have consistency, you need to /first/ solve the problem which I've
> > been pointing out, otherwise we're going to have these new pwrseq
> > callbacks being called with inconsistent, host specific power states
> > being applied to the card.
>
> Thanks a lot for elaborating! I understand your concern now.
>
> In this context, I believe it's important to state that I think these
> host driver behaves badly! _All_ host drivers shall handle their
> "power_up" things while they get MMC_POWER_UP through the ->set_ios()
> callback. They shouldn't be taking short-cuts and ignore the
> MMC_POWER_UP state, that also includes those host drivers which are
> able to handle the power sequence by help from it's controller
> hardware.
You can't solve this in this way. Let's say that you decide to force
the inteligent controllers to power up and enable clocks in the
POWER_UP stage:
pwrseq power sequence host action
power_up no power applied
POWER_UP host applies power to the card, waits,
and applies the clock
power_on power and clock is applied
POWER_ON host does nothing
So now, you have a variability for the power_on() callback, where with
dumb hosts, the clock is not enabled, but for more inteligent hosts,
the clock has already enabled, and likely we have already waited the
74 clocks necessary in the MMC protocol.
> I did quick research and found that the following host's ->set_ios()
> callbacks need to be fixed in this regards.
> au1xmmc_set_ios()
> cb710_mmc_set_ios()
> omap_hsmmc_set_ios()
> sdricoh_set_ios()
> __toshsd_set_ios()
Rather than fixing them, how about introducing a new method for host
controllers:
power_up()
which can be pointed to a library function which makes the ->set_ios
calls as the current mmc_power_up() function does for those dumb
controllers, but for the more inteligent controllers, just invokes
their power up sequence and returns when that is complete.
> > An alternative way to get consistency is to eliminate the pwrseq
> > "power_on" callback altogether; the "power_up" callback will always
> > be made before the host interface power sequence is started - and
> > that is about the only thing we can guarantee given the variability
> > in host interface designs.
>
> Eliminating is likely not an option, since it would mean we will have
> a hard time to cope with requirements for the pwrseq sequences.
>
> An option would be to move the call to the pwrseq ->power_on()
> callback to the end of mmc_power_up(). That would make it as
> consistent as we can from mmc core point of view. Right?
Yes, and you might as well call the pwrseq callbacks "->pre_power_on()"
and "->post_power_on()" - maybe even "->post_pwrclk_on()" to make it
clear that it happens after the power and clocks have been applied
according to the MMC/SD protocol.
What I'm envisioning mmc_power_up() becoming is something like this:
void mmc_power_up(struct mmc_host *host, u32 ocr)
{
if (host->ios.power_mode == MMC_POWER_ON)
return;
mmc_host_clk_hold(host);
if (pwrseq)
pwrseq->pre_power_on(host);
host->ios.vdd = fls(ocr) - 1;
host->ios.clock = host->f_init;
host->ios.power_mode = MMC_POWER_ON;
/* and set remainder of initial state of host->ios */
host->ops->power_up(host, &host->ios);
if (pwrseq)
pwrseq->post_power_on(host);
mmc_host_clk_release(host);
}
The legacy power_up host method can save a copy host->ios, and manipulate
the saved copy to call mmc_set_ios() as we currently do, with all the
delays we have there.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 0/4] mmc: core: Add support for MMC power sequences
2015-01-13 14:23 ` Russell King - ARM Linux
@ 2015-01-13 14:34 ` Ulf Hansson
0 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2015-01-13 14:34 UTC (permalink / raw)
To: linux-arm-kernel
[...]
>>
>> Thanks a lot for elaborating! I understand your concern now.
>>
>> In this context, I believe it's important to state that I think these
>> host driver behaves badly! _All_ host drivers shall handle their
>> "power_up" things while they get MMC_POWER_UP through the ->set_ios()
>> callback. They shouldn't be taking short-cuts and ignore the
>> MMC_POWER_UP state, that also includes those host drivers which are
>> able to handle the power sequence by help from it's controller
>> hardware.
>
> You can't solve this in this way. Let's say that you decide to force
> the inteligent controllers to power up and enable clocks in the
> POWER_UP stage:
>
> pwrseq power sequence host action
>
> power_up no power applied
> POWER_UP host applies power to the card, waits,
> and applies the clock
>
> power_on power and clock is applied
> POWER_ON host does nothing
>
> So now, you have a variability for the power_on() callback, where with
> dumb hosts, the clock is not enabled, but for more inteligent hosts,
> the clock has already enabled, and likely we have already waited the
> 74 clocks necessary in the MMC protocol.
I understand, so I was not thinking of fixing the actual problem but
more to make the behaviour consistent.
>
>> I did quick research and found that the following host's ->set_ios()
>> callbacks need to be fixed in this regards.
>> au1xmmc_set_ios()
>> cb710_mmc_set_ios()
>> omap_hsmmc_set_ios()
>> sdricoh_set_ios()
>> __toshsd_set_ios()
>
> Rather than fixing them, how about introducing a new method for host
> controllers:
>
> power_up()
>
> which can be pointed to a library function which makes the ->set_ios
> calls as the current mmc_power_up() function does for those dumb
> controllers, but for the more inteligent controllers, just invokes
> their power up sequence and returns when that is complete.
Seems like a good idea!
>
>> > An alternative way to get consistency is to eliminate the pwrseq
>> > "power_on" callback altogether; the "power_up" callback will always
>> > be made before the host interface power sequence is started - and
>> > that is about the only thing we can guarantee given the variability
>> > in host interface designs.
>>
>> Eliminating is likely not an option, since it would mean we will have
>> a hard time to cope with requirements for the pwrseq sequences.
>>
>> An option would be to move the call to the pwrseq ->power_on()
>> callback to the end of mmc_power_up(). That would make it as
>> consistent as we can from mmc core point of view. Right?
>
> Yes, and you might as well call the pwrseq callbacks "->pre_power_on()"
> and "->post_power_on()" - maybe even "->post_pwrclk_on()" to make it
> clear that it happens after the power and clocks have been applied
> according to the MMC/SD protocol.
>
> What I'm envisioning mmc_power_up() becoming is something like this:
>
> void mmc_power_up(struct mmc_host *host, u32 ocr)
> {
> if (host->ios.power_mode == MMC_POWER_ON)
> return;
>
> mmc_host_clk_hold(host);
>
> if (pwrseq)
> pwrseq->pre_power_on(host);
>
> host->ios.vdd = fls(ocr) - 1;
> host->ios.clock = host->f_init;
> host->ios.power_mode = MMC_POWER_ON;
> /* and set remainder of initial state of host->ios */
>
> host->ops->power_up(host, &host->ios);
>
> if (pwrseq)
> pwrseq->post_power_on(host);
>
> mmc_host_clk_release(host);
> }
>
> The legacy power_up host method can save a copy host->ios, and manipulate
> the saved copy to call mmc_set_ios() as we currently do, with all the
> delays we have there.
>
Looks good!
I will rename the pwrseq callbacks to your proposal and send a v2.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/4] mmc: core: Add support for MMC power sequences
2015-01-02 16:14 [PATCH 0/4] mmc: core: Add support for MMC power sequences Ulf Hansson
` (4 preceding siblings ...)
2015-01-02 18:14 ` [PATCH 0/4] mmc: core: Add support for MMC power sequences Russell King - ARM Linux
@ 2015-01-12 14:26 ` Tomeu Vizoso
2015-01-12 14:43 ` Ulf Hansson
5 siblings, 1 reply; 14+ messages in thread
From: Tomeu Vizoso @ 2015-01-12 14:26 UTC (permalink / raw)
To: linux-arm-kernel
On 2 January 2015 at 17:14, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> This is yet another try to solve the issues of dealing with power sequences for
> the MMC subsystem. The latest attempt, see link below, took a generic approach
> by adding a new top level driver layer. That's was rejected by several reasons.
> http://lwn.net/Articles/602855/
>
> This time the approach is focused to fix the issues for MMC only.
>
> To give a short background, SOCs may specify a specific MMC power sequence. To
> successfully detect an (e)MMC/SD/SDIO card, that power sequence must be followed
> while initializing the card.
Hi Ulf,
have used this to successfully init a mwifiex_sdio card but had to
also add a delay while the reset line is asserted. May make sense to
add a property to the pwrseq node to specify the minimum amount of
time that the reset GPIO needs to be asserted?
Tested-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Thanks,
Tomeu
> To be able to handle these SOC specific power sequences, we add a MMC power
> sequence interface, which helps the mmc core to deal with such.
>
> A MMC power sequence provider then implements a set of callbacks from the above
> mentioned interface. The provider has a corresponding DT compatibility string
> and relies on CONFIG_OF to be set to find it's various resourses, like for
> example a GPIO reset.
>
> The mmc core will from mmc_of_parse() try find a "mmc-pwrseq" DT node and then
> call the corresponding MMC power sequence provider's initialization function.
>
>
> Ulf Hansson (4):
> mmc: core: Initial support for MMC power sequences
> mmc: pwrseq: Document DT bindings for the simple MMC power sequence
> mmc: pwrseq: Initial support for the simple MMC power sequence
> provider
> mmc: pwrseq_simple: Add support for a reset GPIO pin
>
> .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt | 23 +++++
> Documentation/devicetree/bindings/mmc/mmc.txt | 5 +
> drivers/mmc/core/Makefile | 2 +-
> drivers/mmc/core/core.c | 7 ++
> drivers/mmc/core/host.c | 4 +-
> drivers/mmc/core/pwrseq.c | 109 +++++++++++++++++++++
> drivers/mmc/core/pwrseq.h | 42 ++++++++
> drivers/mmc/core/pwrseq_simple.c | 86 ++++++++++++++++
> include/linux/mmc/host.h | 2 +
> 9 files changed, 278 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
> create mode 100644 drivers/mmc/core/pwrseq.c
> create mode 100644 drivers/mmc/core/pwrseq.h
> create mode 100644 drivers/mmc/core/pwrseq_simple.c
>
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 0/4] mmc: core: Add support for MMC power sequences
2015-01-12 14:26 ` Tomeu Vizoso
@ 2015-01-12 14:43 ` Ulf Hansson
0 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2015-01-12 14:43 UTC (permalink / raw)
To: linux-arm-kernel
On 12 January 2015 at 15:26, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> On 2 January 2015 at 17:14, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> This is yet another try to solve the issues of dealing with power sequences for
>> the MMC subsystem. The latest attempt, see link below, took a generic approach
>> by adding a new top level driver layer. That's was rejected by several reasons.
>> http://lwn.net/Articles/602855/
>>
>> This time the approach is focused to fix the issues for MMC only.
>>
>> To give a short background, SOCs may specify a specific MMC power sequence. To
>> successfully detect an (e)MMC/SD/SDIO card, that power sequence must be followed
>> while initializing the card.
>
> Hi Ulf,
>
> have used this to successfully init a mwifiex_sdio card but had to
> also add a delay while the reset line is asserted. May make sense to
> add a property to the pwrseq node to specify the minimum amount of
> time that the reset GPIO needs to be asserted?
To me that is description of the HW, so I would happily accept such a
DT property. Can you please send a patch, based on top of this
patchset?
BTW, I guess you only need the delay while the ->power_up() callback
is invoked and not for ->power_off()?
>
> Tested-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Thanks for helping out testing!
Kind regards
Uffe
^ permalink raw reply [flat|nested] 14+ messages in thread