diff mbox series

sh: check return code of request_irq

Message ID 20201222205402.2269377-1-ndesaulniers@google.com (mailing list archive)
State New
Headers show
Series sh: check return code of request_irq | expand

Commit Message

Nick Desaulniers Dec. 22, 2020, 8:54 p.m. UTC
request_irq is marked __must_check, but the call in shx3_prepare_cpus
has a void return type, so it can't propagate failure to the caller.
Follow cues from hexagon and just print an error.

Fixes: c7936b9abcf5 ("sh: smp: Hook in to the generic IPI handler for SH-X3 SMP.")
Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/sh/kernel/cpu/sh4a/smp-shx3.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Masahiro Yamada Dec. 23, 2020, 6:33 a.m. UTC | #1
On Wed, Dec 23, 2020 at 5:54 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> request_irq is marked __must_check, but the call in shx3_prepare_cpus
> has a void return type, so it can't propagate failure to the caller.
> Follow cues from hexagon and just print an error.
>
> Fixes: c7936b9abcf5 ("sh: smp: Hook in to the generic IPI handler for SH-X3 SMP.")
> Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>


Thanks for the patch, Nick.

I just wondered if there was a better error handling than
printing the message. I have no idea if the system will
boot up correctly when the request_irq() fails here.

I hope the maintainers will suggest something, if any.




> ---
>  arch/sh/kernel/cpu/sh4a/smp-shx3.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/sh/kernel/cpu/sh4a/smp-shx3.c b/arch/sh/kernel/cpu/sh4a/smp-shx3.c
> index f8a2bec0f260..1261dc7b84e8 100644
> --- a/arch/sh/kernel/cpu/sh4a/smp-shx3.c
> +++ b/arch/sh/kernel/cpu/sh4a/smp-shx3.c
> @@ -73,8 +73,9 @@ static void shx3_prepare_cpus(unsigned int max_cpus)
>         BUILD_BUG_ON(SMP_MSG_NR >= 8);
>
>         for (i = 0; i < SMP_MSG_NR; i++)
> -               request_irq(104 + i, ipi_interrupt_handler,
> -                           IRQF_PERCPU, "IPI", (void *)(long)i);
> +               if (request_irq(104 + i, ipi_interrupt_handler,
> +                           IRQF_PERCPU, "IPI", (void *)(long)i))
> +                       pr_err("Failed to request irq %d\n", i);
>
>         for (i = 0; i < max_cpus; i++)
>                 set_cpu_present(i, true);
> --
> 2.29.2.729.g45daf8777d-goog
>
John Paul Adrian Glaubitz Jan. 1, 2021, 1:50 p.m. UTC | #2
Hi Nick!

On 12/22/20 9:54 PM, Nick Desaulniers wrote:
> request_irq is marked __must_check, but the call in shx3_prepare_cpus
> has a void return type, so it can't propagate failure to the caller.
> Follow cues from hexagon and just print an error.
> 
> Fixes: c7936b9abcf5 ("sh: smp: Hook in to the generic IPI handler for SH-X3 SMP.")
> Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  arch/sh/kernel/cpu/sh4a/smp-shx3.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/sh/kernel/cpu/sh4a/smp-shx3.c b/arch/sh/kernel/cpu/sh4a/smp-shx3.c
> index f8a2bec0f260..1261dc7b84e8 100644
> --- a/arch/sh/kernel/cpu/sh4a/smp-shx3.c
> +++ b/arch/sh/kernel/cpu/sh4a/smp-shx3.c
> @@ -73,8 +73,9 @@ static void shx3_prepare_cpus(unsigned int max_cpus)
>  	BUILD_BUG_ON(SMP_MSG_NR >= 8);
>  
>  	for (i = 0; i < SMP_MSG_NR; i++)
> -		request_irq(104 + i, ipi_interrupt_handler,
> -			    IRQF_PERCPU, "IPI", (void *)(long)i);
> +		if (request_irq(104 + i, ipi_interrupt_handler,
> +			    IRQF_PERCPU, "IPI", (void *)(long)i))
> +			pr_err("Failed to request irq %d\n", i);
>  
>  	for (i = 0; i < max_cpus; i++)
>  		set_cpu_present(i, true);
> 

Verified on my SH-7785LCR board. Boots fine.

Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Miguel Ojeda Jan. 1, 2021, 8:42 p.m. UTC | #3
On Fri, Jan 1, 2021 at 2:50 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
>
> Verified on my SH-7785LCR board. Boots fine.
>
> Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Thanks for testing, John!

I think Masahiro was concerned about the error case (I assume you
tested the happy path).

In any case, if no maintainer suggests something else, looks good to
me (and it is no worse than the status quo unless the `pr_err()` can
somehow kill it), so:

    Reviewed-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel
John Paul Adrian Glaubitz Jan. 18, 2021, 7 p.m. UTC | #4
Hi Miguel!

On 1/1/21 9:42 PM, Miguel Ojeda wrote:
> On Fri, Jan 1, 2021 at 2:50 PM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
>>
>> Verified on my SH-7785LCR board. Boots fine.
>>
>> Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> 
> Thanks for testing, John!
> 
> I think Masahiro was concerned about the error case (I assume you
> tested the happy path).

Not sure about testing the error case though. How do I make request_irq()
fail?

> In any case, if no maintainer suggests something else, looks good to
> me (and it is no worse than the status quo unless the `pr_err()` can
> somehow kill it), so:
> 
>     Reviewed-by: Miguel Ojeda <ojeda@kernel.org>

I agree.

Adrian
diff mbox series

Patch

diff --git a/arch/sh/kernel/cpu/sh4a/smp-shx3.c b/arch/sh/kernel/cpu/sh4a/smp-shx3.c
index f8a2bec0f260..1261dc7b84e8 100644
--- a/arch/sh/kernel/cpu/sh4a/smp-shx3.c
+++ b/arch/sh/kernel/cpu/sh4a/smp-shx3.c
@@ -73,8 +73,9 @@  static void shx3_prepare_cpus(unsigned int max_cpus)
 	BUILD_BUG_ON(SMP_MSG_NR >= 8);
 
 	for (i = 0; i < SMP_MSG_NR; i++)
-		request_irq(104 + i, ipi_interrupt_handler,
-			    IRQF_PERCPU, "IPI", (void *)(long)i);
+		if (request_irq(104 + i, ipi_interrupt_handler,
+			    IRQF_PERCPU, "IPI", (void *)(long)i))
+			pr_err("Failed to request irq %d\n", i);
 
 	for (i = 0; i < max_cpus; i++)
 		set_cpu_present(i, true);