diff mbox series

driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent()

Message ID 20220113194358.xnnbhsoyetihterb@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent() | expand

Commit Message

Uwe Kleine-König Jan. 13, 2022, 7:43 p.m. UTC
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.

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(-)

Comments

Peter Korsgaard Jan. 14, 2022, 6:57 a.m. UTC | #1
>>>>> "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>
Andy Shevchenko Jan. 14, 2022, 1:04 p.m. UTC | #2
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.
Sergey Shtylyov Jan. 14, 2022, 7:45 p.m. UTC | #3
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
Uwe Kleine-König Jan. 14, 2022, 8:29 p.m. UTC | #4
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
Sergey Shtylyov Jan. 15, 2022, 1:08 p.m. UTC | #5
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
Uwe Kleine-König Jan. 15, 2022, 3:45 p.m. UTC | #6
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
Andy Shevchenko Jan. 19, 2022, 6:51 p.m. UTC | #7
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
Andy Shevchenko Jan. 19, 2022, 6:52 p.m. UTC | #8
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.
Sergey Shtylyov Jan. 19, 2022, 7:47 p.m. UTC | #9
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
Andy Shevchenko Jan. 19, 2022, 8:55 p.m. UTC | #10
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
Andy Shevchenko Jan. 24, 2022, 3:01 p.m. UTC | #11
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.
Sergey Shtylyov Jan. 24, 2022, 9:02 p.m. UTC | #12
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
Geert Uytterhoeven Jan. 25, 2022, 8:25 a.m. UTC | #13
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
Matthias Schiffer Jan. 25, 2022, 12:56 p.m. UTC | #14
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
Andy Shevchenko Jan. 25, 2022, 2:01 p.m. UTC | #15
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 mbox series

Patch

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);