Message ID | 1419106577-12891-1-git-send-email-prashant@broadcom.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Sat, Dec 20, 2014 at 12:16:17PM -0800, Prashant Sreedharan wrote: > > This driver bug was exposed because of the commit a7877b17a667 (PCI: Check only > the Vendor ID to identify Configuration Request Retry). Also this issue is only > seen in older generation chipsets like 5722 because config space write to offset > 0 from driver is possible. > > Fixed by initializing the interrupt mailbox registers before calling tg3_halt. > > Please queue for -stable. I gave this patch a try and can confirm what was to be expected: It fixes the issue and the network interface is once again working properly on my system. Thus, I guess the issue is adequately solved. Thanks to everyone involved, especially to Marcelo for additional debugging and the guys at Broadcom for the quick fix! Greetings, Nils -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20-12-2014 20:16, Nils Holland wrote: > On Sat, Dec 20, 2014 at 12:16:17PM -0800, Prashant Sreedharan wrote: >> >> This driver bug was exposed because of the commit a7877b17a667 (PCI: Check only >> the Vendor ID to identify Configuration Request Retry). Also this issue is only >> seen in older generation chipsets like 5722 because config space write to offset >> 0 from driver is possible. >> >> Fixed by initializing the interrupt mailbox registers before calling tg3_halt. >> >> Please queue for -stable. > > I gave this patch a try and can confirm what was to be expected: It > fixes the issue and the network interface is once again working > properly on my system. Thus, I guess the issue is adequately solved. > > Thanks to everyone involved, especially to Marcelo for additional > debugging and the guys at Broadcom for the quick fix! > > Greetings, > Nils Same here, it works again. Thanks! Marcelo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Prashant Sreedharan <prashant@broadcom.com> Date: Sat, 20 Dec 2014 12:16:17 -0800 > During driver load in tg3_init_one, if the driver detects DMA activity before > intializing the chip tg3_halt is called. As part of tg3_halt interrupts are > disabled using routine tg3_disable_ints. This routine was using mailbox value > which was not initialized (default value is 0). As a result driver was writing > 0x00000001 to pci config space register 0, which is the vendor id / device id. > > This driver bug was exposed because of the commit a7877b17a667 (PCI: Check only > the Vendor ID to identify Configuration Request Retry). Also this issue is only > seen in older generation chipsets like 5722 because config space write to offset > 0 from driver is possible. The newer generation chips ignore writes to offset 0. > Also without commit a7877b17a667, for these older chips when a GRC reset is > issued the Bootcode would reprogram the vendor id/device id, which is the reason > this bug was masked earlier. > > Fixed by initializing the interrupt mailbox registers before calling tg3_halt. > > Please queue for -stable. > > Reported-by: Nils Holland <nholland@tisys.org> > Reported-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Signed-off-by: Prashant Sreedharan <prashant@broadcom.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> Applied and queued up for -stable, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index bb48a61..553dcd8 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -17800,23 +17800,6 @@ static int tg3_init_one(struct pci_dev *pdev, goto err_out_apeunmap; } - /* - * Reset chip in case UNDI or EFI driver did not shutdown - * DMA self test will enable WDMAC and we'll see (spurious) - * pending DMA on the PCI bus at that point. - */ - if ((tr32(HOSTCC_MODE) & HOSTCC_MODE_ENABLE) || - (tr32(WDMAC_MODE) & WDMAC_MODE_ENABLE)) { - tw32(MEMARB_MODE, MEMARB_MODE_ENABLE); - tg3_halt(tp, RESET_KIND_SHUTDOWN, 1); - } - - err = tg3_test_dma(tp); - if (err) { - dev_err(&pdev->dev, "DMA engine test failed, aborting\n"); - goto err_out_apeunmap; - } - intmbx = MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW; rcvmbx = MAILBOX_RCVRET_CON_IDX_0 + TG3_64BIT_REG_LOW; sndmbx = MAILBOX_SNDHOST_PROD_IDX_0 + TG3_64BIT_REG_LOW; @@ -17861,6 +17844,23 @@ static int tg3_init_one(struct pci_dev *pdev, sndmbx += 0xc; } + /* + * Reset chip in case UNDI or EFI driver did not shutdown + * DMA self test will enable WDMAC and we'll see (spurious) + * pending DMA on the PCI bus at that point. + */ + if ((tr32(HOSTCC_MODE) & HOSTCC_MODE_ENABLE) || + (tr32(WDMAC_MODE) & WDMAC_MODE_ENABLE)) { + tw32(MEMARB_MODE, MEMARB_MODE_ENABLE); + tg3_halt(tp, RESET_KIND_SHUTDOWN, 1); + } + + err = tg3_test_dma(tp); + if (err) { + dev_err(&pdev->dev, "DMA engine test failed, aborting\n"); + goto err_out_apeunmap; + } + tg3_init_coal(tp); pci_set_drvdata(pdev, dev);