From: Arvind Yadav <arvind.yadav...@gmail.com> Date: Tue, 5 Dec 2017 11:04:55 +0530
> Hi David, > > > On Monday 04 December 2017 11:55 PM, David Miller wrote: >> From: Arvind Yadav <arvind.yadav...@gmail.com> >> Date: Mon, 4 Dec 2017 23:18:20 +0530 >> >>> @@ -120,9 +120,10 @@ static int sni_82596_probe(struct platform_device >>> *dev) >>> netdevice->dev_addr[5] = readb(eth_addr + 0x06); >>> iounmap(eth_addr); >>> - if (!netdevice->irq) { >>> + if (netdevice->irq <= 0) { >>> printk(KERN_ERR "%s: IRQ not found for i82596 at 0x%lx\n", >>> __FILE__, netdevice->base_addr); >>> + retval = netdevice->irq ? netdevice->irq : -ENODEV; >>> goto probe_failed; >>> } >> Ok, thinking about this some more... >> >> It is impossible to use platform_get_irq() without every single call >> site having this funny: >> >> ret = val ? val : -ENODEV; >> >> sequence. >> >> This is unnecessary duplication and it is also error prone, so I >> really think this logic belongs in platform_get_irq() itself. It can >> convert '0' to -ENODEV and that way we need no special logic in the >> callers at all. > platform_get_irq() will return 0 only for sparc, If sparc initialize > platform > data irq[PROMINTR_MAX] as zero. Otherwise platform_get_irq() will > never return > 0. It will return either IRQ number or error (as negative number). But > I am getting > review comment by reviewer/maintainer in other subsystem to add check > for > zero. So I have done same changes here. Please correct me if i am > wrong. If you make the change that I suggest, you instead can check for '-ENODEV' to mean no IRQ.