netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* pull-request: can 2015-01-15
@ 2015-01-15 16:11 Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 1/8] MAINTAINERS: update linux-can git repositories Marc Kleine-Budde
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2015-01-15 16:11 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel

Hello David,

this is a pull request of 8 patches.

Ahmed S. Darwish contributes 4 fixes for the kvaser_usb driver. The two patches
by Oliver Hartkopp mark the m_can driver as non-ISO, as the CANFD standard was
updated. Roger Quadros's patch for the c_can driver fixes the register access
during RAMINIT. And one patch by my, which updates the MAINTAINERS file, as we
moved the git repos to the kernel.org infrastructure.

regards,
Marc

---

The following changes since commit 16dde0d6ac159531a5e03cd3f8bc8a401d9f3fb6:

  be2net: Allow GRE to work concurrently while a VxLAN tunnel is configured (2015-01-15 01:55:05 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-3.19-20150115

for you to fetch changes up to a58518ccf39f86f898a65201518dd8e799b3abeb:

  can: kvaser_usb: Don't dereference skb after a netif_rx() (2015-01-15 16:58:02 +0100)

----------------------------------------------------------------
linux-can-fixes-for-3.19-20150115

----------------------------------------------------------------
Ahmed S. Darwish (4):
      can: kvaser_usb: Don't free packets when tight on URBs
      can: kvaser_usb: Reset all URB tx contexts upon channel close
      can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels
      can: kvaser_usb: Don't dereference skb after a netif_rx()

Marc Kleine-Budde (1):
      MAINTAINERS: update linux-can git repositories

Oliver Hartkopp (2):
      can: dev: fix crtlmode_supported check
      can: m_can: tag current CAN FD controllers as non-ISO

Roger Quadros (1):
      can: c_can: use regmap_update_bits() to modify RAMINIT register

 MAINTAINERS                            |  6 ++++--
 drivers/net/can/c_can/c_can_platform.c | 29 ++++++++++++++++++-----------
 drivers/net/can/dev.c                  |  8 ++++++--
 drivers/net/can/m_can/m_can.c          |  5 +++++
 drivers/net/can/usb/kvaser_usb.c       | 31 +++++++++++++++----------------
 include/uapi/linux/can/netlink.h       |  1 +
 6 files changed, 49 insertions(+), 31 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/8] MAINTAINERS: update linux-can git repositories
  2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
@ 2015-01-15 16:11 ` Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 2/8] can: dev: fix crtlmode_supported check Marc Kleine-Budde
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2015-01-15 16:11 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde

The linux-can upstream git repositories are now hosted on kernel.org, update
MAINTAINERS accordingly.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 MAINTAINERS | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 600d2aad8276..efa5f8d4086d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2346,7 +2346,8 @@ CAN NETWORK LAYER
 M:	Oliver Hartkopp <socketcan@hartkopp.net>
 L:	linux-can@vger.kernel.org
 W:	http://gitorious.org/linux-can
-T:	git git://gitorious.org/linux-can/linux-can-next.git
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
 S:	Maintained
 F:	Documentation/networking/can.txt
 F:	net/can/
@@ -2361,7 +2362,8 @@ M:	Wolfgang Grandegger <wg@grandegger.com>
 M:	Marc Kleine-Budde <mkl@pengutronix.de>
 L:	linux-can@vger.kernel.org
 W:	http://gitorious.org/linux-can
-T:	git git://gitorious.org/linux-can/linux-can-next.git
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
 S:	Maintained
 F:	drivers/net/can/
 F:	include/linux/can/dev.h
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/8] can: dev: fix crtlmode_supported check
  2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 1/8] MAINTAINERS: update linux-can git repositories Marc Kleine-Budde
@ 2015-01-15 16:11 ` Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 3/8] can: m_can: tag current CAN FD controllers as non-ISO Marc Kleine-Budde
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2015-01-15 16:11 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Oliver Hartkopp, Wolfgang Grandegger,
	linux-stable, Marc Kleine-Budde

