* [PATCH net 1/7] can: j1939: Fix UAF in j1939_sk_match_filter during setsockopt(SO_J1939_FILTER)
2023-10-05 9:46 [PATCH net 0/7] pull-request: can 2023-10-05 Marc Kleine-Budde
@ 2023-10-05 9:46 ` Marc Kleine-Budde
2023-10-05 16:44 ` Jakub Kicinski
2023-10-05 9:46 ` [PATCH net 2/7] can: isotp: isotp_sendmsg(): fix TX state detection and wait behavior Marc Kleine-Budde
` (5 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-10-05 9:46 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Oleksij Rempel, Sili Luo, stable,
Marc Kleine-Budde
From: Oleksij Rempel <o.rempel@pengutronix.de>
Lock jsk->sk to prevent UAF when setsockopt(..., SO_J1939_FILTER, ...)
modifies jsk->filters while receiving packets.
Following trace was seen on affected system:
==================================================================
BUG: KASAN: slab-use-after-free in j1939_sk_recv_match_one+0x1af/0x2d0 [can_j1939]
Read of size 4 at addr ffff888012144014 by task j1939/350
CPU: 0 PID: 350 Comm: j1939 Tainted: G W OE 6.5.0-rc5 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
Call Trace:
print_report+0xd3/0x620
? kasan_complete_mode_report_info+0x7d/0x200
? j1939_sk_recv_match_one+0x1af/0x2d0 [can_j1939]
kasan_report+0xc2/0x100
? j1939_sk_recv_match_one+0x1af/0x2d0 [can_j1939]
__asan_load4+0x84/0xb0
j1939_sk_recv_match_one+0x1af/0x2d0 [can_j1939]
j1939_sk_recv+0x20b/0x320 [can_j1939]
? __kasan_check_write+0x18/0x20
? __pfx_j1939_sk_recv+0x10/0x10 [can_j1939]
? j1939_simple_recv+0x69/0x280 [can_j1939]
? j1939_ac_recv+0x5e/0x310 [can_j1939]
j1939_can_recv+0x43f/0x580 [can_j1939]
? __pfx_j1939_can_recv+0x10/0x10 [can_j1939]
? raw_rcv+0x42/0x3c0 [can_raw]
? __pfx_j1939_can_recv+0x10/0x10 [can_j1939]
can_rcv_filter+0x11f/0x350 [can]
can_receive+0x12f/0x190 [can]
? __pfx_can_rcv+0x10/0x10 [can]
can_rcv+0xdd/0x130 [can]
? __pfx_can_rcv+0x10/0x10 [can]
__netif_receive_skb_one_core+0x13d/0x150
? __pfx___netif_receive_skb_one_core+0x10/0x10
? __kasan_check_write+0x18/0x20
? _raw_spin_lock_irq+0x8c/0xe0
__netif_receive_skb+0x23/0xb0
process_backlog+0x107/0x260
__napi_poll+0x69/0x310
net_rx_action+0x2a1/0x580
? __pfx_net_rx_action+0x10/0x10
? __pfx__raw_spin_lock+0x10/0x10
? handle_irq_event+0x7d/0xa0
__do_softirq+0xf3/0x3f8
do_softirq+0x53/0x80
</IRQ>
<TASK>
__local_bh_enable_ip+0x6e/0x70
netif_rx+0x16b/0x180
can_send+0x32b/0x520 [can]
? __pfx_can_send+0x10/0x10 [can]
? __check_object_size+0x299/0x410
raw_sendmsg+0x572/0x6d0 [can_raw]
? __pfx_raw_sendmsg+0x10/0x10 [can_raw]
? apparmor_socket_sendmsg+0x2f/0x40
? __pfx_raw_sendmsg+0x10/0x10 [can_raw]
sock_sendmsg+0xef/0x100
sock_write_iter+0x162/0x220
? __pfx_sock_write_iter+0x10/0x10
? __rtnl_unlock+0x47/0x80
? security_file_permission+0x54/0x320
vfs_write+0x6ba/0x750
? __pfx_vfs_write+0x10/0x10
? __fget_light+0x1ca/0x1f0
? __rcu_read_unlock+0x5b/0x280
ksys_write+0x143/0x170
? __pfx_ksys_write+0x10/0x10
? __kasan_check_read+0x15/0x20
? fpregs_assert_state_consistent+0x62/0x70
__x64_sys_write+0x47/0x60
do_syscall_64+0x60/0x90
? do_syscall_64+0x6d/0x90
? irqentry_exit+0x3f/0x50
? exc_page_fault+0x79/0xf0
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
Allocated by task 348:
kasan_save_stack+0x2a/0x50
kasan_set_track+0x29/0x40
kasan_save_alloc_info+0x1f/0x30
__kasan_kmalloc+0xb5/0xc0
__kmalloc_node_track_caller+0x67/0x160
j1939_sk_setsockopt+0x284/0x450 [can_j1939]
__sys_setsockopt+0x15c/0x2f0
__x64_sys_setsockopt+0x6b/0x80
do_syscall_64+0x60/0x90
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
Freed by task 349:
kasan_save_stack+0x2a/0x50
kasan_set_track+0x29/0x40
kasan_save_free_info+0x2f/0x50
__kasan_slab_free+0x12e/0x1c0
__kmem_cache_free+0x1b9/0x380
kfree+0x7a/0x120
j1939_sk_setsockopt+0x3b2/0x450 [can_j1939]
__sys_setsockopt+0x15c/0x2f0
__x64_sys_setsockopt+0x6b/0x80
do_syscall_64+0x60/0x90
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
Fixes: 9d71dd0c70099 ("can: add support of SAE J1939 protocol")
Reported-by: Sili Luo <rootlab@huawei.com>
Suggested-by: Sili Luo <rootlab@huawei.com>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: stable@vger.kernel.org
Tested-by: Sili Luo <rootlab@huawei.com>
Link: https://lore.kernel.org/all/20230927161456.82772-1-o.rempel@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/j1939/socket.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index b28c976f52a0..2ce24bf78c72 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -262,12 +262,17 @@ static bool j1939_sk_match_dst(struct j1939_sock *jsk,
static bool j1939_sk_match_filter(struct j1939_sock *jsk,
const struct j1939_sk_buff_cb *skcb)
{
- const struct j1939_filter *f = jsk->filters;
- int nfilter = jsk->nfilters;
+ const struct j1939_filter *f;
+ int nfilter;
+
+ lock_sock(&jsk->sk);
+
+ f = jsk->filters;
+ nfilter = jsk->nfilters;
if (!nfilter)
/* receive all when no filters are assigned */
- return true;
+ goto filter_match_found;
for (; nfilter; ++f, --nfilter) {
if ((skcb->addr.pgn & f->pgn_mask) != f->pgn)
@@ -276,9 +281,15 @@ static bool j1939_sk_match_filter(struct j1939_sock *jsk,
continue;
if ((skcb->addr.src_name & f->name_mask) != f->name)
continue;
- return true;
+ goto filter_match_found;
}
+
+ release_sock(&jsk->sk);
return false;
+
+filter_match_found:
+ release_sock(&jsk->sk);
+ return true;
}
static bool j1939_sk_recv_match_one(struct j1939_sock *jsk,
base-commit: d0f95894fda7d4f895b29c1097f92d7fee278cb2
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net 1/7] can: j1939: Fix UAF in j1939_sk_match_filter during setsockopt(SO_J1939_FILTER)
2023-10-05 9:46 ` [PATCH net 1/7] can: j1939: Fix UAF in j1939_sk_match_filter during setsockopt(SO_J1939_FILTER) Marc Kleine-Budde
@ 2023-10-05 16:44 ` Jakub Kicinski
2023-10-06 10:33 ` Oleksij Rempel
0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2023-10-05 16:44 UTC (permalink / raw)
To: Marc Kleine-Budde, Oleksij Rempel
Cc: netdev, davem, linux-can, kernel, Sili Luo, stable
On Thu, 5 Oct 2023 11:46:33 +0200 Marc Kleine-Budde wrote:
> Lock jsk->sk to prevent UAF when setsockopt(..., SO_J1939_FILTER, ...)
> modifies jsk->filters while receiving packets.
Doesn't it potentially introduce sleep in atomic?
j1939_sk_recv_match()
spin_lock_bh(&priv->j1939_socks_lock);
j1939_sk_recv_match_one()
j1939_sk_match_filter()
lock_sock()
sleep
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net 1/7] can: j1939: Fix UAF in j1939_sk_match_filter during setsockopt(SO_J1939_FILTER)
2023-10-05 16:44 ` Jakub Kicinski
@ 2023-10-06 10:33 ` Oleksij Rempel
0 siblings, 0 replies; 10+ messages in thread
From: Oleksij Rempel @ 2023-10-06 10:33 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Marc Kleine-Budde, netdev, stable, linux-can, kernel, Sili Luo,
davem
Hi Jakub,
On Thu, Oct 05, 2023 at 09:44:21AM -0700, Jakub Kicinski wrote:
> On Thu, 5 Oct 2023 11:46:33 +0200 Marc Kleine-Budde wrote:
> > Lock jsk->sk to prevent UAF when setsockopt(..., SO_J1939_FILTER, ...)
> > modifies jsk->filters while receiving packets.
>
> Doesn't it potentially introduce sleep in atomic?
>
> j1939_sk_recv_match()
> spin_lock_bh(&priv->j1939_socks_lock);
> j1939_sk_recv_match_one()
> j1939_sk_match_filter()
> lock_sock()
> sleep
Good point! Thank you for the review.
@Sili Luo, can you please take a look at this?
Regards,
Oleksij
--
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] 10+ messages in thread
* [PATCH net 2/7] can: isotp: isotp_sendmsg(): fix TX state detection and wait behavior
2023-10-05 9:46 [PATCH net 0/7] pull-request: can 2023-10-05 Marc Kleine-Budde
2023-10-05 9:46 ` [PATCH net 1/7] can: j1939: Fix UAF in j1939_sk_match_filter during setsockopt(SO_J1939_FILTER) Marc Kleine-Budde
@ 2023-10-05 9:46 ` Marc Kleine-Budde
2023-10-05 9:46 ` [PATCH net 3/7] can: sun4i_can: Only show Kconfig if ARCH_SUNXI is set Marc Kleine-Budde
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-10-05 9:46 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Lukas Magel, Maxime Jayat,
Oliver Hartkopp, Marc Kleine-Budde
From: Lukas Magel <lukas.magel@posteo.net>
With patch [1], isotp_poll was updated to also queue the poller in the
so->wait queue, which is used for send state changes. Since the queue
now also contains polling tasks that are not interested in sending, the
queue fill state can no longer be used as an indication of send
readiness. As a consequence, nonblocking writes can lead to a race and
lock-up of the socket if there is a second task polling the socket in
parallel.
With this patch, isotp_sendmsg does not consult wq_has_sleepers but
instead tries to atomically set so->tx.state and waits on so->wait if it
is unable to do so. This behavior is in alignment with isotp_poll, which
also checks so->tx.state to determine send readiness.
V2:
- Revert direct exit to goto err_event_drop
[1] https://lore.kernel.org/all/20230331125511.372783-1-michal.sojka@cvut.cz
Reported-by: Maxime Jayat <maxime.jayat@mobile-devices.fr>
Closes: https://lore.kernel.org/linux-can/11328958-453f-447f-9af8-3b5824dfb041@munic.io/
Signed-off-by: Lukas Magel <lukas.magel@posteo.net>
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
Fixes: 79e19fa79cb5 ("can: isotp: isotp_ops: fix poll() to not report false EPOLLOUT events")
Link: https://github.com/pylessard/python-udsoncan/issues/178#issuecomment-1743786590
Link: https://lore.kernel.org/all/20230827092205.7908-1-lukas.magel@posteo.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/isotp.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/net/can/isotp.c b/net/can/isotp.c
index f02b5d3e4733..d1c6f206f429 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -948,21 +948,18 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (!so->bound || so->tx.state == ISOTP_SHUTDOWN)
return -EADDRNOTAVAIL;
-wait_free_buffer:
- /* we do not support multiple buffers - for now */
- if (wq_has_sleeper(&so->wait) && (msg->msg_flags & MSG_DONTWAIT))
- return -EAGAIN;
+ while (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
+ /* we do not support multiple buffers - for now */
+ if (msg->msg_flags & MSG_DONTWAIT)
+ return -EAGAIN;
- /* wait for complete transmission of current pdu */
- err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
- if (err)
- goto err_event_drop;
-
- if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
if (so->tx.state == ISOTP_SHUTDOWN)
return -EADDRNOTAVAIL;
- goto wait_free_buffer;
+ /* wait for complete transmission of current pdu */
+ err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
+ if (err)
+ goto err_event_drop;
}
/* PDU size > default => try max_pdu_size */
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net 3/7] can: sun4i_can: Only show Kconfig if ARCH_SUNXI is set
2023-10-05 9:46 [PATCH net 0/7] pull-request: can 2023-10-05 Marc Kleine-Budde
2023-10-05 9:46 ` [PATCH net 1/7] can: j1939: Fix UAF in j1939_sk_match_filter during setsockopt(SO_J1939_FILTER) Marc Kleine-Budde
2023-10-05 9:46 ` [PATCH net 2/7] can: isotp: isotp_sendmsg(): fix TX state detection and wait behavior Marc Kleine-Budde
@ 2023-10-05 9:46 ` Marc Kleine-Budde
2023-10-05 9:46 ` [PATCH net 4/7] can: sja1000: Always restart the Tx queue after an overrun Marc Kleine-Budde
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-10-05 9:46 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, John Watts, Geert Uytterhoeven,
Geert Uytterhoeven, Marc Kleine-Budde
From: John Watts <contact@jookia.org>
When adding the RISCV option I didn't gate it behind ARCH_SUNXI.
As a result this option shows up with Allwinner support isn't enabled.
Fix that by requiring ARCH_SUNXI to be set if RISCV is set.
Fixes: 8abb95250ae6 ("can: sun4i_can: Add support for the Allwinner D1")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Closes: https://lore.kernel.org/linux-sunxi/CAMuHMdV2m54UAH0X2dG7stEg=grFihrdsz4+o7=_DpBMhjTbkw@mail.gmail.com/
Signed-off-by: John Watts <contact@jookia.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/all/20230905231342.2042759-2-contact@jookia.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 649453a3c858..f8cde9f9f554 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -190,7 +190,7 @@ config CAN_SLCAN
config CAN_SUN4I
tristate "Allwinner A10 CAN controller"
- depends on MACH_SUN4I || MACH_SUN7I || RISCV || COMPILE_TEST
+ depends on MACH_SUN4I || MACH_SUN7I || (RISCV && ARCH_SUNXI) || COMPILE_TEST
help
Say Y here if you want to use CAN controller found on Allwinner
A10/A20/D1 SoCs.
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net 4/7] can: sja1000: Always restart the Tx queue after an overrun
2023-10-05 9:46 [PATCH net 0/7] pull-request: can 2023-10-05 Marc Kleine-Budde
` (2 preceding siblings ...)
2023-10-05 9:46 ` [PATCH net 3/7] can: sun4i_can: Only show Kconfig if ARCH_SUNXI is set Marc Kleine-Budde
@ 2023-10-05 9:46 ` Marc Kleine-Budde
2023-10-05 9:46 ` [PATCH net 5/7] can: tcan4x5x: Fix id2_register for tcan4553 Marc Kleine-Budde
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-10-05 9:46 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Miquel Raynal, stable,
Marc Kleine-Budde
From: Miquel Raynal <miquel.raynal@bootlin.com>
Upstream commit 717c6ec241b5 ("can: sja1000: Prevent overrun stalls with
a soft reset on Renesas SoCs") fixes an issue with Renesas own SJA1000
CAN controller reception: the Rx buffer is only 5 messages long, so when
the bus loaded (eg. a message every 50us), overrun may easily
happen. Upon an overrun situation, due to a possible internal crosstalk
situation, the controller enters a frozen state which only can be
unlocked with a soft reset (experimentally). The solution was to offload
a call to sja1000_start() in a threaded handler. This needs to happen in
process context as this operation requires to sleep. sja1000_start()
basically enters "reset mode", performs a proper software reset and
returns back into "normal mode".
Since this fix was introduced, we no longer observe any stalls in
reception. However it was sporadically observed that the transmit path
would now freeze. Further investigation blamed the fix mentioned above,
and especially the reset operation. Reproducing the reset in a loop
helped identifying what could possibly go wrong. The sja1000 is a single
Tx queue device, which leverages the netdev helpers to process one Tx
message at a time. The logic is: the queue is stopped, the message sent
to the transceiver, once properly transmitted the controller sets a
status bit which triggers an interrupt, in the interrupt handler the
transmission status is checked and the queue woken up. Unfortunately, if
an overrun happens, we might perform the soft reset precisely between
the transmission of the buffer to the transceiver and the advent of the
transmission status bit. We would then stop the transmission operation
without re-enabling the queue, leading to all further transmissions to
be ignored.
The reset interrupt can only happen while the device is "open", and
after a reset we anyway want to resume normal operations, no matter if a
packet to transmit got dropped in the process, so we shall wake up the
queue. Restarting the device and waking-up the queue is exactly what
sja1000_set_mode(CAN_MODE_START) does. In order to be consistent about
the queue state, we must acquire a lock both in the reset handler and in
the transmit path to ensure serialization of both operations. It turns
out, a lock is already held when entering the transmit path, so we can
just acquire/release it as well with the regular net helpers inside the
threaded interrupt handler and this way we should be safe. As the
reset handler might still be called after the transmission of a frame to
the transceiver but before it actually gets transmitted, we must ensure
we don't leak the skb, so we free it (the behavior is consistent, no
matter if there was an skb on the stack or not).
Fixes: 717c6ec241b5 ("can: sja1000: Prevent overrun stalls with a soft reset on Renesas SoCs")
Cc: stable@vger.kernel.org
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/all/20231002160206.190953-1-miquel.raynal@bootlin.com
[mkl: fixed call to can_free_echo_skb()]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/sja1000/sja1000.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index 0ada0e160e93..743c2eb62b87 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -392,7 +392,13 @@ static irqreturn_t sja1000_reset_interrupt(int irq, void *dev_id)
struct net_device *dev = (struct net_device *)dev_id;
netdev_dbg(dev, "performing a soft reset upon overrun\n");
- sja1000_start(dev);
+
+ netif_tx_lock(dev);
+
+ can_free_echo_skb(dev, 0, NULL);
+ sja1000_set_mode(dev, CAN_MODE_START);
+
+ netif_tx_unlock(dev);
return IRQ_HANDLED;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net 5/7] can: tcan4x5x: Fix id2_register for tcan4553
2023-10-05 9:46 [PATCH net 0/7] pull-request: can 2023-10-05 Marc Kleine-Budde
` (3 preceding siblings ...)
2023-10-05 9:46 ` [PATCH net 4/7] can: sja1000: Always restart the Tx queue after an overrun Marc Kleine-Budde
@ 2023-10-05 9:46 ` Marc Kleine-Budde
2023-10-05 9:46 ` [PATCH net 6/7] arm64: dts: imx93: add the Flex-CAN stop mode by GPR Marc Kleine-Budde
2023-10-05 9:46 ` [PATCH net 7/7] can: flexcan: remove the auto stop mode for IMX93 Marc Kleine-Budde
6 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-10-05 9:46 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Markus Schneider-Pargmann,
Sean Anderson, Simon Horman, Marc Kleine-Budde
From: Markus Schneider-Pargmann <msp@baylibre.com>
Fix id2_register content for tcan4553. This slipped through my testing.
Reported-by: Sean Anderson <sean.anderson@seco.com>
Closes: https://lore.kernel.org/lkml/a94e6fc8-4f08-7877-2ba0-29b9c2780136@seco.com/
Fixes: 142c6dc6d9d7 ("can: tcan4x5x: Add support for tcan4552/4553")
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://lore.kernel.org/all/20230919095401.1312259-1-msp@baylibre.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/m_can/tcan4x5x-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index 8a4143809d33..ae8c42f5debd 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -125,7 +125,7 @@ static const struct tcan4x5x_version_info tcan4x5x_versions[] = {
},
[TCAN4553] = {
.name = "4553",
- .id2_register = 0x32353534,
+ .id2_register = 0x33353534,
},
/* generic version with no id2_register at the end */
[TCAN4X5X] = {
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net 6/7] arm64: dts: imx93: add the Flex-CAN stop mode by GPR
2023-10-05 9:46 [PATCH net 0/7] pull-request: can 2023-10-05 Marc Kleine-Budde
` (4 preceding siblings ...)
2023-10-05 9:46 ` [PATCH net 5/7] can: tcan4x5x: Fix id2_register for tcan4553 Marc Kleine-Budde
@ 2023-10-05 9:46 ` Marc Kleine-Budde
2023-10-05 9:46 ` [PATCH net 7/7] can: flexcan: remove the auto stop mode for IMX93 Marc Kleine-Budde
6 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-10-05 9:46 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Haibo Chen, Marc Kleine-Budde
From: Haibo Chen <haibo.chen@nxp.com>
imx93 A0 chip use the internal q-channel handshake signal in LPCG
and CCM to automatically handle the Flex-CAN stop mode. But this
method meet issue when do the system PM stress test. IC can't fix
it easily. So in the new imx93 A1 chip, IC drop this method, and
involve back the old way,use the GPR method to trigger the Flex-CAN
stop mode signal. Now NXP claim to drop imx93 A0, and only support
imx93 A1. So here add the stop mode through GPR.
This patch also fix a typo for aonmix_ns_gpr.
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Link: https://lore.kernel.org/all/20230726112458.3524165-1-haibo.chen@nxp.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
arch/arm64/boot/dts/freescale/imx93.dtsi | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
index 6f85a05ee7e1..dcf6e4846ac9 100644
--- a/arch/arm64/boot/dts/freescale/imx93.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
@@ -185,7 +185,7 @@ aips1: bus@44000000 {
#size-cells = <1>;
ranges;
- anomix_ns_gpr: syscon@44210000 {
+ aonmix_ns_gpr: syscon@44210000 {
compatible = "fsl,imx93-aonmix-ns-syscfg", "syscon";
reg = <0x44210000 0x1000>;
};
@@ -319,6 +319,7 @@ flexcan1: can@443a0000 {
assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
assigned-clock-rates = <40000000>;
fsl,clk-source = /bits/ 8 <0>;
+ fsl,stop-mode = <&aonmix_ns_gpr 0x14 0>;
status = "disabled";
};
@@ -591,6 +592,7 @@ flexcan2: can@425b0000 {
assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
assigned-clock-rates = <40000000>;
fsl,clk-source = /bits/ 8 <0>;
+ fsl,stop-mode = <&wakeupmix_gpr 0x0c 2>;
status = "disabled";
};
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net 7/7] can: flexcan: remove the auto stop mode for IMX93
2023-10-05 9:46 [PATCH net 0/7] pull-request: can 2023-10-05 Marc Kleine-Budde
` (5 preceding siblings ...)
2023-10-05 9:46 ` [PATCH net 6/7] arm64: dts: imx93: add the Flex-CAN stop mode by GPR Marc Kleine-Budde
@ 2023-10-05 9:46 ` Marc Kleine-Budde
6 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-10-05 9:46 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Haibo Chen, Marc Kleine-Budde
From: Haibo Chen <haibo.chen@nxp.com>
IMX93 A0 chip involve the internal q-channel handshake in LPCG and
CCM to automatically handle the Flex-CAN IPG STOP signal. Only after
FLEX-CAN enter stop mode then can support the self-wakeup feature.
But meet issue when do the continue system PM stress test. When config
the CAN as wakeup source, the first time after system suspend, any data
on CAN bus can wakeup the system, this is as expect. But the second time
when system suspend, data on CAN bus can't wakeup the system. If continue
this test, we find in odd time system enter suspend, CAN can wakeup the
system, but in even number system enter suspend, CAN can't wakeup the
system. IC find a bug in the auto stop mode logic, and can't fix it easily.
So for the new imx93 A1, IC drop the auto stop mode and involve the
GPR to support stop mode (used before). IC define a bit in GPR which can
trigger the IPG STOP signal to Flex-CAN, let it go into stop mode.
And NXP claim to drop IMX93 A0, and only support IMX93 A1. So this patch
remove the auto stop mode, and add flag FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR
to imx93.
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Link: https://lore.kernel.org/all/20230726112458.3524165-2-haibo.chen@nxp.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/flexcan/flexcan-core.c | 46 ++++++++------------------
drivers/net/can/flexcan/flexcan.h | 2 --
2 files changed, 13 insertions(+), 35 deletions(-)
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index add39e922b89..d15f85a40c1e 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -348,7 +348,7 @@ static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
static struct flexcan_devtype_data fsl_imx93_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_RX_MAILBOX |
- FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_AUTO_STOP_MODE |
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR |
FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SUPPORT_ECC |
FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
@@ -544,11 +544,6 @@ static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
} else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
- } else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_AUTO_STOP_MODE) {
- /* For the auto stop mode, software do nothing, hardware will cover
- * all the operation automatically after system go into low power mode.
- */
- return 0;
}
return flexcan_low_power_enter_ack(priv);
@@ -574,12 +569,6 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
priv->write(reg_mcr, ®s->mcr);
- /* For the auto stop mode, hardware will exist stop mode
- * automatically after system go out of low power mode.
- */
- if (priv->devtype_data.quirks & FLEXCAN_QUIRK_AUTO_STOP_MODE)
- return 0;
-
return flexcan_low_power_exit_ack(priv);
}
@@ -1994,13 +1983,18 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
ret = flexcan_setup_stop_mode_scfw(pdev);
else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR)
ret = flexcan_setup_stop_mode_gpr(pdev);
- else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_AUTO_STOP_MODE)
- ret = 0;
else
/* return 0 directly if doesn't support stop mode feature */
return 0;
- if (ret)
+ /* If ret is -EINVAL, this means SoC claim to support stop mode, but
+ * dts file lack the stop mode property definition. For this case,
+ * directly return 0, this will skip the wakeup capable setting and
+ * will not block the driver probe.
+ */
+ if (ret == -EINVAL)
+ return 0;
+ else if (ret)
return ret;
device_set_wakeup_capable(&pdev->dev, true);
@@ -2320,16 +2314,8 @@ static int __maybe_unused flexcan_noirq_suspend(struct device *device)
if (netif_running(dev)) {
int err;
- if (device_may_wakeup(device)) {
+ if (device_may_wakeup(device))
flexcan_enable_wakeup_irq(priv, true);
- /* For auto stop mode, need to keep the clock on before
- * system go into low power mode. After system go into
- * low power mode, hardware will config the flexcan into
- * stop mode, and gate off the clock automatically.
- */
- if (priv->devtype_data.quirks & FLEXCAN_QUIRK_AUTO_STOP_MODE)
- return 0;
- }
err = pm_runtime_force_suspend(device);
if (err)
@@ -2347,15 +2333,9 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device)
if (netif_running(dev)) {
int err;
- /* For the wakeup in auto stop mode, no need to gate on the
- * clock here, hardware will do this automatically.
- */
- if (!(device_may_wakeup(device) &&
- priv->devtype_data.quirks & FLEXCAN_QUIRK_AUTO_STOP_MODE)) {
- err = pm_runtime_force_resume(device);
- if (err)
- return err;
- }
+ err = pm_runtime_force_resume(device);
+ if (err)
+ return err;
if (device_may_wakeup(device))
flexcan_enable_wakeup_irq(priv, false);
diff --git a/drivers/net/can/flexcan/flexcan.h b/drivers/net/can/flexcan/flexcan.h
index 91402977780b..025c3417031f 100644
--- a/drivers/net/can/flexcan/flexcan.h
+++ b/drivers/net/can/flexcan/flexcan.h
@@ -68,8 +68,6 @@
#define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR BIT(15)
/* Device supports RX via FIFO */
#define FLEXCAN_QUIRK_SUPPORT_RX_FIFO BIT(16)
-/* auto enter stop mode to support wakeup */
-#define FLEXCAN_QUIRK_AUTO_STOP_MODE BIT(17)
struct flexcan_devtype_data {
u32 quirks; /* quirks needed for different IP cores */
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread