[PATCH net-next v6 0/9] net: dsa: microchip: Add PTP support for the KSZ8463

[PATCH net-next v6 0/9] net: dsa: microchip: Add PTP support for the KSZ8463

[Bastien Curutchet (Schneider Electric)]

Hi all,

This series aims to add two-step PTP support for the KSZ8463 driver

I've encountered weird behavior with IPv4 and IPv6 layers -- maybe that's
related to the incompatibility with the Linux stack mentionned by commit
620e2392db235 ("net: dsa: microchip: Disable PTP function of KSZ8463") ? --
So the support is only added for the L2 layer.

Patches 1 to 4 add IRQ support for the KSZ8463
Patch 5 and 6 add specific dsa_device_ops for the KSZ8463
Patches 7 to 9 add PTP support for the KSZ8463

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
Changes in v6:
- ALL: Address the warnings about lines exceeding 80 characters. Some
  remain because readability is better this way IMO
- PATCH 1: Add a comment to explain why the introduced 16-bits read is safe
  for all the switches.
- Link to v5: https://lore.kernel.org/r/20260226-ksz8463-ptp-v5-0-1bc24fb9627d@bootlin.com

Changes in v5:
- Rebase on v7.0-rc1
- PATCH 1: Add Maxime's Reviewed-By
- Add PATCH 5 and modify PATCH 6 to share more code with already existing
  tag operations. Also address Jakub comment about alphabetical order
- PATCH 7: Add details in commit log
- PATCH 9: Set the pdelayresp_flag in the xmit worker to avoid race
  conditions
- Link to v4: https://lore.kernel.org/r/20260127-ksz8463-ptp-v4-0-652e021aae86@bootlin.com

Changes in v4:
- PATCH 4: Fix the unused variable warning reported by the bot
- Link to v3: https://lore.kernel.org/r/20260126-ksz8463-ptp-v3-0-9f092aeb8492@bootlin.com

Changes in v3:
- PATCH 4: Fix the unitialized value reported by clang
- Link to v2: https://lore.kernel.org/r/20260122-ksz8463-ptp-v2-0-b6683f5ce50b@bootlin.com

Changes in v2:
- PATCH 8: Disable PTP by default.
- PATCH 5: TX path: remove timestamp handling
- PATCH 5: RX path: zero the PTP header's reserved area before
  forwarding packets to userspace
- Link to v1: https://lore.kernel.org/r/20260115-ksz8463-ptp-v1-0-bcfe2830cf50@bootlin.com

---
Bastien Curutchet (Schneider Electric) (9):
      net: dsa: microchip: Add support for KSZ8463 global irq
      net: dsa: microchip: Decorrelate IRQ domain from port
      net: dsa: microchip: Decorrelate msg_irq index from IRQ bit offset
      net: dsa: microchip: Add support for KSZ8463's PTP interrupts
      net: dsa: tag_ksz: Share code for KSZ8795 and KSZ9893 xmit operations
      net: dsa: microchip: Add KSZ8463 tail tag handling
      net: dsa: microchip: Explicitly enable detection of L2 PTP frames
      net: dsa: microchip: Adapt port offset for KSZ8463's PTP register
      net: dsa: microchip: Add two-step PTP support for KSZ8463

 drivers/net/dsa/microchip/ksz8.c        |  14 +-
 drivers/net/dsa/microchip/ksz8_reg.h    |   1 +
 drivers/net/dsa/microchip/ksz_common.c  | 101 +++++++++++----
 drivers/net/dsa/microchip/ksz_common.h  |   6 +
 drivers/net/dsa/microchip/ksz_ptp.c     | 204 ++++++++++++++++++++++++++---
 drivers/net/dsa/microchip/ksz_ptp.h     |   9 ++
 drivers/net/dsa/microchip/ksz_ptp_reg.h |  11 ++
 include/net/dsa.h                       |   2 +
 net/dsa/tag_ksz.c                       | 223 ++++++++++++++++++++------------
 9 files changed, 439 insertions(+), 132 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260109-ksz8463-ptp-bc723ca7fac4

Best regards,
-- 
Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>

[PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet (Schneider Electric)]

KSZ8463's interrupt scheme differs from the others KSZ swicthes. Its
global interrupt handling is done through an 'enable irq' register
instead of a 'mask irq' one, so the bit logic to enable/disable
interrupt is reversed. Also its interrupts registers are 16-bits
registers and don't have the same address.

Add ksz8463-specific global interrupt setup function that still relies
on the ksz_irq_common_setup().
Add a check on the device type in the irq_chip operations to adjust the
bit logic for KSZ8463

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 46 ++++++++++++++++++++++++++++------
 drivers/net/dsa/microchip/ksz_common.h |  3 +++
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index c517478cc47677544b6523faee113ece036c9ed9..8179415d9b3593d4a9d17661b4ed30007dea9958 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2817,14 +2817,20 @@ static void ksz_irq_mask(struct irq_data *d)
 {
 	struct ksz_irq *kirq = irq_data_get_irq_chip_data(d);
 
-	kirq->masked |= BIT(d->hwirq);
+	if (ksz_is_ksz8463(kirq->dev))
+		kirq->masked &= ~BIT(d->hwirq);
+	else
+		kirq->masked |= BIT(d->hwirq);
 }
 
 static void ksz_irq_unmask(struct irq_data *d)
 {
 	struct ksz_irq *kirq = irq_data_get_irq_chip_data(d);
 
-	kirq->masked &= ~BIT(d->hwirq);
+	if (ksz_is_ksz8463(kirq->dev))
+		kirq->masked |= BIT(d->hwirq);
+	else
+		kirq->masked &= ~BIT(d->hwirq);
 }
 
 static void ksz_irq_bus_lock(struct irq_data *d)
@@ -2840,7 +2846,10 @@ static void ksz_irq_bus_sync_unlock(struct irq_data *d)
 	struct ksz_device *dev = kirq->dev;
 	int ret;
 
-	ret = ksz_write8(dev, kirq->reg_mask, kirq->masked);
+	if (ksz_is_ksz8463(dev))
+		ret = ksz_write16(dev, kirq->reg_mask, kirq->masked);
+	else
+		ret = ksz_write8(dev, kirq->reg_mask, kirq->masked);
 	if (ret)
 		dev_err(dev->dev, "failed to change IRQ mask\n");
 
@@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq, void *dev_id)
 	unsigned int nhandled = 0;
 	struct ksz_device *dev;
 	unsigned int sub_irq;
-	u8 data;
+	u16 data;
 	int ret;
 	u8 n;
 
 	dev = kirq->dev;
 
-	/* Read interrupt status register */
-	ret = ksz_read8(dev, kirq->reg_status, &data);
+	/*
+	 * Most of the KSZ switches have a 8-bits long interrupt status
+	 * register, but the KSZ8463 has a 16-bits long one. The overread here
+	 * is safe because we only iterate over kirq->nirqs in the below loop.
+	 */
+	ret = ksz_read16(dev, kirq->reg_status, &data);
 	if (ret)
 		goto out;
 
@@ -2939,6 +2952,22 @@ static int ksz_irq_common_setup(struct ksz_device *dev, struct ksz_irq *kirq)
 	return ret;
 }
 
+static int ksz8463_girq_setup(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_irq *girq = &dev->girq;
+
+	girq->nirqs = 15;
+	girq->reg_mask = KSZ8463_REG_IER;
+	girq->reg_status = KSZ8463_REG_ISR;
+	girq->masked = 0;
+	snprintf(girq->name, sizeof(girq->name), "global_irq");
+
+	girq->irq_num = dev->irq;
+
+	return ksz_irq_common_setup(dev, girq);
+}
+
 static int ksz_girq_setup(struct ksz_device *dev)
 {
 	struct ksz_irq *girq = &dev->girq;
@@ -3044,7 +3073,10 @@ static int ksz_setup(struct dsa_switch *ds)
 	p->learning = true;
 
 	if (dev->irq > 0) {
-		ret = ksz_girq_setup(dev);
+		if (ksz_is_ksz8463(dev))
+			ret = ksz8463_girq_setup(ds);
+		else
+			ret = ksz_girq_setup(dev);
 		if (ret)
 			return ret;
 
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 929aff4c55de5254defdc1afb52b224b3898233b..67a488a3b5787f93f9e2a9266ce04f6611b56bf8 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -839,6 +839,9 @@ static inline bool ksz_is_sgmii_port(struct ksz_device *dev, int port)
 #define KSZ87XX_INT_PME_MASK		BIT(4)
 
 /* Interrupt */
+#define KSZ8463_REG_ISR			0x190
+#define KSZ8463_REG_IER			0x192
+
 #define REG_SW_PORT_INT_STATUS__1	0x001B
 #define REG_SW_PORT_INT_MASK__1		0x001F
 

-- 
2.53.0

[PATCH net-next v6 2/9] net: dsa: microchip: Decorrelate IRQ domain from port

[Bastien Curutchet (Schneider Electric)]

KSZ8463 has one register holding interrupt bits from both port 1 and 2.
So it has to use one IRQ domain for both of its ports. This conflicts
with the current initialization procedure that ties one IRQ domain to
each port.

Decorrelate IRQ domain from port so a port can use an IRQ domain not
directly related to itself.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 4a2cc57a628f97bd51fcb11057bc4effda9205dd..64afe92a3479ec87b5afc66e489b92787a0fc715 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -1099,18 +1099,18 @@ static void ksz_ptp_msg_irq_free(struct ksz_port *port, u8 n)
 	irq_dispose_mapping(ptpmsg_irq->num);
 }
 
-static int ksz_ptp_msg_irq_setup(struct ksz_port *port, u8 n)
+static int ksz_ptp_msg_irq_setup(struct irq_domain *domain,
+				 struct ksz_port *port, u8 n)
 {
 	u16 ts_reg[] = {REG_PTP_PORT_PDRESP_TS, REG_PTP_PORT_XDELAY_TS,
 			REG_PTP_PORT_SYNC_TS};
 	static const char * const name[] = {"pdresp-msg", "xdreq-msg",
 					    "sync-msg"};
 	const struct ksz_dev_ops *ops = port->ksz_dev->dev_ops;
-	struct ksz_irq *ptpirq = &port->ptpirq;
 	struct ksz_ptp_irq *ptpmsg_irq;
 
 	ptpmsg_irq = &port->ptpmsg_irq[n];
-	ptpmsg_irq->num = irq_create_mapping(ptpirq->domain, n);
+	ptpmsg_irq->num = irq_create_mapping(domain, n);
 	if (!ptpmsg_irq->num)
 		return -EINVAL;
 
@@ -1162,7 +1162,7 @@ int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 		goto out;
 
 	for (irq = 0; irq < ptpirq->nirqs; irq++) {
-		ret = ksz_ptp_msg_irq_setup(port, irq);
+		ret = ksz_ptp_msg_irq_setup(ptpirq->domain, port, irq);
 		if (ret)
 			goto out_ptp_msg;
 	}

-- 
2.53.0

[PATCH net-next v6 3/9] net: dsa: microchip: Decorrelate msg_irq index from IRQ bit offset

[Bastien Curutchet (Schneider Electric)]

KSZ8463 has one register containing all the PTP-related interrupts from
all ports. So it will use one IRQ domain for all of them, leading to 4
interrupt bits to be dispatched in two ports. Current implementation
doesn't allow to do so because the IRQ bit offset is also used as index
to store the struct ptpmsg_irq in the table held by the port.

Add a new input to the setup() function to independently provide the
interrupt bit offset and the ptpmsg_irq index.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 64afe92a3479ec87b5afc66e489b92787a0fc715..79f2210df8588e0a75b4f29dae2d7281ede12a3c 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -1100,7 +1100,7 @@ static void ksz_ptp_msg_irq_free(struct ksz_port *port, u8 n)
 }
 
 static int ksz_ptp_msg_irq_setup(struct irq_domain *domain,
-				 struct ksz_port *port, u8 n)
+				 struct ksz_port *port, u8 index, int irq)
 {
 	u16 ts_reg[] = {REG_PTP_PORT_PDRESP_TS, REG_PTP_PORT_XDELAY_TS,
 			REG_PTP_PORT_SYNC_TS};
@@ -1109,15 +1109,15 @@ static int ksz_ptp_msg_irq_setup(struct irq_domain *domain,
 	const struct ksz_dev_ops *ops = port->ksz_dev->dev_ops;
 	struct ksz_ptp_irq *ptpmsg_irq;
 
-	ptpmsg_irq = &port->ptpmsg_irq[n];
-	ptpmsg_irq->num = irq_create_mapping(domain, n);
+	ptpmsg_irq = &port->ptpmsg_irq[index];
+	ptpmsg_irq->num = irq_create_mapping(domain, irq);
 	if (!ptpmsg_irq->num)
 		return -EINVAL;
 
 	ptpmsg_irq->port = port;
-	ptpmsg_irq->ts_reg = ops->get_port_addr(port->num, ts_reg[n]);
+	ptpmsg_irq->ts_reg = ops->get_port_addr(port->num, ts_reg[index]);
 
-	strscpy(ptpmsg_irq->name, name[n]);
+	strscpy(ptpmsg_irq->name, name[index]);
 
 	return request_threaded_irq(ptpmsg_irq->num, NULL,
 				    ksz_ptp_msg_thread_fn, IRQF_ONESHOT,
@@ -1162,7 +1162,7 @@ int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 		goto out;
 
 	for (irq = 0; irq < ptpirq->nirqs; irq++) {
-		ret = ksz_ptp_msg_irq_setup(ptpirq->domain, port, irq);
+		ret = ksz_ptp_msg_irq_setup(ptpirq->domain, port, irq, irq);
 		if (ret)
 			goto out_ptp_msg;
 	}

-- 
2.53.0

[PATCH net-next v6 4/9] net: dsa: microchip: Add support for KSZ8463's PTP interrupts

[Bastien Curutchet (Schneider Electric)]

KSZ8463 PTP interrupts aren't handled by the driver.
The interrupt layout in KSZ8463 has nothing to do with the other
switches:
- all the interrupts of all ports are grouped into one status register
  while others have one interrupt register per port
- xdelay_req and pdresp timestamps share one single interrupt bit on the
  KSZ8463 while each of them has its own interrupt bit on other switches

Add KSZ8463-specific IRQ setup()/free() functions to support KSZ8463.
Both ports share one IRQ domain held by port n°1.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_common.c  |  49 +++++++++-----
 drivers/net/dsa/microchip/ksz_common.h  |   2 +
 drivers/net/dsa/microchip/ksz_ptp.c     | 116 +++++++++++++++++++++++++++++++-
 drivers/net/dsa/microchip/ksz_ptp.h     |   9 +++
 drivers/net/dsa/microchip/ksz_ptp_reg.h |   7 ++
 5 files changed, 164 insertions(+), 19 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 8179415d9b3593d4a9d17661b4ed30007dea9958..3e533a0251358ddf210b3836d534a3837f7a3321 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3080,15 +3080,21 @@ static int ksz_setup(struct dsa_switch *ds)
 		if (ret)
 			return ret;
 
-		dsa_switch_for_each_user_port(dp, dev->ds) {
-			ret = ksz_pirq_setup(dev, dp->index);
+		if (ksz_is_ksz8463(dev)) {
+			ret = ksz8463_ptp_irq_setup(ds);
 			if (ret)
-				goto port_release;
-
-			if (dev->info->ptp_capable) {
-				ret = ksz_ptp_irq_setup(ds, dp->index);
+				goto girq_release;
+		} else {
+			dsa_switch_for_each_user_port(dp, dev->ds) {
+				ret = ksz_pirq_setup(dev, dp->index);
 				if (ret)
-					goto pirq_release;
+					goto port_release;
+
+				if (dev->info->ptp_capable) {
+					ret = ksz_ptp_irq_setup(ds, dp->index);
+					if (ret)
+						goto pirq_release;
+				}
 			}
 		}
 	}
@@ -3123,14 +3129,20 @@ static int ksz_setup(struct dsa_switch *ds)
 		ksz_ptp_clock_unregister(ds);
 port_release:
 	if (dev->irq > 0) {
-		dsa_switch_for_each_user_port_continue_reverse(dp, dev->ds) {
-			if (dev->info->ptp_capable)
-				ksz_ptp_irq_free(ds, dp->index);
+		if (ksz_is_ksz8463(dev)) {
+			ksz8463_ptp_irq_free(ds);
+		} else {
+			dsa_switch_for_each_user_port_continue_reverse(dp, dev->ds) {
+				if (dev->info->ptp_capable)
+					ksz_ptp_irq_free(ds, dp->index);
 pirq_release:
-			ksz_irq_free(&dev->ports[dp->index].pirq);
+				ksz_irq_free(&dev->ports[dp->index].pirq);
+			}
 		}
-		ksz_irq_free(&dev->girq);
 	}
+girq_release:
+	if (dev->irq > 0)
+		ksz_irq_free(&dev->girq);
 
 	return ret;
 }
@@ -3144,11 +3156,14 @@ static void ksz_teardown(struct dsa_switch *ds)
 		ksz_ptp_clock_unregister(ds);
 
 	if (dev->irq > 0) {
-		dsa_switch_for_each_user_port(dp, dev->ds) {
-			if (dev->info->ptp_capable)
-				ksz_ptp_irq_free(ds, dp->index);
-
-			ksz_irq_free(&dev->ports[dp->index].pirq);
+		if (ksz_is_ksz8463(dev)) {
+			ksz8463_ptp_irq_free(ds);
+		} else {
+			dsa_switch_for_each_user_port(dp, dev->ds) {
+				if (dev->info->ptp_capable)
+					ksz_ptp_irq_free(ds, dp->index);
+				ksz_irq_free(&dev->ports[dp->index].pirq);
+			}
 		}
 
 		ksz_irq_free(&dev->girq);
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 67a488a3b5787f93f9e2a9266ce04f6611b56bf8..dfbc3d13daca8d7a8b9d3ffe6a7c1ec9927863f2 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -851,6 +851,8 @@ static inline bool ksz_is_sgmii_port(struct ksz_device *dev, int port)
 #define PORT_SRC_PHY_INT		1
 #define PORT_SRC_PTP_INT		2
 
+#define KSZ8463_SRC_PTP_INT		12
+
 #define KSZ8795_HUGE_PACKET_SIZE	2000
 #define KSZ8863_HUGE_PACKET_SIZE	1916
 #define KSZ8863_NORMAL_PACKET_SIZE	1536
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 79f2210df8588e0a75b4f29dae2d7281ede12a3c..dab7016c6f38a37ed6e2dc60a1f1251aea6a7cbc 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -31,6 +31,9 @@
 #define KSZ_PTP_SUBNS_BITS 32
 
 #define KSZ_PTP_INT_START 13
+#define KSZ8463_PTP_PORT1_INT_START 12
+#define KSZ8463_PTP_PORT2_INT_START 14
+#define KSZ8463_PTP_INT_START KSZ8463_PTP_PORT1_INT_START
 
 static int ksz_ptp_tou_gpio(struct ksz_device *dev)
 {
@@ -1102,6 +1105,7 @@ static void ksz_ptp_msg_irq_free(struct ksz_port *port, u8 n)
 static int ksz_ptp_msg_irq_setup(struct irq_domain *domain,
 				 struct ksz_port *port, u8 index, int irq)
 {
+	static const char * const ksz8463_name[] = {"sync-msg", "delay-msg"};
 	u16 ts_reg[] = {REG_PTP_PORT_PDRESP_TS, REG_PTP_PORT_XDELAY_TS,
 			REG_PTP_PORT_SYNC_TS};
 	static const char * const name[] = {"pdresp-msg", "xdreq-msg",
@@ -1115,15 +1119,110 @@ static int ksz_ptp_msg_irq_setup(struct irq_domain *domain,
 		return -EINVAL;
 
 	ptpmsg_irq->port = port;
-	ptpmsg_irq->ts_reg = ops->get_port_addr(port->num, ts_reg[index]);
 
-	strscpy(ptpmsg_irq->name, name[index]);
+	if (ksz_is_ksz8463(port->ksz_dev)) {
+		ts_reg[0] = KSZ8463_REG_PORT_SYNC_TS;
+		ts_reg[1] = KSZ8463_REG_PORT_DREQ_TS;
+		strscpy(ptpmsg_irq->name, ksz8463_name[index]);
+	} else {
+		strscpy(ptpmsg_irq->name, name[index]);
+	}
+
+	ptpmsg_irq->ts_reg = ops->get_port_addr(port->num, ts_reg[index]);
 
 	return request_threaded_irq(ptpmsg_irq->num, NULL,
 				    ksz_ptp_msg_thread_fn, IRQF_ONESHOT,
 				    ptpmsg_irq->name, ptpmsg_irq);
 }
 
+static int ksz8463_ptp_port_irq_setup(struct ksz_irq *ptpirq,
+				      struct ksz_port *port, int hw_irq)
+{
+	int ret;
+	int i;
+
+	init_completion(&port->tstamp_msg_comp);
+
+	for (i = 0; i < 2; i++) {
+		ret = ksz_ptp_msg_irq_setup(ptpirq->domain, port, i, hw_irq++);
+		if (ret)
+			goto release_msg_irq;
+	}
+
+	return 0;
+
+release_msg_irq:
+	while (i--)
+		ksz_ptp_msg_irq_free(port, i);
+
+	return ret;
+}
+
+static void ksz8463_ptp_port_irq_teardown(struct ksz_port *port)
+{
+	int i;
+
+	for (i = 0; i < 2; i++)
+		ksz_ptp_msg_irq_free(port, i);
+}
+
+int ksz8463_ptp_irq_setup(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_port *port1, *port2;
+	struct ksz_irq *ptpirq;
+	int ret;
+
+	port1 = &dev->ports[0];
+	port2 = &dev->ports[1];
+	ptpirq = &port1->ptpirq;
+
+	ptpirq->irq_num = irq_find_mapping(dev->girq.domain,
+					   KSZ8463_SRC_PTP_INT);
+	if (!ptpirq->irq_num)
+		return -EINVAL;
+
+	ptpirq->dev = dev;
+	ptpirq->nirqs = 4;
+	ptpirq->reg_mask = KSZ8463_PTP_TS_IER;
+	ptpirq->reg_status = KSZ8463_PTP_TS_ISR;
+	ptpirq->irq0_offset = KSZ8463_PTP_INT_START;
+	snprintf(ptpirq->name, sizeof(ptpirq->name), "ptp-irq");
+
+	ptpirq->domain = irq_domain_create_linear(dev_fwnode(dev->dev),
+						  ptpirq->nirqs,
+						  &ksz_ptp_irq_domain_ops,
+						  ptpirq);
+	if (!ptpirq->domain)
+		return -ENOMEM;
+
+	ret = request_threaded_irq(ptpirq->irq_num, NULL, ksz_ptp_irq_thread_fn,
+				   IRQF_ONESHOT, ptpirq->name, ptpirq);
+	if (ret)
+		goto release_domain;
+
+	ret = ksz8463_ptp_port_irq_setup(ptpirq, port1,
+					 KSZ8463_PTP_PORT1_INT_START - KSZ8463_PTP_INT_START);
+	if (ret)
+		goto release_irq;
+
+	ret = ksz8463_ptp_port_irq_setup(ptpirq, port2,
+					 KSZ8463_PTP_PORT2_INT_START - KSZ8463_PTP_INT_START);
+	if (ret)
+		goto free_port1;
+
+	return 0;
+
+free_port1:
+	ksz8463_ptp_port_irq_teardown(port1);
+release_irq:
+	free_irq(ptpirq->irq_num, ptpirq);
+release_domain:
+	irq_domain_remove(ptpirq->domain);
+
+	return ret;
+}
+
 int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 {
 	struct ksz_device *dev = ds->priv;
@@ -1181,6 +1280,19 @@ int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 	return ret;
 }
 
+void ksz8463_ptp_irq_free(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_port *port1 = &dev->ports[0];
+	struct ksz_port *port2 = &dev->ports[1];
+	struct ksz_irq *ptpirq = &port1->ptpirq;
+
+	ksz8463_ptp_port_irq_teardown(port1);
+	ksz8463_ptp_port_irq_teardown(port2);
+	free_irq(ptpirq->irq_num, ptpirq);
+	irq_domain_remove(ptpirq->domain);
+}
+
 void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p)
 {
 	struct ksz_device *dev = ds->priv;
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 3086e519b1b641e9e4126cb6ff43409f6d7f29a5..46494caacc4287b845b8e5c3a68bcfc7a03bcf9d 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -48,6 +48,8 @@ void ksz_port_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb);
 void ksz_port_deferred_xmit(struct kthread_work *work);
 bool ksz_port_rxtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb,
 		       unsigned int type);
+int ksz8463_ptp_irq_setup(struct dsa_switch *ds);
+void ksz8463_ptp_irq_free(struct dsa_switch *ds);
 int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p);
 void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p);
 
@@ -65,6 +67,13 @@ static inline int ksz_ptp_clock_register(struct dsa_switch *ds)
 
 static inline void ksz_ptp_clock_unregister(struct dsa_switch *ds) { }
 
+static inline int ksz8463_ptp_irq_setup(struct dsa_switch *ds)
+{
+	return 0;
+}
+
+static inline void ksz8463_ptp_irq_free(struct dsa_switch *ds) {}
+
 static inline int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 {
 	return 0;
diff --git a/drivers/net/dsa/microchip/ksz_ptp_reg.h b/drivers/net/dsa/microchip/ksz_ptp_reg.h
index eab9aecb7fa8a50323de4140695b2004d1beab8c..e80fb4bd1a0e970ba3570374d3dc82c8e2cc15b4 100644
--- a/drivers/net/dsa/microchip/ksz_ptp_reg.h
+++ b/drivers/net/dsa/microchip/ksz_ptp_reg.h
@@ -121,6 +121,10 @@
 #define REG_PTP_PORT_SYNC_TS		0x0C0C
 #define REG_PTP_PORT_PDRESP_TS		0x0C10
 
+#define KSZ8463_REG_PORT_DREQ_TS	0x0648
+#define KSZ8463_REG_PORT_SYNC_TS	0x064C
+#define KSZ8463_REG_PORT_DRESP_TS	0x0650
+
 #define REG_PTP_PORT_TX_INT_STATUS__2	0x0C14
 #define REG_PTP_PORT_TX_INT_ENABLE__2	0x0C16
 
@@ -131,4 +135,7 @@
 #define KSZ_XDREQ_MSG			1
 #define KSZ_PDRES_MSG			0
 
+#define KSZ8463_PTP_TS_ISR		0x68C
+#define KSZ8463_PTP_TS_IER		0x68E
+
 #endif

-- 
2.53.0

[PATCH net-next v6 5/9] net: dsa: tag_ksz: Share code for KSZ8795 and KSZ9893 xmit operations

[Bastien Curutchet (Schneider Electric)]

KSZ8795 and KSZ9893 have very similar tag handling in the xmit path,
leading to code duplication.

There are only two differences between the two ksz*_xmit():
- the KSZ8795 doesn't handle priorities between frames
- ksz8795_xmit() directly returns the SKB instead of calling
ksz_defer_xmit(). Yet, ksz_defer_xmit() also returns directly the SKB
if no clone is present inside the SKB. Clones are only created by the KSZ
driver when the PTP feature is enabled. Since KSZ8795 doesn't support
PTP, returning the SKB directly or ksz_defer_xmit() is the same.

The upcoming support for the KSZ8463 also requires a similar xmit().

Move KSZ8795 operations below the definition of ksz_defer_xmit().

Gather the common code from ksz8795_xmit() and ksz9893_xmit() into a new
ksz_common_xmit() function that takes three input arguments:
- do_tstamp to tell whether ksz_xmit_timestamp() should be called
- prio to give the priority tag (if any)
- override_mask to give the location of the override bit (if any)

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 net/dsa/tag_ksz.c | 161 +++++++++++++++++++++++++++---------------------------
 1 file changed, 80 insertions(+), 81 deletions(-)

diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index d2475c3bbb7d227bb42b0368914275b00fb2784a..6ec9bcb324a953d988ef665a177566159e27027f 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -99,68 +99,6 @@ static struct sk_buff *ksz_common_rcv(struct sk_buff *skb,
 	return skb;
 }
 
-/*
- * For Ingress (Host -> KSZ8795), 1 byte is added before FCS.
- * ---------------------------------------------------------------------------
- * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag(1byte)|FCS(4bytes)
- * ---------------------------------------------------------------------------
- * tag : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
- *
- * For Egress (KSZ8795 -> Host), 1 byte is added before FCS.
- * ---------------------------------------------------------------------------
- * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
- * ---------------------------------------------------------------------------
- * tag0 : zero-based value represents port
- *	  (eg, 0x0=port1, 0x2=port3, 0x3=port4)
- */
-
-#define KSZ8795_TAIL_TAG_EG_PORT_M	GENMASK(1, 0)
-#define KSZ8795_TAIL_TAG_OVERRIDE	BIT(6)
-#define KSZ8795_TAIL_TAG_LOOKUP		BIT(7)
-
-static struct sk_buff *ksz8795_xmit(struct sk_buff *skb, struct net_device *dev)
-{
-	struct ethhdr *hdr;
-	u8 *tag;
-
-	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
-		return NULL;
-
-	/* Tag encoding */
-	tag = skb_put(skb, KSZ_INGRESS_TAG_LEN);
-	hdr = skb_eth_hdr(skb);
-
-	*tag = dsa_xmit_port_mask(skb, dev);
-	if (is_link_local_ether_addr(hdr->h_dest))
-		*tag |= KSZ8795_TAIL_TAG_OVERRIDE;
-
-	return skb;
-}
-
-static struct sk_buff *ksz8795_rcv(struct sk_buff *skb, struct net_device *dev)
-{
-	u8 *tag;
-
-	if (skb_linearize(skb))
-		return NULL;
-
-	tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
-
-	return ksz_common_rcv(skb, dev, tag[0] & KSZ8795_TAIL_TAG_EG_PORT_M,
-			      KSZ_EGRESS_TAG_LEN);
-}
-
-static const struct dsa_device_ops ksz8795_netdev_ops = {
-	.name	= KSZ8795_NAME,
-	.proto	= DSA_TAG_PROTO_KSZ8795,
-	.xmit	= ksz8795_xmit,
-	.rcv	= ksz8795_rcv,
-	.needed_tailroom = KSZ_INGRESS_TAG_LEN,
-};
-
-DSA_TAG_DRIVER(ksz8795_netdev_ops);
-MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8795, KSZ8795_NAME);
-
 /*
  * For Ingress (Host -> KSZ9477), 2/6 bytes are added before FCS.
  * ---------------------------------------------------------------------------
@@ -274,6 +212,35 @@ static struct sk_buff *ksz_defer_xmit(struct dsa_port *dp, struct sk_buff *skb)
 	return NULL;
 }
 
+static struct sk_buff *ksz_common_xmit(struct sk_buff *skb,
+				       struct net_device *dev,
+				       bool do_tstamp,
+				       u8 prio,
+				       u8 override_mask)
+{
+	struct dsa_port *dp = dsa_user_to_port(dev);
+	struct ethhdr *hdr;
+	u8 *tag;
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
+		return NULL;
+
+	/* Tag encoding */
+	if (do_tstamp)
+		ksz_xmit_timestamp(dp, skb);
+
+	tag = skb_put(skb, KSZ_INGRESS_TAG_LEN);
+	hdr = skb_eth_hdr(skb);
+
+	*tag = dsa_xmit_port_mask(skb, dev);
+	*tag |= prio;
+
+	if (is_link_local_ether_addr(hdr->h_dest))
+		*tag |= override_mask;
+
+	return ksz_defer_xmit(dp, skb);
+}
+
 static struct sk_buff *ksz9477_xmit(struct sk_buff *skb,
 				    struct net_device *dev)
 {
@@ -339,35 +306,67 @@ static const struct dsa_device_ops ksz9477_netdev_ops = {
 DSA_TAG_DRIVER(ksz9477_netdev_ops);
 MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9477, KSZ9477_NAME);
 
-#define KSZ9893_TAIL_TAG_PRIO		GENMASK(4, 3)
-#define KSZ9893_TAIL_TAG_OVERRIDE	BIT(5)
-#define KSZ9893_TAIL_TAG_LOOKUP		BIT(6)
+/*
+ * For Ingress (Host -> KSZ8795), 1 byte is added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * tag : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
+ *
+ * For Egress (KSZ8795 -> Host), 1 byte is added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * tag0 : zero-based value represents port
+ *	  (eg, 0x0=port1, 0x2=port3, 0x3=port4)
+ */
 
-static struct sk_buff *ksz9893_xmit(struct sk_buff *skb,
-				    struct net_device *dev)
+#define KSZ8795_TAIL_TAG_EG_PORT_M	GENMASK(1, 0)
+#define KSZ8795_TAIL_TAG_OVERRIDE	BIT(6)
+#define KSZ8795_TAIL_TAG_LOOKUP		BIT(7)
+
+static struct sk_buff *ksz8795_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	return ksz_common_xmit(skb, dev, false, 0, KSZ8795_TAIL_TAG_OVERRIDE);
+}
+
+static struct sk_buff *ksz8795_rcv(struct sk_buff *skb, struct net_device *dev)
 {
-	u16 queue_mapping = skb_get_queue_mapping(skb);
-	u8 prio = netdev_txq_to_tc(dev, queue_mapping);
-	struct dsa_port *dp = dsa_user_to_port(dev);
-	struct ethhdr *hdr;
 	u8 *tag;
 
-	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
+	if (skb_linearize(skb))
 		return NULL;
 
-	/* Tag encoding */
-	ksz_xmit_timestamp(dp, skb);
+	tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
 
-	tag = skb_put(skb, KSZ_INGRESS_TAG_LEN);
-	hdr = skb_eth_hdr(skb);
+	return ksz_common_rcv(skb, dev, tag[0] & KSZ8795_TAIL_TAG_EG_PORT_M,
+			      KSZ_EGRESS_TAG_LEN);
+}
 
-	*tag = dsa_xmit_port_mask(skb, dev);
-	*tag |= FIELD_PREP(KSZ9893_TAIL_TAG_PRIO, prio);
+static const struct dsa_device_ops ksz8795_netdev_ops = {
+	.name	= KSZ8795_NAME,
+	.proto	= DSA_TAG_PROTO_KSZ8795,
+	.xmit	= ksz8795_xmit,
+	.rcv	= ksz8795_rcv,
+	.needed_tailroom = KSZ_INGRESS_TAG_LEN,
+};
 
-	if (is_link_local_ether_addr(hdr->h_dest))
-		*tag |= KSZ9893_TAIL_TAG_OVERRIDE;
+DSA_TAG_DRIVER(ksz8795_netdev_ops);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8795, KSZ8795_NAME);
 
-	return ksz_defer_xmit(dp, skb);
+#define KSZ9893_TAIL_TAG_PRIO		GENMASK(4, 3)
+#define KSZ9893_TAIL_TAG_OVERRIDE	BIT(5)
+#define KSZ9893_TAIL_TAG_LOOKUP		BIT(6)
+
+static struct sk_buff *ksz9893_xmit(struct sk_buff *skb,
+				    struct net_device *dev)
+{
+	u16 queue_mapping = skb_get_queue_mapping(skb);
+	u8 prio = netdev_txq_to_tc(dev, queue_mapping);
+
+	return ksz_common_xmit(skb, dev, true,
+			       FIELD_PREP(KSZ9893_TAIL_TAG_PRIO, prio),
+			       KSZ9893_TAIL_TAG_OVERRIDE);
 }
 
 static const struct dsa_device_ops ksz9893_netdev_ops = {

-- 
2.53.0

[PATCH net-next v6 6/9] net: dsa: microchip: Add KSZ8463 tail tag handling

[Bastien Curutchet (Schneider Electric)]

KSZ8463 uses the KSZ9893 DSA TAG driver. However, the KSZ8463 doesn't
use the tail tag to convey timestamps to the host as KSZ9893 does. It
uses the reserved fields in the PTP header instead.

Add a KSZ8463-specifig DSA_TAG driver to handle KSZ8463 timestamps.
There is no information in the tail tag to distinguish PTP packets from
others so use the ptp_classify_raw() helper to find the PTP packets and
extract the timestamp from their PTP headers.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_common.c |  5 ++-
 include/net/dsa.h                      |  2 ++
 net/dsa/tag_ksz.c                      | 62 ++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 3e533a0251358ddf210b3836d534a3837f7a3321..9ebdeef07833a278fc419ba21ac3fdb4fbec604f 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3584,8 +3584,10 @@ static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds,
 	if (ksz_is_ksz87xx(dev) || ksz_is_8895_family(dev))
 		proto = DSA_TAG_PROTO_KSZ8795;
 
+	if (dev->chip_id == KSZ8463_CHIP_ID)
+		proto = DSA_TAG_PROTO_KSZ8463;
+
 	if (dev->chip_id == KSZ88X3_CHIP_ID ||
-	    dev->chip_id == KSZ8463_CHIP_ID ||
 	    dev->chip_id == KSZ8563_CHIP_ID ||
 	    dev->chip_id == KSZ9893_CHIP_ID ||
 	    dev->chip_id == KSZ9563_CHIP_ID)
@@ -3613,6 +3615,7 @@ static int ksz_connect_tag_protocol(struct dsa_switch *ds,
 	switch (proto) {
 	case DSA_TAG_PROTO_KSZ8795:
 		return 0;
+	case DSA_TAG_PROTO_KSZ8463:
 	case DSA_TAG_PROTO_KSZ9893:
 	case DSA_TAG_PROTO_KSZ9477:
 	case DSA_TAG_PROTO_LAN937X:
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 6c17446f3dcc29682e07a70362bae8d8fa6233b0..e854cde8b400c2f47e5c474460e914e1abbf8984 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -58,6 +58,7 @@ struct tc_action;
 #define DSA_TAG_PROTO_YT921X_VALUE		30
 #define DSA_TAG_PROTO_MXL_GSW1XX_VALUE		31
 #define DSA_TAG_PROTO_MXL862_VALUE		32
+#define DSA_TAG_PROTO_KSZ8463_VALUE		33
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,
@@ -93,6 +94,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_YT921X		= DSA_TAG_PROTO_YT921X_VALUE,
 	DSA_TAG_PROTO_MXL_GSW1XX	= DSA_TAG_PROTO_MXL_GSW1XX_VALUE,
 	DSA_TAG_PROTO_MXL862		= DSA_TAG_PROTO_MXL862_VALUE,
+	DSA_TAG_PROTO_KSZ8463		= DSA_TAG_PROTO_KSZ8463_VALUE,
 };
 
 struct dsa_switch;
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 6ec9bcb324a953d988ef665a177566159e27027f..bdd0e63b2b468c78b92d231b90a09a8f853e261c 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -12,6 +12,7 @@
 
 #include "tag.h"
 
+#define KSZ8463_NAME "ksz8463"
 #define KSZ8795_NAME "ksz8795"
 #define KSZ9477_NAME "ksz9477"
 #define KSZ9893_NAME "ksz9893"
@@ -382,6 +383,66 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
 DSA_TAG_DRIVER(ksz9893_netdev_ops);
 MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893, KSZ9893_NAME);
 
+#define KSZ8463_TAIL_TAG_PRIO		GENMASK(4, 3)
+#define KSZ8463_TAIL_TAG_EG_PORT_M	GENMASK(2, 0)
+
+static struct sk_buff *ksz8463_xmit(struct sk_buff *skb,
+				    struct net_device *dev)
+{
+	u16 queue_mapping = skb_get_queue_mapping(skb);
+	u8 prio = netdev_txq_to_tc(dev, queue_mapping);
+
+	return ksz_common_xmit(skb, dev, false,
+			       FIELD_PREP(KSZ8463_TAIL_TAG_PRIO, prio),
+			       0);
+}
+
+static struct sk_buff *ksz8463_rcv(struct sk_buff *skb, struct net_device *dev)
+{
+	unsigned int len = KSZ_EGRESS_TAG_LEN;
+	struct ptp_header *ptp_hdr;
+	unsigned int ptp_class;
+	unsigned int port;
+	ktime_t ts;
+	u8 *tag;
+
+	if (skb_linearize(skb))
+		return NULL;
+
+	/* Tag decoding */
+	tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
+	port = tag[0] & KSZ8463_TAIL_TAG_EG_PORT_M;
+
+	__skb_push(skb, ETH_HLEN);
+	ptp_class = ptp_classify_raw(skb);
+	__skb_pull(skb, ETH_HLEN);
+	if (ptp_class == PTP_CLASS_NONE)
+		goto common_rcv;
+
+	ptp_hdr = ptp_parse_header(skb, ptp_class);
+	if (ptp_hdr) {
+		ts = ksz_decode_tstamp(get_unaligned_be32(&ptp_hdr->reserved2));
+		KSZ_SKB_CB(skb)->tstamp = ts;
+		ptp_hdr->reserved2 = 0;
+	}
+
+common_rcv:
+	return ksz_common_rcv(skb, dev, port, len);
+}
+
+static const struct dsa_device_ops ksz8463_netdev_ops = {
+	.name	= KSZ8463_NAME,
+	.proto	= DSA_TAG_PROTO_KSZ8463,
+	.xmit	= ksz8463_xmit,
+	.rcv	= ksz8463_rcv,
+	.connect = ksz_connect,
+	.disconnect = ksz_disconnect,
+	.needed_tailroom = KSZ_INGRESS_TAG_LEN,
+};
+
+DSA_TAG_DRIVER(ksz8463_netdev_ops);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8463, KSZ8463_NAME);
+
 /* For xmit, 2/6 bytes are added before FCS.
  * ---------------------------------------------------------------------------
  * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|tag1(1byte)|
@@ -452,6 +513,7 @@ DSA_TAG_DRIVER(lan937x_netdev_ops);
 MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937X, LAN937X_NAME);
 
 static struct dsa_tag_driver *dsa_tag_driver_array[] = {
+	&DSA_TAG_DRIVER_NAME(ksz8463_netdev_ops),
 	&DSA_TAG_DRIVER_NAME(ksz8795_netdev_ops),
 	&DSA_TAG_DRIVER_NAME(ksz9477_netdev_ops),
 	&DSA_TAG_DRIVER_NAME(ksz9893_netdev_ops),

-- 
2.53.0

[PATCH net-next v6 7/9] net: dsa: microchip: Explicitly enable detection of L2 PTP frames

[Bastien Curutchet (Schneider Electric)]

Detection of L2 PTP frames needs to be enabled for PTP to work at the L2
layer. The bit enabling this detection is set by default on the switches
currently supported by the driver, but it is unset by default on the
KSZ8463 for which support will be added in upcoming patches.

Explicitly enable the detection of L2 PTP frames for all switches when
PTP is enabled.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index dab7016c6f38a37ed6e2dc60a1f1251aea6a7cbc..7bb5191e780f151fe74ff0163ea1fa8b0f3ebee9 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -947,8 +947,9 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
 	/* Currently only P2P mode is supported. When 802_1AS bit is set, it
 	 * forwards all PTP packets to host port and none to other ports.
 	 */
-	ret = ksz_rmw16(dev, regs[PTP_MSG_CONF1], PTP_TC_P2P | PTP_802_1AS,
-			PTP_TC_P2P | PTP_802_1AS);
+	ret = ksz_rmw16(dev, regs[PTP_MSG_CONF1],
+			PTP_TC_P2P | PTP_802_1AS | PTP_ETH_ENABLE,
+			PTP_TC_P2P | PTP_802_1AS | PTP_ETH_ENABLE);
 	if (ret)
 		return ret;
 

-- 
2.53.0

[PATCH net-next v6 8/9] net: dsa: microchip: Adapt port offset for KSZ8463's PTP register

[Bastien Curutchet (Schneider Electric)]

In KSZ8463 register's layout, the offset between port 1 and port 2
registers isn't the same in the generic control register area than in
the PTP register area. The get_port_addr() always uses the same offset
so it doesn't work when it's used to access PTP registers.

Adapt the port offset in get_port_addr() when the accessed register is
in the PTP area.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz8.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
index c354abdafc1b542a32c276ef939a90db30c67f55..a05527899b8bab6d53509ba38c58101b79e98ee5 100644
--- a/drivers/net/dsa/microchip/ksz8.c
+++ b/drivers/net/dsa/microchip/ksz8.c
@@ -2020,6 +2020,9 @@ u32 ksz8_get_port_addr(int port, int offset)
 
 u32 ksz8463_get_port_addr(int port, int offset)
 {
+	if (offset >= KSZ8463_PTP_CLK_CTRL)
+		return offset + 0x20 * port;
+
 	return offset + 0x18 * port;
 }
 

-- 
2.53.0

[PATCH net-next v6 9/9] net: dsa: microchip: Add two-step PTP support for KSZ8463

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 switch supports PTP but it's not supported by driver.

Add L2 two-step PTP support for the KSZ8463. IPv4 and IPv6 layers aren't
supported. Neither is one-step PTP.

The pdelay_req and pdelay_resp timestamps share one interrupt bit status
while they're located in two different registers. So introduce
last_tx_is_pdelayresp to keep track of the last sent event type. This
flag is set by the xmit worker right before sending the packet and then
used in the interrupt handler to retrieve the timestamp location.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz8.c        | 11 +++--
 drivers/net/dsa/microchip/ksz8_reg.h    |  1 +
 drivers/net/dsa/microchip/ksz_common.c  |  1 +
 drivers/net/dsa/microchip/ksz_common.h  |  1 +
 drivers/net/dsa/microchip/ksz_ptp.c     | 73 ++++++++++++++++++++++++++++-----
 drivers/net/dsa/microchip/ksz_ptp_reg.h |  4 ++
 6 files changed, 74 insertions(+), 17 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
index a05527899b8bab6d53509ba38c58101b79e98ee5..718a575997c187a2379e32b2bd3075c69aafa9e3 100644
--- a/drivers/net/dsa/microchip/ksz8.c
+++ b/drivers/net/dsa/microchip/ksz8.c
@@ -143,9 +143,11 @@ int ksz8_reset_switch(struct ksz_device *dev)
 			KSZ8863_GLOBAL_SOFTWARE_RESET | KSZ8863_PCS_RESET, false);
 	} else if (ksz_is_ksz8463(dev)) {
 		ksz_cfg(dev, KSZ8463_REG_SW_RESET,
-			KSZ8463_GLOBAL_SOFTWARE_RESET, true);
+			KSZ8463_GLOBAL_SOFTWARE_RESET | KSZ8463_PTP_SOFTWARE_RESET,
+			true);
 		ksz_cfg(dev, KSZ8463_REG_SW_RESET,
-			KSZ8463_GLOBAL_SOFTWARE_RESET, false);
+			KSZ8463_GLOBAL_SOFTWARE_RESET | KSZ8463_PTP_SOFTWARE_RESET,
+			false);
 	} else {
 		/* reset switch */
 		ksz_write8(dev, REG_POWER_MANAGEMENT_1,
@@ -1763,10 +1765,7 @@ void ksz8_config_cpu_port(struct dsa_switch *ds)
 					   COPPER_RECEIVE_ADJUSTMENT, 0);
 		}
 
