All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V3 0/4] mmc: core: Add support for MMC power sequences
@ 2015-01-16 10:47 ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-mmc, Chris Ball
  Cc: linux-arm-kernel, devicetree, Linus Walleij, Mark Brown,
	Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
	Olof Johansson, Russell King, Hans de Goede, Doug Anderson,
	NeilBrown, Tomeu Vizoso, Mark Rutland, Ulf Hansson

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH V3 0/4] mmc: core: Add support for MMC power sequences
@ 2015-01-16 10:47 ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH V3 1/4] mmc: core: Initial support for MMC power sequences
  2015-01-16 10:47 ` Ulf Hansson
@ 2015-01-16 10:47   ` Ulf Hansson
  -1 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-mmc, Chris Ball
  Cc: linux-arm-kernel, devicetree, Linus Walleij, Mark Brown,
	Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
	Olof Johansson, Russell King, Hans de Goede, Doug Anderson,
	NeilBrown, Tomeu Vizoso, Mark Rutland, 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 v3:
	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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V3 1/4] mmc: core: Initial support for MMC power sequences
@ 2015-01-16 10:47   ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-arm-kernel

System on chip designs may specify a specific MMC power sequence. To
successfully detect an (e)MMC/SD/SDIO card, that power sequence must
be followed while initializing the card.

To be able to handle these SOC specific power sequences, let's add a
MMC power sequence interface. It provides the following functions to
help the mmc core to deal with these power sequences.

mmc_pwrseq_alloc() - Invoked from mmc_of_parse(), to initialize data.
mmc_pwrseq_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 v3:
	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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V3 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence
  2015-01-16 10:47 ` Ulf Hansson
@ 2015-01-16 10:47   ` Ulf Hansson
  -1 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-mmc, Chris Ball
  Cc: linux-arm-kernel, devicetree, Linus Walleij, Mark Brown,
	Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
	Olof Johansson, Russell King, Hans de Goede, Doug Anderson,
	NeilBrown, Tomeu Vizoso, Mark Rutland, 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>
---

Changes in v3:
	- Folded in all documentation into this patch, to get the big picture.
	- Make it clear that we support one reset GPIO.

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V3 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence
@ 2015-01-16 10:47   ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-arm-kernel

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

Changes in v3:
	- Folded in all documentation into this patch, to get the big picture.
	- Make it clear that we support one reset GPIO.

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V3 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider
  2015-01-16 10:47 ` Ulf Hansson
@ 2015-01-16 10:47     ` Ulf Hansson
  -1 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-mmc-u79uwXL29TY76Z2rM5mHXA, Chris Ball
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linus Walleij, Mark Brown,
	Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
	Olof Johansson, Russell King, Hans de Goede, Doug Anderson,
	NeilBrown, Tomeu Vizoso, Mark Rutland, 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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---

Changes in v3:
	- Updated to use the new compatible string for the simple MMC power
	  sequence.

---
 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..7f87bc1
--- /dev/null
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -0,0 +1,48 @@
+/*
+ *  Copyright (C) 2014 Linaro Ltd
+ *
+ * Author: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V3 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider
@ 2015-01-16 10:47     ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-arm-kernel

To add the core part for the MMC power sequence, let's start by adding
initial support for the simple MMC power sequence provider.

In this initial step, the MMC power sequence node are fetched and the
compatible string for the simple MMC power sequence provider are
verified.

At this point we don't parse the node for any properties, but instead
that will be handled from following patches. Since there are no
properties supported yet, let's just implement the ->alloc() and the
->free() callbacks.

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

Changes in v3:
	- Updated to use the new compatible string for the simple MMC power
	  sequence.

---
 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..7f87bc1
--- /dev/null
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -0,0 +1,48 @@
+/*
+ *  Copyright (C) 2014 Linaro Ltd
+ *
+ * Author: Ulf Hansson <ulf.hansson@linaro.org>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ *
+ *  Simple MMC power sequence management
+ */
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/err.h>
+
+#include <linux/mmc/host.h>
+
+#include "pwrseq.h"
+
+struct mmc_pwrseq_simple {
+	struct mmc_pwrseq pwrseq;
+};
+
+static void mmc_pwrseq_simple_free(struct mmc_host *host)
+{
+	struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
+					struct mmc_pwrseq_simple, pwrseq);
+
+	kfree(&pwrseq);
+	host->pwrseq = NULL;
+}
+
+static struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
+	.free = mmc_pwrseq_simple_free,
+};
+
+int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
+{
+	struct mmc_pwrseq_simple *pwrseq;
+
+	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), GFP_KERNEL);
+	if (!pwrseq)
+		return -ENOMEM;
+
+	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
+	host->pwrseq = &pwrseq->pwrseq;
+
+	return 0;
+}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
  2015-01-16 10:47 ` Ulf Hansson
@ 2015-01-16 10:47   ` Ulf Hansson
  -1 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-mmc, Chris Ball
  Cc: linux-arm-kernel, devicetree, Linus Walleij, Mark Brown,
	Arnd Bergmann, Alexandre Courbot, Arend van Spriel, Sascha Hauer,
	Olof Johansson, Russell King, Hans de Goede, Doug Anderson,
	NeilBrown, Tomeu Vizoso, Mark Rutland, 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 v3:
	- Updated commit message to make it clear that we support one reset
	  GPIO.

