Re: [RFC PATCH] pci_request_irq (was [-mm patch] aic7xxx: check irq validity)

Randy Dunlap
2006-10-03T03:56:53Z

On Mon, 2 Oct 2006 20:00:48 +0000 Frederik Deweerdt wrote:

> Hi all,
>
> I've tried to summarize the different proposals made by Jeff Garzik,
> Matthew Wilcox and Arjan van de Ven in the "[-mm patch] aic7xxx: check
> irq validity" thread. I've also added:
> - some kerneldoc

The kernel-doc needs some repair -- see below.

> - renamed valid_irq to is_irq_valid()
> - added pci_release_irq().
>
> I'll send a follow-up patch showing the implied modifications for the
> following - semi-randomly chosen :) - drivers: aic7xxx, aic79xx, tg3
> and drm.
>
> Regards,
> Frederik
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index a544997..ae20a3a 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -15,6 +15,7 @@ #include <linux/init.h>
> #include <linux/pci.h>
> #include <linux/module.h>
> #include <linux/spinlock.h>
> +#include <linux/interrupt.h>
> #include <linux/string.h>
> #include <asm/dma.h> /* isa_dma_bridge_buggy */
> #include "pci.h"
> @@ -810,6 +811,49 @@ err_out:
> }
>
> /**
> + * pci_request_irq - Reserve an IRQ for a PCI device
> + * @pdev: The PCI device whose irq is to be reserved
> + * handler: The interrupt handler function,

* @handler: ...

> + * pci_get_drvdata(pdev) shall be passed as an argument to that function
> + * @flags: The flags to be passed to request_irq()
> + * @name: The name of the device to be associated with the irq
> + *
> + * Returns 0 on success, or a negative value on error. A warning
> + * message is also printed on failure.
> + */
> +int pci_request_irq(struct pci_dev *pdev,
> + irqreturn_t (*handler)(int, void *, struct pt_regs *),
> + unsigned long flags, const char *name)
> +{
> + int rc;
> + const char *actual_name = name;
> +
> + rc = is_irq_valid(pdev->irq);
> + if (!rc) {
> + dev_printk(KERN_ERR, &pdev->dev, "invalid irq #%d\n", pdev->irq);
> + return -EINVAL;
> + }
> +
> + if (!actual_name)
> + actual_name = pci_name(pdev);
> +
> + return request_irq(pdev->irq, handler, flags | IRQF_SHARED,
> + actual_name, pci_get_drvdata(pdev));
> +}
> +EXPORT_SYMBOL(pci_request_irq);
> +
> +/**
> + * pci_free_irq - releases the interrupt line reserved to the PCI
> + * device pointed by @pdev

The first line is function name and <<short>> function description.
It cannot extend more than one line (combined).
If you want to use more text for function description,
you can do so after the list of parameters. See example below.

> + * @pdev: the PCI device whose interrupt is to be freed
*
* This froofroo_irq function only does this on odd phases of
* the moon.

> + */
> +void pci_free_irq(struct pci_dev *pdev)
> +{
> + free_irq(pdev->irq, pci_get_drvdata(pdev));
> +}
> +EXPORT_SYMBOL(pci_free_irq);
> +
> +/**
> * pci_set_master - enables bus-mastering for device dev
> * @dev: the PCI device to enable
> *

---
~Randy