| Message ID | 20220113194358.xnnbhsoyetihterb@pengutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent() | expand |
On Thu, Jan 13, 2022 at 08:43:58PM +0100, Uwe Kleine-König wrote: > The subsystems regulator, clk and gpio have the concept of a dummy > resource. For regulator, clk and gpio there is a semantic difference > between the regular _get() function and the _get_optional() variant. > (One might return the dummy resource, the other won't. Unfortunately > which one implements which isn't the same for these three.) The > difference between platform_get_irq() and platform_get_irq_optional() is > only that the former might emit an error message and the later won't. Reviewed-by: Mark Brown <broonie@kernel.org>
On Thu, Jan 13, 2022 at 11:44 AM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > The subsystems regulator, clk and gpio have the concept of a dummy > resource. For regulator, clk and gpio there is a semantic difference > between the regular _get() function and the _get_optional() variant. > (One might return the dummy resource, the other won't. Unfortunately > which one implements which isn't the same for these three.) The > difference between platform_get_irq() and platform_get_irq_optional() is > only that the former might emit an error message and the later won't. > > To prevent people's expectations that there is a semantic difference > between these too, rename platform_get_irq_optional() to > platform_get_irq_silent() to make the actual difference more obvious. > > The #define for the old name can and should be removed once all patches > currently in flux still relying on platform_get_irq_optional() are > fixed. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> For watchdog: Acked-by: Guenter Roeck <groeck@chromium.org> > > --- > Hello, > > On Thu, Jan 13, 2022 at 02:45:30PM +0000, Mark Brown wrote: > > On Thu, Jan 13, 2022 at 12:08:31PM +0100, Uwe Kleine-König wrote: > > > > > This is all very unfortunate. In my eyes b) is the most sensible > > > sense, but the past showed that we don't agree here. (The most annoying > > > part of regulator_get is the warning that is emitted that regularily > > > makes customers ask what happens here and if this is fixable.) > > > > Fortunately it can be fixed, and it's safer to clearly specify things. > > The prints are there because when the description is wrong enough to > > cause things to blow up we can fail to boot or run messily and > > forgetting to describe some supplies (or typoing so they haven't done > > that) and people were having a hard time figuring out what might've > > happened. > > Yes, that's right. I sent a patch for such a warning in 2019 and pinged > occationally. Still waiting for it to be merged :-\ > (https://lore.kernel.org/r/20190625100412.11815-1-u.kleine-koenig@pengutronix.de) > > > > I think at least c) is easy to resolve because > > > platform_get_irq_optional() isn't that old yet and mechanically > > > replacing it by platform_get_irq_silent() should be easy and safe. > > > And this is orthogonal to the discussion if -ENOXIO is a sensible return > > > value and if it's as easy as it could be to work with errors on irq > > > lookups. > > > > It'd certainly be good to name anything that doesn't correspond to one > > of the existing semantics for the API (!) something different rather > > than adding yet another potentially overloaded meaning. > > It seems we're (at least) three who agree about this. Here is a patch > fixing the name. > > Best regards > Uwe > > drivers/base/platform.c | 12 ++++++------ > drivers/char/ipmi/bt-bmc.c | 2 +- > drivers/char/ipmi/ipmi_si_platform.c | 4 ++-- > drivers/char/tpm/tpm_tis.c | 2 +- > drivers/counter/interrupt-cnt.c | 2 +- > drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- > drivers/dma/mmp_pdma.c | 2 +- > drivers/edac/xgene_edac.c | 2 +- > drivers/gpio/gpio-altera.c | 2 +- > drivers/gpio/gpio-dwapb.c | 2 +- > drivers/gpio/gpio-mvebu.c | 2 +- > drivers/gpio/gpio-realtek-otto.c | 2 +- > drivers/gpio/gpio-tqmx86.c | 2 +- > drivers/gpio/gpio-xilinx.c | 2 +- > drivers/gpu/drm/v3d/v3d_irq.c | 2 +- > drivers/i2c/busses/i2c-brcmstb.c | 2 +- > drivers/i2c/busses/i2c-ocores.c | 2 +- > drivers/i2c/busses/i2c-pca-platform.c | 2 +- > drivers/irqchip/irq-renesas-intc-irqpin.c | 2 +- > drivers/irqchip/irq-renesas-irqc.c | 2 +- > drivers/mfd/intel_pmc_bxt.c | 2 +- > drivers/mmc/host/sh_mmcif.c | 2 +- > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 2 +- > drivers/mtd/nand/raw/renesas-nand-controller.c | 2 +- > drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +- > drivers/net/ethernet/davicom/dm9000.c | 2 +- > drivers/net/ethernet/freescale/fec_ptp.c | 2 +- > drivers/net/ethernet/marvell/mvmdio.c | 2 +- > drivers/net/ethernet/ti/davinci_emac.c | 2 +- > drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 4 ++-- > drivers/perf/arm_smmuv3_pmu.c | 2 +- > drivers/phy/renesas/phy-rcar-gen3-usb2.c | 2 +- > drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +- > drivers/pinctrl/intel/pinctrl-baytrail.c | 2 +- > drivers/pinctrl/intel/pinctrl-lynxpoint.c | 2 +- > drivers/pinctrl/pinctrl-keembay.c | 2 +- > drivers/pinctrl/samsung/pinctrl-samsung.c | 2 +- > drivers/platform/chrome/cros_ec_lpc.c | 2 +- > drivers/platform/x86/hp_accel.c | 2 +- > drivers/platform/x86/intel/punit_ipc.c | 2 +- > drivers/platform/x86/intel_scu_pltdrv.c | 2 +- > drivers/power/supply/mp2629_charger.c | 2 +- > drivers/rtc/rtc-m48t59.c | 2 +- > drivers/spi/spi-hisi-sfc-v3xx.c | 2 +- > drivers/spi/spi-mtk-nor.c | 2 +- > drivers/thermal/rcar_gen3_thermal.c | 2 +- > drivers/tty/serial/8250/8250_mtk.c | 2 +- > drivers/tty/serial/altera_jtaguart.c | 2 +- > drivers/tty/serial/altera_uart.c | 2 +- > drivers/tty/serial/imx.c | 4 ++-- > drivers/tty/serial/qcom_geni_serial.c | 2 +- > drivers/tty/serial/sh-sci.c | 2 +- > drivers/uio/uio_pdrv_genirq.c | 2 +- > drivers/usb/phy/phy-tegra-usb.c | 2 +- > drivers/vfio/platform/vfio_platform.c | 2 +- > drivers/watchdog/dw_wdt.c | 2 +- > drivers/watchdog/orion_wdt.c | 4 ++-- > drivers/watchdog/qcom-wdt.c | 2 +- > include/linux/platform_device.h | 9 ++++++++- > sound/soc/dwc/dwc-i2s.c | 2 +- > sound/soc/intel/keembay/kmb_platform.c | 2 +- > 61 files changed, 77 insertions(+), 70 deletions(-) > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index 6cb04ac48bf0..acb9962b9889 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -149,7 +149,7 @@ EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname); > #endif /* CONFIG_HAS_IOMEM */ > > /** > - * platform_get_irq_optional - get an optional IRQ for a device > + * platform_get_irq_silent - get an optional IRQ for a device > * @dev: platform device > * @num: IRQ number index > * > @@ -160,13 +160,13 @@ EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname); > * > * For example:: > * > - * int irq = platform_get_irq_optional(pdev, 0); > + * int irq = platform_get_irq_silent(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) > +int platform_get_irq_silent(struct platform_device *dev, unsigned int num) > { > int ret; > #ifdef CONFIG_SPARC > @@ -234,7 +234,7 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num) > WARN(ret == 0, "0 is an invalid IRQ number\n"); > return ret; > } > -EXPORT_SYMBOL_GPL(platform_get_irq_optional); > +EXPORT_SYMBOL_GPL(platform_get_irq_silent); > > /** > * platform_get_irq - get an IRQ for a device > @@ -257,7 +257,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_silent(dev, num); > if (ret < 0) > return dev_err_probe(&dev->dev, ret, > "IRQ index %u not found\n", num); > @@ -276,7 +276,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_silent(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..73bdbc59c9d0 100644 > --- a/drivers/char/ipmi/bt-bmc.c > +++ b/drivers/char/ipmi/bt-bmc.c > @@ -379,7 +379,7 @@ static int bt_bmc_config_irq(struct bt_bmc *bt_bmc, > int rc; > u32 reg; > > - bt_bmc->irq = platform_get_irq_optional(pdev, 0); > + bt_bmc->irq = platform_get_irq_silent(pdev, 0); > if (bt_bmc->irq < 0) > return bt_bmc->irq; > > diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c > index 505cc978c97a..4c666eed24d9 100644 > --- a/drivers/char/ipmi/ipmi_si_platform.c > +++ b/drivers/char/ipmi/ipmi_si_platform.c > @@ -192,7 +192,7 @@ static int platform_ipmi_probe(struct platform_device *pdev) > else > io.slave_addr = slave_addr; > > - io.irq = platform_get_irq_optional(pdev, 0); > + io.irq = platform_get_irq_silent(pdev, 0); > if (io.irq > 0) > io.irq_setup = ipmi_std_irq_setup; > else > @@ -368,7 +368,7 @@ static int acpi_ipmi_probe(struct platform_device *pdev) > io.irq = tmp; > io.irq_setup = acpi_gpe_irq_setup; > } else { > - int irq = platform_get_irq_optional(pdev, 0); > + int irq = platform_get_irq_silent(pdev, 0); > > if (irq > 0) { > io.irq = irq; > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index d3f2e5364c27..3e6785ad62f2 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -318,7 +318,7 @@ static int tpm_tis_plat_probe(struct platform_device *pdev) > } > tpm_info.res = *res; > > - tpm_info.irq = platform_get_irq_optional(pdev, 0); > + tpm_info.irq = platform_get_irq_silent(pdev, 0); > if (tpm_info.irq <= 0) { > if (pdev != force_pdev) > tpm_info.irq = -1; > diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c > index 8514a87fcbee..65b9254e63a9 100644 > --- a/drivers/counter/interrupt-cnt.c > +++ b/drivers/counter/interrupt-cnt.c > @@ -155,7 +155,7 @@ static int interrupt_cnt_probe(struct platform_device *pdev) > if (!priv) > return -ENOMEM; > > - priv->irq = platform_get_irq_optional(pdev, 0); > + priv->irq = platform_get_irq_silent(pdev, 0); > if (priv->irq == -ENXIO) > priv->irq = 0; > else if (priv->irq < 0) > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index 05f3d7876e44..3d1fe9ba98a7 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -374,7 +374,7 @@ static int qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy, int index) > * Look for LMh interrupt. If no interrupt line is specified / > * if there is an error, allow cpufreq to be enabled as usual. > */ > - data->throttle_irq = platform_get_irq_optional(pdev, index); > + data->throttle_irq = platform_get_irq_silent(pdev, index); > if (data->throttle_irq == -ENXIO) > return 0; > if (data->throttle_irq < 0) > diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c > index a23563cd118b..707ac21652a6 100644 > --- a/drivers/dma/mmp_pdma.c > +++ b/drivers/dma/mmp_pdma.c > @@ -1059,7 +1059,7 @@ static int mmp_pdma_probe(struct platform_device *op) > pdev->dma_channels = dma_channels; > > for (i = 0; i < dma_channels; i++) { > - if (platform_get_irq_optional(op, i) > 0) > + if (platform_get_irq_silent(op, i) > 0) > irq_num++; > } > > diff --git a/drivers/edac/xgene_edac.c b/drivers/edac/xgene_edac.c > index 2ccd1db5e98f..87aa537f3b72 100644 > --- a/drivers/edac/xgene_edac.c > +++ b/drivers/edac/xgene_edac.c > @@ -1916,7 +1916,7 @@ static int xgene_edac_probe(struct platform_device *pdev) > int i; > > for (i = 0; i < 3; i++) { > - irq = platform_get_irq_optional(pdev, i); > + irq = platform_get_irq_silent(pdev, i); > if (irq < 0) { > dev_err(&pdev->dev, "No IRQ resource\n"); > rc = -EINVAL; > diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c > index b59fae993626..a1a7d8c0ef13 100644 > --- a/drivers/gpio/gpio-altera.c > +++ b/drivers/gpio/gpio-altera.c > @@ -266,7 +266,7 @@ static int altera_gpio_probe(struct platform_device *pdev) > altera_gc->mmchip.gc.owner = THIS_MODULE; > altera_gc->mmchip.gc.parent = &pdev->dev; > > - altera_gc->mapped_irq = platform_get_irq_optional(pdev, 0); > + altera_gc->mapped_irq = platform_get_irq_silent(pdev, 0); > > if (altera_gc->mapped_irq < 0) > goto skip_irq; > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c > index b0f3aca61974..133153a40808 100644 > --- a/drivers/gpio/gpio-dwapb.c > +++ b/drivers/gpio/gpio-dwapb.c > @@ -543,7 +543,7 @@ static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode, > > for (j = 0; j < pp->ngpio; j++) { > if (has_acpi_companion(dev)) > - irq = platform_get_irq_optional(to_platform_device(dev), j); > + irq = platform_get_irq_silent(to_platform_device(dev), j); > else > irq = fwnode_irq_get(fwnode, j); > if (irq > 0) > diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c > index 4c1f9e1091b7..eaaf6fd54d79 100644 > --- a/drivers/gpio/gpio-mvebu.c > +++ b/drivers/gpio/gpio-mvebu.c > @@ -1291,7 +1291,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev) > * pins. > */ > for (i = 0; i < 4; i++) { > - int irq = platform_get_irq_optional(pdev, i); > + int irq = platform_get_irq_silent(pdev, i); > > if (irq < 0) > continue; > diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-otto.c > index bd75401b549d..b945049c1a39 100644 > --- a/drivers/gpio/gpio-realtek-otto.c > +++ b/drivers/gpio/gpio-realtek-otto.c > @@ -289,7 +289,7 @@ static int realtek_gpio_probe(struct platform_device *pdev) > ctrl->gc.ngpio = ngpios; > ctrl->gc.owner = THIS_MODULE; > > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (!(dev_flags & GPIO_INTERRUPTS_DISABLED) && irq > 0) { > girq = &ctrl->gc.irq; > girq->chip = &realtek_gpio_irq_chip; > diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c > index 5b103221b58d..16afb563f813 100644 > --- a/drivers/gpio/gpio-tqmx86.c > +++ b/drivers/gpio/gpio-tqmx86.c > @@ -236,7 +236,7 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) > struct resource *res; > int ret, irq; > > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq < 0 && irq != -ENXIO) > return irq; > > diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c > index b6d3a57e27ed..a451bd19d501 100644 > --- a/drivers/gpio/gpio-xilinx.c > +++ b/drivers/gpio/gpio-xilinx.c > @@ -658,7 +658,7 @@ static int xgpio_probe(struct platform_device *pdev) > > xgpio_save_regs(chip); > > - chip->irq = platform_get_irq_optional(pdev, 0); > + chip->irq = platform_get_irq_silent(pdev, 0); > if (chip->irq <= 0) > goto skip_irq; > > diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c > index e714d5318f30..8c88a1958fd4 100644 > --- a/drivers/gpu/drm/v3d/v3d_irq.c > +++ b/drivers/gpu/drm/v3d/v3d_irq.c > @@ -214,7 +214,7 @@ v3d_irq_init(struct v3d_dev *v3d) > V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS); > V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS); > > - irq1 = platform_get_irq_optional(v3d_to_pdev(v3d), 1); > + irq1 = platform_get_irq_silent(v3d_to_pdev(v3d), 1); > if (irq1 == -EPROBE_DEFER) > return irq1; > if (irq1 > 0) { > diff --git a/drivers/i2c/busses/i2c-brcmstb.c b/drivers/i2c/busses/i2c-brcmstb.c > index 490ee3962645..0e53d149a207 100644 > --- a/drivers/i2c/busses/i2c-brcmstb.c > +++ b/drivers/i2c/busses/i2c-brcmstb.c > @@ -646,7 +646,7 @@ static int brcmstb_i2c_probe(struct platform_device *pdev) > int_name = NULL; > > /* Get the interrupt number */ > - dev->irq = platform_get_irq_optional(pdev, 0); > + dev->irq = platform_get_irq_silent(pdev, 0); > > /* disable the bsc interrupt line */ > brcmstb_i2c_enable_disable_irq(dev, INT_DISABLE); > diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c > index a0af027db04c..c6d21b833964 100644 > --- a/drivers/i2c/busses/i2c-ocores.c > +++ b/drivers/i2c/busses/i2c-ocores.c > @@ -682,7 +682,7 @@ static int ocores_i2c_probe(struct platform_device *pdev) > > init_waitqueue_head(&i2c->wait); > > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > /* > * Since the SoC does have an interrupt, its DT has an interrupt > * property - But this should be bypassed as the IRQ logic in this > diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c > index 86d4f75ef8d3..783b474097f7 100644 > --- a/drivers/i2c/busses/i2c-pca-platform.c > +++ b/drivers/i2c/busses/i2c-pca-platform.c > @@ -138,7 +138,7 @@ static int i2c_pca_pf_probe(struct platform_device *pdev) > int ret = 0; > int irq; > > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > /* If irq is 0, we do polling. */ > if (irq < 0) > irq = 0; > diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c > index 37f9a4499fdb..934669b20d0d 100644 > --- a/drivers/irqchip/irq-renesas-intc-irqpin.c > +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c > @@ -417,7 +417,7 @@ static int intc_irqpin_probe(struct platform_device *pdev) > > /* allow any number of IRQs between 1 and INTC_IRQPIN_MAX */ > for (k = 0; k < INTC_IRQPIN_MAX; k++) { > - ret = platform_get_irq_optional(pdev, k); > + ret = platform_get_irq_silent(pdev, k); > if (ret == -ENXIO) > break; > if (ret < 0) > diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c > index 909325f88239..95ff42746e95 100644 > --- a/drivers/irqchip/irq-renesas-irqc.c > +++ b/drivers/irqchip/irq-renesas-irqc.c > @@ -141,7 +141,7 @@ static int irqc_probe(struct platform_device *pdev) > > /* allow any number of IRQs between 1 and IRQC_IRQ_MAX */ > for (k = 0; k < IRQC_IRQ_MAX; k++) { > - ret = platform_get_irq_optional(pdev, k); > + ret = platform_get_irq_silent(pdev, k); > if (ret == -ENXIO) > break; > if (ret < 0) > diff --git a/drivers/mfd/intel_pmc_bxt.c b/drivers/mfd/intel_pmc_bxt.c > index 9f01d38acc7f..dc58dfd87043 100644 > --- a/drivers/mfd/intel_pmc_bxt.c > +++ b/drivers/mfd/intel_pmc_bxt.c > @@ -309,7 +309,7 @@ static int intel_pmc_get_resources(struct platform_device *pdev, > struct resource *res; > int ret; > > - scu_data->irq = platform_get_irq_optional(pdev, 0); > + scu_data->irq = platform_get_irq_silent(pdev, 0); > > res = platform_get_resource(pdev, IORESOURCE_MEM, > PLAT_RESOURCE_IPC_INDEX); > diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c > index bcc595c70a9f..aa5579520b06 100644 > --- a/drivers/mmc/host/sh_mmcif.c > +++ b/drivers/mmc/host/sh_mmcif.c > @@ -1396,7 +1396,7 @@ static int sh_mmcif_probe(struct platform_device *pdev) > const char *name; > > irq[0] = platform_get_irq(pdev, 0); > - irq[1] = platform_get_irq_optional(pdev, 1); > + irq[1] = platform_get_irq_silent(pdev, 1); > if (irq[0] < 0) > return -ENXIO; > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > index f75929783b94..2aa10a1755ba 100644 > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > @@ -2955,7 +2955,7 @@ static int brcmnand_edu_setup(struct platform_device *pdev) > /* initialize edu */ > brcmnand_edu_init(ctrl); > > - ctrl->edu_irq = platform_get_irq_optional(pdev, 1); > + ctrl->edu_irq = platform_get_irq_silent(pdev, 1); > if (ctrl->edu_irq < 0) { > dev_warn(dev, > "FLASH EDU enabled, using ctlrdy irq\n"); > diff --git a/drivers/mtd/nand/raw/renesas-nand-controller.c b/drivers/mtd/nand/raw/renesas-nand-controller.c > index 428e08362956..c33958bda059 100644 > --- a/drivers/mtd/nand/raw/renesas-nand-controller.c > +++ b/drivers/mtd/nand/raw/renesas-nand-controller.c > @@ -1354,7 +1354,7 @@ static int rnandc_probe(struct platform_device *pdev) > goto disable_hclk; > > rnandc_dis_interrupts(rnandc); > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq == -EPROBE_DEFER) { > ret = irq; > goto disable_eclk; > diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c > index 226f4403cfed..4cfc62a5380a 100644 > --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c > +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c > @@ -3989,7 +3989,7 @@ static int bcmgenet_probe(struct platform_device *pdev) > err = priv->irq1; > goto err; > } > - priv->wol_irq = platform_get_irq_optional(pdev, 2); > + priv->wol_irq = platform_get_irq_silent(pdev, 2); > > priv->base = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(priv->base)) { > diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c > index 0985ab216566..43f181e13bab 100644 > --- a/drivers/net/ethernet/davicom/dm9000.c > +++ b/drivers/net/ethernet/davicom/dm9000.c > @@ -1508,7 +1508,7 @@ dm9000_probe(struct platform_device *pdev) > goto out; > } > > - db->irq_wake = platform_get_irq_optional(pdev, 1); > + db->irq_wake = platform_get_irq_silent(pdev, 1); > if (db->irq_wake >= 0) { > dev_dbg(db->dev, "wakeup irq %d\n", db->irq_wake); > > diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c > index af99017a5453..dc65bb1caad4 100644 > --- a/drivers/net/ethernet/freescale/fec_ptp.c > +++ b/drivers/net/ethernet/freescale/fec_ptp.c > @@ -612,7 +612,7 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx) > > irq = platform_get_irq_byname_optional(pdev, "pps"); > if (irq < 0) > - irq = platform_get_irq_optional(pdev, irq_idx); > + irq = platform_get_irq_silent(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/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c > index ef878973b859..cdf4ff41bd66 100644 > --- a/drivers/net/ethernet/marvell/mvmdio.c > +++ b/drivers/net/ethernet/marvell/mvmdio.c > @@ -349,7 +349,7 @@ static int orion_mdio_probe(struct platform_device *pdev) > } > > > - dev->err_interrupt = platform_get_irq_optional(pdev, 0); > + dev->err_interrupt = platform_get_irq_silent(pdev, 0); > if (dev->err_interrupt > 0 && > resource_size(r) < MVMDIO_ERR_INT_MASK + 4) { > dev_err(&pdev->dev, > diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c > index 31df3267a01a..30d5a785d485 100644 > --- a/drivers/net/ethernet/ti/davinci_emac.c > +++ b/drivers/net/ethernet/ti/davinci_emac.c > @@ -1455,7 +1455,7 @@ static int emac_dev_open(struct net_device *ndev) > > /* Request IRQ */ > if (dev_of_node(&priv->pdev->dev)) { > - while ((ret = platform_get_irq_optional(priv->pdev, res_num)) != -ENXIO) { > + while ((ret = platform_get_irq_silent(priv->pdev, res_num)) != -ENXIO) { > if (ret < 0) > goto rollback; > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > index 23ac353b35fe..5be7e93d4087 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > @@ -1961,13 +1961,13 @@ static int axienet_probe(struct platform_device *pdev) > lp->rx_irq = irq_of_parse_and_map(np, 1); > lp->tx_irq = irq_of_parse_and_map(np, 0); > of_node_put(np); > - lp->eth_irq = platform_get_irq_optional(pdev, 0); > + lp->eth_irq = platform_get_irq_silent(pdev, 0); > } else { > /* Check for these resources directly on the Ethernet node. */ > lp->dma_regs = devm_platform_get_and_ioremap_resource(pdev, 1, NULL); > lp->rx_irq = platform_get_irq(pdev, 1); > lp->tx_irq = platform_get_irq(pdev, 0); > - lp->eth_irq = platform_get_irq_optional(pdev, 2); > + lp->eth_irq = platform_get_irq_silent(pdev, 2); > } > if (IS_ERR(lp->dma_regs)) { > dev_err(&pdev->dev, "could not map DMA regs\n"); > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c > index c49108a72865..c4a709470398 100644 > --- a/drivers/perf/arm_smmuv3_pmu.c > +++ b/drivers/perf/arm_smmuv3_pmu.c > @@ -852,7 +852,7 @@ static int smmu_pmu_probe(struct platform_device *pdev) > smmu_pmu->reloc_base = smmu_pmu->reg_base; > } > > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq > 0) > smmu_pmu->irq = irq; > > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > index 9de617ca9daa..9cbd4e396f0f 100644 > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > @@ -672,7 +672,7 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) > > channel->obint_enable_bits = USB2_OBINT_BITS; > /* get irq number here and request_irq for OTG in phy_init */ > - channel->irq = platform_get_irq_optional(pdev, 0); > + channel->irq = platform_get_irq_silent(pdev, 0); > channel->dr_mode = rcar_gen3_get_dr_mode(dev->of_node); > if (channel->dr_mode != USB_DR_MODE_UNKNOWN) { > int ret; > diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c > index 52fa2f4cd618..1acf9355ab44 100644 > --- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c > +++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c > @@ -848,7 +848,7 @@ static int iproc_gpio_probe(struct platform_device *pdev) > "gpio-ranges"); > > /* optional GPIO interrupt support */ > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq > 0) { > struct irq_chip *irqc; > struct gpio_irq_chip *girq; > diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c > index 4c01333e1406..cc5a74aea6e5 100644 > --- a/drivers/pinctrl/intel/pinctrl-baytrail.c > +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c > @@ -1568,7 +1568,7 @@ static int byt_gpio_probe(struct intel_pinctrl *vg) > #endif > > /* set up interrupts */ > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq > 0) { > struct gpio_irq_chip *girq; > > diff --git a/drivers/pinctrl/intel/pinctrl-lynxpoint.c b/drivers/pinctrl/intel/pinctrl-lynxpoint.c > index 561fa322b0b4..984c5c0b4304 100644 > --- a/drivers/pinctrl/intel/pinctrl-lynxpoint.c > +++ b/drivers/pinctrl/intel/pinctrl-lynxpoint.c > @@ -879,7 +879,7 @@ static int lp_gpio_probe(struct platform_device *pdev) > gc->parent = dev; > > /* set up interrupts */ > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq > 0) { > struct gpio_irq_chip *girq; > > diff --git a/drivers/pinctrl/pinctrl-keembay.c b/drivers/pinctrl/pinctrl-keembay.c > index 152c35bce8ec..628f642ec220 100644 > --- a/drivers/pinctrl/pinctrl-keembay.c > +++ b/drivers/pinctrl/pinctrl-keembay.c > @@ -1490,7 +1490,7 @@ static int keembay_gpiochip_probe(struct keembay_pinctrl *kpc, > struct keembay_gpio_irq *kmb_irq = &kpc->irq[i]; > int irq; > > - irq = platform_get_irq_optional(pdev, i); > + irq = platform_get_irq_silent(pdev, i); > if (irq <= 0) > continue; > > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c > index 0f6e9305fec5..47160ec0407c 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c > @@ -1108,7 +1108,7 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) > } > drvdata->dev = dev; > > - ret = platform_get_irq_optional(pdev, 0); > + ret = platform_get_irq_silent(pdev, 0); > if (ret < 0 && ret != -ENXIO) > return ret; > if (ret > 0) > diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c > index d6306d2a096f..30f06d4b6ad8 100644 > --- a/drivers/platform/chrome/cros_ec_lpc.c > +++ b/drivers/platform/chrome/cros_ec_lpc.c > @@ -397,7 +397,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev) > * Some boards do not have an IRQ allotted for cros_ec_lpc, > * which makes ENXIO an expected (and safe) scenario. > */ > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq > 0) > ec_dev->irq = irq; > else if (irq != -ENXIO) { > diff --git a/drivers/platform/x86/hp_accel.c b/drivers/platform/x86/hp_accel.c > index e9f852f7c27f..bffc9093a629 100644 > --- a/drivers/platform/x86/hp_accel.c > +++ b/drivers/platform/x86/hp_accel.c > @@ -305,7 +305,7 @@ static int lis3lv02d_probe(struct platform_device *device) > lis3_dev.write = lis3lv02d_acpi_write; > > /* obtain IRQ number of our device from ACPI */ > - ret = platform_get_irq_optional(device, 0); > + ret = platform_get_irq_silent(device, 0); > if (ret > 0) > lis3_dev.irq = ret; > > diff --git a/drivers/platform/x86/intel/punit_ipc.c b/drivers/platform/x86/intel/punit_ipc.c > index 66bb39fd0ef9..2f22d5de767a 100644 > --- a/drivers/platform/x86/intel/punit_ipc.c > +++ b/drivers/platform/x86/intel/punit_ipc.c > @@ -277,7 +277,7 @@ static int intel_punit_ipc_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, punit_ipcdev); > > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq < 0) { > dev_warn(&pdev->dev, "Invalid IRQ, using polling mode\n"); > } else { > diff --git a/drivers/platform/x86/intel_scu_pltdrv.c b/drivers/platform/x86/intel_scu_pltdrv.c > index 56ec6ae4c824..2d3e5174da8e 100644 > --- a/drivers/platform/x86/intel_scu_pltdrv.c > +++ b/drivers/platform/x86/intel_scu_pltdrv.c > @@ -23,7 +23,7 @@ static int intel_scu_platform_probe(struct platform_device *pdev) > struct intel_scu_ipc_dev *scu; > const struct resource *res; > > - scu_data.irq = platform_get_irq_optional(pdev, 0); > + scu_data.irq = platform_get_irq_silent(pdev, 0); > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!res) > return -ENOMEM; > diff --git a/drivers/power/supply/mp2629_charger.c b/drivers/power/supply/mp2629_charger.c > index bdf924b73e47..e279a4bdf6a4 100644 > --- a/drivers/power/supply/mp2629_charger.c > +++ b/drivers/power/supply/mp2629_charger.c > @@ -580,7 +580,7 @@ static int mp2629_charger_probe(struct platform_device *pdev) > charger->dev = dev; > platform_set_drvdata(pdev, charger); > > - irq = platform_get_irq_optional(to_platform_device(dev->parent), 0); > + irq = platform_get_irq_silent(to_platform_device(dev->parent), 0); > if (irq < 0) { > dev_err(dev, "get irq fail: %d\n", irq); > return irq; > diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c > index f0f6b9b6daec..aebc8a73acfe 100644 > --- a/drivers/rtc/rtc-m48t59.c > +++ b/drivers/rtc/rtc-m48t59.c > @@ -421,7 +421,7 @@ static int m48t59_rtc_probe(struct platform_device *pdev) > /* Try to get irq number. We also can work in > * the mode without IRQ. > */ > - m48t59->irq = platform_get_irq_optional(pdev, 0); > + m48t59->irq = platform_get_irq_silent(pdev, 0); > if (m48t59->irq <= 0) > m48t59->irq = NO_IRQ; > > diff --git a/drivers/spi/spi-hisi-sfc-v3xx.c b/drivers/spi/spi-hisi-sfc-v3xx.c > index d3a23b1c2a4c..76f4934a23e7 100644 > --- a/drivers/spi/spi-hisi-sfc-v3xx.c > +++ b/drivers/spi/spi-hisi-sfc-v3xx.c > @@ -451,7 +451,7 @@ static int hisi_sfc_v3xx_probe(struct platform_device *pdev) > goto err_put_master; > } > > - host->irq = platform_get_irq_optional(pdev, 0); > + host->irq = platform_get_irq_silent(pdev, 0); > if (host->irq == -EPROBE_DEFER) { > ret = -EPROBE_DEFER; > goto err_put_master; > diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c > index 5c93730615f8..64aec31355bb 100644 > --- a/drivers/spi/spi-mtk-nor.c > +++ b/drivers/spi/spi-mtk-nor.c > @@ -828,7 +828,7 @@ static int mtk_nor_probe(struct platform_device *pdev) > > mtk_nor_init(sp); > > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > > if (irq < 0) { > dev_warn(sp->dev, "IRQ not available."); > diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c > index 43eb25b167bc..ef6c6880a943 100644 > --- a/drivers/thermal/rcar_gen3_thermal.c > +++ b/drivers/thermal/rcar_gen3_thermal.c > @@ -429,7 +429,7 @@ static int rcar_gen3_thermal_request_irqs(struct rcar_gen3_thermal_priv *priv, > int ret, irq; > > for (i = 0; i < 2; i++) { > - irq = platform_get_irq_optional(pdev, i); > + irq = platform_get_irq_silent(pdev, i); > if (irq < 0) > return irq; > > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c > index fb65dc601b23..1f4cbe37627e 100644 > --- a/drivers/tty/serial/8250/8250_mtk.c > +++ b/drivers/tty/serial/8250/8250_mtk.c > @@ -585,7 +585,7 @@ static int mtk8250_probe(struct platform_device *pdev) > goto err_pm_disable; > } > > - data->rx_wakeup_irq = platform_get_irq_optional(pdev, 1); > + data->rx_wakeup_irq = platform_get_irq_silent(pdev, 1); > > return 0; > > diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c > index 37bffe406b18..1cd2bdee3d40 100644 > --- a/drivers/tty/serial/altera_jtaguart.c > +++ b/drivers/tty/serial/altera_jtaguart.c > @@ -439,7 +439,7 @@ static int altera_jtaguart_probe(struct platform_device *pdev) > else > return -ENODEV; > > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq < 0 && irq != -ENXIO) > return irq; > if (irq > 0) > diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c > index 64a352b40197..415883ccfbbd 100644 > --- a/drivers/tty/serial/altera_uart.c > +++ b/drivers/tty/serial/altera_uart.c > @@ -576,7 +576,7 @@ static int altera_uart_probe(struct platform_device *pdev) > else > return -EINVAL; > > - ret = platform_get_irq_optional(pdev, 0); > + ret = platform_get_irq_silent(pdev, 0); > if (ret < 0 && ret != -ENXIO) > return ret; > if (ret > 0) > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index df8a0c8b8b29..8791f51e52cb 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -2258,8 +2258,8 @@ static int imx_uart_probe(struct platform_device *pdev) > rxirq = platform_get_irq(pdev, 0); > if (rxirq < 0) > return rxirq; > - txirq = platform_get_irq_optional(pdev, 1); > - rtsirq = platform_get_irq_optional(pdev, 2); > + txirq = platform_get_irq_silent(pdev, 1); > + rtsirq = platform_get_irq_silent(pdev, 2); > > sport->port.dev = &pdev->dev; > sport->port.mapbase = res->start; > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > index aedc38893e6c..893922e520a9 100644 > --- a/drivers/tty/serial/qcom_geni_serial.c > +++ b/drivers/tty/serial/qcom_geni_serial.c > @@ -1406,7 +1406,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE); > > if (!console) > - port->wakeup_irq = platform_get_irq_optional(pdev, 1); > + port->wakeup_irq = platform_get_irq_silent(pdev, 1); > > if (of_property_read_bool(pdev->dev.of_node, "rx-tx-swap")) > port->rx_tx_swap = true; > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > index 968967d722d4..f2fb298b3aed 100644 > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -2860,7 +2860,7 @@ static int sci_init_single(struct platform_device *dev, > > for (i = 0; i < ARRAY_SIZE(sci_port->irqs); ++i) { > if (i) > - sci_port->irqs[i] = platform_get_irq_optional(dev, i); > + sci_port->irqs[i] = platform_get_irq_silent(dev, i); > else > sci_port->irqs[i] = platform_get_irq(dev, i); > } > diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c > index 63258b6accc4..a2673a8ebd3f 100644 > --- a/drivers/uio/uio_pdrv_genirq.c > +++ b/drivers/uio/uio_pdrv_genirq.c > @@ -160,7 +160,7 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev) > priv->pdev = pdev; > > if (!uioinfo->irq) { > - ret = platform_get_irq_optional(pdev, 0); > + ret = platform_get_irq_silent(pdev, 0); > uioinfo->irq = ret; > if (ret == -ENXIO) > uioinfo->irq = UIO_IRQ_NONE; > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c > index 68cd4b68e3a2..5237c62f60f0 100644 > --- a/drivers/usb/phy/phy-tegra-usb.c > +++ b/drivers/usb/phy/phy-tegra-usb.c > @@ -1353,7 +1353,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev) > return -ENOMEM; > > tegra_phy->soc_config = of_device_get_match_data(&pdev->dev); > - tegra_phy->irq = platform_get_irq_optional(pdev, 0); > + tegra_phy->irq = platform_get_irq_silent(pdev, 0); > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!res) { > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c > index 68a1c87066d7..60b4f5ce5aa1 100644 > --- a/drivers/vfio/platform/vfio_platform.c > +++ b/drivers/vfio/platform/vfio_platform.c > @@ -33,7 +33,7 @@ static int get_platform_irq(struct vfio_platform_device *vdev, int i) > { > struct platform_device *pdev = (struct platform_device *) vdev->opaque; > > - return platform_get_irq_optional(pdev, i); > + return platform_get_irq_silent(pdev, i); > } > > static int vfio_platform_probe(struct platform_device *pdev) > diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c > index cd578843277e..9c792ab66a83 100644 > --- a/drivers/watchdog/dw_wdt.c > +++ b/drivers/watchdog/dw_wdt.c > @@ -617,7 +617,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev) > * pending either until the next watchdog kick event or up to the > * system reset. > */ > - ret = platform_get_irq_optional(pdev, 0); > + ret = platform_get_irq_silent(pdev, 0); > if (ret > 0) { > ret = devm_request_irq(dev, ret, dw_wdt_irq, > IRQF_SHARED | IRQF_TRIGGER_RISING, > diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c > index 127eefc9161d..c533fbb37895 100644 > --- a/drivers/watchdog/orion_wdt.c > +++ b/drivers/watchdog/orion_wdt.c > @@ -602,7 +602,7 @@ static int orion_wdt_probe(struct platform_device *pdev) > set_bit(WDOG_HW_RUNNING, &dev->wdt.status); > > /* Request the IRQ only after the watchdog is disabled */ > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq > 0) { > /* > * Not all supported platforms specify an interrupt for the > @@ -617,7 +617,7 @@ static int orion_wdt_probe(struct platform_device *pdev) > } > > /* Optional 2nd interrupt for pretimeout */ > - irq = platform_get_irq_optional(pdev, 1); > + irq = platform_get_irq_silent(pdev, 1); > if (irq > 0) { > orion_wdt_info.options |= WDIOF_PRETIMEOUT; > ret = devm_request_irq(&pdev->dev, irq, orion_wdt_pre_irq, > diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c > index 0d2209c5eaca..f1bbfed047a1 100644 > --- a/drivers/watchdog/qcom-wdt.c > +++ b/drivers/watchdog/qcom-wdt.c > @@ -257,7 +257,7 @@ static int qcom_wdt_probe(struct platform_device *pdev) > } > > /* check if there is pretimeout support */ > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (data->pretimeout && irq > 0) { > ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0, > "wdt_bark", &wdt->wdd); > diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h > index 7c96f169d274..6d495f15f717 100644 > --- a/include/linux/platform_device.h > +++ b/include/linux/platform_device.h > @@ -69,7 +69,14 @@ extern void __iomem * > devm_platform_ioremap_resource_byname(struct platform_device *pdev, > const char *name); > extern int platform_get_irq(struct platform_device *, unsigned int); > -extern int platform_get_irq_optional(struct platform_device *, unsigned int); > +extern int platform_get_irq_silent(struct platform_device *, unsigned int); > + > +/* > + * platform_get_irq_optional was recently renamed to platform_get_irq_silent. > + * Fixup users to not break patches that were created before the rename. > + */ > +#define platform_get_irq_optional(pdev, index) platform_get_irq_silent(pdev, index) > + > extern int platform_irq_count(struct platform_device *); > extern int devm_platform_get_irqs_affinity(struct platform_device *dev, > struct irq_affinity *affd, > diff --git a/sound/soc/dwc/dwc-i2s.c b/sound/soc/dwc/dwc-i2s.c > index 5cb58929090d..f7cfe8f7cce0 100644 > --- a/sound/soc/dwc/dwc-i2s.c > +++ b/sound/soc/dwc/dwc-i2s.c > @@ -642,7 +642,7 @@ static int dw_i2s_probe(struct platform_device *pdev) > > dev->dev = &pdev->dev; > > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq >= 0) { > ret = devm_request_irq(&pdev->dev, irq, i2s_irq_handler, 0, > pdev->name, dev); > diff --git a/sound/soc/intel/keembay/kmb_platform.c b/sound/soc/intel/keembay/kmb_platform.c > index a6fb74ba1c42..ee0159b7e9f6 100644 > --- a/sound/soc/intel/keembay/kmb_platform.c > +++ b/sound/soc/intel/keembay/kmb_platform.c > @@ -882,7 +882,7 @@ static int kmb_plat_dai_probe(struct platform_device *pdev) > kmb_i2s->use_pio = !(of_property_read_bool(np, "dmas")); > > if (kmb_i2s->use_pio) { > - irq = platform_get_irq_optional(pdev, 0); > + irq = platform_get_irq_silent(pdev, 0); > if (irq > 0) { > ret = devm_request_irq(dev, irq, kmb_i2s_irq_handler, 0, > pdev->name, kmb_i2s); > -- > 2.34.1 > > > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
On 1/13/22 11:17 PM, Mark Brown wrote: >> The subsystems regulator, clk and gpio have the concept of a dummy >> resource. For regulator, clk and gpio there is a semantic difference >> between the regular _get() function and the _get_optional() variant. >> (One might return the dummy resource, the other won't. Unfortunately >> which one implements which isn't the same for these three.) The >> difference between platform_get_irq() and platform_get_irq_optional() is >> only that the former might emit an error message and the later won't. This is only a current difference but I'm still going to return 0 ISO -ENXIO from latform_get_irq_optional(), no way I'd leave that -ENXIO there alone... :-) > Reviewed-by: Mark Brown <broonie@kernel.org> Hm... I'm seeing a tag bit not seeing the patch itself... MBR, Sergey
On 1/13/2022 11:43 AM, Uwe Kleine-König wrote: > The subsystems regulator, clk and gpio have the concept of a dummy > resource. For regulator, clk and gpio there is a semantic difference > between the regular _get() function and the _get_optional() variant. > (One might return the dummy resource, the other won't. Unfortunately > which one implements which isn't the same for these three.) The > difference between platform_get_irq() and platform_get_irq_optional() is > only that the former might emit an error message and the later won't. > > To prevent people's expectations that there is a semantic difference > between these too, rename platform_get_irq_optional() to > platform_get_irq_silent() to make the actual difference more obvious. > > The #define for the old name can and should be removed once all patches > currently in flux still relying on platform_get_irq_optional() are > fixed. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Hello, > > On Thu, Jan 13, 2022 at 02:45:30PM +0000, Mark Brown wrote: >> On Thu, Jan 13, 2022 at 12:08:31PM +0100, Uwe Kleine-König wrote: >> >>> This is all very unfortunate. In my eyes b) is the most sensible >>> sense, but the past showed that we don't agree here. (The most annoying >>> part of regulator_get is the warning that is emitted that regularily >>> makes customers ask what happens here and if this is fixable.) >> >> Fortunately it can be fixed, and it's safer to clearly specify things. >> The prints are there because when the description is wrong enough to >> cause things to blow up we can fail to boot or run messily and >> forgetting to describe some supplies (or typoing so they haven't done >> that) and people were having a hard time figuring out what might've >> happened. > > Yes, that's right. I sent a patch for such a warning in 2019 and pinged > occationally. Still waiting for it to be merged :-\ > (https://lore.kernel.org/r/20190625100412.11815-1-u.kleine-koenig@pengutronix.de) > >>> I think at least c) is easy to resolve because >>> platform_get_irq_optional() isn't that old yet and mechanically >>> replacing it by platform_get_irq_silent() should be easy and safe. >>> And this is orthogonal to the discussion if -ENOXIO is a sensible return >>> value and if it's as easy as it could be to work with errors on irq >>> lookups. >> >> It'd certainly be good to name anything that doesn't correspond to one >> of the existing semantics for the API (!) something different rather >> than adding yet another potentially overloaded meaning. > > It seems we're (at least) three who agree about this. Here is a patch > fixing the name. From an API naming perspective this does not make much sense anymore with the name chosen, it is understood that whent he function is called platform_get_irq_optional(), optional applies to the IRQ. An optional IRQ is something people can reason about because it makes sense. What is a a "silent" IRQ however? It does not apply to the object it is trying to fetch to anymore, but to the message that may not be printed in case the resource failed to be obtained, because said resource is optional. Woah, that's quite a stretch. Following the discussion and original 2 patches set from Sergey, it is not entirely clear to me anymore what is it that we are trying to fix. I nearly forgot, I would paint it blue, sky blue, not navy blue, not light blue ;)
On Thu, Jan 13, 2022 at 11:57:43PM +0300, Sergey Shtylyov wrote: > On 1/13/22 11:17 PM, Mark Brown wrote: > > >> The subsystems regulator, clk and gpio have the concept of a dummy > >> resource. For regulator, clk and gpio there is a semantic difference > >> between the regular _get() function and the _get_optional() variant. > >> (One might return the dummy resource, the other won't. Unfortunately > >> which one implements which isn't the same for these three.) The > >> difference between platform_get_irq() and platform_get_irq_optional() is > >> only that the former might emit an error message and the later won't. > > This is only a current difference but I'm still going to return 0 ISO > -ENXIO from latform_get_irq_optional(), no way I'd leave that -ENXIO there > alone... :-) This would address a bit of the critic in my commit log. But as 0 isn't a dummy value like the dummy values that exist for clk, gpiod and regulator I still think that the naming is a bad idea because it's not in the spirit of the other *_get_optional functions. Seeing you say that -ENXIO is a bad return value for platform_get_irq_optional() and 0 should be used instead, I wonder why not changing platform_get_irq() to return 0 instead of -ENXIO, too. This question is for now only about a sensible semantic. That actually changing platform_get_irq() is probably harder than changing platform_get_irq_optional() is a different story. If only platform_get_irq_optional() is changed and given that the callers have to do something like: if (this_irq_exists()): ... (e.g. request_irq) else: ... (e.g. setup polling) I really think it's a bad idea that this_irq_exists() has to be different for platform_get_irq() vs. platform_get_irq_optional(). > > Reviewed-by: Mark Brown <broonie@kernel.org> > > Hm... I'm seeing a tag bit not seeing the patch itself... See https://lore.kernel.org/all/20220113194358.xnnbhsoyetihterb@pengutronix.de/ This is just a tree-wide s/platform_get_irq_optional/platform_get_irq_silent/ + a macro to not break callers of platform_get_irq_optional(). Best regards Uwe
On Thu, Jan 13, 2022 at 01:42:57PM -0800, Florian Fainelli wrote: > > > On 1/13/2022 11:43 AM, Uwe Kleine-König wrote: > > The subsystems regulator, clk and gpio have the concept of a dummy > > resource. For regulator, clk and gpio there is a semantic difference > > between the regular _get() function and the _get_optional() variant. > > (One might return the dummy resource, the other won't. Unfortunately > > which one implements which isn't the same for these three.) The > > difference between platform_get_irq() and platform_get_irq_optional() is > > only that the former might emit an error message and the later won't. > > > > To prevent people's expectations that there is a semantic difference > > between these too, rename platform_get_irq_optional() to > > platform_get_irq_silent() to make the actual difference more obvious. > > > > The #define for the old name can and should be removed once all patches > > currently in flux still relying on platform_get_irq_optional() are > > fixed. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Hello, > > > > On Thu, Jan 13, 2022 at 02:45:30PM +0000, Mark Brown wrote: > > > On Thu, Jan 13, 2022 at 12:08:31PM +0100, Uwe Kleine-König wrote: > > > > > > > This is all very unfortunate. In my eyes b) is the most sensible > > > > sense, but the past showed that we don't agree here. (The most annoying > > > > part of regulator_get is the warning that is emitted that regularily > > > > makes customers ask what happens here and if this is fixable.) > > > > > > Fortunately it can be fixed, and it's safer to clearly specify things. > > > The prints are there because when the description is wrong enough to > > > cause things to blow up we can fail to boot or run messily and > > > forgetting to describe some supplies (or typoing so they haven't done > > > that) and people were having a hard time figuring out what might've > > > happened. > > > > Yes, that's right. I sent a patch for such a warning in 2019 and pinged > > occationally. Still waiting for it to be merged :-\ > > (https://lore.kernel.org/r/20190625100412.11815-1-u.kleine-koenig@pengutronix.de) > > > > > > I think at least c) is easy to resolve because > > > > platform_get_irq_optional() isn't that old yet and mechanically > > > > replacing it by platform_get_irq_silent() should be easy and safe. > > > > And this is orthogonal to the discussion if -ENOXIO is a sensible return > > > > value and if it's as easy as it could be to work with errors on irq > > > > lookups. > > > > > > It'd certainly be good to name anything that doesn't correspond to one > > > of the existing semantics for the API (!) something different rather > > > than adding yet another potentially overloaded meaning. > > > > It seems we're (at least) three who agree about this. Here is a patch > > fixing the name. > > From an API naming perspective this does not make much sense anymore with > the name chosen, it is understood that whent he function is called > platform_get_irq_optional(), optional applies to the IRQ. An optional IRQ is > something people can reason about because it makes sense. The problem I see is that the semantic is different to the other available *_get_optional() functions. And this isn't fixable for irqs. So better don't use that naming scheme. > What is a a "silent" IRQ however? It does not apply to the object it is > trying to fetch to anymore, but to the message that may not be printed in > case the resource failed to be obtained, because said resource is optional. > Woah, that's quite a stretch. I'm surprised that the semantic isn't obvious, but ok, I can accept that.Can you maek a constructive suggestion here? What name pair would you choose for the two functions functions under discussion? (BTW, my favourite would be that platform_get_irq() doesn't emit an error message and the caller is reliable for emitting that. But I think it's too late for this approach.) > Following the discussion and original 2 patches set from Sergey, it is not > entirely clear to me anymore what is it that we are trying to fix. (I think) Sergey's focus is: platform_get_irq_optional() returning -ENXIO is ugly, other functions return -ENOENT and other *_get_optional() functions return NULL for "This resource doesn't exist". So let's return 0 in this case. My focus is: There cannot be an "optional irq" where the driver doesn't have to care if the irq actually exist or not. So the pattern with *_get_optional() isn't sensible for irqs and if changing the returned value meaning "This resource doesn't exist" is sensible for platform_get_irq_optional(), I claim it's sensible for platform_get_irq(), too. So the semantic difference between platform_get_irq() and platform_get_irq_optional() is only that one emits an error message and the other doesn't. So this function pair should use a different naming than get + get_optional as this naming evokes expectations that must be wrong as there cannot be a dummy irq value. > I nearly forgot, I would paint it blue, sky blue, not navy blue, not light > blue ;) no way. green is the ultimate blue for platform_get_irq() :-) Best regards Uwe
>>>>> "Uwe" == Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes: > The subsystems regulator, clk and gpio have the concept of a dummy > resource. For regulator, clk and gpio there is a semantic difference > between the regular _get() function and the _get_optional() variant. > (One might return the dummy resource, the other won't. Unfortunately > which one implements which isn't the same for these three.) The > difference between platform_get_irq() and platform_get_irq_optional() is > only that the former might emit an error message and the later won't. > To prevent people's expectations that there is a semantic difference > between these too, rename platform_get_irq_optional() to > platform_get_irq_silent() to make the actual difference more obvious. > The #define for the old name can and should be removed once all patches > currently in flux still relying on platform_get_irq_optional() are > fixed. > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> For i2c-ocores.c: Acked-by: Peter Korsgaard <peter@korsgaard.com>
Hi Uwe, On Thu, Jan 13, 2022 at 11:43 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > On Thu, Jan 13, 2022 at 11:57:43PM +0300, Sergey Shtylyov wrote: > > On 1/13/22 11:17 PM, Mark Brown wrote: > > >> The subsystems regulator, clk and gpio have the concept of a dummy > > >> resource. For regulator, clk and gpio there is a semantic difference > > >> between the regular _get() function and the _get_optional() variant. > > >> (One might return the dummy resource, the other won't. Unfortunately > > >> which one implements which isn't the same for these three.) The > > >> difference between platform_get_irq() and platform_get_irq_optional() is > > >> only that the former might emit an error message and the later won't. > > > > This is only a current difference but I'm still going to return 0 ISO > > -ENXIO from latform_get_irq_optional(), no way I'd leave that -ENXIO there > > alone... :-) > > This would address a bit of the critic in my commit log. But as 0 isn't > a dummy value like the dummy values that exist for clk, gpiod and > regulator I still think that the naming is a bad idea because it's not > in the spirit of the other *_get_optional functions. > > Seeing you say that -ENXIO is a bad return value for > platform_get_irq_optional() and 0 should be used instead, I wonder why > not changing platform_get_irq() to return 0 instead of -ENXIO, too. > This question is for now only about a sensible semantic. That actually > changing platform_get_irq() is probably harder than changing > platform_get_irq_optional() is a different story. > > If only platform_get_irq_optional() is changed and given that the > callers have to do something like: > > if (this_irq_exists()): > ... (e.g. request_irq) > else: > ... (e.g. setup polling) > > I really think it's a bad idea that this_irq_exists() has to be > different for platform_get_irq() vs. platform_get_irq_optional(). For platform_get_irq(), the IRQ being absent is an error condition, hence it should return an error code. For platform_get_irq_optional(), the IRQ being absent is not an error condition, hence it should not return an error code, and 0 is OK. 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
On 1/13/22 11:57 PM, Sergey Shtylyov wrote: >>> The subsystems regulator, clk and gpio have the concept of a dummy >>> resource. For regulator, clk and gpio there is a semantic difference >>> between the regular _get() function and the _get_optional() variant. >>> (One might return the dummy resource, the other won't. Unfortunately >>> which one implements which isn't the same for these three.) The >>> difference between platform_get_irq() and platform_get_irq_optional() is >>> only that the former might emit an error message and the later won't. > > This is only a current difference but I'm still going to return 0 ISO > -ENXIO from latform_get_irq_optional(), no way I'd leave that -ENXIO there platform. > alone... :-) > >> Reviewed-by: Mark Brown <broonie@kernel.org> > > Hm... I'm seeing a tag bit not seeing the patch itself... Grr, my mail server tossed it into the spam folder... :-( MBR, Sergey
On Thu, Jan 13, 2022 at 08:43:58PM +0100, Uwe Kleine-König wrote: > The subsystems regulator, clk and gpio have the concept of a dummy > resource. For regulator, clk and gpio there is a semantic difference > between the regular _get() function and the _get_optional() variant. > (One might return the dummy resource, the other won't. Unfortunately > which one implements which isn't the same for these three.) The > difference between platform_get_irq() and platform_get_irq_optional() is > only that the former might emit an error message and the later won't. > > To prevent people's expectations that there is a semantic difference > between these too, rename platform_get_irq_optional() to > platform_get_irq_silent() to make the actual difference more obvious. > > The #define for the old name can and should be removed once all patches > currently in flux still relying on platform_get_irq_optional() are > fixed. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Hello, > > On Thu, Jan 13, 2022 at 02:45:30PM +0000, Mark Brown wrote: > > On Thu, Jan 13, 2022 at 12:08:31PM +0100, Uwe Kleine-König wrote: > > > > > This is all very unfortunate. In my eyes b) is the most sensible > > > sense, but the past showed that we don't agree here. (The most annoying > > > part of regulator_get is the warning that is emitted that regularily > > > makes customers ask what happens here and if this is fixable.) > > > > Fortunately it can be fixed, and it's safer to clearly specify things. > > The prints are there because when the description is wrong enough to > > cause things to blow up we can fail to boot or run messily and > > forgetting to describe some supplies (or typoing so they haven't done > > that) and people were having a hard time figuring out what might've > > happened. > > Yes, that's right. I sent a patch for such a warning in 2019 and pinged > occationally. Still waiting for it to be merged :-\ > (https://lore.kernel.org/r/20190625100412.11815-1-u.kleine-koenig@pengutronix.de) > > > > I think at least c) is easy to resolve because > > > platform_get_irq_optional() isn't that old yet and mechanically > > > replacing it by platform_get_irq_silent() should be easy and safe. > > > And this is orthogonal to the discussion if -ENOXIO is a sensible return > > > value and if it's as easy as it could be to work with errors on irq > > > lookups. > > > > It'd certainly be good to name anything that doesn't correspond to one > > of the existing semantics for the API (!) something different rather > > than adding yet another potentially overloaded meaning. > > It seems we're (at least) three who agree about this. Here is a patch > fixing the name. And similar number of people are on the other side.
On 1/14/22 12:42 AM, Florian Fainelli wrote: >> The subsystems regulator, clk and gpio have the concept of a dummy >> resource. For regulator, clk and gpio there is a semantic difference >> between the regular _get() function and the _get_optional() variant. >> (One might return the dummy resource, the other won't. Unfortunately >> which one implements which isn't the same for these three.) The >> difference between platform_get_irq() and platform_get_irq_optional() is >> only that the former might emit an error message and the later won't. >> >> To prevent people's expectations that there is a semantic difference >> between these too, rename platform_get_irq_optional() to >> platform_get_irq_silent() to make the actual difference more obvious. >> >> The #define for the old name can and should be removed once all patches >> currently in flux still relying on platform_get_irq_optional() are >> fixed. >> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [...] >>>> I think at least c) is easy to resolve because >>>> platform_get_irq_optional() isn't that old yet and mechanically >>>> replacing it by platform_get_irq_silent() should be easy and safe. >>>> And this is orthogonal to the discussion if -ENOXIO is a sensible return >>>> value and if it's as easy as it could be to work with errors on irq >>>> lookups. >>> >>> It'd certainly be good to name anything that doesn't correspond to one >>> of the existing semantics for the API (!) something different rather >>> than adding yet another potentially overloaded meaning. >> >> It seems we're (at least) three who agree about this. Here is a patch >> fixing the name. > > From an API naming perspective this does not make much sense anymore with the name chosen, > it is understood that whent he function is called platform_get_irq_optional(), optional applies > to the IRQ. An optional IRQ is something people can reason about because it makes sense. Right! :-) > What is a a "silent" IRQ however? It does not apply to the object it is trying to fetch to > anymore, but to the message that may not be printed in case the resource failed to be obtained, > because said resource is optional. Woah, that's quite a stretch. Right again! :-) > Following the discussion and original 2 patches set from Sergey, it is not entirely clear to me > anymore what is it that we are trying to fix. Andy and me tried to fix the platform_get_irq[_byname]_optional() value, corresponding to a missing (optional) IRQ resource from -ENXIO to 0, in order to keep the callers error code agnostic. This change completely aligns e.g. platform_get_irq_optional() with clk_get_optional() and gpiod_get_optional()... Unforunately, we can't "fix" request_irq() and company to treat 0 as missing IRQ -- they have to keep the ability to get called from the arch/ code (that doesn't use platform_get_irq(), etc. > I nearly forgot, I would paint it blue, sky blue, not navy blue, not light blue ;) :-) PS: Florian, something was wrong with your mail client -- I had to manually wrap your quotes, else there were super long unbroken paragraphs...
On 1/13/22 10:43 PM, Uwe Kleine-König wrote: > The subsystems regulator, clk and gpio have the concept of a dummy > resource. For regulator, clk and gpio there is a semantic difference > between the regular _get() function and the _get_optional() variant. > (One might return the dummy resource, the other won't. Unfortunately > which one implements which isn't the same for these three.) The > difference between platform_get_irq() and platform_get_irq_optional() is > only that the former might emit an error message and the later won't. > > To prevent people's expectations that there is a semantic difference > between these too, rename platform_get_irq_optional() to > platform_get_irq_silent() to make the actual difference more obvious. > > The #define for the old name can and should be removed once all patches > currently in flux still relying on platform_get_irq_optional() are > fixed. Hm... I'm afraid that with this #define they would never get fixed... :-) > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Hello, > > On Thu, Jan 13, 2022 at 02:45:30PM +0000, Mark Brown wrote: >> On Thu, Jan 13, 2022 at 12:08:31PM +0100, Uwe Kleine-König wrote: >> >>> This is all very unfortunate. In my eyes b) is the most sensible >>> sense, but the past showed that we don't agree here. (The most annoying >>> part of regulator_get is the warning that is emitted that regularily >>> makes customers ask what happens here and if this is fixable.) >> >> Fortunately it can be fixed, and it's safer to clearly specify things. >> The prints are there because when the description is wrong enough to >> cause things to blow up we can fail to boot or run messily and >> forgetting to describe some supplies (or typoing so they haven't done >> that) and people were having a hard time figuring out what might've >> happened. > > Yes, that's right. I sent a patch for such a warning in 2019 and pinged > occationally. Still waiting for it to be merged :-\ > (https://lore.kernel.org/r/20190625100412.11815-1-u.kleine-koenig@pengutronix.de) > >>> I think at least c) is easy to resolve because >>> platform_get_irq_optional() isn't that old yet and mechanically >>> replacing it by platform_get_irq_silent() should be easy and safe. >>> And this is orthogonal to the discussion if -ENOXIO is a sensible return >>> value and if it's as easy as it could be to work with errors on irq >>> lookups. >> >> It'd certainly be good to name anything that doesn't correspond to one >> of the existing semantics for the API (!) something different rather >> than adding yet another potentially overloaded meaning. > > It seems we're (at least) three who agree about this. Here is a patch > fixing the name. I can't say I genrally agree with this patch... [...] > diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h > index 7c96f169d274..6d495f15f717 100644 > --- a/include/linux/platform_device.h > +++ b/include/linux/platform_device.h > @@ -69,7 +69,14 @@ extern void __iomem * > devm_platform_ioremap_resource_byname(struct platform_device *pdev, > const char *name); > extern int platform_get_irq(struct platform_device *, unsigned int); > -extern int platform_get_irq_optional(struct platform_device *, unsigned int); > +extern int platform_get_irq_silent(struct platform_device *, unsigned int); > + > +/* > + * platform_get_irq_optional was recently renamed to platform_get_irq_silent. > + * Fixup users to not break patches that were created before the rename. > + */ > +#define platform_get_irq_optional(pdev, index) platform_get_irq_silent(pdev, index) > + Yeah, why bother fixing if it compiles anyway? I think an inline wrapper with an indication to gcc that the function is deprecated (I just forgot how it should look) would be better instead... > extern int platform_irq_count(struct platform_device *); > extern int devm_platform_get_irqs_affinity(struct platform_device *dev, > struct irq_affinity *affd, [...] MBR, Sergey
On Fri, Jan 14, 2022 at 10:45:38PM +0300, Sergey Shtylyov wrote: > On 1/13/22 10:43 PM, Uwe Kleine-König wrote: > > > The subsystems regulator, clk and gpio have the concept of a dummy > > resource. For regulator, clk and gpio there is a semantic difference > > between the regular _get() function and the _get_optional() variant. > > (One might return the dummy resource, the other won't. Unfortunately > > which one implements which isn't the same for these three.) The > > difference between platform_get_irq() and platform_get_irq_optional() is > > only that the former might emit an error message and the later won't. > > > > To prevent people's expectations that there is a semantic difference > > between these too, rename platform_get_irq_optional() to > > platform_get_irq_silent() to make the actual difference more obvious. > > > > The #define for the old name can and should be removed once all patches > > currently in flux still relying on platform_get_irq_optional() are > > fixed. > > Hm... I'm afraid that with this #define they would never get fixed... :-) I will care for it. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Hello, > > > > On Thu, Jan 13, 2022 at 02:45:30PM +0000, Mark Brown wrote: > >> On Thu, Jan 13, 2022 at 12:08:31PM +0100, Uwe Kleine-König wrote: > >> > >>> This is all very unfortunate. In my eyes b) is the most sensible > >>> sense, but the past showed that we don't agree here. (The most annoying > >>> part of regulator_get is the warning that is emitted that regularily > >>> makes customers ask what happens here and if this is fixable.) > >> > >> Fortunately it can be fixed, and it's safer to clearly specify things. > >> The prints are there because when the description is wrong enough to > >> cause things to blow up we can fail to boot or run messily and > >> forgetting to describe some supplies (or typoing so they haven't done > >> that) and people were having a hard time figuring out what might've > >> happened. > > > > Yes, that's right. I sent a patch for such a warning in 2019 and pinged > > occationally. Still waiting for it to be merged :-\ > > (https://lore.kernel.org/r/20190625100412.11815-1-u.kleine-koenig@pengutronix.de) > > > >>> I think at least c) is easy to resolve because > >>> platform_get_irq_optional() isn't that old yet and mechanically > >>> replacing it by platform_get_irq_silent() should be easy and safe. > >>> And this is orthogonal to the discussion if -ENOXIO is a sensible return > >>> value and if it's as easy as it could be to work with errors on irq > >>> lookups. > >> > >> It'd certainly be good to name anything that doesn't correspond to one > >> of the existing semantics for the API (!) something different rather > >> than adding yet another potentially overloaded meaning. > > > > It seems we're (at least) three who agree about this. Here is a patch > > fixing the name. > > I can't say I genrally agree with this patch... Yes, I didn't count you to the three people signaling agreement. > [...] > > diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h > > index 7c96f169d274..6d495f15f717 100644 > > --- a/include/linux/platform_device.h > > +++ b/include/linux/platform_device.h > > @@ -69,7 +69,14 @@ extern void __iomem * > > devm_platform_ioremap_resource_byname(struct platform_device *pdev, > > const char *name); > > extern int platform_get_irq(struct platform_device *, unsigned int); > > -extern int platform_get_irq_optional(struct platform_device *, unsigned int); > > +extern int platform_get_irq_silent(struct platform_device *, unsigned int); > > + > > +/* > > + * platform_get_irq_optional was recently renamed to platform_get_irq_silent. > > + * Fixup users to not break patches that were created before the rename. > > + */ > > +#define platform_get_irq_optional(pdev, index) platform_get_irq_silent(pdev, index) > > + > > Yeah, why bother fixing if it compiles anyway? The plan is to remove the define in one or two kernel releases. The idea is only to not break patches that are currently in next. > I think an inline wrapper with an indication to gcc that the function is deprecated > (I just forgot how it should look) would be better instead... The deprecated function annotation is generally frowned upon. See 771c035372a0. Best regards Uwe
On 1/14/22 11:29 PM, Uwe Kleine-König wrote: >>> The subsystems regulator, clk and gpio have the concept of a dummy >>> resource. For regulator, clk and gpio there is a semantic difference >>> between the regular _get() function and the _get_optional() variant. >>> (One might return the dummy resource, the other won't. Unfortunately >>> which one implements which isn't the same for these three.) The >>> difference between platform_get_irq() and platform_get_irq_optional() is >>> only that the former might emit an error message and the later won't. >>> >>> To prevent people's expectations that there is a semantic difference >>> between these too, rename platform_get_irq_optional() to >>> platform_get_irq_silent() to make the actual difference more obvious. >>> >>> The #define for the old name can and should be removed once all patches >>> currently in flux still relying on platform_get_irq_optional() are >>> fixed. >> >> Hm... I'm afraid that with this #define they would never get fixed... :-) > > I will care for it. Ah! OK then. :-) >>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> >>> --- >>> Hello, >>> >>> On Thu, Jan 13, 2022 at 02:45:30PM +0000, Mark Brown wrote: >>>> On Thu, Jan 13, 2022 at 12:08:31PM +0100, Uwe Kleine-König wrote: >>>> >>>>> This is all very unfortunate. In my eyes b) is the most sensible >>>>> sense, but the past showed that we don't agree here. (The most annoying >>>>> part of regulator_get is the warning that is emitted that regularily >>>>> makes customers ask what happens here and if this is fixable.) >>>> >>>> Fortunately it can be fixed, and it's safer to clearly specify things. >>>> The prints are there because when the description is wrong enough to >>>> cause things to blow up we can fail to boot or run messily and >>>> forgetting to describe some supplies (or typoing so they haven't done >>>> that) and people were having a hard time figuring out what might've >>>> happened. >>> >>> Yes, that's right. I sent a patch for such a warning in 2019 and pinged >>> occationally. Still waiting for it to be merged :-\ >>> (https://lore.kernel.org/r/20190625100412.11815-1-u.kleine-koenig@pengutronix.de) >>> >>>>> I think at least c) is easy to resolve because >>>>> platform_get_irq_optional() isn't that old yet and mechanically >>>>> replacing it by platform_get_irq_silent() should be easy and safe. >>>>> And this is orthogonal to the discussion if -ENOXIO is a sensible return >>>>> value and if it's as easy as it could be to work with errors on irq >>>>> lookups. >>>> >>>> It'd certainly be good to name anything that doesn't correspond to one >>>> of the existing semantics for the API (!) something different rather >>>> than adding yet another potentially overloaded meaning. >>> >>> It seems we're (at least) three who agree about this. Here is a patch >>> fixing the name. >> >> I can't say I genrally agree with this patch... > > Yes, I didn't count you to the three people signaling agreement. :-D >> [...] >>> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h >>> index 7c96f169d274..6d495f15f717 100644 >>> --- a/include/linux/platform_device.h >>> +++ b/include/linux/platform_device.h >>> @@ -69,7 +69,14 @@ extern void __iomem * >>> devm_platform_ioremap_resource_byname(struct platform_device *pdev, >>> const char *name); >>> extern int platform_get_irq(struct platform_device *, unsigned int); >>> -extern int platform_get_irq_optional(struct platform_device *, unsigned int); >>> +extern int platform_get_irq_silent(struct platform_device *, unsigned int); >>> + >>> +/* >>> + * platform_get_irq_optional was recently renamed to platform_get_irq_silent. >>> + * Fixup users to not break patches that were created before the rename. >>> + */ >>> +#define platform_get_irq_optional(pdev, index) platform_get_irq_silent(pdev, index) >>> + >> >> Yeah, why bother fixing if it compiles anyway? > > The plan is to remove the define in one or two kernel releases. The idea > is only to not break patches that are currently in next. > >> I think an inline wrapper with an indication to gcc that the function is deprecated >> (I just forgot how it should look) would be better instead... > > The deprecated function annotation is generally frowned upon. See > 771c035372a0. Not sure I share the sentiment but good to know about that. > Best regards > Uwe MBR, Sergey
On Fri, Jan 14, 2022 at 08:55:07PM +0300, Sergey Shtylyov wrote: > On 1/14/22 12:42 AM, Florian Fainelli wrote: > > >> The subsystems regulator, clk and gpio have the concept of a dummy > >> resource. For regulator, clk and gpio there is a semantic difference > >> between the regular _get() function and the _get_optional() variant. > >> (One might return the dummy resource, the other won't. Unfortunately > >> which one implements which isn't the same for these three.) The > >> difference between platform_get_irq() and platform_get_irq_optional() is > >> only that the former might emit an error message and the later won't. > >> > >> To prevent people's expectations that there is a semantic difference > >> between these too, rename platform_get_irq_optional() to > >> platform_get_irq_silent() to make the actual difference more obvious. > >> > >> The #define for the old name can and should be removed once all patches > >> currently in flux still relying on platform_get_irq_optional() are > >> fixed. > >> > >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > [...] > >>>> I think at least c) is easy to resolve because > >>>> platform_get_irq_optional() isn't that old yet and mechanically > >>>> replacing it by platform_get_irq_silent() should be easy and safe. > >>>> And this is orthogonal to the discussion if -ENOXIO is a sensible return > >>>> value and if it's as easy as it could be to work with errors on irq > >>>> lookups. > >>> > >>> It'd certainly be good to name anything that doesn't correspond to one > >>> of the existing semantics for the API (!) something different rather > >>> than adding yet another potentially overloaded meaning. > >> > >> It seems we're (at least) three who agree about this. Here is a patch > >> fixing the name. > > > > From an API naming perspective this does not make much sense anymore with the name chosen, > > it is understood that whent he function is called platform_get_irq_optional(), optional applies > > to the IRQ. An optional IRQ is something people can reason about because it makes sense. > > Right! :-) > > > What is a a "silent" IRQ however? It does not apply to the object it is trying to fetch to > > anymore, but to the message that may not be printed in case the resource failed to be obtained, > > because said resource is optional. Woah, that's quite a stretch. > > Right again! :-) So you oppose to the name chosen, but not the renaming as such. I already asked Florian if he has a better name, do you have a constructive suggestion? What about platform_silently_get_irq? Or platform_get_irq_silently? Do you agree that it's unfortunate that platform_get_irq_optional() has a different usage convention than clk_get_optional() and gpiod_get_optional()? Do you agree that the difference between platform_get_irq_optional() and platform_get_irq() is only that one of them issues an error message and the other doesn't? Currently the return values of platform_get_irq_optional() and platform_get_irq() are identical. Do you agree that any change to clean up the return value convention of platform_get_irq_optional() also would be sensible for platform_get_irq()? Do you agree that changing the way how return values of platform_get_irq_optional() have to be evaluated without adapting platform_get_irq() in the same way, artifially breaks the releation between these two functions? > > Following the discussion and original 2 patches set from Sergey, it is not entirely clear to me > > anymore what is it that we are trying to fix. > > Andy and me tried to fix the platform_get_irq[_byname]_optional() value, corresponding to > a missing (optional) IRQ resource from -ENXIO to 0, in order to keep the callers error code > agnostic. This change completely aligns e.g. platform_get_irq_optional() with clk_get_optional() > and gpiod_get_optional()... Did you read and understand my objection? Yes, in the not found case the return value is a 32-bit or 64-bit long zero value (0 or NULL) for these functions. But for irqs you cannot treat that as an irq number. For clks this works, and this is the fact that justifies the "optional" in the name and that simplifies further handling without having to check if the value returned by clk_get_optional corresponds to the dummy clk or a real one. Just returning 0 for not-found doesn't justify "optional" in the name. Or do you think kmalloc should better be called kmalloc_optional because it returns NULL if there is no more memory available? The only thing you accomplish with returning 0 instead of -ENXIO is that the check for this value in the callers has to be adapted. But as you cannot handle 0 and a "normal" irq in the same way (i.e. pass it to request_irq) In my eyes error code agnostic isn't a sensible goal. If you ask for a resource and it's not there and your driver can cope and this cannot be done by just treating the returned value as a valid resource, making it explicit that the error code for "not found" is handled is a good thing. In my opinion it's not a good enough reason to change a function's return conventions just that I can handle one of the various results using if (ret == 0) instead of if (ret == -ENXIO) Also there is no justification to change the value for "not found" to zero. The next developer might be annoyed by having to check for -EPROBE_DEFER and wants to introduce a special function that behaves like platform_get_irq, just returns 0 when platform_get_irq returns -ENOENT. > Unforunately, we can't "fix" request_irq() and company to treat 0 as missing IRQ -- they have > to keep the ability to get called from the arch/ code (that doesn't use platform_get_irq(), etc. Note that even if request_irq would be a noop for 0, the biggest part of the drivers wouldn't be done with request_irq(0, ...) because in the absense of an irq something has to be done about it. Oh my, I failed to not continue this discussion really badly. But I really cannot stand people arguing for wrong things and ignoring my reasoning completely. In case you feel the same way: I agree that -ENXIO is the wrong value for not-found. But to change this wrong behaviour to another wrong behaviour isn't an improvement. Best regards Uwe
On Fri, Jan 14, 2022 at 10:58:51AM +0100, Geert Uytterhoeven wrote: > Hi Uwe, > > On Thu, Jan 13, 2022 at 11:43 PM Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > On Thu, Jan 13, 2022 at 11:57:43PM +0300, Sergey Shtylyov wrote: > > > On 1/13/22 11:17 PM, Mark Brown wrote: > > > >> The subsystems regulator, clk and gpio have the concept of a dummy > > > >> resource. For regulator, clk and gpio there is a semantic difference > > > >> between the regular _get() function and the _get_optional() variant. > > > >> (One might return the dummy resource, the other won't. Unfortunately > > > >> which one implements which isn't the same for these three.) The > > > >> difference between platform_get_irq() and platform_get_irq_optional() is > > > >> only that the former might emit an error message and the later won't. > > > > > > This is only a current difference but I'm still going to return 0 ISO > > > -ENXIO from latform_get_irq_optional(), no way I'd leave that -ENXIO there > > > alone... :-) > > > > This would address a bit of the critic in my commit log. But as 0 isn't > > a dummy value like the dummy values that exist for clk, gpiod and > > regulator I still think that the naming is a bad idea because it's not > > in the spirit of the other *_get_optional functions. > > > > Seeing you say that -ENXIO is a bad return value for > > platform_get_irq_optional() and 0 should be used instead, I wonder why > > not changing platform_get_irq() to return 0 instead of -ENXIO, too. > > This question is for now only about a sensible semantic. That actually > > changing platform_get_irq() is probably harder than changing > > platform_get_irq_optional() is a different story. > > > > If only platform_get_irq_optional() is changed and given that the > > callers have to do something like: > > > > if (this_irq_exists()): > > ... (e.g. request_irq) > > else: > > ... (e.g. setup polling) > > > > I really think it's a bad idea that this_irq_exists() has to be > > different for platform_get_irq() vs. platform_get_irq_optional(). > > For platform_get_irq(), the IRQ being absent is an error condition, > hence it should return an error code. > For platform_get_irq_optional(), the IRQ being absent is not an error > condition, hence it should not return an error code, and 0 is OK. Please show a few examples how this simplifies the code. If it's only that a driver has to check for == 0 instead of == -ENXIO, than that's not a good enough motivation to make platform_get_irq_optional() different to platform_get_irq(). Best regards Uwe
On Fri, Jan 14, 2022 at 03:04:38PM +0200, Andy Shevchenko wrote: > On Thu, Jan 13, 2022 at 08:43:58PM +0100, Uwe Kleine-König wrote: > > > It'd certainly be good to name anything that doesn't correspond to one > > > of the existing semantics for the API (!) something different rather > > > than adding yet another potentially overloaded meaning. > > > > It seems we're (at least) three who agree about this. Here is a patch > > fixing the name. > > And similar number of people are on the other side. If someone already opposed to the renaming (and not only the name) I must have missed that. So you think it's a good idea to keep the name platform_get_irq_optional() despite the "not found" value returned by it isn't usable as if it were a normal irq number? Best regards Uwe
On 1/15/22 6:21 PM, Uwe Kleine-König wrote: [...] >>>>>> The subsystems regulator, clk and gpio have the concept of a dummy >>>>>> resource. For regulator, clk and gpio there is a semantic difference >>>>>> between the regular _get() function and the _get_optional() variant. >>>>>> (One might return the dummy resource, the other won't. Unfortunately >>>>>> which one implements which isn't the same for these three.) The >>>>>> difference between platform_get_irq() and platform_get_irq_optional() is >>>>>> only that the former might emit an error message and the later won't. >>>> >>>> This is only a current difference but I'm still going to return 0 ISO >>>> -ENXIO from latform_get_irq_optional(), no way I'd leave that -ENXIO there >>>> alone... :-) >>> >>> This would address a bit of the critic in my commit log. But as 0 isn't >>> a dummy value like the dummy values that exist for clk, gpiod and >>> regulator I still think that the naming is a bad idea because it's not >>> in the spirit of the other *_get_optional functions. >>> >>> Seeing you say that -ENXIO is a bad return value for >>> platform_get_irq_optional() and 0 should be used instead, I wonder why >>> not changing platform_get_irq() to return 0 instead of -ENXIO, too. >>> This question is for now only about a sensible semantic. That actually >>> changing platform_get_irq() is probably harder than changing >>> platform_get_irq_optional() is a different story. >>> >>> If only platform_get_irq_optional() is changed and given that the >>> callers have to do something like: >>> >>> if (this_irq_exists()): >>> ... (e.g. request_irq) >>> else: >>> ... (e.g. setup polling) >>> >>> I really think it's a bad idea that this_irq_exists() has to be >>> different for platform_get_irq() vs. platform_get_irq_optional(). >> >> For platform_get_irq(), the IRQ being absent is an error condition, >> hence it should return an error code. >> For platform_get_irq_optional(), the IRQ being absent is not an error >> condition, hence it should not return an error code, and 0 is OK. > > Please show a few examples how this simplifies the code. If it's only As for platform_get_irq(), returning -ENXIO simplifies things a lot: you don't have to check for 0 at every freaking call site and have s/th like (every time!): irq = platform_get_irq(); if (irq <= 0) return irq ?: -ENXIO; // any error code you choose instead of just: irq = platform_get_irq(); if (irq < 0) return irq; This scales better in my book. > that a driver has to check for == 0 instead of == -ENXIO, than that's > not a good enough motivation to make platform_get_irq_optional() > different to platform_get_irq(). Again, it scales better... good motivation in my eyes. > Best regards > Uwe MBR, Sergey
On Sat, Jan 15, 2022 at 02:55:50PM +0100, Uwe Kleine-König wrote: > On Fri, Jan 14, 2022 at 08:55:07PM +0300, Sergey Shtylyov wrote: > > On 1/14/22 12:42 AM, Florian Fainelli wrote: > So you oppose to the name chosen, but not the renaming as such. I oppose the name change. The unneeded churn right now since it won't fix the issues with the underneath API (platform_get_irq() in this case) and will require one more iteration over callers again. The main issue that platform_get_irq*() returns magic error code while treating 0 in a very special way (issuing WARN() and considering it as a successful cookie) and this all is quite confusing. If you are going to fix the underlying issue, welcome! Now I see only the step to somewhere. I.o.w. this change _standalone_ makes no sense to me.
On Sat, Jan 15, 2022 at 02:55:50PM +0100, Uwe Kleine-König wrote: > On Fri, Jan 14, 2022 at 08:55:07PM +0300, Sergey Shtylyov wrote: > Or do you think kmalloc should better be called > kmalloc_optional because it returns NULL if there is no more memory > available? Oh, do you know that kmalloc() may return NULL and small cookie value and actually one has to check for that (yes, either before or after against different variables)? kmalloc() is exactly an example that justifies the Sergey's patch.
On Sat, Jan 15, 2022 at 04:45:39PM +0100, Uwe Kleine-König wrote: > On Fri, Jan 14, 2022 at 03:04:38PM +0200, Andy Shevchenko wrote: > > On Thu, Jan 13, 2022 at 08:43:58PM +0100, Uwe Kleine-König wrote: > > > > It'd certainly be good to name anything that doesn't correspond to one > > > > of the existing semantics for the API (!) something different rather > > > > than adding yet another potentially overloaded meaning. > > > > > > It seems we're (at least) three who agree about this. Here is a patch > > > fixing the name. > > > > And similar number of people are on the other side. > > If someone already opposed to the renaming (and not only the name) I > must have missed that. > > So you think it's a good idea to keep the name > platform_get_irq_optional() despite the "not found" value returned by it > isn't usable as if it were a normal irq number? I meant that on the other side people who are in favour of Sergey's patch. Since that I commented already that I opposed the renaming being a standalone change. Do you agree that we have several issues with platform_get_irq*() APIs? 1. The unfortunate naming 2. The vIRQ0 handling: a) WARN() followed by b) returned value 0 3. The specific cookie for "IRQ not found, while no error happened" case
On Fri, Jan 14, 2022 at 10:45:38PM +0300, Sergey Shtylyov wrote: > On 1/13/22 10:43 PM, Uwe Kleine-König wrote: > > > The subsystems regulator, clk and gpio have the concept of a dummy > > resource. For regulator, clk and gpio there is a semantic difference > > between the regular _get() function and the _get_optional() variant. > > (One might return the dummy resource, the other won't. Unfortunately > > which one implements which isn't the same for these three.) The > > difference between platform_get_irq() and platform_get_irq_optional() is > > only that the former might emit an error message and the later won't. > > > > To prevent people's expectations that there is a semantic difference > > between these too, rename platform_get_irq_optional() to > > platform_get_irq_silent() to make the actual difference more obvious. > > > > The #define for the old name can and should be removed once all patches > > currently in flux still relying on platform_get_irq_optional() are > > fixed. > > Hm... I'm afraid that with this #define they would never get fixed... :-) Agree. The problems I have listed in another reply should be fixed at once by the same series.
Hello! On 1/19/22 9:36 PM, Andy Shevchenko wrote: [...] >> So you oppose to the name chosen, but not the renaming as such. > > I oppose the name change. The unneeded churn right now since it won't fix > the issues with the underneath API (platform_get_irq() in this case) and > will require one more iteration over callers again. > > The main issue that platform_get_irq*() returns magic error code while > treating 0 in a very special way (issuing WARN() and considering it as > a successful cookie) and this all is quite confusing. I have a patch for that -- to which you were hostile for some reason I still can't understand. :-) > If you are going to fix the underlying issue, welcome! Now I see only > the step to somewhere. I.o.w. this change _standalone_ makes no sense > to me. We already have a fix, no? It just hasn't been applied still... :-) Without it the 2 patches dealing with *_optional() don't have much sense. MBR, Sergey
On 1/19/22 9:51 PM, Andy Shevchenko wrote: [...] >>>>> It'd certainly be good to name anything that doesn't correspond to one >>>>> of the existing semantics for the API (!) something different rather >>>>> than adding yet another potentially overloaded meaning. >>>> >>>> It seems we're (at least) three who agree about this. Here is a patch >>>> fixing the name. >>> >>> And similar number of people are on the other side. >> >> If someone already opposed to the renaming (and not only the name) I >> must have missed that. >> >> So you think it's a good idea to keep the name >> platform_get_irq_optional() despite the "not found" value returned by it >> isn't usable as if it were a normal irq number? > > I meant that on the other side people who are in favour of Sergey's patch. > Since that I commented already that I opposed the renaming being a standalone > change. > > Do you agree that we have several issues with platform_get_irq*() APIs? > > 1. The unfortunate naming Mmm, "what's in a name?"... is this the topmost prio issue? > 2. The vIRQ0 handling: a) WARN() followed by b) returned value 0 This is the most severe issue, I think... > 3. The specific cookie for "IRQ not found, while no error happened" case MBR, Sergey
On Wed, Jan 19, 2022 at 10:47:06PM +0300, Sergey Shtylyov wrote: > On 1/19/22 9:51 PM, Andy Shevchenko wrote: > >>>>> It'd certainly be good to name anything that doesn't correspond to one > >>>>> of the existing semantics for the API (!) something different rather > >>>>> than adding yet another potentially overloaded meaning. > >>>> > >>>> It seems we're (at least) three who agree about this. Here is a patch > >>>> fixing the name. > >>> > >>> And similar number of people are on the other side. > >> > >> If someone already opposed to the renaming (and not only the name) I > >> must have missed that. > >> > >> So you think it's a good idea to keep the name > >> platform_get_irq_optional() despite the "not found" value returned by it > >> isn't usable as if it were a normal irq number? > > > > I meant that on the other side people who are in favour of Sergey's patch. > > Since that I commented already that I opposed the renaming being a standalone > > change. > > > > Do you agree that we have several issues with platform_get_irq*() APIs? > > > > 1. The unfortunate naming > > Mmm, "what's in a name?"... is this the topmost prio issue? The order is arbitrary. > > 2. The vIRQ0 handling: a) WARN() followed by b) returned value 0 > > This is the most severe issue, I think... > > > 3. The specific cookie for "IRQ not found, while no error happened" case
On Wed, Jan 19, 2022 at 08:51:29PM +0200, Andy Shevchenko wrote: > On Sat, Jan 15, 2022 at 04:45:39PM +0100, Uwe Kleine-König wrote: > > On Fri, Jan 14, 2022 at 03:04:38PM +0200, Andy Shevchenko wrote: > > > On Thu, Jan 13, 2022 at 08:43:58PM +0100, Uwe Kleine-König wrote: > > > > > It'd certainly be good to name anything that doesn't correspond to one > > > > > of the existing semantics for the API (!) something different rather > > > > > than adding yet another potentially overloaded meaning. > > > > > > > > It seems we're (at least) three who agree about this. Here is a patch > > > > fixing the name. > > > > > > And similar number of people are on the other side. > > > > If someone already opposed to the renaming (and not only the name) I > > must have missed that. > > > > So you think it's a good idea to keep the name > > platform_get_irq_optional() despite the "not found" value returned by it > > isn't usable as if it were a normal irq number? > > I meant that on the other side people who are in favour of Sergey's patch. > Since that I commented already that I opposed the renaming being a standalone > change. > > Do you agree that we have several issues with platform_get_irq*() APIs? > > 1. The unfortunate naming unfortunate naming for the currently implemented semantic, yes. > 2. The vIRQ0 handling: a) WARN() followed by b) returned value 0 I'm happy with the vIRQ0 handling. Today platform_get_irq() and it's silent variant returns either a valid and usuable irq number or a negative error value. That's totally fine. > 3. The specific cookie for "IRQ not found, while no error happened" case Not sure what you mean here. I have no problem that a situation I can cope with is called an error for the query function. I just do error handling and continue happily. So the part "while no error happened" is irrelevant to me. Additionally I see the problems: 4. The semantic as implemented in Sergey's patch isn't better than the current one. platform_get_irq*() is still considerably different from (clk|gpiod)_get* because the not-found value for the _optional variant isn't usuable for the irq case. For clk and gpio I get rid of a whole if branch, for irq I only change the if-condition. (And if that change is considered good or bad seems to be subjective.) For the idea to add a warning to platform_get_irq_optional for all but -ENXIO (and -EPROBE_DEFER), I see the problem: 5. platform_get_irq*() issuing an error message is only correct most of the time and given proper error handling in the caller (which might be able to handle not only -ENXIO but maybe also -EINVAL[1]) the error message is irritating. Today platform_get_irq() emits an error message for all but -EPROBE_DEFER. As soon as we find a driver that handles -EINVAL we need a function platform_get_irq_variant1 to be silent for -EINVAL, -EPROBE_DEFER and -ENXIO (or platform_get_irq_variant2 that is only silent for -EINVAL and -EPROBE_DEFER?) IMHO a query function should always be silent and let the caller do the error handling. And if it's only because mydev: IRQ index 0 not found is worse than mydev: neither TX irq not a muxed RX/TX irq found . Also "index 0" is irritating for devices that are expected to have only a single irq (i.e. the majority of all devices). Yes, I admit, we can safe some code by pushing the error message in a query function. But that doesn't only have advantages. Best regards Uwe [1] Looking through the source I wonder: What are the errors that can happen in platform_get_irq*()? (calling everything but a valid irq number an error) Looking at many callers, they only seem to expect "not found" and some "probe defer" (even platform_get_irq() interprets everything but -EPROBE_DEFER as "IRQ index %u not found\n".) IMHO before we should consider to introduce a platform_get_irq*() variant with improved semantics, some cleanup in the internals of the irq lookup are necessary.
On Thu, Jan 20, 2022 at 08:57:18AM +0100, Uwe Kleine-König wrote: > On Wed, Jan 19, 2022 at 08:51:29PM +0200, Andy Shevchenko wrote: > > On Sat, Jan 15, 2022 at 04:45:39PM +0100, Uwe Kleine-König wrote: > > > On Fri, Jan 14, 2022 at 03:04:38PM +0200, Andy Shevchenko wrote: > > > > On Thu, Jan 13, 2022 at 08:43:58PM +0100, Uwe Kleine-König wrote: > > > > > > It'd certainly be good to name anything that doesn't correspond to one > > > > > > of the existing semantics for the API (!) something different rather > > > > > > than adding yet another potentially overloaded meaning. > > > > > > > > > > It seems we're (at least) three who agree about this. Here is a patch > > > > > fixing the name. > > > > > > > > And similar number of people are on the other side. > > > > > > If someone already opposed to the renaming (and not only the name) I > > > must have missed that. > > > > > > So you think it's a good idea to keep the name > > > platform_get_irq_optional() despite the "not found" value returned by it > > > isn't usable as if it were a normal irq number? > > > > I meant that on the other side people who are in favour of Sergey's patch. > > Since that I commented already that I opposed the renaming being a standalone > > change. > > > > Do you agree that we have several issues with platform_get_irq*() APIs? > > > > 1. The unfortunate naming > > unfortunate naming for the currently implemented semantic, yes. Yes. > > 2. The vIRQ0 handling: a) WARN() followed by b) returned value 0 > > I'm happy with the vIRQ0 handling. Today platform_get_irq() and it's > silent variant returns either a valid and usuable irq number or a > negative error value. That's totally fine. It might return 0. Actually it seems that the WARN() can only be issued in two cases: - SPARC with vIRQ0 in one of the array member - fallback to ACPI for GPIO IRQ resource with index 0 But the latter is bogus, because it would mean a bug in the ACPI code. The bottom line here is the SPARC case. Anybody familiar with the platform can shed a light on this. If there is no such case, we may remove warning along with ret = 0 case from platfrom_get_irq(). > > 3. The specific cookie for "IRQ not found, while no error happened" case > > Not sure what you mean here. I have no problem that a situation I can > cope with is called an error for the query function. I just do error > handling and continue happily. So the part "while no error happened" is > irrelevant to me. I meant that instead of using special error code, 0 is very much good for the cases when IRQ is not found. It allows to distinguish -ENXIO from the low layer from -ENXIO with this magic meaning. > Additionally I see the problems: > > 4. The semantic as implemented in Sergey's patch isn't better than the > current one. I disagree on this. See above on why. > platform_get_irq*() is still considerably different from > (clk|gpiod)_get* because the not-found value for the _optional variant > isn't usuable for the irq case. For clk and gpio I get rid of a whole if > branch, for irq I only change the if-condition. (And if that change is > considered good or bad seems to be subjective.) You are mixing up two things: - semantics of the pointer - semantics of the cookie Like I said previously the mistake is in putting an equal sign between apples and oranges (or in terms of Python, which is a good example here, None and False objects, where in both case 0 is magic and Python `if X`, `while `X` will work in the same way, the `typeof(X)` is different semantically). > For the idea to add a warning to platform_get_irq_optional for all but > -ENXIO (and -EPROBE_DEFER), I see the problem: > > 5. platform_get_irq*() issuing an error message is only correct most of > the time and given proper error handling in the caller (which might be > able to handle not only -ENXIO but maybe also -EINVAL[1]) the error message > is irritating. Today platform_get_irq() emits an error message for all > but -EPROBE_DEFER. As soon as we find a driver that handles -EINVAL we > need a function platform_get_irq_variant1 to be silent for -EINVAL, > -EPROBE_DEFER and -ENXIO (or platform_get_irq_variant2 that is only > silent for -EINVAL and -EPROBE_DEFER?) > > IMHO a query function should always be silent and let the caller do the > error handling. And if it's only because > > mydev: IRQ index 0 not found > > is worse than > > mydev: neither TX irq not a muxed RX/TX irq found > > . Also "index 0" is irritating for devices that are expected to have > only a single irq (i.e. the majority of all devices). Yeah, ack the #5. > Yes, I admit, we can safe some code by pushing the error message in a > query function. But that doesn't only have advantages. > [1] Looking through the source I wonder: What are the errors that can happen > in platform_get_irq*()? (calling everything but a valid irq number > an error) Looking at many callers, they only seem to expect "not > found" and some "probe defer" (even platform_get_irq() interprets > everything but -EPROBE_DEFER as "IRQ index %u not found\n".) > IMHO before we should consider to introduce a platform_get_irq*() > variant with improved semantics, some cleanup in the internals of > the irq lookup are necessary.
Hello! On 1/24/22 6:01 PM, Andy Shevchenko wrote: >>>>>>> It'd certainly be good to name anything that doesn't correspond to one >>>>>>> of the existing semantics for the API (!) something different rather >>>>>>> than adding yet another potentially overloaded meaning. >>>>>> >>>>>> It seems we're (at least) three who agree about this. Here is a patch >>>>>> fixing the name. >>>>> >>>>> And similar number of people are on the other side. >>>> >>>> If someone already opposed to the renaming (and not only the name) I >>>> must have missed that. >>>> >>>> So you think it's a good idea to keep the name >>>> platform_get_irq_optional() despite the "not found" value returned by it >>>> isn't usable as if it were a normal irq number? >>> >>> I meant that on the other side people who are in favour of Sergey's patch. >>> Since that I commented already that I opposed the renaming being a standalone >>> change. >>> >>> Do you agree that we have several issues with platform_get_irq*() APIs? [...] >>> 2. The vIRQ0 handling: a) WARN() followed by b) returned value 0 >> >> I'm happy with the vIRQ0 handling. Today platform_get_irq() and it's >> silent variant returns either a valid and usuable irq number or a >> negative error value. That's totally fine. > > It might return 0. > Actually it seems that the WARN() can only be issued in two cases: > - SPARC with vIRQ0 in one of the array member > - fallback to ACPI for GPIO IRQ resource with index 0 You have probably missed the recent discovery that arch/sh/boards/board-aps4*.c causes IRQ0 to be passed as a direct IRQ resource? > But the latter is bogus, because it would mean a bug in the ACPI code. Worth changing >= 0 to > 0 there, maybe? > The bottom line here is the SPARC case. Anybody familiar with the platform > can shed a light on this. If there is no such case, we may remove warning > along with ret = 0 case from platfrom_get_irq(). I'm afraid you're too fast here... :-) We'll have a really hard time if we continue to allow IRQ0 to be returned by platform_get_irq() -- we'll have oto fileter it out in the callers then... >>> 3. The specific cookie for "IRQ not found, while no error happened" case >> >> Not sure what you mean here. I have no problem that a situation I can >> cope with is called an error for the query function. I just do error >> handling and continue happily. So the part "while no error happened" is >> irrelevant to me. > > I meant that instead of using special error code, 0 is very much good for > the cases when IRQ is not found. It allows to distinguish -ENXIO from the > low layer from -ENXIO with this magic meaning. I don't see how -ENXIO can trickle from the lower layers, frankly... [...] MBR, Sergey
Hi Sergey, On Mon, Jan 24, 2022 at 10:02 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > On 1/24/22 6:01 PM, Andy Shevchenko wrote: > >>>>>>> It'd certainly be good to name anything that doesn't correspond to one > >>>>>>> of the existing semantics for the API (!) something different rather > >>>>>>> than adding yet another potentially overloaded meaning. > >>>>>> > >>>>>> It seems we're (at least) three who agree about this. Here is a patch > >>>>>> fixing the name. > >>>>> > >>>>> And similar number of people are on the other side. > >>>> > >>>> If someone already opposed to the renaming (and not only the name) I > >>>> must have missed that. > >>>> > >>>> So you think it's a good idea to keep the name > >>>> platform_get_irq_optional() despite the "not found" value returned by it > >>>> isn't usable as if it were a normal irq number? > >>> > >>> I meant that on the other side people who are in favour of Sergey's patch. > >>> Since that I commented already that I opposed the renaming being a standalone > >>> change. > >>> > >>> Do you agree that we have several issues with platform_get_irq*() APIs? > [...] > >>> 2. The vIRQ0 handling: a) WARN() followed by b) returned value 0 > >> > >> I'm happy with the vIRQ0 handling. Today platform_get_irq() and it's > >> silent variant returns either a valid and usuable irq number or a > >> negative error value. That's totally fine. > > > > It might return 0. > > Actually it seems that the WARN() can only be issued in two cases: > > - SPARC with vIRQ0 in one of the array member > > - fallback to ACPI for GPIO IRQ resource with index 0 > > You have probably missed the recent discovery that arch/sh/boards/board-aps4*.c > causes IRQ0 to be passed as a direct IRQ resource? So far no one reported seeing the big fat warning ;-) > > The bottom line here is the SPARC case. Anybody familiar with the platform > > can shed a light on this. If there is no such case, we may remove warning > > along with ret = 0 case from platfrom_get_irq(). > > I'm afraid you're too fast here... :-) > We'll have a really hard time if we continue to allow IRQ0 to be returned by > platform_get_irq() -- we'll have oto fileter it out in the callers then... So far no one reported seeing the big fat warning? > >>> 3. The specific cookie for "IRQ not found, while no error happened" case > >> > >> Not sure what you mean here. I have no problem that a situation I can > >> cope with is called an error for the query function. I just do error > >> handling and continue happily. So the part "while no error happened" is > >> irrelevant to me. > > > > I meant that instead of using special error code, 0 is very much good for > > the cases when IRQ is not found. It allows to distinguish -ENXIO from the > > low layer from -ENXIO with this magic meaning. > > I don't see how -ENXIO can trickle from the lower layers, frankly... It might one day, leading to very hard to track bugs. 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
On Tue, 2022-01-25 at 09:25 +0100, Geert Uytterhoeven wrote: > Hi Sergey, > > On Mon, Jan 24, 2022 at 10:02 PM Sergey Shtylyov <s.shtylyov@omp.ru> > wrote: > > On 1/24/22 6:01 PM, Andy Shevchenko wrote: > > > > > > > > > It'd certainly be good to name anything that doesn't > > > > > > > > > correspond to one > > > > > > > > > of the existing semantics for the API (!) something > > > > > > > > > different rather > > > > > > > > > than adding yet another potentially overloaded > > > > > > > > > meaning. > > > > > > > > > > > > > > > > It seems we're (at least) three who agree about this. > > > > > > > > Here is a patch > > > > > > > > fixing the name. > > > > > > > > > > > > > > And similar number of people are on the other side. > > > > > > > > > > > > If someone already opposed to the renaming (and not only > > > > > > the name) I > > > > > > must have missed that. > > > > > > > > > > > > So you think it's a good idea to keep the name > > > > > > platform_get_irq_optional() despite the "not found" value > > > > > > returned by it > > > > > > isn't usable as if it were a normal irq number? > > > > > > > > > > I meant that on the other side people who are in favour of > > > > > Sergey's patch. > > > > > Since that I commented already that I opposed the renaming > > > > > being a standalone > > > > > change. > > > > > > > > > > Do you agree that we have several issues with > > > > > platform_get_irq*() APIs? > > [...] > > > > > 2. The vIRQ0 handling: a) WARN() followed by b) returned > > > > > value 0 > > > > > > > > I'm happy with the vIRQ0 handling. Today platform_get_irq() and > > > > it's > > > > silent variant returns either a valid and usuable irq number or > > > > a > > > > negative error value. That's totally fine. > > > > > > It might return 0. > > > Actually it seems that the WARN() can only be issued in two > > > cases: > > > - SPARC with vIRQ0 in one of the array member > > > - fallback to ACPI for GPIO IRQ resource with index 0 > > > > You have probably missed the recent discovery that > > arch/sh/boards/board-aps4*.c > > causes IRQ0 to be passed as a direct IRQ resource? > > So far no one reported seeing the big fat warning ;-) FWIW, we had a similar issue with an IRQ resource passed from the tqmx86 MFD driver do the GPIO driver, which we noticed due to this warning, and which was fixed in a946506c48f3bd09363c9d2b0a178e55733bcbb6 and 9b87f43537acfa24b95c236beba0f45901356eb2. I believe these changes are what promted this whole discussion and led to my "Reported-by" on the patch? It is not entirely clear to me when IRQ 0 is valid and when it isn't, but the warning seems useful to me. Maybe it would make more sense to warn when such an IRQ resource is registered for a platform device, and not when it is looked up? My opinion is that it would be very confusing if there are any places in the kernel (on some platforms) where IRQ 0 is valid, but for platform_get_irq() it would suddenly mean "not found". Keeping a negative return value seems preferable to me for this reason. (An alternative, more involved idea would be to add 1 to all IRQ "cookies", so IRQ 0 would return 1, leaving 0 as a special value. I have absolutely no idea how big the API surface is that would need changes, and it is likely not worth the effort at all.) > > > > The bottom line here is the SPARC case. Anybody familiar with the > > > platform > > > can shed a light on this. If there is no such case, we may remove > > > warning > > > along with ret = 0 case from platfrom_get_irq(). > > > > I'm afraid you're too fast here... :-) > > We'll have a really hard time if we continue to allow IRQ0 to be > > returned by > > platform_get_irq() -- we'll have oto fileter it out in the callers > > then... > > So far no one reported seeing the big fat warning? > > > > > > 3. The specific cookie for "IRQ not found, while no error > > > > > happened" case > > > > > > > > Not sure what you mean here. I have no problem that a situation > > > > I can > > > > cope with is called an error for the query function. I just do > > > > error > > > > handling and continue happily. So the part "while no error > > > > happened" is > > > > irrelevant to me. > > > > > > I meant that instead of using special error code, 0 is very much > > > good for > > > the cases when IRQ is not found. It allows to distinguish -ENXIO > > > from the > > > low layer from -ENXIO with this magic meaning. > > > > I don't see how -ENXIO can trickle from the lower layers, > > frankly... > > It might one day, leading to very hard to track bugs. As gregkh noted, changing the return value without also making the compile fail will be a huge PITA whenever driver patches are back- or forward-ported, as it would require subtle changes in error paths, which can easily slip through unnoticed, in particular with half- automated stable backports. Even if another return value like -ENODEV might be better aligned with ...regulator_get_optional() and similar functions, or we even find a way to make 0 usable for this, none of the proposed changes strike me as big enough a win to outweigh the churn caused by making such a change at all. Kind regards, Matthias > > 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
On Tue, Jan 25, 2022 at 01:56:05PM +0100, Matthias Schiffer wrote: > On Tue, 2022-01-25 at 09:25 +0100, Geert Uytterhoeven wrote: > > On Mon, Jan 24, 2022 at 10:02 PM Sergey Shtylyov <s.shtylyov@omp.ru> > > wrote: > > > On 1/24/22 6:01 PM, Andy Shevchenko wrote: ... > > > > > > 2. The vIRQ0 handling: a) WARN() followed by b) returned > > > > > > value 0 > > > > > > > > > > I'm happy with the vIRQ0 handling. Today platform_get_irq() and > > > > > it's > > > > > silent variant returns either a valid and usuable irq number or > > > > > a > > > > > negative error value. That's totally fine. > > > > > > > > It might return 0. > > > > Actually it seems that the WARN() can only be issued in two > > > > cases: > > > > - SPARC with vIRQ0 in one of the array member > > > > - fallback to ACPI for GPIO IRQ resource with index 0 > > > > > > You have probably missed the recent discovery that > > > arch/sh/boards/board-aps4*.c > > > causes IRQ0 to be passed as a direct IRQ resource? > > > > So far no one reported seeing the big fat warning ;-) > > FWIW, we had a similar issue with an IRQ resource passed from the > tqmx86 MFD driver do the GPIO driver, which we noticed due to this > warning, and which was fixed > in a946506c48f3bd09363c9d2b0a178e55733bcbb6 > and 9b87f43537acfa24b95c236beba0f45901356eb2. No, it's not, unfortunately :-( You just band aided the warning issue, but the root cause is the WARN() and possibility to see valid (v)IRQ0 in the resources. See below. > I believe these changes are what promted this whole discussion and led > to my "Reported-by" on the patch? > > It is not entirely clear to me when IRQ 0 is valid and when it isn't, > but the warning seems useful to me. Maybe it would make more sense to > warn when such an IRQ resource is registered for a platform device, and > not when it is looked up? > > My opinion is that it would be very confusing if there are any places > in the kernel (on some platforms) where IRQ 0 is valid, And those places are board files like yours :( They have to be fixed eventually. Ideally by using IRQ domains. At least that's how it's done elsewhere. > but for > platform_get_irq() it would suddenly mean "not found". Keeping a > negative return value seems preferable to me for this reason. IRQ 0 is valid, vIRQ0 (or read it as cookie) is not. Now, the problem in your case is that you are talking about board files, while ACPI and DT never gives resource with vIRQ0. For board files some (legacy) code decides that it's fine to supply HW IRQ, while the de facto case is that platform_get_resource() returns whatever is in the resource, while platform_get_irq() should return a cookie. > (An alternative, more involved idea would be to add 1 to all IRQ > "cookies", so IRQ 0 would return 1, leaving 0 as a special value. I > have absolutely no idea how big the API surface is that would need > changes, and it is likely not worth the effort at all.) This is what IRQ domains do, they start vIRQs from 1. > > > > The bottom line here is the SPARC case. Anybody familiar with the > > > > platform > > > > can shed a light on this. If there is no such case, we may remove > > > > warning > > > > along with ret = 0 case from platfrom_get_irq(). > > > > > > I'm afraid you're too fast here... :-) > > > We'll have a really hard time if we continue to allow IRQ0 to be > > > returned by > > > platform_get_irq() -- we'll have oto fileter it out in the callers > > > then... > > > > So far no one reported seeing the big fat warning? > > > > > > > > 3. The specific cookie for "IRQ not found, while no error > > > > > > happened" case > > > > > > > > > > Not sure what you mean here. I have no problem that a situation > > > > > I can > > > > > cope with is called an error for the query function. I just do > > > > > error > > > > > handling and continue happily. So the part "while no error > > > > > happened" is > > > > > irrelevant to me. > > > > > > > > I meant that instead of using special error code, 0 is very much > > > > good for > > > > the cases when IRQ is not found. It allows to distinguish -ENXIO > > > > from the > > > > low layer from -ENXIO with this magic meaning. > > > > > > I don't see how -ENXIO can trickle from the lower layers, > > > frankly... > > > > It might one day, leading to very hard to track bugs. > > As gregkh noted, changing the return value without also making the > compile fail will be a huge PITA whenever driver patches are back- or > forward-ported, as it would require subtle changes in error paths, > which can easily slip through unnoticed, in particular with half- > automated stable backports. Let's not modify kernel at all then, because in many cases it is a PITA for back- or forward-porting :-) > Even if another return value like -ENODEV might be better aligned with > ...regulator_get_optional() and similar functions, or we even find a > way to make 0 usable for this, none of the proposed changes strike me > as big enough a win to outweigh the churn caused by making such a > change at all. Yeah, let's continue to suffer from ugly interface and see more band aids landing around...
diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 6cb04ac48bf0..acb9962b9889 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -149,7 +149,7 @@ EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname); #endif /* CONFIG_HAS_IOMEM */ /** - * platform_get_irq_optional - get an optional IRQ for a device + * platform_get_irq_silent - get an optional IRQ for a device * @dev: platform device * @num: IRQ number index * @@ -160,13 +160,13 @@ EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname); * * For example:: * - * int irq = platform_get_irq_optional(pdev, 0); + * int irq = platform_get_irq_silent(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) +int platform_get_irq_silent(struct platform_device *dev, unsigned int num) { int ret; #ifdef CONFIG_SPARC @@ -234,7 +234,7 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num) WARN(ret == 0, "0 is an invalid IRQ number\n"); return ret; } -EXPORT_SYMBOL_GPL(platform_get_irq_optional); +EXPORT_SYMBOL_GPL(platform_get_irq_silent); /** * platform_get_irq - get an IRQ for a device @@ -257,7 +257,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_silent(dev, num); if (ret < 0) return dev_err_probe(&dev->dev, ret, "IRQ index %u not found\n", num); @@ -276,7 +276,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_silent(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..73bdbc59c9d0 100644 --- a/drivers/char/ipmi/bt-bmc.c +++ b/drivers/char/ipmi/bt-bmc.c @@ -379,7 +379,7 @@ static int bt_bmc_config_irq(struct bt_bmc *bt_bmc, int rc; u32 reg; - bt_bmc->irq = platform_get_irq_optional(pdev, 0); + bt_bmc->irq = platform_get_irq_silent(pdev, 0); if (bt_bmc->irq < 0) return bt_bmc->irq; diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c index 505cc978c97a..4c666eed24d9 100644 --- a/drivers/char/ipmi/ipmi_si_platform.c +++ b/drivers/char/ipmi/ipmi_si_platform.c @@ -192,7 +192,7 @@ static int platform_ipmi_probe(struct platform_device *pdev) else io.slave_addr = slave_addr; - io.irq = platform_get_irq_optional(pdev, 0); + io.irq = platform_get_irq_silent(pdev, 0); if (io.irq > 0) io.irq_setup = ipmi_std_irq_setup; else @@ -368,7 +368,7 @@ static int acpi_ipmi_probe(struct platform_device *pdev) io.irq = tmp; io.irq_setup = acpi_gpe_irq_setup; } else { - int irq = platform_get_irq_optional(pdev, 0); + int irq = platform_get_irq_silent(pdev, 0); if (irq > 0) { io.irq = irq; diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index d3f2e5364c27..3e6785ad62f2 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -318,7 +318,7 @@ static int tpm_tis_plat_probe(struct platform_device *pdev) } tpm_info.res = *res; - tpm_info.irq = platform_get_irq_optional(pdev, 0); + tpm_info.irq = platform_get_irq_silent(pdev, 0); if (tpm_info.irq <= 0) { if (pdev != force_pdev) tpm_info.irq = -1; diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c index 8514a87fcbee..65b9254e63a9 100644 --- a/drivers/counter/interrupt-cnt.c +++ b/drivers/counter/interrupt-cnt.c @@ -155,7 +155,7 @@ static int interrupt_cnt_probe(struct platform_device *pdev) if (!priv) return -ENOMEM; - priv->irq = platform_get_irq_optional(pdev, 0); + priv->irq = platform_get_irq_silent(pdev, 0); if (priv->irq == -ENXIO) priv->irq = 0; else if (priv->irq < 0) diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index 05f3d7876e44..3d1fe9ba98a7 100644 --- a/drivers/cpufreq/qcom-cpufreq-hw.c +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -374,7 +374,7 @@ static int qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy, int index) * Look for LMh interrupt. If no interrupt line is specified / * if there is an error, allow cpufreq to be enabled as usual. */ - data->throttle_irq = platform_get_irq_optional(pdev, index); + data->throttle_irq = platform_get_irq_silent(pdev, index); if (data->throttle_irq == -ENXIO) return 0; if (data->throttle_irq < 0) diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c index a23563cd118b..707ac21652a6 100644 --- a/drivers/dma/mmp_pdma.c +++ b/drivers/dma/mmp_pdma.c @@ -1059,7 +1059,7 @@ static int mmp_pdma_probe(struct platform_device *op) pdev->dma_channels = dma_channels; for (i = 0; i < dma_channels; i++) { - if (platform_get_irq_optional(op, i) > 0) + if (platform_get_irq_silent(op, i) > 0) irq_num++; } diff --git a/drivers/edac/xgene_edac.c b/drivers/edac/xgene_edac.c index 2ccd1db5e98f..87aa537f3b72 100644 --- a/drivers/edac/xgene_edac.c +++ b/drivers/edac/xgene_edac.c @@ -1916,7 +1916,7 @@ static int xgene_edac_probe(struct platform_device *pdev) int i; for (i = 0; i < 3; i++) { - irq = platform_get_irq_optional(pdev, i); + irq = platform_get_irq_silent(pdev, i); if (irq < 0) { dev_err(&pdev->dev, "No IRQ resource\n"); rc = -EINVAL; diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c index b59fae993626..a1a7d8c0ef13 100644 --- a/drivers/gpio/gpio-altera.c +++ b/drivers/gpio/gpio-altera.c @@ -266,7 +266,7 @@ static int altera_gpio_probe(struct platform_device *pdev) altera_gc->mmchip.gc.owner = THIS_MODULE; altera_gc->mmchip.gc.parent = &pdev->dev; - altera_gc->mapped_irq = platform_get_irq_optional(pdev, 0); + altera_gc->mapped_irq = platform_get_irq_silent(pdev, 0); if (altera_gc->mapped_irq < 0) goto skip_irq; diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c index b0f3aca61974..133153a40808 100644 --- a/drivers/gpio/gpio-dwapb.c +++ b/drivers/gpio/gpio-dwapb.c @@ -543,7 +543,7 @@ static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode, for (j = 0; j < pp->ngpio; j++) { if (has_acpi_companion(dev)) - irq = platform_get_irq_optional(to_platform_device(dev), j); + irq = platform_get_irq_silent(to_platform_device(dev), j); else irq = fwnode_irq_get(fwnode, j); if (irq > 0) diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c index 4c1f9e1091b7..eaaf6fd54d79 100644 --- a/drivers/gpio/gpio-mvebu.c +++ b/drivers/gpio/gpio-mvebu.c @@ -1291,7 +1291,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev) * pins. */ for (i = 0; i < 4; i++) { - int irq = platform_get_irq_optional(pdev, i); + int irq = platform_get_irq_silent(pdev, i); if (irq < 0) continue; diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-otto.c index bd75401b549d..b945049c1a39 100644 --- a/drivers/gpio/gpio-realtek-otto.c +++ b/drivers/gpio/gpio-realtek-otto.c @@ -289,7 +289,7 @@ static int realtek_gpio_probe(struct platform_device *pdev) ctrl->gc.ngpio = ngpios; ctrl->gc.owner = THIS_MODULE; - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (!(dev_flags & GPIO_INTERRUPTS_DISABLED) && irq > 0) { girq = &ctrl->gc.irq; girq->chip = &realtek_gpio_irq_chip; diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c index 5b103221b58d..16afb563f813 100644 --- a/drivers/gpio/gpio-tqmx86.c +++ b/drivers/gpio/gpio-tqmx86.c @@ -236,7 +236,7 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) struct resource *res; int ret, irq; - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq < 0 && irq != -ENXIO) return irq; diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c index b6d3a57e27ed..a451bd19d501 100644 --- a/drivers/gpio/gpio-xilinx.c +++ b/drivers/gpio/gpio-xilinx.c @@ -658,7 +658,7 @@ static int xgpio_probe(struct platform_device *pdev) xgpio_save_regs(chip); - chip->irq = platform_get_irq_optional(pdev, 0); + chip->irq = platform_get_irq_silent(pdev, 0); if (chip->irq <= 0) goto skip_irq; diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c index e714d5318f30..8c88a1958fd4 100644 --- a/drivers/gpu/drm/v3d/v3d_irq.c +++ b/drivers/gpu/drm/v3d/v3d_irq.c @@ -214,7 +214,7 @@ v3d_irq_init(struct v3d_dev *v3d) V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS); V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS); - irq1 = platform_get_irq_optional(v3d_to_pdev(v3d), 1); + irq1 = platform_get_irq_silent(v3d_to_pdev(v3d), 1); if (irq1 == -EPROBE_DEFER) return irq1; if (irq1 > 0) { diff --git a/drivers/i2c/busses/i2c-brcmstb.c b/drivers/i2c/busses/i2c-brcmstb.c index 490ee3962645..0e53d149a207 100644 --- a/drivers/i2c/busses/i2c-brcmstb.c +++ b/drivers/i2c/busses/i2c-brcmstb.c @@ -646,7 +646,7 @@ static int brcmstb_i2c_probe(struct platform_device *pdev) int_name = NULL; /* Get the interrupt number */ - dev->irq = platform_get_irq_optional(pdev, 0); + dev->irq = platform_get_irq_silent(pdev, 0); /* disable the bsc interrupt line */ brcmstb_i2c_enable_disable_irq(dev, INT_DISABLE); diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c index a0af027db04c..c6d21b833964 100644 --- a/drivers/i2c/busses/i2c-ocores.c +++ b/drivers/i2c/busses/i2c-ocores.c @@ -682,7 +682,7 @@ static int ocores_i2c_probe(struct platform_device *pdev) init_waitqueue_head(&i2c->wait); - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); /* * Since the SoC does have an interrupt, its DT has an interrupt * property - But this should be bypassed as the IRQ logic in this diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c index 86d4f75ef8d3..783b474097f7 100644 --- a/drivers/i2c/busses/i2c-pca-platform.c +++ b/drivers/i2c/busses/i2c-pca-platform.c @@ -138,7 +138,7 @@ static int i2c_pca_pf_probe(struct platform_device *pdev) int ret = 0; int irq; - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); /* If irq is 0, we do polling. */ if (irq < 0) irq = 0; diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c index 37f9a4499fdb..934669b20d0d 100644 --- a/drivers/irqchip/irq-renesas-intc-irqpin.c +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c @@ -417,7 +417,7 @@ static int intc_irqpin_probe(struct platform_device *pdev) /* allow any number of IRQs between 1 and INTC_IRQPIN_MAX */ for (k = 0; k < INTC_IRQPIN_MAX; k++) { - ret = platform_get_irq_optional(pdev, k); + ret = platform_get_irq_silent(pdev, k); if (ret == -ENXIO) break; if (ret < 0) diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c index 909325f88239..95ff42746e95 100644 --- a/drivers/irqchip/irq-renesas-irqc.c +++ b/drivers/irqchip/irq-renesas-irqc.c @@ -141,7 +141,7 @@ static int irqc_probe(struct platform_device *pdev) /* allow any number of IRQs between 1 and IRQC_IRQ_MAX */ for (k = 0; k < IRQC_IRQ_MAX; k++) { - ret = platform_get_irq_optional(pdev, k); + ret = platform_get_irq_silent(pdev, k); if (ret == -ENXIO) break; if (ret < 0) diff --git a/drivers/mfd/intel_pmc_bxt.c b/drivers/mfd/intel_pmc_bxt.c index 9f01d38acc7f..dc58dfd87043 100644 --- a/drivers/mfd/intel_pmc_bxt.c +++ b/drivers/mfd/intel_pmc_bxt.c @@ -309,7 +309,7 @@ static int intel_pmc_get_resources(struct platform_device *pdev, struct resource *res; int ret; - scu_data->irq = platform_get_irq_optional(pdev, 0); + scu_data->irq = platform_get_irq_silent(pdev, 0); res = platform_get_resource(pdev, IORESOURCE_MEM, PLAT_RESOURCE_IPC_INDEX); diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c index bcc595c70a9f..aa5579520b06 100644 --- a/drivers/mmc/host/sh_mmcif.c +++ b/drivers/mmc/host/sh_mmcif.c @@ -1396,7 +1396,7 @@ static int sh_mmcif_probe(struct platform_device *pdev) const char *name; irq[0] = platform_get_irq(pdev, 0); - irq[1] = platform_get_irq_optional(pdev, 1); + irq[1] = platform_get_irq_silent(pdev, 1); if (irq[0] < 0) return -ENXIO; diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index f75929783b94..2aa10a1755ba 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2955,7 +2955,7 @@ static int brcmnand_edu_setup(struct platform_device *pdev) /* initialize edu */ brcmnand_edu_init(ctrl); - ctrl->edu_irq = platform_get_irq_optional(pdev, 1); + ctrl->edu_irq = platform_get_irq_silent(pdev, 1); if (ctrl->edu_irq < 0) { dev_warn(dev, "FLASH EDU enabled, using ctlrdy irq\n"); diff --git a/drivers/mtd/nand/raw/renesas-nand-controller.c b/drivers/mtd/nand/raw/renesas-nand-controller.c index 428e08362956..c33958bda059 100644 --- a/drivers/mtd/nand/raw/renesas-nand-controller.c +++ b/drivers/mtd/nand/raw/renesas-nand-controller.c @@ -1354,7 +1354,7 @@ static int rnandc_probe(struct platform_device *pdev) goto disable_hclk; rnandc_dis_interrupts(rnandc); - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq == -EPROBE_DEFER) { ret = irq; goto disable_eclk; diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c index 226f4403cfed..4cfc62a5380a 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c @@ -3989,7 +3989,7 @@ static int bcmgenet_probe(struct platform_device *pdev) err = priv->irq1; goto err; } - priv->wol_irq = platform_get_irq_optional(pdev, 2); + priv->wol_irq = platform_get_irq_silent(pdev, 2); priv->base = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(priv->base)) { diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c index 0985ab216566..43f181e13bab 100644 --- a/drivers/net/ethernet/davicom/dm9000.c +++ b/drivers/net/ethernet/davicom/dm9000.c @@ -1508,7 +1508,7 @@ dm9000_probe(struct platform_device *pdev) goto out; } - db->irq_wake = platform_get_irq_optional(pdev, 1); + db->irq_wake = platform_get_irq_silent(pdev, 1); if (db->irq_wake >= 0) { dev_dbg(db->dev, "wakeup irq %d\n", db->irq_wake); diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index af99017a5453..dc65bb1caad4 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -612,7 +612,7 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx) irq = platform_get_irq_byname_optional(pdev, "pps"); if (irq < 0) - irq = platform_get_irq_optional(pdev, irq_idx); + irq = platform_get_irq_silent(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/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c index ef878973b859..cdf4ff41bd66 100644 --- a/drivers/net/ethernet/marvell/mvmdio.c +++ b/drivers/net/ethernet/marvell/mvmdio.c @@ -349,7 +349,7 @@ static int orion_mdio_probe(struct platform_device *pdev) } - dev->err_interrupt = platform_get_irq_optional(pdev, 0); + dev->err_interrupt = platform_get_irq_silent(pdev, 0); if (dev->err_interrupt > 0 && resource_size(r) < MVMDIO_ERR_INT_MASK + 4) { dev_err(&pdev->dev, diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c index 31df3267a01a..30d5a785d485 100644 --- a/drivers/net/ethernet/ti/davinci_emac.c +++ b/drivers/net/ethernet/ti/davinci_emac.c @@ -1455,7 +1455,7 @@ static int emac_dev_open(struct net_device *ndev) /* Request IRQ */ if (dev_of_node(&priv->pdev->dev)) { - while ((ret = platform_get_irq_optional(priv->pdev, res_num)) != -ENXIO) { + while ((ret = platform_get_irq_silent(priv->pdev, res_num)) != -ENXIO) { if (ret < 0) goto rollback; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 23ac353b35fe..5be7e93d4087 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1961,13 +1961,13 @@ static int axienet_probe(struct platform_device *pdev) lp->rx_irq = irq_of_parse_and_map(np, 1); lp->tx_irq = irq_of_parse_and_map(np, 0); of_node_put(np); - lp->eth_irq = platform_get_irq_optional(pdev, 0); + lp->eth_irq = platform_get_irq_silent(pdev, 0); } else { /* Check for these resources directly on the Ethernet node. */ lp->dma_regs = devm_platform_get_and_ioremap_resource(pdev, 1, NULL); lp->rx_irq = platform_get_irq(pdev, 1); lp->tx_irq = platform_get_irq(pdev, 0); - lp->eth_irq = platform_get_irq_optional(pdev, 2); + lp->eth_irq = platform_get_irq_silent(pdev, 2); } if (IS_ERR(lp->dma_regs)) { dev_err(&pdev->dev, "could not map DMA regs\n"); diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c index c49108a72865..c4a709470398 100644 --- a/drivers/perf/arm_smmuv3_pmu.c +++ b/drivers/perf/arm_smmuv3_pmu.c @@ -852,7 +852,7 @@ static int smmu_pmu_probe(struct platform_device *pdev) smmu_pmu->reloc_base = smmu_pmu->reg_base; } - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq > 0) smmu_pmu->irq = irq; diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c index 9de617ca9daa..9cbd4e396f0f 100644 --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c @@ -672,7 +672,7 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) channel->obint_enable_bits = USB2_OBINT_BITS; /* get irq number here and request_irq for OTG in phy_init */ - channel->irq = platform_get_irq_optional(pdev, 0); + channel->irq = platform_get_irq_silent(pdev, 0); channel->dr_mode = rcar_gen3_get_dr_mode(dev->of_node); if (channel->dr_mode != USB_DR_MODE_UNKNOWN) { int ret; diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c index 52fa2f4cd618..1acf9355ab44 100644 --- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c +++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c @@ -848,7 +848,7 @@ static int iproc_gpio_probe(struct platform_device *pdev) "gpio-ranges"); /* optional GPIO interrupt support */ - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq > 0) { struct irq_chip *irqc; struct gpio_irq_chip *girq; diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c index 4c01333e1406..cc5a74aea6e5 100644 --- a/drivers/pinctrl/intel/pinctrl-baytrail.c +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c @@ -1568,7 +1568,7 @@ static int byt_gpio_probe(struct intel_pinctrl *vg) #endif /* set up interrupts */ - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq > 0) { struct gpio_irq_chip *girq; diff --git a/drivers/pinctrl/intel/pinctrl-lynxpoint.c b/drivers/pinctrl/intel/pinctrl-lynxpoint.c index 561fa322b0b4..984c5c0b4304 100644 --- a/drivers/pinctrl/intel/pinctrl-lynxpoint.c +++ b/drivers/pinctrl/intel/pinctrl-lynxpoint.c @@ -879,7 +879,7 @@ static int lp_gpio_probe(struct platform_device *pdev) gc->parent = dev; /* set up interrupts */ - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq > 0) { struct gpio_irq_chip *girq; diff --git a/drivers/pinctrl/pinctrl-keembay.c b/drivers/pinctrl/pinctrl-keembay.c index 152c35bce8ec..628f642ec220 100644 --- a/drivers/pinctrl/pinctrl-keembay.c +++ b/drivers/pinctrl/pinctrl-keembay.c @@ -1490,7 +1490,7 @@ static int keembay_gpiochip_probe(struct keembay_pinctrl *kpc, struct keembay_gpio_irq *kmb_irq = &kpc->irq[i]; int irq; - irq = platform_get_irq_optional(pdev, i); + irq = platform_get_irq_silent(pdev, i); if (irq <= 0) continue; diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c index 0f6e9305fec5..47160ec0407c 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.c +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c @@ -1108,7 +1108,7 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) } drvdata->dev = dev; - ret = platform_get_irq_optional(pdev, 0); + ret = platform_get_irq_silent(pdev, 0); if (ret < 0 && ret != -ENXIO) return ret; if (ret > 0) diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c index d6306d2a096f..30f06d4b6ad8 100644 --- a/drivers/platform/chrome/cros_ec_lpc.c +++ b/drivers/platform/chrome/cros_ec_lpc.c @@ -397,7 +397,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev) * Some boards do not have an IRQ allotted for cros_ec_lpc, * which makes ENXIO an expected (and safe) scenario. */ - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq > 0) ec_dev->irq = irq; else if (irq != -ENXIO) { diff --git a/drivers/platform/x86/hp_accel.c b/drivers/platform/x86/hp_accel.c index e9f852f7c27f..bffc9093a629 100644 --- a/drivers/platform/x86/hp_accel.c +++ b/drivers/platform/x86/hp_accel.c @@ -305,7 +305,7 @@ static int lis3lv02d_probe(struct platform_device *device) lis3_dev.write = lis3lv02d_acpi_write; /* obtain IRQ number of our device from ACPI */ - ret = platform_get_irq_optional(device, 0); + ret = platform_get_irq_silent(device, 0); if (ret > 0) lis3_dev.irq = ret; diff --git a/drivers/platform/x86/intel/punit_ipc.c b/drivers/platform/x86/intel/punit_ipc.c index 66bb39fd0ef9..2f22d5de767a 100644 --- a/drivers/platform/x86/intel/punit_ipc.c +++ b/drivers/platform/x86/intel/punit_ipc.c @@ -277,7 +277,7 @@ static int intel_punit_ipc_probe(struct platform_device *pdev) platform_set_drvdata(pdev, punit_ipcdev); - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq < 0) { dev_warn(&pdev->dev, "Invalid IRQ, using polling mode\n"); } else { diff --git a/drivers/platform/x86/intel_scu_pltdrv.c b/drivers/platform/x86/intel_scu_pltdrv.c index 56ec6ae4c824..2d3e5174da8e 100644 --- a/drivers/platform/x86/intel_scu_pltdrv.c +++ b/drivers/platform/x86/intel_scu_pltdrv.c @@ -23,7 +23,7 @@ static int intel_scu_platform_probe(struct platform_device *pdev) struct intel_scu_ipc_dev *scu; const struct resource *res; - scu_data.irq = platform_get_irq_optional(pdev, 0); + scu_data.irq = platform_get_irq_silent(pdev, 0); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) return -ENOMEM; diff --git a/drivers/power/supply/mp2629_charger.c b/drivers/power/supply/mp2629_charger.c index bdf924b73e47..e279a4bdf6a4 100644 --- a/drivers/power/supply/mp2629_charger.c +++ b/drivers/power/supply/mp2629_charger.c @@ -580,7 +580,7 @@ static int mp2629_charger_probe(struct platform_device *pdev) charger->dev = dev; platform_set_drvdata(pdev, charger); - irq = platform_get_irq_optional(to_platform_device(dev->parent), 0); + irq = platform_get_irq_silent(to_platform_device(dev->parent), 0); if (irq < 0) { dev_err(dev, "get irq fail: %d\n", irq); return irq; diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c index f0f6b9b6daec..aebc8a73acfe 100644 --- a/drivers/rtc/rtc-m48t59.c +++ b/drivers/rtc/rtc-m48t59.c @@ -421,7 +421,7 @@ static int m48t59_rtc_probe(struct platform_device *pdev) /* Try to get irq number. We also can work in * the mode without IRQ. */ - m48t59->irq = platform_get_irq_optional(pdev, 0); + m48t59->irq = platform_get_irq_silent(pdev, 0); if (m48t59->irq <= 0) m48t59->irq = NO_IRQ; diff --git a/drivers/spi/spi-hisi-sfc-v3xx.c b/drivers/spi/spi-hisi-sfc-v3xx.c index d3a23b1c2a4c..76f4934a23e7 100644 --- a/drivers/spi/spi-hisi-sfc-v3xx.c +++ b/drivers/spi/spi-hisi-sfc-v3xx.c @@ -451,7 +451,7 @@ static int hisi_sfc_v3xx_probe(struct platform_device *pdev) goto err_put_master; } - host->irq = platform_get_irq_optional(pdev, 0); + host->irq = platform_get_irq_silent(pdev, 0); if (host->irq == -EPROBE_DEFER) { ret = -EPROBE_DEFER; goto err_put_master; diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c index 5c93730615f8..64aec31355bb 100644 --- a/drivers/spi/spi-mtk-nor.c +++ b/drivers/spi/spi-mtk-nor.c @@ -828,7 +828,7 @@ static int mtk_nor_probe(struct platform_device *pdev) mtk_nor_init(sp); - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq < 0) { dev_warn(sp->dev, "IRQ not available."); diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c index 43eb25b167bc..ef6c6880a943 100644 --- a/drivers/thermal/rcar_gen3_thermal.c +++ b/drivers/thermal/rcar_gen3_thermal.c @@ -429,7 +429,7 @@ static int rcar_gen3_thermal_request_irqs(struct rcar_gen3_thermal_priv *priv, int ret, irq; for (i = 0; i < 2; i++) { - irq = platform_get_irq_optional(pdev, i); + irq = platform_get_irq_silent(pdev, i); if (irq < 0) return irq; diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index fb65dc601b23..1f4cbe37627e 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -585,7 +585,7 @@ static int mtk8250_probe(struct platform_device *pdev) goto err_pm_disable; } - data->rx_wakeup_irq = platform_get_irq_optional(pdev, 1); + data->rx_wakeup_irq = platform_get_irq_silent(pdev, 1); return 0; diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c index 37bffe406b18..1cd2bdee3d40 100644 --- a/drivers/tty/serial/altera_jtaguart.c +++ b/drivers/tty/serial/altera_jtaguart.c @@ -439,7 +439,7 @@ static int altera_jtaguart_probe(struct platform_device *pdev) else return -ENODEV; - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq < 0 && irq != -ENXIO) return irq; if (irq > 0) diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c index 64a352b40197..415883ccfbbd 100644 --- a/drivers/tty/serial/altera_uart.c +++ b/drivers/tty/serial/altera_uart.c @@ -576,7 +576,7 @@ static int altera_uart_probe(struct platform_device *pdev) else return -EINVAL; - ret = platform_get_irq_optional(pdev, 0); + ret = platform_get_irq_silent(pdev, 0); if (ret < 0 && ret != -ENXIO) return ret; if (ret > 0) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index df8a0c8b8b29..8791f51e52cb 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -2258,8 +2258,8 @@ static int imx_uart_probe(struct platform_device *pdev) rxirq = platform_get_irq(pdev, 0); if (rxirq < 0) return rxirq; - txirq = platform_get_irq_optional(pdev, 1); - rtsirq = platform_get_irq_optional(pdev, 2); + txirq = platform_get_irq_silent(pdev, 1); + rtsirq = platform_get_irq_silent(pdev, 2); sport->port.dev = &pdev->dev; sport->port.mapbase = res->start; diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index aedc38893e6c..893922e520a9 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -1406,7 +1406,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE); if (!console) - port->wakeup_irq = platform_get_irq_optional(pdev, 1); + port->wakeup_irq = platform_get_irq_silent(pdev, 1); if (of_property_read_bool(pdev->dev.of_node, "rx-tx-swap")) port->rx_tx_swap = true; diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index 968967d722d4..f2fb298b3aed 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -2860,7 +2860,7 @@ static int sci_init_single(struct platform_device *dev, for (i = 0; i < ARRAY_SIZE(sci_port->irqs); ++i) { if (i) - sci_port->irqs[i] = platform_get_irq_optional(dev, i); + sci_port->irqs[i] = platform_get_irq_silent(dev, i); else sci_port->irqs[i] = platform_get_irq(dev, i); } diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c index 63258b6accc4..a2673a8ebd3f 100644 --- a/drivers/uio/uio_pdrv_genirq.c +++ b/drivers/uio/uio_pdrv_genirq.c @@ -160,7 +160,7 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev) priv->pdev = pdev; if (!uioinfo->irq) { - ret = platform_get_irq_optional(pdev, 0); + ret = platform_get_irq_silent(pdev, 0); uioinfo->irq = ret; if (ret == -ENXIO) uioinfo->irq = UIO_IRQ_NONE; diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c index 68cd4b68e3a2..5237c62f60f0 100644 --- a/drivers/usb/phy/phy-tegra-usb.c +++ b/drivers/usb/phy/phy-tegra-usb.c @@ -1353,7 +1353,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev) return -ENOMEM; tegra_phy->soc_config = of_device_get_match_data(&pdev->dev); - tegra_phy->irq = platform_get_irq_optional(pdev, 0); + tegra_phy->irq = platform_get_irq_silent(pdev, 0); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 68a1c87066d7..60b4f5ce5aa1 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -33,7 +33,7 @@ static int get_platform_irq(struct vfio_platform_device *vdev, int i) { struct platform_device *pdev = (struct platform_device *) vdev->opaque; - return platform_get_irq_optional(pdev, i); + return platform_get_irq_silent(pdev, i); } static int vfio_platform_probe(struct platform_device *pdev) diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c index cd578843277e..9c792ab66a83 100644 --- a/drivers/watchdog/dw_wdt.c +++ b/drivers/watchdog/dw_wdt.c @@ -617,7 +617,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev) * pending either until the next watchdog kick event or up to the * system reset. */ - ret = platform_get_irq_optional(pdev, 0); + ret = platform_get_irq_silent(pdev, 0); if (ret > 0) { ret = devm_request_irq(dev, ret, dw_wdt_irq, IRQF_SHARED | IRQF_TRIGGER_RISING, diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c index 127eefc9161d..c533fbb37895 100644 --- a/drivers/watchdog/orion_wdt.c +++ b/drivers/watchdog/orion_wdt.c @@ -602,7 +602,7 @@ static int orion_wdt_probe(struct platform_device *pdev) set_bit(WDOG_HW_RUNNING, &dev->wdt.status); /* Request the IRQ only after the watchdog is disabled */ - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq > 0) { /* * Not all supported platforms specify an interrupt for the @@ -617,7 +617,7 @@ static int orion_wdt_probe(struct platform_device *pdev) } /* Optional 2nd interrupt for pretimeout */ - irq = platform_get_irq_optional(pdev, 1); + irq = platform_get_irq_silent(pdev, 1); if (irq > 0) { orion_wdt_info.options |= WDIOF_PRETIMEOUT; ret = devm_request_irq(&pdev->dev, irq, orion_wdt_pre_irq, diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c index 0d2209c5eaca..f1bbfed047a1 100644 --- a/drivers/watchdog/qcom-wdt.c +++ b/drivers/watchdog/qcom-wdt.c @@ -257,7 +257,7 @@ static int qcom_wdt_probe(struct platform_device *pdev) } /* check if there is pretimeout support */ - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (data->pretimeout && irq > 0) { ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0, "wdt_bark", &wdt->wdd); diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 7c96f169d274..6d495f15f717 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -69,7 +69,14 @@ extern void __iomem * devm_platform_ioremap_resource_byname(struct platform_device *pdev, const char *name); extern int platform_get_irq(struct platform_device *, unsigned int); -extern int platform_get_irq_optional(struct platform_device *, unsigned int); +extern int platform_get_irq_silent(struct platform_device *, unsigned int); + +/* + * platform_get_irq_optional was recently renamed to platform_get_irq_silent. + * Fixup users to not break patches that were created before the rename. + */ +#define platform_get_irq_optional(pdev, index) platform_get_irq_silent(pdev, index) + extern int platform_irq_count(struct platform_device *); extern int devm_platform_get_irqs_affinity(struct platform_device *dev, struct irq_affinity *affd, diff --git a/sound/soc/dwc/dwc-i2s.c b/sound/soc/dwc/dwc-i2s.c index 5cb58929090d..f7cfe8f7cce0 100644 --- a/sound/soc/dwc/dwc-i2s.c +++ b/sound/soc/dwc/dwc-i2s.c @@ -642,7 +642,7 @@ static int dw_i2s_probe(struct platform_device *pdev) dev->dev = &pdev->dev; - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq >= 0) { ret = devm_request_irq(&pdev->dev, irq, i2s_irq_handler, 0, pdev->name, dev); diff --git a/sound/soc/intel/keembay/kmb_platform.c b/sound/soc/intel/keembay/kmb_platform.c index a6fb74ba1c42..ee0159b7e9f6 100644 --- a/sound/soc/intel/keembay/kmb_platform.c +++ b/sound/soc/intel/keembay/kmb_platform.c @@ -882,7 +882,7 @@ static int kmb_plat_dai_probe(struct platform_device *pdev) kmb_i2s->use_pio = !(of_property_read_bool(np, "dmas")); if (kmb_i2s->use_pio) { - irq = platform_get_irq_optional(pdev, 0); + irq = platform_get_irq_silent(pdev, 0); if (irq > 0) { ret = devm_request_irq(dev, irq, kmb_i2s_irq_handler, 0, pdev->name, kmb_i2s);