---
 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 7f87bc1..42d9836 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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
@ 2015-01-16 10:47   ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 10:47 UTC (permalink / raw)
  To: linux-arm-kernel

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 v3:
	- Updated commit message to make it clear that we support one reset
	  GPIO.

---
 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 7f87bc1..42d9836 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
  2015-01-16 10:47   ` Ulf Hansson
@ 2015-01-16 11:34       ` Tomeu Vizoso
  -1 siblings, 0 replies; 18+ messages in thread
From: Tomeu Vizoso @ 2015-01-16 11:34 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: linux-mmc-u79uwXL29TY76Z2rM5mHXA, Chris Ball, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Hans de Goede, Russell King,
	Arnd Bergmann, Alexandre Courbot, NeilBrown, Linus Walleij,
	Doug Anderson, Olof Johansson, Mark Brown, Arend van Spriel,
	Sascha Hauer, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 16 January 2015 at 11:47, 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);
> +       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);

Hi Ulf,

this kfree looks a bit fishy (there's one more instance of it in
mmc_pwrseq_simple_free).

Cheers,

Tomeu
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
@ 2015-01-16 11:34       ` Tomeu Vizoso
  0 siblings, 0 replies; 18+ messages in thread
From: Tomeu Vizoso @ 2015-01-16 11:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 16 January 2015 at 11:47, 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);
> +       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);

Hi Ulf,

this kfree looks a bit fishy (there's one more instance of it in
mmc_pwrseq_simple_free).

Cheers,

Tomeu

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
  2015-01-16 11:34       ` Tomeu Vizoso
@ 2015-01-16 12:37         ` Ulf Hansson
  -1 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 12:37 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: linux-mmc, Chris Ball, Mark Rutland, devicetree, Hans de Goede,
	Russell King, Arnd Bergmann, Alexandre Courbot, NeilBrown,
	Linus Walleij, Doug Anderson, Olof Johansson, Mark Brown,
	Arend van Spriel, Sascha Hauer, linux-arm-kernel

On 16 January 2015 at 12:34, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> On 16 January 2015 at 11:47, 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);
>> +       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);
>
> Hi Ulf,
>
> this kfree looks a bit fishy (there's one more instance of it in
> mmc_pwrseq_simple_free).

This is the error path, which means I need to clean up data that I
have allocated.

mmc_pwrseq_simple_free() is called when host drivers ->remove()
callback invokes mmc_free_host(). In that case the host->pwrseq has
been assigned.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
@ 2015-01-16 12:37         ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 12:37 UTC (permalink / raw)
  To: linux-arm-kernel

On 16 January 2015 at 12:34, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> On 16 January 2015 at 11:47, 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);
>> +       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);
>
> Hi Ulf,
>
> this kfree looks a bit fishy (there's one more instance of it in
> mmc_pwrseq_simple_free).

This is the error path, which means I need to clean up data that I
have allocated.

mmc_pwrseq_simple_free() is called when host drivers ->remove()
callback invokes mmc_free_host(). In that case the host->pwrseq has
been assigned.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
  2015-01-16 12:37         ` Ulf Hansson
