Skip to contentSkip to conversation

mmc: core: Add support for MMC power sequences

PATCH Superseded Ulf Hansson 4 participants 12 messages Jan 14, 2015 – Jan 16, 2015

Subsystems (2)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover mmc: core: Add support for MMC power sequences

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  |  23 +++++
 Documentation/devicetree/bindings/mmc/mmc.txt      |   5 +
 drivers/mmc/core/Makefile                          |   2 +-
 drivers/mmc/core/core.c                            |   7 ++
 drivers/mmc/core/host.c                            |   4 +-
 drivers/mmc/core/pwrseq.c                          | 109 +++++++++++++++++++++
 drivers/mmc/core/pwrseq.h                          |  42 ++++++++
 drivers/mmc/core/pwrseq_simple.c                   |  86 ++++++++++++++++
 include/linux/mmc/host.h                           |   2 +
 9 files changed, 278 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
 create mode 100644 drivers/mmc/core/pwrseq.c
 create mode 100644 drivers/mmc/core/pwrseq.h
 create mode 100644 drivers/mmc/core/pwrseq_simple.c
Signature · 1 line
1.9.1
#1 lore raw

PATCH v2 1/4 mmc: core: Initial support for 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 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.

---

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.
- Updated commit message according to the above changes.

---
 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
drivers/mmc/core/Makefile +1 -1
index 38ed210..ccdd35f 100644
@@ -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
drivers/mmc/core/core.c +7 -0
index d5c176e..1be7055 100644
@@ -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;
 
drivers/mmc/core/host.c +3 -1
index 0763644..8be0df7 100644
@@ -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);
 }
 
drivers/mmc/core/pwrseq.c new file +50 -0

50 lines not on this page · raw message

drivers/mmc/core/pwrseq.h new file +40 -0
new file mode 100644
index 0000000..12aaf2b
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd
+ *
+ * Author: Ulf Hansson <[email protected]>
+ *
+ * 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
include/linux/mmc/host.h +2 -0
index b6bf718..0c8cbe5 100644
@@ -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;
Signature · 1 line
1.9.1
#2 lore raw

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

In reply to Ulf Hansson

The simple MMC power sequence provider, intends to supports a set of common properties between SOC designs. It thus enables us to re-use the same provider for several SOCs.

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

Following patches will step by step add support for new properties to the simple MMC power sequence provider.

---

Changes in v2:
- None.

---
 .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt      | 18 ++++++++++++++++++
 Documentation/devicetree/bindings/mmc/mmc.txt          |  5 +++++
 2 files changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +18 -0
new file mode 100644
index 0000000..e1b7f9c
@@ -0,0 +1,18 @@
+* The simple MMC power sequence provider
+
+System on chip designs may specify a specific MMC power sequence. To
+successfully detect an (e)MMC/SD/SDIO card, that power sequence must be
+maintained while initializing the card.
+
+The simple MMC power sequence provider, intends to supports a set of common
+properties between SOC designs. It thus enables us to re-use the same provider
+for several SOC designs.
+
+Required properties:
+- compatible : contains "mmc,pwrseq-simple".
+
+Example:
+
+	sdhci0_pwrseq {
+		compatible = "mmc,pwrseq-simple";
+	}
Documentation/devicetree/bindings/mmc/mmc.txt +5 -0
index bac1311..b12de1e 100644
@@ -64,6 +64,10 @@ Optional SDIO properties:
 - keep-power-in-suspend: Preserves card power during a suspend/resume cycle
 - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion
 
+Optional MMC power sequence:
+- mmc-pwrseq: phandle to the MMC power sequence node. See "mmc,pwrseq-*"
+	for documentation of MMC power sequence bindings.
+
 
 Use of Function subnodes
 ------------------------
@@ -101,6 +105,7 @@ sdhci at ab000000 {
 	max-frequency = <50000000>;
 	keep-power-in-suspend;
 	enable-sdio-wakeup;
+	mmc-pwrseq = <&sdhci0_pwrseq>
 }
 
 Example with sdio function subnode:
Signature · 1 line
1.9.1
#3 lore raw

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

Hi Ulf,

Quoted 31 lines · Ulf Hansson

