From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 550553803FD; Thu, 11 Jun 2026 21:55:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781214953; cv=none; b=dSehoXgIla6L1mh6RHiZzMN53Ini0qqDZRai9oAdpzrRkvumGGwp7V7UvbotBRy02Fh8lhVhd7PXgzj5yWfIdX2a/bQcCt/ClYOturQU2E/CTW/dlOMS8MuZsCN6ePd+s+flimMIs1Wm+MaDQmMdVMXcRSH999BL7owOOuO/Sew= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781214953; c=relaxed/simple; bh=hKax7yR2WI09guw84khIQBCbLRT7TjWaQRDaCF6tIhg=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=q/oADuFTscPJM44n50yRBXXDhwWKc7g23/362ZVVKjronC+lh8sMcr7/q2BeaYtPKYm+bVNoh/2Yt7Y8k0LMpFkcnSzTq7IWdpCA2Gsk5phJPyxSHLQ5Z3gMuvmwMu9aZ6AVe2SJZlmJRCOR5KsKDxQyVeIfL6MHXO04Og/sVVQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=qD6BZNbr; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="qD6BZNbr" Received: by smtp.kernel.org (Postfix) with ESMTPS id 00861C2BCB0; Thu, 11 Jun 2026 21:55:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1781214953; bh=hKax7yR2WI09guw84khIQBCbLRT7TjWaQRDaCF6tIhg=; h=From:Date:Subject:References:In-Reply-To:To:Cc:Reply-To:From; b=qD6BZNbr+TYemo0Ezk+upuKwykdrQwpdJvDFZcBPxf2kenhxKwZOOK+bs2N7uMFa/ FEtQmSgG0e+3lJIZgBQO31xPNhWpahwdl2+eL0AFaj3siNUh3vACwkA/5vnVburXz6 7tI+fUCsG+p3/PgVtGoLeI8uxraOqDO9kMu1DtZ8ZS3k10uTejzKTthHCbXIzO7Gf3 p5jO80Vq5PJKTOZtth+d5c3g+2ETAjdU0Vm2J2zHGEzIOf3qpt37U8l0yXA59Lg6tL y179aV6OAbs0XI4ZeHzfwNmYgITm+oHplPKkUdLJOKYZaqYd5VST/telfMcmw69OMD k1rW4LjEk49TA== Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id EBD4ACD8C9D; Thu, 11 Jun 2026 21:55:52 +0000 (UTC) From: Selvamani Rajagopal via B4 Relay Date: Thu, 11 Jun 2026 14:55:38 -0700 Subject: [PATCH net v5 1/4] net: ethernet: oa_tc6: Interrupt is active low, level triggered. Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20260611-level-trigger-v5-1-4533a9e85ce2@onsemi.com> References: <20260611-level-trigger-v5-0-4533a9e85ce2@onsemi.com> In-Reply-To: <20260611-level-trigger-v5-0-4533a9e85ce2@onsemi.com> To: Parthiban Veerasooran , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Piergiorgio Beruto Cc: Andrew Lunn , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Conor Dooley , devicetree@vger.kernel.org, Parthiban Veerasooran , Selvamani Rajagopal X-Mailer: b4 0.15.2 X-Developer-Signature: v=1; a=ed25519-sha256; t=1781214947; l=8533; i=Selvamani.Rajagopal@onsemi.com; s=20260531; h=from:subject:message-id; bh=9HwCbl6zOyvNSgQO+oQ9VSxChq2l6gUszgrx3m8o6yc=; b=md+lgI3clDb1X3/dudIQ+fXv8ivDvpMNtxXFSwhjlS84X3LMRECQWpi0yel/APnjtk72SpidN XWsLsgIdjRIC7XcoKyhc6um+DOa1JY8LPnTpR76Ut2Fz+wAj26ueV+7 X-Developer-Key: i=Selvamani.Rajagopal@onsemi.com; a=ed25519; pk=5QRdM0HS/LGWWcUZZ9hVfZ+qbPQGZCumcTXOiN7Fyug= X-Endpoint-Received: by B4 Relay for Selvamani.Rajagopal@onsemi.com/20260531 with auth_id=803 X-Original-From: Selvamani Rajagopal Reply-To: Selvamani.Rajagopal@onsemi.com From: Selvamani Rajagopal According OPEN Alliance 10BASET1x MAC-PHY Serial Interface specification, interrupt is active low, level triggered. Code used edge triggered interrupt which has the risk of losing an interrupt on instances like when interrupt is disabled. Level triggered interrupt won't be deasserted unless handler runs and clear the interrupting conditions. Interrupt handler mechanism is changed to threaded irq from interrupt handler and kernel thread waiting on work queue. Threaded irq mechanism is best suited for level triggered interrupt as it disables the interrupt until handler is run in thread level, while giving us an ability to have interrupt context handler to signal the threaded irq handler. Introduced a logic to disable the device interrupt on error. Error could be due in data chunk's header and footer or SPI interface itself. This will avoid having repeated interrupts, in case the driver couldn't recover from the error condition with the available recovery mechanism. Fixes: 2c6ce5354453 ("net: ethernet: oa_tc6: implement mac-phy interrupt") Signed-off-by: Selvamani Rajagopal --- changes in v5: - Updated the commit message to be more descriptive - Fixed the race condition between interrupt event and threaded irq to avoid interrupt storm and accessing skb after freed. changes in v4: Interrupt handling mechanism changed to threaded irq changes in v3: interrupt registered as level triggerred --- drivers/net/ethernet/oa_tc6.c | 126 +++++++++++++++++++++++++----------------- 1 file changed, 76 insertions(+), 50 deletions(-) diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c index 91a906a7918a..f3ac2875adfb 100644 --- a/drivers/net/ethernet/oa_tc6.c +++ b/drivers/net/ethernet/oa_tc6.c @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -44,6 +45,8 @@ #define INT_MASK0_LOSS_OF_FRAME_ERR_MASK BIT(4) #define INT_MASK0_RX_BUFFER_OVERFLOW_ERR_MASK BIT(3) #define INT_MASK0_TX_PROTOCOL_ERR_MASK BIT(0) +#define INT_MASK0_ALL_INTERRUPTS (GENMASK(5, 0) | \ + GENMASK(12, 7)) /* PHY Clause 22 registers base address and mask */ #define OA_TC6_PHY_STD_REG_ADDR_BASE 0xFF00 @@ -121,14 +124,13 @@ struct oa_tc6 { struct sk_buff *ongoing_tx_skb; struct sk_buff *waiting_tx_skb; struct sk_buff *rx_skb; - struct task_struct *spi_thread; - wait_queue_head_t spi_wq; u16 tx_skb_offset; u16 spi_data_tx_buf_offset; u16 tx_credits; u8 rx_chunks_available; bool rx_buf_overflow; bool int_flag; + bool disable_traffic; }; enum oa_tc6_header_type { @@ -669,6 +671,38 @@ static void oa_tc6_cleanup_ongoing_tx_skb(struct oa_tc6 *tc6) } } +static void oa_tc6_cleanup_waiting_tx_skb(struct oa_tc6 *tc6) +{ + if (tc6->waiting_tx_skb) { + tc6->netdev->stats.tx_dropped++; + kfree_skb(tc6->waiting_tx_skb); + tc6->waiting_tx_skb = NULL; + } +} + +static void oa_tc6_free_pending_skbs(struct oa_tc6 *tc6) +{ + oa_tc6_cleanup_ongoing_tx_skb(tc6); + oa_tc6_cleanup_ongoing_rx_skb(tc6); + oa_tc6_cleanup_waiting_tx_skb(tc6); +} + +/* If the failure is at SPI interface level, masking and clearing + * the interrupt of the device won't work. Since SPI interrupt is + * disabled, it should stop the repeated interrupts. + */ +static void oa_tc6_disable_traffic(struct oa_tc6 *tc6) +{ + u32 regval = INT_MASK0_ALL_INTERRUPTS; + + tc6->disable_traffic = true; + oa_tc6_free_pending_skbs(tc6); + oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval); + oa_tc6_read_register(tc6, OA_TC6_REG_STATUS0, ®val); + oa_tc6_write_register(tc6, OA_TC6_REG_STATUS0, regval); + dev_err(&tc6->spi->dev, "Device interrupt disabled to avoid interrupt storm"); +} + static int oa_tc6_process_extended_status(struct oa_tc6 *tc6) { u32 value; @@ -1105,29 +1139,29 @@ static int oa_tc6_try_spi_transfer(struct oa_tc6 *tc6) return 0; } -static int oa_tc6_spi_thread_handler(void *data) +static irqreturn_t oa_tc6_macphy_threaded_irq(int irq, void *data) { struct oa_tc6 *tc6 = data; - int ret; - - while (likely(!kthread_should_stop())) { - /* This kthread will be waken up if there is a tx skb or mac-phy - * interrupt to perform spi transfer with tx chunks. - */ - wait_event_interruptible(tc6->spi_wq, tc6->int_flag || - (tc6->waiting_tx_skb && - tc6->tx_credits) || - kthread_should_stop()); - - if (kthread_should_stop()) - break; + int ret = 0; - ret = oa_tc6_try_spi_transfer(tc6); - if (ret) - return ret; + /* It is possible that interrupt woke the thread before it is + * disabled. Until we come up with good recovery mechanism, + * no need to attempt spi transfer, once it fails. Pending skbs + * are already freed. + */ + if (!tc6->disable_traffic) { + while (tc6->int_flag || + (tc6->waiting_tx_skb && tc6->tx_credits)) { + ret = oa_tc6_try_spi_transfer(tc6); + if (ret) { + disable_irq_nosync(tc6->spi->irq); + oa_tc6_disable_traffic(tc6); + break; + } + } } - return 0; + return IRQ_HANDLED; } static int oa_tc6_update_buffer_status_from_register(struct oa_tc6 *tc6) @@ -1161,11 +1195,15 @@ static irqreturn_t oa_tc6_macphy_isr(int irq, void *data) * the previous rx footer. * - extended status event not reported in the previous rx footer. */ - tc6->int_flag = true; - /* Wake spi kthread to perform spi transfer */ - wake_up_interruptible(&tc6->spi_wq); - - return IRQ_HANDLED; + if (tc6->disable_traffic) + disable_irq_nosync(tc6->spi->irq); + else + tc6->int_flag = true; + /* Wake IRQ thread to perform spi transfer . In case + * disable_traffic is set, threaded irq may run again + * one more time. + */ + return IRQ_WAKE_THREAD; } /** @@ -1202,7 +1240,7 @@ EXPORT_SYMBOL_GPL(oa_tc6_zero_align_receive_frame_enable); */ netdev_tx_t oa_tc6_start_xmit(struct oa_tc6 *tc6, struct sk_buff *skb) { - if (tc6->waiting_tx_skb) { + if (tc6->disable_traffic || tc6->waiting_tx_skb) { netif_stop_queue(tc6->netdev); return NETDEV_TX_BUSY; } @@ -1217,8 +1255,8 @@ netdev_tx_t oa_tc6_start_xmit(struct oa_tc6 *tc6, struct sk_buff *skb) tc6->waiting_tx_skb = skb; spin_unlock_bh(&tc6->tx_skb_lock); - /* Wake spi kthread to perform spi transfer */ - wake_up_interruptible(&tc6->spi_wq); + /* Wake the threaded IRQ to perform spi transfer. */ + irq_wake_thread(tc6->spi->irq, tc6); return NETDEV_TX_OK; } @@ -1311,24 +1349,15 @@ struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev) goto phy_exit; } - init_waitqueue_head(&tc6->spi_wq); - - tc6->spi_thread = kthread_run(oa_tc6_spi_thread_handler, tc6, - "oa-tc6-spi-thread"); - if (IS_ERR(tc6->spi_thread)) { - dev_err(&tc6->spi->dev, "Failed to create SPI thread\n"); - goto phy_exit; - } - - sched_set_fifo(tc6->spi_thread); - - ret = devm_request_irq(&tc6->spi->dev, tc6->spi->irq, oa_tc6_macphy_isr, - IRQF_TRIGGER_FALLING, dev_name(&tc6->spi->dev), - tc6); + ret = devm_request_threaded_irq(&tc6->spi->dev, tc6->spi->irq, + oa_tc6_macphy_isr, + oa_tc6_macphy_threaded_irq, + IRQF_TRIGGER_LOW | IRQF_ONESHOT, + dev_name(&tc6->spi->dev), tc6); if (ret) { dev_err(&tc6->spi->dev, "Failed to request macphy isr %d\n", ret); - goto kthread_stop; + goto phy_exit; } /* oa_tc6_sw_reset_macphy() function resets and clears the MAC-PHY reset @@ -1338,12 +1367,10 @@ struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev) * 7.7 and 9.2.8.8 in the OPEN Alliance specification for more details. */ tc6->int_flag = true; - wake_up_interruptible(&tc6->spi_wq); + irq_wake_thread(tc6->spi->irq, tc6); return tc6; -kthread_stop: - kthread_stop(tc6->spi_thread); phy_exit: oa_tc6_phy_exit(tc6); return NULL; @@ -1356,11 +1383,10 @@ EXPORT_SYMBOL_GPL(oa_tc6_init); */ void oa_tc6_exit(struct oa_tc6 *tc6) { + tc6->disable_traffic = true; + disable_irq(tc6->spi->irq); oa_tc6_phy_exit(tc6); - kthread_stop(tc6->spi_thread); - dev_kfree_skb_any(tc6->ongoing_tx_skb); - dev_kfree_skb_any(tc6->waiting_tx_skb); - dev_kfree_skb_any(tc6->rx_skb); + oa_tc6_free_pending_skbs(tc6); } EXPORT_SYMBOL_GPL(oa_tc6_exit); -- 2.43.0