[PATCH v2 0/4] mmc: pwrseq: post_ios power sequence

[PATCH v2 0/4] mmc: pwrseq: post_ios power sequence

[Romain Perier]

Some devices, like WiFi chipsets AP6335 require a specific power-up
sequence ordering before being used. You must enable the vqmmc power supply
and wait until it reaches its minimum voltage, gate the clock and wait
at least two cycles and then assert the reset line.

See DS 1/

Currently, there is no generic manner for doing this with pwrseq_simple.
This set of patches proposes an approach to support this use case.

It is related to the old patch 2/

1. http://www.t-firefly.com/download/firefly-rk3288/hardware/AP6335%20datasheet_V1.3_02102014.pdf
2. http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/490681.html

Changes in v2:
- Added missing power_off function in operations for post_ios
- Fixed warning found by 0day-ci about missing
  mmc_pwrseq_post_ios_power_on when CONFIG_OF is disabled.

Romain Perier (4):
  mmc: core: Add post_ios_power_on callback for power sequences
  mmc: pwrseq-simple: Add optional op. for post_ios_power_on callback
  mmc: pwrseq_simple: Add an optional pre-power-on-delay
  arm: dts: rockchip: Enable post_ios_power_on and pre-power-on-delay-ms

 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt  |  2 ++
 arch/arm/boot/dts/rk3288-rock2-square.dts          |  2 ++
 drivers/mmc/core/core.c                            |  1 +
 drivers/mmc/core/pwrseq.c                          |  8 ++++++++
 drivers/mmc/core/pwrseq.h                          |  3 +++
 drivers/mmc/core/pwrseq_simple.c                   | 22 +++++++++++++++++++++-
 6 files changed, 37 insertions(+), 1 deletion(-)

-- 
2.9.3

[PATCH v2 1/4] mmc: core: Add post_ios_power_on callback for power sequences

[Romain Perier]

Currently, ->pre_power_on() callback is called at the beginning of the
mmc_power_up() function before MMC_POWER_UP and MMC_POWER_ON sequences.
The callback ->post_power_on() is called at the end of the
mmc_power_up() function. Some SDIO Chipsets require to gate the clock
after than the vqmmc supply is powered on and then toggle the reset
line. Currently, there is no way for doing this.

This commit introduces a new callback ->post_ios_power_on(), that is
called at the end of the mmc_power_up() function after the mmc_set_ios()
operation. In this way the entire power sequences can be done from this
function after the enablement of the power supply.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---

Changes in v2:
 - Added missing declaration for mmc_pwrseq_post_ios_power_on when
   CONFIG_OF is disabled.

 drivers/mmc/core/core.c   | 1 +
 drivers/mmc/core/pwrseq.c | 8 ++++++++
 drivers/mmc/core/pwrseq.h | 3 +++
 3 files changed, 12 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 1076b9d..d73a050 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1831,6 +1831,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	 * time required to reach a stable voltage.
 	 */
 	mmc_delay(10);
+	mmc_pwrseq_post_ios_power_on(host);
 }
 
 void mmc_power_off(struct mmc_host *host)
diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
index 9386c47..98f50b7 100644
--- a/drivers/mmc/core/pwrseq.c
+++ b/drivers/mmc/core/pwrseq.c
@@ -68,6 +68,14 @@ void mmc_pwrseq_post_power_on(struct mmc_host *host)
 		pwrseq->ops->post_power_on(host);
 }
 
+void mmc_pwrseq_post_ios_power_on(struct mmc_host *host)
+{
+	struct mmc_pwrseq *pwrseq = host->pwrseq;
+
+	if (pwrseq && pwrseq->ops->post_ios_power_on)
+		pwrseq->ops->post_ios_power_on(host);
+}
+
 void mmc_pwrseq_power_off(struct mmc_host *host)
 {
 	struct mmc_pwrseq *pwrseq = host->pwrseq;
diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
index d69e751..ad6e3af 100644
--- a/drivers/mmc/core/pwrseq.h
+++ b/drivers/mmc/core/pwrseq.h
@@ -13,6 +13,7 @@
 struct mmc_pwrseq_ops {
 	void (*pre_power_on)(struct mmc_host *host);
 	void (*post_power_on)(struct mmc_host *host);
+	void (*post_ios_power_on)(struct mmc_host *host);
 	void (*power_off)(struct mmc_host *host);
 };
 
@@ -31,6 +32,7 @@ void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq);
 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_post_ios_power_on(struct mmc_host *host);
 void mmc_pwrseq_power_off(struct mmc_host *host);
 void mmc_pwrseq_free(struct mmc_host *host);
 
@@ -44,6 +46,7 @@ static inline void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq) {}
 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_post_ios_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) {}
 
-- 
2.9.3

[PATCH v2 2/4] mmc: pwrseq-simple: Add optional op. for post_ios_power_on callback

[Romain Perier]

Some devices require to do their entire power sequence after that the
power supply of the MMC has been powered on. This can be done by
only implementing the optional post_ios_power_on() callback that rely on
pre_power_on/post_power_on functions, other functions being NULL. Then
we introduce a new DT property "post_ios_power_on", when this property
is set the driver will use its post_ios_power_on operations, otherwise
it fallbacks to the default operations with pre_power_on/post_power_on.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---

Changes in v2:
 - Added missing power_off function in mmc_pwrseq_post_ios_ops

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

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 1304160..e27019f 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -84,12 +84,23 @@ static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
 	}
 }
 
+static void mmc_pwrseq_simple_post_ios_power_on(struct mmc_host *host)
+{
+	mmc_pwrseq_simple_pre_power_on(host);
+	mmc_pwrseq_simple_post_power_on(host);
+}
+
 static const 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_power_off,
 };
 
+static const struct mmc_pwrseq_ops mmc_pwrseq_post_ios_ops = {
+	.post_ios_power_on = mmc_pwrseq_simple_post_ios_power_on,
+	.power_off = mmc_pwrseq_simple_power_off,
+};
+
 static const struct of_device_id mmc_pwrseq_simple_of_match[] = {
 	{ .compatible = "mmc-pwrseq-simple",},
 	{/* sentinel */},
@@ -121,7 +132,10 @@ static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
 				 &pwrseq->post_power_on_delay_ms);
 
 	pwrseq->pwrseq.dev = dev;
-	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
+	if (device_property_read_bool(dev, "post-ios-power-on"))
+		pwrseq->pwrseq.ops = &mmc_pwrseq_post_ios_ops;
+	else
+		pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
 	pwrseq->pwrseq.owner = THIS_MODULE;
 	platform_set_drvdata(pdev, pwrseq);
 
-- 
2.9.3

Re: [PATCH v2 2/4] mmc: pwrseq-simple: Add optional op. for post_ios_power_on callback

[Ulf Hansson]

On 6 March 2017 at 15:17, Romain Perier <romain.perier@collabora.com> wrote:
> Some devices require to do their entire power sequence after that the
> power supply of the MMC has been powered on. This can be done by
> only implementing the optional post_ios_power_on() callback that rely on
> pre_power_on/post_power_on functions, other functions being NULL. Then
> we introduce a new DT property "post_ios_power_on", when this property
> is set the driver will use its post_ios_power_on operations, otherwise
> it fallbacks to the default operations with pre_power_on/post_power_on.
>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
>
> Changes in v2:
>  - Added missing power_off function in mmc_pwrseq_post_ios_ops
>
>  drivers/mmc/core/pwrseq_simple.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
> index 1304160..e27019f 100644
> --- a/drivers/mmc/core/pwrseq_simple.c
> +++ b/drivers/mmc/core/pwrseq_simple.c
> @@ -84,12 +84,23 @@ static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
>         }
>  }
>
> +static void mmc_pwrseq_simple_post_ios_power_on(struct mmc_host *host)
> +{
> +       mmc_pwrseq_simple_pre_power_on(host);
> +       mmc_pwrseq_simple_post_power_on(host);
> +}
> +
>  static const 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_power_off,
>  };
>
> +static const struct mmc_pwrseq_ops mmc_pwrseq_post_ios_ops = {
> +       .post_ios_power_on = mmc_pwrseq_simple_post_ios_power_on,
> +       .power_off = mmc_pwrseq_simple_power_off,
> +};
> +
>  static const struct of_device_id mmc_pwrseq_simple_of_match[] = {
>         { .compatible = "mmc-pwrseq-simple",},
>         {/* sentinel */},
> @@ -121,7 +132,10 @@ static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
>                                  &pwrseq->post_power_on_delay_ms);
>
>         pwrseq->pwrseq.dev = dev;
> -       pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
> +       if (device_property_read_bool(dev, "post-ios-power-on"))
> +               pwrseq->pwrseq.ops = &mmc_pwrseq_post_ios_ops;

No, this is not going to work "post-ios-power-on" is never going to be
accepted as new mmc pwrseq DT binding.

> +       else
> +               pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
>         pwrseq->pwrseq.owner = THIS_MODULE;
>         platform_set_drvdata(pdev, pwrseq);
>
> --
> 2.9.3
>

Kind regards
Uffe

[PATCH v2 3/4] mmc: pwrseq_simple: Add an optional pre-power-on-delay

[Romain Perier]

Some devices need a while between the enablement of its clk and the time
where the reset line is asserted. When this time happens between the
pre_power_on and the post_power_on callbacks, there is a need to do an
msleep at the end of the pre_power_on callback.

This commit adds an optional DT property for such devices.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 2 ++
 drivers/mmc/core/pwrseq_simple.c                            | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
index e254368..821feaaf 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -18,6 +18,8 @@ Optional properties:
   "ext_clock" (External clock provided to the card).
 - post-power-on-delay-ms : Delay in ms after powering the card and
 	de-asserting the reset-gpios (if any)
