From nobody Sat Nov 30 12:40:06 2024 Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [185.203.201.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 70F5F1BA292 for ; Mon, 9 Sep 2024 14:29:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.203.201.7 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725892193; cv=none; b=ec0IZ52XvsDeK1zZK4bJhlCJ/tBnWov1GlIxCpKUzu77bn7CAkEttPfDgC4TwWLPHv7ITbSsGFgTM99iehGbuJLZuCn9kMgb//zoRheJgrUmbzvjCbydTZQtytER25uZZXNGXYjrC0Ax9XPPUUU3c9bqu9MwGryQKTnvnCEEjq0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725892193; c=relaxed/simple; bh=4eZXYXgUSsLCVtVURyK9jTQpUUPi57sHvLt8I7wOtFs=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=sy2LEzErla71CIaiZOf6YFOiDX7dz7pW2x2k4LuBJr/rBmOeSOgpTp1AJY8dGocTLzuUPmuswGffMi07XEiszlwo5pbbCgJwA0S4ojEv1fR0j5kq+rqZ6PJvGeIuZrHJ0RzrOueTh8idoLJS20/3SbJCGS1Nw5fxpF92rBoR1xs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de; spf=pass smtp.mailfrom=pengutronix.de; arc=none smtp.client-ip=185.203.201.7 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pengutronix.de Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.whiteo.stw.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1snfOm-00027C-Mj for linux-kernel@vger.kernel.org; Mon, 09 Sep 2024 16:29:48 +0200 Received: from [2a0a:edc0:0:b01:1d::7b] (helo=bjornoya.blackshift.org) by drehscheibe.grey.stw.pengutronix.de with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1snfOm-006g3T-86 for linux-kernel@vger.kernel.org; Mon, 09 Sep 2024 16:29:48 +0200 Received: from dspam.blackshift.org (localhost [127.0.0.1]) by bjornoya.blackshift.org (Postfix) with SMTP id E9FB8336B34 for ; Mon, 09 Sep 2024 14:29:47 +0000 (UTC) Received: from hardanger.blackshift.org (unknown [172.20.34.65]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by bjornoya.blackshift.org (Postfix) with ESMTPS id 1606F336B10; Mon, 09 Sep 2024 14:29:45 +0000 (UTC) Received: from [172.20.34.65] (localhost [::1]) by hardanger.blackshift.org (OpenSMTPD) with ESMTP id 2a700a89; Mon, 9 Sep 2024 14:29:44 +0000 (UTC) From: Marc Kleine-Budde Date: Mon, 09 Sep 2024 16:29:35 +0200 Subject: [PATCH can v2 1/2] can: m_can: enable NAPI before enabling interrupts Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20240909-can-m_can-fix-ifup-v2-1-2b35e624a089@pengutronix.de> References: <20240909-can-m_can-fix-ifup-v2-0-2b35e624a089@pengutronix.de> In-Reply-To: <20240909-can-m_can-fix-ifup-v2-0-2b35e624a089@pengutronix.de> To: Chandrasekar Ramakrishnan , Vincent Mailhol , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Dong Aisheng , Fengguang Wu , Varka Bhadram Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Marc Kleine-Budde , "Hamby, Jake (US)" X-Mailer: b4 0.15-dev-7be4f X-Developer-Signature: v=1; a=openpgp-sha256; l=3348; i=mkl@pengutronix.de; h=from:subject:message-id; bh=GDyc89fVl7oa+JYqYaRz+Un6+0wGGF0q6aJsOmqnDdU=; b=owEBbQGS/pANAwAKASg4oj56LbxvAcsmYgBm3wZTU4h3sWvs6kHz3rqg7zmCoQvF8jHgmRIn/ XYtpbcfTlCJATMEAAEKAB0WIQRQQLqG4LYE3Sm8Pl8oOKI+ei28bwUCZt8GUwAKCRAoOKI+ei28 bzdzB/0cO2B27Rl6A+H+zuDccyF+7crS3R1DHhLJnRv27bnR8PJBJ2dTABQ51PCragyyspOb5pk 609qx9OND2OVx75ilgrsyMDqX6otpEP6LfALPwkJCIN4of9pBIBlnv6DCWMPgCRzj+PPXZEVpBB cG0aP4ImBRtiYPjvVXUa0mkIheA1LKuwEGq6cEgpoTp2z7cNuBaL5gAIdG4TUtO3lCrQKrsisUo G3CAAcKHsGV5v7Mnlm2adG5tMxwjbY2cM9oSUIzwQV0033d5E0BSv7Q+gCcAGoGbTuwZI5mWrZo jTWJ/gstgCxPQibugqtqWtYZrvctAEoNIXuLpmVkJTNA82ET X-Developer-Key: i=mkl@pengutronix.de; a=openpgp; fpr=C1400BA0B3989E6FBC7D5B5C2B5EE211C58AEA54 X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 X-SA-Exim-Mail-From: mkl@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org From: "Hamby, Jake (US)" If any error flags are set when bringing up the CAN device, e.g. due to CAN bus traffic before initializing the device, when m_can_start() is called and interrupts are enabled, m_can_isr() is called immediately, which disables all CAN interrupts and calls napi_schedule(). Because napi_enable() isn't called until later in m_can_open(), the call to napi_schedule() never schedules the m_can_poll() callback and the device is left with interrupts disabled and can't receive any CAN packets until rebooted. This can be verified by running "cansend" from another device before setting the bitrate and calling "ip link set up can0" on the test device. Adding debug lines to m_can_isr() shows it's called with flags (IR_EP | IR_EW | IR_CRCE), which calls m_can_disable_all_interrupts() and napi_schedule(), and then m_can_poll() is never called. Move the call to napi_enable() above the call to m_can_start() to enable any initial interrupt flags to be handled by m_can_pol() so that interrupts are reenabled. Add a call to napi_disable() in the error handling section of m_can_open(), to handle the case where later functions return errors. Also, in m_can_close(), move the call to napi_disable() below the call to m_can_stop() to ensure all interrupts are handled when bringing down the device. This race condition is much less likely to occur. Tested on a Microchip SAMA7G54 MPU. The fix should be applicable to any SoC with a Bosch M_CAN controller. Not-Signed-off-by: Hamby, Jake (US) Fixes: e0d1f4816f2a ("can: m_can: add Bosch M_CAN controller support") Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index 012c3d22b01dd3d8558f2a40448770ca1da1aa1e..7754dd2d4cb110eee5b83885f53= 81aed9c67ce03 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1763,13 +1763,14 @@ static int m_can_close(struct net_device *dev) =20 netif_stop_queue(dev); =20 - if (!cdev->is_peripheral) - napi_disable(&cdev->napi); - m_can_stop(dev); m_can_clk_stop(cdev); free_irq(dev->irq, dev); =20 + /* disable NAPI after disabling interrupts */ + if (!cdev->is_peripheral) + napi_disable(&cdev->napi); + m_can_clean(dev); =20 if (cdev->is_peripheral) { @@ -2031,6 +2032,10 @@ static int m_can_open(struct net_device *dev) if (cdev->is_peripheral) can_rx_offload_enable(&cdev->offload); =20 + /* enable NAPI before enabling interrupts */ + if (!cdev->is_peripheral) + napi_enable(&cdev->napi); + /* register interrupt handler */ if (cdev->is_peripheral) { cdev->tx_wq =3D alloc_ordered_workqueue("mcan_wq", @@ -2063,9 +2068,6 @@ static int m_can_open(struct net_device *dev) if (err) goto exit_start_fail; =20 - if (!cdev->is_peripheral) - napi_enable(&cdev->napi); - netif_start_queue(dev); =20 return 0; @@ -2077,6 +2079,8 @@ static int m_can_open(struct net_device *dev) if (cdev->is_peripheral) destroy_workqueue(cdev->tx_wq); out_wq_fail: + if (!cdev->is_peripheral) + napi_disable(&cdev->napi); if (cdev->is_peripheral) can_rx_offload_disable(&cdev->offload); close_candev(dev); --=20 2.45.2 From nobody Sat Nov 30 12:40:06 2024 Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [185.203.201.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3F7D31BA28B for ; Mon, 9 Sep 2024 14:29:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.203.201.7 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725892193; cv=none; b=oE/6gouRbcFtlbSpBLbxxcOFwPxtT0SFpKDqS99IpWQqg/BUaNENojcSWK+fRMUnbksiUQAz6vQOzMCy7LBFIoLOzJWcZkANPqLsHChsMavKUNIRDJ4agUb23H5MKaCGZJ6fW7uKUTcsFlFwmnFSJ7f0Le6sZ1ZpYkIF7m70dho= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725892193; c=relaxed/simple; bh=148obTx++Y3V8ypuU2N7MV5+sIrN7E4EKA3l8o4GR7I=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=H2zz/Vda8o0FAgLTsDYuESHHtNzL6rP/CKiH8GGM7d6nMqBXbO+WgYKBuYyDQNw7FZEvrhR/JFNn5GRDsD6y2sj/ue7ol1bkfU0kxrlNv4j3XwWsyDy/hmfa9PaFP74gF6iU7gftK0/Fsj/ETU0XOXpgDLvPqBD0Tv97MKBUeS0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de; spf=pass smtp.mailfrom=pengutronix.de; arc=none smtp.client-ip=185.203.201.7 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pengutronix.de Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.whiteo.stw.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1snfOm-00026v-KM for linux-kernel@vger.kernel.org; Mon, 09 Sep 2024 16:29:48 +0200 Received: from [2a0a:edc0:0:b01:1d::7b] (helo=bjornoya.blackshift.org) by drehscheibe.grey.stw.pengutronix.de with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1snfOm-006g3P-4D for linux-kernel@vger.kernel.org; Mon, 09 Sep 2024 16:29:48 +0200 Received: from dspam.blackshift.org (localhost [127.0.0.1]) by bjornoya.blackshift.org (Postfix) with SMTP id C7D7A336B32 for ; Mon, 09 Sep 2024 14:29:47 +0000 (UTC) Received: from hardanger.blackshift.org (unknown [172.20.34.65]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by bjornoya.blackshift.org (Postfix) with ESMTPS id 3227C336B12; Mon, 09 Sep 2024 14:29:45 +0000 (UTC) Received: from [172.20.34.65] (localhost [::1]) by hardanger.blackshift.org (OpenSMTPD) with ESMTP id b502aa44; Mon, 9 Sep 2024 14:29:44 +0000 (UTC) From: Marc Kleine-Budde Date: Mon, 09 Sep 2024 16:29:36 +0200 Subject: [PATCH can v2 2/2] can: m_can: m_can_close(): stop clocks after device has been shut down Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20240909-can-m_can-fix-ifup-v2-2-2b35e624a089@pengutronix.de> References: <20240909-can-m_can-fix-ifup-v2-0-2b35e624a089@pengutronix.de> In-Reply-To: <20240909-can-m_can-fix-ifup-v2-0-2b35e624a089@pengutronix.de> To: Chandrasekar Ramakrishnan , Vincent Mailhol , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Dong Aisheng , Fengguang Wu , Varka Bhadram Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Marc Kleine-Budde X-Mailer: b4 0.15-dev-7be4f X-Developer-Signature: v=1; a=openpgp-sha256; l=1326; i=mkl@pengutronix.de; h=from:subject:message-id; bh=148obTx++Y3V8ypuU2N7MV5+sIrN7E4EKA3l8o4GR7I=; b=owEBbQGS/pANAwAKASg4oj56LbxvAcsmYgBm3wZVz0ZMV16OHcZFLF6AUqux4obsFEjdjJ5ip OzO/4D82n2JATMEAAEKAB0WIQRQQLqG4LYE3Sm8Pl8oOKI+ei28bwUCZt8GVQAKCRAoOKI+ei28 b1UPCACXZlb+a6UZUK9oa6Djs9b0NCr/yHH2ee04hXM3eCzPMLzaDfNtF3c/w/tHDCs5qs98g4e LnbQX6ud4ev8uBYN1UJVeJRwNPXt9WmM3cd01mULGwvuWKMjraSzGrxTaSP7fWm1zZqiRZuIoED Atm/BuHiPX3bCvEszTqwlcRAs3SyD5FQzgkVHXUjc9mR5D2r4xrAe47JxTg9Jaxuv72YQUNFE0s dkFFwmGStSxq0criNZY4SBy8tjfeXDEyamp2PIJbnXJU8+jDWDXi5Ykim/360xj0yDhlliqwnK6 3H9j1xKjTIyAMinzxLT2EqEjBJlu/0x3wRu2PYwe6yjiAMVG X-Developer-Key: i=mkl@pengutronix.de; a=openpgp; fpr=C1400BA0B3989E6FBC7D5B5C2B5EE211C58AEA54 X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 X-SA-Exim-Mail-From: mkl@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org After calling m_can_stop() an interrupt may be pending or NAPI might still be executed. This means the driver might still touch registers of the IP core after the clocks has been disabled. This is not good practice and might lead to aborts depending on the SoC integration. To avoid these potential problems, make m_can_close() symmetric to m_can_open(), i.e. stop the clocks at the end, right before shutting down the transceiver. Fixes: e0d1f4816f2a ("can: m_can: add Bosch M_CAN controller support") Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index 7754dd2d4cb110eee5b83885f5381aed9c67ce03..7aeb56bcd1d660e592b527919d3= 708add62bfb75 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1764,7 +1764,6 @@ static int m_can_close(struct net_device *dev) netif_stop_queue(dev); =20 m_can_stop(dev); - m_can_clk_stop(cdev); free_irq(dev->irq, dev); =20 /* disable NAPI after disabling interrupts */ @@ -1781,6 +1780,7 @@ static int m_can_close(struct net_device *dev) =20 close_candev(dev); =20 + m_can_clk_stop(cdev); phy_power_off(cdev->transceiver); =20 return 0; --=20 2.45.2