Skip to content

Commit

Permalink
driver core: platform: Clarify that IRQ 0 is invalid
Browse files Browse the repository at this point in the history
These interfaces return a negative error number or an IRQ:

  platform_get_irq()
  platform_get_irq_optional()
  platform_get_irq_byname()
  platform_get_irq_byname_optional()

The function comments suggest checking for error like this:

  irq = platform_get_irq(...);
  if (irq < 0)
    return irq;

which is what most callers (~900 of 1400) do, so it's implicit that IRQ 0
is invalid.  But some callers check for "irq <= 0", and it's not obvious
from the source that we never return an IRQ 0.

Make this more explicit by updating the comments to say that an IRQ number
is always non-zero and adding a WARN() if we ever do return zero.  If we do
return IRQ 0, it likely indicates a bug in the arch-specific parts of
platform_get_irq().

Relevant prior discussion at [1, 2].

[1] https://lore.kernel.org/r/Pine.LNX.4.64.0701250940220.25027@woody.linux-foundation.org/
[2] https://lore.kernel.org/r/Pine.LNX.4.64.0701252029570.25027@woody.linux-foundation.org/
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
  • Loading branch information
Bjorn Helgaas committed May 12, 2020
1 parent 8f3d9f3 commit a85a6c8
Showing 1 changed file with 25 additions and 15 deletions.
40 changes: 25 additions & 15 deletions drivers/base/platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,31 +152,32 @@ EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname);
* if (irq < 0)
* return irq;
*
* Return: IRQ number on success, negative error number on failure.
* 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 ret;
#ifdef CONFIG_SPARC
/* sparc does not have irqs represented as IORESOURCE_IRQ resources */
if (!dev || num >= dev->archdata.num_irqs)
return -ENXIO;
return dev->archdata.irqs[num];
ret = dev->archdata.irqs[num];
goto out;
#else
struct resource *r;
int ret;

if (IS_ENABLED(CONFIG_OF_IRQ) && dev->dev.of_node) {
ret = of_irq_get(dev->dev.of_node, num);
if (ret > 0 || ret == -EPROBE_DEFER)
return ret;
goto out;
}

r = platform_get_resource(dev, IORESOURCE_IRQ, num);
if (has_acpi_companion(&dev->dev)) {
if (r && r->flags & IORESOURCE_DISABLED) {
ret = acpi_irq_get(ACPI_HANDLE(&dev->dev), num, r);
if (ret)
return ret;
goto out;
}
}

Expand All @@ -190,13 +191,17 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
struct irq_data *irqd;

irqd = irq_get_irq_data(r->start);
if (!irqd)
return -ENXIO;
if (!irqd) {
ret = -ENXIO;
goto out;
}
irqd_set_trigger_type(irqd, r->flags & IORESOURCE_BITS);
}

if (r)
return r->start;
if (r) {
ret = r->start;
goto out;
}

/*
* For the index 0 interrupt, allow falling back to GpioInt
Expand All @@ -209,11 +214,14 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
ret = acpi_dev_gpio_irq_get(ACPI_COMPANION(&dev->dev), num);
/* Our callers expect -ENXIO for missing IRQs. */
if (ret >= 0 || ret == -EPROBE_DEFER)
return ret;
goto out;
}

return -ENXIO;
ret = -ENXIO;
#endif
out:
WARN(ret == 0, "0 is an invalid IRQ number\n");
return ret;
}
EXPORT_SYMBOL_GPL(platform_get_irq_optional);

Expand All @@ -231,7 +239,7 @@ EXPORT_SYMBOL_GPL(platform_get_irq_optional);
* if (irq < 0)
* return irq;
*
* Return: IRQ number on success, negative error number on failure.
* Return: non-zero IRQ number on success, negative error number on failure.
*/
int platform_get_irq(struct platform_device *dev, unsigned int num)
{
Expand Down Expand Up @@ -303,8 +311,10 @@ static int __platform_get_irq_byname(struct platform_device *dev,
}

r = platform_get_resource_byname(dev, IORESOURCE_IRQ, name);
if (r)
if (r) {
WARN(r->start == 0, "0 is an invalid IRQ number\n");
return r->start;
}

return -ENXIO;
}
Expand All @@ -316,7 +326,7 @@ static int __platform_get_irq_byname(struct platform_device *dev,
*
* Get an IRQ like platform_get_irq(), but then by name rather then by index.
*
* Return: IRQ number on success, negative error number on failure.
* Return: non-zero IRQ number on success, negative error number on failure.
*/
int platform_get_irq_byname(struct platform_device *dev, const char *name)
{
Expand All @@ -338,7 +348,7 @@ EXPORT_SYMBOL_GPL(platform_get_irq_byname);
* Get an optional IRQ by name like platform_get_irq_byname(). Except that it
* does not print an error message if an IRQ can not be obtained.
*
* Return: IRQ number on success, negative error number on failure.
* Return: non-zero IRQ number on success, negative error number on failure.
*/
int platform_get_irq_byname_optional(struct platform_device *dev,
const char *name)
Expand Down

0 comments on commit a85a6c8

Please sign in to comment.