Re: [PATCH 1/1] x86: Fix Intel Edison module stuck reboot

Re: [PATCH 1/1] x86: Fix Intel Edison module stuck reboot

[Andy Shevchenko]

On Thu, Feb 22, 2018 at 11:27 AM, Sebastian Panceac <sebastian@resin.io> wrote:
>
> When powering Intel Edison module with 3.3V, the "reboot" command makes the module stuck.
> If the module is powered at a greater voltage, like 4.4V(as the Edison Mini Breakout board does), reboot works OK.
> This patch fixes the reboot problem for Intel Edison powered at 3.3V.

Unfortunately it doesn't explain the root cause. OTOH it seems no one
has any means to find it, no documentation, no person responsible for
Edison firmware responsive.

At least what I would like to mention that this copies a behaviour of
official BSP for Edison, where RESET command applied by default (and
BOOT is enforced by user via kernel command line).

Perhaps someone can dig an internet to see what BOOT SCU command means.

Your patch is mangled. Can you use `git send-email` directly?

> Signed-off-by: Sebastian Panceac <sebastian@resin.io>
> ---
> arch/x86/platform/intel-mid/intel-mid.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
> index 12a2725..d2b2e9f 100644
> --- a/arch/x86/platform/intel-mid/intel-mid.c
> +++ b/arch/x86/platform/intel-mid/intel-mid.c
> @@ -79,7 +79,7 @@ static void intel_mid_power_off(void)
>
> static void intel_mid_reboot(void)
> {
> - intel_scu_ipc_simple_command(IPCMSG_COLD_BOOT, 0);
> + intel_scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
> }
>
> static unsigned long __init intel_mid_calibrate_tsc(void)
> --
> 2.7.4




-- 
With Best Regards,
Andy Shevchenko

[PATCH 1/1] x86: Fix Intel Edison module stuck reboot

[Sebastian Panceac]

Hi Andy,

I fixed the commit description according to your suggestions and used git send-email.

Regards!

When powering Intel Edison module with 3.3V,
the "reboot" command makes the module stuck.
If the module is powered at a greater voltage,
like 4.4V(as the Edison Mini Breakout board does), reboot works OK.

This patch copies the behaviour from the official Intel Edison BSP,
where "IPCMSG_COLD_RESET" message
was sent to SCU by default at reboot time.

In the official BSP, sending the "IPCMSG_COLD_BOOT" message instead
of "IPCMSG_COLD_RESET" was possible,
by using a kernel command line parameter.

Signed-off-by: Sebastian Panceac <sebastian@resin.io>
---
 arch/x86/platform/intel-mid/intel-mid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index 2c67bae..fb1df94 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -79,7 +79,7 @@ static void intel_mid_power_off(void)
 
 static void intel_mid_reboot(void)
 {
-	intel_scu_ipc_simple_command(IPCMSG_COLD_BOOT, 0);
+	intel_scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
 }
 
 static unsigned long __init intel_mid_calibrate_tsc(void)
-- 
2.7.4

Re: [PATCH 1/1] x86: Fix Intel Edison module stuck reboot

[Andy Shevchenko]

On Tue, Feb 27, 2018 at 12:07 PM, Sebastian Panceac <sebastian@resin.io> wrote:
> Hi Andy,

Thanks for an update! My comments below.

> I fixed the commit description according to your suggestions and used git send-email.
>
> Regards!

Changelog, which is effectively above paragraph, should go after --- line below.

Version of the patch should be increased (this one is v2 so far).

You should include all maintainers and designated reviewers of the
code in question.
get_maintainer.pl script (in scripts/) will help you.

So, please resend taking into consideration all above.

> When powering Intel Edison module with 3.3V,
> the "reboot" command makes the module stuck.
> If the module is powered at a greater voltage,
> like 4.4V(as the Edison Mini Breakout board does), reboot works OK.

This looks a bit shorter than we have room for. I mean, 72 characters
per line is pretty much OK.

Also, don't miss a space before (.

> This patch copies the behaviour from the official Intel Edison BSP,
> where "IPCMSG_COLD_RESET" message
> was sent to SCU by default at reboot time.
>
> In the official BSP, sending the "IPCMSG_COLD_BOOT" message instead
> of "IPCMSG_COLD_RESET" was possible,
> by using a kernel command line parameter.
>
> Signed-off-by: Sebastian Panceac <sebastian@resin.io>
> ---

...changelog goes here...

>  arch/x86/platform/intel-mid/intel-mid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)


-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 1/1] x86: Fix Intel Edison module stuck reboot

[Sebastian Panceac]

