[PATCH v2 1/2] platform: make platform_get_irq_optional() optional

[PATCH v2 1/2] platform: make platform_get_irq_optional() optional

[Sergey Shtylyov]

This patch is based on the former Andy Shevchenko's patch:

https://lore.kernel.org/lkml/20210331144526.19439-1-andriy.shevchenko@linux.intel.com/

Currently platform_get_irq_optional() returns an error code even if IRQ
resource simply has not been found.  It prevents the callers from being
error code agnostic in their error handling:

	ret = platform_get_irq_optional(...);
	if (ret < 0 && ret != -ENXIO)
		return ret; // respect deferred probe
	if (ret > 0)
		...we get an IRQ...

All other *_optional() APIs seem to return 0 or NULL in case an optional
resource is not available.  Let's follow this good example, so that the
callers would look like:

	ret = platform_get_irq_optional(...);
	if (ret < 0)
		return ret;
	if (ret > 0)
		...we get an IRQ...

Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
Changes in version 2:
- added the error check using dev_err_probe() to platform_get_irq_optional();
- fixed up the IRQ checks in 'drivers/char/ipmi/bt-bmc.c';
- fixed up the IRQ check in 'drivers/thermal/rcar_gen3_thermal.c';
- removed from the patch 'drivers/edac/xgene_edac.c and
  'drivers/power/supply/mp2629_charger.c' as they were switched to calling
  platform_get_irq();
- shortened the IRQ checking code in 'drivers/vfio/platform/vfio_platform.c';
- rebased atop of the recent platform_get_irq_byname() patch;
- reformatted the patch description.

 drivers/base/platform.c                  | 60 +++++++++++++++---------
 drivers/char/ipmi/bt-bmc.c               |  6 +--
 drivers/counter/interrupt-cnt.c          |  4 +-
 drivers/gpio/gpio-altera.c               |  3 +-
 drivers/gpio/gpio-mvebu.c                |  2 +-
 drivers/gpio/gpio-tqmx86.c               |  2 +-
 drivers/i2c/busses/i2c-brcmstb.c         |  8 ++--
 drivers/i2c/busses/i2c-ocores.c          |  4 +-
 drivers/mmc/host/sh_mmcif.c              |  4 +-
 drivers/mtd/nand/raw/brcmnand/brcmnand.c |  4 +-
 drivers/net/ethernet/davicom/dm9000.c    |  2 +-
 drivers/net/ethernet/freescale/fec_ptp.c |  2 +-
 drivers/phy/renesas/phy-rcar-gen3-usb2.c |  4 +-
 drivers/platform/chrome/cros_ec_lpc.c    |  2 +-
 drivers/platform/x86/intel/punit_ipc.c   |  2 +-
 drivers/spi/spi-hisi-sfc-v3xx.c          |  2 +-
 drivers/spi/spi-mtk-nor.c                |  3 +-
 drivers/thermal/rcar_gen3_thermal.c      |  2 +
 drivers/tty/serial/8250/8250_mtk.c       |  4 +-
 drivers/tty/serial/sh-sci.c              |  6 +--
 drivers/uio/uio_pdrv_genirq.c            |  2 +-
 drivers/vfio/platform/vfio_platform.c    |  3 +-
 sound/soc/dwc/dwc-i2s.c                  |  4 +-
 23 files changed, 76 insertions(+), 59 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 7d08cd8947be..52a8356f8261 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -148,25 +148,7 @@ devm_platform_ioremap_resource_byname(struct platform_device *pdev,
 EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname);
 #endif /* CONFIG_HAS_IOMEM */
 
-/**
- * platform_get_irq_optional - get an optional IRQ for a device
- * @dev: platform device
- * @num: IRQ number index
- *
- * Gets an IRQ for a platform device. Device drivers should check the return
- * value for errors so as to not pass a negative integer value to the
- * request_irq() APIs. This is the same as platform_get_irq(), except that it
- * does not print an error message if an IRQ can not be obtained.
- *
- * For example::
- *
- *		int irq = platform_get_irq_optional(pdev, 0);
- *		if (irq < 0)
- *			return irq;
- *
- * Return: non-zero IRQ number on success, negative error number on failure.
- */
-int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
+static int __platform_get_irq(struct platform_device *dev, unsigned int num)
 {
 	int ret;
 #ifdef CONFIG_SPARC
@@ -235,6 +217,42 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
 		return -EINVAL;
 	return ret;
 }
