[patch 2.5] tg3.c: pci_{save,restore}_extended_state

[patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Ivan Kokshaysky]

This fixes actual bug when the card lose its MSI state after
reset.

The additional tg3 specific fix is still required,
as Jeff.Wiedemeier@hp.com pointed out:
> The TG3 will only do MSI if it's device-specific MSGINT_MODE register is
> set to enable MSI. If it's not set that way, you don't get MSI even if
> the config space says you do.

Ivan.

--- 2.5.59/drivers/net/tg3.c	Fri Jan 17 05:22:16 2003
+++ linux/drivers/net/tg3.c	Fri Jan 24 19:28:44 2003
@@ -3096,7 +3096,12 @@ static void tg3_chip_reset(struct tg3 *t
 		val |= PCISTATE_RETRY_SAME_DMA;
 	pci_write_config_dword(tp->pdev, TG3PCI_PCISTATE, val);
 
-	pci_restore_state(tp->pdev, tp->pci_cfg_state);
+	pci_restore_extended_state(tp->pdev, tp->pci_cfg_state);
+
+	/* Make sure MSGINT_MODE is set if MSI is configured. */
+	pci_read_config_dword(tp->pdev, TG3PCI_MSI_CAP_ID, &val);
+	if ((val >> 16) & PCI_MSI_FLAGS_ENABLE)
+		tw32(MSGINT_MODE, MSGINT_MODE_ENABLE);
 
 	/* Make sure PCI-X relaxed ordering bit is clear. */
 	pci_read_config_dword(tp->pdev, TG3PCI_X_CAPS, &val);
@@ -6684,11 +6689,19 @@ static int __devinit tg3_init_one(struct
 	spin_lock_init(&tp->tx_lock);
 	spin_lock_init(&tp->indirect_lock);
 
+	err = -ENOMEM;
+
+	tp->pci_cfg_state = pci_alloc_extended_state(pdev);
+	if (!tp->pci_cfg_state) {
+		printk(KERN_ERR PFX "Cannot allocate pci_cfg_state, "
+		       "aborting.\n");
+		goto err_out_free_dev;
+	}
+
 	tp->regs = (unsigned long) ioremap(tg3reg_base, tg3reg_len);
 	if (tp->regs == 0UL) {
 		printk(KERN_ERR PFX "Cannot map device registers, "
 		       "aborting.\n");
-		err = -ENOMEM;
 		goto err_out_free_dev;
 	}
 
@@ -6766,7 +6779,7 @@ static int __devinit tg3_init_one(struct
 	 * of the PCI config space.  We need to restore this after
 	 * GRC_MISC_CFG core clock resets and some resume events.
 	 */
-	pci_save_state(tp->pdev, tp->pci_cfg_state);
+	pci_save_extended_state(tp->pdev, tp->pci_cfg_state);
 
 	printk(KERN_INFO "%s: Tigon3 [partno(%s) rev %04x PHY(%s)] (PCI%s:%s:%s) %sBaseT Ethernet ",
 	       dev->name,
@@ -6806,8 +6819,11 @@ static void __devexit tg3_remove_one(str
 	struct net_device *dev = pci_get_drvdata(pdev);
 
 	if (dev) {
+		struct tg3 *tp = dev->priv;
+
 		unregister_netdev(dev);
-		iounmap((void *) ((struct tg3 *)(dev->priv))->regs);
+		iounmap((void *)tp->regs);
+		kfree(tp->pci_cfg_state);
 		kfree(dev);
 		pci_release_regions(pdev);
 		pci_disable_device(pdev);
--- 2.5.59/drivers/net/tg3.h	Fri Jan 17 05:21:42 2003
+++ linux/drivers/net/tg3.h	Fri Jan 24 19:24:02 2003
@@ -1847,7 +1847,7 @@ struct tg3 {
 	u8				pci_lat_timer;
 	u8				pci_hdr_type;
 	u8				pci_bist;
-	u32				pci_cfg_state[64 / sizeof(u32)];
+	u32				*pci_cfg_state;
 
 	int				pm_cap;
 

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Jeff Garzik]

On Fri, Jan 24, 2003 at 09:27:48PM +0300, Ivan Kokshaysky wrote:
> --- 2.5.59/drivers/net/tg3.c	Fri Jan 17 05:22:16 2003
> +++ linux/drivers/net/tg3.c	Fri Jan 24 19:28:44 2003
> @@ -3096,7 +3096,12 @@ static void tg3_chip_reset(struct tg3 *t
>  		val |= PCISTATE_RETRY_SAME_DMA;
>  	pci_write_config_dword(tp->pdev, TG3PCI_PCISTATE, val);
>  
> -	pci_restore_state(tp->pdev, tp->pci_cfg_state);
> +	pci_restore_extended_state(tp->pdev, tp->pci_cfg_state);
> +
> +	/* Make sure MSGINT_MODE is set if MSI is configured. */
> +	pci_read_config_dword(tp->pdev, TG3PCI_MSI_CAP_ID, &val);
> +	if ((val >> 16) & PCI_MSI_FLAGS_ENABLE)
> +		tw32(MSGINT_MODE, MSGINT_MODE_ENABLE);
>  
>  	/* Make sure PCI-X relaxed ordering bit is clear. */
>  	pci_read_config_dword(tp->pdev, TG3PCI_X_CAPS, &val);

hmmmm.  We don't use MSI in the driver anymore, unless I am missing
something.

So, the above patch is probably the wrong thing to do.  I'll need to
check to be sure, but I think that h/w reset clears MSGINT_MODE_ENABLE,
so we wouldn't want to be randomly enabling it when it is purposefully
disabled.

DaveM/Jeff, corrections?

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[David S. Miller]

   From: Jeff Garzik <jgarzik@pobox.com>
   Date: Fri, 24 Jan 2003 14:31:35 -0500

   So, the above patch is probably the wrong thing to do.  I'll need to
   check to be sure, but I think that h/w reset clears MSGINT_MODE_ENABLE,
   so we wouldn't want to be randomly enabling it when it is purposefully
   disabled.
   
   DaveM/Jeff, corrections?
   
You're right Jeff, I hadn't noticed the forced enabling of MSI, that
is totally wrong.

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Wiedemeier, Jeff]

On Fri, Jan 24, 2003 at 02:31:35PM -0500, Jeff Garzik wrote:
> On Fri, Jan 24, 2003 at 09:27:48PM +0300, Ivan Kokshaysky wrote:
> > --- 2.5.59/drivers/net/tg3.c	Fri Jan 17 05:22:16 2003
> > +++ linux/drivers/net/tg3.c	Fri Jan 24 19:28:44 2003
> > @@ -3096,7 +3096,12 @@ static void tg3_chip_reset(struct tg3 *t
> >  		val |= PCISTATE_RETRY_SAME_DMA;
> >  	pci_write_config_dword(tp->pdev, TG3PCI_PCISTATE, val);
> >  
> > -	pci_restore_state(tp->pdev, tp->pci_cfg_state);
> > +	pci_restore_extended_state(tp->pdev, tp->pci_cfg_state);
> > +
> > +	/* Make sure MSGINT_MODE is set if MSI is configured. */
> > +	pci_read_config_dword(tp->pdev, TG3PCI_MSI_CAP_ID, &val);
> > +	if ((val >> 16) & PCI_MSI_FLAGS_ENABLE)
> > +		tw32(MSGINT_MODE, MSGINT_MODE_ENABLE);
> >  
> >  	/* Make sure PCI-X relaxed ordering bit is clear. */
> >  	pci_read_config_dword(tp->pdev, TG3PCI_X_CAPS, &val);
> 
> hmmmm.  We don't use MSI in the driver anymore, unless I am missing
> something.
> 
> So, the above patch is probably the wrong thing to do.  I'll need to
> check to be sure, but I think that h/w reset clears MSGINT_MODE_ENABLE,
> so we wouldn't want to be randomly enabling it when it is purposefully
> disabled.
> 
> DaveM/Jeff, corrections?

The problem is that if the chip is configured for MSI (through config
space) and the platform's irq mapping code therefore filled in
pci_dev->irq with an appropriate vector for the MSI interrupt the chip
is assigned instead of the LSI interrupt it may also be assigned, then
unless MSGINT_MODE matches PCI_MSI_FLAGS_ENABLE, the driver will grab
wrong interrupt.

/jeff

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[David S. Miller]

   From: "Wiedemeier, Jeff" <Jeff.Wiedemeier@hp.com>
   Date: Fri, 24 Jan 2003 15:00:06 -0500

   The problem is that if the chip is configured for MSI (through config
   space) and the platform's irq mapping code therefore filled in
   pci_dev->irq with an appropriate vector for the MSI interrupt the chip
   is assigned instead of the LSI interrupt it may also be assigned, then
   unless MSGINT_MODE matches PCI_MSI_FLAGS_ENABLE, the driver will grab
   wrong interrupt.
   
Why isn't it enabled at the point where we save the extended state?

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Wiedemeier, Jeff]

On Fri, Jan 24, 2003 at 11:53:55AM -0800, David S. Miller wrote:
>    From: "Wiedemeier, Jeff" <Jeff.Wiedemeier@hp.com>
>    Date: Fri, 24 Jan 2003 15:00:06 -0500
> 
>    The problem is that if the chip is configured for MSI (through config
>    space) and the platform's irq mapping code therefore filled in
>    pci_dev->irq with an appropriate vector for the MSI interrupt the chip
>    is assigned instead of the LSI interrupt it may also be assigned, then
>    unless MSGINT_MODE matches PCI_MSI_FLAGS_ENABLE, the driver will grab
>    wrong interrupt.
>    
> Why isn't it enabled at the point where we save the extended state?

It was (in config space) and that's why PCI_MSI_FLAGS_ENABLE is still
enabled after the restore of extended state. MSGINT_MODE is a
tigon3-specific register (not config space) and it's state does not
follow PCI_MSI_FLAGS_ENABLE in the MSI capability.

/jeff

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Jeff Garzik]

On Fri, Jan 24, 2003 at 03:00:06PM -0500, Wiedemeier, Jeff wrote:
> The problem is that if the chip is configured for MSI (through config
> space) and the platform's irq mapping code therefore filled in
> pci_dev->irq with an appropriate vector for the MSI interrupt the chip
> is assigned instead of the LSI interrupt it may also be assigned, then
> unless MSGINT_MODE matches PCI_MSI_FLAGS_ENABLE, the driver will grab
> wrong interrupt.

That implies we should be disabling PCI_MSI_FLAGS_ENABLE when we first
initialize each board, if hardware reset does not automatically do that
for us...

	Jeff

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Wiedemeier, Jeff]

On Fri, Jan 24, 2003 at 03:05:38PM -0500, Jeff Garzik wrote:
> On Fri, Jan 24, 2003 at 03:00:06PM -0500, Wiedemeier, Jeff wrote:
> > The problem is that if the chip is configured for MSI (through config
> > space) and the platform's irq mapping code therefore filled in
> > pci_dev->irq with an appropriate vector for the MSI interrupt the chip
> > is assigned instead of the LSI interrupt it may also be assigned, then
> > unless MSGINT_MODE matches PCI_MSI_FLAGS_ENABLE, the driver will grab
> > wrong interrupt.
> 
> That implies we should be disabling PCI_MSI_FLAGS_ENABLE when we first
> initialize each board, if hardware reset does not automatically do that
> for us...

A true hardware reset does reset this bit. It should only be disabled
arbitrarily if the intent is to *never* use MSI. 

The PCI 2.2 spec states about PCI_MSI_FLAGS_ENABLE:

    If 1, the function is permitted to use MSI to request service
    and is prohibited from using it's INTx# pin (if implemented).

The fact that the tg3 has an extra config register that has to be set on
top of this is not consistent with the spec. 

If the intent is to just not use MSI on tg3 devices, I can use the pci
quirks to make sure that MSI gets turned off for tg3 devices.

/jeff

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Jeff Garzik]

On Fri, Jan 24, 2003 at 03:24:53PM -0500, Wiedemeier, Jeff wrote:
> On Fri, Jan 24, 2003 at 03:05:38PM -0500, Jeff Garzik wrote:
> > That implies we should be disabling PCI_MSI_FLAGS_ENABLE when we first
> > initialize each board, if hardware reset does not automatically do that
> > for us...
> 
> A true hardware reset does reset this bit. It should only be disabled
> arbitrarily if the intent is to *never* use MSI. 

Yes, that's the intent :)


> If the intent is to just not use MSI on tg3 devices, I can use the pci
> quirks to make sure that MSI gets turned off for tg3 devices.

hmmm, maybe I am missing something?

AFAICS, this is a per-driver decision, and needs to be done at the
driver level, in the tg3 driver source.

A quirk would need to be carried to every other PCI-capable
architecture, and every PCI-capable arch would need to be changed if
tg3 suddenly started using MSI again.

	Jeff

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Wiedemeier, Jeff]

On Fri, Jan 24, 2003 at 03:34:02PM -0500, Jeff Garzik wrote:
> On Fri, Jan 24, 2003 at 03:24:53PM -0500, Wiedemeier, Jeff wrote:
> > On Fri, Jan 24, 2003 at 03:05:38PM -0500, Jeff Garzik wrote:
> > If the intent is to just not use MSI on tg3 devices, I can use the pci
> > quirks to make sure that MSI gets turned off for tg3 devices.
> 
> hmmm, maybe I am missing something?

Quoting section 6.8.1.3 of the PCI 2.2 spec (talking about message
control in PCI config space):

  This register provides system software control over MSI. After reset,
  MSI is disabled (bit 0 is cleared) and the function requests servicing
  via its INTx# pin (if supported). System sofware can enable MSI by 
  setting bit 0 of this register. System software is permitted to 
  modify the Message Control register's read/write bits and fields.
  A device driver is not permitted to modify the Message Control
  register's read/write bits and fields.

> AFAICS, this is a per-driver decision, and needs to be done at the
> driver level, in the tg3 driver source.

The last sentence in the quote above indicates that it is not intended
(by the PCI spec) to be a per-driver decision, but rather a system
decision. The messages used are also a per-bus system resource and how
an MSI goes from the PCI bus to the rest of the system (i.e. the CPU(s))
is implementation dependent.
 
/jeff

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[David S. Miller]

   From: "Wiedemeier, Jeff" <Jeff.Wiedemeier@hp.com>
   Date: Fri, 24 Jan 2003 15:46:35 -0500

   On Fri, Jan 24, 2003 at 03:34:02PM -0500, Jeff Garzik wrote:
   > AFAICS, this is a per-driver decision, and needs to be done at the
   > driver level, in the tg3 driver source.
   
   The last sentence in the quote above indicates that it is not intended
   (by the PCI spec) to be a per-driver decision, but rather a system
   decision. The messages used are also a per-bus system resource and how
   an MSI goes from the PCI bus to the rest of the system (i.e. the CPU(s))
   is implementation dependent.

Yes, this is understood.

But the tg3 hw designers have decided to do something which makes this
not possible.

So, for tg3's case, it has to become a driver specific decision
whether to support MSI or not.

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Wiedemeier, Jeff]

On Fri, Jan 24, 2003 at 12:51:21PM -0800, David S. Miller wrote:
>    From: "Wiedemeier, Jeff" <Jeff.Wiedemeier@hp.com>
>    Date: Fri, 24 Jan 2003 15:46:35 -0500
> 
>    On Fri, Jan 24, 2003 at 03:34:02PM -0500, Jeff Garzik wrote:
>    > AFAICS, this is a per-driver decision, and needs to be done at the
>    > driver level, in the tg3 driver source.
>    
>    The last sentence in the quote above indicates that it is not intended
>    (by the PCI spec) to be a per-driver decision, but rather a system
>    decision. The messages used are also a per-bus system resource and how
>    an MSI goes from the PCI bus to the rest of the system (i.e. the CPU(s))
>    is implementation dependent.
> 
> Yes, this is understood.
> 
> But the tg3 hw designers have decided to do something which makes this
> not possible.

True. 
 
> So, for tg3's case, it has to become a driver specific decision
> whether to support MSI or not.

But right now, the driver does not have enough information to make it a
driver specific decision. INT_LINE may not be enough to determine the
vector to claim for LSIs and "Message Data" may not be enough to
determine the vector to claim for MSIs. What is there is the irq field
in struct pci_dev.

The intent I had in making sure that MSGINT_MODE was set to match
PCI_MSI_FLAGS_ENABLE and making sure the capability was saved and
restored was to come as close as possible to the spec.

If it needs to be made a driver decision, there needs to be some way to
communicate the correct vector information for whichever option the
driver is using (if there already is and I missed it, please let me
know). Otherwise, it seems that trying to match spec behavior given the
hardware design or disabling MSI at config time for these devices (such
as through quirks) are the options.

/jeff

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[David S. Miller]

   From: "Wiedemeier, Jeff" <Jeff.Wiedemeier@hp.com>
   Date: Fri, 24 Jan 2003 16:33:41 -0500
   
   If it needs to be made a driver decision, there needs to be some way to
   communicate the correct vector information for whichever option the
   driver is using (if there already is and I missed it, please let me
   know). Otherwise, it seems that trying to match spec behavior given the
   hardware design or disabling MSI at config time for these devices (such
   as through quirks) are the options.

Right, the whole issue is that we're generally MSI ignorant in our PCI
layer right now.  And you're trying to make use of MSI on some
platform :-)

So because the driver has no way to ask the platform "are you going
to use MSI for this device?" there is no way for tg3 to portably
deal with this issue.

That being said, why don't we add "pci_using_msi(pdev)" to asm/pci.h?
Once we have that, tg3.c can then go and set the tg3 specific MSI
enable bit to match whatever pci_using_msi(pdev) returns.  This, plus
the extended state save/restore, should solve all the problems.

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Ivan Kokshaysky]

On Fri, Jan 24, 2003 at 01:34:34PM -0800, David S. Miller wrote:
> Right, the whole issue is that we're generally MSI ignorant in our PCI
> layer right now.  And you're trying to make use of MSI on some
> platform :-)

Right. Alpha is way ahead, as usual. ;-)

> So because the driver has no way to ask the platform "are you going
> to use MSI for this device?" there is no way for tg3 to portably
> deal with this issue.
> 
> That being said, why don't we add "pci_using_msi(pdev)" to asm/pci.h?
> Once we have that, tg3.c can then go and set the tg3 specific MSI
> enable bit to match whatever pci_using_msi(pdev) returns.  This, plus
> the extended state save/restore, should solve all the problems.

I don't understand the issue, really. The config register says:
"MSIs are enabled". Which means: "My platform is *really* going to
use MSI". Why do you want to ignore that?
In the case when firmware has MSI enabled by mistake and it's known
that this platform cannot support MSI, we'll have a quirk that
fixes the config register. No?

