[PATCH net V2 0/4] net: vertexcom: mse102x: Fix RX handling

[PATCH net V2 0/4] net: vertexcom: mse102x: Fix RX handling

[Stefan Wahren]

This series is the first part of two series for the Vertexcom driver.
It contains substantial fixes for the RX handling of the Vertexcom MSE102x.

Changes in V2:
- clarify cover letter
- add footnote to Patch 1
- postpone DT binding changes to second series as suggested by Jakub K.

Stefan Wahren (4):
  net: vertexcom: mse102x: Fix possible stuck of SPI interrupt
  net: vertexcom: mse102x: Fix LEN_MASK
  net: vertexcom: mse102x: Add range check for CMD_RTS
  net: vertexcom: mse102x: Fix RX error handling

 drivers/net/ethernet/vertexcom/mse102x.c | 36 +++++++++++++++++++-----
 1 file changed, 29 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH net V2 1/4] net: vertexcom: mse102x: Fix possible stuck of SPI interrupt

[Stefan Wahren]

The MSE102x doesn't provide any SPI commands for interrupt handling.
So in case the interrupt fired before the driver requests the IRQ,
the interrupt will never fire again. In order to fix this always poll
for pending packets after opening the interface.

Fixes: 2f207cbf0dd4 ("net: vertexcom: Add MSE102x SPI support")
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/ethernet/vertexcom/mse102x.c | 8 ++++++++
 1 file changed, 8 insertions(+)

I'm aware that this might trigger increment of invalid_cmd. Since this
counter is pointless for other reasons, this will be dropped in the
second (upcoming) series which provide further improvements for this
driver.

diff --git a/drivers/net/ethernet/vertexcom/mse102x.c b/drivers/net/ethernet/vertexcom/mse102x.c
index 89dc4c401a8d..92ebf1633159 100644
--- a/drivers/net/ethernet/vertexcom/mse102x.c
+++ b/drivers/net/ethernet/vertexcom/mse102x.c
@@ -509,6 +509,7 @@ static irqreturn_t mse102x_irq(int irq, void *_mse)
 static int mse102x_net_open(struct net_device *ndev)
 {
 	struct mse102x_net *mse = netdev_priv(ndev);
+	struct mse102x_net_spi *mses = to_mse102x_spi(mse);
 	int ret;
 
 	ret = request_threaded_irq(ndev->irq, NULL, mse102x_irq, IRQF_ONESHOT,
@@ -524,6 +525,13 @@ static int mse102x_net_open(struct net_device *ndev)
 
 	netif_carrier_on(ndev);
 
+	/* The SPI interrupt can stuck in case of pending packet(s).
+	 * So poll for possible packet(s) to re-arm the interrupt.
+	 */
+	mutex_lock(&mses->lock);
+	mse102x_rx_pkt_spi(mse);
+	mutex_unlock(&mses->lock);
+
 	netif_dbg(mse, ifup, ndev, "network device up\n");
 
 	return 0;
-- 
2.34.1

Re: [PATCH net V2 1/4] net: vertexcom: mse102x: Fix possible stuck of SPI interrupt

[Andrew Lunn]

On Wed, Apr 30, 2025 at 03:30:40PM +0200, Stefan Wahren wrote:
> The MSE102x doesn't provide any SPI commands for interrupt handling.
> So in case the interrupt fired before the driver requests the IRQ,
> the interrupt will never fire again. In order to fix this always poll
> for pending packets after opening the interface.
> 
> Fixes: 2f207cbf0dd4 ("net: vertexcom: Add MSE102x SPI support")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net V2 2/4] net: vertexcom: mse102x: Fix LEN_MASK

[Stefan Wahren]

The LEN_MASK for CMD_RTS doesn't cover the whole parameter mask.
The Bit 11 is reserved, so adjust LEN_MASK accordingly.

Fixes: 2f207cbf0dd4 ("net: vertexcom: Add MSE102x SPI support")
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/ethernet/vertexcom/mse102x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/vertexcom/mse102x.c b/drivers/net/ethernet/vertexcom/mse102x.c
index 92ebf1633159..3edf2c3753f0 100644
--- a/drivers/net/ethernet/vertexcom/mse102x.c
+++ b/drivers/net/ethernet/vertexcom/mse102x.c
@@ -33,7 +33,7 @@
 #define CMD_CTR		(0x2 << CMD_SHIFT)
 
 #define CMD_MASK	GENMASK(15, CMD_SHIFT)
-#define LEN_MASK	GENMASK(CMD_SHIFT - 1, 0)
+#define LEN_MASK	GENMASK(CMD_SHIFT - 2, 0)
 
 #define DET_CMD_LEN	4
 #define DET_SOF_LEN	2
-- 
2.34.1

Re: [PATCH net V2 2/4] net: vertexcom: mse102x: Fix LEN_MASK

[Andrew Lunn]

On Wed, Apr 30, 2025 at 03:30:41PM +0200, Stefan Wahren wrote:
> The LEN_MASK for CMD_RTS doesn't cover the whole parameter mask.
> The Bit 11 is reserved, so adjust LEN_MASK accordingly.
> 
> Fixes: 2f207cbf0dd4 ("net: vertexcom: Add MSE102x SPI support")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net V2 3/4] net: vertexcom: mse102x: Add range check for CMD_RTS

[Stefan Wahren]

Since there is no protection in the SPI protocol against electrical
interferences, the driver shouldn't blindly trust the length payload
of CMD_RTS. So introduce a bounds check for incoming frames.

Fixes: 2f207cbf0dd4 ("net: vertexcom: Add MSE102x SPI support")
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/ethernet/vertexcom/mse102x.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/vertexcom/mse102x.c b/drivers/net/ethernet/vertexcom/mse102x.c
index 3edf2c3753f0..2c06d1d05164 100644
--- a/drivers/net/ethernet/vertexcom/mse102x.c
+++ b/drivers/net/ethernet/vertexcom/mse102x.c
@@ -6,6 +6,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/if_vlan.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -337,8 +338,9 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse)
 	}
 
 	rxlen = cmd_resp & LEN_MASK;
-	if (!rxlen) {
-		net_dbg_ratelimited("%s: No frame length defined\n", __func__);
+	if (rxlen < ETH_ZLEN || rxlen > VLAN_ETH_FRAME_LEN) {
+		net_dbg_ratelimited("%s: Invalid frame length: %d\n", __func__,
+				    rxlen);
 		mse->stats.invalid_len++;
 		return;
 	}
-- 
2.34.1

Re: [PATCH net V2 3/4] net: vertexcom: mse102x: Add range check for CMD_RTS

[Andrew Lunn]

On Wed, Apr 30, 2025 at 03:30:42PM +0200, Stefan Wahren wrote:
> Since there is no protection in the SPI protocol against electrical
> interferences, the driver shouldn't blindly trust the length payload
> of CMD_RTS. So introduce a bounds check for incoming frames.
> 
> Fixes: 2f207cbf0dd4 ("net: vertexcom: Add MSE102x SPI support")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net V2 4/4] net: vertexcom: mse102x: Fix RX error handling

[Stefan Wahren]

In case the CMD_RTS got corrupted by interferences, the MSE102x
doesn't allow a retransmission of the command. Instead the Ethernet
frame must be shifted out of the SPI FIFO. Since the actual length is
unknown, assume the maximum possible value.

Fixes: 2f207cbf0dd4 ("net: vertexcom: Add MSE102x SPI support")
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/ethernet/vertexcom/mse102x.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/vertexcom/mse102x.c b/drivers/net/ethernet/vertexcom/mse102x.c
index 2c06d1d05164..e4d993f31374 100644
--- a/drivers/net/ethernet/vertexcom/mse102x.c
+++ b/drivers/net/ethernet/vertexcom/mse102x.c
@@ -263,7 +263,7 @@ static int mse102x_tx_frame_spi(struct mse102x_net *mse, struct sk_buff *txp,
 }
 
 static int mse102x_rx_frame_spi(struct mse102x_net *mse, u8 *buff,
-				unsigned int frame_len)
+				unsigned int frame_len, bool drop)
 {
 	struct mse102x_net_spi *mses = to_mse102x_spi(mse);
 	struct spi_transfer *xfer = &mses->spi_xfer;
@@ -281,6 +281,9 @@ static int mse102x_rx_frame_spi(struct mse102x_net *mse, u8 *buff,
 		netdev_err(mse->ndev, "%s: spi_sync() failed: %d\n",
 			   __func__, ret);
 		mse->stats.xfer_err++;
+	} else if (drop) {
+		netdev_dbg(mse->ndev, "%s: Drop frame\n", __func__);
+		ret = -EINVAL;
 	} else if (*sof != cpu_to_be16(DET_SOF)) {
 		netdev_dbg(mse->ndev, "%s: SPI start of frame is invalid (0x%04x)\n",
 			   __func__, *sof);
@@ -308,6 +311,7 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse)
 	struct sk_buff *skb;
 	unsigned int rxalign;
 	unsigned int rxlen;
+	bool drop = false;
 	__be16 rx = 0;
 	u16 cmd_resp;
 	u8 *rxpkt;
@@ -330,7 +334,8 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse)
 			net_dbg_ratelimited("%s: Unexpected response (0x%04x)\n",
 					    __func__, cmd_resp);
 			mse->stats.invalid_rts++;
-			return;
+			drop = true;
+			goto drop;
 		}
 
 		net_dbg_ratelimited("%s: Unexpected response to first CMD\n",
@@ -342,9 +347,16 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse)
 		net_dbg_ratelimited("%s: Invalid frame length: %d\n", __func__,
 				    rxlen);
 		mse->stats.invalid_len++;
-		return;
+		drop = true;
 	}
 
+	/* In case of a invalid CMD_RTS, the frame must be consumed anyway.
+	 * So assume the maximum possible frame length.
+	 */
+drop:
+	if (drop)
+		rxlen = VLAN_ETH_FRAME_LEN;
+
 	rxalign = ALIGN(rxlen + DET_SOF_LEN + DET_DFT_LEN, 4);
 	skb = netdev_alloc_skb_ip_align(mse->ndev, rxalign);
 	if (!skb)
@@ -355,7 +367,7 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse)
 	 * They are copied, but ignored.
 	 */
 	rxpkt = skb_put(skb, rxlen) - DET_SOF_LEN;
-	if (mse102x_rx_frame_spi(mse, rxpkt, rxlen)) {
+	if (mse102x_rx_frame_spi(mse, rxpkt, rxlen, drop)) {
 		mse->ndev->stats.rx_errors++;
 		dev_kfree_skb(skb);
 		return;
-- 
2.34.1

Re: [PATCH net V2 4/4] net: vertexcom: mse102x: Fix RX error handling

[Andrew Lunn]

On Wed, Apr 30, 2025 at 03:30:43PM +0200, Stefan Wahren wrote:
> In case the CMD_RTS got corrupted by interferences, the MSE102x
> doesn't allow a retransmission of the command. Instead the Ethernet
> frame must be shifted out of the SPI FIFO. Since the actual length is
> unknown, assume the maximum possible value.

I assume the SPI transfer won't get the beginning of the next frames
to make up the shortfall of bytes when the frame is short?

> Fixes: 2f207cbf0dd4 ("net: vertexcom: Add MSE102x SPI support")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net V2 4/4] net: vertexcom: mse102x: Fix RX error handling

[Stefan Wahren]

Hi Andrew,

Am 30.04.25 um 15:47 schrieb Andrew Lunn:
> On Wed, Apr 30, 2025 at 03:30:43PM +0200, Stefan Wahren wrote:
>> In case the CMD_RTS got corrupted by interferences, the MSE102x
>> doesn't allow a retransmission of the command. Instead the Ethernet
>> frame must be shifted out of the SPI FIFO. Since the actual length is
>> unknown, assume the maximum possible value.
> I assume the SPI transfer won't get the beginning of the next frames
> to make up the shortfall of bytes when the frame is short?
Correct. The only possibility would be to triggered on the falling edge 
/ low level of the SPI interrupt and try to cancel the possibly ongoing 
SPI transfer.
I think this would introduce a lot complexity for an issue which is 
actually caused by poor hardware design.

I think it's not worth the trouble.

Regards
>
>> Fixes: 2f207cbf0dd4 ("net: vertexcom: Add MSE102x SPI support")
>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
>      Andrew

Re: [PATCH net V2 0/4] net: vertexcom: mse102x: Fix RX handling

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 30 Apr 2025 15:30:39 +0200 you wrote:
> This series is the first part of two series for the Vertexcom driver.
> It contains substantial fixes for the RX handling of the Vertexcom MSE102x.
> 
> Changes in V2:
> - clarify cover letter
> - add footnote to Patch 1
> - postpone DT binding changes to second series as suggested by Jakub K.
> 
> [...]

Here is the summary with links:
  - [net,V2,1/4] net: vertexcom: mse102x: Fix possible stuck of SPI interrupt
    https://git.kernel.org/netdev/net/c/55f362885951
  - [net,V2,2/4] net: vertexcom: mse102x: Fix LEN_MASK
    https://git.kernel.org/netdev/net/c/74987089ec67
  - [net,V2,3/4] net: vertexcom: mse102x: Add range check for CMD_RTS
    https://git.kernel.org/netdev/net/c/d4dda902dac1
  - [net,V2,4/4] net: vertexcom: mse102x: Fix RX error handling
    https://git.kernel.org/netdev/net/c/ee512922ddd7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html