Re: [PATCH] NAPI support for Sibyte MAC

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

Hello, you wrote:

>   This patch completes the NAPI functionality for SB1250 MAC, including making
>   NAPI a kernel option that can be turned on or off and adds the "sbmac_poll"
>   routine.

> Index: linux-2.6.14-cgl/drivers/net/Kconfig
> ===================================================================
> --- linux-2.6.14-cgl.orig/drivers/net/Kconfig	2006-09-20 14:58:54.000000000 -0700
> +++ linux-2.6.14-cgl/drivers/net/Kconfig	2006-09-20 17:04:31.000000000 -0700

[...]

> @@ -2075,12 +2143,52 @@
>  		 */
>  
>  		if (isr & (M_MAC_INT_CHANNEL << S_MAC_TX_CH0)) {
> -			sbdma_tx_process(sc,&(sc->sbm_txdma));
> +			sbdma_tx_process(sc,&(sc->sbm_txdma), 0);
> +#ifdef CONFIG_NETPOLL_TRAP
> +		       if (netpoll_trap()) {
> +			       if (test_and_clear_bit(__LINK_STATE_XOFF, &dev->state)) 
> +				       __netif_schedule(dev);
> +		       }
> +#endif
>  		}

    This just doesn't make sense. That option is enabled to *prevent* calls to 
__netif_schedule() -- you can't override it that way. (Well, how it works 
currently, doesn't make much sense either since it totally breaks the TX queue 
control -- I was going to post a patch).

> +	if (isr & (M_MAC_INT_CHANNEL << S_MAC_RX_CH0)) {
> +		if (netif_rx_schedule_prep(dev)) {
> +			__raw_writeq(0, sc->sbm_imr);
> +			__netif_rx_schedule(dev);
> +			/* Depend on the exit from poll to reenable intr */
> +		}
> +		else {
> +			/* may leave some packets behind */
> +			sbdma_rx_process(sc,&(sc->sbm_rxdma),
> +					 SBMAC_MAX_RXDESCR * 2, 0);
> +		}
> +	}
> +#else
> +	/* Non NAPI */
> +	for (;;) {
> + 
>  		/*
> -		 * Receives on channel 0
> +		 * Read the ISR (this clears the bits in the real
> +		 * register, except for counter addr)
>  		 */
> +		isr = __raw_readq(sc->sbm_isr) & ~M_MAC_COUNTER_ADDR;
> +
> +		if (isr == 0)
> +			break;
> +
> +		handled = 1;
> +
> +		if (isr & (M_MAC_INT_CHANNEL << S_MAC_TX_CH0)) {
> +			sbdma_tx_process(sc,&(sc->sbm_txdma),
> +					 SBMAC_MAX_RXDESCR * 2);
> +#ifdef CONFIG_NETPOLL_TRAP
> +		       if (netpoll_trap()) {
> +			       if (test_and_clear_bit(__LINK_STATE_XOFF, &dev->state)) 
> +				       __netif_schedule(dev);
> +		       }
> +#endif
> +		}

    Same here.

WBR, Sergei