[parent not found: <1420215248-20650-1-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* [PATCH 1/4] mmc: core: Initial support for MMC power sequences
[not found] ` <1420215248-20650-1-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2015-01-02 16:14 ` Ulf Hansson
0 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2015-01-02 16:14 UTC (permalink / raw)
To: linux-mmc-u79uwXL29TY76Z2rM5mHXA, Chris Ball
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linus Walleij, Mark Brown,
Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
Olof Johansson, Russell King, Hans de Goede, Doug Anderson,
NeilBrown, Ulf Hansson
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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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
[not found] ` <1420215248-20650-1-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 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-mmc, Chris Ball
Cc: linux-arm-kernel, devicetree, Linus Walleij, Mark Brown,
Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
Olof Johansson, Russell King, Hans de Goede, Doug Anderson,
NeilBrown, Ulf Hansson
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@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
[not found] ` <1420215248-20650-1-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
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-mmc, Chris Ball
Cc: linux-arm-kernel, devicetree, Linus Walleij, Mark Brown,
Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
Olof Johansson, Russell King, Hans de Goede, Doug Anderson,
NeilBrown, Ulf Hansson
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-mmc, Chris Ball
Cc: linux-arm-kernel, devicetree, Linus Walleij, Mark Brown,
Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
Olof Johansson, Russell King, Hans de Goede, Doug Anderson,
NeilBrown, Ulf Hansson
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
* Re: [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: Ulf Hansson
Cc: linux-mmc, Chris Ball, linux-arm-kernel, devicetree,
Linus Walleij, Mark Brown, Arnd Bergmann, Alexandre Courbot,
Arend van Spriel, Sascha Hauer, Olof Johansson, Hans de Goede,
Doug Anderson, NeilBrown
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
* Re: [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: Russell King - ARM Linux, Ulf Hansson
Cc: linux-mmc, Chris Ball, linux-arm-kernel, devicetree,
Linus Walleij, Mark Brown, Arnd Bergmann, Alexandre Courbot,
Arend van Spriel, Sascha Hauer, Olof Johansson, Doug Anderson,
NeilBrown
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
* Re: [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
[not found] ` <CAPDyKFpo=WpE3hDxqETVXOYX6pFsSmO7YybLJF=kJFyYFVcaoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
1 sibling, 1 reply; 14+ messages in thread
From: Ulf Hansson @ 2015-01-12 14:29 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-mmc, Chris Ball, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, Linus Walleij, Mark Brown,
Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
Olof Johansson, Hans de Goede, Doug Anderson, NeilBrown
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
* Re: [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: Ulf Hansson
Cc: linux-mmc, Chris Ball, devicetree@vger.kernel.org, Hans de Goede,
Russell King, Arnd Bergmann, Alexandre Courbot, NeilBrown,
Linus Walleij, Doug Anderson, Olof Johansson, Mark Brown,
Arend van Spriel, Sascha Hauer,
linux-arm-kernel@lists.infradead.org
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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [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: Tomeu Vizoso
Cc: linux-mmc, Chris Ball, devicetree@vger.kernel.org, Hans de Goede,
Russell King, Arnd Bergmann, Alexandre Courbot, NeilBrown,
Linus Walleij, Doug Anderson, Olof Johansson, Mark Brown,
Arend van Spriel, Sascha Hauer,
linux-arm-kernel@lists.infradead.org
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