From a29a72866616ef670c4b050419c6753ca6b0245c Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Fri, 9 May 2025 14:04:30 +0200 Subject: [PATCH 1/6] dt-bindings: vertexcom-mse102x: Fix IRQ type in example According to the MSE102x documentation the trigger type is a high level. Signed-off-by: Stefan Wahren Reviewed-by: Andrew Lunn Acked-by: Conor Dooley Link: https://patch.msgid.link/20250509120435.43646-2-wahrenst@gmx.net Signed-off-by: Jakub Kicinski --- Documentation/devicetree/bindings/net/vertexcom-mse102x.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/net/vertexcom-mse102x.yaml b/Documentation/devicetree/bindings/net/vertexcom-mse102x.yaml index 4158673f723c..8359de7ad272 100644 --- a/Documentation/devicetree/bindings/net/vertexcom-mse102x.yaml +++ b/Documentation/devicetree/bindings/net/vertexcom-mse102x.yaml @@ -63,7 +63,7 @@ examples: compatible = "vertexcom,mse1021"; reg = <0>; interrupt-parent = <&gpio>; - interrupts = <23 IRQ_TYPE_EDGE_RISING>; + interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; spi-cpha; spi-cpol; spi-max-frequency = <7142857>; From fed56943a8ba0409570ed79b040acf75e47d676d Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Fri, 9 May 2025 14:04:31 +0200 Subject: [PATCH 2/6] net: vertexcom: mse102x: Add warning about IRQ trigger type The example of the initial DT binding of the Vertexcom MSE 102x suggested a IRQ_TYPE_EDGE_RISING, which is wrong. So warn everyone to fix their device tree to level based IRQ. Signed-off-by: Stefan Wahren Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/20250509120435.43646-3-wahrenst@gmx.net Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/vertexcom/mse102x.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/net/ethernet/vertexcom/mse102x.c b/drivers/net/ethernet/vertexcom/mse102x.c index e4d993f31374..78a50a68c567 100644 --- a/drivers/net/ethernet/vertexcom/mse102x.c +++ b/drivers/net/ethernet/vertexcom/mse102x.c @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -522,10 +523,25 @@ static irqreturn_t mse102x_irq(int irq, void *_mse) static int mse102x_net_open(struct net_device *ndev) { + struct irq_data *irq_data = irq_get_irq_data(ndev->irq); struct mse102x_net *mse = netdev_priv(ndev); struct mse102x_net_spi *mses = to_mse102x_spi(mse); int ret; + if (!irq_data) { + netdev_err(ndev, "Invalid IRQ: %d\n", ndev->irq); + return -EINVAL; + } + + switch (irqd_get_trigger_type(irq_data)) { + case IRQ_TYPE_LEVEL_HIGH: + case IRQ_TYPE_LEVEL_LOW: + break; + default: + netdev_warn_once(ndev, "Only IRQ type level recommended, please update your device tree firmware.\n"); + break; + } + ret = request_threaded_irq(ndev->irq, NULL, mse102x_irq, IRQF_ONESHOT, ndev->name, mse); if (ret < 0) { From aeb90c40ee9a8a67c5cac5445162c36586f2816c Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Fri, 9 May 2025 14:04:32 +0200 Subject: [PATCH 3/6] net: vertexcom: mse102x: Drop invalid cmd stats There are several reasons for an invalid command response by the MSE102x: * SPI line interferences * MSE102x is in reset or has no firmware * MSE102x is busy * no packet in MSE102x receive buffer So the counter for invalid command isn't very helpful without further context. So drop the confusing statistics counter, but keep the debug messages about "unexpected response" in order to debug possible hardware issues. Signed-off-by: Stefan Wahren Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/20250509120435.43646-4-wahrenst@gmx.net Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/vertexcom/mse102x.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/ethernet/vertexcom/mse102x.c b/drivers/net/ethernet/vertexcom/mse102x.c index 78a50a68c567..954256fddd7c 100644 --- a/drivers/net/ethernet/vertexcom/mse102x.c +++ b/drivers/net/ethernet/vertexcom/mse102x.c @@ -46,7 +46,6 @@ struct mse102x_stats { u64 xfer_err; - u64 invalid_cmd; u64 invalid_ctr; u64 invalid_dft; u64 invalid_len; @@ -57,7 +56,6 @@ struct mse102x_stats { static const char mse102x_gstrings_stats[][ETH_GSTRING_LEN] = { "SPI transfer errors", - "Invalid command", "Invalid CTR", "Invalid DFT", "Invalid frame length", @@ -195,7 +193,6 @@ static int mse102x_rx_cmd_spi(struct mse102x_net *mse, u8 *rxb) } else if (*cmd != cpu_to_be16(DET_CMD)) { net_dbg_ratelimited("%s: Unexpected response (0x%04x)\n", __func__, *cmd); - mse->stats.invalid_cmd++; ret = -EIO; } else { memcpy(rxb, trx + 2, 2); From 6ce9348468c5a8862a036fa8534838fec2c1fabf Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Fri, 9 May 2025 14:04:33 +0200 Subject: [PATCH 4/6] net: vertexcom: mse102x: Implement flag for valid CMD After removal of the invalid command counter only a relevant debug message is left, which can be cumbersome. So add a new flag to debugfs, which indicates whether the driver has ever received a valid CMD. This helps to differentiate between general and temporary receive issues. Signed-off-by: Stefan Wahren Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/20250509120435.43646-5-wahrenst@gmx.net Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/vertexcom/mse102x.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/vertexcom/mse102x.c b/drivers/net/ethernet/vertexcom/mse102x.c index 954256fddd7c..01bbcb4d540b 100644 --- a/drivers/net/ethernet/vertexcom/mse102x.c +++ b/drivers/net/ethernet/vertexcom/mse102x.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -84,6 +85,8 @@ struct mse102x_net_spi { struct spi_message spi_msg; struct spi_transfer spi_xfer; + bool valid_cmd_received; + #ifdef CONFIG_DEBUG_FS struct dentry *device_root; #endif @@ -97,16 +100,18 @@ static int mse102x_info_show(struct seq_file *s, void *what) { struct mse102x_net_spi *mses = s->private; - seq_printf(s, "TX ring size : %u\n", + seq_printf(s, "TX ring size : %u\n", skb_queue_len(&mses->mse102x.txq)); - seq_printf(s, "IRQ : %d\n", + seq_printf(s, "IRQ : %d\n", mses->spidev->irq); - seq_printf(s, "SPI effective speed : %lu\n", + seq_printf(s, "SPI effective speed : %lu\n", (unsigned long)mses->spi_xfer.effective_speed_hz); - seq_printf(s, "SPI mode : %x\n", + seq_printf(s, "SPI mode : %x\n", mses->spidev->mode); + seq_printf(s, "Received valid CMD once : %s\n", + str_yes_no(mses->valid_cmd_received)); return 0; } @@ -196,6 +201,7 @@ static int mse102x_rx_cmd_spi(struct mse102x_net *mse, u8 *rxb) ret = -EIO; } else { memcpy(rxb, trx + 2, 2); + mses->valid_cmd_received = true; } return ret; From 4ecf56f4b66011b583644bf9a62188d05dfcd78c Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Fri, 9 May 2025 14:04:34 +0200 Subject: [PATCH 5/6] net: vertexcom: mse102x: Return code for mse102x_rx_pkt_spi The MSE102x doesn't provide any interrupt register, so the only way to handle the level interrupt is to fetch the whole packet from the MSE102x internal buffer via SPI. So in cases the interrupt handler fails to do this, it should return IRQ_NONE. This allows the core to disable the interrupt in case the issue persists and prevent an interrupt storm. Signed-off-by: Stefan Wahren Link: https://patch.msgid.link/20250509120435.43646-6-wahrenst@gmx.net Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/vertexcom/mse102x.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/vertexcom/mse102x.c b/drivers/net/ethernet/vertexcom/mse102x.c index 01bbcb4d540b..5abb1240ad4c 100644 --- a/drivers/net/ethernet/vertexcom/mse102x.c +++ b/drivers/net/ethernet/vertexcom/mse102x.c @@ -310,7 +310,7 @@ static void mse102x_dump_packet(const char *msg, int len, const char *data) data, len, true); } -static void mse102x_rx_pkt_spi(struct mse102x_net *mse) +static irqreturn_t mse102x_rx_pkt_spi(struct mse102x_net *mse) { struct sk_buff *skb; unsigned int rxalign; @@ -331,7 +331,7 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse) mse102x_tx_cmd_spi(mse, CMD_CTR); ret = mse102x_rx_cmd_spi(mse, (u8 *)&rx); if (ret) - return; + return IRQ_NONE; cmd_resp = be16_to_cpu(rx); if ((cmd_resp & CMD_MASK) != CMD_RTS) { @@ -364,7 +364,7 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse) rxalign = ALIGN(rxlen + DET_SOF_LEN + DET_DFT_LEN, 4); skb = netdev_alloc_skb_ip_align(mse->ndev, rxalign); if (!skb) - return; + return IRQ_NONE; /* 2 bytes Start of frame (before ethernet header) * 2 bytes Data frame tail (after ethernet frame) @@ -374,7 +374,7 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse) if (mse102x_rx_frame_spi(mse, rxpkt, rxlen, drop)) { mse->ndev->stats.rx_errors++; dev_kfree_skb(skb); - return; + return IRQ_HANDLED; } if (netif_msg_pktdata(mse)) @@ -385,6 +385,8 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse) mse->ndev->stats.rx_packets++; mse->ndev->stats.rx_bytes += rxlen; + + return IRQ_HANDLED; } static int mse102x_tx_pkt_spi(struct mse102x_net *mse, struct sk_buff *txb, @@ -516,12 +518,13 @@ static irqreturn_t mse102x_irq(int irq, void *_mse) { struct mse102x_net *mse = _mse; struct mse102x_net_spi *mses = to_mse102x_spi(mse); + irqreturn_t ret; mutex_lock(&mses->lock); - mse102x_rx_pkt_spi(mse); + ret = mse102x_rx_pkt_spi(mse); mutex_unlock(&mses->lock); - return IRQ_HANDLED; + return ret; } static int mse102x_net_open(struct net_device *ndev) From 8ea6e51e54c512f4042b1b39e404bcffb1a7f059 Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Fri, 9 May 2025 14:04:35 +0200 Subject: [PATCH 6/6] net: vertexcom: mse102x: Simplify mse102x_rx_pkt_spi The function mse102x_rx_pkt_spi is used in two cases: * initial polling to re-arm RX interrupt * level based RX interrupt handler Both of them doesn't need an open-coded retry mechanism. In the first case the function can be called again, if the return code is IRQ_NONE. This keeps the error behavior during netdev open. In the second case the proper retry would be handled implicit by the SPI interrupt. So drop the retry code and simplify the receive path. Signed-off-by: Stefan Wahren Link: https://patch.msgid.link/20250509120435.43646-7-wahrenst@gmx.net Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/vertexcom/mse102x.c | 34 +++++++++--------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/vertexcom/mse102x.c b/drivers/net/ethernet/vertexcom/mse102x.c index 5abb1240ad4c..a75bca1243e3 100644 --- a/drivers/net/ethernet/vertexcom/mse102x.c +++ b/drivers/net/ethernet/vertexcom/mse102x.c @@ -319,31 +319,20 @@ static irqreturn_t mse102x_rx_pkt_spi(struct mse102x_net *mse) __be16 rx = 0; u16 cmd_resp; u8 *rxpkt; - int ret; mse102x_tx_cmd_spi(mse, CMD_CTR); - ret = mse102x_rx_cmd_spi(mse, (u8 *)&rx); - cmd_resp = be16_to_cpu(rx); - - if (ret || ((cmd_resp & CMD_MASK) != CMD_RTS)) { + if (mse102x_rx_cmd_spi(mse, (u8 *)&rx)) { usleep_range(50, 100); + return IRQ_NONE; + } - mse102x_tx_cmd_spi(mse, CMD_CTR); - ret = mse102x_rx_cmd_spi(mse, (u8 *)&rx); - if (ret) - return IRQ_NONE; - - cmd_resp = be16_to_cpu(rx); - if ((cmd_resp & CMD_MASK) != CMD_RTS) { - net_dbg_ratelimited("%s: Unexpected response (0x%04x)\n", - __func__, cmd_resp); - mse->stats.invalid_rts++; - drop = true; - goto drop; - } - - net_dbg_ratelimited("%s: Unexpected response to first CMD\n", - __func__); + cmd_resp = be16_to_cpu(rx); + if ((cmd_resp & CMD_MASK) != CMD_RTS) { + net_dbg_ratelimited("%s: Unexpected response (0x%04x)\n", + __func__, cmd_resp); + mse->stats.invalid_rts++; + drop = true; + goto drop; } rxlen = cmd_resp & LEN_MASK; @@ -565,7 +554,8 @@ static int mse102x_net_open(struct net_device *ndev) * So poll for possible packet(s) to re-arm the interrupt. */ mutex_lock(&mses->lock); - mse102x_rx_pkt_spi(mse); + if (mse102x_rx_pkt_spi(mse) == IRQ_NONE) + mse102x_rx_pkt_spi(mse); mutex_unlock(&mses->lock); netif_dbg(mse, ifup, ndev, "network device up\n");