Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 15 Jul 2008 10:11:23 +0800
From:      mirnshi@gmail.com
To:        pyunyh@gmail.com
Cc:        freebsd-drivers@freebsd.org
Subject:   Re: patch for Attansic L2 FastEthernet
Message-ID:  <fe3493cd0807141911n2f0191d0m10d891e355cb2cbf@mail.gmail.com>
In-Reply-To: <20080715004301.GB40212@cdnetworks.co.kr>
References:  <fe3493cd0807140730j1dd3bc09j27d2c8dacdc2ebad@mail.gmail.com> <20080715004301.GB40212@cdnetworks.co.kr>

next in thread | previous in thread | raw e-mail | index | archive | help
2008/7/15 Pyun YongHyeon <pyunyh@gmail.com>:

> On Mon, Jul 14, 2008 at 10:30:36PM +0800, mirnshi@gmail.com wrote:
>  > Recently, I got an eeepc 701. And I installed FreeBSD 7.0 on it . The
> url
>  > http://wiki.freebsd.org/AsusEee gives me a big hand. But, the driver of
> lan
>  > did not work. The problem is that the driver can be loaded by kldload,
> but
>  > the interrupt storm caused the system to stop responding when I use
> ifconfig
>  > to mark the nic 'up'.  BTW, I used 10MB hub.
>
> Sound like interrupts are enabled in taskqueue handler.


>
>  >
>  > The driver uses taskqueue_enqueue. 'ae_intr' reads nic register, but
>  > 'ae_int_task' reads it again. So I merge them into the new 'ae_intr'.
>  > I test 'ping' and 'ftp', it works well.
>  >
>
> You can't hold a mutex in fast interrupt handler.


Do you mean the driver will slow down? Many GB driver use LOCK in their
xxx_intr.


> Since there is no documentation for L2 controller I'm not sure but
> ae_int_task() seems to have a code that reenables interrupts which
> was already disabled in ae_intr(). How about nuking AE_ISR_DISABLE
> in ae_int_task()?
>
> From:
>         /* Clear interrupts and disable them. */
>        AE_WRITE_4(sc, AE_ISR_REG, val | AE_ISR_DISABLE);
>
> To:
>        /* Clear interrupts. */
>        AE_WRITE_4(sc, AE_ISR_REG, val);


enable or disable the interrupt? I think this code enable the interrupt,
right?

Stanislav did not write the status word back, just disable the interrupt.
      AE_WRITE_4(sc, AE_ISR_REG, AE_ISR_DISABLE);
but, the linux does it
     AT_WRITE_REG(hw, REG_ISR, status|ISR_DIS_INT);

In 'ae_int_task', Stanislav read the status word again, like in linux, write
back.
    val = AE_READ_4(sc, AE_ISR_REG);
    AE_WRITE_4(sc, AE_ISR_REG, val | AE_ISR_DISABLE);

Is the status word still there ?



> CCed to Stanislav Sedov, the author of driver.
>
>  > Please refer to the patch for details.
>  >
>  > --- if_ae.c    2008-06-27 20:19:43.000000000 +0800
>  > +++ /sys/dev/if_ae/if_ae.c    2008-07-14 21:54:06.000000000 +0800
>  > @@ -1450,20 +1450,56 @@
>  >  {
>  >      ae_softc_t *sc;
>  >      uint32_t val;
>  > +    struct ifnet *ifp;
>  > +    struct mii_data *mii;
>  >
>  >      sc = (ae_softc_t *)arg;
>  > +    AE_LOCK(sc);
>  >      KASSERT(sc != NULL, ("[ae, %d]: sc is null", __LINE__));
>  >
>  >      val = AE_READ_4(sc, AE_ISR_REG);
>  > -    if (val == 0 || (val & AE_IMR_DEFAULT) == 0)
>  > +    if (val == 0 || (val & AE_IMR_DEFAULT) == 0) {
>  > +        AE_UNLOCK(sc);
>  >          return FILTER_STRAY;
>  > +    }
>  >
>  > -    /* Disable interrupts. */
>  > -    AE_WRITE_4(sc, AE_ISR_REG, AE_ISR_DISABLE);
>  > +    /* Clear interrupts and disable them. */
>  > +    AE_WRITE_4(sc, AE_ISR_REG, val | AE_ISR_DISABLE);
>  >
>  > -    /* Schedule interrupt processing. */
>  > -    taskqueue_enqueue(sc->tq, &sc->int_task);
>  > +    ifp = sc->ifp;
>  >
>  > +    if ((val & AE_ISR_PHY) != 0) {
>  > +        /*
>  > +         * Clear PHY interrupt. Not sure if it needed. From Linux.
>  > +         */
>  > +        ae_miibus_readreg(sc->miibus, 1, 19);
>  > +    }
>  > +
>  > +#ifdef AE_DEBUG
>  > +    if_printf(ifp, "Interrupt received: 0x%08x\n", val);
>  > +#endif
>  > +
>  > +    if ((val & (AE_ISR_PHY | AE_ISR_MANUAL)) != 0) {
>  > +        mii = device_get_softc(sc->miibus);
>  > +        mii_mediachg(mii);
>  > +    }
>  > +
>  > +    if ((val & (AE_ISR_DMAR_TIMEOUT | AE_ISR_DMAW_TIMEOUT |
>  > +        AE_ISR_PHY_LINKDOWN)) != 0) {
>  > +        ae_init_locked(sc);
>  > +    }
>  > +    if ((ifp->if_drv_flags & IFF_DRV_RUNNING) != 0) {
>  > +        if ((val & AE_ISR_TX_EVENT) != 0)
>  > +            ae_tx_intr(sc);
>  > +
>  > +        if ((val & AE_ISR_RX_EVENT) != 0)
>  > +            ae_rx_intr(sc);
>  > +    }
>  > +
>  > +    /* Re-enable interrupts. */
>  > +    AE_WRITE_4(sc, AE_ISR_REG, 0);
>  > +
>  > +    AE_UNLOCK(sc);
>  >      return (FILTER_HANDLED);
>  >  }
>
> --
> Regards,
> Pyun YongHyeon
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?fe3493cd0807141911n2f0191d0m10d891e355cb2cbf>