The simple MMC power sequence provider, intends to supports a set of common properties between SOC designs. It thus enables us to re-use the same provider for several SOCs.

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

Following patches will step by step add support for new properties to the simple MMC power sequence provider.

---

Changes in v2:
- None.

---
 .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt      | 18 ++++++++++++++++++
 Documentation/devicetree/bindings/mmc/mmc.txt          |  5 +++++
 2 files changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +2 -0
new file mode 100644
index 0000000..e1b7f9c
@@ -0,0 +1,18 @@
+* The simple MMC power sequence provider
+

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt line 12

Quoted 10 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +10 -0
+System on chip designs may specify a specific MMC power sequence. To
+successfully detect an (e)MMC/SD/SDIO card, that power sequence must be
+maintained while initializing the card.
+
+The simple MMC power sequence provider, intends to supports a set of common
+properties between SOC designs. It thus enables us to re-use the same provider
+for several SOC designs.
+
+Required properties:
+- compatible : contains "mmc,pwrseq-simple".

Nit: "mmc" is not a vendor prefix.

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt line 18

Quoted 6 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +6 -0
+
+Example:
+
+	sdhci0_pwrseq {
+		compatible = "mmc,pwrseq-simple";
+	}

I'm a little confused here. What specific sequence is described by this node? We don't appear to have referred to any resources used as part of that sequence, and the description above only mentions that there could be a specific sequence, not what that sequence is.

So I don't think this makes sense on its own, and should probably be folded with patches adding the initial support for the resources used as part of the sequence (e.g. the GPIOs added in a later patch).

Thanks,
Mark.

Quoted 31 lines
Documentation/devicetree/bindings/mmc/mmc.txt +5 -0
index bac1311..b12de1e 100644
@@ -64,6 +64,10 @@ Optional SDIO properties:
 - keep-power-in-suspend: Preserves card power during a suspend/resume cycle
 - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion
 
+Optional MMC power sequence:
+- mmc-pwrseq: phandle to the MMC power sequence node. See "mmc,pwrseq-*"
+	for documentation of MMC power sequence bindings.
+
 
 Use of Function subnodes
 ------------------------
@@ -101,6 +105,7 @@ sdhci at ab000000 {
 	max-frequency = <50000000>;
 	keep-power-in-suspend;
 	enable-sdio-wakeup;
+	mmc-pwrseq = <&sdhci0_pwrseq>
 }
 
 Example with sdio function subnode:
Signature · 6 lines
1.9.1

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

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

Quoted 59 lines · On Thu, 15 Jan 2015 16:58:26 +0000 Mark Rutland <[email protected]> wrote:

Hi Ulf,

Quoted 31 lines · Ulf Hansson

The simple MMC power sequence provider, intends to supports a set of common properties between SOC designs. It thus enables us to re-use the same provider for several SOCs.

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

Following patches will step by step add support for new properties to the simple MMC power sequence provider.

---

Changes in v2:
- None.

---
 .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt      | 18 ++++++++++++++++++
 Documentation/devicetree/bindings/mmc/mmc.txt          |  5 +++++
 2 files changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +2 -0
new file mode 100644
index 0000000..e1b7f9c
@@ -0,0 +1,18 @@
+* The simple MMC power sequence provider
+

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt line 12

Quoted 10 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +10 -0
+System on chip designs may specify a specific MMC power sequence. To
+successfully detect an (e)MMC/SD/SDIO card, that power sequence must be
+maintained while initializing the card.
+
+The simple MMC power sequence provider, intends to supports a set of common
+properties between SOC designs. It thus enables us to re-use the same provider
+for several SOC designs.
+
+Required properties:
+- compatible : contains "mmc,pwrseq-simple".

Nit: "mmc" is not a vendor prefix.

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt line 18

Quoted 6 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +6 -0
+
+Example:
+
+	sdhci0_pwrseq {
+		compatible = "mmc,pwrseq-simple";
+	}

I'm a little confused here. What specific sequence is described by this node? We don't appear to have referred to any resources used as part of that sequence, and the description above only mentions that there could be a specific sequence, not what that sequence is.

Quoted 4 lines

