[gPXE-devel] [PATCH] [sky2] Disable interrupts by default

[Gilles Martin]

Le vendredi 30 juillet 2010 à 20:39 +0100, Stefan Hajnoczi a écrit :
> On Thu, Jul 29, 2010 at 8:00 PM, Joshua Oreman <oremanj at rwcr.net> wrote:
> > On Thu, Jul 29, 2010 at 11:56 AM, Stefan Hajnoczi <stefanha at gmail.com> 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:

Yes sure. I just have a little problem is that I don't have access to
Git from my place (Blocked by firewall). Is there a way to patch the
sky2.c file without using Git (I'm not really used to patch...)? Or I
just need a link with the GPXE patched version of sky2.c.

--
Gilles




> 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 <stefanha at gmail.com>
> >> ---
> >> 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
> >>
> >>
> >

-- 
Gilles Martin          Gilles.Martin at psi.ch 
Center for Proton Therapy 
Paul Scherrer Institute 
5232 Villigen PSI      tel. (+41) 56 310 36 90 
Switzerland            fax (+41) 56 310 35 15