Re: [RTnet-developers] RT IRQ sharing
Brought to you by:
bet-frogger,
kiszka
Menu
▾
▴
Re: [RTnet-developers] RT IRQ sharing
|
From: Jan K. <jan...@we...> - 2007-02-13 17:33:36
|
Marcelo Coelho wrote:
>> Please clean up your patches before you post. Means: no unrelated
>> changes, no uncommented old code, no unused function argument
>> (ioaddr...), etc.
> Sorry, I'm a little under pressure and sometimes I forget somethings...=
>=20
> This patch already has the kernel's interrupt handler routine, kind of =
a mix between the kernel version and RTnet's one.
> But it still gives me a problem: it looks like I don't transmit anythin=
g to the network, don't know why.
>=20
> I'll keep on working on this. If someone finds a problem here, please l=
et me know ASAP.
>=20
>=20
>=20
> Thanks for the help!
> Marcelo
>=20
>=20
>=20
>=20
>=20
> -----------------------------------------------------------------------=
-
>=20
> Index: drivers/rt_8139too.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- drivers/rt_8139too.c (revision 1106)
> +++ drivers/rt_8139too.c (working copy)
> @@ -379,7 +379,7 @@
> =20
> =20
> /* Twister tuning parameters from RealTek.
> - Completely undocumented, but required to tune bad links. */
> + Completely undocumented, but required to tune bad links on some boa=
rds. */
> enum CSCRBits {
> CSCR_LinkOKBit =3D 0x0400,
> CSCR_LinkChangeBit =3D 0x0800,
> @@ -595,11 +595,37 @@
> PCIErr | PCSTimeout | RxUnderrun | RxOverflow | RxFIFOOver |
> TxErr | TxOK | RxErr | RxOK;
> =20
> +static const u16 rtl8139_norx_intr_mask =3D
> + PCIErr | PCSTimeout | RxUnderrun |
> + TxErr | TxOK | RxErr ;
> +
> +#if RX_BUF_LEN_IDX =3D=3D 0
> static const unsigned int rtl8139_rx_config =3D
> + RxCfgRcv8K | RxNoWrap |
> + (RX_FIFO_THRESH << RxCfgFIFOShift) |
> + (RX_DMA_BURST << RxCfgDMAShift);
> +#elif RX_BUF_LEN_IDX =3D=3D 1
> +static const unsigned int rtl8139_rx_config =3D
> + RxCfgRcv16K | RxNoWrap |
> + (RX_FIFO_THRESH << RxCfgFIFOShift) |
> + (RX_DMA_BURST << RxCfgDMAShift);
> +#elif RX_BUF_LEN_IDX =3D=3D 2
> +static const unsigned int rtl8139_rx_config =3D
> RxCfgRcv32K | RxNoWrap |
> (RX_FIFO_THRESH << RxCfgFIFOShift) |
> (RX_DMA_BURST << RxCfgDMAShift);
> +#elif RX_BUF_LEN_IDX =3D=3D 3
> +static const unsigned int rtl8139_rx_config =3D
> + RxCfgRcv64K |
> + (RX_FIFO_THRESH << RxCfgFIFOShift) |
> + (RX_DMA_BURST << RxCfgDMAShift);
> +#else
> +#error "Invalid configuration for 8139_RX_BUF_LEN_IDX"
> +#endif
Can those changes be safely broken out into a separate patch? Would help
to test things independently.
> =20
> +
> +
> +
> static const unsigned int rtl8139_tx_config =3D
> TxIFG96 | (TX_DMA_BURST << TxDMAShift) | (TX_RETRY << TxRetryS=
hift);
> =20
> @@ -1155,7 +1181,7 @@
> rt_stack_connect(rtdev, &STACK_manager);
> =20
> retval =3D rtdm_irq_request(&tp->irq_handle, rtdev->irq,
> - rtl8139_interrupt, 0, rtdev->name, r=
tdev);
> + rtl8139_interrupt, RTDM_IRQTYPE_SHAR=
ED, rtdev->name, rtdev);
> if (retval)
> return retval;
> =20
> @@ -1656,6 +1682,7 @@
> }
> }
> =20
> +
> /* The interrupt handler does all of the Rx thread work and cleans up
> after the Tx thread. */
> static int rtl8139_interrupt(rtdm_irq_t *irq_handle)
> @@ -1665,80 +1692,82 @@
> struct rtnet_device *rtdev =3D rtdm_irq_get_arg(irq_handle, st=
ruct rtnet_device);
> struct rtl8139_private *tp =3D rtdev->priv;
> =20
> - int boguscnt =3D max_interrupt_work;
> void *ioaddr =3D tp->mmio_addr;
> =20
> int ackstat;
> int status;
> int link_changed =3D 0; /* avoid bogus "uninit" warning */
> =20
> - int saved_status =3D 0;
> + int handled =3D 0;
> =20
> +
> rtdm_lock_get(&tp->lock);
> =20
> - do {
> - status =3D RTL_R16 (IntrStatus);
> + status =3D RTL_R16 (IntrStatus);
> =20
> - /* h/w no longer present (hotplug?) or major error, ba=
il */
> - if (status =3D=3D 0xFFFF)
> - break;
> + /* shared irq? */
> + if (unlikely((status & rtl8139_intr_mask) =3D=3D 0))
> + goto out;
> =20
> - if ((status &
> - (PCIErr | PCSTimeout | RxUnderrun | RxOverflow | =
RxFIFOOver | TxErr | TxOK | RxErr | RxOK)) =3D=3D 0)
> - break;
> + handled =3D 1;
> =20
> - /* Acknowledge all of the current interrupt sources AS=
AP, but
> - an first get an additional status bit from CSCR. */=
> - if (status & RxUnderrun)
> - link_changed =3D RTL_R16 (CSCR) & CSCR_LinkCha=
ngeBit;
> + /* h/w no longer present (hotplug?) or major error, bail */
> + if (status =3D=3D 0xFFFF)
> + goto out;
> =20
> - /* The chip takes special action when we clear RxAckBi=
ts,
> - * so we clear them later in rtl8139_rx_interrupt
> - */
> - ackstat =3D status & ~(RxAckBits | TxErr);
> - RTL_W16 (IntrStatus, ackstat);
> =20
> - if (rtnetif_running (rtdev) && (status & RxAckBits)) {=
> - saved_status |=3D RxAckBits;
> - rtl8139_rx_interrupt (rtdev, tp, ioaddr, &time=
_stamp);
> - }
> + /* close possible race's with dev_close */
> + if (!rtnetif_running(rtdev)) {
> + RTL_W16 (IntrMask, 0);
> + goto out;
> + }
Hmm, that points to an existing issue of our rtl8139_close: the IRQ is
released too early. But that's "only" unrelated cleanup stuff, will fix
later.
> =20
> - /* Check uncommon events with one test. */
> - if (status & (PCIErr | PCSTimeout | RxUnderrun | RxOve=
rflow | RxFIFOOver | RxErr)) {
> - rtl8139_weird_interrupt (rtdev, tp, ioaddr, st=
atus, link_changed);
> - }
> =20
> - if (rtnetif_running (rtdev) && (status & TxOK)) {
> - rtl8139_tx_interrupt (rtdev, tp, ioaddr);
> - if (status & TxErr)
> - RTL_W16 (IntrStatus, TxErr);
> - rtnetif_tx(rtdev);
> - }
> + /* Acknowledge all of the current interrupt sources ASAP, but
> + an first get an additional status bit from CSCR. */
> + if (status & RxUnderrun)
> + link_changed =3D RTL_R16 (CSCR) & CSCR_LinkChangeBit;
> =20
> - if (rtnetif_running (rtdev) && (status & TxErr)) {
> - saved_status|=3DTxErr;
> - }
> + ackstat =3D status & ~(RxAckBits | TxErr);
> + /*if (ackstat)
> + */RTL_W16 (IntrStatus, ackstat);
???
> =20
> - boguscnt--;
> - } while (boguscnt > 0);
> - if (boguscnt <=3D 0) {
> - rtdm_printk(KERN_WARNING "%s: Too much work at interru=
pt, "
> - "IntrStatus=3D0x%4.4x.\n", rtdev->name, sta=
tus);
> - /* Clear all interrupt sources. */
> - RTL_W16 (IntrStatus, 0xffff);
> - }
> + /* Receive packets are processed by poll routine.
> + If not running start it now. */
> + if (rtnetif_running (rtdev) && (status & RxAckBits)){
> + rtl8139_rx_interrupt (rtdev, tp, ioaddr, &time_stamp);=
> =20
> - rtdm_lock_put(&tp->lock);
Again: don't move this.
> =20
> - if (saved_status & RxAckBits) {
> + //RTL_W16_F (IntrMask, rtl8139_norx_intr_mask);
Tss, tss.
> rt_mark_stack_mgr(rtdev);
> }
> =20
> - if (saved_status & TxErr) {
> - rtnetif_err_tx(rtdev);
> + /* Check uncommon events with one test. */
> + if (unlikely(status & (PCIErr | PCSTimeout | RxUnderrun | RxOv=
erflow | RxFIFOOver | RxErr)))
> + rtl8139_weird_interrupt (rtdev, tp, ioaddr,
> + status, link_changed);
> +
> + if (status & (TxOK | TxErr)) {
> + rtl8139_tx_interrupt (rtdev, tp, ioaddr);
> + if (status & TxErr)
> + RTL_W16 (IntrStatus, TxErr);
> + rtnetif_tx(rtdev);
> }
> + if (rtnetif_running (rtdev) && (status & TxErr)) {
> + rtnetif_err_tx(rtdev);
> + }
> =20
> - return RTDM_IRQ_HANDLED;
> + /* This wasn't in the kernel driver, but it was in the previou=
s rtnet
> + driver so i put it here. */
> + RTL_W16(IntrStatus, 0xffff);
That's from the "Too much work" error path. You don't check for this
anymore, so this clearing has to go (we may loose events otherwise).
> + out:
> + spin_unlock (&tp->lock);
Wrong locking primitive (recent I-pipe will bark at you). Put the
ordering like this:
- check for rx
- check for weird
- check for tx
- unlock
- if there was rx, rt_mark_stack_mgr
> +
> +
> + if( handled =3D=3D 1 )
> + return RTDM_IRQ_HANDLED;
> + else
> + return RTDM_IRQ_NONE;
Save the return value directly in a local variable, saves one comparison
here.
Jan
|
