From d9c2ba65e651467de739324d978b04ed8729f483 Mon Sep 17 00:00:00 2001 From: Lukas Magel Date: Sun, 27 Aug 2023 09:22:05 +0000 Subject: [PATCH 1/6] can: isotp: isotp_sendmsg(): fix TX state detection and wait behavior 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 Closes: https://lore.kernel.org/linux-can/11328958-453f-447f-9af8-3b5824dfb041@munic.io/ Signed-off-by: Lukas Magel Reviewed-by: Oliver Hartkopp 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 --- 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 */ From 1f223208ebdef84f21c15e9958c005a93c871aa2 Mon Sep 17 00:00:00 2001 From: John Watts Date: Wed, 6 Sep 2023 09:13:43 +1000 Subject: [PATCH 2/6] can: sun4i_can: Only show Kconfig if ARCH_SUNXI is set 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 Closes: https://lore.kernel.org/linux-sunxi/CAMuHMdV2m54UAH0X2dG7stEg=grFihrdsz4+o7=_DpBMhjTbkw@mail.gmail.com/ Signed-off-by: John Watts Reviewed-by: Geert Uytterhoeven Link: https://lore.kernel.org/all/20230905231342.2042759-2-contact@jookia.org Signed-off-by: Marc Kleine-Budde --- 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. From 23ed2be5404da7cee6a519fa69bf22d0f69da4e4 Mon Sep 17 00:00:00 2001 From: Haibo Chen Date: Wed, 26 Jul 2023 19:24:57 +0800 Subject: [PATCH 3/6] arm64: dts: imx93: add the Flex-CAN stop mode by GPR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Link: https://lore.kernel.org/all/20230726112458.3524165-1-haibo.chen@nxp.com Signed-off-by: Marc Kleine-Budde --- 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 @@ #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 @@ 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 @@ 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"; }; From b5efb4e6fbb06da928526eca746f3de243c12ab2 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Mon, 2 Oct 2023 18:02:06 +0200 Subject: [PATCH 4/6] can: sja1000: Always restart the Tx queue after an overrun 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 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 --- 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; } From 63ead535570f13d0e06fda3f2d020c8f5394e998 Mon Sep 17 00:00:00 2001 From: Haibo Chen Date: Wed, 26 Jul 2023 19:24:58 +0800 Subject: [PATCH 5/6] can: flexcan: remove the auto stop mode for IMX93 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 Link: https://lore.kernel.org/all/20230726112458.3524165-2-haibo.chen@nxp.com Signed-off-by: Marc Kleine-Budde --- 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 */ From a9967c9ad290c086e1445b660263375985dffb3a Mon Sep 17 00:00:00 2001 From: Markus Schneider-Pargmann Date: Tue, 19 Sep 2023 11:54:01 +0200 Subject: [PATCH 6/6] can: tcan4x5x: Fix id2_register for tcan4553 Fix id2_register content for tcan4553. This slipped through my testing. Reported-by: Sean Anderson 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 Reviewed-by: Simon Horman Link: https://lore.kernel.org/all/20230919095401.1312259-1-msp@baylibre.com Signed-off-by: Marc Kleine-Budde --- 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] = {