[alsa-devel] [PATCH 7/7] ASoC: sh: FSI:: don't check platform_get_irq's return value against zero
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;
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
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
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.
participants (3)
-
Kuninori Morimoto -
Mark Brown -
Uwe Kleine-König