Date: Wed, 16 Jul 2008 02:39:07 +0400 From: Stanislav Sedov <stas@FreeBSD.org> To: mirnshi@gmail.com Cc: freebsd-drivers@freebsd.org Subject: Re: patch for Attansic L2 FastEthernet Message-ID: <20080716023907.d11f15b2.stas@FreeBSD.org> In-Reply-To: <fe3493cd0807141911n2f0191d0m10d891e355cb2cbf@mail.gmail.com> References: <fe3493cd0807140730j1dd3bc09j27d2c8dacdc2ebad@mail.gmail.com> <20080715004301.GB40212@cdnetworks.co.kr> <fe3493cd0807141911n2f0191d0m10d891e355cb2cbf@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--Signature=_Wed__16_Jul_2008_02_39_07_+0400_CS8TFO5FFqhy6XSI
Content-Type: text/plain; charset=US-ASCII
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Tue, 15 Jul 2008 10:11:23 +0800
mirnshi@gmail.com mentioned:
>=20
> Do you mean the driver will slow down? Many GB driver use LOCK in their
> xxx_intr.
>=20
You just can't hold locks in FILTER isrs. If you need locking, you're
definitely not going to use fast interrupt handlers. And I see no
good reason not to use them.
>=20
> > 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);
>=20
>=20
> enable or disable the interrupt? I think this code enable the interrupt,
> right?
>=20
> 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);
>=20
> In 'ae_int_task', Stanislav read the status word again, like in linux, wr=
ite
> back.
> val =3D AE_READ_4(sc, AE_ISR_REG);
> AE_WRITE_4(sc, AE_ISR_REG, val | AE_ISR_DISABLE);
>=20
> Is the status word still there ?
>=20
That's because I need to pass ISR status word between fast interrupt
handler and the actual isr processor. Thus I just disable interrupts
in filter, and clear the interrupt status word after processing
in ae_int_task.
>=20
>=20
> > 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 =3D (ae_softc_t *)arg;
> > > + AE_LOCK(sc);
> > > KASSERT(sc !=3D NULL, ("[ae, %d]: sc is null", __LINE__));
> > >
> > > val =3D AE_READ_4(sc, AE_ISR_REG);
> > > - if (val =3D=3D 0 || (val & AE_IMR_DEFAULT) =3D=3D 0)
> > > + if (val =3D=3D 0 || (val & AE_IMR_DEFAULT) =3D=3D 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 =3D sc->ifp;
> > >
> > > + if ((val & AE_ISR_PHY) !=3D 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)) !=3D 0) {
> > > + mii =3D device_get_softc(sc->miibus);
> > > + mii_mediachg(mii);
> > > + }
> > > +
> > > + if ((val & (AE_ISR_DMAR_TIMEOUT | AE_ISR_DMAW_TIMEOUT |
> > > + AE_ISR_PHY_LINKDOWN)) !=3D 0) {
> > > + ae_init_locked(sc);
> > > + }
> > > + if ((ifp->if_drv_flags & IFF_DRV_RUNNING) !=3D 0) {
> > > + if ((val & AE_ISR_TX_EVENT) !=3D 0)
> > > + ae_tx_intr(sc);
> > > +
> > > + if ((val & AE_ISR_RX_EVENT) !=3D 0)
> > > + ae_rx_intr(sc);
> > > + }
> > > +
> > > + /* Re-enable interrupts. */
> > > + AE_WRITE_4(sc, AE_ISR_REG, 0);
> > > +
> > > + AE_UNLOCK(sc);
> > > return (FILTER_HANDLED);
> > > }
> >
I don't see how this can help with your issue. One guy has reported
he's able to stably reproduce the bug by booting into FreeBSD just
after Linux touches the hardware. This explains why I've never
seen that, as I never booted Linux on my box.
I think the problem caused by the ukphy driver which just can't handle
the F2 phy. I plan to use agephy to handle the Attansic L2 phy also.
Please, wait a couple of days - I'll try to reproduce the problem,
and incorporate appropriate fixes. Also, as was mentioned in my
previous mail, the drives isn't complete now - there're a lot
of places that had to be fixed, not speaking of the public
version of the driver isn't the latest one. I'm a bit busy now,
but I hope I'll roll out the updated version by the end of
the week.
--=20
Stanislav Sedov
ST4096-RIPE
--Signature=_Wed__16_Jul_2008_02_39_07_+0400_CS8TFO5FFqhy6XSI
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (FreeBSD)
iEYEARECAAYFAkh9JwsACgkQK/VZk+smlYHgJgCfcpAQ7dORRL/U4m++CqFOaqXl
cLkAnRmHFq/uK3nygKKW4lEM00dcQR+c
=AGoz
-----END PGP SIGNATURE-----
--Signature=_Wed__16_Jul_2008_02_39_07_+0400_CS8TFO5FFqhy6XSI--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080716023907.d11f15b2.stas>