-		/* Turn off PTP function as the switch's proprietary way of
-		 * handling timestamp is not supported in current Linux PTP
-		 * stack implementation.
-		 */
+		/* Turn off PTP function as the switch enables it by default */
 		regmap_update_bits(ksz_regmap_16(dev),
 				   KSZ8463_PTP_MSG_CONF1,
 				   PTP_ENABLE, 0);
diff --git a/drivers/net/dsa/microchip/ksz8_reg.h b/drivers/net/dsa/microchip/ksz8_reg.h
index 332408567b473c141c3695328a524f257f2cfc70..0558740ae57738fa7e4a8f3f429254033c54af12 100644
--- a/drivers/net/dsa/microchip/ksz8_reg.h
+++ b/drivers/net/dsa/microchip/ksz8_reg.h
@@ -765,6 +765,7 @@
 #define KSZ8463_REG_SW_RESET		0x126
 
 #define KSZ8463_GLOBAL_SOFTWARE_RESET	BIT(0)
+#define KSZ8463_PTP_SOFTWARE_RESET	BIT(2)
 
 #define KSZ8463_PTP_CLK_CTRL		0x600
 
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 9ebdeef07833a278fc419ba21ac3fdb4fbec604f..f7a8767b2d2adbeb074814ef3bc36e83c4d02174 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1512,6 +1512,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.supports_mii = {false, false, true},
 		.supports_rmii = {false, false, true},
 		.internal_phy = {true, true, false},
