All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: linux-can <linux-can@vger.kernel.org>
Cc: Alexander Stein <alexander.stein@systec-electronic.com>,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	Wolfgang Grandegger <wg@grandegger.com>, Mark <mark5@del-llc.com>
Subject: [patch V2 02/21] can: c_can: Fix startup logic
Date: Fri, 11 Apr 2014 08:13:10 -0000	[thread overview]
Message-ID: <20140411080650.634599352@linutronix.de> (raw)
In-Reply-To: 20140411080547.845836199@linutronix.de

[-- Attachment #1: can-c_can-fix-startup-logic.patch --]
[-- Type: text/plain, Size: 4143 bytes --]

c_can_start() enables interrupts way too early. The first enabling
happens when setting the control mode in c_can_chip_config() and then
again at the end of the function.

But that happens before napi_enable() and that means that an interrupt
which comes in will disable interrupts again and call napi_schedule,
which ignores the request and the later napi_enable() is not making
thinks work either. So the interface is up with all device interrupts
disabled.

Move the device interrupt after napi_enable() and add it to the other
callsites of c_can_start() in c_can_set_mode() and c_can_power_up()

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/net/can/c_can/c_can.c |   35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

Index: linux-2.6/drivers/net/can/c_can/c_can.c
===================================================================
--- linux-2.6.orig/drivers/net/can/c_can/c_can.c
+++ linux-2.6/drivers/net/can/c_can/c_can.c
@@ -612,30 +612,22 @@ static int c_can_chip_config(struct net_
 	struct c_can_priv *priv = netdev_priv(dev);
 
 	/* enable automatic retransmission */
-	priv->write_reg(priv, C_CAN_CTRL_REG,
-			CONTROL_ENABLE_AR);
+	priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_ENABLE_AR);
 
 	if ((priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) &&
 	    (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)) {
 		/* loopback + silent mode : useful for hot self-test */
-		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
-				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
-		priv->write_reg(priv, C_CAN_TEST_REG,
-				TEST_LBACK | TEST_SILENT);
+		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
+		priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK | TEST_SILENT);
 	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
 		/* loopback mode : useful for self-test function */
-		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
-				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
+		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
 		priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK);
 	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
 		/* silent mode : bus-monitoring mode */
-		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
-				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
+		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
 		priv->write_reg(priv, C_CAN_TEST_REG, TEST_SILENT);
-	} else
-		/* normal mode*/
-		priv->write_reg(priv, C_CAN_CTRL_REG,
-				CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
+	}
 
 	/* configure message objects */
 	c_can_configure_msg_objects(dev);
@@ -662,9 +654,6 @@ static int c_can_start(struct net_device
 	/* reset tx helper pointers */
 	priv->tx_next = priv->tx_echo = 0;
 
-	/* enable status change, error and module interrupts */
-	c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
-
 	return 0;
 }
 
@@ -681,6 +670,7 @@ static void c_can_stop(struct net_device
 
 static int c_can_set_mode(struct net_device *dev, enum can_mode mode)
 {
+	struct c_can_priv *priv = netdev_priv(dev);
 	int err;
 
 	switch (mode) {
@@ -689,6 +679,8 @@ static int c_can_set_mode(struct net_dev
 		if (err)
 			return err;
 		netif_wake_queue(dev);
+		/* enable status change, error and module interrupts */
+		c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
 		break;
 	default:
 		return -EOPNOTSUPP;
@@ -1184,6 +1176,8 @@ static int c_can_open(struct net_device
 	can_led_event(dev, CAN_LED_EVENT_OPEN);
 
 	napi_enable(&priv->napi);
+	/* enable status change, error and module interrupts */
+	c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
 	netif_start_queue(dev);
 
 	return 0;
@@ -1281,6 +1275,7 @@ int c_can_power_up(struct net_device *de
 	u32 val;
 	unsigned long time_out;
 	struct c_can_priv *priv = netdev_priv(dev);
+	int ret;
 
 	if (!(dev->flags & IFF_UP))
 		return 0;
@@ -1307,7 +1302,11 @@ int c_can_power_up(struct net_device *de
 	if (time_after(jiffies, time_out))
 		return -ETIMEDOUT;
 
-	return c_can_start(dev);
+	ret = c_can_start(dev);
+	if (!ret)
+		c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(c_can_power_up);
 #endif



  reply	other threads:[~2014-04-11  8:12 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-11  8:13 [patch V2 00/21] can: c_can: Another pile of fixes and improvements Thomas Gleixner
2014-04-11  8:13 ` Thomas Gleixner [this message]
2014-04-11  8:13 ` [patch V2 01/21] can: c_can_pci: Set the type of the IP core Thomas Gleixner
2014-04-11  8:13 ` [patch V2 03/21] can: c_can: Make bus off interrupt disable logic work Thomas Gleixner
2014-04-11  8:13 ` [patch V2 04/21] can: c_can: Do not access skb after net_receive_skb() Thomas Gleixner
2014-04-11  8:13 ` [patch V2 05/21] can: c_can: Handle state change correctly Thomas Gleixner
2014-04-11  8:13 ` [patch V2 07/21] can: c_can: Always update error stats Thomas Gleixner
2014-04-11  8:13 ` [patch V2 06/21] can: c_can: Fix berr reporting Thomas Gleixner
2014-04-11  8:13 ` [patch V2 08/21] can: c_can: Simplify buffer reenabling Thomas Gleixner
2014-04-11  8:13 ` [patch V2 10/21] can: c_can: Get rid of pointless interrupts Thomas Gleixner
2014-04-11  8:13 ` [patch V2 09/21] can: c_can: Avoid status register update for D_CAN Thomas Gleixner
2014-04-11  8:13 ` [patch V2 11/21] can: c_can : Disable rx split as workaround Thomas Gleixner
2014-04-11  8:13 ` [patch V2 13/21] can: c_can: Cleanup irq enable/disable Thomas Gleixner
2014-04-11  8:13 ` [patch V2 12/21] can: c_can": Work around C_CAN RX wreckage Thomas Gleixner
2014-04-14  8:38   ` Alexander Stein
2014-04-14 20:13     ` Thomas Gleixner
2014-04-14 20:17       ` Marc Kleine-Budde
2014-04-11  8:13 ` [patch V2 14/21] can: c_can: Cleanup c_can_read_msg_object() Thomas Gleixner
2014-04-11  8:13 ` [patch V2 15/21] can: c_can Cleanup setup of receive buffers Thomas Gleixner
2014-04-11  8:13 ` [patch V2 16/21] can: c_can: Cleanup c_can_inval_msg_object() Thomas Gleixner
2014-04-11  8:13 ` [patch V2 17/21] can: c_can: Cleanup c_can_msg_obj_put/get() Thomas Gleixner
2014-04-11  8:13 ` [patch V2 18/21] can: c_can: Cleanup c_can_write_msg_object() Thomas Gleixner
2014-04-11  8:13 ` [patch V2 19/21] can: c_can: Use proper u32 variables in c_can_write_msg_object() Thomas Gleixner
2014-04-11  8:13 ` [patch V2 21/21] can: c_can: Speed up tx buffer invalidation Thomas Gleixner
2014-04-11  8:13 ` [patch V2 20/21] can: c_can: Remove tx locking Thomas Gleixner
2014-04-14  8:38 ` [patch V2 00/21] can: c_can: Another pile of fixes and improvements Alexander Stein

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140411080650.634599352@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=alexander.stein@systec-electronic.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mark5@del-llc.com \
    --cc=mkl@pengutronix.de \
    --cc=socketcan@hartkopp.net \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.