From: Oliver Hartkopp <socketcan@hartkopp.net>

When changing flags in the CAN drivers ctrlmode the provided new content has to
be checked whether the bits are allowed to be changed. The bits that are to be
changed are given as a bitfield in cm->mask. Therefore checking against
cm->flags is wrong as the content can hold any kind of values.

The iproute2 tool sets the bits in cm->mask and cm->flags depending on the
detected command line options. To be robust against bogus user space
applications additionally sanitize the provided flags with the provided mask.

Cc: Wolfgang Grandegger <wg@grandegger.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/dev.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3ec8f6f25e5f..847c1f813261 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -807,10 +807,14 @@ static int can_changelink(struct net_device *dev,
 		if (dev->flags & IFF_UP)
 			return -EBUSY;
 		cm = nla_data(data[IFLA_CAN_CTRLMODE]);
-		if (cm->flags & ~priv->ctrlmode_supported)
+
+		/* check whether changed bits are allowed to be modified */
+		if (cm->mask & ~priv->ctrlmode_supported)
 			return -EOPNOTSUPP;
+
+		/* clear bits to be modified and copy the flag values */
 		priv->ctrlmode &= ~cm->mask;
-		priv->ctrlmode |= cm->flags;
+		priv->ctrlmode |= (cm->flags & cm->mask);
 
 		/* CAN_CTRLMODE_FD can only be set when driver supports FD */
 		if (priv->ctrlmode & CAN_CTRLMODE_FD)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/8] can: m_can: tag current CAN FD controllers as non-ISO
  2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 1/8] MAINTAINERS: update linux-can git repositories Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 2/8] can: dev: fix crtlmode_supported check Marc Kleine-Budde
@ 2015-01-15 16:11 ` Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 4/8] can: c_can: use regmap_update_bits() to modify RAMINIT register Marc Kleine-Budde
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2015-01-15 16:11 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Oliver Hartkopp, linux-stable,
	Marc Kleine-Budde

From: Oliver Hartkopp <socketcan@hartkopp.net>

During the CAN FD standardization process within the ISO it turned out that
the failure detection capability has to be improved.

The CAN in Automation organization (CiA) defined the already implemented CAN
FD controllers as 'non-ISO' and the upcoming improved CAN FD controllers as
'ISO' compliant. See at http://www.can-cia.com/index.php?id=1937

Finally there will be three types of CAN FD controllers in the future:

1. ISO compliant (fixed)
2. non-ISO compliant (fixed, like the M_CAN IP v3.0.1 in m_can.c)
3. ISO/non-ISO CAN FD controllers (switchable, like the PEAK USB FD)

So the current M_CAN driver for the M_CAN IP v3.0.1 has to expose its non-ISO
implementation by setting the CAN_CTRLMODE_FD_NON_ISO ctrlmode at startup.
As this bit cannot be switched at configuration time CAN_CTRLMODE_FD_NON_ISO
must not be set in ctrlmode_supported of the current M_CAN driver.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c    | 5 +++++
 include/uapi/linux/can/netlink.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index d7bc462aafdc..244529881be9 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -955,6 +955,11 @@ static struct net_device *alloc_m_can_dev(void)
 	priv->can.data_bittiming_const = &m_can_data_bittiming_const;
 	priv->can.do_set_mode = m_can_set_mode;
 	priv->can.do_get_berr_counter = m_can_get_berr_counter;
+
+	/* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.1 */
+	priv->can.ctrlmode = CAN_CTRLMODE_FD_NON_ISO;
+
+	/* CAN_CTRLMODE_FD_NON_ISO can not be changed with M_CAN IP v3.0.1 */
 	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
 					CAN_CTRLMODE_LISTENONLY |
 					CAN_CTRLMODE_BERR_REPORTING |
diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index 3e4323a3918d..94ffe0c83ce7 100644
--- a/include/uapi/linux/can/netlink.h
+++ b/include/uapi/linux/can/netlink.h
@@ -98,6 +98,7 @@ struct can_ctrlmode {
 #define CAN_CTRLMODE_BERR_REPORTING	0x10	/* Bus-error reporting */
 #define CAN_CTRLMODE_FD			0x20	/* CAN FD mode */
 #define CAN_CTRLMODE_PRESUME_ACK	0x40	/* Ignore missing CAN ACKs */
+#define CAN_CTRLMODE_FD_NON_ISO		0x80	/* CAN FD in non-ISO mode */
 
 /*
  * CAN device statistics
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/8] can: c_can: use regmap_update_bits() to modify RAMINIT register
  2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
                   ` (2 preceding siblings ...)
  2015-01-15 16:11 ` [PATCH 3/8] can: m_can: tag current CAN FD controllers as non-ISO Marc Kleine-Budde
@ 2015-01-15 16:11 ` Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 5/8] can: kvaser_usb: Don't free packets when tight on URBs Marc Kleine-Budde
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2015-01-15 16:11 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Roger Quadros, Marc Kleine-Budde

