On 02/26/2014 02:07 PM, Appana Durga Kedareswara Rao wrote:

quoted

This loop looks broken. Can you explain how it works.

What it shoud do is:
We have put (priv->tx_head - priv->tx_tail) CAN frames into the FIFO.
This means at maximum there could be this amount of CAN frames which
have been successfully transmitted. For every cycle in this while loop you
should:
a) check if a CAN frame has successfully been transmitted
   (as this CAN core uses a FIFO it should be "oldest")
   A read_reg() of some kind is missing in your loop.
b) if needed, remove this event from the FIFO or
   mark the interrupt as done. Whatever you hardware needs.
c) update your statistics
d) Use can_get_echo_skb to push this frame into the networking stack
e) As a CAN frame has been transmitted successfully, wake the tx_queue.

quoted

+   while (priv->tx_head - priv->tx_tail > 0) {
+           if (isr & XCAN_IXR_TXFLL_MASK) {
+                   priv->write_reg(priv, XCAN_ICR_OFFSET,
+                                   XCAN_IXR_TXFLL_MASK);
+                   netif_stop_queue(ndev);

Why do you stop the queue here? A CAN frame has successfully been
transmitted, there should be room in the FIFO.

quoted

+                   break;
+           }
+           can_get_echo_skb(ndev, priv->tx_tail %
+                                   priv->xcan_echo_skb_max_tx);
+           priv->tx_tail++;
+   }
+

The below are the bit fields available for the Transmit FIFO.
1) In the ISR(interrupt status register)  Tx Ok interrupt and Tx fifo full interrupt.
2) in the SR(Status Register) Tx fifo full condition.


I am modifying the entire tx interrupt logic to like below.

static void xcan_tx_interrupt(struct net_device *ndev, u32 isr)
{
        struct xcan_priv *priv = netdev_priv(ndev);
        struct net_device_stats *stats = &ndev->stats;

            while (priv->tx_head - priv->tx_tail > 0) {
                if (isr & XCAN_IXR_TXFLL_MASK) {
                        priv->write_reg(priv, XCAN_ICR_OFFSET,
                                        XCAN_IXR_TXFLL_MASK);
                          break;
                }
                can_get_echo_skb(ndev, priv->tx_tail %
                                        priv->xcan_echo_skb_max_tx);
                priv->tx_tail++;
         stats->tx_packets++;
        netif_wake_queue(ndev);
              can_led_event(ndev, CAN_LED_EVENT_TX);

        }

You just need to wake the queue once.

}


Are you Ok with the above logic?

No, how can you tell how many frames have been transmitted?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |