[PATCH v2 0/3] Add support for hardware reset of eMMC card on reboot

[PATCH v2 0/3] Add support for hardware reset of eMMC card on reboot

[Marek Szyprowski]

Hello,

This patchset adds new mmc power sequence driver, which performs eMMC
card hardware reset. Such power sequence, involving resetting emmc card,
is required on some boards to properly perform system reboot procedure.

This patchset originates from fixes for the reboot hang issue on
Hardkernel's Odroid boards with eMMC card. Those boards are designed in
such a way, that the eMMC nreset signal is routed to SoC GPIO line
instead of the board reset logic. To properly restard system and let ROM
bootloader to read next bootloader from eMMC, one need to reset emmc
card before performing reboot.

The initial patches consisted of a complete reset/power off driver, but
after further analysis, it turned out that only eMMC nreset line control
logic is specific for those boards. Everything else can be done by the
MMC code. Sjoerd Simons and Jaehoon Chung suggested that such feature is
something generic, not really specific to the MMC host controller used
on Odroid boards, so it should be implemented in MMC core. Then, Ulf
Hansson suggested to use recently introduced power sequences driver for
implementing this feature.

This patches provides a simple mmc-pwrseq-emmc driver, which controls
single gpio line and performs eMMC reset procedure just after card power
on and just before turning it off or performing system reboot. This
driver has been instantiated on Odroid X2/U3 and XU3 boards.

Patches are prepared on top of linux-next.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Changelog:

v2:
- moved all code to separate mmc-prwseq-emmc driver

v1:
https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg41641.html
- moved all the code and logic to mmc core (slot-gpio.c), so it is
  available to all mmc drivers, which uses device tree

v0:
http://thread.gmane.org/gmane.linux.kernel.samsung-soc/42477/
- separated emmc reset logic from other code and moved to exynos dw-mmc
  driver

initial version:
http://thread.gmane.org/gmane.linux.power-management.general/51855/
- separate reset driver implementing all the logic

Patch summary:

Marek Szyprowski (3):
  mmc: pwrseq: add driver for emmc hardware reset
  ARM: dts: exynos4412-odroid*: add eMMC reset line
  ARM: dts: exynos5422-odroidxu3: add eMMC reset line

 .../devicetree/bindings/mmc/mmc-pwrseq-emmc.txt    |  25 +++++
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |  14 +++
 arch/arm/boot/dts/exynos5422-odroidxu3.dts         |  17 ++++
 drivers/mmc/core/Makefile                          |   2 +-
 drivers/mmc/core/pwrseq.c                          |   3 +
 drivers/mmc/core/pwrseq.h                          |   1 +
 drivers/mmc/core/pwrseq_emmc.c                     | 108 +++++++++++++++++++++
 7 files changed, 169 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
 create mode 100644 drivers/mmc/core/pwrseq_emmc.c

-- 
1.9.2

[PATCH v2 1/3] mmc: pwrseq: add driver for emmc hardware reset

[Marek Szyprowski]

This patch provides a simple mmc-pwrseq-emmc driver, which controls
single gpio line. It perform standard eMMC hw reset procedure, as
descibed by Jedec 4.4 specification. This procedure is performed just
after MMC core enabled power to the given mmc host (to fix possible
issues if bootloader has left eMMC card in initialized or unknown
state), and before performing complete system reboot (also in case of
emergency reboot call). The latter is needed on boards, which doesn't
have hardware reset logic connected to emmc card and (limited or broken)
ROM bootloaders are unable to read second stage from the emmc card if
the card is left in unknown or already initialized state.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../devicetree/bindings/mmc/mmc-pwrseq-emmc.txt    |  25 +++++
 drivers/mmc/core/Makefile                          |   2 +-
 drivers/mmc/core/pwrseq.c                          |   3 +
 drivers/mmc/core/pwrseq.h                          |   1 +
 drivers/mmc/core/pwrseq_emmc.c                     | 108 +++++++++++++++++++++
 5 files changed, 138 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
 create mode 100644 drivers/mmc/core/pwrseq_emmc.c

diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
new file mode 100644
index 000000000000..4baeac828f6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
@@ -0,0 +1,25 @@
+* The simple eMMC hardware reset provider
+
+The purpose of this driver is to perform standard eMMC hw reset
+procedure, as descibed by Jedec 4.4 specification. This procedure is
+performed just after MMC core enabled power to the given mmc host (to
+fix possible issues if bootloader has left eMMC card in initialized or
+unknown state), and before performing complete system reboot (also in
+case of emergency reboot call). The latter is needed on boards, which
+doesn't have hardware reset logic connected to emmc card and (limited or
+broken) ROM bootloaders are unable to read second stage from the emmc
+card if the card is left in unknown or already initialized state.
+
+Required properties:
+- compatible : contains "mmc-pwrseq-emmc".
+- reset-gpios : contains a GPIO specifier. The reset GPIO is pulled
+	down and then pulled up to perform eMMC card reset. To perform
+	reset procedure as described in Jedec 4.4 specification, the
+	gpio line should be defined as GPIO_ACTIVE_LOW.
+
+Example:
+
+	sdhci0_pwrseq {
+		compatible = "mmc-pwrseq-emmc";
+		reset-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
+	}
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index b39cbd2f830b..2c25138f28b7 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 pwrseq_simple.o
+mmc_core-$(CONFIG_OF)		+= pwrseq.o pwrseq_simple.o pwrseq_emmc.o
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
index 2cea00ed4e65..862356123d78 100644
--- a/drivers/mmc/core/pwrseq.c
+++ b/drivers/mmc/core/pwrseq.c
@@ -26,6 +26,9 @@ static struct mmc_pwrseq_match pwrseq_match[] = {
 	{
 		.compatible = "mmc-pwrseq-simple",
 		.alloc = mmc_pwrseq_simple_alloc,
+	}, {
+		.compatible = "mmc-pwrseq-emmc",
+		.alloc = mmc_pwrseq_emmc_alloc,
 	},
 };
 
diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
index bd860d88f116..aba3409e8d6e 100644
--- a/drivers/mmc/core/pwrseq.h
+++ b/drivers/mmc/core/pwrseq.h
@@ -28,6 +28,7 @@ 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);
+int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev);
 
 #else
 
diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
new file mode 100644
index 000000000000..bedf969244fa
--- /dev/null
+++ b/drivers/mmc/core/pwrseq_emmc.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2015, Samsung Electronics Co., Ltd.
+ *
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ *
+ * Simple eMMC hardware reset trigger
+ */
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/reboot.h>
+
+#include <linux/mmc/host.h>
+
+#include "pwrseq.h"
+
+struct mmc_pwrseq_emmc {
+	struct mmc_pwrseq pwrseq;
+	struct notifier_block reset_nb;
+	struct gpio_desc *reset_gpio;
+};
+
+static void mmc_pwrseq_perform_emmc_reset(struct mmc_pwrseq_emmc *pwrseq)
+{
+	gpiod_set_value(pwrseq->reset_gpio, 1);
+	/* For eMMC, minimum is 1us but give it 10us for good measure */
+	udelay(10);
+	gpiod_set_value(pwrseq->reset_gpio, 0);
+	/* For eMMC, minimum is 200us but give it 300us for good measure */
+	udelay(300);
+}
+
+static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
+{
+	struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
+					struct mmc_pwrseq_emmc, pwrseq);
+
+	if (!IS_ERR(pwrseq->reset_gpio))
+		mmc_pwrseq_perform_emmc_reset(pwrseq);
+}
+
+static void mmc_pwrseq_emmc_free(struct mmc_host *host)
+{
+	struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
+					struct mmc_pwrseq_emmc, pwrseq);
+
+	if (!IS_ERR(pwrseq->reset_gpio))
+		gpiod_put(pwrseq->reset_gpio);
+
+	unregister_restart_handler(&pwrseq->reset_nb);
+	kfree(pwrseq);
+	host->pwrseq = NULL;
+}
+
+static struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
+	.post_power_on = mmc_pwrseq_emmc_reset,
+	.power_off = mmc_pwrseq_emmc_reset,
+	.free = mmc_pwrseq_emmc_free,
+};
+
+static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
+				    unsigned long mode, void *cmd)
+{
+	struct mmc_pwrseq_emmc *pwrseq;
+	pwrseq = container_of(this, struct mmc_pwrseq_emmc, reset_nb);
+	if (!IS_ERR(pwrseq->reset_gpio))
+		mmc_pwrseq_perform_emmc_reset(pwrseq);
+	return NOTIFY_DONE;
+}
+
+int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev)
+{
+	struct mmc_pwrseq_emmc *pwrseq;
+	int ret = 0;
+
+	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_emmc), GFP_KERNEL);
+	if (!pwrseq)
+		return -ENOMEM;
+
+	pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_LOW);
+	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;
+	}
+
+	/*
+	 * register reset handler to ensure emmc reset also from
+	 * emergency_reboot()
+	 */
+	pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
+	pwrseq->reset_nb.priority = 255;
+	register_restart_handler(&pwrseq->reset_nb);
+
+	pwrseq->pwrseq.ops = &mmc_pwrseq_emmc_ops;
+	host->pwrseq = &pwrseq->pwrseq;
+
+	return 0;
+free:
+	kfree(pwrseq);
+	return ret;
+}
-- 
1.9.2

[PATCH v2 2/3] ARM: dts: exynos4412-odroid*: add eMMC reset line

[Marek Szyprowski]

This patch adds reset-gpios property to the eMMC slot, so the MMC driver
is able to properly reset eMMC card on system restart and thus fixes
system hang on software reboot.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index ffcf17bbf63b..c97ffbf0a6be 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -57,10 +57,18 @@
 				<19200000>;
 	};
 
+	emmc_pwrseq: pwrseq {
+		pinctrl-0 = <&sd1_cd>;
+		pinctrl-names = "default";
+		compatible = "mmc-pwrseq-emmc";
+		reset-gpios = <&gpk1 2 1>;
+	};
+
 	mmc@12550000 {
 		pinctrl-0 = <&sd4_clk &sd4_cmd &sd4_bus4 &sd4_bus8>;
 		pinctrl-names = "default";
 		vmmc-supply = <&ldo20_reg &buck8_reg>;
+		mmc-pwrseq = <&emmc_pwrseq>;
 		status = "okay";
 
 		num-slots = <1>;
@@ -392,6 +400,12 @@
 	};
 };
 
+/* RSTN signal for eMMC */
+&sd1_cd {
+	samsung,pin-pud = <0>;
+	samsung,pin-drv = <0>;
+};
+
 &pinctrl_1 {
 	gpio_power_key: power_key {
 		samsung,pins = "gpx1-3";
-- 
1.9.2

[PATCH v2 3/3] ARM: dts: exynos5422-odroidxu3: add eMMC reset line

[Marek Szyprowski]

This patch adds reset-gpios property to the eMMC slot, so the MMC driver
is able to properly reset eMMC card on system restart and thus fixes
system hang on software reboot.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos5422-odroidxu3.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3.dts b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
index a519c863248d..790fb4ce295d 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3.dts
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
@@ -264,6 +264,13 @@
 		};
 	};
 
+	emmc_pwrseq: pwrseq {
+		pinctrl-0 = <&emmc_nrst_pin>;
+		pinctrl-names = "default";
+		compatible = "mmc-pwrseq-emmc";
+		reset-gpios = <&gpd1 0 1>;
+	};
+
 	i2c_2: i2c@12C80000 {
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <66000>;
@@ -298,6 +305,7 @@
 
 &mmc_0 {
 	status = "okay";
+	mmc-pwrseq = <&emmc_pwrseq>;
 	broken-cd;
 	card-detect-delay = <200>;
 	samsung,dw-mshc-ciu-div = <3>;
@@ -330,6 +338,15 @@
 	};
 };
 
+&pinctrl_1 {
+	emmc_nrst_pin: emmc-nrst {
+		samsung,pins = "gpd1-0";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <0>;
+		samsung,pin-drv = <0>;
+	};
+};
+
 &usbdrd_dwc3_0 {
 	dr_mode = "host";
 };
-- 
1.9.2

Re: [PATCH v2 0/3] Add support for hardware reset of eMMC card on reboot

[Javier Martinez Canillas]

Hello Marek,

On Mon, Feb 2, 2015 at 9:33 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hello,
>
> This patchset adds new mmc power sequence driver, which performs eMMC
> card hardware reset. Such power sequence, involving resetting emmc card,
> is required on some boards to properly perform system reboot procedure.
>

Patches looks good to me, I've only one comment on patch 1/3. Once
that is addressed, feel free to add for all the series:

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

Best regards,
Javier

Re: [PATCH v2 1/3] mmc: pwrseq: add driver for emmc hardware reset

[Javier Martinez Canillas]

Hello Marek,

On Mon, Feb 2, 2015 at 9:33 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> +
> +Required properties:
> +- compatible : contains "mmc-pwrseq-emmc".
> +- reset-gpios : contains a GPIO specifier. The reset GPIO is pulled

The DT binding says that the "reset-gpios" is a required property...

> +int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev)
> +{
> +       struct mmc_pwrseq_emmc *pwrseq;
> +       int ret = 0;
> +
> +       pwrseq = kzalloc(sizeof(struct mmc_pwrseq_emmc), GFP_KERNEL);
> +       if (!pwrseq)
> +               return -ENOMEM;
> +
> +       pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_LOW);
> +       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;
> +       }