So I don't think this makes sense on its own, and should probably be folded with patches adding the initial support for the resources used as part of the sequence (e.g. the GPIOs added in a later patch).

I guess I assumed that this "simple" referred to the current behaviour, which includes some of vmmc-supply, vqmmc-supply, vmmc_aux-supply and pbias-supply being switched at "appropriate" times.

Would it make sense to bring all of these explicitly under the "pwrseq" umbrella, leaving the current behaviour only when no pwrseq node is provided?

Also, it isn't clear to me whether the need for power/reset is a function of the mmc-host, or a function of the device attached to the host. I suspect some are needed by one, some by the other. Any by both?

Should the two needs be kept separate?

NeilBrown

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150116/f9d7d593/attachment.sig>

#5 lore raw

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

Quoted 66 lines · NeilBrown
Quoted 59 lines · On Thu, 15 Jan 2015 16:58:26 +0000 Mark Rutland <[email protected]> wrote:

Hi Ulf,

Quoted 31 lines · Ulf Hansson

The simple MMC power sequence provider, intends to supports a set of common properties between SOC designs. It thus enables us to re-use the same provider for several SOCs.

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

Following patches will step by step add support for new properties to the simple MMC power sequence provider.

---

Changes in v2:
- None.

---
 .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt      | 18 ++++++++++++++++++
 Documentation/devicetree/bindings/mmc/mmc.txt          |  5 +++++
 2 files changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +2 -0
new file mode 100644
index 0000000..e1b7f9c
@@ -0,0 +1,18 @@
+* The simple MMC power sequence provider
+

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt line 12

Quoted 10 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +10 -0
+System on chip designs may specify a specific MMC power sequence. To
+successfully detect an (e)MMC/SD/SDIO card, that power sequence must be
+maintained while initializing the card.
+
+The simple MMC power sequence provider, intends to supports a set of common
+properties between SOC designs. It thus enables us to re-use the same provider
+for several SOC designs.
+
+Required properties:
+- compatible : contains "mmc,pwrseq-simple".

Nit: "mmc" is not a vendor prefix.

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt line 18

Quoted 6 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +6 -0
+
+Example:
+
+   sdhci0_pwrseq {
+           compatible = "mmc,pwrseq-simple";
+   }

I'm a little confused here. What specific sequence is described by this node? We don't appear to have referred to any resources used as part of that sequence, and the description above only mentions that there could be a specific sequence, not what that sequence is.

Quoted 4 lines

So I don't think this makes sense on its own, and should probably be folded with patches adding the initial support for the resources used as part of the sequence (e.g. the GPIOs added in a later patch).

Quoted 6 lines

I guess I assumed that this "simple" referred to the current behaviour, which includes some of vmmc-supply, vqmmc-supply, vmmc_aux-supply and pbias-supply being switched at "appropriate" times.

Would it make sense to bring all of these explicitly under the "pwrseq" umbrella, leaving the current behaviour only when no pwrseq node is provided?

In principle there is nothing that prevents us from following your suggestion. But why should we?

Moreover, I don't think it's required at this point, let's first get the basic mmc-pwrseq support in place. Then we can evolve it. :-)

Quoted 4 lines

Also, it isn't clear to me whether the need for power/reset is a function of the mmc-host, or a function of the device attached to the host. I suspect some are needed by one, some by the other. Any by both?

To me the the power is coupled with the host and thus controlled by the mmc layer. But I guess we can discuss this in a forever long loop. :-)

They may be other resources which is coupled with an SDIO function / eMMC,SDIO card, such as irqs. Those shall be described in the child node of the host.
I have queued the below patchset from Hans, which I believe should address this:

https://www.mail-archive.com/linux-mmc at vger.kernel.org/msg27241.html

Quoted 2 lines

Should the two needs be kept separate?

Yes, I think so.

Kind regards
Uffe

#6 lore raw

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

In reply to NeilBrown
Quoted 3 lines · NeilBrown

Also, it isn't clear to me whether the need for power/reset is a function of the mmc-host, or a function of the device attached to the host. I suspect some are needed by one, some by the other. Any by both?

Neil,

