Re: [PATCH] sh: check return code of request_irq
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 > wrote: >> >> Verified on my SH-7785LCR board. Boots fine. >> >> Tested-by: John Paul Adrian Glaubitz > > 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 I agree. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [PATCH] sh: check return code of request_irq
On Fri, Jan 1, 2021 at 2:50 PM John Paul Adrian Glaubitz wrote: > > Verified on my SH-7785LCR board. Boots fine. > > Tested-by: John Paul Adrian Glaubitz 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 Cheers, Miguel
Re: [PATCH] sh: check return code of request_irq
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
> Cc: Paul Mundt
> Reported-by: Guenter Roeck
> Signed-off-by: Nick Desaulniers
> ---
> 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
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaub...@debian.org
`. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
`-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [PATCH] sh: check return code of request_irq
On Wed, Dec 23, 2020 at 5:54 AM 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
> Cc: Paul Mundt
> Reported-by: Guenter Roeck
> Signed-off-by: Nick Desaulniers
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
[PATCH] sh: check return code of request_irq
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
Cc: Paul Mundt
Reported-by: Guenter Roeck
Signed-off-by: Nick Desaulniers
---
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
