On Thu, Jul 29, 2010 at 8:00 PM, Joshua Oreman <[email protected]> wrote: > On Thu, Jul 29, 2010 at 11:56 AM, Stefan Hajnoczi <[email protected]> wrote: >> Device interrupts should be disabled by default in the sky2 driver. >> They are explicitly enabled when a PXE NBP client program uses the UNDI >> interface, which is designed around interrupts. Having interrupts >> enabled when no handler is installed to service them results in hangs >> because the spurious interrupt is never reset and remains high. > > Looks good to me. Thanks, Stefan! > > Feel free to push after 24 hours. > > -- Josh
Thanks the the review Josh. Gilles: I'd be thankful if you're able to run two tests with the patch applied: 1. DHCP and TFTP or HTTP boot using a gPXE sky2 image. If you want a quick over-the-internet boot test, try this: dhcp net0 chain http://vmsplice.net/tinycore.gpxe It will boot Tiny Core Linux over HTTP. 2. Same test but from a chainloaded undionly.kpxe. This means first loading undionly.kpxe from a gPXE sky2 image: dhcp net0 chain http://myserver/undionly.kpxe And then at undionly.kpxe's gPXE prompt: dhcp net0 chain http://vmsplice.net/tinycore.gpxe The undionly.kpxe gPXE image will be loading Tiny Core Linux using its UNDI driver. The gPXE sky2 image underneath provides the UNDI interface and performs the actual network I/O using the native sky2 driver. I don't have a sky2 card here and have no datasheet either. Testing would be very helpful. Thanks, Stefan > >> Signed-off-by: Stefan Hajnoczi <[email protected]> >> --- >> I wrote this on the train. Now after comparing against Daniel's original >> patch >> it is very similar and so may require further debugging to get the UNDI case >> working. >> >> src/drivers/net/sky2.c | 24 ++++++++++-------------- >> 1 files changed, 10 insertions(+), 14 deletions(-) >> >> diff --git a/src/drivers/net/sky2.c b/src/drivers/net/sky2.c >> index 00940af..07487ac 100644 >> --- a/src/drivers/net/sky2.c >> +++ b/src/drivers/net/sky2.c >> @@ -1130,7 +1130,7 @@ static int sky2_up(struct net_device *dev) >> struct sky2_port *sky2 = netdev_priv(dev); >> struct sky2_hw *hw = sky2->hw; >> unsigned port = sky2->port; >> - u32 imask, ramsize; >> + u32 ramsize; >> int err = -ENOMEM; >> >> netdev_link_down(dev); >> @@ -1198,11 +1198,6 @@ static int sky2_up(struct net_device *dev) >> if (err) >> goto err_out; >> >> - /* Enable interrupts from phy/mac for port */ >> - imask = sky2_read32(hw, B0_IMSK); >> - imask |= portirq_msk[port]; >> - sky2_write32(hw, B0_IMSK, imask); >> - >> DBGIO(PFX "%s: le bases: st %p [%x], rx %p [%x], tx %p [%x]\n", >> dev->name, hw->st_le, hw->st_dma, sky2->rx_le, sky2->rx_le_map, >> sky2->tx_le, sky2->tx_le_map); >> @@ -1314,7 +1309,6 @@ static void sky2_down(struct net_device *dev) >> struct sky2_hw *hw = sky2->hw; >> unsigned port = sky2->port; >> u16 ctrl; >> - u32 imask; >> >> /* Never really got started! */ >> if (!sky2->tx_le) >> @@ -1323,9 +1317,7 @@ static void sky2_down(struct net_device *dev) >> DBG2(PFX "%s: disabling interface\n", dev->name); >> >> /* Disable port IRQ */ >> - imask = sky2_read32(hw, B0_IMSK); >> - imask &= ~portirq_msk[port]; >> - sky2_write32(hw, B0_IMSK, imask); >> + netdev_irq(dev, 0); >> >> sky2_gmac_reset(hw, port); >> >> @@ -2250,9 +2242,14 @@ static void sky2_net_irq(struct net_device *dev, int >> enable) >> >> u32 imask = sky2_read32(hw, B0_IMSK); >> if (enable) >> - imask |= portirq_msk[sky2->port]; >> + imask |= Y2_IS_BASE | portirq_msk[sky2->port]; >> else >> imask &= ~portirq_msk[sky2->port]; >> + >> + /* Fully turn off interrupts if disabled for both ports */ >> + if ((imask & ~Y2_IS_BASE) == 0) >> + imask = 0; >> + >> sky2_write32(hw, B0_IMSK, imask); >> } >> >> @@ -2321,7 +2318,8 @@ static int sky2_probe(struct pci_device *pdev, >> goto err_out_free_netdev; >> } >> >> - sky2_write32(hw, B0_IMSK, Y2_IS_BASE); >> + /* Disable interrupts by default */ >> + sky2_write32(hw, B0_IMSK, 0); >> >> sky2_show_addr(dev); >> >> @@ -2370,8 +2368,6 @@ static void sky2_remove(struct pci_device *pdev) >> for (i = hw->ports-1; i >= 0; --i) >> unregister_netdev(hw->dev[i]); >> >> - sky2_write32(hw, B0_IMSK, 0); >> - >> sky2_power_aux(hw); >> >> sky2_write16(hw, B0_Y2LED, LED_STAT_OFF); >> -- >> 1.7.1 >> >> > _______________________________________________ gPXE-devel mailing list [email protected] http://etherboot.org/mailman/listinfo/gpxe-devel
