[PATCH net-next V3 0/6] net: vertexcom: mse102x: Improve RX handling

[PATCH net-next V3 0/6] net: vertexcom: mse102x: Improve RX handling

[Stefan Wahren]

This series is the second part of two series for the Vertexcom driver.
It contains some improvements for the RX handling of the Vertexcom MSE102x.

Changes in V3:
- Fix whitespace issue in patch 4

Changes in V2:
- Add Andrew's Reviewed-by to patch 1-2
- Fix build issue reported by Jakub & kernel test bot in patch 2
- Use Andrew's suggestion for netdev_warn_once in patch 2
- Improve commit logs for patch 3 & 5
- Add new patch 4 to compensate loss of invalid CMD counter

Stefan Wahren (6):
  dt-bindings: vertexcom-mse102x: Fix IRQ type in example
  net: vertexcom: mse102x: Add warning about IRQ trigger type
  net: vertexcom: mse102x: Drop invalid cmd stats
  net: vertexcom: mse102x: Implement flag for valid CMD
  net: vertexcom: mse102x: Return code for mse102x_rx_pkt_spi
  net: vertexcom: mse102x: Simplify mse102x_rx_pkt_spi

 .../bindings/net/vertexcom-mse102x.yaml       |  2 +-
 drivers/net/ethernet/vertexcom/mse102x.c      | 80 +++++++++++--------
 2 files changed, 47 insertions(+), 35 deletions(-)

-- 
2.34.1

[PATCH net-next V3 1/6] dt-bindings: vertexcom-mse102x: Fix IRQ type in example

[Stefan Wahren]

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

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 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

[PATCH net-next V3 2/6] net: vertexcom: mse102x: Add warning about IRQ trigger type

[Stefan Wahren]

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 <wahrenst@gmx.net>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 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 <linux/if_vlan.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
@@ -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) {
-- 
2.34.1

[PATCH net-next V3 3/6] net: vertexcom: mse102x: Drop invalid cmd stats

[Stefan Wahren]

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 <wahrenst@gmx.net>
---
 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);
-- 
2.34.1

[PATCH net-next V3 4/6] net: vertexcom: mse102x: Implement flag for valid CMD

[Stefan Wahren]

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 <wahrenst@gmx.net>
---
 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..c2b8df604238 100644
--- a/drivers/net/ethernet/vertexcom/mse102x.c
+++ b/drivers/net/ethernet/vertexcom/mse102x.c
@@ -17,6 +17,7 @@
 #include <linux/cache.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
+#include <linux/string_choices.h>
 
 #include <linux/spi/spi.h>
 #include <linux/of_net.h>
@@ -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;
-- 
2.34.1

[PATCH net-next V3 5/6] net: vertexcom: mse102x: Return code for mse102x_rx_pkt_spi

[Stefan Wahren]

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 <wahrenst@gmx.net>
---
 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 c2b8df604238..62219fd818f5 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)
-- 
2.34.1

[PATCH net-next V3 6/6] net: vertexcom: mse102x: Simplify mse102x_rx_pkt_spi

[Stefan Wahren]

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 <wahrenst@gmx.net>
---
 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 62219fd818f5..873da57a1d6b 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");
-- 
2.34.1

Re: [PATCH net-next V3 3/6] net: vertexcom: mse102x: Drop invalid cmd stats

[Andrew Lunn]

On Fri, May 09, 2025 at 02:04:32PM +0200, Stefan Wahren wrote:
> 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 <wahrenst@gmx.net>

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

    Andrew

Re: [PATCH net-next V3 4/6] net: vertexcom: mse102x: Implement flag for valid CMD

[Andrew Lunn]

On Fri, May 09, 2025 at 02:04:33PM +0200, Stefan Wahren wrote:
> 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 <wahrenst@gmx.net>

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

    Andrew

Re: [PATCH net-next V3 1/6] dt-bindings: vertexcom-mse102x: Fix IRQ type in example

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]

On Fri, May 09, 2025 at 02:04:30PM +0200, Stefan Wahren wrote:
> According to the MSE102x documentation the trigger type is a
> high level.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
(but I think something like this doesn't need one)

> ---
>  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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next V3 0/6] net: vertexcom: mse102x: Improve RX handling

[patchwork-bot+netdevbpf]

Hello:

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

On Fri,  9 May 2025 14:04:29 +0200 you wrote:
> This series is the second part of two series for the Vertexcom driver.
> It contains some improvements for the RX handling of the Vertexcom MSE102x.
> 
> Changes in V3:
> - Fix whitespace issue in patch 4
> 
> Changes in V2:
> - Add Andrew's Reviewed-by to patch 1-2
> - Fix build issue reported by Jakub & kernel test bot in patch 2
> - Use Andrew's suggestion for netdev_warn_once in patch 2
> - Improve commit logs for patch 3 & 5
> - Add new patch 4 to compensate loss of invalid CMD counter
> 
> [...]

Here is the summary with links:
  - [net-next,V3,1/6] dt-bindings: vertexcom-mse102x: Fix IRQ type in example
    https://git.kernel.org/netdev/net-next/c/a29a72866616
  - [net-next,V3,2/6] net: vertexcom: mse102x: Add warning about IRQ trigger type
    https://git.kernel.org/netdev/net-next/c/fed56943a8ba
  - [net-next,V3,3/6] net: vertexcom: mse102x: Drop invalid cmd stats
    https://git.kernel.org/netdev/net-next/c/aeb90c40ee9a
  - [net-next,V3,4/6] net: vertexcom: mse102x: Implement flag for valid CMD
    https://git.kernel.org/netdev/net-next/c/6ce9348468c5
  - [net-next,V3,5/6] net: vertexcom: mse102x: Return code for mse102x_rx_pkt_spi
    https://git.kernel.org/netdev/net-next/c/4ecf56f4b660
  - [net-next,V3,6/6] net: vertexcom: mse102x: Simplify mse102x_rx_pkt_spi
    https://git.kernel.org/netdev/net-next/c/8ea6e51e54c5

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