[PATCH] MIPS: ralink: remove ralink_halt()

[PATCH] MIPS: ralink: remove ralink_halt()

[NeilBrown]

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


ralink_halt() does nothing that machine_halt()
doesn't already do, so it adds no value.

It actually causes incorrect behaviour due to the
"unreachable()" at the end.  This tell the compiler that the
end of the function will never be reached, which isn't true.
The compiler responds by not adding a 'return' instruction,
so control simply moves on to whatever bytes come afterwards
in memory.  In my tested, that was the ralink_restart()
function.  This means that an attempt to 'halt' the machine
would actually cause a reboot.

So remove ralink_halt() so that a 'halt' really does halt.

Signed-off-by: NeilBrown <neil@brown.name>
---
 arch/mips/ralink/reset.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/mips/ralink/reset.c b/arch/mips/ralink/reset.c
index 64543d66e76b..e9531fea23a2 100644
--- a/arch/mips/ralink/reset.c
+++ b/arch/mips/ralink/reset.c
@@ -96,16 +96,9 @@ static void ralink_restart(char *command)
 	unreachable();
 }
 
-static void ralink_halt(void)
-{
-	local_irq_disable();
-	unreachable();
-}
-
 static int __init mips_reboot_setup(void)
 {
 	_machine_restart = ralink_restart;
-	_machine_halt = ralink_halt;
 
 	return 0;
 }
-- 
2.14.0.rc0.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] MIPS: ralink: remove ralink_halt()

[James Hogan]

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

On Tue, Mar 20, 2018 at 07:29:51PM +1100, NeilBrown wrote:
> 
> ralink_halt() does nothing that machine_halt()
> doesn't already do, so it adds no value.
> 
> It actually causes incorrect behaviour due to the
> "unreachable()" at the end.  This tell the compiler that the
> end of the function will never be reached, which isn't true.
> The compiler responds by not adding a 'return' instruction,
> so control simply moves on to whatever bytes come afterwards
> in memory.  In my tested, that was the ralink_restart()
> function.  This means that an attempt to 'halt' the machine
> would actually cause a reboot.
> 
> So remove ralink_halt() so that a 'halt' really does halt.
> 
> Signed-off-by: NeilBrown <neil@brown.name>

Thanks, I've cosmetically tweaked the commit message (mainly reflow to
72 characters) and added:

Fixes: c06e836ada59 ("MIPS: ralink: adds reset code")
Cc: <stable@vger.kernel.org> # 3.9+

and applied for 4.16.

BTW, I'm intrigued to know if there's a particular reason you don't
author / sign-off as "Neil Brown"? Its supposed to be real names, though
"NeilBrown" is hardly difficult to figure out so I don't actually
object.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] MIPS: ralink: remove ralink_halt()

[NeilBrown]

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

On Wed, Mar 21 2018, James Hogan wrote:

> On Tue, Mar 20, 2018 at 07:29:51PM +1100, NeilBrown wrote:
>> 
>> ralink_halt() does nothing that machine_halt()
>> doesn't already do, so it adds no value.
>> 
>> It actually causes incorrect behaviour due to the
>> "unreachable()" at the end.  This tell the compiler that the
>> end of the function will never be reached, which isn't true.
>> The compiler responds by not adding a 'return' instruction,
>> so control simply moves on to whatever bytes come afterwards
>> in memory.  In my tested, that was the ralink_restart()
>> function.  This means that an attempt to 'halt' the machine
>> would actually cause a reboot.
>> 
>> So remove ralink_halt() so that a 'halt' really does halt.
>> 
>> Signed-off-by: NeilBrown <neil@brown.name>
>
> Thanks, I've cosmetically tweaked the commit message (mainly reflow to
> 72 characters) and added:
>
> Fixes: c06e836ada59 ("MIPS: ralink: adds reset code")
> Cc: <stable@vger.kernel.org> # 3.9+
>
> and applied for 4.16.
>
> BTW, I'm intrigued to know if there's a particular reason you don't
> author / sign-off as "Neil Brown"? Its supposed to be real names, though
> "NeilBrown" is hardly difficult to figure out so I don't actually
> object.

I started using NeilBrown way back when I was an undergrad student and
it stuck.  When you grow up as a Brown, you know your name isn't going
to make you unique. e.g. I'm not an author of "Red Hat Linux System
Administration Unleashed" (he has a space in his name!!).  So I chose a
different way to make my name distinctive.
Yes, it isn't technically compliant - you are the second person to
comment in the nearly twenty years I've been working on Linux :-)

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]