* [RFC PATCH] move aic7xxx to pci_request_irq
2006-10-03 22:07 [RFC PATCH] add pci_{request,free}_irq take #2 Frederik Deweerdt
@ 2006-10-03 22:14 ` Frederik Deweerdt
2006-10-03 22:14 ` [RFC PATCH] add pci_{request,free}_irq take #2 Jeff Garzik
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Frederik Deweerdt @ 2006-10-03 22:14 UTC (permalink / raw)
To: linux-kernel; +Cc: arjan, matthew, alan, jeff, akpm, rdunlap, gregkh
Hi,
This proof-of-concept patch converts the aic7xxx driver to use the
pci_request_irq() function.
Please note that I'm not submitting the driver changes, they're there
only for illustration purposes. I'll CC the appropriate maintainers
when/if an API is agreed upon.
Regards,
Frederik
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.h b/drivers/scsi/aic7xxx/aic7xxx_osm.h
index 86ea7ac..24f70bc 100644
Index: 2.6.18-mm3/drivers/scsi/aic7xxx/aic7xxx_osm.h
===================================================================
--- 2.6.18-mm3.orig/drivers/scsi/aic7xxx/aic7xxx_osm.h
+++ 2.6.18-mm3/drivers/scsi/aic7xxx/aic7xxx_osm.h
@@ -561,7 +561,6 @@ static inline void ahc_linux_eisa_exit(v
int ahc_linux_pci_init(void);
void ahc_linux_pci_exit(void);
int ahc_pci_map_registers(struct ahc_softc *ahc);
-int ahc_pci_map_int(struct ahc_softc *ahc);
static __inline uint32_t ahc_pci_read_config(ahc_dev_softc_t pci,
int reg, int width);
Index: 2.6.18-mm3/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
===================================================================
--- 2.6.18-mm3.orig/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
+++ 2.6.18-mm3/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
@@ -368,16 +368,3 @@ ahc_pci_map_registers(struct ahc_softc *
return (error);
}
-int
-ahc_pci_map_int(struct ahc_softc *ahc)
-{
- int error;
-
- error = request_irq(ahc->dev_softc->irq, ahc_linux_isr,
- IRQF_SHARED, "aic7xxx", ahc);
- if (error == 0)
- ahc->platform_data->irq = ahc->dev_softc->irq;
-
- return (-error);
-}
-
Index: 2.6.18-mm3/drivers/scsi/aic7xxx/aic7xxx_pci.c
===================================================================
--- 2.6.18-mm3.orig/drivers/scsi/aic7xxx/aic7xxx_pci.c
+++ 2.6.18-mm3/drivers/scsi/aic7xxx/aic7xxx_pci.c
@@ -968,13 +968,19 @@ ahc_pci_config(struct ahc_softc *ahc, st
/*
* Allow interrupts now that we are completely setup.
- */
- error = ahc_pci_map_int(ahc);
- if (error != 0)
- return (error);
+ *
+ * Note: pci_request_irq return value is negated due to aic7xxx
+ * error handling style
+ */
+ error = -pci_request_irq(ahc->dev_softc, ahc_linux_isr,
+ IRQF_SHARED, "aic7xxx");
+
+ if (!error) {
+ ahc->platform_data->irq = ahc->dev_softc->irq;
+ ahc->init_level++;
+ }
- ahc->init_level++;
- return (0);
+ return error;
}
/*
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC PATCH] add pci_{request,free}_irq take #2
2006-10-03 22:07 [RFC PATCH] add pci_{request,free}_irq take #2 Frederik Deweerdt
2006-10-03 22:14 ` [RFC PATCH] move aic7xxx to pci_request_irq Frederik Deweerdt
@ 2006-10-03 22:14 ` Jeff Garzik
2006-10-03 22:29 ` Frederik Deweerdt
2006-10-03 22:19 ` [RFC PATCH] move aic79xx to pci_request_irq Frederik Deweerdt
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2006-10-03 22:14 UTC (permalink / raw)
To: Frederik Deweerdt
Cc: linux-kernel, arjan, matthew, alan, akpm, rdunlap, gregkh
Frederik Deweerdt wrote:
> +int pci_request_irq(struct pci_dev *pdev,
> + irqreturn_t(*handler) (int, void *, struct pt_regs *),
> + unsigned long flags, const char *name)
> +{
> + if (!is_irq_valid(pdev->irq)) {
> + dev_printk(KERN_ERR, &pdev->dev,
> + "No usable irq line was found (got #%d)\n",
> + pdev->irq);
> + return -EINVAL;
> + }
> +
> + return request_irq(pdev->irq, handler, flags,
> + name ? name : pdev->driver->name,
> + pci_get_drvdata(pdev));
> +}
> +EXPORT_SYMBOL(pci_request_irq);
> +void pci_free_irq(struct pci_dev *pdev)
> +{
> + free_irq(pdev->irq, pci_get_drvdata(pdev));
> +}
> +EXPORT_SYMBOL(pci_free_irq);
ACK these parts
> Index: 2.6.18-mm3/include/linux/interrupt.h
> ===================================================================
> --- 2.6.18-mm3.orig/include/linux/interrupt.h
> +++ 2.6.18-mm3/include/linux/interrupt.h
> @@ -75,6 +75,13 @@ struct irqaction {
> struct proc_dir_entry *dir;
> };
>
> +#ifndef ARCH_VALIDATE_PCI_IRQ
> +static inline int is_irq_valid(unsigned int irq)
> +{
> + return irq ? 1 : 0;
> +}
> +#endif /* ARCH_VALIDATE_PCI_IRQ */
It's not appropriate to have PCI IRQ stuff in linux/interrupt.h.
This is precisely why I passed 'struct pci_dev *' to a PCI-specific irq
validation function, and prototyped it in linux/pci.h.
Jeff
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC PATCH] add pci_{request,free}_irq take #2
2006-10-03 22:14 ` [RFC PATCH] add pci_{request,free}_irq take #2 Jeff Garzik
@ 2006-10-03 22:29 ` Frederik Deweerdt
2006-10-03 22:37 ` Jeff Garzik
0 siblings, 1 reply; 15+ messages in thread
From: Frederik Deweerdt @ 2006-10-03 22:29 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-kernel, arjan, matthew, alan, akpm, rdunlap, gregkh
> >Index: 2.6.18-mm3/include/linux/interrupt.h
> >===================================================================
> >--- 2.6.18-mm3.orig/include/linux/interrupt.h
> >+++ 2.6.18-mm3/include/linux/interrupt.h
> >@@ -75,6 +75,13 @@ struct irqaction {
> > struct proc_dir_entry *dir;
> > };
> > +#ifndef ARCH_VALIDATE_PCI_IRQ
> >+static inline int is_irq_valid(unsigned int irq)
> >+{
> >+ return irq ? 1 : 0;
> >+}
> >+#endif /* ARCH_VALIDATE_PCI_IRQ */
>
> It's not appropriate to have PCI IRQ stuff in linux/interrupt.h.
>
> This is precisely why I passed 'struct pci_dev *' to a PCI-specific irq validation function, and prototyped it in linux/pci.h.
>
My bad, I've mixed your proposal and Matthew's, isn't this just a
matter of:
s/ARCH_VALIDATE_PCI_IRQ/ARCH_VALIDATE_IRQ/ ?
I'll look if there's some non-PCI code that might check the irq's value
and thus might benefit from this.
Regards,
Frederik
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC PATCH] add pci_{request,free}_irq take #2
2006-10-03 22:29 ` Frederik Deweerdt
@ 2006-10-03 22:37 ` Jeff Garzik
2006-10-04 2:59 ` Matthew Wilcox
0 siblings, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2006-10-03 22:37 UTC (permalink / raw)
To: Frederik Deweerdt
Cc: linux-kernel, arjan, matthew, alan, akpm, rdunlap, gregkh
Frederik Deweerdt wrote:
> My bad, I've mixed your proposal and Matthew's, isn't this just a
> matter of:
> s/ARCH_VALIDATE_PCI_IRQ/ARCH_VALIDATE_IRQ/ ?
>
> I'll look if there's some non-PCI code that might check the irq's value
> and thus might benefit from this.
The irq value comes from the PCI subsystem... The PCI subsystem should
validate it.
Jeff
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC PATCH] add pci_{request,free}_irq take #2
2006-10-03 22:37 ` Jeff Garzik
@ 2006-10-04 2:59 ` Matthew Wilcox
0 siblings, 0 replies; 15+ messages in thread
From: Matthew Wilcox @ 2006-10-04 2:59 UTC (permalink / raw)
To: Jeff Garzik
Cc: Frederik Deweerdt, linux-kernel, arjan, alan, akpm, rdunlap,
gregkh
On Tue, Oct 03, 2006 at 06:37:12PM -0400, Jeff Garzik wrote:
> Frederik Deweerdt wrote:
> >My bad, I've mixed your proposal and Matthew's, isn't this just a
> >matter of:
> >s/ARCH_VALIDATE_PCI_IRQ/ARCH_VALIDATE_IRQ/ ?
> >
> >I'll look if there's some non-PCI code that might check the irq's value
> >and thus might benefit from this.
>
> The irq value comes from the PCI subsystem... The PCI subsystem should
> validate it.
That's not true. The value in the pci_dev->irq field has been changed
by the architecture. See, for example, pci_read_irq_line() in
arch/powerpc/kernel/pci_32.c. It's a Linux IRQ number, not a PCI IRQ
number.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH] move aic79xx to pci_request_irq
2006-10-03 22:07 [RFC PATCH] add pci_{request,free}_irq take #2 Frederik Deweerdt
2006-10-03 22:14 ` [RFC PATCH] move aic7xxx to pci_request_irq Frederik Deweerdt
2006-10-03 22:14 ` [RFC PATCH] add pci_{request,free}_irq take #2 Jeff Garzik
@ 2006-10-03 22:19 ` Frederik Deweerdt
2006-10-03 22:22 ` [RFC PATCH] move tg3 " Frederik Deweerdt
2006-10-03 22:23 ` [RFC PATCH] move e1000 " Frederik Deweerdt
4 siblings, 0 replies; 15+ messages in thread
From: Frederik Deweerdt @ 2006-10-03 22:19 UTC (permalink / raw)
To: linux-kernel; +Cc: arjan, matthew, alan, jeff, akpm, rdunlap, gregkh
Hi,
This proof-of-concept patch converts the aic79xx driver to use the
pci_request_irq() function.
Please note that I'm not submitting the driver changes, they're there
only for illustration purposes. I'll CC the appropriate maintainers
when/if an API is agreed upon.
Regards,
Frederik
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.h b/drivers/scsi/aic7xxx/aic79xx_osm.h
index 448c39c..67897d4 100644
Index: 2.6.18-mm3/drivers/scsi/aic7xxx/aic79xx_osm.h
===================================================================
--- 2.6.18-mm3.orig/drivers/scsi/aic7xxx/aic79xx_osm.h
+++ 2.6.18-mm3/drivers/scsi/aic7xxx/aic79xx_osm.h
@@ -594,7 +594,6 @@ void ahd_power_state_change(struct ahd_s
int ahd_linux_pci_init(void);
void ahd_linux_pci_exit(void);
int ahd_pci_map_registers(struct ahd_softc *ahd);
-int ahd_pci_map_int(struct ahd_softc *ahd);
static __inline uint32_t ahd_pci_read_config(ahd_dev_softc_t pci,
int reg, int width);
Index: 2.6.18-mm3/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
===================================================================
--- 2.6.18-mm3.orig/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
+++ 2.6.18-mm3/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
@@ -336,19 +336,6 @@ ahd_pci_map_registers(struct ahd_softc *
return (error);
}
-int
-ahd_pci_map_int(struct ahd_softc *ahd)
-{
- int error;
-
- error = request_irq(ahd->dev_softc->irq, ahd_linux_isr,
- IRQF_SHARED, "aic79xx", ahd);
- if (!error)
- ahd->platform_data->irq = ahd->dev_softc->irq;
-
- return (-error);
-}
-
void
ahd_power_state_change(struct ahd_softc *ahd, ahd_power_state new_state)
{
Index: 2.6.18-mm3/drivers/scsi/aic7xxx/aic79xx_pci.c
===================================================================
--- 2.6.18-mm3.orig/drivers/scsi/aic7xxx/aic79xx_pci.c
+++ 2.6.18-mm3/drivers/scsi/aic7xxx/aic79xx_pci.c
@@ -376,10 +376,18 @@ ahd_pci_config(struct ahd_softc *ahd, st
/*
* Allow interrupts now that we are completely setup.
+ *
+ * Note: pci_request_irq return value is negated due to aic79xx
+ * error handling style
*/
- error = ahd_pci_map_int(ahd);
- if (!error)
+
+ error = -pci_request_irq(ahd->dev_softc, ahd_linux_isr,
+ IRQF_SHARED, "aic79xx");
+ if (!error) {
+ ahd->platform_data->irq = ahd->dev_softc->irq;
ahd->init_level++;
+ }
+
return error;
}
^ permalink raw reply [flat|nested] 15+ messages in thread* [RFC PATCH] move tg3 to pci_request_irq
2006-10-03 22:07 [RFC PATCH] add pci_{request,free}_irq take #2 Frederik Deweerdt
` (2 preceding siblings ...)
2006-10-03 22:19 ` [RFC PATCH] move aic79xx to pci_request_irq Frederik Deweerdt
@ 2006-10-03 22:22 ` Frederik Deweerdt
2006-10-03 22:37 ` Jeff Garzik
2006-10-03 22:23 ` [RFC PATCH] move e1000 " Frederik Deweerdt
4 siblings, 1 reply; 15+ messages in thread
From: Frederik Deweerdt @ 2006-10-03 22:22 UTC (permalink / raw)
To: linux-kernel; +Cc: arjan, matthew, alan, jeff, akpm, rdunlap, gregkh
Hi,
This proof-of-concept patch converts the tg3 driver to use the
pci_request_irq() function.
Please note that I'm not submitting the driver changes, they're there
only for illustration purposes. I'll CC the appropriate maintainers
when/if an API is agreed upon.
Regards,
Frederik
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index c25ba27..23660c6 100644
Index: 2.6.18-mm3/drivers/net/tg3.c
===================================================================
--- 2.6.18-mm3.orig/drivers/net/tg3.c
+++ 2.6.18-mm3/drivers/net/tg3.c
@@ -6853,7 +6853,7 @@ static int tg3_request_irq(struct tg3 *t
fn = tg3_interrupt_tagged;
flags = IRQF_SHARED | IRQF_SAMPLE_RANDOM;
}
- return (request_irq(tp->pdev->irq, fn, flags, dev->name, dev));
+ return pci_request_irq(tp->pdev, fn, flags, dev->name);
}
static int tg3_test_interrupt(struct tg3 *tp)
@@ -6866,10 +6866,10 @@ static int tg3_test_interrupt(struct tg3
tg3_disable_ints(tp);
- free_irq(tp->pdev->irq, dev);
+ pci_free_irq(tp->pdev);
- err = request_irq(tp->pdev->irq, tg3_test_isr,
- IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev);
+ err = pci_request_irq(tp->pdev, tg3_test_isr,
+ IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name);
if (err)
return err;
@@ -6897,7 +6897,7 @@ static int tg3_test_interrupt(struct tg3
tg3_disable_ints(tp);
- free_irq(tp->pdev->irq, dev);
+ pci_free_irq(tp->pdev);
err = tg3_request_irq(tp);
@@ -6915,7 +6915,6 @@ static int tg3_test_interrupt(struct tg3
*/
static int tg3_test_msi(struct tg3 *tp)
{
- struct net_device *dev = tp->dev;
int err;
u16 pci_cmd;
@@ -6946,7 +6945,7 @@ static int tg3_test_msi(struct tg3 *tp)
"the PCI maintainer and include system chipset information.\n",
tp->dev->name);
- free_irq(tp->pdev->irq, dev);
+ pci_free_irq(tp->pdev);
pci_disable_msi(tp->pdev);
tp->tg3_flags2 &= ~TG3_FLG2_USING_MSI;
@@ -6966,7 +6965,7 @@ static int tg3_test_msi(struct tg3 *tp)
tg3_full_unlock(tp);
if (err)
- free_irq(tp->pdev->irq, dev);
+ pci_free_irq(tp->pdev);
return err;
}
@@ -7051,7 +7050,7 @@ static int tg3_open(struct net_device *d
tg3_full_unlock(tp);
if (err) {
- free_irq(tp->pdev->irq, dev);
+ pci_free_irq(tp->pdev);
if (tp->tg3_flags2 & TG3_FLG2_USING_MSI) {
pci_disable_msi(tp->pdev);
tp->tg3_flags2 &= ~TG3_FLG2_USING_MSI;
@@ -7363,7 +7362,7 @@ static int tg3_close(struct net_device *
tg3_full_unlock(tp);
- free_irq(tp->pdev->irq, dev);
+ pci_free_irq(tp->pdev);
if (tp->tg3_flags2 & TG3_FLG2_USING_MSI) {
pci_disable_msi(tp->pdev);
tp->tg3_flags2 &= ~TG3_FLG2_USING_MSI;
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC PATCH] move tg3 to pci_request_irq
2006-10-03 22:22 ` [RFC PATCH] move tg3 " Frederik Deweerdt
@ 2006-10-03 22:37 ` Jeff Garzik
2006-10-03 22:41 ` Frederik Deweerdt
0 siblings, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2006-10-03 22:37 UTC (permalink / raw)
To: Frederik Deweerdt
Cc: linux-kernel, arjan, matthew, alan, akpm, rdunlap, gregkh
Frederik Deweerdt wrote:
> Hi,
>
> This proof-of-concept patch converts the tg3 driver to use the
> pci_request_irq() function.
>
> Please note that I'm not submitting the driver changes, they're there
> only for illustration purposes. I'll CC the appropriate maintainers
> when/if an API is agreed upon.
>
> Regards,
> Frederik
>
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index c25ba27..23660c6 100644
> Index: 2.6.18-mm3/drivers/net/tg3.c
> ===================================================================
> --- 2.6.18-mm3.orig/drivers/net/tg3.c
> +++ 2.6.18-mm3/drivers/net/tg3.c
> @@ -6853,7 +6853,7 @@ static int tg3_request_irq(struct tg3 *t
> fn = tg3_interrupt_tagged;
> flags = IRQF_SHARED | IRQF_SAMPLE_RANDOM;
> }
> - return (request_irq(tp->pdev->irq, fn, flags, dev->name, dev));
> + return pci_request_irq(tp->pdev, fn, flags, dev->name);
> }
>
> static int tg3_test_interrupt(struct tg3 *tp)
> @@ -6866,10 +6866,10 @@ static int tg3_test_interrupt(struct tg3
>
> tg3_disable_ints(tp);
>
> - free_irq(tp->pdev->irq, dev);
> + pci_free_irq(tp->pdev);
>
> - err = request_irq(tp->pdev->irq, tg3_test_isr,
> - IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev);
> + err = pci_request_irq(tp->pdev, tg3_test_isr,
> + IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name);
IRQF_SHARED flags are still left hanging around...
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] move tg3 to pci_request_irq
2006-10-03 22:37 ` Jeff Garzik
@ 2006-10-03 22:41 ` Frederik Deweerdt
2006-10-04 5:38 ` Jeff Garzik
0 siblings, 1 reply; 15+ messages in thread
From: Frederik Deweerdt @ 2006-10-03 22:41 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-kernel, arjan, matthew, alan, akpm, rdunlap, gregkh
On Tue, Oct 03, 2006 at 06:37:43PM -0400, Jeff Garzik wrote:
> Frederik Deweerdt wrote:
> >Hi,
> >This proof-of-concept patch converts the tg3 driver to use the
> >pci_request_irq() function.
> >Please note that I'm not submitting the driver changes, they're there
> >only for illustration purposes. I'll CC the appropriate maintainers
> >when/if an API is agreed upon.
> >Regards,
> >Frederik diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> >index c25ba27..23660c6 100644
> >Index: 2.6.18-mm3/drivers/net/tg3.c
> >===================================================================
> >--- 2.6.18-mm3.orig/drivers/net/tg3.c
> >+++ 2.6.18-mm3/drivers/net/tg3.c
> >@@ -6853,7 +6853,7 @@ static int tg3_request_irq(struct tg3 *t
> > fn = tg3_interrupt_tagged;
> > flags = IRQF_SHARED | IRQF_SAMPLE_RANDOM;
> > }
> >- return (request_irq(tp->pdev->irq, fn, flags, dev->name, dev));
> >+ return pci_request_irq(tp->pdev, fn, flags, dev->name);
> > }
> > static int tg3_test_interrupt(struct tg3 *tp)
> >@@ -6866,10 +6866,10 @@ static int tg3_test_interrupt(struct tg3
> > tg3_disable_ints(tp);
> > - free_irq(tp->pdev->irq, dev);
> >+ pci_free_irq(tp->pdev);
> > - err = request_irq(tp->pdev->irq, tg3_test_isr,
> >- IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev);
> >+ err = pci_request_irq(tp->pdev, tg3_test_isr,
> >+ IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name);
>
> IRQF_SHARED flags are still left hanging around...
I did it on purpose (see parent post): some parts of tg3 for example
don't pass IRQF_SHARED, so I though it wasn't a good idea to enforce
IRQF_SHARED in all cases. Did I miss something?
Frederik
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] move tg3 to pci_request_irq
2006-10-03 22:41 ` Frederik Deweerdt
@ 2006-10-04 5:38 ` Jeff Garzik
2006-10-04 6:27 ` David Miller
0 siblings, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2006-10-04 5:38 UTC (permalink / raw)
To: Frederik Deweerdt
Cc: linux-kernel, arjan, matthew, alan, akpm, rdunlap, gregkh
Frederik Deweerdt wrote:
> On Tue, Oct 03, 2006 at 06:37:43PM -0400, Jeff Garzik wrote:
>> Frederik Deweerdt wrote:
>>> Hi,
>>> This proof-of-concept patch converts the tg3 driver to use the
>>> pci_request_irq() function.
>>> Please note that I'm not submitting the driver changes, they're there
>>> only for illustration purposes. I'll CC the appropriate maintainers
>>> when/if an API is agreed upon.
>>> Regards,
>>> Frederik diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
>>> index c25ba27..23660c6 100644
>>> Index: 2.6.18-mm3/drivers/net/tg3.c
>>> ===================================================================
>>> --- 2.6.18-mm3.orig/drivers/net/tg3.c
>>> +++ 2.6.18-mm3/drivers/net/tg3.c
>>> @@ -6853,7 +6853,7 @@ static int tg3_request_irq(struct tg3 *t
>>> fn = tg3_interrupt_tagged;
>>> flags = IRQF_SHARED | IRQF_SAMPLE_RANDOM;
>>> }
>>> - return (request_irq(tp->pdev->irq, fn, flags, dev->name, dev));
>>> + return pci_request_irq(tp->pdev, fn, flags, dev->name);
>>> }
>>> static int tg3_test_interrupt(struct tg3 *tp)
>>> @@ -6866,10 +6866,10 @@ static int tg3_test_interrupt(struct tg3
>>> tg3_disable_ints(tp);
>>> - free_irq(tp->pdev->irq, dev);
>>> + pci_free_irq(tp->pdev);
>>> - err = request_irq(tp->pdev->irq, tg3_test_isr,
>>> - IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev);
>>> + err = pci_request_irq(tp->pdev, tg3_test_isr,
>>> + IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name);
>> IRQF_SHARED flags are still left hanging around...
> I did it on purpose (see parent post): some parts of tg3 for example
> don't pass IRQF_SHARED, so I though it wasn't a good idea to enforce
> IRQF_SHARED in all cases. Did I miss something?
When it's hardcoded into the definition of pci_request_irq(), then
setting the flag in the above code is logically superfluous.
As for why the flag may be missing -- PCI MSI interrupts are never
shared. However, it won't _hurt_ anything to set the flag needlessly,
AFAIK.
Jeff
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] move tg3 to pci_request_irq
2006-10-04 5:38 ` Jeff Garzik
@ 2006-10-04 6:27 ` David Miller
2006-10-04 9:09 ` Frederik Deweerdt
0 siblings, 1 reply; 15+ messages in thread
From: David Miller @ 2006-10-04 6:27 UTC (permalink / raw)
To: jeff; +Cc: deweerdt, linux-kernel, arjan, matthew, alan, akpm, rdunlap,
gregkh
From: Jeff Garzik <jeff@garzik.org>
Date: Wed, 04 Oct 2006 01:38:31 -0400
> As for why the flag may be missing -- PCI MSI interrupts are never
> shared. However, it won't _hurt_ anything to set the flag needlessly,
> AFAIK.
That's right.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] move tg3 to pci_request_irq
2006-10-04 6:27 ` David Miller
@ 2006-10-04 9:09 ` Frederik Deweerdt
0 siblings, 0 replies; 15+ messages in thread
From: Frederik Deweerdt @ 2006-10-04 9:09 UTC (permalink / raw)
To: David Miller
Cc: jeff, linux-kernel, arjan, matthew, alan, akpm, rdunlap, gregkh
On Tue, Oct 03, 2006 at 11:27:48PM -0700, David Miller wrote:
> From: Jeff Garzik <jeff@garzik.org>
> Date: Wed, 04 Oct 2006 01:38:31 -0400
>
> > As for why the flag may be missing -- PCI MSI interrupts are never
> > shared. However, it won't _hurt_ anything to set the flag needlessly,
> > AFAIK.
>
> That's right.
Just to make sure I understood: does this means that the code at lines
296 and 297 in e1000_main.c can be safely removed?
296 if (adapter->have_msi)
297 flags &= ~IRQF_SHARED;
298 #endif
299 if ((err = request_irq(adapter->pdev->irq, &e1000_intr, flags,
300 netdev->name, netdev)))
> -
> 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/
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH] move e1000 to pci_request_irq
2006-10-03 22:07 [RFC PATCH] add pci_{request,free}_irq take #2 Frederik Deweerdt
` (3 preceding siblings ...)
2006-10-03 22:22 ` [RFC PATCH] move tg3 " Frederik Deweerdt
@ 2006-10-03 22:23 ` Frederik Deweerdt
2006-10-03 22:36 ` Jeff Garzik
4 siblings, 1 reply; 15+ messages in thread
From: Frederik Deweerdt @ 2006-10-03 22:23 UTC (permalink / raw)
To: linux-kernel; +Cc: arjan, matthew, alan, jeff, akpm, rdunlap, gregkh
Hi,
This proof-of-concept patch converts the e1000 driver to use the
pci_request_irq() function.
Please note that I'm not submitting the driver changes, they're there
only for illustration purposes. I'll CC the appropriate maintainers
when/if an API is agreed upon.
Regards,
Frederik
diff --git a/drivers/net/e1000/e1000_ethtool.c b/drivers/net/e1000/e1000_ethtool.c
index 778ede3..8b7be73 100644
Index: 2.6.18-mm3/drivers/net/e1000/e1000_ethtool.c
===================================================================
--- 2.6.18-mm3.orig/drivers/net/e1000/e1000_ethtool.c
+++ 2.6.18-mm3/drivers/net/e1000/e1000_ethtool.c
@@ -899,17 +899,16 @@ e1000_intr_test(struct e1000_adapter *ad
{
struct net_device *netdev = adapter->netdev;
uint32_t mask, i=0, shared_int = TRUE;
- uint32_t irq = adapter->pdev->irq;
*data = 0;
/* NOTE: we don't test MSI interrupts here, yet */
/* Hook up test interrupt handler just for this test */
- if (!request_irq(irq, &e1000_test_intr, IRQF_PROBE_SHARED,
- netdev->name, netdev))
+ if (!pci_request_irq(adapter->pdev, &e1000_test_intr, IRQF_PROBE_SHARED,
+ netdev->name))
shared_int = FALSE;
- else if (request_irq(irq, &e1000_test_intr, IRQF_SHARED,
- netdev->name, netdev)) {
+ else if (pci_request_irq(adapter->pdev, &e1000_test_intr, IRQF_SHARED,
+ netdev->name)) {
*data = 1;
return -1;
}
@@ -986,7 +985,7 @@ e1000_intr_test(struct e1000_adapter *ad
msleep(10);
/* Unhook test interrupt handler */
- free_irq(irq, netdev);
+ pci_free_irq(adapter->pdev);
return *data;
}
Index: 2.6.18-mm3/drivers/net/e1000/e1000_main.c
===================================================================
--- 2.6.18-mm3.orig/drivers/net/e1000/e1000_main.c
+++ 2.6.18-mm3/drivers/net/e1000/e1000_main.c
@@ -296,19 +296,13 @@ static int e1000_request_irq(struct e100
if (adapter->have_msi)
flags &= ~IRQF_SHARED;
#endif
- if ((err = request_irq(adapter->pdev->irq, &e1000_intr, flags,
- netdev->name, netdev)))
- DPRINTK(PROBE, ERR,
- "Unable to allocate interrupt Error: %d\n", err);
-
+ err = pci_request_irq(adapter->pdev, &e1000_intr, flags, netdev->name);
return err;
}
static void e1000_free_irq(struct e1000_adapter *adapter)
{
- struct net_device *netdev = adapter->netdev;
-
- free_irq(adapter->pdev->irq, netdev);
+ pci_free_irq(adapter->pdev);
#ifdef CONFIG_PCI_MSI
if (adapter->have_msi)
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC PATCH] move e1000 to pci_request_irq
2006-10-03 22:23 ` [RFC PATCH] move e1000 " Frederik Deweerdt
@ 2006-10-03 22:36 ` Jeff Garzik
0 siblings, 0 replies; 15+ messages in thread
From: Jeff Garzik @ 2006-10-03 22:36 UTC (permalink / raw)
To: Frederik Deweerdt
Cc: linux-kernel, arjan, matthew, alan, akpm, rdunlap, gregkh
Frederik Deweerdt wrote:
> + else if (pci_request_irq(adapter->pdev, &e1000_test_intr, IRQF_SHARED,
> + netdev->name)) {
doesn't need IRQF_SHARED flag anymore
^ permalink raw reply [flat|nested] 15+ messages in thread