+- pre-power-on-delay-ms : Delay in ms before powering the card and
+	asserting the reset-gpios (if any)
 
 Example:
 
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index e27019f..d8d7166 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -27,6 +27,7 @@ struct mmc_pwrseq_simple {
 	struct mmc_pwrseq pwrseq;
 	bool clk_enabled;
 	u32 post_power_on_delay_ms;
+	u32 pre_power_on_delay_ms;
 	struct clk *ext_clk;
 	struct gpio_descs *reset_gpios;
 };
@@ -60,6 +61,9 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
 	}
 
 	mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
+
+	if (pwrseq->pre_power_on_delay_ms)
+		msleep(pwrseq->pre_power_on_delay_ms);
 }
 
 static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
@@ -130,6 +134,8 @@ static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
 
 	device_property_read_u32(dev, "post-power-on-delay-ms",
 				 &pwrseq->post_power_on_delay_ms);
+	device_property_read_u32(dev, "pre-power-on-delay-ms",
+				 &pwrseq->pre_power_on_delay_ms);
 
 	pwrseq->pwrseq.dev = dev;
 	if (device_property_read_bool(dev, "post-ios-power-on"))
-- 
2.9.3

Re: [PATCH v2 3/4] mmc: pwrseq_simple: Add an optional pre-power-on-delay

[Ulf Hansson]

On 6 March 2017 at 15:17, Romain Perier <romain.perier@collabora.com> wrote:
> Some devices need a while between the enablement of its clk and the time
> where the reset line is asserted. When this time happens between the
> pre_power_on and the post_power_on callbacks, there is a need to do an
> msleep at the end of the pre_power_on callback.
>
> This commit adds an optional DT property for such devices.
>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
>  Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 2 ++

Split DT documentation for code changes. And make sure DT maintainers
are in the loop discussing new DT bindings.

>  drivers/mmc/core/pwrseq_simple.c                            | 6 ++++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> index e254368..821feaaf 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> @@ -18,6 +18,8 @@ Optional properties:
>    "ext_clock" (External clock provided to the card).
>  - post-power-on-delay-ms : Delay in ms after powering the card and
>         de-asserting the reset-gpios (if any)
> +- pre-power-on-delay-ms : Delay in ms before powering the card and
> +       asserting the reset-gpios (if any)

We have been thinking of adding this before. It seems reasonable to
me, however in the earlier case we ended up adding a new mmc pwrseq
variant.

Let's see where we ends up here...

>
>  Example:
>
> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
> index e27019f..d8d7166 100644
> --- a/drivers/mmc/core/pwrseq_simple.c
> +++ b/drivers/mmc/core/pwrseq_simple.c
> @@ -27,6 +27,7 @@ struct mmc_pwrseq_simple {
>         struct mmc_pwrseq pwrseq;
>         bool clk_enabled;
>         u32 post_power_on_delay_ms;
> +       u32 pre_power_on_delay_ms;
>         struct clk *ext_clk;
>         struct gpio_descs *reset_gpios;
>  };
> @@ -60,6 +61,9 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
>         }
>
>         mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
> +
> +       if (pwrseq->pre_power_on_delay_ms)
> +               msleep(pwrseq->pre_power_on_delay_ms);
>  }
>
>  static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
> @@ -130,6 +134,8 @@ static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
>
>         device_property_read_u32(dev, "post-power-on-delay-ms",
>                                  &pwrseq->post_power_on_delay_ms);
> +       device_property_read_u32(dev, "pre-power-on-delay-ms",
> +                                &pwrseq->pre_power_on_delay_ms);
>
>         pwrseq->pwrseq.dev = dev;
>         if (device_property_read_bool(dev, "post-ios-power-on"))
> --
> 2.9.3
>

Kind regards
Uffe

[PATCH v2 4/4] arm: dts: rockchip: Enable post_ios_power_on and pre-power-on-delay-ms

[Romain Perier]

This enables the property post_ios_power_on. With this property, the
driver pwrseq_simple will do its entire power sequence at the end of the
mmc_power_up() function. The property pre-power-on-delay-ms adds a delay
of 1ms between the enablement of the clk and the assertion of the reset
line.

It fixes the power-up sequence for the Wifi chipset AP6335.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 arch/arm/boot/dts/rk3288-rock2-square.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts b/arch/arm/boot/dts/rk3288-rock2-square.dts
index 818c4bf..9bf2991 100644
--- a/arch/arm/boot/dts/rk3288-rock2-square.dts
+++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
@@ -106,6 +106,8 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&wifi_enable>;
 		reset-gpios = <&gpio4 28 GPIO_ACTIVE_LOW>;
+		pre-power-on-delay-ms = <1>;
+		post-ios-power-on;
 	};
 
 	vcc_usb_host: vcc-host-regulator {
-- 
2.9.3