On 4/9/19 8:40 AM, Andrew Jones wrote:
> On Mon, Apr 08, 2019 at 04:11:25PM +0100, Alexandru Elisei wrote:
>> The prototype for the halt() function is incorrect, because halt() doesn't
>> take any arguments. Fix it by using the prototype from smp.h.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
>> ---
>> lib/arm/io.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/arm/io.c b/lib/arm/io.c
>> index 8226b765bdc5..6d3d7afed002 100644
>> --- a/lib/arm/io.c
>> +++ b/lib/arm/io.c
>> @@ -15,11 +15,10 @@
>> #include <asm/psci.h>
>> #include <asm/spinlock.h>
>> #include <asm/io.h>
>> +#include <asm/smp.h>
>>
>> #include "io.h"
>>
>> -extern void halt(int code);
>> -
>> static struct spinlock uart_lock;
>> /*
>> * Use this guess for the uart base in order to make an attempt at
>> @@ -93,6 +92,6 @@ void exit(int code)
>> {
>> chr_testdev_exit(code);
>> psci_system_off();
>> - halt(code);
>> + halt();
>> __builtin_unreachable();
>> }
>> --
>> 2.17.0
>>
> I don't mind this change, because per the code it is the "correct"
> thing to do. However, I was being a bit tricky here when I wrote it.
> By changing the prototype to take 'code' as argument we guarantee
> that 'code' will be in x0/r0 when we halt, giving us a last chance
> to see it when inspecting the halted unit test state.
>
> Anyway, like I said, I'm fine with the cleanup, but the prototype
> abuse does serve a purpose - maybe just not a good enough purpose
> to justify the weirdness.
Now it makes sense. I didn't think it was intentional, but now that you have
mentioned it, the same pattern is used by powerpc.
Perhaps a comment explaining that having different prototypes was on purpose
would be the best solution?
>
> Thanks,
> drew
