Re: [RFC PATCH] common receive API + r8169 use

[Francois Romieu <romieu@fr.zoreil.com>]

Michał Mirosław <mirq-linux@rere.qmqm.pl> :
[...]
> @@ -4808,6 +4844,29 @@ static inline void rtl8169_mark_as_last_descriptor(struct RxDesc *desc)
>  	desc->opts1 |= cpu_to_le32(RingEnd);
>  }
>  
> +static int rtl_add_rx_buffer(struct netdev_ring *ring, void *buf,
> +	dma_addr_t dma)
> +{
> +	unsigned next_tail = (ring->tail + 1) & (NUM_RX_DESC - 1);
> +	struct RxDesc *rxd = (struct RxDesc *)ring->desc_table + ring->tail;
> +
> +	if (next_tail == ACCESS_ONCE(ring->head))
> +		return -ENOSPC;
> +	ring->buf_table[ring->tail] = buf;
> +	ring->tail = next_tail;

The four lines above are driver agnostic.

[...]
> @@ -4841,9 +4900,16 @@ static int rtl8169_init_ring(struct net_device *dev)
>  	rtl8169_init_ring_indexes(tp);
>  
>  	memset(tp->tx_skb, 0x0, NUM_TX_DESC * sizeof(struct ring_info));
> +#ifdef NO_COMMON_RX_API
>  	memset(tp->Rx_databuff, 0x0, NUM_RX_DESC * sizeof(void *));
>  
>  	return rtl8169_rx_fill(tp);
> +#else
> +	rtl8169_mark_as_last_descriptor((struct RxDesc *)tp->rx_ring.desc_table +
> +		NUM_RX_DESC - 1);

> +	tp->rx_ring.bufsz = 0x4000;
> +	return netdev_fill_rx_ring(&tp->rx_ring);

return netdev_init_rx_ring(..., 0x4000);

[...]
> @@ -4955,10 +5025,12 @@ static void rtl8169_reset_task(struct work_struct *work)
>  		goto out_unlock;
>  
>  	rtl8169_wait_for_quiescence(dev);
> -
> +#ifdef NO_COMMON_RX_API
>  	for (i = 0; i < NUM_RX_DESC; i++)
>  		rtl8169_mark_to_asic(tp->RxDescArray + i, rx_buf_sz);
> -
> +#else
> +	netdev_reset_rx_ring(&tp->rx_ring, tp->rx_ring.bufsz);

netdev_reset_rx_ring() with a single parameter, netdev_resize_rx_ring()
otherwise ?

> +#endif
>  	rtl8169_tx_clear(tp);
>  
>  	rtl8169_hw_reset(tp);
> @@ -5356,6 +5428,91 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
>  	return count;
>  }
>  
> +static int rtl_rx_buffer(struct netdev_ring *ring)
> +{
> +	struct net_device *dev = ring->napi.dev;
> +	struct RxDesc *rxd = (struct RxDesc *)ring->desc_table + ring->head;
> +	dma_addr_t dma = le64_to_cpu(rxd->addr);
> +	void *buf = ring->buf_table[ring->head];

	void *buf = netdev_head_buf(ring); ?

The driver does not really use it. It could / should be really opaque.

> +	struct sk_buff *skb;
> +	u32 status;
> +
> +	status = le32_to_cpu(ACCESS_ONCE(rxd->opts1));
> +	if (status & DescOwn)
> +		return -ENOENT;
> +
> +	netdev_dbg(dev, "RxDesc[%d] = %08x %08x %016llx %p\n",
> +		ring->head, status, le32_to_cpu(rxd->opts2), dma, buf);
> +
> +	/*
> +	 * release this descriptor - it won't be reused at least until
> +	 * netdev_reuse_rx_buffer() or this function returns.
> +	 */
> +	if (!(status & RingEnd))
> +		++ring->head;
> +	else
> +		ring->head = 0;

You can probably add an helper for the lines above.

The style is a bit raw but it looks interesting.

-- 
Ueimor