...but here the GPIO is made optional since this will return and
propagate the error only for -EPROBE_DEFER error but won't do it for
-ENOENT and -ENOSYS.

Best regards,
Javier

Re: [PATCH v2 1/3] mmc: pwrseq: add driver for emmc hardware reset

[Marek Szyprowski]

Hello,

On 2015-02-02 09:59, Javier Martinez Canillas wrote:
> On Mon, Feb 2, 2015 at 9:33 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> +
>> +Required properties:
>> +- compatible : contains "mmc-pwrseq-emmc".
>> +- reset-gpios : contains a GPIO specifier. The reset GPIO is pulled
> The DT binding says that the "reset-gpios" is a required property...
>
>> +int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev)
>> +{
>> +       struct mmc_pwrseq_emmc *pwrseq;
>> +       int ret = 0;
>> +
>> +       pwrseq = kzalloc(sizeof(struct mmc_pwrseq_emmc), GFP_KERNEL);
>> +       if (!pwrseq)
>> +               return -ENOMEM;
>> +
>> +       pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_LOW);
>> +       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;
>> +       }
> ...but here the GPIO is made optional since this will return and
> propagate the error only for -EPROBE_DEFER error but won't do it for
> -ENOENT and -ENOSYS.

Thanks for spotting this. reset-gpios is required, this driver really 
makes no
sense without a gpio pin. I copied code from mmc-pwrseq-simple, which 
handled
optional gpio pins. I will fix this is a few minutes.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH v3 1/3] mmc: pwrseq: add driver for emmc hardware reset

[Marek Szyprowski]

This patch provides a simple mmc-pwrseq-emmc driver, which controls
single gpio line. It perform standard eMMC hw reset procedure, as
descibed by Jedec 4.4 specification. This procedure is performed just
after MMC core enabled power to the given mmc host (to fix possible
issues if bootloader has left eMMC card in initialized or unknown
state), and before performing complete system reboot (also in case of
emergency reboot call). The latter is needed on boards, which doesn't
have hardware reset logic connected to emmc card and (limited or broken)
ROM bootloaders are unable to read second stage from the emmc card if
the card is left in unknown or already initialized state.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../devicetree/bindings/mmc/mmc-pwrseq-emmc.txt    |  25 +++++
 drivers/mmc/core/Makefile                          |   2 +-
 drivers/mmc/core/pwrseq.c                          |   3 +
 drivers/mmc/core/pwrseq.h                          |   1 +
 drivers/mmc/core/pwrseq_emmc.c                     | 103 +++++++++++++++++++++
 5 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
 create mode 100644 drivers/mmc/core/pwrseq_emmc.c

diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
new file mode 100644
index 000000000000..4baeac828f6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
@@ -0,0 +1,25 @@
+* The simple eMMC hardware reset provider
+
+The purpose of this driver is to perform standard eMMC hw reset
+procedure, as descibed by Jedec 4.4 specification. This procedure is
+performed just after MMC core enabled power to the given mmc host (to
+fix possible issues if bootloader has left eMMC card in initialized or
+unknown state), and before performing complete system reboot (also in
+case of emergency reboot call). The latter is needed on boards, which
+doesn't have hardware reset logic connected to emmc card and (limited or
+broken) ROM bootloaders are unable to read second stage from the emmc
+card if the card is left in unknown or already initialized state.
+
+Required properties:
+- compatible : contains "mmc-pwrseq-emmc".
+- reset-gpios : contains a GPIO specifier. The reset GPIO is pulled
+	down and then pulled up to perform eMMC card reset. To perform
+	reset procedure as described in Jedec 4.4 specification, the
+	gpio line should be defined as GPIO_ACTIVE_LOW.
+
+Example:
+
+	sdhci0_pwrseq {
+		compatible = "mmc-pwrseq-emmc";
+		reset-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
+	}
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index b39cbd2f830b..2c25138f28b7 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 pwrseq_simple.o
+mmc_core-$(CONFIG_OF)		+= pwrseq.o pwrseq_simple.o pwrseq_emmc.o
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
index 2cea00ed4e65..862356123d78 100644
--- a/drivers/mmc/core/pwrseq.c
+++ b/drivers/mmc/core/pwrseq.c
@@ -26,6 +26,9 @@ static struct mmc_pwrseq_match pwrseq_match[] = {
 	{
 		.compatible = "mmc-pwrseq-simple",
 		.alloc = mmc_pwrseq_simple_alloc,
+	}, {
+		.compatible = "mmc-pwrseq-emmc",
+		.alloc = mmc_pwrseq_emmc_alloc,
 	},
 };
 
diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
index bd860d88f116..aba3409e8d6e 100644
--- a/drivers/mmc/core/pwrseq.h
+++ b/drivers/mmc/core/pwrseq.h
@@ -28,6 +28,7 @@ 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);
+int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev);
 
 #else
 
diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
new file mode 100644
index 000000000000..39dc43dd316f
--- /dev/null
+++ b/drivers/mmc/core/pwrseq_emmc.c
@@ -0,0 +1,103 @@
+/*
+ * Copyright (C) 2015, Samsung Electronics Co., Ltd.
+ *
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ *
+ * Simple eMMC hardware reset provider
+ */
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/reboot.h>
+
+#include <linux/mmc/host.h>
+
+#include "pwrseq.h"
+
+struct mmc_pwrseq_emmc {
+	struct mmc_pwrseq pwrseq;
+	struct notifier_block reset_nb;
+	struct gpio_desc *reset_gpio;
+};
+
+static void mmc_pwrseq_perform_emmc_reset(struct mmc_pwrseq_emmc *pwrseq)
+{
+	gpiod_set_value(pwrseq->reset_gpio, 1);
+	/* For eMMC, minimum is 1us but give it 10us for good measure */
+	udelay(10);
+	gpiod_set_value(pwrseq->reset_gpio, 0);
+	/* For eMMC, minimum is 200us but give it 300us for good measure */
+	udelay(300);
+}
+
+static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
+{
+	struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
+					struct mmc_pwrseq_emmc, pwrseq);
+
+	mmc_pwrseq_perform_emmc_reset(pwrseq);
+}
+
+static void mmc_pwrseq_emmc_free(struct mmc_host *host)
+{
+	struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
+					struct mmc_pwrseq_emmc, pwrseq);
+
+	gpiod_put(pwrseq->reset_gpio);
+	unregister_restart_handler(&pwrseq->reset_nb);
+	kfree(pwrseq);
+	host->pwrseq = NULL;
+}
+
+static struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
+	.post_power_on = mmc_pwrseq_emmc_reset,
+	.power_off = mmc_pwrseq_emmc_reset,
+	.free = mmc_pwrseq_emmc_free,
+};
+
+static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
+				    unsigned long mode, void *cmd)
+{
+	struct mmc_pwrseq_emmc *pwrseq;
+	pwrseq = container_of(this, struct mmc_pwrseq_emmc, reset_nb);
+
+	mmc_pwrseq_perform_emmc_reset(pwrseq);
+	return NOTIFY_DONE;
+}
+
+int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev)
+{
+	struct mmc_pwrseq_emmc *pwrseq;
+	int ret = 0;
+
+	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_emmc), GFP_KERNEL);
+	if (!pwrseq)
+		return -ENOMEM;
+
+	pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_LOW);
+	if (IS_ERR(pwrseq->reset_gpio)) {
+		ret = PTR_ERR(pwrseq->reset_gpio);
+		goto free;
+	}
+
+	/*
+	 * register reset handler to ensure emmc reset also from
+	 * emergency_reboot()
+	 */
+	pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
+	pwrseq->reset_nb.priority = 255;
+	register_restart_handler(&pwrseq->reset_nb);
+
+	pwrseq->pwrseq.ops = &mmc_pwrseq_emmc_ops;
+	host->pwrseq = &pwrseq->pwrseq;
+
+	return 0;
+free:
+	kfree(pwrseq);
+	return ret;
+}
-- 
1.9.2

Re: [PATCH v3 1/3] mmc: pwrseq: add driver for emmc hardware reset

[Ulf Hansson]

On 2 February 2015 at 11:05, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> This patch provides a simple mmc-pwrseq-emmc driver, which controls
> single gpio line. It perform standard eMMC hw reset procedure, as
> descibed by Jedec 4.4 specification. This procedure is performed just
> after MMC core enabled power to the given mmc host (to fix possible
> issues if bootloader has left eMMC card in initialized or unknown
> state), and before performing complete system reboot (also in case of
> emergency reboot call). The latter is needed on boards, which doesn't
> have hardware reset logic connected to emmc card and (limited or broken)
> ROM bootloaders are unable to read second stage from the emmc card if
> the card is left in unknown or already initialized state.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../devicetree/bindings/mmc/mmc-pwrseq-emmc.txt    |  25 +++++
>  drivers/mmc/core/Makefile                          |   2 +-
>  drivers/mmc/core/pwrseq.c                          |   3 +
>  drivers/mmc/core/pwrseq.h                          |   1 +
>  drivers/mmc/core/pwrseq_emmc.c                     | 103 +++++++++++++++++++++
>  5 files changed, 133 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
>  create mode 100644 drivers/mmc/core/pwrseq_emmc.c
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
> new file mode 100644
> index 000000000000..4baeac828f6b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
> @@ -0,0 +1,25 @@
> +* The simple eMMC hardware reset provider
> +
> +The purpose of this driver is to perform standard eMMC hw reset
> +procedure, as descibed by Jedec 4.4 specification. This procedure is
> +performed just after MMC core enabled power to the given mmc host (to
> +fix possible issues if bootloader has left eMMC card in initialized or
> +unknown state), and before performing complete system reboot (also in
> +case of emergency reboot call). The latter is needed on boards, which
> +doesn't have hardware reset logic connected to emmc card and (limited or
> +broken) ROM bootloaders are unable to read second stage from the emmc
> +card if the card is left in unknown or already initialized state.
> +
> +Required properties:
> +- compatible : contains "mmc-pwrseq-emmc".
> +- reset-gpios : contains a GPIO specifier. The reset GPIO is pulled
> +       down and then pulled up to perform eMMC card reset. To perform

Could we maybe use the word "(de)asserted" instead of "pulled down/up".

> +       reset procedure as described in Jedec 4.4 specification, the
> +       gpio line should be defined as GPIO_ACTIVE_LOW.
> +
> +Example:
> +
> +       sdhci0_pwrseq {
> +               compatible = "mmc-pwrseq-emmc";
> +               reset-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
> +       }
> diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
> index b39cbd2f830b..2c25138f28b7 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 pwrseq_simple.o
> +mmc_core-$(CONFIG_OF)          += pwrseq.o pwrseq_simple.o pwrseq_emmc.o
>  mmc_core-$(CONFIG_DEBUG_FS)    += debugfs.o
> diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
> index 2cea00ed4e65..862356123d78 100644
> --- a/drivers/mmc/core/pwrseq.c
> +++ b/drivers/mmc/core/pwrseq.c
> @@ -26,6 +26,9 @@ static struct mmc_pwrseq_match pwrseq_match[] = {
>         {
>                 .compatible = "mmc-pwrseq-simple",
>                 .alloc = mmc_pwrseq_simple_alloc,
> +       }, {
> +               .compatible = "mmc-pwrseq-emmc",
> +               .alloc = mmc_pwrseq_emmc_alloc,
>         },
>  };
>
> diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
> index bd860d88f116..aba3409e8d6e 100644
> --- a/drivers/mmc/core/pwrseq.h
> +++ b/drivers/mmc/core/pwrseq.h
> @@ -28,6 +28,7 @@ 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);
> +int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev);
>
>  #else
>
> diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
> new file mode 100644
> index 000000000000..39dc43dd316f
> --- /dev/null
> +++ b/drivers/mmc/core/pwrseq_emmc.c
> @@ -0,0 +1,103 @@
> +/*
> + * Copyright (C) 2015, Samsung Electronics Co., Ltd.
> + *
> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + *
> + * Simple eMMC hardware reset provider
> + */
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/reboot.h>
> +
> +#include <linux/mmc/host.h>
> +
> +#include "pwrseq.h"
> +
> +struct mmc_pwrseq_emmc {
> +       struct mmc_pwrseq pwrseq;
> +       struct notifier_block reset_nb;
> +       struct gpio_desc *reset_gpio;
> +};
> +
> +static void mmc_pwrseq_perform_emmc_reset(struct mmc_pwrseq_emmc *pwrseq)

I would prefer the name of this function to be __mmc_pwrseq_emmc_reset()

> +{
> +       gpiod_set_value(pwrseq->reset_gpio, 1);
> +       /* For eMMC, minimum is 1us but give it 10us for good measure */
> +       udelay(10);
> +       gpiod_set_value(pwrseq->reset_gpio, 0);
> +       /* For eMMC, minimum is 200us but give it 300us for good measure */
> +       udelay(300);

These delays will affect the total power up time for the card.
According to the specification 1 us and 200 us, should be enough. Do
you think there are a good reason to have them extended to the values
you suggested?

> +}
> +
> +static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
> +{
> +       struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
> +                                       struct mmc_pwrseq_emmc, pwrseq);
> +
> +       mmc_pwrseq_perform_emmc_reset(pwrseq);
> +}
> +
> +static void mmc_pwrseq_emmc_free(struct mmc_host *host)
> +{
> +       struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
> +                                       struct mmc_pwrseq_emmc, pwrseq);
> +
> +       gpiod_put(pwrseq->reset_gpio);
> +       unregister_restart_handler(&pwrseq->reset_nb);

I suggest you to move the unregister_restart_handler() to be done
prior gpiod_put(), since else (theoretically) the notifier may be
called without a valid gpio requested.

> +       kfree(pwrseq);
> +       host->pwrseq = NULL;
> +}
> +
> +static struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
> +       .post_power_on = mmc_pwrseq_emmc_reset,
> +       .power_off = mmc_pwrseq_emmc_reset,

I don't think you should assign the ->power_off() callback, unless you
think there are a reason to reset the card while cutting powering for
it!?

> +       .free = mmc_pwrseq_emmc_free,
> +};
> +
> +static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
> +                                   unsigned long mode, void *cmd)
> +{
> +       struct mmc_pwrseq_emmc *pwrseq;
> +       pwrseq = container_of(this, struct mmc_pwrseq_emmc, reset_nb);
> +
> +       mmc_pwrseq_perform_emmc_reset(pwrseq);
> +       return NOTIFY_DONE;
> +}
> +
> +int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev)
> +{
> +       struct mmc_pwrseq_emmc *pwrseq;
> +       int ret = 0;
> +
> +       pwrseq = kzalloc(sizeof(struct mmc_pwrseq_emmc), GFP_KERNEL);
> +       if (!pwrseq)
> +               return -ENOMEM;
> +
> +       pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_LOW);
> +       if (IS_ERR(pwrseq->reset_gpio)) {
> +               ret = PTR_ERR(pwrseq->reset_gpio);
> +               goto free;
> +       }
> +
> +       /*
> +        * register reset handler to ensure emmc reset also from
> +        * emergency_reboot()
> +        */
> +       pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
> +       pwrseq->reset_nb.priority = 255;

Why 255? I guess it could deserve a comment!?

> +       register_restart_handler(&pwrseq->reset_nb);
> +
> +       pwrseq->pwrseq.ops = &mmc_pwrseq_emmc_ops;
> +       host->pwrseq = &pwrseq->pwrseq;
> +
> +       return 0;
> +free:
> +       kfree(pwrseq);
> +       return ret;
> +}
> --
> 1.9.2
>

Some final generic thoughts; since the reset GPIO will be toggled for
each call to mmc_power_up(), that means the card will be reset for
each system PM suspend to resume cycle. Is that really what we want?

For some cases that also means the card will be reset when it's in
sleep state (CMD5). Do you know if that's okay?

Kind regards
Uffe

Re: [PATCH v3 1/3] mmc: pwrseq: add driver for emmc hardware reset

[Marek Szyprowski]

Hello,

On 2015-02-02 13:53, Ulf Hansson wrote:
> On 2 February 2015 at 11:05, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> This patch provides a simple mmc-pwrseq-emmc driver, which controls
>> single gpio line. It perform standard eMMC hw reset procedure, as
>> descibed by Jedec 4.4 specification. This procedure is performed just
>> after MMC core enabled power to the given mmc host (to fix possible
>> issues if bootloader has left eMMC card in initialized or unknown
>> state), and before performing complete system reboot (also in case of
>> emergency reboot call). The latter is needed on boards, which doesn't
>> have hardware reset logic connected to emmc card and (limited or broken)
>> ROM bootloaders are unable to read second stage from the emmc card if
>> the card is left in unknown or already initialized state.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   .../devicetree/bindings/mmc/mmc-pwrseq-emmc.txt    |  25 +++++
>>   drivers/mmc/core/Makefile                          |   2 +-
>>   drivers/mmc/core/pwrseq.c                          |   3 +
>>   drivers/mmc/core/pwrseq.h                          |   1 +
>>   drivers/mmc/core/pwrseq_emmc.c                     | 103 +++++++++++++++++++++
>>   5 files changed, 133 insertions(+), 1 deletion(-)
>>   create mode 100644 Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
>>   create mode 100644 drivers/mmc/core/pwrseq_emmc.c
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
>> new file mode 100644
>> index 000000000000..4baeac828f6b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
>> @@ -0,0 +1,25 @@
>> +* The simple eMMC hardware reset provider
>> +
>> +The purpose of this driver is to perform standard eMMC hw reset
>> +procedure, as descibed by Jedec 4.4 specification. This procedure is
>> +performed just after MMC core enabled power to the given mmc host (to
>> +fix possible issues if bootloader has left eMMC card in initialized or
>> +unknown state), and before performing complete system reboot (also in
>> +case of emergency reboot call). The latter is needed on boards, which
>> +doesn't have hardware reset logic connected to emmc card and (limited or
>> +broken) ROM bootloaders are unable to read second stage from the emmc
>> +card if the card is left in unknown or already initialized state.
>> +
>> +Required properties:
>> +- compatible : contains "mmc-pwrseq-emmc".
>> +- reset-gpios : contains a GPIO specifier. The reset GPIO is pulled
>> +       down and then pulled up to perform eMMC card reset. To perform
> Could we maybe use the word "(de)asserted" instead of "pulled down/up".
>
>> +       reset procedure as described in Jedec 4.4 specification, the
>> +       gpio line should be defined as GPIO_ACTIVE_LOW.
>> +
>> +Example:
>> +
>> +       sdhci0_pwrseq {
>> +               compatible = "mmc-pwrseq-emmc";
>> +               reset-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
>> +       }
>> diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
>> index b39cbd2f830b..2c25138f28b7 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 pwrseq_simple.o
>> +mmc_core-$(CONFIG_OF)          += pwrseq.o pwrseq_simple.o pwrseq_emmc.o
>>   mmc_core-$(CONFIG_DEBUG_FS)    += debugfs.o
>> diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
>> index 2cea00ed4e65..862356123d78 100644
>> --- a/drivers/mmc/core/pwrseq.c
>> +++ b/drivers/mmc/core/pwrseq.c
>> @@ -26,6 +26,9 @@ static struct mmc_pwrseq_match pwrseq_match[] = {
>>          {
>>                  .compatible = "mmc-pwrseq-simple",
>>                  .alloc = mmc_pwrseq_simple_alloc,
>> +       }, {
>> +               .compatible = "mmc-pwrseq-emmc",
>> +               .alloc = mmc_pwrseq_emmc_alloc,
>>          },
>>   };
>>
>> diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
>> index bd860d88f116..aba3409e8d6e 100644
>> --- a/drivers/mmc/core/pwrseq.h
>> +++ b/drivers/mmc/core/pwrseq.h
>> @@ -28,6 +28,7 @@ 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);
>> +int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev);
>>
>>   #else
>>
>> diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
>> new file mode 100644
>> index 000000000000..39dc43dd316f
>> --- /dev/null
>> +++ b/drivers/mmc/core/pwrseq_emmc.c
>> @@ -0,0 +1,103 @@
>> +/*
>> + * Copyright (C) 2015, Samsung Electronics Co., Ltd.
>> + *
>> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
>> + *
>> + * License terms: GNU General Public License (GPL) version 2
>> + *
>> + * Simple eMMC hardware reset provider
>> + */
>> +#include <linux/delay.h>
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/reboot.h>
>> +
>> +#include <linux/mmc/host.h>
>> +
>> +#include "pwrseq.h"
>> +
>> +struct mmc_pwrseq_emmc {
>> +       struct mmc_pwrseq pwrseq;
>> +       struct notifier_block reset_nb;
>> +       struct gpio_desc *reset_gpio;
>> +};
>> +
>> +static void mmc_pwrseq_perform_emmc_reset(struct mmc_pwrseq_emmc *pwrseq)
> I would prefer the name of this function to be __mmc_pwrseq_emmc_reset()
>
>> +{
>> +       gpiod_set_value(pwrseq->reset_gpio, 1);
>> +       /* For eMMC, minimum is 1us but give it 10us for good measure */
>> +       udelay(10);
>> +       gpiod_set_value(pwrseq->reset_gpio, 0);
>> +       /* For eMMC, minimum is 200us but give it 300us for good measure */
>> +       udelay(300);
> These delays will affect the total power up time for the card.
> According to the specification 1 us and 200 us, should be enough. Do
> you think there are a good reason to have them extended to the values
> you suggested?

I've just copied those from sdhci-pci driver. 1us/200us works fine for me.

>
>> +}
>> +
>> +static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
>> +{
>> +       struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
>> +                                       struct mmc_pwrseq_emmc, pwrseq);
>> +
>> +       mmc_pwrseq_perform_emmc_reset(pwrseq);
>> +}
>> +
>> +static void mmc_pwrseq_emmc_free(struct mmc_host *host)
>> +{
>> +       struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
>> +                                       struct mmc_pwrseq_emmc, pwrseq);
>> +
>> +       gpiod_put(pwrseq->reset_gpio);
>> +       unregister_restart_handler(&pwrseq->reset_nb);
> I suggest you to move the unregister_restart_handler() to be done
> prior gpiod_put(), since else (theoretically) the notifier may be
> called without a valid gpio requested.
>
>> +       kfree(pwrseq);
>> +       host->pwrseq = NULL;
>> +}
>> +
>> +static struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
>> +       .post_power_on = mmc_pwrseq_emmc_reset,
>> +       .power_off = mmc_pwrseq_emmc_reset,
> I don't think you should assign the ->power_off() callback, unless you
> think there are a reason to reset the card while cutting powering for
> it!?
>
>> +       .free = mmc_pwrseq_emmc_free,
>> +};
>> +
>> +static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
>> +                                   unsigned long mode, void *cmd)
>> +{
>> +       struct mmc_pwrseq_emmc *pwrseq;
>> +       pwrseq = container_of(this, struct mmc_pwrseq_emmc, reset_nb);
>> +
>> +       mmc_pwrseq_perform_emmc_reset(pwrseq);
>> +       return NOTIFY_DONE;
>> +}
>> +
>> +int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev)
>> +{
>> +       struct mmc_pwrseq_emmc *pwrseq;
>> +       int ret = 0;
>> +
>> +       pwrseq = kzalloc(sizeof(struct mmc_pwrseq_emmc), GFP_KERNEL);
>> +       if (!pwrseq)
>> +               return -ENOMEM;
>> +
>> +       pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_LOW);
>> +       if (IS_ERR(pwrseq->reset_gpio)) {
>> +               ret = PTR_ERR(pwrseq->reset_gpio);
>> +               goto free;
>> +       }
>> +
>> +       /*
>> +        * register reset handler to ensure emmc reset also from
>> +        * emergency_reboot()
>> +        */
>> +       pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
>> +       pwrseq->reset_nb.priority = 255;
> Why 255? I guess it could deserve a comment!?

The default architecture reset handler is called with priority 128. The 
goal here
is to be called before it.

>
>> +       register_restart_handler(&pwrseq->reset_nb);
>> +
>> +       pwrseq->pwrseq.ops = &mmc_pwrseq_emmc_ops;
>> +       host->pwrseq = &pwrseq->pwrseq;
>> +
>> +       return 0;
>> +free:
>> +       kfree(pwrseq);
>> +       return ret;
>> +}
>> --
>> 1.9.2
>>
> Some final generic thoughts; since the reset GPIO will be toggled for
> each call to mmc_power_up(), that means the card will be reset for
> each system PM suspend to resume cycle. Is that really what we want?

I wanted to provide a generic driver. I already noticed that some mmc hosts
perform emmc hardware reset on initialization, so I though that this is 
not an
issue if this driver will also do it. This will also solve strange issue
I've observed some time ago, when bootloader's code was leaving the emmc 
card
initialized.

If you think that one might not be interested in such behavior, maybe we 
just
need to add additional boolean properties to this driver, like 
'reset-on-init'
and 'reset-on-reboot'?

> For some cases that also means the card will be reset when it's in
> sleep state (CMD5). Do you know if that's okay?

I've didn't observe any issues yet.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH v4 1/3] mmc: pwrseq: add driver for emmc hardware reset

[Marek Szyprowski]

This patch provides a simple mmc-pwrseq-emmc driver, which controls
single gpio line. It perform standard eMMC hw reset procedure, as
descibed by Jedec 4.4 specification. This procedure is performed just
after MMC core enabled power to the given mmc host (to fix possible
issues if bootloader has left eMMC card in initialized or unknown
state), and before performing complete system reboot (also in case of
emergency reboot call). The latter is needed on boards, which doesn't
have hardware reset logic connected to emmc card and (limited or broken)
ROM bootloaders are unable to read second stage from the emmc card if
the card is left in unknown or already initialized state.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
Changelog:
v4:
- fixed minor issues reported by Ulf Hansson (comment adjustments,
  removed .shutdown callback, reduced delay time)

v3: http://www.spinics.net/lists/linux-samsung-soc/msg41956.html
- removed optional support for reset-gpio, reported by Javier Martinez Canillas

v2: http://www.spinics.net/lists/linux-mmc/msg30621.html
---
 .../devicetree/bindings/mmc/mmc-pwrseq-emmc.txt    |  25 +++++
 drivers/mmc/core/Makefile                          |   2 +-
 drivers/mmc/core/pwrseq.c                          |   3 +
 drivers/mmc/core/pwrseq.h                          |   1 +
 drivers/mmc/core/pwrseq_emmc.c                     | 101 +++++++++++++++++++++
 5 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
 create mode 100644 drivers/mmc/core/pwrseq_emmc.c

diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
new file mode 100644
index 0000000..0cb827b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
@@ -0,0 +1,25 @@
+* The simple eMMC hardware reset provider
+
+The purpose of this driver is to perform standard eMMC hw reset
+procedure, as descibed by Jedec 4.4 specification. This procedure is
+performed just after MMC core enabled power to the given mmc host (to
+fix possible issues if bootloader has left eMMC card in initialized or
+unknown state), and before performing complete system reboot (also in
+case of emergency reboot call). The latter is needed on boards, which
+doesn't have hardware reset logic connected to emmc card and (limited or
+broken) ROM bootloaders are unable to read second stage from the emmc
+card if the card is left in unknown or already initialized state.
+
+Required properties:
+- compatible : contains "mmc-pwrseq-emmc".
+- reset-gpios : contains a GPIO specifier. The reset GPIO is asserted
+	and then deasserted to perform eMMC card reset. To perform
+	reset procedure as described in Jedec 4.4 specification, the
+	gpio line should be defined as GPIO_ACTIVE_LOW.
+
+Example:
+
+	sdhci0_pwrseq {
+		compatible = "mmc-pwrseq-emmc";
+		reset-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
+	}
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index b39cbd2..2c25138 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 pwrseq_simple.o
+mmc_core-$(CONFIG_OF)		+= pwrseq.o pwrseq_simple.o pwrseq_emmc.o
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
index 2cea00e..8623561 100644
--- a/drivers/mmc/core/pwrseq.c
+++ b/drivers/mmc/core/pwrseq.c
@@ -26,6 +26,9 @@ static struct mmc_pwrseq_match pwrseq_match[] = {
 	{
 		.compatible = "mmc-pwrseq-simple",
 		.alloc = mmc_pwrseq_simple_alloc,
+	}, {
+		.compatible = "mmc-pwrseq-emmc",
+		.alloc = mmc_pwrseq_emmc_alloc,
 	},
 };
 
diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
index bd860d8..aba3409 100644
--- a/drivers/mmc/core/pwrseq.h
+++ b/drivers/mmc/core/pwrseq.h
@@ -28,6 +28,7 @@ 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);
+int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev);
 
 #else
 
diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
new file mode 100644
index 0000000..8a434d7
--- /dev/null
+++ b/drivers/mmc/core/pwrseq_emmc.c
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2015, Samsung Electronics Co., Ltd.
+ *
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ *
+ * Simple eMMC hardware reset provider
+ */
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/reboot.h>
+
+#include <linux/mmc/host.h>
+
+#include "pwrseq.h"
+
+struct mmc_pwrseq_emmc {
+	struct mmc_pwrseq pwrseq;
+	struct notifier_block reset_nb;
+	struct gpio_desc *reset_gpio;
+};
+
+static void __mmc_pwrseq_emmc_reset(struct mmc_pwrseq_emmc *pwrseq)
+{
+	gpiod_set_value(pwrseq->reset_gpio, 1);
+	udelay(1);
+	gpiod_set_value(pwrseq->reset_gpio, 0);
+	udelay(200);
+}
+
+static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
+{
+	struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
+					struct mmc_pwrseq_emmc, pwrseq);
+
+	__mmc_pwrseq_emmc_reset(pwrseq);
+}
+
+static void mmc_pwrseq_emmc_free(struct mmc_host *host)
+{
+	struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
+					struct mmc_pwrseq_emmc, pwrseq);
+
+	unregister_restart_handler(&pwrseq->reset_nb);
+	gpiod_put(pwrseq->reset_gpio);
+	kfree(pwrseq);
+	host->pwrseq = NULL;
+}
+
+static struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
+	.post_power_on = mmc_pwrseq_emmc_reset,
+	.free = mmc_pwrseq_emmc_free,
+};
+
+static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
+				    unsigned long mode, void *cmd)
+{
+	struct mmc_pwrseq_emmc *pwrseq;
+	pwrseq = container_of(this, struct mmc_pwrseq_emmc, reset_nb);
+
+	__mmc_pwrseq_emmc_reset(pwrseq);
+	return NOTIFY_DONE;
+}
+
+int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev)
+{
+	struct mmc_pwrseq_emmc *pwrseq;
+	int ret = 0;
+
+	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_emmc), GFP_KERNEL);
+	if (!pwrseq)
+		return -ENOMEM;
+
+	pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_LOW);
+	if (IS_ERR(pwrseq->reset_gpio)) {
+		ret = PTR_ERR(pwrseq->reset_gpio);
+		goto free;
+	}
+
+	/*
+	 * register reset handler to ensure emmc reset also from
+	 * emergency_reboot(), priority 129 schedules it just before
+	 * system reboot
+	 */
+	pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
+	pwrseq->reset_nb.priority = 129;
+	register_restart_handler(&pwrseq->reset_nb);
+
+	pwrseq->pwrseq.ops = &mmc_pwrseq_emmc_ops;
+	host->pwrseq = &pwrseq->pwrseq;
+
+	return 0;
+free:
+	kfree(pwrseq);
+	return ret;
+}
-- 
1.9.2

Re: [PATCH v4 1/3] mmc: pwrseq: add driver for emmc hardware reset

[Ulf Hansson]

On 3 February 2015 at 14:07, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> This patch provides a simple mmc-pwrseq-emmc driver, which controls
> single gpio line. It perform standard eMMC hw reset procedure, as
> descibed by Jedec 4.4 specification. This procedure is performed just
> after MMC core enabled power to the given mmc host (to fix possible
> issues if bootloader has left eMMC card in initialized or unknown
> state), and before performing complete system reboot (also in case of
> emergency reboot call). The latter is needed on boards, which doesn't
> have hardware reset logic connected to emmc card and (limited or broken)
> ROM bootloaders are unable to read second stage from the emmc card if
> the card is left in unknown or already initialized state.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Thanks! Applied for next, with a minor fix for a checkpatch warning. See below.

Kind regards
Uffe

> ---
> Changelog:
> v4:
> - fixed minor issues reported by Ulf Hansson (comment adjustments,
>   removed .shutdown callback, reduced delay time)
>
> v3: http://www.spinics.net/lists/linux-samsung-soc/msg41956.html
> - removed optional support for reset-gpio, reported by Javier Martinez Canillas
>
> v2: http://www.spinics.net/lists/linux-mmc/msg30621.html
> ---
>  .../devicetree/bindings/mmc/mmc-pwrseq-emmc.txt    |  25 +++++
>  drivers/mmc/core/Makefile                          |   2 +-
>  drivers/mmc/core/pwrseq.c                          |   3 +
>  drivers/mmc/core/pwrseq.h                          |   1 +
>  drivers/mmc/core/pwrseq_emmc.c                     | 101 +++++++++++++++++++++
>  5 files changed, 131 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
>  create mode 100644 drivers/mmc/core/pwrseq_emmc.c
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
> new file mode 100644
> index 0000000..0cb827b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
> @@ -0,0 +1,25 @@
> +* The simple eMMC hardware reset provider
> +
> +The purpose of this driver is to perform standard eMMC hw reset
> +procedure, as descibed by Jedec 4.4 specification. This procedure is
> +performed just after MMC core enabled power to the given mmc host (to
> +fix possible issues if bootloader has left eMMC card in initialized or
> +unknown state), and before performing complete system reboot (also in
> +case of emergency reboot call). The latter is needed on boards, which
> +doesn't have hardware reset logic connected to emmc card and (limited or
> +broken) ROM bootloaders are unable to read second stage from the emmc
> +card if the card is left in unknown or already initialized state.
> +
> +Required properties:
> +- compatible : contains "mmc-pwrseq-emmc".
> +- reset-gpios : contains a GPIO specifier. The reset GPIO is asserted
> +       and then deasserted to perform eMMC card reset. To perform
> +       reset procedure as described in Jedec 4.4 specification, the
> +       gpio line should be defined as GPIO_ACTIVE_LOW.
> +
> +Example:
> +
> +       sdhci0_pwrseq {
> +               compatible = "mmc-pwrseq-emmc";
> +               reset-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
> +       }
> diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
> index b39cbd2..2c25138 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 pwrseq_simple.o
> +mmc_core-$(CONFIG_OF)          += pwrseq.o pwrseq_simple.o pwrseq_emmc.o
>  mmc_core-$(CONFIG_DEBUG_FS)    += debugfs.o
> diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
> index 2cea00e..8623561 100644
> --- a/drivers/mmc/core/pwrseq.c
> +++ b/drivers/mmc/core/pwrseq.c
> @@ -26,6 +26,9 @@ static struct mmc_pwrseq_match pwrseq_match[] = {
>         {
>                 .compatible = "mmc-pwrseq-simple",
>                 .alloc = mmc_pwrseq_simple_alloc,
> +       }, {
> +               .compatible = "mmc-pwrseq-emmc",
> +               .alloc = mmc_pwrseq_emmc_alloc,
>         },
>  };
>
> diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
> index bd860d8..aba3409 100644
> --- a/drivers/mmc/core/pwrseq.h
> +++ b/drivers/mmc/core/pwrseq.h
> @@ -28,6 +28,7 @@ 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);
> +int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev);
>
>  #else
>
> diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
> new file mode 100644
> index 0000000..8a434d7
> --- /dev/null
> +++ b/drivers/mmc/core/pwrseq_emmc.c
> @@ -0,0 +1,101 @@
> +/*
> + * Copyright (C) 2015, Samsung Electronics Co., Ltd.
> + *
> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + *
> + * Simple eMMC hardware reset provider
> + */
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/reboot.h>
> +
> +#include <linux/mmc/host.h>
> +
> +#include "pwrseq.h"
> +
> +struct mmc_pwrseq_emmc {
> +       struct mmc_pwrseq pwrseq;
> +       struct notifier_block reset_nb;
> +       struct gpio_desc *reset_gpio;
> +};
> +
> +static void __mmc_pwrseq_emmc_reset(struct mmc_pwrseq_emmc *pwrseq)
> +{
> +       gpiod_set_value(pwrseq->reset_gpio, 1);
> +       udelay(1);
> +       gpiod_set_value(pwrseq->reset_gpio, 0);
> +       udelay(200);
> +}
> +
> +static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
> +{
> +       struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
> +                                       struct mmc_pwrseq_emmc, pwrseq);
> +
> +       __mmc_pwrseq_emmc_reset(pwrseq);
> +}
> +
> +static void mmc_pwrseq_emmc_free(struct mmc_host *host)
> +{
> +       struct mmc_pwrseq_emmc *pwrseq = container_of(host->pwrseq,
> +                                       struct mmc_pwrseq_emmc, pwrseq);
> +
> +       unregister_restart_handler(&pwrseq->reset_nb);
> +       gpiod_put(pwrseq->reset_gpio);
> +       kfree(pwrseq);
> +       host->pwrseq = NULL;
> +}
> +
> +static struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
> +       .post_power_on = mmc_pwrseq_emmc_reset,
> +       .free = mmc_pwrseq_emmc_free,
> +};
> +
> +static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
> +                                   unsigned long mode, void *cmd)
> +{
> +       struct mmc_pwrseq_emmc *pwrseq;
> +       pwrseq = container_of(this, struct mmc_pwrseq_emmc, reset_nb);

Assign pwrseq while declaring it, will follow the looks from other
functions and silence a checkpatch warning.

> +
> +       __mmc_pwrseq_emmc_reset(pwrseq);
> +       return NOTIFY_DONE;
> +}
> +
> +int mmc_pwrseq_emmc_alloc(struct mmc_host *host, struct device *dev)
> +{
> +       struct mmc_pwrseq_emmc *pwrseq;
> +       int ret = 0;
> +
> +       pwrseq = kzalloc(sizeof(struct mmc_pwrseq_emmc), GFP_KERNEL);
> +       if (!pwrseq)
> +               return -ENOMEM;
> +
> +       pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_LOW);
> +       if (IS_ERR(pwrseq->reset_gpio)) {
> +               ret = PTR_ERR(pwrseq->reset_gpio);
> +               goto free;
> +       }
> +
> +       /*
> +        * register reset handler to ensure emmc reset also from
> +        * emergency_reboot(), priority 129 schedules it just before
> +        * system reboot
> +        */
> +       pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
> +       pwrseq->reset_nb.priority = 129;
> +       register_restart_handler(&pwrseq->reset_nb);
> +
> +       pwrseq->pwrseq.ops = &mmc_pwrseq_emmc_ops;
> +       host->pwrseq = &pwrseq->pwrseq;
> +
> +       return 0;
> +free:
> +       kfree(pwrseq);
> +       return ret;
> +}
> --
> 1.9.2
>

Re: [PATCH v2 0/3] Add support for hardware reset of eMMC card on reboot

[Kukjin Kim]

On 02/02/15 17:51, Javier Martinez Canillas wrote:
> Hello Marek,
> 
Hi Marek,

> On Mon, Feb 2, 2015 at 9:33 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> Hello,
>>
>> This patchset adds new mmc power sequence driver, which performs eMMC
>> card hardware reset. Such power sequence, involving resetting emmc card,
>> is required on some boards to properly perform system reboot procedure.
>>
> 
> Patches looks good to me, I've only one comment on patch 1/3. Once
> that is addressed, feel free to add for all the series:
> 
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> 
Applied 2/3 and 3/3 DT updates.

Thanks,
Kukjin