Messages in this thread | | | Date | Sun, 1 Oct 2006 08:28:07 -0600 | From | Matthew Wilcox <> | Subject | Re: [-mm patch] aic7xxx: check irq validity (was Re: 2.6.18-mm2) |
| |
On Sat, Sep 30, 2006 at 07:58:18PM -0400, Jeff Garzik wrote: > Actually, rather than adding this check to every driver, I would rather > do something like the attached patch: create a pci_request_irq(), and > pass a struct pci_device to it. Then the driver author doesn't have to > worry about such details.
I like pci_request_irq(), but pci_valid_irq is bad.
> +#ifndef ARCH_VALIDATE_PCI_IRQ > +int pci_valid_irq(struct pci_dev *pdev) > +{ > + if (pdev->irq == 0) > + return -EINVAL; > + > + return 0; > +} > +EXPORT_SYMBOL(pci_valid_irq); > +#endif /* ARCH_VALIDATE_PCI_IRQ */
Better would be:
#ifndef ARCH_VALIDATE_IRQ static inline int valid_irq(unsigned int irq) { return irq ? 1 : 0; } #endif
in linux/interrupt.h (around request_irq).
And it doesn't need to be a __must_check. There's no point -- it has no side-effects. The only reason to call it is if you want the answer to the question. You had the sense of the return code wrong too; you want to use it as:
int pci_request_irq(struct pci_dev *pdev, irq_handler_t handler, unsigned long flags, const char *name, void *data) { if (!valid_irq(pdev->irq)) { dev_printk(KERN_ERR, &pdev->dev, "invalid irq\n"); return -EINVAL; }
return request_irq(pdev->irq, handler, flags | IRQF_SHARED, name, data); }
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |