| Message ID | 20210610140118.1437-1-hbut_tan@163.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 950fd045d76c96ada8c7a6e80f1f4e40de4beb17 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | soc: qcom: ipa: Remove superfluous error message around platform_get_irq() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 12 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
From: Zhongjun Tan <hbut_tan@163.com> Date: Thu, 10 Jun 2021 22:01:18 +0800 > diff --git a/drivers/net/ipa/ipa_smp2p.c b/drivers/net/ipa/ipa_smp2p.c > index 34b68dc43886..93270e50b6b3 100644 > --- a/drivers/net/ipa/ipa_smp2p.c > +++ b/drivers/net/ipa/ipa_smp2p.c > @@ -177,11 +177,8 @@ static int ipa_smp2p_irq_init(struct ipa_smp2p *smp2p, const char *name, > int ret; > > ret = platform_get_irq_byname(smp2p->ipa->pdev, name); > - if (ret <= 0) { > - dev_err(dev, "DT error %d getting \"%s\" IRQ property\n", > - ret, name); > + if (ret <= 0) Applied, but this code still rejects an irq of zero which is a valid irq number.
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Thu, 10 Jun 2021 22:01:18 +0800 you wrote: > From: Tan Zhongjun <tanzhongjun@yulong.com> > > The platform_get_irq() prints error message telling that interrupt is > missing,hence there is no need to duplicated that message in the > drivers. > > Signed-off-by: Tan Zhongjun <tanzhongjun@yulong.com> > > [...] Here is the summary with links: - soc: qcom: ipa: Remove superfluous error message around platform_get_irq() https://git.kernel.org/netdev/net-next/c/950fd045d76c You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On 6/10/21 4:11 PM, David Miller wrote: > From: Zhongjun Tan <hbut_tan@163.com> > Date: Thu, 10 Jun 2021 22:01:18 +0800 > >> diff --git a/drivers/net/ipa/ipa_smp2p.c b/drivers/net/ipa/ipa_smp2p.c >> index 34b68dc43886..93270e50b6b3 100644 >> --- a/drivers/net/ipa/ipa_smp2p.c >> +++ b/drivers/net/ipa/ipa_smp2p.c >> @@ -177,11 +177,8 @@ static int ipa_smp2p_irq_init(struct ipa_smp2p *smp2p, const char *name, >> int ret; >> >> ret = platform_get_irq_byname(smp2p->ipa->pdev, name); >> - if (ret <= 0) { >> - dev_err(dev, "DT error %d getting \"%s\" IRQ property\n", >> - ret, name); >> + if (ret <= 0) > Applied, but this code still rejects an irq of zero which is a valid irq number. It rejects IRQ 0 intentionally. And if 0 is returned, there will now be no message printed by the platform code. As I recall, I looked for a *long* time to see whether IRQ 0 was a valid IRQ number in Linux. One reason I even questioned it is that NO_IRQ is defined with value 0 on some architectures (though not for Arm). I even asked Rob Herring about privately it a few years back and he suggested I shouldn't allow 0. Yes, it *looked* like IRQ 0 could be a valid return. But I decided it was safer to just reject it, on the assumption that it's unlikely to be returned (I don't believe it is or ever will be used as the IRQ for SMP2P). If you are certain it's valid, and should be allowed, I have no objection to changing that "<=" to be "<". -Alex PS A quick search found this oldie: https://yarchive.net/comp/linux/no_irq.html
On Thu, 10 Jun 2021 16:38:43 -0500 Alex Elder <elder@ieee.org> wrote: > On 6/10/21 4:11 PM, David Miller wrote: > > From: Zhongjun Tan <hbut_tan@163.com> > > Date: Thu, 10 Jun 2021 22:01:18 +0800 > > > >> diff --git a/drivers/net/ipa/ipa_smp2p.c > >> b/drivers/net/ipa/ipa_smp2p.c index 34b68dc43886..93270e50b6b3 > >> 100644 --- a/drivers/net/ipa/ipa_smp2p.c > >> +++ b/drivers/net/ipa/ipa_smp2p.c > >> @@ -177,11 +177,8 @@ static int ipa_smp2p_irq_init(struct > >> ipa_smp2p *smp2p, const char *name, int ret; > >> > >> ret = platform_get_irq_byname(smp2p->ipa->pdev, name); > >> - if (ret <= 0) { > >> - dev_err(dev, "DT error %d getting \"%s\" IRQ > >> property\n", > >> - ret, name); > >> + if (ret <= 0) > > Applied, but this code still rejects an irq of zero which is a > > valid irq number. > > It rejects IRQ 0 intentionally. And if 0 is returned, there > will now be no message printed by the platform code. > > As I recall, I looked for a *long* time to see whether IRQ 0 > was a valid IRQ number in Linux. One reason I even questioned > it is that NO_IRQ is defined with value 0 on some architectures > (though not for Arm). I even asked Rob Herring about privately > it a few years back and he suggested I shouldn't allow 0. > > Yes, it *looked* like IRQ 0 could be a valid return. But I > decided it was safer to just reject it, on the assumption > that it's unlikely to be returned (I don't believe it is > or ever will be used as the IRQ for SMP2P). > > If you are certain it's valid, and should be allowed, I > have no objection to changing that "<=" to be "<". > > -Alex > > PS A quick search found this oldie: > https://yarchive.net/comp/linux/no_irq.html I think so , It is better to change "<=" to be "<".
diff --git a/drivers/net/ipa/ipa_smp2p.c b/drivers/net/ipa/ipa_smp2p.c index 34b68dc43886..93270e50b6b3 100644 --- a/drivers/net/ipa/ipa_smp2p.c +++ b/drivers/net/ipa/ipa_smp2p.c @@ -177,11 +177,8 @@ static int ipa_smp2p_irq_init(struct ipa_smp2p *smp2p, const char *name, int ret; ret = platform_get_irq_byname(smp2p->ipa->pdev, name); - if (ret <= 0) { - dev_err(dev, "DT error %d getting \"%s\" IRQ property\n", - ret, name); + if (ret <= 0) return ret ? : -EINVAL; - } irq = ret; ret = request_threaded_irq(irq, NULL, handler, 0, name, smp2p);