+		.ptp_capable = true,
 	},
 
 	[KSZ8563] = {
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index dfbc3d13daca8d7a8b9d3ffe6a7c1ec9927863f2..1fface82086eed87749d4702b046fcab313663e9 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -150,6 +150,7 @@ struct ksz_port {
 	struct kernel_hwtstamp_config tstamp_config;
 	bool hwts_tx_en;
 	bool hwts_rx_en;
+	bool last_tx_is_pdelayresp;
 	struct ksz_irq ptpirq;
 	struct ksz_ptp_irq ptpmsg_irq[3];
 	ktime_t tstamp_msg;
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 7bb5191e780f151fe74ff0163ea1fa8b0f3ebee9..f3124bce0a2ff9a3663e50a306c3caadcfa53a27 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -308,15 +308,20 @@ int ksz_get_ts_info(struct dsa_switch *ds, int port, struct kernel_ethtool_ts_in
 			      SOF_TIMESTAMPING_RX_HARDWARE |
 			      SOF_TIMESTAMPING_RAW_HARDWARE;
 
-	ts->tx_types = BIT(HWTSTAMP_TX_OFF) | BIT(HWTSTAMP_TX_ONESTEP_P2P);
+	ts->tx_types = BIT(HWTSTAMP_TX_OFF);
 
-	if (is_lan937x(dev))
+	if (!ksz_is_ksz8463(dev))
+		ts->tx_types |= BIT(HWTSTAMP_TX_ONESTEP_P2P);
+
+	if (is_lan937x(dev) || ksz_is_ksz8463(dev))
 		ts->tx_types |= BIT(HWTSTAMP_TX_ON);
 
 	ts->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
-			 BIT(HWTSTAMP_FILTER_PTP_V2_L4_EVENT) |
-			 BIT(HWTSTAMP_FILTER_PTP_V2_L2_EVENT) |
-			 BIT(HWTSTAMP_FILTER_PTP_V2_EVENT);
+			 BIT(HWTSTAMP_FILTER_PTP_V2_L2_EVENT);
+	if (!ksz_is_ksz8463(dev)) {
+		ts->rx_filters |= BIT(HWTSTAMP_FILTER_PTP_V2_L4_EVENT) |
+				  BIT(HWTSTAMP_FILTER_PTP_V2_EVENT);
+	}
 
 	ts->phc_index = ptp_clock_index(ptp_data->clock);
 
@@ -353,6 +358,9 @@ static int ksz_set_hwtstamp_config(struct ksz_device *dev,
 		prt->hwts_tx_en = false;
 		break;
 	case HWTSTAMP_TX_ONESTEP_P2P:
+		if (ksz_is_ksz8463(dev))
+			return -ERANGE;
+
 		prt->ptpmsg_irq[KSZ_SYNC_MSG].ts_en  = false;
 		prt->ptpmsg_irq[KSZ_XDREQ_MSG].ts_en = true;
 		prt->ptpmsg_irq[KSZ_PDRES_MSG].ts_en = false;
@@ -364,14 +372,19 @@ static int ksz_set_hwtstamp_config(struct ksz_device *dev,
 
 		break;
 	case HWTSTAMP_TX_ON:
-		if (!is_lan937x(dev))
+		if (!is_lan937x(dev) && !ksz_is_ksz8463(dev))
 			return -ERANGE;
 
-		prt->ptpmsg_irq[KSZ_SYNC_MSG].ts_en  = true;
-		prt->ptpmsg_irq[KSZ_XDREQ_MSG].ts_en = true;
-		prt->ptpmsg_irq[KSZ_PDRES_MSG].ts_en = true;
-		prt->hwts_tx_en = true;
+		if (ksz_is_ksz8463(dev)) {
+			prt->ptpmsg_irq[KSZ8463_SYNC_MSG].ts_en  = true;
+			prt->ptpmsg_irq[KSZ8463_XDREQ_PDRES_MSG].ts_en = true;
+		} else {
+			prt->ptpmsg_irq[KSZ_SYNC_MSG].ts_en  = true;
+			prt->ptpmsg_irq[KSZ_XDREQ_MSG].ts_en = true;
+			prt->ptpmsg_irq[KSZ_PDRES_MSG].ts_en = true;
+		}
 
+		prt->hwts_tx_en = true;
 		ret = ksz_rmw16(dev, regs[PTP_MSG_CONF1], PTP_1STEP, 0);
 		if (ret)
 			return ret;
@@ -387,6 +400,8 @@ static int ksz_set_hwtstamp_config(struct ksz_device *dev,
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
+		if (ksz_is_ksz8463(dev))
+			return -ERANGE;
 		config->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
 		prt->hwts_rx_en = true;
 		break;
@@ -397,6 +412,8 @@ static int ksz_set_hwtstamp_config(struct ksz_device *dev,
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_SYNC:
+		if (ksz_is_ksz8463(dev))
+			return -ERANGE;
 		config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
 		prt->hwts_rx_en = true;
 		break;
@@ -565,6 +582,28 @@ static void ksz_ptp_txtstamp_skb(struct ksz_device *dev,
 	skb_complete_tx_timestamp(skb, &hwtstamps);
 }
 
+static void ksz8463_set_pdelayresp_flag(struct ksz_port *prt,
+					struct sk_buff *skb)
+{
+	struct ptp_header *hdr;
+	unsigned int type;
+	u8 ptp_msg_type;
+
+	if (!ksz_is_ksz8463(prt->ksz_dev))
+		return;
+
+	type = ptp_classify_raw(skb);
+	if (type == PTP_CLASS_NONE)
+		return;
+
+	hdr = ptp_parse_header(skb, type);
+	if (!hdr)
+		return;
+
+	ptp_msg_type = ptp_get_msgtype(hdr, type);
+	prt->last_tx_is_pdelayresp = (ptp_msg_type == PTP_MSGTYPE_PDELAY_RESP);
+}
+
 void ksz_port_deferred_xmit(struct kthread_work *work)
 {
 	struct ksz_deferred_xmit_work *xmit_work = work_to_xmit_work(work);
@@ -581,6 +620,8 @@ void ksz_port_deferred_xmit(struct kthread_work *work)
 
 	reinit_completion(&prt->tstamp_msg_comp);
 
+	ksz8463_set_pdelayresp_flag(prt, skb);
+
 	dsa_enqueue_skb(skb, skb->dev);
 
 	ksz_ptp_txtstamp_skb(dev, prt, clone);
@@ -973,7 +1014,17 @@ void ksz_ptp_clock_unregister(struct dsa_switch *ds)
 
 static int ksz_read_ts(struct ksz_port *port, u16 reg, u32 *ts)
 {
-	return ksz_read32(port->ksz_dev, reg, ts);
+	u16 ts_reg = reg;
+
+	/**
+	 * On KSZ8463 DREQ and DRESP timestamps share one interrupt line
+	 * so we have to check the nature of the latest event sent to know
+	 * where the timestamp is located
+	 */
+	if (ksz_is_ksz8463(port->ksz_dev) && port->last_tx_is_pdelayresp)
+		ts_reg += KSZ8463_DRESP_TS_OFFSET;
+
+	return ksz_read32(port->ksz_dev, ts_reg, ts);
 }
 
 static irqreturn_t ksz_ptp_msg_thread_fn(int irq, void *dev_id)
diff --git a/drivers/net/dsa/microchip/ksz_ptp_reg.h b/drivers/net/dsa/microchip/ksz_ptp_reg.h
index e80fb4bd1a0e970ba3570374d3dc82c8e2cc15b4..ac9d0f2b348b0469abbeed0e645fe8ef441d35fb 100644
--- a/drivers/net/dsa/microchip/ksz_ptp_reg.h
+++ b/drivers/net/dsa/microchip/ksz_ptp_reg.h
@@ -125,6 +125,10 @@
 #define KSZ8463_REG_PORT_SYNC_TS	0x064C
 #define KSZ8463_REG_PORT_DRESP_TS	0x0650
 
+#define KSZ8463_DRESP_TS_OFFSET		(KSZ8463_REG_PORT_DRESP_TS - KSZ8463_REG_PORT_DREQ_TS)
+#define KSZ8463_SYNC_MSG		0
+#define KSZ8463_XDREQ_PDRES_MSG		1
+
 #define REG_PTP_PORT_TX_INT_STATUS__2	0x0C14
 #define REG_PTP_PORT_TX_INT_ENABLE__2	0x0C16
 

-- 
2.53.0

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean]

On Wed, Mar 04, 2026 at 11:18:52AM +0100, Bastien Curutchet (Schneider Electric) wrote:
> @@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq, void *dev_id)
>  	unsigned int nhandled = 0;
>  	struct ksz_device *dev;
>  	unsigned int sub_irq;
> -	u8 data;
> +	u16 data;
>  	int ret;
>  	u8 n;
>  
>  	dev = kirq->dev;
>  
> -	/* Read interrupt status register */
> -	ret = ksz_read8(dev, kirq->reg_status, &data);
> +	/*
> +	 * Most of the KSZ switches have a 8-bits long interrupt status
> +	 * register, but the KSZ8463 has a 16-bits long one. The overread here
> +	 * is safe because we only iterate over kirq->nirqs in the below loop.

FWIW, this isn't the only thing making an overread "safe".
If the adjacent register also has "clear on read" semantics, that's not
good.

I can't tell whether that's the case here, though. There are just too
many hardware variations to check for. I just wanted to point out that
the reasoning is incomplete.

> +	 */
> +	ret = ksz_read16(dev, kirq->reg_status, &data);

I guess I don't fully understand the KSZ_REGMAP_TABLE() layer, but I
have a concern here.

Take the normal ksz_girq_setup() case for example, where we set
kirq->reg_status = REG_SW_PORT_INT_STATUS__1.

With ksz_read8(), we read u8 data from address REG_SW_PORT_INT_STATUS__1.
With ksz_read16(), how do we read u16 data? Don't we read
data[15:8] from REG_SW_PORT_INT_STATUS__1 and
data[7:0] from REG_SW_PORT_INT_STATUS__1 + 1?
But then we still look at the 0..kirq->nirq bits, effectively from address
REG_SW_PORT_INT_STATUS__1 + 1 now?

Or are the registers at address i 16-bits wide without spilling over
into address i+1? Because if that's the case, I don't understand the
comment/concern about overreading.

>  	if (ret)
>  		goto out;

Re: [PATCH net-next v6 2/9] net: dsa: microchip: Decorrelate IRQ domain from port

[Vladimir Oltean]

On Wed, Mar 04, 2026 at 11:18:53AM +0100, Bastien Curutchet (Schneider Electric) wrote:
> KSZ8463 has one register holding interrupt bits from both port 1 and 2.
> So it has to use one IRQ domain for both of its ports. This conflicts
> with the current initialization procedure that ties one IRQ domain to
> each port.
> 
> Decorrelate IRQ domain from port so a port can use an IRQ domain not
> directly related to itself.
> 
> Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
> ---
>  drivers/net/dsa/microchip/ksz_ptp.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 4a2cc57a628f97bd51fcb11057bc4effda9205dd..64afe92a3479ec87b5afc66e489b92787a0fc715 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -1099,18 +1099,18 @@ static void ksz_ptp_msg_irq_free(struct ksz_port *port, u8 n)
>  	irq_dispose_mapping(ptpmsg_irq->num);
>  }
>  
> -static int ksz_ptp_msg_irq_setup(struct ksz_port *port, u8 n)
> +static int ksz_ptp_msg_irq_setup(struct irq_domain *domain,
> +				 struct ksz_port *port, u8 n)
>  {
>  	u16 ts_reg[] = {REG_PTP_PORT_PDRESP_TS, REG_PTP_PORT_XDELAY_TS,
>  			REG_PTP_PORT_SYNC_TS};
>  	static const char * const name[] = {"pdresp-msg", "xdreq-msg",
>  					    "sync-msg"};
>  	const struct ksz_dev_ops *ops = port->ksz_dev->dev_ops;
> -	struct ksz_irq *ptpirq = &port->ptpirq;
>  	struct ksz_ptp_irq *ptpmsg_irq;
>  
>  	ptpmsg_irq = &port->ptpmsg_irq[n];
> -	ptpmsg_irq->num = irq_create_mapping(ptpirq->domain, n);
> +	ptpmsg_irq->num = irq_create_mapping(domain, n);

Looking at this function, I think there's a pre-existing bug.
If request_threaded_irq() later fails, irq_dispose_mapping() needs to be
called, because ksz_ptp_msg_irq_free() only frees the IRQ domains that
were already successfully set up.

>  	if (!ptpmsg_irq->num)
>  		return -EINVAL;
>  
> @@ -1162,7 +1162,7 @@ int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
>  		goto out;
>  
>  	for (irq = 0; irq < ptpirq->nirqs; irq++) {
> -		ret = ksz_ptp_msg_irq_setup(port, irq);
> +		ret = ksz_ptp_msg_irq_setup(ptpirq->domain, port, irq);
>  		if (ret)
>  			goto out_ptp_msg;
>  	}
> 
> -- 
> 2.53.0
> 

Re: [PATCH net-next v6 4/9] net: dsa: microchip: Add support for KSZ8463's PTP interrupts

[Vladimir Oltean]

On Wed, Mar 04, 2026 at 11:18:55AM +0100, Bastien Curutchet (Schneider Electric) wrote:
> +void ksz8463_ptp_irq_free(struct dsa_switch *ds)
> +{
> +	struct ksz_device *dev = ds->priv;
> +	struct ksz_port *port1 = &dev->ports[0];
> +	struct ksz_port *port2 = &dev->ports[1];
> +	struct ksz_irq *ptpirq = &port1->ptpirq;
> +
> +	ksz8463_ptp_port_irq_teardown(port1);
> +	ksz8463_ptp_port_irq_teardown(port2);
> +	free_irq(ptpirq->irq_num, ptpirq);
> +	irq_domain_remove(ptpirq->domain);
> +}

Teardown in the exact reverse order of ksz8463_ptp_irq_setup(), please
(port1 vs port2). Place them next to each other so it's more obvious.

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet]

Hi Vladimir,

On 3/5/26 10:56 AM, Vladimir Oltean wrote:
> On Wed, Mar 04, 2026 at 11:18:52AM +0100, Bastien Curutchet (Schneider Electric) wrote:
>> @@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq, void *dev_id)
>>   	unsigned int nhandled = 0;
>>   	struct ksz_device *dev;
>>   	unsigned int sub_irq;
>> -	u8 data;
>> +	u16 data;
>>   	int ret;
>>   	u8 n;
>>   
>>   	dev = kirq->dev;
>>   
>> -	/* Read interrupt status register */
>> -	ret = ksz_read8(dev, kirq->reg_status, &data);
>> +	/*
>> +	 * Most of the KSZ switches have a 8-bits long interrupt status
>> +	 * register, but the KSZ8463 has a 16-bits long one. The overread here
>> +	 * is safe because we only iterate over kirq->nirqs in the below loop.
> 
> FWIW, this isn't the only thing making an overread "safe".
> If the adjacent register also has "clear on read" semantics, that's not
> good.
> 
> I can't tell whether that's the case here, though. There are just too
> many hardware variations to check for. I just wanted to point out that
> the reasoning is incomplete.
> 
You're right, I hadn't thought about the 'clear on read' case.

I'll use ksz_read16() only for the KSZ8463 to be 100% sure it won't 
break anything for other switches.


Best regards,
Bastien

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean]

On Thu, Mar 05, 2026 at 01:39:29PM +0100, Bastien Curutchet wrote:
> Hi Vladimir,
> 
> On 3/5/26 10:56 AM, Vladimir Oltean wrote:
> > On Wed, Mar 04, 2026 at 11:18:52AM +0100, Bastien Curutchet (Schneider Electric) wrote:
> > > @@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq, void *dev_id)
> > >   	unsigned int nhandled = 0;
> > >   	struct ksz_device *dev;
> > >   	unsigned int sub_irq;
> > > -	u8 data;
> > > +	u16 data;
> > >   	int ret;
> > >   	u8 n;
> > >   	dev = kirq->dev;
> > > -	/* Read interrupt status register */
> > > -	ret = ksz_read8(dev, kirq->reg_status, &data);
> > > +	/*
> > > +	 * Most of the KSZ switches have a 8-bits long interrupt status
> > > +	 * register, but the KSZ8463 has a 16-bits long one. The overread here
> > > +	 * is safe because we only iterate over kirq->nirqs in the below loop.
> > 
> > FWIW, this isn't the only thing making an overread "safe".
> > If the adjacent register also has "clear on read" semantics, that's not
> > good.
> > 
> > I can't tell whether that's the case here, though. There are just too
> > many hardware variations to check for. I just wanted to point out that
> > the reasoning is incomplete.
> > 
> You're right, I hadn't thought about the 'clear on read' case.
> 
> I'll use ksz_read16() only for the KSZ8463 to be 100% sure it won't break
> anything for other switches.

I'm still on the fence on whether to say this or not, because I don't
really want to get so involved in internal driver bookkeeping, but...
this driver is just becoming a hell to review, even if I want to
concentrate exclusively on correct API use, like I try to do.

Linus Walleij has added a new ks8995 driver, which has some overlap with
the common ksz driver for the KSZ8 family. Now he wants to remove the
overlapping device support:
https://lore.kernel.org/netdev/20260219-ks8995-fixups-v3-0-a7fc63fe1916@kernel.org/

Maybe we should go the other way around, migrate KSZ8 support to the
ks8995 driver instead? The common ksz driver is becoming just extremely
convoluted to handle all hardware variations. Would it help in any way
to maintain cleaner code paths, what do you think?

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet]

On 3/5/26 1:51 PM, Vladimir Oltean wrote:
> On Thu, Mar 05, 2026 at 01:39:29PM +0100, Bastien Curutchet wrote:
>> Hi Vladimir,
>>
>> On 3/5/26 10:56 AM, Vladimir Oltean wrote:
>>> On Wed, Mar 04, 2026 at 11:18:52AM +0100, Bastien Curutchet (Schneider Electric) wrote:
>>>> @@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq, void *dev_id)
>>>>    	unsigned int nhandled = 0;
>>>>    	struct ksz_device *dev;
>>>>    	unsigned int sub_irq;
>>>> -	u8 data;
>>>> +	u16 data;
>>>>    	int ret;
>>>>    	u8 n;
>>>>    	dev = kirq->dev;
>>>> -	/* Read interrupt status register */
>>>> -	ret = ksz_read8(dev, kirq->reg_status, &data);
>>>> +	/*
>>>> +	 * Most of the KSZ switches have a 8-bits long interrupt status
>>>> +	 * register, but the KSZ8463 has a 16-bits long one. The overread here
>>>> +	 * is safe because we only iterate over kirq->nirqs in the below loop.
>>>
>>> FWIW, this isn't the only thing making an overread "safe".
>>> If the adjacent register also has "clear on read" semantics, that's not
>>> good.
>>>
>>> I can't tell whether that's the case here, though. There are just too
>>> many hardware variations to check for. I just wanted to point out that
>>> the reasoning is incomplete.
>>>
>> You're right, I hadn't thought about the 'clear on read' case.
>>
>> I'll use ksz_read16() only for the KSZ8463 to be 100% sure it won't break
>> anything for other switches.
> 
> I'm still on the fence on whether to say this or not, because I don't
> really want to get so involved in internal driver bookkeeping, but...
> this driver is just becoming a hell to review, even if I want to
> concentrate exclusively on correct API use, like I try to do.
> > Linus Walleij has added a new ks8995 driver, which has some overlap with
> the common ksz driver for the KSZ8 family. Now he wants to remove the
> overlapping device support:
> https://lore.kernel.org/netdev/20260219-ks8995-fixups-v3-0-a7fc63fe1916@kernel.org/
> 
> Maybe we should go the other way around, migrate KSZ8 support to the
> ks8995 driver instead? The common ksz driver is becoming just extremely
> convoluted to handle all hardware variations. Would it help in any way
> to maintain cleaner code paths, what do you think?

I agree, this driver is extremely convoluted because of all the 
different hardware it handles. It wasn't easy to fit PTP support for the 
KSZ8463 into it. And I encountered the same kind of difficulties when 
adding periodic output support (I have another series ready for this 
once this PTP support will be merged).

Regarding migrating the KSZ8 support into the ksz8995, I think we would 
quickly hit the same pain points than in ksz_common. Even inside the 
KSZ8 family we can find a quite big amount of differences between the 
switches. For instance, both KSZ8463 and KSZ8563 support PTP, they share 
lots of common registers but their interrupt scheme is very different.

I've added Tristram in Cc:, who works at Microchip. Maybe, Tristram, you 
have some insights about which switches could share code if we decide to 
split the big ksz_common into several drivers ?


Best regards,
Bastien

RE: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Tristram.Ha]

> On 3/5/26 1:51 PM, Vladimir Oltean wrote:
> > On Thu, Mar 05, 2026 at 01:39:29PM +0100, Bastien Curutchet wrote:
> >> Hi Vladimir,
> >>
> >> On 3/5/26 10:56 AM, Vladimir Oltean wrote:
> >>> On Wed, Mar 04, 2026 at 11:18:52AM +0100, Bastien Curutchet (Schneider
> Electric) wrote:
> >>>> @@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq,
> void *dev_id)
> >>>>            unsigned int nhandled = 0;
> >>>>            struct ksz_device *dev;
> >>>>            unsigned int sub_irq;
> >>>> -  u8 data;
> >>>> +  u16 data;
> >>>>            int ret;
> >>>>            u8 n;
> >>>>            dev = kirq->dev;
> >>>> -  /* Read interrupt status register */
> >>>> -  ret = ksz_read8(dev, kirq->reg_status, &data);
> >>>> +  /*
> >>>> +   * Most of the KSZ switches have a 8-bits long interrupt status
> >>>> +   * register, but the KSZ8463 has a 16-bits long one. The overread here
> >>>> +   * is safe because we only iterate over kirq->nirqs in the below loop.
> >>>
> >>> FWIW, this isn't the only thing making an overread "safe".
> >>> If the adjacent register also has "clear on read" semantics, that's not
> >>> good.
> >>>
> >>> I can't tell whether that's the case here, though. There are just too
> >>> many hardware variations to check for. I just wanted to point out that
> >>> the reasoning is incomplete.
> >>>
> >> You're right, I hadn't thought about the 'clear on read' case.
> >>
> >> I'll use ksz_read16() only for the KSZ8463 to be 100% sure it won't break
> >> anything for other switches.
> >
> > I'm still on the fence on whether to say this or not, because I don't
> > really want to get so involved in internal driver bookkeeping, but...
> > this driver is just becoming a hell to review, even if I want to
> > concentrate exclusively on correct API use, like I try to do.
> > > Linus Walleij has added a new ks8995 driver, which has some overlap with
> > the common ksz driver for the KSZ8 family. Now he wants to remove the
> > overlapping device support:
> > https://lore.kernel.org/netdev/20260219-ks8995-fixups-v3-0-
> a7fc63fe1916@kernel.org/
> >
> > Maybe we should go the other way around, migrate KSZ8 support to the
> > ks8995 driver instead? The common ksz driver is becoming just extremely
> > convoluted to handle all hardware variations. Would it help in any way
> > to maintain cleaner code paths, what do you think?
> 
> I agree, this driver is extremely convoluted because of all the
> different hardware it handles. It wasn't easy to fit PTP support for the
> KSZ8463 into it. And I encountered the same kind of difficulties when
> adding periodic output support (I have another series ready for this
> once this PTP support will be merged).
> 
> Regarding migrating the KSZ8 support into the ksz8995, I think we would
> quickly hit the same pain points than in ksz_common. Even inside the
> KSZ8 family we can find a quite big amount of differences between the
> switches. For instance, both KSZ8463 and KSZ8563 support PTP, they share
> lots of common registers but their interrupt scheme is very different.
> 
> I've added Tristram in Cc:, who works at Microchip. Maybe, Tristram, you
> have some insights about which switches could share code if we decide to
> split the big ksz_common into several drivers ?
 
Although KSZ8463 uses 16-bit register for its interrupt operation, the
bits that are necessary for actual operation are all in the high byte, so
it is possible to simulate using 8-bit register for interrupt operation
just like the other switches.

Note also the previous interrupt code only works for KSZ9477 and LAN937X
families of switches, which have interrupts for each port.  The older
switches like KSZ8863, KSZ8895, and KSZ8795 all have only one global
interrupt and need change to work in this framework.

These switches have link change interrupt bits for external ports, so it
is easy to simulate them as port interrupts.  But KSZ8463 only has 1 bit
for link, so it is necessary to add some special code to return a value of
3 to indicate both ports have link change.

KSZ8463 is based from KSZ8863 so it should have about the same 8-bit
register definitions as KSZ8863.  But when it was designed it had a
companion chip that operates as a 16-bit network controller.  Because of
that all register definitions are described in 16-bit.

I need to try your patches to see how it works regarding to the PTP
interrupts.

Regarding to the old ks8995 driver I think it was a simple SPI driver to
start the KSZ8995 switch, which is an older version of KSZ8895.  It was
used to locate under the PHY drivers.  I did not know it was upgraded to
become a full DSA driver and is now trying to expand to the other
switches.

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet]

Hi,

On 3/6/26 2:10 AM, Tristram.Ha@microchip.com wrote:
>> On 3/5/26 1:51 PM, Vladimir Oltean wrote:
>>> On Thu, Mar 05, 2026 at 01:39:29PM +0100, Bastien Curutchet wrote:
>>>> Hi Vladimir,
>>>>
>>>> On 3/5/26 10:56 AM, Vladimir Oltean wrote:
>>>>> On Wed, Mar 04, 2026 at 11:18:52AM +0100, Bastien Curutchet (Schneider
>> Electric) wrote:
>>>>>> @@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq,
>> void *dev_id)
>>>>>>             unsigned int nhandled = 0;
>>>>>>             struct ksz_device *dev;
>>>>>>             unsigned int sub_irq;
>>>>>> -  u8 data;
>>>>>> +  u16 data;
>>>>>>             int ret;
>>>>>>             u8 n;
>>>>>>             dev = kirq->dev;
>>>>>> -  /* Read interrupt status register */
>>>>>> -  ret = ksz_read8(dev, kirq->reg_status, &data);
>>>>>> +  /*
>>>>>> +   * Most of the KSZ switches have a 8-bits long interrupt status
>>>>>> +   * register, but the KSZ8463 has a 16-bits long one. The overread here
>>>>>> +   * is safe because we only iterate over kirq->nirqs in the below loop.
>>>>>
>>>>> FWIW, this isn't the only thing making an overread "safe".
>>>>> If the adjacent register also has "clear on read" semantics, that's not
>>>>> good.
>>>>>
>>>>> I can't tell whether that's the case here, though. There are just too
>>>>> many hardware variations to check for. I just wanted to point out that
>>>>> the reasoning is incomplete.
>>>>>
>>>> You're right, I hadn't thought about the 'clear on read' case.
>>>>
>>>> I'll use ksz_read16() only for the KSZ8463 to be 100% sure it won't break
>>>> anything for other switches.
>>>
>>> I'm still on the fence on whether to say this or not, because I don't
>>> really want to get so involved in internal driver bookkeeping, but...
>>> this driver is just becoming a hell to review, even if I want to
>>> concentrate exclusively on correct API use, like I try to do.
>>>> Linus Walleij has added a new ks8995 driver, which has some overlap with
>>> the common ksz driver for the KSZ8 family. Now he wants to remove the
>>> overlapping device support:
>>> https://lore.kernel.org/netdev/20260219-ks8995-fixups-v3-0-
>> a7fc63fe1916@kernel.org/
>>>
>>> Maybe we should go the other way around, migrate KSZ8 support to the
>>> ks8995 driver instead? The common ksz driver is becoming just extremely
>>> convoluted to handle all hardware variations. Would it help in any way
>>> to maintain cleaner code paths, what do you think?
>>
>> I agree, this driver is extremely convoluted because of all the
>> different hardware it handles. It wasn't easy to fit PTP support for the
>> KSZ8463 into it. And I encountered the same kind of difficulties when
>> adding periodic output support (I have another series ready for this
>> once this PTP support will be merged).
>>
>> Regarding migrating the KSZ8 support into the ksz8995, I think we would
>> quickly hit the same pain points than in ksz_common. Even inside the
>> KSZ8 family we can find a quite big amount of differences between the
>> switches. For instance, both KSZ8463 and KSZ8563 support PTP, they share
>> lots of common registers but their interrupt scheme is very different.
>>
>> I've added Tristram in Cc:, who works at Microchip. Maybe, Tristram, you
>> have some insights about which switches could share code if we decide to
>> split the big ksz_common into several drivers ?
>   
> Although KSZ8463 uses 16-bit register for its interrupt operation, the
> bits that are necessary for actual operation are all in the high byte, so
> it is possible to simulate using 8-bit register for interrupt operation
> just like the other switches.
> 

I hadn't though about using only the high byte, I like this idea. I'll 
implement it in the next iteration with a comment explaining why it 
works for the KSZ8463.

> Note also the previous interrupt code only works for KSZ9477 and LAN937X
> families of switches, which have interrupts for each port.  The older
> switches like KSZ8863, KSZ8895, and KSZ8795 all have only one global
> interrupt and need change to work in this framework.

Indeed, that's what patches 1 to 4 are here for.

> 
> These switches have link change interrupt bits for external ports, so it
> is easy to simulate them as port interrupts.  But KSZ8463 only has 1 bit
> for link, so it is necessary to add some special code to return a value of
> 3 to indicate both ports have link change.
> 

This series only use interrupt for PTP means. TBH I haven't looked at 
the link change interrupt mechanism.

> KSZ8463 is based from KSZ8863 so it should have about the same 8-bit
> register definitions as KSZ8863.  But when it was designed it had a
> companion chip that operates as a 16-bit network controller.  Because of
> that all register definitions are described in 16-bit.
> 
> I need to try your patches to see how it works regarding to the PTP
> interrupts.
> 
> Regarding to the old ks8995 driver I think it was a simple SPI driver to
> start the KSZ8995 switch, which is an older version of KSZ8895.  It was
> used to locate under the PHY drivers.  I did not know it was upgraded to
> become a full DSA driver and is now trying to expand to the other
> switches.

Ok, thanks for these information

Best regards,
Bastien

Re: [PATCH net-next v6 2/9] net: dsa: microchip: Decorrelate IRQ domain from port

[Bastien Curutchet]

Hi Vladimir,

On 3/5/26 11:07 AM, Vladimir Oltean wrote:
> On Wed, Mar 04, 2026 at 11:18:53AM +0100, Bastien Curutchet (Schneider Electric) wrote:
>> KSZ8463 has one register holding interrupt bits from both port 1 and 2.
>> So it has to use one IRQ domain for both of its ports. This conflicts
>> with the current initialization procedure that ties one IRQ domain to
>> each port.
>>
>> Decorrelate IRQ domain from port so a port can use an IRQ domain not
>> directly related to itself.
>>
>> Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
>> ---
>>   drivers/net/dsa/microchip/ksz_ptp.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
>> index 4a2cc57a628f97bd51fcb11057bc4effda9205dd..64afe92a3479ec87b5afc66e489b92787a0fc715 100644
>> --- a/drivers/net/dsa/microchip/ksz_ptp.c
>> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
>> @@ -1099,18 +1099,18 @@ static void ksz_ptp_msg_irq_free(struct ksz_port *port, u8 n)
>>   	irq_dispose_mapping(ptpmsg_irq->num);
>>   }
>>   
>> -static int ksz_ptp_msg_irq_setup(struct ksz_port *port, u8 n)
>> +static int ksz_ptp_msg_irq_setup(struct irq_domain *domain,
>> +				 struct ksz_port *port, u8 n)
>>   {
>>   	u16 ts_reg[] = {REG_PTP_PORT_PDRESP_TS, REG_PTP_PORT_XDELAY_TS,
>>   			REG_PTP_PORT_SYNC_TS};
>>   	static const char * const name[] = {"pdresp-msg", "xdreq-msg",
>>   					    "sync-msg"};
>>   	const struct ksz_dev_ops *ops = port->ksz_dev->dev_ops;
>> -	struct ksz_irq *ptpirq = &port->ptpirq;
>>   	struct ksz_ptp_irq *ptpmsg_irq;
>>   
>>   	ptpmsg_irq = &port->ptpmsg_irq[n];
>> -	ptpmsg_irq->num = irq_create_mapping(ptpirq->domain, n);
>> +	ptpmsg_irq->num = irq_create_mapping(domain, n);
> 
> Looking at this function, I think there's a pre-existing bug.
> If request_threaded_irq() later fails, irq_dispose_mapping() needs to be
> called, because ksz_ptp_msg_irq_free() only frees the IRQ domains that
> were already successfully set up.
I think you're right, I'll send a fix for this.


Best regards,
Bastien

Re: [PATCH net-next v6 4/9] net: dsa: microchip: Add support for KSZ8463's PTP interrupts

[Bastien Curutchet]

Hi Vladimir

On 3/5/26 11:19 AM, Vladimir Oltean wrote:
> On Wed, Mar 04, 2026 at 11:18:55AM +0100, Bastien Curutchet (Schneider Electric) wrote:
>> +void ksz8463_ptp_irq_free(struct dsa_switch *ds)
>> +{
>> +	struct ksz_device *dev = ds->priv;
>> +	struct ksz_port *port1 = &dev->ports[0];
>> +	struct ksz_port *port2 = &dev->ports[1];
>> +	struct ksz_irq *ptpirq = &port1->ptpirq;
>> +
>> +	ksz8463_ptp_port_irq_teardown(port1);
>> +	ksz8463_ptp_port_irq_teardown(port2);
>> +	free_irq(ptpirq->irq_num, ptpirq);
>> +	irq_domain_remove(ptpirq->domain);
>> +}
> 
> Teardown in the exact reverse order of ksz8463_ptp_irq_setup(), please
> (port1 vs port2). Place them next to each other so it's more obvious.

Ok, I'll do it in next iteration.


Best regards,
Bastien

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet]

Hi Vladimir,


On 3/6/26 2:10 AM, Tristram.Ha@microchip.com wrote:
>> On 3/5/26 1:51 PM, Vladimir Oltean wrote:
>>> On Thu, Mar 05, 2026 at 01:39:29PM +0100, Bastien Curutchet wrote:
>>>> Hi Vladimir,
>>>>
>>>> On 3/5/26 10:56 AM, Vladimir Oltean wrote:
>>>>> On Wed, Mar 04, 2026 at 11:18:52AM +0100, Bastien Curutchet (Schneider
>> Electric) wrote:
>>>>>> @@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq,
>> void *dev_id)
>>>>>>             unsigned int nhandled = 0;
>>>>>>             struct ksz_device *dev;
>>>>>>             unsigned int sub_irq;
>>>>>> -  u8 data;
>>>>>> +  u16 data;
>>>>>>             int ret;
>>>>>>             u8 n;
>>>>>>             dev = kirq->dev;
>>>>>> -  /* Read interrupt status register */
>>>>>> -  ret = ksz_read8(dev, kirq->reg_status, &data);
>>>>>> +  /*
>>>>>> +   * Most of the KSZ switches have a 8-bits long interrupt status
>>>>>> +   * register, but the KSZ8463 has a 16-bits long one. The overread here
>>>>>> +   * is safe because we only iterate over kirq->nirqs in the below loop.
>>>>>
>>>>> FWIW, this isn't the only thing making an overread "safe".
>>>>> If the adjacent register also has "clear on read" semantics, that's not
>>>>> good.
>>>>>
>>>>> I can't tell whether that's the case here, though. There are just too
>>>>> many hardware variations to check for. I just wanted to point out that
>>>>> the reasoning is incomplete.
>>>>>
>>>> You're right, I hadn't thought about the 'clear on read' case.
>>>>
>>>> I'll use ksz_read16() only for the KSZ8463 to be 100% sure it won't break
>>>> anything for other switches.
>>>
>>> I'm still on the fence on whether to say this or not, because I don't
>>> really want to get so involved in internal driver bookkeeping, but...
>>> this driver is just becoming a hell to review, even if I want to
>>> concentrate exclusively on correct API use, like I try to do.
>>>> Linus Walleij has added a new ks8995 driver, which has some overlap with
>>> the common ksz driver for the KSZ8 family. Now he wants to remove the
>>> overlapping device support:
>>> https://lore.kernel.org/netdev/20260219-ks8995-fixups-v3-0-
>> a7fc63fe1916@kernel.org/
>>>
>>> Maybe we should go the other way around, migrate KSZ8 support to the
>>> ks8995 driver instead? The common ksz driver is becoming just extremely
>>> convoluted to handle all hardware variations. Would it help in any way
>>> to maintain cleaner code paths, what do you think?
>>
>> I agree, this driver is extremely convoluted because of all the
>> different hardware it handles. It wasn't easy to fit PTP support for the
>> KSZ8463 into it. And I encountered the same kind of difficulties when
>> adding periodic output support (I have another series ready for this
>> once this PTP support will be merged).
>>
>> Regarding migrating the KSZ8 support into the ksz8995, I think we would
>> quickly hit the same pain points than in ksz_common. Even inside the
>> KSZ8 family we can find a quite big amount of differences between the
>> switches. For instance, both KSZ8463 and KSZ8563 support PTP, they share
>> lots of common registers but their interrupt scheme is very different.
>>
>> I've added Tristram in Cc:, who works at Microchip. Maybe, Tristram, you
>> have some insights about which switches could share code if we decide to
>> split the big ksz_common into several drivers ?
>   
> Although KSZ8463 uses 16-bit register for its interrupt operation, the
> bits that are necessary for actual operation are all in the high byte, so
> it is possible to simulate using 8-bit register for interrupt operation
> just like the other switches.
> 
> Note also the previous interrupt code only works for KSZ9477 and LAN937X
> families of switches, which have interrupts for each port.  The older
> switches like KSZ8863, KSZ8895, and KSZ8795 all have only one global
> interrupt and need change to work in this framework.
> 
> These switches have link change interrupt bits for external ports, so it
> is easy to simulate them as port interrupts.  But KSZ8463 only has 1 bit
> for link, so it is necessary to add some special code to return a value of
> 3 to indicate both ports have link change.
> 
> KSZ8463 is based from KSZ8863 so it should have about the same 8-bit
> register definitions as KSZ8863.  But when it was designed it had a
> companion chip that operates as a 16-bit network controller.  Because of
> that all register definitions are described in 16-bit.
> 
> I need to try your patches to see how it works regarding to the PTP
> interrupts.
> 
> Regarding to the old ks8995 driver I think it was a simple SPI driver to
> start the KSZ8995 switch, which is an older version of KSZ8895.  It was
> used to locate under the PHY drivers.  I did not know it was upgraded to
> become a full DSA driver and is now trying to expand to the other
> switches.
> 

I have a new iteration ready. It uses only the high byte of the 
interrupt registers for the KSZ8463, which keeps unchanged the current 
8-bit accesses logic.

Shall I send it ? Or would you like more time to think/discuss about 
whether we should split ksz_common into several drivers ?


Best regards,
Bastien

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean]

On Mon, Mar 09, 2026 at 01:54:28PM +0100, Bastien Curutchet wrote:
> I have a new iteration ready. It uses only the high byte of the interrupt
> registers for the KSZ8463, which keeps unchanged the current 8-bit accesses
> logic.
> 
> Shall I send it ? Or would you like more time to think/discuss about whether
> we should split ksz_common into several drivers ?

"Send it", you mean on Thursday after the weekly 'net' -> 'net-next'
merge so it doesn't conflict with your "net: dsa: microchip: Fix error
path in PTP IRQ setup" change, right?

I don't know. I have to say that I may be partly responsible for guiding
Arun Ramadoss a few years ago towards unifying all ksz_switch drivers
under a single dsa_switch_ops, and that doesn't seem to have been a
great move, given their amount of differences.

Prior to Arun Ramadoss' refactoring such that lan937x could come in and
make use of ksz9477 code, we had two distinct core drivers: ksz8795.c
and ksz9477.c.  Having checked out an old kernel before his changes,
I'm not sure why I was so blinded by all the false code sharing hidden
behind dev_ops that was already there.  If you factor out the MIB worker
thread and other inconsequential small driver things like that, they're
perfectly separate hardware architectures which could use perfectly
isolated drivers with only minimal duplication, and we possibly wouldn't
even need the 8/16/32 regmap tables.

"Resources global in one family and per-port in another" is a recurring
theme, and the inability to have clearly distinct code paths that handle
these clearly distinct hardware architectures is a problem. I'm not sure
of the extent to which I was aware of it when making those suggestions
to Arun. I can recall this thread that indicates a still unsolved, to
this day, problem which was caused by the desire to have a unified driver:
https://patchwork.kernel.org/project/netdevbpf/patch/20230316161250.3286055-3-vladimir.oltean@nxp.com/

I don't have a good plan, but the current state of affairs is not your
fault, and I don't want it to stop you from making progress with the
KSZ8463 PTP support. Maybe we can continue discussing a clean line for a
KSZ8 (and maybe even KSZ8463) split where the code duplication would be
minimal, with further input from Tristram.

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet]

On 3/9/26 9:54 PM, Vladimir Oltean wrote:
> On Mon, Mar 09, 2026 at 01:54:28PM +0100, Bastien Curutchet wrote:
>> I have a new iteration ready. It uses only the high byte of the interrupt
>> registers for the KSZ8463, which keeps unchanged the current 8-bit accesses
>> logic.
>>
>> Shall I send it ? Or would you like more time to think/discuss about whether
>> we should split ksz_common into several drivers ?
> 
> "Send it", you mean on Thursday after the weekly 'net' -> 'net-next'
> merge so it doesn't conflict with your "net: dsa: microchip: Fix error
> path in PTP IRQ setup" change, right?
> 
> I don't know. I have to say that I may be partly responsible for guiding
> Arun Ramadoss a few years ago towards unifying all ksz_switch drivers
> under a single dsa_switch_ops, and that doesn't seem to have been a
> great move, given their amount of differences.
> 
> Prior to Arun Ramadoss' refactoring such that lan937x could come in and
> make use of ksz9477 code, we had two distinct core drivers: ksz8795.c
> and ksz9477.c.  Having checked out an old kernel before his changes,
> I'm not sure why I was so blinded by all the false code sharing hidden
> behind dev_ops that was already there.  If you factor out the MIB worker
> thread and other inconsequential small driver things like that, they're
> perfectly separate hardware architectures which could use perfectly
> isolated drivers with only minimal duplication, and we possibly wouldn't
> even need the 8/16/32 regmap tables.
> 
> "Resources global in one family and per-port in another" is a recurring
> theme, and the inability to have clearly distinct code paths that handle
> these clearly distinct hardware architectures is a problem. I'm not sure
> of the extent to which I was aware of it when making those suggestions
> to Arun. I can recall this thread that indicates a still unsolved, to
> this day, problem which was caused by the desire to have a unified driver:
> https://patchwork.kernel.org/project/netdevbpf/patch/20230316161250.3286055-3-vladimir.oltean@nxp.com/
> > I don't have a good plan, but the current state of affairs is not your
> fault, and I don't want it to stop you from making progress with the
> KSZ8463 PTP support. Maybe we can continue discussing a clean line for a
> KSZ8 (and maybe even KSZ8463) split where the code duplication would be
> minimal, with further input from Tristram.

I, more or less, managed to fit this PTP support in the current 
framework, but the next series I prepared to add periodic output support 
is full of is_ksz8463() branches. So I think that I'll go with what you 
suggested and try to move the KSZ8463 support in the ksz8995.c driver. 
It will probably be cleaner this way.

TBH I'm a bit lost with all these KSZ variants but IIUC KSZ8463 and 
KSZ8995 both belong to the 'old generation' of the hardware. Maybe a 
good split would be to have these old switches in the ksz8995.c driver 
and keep the rest under ksz_common infra.


Best regards,
BAstein

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean]

On Wed, Mar 11, 2026 at 11:02:35AM +0100, Bastien Curutchet wrote:
> I, more or less, managed to fit this PTP support in the current framework,
> but the next series I prepared to add periodic output support is full of
> is_ksz8463() branches. So I think that I'll go with what you suggested and
> try to move the KSZ8463 support in the ksz8995.c driver. It will probably be
> cleaner this way.
> 
> TBH I'm a bit lost with all these KSZ variants but IIUC KSZ8463 and KSZ8995
> both belong to the 'old generation' of the hardware. Maybe a good split
> would be to have these old switches in the ksz8995.c driver and keep the
> rest under ksz_common infra.

Wait, to make sure I understand, you said you're going to rebase this
series onto the ksz8995 driver?

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet]

On 3/11/26 12:53 PM, Vladimir Oltean wrote:
> On Wed, Mar 11, 2026 at 11:02:35AM +0100, Bastien Curutchet wrote:
>> I, more or less, managed to fit this PTP support in the current framework,
>> but the next series I prepared to add periodic output support is full of
>> is_ksz8463() branches. So I think that I'll go with what you suggested and
>> try to move the KSZ8463 support in the ksz8995.c driver. It will probably be
>> cleaner this way.
>>
>> TBH I'm a bit lost with all these KSZ variants but IIUC KSZ8463 and KSZ8995
>> both belong to the 'old generation' of the hardware. Maybe a good split
>> would be to have these old switches in the ksz8995.c driver and keep the
>> rest under ksz_common infra.
> 
> Wait, to make sure I understand, you said you're going to rebase this
> series onto the ksz8995 driver?


Well I'll first add basic support for the KSZ8463 in the ksz8995 driver 
and then add interrupt and PTP support on top of it yes. Did I 
misunderstand you're suggestion ?

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean]

On Wed, Mar 11, 2026 at 01:53:38PM +0100, Bastien Curutchet wrote:
> On 3/11/26 12:53 PM, Vladimir Oltean wrote:
> > On Wed, Mar 11, 2026 at 11:02:35AM +0100, Bastien Curutchet wrote:
> > > I, more or less, managed to fit this PTP support in the current framework,
> > > but the next series I prepared to add periodic output support is full of
> > > is_ksz8463() branches. So I think that I'll go with what you suggested and
> > > try to move the KSZ8463 support in the ksz8995.c driver. It will probably be
> > > cleaner this way.
> > > 
> > > TBH I'm a bit lost with all these KSZ variants but IIUC KSZ8463 and KSZ8995
> > > both belong to the 'old generation' of the hardware. Maybe a good split
> > > would be to have these old switches in the ksz8995.c driver and keep the
> > > rest under ksz_common infra.
> > 
> > Wait, to make sure I understand, you said you're going to rebase this
> > series onto the ksz8995 driver?
> 
> Well I'll first add basic support for the KSZ8463 in the ksz8995 driver and
> then add interrupt and PTP support on top of it yes. Did I misunderstand
> you're suggestion ?

If we're splitting drivers, I would like to see them grouped by actual similarity,
so as to not repeat the same mistakes.

I took a random feature, user port separation, and I'm comparing how Linus
implemented it in ksz8995:
https://lore.kernel.org/netdev/20260219-ks8995-fixups-v3-5-a7fc63fe1916@kernel.org/
vs the register map from KSZ8463:
https://www.microchip.com/en-us/product/ksz8463#Documentation

Apart from some funky naming differences (KS8995_REG_PC1 corresponds to
"PORT 1 CONTROL REGISTER 2"), it should be pretty similar, right? Do you
see some differences requiring major surgery in ks8995?

Apart from the obvious incompleteness, of course. That driver currently
reports DSA_TAG_PROTO_NONE. It will have to return DSA_TAG_PROTO_KS8995
for ks8995:
https://lore.kernel.org/netdev/20260107-ks8995-dsa-tagging-v1-1-1a92832c1540@kernel.org/
and DSA_TAG_PROTO_KSZ9893 for your KSZ8463. Right?

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet]

On 3/11/26 2:56 PM, Vladimir Oltean wrote:
> On Wed, Mar 11, 2026 at 01:53:38PM +0100, Bastien Curutchet wrote:
>> On 3/11/26 12:53 PM, Vladimir Oltean wrote:
>>> On Wed, Mar 11, 2026 at 11:02:35AM +0100, Bastien Curutchet wrote:
>>>> I, more or less, managed to fit this PTP support in the current framework,
>>>> but the next series I prepared to add periodic output support is full of
>>>> is_ksz8463() branches. So I think that I'll go with what you suggested and
>>>> try to move the KSZ8463 support in the ksz8995.c driver. It will probably be
>>>> cleaner this way.
>>>>
>>>> TBH I'm a bit lost with all these KSZ variants but IIUC KSZ8463 and KSZ8995
>>>> both belong to the 'old generation' of the hardware. Maybe a good split
>>>> would be to have these old switches in the ksz8995.c driver and keep the
>>>> rest under ksz_common infra.
>>>
>>> Wait, to make sure I understand, you said you're going to rebase this
>>> series onto the ksz8995 driver?
>>
>> Well I'll first add basic support for the KSZ8463 in the ksz8995 driver and
>> then add interrupt and PTP support on top of it yes. Did I misunderstand
>> you're suggestion ?
> 
> If we're splitting drivers, I would like to see them grouped by actual similarity,
> so as to not repeat the same mistakes.
> 
> I took a random feature, user port separation, and I'm comparing how Linus
> implemented it in ksz8995:
> https://lore.kernel.org/netdev/20260219-ks8995-fixups-v3-5-a7fc63fe1916@kernel.org/
> vs the register map from KSZ8463:
> https://www.microchip.com/en-us/product/ksz8463#Documentation
> 

I took a look at the register maps from both datasheets. Registers 
aren't always at the same address and some bits differ here and there 
but following areas look very similar:
- global control
- port control
- TOS priority
- MAC address config
- Indirect access
- Data rate limit

> Apart from some funky naming differences (KS8995_REG_PC1 corresponds to
> "PORT 1 CONTROL REGISTER 2"), it should be pretty similar, right? Do you
> see some differences requiring major surgery in ks8995?
> 

The biggest difference I see is the way KSZ8463 accesses the registers 
through SPI. It uses byte enable bits in the command word that don't 
exist on the other switches.

> Apart from the obvious incompleteness, of course. That driver currently
> reports DSA_TAG_PROTO_NONE. It will have to return DSA_TAG_PROTO_KS8995
> for ks8995:
> https://lore.kernel.org/netdev/20260107-ks8995-dsa-tagging-v1-1-1a92832c1540@kernel.org/
> and DSA_TAG_PROTO_KSZ9893 for your KSZ8463. Right?

Yes.

In the end, it feels OK to me to migrate KSZ8463 into ksz8863.c but it's 
hard to know for sure before actually doing it.

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Andrew Lunn]

> The biggest difference I see is the way KSZ8463 accesses the registers
> through SPI. It uses byte enable bits in the command word that don't exist
> on the other switches.

Can that be hidden away in regmap? Add reg_read/reg_write functions
specific for this device which handle these byte enable bits?

	Andrew

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean]

On Wed, Mar 11, 2026 at 05:58:32PM +0100, Bastien Curutchet wrote:
> In the end, it feels OK to me to migrate KSZ8463 into ksz8863.c but it's
> hard to know for sure before actually doing it.

Sorry, what ksz8863.c? I may be misunderstanding something.

RE: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Tristram.Ha]

> On 3/11/26 2:56 PM, Vladimir Oltean wrote:
> > On Wed, Mar 11, 2026 at 01:53:38PM +0100, Bastien Curutchet wrote:
> >> On 3/11/26 12:53 PM, Vladimir Oltean wrote:
> >>> On Wed, Mar 11, 2026 at 11:02:35AM +0100, Bastien Curutchet wrote:
> >>>> I, more or less, managed to fit this PTP support in the current framework,
> >>>> but the next series I prepared to add periodic output support is full of
> >>>> is_ksz8463() branches. So I think that I'll go with what you suggested and
> >>>> try to move the KSZ8463 support in the ksz8995.c driver. It will probably be
> >>>> cleaner this way.
> >>>>
> >>>> TBH I'm a bit lost with all these KSZ variants but IIUC KSZ8463 and KSZ8995
> >>>> both belong to the 'old generation' of the hardware. Maybe a good split
> >>>> would be to have these old switches in the ksz8995.c driver and keep the
> >>>> rest under ksz_common infra.
> >>>
> >>> Wait, to make sure I understand, you said you're going to rebase this
> >>> series onto the ksz8995 driver?
> >>
> >> Well I'll first add basic support for the KSZ8463 in the ksz8995 driver and
> >> then add interrupt and PTP support on top of it yes. Did I misunderstand
> >> you're suggestion ?
> >
> > If we're splitting drivers, I would like to see them grouped by actual similarity,
> > so as to not repeat the same mistakes.
> >
> > I took a random feature, user port separation, and I'm comparing how Linus
> > implemented it in ksz8995:
> > https://lore.kernel.org/netdev/20260219-ks8995-fixups-v3-5-
> a7fc63fe1916@kernel.org/
> > vs the register map from KSZ8463:
> > https://www.microchip.com/en-us/product/ksz8463#Documentation
> >
> 
> I took a look at the register maps from both datasheets. Registers
> aren't always at the same address and some bits differ here and there
> but following areas look very similar:
> - global control
> - port control
> - TOS priority
> - MAC address config
> - Indirect access
> - Data rate limit
> 
> > Apart from some funky naming differences (KS8995_REG_PC1 corresponds to
> > "PORT 1 CONTROL REGISTER 2"), it should be pretty similar, right? Do you
> > see some differences requiring major surgery in ks8995?
> >
> 
> The biggest difference I see is the way KSZ8463 accesses the registers
> through SPI. It uses byte enable bits in the command word that don't
> exist on the other switches.
> 
> > Apart from the obvious incompleteness, of course. That driver currently
> > reports DSA_TAG_PROTO_NONE. It will have to return
> DSA_TAG_PROTO_KS8995
> > for ks8995:
> > https://lore.kernel.org/netdev/20260107-ks8995-dsa-tagging-v1-1-
> 1a92832c1540@kernel.org/
> > and DSA_TAG_PROTO_KSZ9893 for your KSZ8463. Right?
> 
> Yes.
> 
> In the end, it feels OK to me to migrate KSZ8463 into ksz8863.c but it's
> hard to know for sure before actually doing it.

I just tried the ks8995 driver and I do not think it is ready to do major
DSA operation.  Actually I could not make it work if I registered it as a
DSA switch.  DSA could not register the switch PHY.  If I removed the port
information in the device tree there was no error message but still the
MAC could not send out anything.  If I skipped the registration at least
the MAC can communicate.

The driver is using DSA_TAG_PROTO_NONE so there is no tagging involved.
But then I do not know how to define the device tree for the DSA to change
the STP state of each port to allow communication.

The primary purpose of this driver in the old time was to enable the
switch as the hardware default is disabled.

Even though KSZ8795 was added to this driver its register set is not a
complete match to KSZ8895.  So some functions may not work correctly like
the change_mtu one.  And I do not think KSZ8463/KSZ8863 can work at all.

Since we have a working KSZ8463 PTP driver we should stick with the
Microchip KSZ DSA model and work from there.

Out of topic I found that the NAND driver is not started in the new 7.0
kernel while 6.19 is still okay.  I do not think there is a major overhaul
in that area, and the Atmel NAND driver code is about the same.  I thought
the device name may be changed so the right driver is not picked up, but
using the device tree in the 7.0 kernel still does not work.

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean]

On Thu, Mar 12, 2026 at 12:14:40AM +0000, Tristram.Ha@microchip.com wrote:
> I just tried the ks8995 driver and I do not think it is ready to do major
> DSA operation.  Actually I could not make it work if I registered it as a
> DSA switch.  DSA could not register the switch PHY.  If I removed the port
> information in the device tree there was no error message but still the
> MAC could not send out anything.  If I skipped the registration at least
> the MAC can communicate.
> 
> The driver is using DSA_TAG_PROTO_NONE so there is no tagging involved.
> But then I do not know how to define the device tree for the DSA to change
> the STP state of each port to allow communication.
> 
> The primary purpose of this driver in the old time was to enable the
> switch as the hardware default is disabled.
> 
> Even though KSZ8795 was added to this driver its register set is not a
> complete match to KSZ8895.  So some functions may not work correctly like
> the change_mtu one.  And I do not think KSZ8463/KSZ8863 can work at all.
> 
> Since we have a working KSZ8463 PTP driver we should stick with the
> Microchip KSZ DSA model and work from there.
> 
> Out of topic I found that the NAND driver is not started in the new 7.0
> kernel while 6.19 is still okay.  I do not think there is a major overhaul
> in that area, and the Atmel NAND driver code is about the same.  I thought
> the device name may be changed so the right driver is not picked up, but
> using the device tree in the 7.0 kernel still does not work.

Thanks for the comment. I took a deeper look at the ksz_switch common
driver and I've started working on some patches. I'll return when
they're more or less ready.

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet]

On 3/11/26 10:24 PM, Vladimir Oltean wrote:
> On Wed, Mar 11, 2026 at 05:58:32PM +0100, Bastien Curutchet wrote:
>> In the end, it feels OK to me to migrate KSZ8463 into ksz8863.c but it's
>> hard to know for sure before actually doing it.
> 
> Sorry, what ksz8863.c? I may be misunderstanding something.

I meant ksz8995.c sorry

RE: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Tristram.Ha]

> On 3/11/26 10:24 PM, Vladimir Oltean wrote:
> > On Wed, Mar 11, 2026 at 05:58:32PM +0100, Bastien Curutchet wrote:
> >> In the end, it feels OK to me to migrate KSZ8463 into ksz8863.c but it's
> >> hard to know for sure before actually doing it.
> >
> > Sorry, what ksz8863.c? I may be misunderstanding something.
> 
> I meant ksz8995.c sorry

I am finally able to bring up the ks8995 driver by using fixed link for
all external ports.  Now I have lan1, lan2, lan3, and lan4 but I do not
know how to use them.  Using a br0 to combine them will send out 4 packets
for each one to br0.  And it cannot receive properly.  The individual lan1
device can send but also cannot receive.  Only eth0 can send and receive
so it is the only one which can ping.

The lan1 device cannot tell the speed of the port as fixed link is used.
Its other purpose is to shutdown the port, but there is nothing
implemented inside the driver.

So the ks8995 driver is not completed?

RE: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Tristram.Ha]

> > On 3/11/26 10:24 PM, Vladimir Oltean wrote:
> > > On Wed, Mar 11, 2026 at 05:58:32PM +0100, Bastien Curutchet wrote:
> > >> In the end, it feels OK to me to migrate KSZ8463 into ksz8863.c but it's
> > >> hard to know for sure before actually doing it.
> > >
> > > Sorry, what ksz8863.c? I may be misunderstanding something.
> >
> > I meant ksz8995.c sorry
> 
> I am finally able to bring up the ks8995 driver by using fixed link for
> all external ports.  Now I have lan1, lan2, lan3, and lan4 but I do not
> know how to use them.  Using a br0 to combine them will send out 4 packets
> for each one to br0.  And it cannot receive properly.  The individual lan1
> device can send but also cannot receive.  Only eth0 can send and receive
> so it is the only one which can ping.
> 
> The lan1 device cannot tell the speed of the port as fixed link is used.
> Its other purpose is to shutdown the port, but there is nothing
> implemented inside the driver.
> 
> So the ks8995 driver is not completed?

The STP state of the port will be changed, so the port can indeed be
shutdown.

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean]

On Thu, Mar 12, 2026 at 03:45:26PM +0200, Vladimir Oltean wrote:
> Thanks for the comment. I took a deeper look at the ksz_switch common
> driver and I've started working on some patches. I'll return when
> they're more or less ready.

So if the common ksz_switch were to remain preferable, we still have the
problem that it's very cluttered/abstract and impossible to follow and
ensure correctness/lack of regressions for other hardware.

Bastien, Tristram, I've tried to address that very particular issue by
giving KSZ8463 its very own dsa_switch_ops structure, which it doesn't
have to share with anyone else.

I have no hardware to test, but some patches are here:
https://github.com/vladimiroltean/linux/tree/ksz_separate_dsa_switch_ops

Do you think this would be useful? For now I've created a common ksz8_setup(),
but it can be further split into whatever granularity is required for
ksz8463 to have its own IRQ domain operations.

One could conceivable walk this alternative route of having independent
per-family dsa_switch_ops and only call ksz_common.c when something is
truly common ad infinitum. We could use it to get rid of dev->info->regs[],
dev->info->xmii_ctrl1, etc. We could further give ksz8463 a separate
ksz_ptp_clock_register() implementation with different PTP operations,
to address Bastien's concern. I stopped at only 19 patches though :-/

What do you think, is this direction something worth exploring? Does it
help with the PTP IRQ differences here?

RE: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Tristram.Ha]

> On Thu, Mar 12, 2026 at 03:45:26PM +0200, Vladimir Oltean wrote:
> > Thanks for the comment. I took a deeper look at the ksz_switch common
> > driver and I've started working on some patches. I'll return when
> > they're more or less ready.
> 
> So if the common ksz_switch were to remain preferable, we still have the
> problem that it's very cluttered/abstract and impossible to follow and
> ensure correctness/lack of regressions for other hardware.
> 
> Bastien, Tristram, I've tried to address that very particular issue by
> giving KSZ8463 its very own dsa_switch_ops structure, which it doesn't
> have to share with anyone else.
> 
> I have no hardware to test, but some patches are here:
> https://github.com/vladimiroltean/linux/tree/ksz_separate_dsa_switch_ops
> 
> Do you think this would be useful? For now I've created a common ksz8_setup(),
> but it can be further split into whatever granularity is required for
> ksz8463 to have its own IRQ domain operations.
> 
> One could conceivable walk this alternative route of having independent
> per-family dsa_switch_ops and only call ksz_common.c when something is
> truly common ad infinitum. We could use it to get rid of dev->info->regs[],
> dev->info->xmii_ctrl1, etc. We could further give ksz8463 a separate
> ksz_ptp_clock_register() implementation with different PTP operations,
> to address Bastien's concern. I stopped at only 19 patches though :-/
> 
> What do you think, is this direction something worth exploring? Does it
> help with the PTP IRQ differences here?

What I did was to simulate the port interrupt mechanism used by KSZ9477
and LAN937X switch families.  I think that does minimum disruption to the
original code.  That new interrupt code also allows interrupt use for
KSZ8863, KSZ8895, and KSZ8795.  I provided that code to Bastien to test.

KSZ8463, KSZ8863, KSZ8895, and KSZ8795 all have minor register differences
so I never did a single driver to support them all.  In the original there
were separate KSZ8863, KSZ8795, and KSZ9477 driver code.  But Linux
developers generally like to have one big driver and so eventually they
were merged.

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Bastien Curutchet]

Hi all,

On 3/13/26 6:29 PM, Tristram.Ha@microchip.com wrote:
>> On Thu, Mar 12, 2026 at 03:45:26PM +0200, Vladimir Oltean wrote:
>>> Thanks for the comment. I took a deeper look at the ksz_switch common
>>> driver and I've started working on some patches. I'll return when
>>> they're more or less ready.
>>
>> So if the common ksz_switch were to remain preferable, we still have the
>> problem that it's very cluttered/abstract and impossible to follow and
>> ensure correctness/lack of regressions for other hardware.
>>
>> Bastien, Tristram, I've tried to address that very particular issue by
>> giving KSZ8463 its very own dsa_switch_ops structure, which it doesn't
>> have to share with anyone else.
>>
>> I have no hardware to test, but some patches are here:
>> https://github.com/vladimiroltean/linux/tree/ksz_separate_dsa_switch_ops
>>
>> Do you think this would be useful? For now I've created a common ksz8_setup(),
>> but it can be further split into whatever granularity is required for
>> ksz8463 to have its own IRQ domain operations.
>>>> One could conceivable walk this alternative route of having independent
>> per-family dsa_switch_ops and only call ksz_common.c when something is
>> truly common ad infinitum. We could use it to get rid of dev->info->regs[],
>> dev->info->xmii_ctrl1, etc. We could further give ksz8463 a separate
>> ksz_ptp_clock_register() implementation with different PTP operations,
>> to address Bastien's concern. I stopped at only 19 patches though :-/
>>
>> What do you think, is this direction something worth exploring? Does it
>> help with the PTP IRQ differences here?
> 
> What I did was to simulate the port interrupt mechanism used by KSZ9477
> and LAN937X switch families.  I think that does minimum disruption to the
> original code.  That new interrupt code also allows interrupt use for
> KSZ8863, KSZ8895, and KSZ8795.  I provided that code to Bastien to test.
> 

I took a look at Tristram's patches, they should work but I'm not a big 
fan of emulating other hardware behavior to fit the existing driver. I'd 
rather go with Vladimir's approach.

Thanks for these patches Vladimir, they provide a clear path forward for 
the remaining work. I'll rebase my work on top of them. I'll implement a 
ksz8463_setup() function to match the KSZ interrupt scheme before adding 
PTP support.

And, indeed, implementing a ksz8463_ptp_clock_register() should then 
make it easier to add the periodic output support I plan to implement 
afterwards.


Best regards,
Bastien

Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean]

On Wed, Mar 18, 2026 at 10:26:49AM +0100, Bastien Curutchet wrote:
> I took a look at Tristram's patches, they should work but I'm not a big fan
> of emulating other hardware behavior to fit the existing driver. I'd rather
> go with Vladimir's approach.
> 
> Thanks for these patches Vladimir, they provide a clear path forward for the
> remaining work. I'll rebase my work on top of them. I'll implement a
> ksz8463_setup() function to match the KSZ interrupt scheme before adding PTP
> support.

My only ask is that you take over these patches and submit whatever part
you consider necessary. Sadly I don't really have the time to do that myself.