On Mon, Feb 1, 2010 at 8:19 AM, christian pellegrin <[email protected]> wrote:
> even with the MERR handling (but can you confirm this?). The first
> thing that comes to my mind is using the standard netdev tx watchdog
> to restart it, but I don't know it's an elegant solution.
After a second thought and after reading the chapter about one shot
mode on a CAN book I've understood that this is mode is all about real
time behavior. So I think it's better to give a precise control to the
user of write syscall on when the packets can be send. So in
CAN_ONE_SHOT_MODE frame transmit returns error until the TX buffer is
free. There is no netif queue flow control, so a write in a busy loop
can happily overload the system. Unfortunately I don't see any other
way to get a feedback in one shot mode after a lost arbitration (the
basic problem is that the mcp2515 doesn't have a "tx buffer free"
interrupt but a "frame txed" one) after studying the data sheet of the
mcp can controller (message error could be trapped by MERR interrupt
but lost of arbitration cannot). Let me know what you think about the
patch below. Another problem is that right now I don't have a mcp2515,
so Paul may you kindly test it? TIA!
diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index 884d309..4ae2e04 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -465,15 +465,20 @@ static netdev_tx_t
mcp251x_hard_start_xmit(struct sk_buff *skb,
struct mcp251x_priv *priv = netdev_priv(net);
struct spi_device *spi = priv->spi;
- if (priv->tx_skb || priv->tx_len) {
- dev_warn(&spi->dev, "hard_xmit called while tx busy\n");
- return NETDEV_TX_BUSY;
- }
-
if (can_dropped_invalid_skb(net, skb))
return NETDEV_TX_OK;
- netif_stop_queue(net);
+ if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT) {
+ if (priv->tx_skb)
+ return NETDEV_TX_BUSY;
+ }
+ else {
+ if (priv->tx_skb || priv->tx_len) {
+ dev_warn(&spi->dev, "hard_xmit called while tx busy\n");
+ return NETDEV_TX_BUSY;
+ }
+ netif_stop_queue(net);
+ }
priv->tx_skb = skb;
net->trans_start = jiffies;
queue_work(priv->wq, &priv->tx_work);
@@ -699,8 +704,19 @@ static void mcp251x_tx_work_handler(struct work_struct *ws)
if (priv->can.state == CAN_STATE_BUS_OFF) {
mcp251x_clean(net);
} else {
+ if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT) {
+ if (priv->tx_len > 0 &&
+ (mcp251x_read_reg(spi, TXBCTRL(0)) &
+ TXBCTRL_TXREQ) == 0) {
+ /* TX buffer was not sent so clear */
+ mcp251x_clean(net);
+ } else {
+ /* else try again after some time */
+ queue_work(priv->wq, &priv->tx_work);
+ goto mcp_unlock;
+ }
+ }
frame = (struct can_frame *)priv->tx_skb->data;
-
if (frame->can_dlc > CAN_FRAME_MAX_DATA_LEN)
frame->can_dlc = CAN_FRAME_MAX_DATA_LEN;
mcp251x_hw_tx(spi, frame, 0);
@@ -709,6 +725,7 @@ static void mcp251x_tx_work_handler(struct work_struct *ws)
priv->tx_skb = NULL;
}
}
+mcp_unlock:
mutex_unlock(&priv->mcp_lock);
}
@@ -848,7 +865,9 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
can_get_echo_skb(net, 0);
priv->tx_len = 0;
}
- netif_wake_queue(net);
+ if (!(priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)) {
+ netif_wake_queue(net);
+ }
}
}
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