Ivan.

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[David S. Miller]

   From: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
   Date: Sat, 25 Jan 2003 01:41:02 +0300
   
   I don't understand the issue, really. The config register says:
   "MSIs are enabled". Which means: "My platform is *really* going to
   use MSI". Why do you want to ignore that?

I see, why not code up a generic pci_using_msi(pdev) that
does this?

I suggested a per-arch version because I was not clear on how
this exactly worked.

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Jeff Garzik]

David S. Miller wrote:
>    From: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>    Date: Sat, 25 Jan 2003 01:41:02 +0300
>    
>    I don't understand the issue, really. The config register says:
>    "MSIs are enabled". Which means: "My platform is *really* going to
>    use MSI". Why do you want to ignore that?

Let us define "platform."  If you mean ia32 or alpha or sparc64, yes 
this is a quirk.  If you mean tg3, no, this is not a quirk.


> I see, why not code up a generic pci_using_msi(pdev) that
> does this?


Great minds think alike :)  This was going to be my suggestion:
call pci_using_msi(pdev), and require that it be called before 
pci_enable_device().

This fits Jeff's "disabling MSI at config time" AFAICS...

	Jeff

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Wiedemeier, Jeff]

On Fri, Jan 24, 2003 at 02:32:52PM -0800, David S. Miller wrote:
>    From: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>    Date: Sat, 25 Jan 2003 01:41:02 +0300
>    
>    I don't understand the issue, really. The config register says:
>    "MSIs are enabled". Which means: "My platform is *really* going to
>    use MSI". Why do you want to ignore that?
> 
> I see, why not code up a generic pci_using_msi(pdev) that
> does this?
> 
> I suggested a per-arch version because I was not clear on how
> this exactly worked.

