[PATCH V4 0/4] mmc: core: Add support for MMC power sequences

[PATCH V4 0/4] mmc: core: Add support for MMC power sequences

[Ulf Hansson]

Changes in v4:
	- Fixed call to kfree() in patch3 and patch4.

Changes in v3:
	Fixed comments from Mark Rutland:
	- Document binding in one patch to get the big picture.
	- Keep code and DT document consistent around how many GPIO resets we
	  support. I decided to go for one GPIO, we can extend that if needed
	  later on.
	- Change compatible string for simple MMC power sequence and renamed the
	  file for its documentation.
	- Updated commit messages according to above changes.

Changes in v2:
	Fixed comments from Russell King:
	- Renamed pwrseq callbacks and corresponding interface functions.
	- Move the call to the previous namned ->power_on() callback, to the
	  end of mmc_power_up() to get consistent behavior.


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.

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  |  20 ++++
 Documentation/devicetree/bindings/mmc/mmc.txt      |  14 +++
 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, 284 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

[PATCH V4 0/4] mmc: core: Add support for MMC power sequences

[Ulf Hansson]

Changes in v4:
	- Fixed call to kfree() in patch3 and patch4.

Changes in v3:
	Fixed comments from Mark Rutland:
	- Document binding in one patch to get the big picture.
	- Keep code and DT document consistent around how many GPIO resets we
	  support. I decided to go for one GPIO, we can extend that if needed
	  later on.
	- Change compatible string for simple MMC power sequence and renamed the
	  file for its documentation.
	- Updated commit messages according to above changes.

Changes in v2:
	Fixed comments from Russell King:
	- Renamed pwrseq callbacks and corresponding interface functions.
	- Move the call to the previous namned ->power_on() callback, to the
	  end of mmc_power_up() to get consistent behavior.


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.

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  |  20 ++++
 Documentation/devicetree/bindings/mmc/mmc.txt      |  14 +++
 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, 284 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

[PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[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_pre_power_on()- Invoked in the beginning of mmc_power_up().
mmc_pwrseq_post_power_on()- Invoked at the end in mmc_power_up().
mmc_pwrseq_power_off()- Invoked from mmc_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>
---

Changes in v4:
	- None.

---
 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 d5c176e..1be7055 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"
@@ -1615,6 +1616,8 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 
 	mmc_host_clk_hold(host);
 
+	mmc_pwrseq_pre_power_on(host);
+
 	host->ios.vdd = fls(ocr) - 1;
 	host->ios.power_mode = MMC_POWER_UP;
 	/* Set initial state and call mmc_set_ios */
@@ -1645,6 +1648,8 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	 */
 	mmc_delay(10);
 
+	mmc_pwrseq_post_power_on(host);
+
 	mmc_host_clk_release(host);
 }
 
@@ -1655,6 +1660,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..bd08772
--- /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_pre_power_on(struct mmc_host *host)
+{
+	struct mmc_pwrseq *pwrseq = host->pwrseq;
+
+	if (pwrseq && pwrseq->ops && pwrseq->ops->pre_power_on)
+		pwrseq->ops->pre_power_on(host);
+}
+
+void mmc_pwrseq_post_power_on(struct mmc_host *host)
+{
+	struct mmc_pwrseq *pwrseq = host->pwrseq;
+
+	if (pwrseq && pwrseq->ops && pwrseq->ops->post_power_on)
+		pwrseq->ops->post_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..12aaf2b
--- /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 (*pre_power_on)(struct mmc_host *host);
+	void (*post_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_pre_power_on(struct mmc_host *host);
+void mmc_pwrseq_post_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_pre_power_on(struct mmc_host *host) {}
+static inline void mmc_pwrseq_post_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

[PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[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_pre_power_on()- Invoked in the beginning of mmc_power_up().
mmc_pwrseq_post_power_on()- Invoked at the end in mmc_power_up().
mmc_pwrseq_power_off()- Invoked from mmc_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>
---

Changes in v4:
	- None.

---
 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 d5c176e..1be7055 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"
@@ -1615,6 +1616,8 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 
 	mmc_host_clk_hold(host);
 
+	mmc_pwrseq_pre_power_on(host);
+
 	host->ios.vdd = fls(ocr) - 1;
 	host->ios.power_mode = MMC_POWER_UP;
 	/* Set initial state and call mmc_set_ios */
@@ -1645,6 +1648,8 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	 */
 	mmc_delay(10);
 
+	mmc_pwrseq_post_power_on(host);
+
 	mmc_host_clk_release(host);
 }
 
@@ -1655,6 +1660,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..bd08772
--- /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_pre_power_on(struct mmc_host *host)
+{
+	struct mmc_pwrseq *pwrseq = host->pwrseq;
+
+	if (pwrseq && pwrseq->ops && pwrseq->ops->pre_power_on)
+		pwrseq->ops->pre_power_on(host);
+}
+
+void mmc_pwrseq_post_power_on(struct mmc_host *host)
+{
+	struct mmc_pwrseq *pwrseq = host->pwrseq;
+
+	if (pwrseq && pwrseq->ops && pwrseq->ops->post_power_on)
+		pwrseq->ops->post_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..12aaf2b
--- /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 (*pre_power_on)(struct mmc_host *host);
+	void (*post_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_pre_power_on(struct mmc_host *host);
+void mmc_pwrseq_post_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_pre_power_on(struct mmc_host *host) {}
+static inline void mmc_pwrseq_post_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

[PATCH V4 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence

[Ulf Hansson]

To support SOCs which specifies specific MMC power sequences, document
some MMC DT bindings to be able to describe these hardwares.

Let's also document bindings for a simple MMC power sequence provider,
which purpose is to support a set of common properties between various
SOCs.

In this initial step, let's also document a top level description of
the MMC power sequence and describe the compatible string used for the
simple MMC power sequence provider.

The simple MMC power sequence provider will initially support a reset
GPIO. From several earlier posted patches, it's clear that such
hardware exists. Especially some WLAN chips which are attached to an
SDIO interface may use a GPIO reset.

Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---

Changed in v4:
	- None.

---
 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt    | 20 ++++++++++++++++++++
 Documentation/devicetree/bindings/mmc/mmc.txt        | 14 ++++++++++++++
 2 files changed, 34 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..da333d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -0,0 +1,20 @@
+* The simple MMC power sequence provider
+
+The purpose of the simple MMC power sequence provider is to supports a set of
+common properties between various SOC designs. It thus enables us to use the
+same provider for several SOC designs.
+
+Required properties:
+- compatible : contains "mmc-pwrseq-simple".
+
+Optional properties:
+- reset-gpios : contains a GPIO specifier. The reset GPIO is asserted at
+	initialization and prior we start the power up procedure of the card. It
+	will be de-asserted right after the power has been provided to the card.
+
+Example:
+
+	sdhci0_pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		reset-gpios = <&gpio1 12 0>;
+	}
diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index bac1311..438899e 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -65,6 +65,18 @@ Optional SDIO properties:
 - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion
 
 
+MMC power sequences:
+--------------------
+
+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.
+
+Optional property:
+- 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 +113,7 @@ sdhci@ab000000 {
 	max-frequency = <50000000>;
 	keep-power-in-suspend;
 	enable-sdio-wakeup;
+	mmc-pwrseq = <&sdhci0_pwrseq>
 }
 
 Example with sdio function subnode:
@@ -114,6 +127,7 @@ mmc3: mmc@01c12000 {
 	vmmc-supply = <&reg_vmmc3>;
 	bus-width = <4>;
 	non-removable;
+	mmc-pwrseq = <&sdhci0_pwrseq>
 	status = "okay";
 
 	brcmf: bcrmf@1 {
-- 
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

[PATCH V4 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence

[Ulf Hansson]

To support SOCs which specifies specific MMC power sequences, document
some MMC DT bindings to be able to describe these hardwares.

Let's also document bindings for a simple MMC power sequence provider,
which purpose is to support a set of common properties between various
SOCs.

In this initial step, let's also document a top level description of
the MMC power sequence and describe the compatible string used for the
simple MMC power sequence provider.

The simple MMC power sequence provider will initially support a reset
GPIO. From several earlier posted patches, it's clear that such
hardware exists. Especially some WLAN chips which are attached to an
SDIO interface may use a GPIO reset.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changed in v4:
	- None.

---
 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt    | 20 ++++++++++++++++++++
 Documentation/devicetree/bindings/mmc/mmc.txt        | 14 ++++++++++++++
 2 files changed, 34 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..da333d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -0,0 +1,20 @@
+* The simple MMC power sequence provider
+
+The purpose of the simple MMC power sequence provider is to supports a set of
+common properties between various SOC designs. It thus enables us to use the
+same provider for several SOC designs.
+
+Required properties:
+- compatible : contains "mmc-pwrseq-simple".
+
+Optional properties:
+- reset-gpios : contains a GPIO specifier. The reset GPIO is asserted at
+	initialization and prior we start the power up procedure of the card. It
+	will be de-asserted right after the power has been provided to the card.
+
+Example:
+
+	sdhci0_pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		reset-gpios = <&gpio1 12 0>;
+	}
diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index bac1311..438899e 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -65,6 +65,18 @@ Optional SDIO properties:
 - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion
 
 
+MMC power sequences:
+--------------------
+
+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.
+
+Optional property:
+- 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 +113,7 @@ sdhci at ab000000 {
 	max-frequency = <50000000>;
 	keep-power-in-suspend;
 	enable-sdio-wakeup;
+	mmc-pwrseq = <&sdhci0_pwrseq>
 }
 
 Example with sdio function subnode:
@@ -114,6 +127,7 @@ mmc3: mmc at 01c12000 {
 	vmmc-supply = <&reg_vmmc3>;
 	bus-width = <4>;
 	non-removable;
+	mmc-pwrseq = <&sdhci0_pwrseq>
 	status = "okay";
 
 	brcmf: bcrmf at 1 {
-- 
1.9.1

[PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

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

Changes in v4:
	- Fixed call to kfree().

---
 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 bd08772..2cea00e 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_pre_power_on(struct mmc_host *host)
diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
index 12aaf2b..bd860d8 100644
--- a/drivers/mmc/core/pwrseq.h
+++ b/drivers/mmc/core/pwrseq.h
@@ -27,6 +27,8 @@ void mmc_pwrseq_post_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..61c991e
--- /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

[PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

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

Changes in v4:
	- Fixed call to kfree().

---
 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 bd08772..2cea00e 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_pre_power_on(struct mmc_host *host)
diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
index 12aaf2b..bd860d8 100644
--- a/drivers/mmc/core/pwrseq.h
+++ b/drivers/mmc/core/pwrseq.h
@@ -27,6 +27,8 @@ void mmc_pwrseq_post_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..61c991e
--- /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

[PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Ulf Hansson]

The need for reset GPIOs has several times been pointed out from
erlier posted patchsets. Especially some WLAN chips which are
attached to an SDIO interface may use a GPIO reset.

The reset GPIO is asserted at initialization and prior we start the
power up procedure. The GPIO will be de-asserted right after the power
has been provided to the card, from the ->post_power_on() callback.

Note, the reset GPIO is optional. Thus we don't return an error even if
we can't find a GPIO for the consumer.

Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---

Changes in v4:
	- Fixed call to kfree().

---
 drivers/mmc/core/pwrseq_simple.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 61c991e..0958c69 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_pre_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, 1);
+}
+
+static void mmc_pwrseq_simple_post_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 = {
+	.pre_power_on = mmc_pwrseq_simple_pre_power_on,
+	.post_power_on = mmc_pwrseq_simple_post_power_on,
+	.power_off = mmc_pwrseq_simple_pre_power_on,
 	.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

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

[PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Ulf Hansson]

The need for reset GPIOs has several times been pointed out from
erlier posted patchsets. Especially some WLAN chips which are
attached to an SDIO interface may use a GPIO reset.

The reset GPIO is asserted at initialization and prior we start the
power up procedure. The GPIO will be de-asserted right after the power
has been provided to the card, from the ->post_power_on() callback.

Note, the reset GPIO is optional. Thus we don't return an error even if
we can't find a GPIO for the consumer.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v4:
	- Fixed call to kfree().

---
 drivers/mmc/core/pwrseq_simple.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 61c991e..0958c69 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_pre_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, 1);
+}
+
+static void mmc_pwrseq_simple_post_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 = {
+	.pre_power_on = mmc_pwrseq_simple_pre_power_on,
+	.post_power_on = mmc_pwrseq_simple_post_power_on,
+	.power_off = mmc_pwrseq_simple_pre_power_on,
 	.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

Re: [PATCH V4 0/4] mmc: core: Add support for MMC power sequences

[Ulf Hansson]

On 19 January 2015 at 10:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Changes in v4:
>         - Fixed call to kfree() in patch3 and patch4.
>
> Changes in v3:
>         Fixed comments from Mark Rutland:
>         - Document binding in one patch to get the big picture.
>         - Keep code and DT document consistent around how many GPIO resets we
>           support. I decided to go for one GPIO, we can extend that if needed
>           later on.
>         - Change compatible string for simple MMC power sequence and renamed the
>           file for its documentation.
>         - Updated commit messages according to above changes.
>
> Changes in v2:
>         Fixed comments from Russell King:
>         - Renamed pwrseq callbacks and corresponding interface functions.
>         - Move the call to the previous namned ->power_on() callback, to the
>           end of mmc_power_up() to get consistent behavior.
>
>
> 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.
>
> 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  |  20 ++++
>  Documentation/devicetree/bindings/mmc/mmc.txt      |  14 +++
>  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, 284 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
>

To get some more testing of this code, I have now applied this v4 of
this patchset for my next branch.

Kind regards
Uffe

[PATCH V4 0/4] mmc: core: Add support for MMC power sequences

[Ulf Hansson]

On 19 January 2015 at 10:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Changes in v4:
>         - Fixed call to kfree() in patch3 and patch4.
>
> Changes in v3:
>         Fixed comments from Mark Rutland:
>         - Document binding in one patch to get the big picture.
>         - Keep code and DT document consistent around how many GPIO resets we
>           support. I decided to go for one GPIO, we can extend that if needed
>           later on.
>         - Change compatible string for simple MMC power sequence and renamed the
>           file for its documentation.
>         - Updated commit messages according to above changes.
>
> Changes in v2:
>         Fixed comments from Russell King:
>         - Renamed pwrseq callbacks and corresponding interface functions.
>         - Move the call to the previous namned ->power_on() callback, to the
>           end of mmc_power_up() to get consistent behavior.
>
>
> 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.
>
> 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  |  20 ++++
>  Documentation/devicetree/bindings/mmc/mmc.txt      |  14 +++
>  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, 284 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
>

To get some more testing of this code, I have now applied this v4 of
this patchset for my next branch.

Kind regards
Uffe

Re: [PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
>  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);

Any reason to not use the devm_gpiod_get_index() managed version instead?

AFAICT mmc_free_host() will free the device so in that case you won't
need to call gpiod_put() in mmc_pwrseq_simple_free().

This will also make easier to extend pwrseq_simple to support multiple
GPIOs like the DT binding implies.

Best regards,
Javier

[PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
>  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);

Any reason to not use the devm_gpiod_get_index() managed version instead?

AFAICT mmc_free_host() will free the device so in that case you won't
need to call gpiod_put() in mmc_pwrseq_simple_free().

This will also make easier to extend pwrseq_simple to support multiple
GPIOs like the DT binding implies.

Best regards,
Javier

Re: [PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Ulf Hansson]

On 23 January 2015 at 16:56, Javier Martinez Canillas
<javier-0uQlZySMnqxg9hUCZPvPmw@public.gmane.org> wrote:
> Hello Ulf,
>
> On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>
>>  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);
>
> Any reason to not use the devm_gpiod_get_index() managed version instead?

This struct device don't have a bound driver to it. Thus this device
won't be freed automagically from the ->remove() or failed ->probe()
path.

>
> AFAICT mmc_free_host() will free the device so in that case you won't
> need to call gpiod_put() in mmc_pwrseq_simple_free().
>
> This will also make easier to extend pwrseq_simple to support multiple
> GPIOs like the DT binding implies.
>

Kind regards
Uffe
--
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

[PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Ulf Hansson]

On 23 January 2015 at 16:56, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Ulf,
>
> On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>>  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);
>
> Any reason to not use the devm_gpiod_get_index() managed version instead?

This struct device don't have a bound driver to it. Thus this device
won't be freed automagically from the ->remove() or failed ->probe()
path.

>
> AFAICT mmc_free_host() will free the device so in that case you won't
> need to call gpiod_put() in mmc_pwrseq_simple_free().
>
> This will also make easier to extend pwrseq_simple to support multiple
> GPIOs like the DT binding implies.
>

Kind regards
Uffe

Re: [PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Javier Martinez Canillas]

Hello Ulf,

On Fri, Jan 23, 2015 at 6:01 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> Any reason to not use the devm_gpiod_get_index() managed version instead?
>
> This struct device don't have a bound driver to it. Thus this device
> won't be freed automagically from the ->remove() or failed ->probe()
> path.
>

Right, I missed that. Thanks a lot for the clarification.

Best regards,
Javier

[PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Javier Martinez Canillas]

Hello Ulf,

On Fri, Jan 23, 2015 at 6:01 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> Any reason to not use the devm_gpiod_get_index() managed version instead?
>
> This struct device don't have a bound driver to it. Thus this device
> won't be freed automagically from the ->remove() or failed ->probe()
> path.
>

Right, I missed that. Thanks a lot for the clarification.

Best regards,
Javier

Re: [PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 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_pre_power_on()- Invoked in the beginning of mmc_power_up().
> mmc_pwrseq_post_power_on()- Invoked at the end in mmc_power_up().
> mmc_pwrseq_power_off()- Invoked from mmc_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>
> ---

Patch looks good to me and I could successfully get the SDIO wlan chip
in the Snow Chromebook after extending the pwrseq_simple driver so on
Exynos5250 Snow Chromebook:

Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

[PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 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_pre_power_on()- Invoked in the beginning of mmc_power_up().
> mmc_pwrseq_post_power_on()- Invoked at the end in mmc_power_up().
> mmc_pwrseq_power_off()- Invoked from mmc_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>
> ---

Patch looks good to me and I could successfully get the SDIO wlan chip
in the Snow Chromebook after extending the pwrseq_simple driver so on
Exynos5250 Snow Chromebook:

Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

Re: [PATCH V4 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> To support SOCs which specifies specific MMC power sequences, document
> some MMC DT bindings to be able to describe these hardwares.
>
> Let's also document bindings for a simple MMC power sequence provider,
> which purpose is to support a set of common properties between various
> SOCs.
>
> In this initial step, let's also document a top level description of
> the MMC power sequence and describe the compatible string used for the
> simple MMC power sequence provider.
>
> The simple MMC power sequence provider will initially support a reset
> GPIO. From several earlier posted patches, it's clear that such
> hardware exists. Especially some WLAN chips which are attached to an
> SDIO interface may use a GPIO reset.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

[PATCH V4 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> To support SOCs which specifies specific MMC power sequences, document
> some MMC DT bindings to be able to describe these hardwares.
>
> Let's also document bindings for a simple MMC power sequence provider,
> which purpose is to support a set of common properties between various
> SOCs.
>
> In this initial step, let's also document a top level description of
> the MMC power sequence and describe the compatible string used for the
> simple MMC power sequence provider.
>
> The simple MMC power sequence provider will initially support a reset
> GPIO. From several earlier posted patches, it's clear that such
> hardware exists. Especially some WLAN chips which are attached to an
> SDIO interface may use a GPIO reset.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

Re: [PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 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>
> ---
>

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

[PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 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>
> ---
>

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

Re: [PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The need for reset GPIOs has several times been pointed out from
> erlier posted patchsets. Especially some WLAN chips which are
> attached to an SDIO interface may use a GPIO reset.
>
> The reset GPIO is asserted at initialization and prior we start the
> power up procedure. The GPIO will be de-asserted right after the power
> has been provided to the card, from the ->post_power_on() callback.
>
> Note, the reset GPIO is optional. Thus we don't return an error even if
> we can't find a GPIO for the consumer.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

[PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Javier Martinez Canillas]

Hello Ulf,

On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The need for reset GPIOs has several times been pointed out from
> erlier posted patchsets. Especially some WLAN chips which are
> attached to an SDIO interface may use a GPIO reset.
>
> The reset GPIO is asserted at initialization and prior we start the
> power up procedure. The GPIO will be de-asserted right after the power
> has been provided to the card, from the ->post_power_on() callback.
>
> Note, the reset GPIO is optional. Thus we don't return an error even if
> we can't find a GPIO for the consumer.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

Re: [PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Ulf Hansson]

On 28 January 2015 at 11:17, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Ulf,
>
> On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> 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_pre_power_on()- Invoked in the beginning of mmc_power_up().
>> mmc_pwrseq_post_power_on()- Invoked at the end in mmc_power_up().
>> mmc_pwrseq_power_off()- Invoked from mmc_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>
>> ---
>
> Patch looks good to me and I could successfully get the SDIO wlan chip
> in the Snow Chromebook after extending the pwrseq_simple driver so on
> Exynos5250 Snow Chromebook:

By extending you mean?

>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>
> Best regards,
> Javier

Thanks for testing and reviewing! I have applied your tags to the patches.

Kind regards
Uffe

[PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Ulf Hansson]

On 28 January 2015 at 11:17, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Ulf,
>
> On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> 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_pre_power_on()- Invoked in the beginning of mmc_power_up().
>> mmc_pwrseq_post_power_on()- Invoked at the end in mmc_power_up().
>> mmc_pwrseq_power_off()- Invoked from mmc_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>
>> ---
>
> Patch looks good to me and I could successfully get the SDIO wlan chip
> in the Snow Chromebook after extending the pwrseq_simple driver so on
> Exynos5250 Snow Chromebook:

By extending you mean?

>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>
> Best regards,
> Javier

Thanks for testing and reviewing! I have applied your tags to the patches.

Kind regards
Uffe

Re: [PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Javier Martinez Canillas]

Hello Ulf,

On Wed, Jan 28, 2015 at 12:36 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> ---
>>
>> Patch looks good to me and I could successfully get the SDIO wlan chip
>> in the Snow Chromebook after extending the pwrseq_simple driver so on
>> Exynos5250 Snow Chromebook:
>
> By extending you mean?
>

Sorry, I shoud had mentioned it. I meant the "Add multiple GPIO and
external clock to MMC pwrseq_simple" series [0] I posted before.

>>
>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>
>> Best regards,
>> Javier
>
> Thanks for testing and reviewing! I have applied your tags to the patches.
>

You are welcome, thanks a lot for working on this.

Best regards,
Javier

[0]: http://www.spinics.net/lists/arm-kernel/msg395445.html

[PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Javier Martinez Canillas]

Hello Ulf,

On Wed, Jan 28, 2015 at 12:36 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> ---
>>
>> Patch looks good to me and I could successfully get the SDIO wlan chip
>> in the Snow Chromebook after extending the pwrseq_simple driver so on
>> Exynos5250 Snow Chromebook:
>
> By extending you mean?
>

Sorry, I shoud had mentioned it. I meant the "Add multiple GPIO and
external clock to MMC pwrseq_simple" series [0] I posted before.

>>
>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>
>> Best regards,
>> Javier
>
> Thanks for testing and reviewing! I have applied your tags to the patches.
>

You are welcome, thanks a lot for working on this.

Best regards,
Javier

[0]: http://www.spinics.net/lists/arm-kernel/msg395445.html

Re: [PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Ulf Hansson]

+Russell King

On 30 January 2015 at 23:27, NeilBrown <neilb@suse.de> wrote:
>
> On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index d5c176e87951..1be7055548cb 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"
>> @@ -1615,6 +1616,8 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
>>
>>       mmc_host_clk_hold(host);
>>
>> +     mmc_pwrseq_pre_power_on(host);
>> +
>>       host->ios.vdd = fls(ocr) - 1;
>>       host->ios.power_mode = MMC_POWER_UP;
>>       /* Set initial state and call mmc_set_ios */
>> @@ -1645,6 +1648,8 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
>>        */
>>       mmc_delay(10);
>>
>> +     mmc_pwrseq_post_power_on(host);
>> +
>>       mmc_host_clk_release(host);
>>  }
>
> Hi Ulf,
>  I think this may be too late for the _post_power_on() call.
>
> This is exactly where I release the reset line in my patch, and I've just discovered
> that it is causing one of my problems.
>
> Shortly before this call is
>
>         host->ios.power_mode = MMC_POWER_ON;
>         mmc_set_ios(host);
>
> and omap_hsmmc_set_ios() contains:
>
>                 switch (ios->power_mode) {
>                 ....
>                 case MMC_POWER_ON:
>                         do_send_init_stream = 1;
>                         break;
>
>
>         if (do_send_init_stream)
>                 send_init_stream(host);
>
> Which sends the "init stream" to the card.
> If the card is still being reset at this time, the stream may not be effective.
>
> I find that about 10%-20% of the time when I release the reset line *after* the
> sequence is sent, my card fails to initialised.  When I release *before* the sequence
> is sent, it never fails.

Okay, thanks for providing these details.

>
> I note that other drivers handle the init stream differently.
> atmel-mci just sets a flag in set_ios, and the actually sends the stream in
> atmci_start_request()
>
> dw_mmc, jz4740_mmc, mxcmmc, pxamci do much the same
>
> mmc_spi does it during set_ios like omap_hsmmc.
>
> The others I am not able to interpret easily.
>
> So it might make sense to change omap_hsmmc and mmc_spi to delay the init
> stream until later, but I think it is probably safest to move the post_power_on call
> to before the set_ios call.

Well, the problem is that there are host drivers that don't consider
MMC_POWER_UP and delays initialization/power_up to MMC_POWER_ON. So we
might fix the issue for some, but breaks it for another.

I had a discussion around inconsistent host driver behaviours from
->set_ios() callbacks with Russell, for the first version of this
patchset.

That's why I converted to mmc_pwrseq_post_power_on() instead of
mmc_pwrseq_power_on() as it was in v1. At that point I didn't realize
that the "initstream" sequence is an important factor to consider here
as well.

Additionally, those host drivers that ignores MMC_POWER_UP definitely
behaves wrong. I did a research about this earlier and decided to go
for a deeper look one more time. Actually I found only three drivers
that needs some attention and surely those are easy to fix.

au1xmmc
cb710-mmc
toshsd

To have something working for 3.20, I suggest we follow your proposal
about moving the call to mmc_pwrseq_post_power_on() to before the call
to ->set_ios() with MMC_POWER_ON . I send a patch we can test.

For reference, I still agree with Russell's proposal that the proper
solution, long-term wise, should be to replace/extend
MMC_POWER_UP|ON|OFF with new host callbacks. That will enable each
host to perform its own optimized power up/off sequence.

Kind regards
Uffe

Re: [PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Russell King - ARM Linux]

On Mon, Feb 02, 2015 at 03:57:37PM +0100, Ulf Hansson wrote:
> On 30 January 2015 at 23:27, NeilBrown <neilb@suse.de> wrote:
> > Shortly before this call is
> >
> >         host->ios.power_mode = MMC_POWER_ON;
> >         mmc_set_ios(host);
> >
> > and omap_hsmmc_set_ios() contains:
> >
> >                 switch (ios->power_mode) {
> >                 ....
> >                 case MMC_POWER_ON:
> >                         do_send_init_stream = 1;
> >                         break;
> >
> >
> >         if (do_send_init_stream)
> >                 send_init_stream(host);
> >
> > Which sends the "init stream" to the card.
> > If the card is still being reset at this time, the stream may not
> > be effective.
> >
> > I find that about 10%-20% of the time when I release the reset line
> > *after* the sequence is sent, my card fails to initialised.  When I
> > release *before* the sequence is sent, it never fails.
> 
> Okay, thanks for providing these details.

The right thing to do then is to pulse the reset line at the pre-power-up
stage.

> > I note that other drivers handle the init stream differently.
> > atmel-mci just sets a flag in set_ios, and the actually sends the stream in
> > atmci_start_request()
> >
> > dw_mmc, jz4740_mmc, mxcmmc, pxamci do much the same

Yes.  PXAMCI is one of those dumb "inteligent" controllers that takes
some of the protocol handing away from the host software implementation.
The result of that is things happen at different times.

For PXAMCI, it's not possible to send the initialisation clocks on their
own; you have to queue up a command first.

> Well, the problem is that there are host drivers that don't consider
> MMC_POWER_UP and delays initialization/power_up to MMC_POWER_ON. So we
> might fix the issue for some, but breaks it for another.
> 
> I had a discussion around inconsistent host driver behaviours from
> ->set_ios() callbacks with Russell, for the first version of this
> patchset.

Yes, as I've said a few times now, I regard my original design of this
stuff (which was to separate MMC_POWER_UP and MMC_POWER_ON) to be a big
design mistake, and we should really kill MMC_POWER_UP ASAP, combining
both into a /single/ call into the host driver.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Ulf Hansson]

On 2 February 2015 at 16:05, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Feb 02, 2015 at 03:57:37PM +0100, Ulf Hansson wrote:
>> On 30 January 2015 at 23:27, NeilBrown <neilb@suse.de> wrote:
>> > Shortly before this call is
>> >
>> >         host->ios.power_mode = MMC_POWER_ON;
>> >         mmc_set_ios(host);
>> >
>> > and omap_hsmmc_set_ios() contains:
>> >
>> >                 switch (ios->power_mode) {
>> >                 ....
>> >                 case MMC_POWER_ON:
>> >                         do_send_init_stream = 1;
>> >                         break;
>> >
>> >
>> >         if (do_send_init_stream)
>> >                 send_init_stream(host);
>> >
>> > Which sends the "init stream" to the card.
>> > If the card is still being reset at this time, the stream may not
>> > be effective.
>> >
>> > I find that about 10%-20% of the time when I release the reset line
>> > *after* the sequence is sent, my card fails to initialised.  When I
>> > release *before* the sequence is sent, it never fails.
>>
>> Okay, thanks for providing these details.
>
> The right thing to do then is to pulse the reset line at the pre-power-up
> stage.

I don't think that will work, since the card hasn't yet been powered
and will thus not respond properly to the reset.

Kind regards
Uffe

Re: [PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1955 bytes --]

On Mon, 2 Feb 2015 16:10:29 +0100 Ulf Hansson <ulf.hansson@linaro.org> wrote:

> On 2 February 2015 at 16:05, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Feb 02, 2015 at 03:57:37PM +0100, Ulf Hansson wrote:
> >> On 30 January 2015 at 23:27, NeilBrown <neilb@suse.de> wrote:
> >> > Shortly before this call is
> >> >
> >> >         host->ios.power_mode = MMC_POWER_ON;
> >> >         mmc_set_ios(host);
> >> >
> >> > and omap_hsmmc_set_ios() contains:
> >> >
> >> >                 switch (ios->power_mode) {
> >> >                 ....
> >> >                 case MMC_POWER_ON:
> >> >                         do_send_init_stream = 1;
> >> >                         break;
> >> >
> >> >
> >> >         if (do_send_init_stream)
> >> >                 send_init_stream(host);
> >> >
> >> > Which sends the "init stream" to the card.
> >> > If the card is still being reset at this time, the stream may not
> >> > be effective.
> >> >
> >> > I find that about 10%-20% of the time when I release the reset line
> >> > *after* the sequence is sent, my card fails to initialised.  When I
> >> > release *before* the sequence is sent, it never fails.
> >>
> >> Okay, thanks for providing these details.
> >
> > The right thing to do then is to pulse the reset line at the pre-power-up
> > stage.
> 
> I don't think that will work, since the card hasn't yet been powered
> and will thus not respond properly to the reset.

My understanding, at least in the case of my hardware, is that the power-on
is sufficient to reset the device.
In my case, the one supply is shared between two devices (wifi and bluetooth)
so if the bluetooth is on, then the wifi cannot be power-cycled.  That is
when the reset is particularly needed.

I'll try to arrange some testing to confirm this is the case.  Of course,
this would not be general result, it would only be relevant for my hardware.

NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[Ulf Hansson]

On 2 February 2015 at 23:10, NeilBrown <neilb@suse.de> wrote:
> On Mon, 2 Feb 2015 16:10:29 +0100 Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
>> On 2 February 2015 at 16:05, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Mon, Feb 02, 2015 at 03:57:37PM +0100, Ulf Hansson wrote:
>> >> On 30 January 2015 at 23:27, NeilBrown <neilb@suse.de> wrote:
>> >> > Shortly before this call is
>> >> >
>> >> >         host->ios.power_mode = MMC_POWER_ON;
>> >> >         mmc_set_ios(host);
>> >> >
>> >> > and omap_hsmmc_set_ios() contains:
>> >> >
>> >> >                 switch (ios->power_mode) {
>> >> >                 ....
>> >> >                 case MMC_POWER_ON:
>> >> >                         do_send_init_stream = 1;
>> >> >                         break;
>> >> >
>> >> >
>> >> >         if (do_send_init_stream)
>> >> >                 send_init_stream(host);
>> >> >
>> >> > Which sends the "init stream" to the card.
>> >> > If the card is still being reset at this time, the stream may not
>> >> > be effective.
>> >> >
>> >> > I find that about 10%-20% of the time when I release the reset line
>> >> > *after* the sequence is sent, my card fails to initialised.  When I
>> >> > release *before* the sequence is sent, it never fails.
>> >>
>> >> Okay, thanks for providing these details.
>> >
>> > The right thing to do then is to pulse the reset line at the pre-power-up
>> > stage.
>>
>> I don't think that will work, since the card hasn't yet been powered
>> and will thus not respond properly to the reset.
>
> My understanding, at least in the case of my hardware, is that the power-on
> is sufficient to reset the device.
> In my case, the one supply is shared between two devices (wifi and bluetooth)
> so if the bluetooth is on, then the wifi cannot be power-cycled.  That is
> when the reset is particularly needed.
>
> I'll try to arrange some testing to confirm this is the case.  Of course,
> this would not be general result, it would only be relevant for my hardware.

I refreshed my memory for how the cw1200 WLAN device works on the
ux500 SOC. I decided to post the complete information here, also for
my own reference.

These resources need to be controlled during the power sequence of CW1200:
There are two GPIOs (one reset and one for SPI/SDIO selection), one LP
(low power) clock and a few power supplies.

The sequence for power up that needs to be followed are:
1. Keep reset GPIO asserted.
2. Enable power supplies.
3. Enable LP clock.
4. Wait a few LP clock cycles to let power/clock stabilize.
5. Make sure SPI/SDIO select pin is in SDIO mode. The value will be
sampled by the WLAN device when reset GPIO is de-asserted.
6. De-assert reset GPIO.
7. Wait 30 ms for the WLAN device to power up.

The power supplies may also be shared with a bluetooth device. That
also means the reset GPIO must be kept asserted in power off state, to
prevent power up of the WLAN device.

>From the above, it's clear that toggling the reset GPIOs in the
->pre_power_on() phase won't work for CW1200/UX500 case.  That in
conjunction with the "init stream" issue tells me that we need to
progress with the below patch:

"mmc: core: Invoke mmc_pwrseq_post_power_on() prior MMC_POWER_ON state"
http://www.spinics.net/lists/arm-kernel/msg396592.html

Kind regards
Uffe

Re: [PATCH V4 1/4] mmc: core: Initial support for MMC power sequences

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 6722 bytes --]

On Tue, 3 Feb 2015 13:39:13 +0100 Ulf Hansson <ulf.hansson@linaro.org> wrote:

> On 2 February 2015 at 23:10, NeilBrown <neilb@suse.de> wrote:
> > On Mon, 2 Feb 2015 16:10:29 +0100 Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> >> On 2 February 2015 at 16:05, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >> > On Mon, Feb 02, 2015 at 03:57:37PM +0100, Ulf Hansson wrote:
> >> >> On 30 January 2015 at 23:27, NeilBrown <neilb@suse.de> wrote:
> >> >> > Shortly before this call is
> >> >> >
> >> >> >         host->ios.power_mode = MMC_POWER_ON;
> >> >> >         mmc_set_ios(host);
> >> >> >
> >> >> > and omap_hsmmc_set_ios() contains:
> >> >> >
> >> >> >                 switch (ios->power_mode) {
> >> >> >                 ....
> >> >> >                 case MMC_POWER_ON:
> >> >> >                         do_send_init_stream = 1;
> >> >> >                         break;
> >> >> >
> >> >> >
> >> >> >         if (do_send_init_stream)
> >> >> >                 send_init_stream(host);
> >> >> >
> >> >> > Which sends the "init stream" to the card.
> >> >> > If the card is still being reset at this time, the stream may not
> >> >> > be effective.
> >> >> >
> >> >> > I find that about 10%-20% of the time when I release the reset line
> >> >> > *after* the sequence is sent, my card fails to initialised.  When I
> >> >> > release *before* the sequence is sent, it never fails.
> >> >>
> >> >> Okay, thanks for providing these details.
> >> >
> >> > The right thing to do then is to pulse the reset line at the pre-power-up
> >> > stage.
> >>
> >> I don't think that will work, since the card hasn't yet been powered
> >> and will thus not respond properly to the reset.
> >
> > My understanding, at least in the case of my hardware, is that the power-on
> > is sufficient to reset the device.
> > In my case, the one supply is shared between two devices (wifi and bluetooth)
> > so if the bluetooth is on, then the wifi cannot be power-cycled.  That is
> > when the reset is particularly needed.
> >
> > I'll try to arrange some testing to confirm this is the case.  Of course,
> > this would not be general result, it would only be relevant for my hardware.
> 
> I refreshed my memory for how the cw1200 WLAN device works on the
> ux500 SOC. I decided to post the complete information here, also for
> my own reference.
> 
> These resources need to be controlled during the power sequence of CW1200:
> There are two GPIOs (one reset and one for SPI/SDIO selection), one LP
> (low power) clock and a few power supplies.
> 
> The sequence for power up that needs to be followed are:
> 1. Keep reset GPIO asserted.
> 2. Enable power supplies.
> 3. Enable LP clock.
> 4. Wait a few LP clock cycles to let power/clock stabilize.
> 5. Make sure SPI/SDIO select pin is in SDIO mode. The value will be
> sampled by the WLAN device when reset GPIO is de-asserted.
> 6. De-assert reset GPIO.
> 7. Wait 30 ms for the WLAN device to power up.
> 
> The power supplies may also be shared with a bluetooth device. That
> also means the reset GPIO must be kept asserted in power off state, to
> prevent power up of the WLAN device.
> 
> >From the above, it's clear that toggling the reset GPIOs in the
> ->pre_power_on() phase won't work for CW1200/UX500 case.  That in
> conjunction with the "init stream" issue tells me that we need to
> progress with the below patch:
> 
> "mmc: core: Invoke mmc_pwrseq_post_power_on() prior MMC_POWER_ON state"
> http://www.spinics.net/lists/arm-kernel/msg396592.html
> 
> Kind regards
> Uffe

+ gta04-owner.

Thanks for those details.  They reminded me that I have seen something
similar for the wifi chip I use, and I found it here:

 http://lists.goldelico.com/pipermail/gta04-owner/2012-March/001917.html

In particular:

  After power-up the reset line should be active for >500us and then
  go high. A short impulse (1-20 us) can later reset the WiFi part.

Also, the wifi and bluetooth share both a power supply and a reset line.
However:

  BT also does not need a reset (unless you try to communicate
  with wrong baudrate and the HCI protocol gets unsynchronized).

  An impulse >5ms does a BT reset. Or a break condition on the
  RX line.


My experiments confirm that if the bluetooth is inactive so that the mmc host
is actually able to power-cycle the wifi, then the reset line isn't needed.
However if the bluetooth is holding the power supply enabled, then we
definitely need the reset to be pulsed low in mmc_power_up(), or the wifi
doesn't initialise properly.

Putting this all together, it seems like the ideal would be:
 - if power supply is already on, pulse reset for 20us, else
   pull reset low
 - enable power supply
 - wait 1ms
 - raise reset line
 - wait 30ms

This should ensure that the BT doesn't get reset, but that the wifi does.

However my further test results raise some questions.
If I have  BT active and pulse the reset for 1ms (using mdelay(1)), the wifi
doesn't initialise.  I get ETIMEDOUT from mmc_send_io_op_cond.
I need to increase the delay to mdelay(10) before that error goes away.

With that in place the wifi seems to work, but I get regular

  Bluetooth: hci0 hardware error 0x37

errors.  So I'm guessing that tying the two reset lines together might not
have been such a good idea.  I don't know yet how that actually affect any
active bluetooth session.

Even with the 10ms delay I do sometimes get errors ... I think.  I only have
concrete evidence when I was testing with "mmc_delay(1)" which in practice
appears to often delay for 10ms or more, probably due to the cond_resched();

However I sometimes got messages like:

[ 5349.438903] mmc1: queuing unknown CIS tuple 0x36 (1 bytes)
[ 5349.502868] mmc1: queuing unknown CIS tuple 0x0c (97 bytes)
[ 5349.518737] mmc1: queuing unknown CIS tuple 0x01 (16 bytes)

If I still get these with the more reliable mdelay(), I'm leaning towards simply trying
sdio_init_card() within mmc_sdio_power_restore().  The errors occur less than 1% of the
time, so this might be effective.

With respect to the current patch, this all simply means that a 10ms down
pulse on the reset is needed.  You currently have mmc_delay(10) in the and
that should be sufficient.  mmc_delay() may take longer than the requested
time, but should never take less time.

I had thought that a strict upper limit on the pulse might be important for
BT.  It seems that I cannot leave BT completely unaffected but, on my device
at least, holding reset low whenever wifi is not enabled would not be good as
it would break BT.

Thanks,
NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

[Alexandre Courbot]

On Mon, Jan 19, 2015 at 6:13 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 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>
> ---
>
> Changes in v4:
>         - Fixed call to kfree().
>
> ---
>  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 bd08772..2cea00e 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_pre_power_on(struct mmc_host *host)
> diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
> index 12aaf2b..bd860d8 100644
> --- a/drivers/mmc/core/pwrseq.h
> +++ b/drivers/mmc/core/pwrseq.h
> @@ -27,6 +27,8 @@ void mmc_pwrseq_post_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..61c991e
> --- /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;

How about making this function return a struct mmc_pwrseq * so this
last line can be moved to mmc_pwrseq_alloc() instead of requiring all
power sequences to do it?

The same applies to

    host->pwrseq = NULL;

in mmc_pwrseq_simple_free(), which could be done in mmc_pwrseq_free() it seems.

[PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

[Alexandre Courbot]

On Mon, Jan 19, 2015 at 6:13 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 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>
> ---
>
> Changes in v4:
>         - Fixed call to kfree().
>
> ---
>  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 bd08772..2cea00e 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_pre_power_on(struct mmc_host *host)
> diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
> index 12aaf2b..bd860d8 100644
> --- a/drivers/mmc/core/pwrseq.h
> +++ b/drivers/mmc/core/pwrseq.h
> @@ -27,6 +27,8 @@ void mmc_pwrseq_post_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..61c991e
> --- /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;

How about making this function return a struct mmc_pwrseq * so this
last line can be moved to mmc_pwrseq_alloc() instead of requiring all
power sequences to do it?

The same applies to

    host->pwrseq = NULL;

in mmc_pwrseq_simple_free(), which could be done in mmc_pwrseq_free() it seems.

Re: [PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Alexandre Courbot]

On Mon, Jan 19, 2015 at 6:13 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The need for reset GPIOs has several times been pointed out from
> erlier posted patchsets. Especially some WLAN chips which are
> attached to an SDIO interface may use a GPIO reset.
>
> The reset GPIO is asserted at initialization and prior we start the
> power up procedure. The GPIO will be de-asserted right after the power
> has been provided to the card, from the ->post_power_on() callback.
>
> Note, the reset GPIO is optional. Thus we don't return an error even if
> we can't find a GPIO for the consumer.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Excellent, with this I can probe the wifi chip on NVIDIA SHIELD which
requires a GPIO to be high for reset to be de-asserted.

The series:

Tested-by: Alexandre Courbot <acourbot@nvidia.com>

> ---
>
> Changes in v4:
>         - Fixed call to kfree().
>
> ---
>  drivers/mmc/core/pwrseq_simple.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
> index 61c991e..0958c69 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_pre_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, 1);
> +}
> +
> +static void mmc_pwrseq_simple_post_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 = {
> +       .pre_power_on = mmc_pwrseq_simple_pre_power_on,
> +       .post_power_on = mmc_pwrseq_simple_post_power_on,
> +       .power_off = mmc_pwrseq_simple_pre_power_on,
>         .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);

gpiod_get() will translate to exactly the same, with less characters.

Actually I see that the version in -next has support for multiple
GPIOs. You will probably want to look at Rojhalat's latest work on
GPIO arrays:

http://permalink.gmane.org/gmane.linux.kernel.gpio/6126

This code would be a great candidate to use this GPIO array API, but
since it is not in -next yet (should happen soon though) you might
want to consider doing it later.

Btw, I wasted a considerable amount of time on one of the defunct
previous attempts at power sequences, so I'm interested in reviewing
future versions of this patchset if you don't mind adding me to the CC
list. :)

[PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Alexandre Courbot]

On Mon, Jan 19, 2015 at 6:13 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The need for reset GPIOs has several times been pointed out from
> erlier posted patchsets. Especially some WLAN chips which are
> attached to an SDIO interface may use a GPIO reset.
>
> The reset GPIO is asserted at initialization and prior we start the
> power up procedure. The GPIO will be de-asserted right after the power
> has been provided to the card, from the ->post_power_on() callback.
>
> Note, the reset GPIO is optional. Thus we don't return an error even if
> we can't find a GPIO for the consumer.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Excellent, with this I can probe the wifi chip on NVIDIA SHIELD which
requires a GPIO to be high for reset to be de-asserted.

The series:

Tested-by: Alexandre Courbot <acourbot@nvidia.com>

> ---
>
> Changes in v4:
>         - Fixed call to kfree().
>
> ---
>  drivers/mmc/core/pwrseq_simple.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
> index 61c991e..0958c69 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_pre_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, 1);
> +}
> +
> +static void mmc_pwrseq_simple_post_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 = {
> +       .pre_power_on = mmc_pwrseq_simple_pre_power_on,
> +       .post_power_on = mmc_pwrseq_simple_post_power_on,
> +       .power_off = mmc_pwrseq_simple_pre_power_on,
>         .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);

gpiod_get() will translate to exactly the same, with less characters.

Actually I see that the version in -next has support for multiple
GPIOs. You will probably want to look at Rojhalat's latest work on
GPIO arrays:

http://permalink.gmane.org/gmane.linux.kernel.gpio/6126

This code would be a great candidate to use this GPIO array API, but
since it is not in -next yet (should happen soon though) you might
want to consider doing it later.

Btw, I wasted a considerable amount of time on one of the defunct
previous attempts at power sequences, so I'm interested in reviewing
future versions of this patchset if you don't mind adding me to the CC
list. :)

Re: [PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

[Ulf Hansson]

[...]

>> +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;
>
> How about making this function return a struct mmc_pwrseq * so this
> last line can be moved to mmc_pwrseq_alloc() instead of requiring all
> power sequences to do it?
>
> The same applies to
>
>     host->pwrseq = NULL;
>
> in mmc_pwrseq_simple_free(), which could be done in mmc_pwrseq_free() it seems.

Thanks for reviewing!

I like you suggestion, though $subject patch is already part of the PR for 3.20.

Feel free to send a patch, I would happily apply it.

Kind regards
Uffe

[PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

[Ulf Hansson]

[...]

>> +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;
>
> How about making this function return a struct mmc_pwrseq * so this
> last line can be moved to mmc_pwrseq_alloc() instead of requiring all
> power sequences to do it?
>
> The same applies to
>
>     host->pwrseq = NULL;
>
> in mmc_pwrseq_simple_free(), which could be done in mmc_pwrseq_free() it seems.

Thanks for reviewing!

I like you suggestion, though $subject patch is already part of the PR for 3.20.

Feel free to send a patch, I would happily apply it.

Kind regards
Uffe

Re: [PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

[Alexandre Courbot]

On Wed, Feb 11, 2015 at 1:28 PM, Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> [...]
>
>>> +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;
>>
>> How about making this function return a struct mmc_pwrseq * so this
>> last line can be moved to mmc_pwrseq_alloc() instead of requiring all
>> power sequences to do it?
>>
>> The same applies to
>>
>>     host->pwrseq = NULL;
>>
>> in mmc_pwrseq_simple_free(), which could be done in mmc_pwrseq_free() it seems.
>
> Thanks for reviewing!
>
> I like you suggestion, though $subject patch is already part of the PR for 3.20.
>
> Feel free to send a patch, I would happily apply it.

Damn, why am I always so late to review patches...

I will send a fixup patch - better to have this done early.

Thanks!
--
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

[PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider

[Alexandre Courbot]

On Wed, Feb 11, 2015 at 1:28 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> [...]
>
>>> +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;
>>
>> How about making this function return a struct mmc_pwrseq * so this
>> last line can be moved to mmc_pwrseq_alloc() instead of requiring all
>> power sequences to do it?
>>
>> The same applies to
>>
>>     host->pwrseq = NULL;
>>
>> in mmc_pwrseq_simple_free(), which could be done in mmc_pwrseq_free() it seems.
>
> Thanks for reviewing!
>
> I like you suggestion, though $subject patch is already part of the PR for 3.20.
>
> Feel free to send a patch, I would happily apply it.

Damn, why am I always so late to review patches...

I will send a fixup patch - better to have this done early.

Thanks!

Re: [PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Ulf Hansson]

On 10 February 2015 at 10:12, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Mon, Jan 19, 2015 at 6:13 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> The need for reset GPIOs has several times been pointed out from
>> erlier posted patchsets. Especially some WLAN chips which are
>> attached to an SDIO interface may use a GPIO reset.
>>
>> The reset GPIO is asserted at initialization and prior we start the
>> power up procedure. The GPIO will be de-asserted right after the power
>> has been provided to the card, from the ->post_power_on() callback.
>>
>> Note, the reset GPIO is optional. Thus we don't return an error even if
>> we can't find a GPIO for the consumer.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Excellent, with this I can probe the wifi chip on NVIDIA SHIELD which
> requires a GPIO to be high for reset to be de-asserted.
>
> The series:
>
> Tested-by: Alexandre Courbot <acourbot@nvidia.com>

Thanks for testing! Unfortunate, I can't add you tag since this
patchset is already in the PR for 3.20.

>
>> ---
>>
>> Changes in v4:
>>         - Fixed call to kfree().
>>
>> ---
>>  drivers/mmc/core/pwrseq_simple.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>
>> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
>> index 61c991e..0958c69 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_pre_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, 1);
>> +}
>> +
>> +static void mmc_pwrseq_simple_post_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 = {
>> +       .pre_power_on = mmc_pwrseq_simple_pre_power_on,
>> +       .post_power_on = mmc_pwrseq_simple_post_power_on,
>> +       .power_off = mmc_pwrseq_simple_pre_power_on,
>>         .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);
>
> gpiod_get() will translate to exactly the same, with less characters.
>
> Actually I see that the version in -next has support for multiple
> GPIOs. You will probably want to look at Rojhalat's latest work on
> GPIO arrays:
>
> http://permalink.gmane.org/gmane.linux.kernel.gpio/6126

Cool!

>
> This code would be a great candidate to use this GPIO array API, but
> since it is not in -next yet (should happen soon though) you might
> want to consider doing it later.
>
> Btw, I wasted a considerable amount of time on one of the defunct
> previous attempts at power sequences, so I'm interested in reviewing
> future versions of this patchset if you don't mind adding me to the CC
> list. :)

:-)

I will keep you posted for future updates. Feel free to post patches
improving the mmc-pwrseq code yourself, I will happily review them.

Kind regards
Uffe

[PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Ulf Hansson]

On 10 February 2015 at 10:12, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Mon, Jan 19, 2015 at 6:13 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> The need for reset GPIOs has several times been pointed out from
>> erlier posted patchsets. Especially some WLAN chips which are
>> attached to an SDIO interface may use a GPIO reset.
>>
>> The reset GPIO is asserted at initialization and prior we start the
>> power up procedure. The GPIO will be de-asserted right after the power
>> has been provided to the card, from the ->post_power_on() callback.
>>
>> Note, the reset GPIO is optional. Thus we don't return an error even if
>> we can't find a GPIO for the consumer.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Excellent, with this I can probe the wifi chip on NVIDIA SHIELD which
> requires a GPIO to be high for reset to be de-asserted.
>
> The series:
>
> Tested-by: Alexandre Courbot <acourbot@nvidia.com>

Thanks for testing! Unfortunate, I can't add you tag since this
patchset is already in the PR for 3.20.

>
>> ---
>>
>> Changes in v4:
>>         - Fixed call to kfree().
>>
>> ---
>>  drivers/mmc/core/pwrseq_simple.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>
>> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
>> index 61c991e..0958c69 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_pre_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, 1);
>> +}
>> +
>> +static void mmc_pwrseq_simple_post_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 = {
>> +       .pre_power_on = mmc_pwrseq_simple_pre_power_on,
>> +       .post_power_on = mmc_pwrseq_simple_post_power_on,
>> +       .power_off = mmc_pwrseq_simple_pre_power_on,
>>         .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);
>
> gpiod_get() will translate to exactly the same, with less characters.
>
> Actually I see that the version in -next has support for multiple
> GPIOs. You will probably want to look at Rojhalat's latest work on
> GPIO arrays:
>
> http://permalink.gmane.org/gmane.linux.kernel.gpio/6126

Cool!

>
> This code would be a great candidate to use this GPIO array API, but
> since it is not in -next yet (should happen soon though) you might
> want to consider doing it later.
>
> Btw, I wasted a considerable amount of time on one of the defunct
> previous attempts at power sequences, so I'm interested in reviewing
> future versions of this patchset if you don't mind adding me to the CC
> list. :)

:-)

I will keep you posted for future updates. Feel free to post patches
improving the mmc-pwrseq code yourself, I will happily review them.

Kind regards
Uffe

Re: [PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Linus Walleij]

On Wed, Feb 11, 2015 at 5:34 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 10 February 2015 at 10:12, Alexandre Courbot <gnurou@gmail.com> wrote:

>> This code would be a great candidate to use this GPIO array API, but
>> since it is not in -next yet (should happen soon though) you might
>> want to consider doing it later.

This is now in my git and -next, I can take patches to pwrseq if Ulf
ACKs them to go through my GPIO tree.

Yours,
Linus Walleij

[PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin

[Linus Walleij]

On Wed, Feb 11, 2015 at 5:34 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 10 February 2015 at 10:12, Alexandre Courbot <gnurou@gmail.com> wrote:

>> This code would be a great candidate to use this GPIO array API, but
>> since it is not in -next yet (should happen soon though) you might
>> want to consider doing it later.

This is now in my git and -next, I can take patches to pwrseq if Ulf
ACKs them to go through my GPIO tree.

Yours,
Linus Walleij