@ 2015-01-16 12:45           ` Sascha Hauer
  -1 siblings, 0 replies; 18+ messages in thread
From: Sascha Hauer @ 2015-01-16 12:45 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Tomeu Vizoso, linux-mmc, Chris Ball, Mark Rutland, devicetree,
	Hans de Goede, Russell King, Arnd Bergmann, Alexandre Courbot,
	NeilBrown, Linus Walleij, Doug Anderson, Olof Johansson,
	Mark Brown, Arend van Spriel, linux-arm-kernel

On Fri, Jan 16, 2015 at 01:37:41PM +0100, Ulf Hansson wrote:
> On 16 January 2015 at 12:34, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> > On 16 January 2015 at 11:47, 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);
> >> +       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);
> >
> > Hi Ulf,
> >
> > this kfree looks a bit fishy (there's one more instance of it in
> > mmc_pwrseq_simple_free).
> 
> This is the error path, which means I need to clean up data that I
> have allocated.

I think Tomeu meant that the '&' must be removed.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
@ 2015-01-16 12:45           ` Sascha Hauer
  0 siblings, 0 replies; 18+ messages in thread
From: Sascha Hauer @ 2015-01-16 12:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 16, 2015 at 01:37:41PM +0100, Ulf Hansson wrote:
> On 16 January 2015 at 12:34, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> > On 16 January 2015 at 11:47, 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);
> >> +       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);
> >
> > Hi Ulf,
> >
> > this kfree looks a bit fishy (there's one more instance of it in
> > mmc_pwrseq_simple_free).
> 
> This is the error path, which means I need to clean up data that I
> have allocated.

I think Tomeu meant that the '&' must be removed.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
  2015-01-16 12:45           ` Sascha Hauer
@ 2015-01-16 12:52             ` Ulf Hansson
  -1 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 12:52 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Tomeu Vizoso, linux-mmc, Chris Ball, Mark Rutland, devicetree,
	Hans de Goede, Russell King, Arnd Bergmann, Alexandre Courbot,
	NeilBrown, Linus Walleij, Doug Anderson, Olof Johansson,
	Mark Brown, Arend van Spriel, linux-arm-kernel

On 16 January 2015 at 13:45, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, Jan 16, 2015 at 01:37:41PM +0100, Ulf Hansson wrote:
>> On 16 January 2015 at 12:34, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> > On 16 January 2015 at 11:47, 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);
>> >> +       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);
>> >
>> > Hi Ulf,
>> >
>> > this kfree looks a bit fishy (there's one more instance of it in
>> > mmc_pwrseq_simple_free).
>>
>> This is the error path, which means I need to clean up data that I
>> have allocated.
>
> I think Tomeu meant that the '&' must be removed.
>

Of course, thanks!

Unless I get some more comments I will fix it when I queue the patch.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
@ 2015-01-16 12:52             ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2015-01-16 12:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 16 January 2015 at 13:45, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, Jan 16, 2015 at 01:37:41PM +0100, Ulf Hansson wrote:
>> On 16 January 2015 at 12:34, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> > On 16 January 2015 at 11:47, 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);
>> >> +       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);
>> >
>> > Hi Ulf,
>> >
>> > this kfree looks a bit fishy (there's one more instance of it in
>> > mmc_pwrseq_simple_free).
>>
>> This is the error path, which means I need to clean up data that I
>> have allocated.
>
> I think Tomeu meant that the '&' must be removed.
>

Of course, thanks!

Unless I get some more comments I will fix it when I queue the patch.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-01-16 12:52 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-16 10:47 [PATCH V3 0/4] mmc: core: Add support for MMC power sequences Ulf Hansson
2015-01-16 10:47 ` Ulf Hansson
2015-01-16 10:47 ` [PATCH V3 1/4] mmc: core: Initial " Ulf Hansson
2015-01-16 10:47   ` Ulf Hansson
2015-01-16 10:47 ` [PATCH V3 2/4] mmc: pwrseq: Document DT bindings for the simple MMC power sequence Ulf Hansson
2015-01-16 10:47   ` Ulf Hansson
     [not found] ` <1421405273-19117-1-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-01-16 10:47   ` [PATCH V3 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider Ulf Hansson
2015-01-16 10:47     ` Ulf Hansson
2015-01-16 10:47 ` [PATCH V3 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin Ulf Hansson
2015-01-16 10:47   ` Ulf Hansson
     [not found]   ` <1421405273-19117-5-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-01-16 11:34     ` Tomeu Vizoso
2015-01-16 11:34       ` Tomeu Vizoso
2015-01-16 12:37       ` Ulf Hansson
2015-01-16 12:37         ` Ulf Hansson
2015-01-16 12:45         ` Sascha Hauer
2015-01-16 12:45           ` Sascha Hauer
2015-01-16 12:52           ` Ulf Hansson
2015-01-16 12:52             ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.