From: Roger Quadros <rogerq@ti.com>

use of regmap_read() and regmap_write() in c_can_hw_raminit_syscon()
is not safe as the RAMINIT register can be shared between different drivers
at least for TI SoCs.

To make the modification atomic we switch to using regmap_update_bits().

regmap_update_bits() skips writing to the register if it's read content is the
same as what is going to be written. This causes an issue for us when we
need to clear the DONE bit with the initial condition START:0, DONE:1 as
DONE bit must be written with 1 to clear it.

So we defer the clearing of DONE bit to later when we set the START bit.
There we are sure that START bit is changed from 0 to 1 so the write of
1 to already set DONE bit will happen.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can_platform.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index f363972cd77d..e36d10520e24 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -103,27 +103,34 @@ static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
 	mask = 1 << raminit->bits.start | 1 << raminit->bits.done;
 	regmap_read(raminit->syscon, raminit->reg, &ctrl);
 
-	/* We clear the done and start bit first. The start bit is
+	/* We clear the start bit first. The start bit is
 	 * looking at the 0 -> transition, but is not self clearing;
-	 * And we clear the init done bit as well.
 	 * NOTE: DONE must be written with 1 to clear it.
+	 * We can't clear the DONE bit here using regmap_update_bits()
+	 * as it will bypass the write if initial condition is START:0 DONE:1
+	 * e.g. on DRA7 which needs START pulse.
 	 */
-	ctrl &= ~(1 << raminit->bits.start);
-	ctrl |= 1 << raminit->bits.done;
-	regmap_write(raminit->syscon, raminit->reg, ctrl);
+	ctrl &= ~mask;	/* START = 0, DONE = 0 */
+	regmap_update_bits(raminit->syscon, raminit->reg, mask, ctrl);
 
-	ctrl &= ~(1 << raminit->bits.done);
-	c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
+	/* check if START bit is 0. Ignore DONE bit for now
+	 * as it can be either 0 or 1.
+	 */
+	c_can_hw_raminit_wait_syscon(priv, 1 << raminit->bits.start, ctrl);
 
 	if (enable) {
-		/* Set start bit and wait for the done bit. */
+		/* Clear DONE bit & set START bit. */
 		ctrl |= 1 << raminit->bits.start;
-		regmap_write(raminit->syscon, raminit->reg, ctrl);
-
+		/* DONE must be written with 1 to clear it */
+		ctrl |= 1 << raminit->bits.done;
+		regmap_update_bits(raminit->syscon, raminit->reg, mask, ctrl);
+		/* prevent further clearing of DONE bit */
+		ctrl &= ~(1 << raminit->bits.done);
 		/* clear START bit if start pulse is needed */
 		if (raminit->needs_pulse) {
 			ctrl &= ~(1 << raminit->bits.start);
-			regmap_write(raminit->syscon, raminit->reg, ctrl);
+			regmap_update_bits(raminit->syscon, raminit->reg,
+					   mask, ctrl);
 		}
 
 		ctrl |= 1 << raminit->bits.done;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/8] can: kvaser_usb: Don't free packets when tight on URBs
  2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
                   ` (3 preceding siblings ...)
  2015-01-15 16:11 ` [PATCH 4/8] can: c_can: use regmap_update_bits() to modify RAMINIT register Marc Kleine-Budde
