Re: [alsa-devel] [PATCH 1/2] ASoC: nuc900: Fix platform_get_irq() error checking some more

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,


On Monday 11 December 2017 02:10 PM, Dan Carpenter wrote:
On Sun, Dec 10, 2017 at 08:10:26AM +0530, arvindY wrote:

On Sunday 10 December 2017 07:22 AM, Dan Carpenter wrote:
On Sat, Dec 09, 2017 at 06:27:32PM +0100, Alexandre Belloni wrote:
diff --git a/sound/soc/nuc900/nuc900-ac97.c b/sound/soc/nuc900/nuc900-ac97.c
index 5e4fbd2d3479..71fce7c85c93 100644
--- a/sound/soc/nuc900/nuc900-ac97.c
+++ b/sound/soc/nuc900/nuc900-ac97.c
@@ -345,11 +345,10 @@ static int nuc900_ac97_drvprobe(struct platform_device *pdev)
    		goto out;
    	}
-	nuc900_audio->irq_num = platform_get_irq(pdev, 0);
-	if (nuc900_audio->irq_num <= 0) {
-		ret = nuc900_audio->irq_num < 0 ? nuc900_audio->irq_num : -EBUSY;
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0)
The <= 0 was ok, see:
https://lkml.org/lkml/2017/11/18/41

Yeah, but is it ever going to return 0?  That seems like a design error
and also really crap commenting if so
yes, It can return 0 on sprac platform and If you see the return of
platform_get_irq() 'return r ? r->start : -ENXIO;'. It should be
'return r && r->start? r->start : -ENXIO;'. We can not add checks here,
Because There's a bunch of platforms in the kernel they still use IRQ0 as
valid.
I have separate mails where few maintainer ask me to add check for 0 and few
not.
Adding check for 0 will never harm.
What you're saying doesn't make sense.
I am following a below link. Where they have point out irq 0 is not valid.
https://lwn.net/Articles/470820/

You *can't* treat 0 as an error on Sparc because that's a valid IRQ.  In
fact, it seems like if you want to write portable code you should never
treated zero as an error.

It doesn't make sense that someone would register an IRQ resource with
an invalid IRQ number.  In other words, r->start is never going to be
zero on a platform where that's invalid.

So I'm pretty sure "if (ret < 0) " is the correct way to write code and
"if (ret <= 0) " is incorrect but generally harmless except perhaps in
limited situations on SPARC or other similar arches.

regards,
dan carpenter

~arvind
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux