From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm@xmission.com (Eric W. Biederman) Subject: Re: SATA resume slowness, e1000 MSI warning Date: Sun, 11 Mar 2007 05:11:59 -0600 Message-ID: References: <20070227103021.GA2250@kernel.dk> <20070308102354.GC5149@mellanox.co.il> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20070308102354.GC5149@mellanox.co.il> (Michael S. Tsirkin's message of "Thu, 8 Mar 2007 12:23:54 +0200") List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-pm-bounces@lists.osdl.org Errors-To: linux-pm-bounces@lists.osdl.org To: "Michael S. Tsirkin" Cc: "Kok, Auke" , Michal Piotrowski , Jeff Garzik , Ingo Molnar , linux-pm@lists.osdl.org, Linux Kernel Mailing List , Adrian Bunk , Pavel Machek , Jens Axboe , Thomas Gleixner , Linus Torvalds , Andrew Morton List-Id: linux-pm@vger.kernel.org "Michael S. Tsirkin" writes: >> The only case I can see which might trigger this is if we saved >> pci-X state and then didn't restore it because we could not find >> the capability on restore. > > Hmm. pci_save_pcix_state/pci_restore_pcix_state seem to only handle > regular devices and seem to ignore the fact that for bridge PCI-X > capability has a different structure. > > Is this intentional? = Probably not a such. I don't think we have any drivers for bridge devices so I don't think it matters. It likely wouldn't hurt to fix it just in case though. Do any of the mellanox cards do anything with the bridge on the card? > If not, here's a patch to fix this. Warning: completely untested. If you fix the offsets and diff this against my last fix (to never free the buffer) I think your patch makes sense. > PCI: restore bridge PCI-X capability registers after PM event > > Restore PCI-X bridge up/downstream capability registers > after PM event. This includes maxumum split transaction > commitment limit which might be vital for PCI X. > > Signed-off-by: Michael S. Tsirkin > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index df49530..4b788ef 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -597,14 +597,19 @@ static int pci_save_pcix_state(struct pci_dev *dev) > if (pos <=3D 0) > return 0; > = > - save_state =3D kzalloc(sizeof(*save_state) + sizeof(u16), GFP_KERNEL); > + save_state =3D kzalloc(sizeof(*save_state) + sizeof(u16) * 2, GFP_KERNE= L); > if (!save_state) { > - dev_err(&dev->dev, "Out of memory in pci_save_pcie_state\n"); > + dev_err(&dev->dev, "Out of memory in pci_save_pcix_state\n"); > return -ENOMEM; > } > cap =3D (u16 *)&save_state->data[0]; > = > - pci_read_config_word(dev, pos + PCI_X_CMD, &cap[i++]); > + if (dev->hdr_type =3D=3D PCI_HEADER_TYPE_BRIDGE) { This appears to be the proper test. > + pci_read_config_word(dev, pos + PCI_X_BRIDGE_UP_SPL_CTL, &cap[i++]); > + pci_read_config_word(dev, pos + PCI_X_BRIDGE_DN_SPL_CTL, &cap[i++]); > + } else > + pci_read_config_word(dev, pos + PCI_X_CMD, &cap[i++]); > + > pci_add_saved_cap(dev, save_state); > return 0; > } > @@ -621,7 +626,11 @@ static void pci_restore_pcix_state(struct pci_dev *d= ev) > return; > cap =3D (u16 *)&save_state->data[0]; > = > - pci_write_config_word(dev, pos + PCI_X_CMD, cap[i++]); > + if (dev->hdr_type =3D=3D PCI_HEADER_TYPE_BRIDGE) { > + pci_write_config_word(dev, pos + PCI_X_BRIDGE_UP_SPL_CTL, cap[i++]); > + pci_write_config_word(dev, pos + PCI_X_BRIDGE_DN_SPL_CTL, cap[i++]); These look like the proper two registers to save. > + } else > + pci_write_config_word(dev, pos + PCI_X_CMD, cap[i++]); > pci_remove_saved_cap(save_state); > kfree(save_state); > } > diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h > index f09cce2..fb7eefd 100644 > --- a/include/linux/pci_regs.h > +++ b/include/linux/pci_regs.h > @@ -332,6 +332,8 @@ > #define PCI_X_STATUS_SPL_ERR 0x20000000 /* Rcvd Split Completion Error M= sg */ > #define PCI_X_STATUS_266MHZ 0x40000000 /* 266 MHz capable */ > #define PCI_X_STATUS_533MHZ 0x80000000 /* 533 MHz capable */ > +#define PCI_X_BRIDGE_UP_SPL_CTL 10 /* PCI-X upstream split transaction l= imit */ > +#define PCI_X_BRIDGE_DN_SPL_CTL 14 /* PCI-X downstream split transaction= limit */ Unless I am completely misreading the spec. While you have picked the right register to save the offsets should be 0x08 and 0x0c or 8 and 12.... Eric