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

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

[Stefan Wahren]

The series contains substantial fixes for the RX handling of
the Vertexcom MSE102x.

Stefan Wahren (5):
  net: vertexcom: mse102x: Fix possible stuck of SPI interrupt
  dt-bindings: vertexcom-mse102x: Fix IRQ type in example
  net: vertexcom: mse102x: Fix LEN_MASK
  net: vertexcom: mse102x: Add range check for CMD_RTS
  net: vertexcom: mse102x: Fix RX error handling

 .../bindings/net/vertexcom-mse102x.yaml       |  2 +-
 drivers/net/ethernet/vertexcom/mse102x.c      | 36 +++++++++++++++----
 2 files changed, 30 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH net 1/5] 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(+)

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 1/5] net: vertexcom: mse102x: Fix possible stuck of SPI interrupt

[Jakub Kicinski]

On Wed, 23 Apr 2025 09:45:49 +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.

Wouldn't this cause invalid_rts or some other error counter 
to increment every time the user opens the device?

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

[Stefan Wahren]

Hi Jakub,

Am 25.04.25 um 03:18 schrieb Jakub Kicinski:
> On Wed, 23 Apr 2025 09:45:49 +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.
> Wouldn't this cause invalid_rts or some other error counter
> to increment every time the user opens the device?
you are right, this would increase the invalid_cmd counter.

I'm missed to mention in the cover letter, that this series is only the 
first part of two patch series. The first one only contains fixes, which 
should be backported. The second one contains improvement, which can go 
to net-next.

Regarding the invalid_cmd there is already a patch in my backlog, but I 
didn't consider this as a real fix:

net: vertexcom: mse102x: drop invalid cmd stats

Since the SPI implementation on the MSE102x MCU is in software, it 
cannot reply to SPI commands in busy state. So drop the scaring 
statistics about "invalid" command replies.

https://github.com/chargebyte/linux/commit/9f8a69e5c0d6c4482e89d7b86f72069b89a94547

Should I add it as a fix?

Regards

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

[Jakub Kicinski]

On Fri, 25 Apr 2025 09:35:04 +0200 Stefan Wahren wrote:
> Since the SPI implementation on the MSE102x MCU is in software, it 
> cannot reply to SPI commands in busy state. So drop the scaring 
> statistics about "invalid" command replies.
> 
> https://github.com/chargebyte/linux/commit/9f8a69e5c0d6c4482e89d7b86f72069b89a94547
> 
> Should I add it as a fix?

I see. I don't think we have to add that to the fixes series.
Worst case if people complain we can request a backport later.

[PATCH net 2/5] dt-bindings: vertexcom-mse102x: Fix IRQ type in example

[Stefan Wahren]

According to the MSE102x documentation the trigger type is a
high level.

Fixes: 2717566f6661 ("dt-bindings: net: add Vertexcom MSE102x support")
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 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>;
-- 
2.34.1

Re: [PATCH net 2/5] dt-bindings: vertexcom-mse102x: Fix IRQ type in example

[Jakub Kicinski]

On Wed, 23 Apr 2025 09:45:50 +0200 Stefan Wahren wrote:
> According to the MSE102x documentation the trigger type is a
> high level.
> 
> Fixes: 2717566f6661 ("dt-bindings: net: add Vertexcom MSE102x support")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

I noticed after sending previous reply that the patchset already 
got marked as changes requested. Let me use that as an excuse to
ask to drop this patch (and resend to net-next). Happy to oblige
if DT maintainers disagree but I'm not sure we should be treating
changes to an example as a fix.

[PATCH net 3/5] 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

[PATCH net 4/5] 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

[PATCH net 5/5] 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