There's a horrid issue there. The standard model of MMC/SD is that the device will be powered up by the host, and will then be probed to find out what the device is. This normally works fine as the host is responsible for controlling the power to the socket which the card is plugged into.

Where things fall down is when you have a MMC/SD device embedded onto a board, and they decide that it'll be given separate controls for power and reset which are not under the control of the host.

If the MMC/SD device is not powered up before we probe the bus, then the device is not discoverable, and this breaks the standard model:
- If we run the standard MMC/SD device discovery code with the device powered down, it will find no device attached, so no device will be created.

- If we create a device, then the device driver will be probed immediately. While the device driver can then bind, discover the power and reset controls, and activate them, it can't then talk to its device because the MMC layer hasn't gone through the required standard device discovery and probe sequence. If we could do that, we would then need some way to stop that mechanism creating another device.

If we instead take the view that the host is responsible for powering up the device, then we are merely keeping with the standard MMC/SD model, and we don't have to hack around his.

Keeping to the standard model of a host responsible for power control of its child devices is just a whole lot nicer.

Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
#7 lore raw

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

In reply to Mark Rutland
Quoted 59 lines · Mark Rutland

Hi Ulf,

Quoted 31 lines · Ulf Hansson

The simple MMC power sequence provider, intends to supports a set of common properties between SOC designs. It thus enables us to re-use the same provider for several SOCs.

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

Following patches will step by step add support for new properties to the simple MMC power sequence provider.

---

Changes in v2:
- None.

---
 .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt      | 18 ++++++++++++++++++
 Documentation/devicetree/bindings/mmc/mmc.txt          |  5 +++++
 2 files changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +2 -0
new file mode 100644
index 0000000..e1b7f9c
@@ -0,0 +1,18 @@
+* The simple MMC power sequence provider
+

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt line 12

Quoted 10 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +10 -0
+System on chip designs may specify a specific MMC power sequence. To
+successfully detect an (e)MMC/SD/SDIO card, that power sequence must be
+maintained while initializing the card.
+
+The simple MMC power sequence provider, intends to supports a set of common
+properties between SOC designs. It thus enables us to re-use the same provider
+for several SOC designs.
+
+Required properties:
+- compatible : contains "mmc,pwrseq-simple".

Nit: "mmc" is not a vendor prefix.

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt line 18

Quoted 6 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt new file +6 -0
+
+Example:
+
+     sdhci0_pwrseq {
+             compatible = "mmc,pwrseq-simple";
+     }

I'm a little confused here. What specific sequence is described by this node? We don't appear to have referred to any resources used as part of that sequence, and the description above only mentions that there could be a specific sequence, not what that sequence is.

Quoted 3 lines

So I don't think this makes sense on its own, and should probably be folded with patches adding the initial support for the resources used as part of the sequence (e.g. the GPIOs added in a later patch).

You are absolutely right. I fix it.

Again, thanks for reviewing.

Kind regards
Uffe

#8 lore raw

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

In reply to 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.

---

Changes in v2:
- None.

---
 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
drivers/mmc/core/Makefile +1 -1
index ccdd35f..b39cbd2 100644
@@ -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
drivers/mmc/core/pwrseq.c +60 -1

74 lines not on this page · raw message

drivers/mmc/core/pwrseq.h +2 -0
index 12aaf2b..bd860d8 100644
@@ -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; }
drivers/mmc/core/pwrseq_simple.c new file +48 -0

48 lines not on this page · raw message

Signature · 1 line
1.9.1
#9 lore raw

PATCH v2 4/4 mmc: pwrseq_simple: Add support for a reset GPIO pin

In reply to Ulf Hansson

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

In this first version, one reset pin is supported. We may want to extend the support to cover more pins, but let's leave that as a future change. The added DT binding for the reset GPIO can easily be extended to manage several pins.

The reset GPIO is asserted at initialization and prior we start the power up procedure. It will then be de-asserted right after the power has been provided to the external chip/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 pin for the consumer.

---

Changes in v2:
- Adopted to the changed names of the pwrseq callbacks.

---
 .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt  |  5 +++
 drivers/mmc/core/pwrseq_simple.c                   | 38 ++++++++++++++++++++++
 2 files changed, 43 insertions(+)
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt +5 -0
index e1b7f9c..6fe0cd6 100644
@@ -11,8 +11,13 @@ for several SOC designs.
 Required properties:
 - compatible : contains "mmc,pwrseq-simple".
 
+Optional properties:
+- reset-gpios : contains a list of GPIO specifiers, though currently only one
+		specifier is supported.
+
 Example:
 
 	sdhci0_pwrseq {
 		compatible = "mmc,pwrseq-simple";
+		reset-gpios = <&gpio1 12 0>;
 	}
drivers/mmc/core/pwrseq_simple.c +38 -0
index 7f87bc1..42d9836 100644
@@ -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;
 }
Signature · 1 line
1.9.1
#10 lore raw

PATCH v2 4/4 mmc: pwrseq_simple: Add support for a reset GPIO pin

Hi,

Quoted 9 lines · Ulf Hansson

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

In this first version, one reset pin is supported. We may want to extend the support to cover more pins, but let's leave that as a future change. The added DT binding for the reset GPIO can easily be extended to manage several pins.

Quoted 4 lines

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

This needs to be specified a little more explicitly in the binding document below, as it forms part of the contract of the binding.

Quoted 20 lines

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

---

Changes in v2:
- Adopted to the changed names of the pwrseq callbacks.

---
 .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt  |  5 +++
 drivers/mmc/core/pwrseq_simple.c                   | 38 ++++++++++++++++++++++
 2 files changed, 43 insertions(+)
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt +0 -0
index e1b7f9c..6fe0cd6 100644

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt for several SOC designs. · line 16

Quoted 6 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt +3 -0
@@ -11,8 +11,13 @@ for several SOC designs.
 Required properties:
 - compatible : contains "mmc,pwrseq-simple".
 
+Optional properties:
+- reset-gpios : contains a list of GPIO specifiers, though currently only one
+		specifier is supported.

The support is a Linux issue. If the binding is meant to describe a list, mention that, and what the expected semantics are.

Is it that difficult to have the driver iterate over the list now?

Thanks,
Mark.

Quoted 96 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt +2 -0
+
 Example:
 
 	sdhci0_pwrseq {
 		compatible = "mmc,pwrseq-simple";
+		reset-gpios = <&gpio1 12 0>;
 	}
drivers/mmc/core/pwrseq_simple.c +38 -0
index 7f87bc1..42d9836 100644
@@ -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;
 }
Signature · 6 lines
1.9.1

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

PATCH v2 4/4 mmc: pwrseq_simple: Add support for a reset GPIO pin

Quoted 43 lines · Mark Rutland

Hi,

Quoted 9 lines · Ulf Hansson

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

In this first version, one reset pin is supported. We may want to extend the support to cover more pins, but let's leave that as a future change. The added DT binding for the reset GPIO can easily be extended to manage several pins.

Quoted 4 lines

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

This needs to be specified a little more explicitly in the binding document below, as it forms part of the contract of the binding.

Quoted 23 lines

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

---

Changes in v2:
- Adopted to the changed names of the pwrseq callbacks.

---
 .../devicetree/bindings/mmc/mmc,pwrseq-simple.txt  |  5 +++
 drivers/mmc/core/pwrseq_simple.c                   | 38 ++++++++++++++++++++++
 2 files changed, 43 insertions(+)
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt +0 -0
index e1b7f9c..6fe0cd6 100644
@@ -11,8 +11,13 @@ for several SOC designs.
 Required properties:
 - compatible : contains "mmc,pwrseq-simple".
 
Quoted 8 lines

Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt for several SOC designs. · line 16

Quoted 3 lines
Documentation/devicetree/bindings/mmc/mmc,pwrseq-simple.txt +3 -0
+Optional properties:
+- reset-gpios : contains a list of GPIO specifiers, though currently only one
+             specifier is supported.

The support is a Linux issue. If the binding is meant to describe a list, mention that, and what the expected semantics are.

Is it that difficult to have the driver iterate over the list now?

I was trying to keep code simple, maybe too simple. :-)

I will update the DT document according to your proposal and fix the code.

Thanks for reviewing!

Kind regards
Uffe