[RFC] net: vertexcom: mse102x: Fix SPI interrupt handling

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

arch/arm64/boot/dts/freescale/imx93-charge-som.dtsi

@@ -98,8 +98,7 @@
 	ethernet@0 {
 		compatible = "vertexcom,mse1021";
 		reg = <0>;
-		interrupt-parent = <&gpio2>;
-		interrupts = <7 IRQ_TYPE_EDGE_RISING>;
+		int-gpios = <&gpio2 7 GPIO_ACTIVE_HIGH>;
 		spi-cpha;
 		spi-cpol;
 		spi-max-frequency = <7142857>;

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>
@@ -44,7 +45,6 @@
 
 struct mse102x_stats {
 	u64 xfer_err;
-	u64 invalid_cmd;
 	u64 invalid_ctr;
 	u64 invalid_dft;
 	u64 invalid_len;
@@ -55,7 +55,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",
@@ -83,6 +82,7 @@ struct mse102x_net_spi {
 	struct spi_device	*spidev;
 	struct spi_message	spi_msg;
 	struct spi_transfer	spi_xfer;
+	struct gpio_desc	*spi_int;
 
 #ifdef CONFIG_DEBUG_FS
 	struct dentry		*device_root;
@@ -193,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);
@@ -262,7 +261,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;
@@ -280,6 +279,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);
@@ -310,38 +312,33 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse)
 	__be16 rx = 0;
 	u16 cmd_resp;
 	u8 *rxpkt;
-	int ret;
+	bool drop = false;
 
 	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)) {
-		usleep_range(50, 100);
-
-		mse102x_tx_cmd_spi(mse, CMD_CTR);
-		ret = mse102x_rx_cmd_spi(mse, (u8 *)&rx);
-		if (ret)
-			return;
+	if (mse102x_rx_cmd_spi(mse, (u8 *)&rx))
+		return;
 
-		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++;
-			return;
+	cmd_resp = be16_to_cpu(rx);
+	if ((cmd_resp & CMD_MASK) == CMD_RTS) {
+		rxlen = cmd_resp & LEN_MASK;
+		if (rxlen < ETH_ZLEN || rxlen > VLAN_ETH_FRAME_LEN) {
+			net_dbg_ratelimited("%s: Invalid frame length: %d\n",
+					    __func__, rxlen);
+			mse->stats.invalid_len++;
+			drop = true;
 		}
-
-		net_dbg_ratelimited("%s: Unexpected response to first CMD\n",
-				    __func__);
+	} else {
+		net_dbg_ratelimited("%s: Unexpected response (0x%04x)\n",
+				    __func__, cmd_resp);
+		mse->stats.invalid_rts++;
+		drop = true;
 	}
 
-	rxlen = cmd_resp & LEN_MASK;
-	if (!rxlen) {
-		net_dbg_ratelimited("%s: No frame length defined\n", __func__);
-		mse->stats.invalid_len++;
-		return;
-	}
+	/* In case of a invalid CMD_RTS, the frame must be consumed anyway.
+	 * So assume the maximum possible frame length.
+	 */
+	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);
@@ -353,7 +350,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;
@@ -508,9 +505,23 @@ 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);
+	unsigned long flags = IRQF_ONESHOT;
 	int ret;
 
-	ret = request_threaded_irq(ndev->irq, NULL, mse102x_irq, IRQF_ONESHOT,
+	switch (irqd_get_trigger_type(irq_get_irq_data(ndev->irq))) {
+	case IRQ_TYPE_LEVEL_HIGH:
+	case IRQ_TYPE_LEVEL_LOW:
+		break;
+	case IRQ_TYPE_NONE:
+		flags |= IRQ_TYPE_LEVEL_HIGH;
+		break;
+	default:
+		netdev_warn_once(ndev, "Only IRQ type level recommended, please update your DT.\n");
+		break;
+	}
+
+	ret = request_threaded_irq(ndev->irq, NULL, mse102x_irq, flags,
 				   ndev->name, mse);
 	if (ret < 0) {
 		netdev_err(ndev, "Failed to get irq: %d\n", ret);
@@ -523,6 +534,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;
@@ -696,6 +714,21 @@ static int mse102x_probe_spi(struct spi_device *spi)
 	mse = netdev_priv(ndev);
 	mses = to_mse102x_spi(mse);
 
+	mses->spi_int = devm_gpiod_get_optional(&spi->dev, "int", GPIOD_IN);
+	if (IS_ERR(mses->spi_int))
+		return PTR_ERR(mses->spi_int);
+
+	if (mses->spi_int) {
+		spi->irq = gpiod_to_irq(mses->spi_int);
+
+		if (spi->irq < 0) {
+			dev_err(dev, "Unable to get SPI IRQ: %d\n", spi->irq);
+			return spi->irq;
+		}
+
+		dev_info(dev, "Use GPIO instead of interrupt\n");
+	}
+
 	mses->spidev = spi;
 	mutex_init(&mses->lock);
 	INIT_WORK(&mses->tx_work, mse102x_tx_work);