+
+/**
+ * platform_get_irq_optional - get an optional IRQ for a device
+ * @dev: platform device
+ * @num: IRQ number index
+ *
+ * Gets an IRQ for a platform device. Device drivers should check the return
+ * value for errors so as to not pass a negative integer value to the
+ * request_irq() APIs. This is the same as platform_get_irq(), except that it
+ * does not print an error message if an IRQ can not be obtained and returns
+ * 0 when IRQ resource has not been found.
+ *
+ * For example::
+ *
+ *		int irq = platform_get_irq_optional(pdev, 0);
+ *		if (irq < 0)
+ *			return irq;
+ *		if (irq > 0)
+ *			...we have IRQ line defined...
+ *
+ * Return: non-zero IRQ number on success, 0 if IRQ wasn't found, negative error
+ * number on failure.
+ */
+int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
+{
+	int ret;
+
+	ret = __platform_get_irq(dev, num);
+	if (ret == -ENXIO)
+		return 0;
+	if (ret < 0)
+		return dev_err_probe(&dev->dev, ret,
+				     "IRQ index %u not found\n", num);
+
+	return ret;
+}
 EXPORT_SYMBOL_GPL(platform_get_irq_optional);
 
 /**
@@ -258,7 +276,7 @@ int platform_get_irq(struct platform_device *dev, unsigned int num)
 {
 	int ret;
 
-	ret = platform_get_irq_optional(dev, num);
+	ret = __platform_get_irq(dev, num);
 	if (ret < 0)
 		return dev_err_probe(&dev->dev, ret,
 				     "IRQ index %u not found\n", num);
@@ -277,7 +295,7 @@ int platform_irq_count(struct platform_device *dev)
 {
 	int ret, nr = 0;
 
-	while ((ret = platform_get_irq_optional(dev, nr)) >= 0)
+	while ((ret = __platform_get_irq(dev, nr)) >= 0)
 		nr++;
 
 	if (ret == -EPROBE_DEFER)
diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index 7450904e330a..289acaf4e720 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -380,7 +380,7 @@ static int bt_bmc_config_irq(struct bt_bmc *bt_bmc,
 	u32 reg;
 
 	bt_bmc->irq = platform_get_irq_optional(pdev, 0);
-	if (bt_bmc->irq < 0)
+	if (bt_bmc->irq <= 0)
 		return bt_bmc->irq;
 
 	rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq, IRQF_SHARED,
@@ -438,7 +438,7 @@ static int bt_bmc_probe(struct platform_device *pdev)
 
 	bt_bmc_config_irq(bt_bmc, pdev);
 
-	if (bt_bmc->irq >= 0) {
+	if (bt_bmc->irq > 0) {
 		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
 	} else {
 		dev_info(dev, "No IRQ; using timer\n");
@@ -464,7 +464,7 @@ static int bt_bmc_remove(struct platform_device *pdev)
 	struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
 
 	misc_deregister(&bt_bmc->miscdev);
-	if (bt_bmc->irq < 0)
+	if (bt_bmc->irq <= 0)
 		del_timer_sync(&bt_bmc->poll_timer);
 	return 0;
 }
diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c
index 9e99702470c2..a2443c66330b 100644
--- a/drivers/counter/interrupt-cnt.c
+++ b/drivers/counter/interrupt-cnt.c
@@ -157,9 +157,7 @@ static int interrupt_cnt_probe(struct platform_device *pdev)
 	priv = counter_priv(counter);
 
 	priv->irq = platform_get_irq_optional(pdev,  0);
-	if (priv->irq == -ENXIO)
-		priv->irq = 0;
-	else if (priv->irq < 0)
+	if (priv->irq < 0)
 		return dev_err_probe(dev, priv->irq, "failed to get IRQ\n");
 
 	priv->gpio = devm_gpiod_get_optional(dev, NULL, GPIOD_IN);
diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
index b59fae993626..02a2995aa368 100644
--- a/drivers/gpio/gpio-altera.c
+++ b/drivers/gpio/gpio-altera.c
@@ -267,8 +267,7 @@ static int altera_gpio_probe(struct platform_device *pdev)
 	altera_gc->mmchip.gc.parent		= &pdev->dev;
 
 	altera_gc->mapped_irq = platform_get_irq_optional(pdev, 0);
-
-	if (altera_gc->mapped_irq < 0)
+	if (altera_gc->mapped_irq <= 0)
 		goto skip_irq;
 
 	if (of_property_read_u32(node, "altr,interrupt-type", &reg)) {
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 4c1f9e1091b7..d94594807697 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -1293,7 +1293,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
 	for (i = 0; i < 4; i++) {
 		int irq = platform_get_irq_optional(pdev, i);
 
-		if (irq < 0)
+		if (irq <= 0)
 			continue;
 		irq_set_chained_handler_and_data(irq, mvebu_gpio_irq_handler,
 						 mvchip);
diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
index 5b103221b58d..dc0f83236ce8 100644
--- a/drivers/gpio/gpio-tqmx86.c
+++ b/drivers/gpio/gpio-tqmx86.c
@@ -237,7 +237,7 @@ static int tqmx86_gpio_probe(struct platform_device *pdev)
 	int ret, irq;
 
 	irq = platform_get_irq_optional(pdev, 0);
-	if (irq < 0 && irq != -ENXIO)
+	if (irq < 0)
 		return irq;
 
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
diff --git a/drivers/i2c/busses/i2c-brcmstb.c b/drivers/i2c/busses/i2c-brcmstb.c
index 490ee3962645..69395ae27a1a 100644
--- a/drivers/i2c/busses/i2c-brcmstb.c
+++ b/drivers/i2c/busses/i2c-brcmstb.c
@@ -250,7 +250,7 @@ static int brcmstb_i2c_wait_for_completion(struct brcmstb_i2c_dev *dev)
 	int ret = 0;
 	unsigned long timeout = msecs_to_jiffies(I2C_TIMEOUT);
 
-	if (dev->irq >= 0) {
+	if (dev->irq > 0) {
 		if (!wait_for_completion_timeout(&dev->done, timeout))
 			ret = -ETIMEDOUT;
 	} else {
@@ -297,7 +297,7 @@ static int brcmstb_send_i2c_cmd(struct brcmstb_i2c_dev *dev,
 		return rc;
 
 	/* only if we are in interrupt mode */
-	if (dev->irq >= 0)
+	if (dev->irq > 0)
 		reinit_completion(&dev->done);
 
 	/* enable BSC CTL interrupt line */
@@ -652,7 +652,7 @@ static int brcmstb_i2c_probe(struct platform_device *pdev)
 	brcmstb_i2c_enable_disable_irq(dev, INT_DISABLE);
 
 	/* register the ISR handler */
-	if (dev->irq >= 0) {
+	if (dev->irq > 0) {
 		rc = devm_request_irq(&pdev->dev, dev->irq, brcmstb_i2c_isr,
 				      IRQF_SHARED,
 				      int_name ? int_name : pdev->name,
@@ -696,7 +696,7 @@ static int brcmstb_i2c_probe(struct platform_device *pdev)
 
 	dev_info(dev->device, "%s@%dhz registered in %s mode\n",
 		 int_name ? int_name : " ", dev->clk_freq_hz,
-		 (dev->irq >= 0) ? "interrupt" : "polling");
+		 (dev->irq > 0) ? "interrupt" : "polling");
 
 	return 0;
 
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index a0af027db04c..1f4d5e52ff42 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -691,10 +691,10 @@ static int ocores_i2c_probe(struct platform_device *pdev)
 	if (of_device_is_compatible(pdev->dev.of_node,
 				    "sifive,fu540-c000-i2c")) {
 		i2c->flags |= OCORES_FLAG_BROKEN_IRQ;
-		irq = -ENXIO;
+		irq = 0;
 	}
 
-	if (irq == -ENXIO) {
+	if (!irq) {
 		ocores_algorithm.master_xfer = ocores_xfer_polling;
 	} else {
 		if (irq < 0)
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index bcc595c70a9f..f558b9862032 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1465,14 +1465,14 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	sh_mmcif_sync_reset(host);
 	sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
 
-	name = irq[1] < 0 ? dev_name(dev) : "sh_mmc:error";
+	name = irq[1] <= 0 ? dev_name(dev) : "sh_mmc:error";
 	ret = devm_request_threaded_irq(dev, irq[0], sh_mmcif_intr,
 					sh_mmcif_irqt, 0, name, host);
 	if (ret) {
 		dev_err(dev, "request_irq error (%s)\n", name);
 		goto err_clk;
 	}
-	if (irq[1] >= 0) {
+	if (irq[1] > 0) {
 		ret = devm_request_threaded_irq(dev, irq[1],
 						sh_mmcif_intr, sh_mmcif_irqt,
 						0, "sh_mmc:int", host);
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index f75929783b94..ac222985efde 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1521,7 +1521,7 @@ static irqreturn_t brcmnand_ctlrdy_irq(int irq, void *data)
 
 	/* check if you need to piggy back on the ctrlrdy irq */
 	if (ctrl->edu_pending) {
-		if (irq == ctrl->irq && ((int)ctrl->edu_irq >= 0))
+		if (irq == ctrl->irq && ((int)ctrl->edu_irq > 0))
 	/* Discard interrupts while using dedicated edu irq */
 			return IRQ_HANDLED;
 
@@ -2956,7 +2956,7 @@ static int brcmnand_edu_setup(struct platform_device *pdev)
 		brcmnand_edu_init(ctrl);
 
 		ctrl->edu_irq = platform_get_irq_optional(pdev, 1);
-		if (ctrl->edu_irq < 0) {
+		if (ctrl->edu_irq <= 0) {
 			dev_warn(dev,
 				 "FLASH EDU enabled, using ctlrdy irq\n");
 		} else {
diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c
index 0985ab216566..740c660a9411 100644
--- a/drivers/net/ethernet/davicom/dm9000.c
+++ b/drivers/net/ethernet/davicom/dm9000.c
@@ -1509,7 +1509,7 @@ dm9000_probe(struct platform_device *pdev)
 	}
 
 	db->irq_wake = platform_get_irq_optional(pdev, 1);
-	if (db->irq_wake >= 0) {
+	if (db->irq_wake > 0) {
 		dev_dbg(db->dev, "wakeup irq %d\n", db->irq_wake);
 
 		ret = request_irq(db->irq_wake, dm9000_wol_interrupt,
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index af99017a5453..de1d23808b6c 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -616,7 +616,7 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx)
 	/* Failure to get an irq is not fatal,
 	 * only the PTP_CLOCK_PPS clock events should stop
 	 */
-	if (irq >= 0) {
+	if (irq > 0) {
 		ret = devm_request_irq(&pdev->dev, irq, fec_pps_interrupt,
 				       0, pdev->name, ndev);
 		if (ret < 0)
diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
index 9de617ca9daa..4914d6aca208 100644
--- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
@@ -439,7 +439,7 @@ static int rcar_gen3_phy_usb2_init(struct phy *p)
 	u32 val;
 	int ret;
 
-	if (!rcar_gen3_is_any_rphy_initialized(channel) && channel->irq >= 0) {
+	if (!rcar_gen3_is_any_rphy_initialized(channel) && channel->irq > 0) {
 		INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
 		ret = request_irq(channel->irq, rcar_gen3_phy_usb2_irq,
 				  IRQF_SHARED, dev_name(channel->dev), channel);
@@ -486,7 +486,7 @@ static int rcar_gen3_phy_usb2_exit(struct phy *p)
 		val &= ~USB2_INT_ENABLE_UCOM_INTEN;
 	writel(val, usb2_base + USB2_INT_ENABLE);
 
-	if (channel->irq >= 0 && !rcar_gen3_is_any_rphy_initialized(channel))
+	if (channel->irq > 0 && !rcar_gen3_is_any_rphy_initialized(channel))
 		free_irq(channel->irq, channel);
 
 	return 0;
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index d6306d2a096f..91686d306534 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -400,7 +400,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	irq = platform_get_irq_optional(pdev, 0);
 	if (irq > 0)
 		ec_dev->irq = irq;
-	else if (irq != -ENXIO) {
+	else if (irq < 0) {
 		dev_err(dev, "couldn't retrieve IRQ number (%d)\n", irq);
 		return irq;
 	}
diff --git a/drivers/platform/x86/intel/punit_ipc.c b/drivers/platform/x86/intel/punit_ipc.c
index 66bb39fd0ef9..f3cf5ee1466f 100644
--- a/drivers/platform/x86/intel/punit_ipc.c
+++ b/drivers/platform/x86/intel/punit_ipc.c
@@ -278,7 +278,7 @@ static int intel_punit_ipc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, punit_ipcdev);
 
 	irq = platform_get_irq_optional(pdev, 0);
-	if (irq < 0) {
+	if (irq <= 0) {
 		dev_warn(&pdev->dev, "Invalid IRQ, using polling mode\n");
 	} else {
 		ret = devm_request_irq(&pdev->dev, irq, intel_punit_ioc,
diff --git a/drivers/spi/spi-hisi-sfc-v3xx.c b/drivers/spi/spi-hisi-sfc-v3xx.c
index d3a23b1c2a4c..476ddc081c60 100644
--- a/drivers/spi/spi-hisi-sfc-v3xx.c
+++ b/drivers/spi/spi-hisi-sfc-v3xx.c
@@ -467,7 +467,7 @@ static int hisi_sfc_v3xx_probe(struct platform_device *pdev)
 			dev_err(dev, "failed to request irq%d, ret = %d\n", host->irq, ret);
 			host->irq = 0;
 		}
-	} else {
+	} else if (host->irq < 0) {
 		host->irq = 0;
 	}
 
diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index 5c93730615f8..2422b0545936 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -829,8 +829,7 @@ static int mtk_nor_probe(struct platform_device *pdev)
 	mtk_nor_init(sp);
 
 	irq = platform_get_irq_optional(pdev, 0);
-
-	if (irq < 0) {
+	if (irq <= 0) {
 		dev_warn(sp->dev, "IRQ not available.");
 	} else {
 		ret = devm_request_irq(sp->dev, irq, mtk_nor_irq_handler, 0,
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 43eb25b167bc..359b9941c42b 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -432,6 +432,8 @@ static int rcar_gen3_thermal_request_irqs(struct rcar_gen3_thermal_priv *priv,
 		irq = platform_get_irq_optional(pdev, i);
 		if (irq < 0)
 			return irq;
+		if (!irq)
+			return -ENXIO;
 
 		irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:ch%d",
 					 dev_name(dev), i);
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index fb65dc601b23..328ab074fd89 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -621,7 +621,7 @@ static int __maybe_unused mtk8250_suspend(struct device *dev)
 	serial8250_suspend_port(data->line);
 
 	pinctrl_pm_select_sleep_state(dev);
-	if (irq >= 0) {
+	if (irq > 0) {
 		err = enable_irq_wake(irq);
 		if (err) {
 			dev_err(dev,
@@ -641,7 +641,7 @@ static int __maybe_unused mtk8250_resume(struct device *dev)
 	struct mtk8250_data *data = dev_get_drvdata(dev);
 	int irq = data->rx_wakeup_irq;
 
-	if (irq >= 0)
+	if (irq > 0)
 		disable_irq_wake(irq);
 	pinctrl_pm_select_default_state(dev);
 
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 968967d722d4..96c3146ff6a4 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1915,7 +1915,7 @@ static int sci_request_irq(struct sci_port *port)
 			 * Certain port types won't support all of the
 			 * available interrupt sources.
 			 */
-			if (unlikely(irq < 0))
+			if (unlikely(irq <= 0))
 				continue;
 		}
 
@@ -1963,7 +1963,7 @@ static void sci_free_irq(struct sci_port *port)
 		 * Certain port types won't support all of the available
 		 * interrupt sources.
 		 */
-		if (unlikely(irq < 0))
+		if (unlikely(irq <= 0))
 			continue;
 
 		/* Check if already freed (irq was muxed) */
@@ -2875,7 +2875,7 @@ static int sci_init_single(struct platform_device *dev,
 	if (sci_port->irqs[0] < 0)
 		return -ENXIO;
 
-	if (sci_port->irqs[1] < 0)
+	if (sci_port->irqs[1] <= 0)
 		for (i = 1; i < ARRAY_SIZE(sci_port->irqs); i++)
 			sci_port->irqs[i] = sci_port->irqs[0];
 
diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
index 63258b6accc4..7fd275fc6ceb 100644
--- a/drivers/uio/uio_pdrv_genirq.c
+++ b/drivers/uio/uio_pdrv_genirq.c
@@ -162,7 +162,7 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 	if (!uioinfo->irq) {
 		ret = platform_get_irq_optional(pdev, 0);
 		uioinfo->irq = ret;
-		if (ret == -ENXIO)
+		if (!ret)
 			uioinfo->irq = UIO_IRQ_NONE;
 		else if (ret == -EPROBE_DEFER)
 			return ret;
diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
index 68a1c87066d7..f7423d10cefd 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -32,8 +32,9 @@ static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
 static int get_platform_irq(struct vfio_platform_device *vdev, int i)
 {
 	struct platform_device *pdev = (struct platform_device *) vdev->opaque;
+	int ret = platform_get_irq_optional(pdev, i);
 
-	return platform_get_irq_optional(pdev, i);
+	return ret ? ret : -ENXIO;
 }
 
 static int vfio_platform_probe(struct platform_device *pdev)
diff --git a/sound/soc/dwc/dwc-i2s.c b/sound/soc/dwc/dwc-i2s.c
index 5cb58929090d..ff19c5130459 100644
--- a/sound/soc/dwc/dwc-i2s.c
+++ b/sound/soc/dwc/dwc-i2s.c
@@ -643,7 +643,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
 	dev->dev = &pdev->dev;
 
 	irq = platform_get_irq_optional(pdev, 0);
-	if (irq >= 0) {
+	if (irq > 0) {
 		ret = devm_request_irq(&pdev->dev, irq, i2s_irq_handler, 0,
 				pdev->name, dev);
 		if (ret < 0) {
@@ -697,7 +697,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
 	}
 
 	if (!pdata) {
-		if (irq >= 0) {
+		if (irq > 0) {
 			ret = dw_pcm_register(pdev);
 			dev->use_pio = true;
 		} else {
-- 
2.26.3


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] platform: make platform_get_irq_optional() optional

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 1621 bytes --]

On Sat, Feb 12, 2022 at 11:16:30PM +0300, Sergey Shtylyov wrote:
> This patch is based on the former Andy Shevchenko's patch:
> 
> https://lore.kernel.org/lkml/20210331144526.19439-1-andriy.shevchenko@linux.intel.com/
> 
> Currently platform_get_irq_optional() returns an error code even if IRQ
> resource simply has not been found.  It prevents the callers from being
> error code agnostic in their error handling:
> 
> 	ret = platform_get_irq_optional(...);
> 	if (ret < 0 && ret != -ENXIO)
> 		return ret; // respect deferred probe
> 	if (ret > 0)
> 		...we get an IRQ...
> 
> All other *_optional() APIs seem to return 0 or NULL in case an optional
> resource is not available.  Let's follow this good example, so that the
> callers would look like:
> 
> 	ret = platform_get_irq_optional(...);
> 	if (ret < 0)
> 		return ret;
> 	if (ret > 0)
> 		...we get an IRQ...
> 
> Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

While this patch is better than v1, I still don't like it for the
reasons discussed for v1. (i.e. 0 isn't usable as a dummy value which I
consider the real advantage for the other _get_optional() functions.)

Apart from that, I think the subject is badly chosen. With "Make
somefunc() optional" I would expect that you introduce a Kconfig symbol
that results in the function not being available when disabled.

Best regards
Uwe

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] platform: make platform_get_irq_optional() optional

[Geert Uytterhoeven]

Hi Uwe,

On Mon, Feb 14, 2022 at 8:29 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Sat, Feb 12, 2022 at 11:16:30PM +0300, Sergey Shtylyov wrote:
> > This patch is based on the former Andy Shevchenko's patch:
> >
> > https://lore.kernel.org/lkml/20210331144526.19439-1-andriy.shevchenko@linux.intel.com/
> >
> > Currently platform_get_irq_optional() returns an error code even if IRQ
> > resource simply has not been found.  It prevents the callers from being
> > error code agnostic in their error handling:
> >
> >       ret = platform_get_irq_optional(...);
> >       if (ret < 0 && ret != -ENXIO)
> >               return ret; // respect deferred probe
> >       if (ret > 0)
> >               ...we get an IRQ...
> >
> > All other *_optional() APIs seem to return 0 or NULL in case an optional
> > resource is not available.  Let's follow this good example, so that the
> > callers would look like:
> >
> >       ret = platform_get_irq_optional(...);
> >       if (ret < 0)
> >               return ret;
> >       if (ret > 0)
> >               ...we get an IRQ...
> >
> > Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>
> While this patch is better than v1, I still don't like it for the
> reasons discussed for v1. (i.e. 0 isn't usable as a dummy value which I
> consider the real advantage for the other _get_optional() functions.)

IMHO the real advantage is the simplified error handling, which is the
area where most of the current bugs are. So I applaud the core change.

Also IMHO, the dummy value handling is a red herring.  Contrary to
optional clocks and resets, a missing optional interrupt does not
always mean there is nothing to do: in case of polling, something
else must definitely be done.  So even if request_irq() would accept
a dummy interrupt zero and just do nothing, it would give the false
impression that that is all there is to do, while an actual check
for zero with polling code handling may still need to be present,
thus leading to more not less bugs.

> Apart from that, I think the subject is badly chosen. With "Make
> somefunc() optional" I would expect that you introduce a Kconfig symbol
> that results in the function not being available when disabled.

Agreed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] platform: make platform_get_irq_optional() optional

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 1011 bytes --]

Hello Geert,

On Mon, Feb 14, 2022 at 10:01:14AM +0100, Geert Uytterhoeven wrote:
> Also IMHO, the dummy value handling is a red herring.  Contrary to
> optional clocks and resets, a missing optional interrupt does not
> always mean there is nothing to do: in case of polling, something
> else must definitely be done. 

Note this is exactly why I don't like this change. I'd even go so far
and claim that "a missing optional interrupt hardly ever means there is
nothing to do".

> So even if request_irq() would accept a dummy interrupt zero and just
> do nothing, it would give the false impression that that is all there
> is to do, while an actual check for zero with polling code handling
> may still need to be present, thus leading to more not less bugs.

Yes, a dummy irq value will just not be possible.

Best regards
Uwe

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] platform: make platform_get_irq_optional() optional

[Andy Shevchenko]

On Mon, Feb 14, 2022 at 08:13:51AM +0100, Uwe Kleine-König wrote:
> On Sat, Feb 12, 2022 at 11:16:30PM +0300, Sergey Shtylyov wrote:
> > This patch is based on the former Andy Shevchenko's patch:
> > 
> > https://lore.kernel.org/lkml/20210331144526.19439-1-andriy.shevchenko@linux.intel.com/
> > 
> > Currently platform_get_irq_optional() returns an error code even if IRQ
> > resource simply has not been found.  It prevents the callers from being
> > error code agnostic in their error handling:
> > 
> > 	ret = platform_get_irq_optional(...);
> > 	if (ret < 0 && ret != -ENXIO)
> > 		return ret; // respect deferred probe
> > 	if (ret > 0)
> > 		...we get an IRQ...
> > 
> > All other *_optional() APIs seem to return 0 or NULL in case an optional
> > resource is not available.  Let's follow this good example, so that the
> > callers would look like:
> > 
> > 	ret = platform_get_irq_optional(...);
> > 	if (ret < 0)
> > 		return ret;
> > 	if (ret > 0)
> > 		...we get an IRQ...
> > 
> > Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> While this patch is better than v1, I still don't like it for the
> reasons discussed for v1. (i.e. 0 isn't usable as a dummy value which I
> consider the real advantage for the other _get_optional() functions.)

I think you haven't reacted anyhow to my point that you mixing apples and
bananas together when comparing this 0 to the others _optional APIs.

> Apart from that, I think the subject is badly chosen. With "Make
> somefunc() optional" I would expect that you introduce a Kconfig symbol
> that results in the function not being available when disabled.


-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] platform: make platform_get_irq_optional() optional

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 2151 bytes --]

Hello Andy,

On Mon, Feb 14, 2022 at 12:13:01PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 14, 2022 at 08:13:51AM +0100, Uwe Kleine-König wrote:
> > On Sat, Feb 12, 2022 at 11:16:30PM +0300, Sergey Shtylyov wrote:
> > > This patch is based on the former Andy Shevchenko's patch:
> > > 
> > > https://lore.kernel.org/lkml/20210331144526.19439-1-andriy.shevchenko@linux.intel.com/
> > > 
> > > Currently platform_get_irq_optional() returns an error code even if IRQ
> > > resource simply has not been found.  It prevents the callers from being
> > > error code agnostic in their error handling:
> > > 
> > > 	ret = platform_get_irq_optional(...);
> > > 	if (ret < 0 && ret != -ENXIO)
> > > 		return ret; // respect deferred probe
> > > 	if (ret > 0)
> > > 		...we get an IRQ...
> > > 
> > > All other *_optional() APIs seem to return 0 or NULL in case an optional
> > > resource is not available.  Let's follow this good example, so that the
> > > callers would look like:
> > > 
> > > 	ret = platform_get_irq_optional(...);
> > > 	if (ret < 0)
> > > 		return ret;
> > > 	if (ret > 0)
> > > 		...we get an IRQ...
> > > 
> > > Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> > 
> > While this patch is better than v1, I still don't like it for the
> > reasons discussed for v1. (i.e. 0 isn't usable as a dummy value which I
> > consider the real advantage for the other _get_optional() functions.)
> 
> I think you haven't reacted anyhow to my point that you mixing apples and
> bananas together when comparing this 0 to the others _optional APIs.

Is this a question to me or Sergey?

I fully agree, when the 0 of platform_get_irq_optional is an apple and
the NULL of gpio_get_optional is a banana, I doubt "All other
*_optional() APIs seem to return 0 or NULL in case an optional resource
is not available.  Let's follow this good example, [...]".

Best regards
Uwe

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] platform: make platform_get_irq_optional() optional

[Geert Uytterhoeven]

Hi Sergey,

On Sat, Feb 12, 2022 at 9:17 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote:
> This patch is based on the former Andy Shevchenko's patch:
>
> https://lore.kernel.org/lkml/20210331144526.19439-1-andriy.shevchenko@linux.intel.com/
>
> Currently platform_get_irq_optional() returns an error code even if IRQ
> resource simply has not been found.  It prevents the callers from being
> error code agnostic in their error handling:
>
>         ret = platform_get_irq_optional(...);
>         if (ret < 0 && ret != -ENXIO)
>                 return ret; // respect deferred probe
>         if (ret > 0)
>                 ...we get an IRQ...
>
> All other *_optional() APIs seem to return 0 or NULL in case an optional
> resource is not available.  Let's follow this good example, so that the
> callers would look like:
>
>         ret = platform_get_irq_optional(...);
>         if (ret < 0)
>                 return ret;
>         if (ret > 0)
>                 ...we get an IRQ...
>
> Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> ---
> Changes in version 2:

Thanks for the update!

>  drivers/base/platform.c                  | 60 +++++++++++++++---------

The core change LGTM.

I'm only looking at Renesas drivers below...

> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1465,14 +1465,14 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>         sh_mmcif_sync_reset(host);
>         sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
>
> -       name = irq[1] < 0 ? dev_name(dev) : "sh_mmc:error";
> +       name = irq[1] <= 0 ? dev_name(dev) : "sh_mmc:error";

"== 0" should be sufficient here, if the code above would bail out
on errors returned by platform_get_irq_optional(), which it currently
doesn't do.
As this adds missing error handling, this is to be fixed by a separate
patch later?

>         ret = devm_request_threaded_irq(dev, irq[0], sh_mmcif_intr,
>                                         sh_mmcif_irqt, 0, name, host);
>         if (ret) {
>                 dev_err(dev, "request_irq error (%s)\n", name);
>                 goto err_clk;
>         }
> -       if (irq[1] >= 0) {
> +       if (irq[1] > 0) {

OK.

>                 ret = devm_request_threaded_irq(dev, irq[1],
>                                                 sh_mmcif_intr, sh_mmcif_irqt,
>                                                 0, "sh_mmc:int", host);

> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -439,7 +439,7 @@ static int rcar_gen3_phy_usb2_init(struct phy *p)
>         u32 val;
>         int ret;
>
> -       if (!rcar_gen3_is_any_rphy_initialized(channel) && channel->irq >= 0) {
> +       if (!rcar_gen3_is_any_rphy_initialized(channel) && channel->irq > 0) {
>                 INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
>                 ret = request_irq(channel->irq, rcar_gen3_phy_usb2_irq,
>                                   IRQF_SHARED, dev_name(channel->dev), channel);
> @@ -486,7 +486,7 @@ static int rcar_gen3_phy_usb2_exit(struct phy *p)
>                 val &= ~USB2_INT_ENABLE_UCOM_INTEN;
>         writel(val, usb2_base + USB2_INT_ENABLE);
>
> -       if (channel->irq >= 0 && !rcar_gen3_is_any_rphy_initialized(channel))
> +       if (channel->irq > 0 && !rcar_gen3_is_any_rphy_initialized(channel))
>                 free_irq(channel->irq, channel);
>
>         return 0;

LGTM, but note that all errors returned by platform_get_irq_optional()
are currently ignored, even real errors, which should be propagated
up.
As this adds missing error handling, this is to be fixed by a separate
patch later?

> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -432,6 +432,8 @@ static int rcar_gen3_thermal_request_irqs(struct rcar_gen3_thermal_priv *priv,
>                 irq = platform_get_irq_optional(pdev, i);
>                 if (irq < 0)
>                         return irq;
> +               if (!irq)
> +                       return -ENXIO;

While correct, and preserving existing behavior, this looks strange
to me.  Probably this should return zero instead (i.e. the check
above should be changed to "<= 0"), and the caller should start caring
about and propagating up real errors.
As this adds missing error handling, this is to be fixed by a separate
patch later?

>
>                 irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:ch%d",
>                                          dev_name(dev), i);
> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> index fb65dc601b23..328ab074fd89 100644

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c

I think you missed

    #define SCIx_IRQ_IS_MUXED(port)                 \
            ((port)->irqs[SCIx_ERI_IRQ] ==  \
             (port)->irqs[SCIx_RXI_IRQ]) || \
            ((port)->irqs[SCIx_ERI_IRQ] &&  \
             ((port)->irqs[SCIx_RXI_IRQ] < 0))

above? The last condition should become "<= 0".

> @@ -1915,7 +1915,7 @@ static int sci_request_irq(struct sci_port *port)
>                          * Certain port types won't support all of the
>                          * available interrupt sources.
>                          */
> -                       if (unlikely(irq < 0))
> +                       if (unlikely(irq <= 0))
>                                 continue;
>                 }
>
> @@ -1963,7 +1963,7 @@ static void sci_free_irq(struct sci_port *port)
>                  * Certain port types won't support all of the available
>                  * interrupt sources.
>                  */
> -               if (unlikely(irq < 0))
> +               if (unlikely(irq <= 0))
>                         continue;
>
>                 /* Check if already freed (irq was muxed) */
> @@ -2875,7 +2875,7 @@ static int sci_init_single(struct platform_device *dev,
>         if (sci_port->irqs[0] < 0)
>                 return -ENXIO;
>
> -       if (sci_port->irqs[1] < 0)
> +       if (sci_port->irqs[1] <= 0)
>                 for (i = 1; i < ARRAY_SIZE(sci_port->irqs); i++)
>                         sci_port->irqs[i] = sci_port->irqs[0];
>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] platform: make platform_get_irq_optional() optional

[Sergey Shtylyov]

Hello!

On 2/14/22 11:54 AM, Geert Uytterhoeven wrote:

[...]

>> This patch is based on the former Andy Shevchenko's patch:
>>
>> https://lore.kernel.org/lkml/20210331144526.19439-1-andriy.shevchenko@linux.intel.com/
>>
>> Currently platform_get_irq_optional() returns an error code even if IRQ
>> resource simply has not been found.  It prevents the callers from being
>> error code agnostic in their error handling:
>>
>>         ret = platform_get_irq_optional(...);
>>         if (ret < 0 && ret != -ENXIO)
>>                 return ret; // respect deferred probe
>>         if (ret > 0)
>>                 ...we get an IRQ...
>>
>> All other *_optional() APIs seem to return 0 or NULL in case an optional
>> resource is not available.  Let's follow this good example, so that the
>> callers would look like:
>>
>>         ret = platform_get_irq_optional(...);
>>         if (ret < 0)
>>                 return ret;
>>         if (ret > 0)
>>                 ...we get an IRQ...
>>
>> Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>> ---
>> Changes in version 2:
> 
> Thanks for the update!
> 
>>  drivers/base/platform.c                  | 60 +++++++++++++++---------
> 
> The core change LGTM.

   Thanx! :-)

> I'm only looking at Renesas drivers below...
> 
>> --- a/drivers/mmc/host/sh_mmcif.c
>> +++ b/drivers/mmc/host/sh_mmcif.c
>> @@ -1465,14 +1465,14 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>>         sh_mmcif_sync_reset(host);
>>         sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
>>
>> -       name = irq[1] < 0 ? dev_name(dev) : "sh_mmc:error";
>> +       name = irq[1] <= 0 ? dev_name(dev) : "sh_mmc:error";
> 
> "== 0" should be sufficient here, if the code above would bail out
> on errors returned by platform_get_irq_optional(), which it currently
> doesn't do.
> As this adds missing error handling, this is to be fixed by a separate
> patch later?

   Yes.

[...]
>>                 ret = devm_request_threaded_irq(dev, irq[1],
>>                                                 sh_mmcif_intr, sh_mmcif_irqt,
>>                                                 0, "sh_mmc:int", host);
> 
>> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>> @@ -439,7 +439,7 @@ static int rcar_gen3_phy_usb2_init(struct phy *p)
>>         u32 val;
>>         int ret;
>>
>> -       if (!rcar_gen3_is_any_rphy_initialized(channel) && channel->irq >= 0) {
>> +       if (!rcar_gen3_is_any_rphy_initialized(channel) && channel->irq > 0) {
>>                 INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
>>                 ret = request_irq(channel->irq, rcar_gen3_phy_usb2_irq,
>>                                   IRQF_SHARED, dev_name(channel->dev), channel);
>> @@ -486,7 +486,7 @@ static int rcar_gen3_phy_usb2_exit(struct phy *p)
>>                 val &= ~USB2_INT_ENABLE_UCOM_INTEN;
>>         writel(val, usb2_base + USB2_INT_ENABLE);
>>
>> -       if (channel->irq >= 0 && !rcar_gen3_is_any_rphy_initialized(channel))
>> +       if (channel->irq > 0 && !rcar_gen3_is_any_rphy_initialized(channel))
>>                 free_irq(channel->irq, channel);
>>
>>         return 0;
> 
> LGTM, but note that all errors returned by platform_get_irq_optional()
> are currently ignored, even real errors, which should be propagated
> up.
> As this adds missing error handling, this is to be fixed by a separate
> patch later?

   Yes.

>> --- a/drivers/thermal/rcar_gen3_thermal.c
>> +++ b/drivers/thermal/rcar_gen3_thermal.c
>> @@ -432,6 +432,8 @@ static int rcar_gen3_thermal_request_irqs(struct rcar_gen3_thermal_priv *priv,
>>                 irq = platform_get_irq_optional(pdev, i);
>>                 if (irq < 0)
>>                         return irq;
>> +               if (!irq)
>> +                       return -ENXIO;
> 
> While correct, and preserving existing behavior, this looks strange
> to me.  Probably this should return zero instead (i.e. the check
> above should be changed to "<= 0"), and the caller should start caring
> about and propagating up real errors.

   Hm, you're right... should be <= 0 there, it seems.

> As this adds missing error handling, this is to be fixed by a separate
> patch later?

   Propagating errors from the probe() method is a matter of separate patch, yes.

>>
>>                 irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:ch%d",
>>                                          dev_name(dev), i);
>> diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
>> index fb65dc601b23..328ab074fd89 100644
> 
>> --- a/drivers/tty/serial/sh-sci.c
>> +++ b/drivers/tty/serial/sh-sci.c
> 
> I think you missed
> 
>     #define SCIx_IRQ_IS_MUXED(port)                 \
>             ((port)->irqs[SCIx_ERI_IRQ] ==  \
>              (port)->irqs[SCIx_RXI_IRQ]) || \
>             ((port)->irqs[SCIx_ERI_IRQ] &&  \
>              ((port)->irqs[SCIx_RXI_IRQ] < 0))
> 
> above? The last condition should become "<= 0".

   Yes, probably... TY!

> Gr{oetje,eeting}s,
> 
>                         Geert

MBR, Sergey

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v2 2/2] platform: make platform_get_irq_byname_optional() optional

[Sergey Shtylyov]

Currently platform_get_irq_byname_optional() returns an error code even
if IRQ resource simply has not been found.  It prevents the callers from
being error code agnostic in their error handling:

	ret = platform_get_irq_byname_optional(...);
	if (ret < 0 && ret != -ENXIO)
		return ret; // respect deferred probe
	if (ret > 0)
		...we get an IRQ...

All other *_optional() APIs seem to return 0 or NULL in case an optional
resource is not available.  Let's follow this good example, so that the
callers would look like:

	ret = platform_get_irq_byname_optional(...);
	if (ret < 0)
		return ret;
	if (ret > 0)
		...we get an IRQ...

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
Changes in version 2:
- added the error check using dev_err_probe() to
  platform_get_irq_byname_optional();
- reformatted the patch description.

 drivers/base/platform.c                         | 17 ++++++++++++++---
 drivers/gpu/drm/lima/lima_device.c              |  2 +-
 drivers/mailbox/tegra-hsp.c                     |  4 ++--
 drivers/net/can/rcar/rcar_canfd.c               |  4 ++--
 drivers/net/dsa/b53/b53_srab.c                  |  2 +-
 drivers/net/ethernet/freescale/fec_main.c       |  2 +-
 drivers/net/ethernet/freescale/fec_ptp.c        |  2 +-
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c   |  4 ++--
 .../ethernet/stmicro/stmmac/stmmac_platform.c   |  4 ++--
 .../pci/controller/dwc/pcie-designware-host.c   |  2 +-
 drivers/spi/spi-bcm-qspi.c                      |  2 +-
 drivers/spi/spi-rspi.c                          |  8 ++++----
 drivers/usb/cdns3/cdns3-plat.c                  |  5 +----
 drivers/usb/host/xhci-mtk.c                     |  2 +-
 drivers/usb/mtu3/mtu3_core.c                    |  2 +-
 15 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 52a8356f8261..c704d51a6dd5 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -500,14 +500,25 @@ EXPORT_SYMBOL_GPL(platform_get_irq_byname);
  * @name: IRQ name
  *
  * Get an optional IRQ by name like platform_get_irq_byname(). Except that it
- * does not print an error message if an IRQ can not be obtained.
+ * does not print an error message if an IRQ can not be obtained and returns
+ * 0 when IRQ resource has not been found.
  *
- * Return: non-zero IRQ number on success, negative error number on failure.
+ * Return: non-zero IRQ number on success, 0 if IRQ wasn't found, negative error
+ * number on failure.
  */
 int platform_get_irq_byname_optional(struct platform_device *dev,
 				     const char *name)
 {
-	return __platform_get_irq_byname(dev, name);
+	int ret;
+
+	ret = __platform_get_irq_byname(dev, name);
+	if (ret == -ENXIO)
+		return 0;
+	if (ret < 0)
+		return dev_err_probe(&dev->dev, ret, "IRQ %s not found\n",
+				     name);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(platform_get_irq_byname_optional);
 
diff --git a/drivers/gpu/drm/lima/lima_device.c b/drivers/gpu/drm/lima/lima_device.c
index 02cef0cea657..08a86484ce6f 100644
--- a/drivers/gpu/drm/lima/lima_device.c
+++ b/drivers/gpu/drm/lima/lima_device.c
@@ -224,7 +224,7 @@ static int lima_init_ip(struct lima_device *dev, int index)
 	if (irq_name) {
 		err = must ? platform_get_irq_byname(pdev, irq_name) :
 			     platform_get_irq_byname_optional(pdev, irq_name);
-		if (err < 0)
+		if (err <= 0)
 			goto out;
 		ip->irq = err;
 	}
diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index acd0675da681..17aa88e31445 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -667,7 +667,7 @@ static int tegra_hsp_probe(struct platform_device *pdev)
 	hsp->num_si = (value >> HSP_nSI_SHIFT) & HSP_nINT_MASK;
 
 	err = platform_get_irq_byname_optional(pdev, "doorbell");
-	if (err >= 0)
+	if (err > 0)
 		hsp->doorbell_irq = err;
 
 	if (hsp->num_si > 0) {
@@ -687,7 +687,7 @@ static int tegra_hsp_probe(struct platform_device *pdev)
 				return -ENOMEM;
 
 			err = platform_get_irq_byname_optional(pdev, name);
-			if (err >= 0) {
+			if (err > 0) {
 				hsp->shared_irqs[i] = err;
 				count++;
 			}
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index b7dc1c32875f..fa163ec0db80 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1772,7 +1772,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 
 	if (chip_id == RENESAS_RCAR_GEN3) {
 		ch_irq = platform_get_irq_byname_optional(pdev, "ch_int");
-		if (ch_irq < 0) {
+		if (ch_irq <= 0) {
 			/* For backward compatibility get irq by index */
 			ch_irq = platform_get_irq(pdev, 0);
 			if (ch_irq < 0)
@@ -1780,7 +1780,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		}
 
 		g_irq = platform_get_irq_byname_optional(pdev, "g_int");
-		if (g_irq < 0) {
+		if (g_irq <= 0) {
 			/* For backward compatibility get irq by index */
 			g_irq = platform_get_irq(pdev, 1);
 			if (g_irq < 0)
diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
index 4591bb1c05d2..80b7c8f053ad 100644
--- a/drivers/net/dsa/b53/b53_srab.c
+++ b/drivers/net/dsa/b53/b53_srab.c
@@ -420,7 +420,7 @@ static int b53_srab_irq_enable(struct b53_device *dev, int port)
 	/* Interrupt is optional and was not specified, do not make
 	 * this fatal
 	 */
-	if (p->irq == -ENXIO)
+	if (!p->irq)
 		return ret;
 
 	ret = request_threaded_irq(p->irq, b53_srab_port_isr,
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 796133de527e..93d1cca831dd 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3941,7 +3941,7 @@ fec_probe(struct platform_device *pdev)
 	for (i = 0; i < irq_cnt; i++) {
 		snprintf(irq_name, sizeof(irq_name), "int%d", i);
 		irq = platform_get_irq_byname_optional(pdev, irq_name);
-		if (irq < 0)
+		if (irq <= 0)
 			irq = platform_get_irq(pdev, i);
 		if (irq < 0) {
 			ret = irq;
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index de1d23808b6c..a7ef0aaaf2be 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -611,7 +611,7 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx)
 	INIT_DELAYED_WORK(&fep->time_keep, fec_time_keep);
 
 	irq = platform_get_irq_byname_optional(pdev, "pps");
-	if (irq < 0)
+	if (irq <= 0)
 		irq = platform_get_irq_optional(pdev, irq_idx);
 	/* Failure to get an irq is not fatal,
 	 * only the PTP_CLOCK_PPS clock events should stop
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 2b38a499a404..5519b5b35365 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -342,7 +342,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
 	if (dwmac->irq_pwr_wakeup == -EPROBE_DEFER)
 		return -EPROBE_DEFER;
 
-	if (!dwmac->clk_eth_ck && dwmac->irq_pwr_wakeup >= 0) {
+	if (!dwmac->clk_eth_ck && dwmac->irq_pwr_wakeup > 0) {
 		err = device_init_wakeup(&pdev->dev, true);
 		if (err) {
 			dev_err(&pdev->dev, "Failed to init wake up irq\n");
@@ -426,7 +426,7 @@ static int stm32_dwmac_remove(struct platform_device *pdev)
 
 	stm32_dwmac_clk_disable(priv->plat->bsp_priv);
 
-	if (dwmac->irq_pwr_wakeup >= 0) {
+	if (dwmac->irq_pwr_wakeup > 0) {
 		dev_pm_clear_wake_irq(&pdev->dev);
 		device_init_wakeup(&pdev->dev, false);
 	}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 5d29f336315b..33fdfab93aa6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -679,7 +679,7 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
 	 */
 	stmmac_res->wol_irq =
 		platform_get_irq_byname_optional(pdev, "eth_wake_irq");
-	if (stmmac_res->wol_irq < 0) {
+	if (stmmac_res->wol_irq <= 0) {
 		if (stmmac_res->wol_irq == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
 		dev_info(&pdev->dev, "IRQ eth_wake_irq not found\n");
@@ -688,7 +688,7 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
 
 	stmmac_res->lpi_irq =
 		platform_get_irq_byname_optional(pdev, "eth_lpi");
-	if (stmmac_res->lpi_irq < 0) {
+	if (stmmac_res->lpi_irq <= 0) {
 		if (stmmac_res->lpi_irq == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
 		dev_info(&pdev->dev, "IRQ eth_lpi not found\n");
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index f4755f3a03be..00e1a33fd06d 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -364,7 +364,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		} else if (pp->has_msi_ctrl) {
 			if (!pp->msi_irq) {
 				pp->msi_irq = platform_get_irq_byname_optional(pdev, "msi");
-				if (pp->msi_irq < 0) {
+				if (pp->msi_irq <= 0) {
 					pp->msi_irq = platform_get_irq(pdev, 0);
 					if (pp->msi_irq < 0)
 						return pp->msi_irq;
diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c
index c9a769b8594b..21fb80291d03 100644
--- a/drivers/spi/spi-bcm-qspi.c
+++ b/drivers/spi/spi-bcm-qspi.c
@@ -1627,7 +1627,7 @@ int bcm_qspi_probe(struct platform_device *pdev,
 			irq = platform_get_irq(pdev, 0);
 		}
 
-		if (irq  >= 0) {
+		if (irq > 0) {
 			ret = devm_request_irq(&pdev->dev, irq,
 					       qspi_irq_tab[val].irq_handler, 0,
 					       name,
diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index bd5708d7e5a1..1ddbd30e68d5 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -1355,16 +1355,16 @@ static int rspi_probe(struct platform_device *pdev)
 	ctlr->max_native_cs = rspi->ops->num_hw_ss;
 
 	ret = platform_get_irq_byname_optional(pdev, "rx");
-	if (ret < 0) {
+	if (ret <= 0) {
 		ret = platform_get_irq_byname_optional(pdev, "mux");
-		if (ret < 0)
+		if (ret <= 0)
 			ret = platform_get_irq(pdev, 0);
-		if (ret >= 0)
+		if (ret > 0)
 			rspi->rx_irq = rspi->tx_irq = ret;
 	} else {
 		rspi->rx_irq = ret;
 		ret = platform_get_irq_byname(pdev, "tx");
-		if (ret >= 0)
+		if (ret > 0)
 			rspi->tx_irq = ret;
 	}
 
diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
index dc068e940ed5..3a0bdf726af2 100644
--- a/drivers/usb/cdns3/cdns3-plat.c
+++ b/drivers/usb/cdns3/cdns3-plat.c
@@ -110,10 +110,7 @@ static int cdns3_plat_probe(struct platform_device *pdev)
 	cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");
 	if (cdns->wakeup_irq == -EPROBE_DEFER)
 		return cdns->wakeup_irq;
-	else if (cdns->wakeup_irq == 0)
-		return -EINVAL;
-
-	if (cdns->wakeup_irq < 0) {
+	if (cdns->wakeup_irq <= 0) {
 		dev_dbg(dev, "couldn't get wakeup irq\n");
 		cdns->wakeup_irq = 0x0;
 	}
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 91738af0ab14..19c742bf3ce0 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -493,7 +493,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		return ret;
 
 	irq = platform_get_irq_byname_optional(pdev, "host");
-	if (irq < 0) {
+	if (irq <= 0) {
 		if (irq == -EPROBE_DEFER)
 			return irq;
 
diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
index c4a2c37abf62..08173c05a1d6 100644
--- a/drivers/usb/mtu3/mtu3_core.c
+++ b/drivers/usb/mtu3/mtu3_core.c
@@ -925,7 +925,7 @@ int ssusb_gadget_init(struct ssusb_mtk *ssusb)
 		return -ENOMEM;
 
 	mtu->irq = platform_get_irq_byname_optional(pdev, "device");
-	if (mtu->irq < 0) {
+	if (mtu->irq <= 0) {
 		if (mtu->irq == -EPROBE_DEFER)
 			return mtu->irq;
 
-- 
2.26.3


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek