[PATCH] sh: check return code of request_irq

[PATCH] sh: check return code of request_irq

[Nick Desaulniers]

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);
-- 
2.29.2.729.g45daf8777d-goog

Re: [PATCH] sh: check return code of request_irq

[Masahiro Yamada]

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
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] sh: check return code of request_irq

[John Paul Adrian Glaubitz]

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>

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] sh: check return code of request_irq

[Miguel Ojeda]

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

Re: [PATCH] sh: check return code of request_irq

[John Paul Adrian Glaubitz]

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

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913