[PATCH 0/2] Fix reboot issue on Odroid boards with eMMC card

[PATCH 0/2] Fix reboot issue on Odroid boards with eMMC card

[Marek Szyprowski]

Hello,

This patchset fixes 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, one need to set this line to zero 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
generic Exynos code, so the code has been moved to Exynos DW MMC driver.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Changelog:

initial version:
http://thread.gmane.org/gmane.linux.power-management.general/51855/


Path summary:

Marek Szyprowski (2):
  mmc: dw_mmc-exynos: add support for controlling emmc reset pin
  ARM: dts: exynos*-odroid*: add eMMC reset line

 .../devicetree/bindings/mmc/exynos-dw-mshc.txt     |  6 +++
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |  1 +
 arch/arm/boot/dts/exynos5422-odroidxu3.dts         |  1 +
 drivers/mmc/host/dw_mmc-exynos.c                   | 43 +++++++++++++++++++++-
 4 files changed, 50 insertions(+), 1 deletion(-)

-- 
1.9.2

[PATCH 1/2] mmc: dw_mmc-exynos: add support for controlling emmc reset pin

[Marek Szyprowski]

There are boards (like Hardkernel's Odroid boards) on which eMMC card's
reset line is connected to SoC GPIO line instead of the hardware reset
logic. In case of such boards, before performing system reboot,
additional reset of eMMC card is required to boot again properly.
This patch adds code for handling such cases.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../devicetree/bindings/mmc/exynos-dw-mshc.txt     |  6 +++
 drivers/mmc/host/dw_mmc-exynos.c                   | 43 +++++++++++++++++++++-
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
index ee4fc0576c7d..fc53d335e7db 100644
--- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
@@ -50,6 +50,12 @@ Required Properties:
       - if CIU clock divider value is 0 (that is divide by 1), both tx and rx
         phase shift clocks should be 0.
 
+Optional properties:
+
+* dw-mshc-reset-gpios: optional property specifying gpio for the eMMC nreset
+  line, it will be triggered on system reboot to properly reset eMMC card for
+  next system boot.
+
 Required properties for a slot (Deprecated - Recommend to use one slot per host):
 
 * gpios: specifies a list of gpios used for command, clock and data bus. The
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 509365cb22c6..2add5a93859d 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -12,12 +12,14 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/dw_mmc.h>
 #include <linux/mmc/mmc.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/slab.h>
+#include <linux/reboot.h>
 
 #include "dw_mmc.h"
 #include "dw_mmc-pltfm.h"
@@ -77,8 +79,23 @@ struct dw_mci_exynos_priv_data {
 	u32				sdr_timing;
 	u32				ddr_timing;
 	u32				cur_speed;
+	struct gpio_desc 		*reset_gpio;
+	struct notifier_block		reset_nb;
 };
 
+static int dw_mci_restart_handler(struct notifier_block *this,
+				  unsigned long mode, void *cmd)
+{
+	struct dw_mci_exynos_priv_data *data;
+	data = container_of(this, struct dw_mci_exynos_priv_data, reset_nb);
+
+	gpiod_direction_output(data->reset_gpio, 0);
+	mdelay(150);
+	gpiod_direction_output(data->reset_gpio, 1);
+
+	return NOTIFY_DONE;
+}
+
 static struct dw_mci_exynos_compatible {
 	char				*compatible;
 	enum dw_mci_exynos_type		ctrl_type;
@@ -295,7 +312,20 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
 		return ret;
 
 	priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
+
+	priv->reset_gpio = devm_gpiod_get_optional(host->dev,
+						   "samsung,dw-mshc-reset",
+						   GPIOD_OUT_HIGH);
+	if (!IS_ERR_OR_NULL(priv->reset_gpio)) {
+		priv->reset_nb.notifier_call = dw_mci_restart_handler;
+		priv->reset_nb.priority = 255;
+		ret = register_restart_handler(&priv->reset_nb);
+		if (ret)
+			dev_err(host->dev, "cannot register restart handler\n");
+	}
+
 	host->priv = priv;
+
 	return 0;
 }
 
@@ -490,6 +520,17 @@ static int dw_mci_exynos_probe(struct platform_device *pdev)
 	return dw_mci_pltfm_register(pdev, drv_data);
 }
 
