All of lore.kernel.org
 help / color / mirror / Atom feed
* pull-request: can 2021-03-01
@ 2021-03-01 11:20 Marc Kleine-Budde
  2021-03-01 11:20 ` [net 1/6] can: flexcan: assert FRZ bit in flexcan_chip_freeze() Marc Kleine-Budde
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-03-01 11:20 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel

Hello Jakub, hello David,

this is a pull request of 6 patches for net/master.

The first 3 patches are by Joakim Zhang for the flexcan driver and fix
the probing and starting of the chip.

The next patch is by me, for the mcp251xfd driver and reverts the BQL
support. BQL support got mainline with rc1 and assumes that CAN frames
are always echoed, which is not the case. A proper fix requires
changes more changes and will be rolled out via linux-can-next later.

Oleksij Rempel's patch fixes the socket ref counting if socket was
closed before setting skb ownership.

Torin Cooper-Bennun's patch for the tcan4x5x driver fixes a race
condition, where the chip is first attached the bus and then the MRAM
is initialized, which may result in lost data.

regards,
Marc

---

The following changes since commit 447621e373bd1b22300445639b43c39f399e4c73:

  Merge branch 'net-hns3-fixes-fot-net' (2021-02-28 12:04:02 -0800)

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-5.12-20210301

for you to fetch changes up to 2712625200ed69c642b9abc3a403830c4643364c:

  can: tcan4x5x: tcan4x5x_init(): fix initialization - clear MRAM before entering Normal Mode (2021-03-01 11:45:15 +0100)

----------------------------------------------------------------
linux-can-fixes-for-5.12-20210301

----------------------------------------------------------------
Joakim Zhang (3):
      can: flexcan: assert FRZ bit in flexcan_chip_freeze()
      can: flexcan: enable RX FIFO after FRZ/HALT valid
      can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode

Marc Kleine-Budde (1):
      can: mcp251xfd: revert "can: mcp251xfd: add BQL support"

Oleksij Rempel (1):
      can: skb: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership

Torin Cooper-Bennun (1):
      can: tcan4x5x: tcan4x5x_init(): fix initialization - clear MRAM before entering Normal Mode

 drivers/net/can/flexcan.c                      | 24 +++++++++++++++---------
 drivers/net/can/m_can/tcan4x5x-core.c          |  6 +++---
 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 21 ++++-----------------
 include/linux/can/skb.h                        |  8 ++++++--
 4 files changed, 28 insertions(+), 31 deletions(-)




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

* [net 1/6] can: flexcan: assert FRZ bit in flexcan_chip_freeze()
  2021-03-01 11:20 pull-request: can 2021-03-01 Marc Kleine-Budde
@ 2021-03-01 11:20 ` Marc Kleine-Budde
  2021-03-01 11:20 ` [net 2/6] can: flexcan: enable RX FIFO after FRZ/HALT valid Marc Kleine-Budde
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-03-01 11:20 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Joakim Zhang, Marc Kleine-Budde

From: Joakim Zhang <qiangqing.zhang@nxp.com>

Assert HALT bit to enter freeze mode, there is a premise that FRZ bit is
asserted. This patch asserts FRZ bit in flexcan_chip_freeze, although
the reset value is 1b'1. This is a prepare patch, later patch will
invoke flexcan_chip_freeze() to enter freeze mode, which polling freeze
mode acknowledge.

Fixes: b1aa1c7a2165b ("can: flexcan: fix transition from and to freeze mode in chip_{,un}freeze")
Link: https://lore.kernel.org/r/20210218110037.16591-2-qiangqing.zhang@nxp.com
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 971ada36e37f..ee2d4967d66a 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -701,7 +701,7 @@ static int flexcan_chip_freeze(struct flexcan_priv *priv)
 	u32 reg;
 
 	reg = priv->read(&regs->mcr);
-	reg |= FLEXCAN_MCR_HALT;
+	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT;
 	priv->write(reg, &regs->mcr);
 
 	while (timeout-- && !(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))

base-commit: 447621e373bd1b22300445639b43c39f399e4c73
-- 
2.30.1



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

* [net 2/6] can: flexcan: enable RX FIFO after FRZ/HALT valid
  2021-03-01 11:20 pull-request: can 2021-03-01 Marc Kleine-Budde
  2021-03-01 11:20 ` [net 1/6] can: flexcan: assert FRZ bit in flexcan_chip_freeze() Marc Kleine-Budde