You mean something like this? My reading of the PCI spec says that if
the enable bit in Message Command (part of which is PCI_MSI_FLAGS as
defined in pci.h) is that if the enable bit is set, MSIs are being
used... 

Then the tg3 code after the pci_restore_extended_state could be
     if (pci_using_msi(tp->pdev))
        tw32(MSGINT_MODE, MSGINT_MODE_ENABLE);

/jeff
------
diff -Nuar linux-2.5.59/drivers/pci/pci.c pci_using_msi/drivers/pci/pci.c
--- linux-2.5.59/drivers/pci/pci.c	Thu Jan 16 21:21:48 2003
+++ pci_using_msi/drivers/pci/pci.c	Fri Jan 24 19:00:53 2003
@@ -679,6 +679,26 @@
 	}
 }
 
+/**
+ * pci_using_msi - is this pci device configured to use MSI?
+ * @dev: PCI device structure of device being queried
+ *
+ * Tells whether or not a pci device is configured to use Message Signalled
+ * Interrupts. Returns non-zero if configured for MSI, else 0.
+ */
+int
+pci_using_msi(struct pci_dev *dev)
+{
+	int msi = pci_find_capability(dev, PCI_CAP_ID_MSI);
+	u8 flags;
+
+	if (!msi)
+		return 0;
+
+	pci_read_config_byte(dev, msi + PCI_MSI_FLAGS, &flags);
+	return (flags & PCI_MSI_FLAGS_ENABLE);
+}
+
 int
 pci_set_dma_mask(struct pci_dev *dev, u64 mask)
 {
@@ -749,6 +769,7 @@
 EXPORT_SYMBOL(pci_set_master);
 EXPORT_SYMBOL(pci_set_mwi);
 EXPORT_SYMBOL(pci_clear_mwi);
+EXPORT_SYMBOL(pci_using_msi);
 EXPORT_SYMBOL(pci_set_dma_mask);
 EXPORT_SYMBOL(pci_dac_set_dma_mask);
 EXPORT_SYMBOL(pci_assign_resource);
diff -Nuar linux-2.5.59/include/linux/pci.h pci_using_msi/include/linux/pci.h
--- linux-2.5.59/include/linux/pci.h	Thu Jan 16 21:22:08 2003
+++ pci_using_msi/include/linux/pci.h	Fri Jan 24 19:01:26 2003
@@ -622,6 +622,7 @@
 #define HAVE_PCI_SET_MWI
 int pci_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
+int pci_using_msi(struct pci_dev *dev);
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_dac_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_assign_resource(struct pci_dev *dev, int i);

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[David S. Miller]

   From: "Wiedemeier, Jeff" <Jeff.Wiedemeier@hp.com>
   Date: Fri, 24 Jan 2003 19:33:53 -0500
   
   Then the tg3 code after the pci_restore_extended_state could be
        if (pci_using_msi(tp->pdev))
           tw32(MSGINT_MODE, MSGINT_MODE_ENABLE);

Ok.

I wonder if it would be a good idea to write MSGINT_MODE_RESET
to this register in the other cases.

Re: [patch 2.5] tg3.c: pci_{save,restore}_extended_state

[Jeff Garzik]

Wiedemeier, Jeff wrote:
> But right now, the driver does not have enough information to make it a
> driver specific decision. INT_LINE may not be enough to determine the
> vector to claim for LSIs and "Message Data" may not be enough to
> determine the vector to claim for MSIs. What is there is the irq field
> in struct pci_dev.
[...]

> If it needs to be made a driver decision, there needs to be some way to
> communicate the correct vector information for whichever option the
> driver is using (if there already is and I missed it, please let me
> know). Otherwise, it seems that trying to match spec behavior given the
> hardware design or disabling MSI at config time for these devices (such
> as through quirks) are the options.


Just to add... I think that the proposed pci_using_msi() could certainly 
store additional information in struct pci_dev, if it needed to... 
whether it is stored in arch-specific sysdata or in generic struct 
pci-dev, I leave as a question to the implementor ;-)

If "what is there is the irq field", then add the additional information 
you want :)