On Wed, Feb 1, 2017 at 3:09 AM, David Laight [off-list ref] wrote:

From Florian Fainelli

quoted

Sent: 31 January 2017 20:33
On 01/31/2017 11:03 AM, Iyappan Subramanian wrote:

quoted

This patch adds,
    - Transmit
    - Transmit completion poll
    - Receive poll
    - NAPI handler

and enables the driver.

Signed-off-by: Iyappan Subramanian <redacted>
Signed-off-by: Keyur Chudgar <redacted>
---

quoted

+
+   tx_ring = pdata->tx_ring;
+   tail = tx_ring->tail;
+   len = skb_headlen(skb);
+   raw_desc = &tx_ring->raw_desc[tail];
+
+   /* Tx descriptor not available */
+   if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)) ||
+       GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0)))
+           return NETDEV_TX_BUSY;

Aren't you supposed to detect 'ring full' and stop the code
giving you packets to transmit.

I'll add stop queue and wake queue.

quoted

quoted

+
+   /* Packet buffers should be 64B aligned */

Is that really a requirement of the hardware?
Almost all ethernet frames are 4n+2 aligned.

Yes, it's a hardware requirement.

quoted

quoted

+   pkt_buf = dma_alloc_coherent(dev, XGENE_ENET_STD_MTU, &dma_addr,
+                                GFP_ATOMIC);
+   if (unlikely(!pkt_buf))
+           goto out;

Can't you obtain a DMA-API mapping for skb->data and pass it down to the
hardware? This copy here is inefficient.

quoted

+
+   memcpy(pkt_buf, skb->data, len);

You really need to verify that the len <= XGENE_ENET_STD_MTU.

This version of the driver, doesn't support jumbo frame.  So, the
check is not required.

Isn't this code only transmitting the 'head' of the packet?
What about the fragments??

This driver doesn't enable SG yet.

...
        David