Skip to content

Commit a85a6c8

Browse files
committed
driver core: platform: Clarify that IRQ 0 is invalid
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/[email protected]/ [2] https://lore.kernel.org/r/[email protected]/ Signed-off-by: Bjorn Helgaas <[email protected]> Acked-by: Greg Kroah-Hartman <[email protected]> Acked-by: Linus Walleij <[email protected]>
1 parent 8f3d9f3 commit a85a6c8

File tree

1 file changed

+25
-15
lines changed

1 file changed

+25
-15
lines changed

drivers/base/platform.c

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -152,31 +152,32 @@ EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname);
152152
* if (irq < 0)
153153
* return irq;
154154
*
155-
* Return: IRQ number on success, negative error number on failure.
155+
* Return: non-zero IRQ number on success, negative error number on failure.
156156
*/
157157
int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
158158
{
159+
int ret;
159160
#ifdef CONFIG_SPARC
160161
/* sparc does not have irqs represented as IORESOURCE_IRQ resources */
161162
if (!dev || num >= dev->archdata.num_irqs)
162163
return -ENXIO;
163-
return dev->archdata.irqs[num];
164+
ret = dev->archdata.irqs[num];
165+
goto out;
164166
#else
165167
struct resource *r;
166-
int ret;
167168

168169
if (IS_ENABLED(CONFIG_OF_IRQ) && dev->dev.of_node) {
169170
ret = of_irq_get(dev->dev.of_node, num);
170171
if (ret > 0 || ret == -EPROBE_DEFER)
171-
return ret;
172+
goto out;
172173
}
173174

174175
r = platform_get_resource(dev, IORESOURCE_IRQ, num);
175176
if (has_acpi_companion(&dev->dev)) {
176177
if (r && r->flags & IORESOURCE_DISABLED) {
177178
ret = acpi_irq_get(ACPI_HANDLE(&dev->dev), num, r);
178179
if (ret)
179-
return ret;
180+
goto out;
180181
}
181182
}
182183

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

192193
irqd = irq_get_irq_data(r->start);
193-
if (!irqd)
194-
return -ENXIO;
194+
if (!irqd) {
195+
ret = -ENXIO;
196+
goto out;
197+
}
195198
irqd_set_trigger_type(irqd, r->flags & IORESOURCE_BITS);
196199
}
197200

198-
if (r)
199-
return r->start;
201+
if (r) {
202+
ret = r->start;
203+
goto out;
204+
}
200205

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

215-
return -ENXIO;
220+
ret = -ENXIO;
216221
#endif
222+
out:
223+
WARN(ret == 0, "0 is an invalid IRQ number\n");
224+
return ret;
217225
}
218226
EXPORT_SYMBOL_GPL(platform_get_irq_optional);
219227

@@ -231,7 +239,7 @@ EXPORT_SYMBOL_GPL(platform_get_irq_optional);
231239
* if (irq < 0)
232240
* return irq;
233241
*
234-
* Return: IRQ number on success, negative error number on failure.
242+
* Return: non-zero IRQ number on success, negative error number on failure.
235243
*/
236244
int platform_get_irq(struct platform_device *dev, unsigned int num)
237245
{
@@ -303,8 +311,10 @@ static int __platform_get_irq_byname(struct platform_device *dev,
303311
}
304312

305313
r = platform_get_resource_byname(dev, IORESOURCE_IRQ, name);
306-
if (r)
314+
if (r) {
315+
WARN(r->start == 0, "0 is an invalid IRQ number\n");
307316
return r->start;
317+
}
308318

309319
return -ENXIO;
310320
}
@@ -316,7 +326,7 @@ static int __platform_get_irq_byname(struct platform_device *dev,
316326
*
317327
* Get an IRQ like platform_get_irq(), but then by name rather then by index.
318328
*
319-
* Return: IRQ number on success, negative error number on failure.
329+
* Return: non-zero IRQ number on success, negative error number on failure.
320330
*/
321331
int platform_get_irq_byname(struct platform_device *dev, const char *name)
322332
{
@@ -338,7 +348,7 @@ EXPORT_SYMBOL_GPL(platform_get_irq_byname);
338348
* Get an optional IRQ by name like platform_get_irq_byname(). Except that it
339349
* does not print an error message if an IRQ can not be obtained.
340350
*
341-
* Return: IRQ number on success, negative error number on failure.
351+
* Return: non-zero IRQ number on success, negative error number on failure.
342352
*/
343353
int platform_get_irq_byname_optional(struct platform_device *dev,
344354
const char *name)

0 commit comments

Comments
 (0)