When powering Intel Edison module with 3.3V, the "reboot" command makes
the module stuck.
If the module is powered at a greater voltage, like 4.4V (as the Edison
Mini Breakout board does), reboot works OK.

This patch copies the behaviour from the official Intel Edison BSP,
where "IPCMSG_COLD_RESET" message was sent to SCU by default at
reboot time.

In the official BSP, sending the "IPCMSG_COLD_BOOT" message instead of
"IPCMSG_COLD_RESET" was possible, by using a kernel command
line parameter.

Signed-off-by: Sebastian Panceac <sebastian@resin.io>
---

Hi,

I submitted again the patch, after taking into consideration Andy's suggestions.

Regards!

 arch/x86/platform/intel-mid/intel-mid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index 2c67bae..fb1df94 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -79,7 +79,7 @@ static void intel_mid_power_off(void)
 
 static void intel_mid_reboot(void)
 {
-	intel_scu_ipc_simple_command(IPCMSG_COLD_BOOT, 0);
+	intel_scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
 }
 
 static unsigned long __init intel_mid_calibrate_tsc(void)
-- 
2.7.4

Re: [PATCH v3 1/1] x86: Fix Intel Edison module stuck reboot

[Andy Shevchenko]

On Wed, Feb 28, 2018 at 11:40 AM, Sebastian Panceac <sebastian@resin.io> wrote:
> When powering Intel Edison module with 3.3V, the "reboot" command makes
> the module stuck.
> If the module is powered at a greater voltage, like 4.4V (as the Edison
> Mini Breakout board does), reboot works OK.
>
> This patch copies the behaviour from the official Intel Edison BSP,
> where "IPCMSG_COLD_RESET" message was sent to SCU by default at
> reboot time.
>
> In the official BSP, sending the "IPCMSG_COLD_BOOT" message instead of
> "IPCMSG_COLD_RESET" was possible, by using a kernel command
> line parameter.
>

Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Sebastian Panceac <sebastian@resin.io>
> ---
>
> Hi,
>
> I submitted again the patch, after taking into consideration Andy's suggestions.
>
> Regards!
>
>  arch/x86/platform/intel-mid/intel-mid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
> index 2c67bae..fb1df94 100644
> --- a/arch/x86/platform/intel-mid/intel-mid.c
> +++ b/arch/x86/platform/intel-mid/intel-mid.c
> @@ -79,7 +79,7 @@ static void intel_mid_power_off(void)
>
>  static void intel_mid_reboot(void)
>  {
> -       intel_scu_ipc_simple_command(IPCMSG_COLD_BOOT, 0);
> +       intel_scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
>  }
>
>  static unsigned long __init intel_mid_calibrate_tsc(void)
> --
> 2.7.4



-- 
With Best Regards,
Andy Shevchenko

[tip:x86/urgent] x86/platform/intel-mid: Handle Intel Edison reboot correctly

[tip-bot for Sebastian Panceac]

Commit-ID:  028091f82eefd5e84f81cef81a7673016ecbe78b
Gitweb:     https://git.kernel.org/tip/028091f82eefd5e84f81cef81a7673016ecbe78b
Author:     Sebastian Panceac <sebastian@resin.io>
AuthorDate: Wed, 28 Feb 2018 11:40:49 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 28 Feb 2018 18:17:18 +0100

x86/platform/intel-mid: Handle Intel Edison reboot correctly

When the Intel Edison module is powered with 3.3V, the reboot command makes
the module stuck.  If the module is powered at a greater voltage, like 4.4V
(as the Edison Mini Breakout board does), reboot works OK.

The official Intel Edison BSP sends the IPCMSG_COLD_RESET message to the
SCU by default. The IPCMSG_COLD_BOOT which is used by the upstream kernel
is only sent when explicitely selected on the kernel command line.

Use IPCMSG_COLD_RESET unconditionally which makes reboot work independent
of the power supply voltage.

[ tglx: Massaged changelog ]

Fixes: bda7b072de99 ("x86/platform/intel-mid: Implement power off sequence")
Signed-off-by: Sebastian Panceac <sebastian@resin.io>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/1519810849-15131-1-git-send-email-sebastian@resin.io

---
 arch/x86/platform/intel-mid/intel-mid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index 2c67bae6bb53..fb1df9488e98 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -79,7 +79,7 @@ static void intel_mid_power_off(void)
 
 static void intel_mid_reboot(void)
 {
-	intel_scu_ipc_simple_command(IPCMSG_COLD_BOOT, 0);
+	intel_scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
 }
 
 static unsigned long __init intel_mid_calibrate_tsc(void)