@ 2021-03-01 11:20 ` Marc Kleine-Budde
  2021-03-01 11:20 ` [net 3/6] can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode Marc Kleine-Budde
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-03-01 11:20 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Joakim Zhang, Marc Kleine-Budde

From: Joakim Zhang <qiangqing.zhang@nxp.com>

RX FIFO enable failed could happen when do system reboot stress test:

[    0.303958] flexcan 5a8d0000.can: 5a8d0000.can supply xceiver not found, using dummy regulator
[    0.304281] flexcan 5a8d0000.can (unnamed net_device) (uninitialized): Could not enable RX FIFO, unsupported core
[    0.314640] flexcan 5a8d0000.can: registering netdev failed
[    0.320728] flexcan 5a8e0000.can: 5a8e0000.can supply xceiver not found, using dummy regulator
[    0.320991] flexcan 5a8e0000.can (unnamed net_device) (uninitialized): Could not enable RX FIFO, unsupported core
[    0.331360] flexcan 5a8e0000.can: registering netdev failed
[    0.337444] flexcan 5a8f0000.can: 5a8f0000.can supply xceiver not found, using dummy regulator
[    0.337716] flexcan 5a8f0000.can (unnamed net_device) (uninitialized): Could not enable RX FIFO, unsupported core
[    0.348117] flexcan 5a8f0000.can: registering netdev failed

RX FIFO should be enabled after the FRZ/HALT are valid. But the current
code enable RX FIFO and FRZ/HALT at the same time.

Fixes: e955cead03117 ("CAN: Add Flexcan CAN controller driver")
Link: https://lore.kernel.org/r/20210218110037.16591-3-qiangqing.zhang@nxp.com
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index ee2d4967d66a..e66a51dbea0a 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -1865,10 +1865,14 @@ static int register_flexcandev(struct net_device *dev)
 	if (err)
 		goto out_chip_disable;
 
-	/* set freeze, halt and activate FIFO, restrict register access */
+	/* set freeze, halt */
+	err = flexcan_chip_freeze(priv);
+	if (err)
+		goto out_chip_disable;
+
+	/* activate FIFO, restrict register access */
 	reg = priv->read(&regs->mcr);
-	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
-		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
+	reg |=  FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
 	priv->write(reg, &regs->mcr);
 
 	/* Currently we only support newer versions of this core
-- 
2.30.1



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

* [net 3/6] can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode
  2021-03-01 11:20 pull-request: can 2021-03-01 Marc Kleine-Budde
  2021-03-01 11:20 ` [net 1/6] can: flexcan: assert FRZ bit in flexcan_chip_freeze() Marc Kleine-Budde
  2021-03-01 11:20 ` [net 2/6] can: flexcan: enable RX FIFO after FRZ/HALT valid Marc Kleine-Budde
@ 2021-03-01 11:20 ` Marc Kleine-Budde
  2021-03-16 16:00   ` [BUG] " Ahmad Fatoum
  2021-03-01 11:20 ` [net 4/6] can: mcp251xfd: revert "can: mcp251xfd: add BQL support" Marc Kleine-Budde
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-03-01 11:20 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Joakim Zhang, Marc Kleine-Budde

From: Joakim Zhang <qiangqing.zhang@nxp.com>

Invoke flexcan_chip_freeze() to enter freeze mode, since need poll
freeze mode acknowledge.

Fixes: e955cead03117 ("CAN: Add Flexcan CAN controller driver")
Link: https://lore.kernel.org/r/20210218110037.16591-4-qiangqing.zhang@nxp.com
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index e66a51dbea0a..134c05757a3b 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -1480,10 +1480,13 @@ static int flexcan_chip_start(struct net_device *dev)
 
 	flexcan_set_bittiming(dev);
 
+	/* set freeze, halt */
+	err = flexcan_chip_freeze(priv);
+	if (err)
+		goto out_chip_disable;
+
 	/* MCR
 	 *
-	 * enable freeze
-	 * halt now
 	 * only supervisor access
 	 * enable warning int
 	 * enable individual RX masking
@@ -1492,9 +1495,8 @@ static int flexcan_chip_start(struct net_device *dev)
 	 */
 	reg_mcr = priv->read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