@ 2015-01-15 16:11 ` Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 6/8] can: kvaser_usb: Reset all URB tx contexts upon channel close Marc Kleine-Budde
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2015-01-15 16:11 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Ahmed S. Darwish, linux-stable,
	Marc Kleine-Budde

From: "Ahmed S. Darwish" <ahmed.darwish@valeo.com>

Flooding the Kvaser CAN to USB dongle with multiple reads and
writes in high frequency caused seemingly-random panics in the
kernel.

On further inspection, it seems the driver erroneously freed the
to-be-transmitted packet upon getting tight on URBs and returning
NETDEV_TX_BUSY, leading to invalid memory writes and double frees
at a later point in time.

Note:

Finding no more URBs/transmit-contexts and returning NETDEV_TX_BUSY
is a driver bug in and out of itself: it means that our start/stop
queue flow control is broken.

This patch only fixes the (buggy) error handling code; the root
cause shall be fixed in a later commit.

Acked-by: Olivier Sobrie <olivier@sobrie.be>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 541fb7a05625..2e7d513a7c11 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1294,12 +1294,14 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 	if (!urb) {
 		netdev_err(netdev, "No memory left for URBs\n");
 		stats->tx_dropped++;
-		goto nourbmem;
+		dev_kfree_skb(skb);
+		return NETDEV_TX_OK;
 	}
 
 	buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
 	if (!buf) {
 		stats->tx_dropped++;
+		dev_kfree_skb(skb);
 		goto nobufmem;
 	}
 
@@ -1334,6 +1336,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 		}
 	}
 
+	/* This should never happen; it implies a flow control bug */
 	if (!context) {
 		netdev_warn(netdev, "cannot find free context\n");
 		ret =  NETDEV_TX_BUSY;
@@ -1364,9 +1367,6 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 	if (unlikely(err)) {
 		can_free_echo_skb(netdev, context->echo_index);
 
-		skb = NULL; /* set to NULL to avoid double free in
-			     * dev_kfree_skb(skb) */
-
 		atomic_dec(&priv->active_tx_urbs);
 		usb_unanchor_urb(urb);
 
@@ -1388,8 +1388,6 @@ releasebuf:
 	kfree(buf);
 nobufmem:
 	usb_free_urb(urb);
-nourbmem:
-	dev_kfree_skb(skb);
 	return ret;
 }
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/8] can: kvaser_usb: Reset all URB tx contexts upon channel close
  2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
                   ` (4 preceding siblings ...)
  2015-01-15 16:11 ` [PATCH 5/8] can: kvaser_usb: Don't free packets when tight on URBs Marc Kleine-Budde
@ 2015-01-15 16:11 ` Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 7/8] can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels Marc Kleine-Budde
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2015-01-15 16:11 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Ahmed S. Darwish, linux-stable,
	Marc Kleine-Budde

From: "Ahmed S. Darwish" <ahmed.darwish@valeo.com>

Flooding the Kvaser CAN to USB dongle with multiple reads and
writes in very high frequency (*), closing the CAN channel while
all the transmissions are on (#), opening the device again (@),
then sending a small number of packets would make the driver
enter an almost infinite loop of:

[....]
[15959.853988] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853990] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853991] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853993] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853994] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853995] kvaser_usb 4-3:1.0 can0: cannot find free context
[....]

_dragging the whole system down_ in the process due to the
excessive logging output.

Initially, this has caused random panics in the kernel due to a
buggy error recovery path.  That got fixed in an earlier commit.(%)
This patch aims at solving the root cause. -->

16 tx URBs and contexts are allocated per CAN channel per USB
device. Such URBs are protected by:

a) A simple atomic counter, up to a value of MAX_TX_URBS (16)
b) A flag in each URB context, stating if it's free
c) The fact that ndo_start_xmit calls are themselves protected
   by the networking layers higher above

After grabbing one of the tx URBs, if the driver noticed that all
of them are now taken, it stops the netif transmission queue.
Such queue is worken up again only if an acknowedgment was received
from the firmware on one of our earlier-sent frames.

Meanwhile, upon channel close (#), the driver sends a CMD_STOP_CHIP
to the firmware, effectively closing all further communication.  In
the high traffic case, the atomic counter remains at MAX_TX_URBS,
and all the URB contexts remain marked as active.  While opening
the channel again (@), it cannot send any further frames since no
more free tx URB contexts are available.

Reset all tx URB contexts upon CAN channel close.

(*) 50 parallel instances of `cangen0 -g 0 -ix`
(#) `ifconfig can0 down`
(@) `ifconfig can0 up`
(%) "can: kvaser_usb: Don't free packets when tight on URBs"

Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 2e7d513a7c11..9accc8272c27 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1246,6 +1246,9 @@ static int kvaser_usb_close(struct net_device *netdev)
 	if (err)
 		netdev_warn(netdev, "Cannot stop device, error %d\n", err);
 
+	/* reset tx contexts */
+	kvaser_usb_unlink_tx_urbs(priv);
+
 	priv->can.state = CAN_STATE_STOPPED;
 	close_candev(priv->netdev);
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/8] can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels
  2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
                   ` (5 preceding siblings ...)
  2015-01-15 16:11 ` [PATCH 6/8] can: kvaser_usb: Reset all URB tx contexts upon channel close Marc Kleine-Budde
@ 2015-01-15 16:11 ` Marc Kleine-Budde
  2015-01-15 16:11 ` [PATCH 8/8] can: kvaser_usb: Don't dereference skb after a netif_rx() Marc Kleine-Budde
  2015-01-16  0:39 ` pull-request: can 2015-01-15 David Miller
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2015-01-15 16:11 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Ahmed S. Darwish, Olivier Sobrie,
	linux-stable, Marc Kleine-Budde

From: "Ahmed S. Darwish" <ahmed.darwish@valeo.com>

Recent Leaf firmware versions (>= 3.1.557) do not allow to send
commands for non-existing channels.  If a command is sent for a
non-existing channel, the firmware crashes.

Reported-by: Christopher Storah <Christopher.Storah@invetech.com.au>
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 9accc8272c27..cc7bfc0c0a71 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1503,6 +1503,10 @@ static int kvaser_usb_init_one(struct usb_interface *intf,
 	struct kvaser_usb_net_priv *priv;
 	int i, err;
 
+	err = kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, channel);
+	if (err)
+		return err;
+
 	netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
 	if (!netdev) {
 		dev_err(&intf->dev, "Cannot alloc candev\n");
@@ -1607,9 +1611,6 @@ static int kvaser_usb_probe(struct usb_interface *intf,
 
 	usb_set_intfdata(intf, dev);
 
-	for (i = 0; i < MAX_NET_DEVICES; i++)
-		kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
-
 	err = kvaser_usb_get_software_info(dev);
 	if (err) {
 		dev_err(&intf->dev,
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/8] can: kvaser_usb: Don't dereference skb after a netif_rx()
  2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
                   ` (6 preceding siblings ...)
  2015-01-15 16:11 ` [PATCH 7/8] can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels Marc Kleine-Budde
@ 2015-01-15 16:11 ` Marc Kleine-Budde
  2015-01-16  0:39 ` pull-request: can 2015-01-15 David Miller
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2015-01-15 16:11 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Ahmed S. Darwish, Marc Kleine-Budde

From: "Ahmed S. Darwish" <ahmed.darwish@valeo.com>

We should not touch the packet after a netif_rx: it might
get freed behind our back.

Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index cc7bfc0c0a71..c32cd61073bc 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -520,10 +520,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
 		skb = alloc_can_err_skb(priv->netdev, &cf);
 		if (skb) {
 			cf->can_id |= CAN_ERR_RESTARTED;
-			netif_rx(skb);
 
 			stats->rx_packets++;
 			stats->rx_bytes += cf->can_dlc;
+			netif_rx(skb);
 		} else {
 			netdev_err(priv->netdev,
 				   "No memory left for err_skb\n");
@@ -770,10 +770,9 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 
 	priv->can.state = new_state;
 
-	netif_rx(skb);
-
 	stats->rx_packets++;
 	stats->rx_bytes += cf->can_dlc;
+	netif_rx(skb);
 }
 
 static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
@@ -805,10 +804,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
 		stats->rx_over_errors++;
 		stats->rx_errors++;
 
-		netif_rx(skb);
-
 		stats->rx_packets++;
 		stats->rx_bytes += cf->can_dlc;
+		netif_rx(skb);
 	}
 }
 
@@ -887,10 +885,9 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
 			       cf->can_dlc);
 	}
 
-	netif_rx(skb);
-
 	stats->rx_packets++;
 	stats->rx_bytes += cf->can_dlc;
+	netif_rx(skb);
 }
 
 static void kvaser_usb_start_chip_reply(const struct kvaser_usb *dev,
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: pull-request: can 2015-01-15
  2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
                   ` (7 preceding siblings ...)
  2015-01-15 16:11 ` [PATCH 8/8] can: kvaser_usb: Don't dereference skb after a netif_rx() Marc Kleine-Budde
@ 2015-01-16  0:39 ` David Miller
  8 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2015-01-16  0:39 UTC (permalink / raw)
  To: mkl; +Cc: netdev, linux-can, kernel

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Thu, 15 Jan 2015 17:11:15 +0100

> this is a pull request of 8 patches.
> 
> Ahmed S. Darwish contributes 4 fixes for the kvaser_usb driver. The two patches
> by Oliver Hartkopp mark the m_can driver as non-ISO, as the CANFD standard was
> updated. Roger Quadros's patch for the c_can driver fixes the register access
> during RAMINIT. And one patch by my, which updates the MAINTAINERS file, as we
> moved the git repos to the kernel.org infrastructure.

Pulled, thank you Marc.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-01-16  0:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-15 16:11 pull-request: can 2015-01-15 Marc Kleine-Budde
2015-01-15 16:11 ` [PATCH 1/8] MAINTAINERS: update linux-can git repositories Marc Kleine-Budde
2015-01-15 16:11 ` [PATCH 2/8] can: dev: fix crtlmode_supported check Marc Kleine-Budde
2015-01-15 16:11 ` [PATCH 3/8] can: m_can: tag current CAN FD controllers as non-ISO Marc Kleine-Budde
2015-01-15 16:11 ` [PATCH 4/8] can: c_can: use regmap_update_bits() to modify RAMINIT register Marc Kleine-Budde
2015-01-15 16:11 ` [PATCH 5/8] can: kvaser_usb: Don't free packets when tight on URBs Marc Kleine-Budde
2015-01-15 16:11 ` [PATCH 6/8] can: kvaser_usb: Reset all URB tx contexts upon channel close Marc Kleine-Budde
2015-01-15 16:11 ` [PATCH 7/8] can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels Marc Kleine-Budde
2015-01-15 16:11 ` [PATCH 8/8] can: kvaser_usb: Don't dereference skb after a netif_rx() Marc Kleine-Budde
2015-01-16  0:39 ` pull-request: can 2015-01-15 David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).