+static int dw_mci_exynos_remove(struct platform_device *pdev)
+{
+	struct dw_mci *host = platform_get_drvdata(pdev);
+	struct dw_mci_exynos_priv_data *priv = host->priv;
+
+	if (priv->reset_gpio)
+		unregister_restart_handler(&priv->reset_nb);
+
+	return dw_mci_pltfm_remove(pdev);
+}
+
 static const struct dev_pm_ops dw_mci_exynos_pmops = {
 	SET_SYSTEM_SLEEP_PM_OPS(dw_mci_exynos_suspend, dw_mci_exynos_resume)
 	.resume_noirq = dw_mci_exynos_resume_noirq,
@@ -499,7 +540,7 @@ static const struct dev_pm_ops dw_mci_exynos_pmops = {
 
 static struct platform_driver dw_mci_exynos_pltfm_driver = {
 	.probe		= dw_mci_exynos_probe,
-	.remove		= __exit_p(dw_mci_pltfm_remove),
+	.remove		= dw_mci_exynos_remove,
 	.driver		= {
 		.name		= "dwmmc_exynos",
 		.of_match_table	= dw_mci_exynos_match,
-- 
1.9.2

[PATCH 2/2] ARM: dts: exynos*-odroid*: add eMMC reset line

[Marek Szyprowski]

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

Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
 arch/arm/boot/dts/exynos5422-odroidxu3.dts      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 74e89cbfb00c..97df0ef81d1a 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -69,6 +69,7 @@
 		samsung,dw-mshc-ciu-div = <3>;
 		samsung,dw-mshc-sdr-timing = <2 3>;
 		samsung,dw-mshc-ddr-timing = <1 2>;
+		samsung,dw-mshc-reset-gpios = <&gpk1 2 1>;
 		bus-width = <8>;
 		cap-mmc-highspeed;
 	};
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3.dts b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
index d829e220eedb..20064e36a5be 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3.dts
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
@@ -336,6 +336,7 @@
 	samsung,dw-mshc-ciu-div = <3>;
 	samsung,dw-mshc-sdr-timing = <0 4>;
 	samsung,dw-mshc-ddr-timing = <0 2>;
+	samsung,dw-mshc-reset-gpios = <&gpd1 0 0>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4 &sd0_bus8>;
 	bus-width = <8>;
-- 
1.9.2

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

Re: [PATCH 1/2] mmc: dw_mmc-exynos: add support for controlling emmc reset pin

[Jaehoon Chung]

Hi, Marek.

your patch should be conflicted with "https://patchwork.kernel.org/patch/5698421/"

On 01/27/2015 05:11 PM, Marek Szyprowski wrote:
> There are boards (like Hardkernel's Odroid boards) on which eMMC card's
> reset line is connected to SoC GPIO line instead of the hardware reset
> logic. In case of such boards, before performing system reboot,
> additional reset of eMMC card is required to boot again properly.
> This patch adds code for handling such cases.

mmc core supported to hw_reset function.
So i think we can use it. It's called at only initial time to clear the previous status.
But i think it can be called at reboot time. (it needs to implement codes.)
how about?

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../devicetree/bindings/mmc/exynos-dw-mshc.txt     |  6 +++
>  drivers/mmc/host/dw_mmc-exynos.c                   | 43 +++++++++++++++++++++-
>  2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
> index ee4fc0576c7d..fc53d335e7db 100644
> --- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
> @@ -50,6 +50,12 @@ Required Properties:
>        - if CIU clock divider value is 0 (that is divide by 1), both tx and rx
>          phase shift clocks should be 0.
>  
> +Optional properties:
> +
> +* dw-mshc-reset-gpios: optional property specifying gpio for the eMMC nreset
> +  line, it will be triggered on system reboot to properly reset eMMC card for
> +  next system boot.
> +
>  Required properties for a slot (Deprecated - Recommend to use one slot per host):
>  
>  * gpios: specifies a list of gpios used for command, clock and data bus. The
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index 509365cb22c6..2add5a93859d 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -12,12 +12,14 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
> +#include <linux/delay.h>
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/dw_mmc.h>
>  #include <linux/mmc/mmc.h>
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
>  #include <linux/slab.h>
> +#include <linux/reboot.h>
>  
>  #include "dw_mmc.h"
>  #include "dw_mmc-pltfm.h"
> @@ -77,8 +79,23 @@ struct dw_mci_exynos_priv_data {
>  	u32				sdr_timing;
>  	u32				ddr_timing;
>  	u32				cur_speed;
> +	struct gpio_desc 		*reset_gpio;
> +	struct notifier_block		reset_nb;
>  };
>  
> +static int dw_mci_restart_handler(struct notifier_block *this,
> +				  unsigned long mode, void *cmd)
> +{
> +	struct dw_mci_exynos_priv_data *data;
> +	data = container_of(this, struct dw_mci_exynos_priv_data, reset_nb);
> +
> +	gpiod_direction_output(data->reset_gpio, 0);
> +	mdelay(150);
> +	gpiod_direction_output(data->reset_gpio, 1);
> +
> +	return NOTIFY_DONE;
> +}
> +
>  static struct dw_mci_exynos_compatible {
>  	char				*compatible;
>  	enum dw_mci_exynos_type		ctrl_type;
> @@ -295,7 +312,20 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
>  		return ret;
>  
>  	priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
> +
> +	priv->reset_gpio = devm_gpiod_get_optional(host->dev,
> +						   "samsung,dw-mshc-reset",
> +						   GPIOD_OUT_HIGH);
> +	if (!IS_ERR_OR_NULL(priv->reset_gpio)) {
> +		priv->reset_nb.notifier_call = dw_mci_restart_handler;
> +		priv->reset_nb.priority = 255;
> +		ret = register_restart_handler(&priv->reset_nb);
> +		if (ret)
> +			dev_err(host->dev, "cannot register restart handler\n");
> +	}
> +
>  	host->priv = priv;
> +
>  	return 0;
>  }
>  
> @@ -490,6 +520,17 @@ static int dw_mci_exynos_probe(struct platform_device *pdev)
>  	return dw_mci_pltfm_register(pdev, drv_data);
>  }
>  
> +static int dw_mci_exynos_remove(struct platform_device *pdev)
> +{
> +	struct dw_mci *host = platform_get_drvdata(pdev);
> +	struct dw_mci_exynos_priv_data *priv = host->priv;
> +
> +	if (priv->reset_gpio)
> +		unregister_restart_handler(&priv->reset_nb);
> +
> +	return dw_mci_pltfm_remove(pdev);
> +}
> +
>  static const struct dev_pm_ops dw_mci_exynos_pmops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(dw_mci_exynos_suspend, dw_mci_exynos_resume)
>  	.resume_noirq = dw_mci_exynos_resume_noirq,
> @@ -499,7 +540,7 @@ static const struct dev_pm_ops dw_mci_exynos_pmops = {
>  
>  static struct platform_driver dw_mci_exynos_pltfm_driver = {
>  	.probe		= dw_mci_exynos_probe,
> -	.remove		= __exit_p(dw_mci_pltfm_remove),
> +	.remove		= dw_mci_exynos_remove,
>  	.driver		= {
>  		.name		= "dwmmc_exynos",
>  		.of_match_table	= dw_mci_exynos_match,
> 

Re: [PATCH 0/2] Fix reboot issue on Odroid boards with eMMC card

[Sjoerd Simons]

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

On Tue, 2015-01-27 at 09:11 +0100, Marek Szyprowski wrote:
> Hello,
> 
> This patchset fixes 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, one need to set this line to zero 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
> generic Exynos code, so the code has been moved to Exynos DW MMC driver.

This seems a general board design quirk, rather then an exynos specific
one. Potentially better to have this in the mmc core so it can be
re-used by non-exynos boards?

A very quick peek at the schematic for the hardkernel C1 (amlogic
based), shows that for that board the eMMC reset line is hooked up to an
SoC gpio line as well. (Although ofcourse, it may not need to be reset
before a warm restart depending on what the amlogic rom code does).


Random side-note, at least the samsung peach chromebooks are also hooked
up this way. But don't run into this issue as they load their initial
stages from flash rather then the eMMC. 


> Best regards
> Marek Szyprowski
> Samsung R&D Institute Poland
> 
> 
> Changelog:
> 
> initial version:
> http://thread.gmane.org/gmane.linux.power-management.general/51855/
> 
> 
> Path summary:
> 
> Marek Szyprowski (2):
>   mmc: dw_mmc-exynos: add support for controlling emmc reset pin
>   ARM: dts: exynos*-odroid*: add eMMC reset line
> 
>  .../devicetree/bindings/mmc/exynos-dw-mshc.txt     |  6 +++
>  arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |  1 +
>  arch/arm/boot/dts/exynos5422-odroidxu3.dts         |  1 +
>  drivers/mmc/host/dw_mmc-exynos.c                   | 43 +++++++++++++++++++++-
>  4 files changed, 50 insertions(+), 1 deletion(-)
> 



[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6170 bytes --]

Re: [PATCH 0/2] Fix reboot issue on Odroid boards with eMMC card

[Marek Szyprowski]

Hello,

On 2015-01-27 09:56, Sjoerd Simons wrote:
> On Tue, 2015-01-27 at 09:11 +0100, Marek Szyprowski wrote:
>> This patchset fixes 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, one need to set this line to zero 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
>> generic Exynos code, so the code has been moved to Exynos DW MMC driver.
> This seems a general board design quirk, rather then an exynos specific
> one. Potentially better to have this in the mmc core so it can be
> re-used by non-exynos boards?
>
> A very quick peek at the schematic for the hardkernel C1 (amlogic
> based), shows that for that board the eMMC reset line is hooked up to an
> SoC gpio line as well. (Although ofcourse, it may not need to be reset
> before a warm restart depending on what the amlogic rom code does).
>
>
> Random side-note, at least the samsung peach chromebooks are also hooked
> up this way. But don't run into this issue as they load their initial
> stages from flash rather then the eMMC.

Okay, I will try to move all the code to mmc core. I've also noticed that
sdhci-pci driver already implements hw_reset feature with gpio line, 
although
I cannot find any user of it (I cannot find any platform data provider for
sdhci-pci driver).

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

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

Re: [PATCH 1/2] mmc: dw_mmc-exynos: add support for controlling emmc reset pin

[Tobias Jakobi]

Hello!

Jaehoon Chung wrote:
> mmc core supported to hw_reset function.
> So i think we can use it. It's called at only initial time to clear the previous status.
> But i think it can be called at reboot time. (it needs to implement codes.)
> how about?
I don't think that's going the work. The problem here is that depending
on the board configuration, the eMMC might carry the bootloader. If the
eMMC subsystem is not properly reset _during_ reboot, the board won't
even start since no bootloader is found. So we don't even reach the
point where the kernel does anything.


With best wishes,
Tobias



> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  .../devicetree/bindings/mmc/exynos-dw-mshc.txt     |  6 +++
>>  drivers/mmc/host/dw_mmc-exynos.c                   | 43 +++++++++++++++++++++-
>>  2 files changed, 48 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>> index ee4fc0576c7d..fc53d335e7db 100644
>> --- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>> @@ -50,6 +50,12 @@ Required Properties:
>>        - if CIU clock divider value is 0 (that is divide by 1), both tx and rx
>>          phase shift clocks should be 0.
>>  
>> +Optional properties:
>> +
>> +* dw-mshc-reset-gpios: optional property specifying gpio for the eMMC nreset
>> +  line, it will be triggered on system reboot to properly reset eMMC card for
>> +  next system boot.
>> +
>>  Required properties for a slot (Deprecated - Recommend to use one slot per host):
>>  
>>  * gpios: specifies a list of gpios used for command, clock and data bus. The
>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
>> index 509365cb22c6..2add5a93859d 100644
>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>> @@ -12,12 +12,14 @@
>>  #include <linux/module.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/clk.h>
>> +#include <linux/delay.h>
>>  #include <linux/mmc/host.h>
>>  #include <linux/mmc/dw_mmc.h>
>>  #include <linux/mmc/mmc.h>
>>  #include <linux/of.h>
>>  #include <linux/of_gpio.h>
>>  #include <linux/slab.h>
>> +#include <linux/reboot.h>
>>  
>>  #include "dw_mmc.h"
>>  #include "dw_mmc-pltfm.h"
>> @@ -77,8 +79,23 @@ struct dw_mci_exynos_priv_data {
>>  	u32				sdr_timing;
>>  	u32				ddr_timing;
>>  	u32				cur_speed;
>> +	struct gpio_desc 		*reset_gpio;
>> +	struct notifier_block		reset_nb;
>>  };
>>  
>> +static int dw_mci_restart_handler(struct notifier_block *this,
>> +				  unsigned long mode, void *cmd)
>> +{
>> +	struct dw_mci_exynos_priv_data *data;
>> +	data = container_of(this, struct dw_mci_exynos_priv_data, reset_nb);
>> +
>> +	gpiod_direction_output(data->reset_gpio, 0);
>> +	mdelay(150);
>> +	gpiod_direction_output(data->reset_gpio, 1);
>> +
>> +	return NOTIFY_DONE;
>> +}
>> +
>>  static struct dw_mci_exynos_compatible {
>>  	char				*compatible;
>>  	enum dw_mci_exynos_type		ctrl_type;
>> @@ -295,7 +312,20 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
>>  		return ret;
>>  
>>  	priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
>> +
>> +	priv->reset_gpio = devm_gpiod_get_optional(host->dev,
>> +						   "samsung,dw-mshc-reset",
>> +						   GPIOD_OUT_HIGH);
>> +	if (!IS_ERR_OR_NULL(priv->reset_gpio)) {
>> +		priv->reset_nb.notifier_call = dw_mci_restart_handler;
>> +		priv->reset_nb.priority = 255;
>> +		ret = register_restart_handler(&priv->reset_nb);
>> +		if (ret)
>> +			dev_err(host->dev, "cannot register restart handler\n");
>> +	}
>> +
>>  	host->priv = priv;
>> +
>>  	return 0;
>>  }
>>  
>> @@ -490,6 +520,17 @@ static int dw_mci_exynos_probe(struct platform_device *pdev)
>>  	return dw_mci_pltfm_register(pdev, drv_data);
>>  }
>>  
>> +static int dw_mci_exynos_remove(struct platform_device *pdev)
>> +{
>> +	struct dw_mci *host = platform_get_drvdata(pdev);
>> +	struct dw_mci_exynos_priv_data *priv = host->priv;
>> +
>> +	if (priv->reset_gpio)
>> +		unregister_restart_handler(&priv->reset_nb);
>> +
>> +	return dw_mci_pltfm_remove(pdev);
>> +}
>> +
>>  static const struct dev_pm_ops dw_mci_exynos_pmops = {
>>  	SET_SYSTEM_SLEEP_PM_OPS(dw_mci_exynos_suspend, dw_mci_exynos_resume)
>>  	.resume_noirq = dw_mci_exynos_resume_noirq,
>> @@ -499,7 +540,7 @@ static const struct dev_pm_ops dw_mci_exynos_pmops = {
>>  
>>  static struct platform_driver dw_mci_exynos_pltfm_driver = {
>>  	.probe		= dw_mci_exynos_probe,
>> -	.remove		= __exit_p(dw_mci_pltfm_remove),
>> +	.remove		= dw_mci_exynos_remove,
>>  	.driver		= {
>>  		.name		= "dwmmc_exynos",
>>  		.of_match_table	= dw_mci_exynos_match,
>>
> 
> 

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

Re: [PATCH 1/2] mmc: dw_mmc-exynos: add support for controlling emmc reset pin

[Ulf Hansson]

On 28 January 2015 at 13:41, Tobias Jakobi <liquid.acid-hi6Y0CQ0nG0@public.gmane.org> wrote:
> Hello!
>
> Jaehoon Chung wrote:
>> mmc core supported to hw_reset function.
>> So i think we can use it. It's called at only initial time to clear the previous status.
>> But i think it can be called at reboot time. (it needs to implement codes.)
>> how about?
> I don't think that's going the work. The problem here is that depending
> on the board configuration, the eMMC might carry the bootloader. If the
> eMMC subsystem is not properly reset _during_ reboot, the board won't
> even start since no bootloader is found. So we don't even reach the
> point where the kernel does anything.

I guess it depends on what's being done during the reboot sequence.

The most proper thing would be to let the boot loader control the GPIO
to trigger the HW reset, but that would mean the boot loader need to
know about board specific configurations, such as which GPIO pin. So
maybe SOC vendors need to state what GPIO pin to use and don't leave
that as a configurable option.

>From kernel perspective, the best we can do is to the GPIO, when doing
a controlled reset (soft reset, or whatever we call it), but I am not
sure where that should be done? Is there a guarantee that the mmc bus'
->shutdown() callback gets called in this sequence?

Moreover, adding the reset GPIO as part of the initialization
procedure in the mmc core, gives us other benefits and it won't hurt.
So no matter, I think it's worth to proceed and discuss Marek's
proposal.

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

Re: [PATCH 1/2] mmc: dw_mmc-exynos: add support for controlling emmc reset pin

[Tobias Jakobi]

Ulf Hansson wrote:
> On 28 January 2015 at 13:41, Tobias Jakobi <liquid.acid-hi6Y0CQ0nG0@public.gmane.org> wrote:
>> Hello!
>>
>> Jaehoon Chung wrote:
>>> mmc core supported to hw_reset function.
>>> So i think we can use it. It's called at only initial time to clear the previous status.
>>> But i think it can be called at reboot time. (it needs to implement codes.)
>>> how about?
>> I don't think that's going the work. The problem here is that depending
>> on the board configuration, the eMMC might carry the bootloader. If the
>> eMMC subsystem is not properly reset _during_ reboot, the board won't
>> even start since no bootloader is found. So we don't even reach the
>> point where the kernel does anything.
> 
> I guess it depends on what's being done during the reboot sequence.
> 
> The most proper thing would be to let the boot loader control the GPIO
> to trigger the HW reset, but that would mean the boot loader need to
> know about board specific configurations, such as which GPIO pin. So
> maybe SOC vendors need to state what GPIO pin to use and don't leave
> that as a configurable option.
Not the bootloader, but the iROM code would have to do this, and as far
as I know the iROM can't be modified. Or am I missing something here?


With best wishes,
Tobias

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

Re: [PATCH 1/2] mmc: dw_mmc-exynos: add support for controlling emmc reset pin

[Ulf Hansson]

On 28 January 2015 at 16:23, Tobias Jakobi <liquid.acid@gmx.net> wrote:
> Ulf Hansson wrote:
>> On 28 January 2015 at 13:41, Tobias Jakobi <liquid.acid@gmx.net> wrote:
>>> Hello!
>>>
>>> Jaehoon Chung wrote:
>>>> mmc core supported to hw_reset function.
>>>> So i think we can use it. It's called at only initial time to clear the previous status.
>>>> But i think it can be called at reboot time. (it needs to implement codes.)
>>>> how about?
>>> I don't think that's going the work. The problem here is that depending
>>> on the board configuration, the eMMC might carry the bootloader. If the
>>> eMMC subsystem is not properly reset _during_ reboot, the board won't
>>> even start since no bootloader is found. So we don't even reach the
>>> point where the kernel does anything.
>>
>> I guess it depends on what's being done during the reboot sequence.
>>
>> The most proper thing would be to let the boot loader control the GPIO
>> to trigger the HW reset, but that would mean the boot loader need to
>> know about board specific configurations, such as which GPIO pin. So
>> maybe SOC vendors need to state what GPIO pin to use and don't leave
>> that as a configurable option.
> Not the bootloader, but the iROM code would have to do this, and as far
> as I know the iROM can't be modified. Or am I missing something here?

You are right! The "bootloader" can very likely be stored in ROM code.
That's why I also said the GPIO shouldn't be configurable. :-)

Kind regards
Uffe