-	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV |
-		FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_IRMQ | FLEXCAN_MCR_IDAM_C |
-		FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
+	reg_mcr |= FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_IRMQ |
+		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
 
 	/* MCR
 	 *
-- 
2.30.1



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

* [net 4/6] can: mcp251xfd: revert "can: mcp251xfd: add BQL support"
  2021-03-01 11:20 pull-request: can 2021-03-01 Marc Kleine-Budde
                   ` (2 preceding siblings ...)
  2021-03-01 11:20 ` [net 3/6] can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode Marc Kleine-Budde
@ 2021-03-01 11:20 ` Marc Kleine-Budde
  2021-03-01 11:20 ` [net 5/6] can: skb: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership Marc Kleine-Budde
  2021-03-01 11:21 ` [net 6/6] can: tcan4x5x: tcan4x5x_init(): fix initialization - clear MRAM before entering Normal Mode Marc Kleine-Budde
  5 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-03-01 11:20 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde,
	Manivannan Sadhasivam, Thomas Kopp

In the following 4 patches

| 99842c9685ab can: dev: can_rx_offload_get_echo_skb(): extend to return can frame length
| 9420e1d495e2 can: dev: can_get_echo_skb(): extend to return can frame length
| 1dcb6e57db83 can: dev: can_put_echo_skb(): extend to handle frame_len
| f0ef72febc9a can: dev: extend struct can_skb_priv to hold CAN frame length

the CAN echo SKB support was extended to hold the CAN frame
length (which is the length of the CAN frame on the wire). It is meant
as a helper for BQL support, to avoid the re-calculation of the frame
length before sending it and on TX-completion.

However if the CAN frame is send without the request to be looped back
the SKB is discarded in can_put_echo_skb() and the subsequent
can_get_echo_skb() and can_rx_offload_get_echo_skb() return 0 for the
CAN frame length. This results in BQL stalling the TX queue after a
few packages.

Until the BQL helpers can_get_echo_skb() and
can_rx_offload_get_echo_skb() are fixed, revert the BQL support for
the mcp251xfd driver.

This reverts commit 4162e18e949ba520d5116ac0323500355479a00e.

Fixes: 4162e18e949b ("can: mcp251xfd: add BQL support")
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Thomas Kopp <thomas.kopp@microchip.com>
Link: https://lore.kernel.org/r/20210228083347.28580-1-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 .../net/can/spi/mcp251xfd/mcp251xfd-core.c    | 21 ++++---------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index 3c5b92911d46..799e9d5d3481 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -335,8 +335,6 @@ static void mcp251xfd_ring_init(struct mcp251xfd_priv *priv)
 	u8 len;
 	int i, j;
 
-	netdev_reset_queue(priv->ndev);
-
 	/* TEF */
 	tef_ring = priv->tef;
 	tef_ring->head = 0;
@@ -1249,8 +1247,7 @@ mcp251xfd_handle_tefif_recover(const struct mcp251xfd_priv *priv, const u32 seq)
 
 static int
 mcp251xfd_handle_tefif_one(struct mcp251xfd_priv *priv,
-			   const struct mcp251xfd_hw_tef_obj *hw_tef_obj,
-			   unsigned int *frame_len_ptr)
+			   const struct mcp251xfd_hw_tef_obj *hw_tef_obj)
 {
 	struct net_device_stats *stats = &priv->ndev->stats;
 	u32 seq, seq_masked, tef_tail_masked;
@@ -1272,8 +1269,7 @@ mcp251xfd_handle_tefif_one(struct mcp251xfd_priv *priv,
 	stats->tx_bytes +=
 		can_rx_offload_get_echo_skb(&priv->offload,
 					    mcp251xfd_get_tef_tail(priv),
-					    hw_tef_obj->ts,
-					    frame_len_ptr);
+					    hw_tef_obj->ts, NULL);
 	stats->tx_packets++;
 	priv->tef->tail++;
 
@@ -1331,7 +1327,6 @@ mcp251xfd_tef_obj_read(const struct mcp251xfd_priv *priv,
 static int mcp251xfd_handle_tefif(struct mcp251xfd_priv *priv)
 {
 	struct mcp251xfd_hw_tef_obj hw_tef_obj[MCP251XFD_TX_OBJ_NUM_MAX];
-	unsigned int total_frame_len = 0;
 	u8 tef_tail, len, l;
 	int err, i;
 
@@ -1353,9 +1348,7 @@ static int mcp251xfd_handle_tefif(struct mcp251xfd_priv *priv)
 	}
 
 	for (i = 0; i < len; i++) {
-		unsigned int frame_len;
-
-		err = mcp251xfd_handle_tefif_one(priv, &hw_tef_obj[i], &frame_len);
+		err = mcp251xfd_handle_tefif_one(priv, &hw_tef_obj[i]);
 		/* -EAGAIN means the Sequence Number in the TEF
 		 * doesn't match our tef_tail. This can happen if we
 		 * read the TEF objects too early. Leave loop let the
@@ -1365,8 +1358,6 @@ static int mcp251xfd_handle_tefif(struct mcp251xfd_priv *priv)
 			goto out_netif_wake_queue;
 		if (err)
 			return err;
-
-		total_frame_len += frame_len;
 	}
 
  out_netif_wake_queue:
@@ -1397,7 +1388,6 @@ static int mcp251xfd_handle_tefif(struct mcp251xfd_priv *priv)
 			return err;
 
 		tx_ring->tail += len;
-		netdev_completed_queue(priv->ndev, len, total_frame_len);
 
 		err = mcp251xfd_check_tef_tail(priv);
 		if (err)
@@ -2443,7 +2433,6 @@ static netdev_tx_t mcp251xfd_start_xmit(struct sk_buff *skb,
 	struct mcp251xfd_priv *priv = netdev_priv(ndev);
 	struct mcp251xfd_tx_ring *tx_ring = priv->tx;
 	struct mcp251xfd_tx_obj *tx_obj;
-	unsigned int frame_len;
 	u8 tx_head;
 	int err;
 
@@ -2462,9 +2451,7 @@ static netdev_tx_t mcp251xfd_start_xmit(struct sk_buff *skb,
 	if (mcp251xfd_get_tx_free(tx_ring) == 0)
 		netif_stop_queue(ndev);
 
-	frame_len = can_skb_get_frame_len(skb);
-	can_put_echo_skb(skb, ndev, tx_head, frame_len);
-	netdev_sent_queue(priv->ndev, frame_len);
+	can_put_echo_skb(skb, ndev, tx_head, 0);
 
 	err = mcp251xfd_tx_obj_write(priv, tx_obj);
 	if (err)
-- 
2.30.1



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

* [net 5/6] can: skb: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership
  2021-03-01 11:20 pull-request: can 2021-03-01 Marc Kleine-Budde
                   ` (3 preceding siblings ...)
  2021-03-01 11:20 ` [net 4/6] can: mcp251xfd: revert "can: mcp251xfd: add BQL support" Marc Kleine-Budde
@ 2021-03-01 11:20 ` Marc Kleine-Budde
  2021-03-01 11:21 ` [net 6/6] can: tcan4x5x: tcan4x5x_init(): fix initialization - clear MRAM before entering Normal Mode Marc Kleine-Budde
  5 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-03-01 11:20 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Oleksij Rempel, Oliver Hartkopp,
	Andre Naujoks, Eric Dumazet, Marc Kleine-Budde

From: Oleksij Rempel <o.rempel@pengutronix.de>

There are two ref count variables controlling the free()ing of a socket:
- struct sock::sk_refcnt - which is changed by sock_hold()/sock_put()
- struct sock::sk_wmem_alloc - which accounts the memory allocated by
  the skbs in the send path.

In case there are still TX skbs on the fly and the socket() is closed,
the struct sock::sk_refcnt reaches 0. In the TX-path the CAN stack
clones an "echo" skb, calls sock_hold() on the original socket and
references it. This produces the following back trace:

| WARNING: CPU: 0 PID: 280 at lib/refcount.c:25 refcount_warn_saturate+0x114/0x134
| refcount_t: addition on 0; use-after-free.
| Modules linked in: coda_vpu(E) v4l2_jpeg(E) videobuf2_vmalloc(E) imx_vdoa(E)
| CPU: 0 PID: 280 Comm: test_can.sh Tainted: G            E     5.11.0-04577-gf8ff6603c617 #203
| Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
| Backtrace:
| [<80bafea4>] (dump_backtrace) from [<80bb0280>] (show_stack+0x20/0x24) r7:00000000 r6:600f0113 r5:00000000 r4:81441220
| [<80bb0260>] (show_stack) from [<80bb593c>] (dump_stack+0xa0/0xc8)
| [<80bb589c>] (dump_stack) from [<8012b268>] (__warn+0xd4/0x114) r9:00000019 r8:80f4a8c2 r7:83e4150c r6:00000000 r5:00000009 r4:80528f90
| [<8012b194>] (__warn) from [<80bb09c4>] (warn_slowpath_fmt+0x88/0xc8) r9:83f26400 r8:80f4a8d1 r7:00000009 r6:80528f90 r5:00000019 r4:80f4a8c2
| [<80bb0940>] (warn_slowpath_fmt) from [<80528f90>] (refcount_warn_saturate+0x114/0x134) r8:00000000 r7:00000000 r6:82b44000 r5:834e5600 r4:83f4d540
| [<80528e7c>] (refcount_warn_saturate) from [<8079a4c8>] (__refcount_add.constprop.0+0x4c/0x50)
| [<8079a47c>] (__refcount_add.constprop.0) from [<8079a57c>] (can_put_echo_skb+0xb0/0x13c)
| [<8079a4cc>] (can_put_echo_skb) from [<8079ba98>] (flexcan_start_xmit+0x1c4/0x230) r9:00000010 r8:83f48610 r7:0fdc0000 r6:0c080000 r5:82b44000 r4:834e5600
| [<8079b8d4>] (flexcan_start_xmit) from [<80969078>] (netdev_start_xmit+0x44/0x70) r9:814c0ba0 r8:80c8790c r7:00000000 r6:834e5600 r5:82b44000 r4:82ab1f00
| [<80969034>] (netdev_start_xmit) from [<809725a4>] (dev_hard_start_xmit+0x19c/0x318) r9:814c0ba0 r8:00000000 r7:82ab1f00 r6:82b44000 r5:00000000 r4:834e5600
| [<80972408>] (dev_hard_start_xmit) from [<809c6584>] (sch_direct_xmit+0xcc/0x264) r10:834e5600 r9:00000000 r8:00000000 r7:82b44000 r6:82ab1f00 r5:834e5600 r4:83f27400
| [<809c64b8>] (sch_direct_xmit) from [<809c6c0c>] (__qdisc_run+0x4f0/0x534)

To fix this problem, only set skb ownership to sockets which have still
a ref count > 0.

Fixes: 0ae89beb283a ("can: add destructor for self generated skbs")
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Andre Naujoks <nautsch2@gmail.com>
Link: https://lore.kernel.org/r/20210226092456.27126-1-o.rempel@pengutronix.de
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 include/linux/can/skb.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 685f34cfba20..d438eb058069 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -65,8 +65,12 @@ static inline void can_skb_reserve(struct sk_buff *skb)
 
 static inline void can_skb_set_owner(struct sk_buff *skb, struct sock *sk)
 {
-	if (sk) {
-		sock_hold(sk);
+	/* If the socket has already been closed by user space, the
+	 * refcount may already be 0 (and the socket will be freed
+	 * after the last TX skb has been freed). So only increase
+	 * socket refcount if the refcount is > 0.
+	 */
+	if (sk && refcount_inc_not_zero(&sk->sk_refcnt)) {
 		skb->destructor = sock_efree;
 		skb->sk = sk;
 	}
-- 
2.30.1



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

* [net 6/6] can: tcan4x5x: tcan4x5x_init(): fix initialization - clear MRAM before entering Normal Mode
  2021-03-01 11:20 pull-request: can 2021-03-01 Marc Kleine-Budde
                   ` (4 preceding siblings ...)
  2021-03-01 11:20 ` [net 5/6] can: skb: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership Marc Kleine-Budde
@ 2021-03-01 11:21 ` Marc Kleine-Budde
  5 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-03-01 11:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Torin Cooper-Bennun, Marc Kleine-Budde

From: Torin Cooper-Bennun <torin@maxiluxsystems.com>

This patch prevents a potentially destructive race condition. The
device is fully operational on the bus after entering Normal Mode, so
zeroing the MRAM after entering this mode may lead to loss of
information, e.g. new received messages.

This patch fixes the problem by first initializing the MRAM, then
bringing the device into Normale Mode.

Fixes: 5443c226ba91 ("can: tcan4x5x: Add tcan4x5x driver to the kernel")
Link: https://lore.kernel.org/r/20210226163440.313628-1-torin@maxiluxsystems.com
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Torin Cooper-Bennun <torin@maxiluxsystems.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/tcan4x5x-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index b7caec769ddb..4147cecfbbd6 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -237,14 +237,14 @@ static int tcan4x5x_init(struct m_can_classdev *cdev)
 	if (ret)
 		return ret;
 
+	/* Zero out the MCAN buffers */
+	m_can_init_ram(cdev);
+
 	ret = regmap_update_bits(tcan4x5x->regmap, TCAN4X5X_CONFIG,
 				 TCAN4X5X_MODE_SEL_MASK, TCAN4X5X_MODE_NORMAL);
 	if (ret)
 		return ret;
 
-	/* Zero out the MCAN buffers */
-	m_can_init_ram(cdev);
-
 	return ret;
 }
 
-- 
2.30.1



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

* [BUG] Re: [net 3/6] can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode
  2021-03-01 11:20 ` [net 3/6] can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode Marc Kleine-Budde
@ 2021-03-16 16:00   ` Ahmad Fatoum
  2021-03-17  8:18     ` Marc Kleine-Budde
  0 siblings, 1 reply; 11+ messages in thread
From: Ahmad Fatoum @ 2021-03-16 16:00 UTC (permalink / raw)
  To: Marc Kleine-Budde, netdev; +Cc: Joakim Zhang, linux-can, kernel, kuba, davem

Hello Joakim, Marc,

On 01.03.21 12:20, Marc Kleine-Budde wrote:
> From: Joakim Zhang <qiangqing.zhang@nxp.com>
> 
> Invoke flexcan_chip_freeze() to enter freeze mode, since need poll
> freeze mode acknowledge.
> 
> Fixes: e955cead03117 ("CAN: Add Flexcan CAN controller driver")
> Link: https://lore.kernel.org/r/20210218110037.16591-4-qiangqing.zhang@nxp.com
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/can/flexcan.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index e66a51dbea0a..134c05757a3b 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -1480,10 +1480,13 @@ static int flexcan_chip_start(struct net_device *dev)
>  
>  	flexcan_set_bittiming(dev);
>  
> +	/* set freeze, halt */
> +	err = flexcan_chip_freeze(priv);
> +	if (err)
> +		goto out_chip_disable;

With v5.12-rc3, both my FlexCAN controllers on an i.MX6Q now divide by zero
on probe because priv->can.bittiming.bitrate == 0 inside of flexcan_chip_freeze.

Reverting this patch fixes it.

> +
>  	/* MCR
>  	 *
> -	 * enable freeze
> -	 * halt now
>  	 * only supervisor access
>  	 * enable warning int
>  	 * enable individual RX masking
> @@ -1492,9 +1495,8 @@ static int flexcan_chip_start(struct net_device *dev)
>  	 */
>  	reg_mcr = priv->read(&regs->mcr);
>  	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
> -	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV |
> -		FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_IRMQ | FLEXCAN_MCR_IDAM_C |
> -		FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
> +	reg_mcr |= FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_IRMQ |
> +		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
>  
>  	/* MCR
>  	 *
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [BUG] Re: [net 3/6] can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode
  2021-03-16 16:00   ` [BUG] " Ahmad Fatoum
@ 2021-03-17  8:18     ` Marc Kleine-Budde
  2021-03-17 11:49       ` Marc Kleine-Budde
  0 siblings, 1 reply; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-03-17  8:18 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: netdev, Joakim Zhang, linux-can, kernel, kuba, davem

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On 16.03.2021 17:00:25, Ahmad Fatoum wrote:
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -1480,10 +1480,13 @@ static int flexcan_chip_start(struct net_device *dev)
> >  
> >  	flexcan_set_bittiming(dev);
> >  
> > +	/* set freeze, halt */
> > +	err = flexcan_chip_freeze(priv);
> > +	if (err)
> > +		goto out_chip_disable;
> 
> With v5.12-rc3, both my FlexCAN controllers on an i.MX6Q now divide by zero
> on probe because priv->can.bittiming.bitrate == 0 inside of flexcan_chip_freeze.
> 
> Reverting this patch fixes it.

A fix for this in on its way to net/master:

https://lore.kernel.org/linux-can/20210316082104.4027260-6-mkl@pengutronix.de/

regards,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [BUG] Re: [net 3/6] can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode
  2021-03-17  8:18     ` Marc Kleine-Budde
@ 2021-03-17 11:49       ` Marc Kleine-Budde
  2021-03-18  9:23         ` Ahmad Fatoum
  0 siblings, 1 reply; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-03-17 11:49 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: netdev, Joakim Zhang, linux-can, kernel, kuba, davem

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

On 17.03.2021 09:18:31, Marc Kleine-Budde wrote:
> A fix for this in on its way to net/master:
> 
> https://lore.kernel.org/linux-can/20210316082104.4027260-6-mkl@pengutronix.de/

It's already in net/master.

regards,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [BUG] Re: [net 3/6] can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode
  2021-03-17 11:49       ` Marc Kleine-Budde
@ 2021-03-18  9:23         ` Ahmad Fatoum
  0 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2021-03-18  9:23 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: netdev, Joakim Zhang, linux-can, kernel, kuba, davem

On 17.03.21 12:49, Marc Kleine-Budde wrote:
> On 17.03.2021 09:18:31, Marc Kleine-Budde wrote:
>> A fix for this in on its way to net/master:
>>
>> https://lore.kernel.org/linux-can/20210316082104.4027260-6-mkl@pengutronix.de/
> 
> It's already in net/master.

Cherry-picked and works for me.

Thanks,
Ahmad

> 
> regards,
> Marc
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

end of thread, other threads:[~2021-03-18  9:24 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01 11:20 pull-request: can 2021-03-01 Marc Kleine-Budde
2021-03-01 11:20 ` [net 1/6] can: flexcan: assert FRZ bit in flexcan_chip_freeze() Marc Kleine-Budde
2021-03-01 11:20 ` [net 2/6] can: flexcan: enable RX FIFO after FRZ/HALT valid Marc Kleine-Budde
2021-03-01 11:20 ` [net 3/6] can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode Marc Kleine-Budde
2021-03-16 16:00   ` [BUG] " Ahmad Fatoum
2021-03-17  8:18     ` Marc Kleine-Budde
2021-03-17 11:49       ` Marc Kleine-Budde
2021-03-18  9:23         ` Ahmad Fatoum
2021-03-01 11:20 ` [net 4/6] can: mcp251xfd: revert "can: mcp251xfd: add BQL support" Marc Kleine-Budde
2021-03-01 11:20 ` [net 5/6] can: skb: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership Marc Kleine-Budde
2021-03-01 11:21 ` [net 6/6] can: tcan4x5x: tcan4x5x_init(): fix initialization - clear MRAM before entering Normal Mode Marc Kleine-Budde

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.