* [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero [not found] ` <1260979809-24811-3-git-send-email-u.kleine-koenig@pengutronix.de> @ 2009-12-16 16:10 ` Uwe Kleine-König 2009-12-16 16:27 ` Wolfgang Grandegger 2009-12-16 18:58 ` Wolfgang Grandegger 0 siblings, 2 replies; 8+ messages in thread From: Uwe Kleine-König @ 2009-12-16 16:10 UTC (permalink / raw) To: linux-kernel Cc: David Vrabel, Greg Kroah-Hartman, Urs Thuermann, Oliver Hartkopp, David S. Miller, Wolfgang Grandegger, Kurt Van Dijck, netdev platform_get_irq returns -ENXIO on failure, so !irq was probably always true. Better use (int)irq <= 0. Note that a return value of zero is still handled as error even though this could mean irq0. This is a followup to 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 that changed the return value of platform_get_irq from 0 to -ENXIO on error. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Cc: David Vrabel <dvrabel@arcom.com> Cc: Greg Kroah-Hartman <gregkh@suse.de> Cc: Urs Thuermann <urs.thuermann@volkswagen.de> Cc: Oliver Hartkopp <oliver.hartkopp@volkswagen.de> Cc: David S. Miller <davem@davemloft.net> Cc: Wolfgang Grandegger <wg@grandegger.com> Cc: Kurt Van Dijck <kurt.van.dijck@eia.be> Cc: netdev@vger.kernel.org --- drivers/net/can/at91_can.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c index cbe3fce..631d404 100644 --- a/drivers/net/can/at91_can.c +++ b/drivers/net/can/at91_can.c @@ -1037,7 +1037,7 @@ static int __init at91_can_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); irq = platform_get_irq(pdev, 0); - if (!res || !irq) { + if (!res || irq <= 0) { err = -ENODEV; goto exit_put; } -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero 2009-12-16 16:10 ` [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero Uwe Kleine-König @ 2009-12-16 16:27 ` Wolfgang Grandegger 2009-12-16 17:08 ` Uwe Kleine-König 2009-12-16 18:58 ` Wolfgang Grandegger 1 sibling, 1 reply; 8+ messages in thread From: Wolfgang Grandegger @ 2009-12-16 16:27 UTC (permalink / raw) To: Uwe Kleine-König Cc: linux-kernel, David Vrabel, Greg Kroah-Hartman, Urs Thuermann, Oliver Hartkopp, David S. Miller, Kurt Van Dijck, netdev Uwe Kleine-König wrote: > platform_get_irq returns -ENXIO on failure, so !irq was probably > always true. Better use (int)irq <= 0. Note that a return value of > zero is still handled as error even though this could mean irq0. But only on ARM, which is the only platform still using the infamous NO_IRQ (=-1). As this is a driver for ARM hardware, using irq == NO_IRQ would make sense, though. Wolfgang. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero 2009-12-16 16:27 ` Wolfgang Grandegger @ 2009-12-16 17:08 ` Uwe Kleine-König 2009-12-16 17:47 ` Alan Cox 2009-12-16 18:57 ` Wolfgang Grandegger 0 siblings, 2 replies; 8+ messages in thread From: Uwe Kleine-König @ 2009-12-16 17:08 UTC (permalink / raw) To: Wolfgang Grandegger Cc: linux-kernel, David Vrabel, Greg Kroah-Hartman, Urs Thuermann, Oliver Hartkopp, David S. Miller, Kurt Van Dijck, netdev On Wed, Dec 16, 2009 at 05:27:03PM +0100, Wolfgang Grandegger wrote: > Uwe Kleine-König wrote: > > platform_get_irq returns -ENXIO on failure, so !irq was probably > > always true. Better use (int)irq <= 0. Note that a return value of > > zero is still handled as error even though this could mean irq0. > > But only on ARM, which is the only platform still using the infamous > NO_IRQ (=-1). As this is a driver for ARM hardware, using irq == NO_IRQ > would make sense, though. This has nothing to do with NO_IRQ. You could do: - if (!res || !irq) { + if (!res || irq <= (int)NO_IRQ) { but this looks too ugly. (IMHO using NO_IRQ is already ugly.) Still, before my patch platform_get_irq return 0 was an error and if this should be handled as irq0 this is a separate issue that should be fixed in a separate patch. Best regards Uwe PS: linux-2.6$ git grep -E 'define *NO_IRQ\>' arch/*/include arch/arm/include/asm/irq.h:#define NO_IRQ ((unsigned int)(-1)) arch/microblaze/include/asm/irq.h:#define NO_IRQ (-1) arch/mn10300/include/asm/irq.h:#define NO_IRQ INT_MAX arch/parisc/include/asm/irq.h:#define NO_IRQ (-1) arch/powerpc/include/asm/irq.h:#define NO_IRQ (0) -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero 2009-12-16 17:08 ` Uwe Kleine-König @ 2009-12-16 17:47 ` Alan Cox 2009-12-17 7:17 ` Wolfgang Grandegger 2009-12-16 18:57 ` Wolfgang Grandegger 1 sibling, 1 reply; 8+ messages in thread From: Alan Cox @ 2009-12-16 17:47 UTC (permalink / raw) To: Uwe Kleine-König Cc: Wolfgang Grandegger, linux-kernel, David Vrabel, Greg Kroah-Hartman, Urs Thuermann, Oliver Hartkopp, David S. Miller, Kurt Van Dijck, netdev > + if (!res || irq <= (int)NO_IRQ) { > > but this looks too ugly. (IMHO using NO_IRQ is already ugly.) No IRQ was a private variable for the drivers/ide stack internally and only present in any form on a few odd platforms where it got "borrowed" and hasn't yet been eliminated The absence of an IRQ is zero. A bus IRQ of zero is remapped by the OS. Alan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero 2009-12-16 17:47 ` Alan Cox @ 2009-12-17 7:17 ` Wolfgang Grandegger 0 siblings, 0 replies; 8+ messages in thread From: Wolfgang Grandegger @ 2009-12-17 7:17 UTC (permalink / raw) To: Alan Cox Cc: Uwe Kleine-König, linux-kernel, David Vrabel, Greg Kroah-Hartman, Urs Thuermann, Oliver Hartkopp, David S. Miller, Kurt Van Dijck, netdev Alan Cox wrote: >> + if (!res || irq <= (int)NO_IRQ) { >> >> but this looks too ugly. (IMHO using NO_IRQ is already ugly.) > > No IRQ was a private variable for the drivers/ide stack internally and > only present in any form on a few odd platforms where it got "borrowed" > and hasn't yet been eliminated > > The absence of an IRQ is zero. A bus IRQ of zero is remapped by the OS. OK, I was remembering some old issues with NO_IRQ. Next time I will read the commit message more carefully. Sorry for the noise. Wolfgang. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero 2009-12-16 17:08 ` Uwe Kleine-König 2009-12-16 17:47 ` Alan Cox @ 2009-12-16 18:57 ` Wolfgang Grandegger 1 sibling, 0 replies; 8+ messages in thread From: Wolfgang Grandegger @ 2009-12-16 18:57 UTC (permalink / raw) To: Uwe Kleine-König Cc: linux-kernel, David Vrabel, Greg Kroah-Hartman, Urs Thuermann, Oliver Hartkopp, David S. Miller, Kurt Van Dijck, netdev Uwe Kleine-König wrote: > On Wed, Dec 16, 2009 at 05:27:03PM +0100, Wolfgang Grandegger wrote: >> Uwe Kleine-König wrote: >>> platform_get_irq returns -ENXIO on failure, so !irq was probably >>> always true. Better use (int)irq <= 0. Note that a return value of >>> zero is still handled as error even though this could mean irq0. >> But only on ARM, which is the only platform still using the infamous >> NO_IRQ (=-1). As this is a driver for ARM hardware, using irq == NO_IRQ >> would make sense, though. > > This has nothing to do with NO_IRQ. You could do: Right, sorry for the noise. > - if (!res || !irq) { > + if (!res || irq <= (int)NO_IRQ) { > > but this looks too ugly. (IMHO using NO_IRQ is already ugly.) > > Still, before my patch platform_get_irq return 0 was an error and if > this should be handled as irq0 this is a separate issue that should be > fixed in a separate patch. "irq <= 0" seems then the best solution. Will add my signed-off-by to the patch. Thanks, Wolfgang. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero 2009-12-16 16:10 ` [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero Uwe Kleine-König 2009-12-16 16:27 ` Wolfgang Grandegger @ 2009-12-16 18:58 ` Wolfgang Grandegger 2009-12-19 4:32 ` David Miller 1 sibling, 1 reply; 8+ messages in thread From: Wolfgang Grandegger @ 2009-12-16 18:58 UTC (permalink / raw) To: Uwe Kleine-König Cc: linux-kernel, David Vrabel, Greg Kroah-Hartman, Urs Thuermann, Oliver Hartkopp, David S. Miller, Kurt Van Dijck, netdev Uwe Kleine-König wrote: > platform_get_irq returns -ENXIO on failure, so !irq was probably > always true. Better use (int)irq <= 0. Note that a return value of > zero is still handled as error even though this could mean irq0. > > This is a followup to 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 that > changed the return value of platform_get_irq from 0 to -ENXIO on error. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Cc: David Vrabel <dvrabel@arcom.com> > Cc: Greg Kroah-Hartman <gregkh@suse.de> > Cc: Urs Thuermann <urs.thuermann@volkswagen.de> > Cc: Oliver Hartkopp <oliver.hartkopp@volkswagen.de> > Cc: David S. Miller <davem@davemloft.net> > Cc: Wolfgang Grandegger <wg@grandegger.com> > Cc: Kurt Van Dijck <kurt.van.dijck@eia.be> > Cc: netdev@vger.kernel.org Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> > --- > drivers/net/can/at91_can.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c > index cbe3fce..631d404 100644 > --- a/drivers/net/can/at91_can.c > +++ b/drivers/net/can/at91_can.c > @@ -1037,7 +1037,7 @@ static int __init at91_can_probe(struct platform_device *pdev) > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > irq = platform_get_irq(pdev, 0); > - if (!res || !irq) { > + if (!res || irq <= 0) { > err = -ENODEV; > goto exit_put; > } Thanks, Wolfgang. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero 2009-12-16 18:58 ` Wolfgang Grandegger @ 2009-12-19 4:32 ` David Miller 0 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2009-12-19 4:32 UTC (permalink / raw) To: wg Cc: u.kleine-koenig, linux-kernel, dvrabel, gregkh, urs.thuermann, oliver.hartkopp, kurt.van.dijck, netdev From: Wolfgang Grandegger <wg@grandegger.com> Date: Wed, 16 Dec 2009 19:58:38 +0100 > Uwe Kleine-König wrote: >> platform_get_irq returns -ENXIO on failure, so !irq was probably >> always true. Better use (int)irq <= 0. Note that a return value of >> zero is still handled as error even though this could mean irq0. >> >> This is a followup to 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 that >> changed the return value of platform_get_irq from 0 to -ENXIO on error. >> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> Applied, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-12-19 4:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1260979809-24811-1-git-send-email-u.kleine-koenig@pengutronix.de>
[not found] ` <1260979809-24811-2-git-send-email-u.kleine-koenig@pengutronix.de>
[not found] ` <1260979809-24811-3-git-send-email-u.kleine-koenig@pengutronix.de>
2009-12-16 16:10 ` [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero Uwe Kleine-König
2009-12-16 16:27 ` Wolfgang Grandegger
2009-12-16 17:08 ` Uwe Kleine-König
2009-12-16 17:47 ` Alan Cox
2009-12-17 7:17 ` Wolfgang Grandegger
2009-12-16 18:57 ` Wolfgang Grandegger
2009-12-16 18:58 ` Wolfgang Grandegger
2009-12-19 4:32 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).