[PATCH 1/7] [AVR32] don't check platform_get_irq's return value against zero

[PATCH 1/7] [AVR32] don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

platform_get_irq returns -ENXIO on failure, so !int_irq was probably
always true.  Better use (int)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: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 arch/avr32/mach-at32ap/extint.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/avr32/mach-at32ap/extint.c b/arch/avr32/mach-at32ap/extint.c
index 310477b..033f693 100644
--- a/arch/avr32/mach-at32ap/extint.c
+++ b/arch/avr32/mach-at32ap/extint.c
@@ -198,7 +198,7 @@ static int __init eic_probe(struct platform_device *pdev)
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	int_irq = platform_get_irq(pdev, 0);
-	if (!regs || !int_irq) {
+	if (!regs || (int)int_irq <= 0) {
 		dev_dbg(&pdev->dev, "missing regs and/or irq resource\n");
 		return -ENXIO;
 	}
-- 
1.6.5.2

[PATCH 2/7] V4L/DVB mx1_camera: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

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: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Antonio Ospite <ospite@studenti.unina.it>
Cc: Paulius Zaleckas <paulius.zaleckas@teltonika.lt>
Cc: linux-media@vger.kernel.org
---
 drivers/media/video/mx1_camera.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/mx1_camera.c b/drivers/media/video/mx1_camera.c
index 7280229..f7a472f 100644
--- a/drivers/media/video/mx1_camera.c
+++ b/drivers/media/video/mx1_camera.c
@@ -650,7 +650,7 @@ static int __init mx1_camera_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
-	if (!res || !irq) {
+	if (!res || (int)irq <= 0) {
 		err = -ENODEV;
 		goto exit;
 	}
-- 
1.6.5.2

[PATCH 3/7] V4L/DVB sh_mobile_ceu: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

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: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <damm@igel.co.jp>
Cc: Kuninori Morimoto <morimoto.kuninori@renesas.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: linux-media@vger.kernel.org
---
 drivers/media/video/sh_mobile_ceu_camera.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 961e448..f18e674 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -1709,7 +1709,7 @@ static int __devinit sh_mobile_ceu_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
-	if (!res || !irq) {
+	if (!res || (int)irq <= 0) {
 		dev_err(&pdev->dev, "Not enough CEU platform resources.\n");
 		err = -ENODEV;
 		goto exit;
-- 
1.6.5.2

[PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

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

[PATCH 5/7] pcmcia/bfin_cf: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

platform_get_irq returns -ENXIO on failure, so !irq was probably
always true.  Better use 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: Rafael J. Wysocki <rjw@sisk.pl>
Cc: linux-pcmcia@lists.infradead.org
---
 drivers/pcmcia/bfin_cf_pcmcia.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/pcmcia/bfin_cf_pcmcia.c b/drivers/pcmcia/bfin_cf_pcmcia.c
index 300b368..2482ce7 100644
--- a/drivers/pcmcia/bfin_cf_pcmcia.c
+++ b/drivers/pcmcia/bfin_cf_pcmcia.c
@@ -205,7 +205,7 @@ static int __devinit bfin_cf_probe(struct platform_device *pdev)
 	dev_info(&pdev->dev, "Blackfin CompactFlash/PCMCIA Socket Driver\n");
 
 	irq = platform_get_irq(pdev, 0);
-	if (!irq)
+	if (irq <= 0)
 		return -EINVAL;
 
 	cd_pfx = platform_get_irq(pdev, 1);	/*Card Detect GPIO PIN */
-- 
1.6.5.2

[PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

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: David Brownell <dbrownell@users.sourceforge.net>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Anton Vorontsov <avorontsov@ru.mvista.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: spi-devel-general@lists.sourceforge.net
---
 drivers/spi/spi_mpc8xxx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi_mpc8xxx.c b/drivers/spi/spi_mpc8xxx.c
index e9390d7..b13501a 100644
--- a/drivers/spi/spi_mpc8xxx.c
+++ b/drivers/spi/spi_mpc8xxx.c
@@ -1339,7 +1339,7 @@ static int __devinit plat_mpc8xxx_spi_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	irq = platform_get_irq(pdev, 0);
-	if (!irq)
+	if ((int)irq <= 0)
 		return -EINVAL;
 
 	master = mpc8xxx_spi_probe(&pdev->dev, mem, irq);
-- 
1.6.5.2

[PATCH 7/7] ASoC: sh: FSI:: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

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: Liam Girdwood <lrg@slimlogic.co.uk>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Kuninori Morimoto <morimoto.kuninori@renesas.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: alsa-devel@alsa-project.org
---
 sound/soc/sh/fsi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 9c49c11..42813b8 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -876,7 +876,7 @@ static int fsi_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
-	if (!res || !irq) {
+	if (!res || (int)irq <= 0) {
 		dev_err(&pdev->dev, "Not enough FSI platform resources.\n");
 		ret = -ENODEV;
 		goto exit;
-- 
1.6.5.2

Re: [PATCH 7/7] ASoC: sh: FSI:: don't check platform_get_irq's return value against zero

[Kuninori Morimoto]

Dear Uwe

> 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.
(snip)
> -	if (!res || !irq) {
> +	if (!res || (int)irq <= 0) {

Ohh..
Thank you for checking.

Hmm..  now I tried to check about platform_get_irq in Linux kernel.
In my easy check, I can find a lot of drivers which are...

o doesn't check irq value          ex) request_irq(platform_get_irq(...))
o checked irq but it have miss (?) ex) if (irq >= 0) OK
                                       if (irq) OK
                                       if (irq == -Exxx ) NG
o checked irq but don't care zero  ex) if (irq < 0) NG
o used above style                 ex) if (!irq) NG

Should we modify their too ?
How about create new macro/function to check it ?

Below came from 2.6.32-rc6

-- doesn't check irq ---------------

 arch/arm/plat-omap/iommu.c :: 945
 arch/avr32/mach-at32ap/pio.c :: 392
 arch/sh/drivers/push-switch.c :: 55
 drivers/ata/pata_bf54x.c :: 1627
 drivers/ata/pata_octeon_cf.c :: 905
 drivers/ata/sata_mv.c :: 4063
 drivers/dma/at_hdmac.c :: 1135
 drivers/dma/dw_dmac.c :: 1379
 drivers/edac/mv64x60_edac.c :: 177
 drivers/edac/mv64x60_edac.c :: 344
 drivers/edac/mv64x60_edac.c :: 539
 drivers/edac/mv64x60_edac.c :: 784
 drivers/i2c/busses/i2c-pca-platform.c :: 141
 drivers/i2c/busses/i2c-sh7760.c :: 474
 drivers/i2c/busses/i2c-stu300.c :: 917
 drivers/input/keyboard/pxa930_rotary.c :: 180
 drivers/input/keyboard/sh_keysc.c :: 254
 drivers/input/keyboard/sh_keysc.c :: 270
 drivers/input/keyboard/sh_keysc.c :: 290
 drivers/input/keyboard/twl4030_keypad.c :: 362
 drivers/input/misc/twl4030-pwrbutton.c :: 68
 drivers/input/misc/twl4030-pwrbutton.c :: 111
 drivers/input/misc/wm831x-on.c :: 75
 drivers/input/misc/wm831x-on.c :: 128
 drivers/input/mouse/pxa930_trkball.c :: 236
 drivers/input/touchscreen/w90p910_ts.c :: 279
 drivers/mmc/host/at91_mci.c :: 1082
 drivers/mmc/host/atmel-mci.c :: 1732
 drivers/mmc/host/sdhci-pltfm.c :: 80
 drivers/mtd/nand/mxc_nand.c :: 935
 drivers/mtd/nand/tmio_nand.c :: 380
 drivers/mtd/onenand/generic.c :: 67
 drivers/net/dnet.c :: 847
 drivers/net/fec.c :: 1890
 drivers/net/jazzsonic.c :: 237
 drivers/net/macb.c :: 1183
 drivers/net/ne.c :: 808
 drivers/net/s6gmac.c :: 975
 drivers/net/smc911x.c :: 2088
 drivers/pcmcia/bfin_cf_pcmcia.c :: 211
 drivers/regulator/wm831x-isink.c :: 197
 drivers/regulator/wm831x-isink.c :: 223
 drivers/rtc/rtc-cmos.c :: 1147
 drivers/rtc/rtc-coh901331.c :: 200
 drivers/rtc/rtc-stmp3xxx.c :: 201
 drivers/rtc/rtc-stmp3xxx.c :: 202
 drivers/rtc/rtc-twl4030.c :: 471
 drivers/scsi/jazz_esp.c :: 172
 drivers/scsi/sni_53c710.c :: 100
 drivers/scsi/sun3x_esp.c :: 235
 drivers/serial/mpsc.c :: 2058
 drivers/serial/sc26xx.c :: 654
 drivers/usb/gadget/at91_udc.c :: 1733
 drivers/usb/gadget/m66592-udc.c :: 1553
 drivers/usb/gadget/pxa25x_udc.c :: 2326
 drivers/usb/gadget/r8a66597-udc.c :: 1502
 drivers/usb/host/ohci-tmio.c :: 192
 drivers/usb/otg/twl4030-usb.c :: 672
 drivers/video/pxafb.c :: 2234
 drivers/video/s3c2410fb.c :: 1041
 drivers/video/tmiofb.c :: 691
 drivers/video/tmiofb.c :: 816
 drivers/watchdog/coh901327_wdt.c :: 431
 sound/drivers/ml403-ac97cr.c :: 1154
 sound/drivers/ml403-ac97cr.c :: 1167

---- checked irq but it have miss (?) ---------

 drivers/ata/pata_ixp4xx_cf.c :: 168
 drivers/block/mg_disk.c :: 926
 drivers/crypto/mv_cesa.c :: 517
 drivers/dma/at_hdmac.c :: 1110
 drivers/dma/txx9dmac.c :: 1256
 drivers/gpio/vr41xx_giu.c :: 548
 drivers/i2c/busses/i2c-highlander.c :: 385
 drivers/i2c/busses/i2c-pmcmsp.c :: 306
 drivers/i2c/busses/i2c-pxa.c :: 1006
 drivers/input/keyboard/omap-keypad.c :: 399
 drivers/mfd/t7l66xb.c :: 313
 drivers/mfd/tc6387xb.c :: 123
 drivers/mfd/tc6393xb.c :: 588
 drivers/misc/atmel_tclib.c :: 114
 drivers/misc/atmel_tclib.c :: 139
 drivers/misc/atmel_tclib.c :: 142
 drivers/mmc/host/s3cmci.c :: 1630
 drivers/mmc/host/tmio_mmc.c :: 585
 drivers/rtc/rtc-mxc.c :: 446
 drivers/rtc/rtc-sh.c :: 631
 drivers/rtc/rtc-sh.c :: 632
 drivers/serial/imx.c :: 1250
 drivers/serial/imx.c :: 1251
 drivers/serial/imx.c :: 1252
 drivers/serial/imx.c :: 1253
 drivers/serial/vr41xx_siu.c :: 721
 drivers/usb/musb/musb_core.c :: 2123
 drivers/usb/musb/musbhsdma.c :: 358
 drivers/video/sh7760fb.c :: 476

---- checked irq but don't care zero ----------

 arch/arm/common/locomo.c :: 795
 arch/arm/common/sa1111.c :: 950
 arch/arm/mach-omap2/mailbox.c :: 308
 arch/arm/mach-pxa/ssp.c :: 388
 arch/arm/plat-omap/iommu.c :: 897
 arch/sh/drivers/push-switch.c :: 103
 drivers/ata/pata_at32.c :: 287
 drivers/clocksource/sh_cmt.c :: 588
 drivers/clocksource/sh_mtu2.c :: 267
 drivers/clocksource/sh_tmu.c :: 372
 drivers/dma/at_hdmac.c :: 1001
 drivers/dma/dw_dmac.c :: 1258
 drivers/dma/iop-adma.c :: 1560
 drivers/dma/ipu/ipu_idmac.c :: 1758
 drivers/dma/ipu/ipu_idmac.c :: 1763
 drivers/dma/mv_xor.c :: 1203
 drivers/dma/txx9dmac.c :: 1171
 drivers/i2c/busses/i2c-bfin-twi.c :: 662
 drivers/i2c/busses/i2c-imx.c :: 484
 drivers/i2c/busses/i2c-iop3xx.c :: 469
 drivers/i2c/busses/i2c-mv64xxx.c :: 523
 drivers/i2c/busses/i2c-s6000.c :: 267
 drivers/ide/au1xxx-ide.c :: 520
 drivers/ide/tx4938ide.c :: 141
 drivers/ide/tx4939ide.c :: 546
 drivers/input/keyboard/bf54x-keys.c :: 245
 drivers/input/keyboard/ep93xx_keypad.c :: 277
 drivers/input/keyboard/opencores-kbd.c :: 52
 drivers/input/keyboard/pxa27x_keypad.c :: 460
 drivers/input/keyboard/pxa930_rotary.c :: 93
 drivers/input/keyboard/sh_keysc.c :: 150
 drivers/input/keyboard/w90p910_keypad.c :: 137
 drivers/input/misc/bfin_rotary.c :: 126
 drivers/input/misc/dm355evm_keys.c :: 230
 drivers/input/mouse/pxa930_trkball.c :: 153
 drivers/input/serio/at32psif.c :: 258
 drivers/input/touchscreen/atmel_tsadcc.c :: 200
 drivers/input/touchscreen/corgi_ts.c :: 284
 drivers/media/video/pxa_camera.c :: 1635
 drivers/mfd/asic3.c :: 380
 drivers/mfd/asic3.c :: 785
 drivers/mfd/sm501.c :: 1417
 drivers/misc/atmel_pwm.c :: 308
 drivers/mmc/host/atmel-mci.c :: 1612
 drivers/mmc/host/imxmmc.c :: 947
 drivers/mmc/host/mvsdio.c :: 712
 drivers/mmc/host/mxcmmc.c :: 688
 drivers/mmc/host/omap.c :: 1413
 drivers/mmc/host/omap_hsmmc.c :: 1632
 drivers/mmc/host/pxamci.c :: 555
 drivers/mmc/host/sdhci-s3c.c :: 231
 drivers/mtd/nand/pxa3xx_nand.c :: 1215
 drivers/net/arm/am79c961a.c :: 709
 drivers/net/arm/w90p910_ether.c :: 1013
 drivers/net/arm/w90p910_ether.c :: 1020
 drivers/net/fec.c :: 1884
 drivers/net/ks8842.c :: 649
 drivers/net/ks8851_mll.c :: 1564
 drivers/net/s6gmac.c :: 1012
 drivers/net/sh_eth.c :: 1410
 drivers/pcmcia/omap_cf.c :: 217
 drivers/rtc/rtc-pxa.c :: 373
 drivers/rtc/rtc-pxa.c :: 378
 drivers/rtc/rtc-s3c.c :: 412
 drivers/rtc/rtc-s3c.c :: 418
 drivers/rtc/rtc-tx4939.c :: 246
 drivers/serial/msm_serial.c :: 714
 drivers/serial/samsung.c :: 1100
 drivers/serial/timbuart.c :: 456
 drivers/spi/atmel_spi.c :: 753
 drivers/spi/spi_bfin5xx.c :: 1304
 drivers/spi/spi_s3c24xx.c :: 370
 drivers/spi/spi_stmp.c :: 500
 drivers/spi/spi_stmp.c :: 505
 drivers/spi/spi_txx9.c :: 393
 drivers/uio/uio_smx.c :: 77
 drivers/usb/gadget/atmel_usba_udc.c :: 1897
 drivers/usb/gadget/imx_udc.c :: 1461
 drivers/usb/gadget/pxa25x_udc.c :: 2176
 drivers/usb/gadget/pxa27x_udc.c :: 2414
 drivers/usb/gadget/s3c-hsotg.c :: 3145
 drivers/usb/host/ehci-w90x900.c :: 77
 drivers/usb/host/isp1362-hcd.c :: 2727
 drivers/usb/host/ohci-omap.c :: 361
 drivers/usb/host/ohci-pnx4008.c :: 389
 drivers/usb/host/ohci-pxa27x.c :: 297
 drivers/usb/host/ohci-sh.c :: 97
 drivers/usb/host/ohci-sm501.c :: 95
 drivers/video/atmel_lcdfb.c :: 864
 drivers/video/bf54x-lq043fb.c :: 657
 drivers/video/bfin-t350mcqb-fb.c :: 551
 drivers/video/da8xx-fb.c :: 758
 drivers/video/msm/mdp.c :: 402
 drivers/video/pxa168fb.c :: 629
 drivers/video/pxafb.c :: 2129
 drivers/video/s3c2410fb.c :: 846
 drivers/video/sa1100fb.c :: 1443
 drivers/video/sh_mobile_lcdcfb.c :: 904
 drivers/video/sm501fb.c :: 1326
 drivers/w1/masters/omap_hdq.c :: 630
 drivers/watchdog/mpcore_wdt.c :: 348
 sound/atmel/abdac.c :: 405
 sound/atmel/ac97c.c :: 772
 sound/soc/s6000/s6000-i2s.c :: 540
 sound/soc/txx9/txx9aclc-ac97.c :: 193

---- it use !irq  ------------------------

 arch/avr32/mach-at32ap/extint.c :: 200
 drivers/media/video/mx1_camera.c :: 651
 drivers/media/video/sh_mobile_ceu_camera.c :: 1655
 drivers/misc/atmel-ssc.c :: 110
 drivers/net/can/at91_can.c :: 1071
 drivers/net/sni_82596.c :: 112
 drivers/pcmcia/bfin_cf_pcmcia.c :: 207
 drivers/spi/spi_imx.c :: 553
 drivers/spi/spi_mpc8xxx.c :: 885
 drivers/usb/gadget/fsl_udc_core.c :: 2303



Best regards
--
Kuninori Morimoto
 

Re: [PATCH 7/7] ASoC: sh: FSI:: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

On Thu, Dec 17, 2009 at 10:42:25AM +0900, Kuninori Morimoto wrote:
> 
> Dear Uwe
> 
> > 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.
> (snip)
> > -	if (!res || !irq) {
> > +	if (!res || (int)irq <= 0) {
> 
> Ohh..
> Thank you for checking.
> 
> Hmm..  now I tried to check about platform_get_irq in Linux kernel.
> In my easy check, I can find a lot of drivers which are...
> 
> o doesn't check irq value          ex) request_irq(platform_get_irq(...))
This is probably worth fixing.

> o checked irq but it have miss (?) ex) if (irq >= 0) OK
>                                        if (irq) OK
>                                        if (irq == -Exxx ) NG
For some drivers if (irq >= 0) is OK.  if (irq) is wrong.  if (irq ==
-Exxx ) IMHO isn't carefully enough.

> o checked irq but don't care zero  ex) if (irq < 0) NG
Same as if (irq >= 0)

> ---- it use !irq  ------------------------
> 
>  arch/avr32/mach-at32ap/extint.c :: 200
>  drivers/media/video/mx1_camera.c :: 651
>  drivers/media/video/sh_mobile_ceu_camera.c :: 1655
>  drivers/misc/atmel-ssc.c :: 110
>  drivers/net/can/at91_can.c :: 1071
>  drivers/net/sni_82596.c :: 112
>  drivers/pcmcia/bfin_cf_pcmcia.c :: 207
>  drivers/spi/spi_imx.c :: 553
>  drivers/spi/spi_mpc8xxx.c :: 885
>  drivers/usb/gadget/fsl_udc_core.c :: 2303
Most of these are fixed in this series.  I seem to have missed

	drivers/misc/atmel-ssc.c
	drivers/net/sni_82596.c
	drivers/usb/gadget/fsl_udc_core.c

and I found sound/soc/sh/fsi.c and drivers/spi/spi_imx.c should already
be fixed in Linus' Tree.

I wait to see the result for the already existing patches.  If they are
taken I might look for the others, too.  If you want to prepare some
more patches, feel free to do it.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 7/7] ASoC: sh: FSI:: don't check platform_get_irq's return value against zero

[Mark Brown]

On Wed, Dec 16, 2009 at 05:10:09PM +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>

Applied, thanks.

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Anton Vorontsov]

On Wed, Dec 16, 2009 at 05:10:08PM +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>
> ---

Noooooo... :-(

Please revert 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 instead,
and fix platforms to remap HWIRQ0 to something that is not VIRQ0.

IRQ0 is invalid for everything that is outside of arch/*.

http://lkml.org/lkml/2005/11/22/159
http://lkml.org/lkml/2005/11/22/213
http://lkml.org/lkml/2005/11/22/227

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

Hello,

[Note the email address used for David Vrabel isn't valid any more,
this mail uses his last used address.]

On Wed, Dec 16, 2009 at 07:32:29PM +0300, Anton Vorontsov wrote:
> On Wed, Dec 16, 2009 at 05:10:08PM +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>
> > ---
> 
> Noooooo... :-(
> 
> Please revert 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 instead,
> and fix platforms to remap HWIRQ0 to something that is not VIRQ0.
> 
> IRQ0 is invalid for everything that is outside of arch/*.
> 
> http://lkml.org/lkml/2005/11/22/159
> http://lkml.org/lkml/2005/11/22/213
> http://lkml.org/lkml/2005/11/22/227
First note that my check is safe with both variants (e.g. it does the
right thing independent of the error being signaled by 0 or
-ESOMETHING.)

Then arch/arm/mach-pxa/devices.c has:

static struct resource pxa27x_resource_ssp3[] = {
	...
	[1] = {
		.start  = IRQ_SSP3,
		.end    = IRQ_SSP3,
		.flags  = IORESOURCE_IRQ,
	},
	...
}

with IRQ_SSP3 being zero (sometimes).  The driver is implemented in
arch/arm/mach-pxa/ssp.c and uses platform_get_irq.  So according to your
definition it's allowed (arch/* only).  Still this would break if you
revert 305b3228f9.

Actually I don't care much, but as platform_get_irq returns an int I
think it's fine for it to signal an error using a value < 0 as irqs are
not negative.

My position regarding irq0 is:  If a variable holds either a valid irq
or a value indicating "no irq", then feel free to use 0 as "no irq" and
a value > 0 for a valid irq (without offset).  If you want irq0 here,
you're out of luck.  But if you have a variable holding a valid irq only
(that is, there is no doubt if the value is valid or not) I see no
reason to dogmatically prohibit irq0.

I'm a bit annoyed as this is the third time[1] this month this irq0
discussion pops up for me.  I think people see that irq0 is involved
somehow, start wailing and stop seeing the issues being fixed.

Best regards
Uwe

[1] one is:
	http://thread.gmane.org/gmane.linux.kernel/924739
    the other wasn't on lkml, only mm-commits.  Cannot find it on the
    net now.
-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Anton Vorontsov]

On Wed, Dec 16, 2009 at 06:49:04PM +0100, Uwe Kleine-König wrote:
[...]
> > Noooooo... :-(
> > 
> > Please revert 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 instead,
> > and fix platforms to remap HWIRQ0 to something that is not VIRQ0.
> > 
> > IRQ0 is invalid for everything that is outside of arch/*.
> > 
> > http://lkml.org/lkml/2005/11/22/159
> > http://lkml.org/lkml/2005/11/22/213
> > http://lkml.org/lkml/2005/11/22/227
> First note that my check is safe with both variants (e.g. it does the
> right thing independent of the error being signaled by 0 or
> -ESOMETHING.)
> 
> Then arch/arm/mach-pxa/devices.c has:
> 
> static struct resource pxa27x_resource_ssp3[] = {
> 	...
> 	[1] = {
> 		.start  = IRQ_SSP3,
> 		.end    = IRQ_SSP3,
> 		.flags  = IORESOURCE_IRQ,
> 	},
> 	...
> }
> 
> with IRQ_SSP3 being zero (sometimes).  The driver is implemented in
> arch/arm/mach-pxa/ssp.c and uses platform_get_irq.

So fix this *one* driver? Implement arm-specific platform_get_irq() as
a band-aid. Or better, implement virtual irqs <-> hardware irqs mapping
for ARM.

[...]
> I'm a bit annoyed as this is the third time[1] this month this irq0
> discussion pops up for me.  I think people see that irq0 is involved
> somehow, start wailing and stop seeing the issues being fixed.

For this particular driver, there is NO issue whatsoever. It is
only used for PowerPC, which has VIRQ0 == invalid IRQ. And note
that there still could be HWIRQ0 on PowerPC, but it is *never*
mapped to VIRQ0.

[...]
> [1] one is:
> 	http://thread.gmane.org/gmane.linux.kernel/924739

No wonder the discussion popped up. You're adding some ugly
#ifdef stuff that adds some arch-specific knowledge to a generic
code.

Sure, there's a lot of ugly code even in the kernel/ directly, but
you have to prepare for resistance when you add more of it.

So, if you want to fix the root cause of the issue: revert the
305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0, and try to improve the
ARM land, do not band-aid the whole kernel all over the place.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

Hi Anton,

On Wed, Dec 16, 2009 at 09:20:34PM +0300, Anton Vorontsov wrote:
> On Wed, Dec 16, 2009 at 06:49:04PM +0100, Uwe Kleine-König wrote:
> [...]
> > > Noooooo... :-(
> > > 
> > > Please revert 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 instead,
> > > and fix platforms to remap HWIRQ0 to something that is not VIRQ0.
> > > 
> > > IRQ0 is invalid for everything that is outside of arch/*.
> > > 
> > > http://lkml.org/lkml/2005/11/22/159
> > > http://lkml.org/lkml/2005/11/22/213
> > > http://lkml.org/lkml/2005/11/22/227
> > First note that my check is safe with both variants (e.g. it does the
> > right thing independent of the error being signaled by 0 or
> > -ESOMETHING.)
> > 
> > Then arch/arm/mach-pxa/devices.c has:
> > 
> > static struct resource pxa27x_resource_ssp3[] = {
> > 	...
> > 	[1] = {
> > 		.start  = IRQ_SSP3,
> > 		.end    = IRQ_SSP3,
> > 		.flags  = IORESOURCE_IRQ,
> > 	},
> > 	...
> > }
> > 
> > with IRQ_SSP3 being zero (sometimes).  The driver is implemented in
> > arch/arm/mach-pxa/ssp.c and uses platform_get_irq.
> 
> So fix this *one* driver? Implement arm-specific platform_get_irq() as
> a band-aid. Or better, implement virtual irqs <-> hardware irqs mapping
> for ARM.
> 
> [...]
> > I'm a bit annoyed as this is the third time[1] this month this irq0
> > discussion pops up for me.  I think people see that irq0 is involved
> > somehow, start wailing and stop seeing the issues being fixed.
> 
> For this particular driver, there is NO issue whatsoever. It is
> only used for PowerPC, which has VIRQ0 == invalid IRQ. And note
> that there still could be HWIRQ0 on PowerPC, but it is *never*
> mapped to VIRQ0.
Yes, there is an issue.  If the platform device doesn't have a resource
specifing the irq, platform_get_irq returns -ENXIO.  So in the driver
(unsigned)(-ENXIO) is passed to mpc8xxx_spi_probe as (!(-ENXIO)) is
false and so the error isn't catched.

> [...]
> > [1] one is:
> > 	http://thread.gmane.org/gmane.linux.kernel/924739
> 
> No wonder the discussion popped up. You're adding some ugly
> #ifdef stuff that adds some arch-specific knowledge to a generic
> code.
I wouldn't argue if people objected to the arch-specific #ifdef.  The
arch-specific code is already in there and I don't object to just
removing it.  But answering that irq0 must not be used isn't helpful.
 
Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Anton Vorontsov]

On Wed, Dec 16, 2009 at 08:18:39PM +0100, Uwe Kleine-König wrote:
[...]
> > > I'm a bit annoyed as this is the third time[1] this month this irq0
> > > discussion pops up for me.  I think people see that irq0 is involved
> > > somehow, start wailing and stop seeing the issues being fixed.
> > 
> > For this particular driver, there is NO issue whatsoever. It is
> > only used for PowerPC, which has VIRQ0 == invalid IRQ. And note
> > that there still could be HWIRQ0 on PowerPC, but it is *never*
> > mapped to VIRQ0.
> Yes, there is an issue.  If the platform device doesn't have a resource
> specifing the irq, platform_get_irq returns -ENXIO.  So in the driver
> (unsigned)(-ENXIO) is passed to mpc8xxx_spi_probe as (!(-ENXIO)) is
> false and so the error isn't catched.

If you look into how we create the platform device, you'll notice
that -ENXIO isn't possible.
It's in arch/powerpc/platforms/83xx/mpc832x_rdb.c:of_fsl_spi_probe(),
which is a legacy interface that I'd like to be removed anyway.

Though, if you want to fix the inconsistency in the platform device
API, then I'd suggest to fix the platform_get_irq(). The driver itself
is correct.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Anton Vorontsov]

On Wed, Dec 16, 2009 at 10:37:45PM +0300, Anton Vorontsov wrote:
[...]
> which is a legacy interface that I'd like to be removed anyway.

...which means I really don't care that much to nack or ack the
patch. Do whatever you feel the best, but you must realize that
what you're doing is just papering over the real problem, and
maybe even spreading the problem further.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

Hi Anton,

On Wed, Dec 16, 2009 at 10:37:45PM +0300, Anton Vorontsov wrote:
> On Wed, Dec 16, 2009 at 08:18:39PM +0100, Uwe Kleine-König wrote:
> [...]
> > > > I'm a bit annoyed as this is the third time[1] this month this irq0
> > > > discussion pops up for me.  I think people see that irq0 is involved
> > > > somehow, start wailing and stop seeing the issues being fixed.
> > > 
> > > For this particular driver, there is NO issue whatsoever. It is
> > > only used for PowerPC, which has VIRQ0 == invalid IRQ. And note
> > > that there still could be HWIRQ0 on PowerPC, but it is *never*
> > > mapped to VIRQ0.
> > Yes, there is an issue.  If the platform device doesn't have a resource
> > specifing the irq, platform_get_irq returns -ENXIO.  So in the driver
> > (unsigned)(-ENXIO) is passed to mpc8xxx_spi_probe as (!(-ENXIO)) is
> > false and so the error isn't catched.
> 
> If you look into how we create the platform device, you'll notice
> that -ENXIO isn't possible.
> It's in arch/powerpc/platforms/83xx/mpc832x_rdb.c:of_fsl_spi_probe(),
> which is a legacy interface that I'd like to be removed anyway.
> 
> Though, if you want to fix the inconsistency in the platform device
> API, then I'd suggest to fix the platform_get_irq(). The driver itself
> is correct.
With platform_get_irq as it is today, the check in

	irq = platform_get_irq(pdev, 0);
	if (!irq)
		return -EINVAL;

is non-sense as !irq always evaluates to 0.  If your argue that the
resources are right, then the logical consequence is to strip down
plat_mpc8xxx_spi_probe to just

	struct spi_master *master;
	master = mpc8xxx_spi_probe(&pdev->dev,
		platform_get_resource(pdev, IORESOURCE_MEM, 0)
		platform_get_irq(pdev, 0));
	if (IS_ERR(master))
		return PTR_ERR(master);
	return 0;

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Anton Vorontsov]

On Thu, Dec 17, 2009 at 02:05:49PM +0100, Uwe Kleine-König wrote:
[...]
> > > Yes, there is an issue.  If the platform device doesn't have a resource
> > > specifing the irq, platform_get_irq returns -ENXIO.  So in the driver
> > > (unsigned)(-ENXIO) is passed to mpc8xxx_spi_probe as (!(-ENXIO)) is
> > > false and so the error isn't catched.
> > 
> > If you look into how we create the platform device, you'll notice
> > that -ENXIO isn't possible.
> > It's in arch/powerpc/platforms/83xx/mpc832x_rdb.c:of_fsl_spi_probe(),
> > which is a legacy interface that I'd like to be removed anyway.
> > 
> > Though, if you want to fix the inconsistency in the platform device
> > API, then I'd suggest to fix the platform_get_irq(). The driver itself
> > is correct.
> With platform_get_irq as it is today, the check in
> 
> 	irq = platform_get_irq(pdev, 0);
> 	if (!irq)
> 		return -EINVAL;

And I see no problem with this piece of code, really. I see the
problem in platform_get_irq() implementation (not that easy to fix,
see below).

> is non-sense as !irq always evaluates to 0.  If your argue that the
> resources are right,

No, I'm arguing that there is no issue. There *could* be an issue in
the future (if someone break the platform code), but the way you're
trying to fix the *possibility* isn't quite correct.

Believe me, I'd have no problem with this particular patch if the
patch could possibly fix any real world issue with this driver.

For example, you may notice the following commit:

commit 2fac6674ddf3164da42a76d62f8912073d629a30
Author: Anton Vorontsov <avorontsov@ru.mvista.com>
Date:   Tue Jan 6 14:42:11 2009 -0800

    rtc: bunch of drivers: fix 'no irq' case handing

    This patch fixes a bunch of irq checking misuses.  Most drivers were
    getting irq via platform_get_irq(), which returns -ENXIO or r->start.

Sounds similar, eh? But you may notice that the unfixed code
was really wrong and didn't work for every sane platform:

        m48t59->irq = platform_get_irq(pdev, 0);
-       if (m48t59->irq < 0)
+       if (m48t59->irq <= 0)

The checks were irq < 0, so the drivers were requesting irq0, and
then were failing to probe.

Unfortunately, I stopped half way, afraid to possibly break current
platforms that use these drivers, so I kept the "<" part. Which is
a shame, because... um, it seems I spread the bad example further.

Anyway, I just did 'grep platform_get_irq -A 2 -r drivers/', and
it looks horrible, most callers check for irq < 0, which means
the code won't work on anything but arches with NO_IRQ == -1
(ARM, parisc, xtensa).

It seems the situation is near to hopeless. Maybe someone needs to
sit down and cleanup this mess once and for ever...

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[David Vrabel]

Uwe Kleine-König wrote:
> Hello,
> 
> [Note the email address used for David Vrabel isn't valid any more,
> this mail uses his last used address.]

I've not been involved in PXA27x stuff for years.  Please drop me from 
the CC, thanks.

David

Re: [PATCH 6/7] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Anton Vorontsov]

On Wed, Dec 16, 2009 at 05:10:08PM +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>
> Cc: David Vrabel <dvrabel@arcom.com>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: David Brownell <dbrownell@users.sourceforge.net>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: Anton Vorontsov <avorontsov@ru.mvista.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: spi-devel-general@lists.sourceforge.net
> ---
>  drivers/spi/spi_mpc8xxx.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/spi/spi_mpc8xxx.c b/drivers/spi/spi_mpc8xxx.c
> index e9390d7..b13501a 100644
> --- a/drivers/spi/spi_mpc8xxx.c
> +++ b/drivers/spi/spi_mpc8xxx.c
> @@ -1339,7 +1339,7 @@ static int __devinit plat_mpc8xxx_spi_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  
>  	irq = platform_get_irq(pdev, 0);
> -	if (!irq)
> +	if ((int)irq <= 0)

I tend to think that it's really hopeless to fix the
platform_get_irq() in its current form, so can you get rid
of this ugly cast and just make the irq signed?

And I'll be fine with it. :-(

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

platform_get_irq returns -ENXIO on failure, so !irq was probably
always true.  Make irq a signed variable and compare 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: David Brownell <dbrownell@users.sourceforge.net>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Anton Vorontsov <avorontsov@ru.mvista.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: spi-devel-general@lists.sourceforge.net
---
Hello,

as requested by Anton Vorontsov I made irq signed instead of casting to
int when comparing to zero.

Best regards
Uwe

 drivers/spi/spi_mpc8xxx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi_mpc8xxx.c b/drivers/spi/spi_mpc8xxx.c
index 1fb2a6e..08065fb 100644
--- a/drivers/spi/spi_mpc8xxx.c
+++ b/drivers/spi/spi_mpc8xxx.c
@@ -1328,7 +1328,7 @@ static struct of_platform_driver of_mpc8xxx_spi_driver = {
 static int __devinit plat_mpc8xxx_spi_probe(struct platform_device *pdev)
 {
 	struct resource *mem;
-	unsigned int irq;
+	int irq;
 	struct spi_master *master;
 
 	if (!pdev->dev.platform_data)
@@ -1339,7 +1339,7 @@ static int __devinit plat_mpc8xxx_spi_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	irq = platform_get_irq(pdev, 0);
-	if (!irq)
+	if (irq <= 0)
 		return -EINVAL;
 
 	master = mpc8xxx_spi_probe(&pdev->dev, mem, irq);
-- 
1.6.5.2

Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero

[Wolfgang Grandegger]

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.

Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

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/  |

Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero

[Alan Cox]

> +       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

Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero

[Wolfgang Grandegger]

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.

Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero

[Wolfgang Grandegger]

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.

Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero

[Wolfgang Grandegger]

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.

Re: [PATCH 4/7] can/at91: don't check platform_get_irq's return value against zero

[David Miller]

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.

[RESEND PATCH 1/5] [AVR32] don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

platform_get_irq returns -ENXIO on failure, so !int_irq was probably
always true.  Better use (int)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: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 arch/avr32/mach-at32ap/extint.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/avr32/mach-at32ap/extint.c b/arch/avr32/mach-at32ap/extint.c
index 310477b..033f693 100644
--- a/arch/avr32/mach-at32ap/extint.c
+++ b/arch/avr32/mach-at32ap/extint.c
@@ -198,7 +198,7 @@ static int __init eic_probe(struct platform_device *pdev)
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	int_irq = platform_get_irq(pdev, 0);
-	if (!regs || !int_irq) {
+	if (!regs || (int)int_irq <= 0) {
 		dev_dbg(&pdev->dev, "missing regs and/or irq resource\n");
 		return -ENXIO;
 	}
-- 
1.6.6

[RESEND PATCH 2/5] V4L/DVB mx1_camera: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

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: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Antonio Ospite <ospite@studenti.unina.it>
Cc: Paulius Zaleckas <paulius.zaleckas@teltonika.lt>
Cc: linux-media@vger.kernel.org
---
 drivers/media/video/mx1_camera.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/mx1_camera.c b/drivers/media/video/mx1_camera.c
index 2ba14fb..c167cc3 100644
--- a/drivers/media/video/mx1_camera.c
+++ b/drivers/media/video/mx1_camera.c
@@ -718,7 +718,7 @@ static int __init mx1_camera_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
-	if (!res || !irq) {
+	if (!res || (int)irq <= 0) {
 		err = -ENODEV;
 		goto exit;
 	}
-- 
1.6.6

Re: platform_get_irq() v4l fixes

[Guennadi Liakhovetski]

Hi Uwe

Your patches have been included in my pull request of 10.01.2010:

http://www.spinics.net/lists/linux-media/msg14449.html

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[RESEND PATCH 3/5] V4L/DVB sh_mobile_ceu: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

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: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <damm@igel.co.jp>
Cc: Kuninori Morimoto <morimoto.kuninori@renesas.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: linux-media@vger.kernel.org
---
 drivers/media/video/sh_mobile_ceu_camera.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index d69363f..f09c714 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -1827,7 +1827,7 @@ static int __devinit sh_mobile_ceu_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
-	if (!res || !irq) {
+	if (!res || (int)irq <= 0) {
 		dev_err(&pdev->dev, "Not enough CEU platform resources.\n");
 		err = -ENODEV;
 		goto exit;
-- 
1.6.6

[RESEND PATCH 4/5] pcmcia/bfin_cf: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

platform_get_irq returns -ENXIO on failure, so !irq was probably
always true.  Better use 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: Rafael J. Wysocki <rjw@sisk.pl>
Cc: linux-pcmcia@lists.infradead.org
---
 drivers/pcmcia/bfin_cf_pcmcia.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/pcmcia/bfin_cf_pcmcia.c b/drivers/pcmcia/bfin_cf_pcmcia.c
index 300b368..2482ce7 100644
--- a/drivers/pcmcia/bfin_cf_pcmcia.c
+++ b/drivers/pcmcia/bfin_cf_pcmcia.c
@@ -205,7 +205,7 @@ static int __devinit bfin_cf_probe(struct platform_device *pdev)
 	dev_info(&pdev->dev, "Blackfin CompactFlash/PCMCIA Socket Driver\n");
 
 	irq = platform_get_irq(pdev, 0);
-	if (!irq)
+	if (irq <= 0)
 		return -EINVAL;
 
 	cd_pfx = platform_get_irq(pdev, 1);	/*Card Detect GPIO PIN */
-- 
1.6.6

[RESEND PATCH 5/5] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

platform_get_irq returns -ENXIO on failure, so !irq was probably
always true.  Make irq a signed variable and compare 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: David Brownell <dbrownell@users.sourceforge.net>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Anton Vorontsov <avorontsov@ru.mvista.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: spi-devel-general@lists.sourceforge.net
---
 drivers/spi/spi_mpc8xxx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi_mpc8xxx.c b/drivers/spi/spi_mpc8xxx.c
index 1fb2a6e..08065fb 100644
--- a/drivers/spi/spi_mpc8xxx.c
+++ b/drivers/spi/spi_mpc8xxx.c
@@ -1328,7 +1328,7 @@ static struct of_platform_driver of_mpc8xxx_spi_driver = {
 static int __devinit plat_mpc8xxx_spi_probe(struct platform_device *pdev)
 {
 	struct resource *mem;
-	unsigned int irq;
+	int irq;
 	struct spi_master *master;
 
 	if (!pdev->dev.platform_data)
@@ -1339,7 +1339,7 @@ static int __devinit plat_mpc8xxx_spi_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	irq = platform_get_irq(pdev, 0);
-	if (!irq)
+	if (irq <= 0)
 		return -EINVAL;
 
 	master = mpc8xxx_spi_probe(&pdev->dev, mem, irq);
-- 
1.6.6

Re: [RESEND PATCH 5/5] spi/mpc8xxx: don't check platform_get_irq's return value against zero

[Anton Vorontsov]

On Wed, Jan 13, 2010 at 12:05:46PM +0100, Uwe Kleine-König wrote:
> platform_get_irq returns -ENXIO on failure, so !irq was probably
> always true.  Make irq a signed variable and compare 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: David Brownell <dbrownell@users.sourceforge.net>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: Anton Vorontsov <avorontsov@ru.mvista.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: spi-devel-general@lists.sourceforge.net
> ---

Acked-by: Anton Vorontsov <avorontsov@ru.mvista.com>

Thanks!

Re: [PATCH 1/7] [AVR32] don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

On Wed, Dec 16, 2009 at 05:10:03PM +0100, Uwe Kleine-König wrote:
> platform_get_irq returns -ENXIO on failure, so !int_irq was probably
> always true.  Better use (int)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: Haavard Skinnemoen <hskinnemoen@atmel.com>
> ---
>  arch/avr32/mach-at32ap/extint.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/avr32/mach-at32ap/extint.c b/arch/avr32/mach-at32ap/extint.c
> index 310477b..033f693 100644
> --- a/arch/avr32/mach-at32ap/extint.c
> +++ b/arch/avr32/mach-at32ap/extint.c
> @@ -198,7 +198,7 @@ static int __init eic_probe(struct platform_device *pdev)
>  
>  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	int_irq = platform_get_irq(pdev, 0);
> -	if (!regs || !int_irq) {
> +	if (!regs || (int)int_irq <= 0) {
>  		dev_dbg(&pdev->dev, "missing regs and/or irq resource\n");
>  		return -ENXIO;
>  	}
ping

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 1/7] [AVR32] don't check platform_get_irq's return value against zero

[Hans-Christian Egtvedt]

On Tue, 2010-10-19 at 08:45 +0200, Uwe Kleine-König wrote: 
> On Wed, Dec 16, 2009 at 05:10:03PM +0100, Uwe Kleine-König wrote:
> > platform_get_irq returns -ENXIO on failure, so !int_irq was probably
> > always true.  Better use (int)int_irq <= 0.  Note that a return value of
> > zero is still handled as error even though this could mean irq0.

Indeed, but external interrupts are numbered after the internal
interrupt lines, so in practice this does not happen. At least for now
with the AP700X series.

> > This is a followup to 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 that
> > changed the return value of platform_get_irq from 0 to -ENXIO on error.

Thanks for this fix.

> > 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: Haavard Skinnemoen <hskinnemoen@atmel.com>

Acked-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>

<snipp>

-- 
Hans-Christian Egtvedt

Re: [PATCH 1/7] [AVR32] don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

Hello Hans-Christian,

On Mon, Nov 01, 2010 at 10:58:03AM +0100, Hans-Christian Egtvedt wrote:
> On Tue, 2010-10-19 at 08:45 +0200, Uwe Kleine-König wrote: 
> > On Wed, Dec 16, 2009 at 05:10:03PM +0100, Uwe Kleine-König wrote:
> > > platform_get_irq returns -ENXIO on failure, so !int_irq was probably
> > > always true.  Better use (int)int_irq <= 0.  Note that a return value of
> > > zero is still handled as error even though this could mean irq0.
> 
> Indeed, but external interrupts are numbered after the internal
> interrupt lines, so in practice this does not happen. At least for now
> with the AP700X series.
> 
> > > This is a followup to 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 that
> > > changed the return value of platform_get_irq from 0 to -ENXIO on error.
> 
> Thanks for this fix.
> 
> > > 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: Haavard Skinnemoen <hskinnemoen@atmel.com>
> 
> Acked-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
I thought this to go via the avr32 tree.  You "only" acked, so what tree
do you consider here?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 1/7] [AVR32] don't check platform_get_irq's return value against zero

[Hans-Christian Egtvedt]

On Tue, 2010-11-02 at 10:15 +0100, Uwe Kleine-König wrote: 
> Hello Hans-Christian,
> 
> On Mon, Nov 01, 2010 at 10:58:03AM +0100, Hans-Christian Egtvedt wrote:
> > On Tue, 2010-10-19 at 08:45 +0200, Uwe Kleine-König wrote: 
> > > On Wed, Dec 16, 2009 at 05:10:03PM +0100, Uwe Kleine-König wrote:
> > > > platform_get_irq returns -ENXIO on failure, so !int_irq was probably
> > > > always true.  Better use (int)int_irq <= 0.  Note that a return value of
> > > > zero is still handled as error even though this could mean irq0.
> > 
> > Indeed, but external interrupts are numbered after the internal
> > interrupt lines, so in practice this does not happen. At least for now
> > with the AP700X series.
> > 
> > > > This is a followup to 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 that
> > > > changed the return value of platform_get_irq from 0 to -ENXIO on error.
> > 
> > Thanks for this fix.
> > 
> > > > 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: Haavard Skinnemoen <hskinnemoen@atmel.com>
> > 
> > Acked-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
> I thought this to go via the avr32 tree.  You "only" acked, so what tree
> do you consider here?

Right now there isn't any AVR32 tree, since I have yet to receive an
answer to my kernel.org account request. Could you push it through Linus
tree?

-- 
Hans-Christian Egtvedt

[PATCH RESENT#2] [AVR32] don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

platform_get_irq returns -ENXIO on failure, so !int_irq was probably
always true.  Better use (int)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.

Acked-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

Hans-Christian Egtvedt asked to push this patch via Linus as there is no avr32
tree.

@Andrew: do you take it?

Best regards
Uwe

 arch/avr32/mach-at32ap/extint.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/avr32/mach-at32ap/extint.c b/arch/avr32/mach-at32ap/extint.c
index e9d1205..dae8682 100644
--- a/arch/avr32/mach-at32ap/extint.c
+++ b/arch/avr32/mach-at32ap/extint.c
@@ -199,7 +199,7 @@ static int __init eic_probe(struct platform_device *pdev)
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	int_irq = platform_get_irq(pdev, 0);
-	if (!regs || !int_irq) {
+	if (!regs || (int)int_irq <= 0) {
 		dev_dbg(&pdev->dev, "missing regs and/or irq resource\n");
 		return -ENXIO;
 	}
-- 
1.7.2.3

Re: [PATCH RESENT#2] [AVR32] don't check platform_get_irq's return value against zero

[Alan Cox]

On Wed,  9 Feb 2011 11:28:04 +0100
Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> wrote:

> platform_get_irq returns -ENXIO on failure, so !int_irq was probably
> always true.  Better use (int)int_irq <= 0.  Note that a return value of
> zero is still handled as error even though this could mean irq0.

We don't have an IRQ 0 in Linux so that's fine.

Re: [PATCH RESENT#2] [AVR32] don't check platform_get_irq's return value against zero

[Hans-Christian Egtvedt]

On Wed, 2011-02-09 at 11:28 +0100, Uwe Kleine-König wrote: 
> platform_get_irq returns -ENXIO on failure, so !int_irq was probably
> always true.  Better use (int)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.
> 
> Acked-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> Hans-Christian Egtvedt asked to push this patch via Linus as there is no avr32
> tree.
> 
> @Andrew: do you take it?

Actually, now I have a tree for AVR32 on git.kernel.org, but I have no
other updates lined up for 2.6.38. Will you Andrew add it to your
series, or should I push this one through my git tree?

-- 
Hans-Christian Egtvedt

Re: [PATCH RESENT#2] [AVR32] don't check platform_get_irq's return value against zero

[Uwe Kleine-König]

On Wed, Feb 09, 2011 at 01:28:52PM +0100, Hans-Christian Egtvedt wrote:
> On Wed, 2011-02-09 at 11:28 +0100, Uwe Kleine-König wrote: 
> > platform_get_irq returns -ENXIO on failure, so !int_irq was probably
> > always true.  Better use (int)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.
> > 
> > Acked-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > Hans-Christian Egtvedt asked to push this patch via Linus as there is no avr32
> > tree.
> > 
> > @Andrew: do you take it?
> 
> Actually, now I have a tree for AVR32 on git.kernel.org, but I have no
> other updates lined up for 2.6.38. Will you Andrew add it to your
> series, or should I push this one through my git tree?
I don't consider it that critical.  So for me getting it in the avr32
tree now and then getting it merged for .39 would be OK for me.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH RESENT#2] [AVR32] don't check platform_get_irq's return value against zero

[Hans-Christian Egtvedt]

On Wed, 2011-02-09 at 13:44 +0100, Uwe Kleine-König wrote: 
> On Wed, Feb 09, 2011 at 01:28:52PM +0100, Hans-Christian Egtvedt wrote:
> > On Wed, 2011-02-09 at 11:28 +0100, Uwe Kleine-König wrote: 
> > > platform_get_irq returns -ENXIO on failure, so !int_irq was probably
> > > always true.  Better use (int)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.
> > > 
> > > Acked-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Hello,
> > > 
> > > Hans-Christian Egtvedt asked to push this patch via Linus as there is no avr32
> > > tree.
> > > 
> > > @Andrew: do you take it?
> > 
> > Actually, now I have a tree for AVR32 on git.kernel.org, but I have no
> > other updates lined up for 2.6.38. Will you Andrew add it to your
> > series, or should I push this one through my git tree?
> I don't consider it that critical.  So for me getting it in the avr32
> tree now and then getting it merged for .39 would be OK for me.

Okay, then I'll schedule it for the next release.

-- 
Hans-Christian Egtvedt