[PATCH net-next v3 00/10] net: dsa: microchip: add periodic output support for the KSZ8463

[PATCH net-next v3 00/10] net: dsa: microchip: add periodic output support for the KSZ8463

[Bastien Curutchet (Schneider Electric)]

This series aims to add periodic output support for the KSZ8463. The
way the KSZ8463 handles periodic output differs from that of the other
KSZ switches. It therefore needs its own set of perout callbacks
(configure, start, reset, enable, restart).

Patches 1 to 3 prepare the driver to accept different kinds of periodic
output settings.

Patches 4 to 9 extract a set of functions from the existing periodic
output support that can be reused by the KSZ8463, avoiding code
duplication as much as possible.

Patch 10 adds periodic output support for the KSZ8463.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
Changes in v3:
- PATCH 10: Fix build error when CONFIG_NET_DSA_MICROCHIP_KSZ_PTP=n
- Link to v2: https://lore.kernel.org/r/20260902-ksz-perout-v2-0-6f277fcc9e68@bootlin.com

Changes in v2:
- PATCH 1: Init n_pins for all PTP capable switches
- PATCH 2: Init n_per_out for all PTP capable switches
- Link to v1: https://lore.kernel.org/r/20260831-ksz-perout-v1-0-14202db763b3@bootlin.com

---
Bastien Curutchet (Schneider Electric) (10):
      net: dsa: microchip: add the number of pins to chip infos
      net: dsa: microchip: add the number of periodic signals to chip infos
      net: dsa: microchip: use dynamic mask to check pulse width validity
      net: dsa: microchip: extract PTP callbacks configuration from PTP registration
      net: dsa: microchip: extract ptp_get_pin
      net: dsa: microchip: extract compute_width
      net: dsa: microchip: extract prepare reset
      net: dsa: microchip: extract time update
      net: dsa: microchip: extract time adjustment
      net: dsa: microchip: add periodic output support for the KSZ8463

 drivers/net/dsa/microchip/ksz8.c         |   2 +
 drivers/net/dsa/microchip/ksz9477.c      |   1 +
 drivers/net/dsa/microchip/ksz_common.c   |  22 ++
 drivers/net/dsa/microchip/ksz_common.h   |   2 +
 drivers/net/dsa/microchip/ksz_ptp.c      | 473 ++++++++++++++++++++++++++-----
 drivers/net/dsa/microchip/ksz_ptp.h      |   6 +-
 drivers/net/dsa/microchip/ksz_ptp_reg.h  |  16 ++
 drivers/net/dsa/microchip/lan937x_main.c |   1 +
 8 files changed, 452 insertions(+), 71 deletions(-)
---
base-commit: f7f619ecda5761bd48578764ef5ff9e8fd97b916
change-id: 20260825-ksz-perout-af9813e12158

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

[PATCH net-next v3 01/10] net: dsa: microchip: add the number of pins to chip infos

[Bastien Curutchet (Schneider Electric)]

The number of periodic output pins available is hardcoded to 2 while
KSZ8463 has 12 pins that can be used as periodic outputs.

Add an n_pin attribute to the struct ksz_chip_data to make this setting
configurable.
Set it to 2 for all the PTP-capable switches.

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

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 1a9d6f83a023..d01fba253af5 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1194,6 +1194,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.ptp_capable = true,
 		.wr_table = &ksz8563_register_set,
 		.rd_table = &ksz8563_register_set,
+		.n_pins = 2,
 	},
 
 	[KSZ8795] = {
@@ -1414,6 +1415,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.sgmii_port = 7,
 		.wr_table = &ksz9477_register_set,
 		.rd_table = &ksz9477_register_set,
+		.n_pins = 2,
 	},
 
 	[KSZ9896] = {
@@ -1544,6 +1546,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy = {true, true, false},
 		.gbit_capable = {true, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[KSZ8567] = {
@@ -1581,6 +1584,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.gbit_capable	= {false, false, false, false, false,
 				   true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[KSZ9567] = {
@@ -1615,6 +1619,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   true, false, false},
 		.gbit_capable	= {true, true, true, true, true, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9370] = {
@@ -1646,6 +1651,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.supports_rgmii = {false, false, false, false, true},
 		.internal_phy = {true, true, true, true, false},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9371] = {
@@ -1677,6 +1683,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.supports_rgmii = {false, false, false, false, true, true},
 		.internal_phy = {true, true, true, true, false, false},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9372] = {
@@ -1712,6 +1719,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy	= {true, true, true, true,
 				   false, false, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9373] = {
@@ -1747,6 +1755,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy	= {true, true, true, false,
 				   false, false, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9374] = {
@@ -1782,6 +1791,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy	= {true, true, true, true,
 				   false, false, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9646] = {
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index cbe98494578c..c678fc6597dc 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -139,6 +139,7 @@ struct ksz_chip_data {
 	u8 sgmii_port;
 	const struct regmap_access_table *wr_table;
 	const struct regmap_access_table *rd_table;
+	const u8 n_pins;
 };
 
 struct ksz_irq {
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 39cc70d65900..c92936bf78d3 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -1050,14 +1050,14 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
 	ptp_data->caps.do_aux_work	= ksz_ptp_do_aux_work;
 	ptp_data->caps.enable		= ksz_ptp_enable;
 	ptp_data->caps.verify		= ksz_ptp_verify_pin;
-	ptp_data->caps.n_pins		= KSZ_PTP_N_GPIO;
+	ptp_data->caps.n_pins		= dev->info->n_pins;
 	ptp_data->caps.n_per_out	= 3;
 
 	ret = ksz_ptp_start_clock(dev);
 	if (ret)
 		return ret;
 
-	for (i = 0; i < KSZ_PTP_N_GPIO; i++) {
+	for (i = 0; i < dev->info->n_pins; i++) {
 		struct ptp_pin_desc *ptp_pin = &ptp_data->pin_config[i];
 
 		snprintf(ptp_pin->name,

-- 
2.55.0

[PATCH net-next v3 02/10] net: dsa: microchip: add the number of periodic signals to chip infos

[Bastien Curutchet (Schneider Electric)]

The number of periodic signals available is hardcoded to 3 while
KSZ8463 can produce 12 different periodic signals.

Add an n_per_out attribute to the struct ksz_chip_data to make this
setting configurable.
Set it to 3 for all the PTP-capable switches.

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

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index d01fba253af5..041bb4f96365 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1195,6 +1195,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.wr_table = &ksz8563_register_set,
 		.rd_table = &ksz8563_register_set,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[KSZ8795] = {
@@ -1416,6 +1417,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.wr_table = &ksz9477_register_set,
 		.rd_table = &ksz9477_register_set,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[KSZ9896] = {
@@ -1547,6 +1549,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.gbit_capable = {true, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[KSZ8567] = {
@@ -1585,6 +1588,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[KSZ9567] = {
@@ -1620,6 +1624,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.gbit_capable	= {true, true, true, true, true, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9370] = {
@@ -1652,6 +1657,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy = {true, true, true, true, false},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9371] = {
@@ -1684,6 +1690,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy = {true, true, true, true, false, false},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9372] = {
@@ -1720,6 +1727,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   false, false, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9373] = {
@@ -1756,6 +1764,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   false, false, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9374] = {
@@ -1792,6 +1801,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   false, false, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9646] = {
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index c678fc6597dc..1155b39350c2 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -140,6 +140,7 @@ struct ksz_chip_data {
 	const struct regmap_access_table *wr_table;
 	const struct regmap_access_table *rd_table;
 	const u8 n_pins;
+	const u8 n_per_out;
 };
 
 struct ksz_irq {
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index c92936bf78d3..f80015250e5a 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -1051,7 +1051,7 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
 	ptp_data->caps.enable		= ksz_ptp_enable;
 	ptp_data->caps.verify		= ksz_ptp_verify_pin;
 	ptp_data->caps.n_pins		= dev->info->n_pins;
-	ptp_data->caps.n_per_out	= 3;
+	ptp_data->caps.n_per_out	= dev->info->n_per_out;
 
 	ret = ksz_ptp_start_clock(dev);
 	if (ret)

-- 
2.55.0

[PATCH net-next v3 03/10] net: dsa: microchip: use dynamic mask to check pulse width validity

[Bastien Curutchet (Schneider Electric)]

The pulse width validity is checked against an hardcoded mask. KSZ8463
uses a smaller register than other switches so this check won't work for
it.

Move the mask as a ksz_ptp_tou_pulse_verify()'s input to allow changing
it for KSZ8463 case.

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

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index f80015250e5a..7bf83e0b5918 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -86,7 +86,7 @@ static int ksz_ptp_tou_reset(struct ksz_device *dev, u8 unit)
 			 0);
 }
 
-static int ksz_ptp_tou_pulse_verify(u64 pulse_ns)
+static int ksz_ptp_tou_pulse_verify(u64 pulse_ns, u32 mask)
 {
 	u32 data;
 
@@ -94,7 +94,7 @@ static int ksz_ptp_tou_pulse_verify(u64 pulse_ns)
 		return -EINVAL;
 
 	data = (pulse_ns / 8);
-	if (!FIELD_FIT(TRIG_PULSE_WIDTH_M, data))
+	if ((mask & data) != data)
 		return -ERANGE;
 
 	return 0;
@@ -245,7 +245,7 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 				       KSZ_MAX_PULSE_WIDTH);
 	}
 
-	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns);
+	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns, TRIG_PULSE_WIDTH_M);
 	if (ret)
 		return ret;
 

-- 
2.55.0

[PATCH net-next v3 04/10] net: dsa: microchip: extract PTP callbacks configuration from PTP registration

[Bastien Curutchet (Schneider Electric)]

KSZ8463 also has periodic output capabilities but the way its outputs
are driven differs from the other switches. It will need its own set of
PTP callbacks to implements this behaviour.

Extract the PTP callbacks configuration in a dedicated function to be
called before the PTP registration to ease the use of others callbacks
when needed.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz8.c         |  2 ++
 drivers/net/dsa/microchip/ksz9477.c      |  1 +
 drivers/net/dsa/microchip/ksz_ptp.c      | 20 ++++++++++++++------
 drivers/net/dsa/microchip/ksz_ptp.h      |  2 ++
 drivers/net/dsa/microchip/lan937x_main.c |  1 +
 5 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
index 941ae9f66f70..f2a23e2e474a 100644
--- a/drivers/net/dsa/microchip/ksz8.c
+++ b/drivers/net/dsa/microchip/ksz8.c
@@ -2478,6 +2478,7 @@ static int ksz8463_setup(struct dsa_switch *ds)
 		if (ret)
 			goto free_girq;
 
+		ksz_ptp_set_caps(ds);
 		ret = ksz_ptp_clock_register(ds);
 		if (ret) {
 			dev_err(dev->dev, "Failed to register PTP clock: %d\n",
@@ -2775,6 +2776,7 @@ static int ksz8_setup(struct dsa_switch *ds)
 	}
 
 	if (dev->info->ptp_capable) {
+		ksz_ptp_set_caps(ds);
 		ret = ksz_ptp_clock_register(ds);
 		if (ret) {
 			dev_err(dev->dev, "Failed to register PTP clock: %d\n",
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 3ee995545c57..72528a53b67d 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1781,6 +1781,7 @@ static int ksz9477_setup(struct dsa_switch *ds)
 	}
 
 	if (dev->info->ptp_capable) {
+		ksz_ptp_set_caps(ds);
 		ret = ksz_ptp_clock_register(ds);
 		if (ret) {
 			dev_err(dev->dev, "Failed to register PTP clock: %d\n",
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 7bf83e0b5918..ea22601f6b26 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -1028,17 +1028,12 @@ static int ksz_ptp_start_clock(struct ksz_device *dev)
 	return 0;
 }
 
-int ksz_ptp_clock_register(struct dsa_switch *ds)
+void ksz_ptp_set_caps(struct dsa_switch *ds)
 {
 	struct ksz_device *dev = ds->priv;
-	const u16 *regs = dev->info->regs;
 	struct ksz_ptp_data *ptp_data;
-	int ret;
-	u8 i;
 
 	ptp_data = &dev->ptp_data;
-	mutex_init(&ptp_data->lock);
-	spin_lock_init(&ptp_data->clock_lock);
 
 	ptp_data->caps.owner		= THIS_MODULE;
 	snprintf(ptp_data->caps.name, 16, "Microchip Clock");
@@ -1052,6 +1047,19 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
 	ptp_data->caps.verify		= ksz_ptp_verify_pin;
 	ptp_data->caps.n_pins		= dev->info->n_pins;
 	ptp_data->caps.n_per_out	= dev->info->n_per_out;
+}
+
+int ksz_ptp_clock_register(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	const u16 *regs = dev->info->regs;
+	struct ksz_ptp_data *ptp_data;
+	int ret;
+	u8 i;
+
+	ptp_data = &dev->ptp_data;
+	mutex_init(&ptp_data->lock);
+	spin_lock_init(&ptp_data->clock_lock);
 
 	ret = ksz_ptp_start_clock(dev);
 	if (ret)
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 7067ec9bd1e6..89716b31596b 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -33,6 +33,7 @@ struct ksz_ptp_data {
 	struct timespec64 perout_period;
 };
 
+void ksz_ptp_set_caps(struct dsa_switch *ds);
 int ksz_ptp_clock_register(struct dsa_switch *ds);
 
 void ksz_ptp_clock_unregister(struct dsa_switch *ds);
@@ -65,6 +66,7 @@ struct ksz_ptp_data {
 	struct mutex lock;
 };
 
+static inline void ksz_ptp_set_caps(struct dsa_switch *ds) { }
 static inline int ksz_ptp_clock_register(struct dsa_switch *ds)
 {
 	return 0;
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index 86ce3a86705f..3a209122fc7d 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -867,6 +867,7 @@ static int lan937x_setup(struct dsa_switch *ds)
 		}
 	}
 
+	ksz_ptp_set_caps(ds);
 	ret = ksz_ptp_clock_register(ds);
 	if (ret) {
 		dev_err(dev->dev, "Failed to register PTP clock: %d\n",

-- 
2.55.0

[PATCH net-next v3 05/10] net: dsa: microchip: extract ptp_get_pin

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_enable_perout() function needs to be created. This
function will use the same algorithm to select the output pin as the
common ksz_ptp_enable_perout() function.

Extract the pin selection algorithm into a dedicated function so it can
be used later by the KSZ8463 support.

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

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index ea22601f6b26..f97ea9d69ebc 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -183,17 +183,11 @@ static int ksz_ptp_configure_perout(struct ksz_device *dev,
 	return 0;
 }
 
-static int ksz_ptp_enable_perout(struct ksz_device *dev,
-				 struct ptp_perout_request const *request,
-				 int on)
+static int ksz_ptp_get_pin(struct ksz_device *dev,
+			   struct ptp_perout_request const *request)
 {
 	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
-	u64 req_pulse_width_ns;
-	u64 cycle_width_ns;
-	u64 pulse_width_ns;
-	int pin = 0;
-	u32 data32;
-	int ret;
+	int pin;
 
 	if (request->flags & ~PTP_PEROUT_DUTY_CYCLE)
 		return -EOPNOTSUPP;
@@ -206,6 +200,25 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 	if (pin < 0)
 		return -EINVAL;
 
+	return pin;
+}
+
+static int ksz_ptp_enable_perout(struct ksz_device *dev,
+				 struct ptp_perout_request const *request,
+				 int on)
+{
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	u64 req_pulse_width_ns;
+	u64 cycle_width_ns;
+	u64 pulse_width_ns;
+	int pin = 0;
+	u32 data32;
+	int ret;
+
+	pin = ksz_ptp_get_pin(dev, request);
+	if (pin < 0)
+		return pin;
+
 	data32 = FIELD_PREP(PTP_GPIO_INDEX, pin) |
 		 FIELD_PREP(PTP_TOU_INDEX, request->index);
 	ret = ksz_rmw32(dev, REG_PTP_UNIT_INDEX__4,

-- 
2.55.0

[PATCH net-next v3 06/10] net: dsa: microchip: extract compute_width

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_enable_perout() function needs to be created. This
function will use the same algorithm to compute the periodic cycles as
the common ksz_ptp_enable_perout() function.

Extract these algorithms into dedicated functions so they can be used
later by the KSZ8463 support.

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

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index f97ea9d69ebc..2f141df65eb0 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -203,12 +203,49 @@ static int ksz_ptp_get_pin(struct ksz_device *dev,
 	return pin;
 }
 
+static int ksz_ptp_compute_perout_cycle(struct ksz_device *dev,
+					struct ptp_perout_request const *request,
+					u64 *cycle_width_ns)
+{
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+
+	ptp_data->perout_target_time_first.tv_sec  = request->start.sec;
+	ptp_data->perout_target_time_first.tv_nsec = request->start.nsec;
+
+	ptp_data->perout_period.tv_sec = request->period.sec;
+	ptp_data->perout_period.tv_nsec = request->period.nsec;
+
+	*cycle_width_ns = timespec64_to_ns(&ptp_data->perout_period);
+	if ((*cycle_width_ns & TRIG_CYCLE_WIDTH_M) != *cycle_width_ns) {
+		*cycle_width_ns = 0;
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static u64 ksz_ptp_compute_perout_pulse(struct ksz_device *dev,
+					struct ptp_perout_request const *request,
+					u64 max_pulse_width)
+{
+	u64 req_pulse_width_ns;
+
+	if (request->flags & PTP_PEROUT_DUTY_CYCLE)
+		return request->on.sec * NSEC_PER_SEC +	request->on.nsec;
+
+	/* Use a duty cycle of 50%. Maximum pulse width supported by the
+	 * hardware is a little bit more than 125 ms.
+	 */
+	req_pulse_width_ns = (request->period.sec * NSEC_PER_SEC +
+			      request->period.nsec) / 2;
+	return min_t(u64, req_pulse_width_ns, max_pulse_width);
+}
+
 static int ksz_ptp_enable_perout(struct ksz_device *dev,
 				 struct ptp_perout_request const *request,
 				 int on)
 {
 	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
-	u64 req_pulse_width_ns;
 	u64 cycle_width_ns;
 	u64 pulse_width_ns;
 	int pin = 0;
@@ -234,29 +271,11 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 		ptp_data->tou_mode = KSZ_PTP_TOU_IDLE;
 		return 0;
 	}
-
-	ptp_data->perout_target_time_first.tv_sec  = request->start.sec;
-	ptp_data->perout_target_time_first.tv_nsec = request->start.nsec;
-
-	ptp_data->perout_period.tv_sec = request->period.sec;
-	ptp_data->perout_period.tv_nsec = request->period.nsec;
-
-	cycle_width_ns = timespec64_to_ns(&ptp_data->perout_period);
-	if ((cycle_width_ns & TRIG_CYCLE_WIDTH_M) != cycle_width_ns)
-		return -EINVAL;
-
-	if (request->flags & PTP_PEROUT_DUTY_CYCLE) {
-		pulse_width_ns = request->on.sec * NSEC_PER_SEC +
-			request->on.nsec;
-	} else {
-		/* Use a duty cycle of 50%. Maximum pulse width supported by the
-		 * hardware is a little bit more than 125 ms.
-		 */
-		req_pulse_width_ns = (request->period.sec * NSEC_PER_SEC +
-				      request->period.nsec) / 2;
-		pulse_width_ns = min_t(u64, req_pulse_width_ns,
-				       KSZ_MAX_PULSE_WIDTH);
-	}
+	ret = ksz_ptp_compute_perout_cycle(dev, request, &cycle_width_ns);
+	if (ret)
+		return ret;
+	pulse_width_ns = ksz_ptp_compute_perout_pulse(dev, request,
+						      KSZ_MAX_PULSE_WIDTH);
 
 	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns, TRIG_PULSE_WIDTH_M);
 	if (ret)

-- 
2.55.0

[PATCH net-next v3 07/10] net: dsa: microchip: extract prepare reset

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_restart_perout() function needs to be created. This
function will use the same request initialization as the common
ksz_ptp_restart_perout() function.

Extract the inialization into a dedicated function so it can be used
later by the KSZ8463 support.

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

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 2f141df65eb0..e262bc7448f2 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -788,11 +788,11 @@ static int ksz_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
 	return ret;
 }
 
-static int ksz_ptp_restart_perout(struct ksz_device *dev)
+static int ksz_ptp_prepare_restart_perout(struct ksz_device *dev,
+					  struct ptp_perout_request *request)
 {
 	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
 	s64 now_ns, first_ns, period_ns, next_ns;
-	struct ptp_perout_request request;
 	struct timespec64 next;
 	struct timespec64 now;
 	unsigned int count;
@@ -823,12 +823,24 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
 
 	/* Restart periodic output signal */
 	next = ns_to_timespec64(next_ns);
-	request.start.sec  = next.tv_sec;
-	request.start.nsec = next.tv_nsec;
-	request.period.sec  = ptp_data->perout_period.tv_sec;
-	request.period.nsec = ptp_data->perout_period.tv_nsec;
-	request.index = 0;
-	request.flags = 0;
+	request->start.sec  = next.tv_sec;
+	request->start.nsec = next.tv_nsec;
+	request->period.sec  = ptp_data->perout_period.tv_sec;
+	request->period.nsec = ptp_data->perout_period.tv_nsec;
+	request->index = 0;
+	request->flags = 0;
+
+	return 0;
+}
+
+static int ksz_ptp_restart_perout(struct ksz_device *dev)
+{
+	struct ptp_perout_request request;
+	int ret;
+
+	ret = ksz_ptp_prepare_restart_perout(dev, &request);
+	if (ret)
+		return ret;
 
 	return ksz_ptp_enable_perout(dev, &request, 1);
 }

-- 
2.55.0

[PATCH net-next v3 08/10] net: dsa: microchip: extract time update

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_settime() function needs to be created. This
function will access the same registers than the common
ksz_ptp_settime().

Extract the register accesses into a dedicated function so it can be
used later by the KSZ8463 support.

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

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index e262bc7448f2..aa73ad1f05d8 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -845,30 +845,42 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
 	return ksz_ptp_enable_perout(dev, &request, 1);
 }
 
-static int ksz_ptp_settime(struct ptp_clock_info *ptp,
-			   const struct timespec64 *ts)
+static int __ksz_ptp_settime(struct ksz_device *dev,
+			     const struct timespec64 *ts)
 {
-	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
-	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
 	const u16 *regs = dev->info->regs;
 	int ret;
 
-	mutex_lock(&ptp_data->lock);
-
 	/* Write to shadow registers and Load PTP clock */
 	ret = ksz_write16(dev, regs[PTP_RTC_SUB_NANOSEC], PTP_RTC_0NS);
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_write32(dev, regs[PTP_RTC_NANOSEC], ts->tv_nsec);
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_write32(dev, regs[PTP_RTC_SEC], ts->tv_sec);
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_rmw16(dev, regs[PTP_CLK_CTRL], PTP_LOAD_TIME, PTP_LOAD_TIME);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ksz_ptp_settime(struct ptp_clock_info *ptp,
+			   const struct timespec64 *ts)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	ret = __ksz_ptp_settime(dev, ts);
 	if (ret)
 		goto unlock;
 

-- 
2.55.0

[PATCH net-next v3 09/10] net: dsa: microchip: extract time adjustment

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_adjtime() function needs to be created. This
function will access the same registers as the common ksz_ptp_adjtime()
function.

Extract the register accesses into a dedicated function so it can be
used later by the KSZ8463 support.

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

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index aa73ad1f05d8..0a77b46cfad2 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -946,18 +946,13 @@ static int ksz_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 	return ret;
 }
 
-static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+static int __ksz_ptp_adjtime(struct ksz_device *dev, s64 delta)
 {
-	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
-	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
-	struct timespec64 delta64 = ns_to_timespec64(delta);
 	const u16 *regs = dev->info->regs;
 	s32 sec, nsec;
 	u16 data16;
 	int ret;
 
-	mutex_lock(&ptp_data->lock);
-
 	/* do not use ns_to_timespec64(),
 	 * both sec and nsec are subtracted by hw
 	 */
@@ -965,15 +960,15 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 
 	ret = ksz_write32(dev, regs[PTP_RTC_NANOSEC], abs(nsec));
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_write32(dev, regs[PTP_RTC_SEC], abs(sec));
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_read16(dev, regs[PTP_CLK_CTRL], &data16);
 	if (ret)
-		goto unlock;
+		return ret;
 
 	data16 |= PTP_STEP_ADJ;
 
@@ -984,6 +979,22 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 		data16 |= PTP_STEP_DIR;
 
 	ret = ksz_write16(dev, regs[PTP_CLK_CTRL], data16);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	struct timespec64 delta64 = ns_to_timespec64(delta);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	ret = __ksz_ptp_adjtime(dev, delta);
 	if (ret)
 		goto unlock;
 

-- 
2.55.0

[PATCH net-next v3 10/10] net: dsa: microchip: add periodic output support for the KSZ8463

[Bastien Curutchet (Schneider Electric)]

KSZ8463 has several periodic outputs. The way it handles them differ
from the other KSZ switches:
 - KSZ8463 has one set of registers for each of its 12 units while others
   switches share on set of configuration registers for all their units.
 - Maximum size of the KSZ8463 pulse width is smaller
 - KSZ8463 has 12 outputs while others only have 2

Add support for the KSZ8463 periodics outputs through a set of KSZ8463
specific functions.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz8.c        |   2 +-
 drivers/net/dsa/microchip/ksz_common.c  |   2 +
 drivers/net/dsa/microchip/ksz_ptp.c     | 258 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_ptp.h     |   4 +-
 drivers/net/dsa/microchip/ksz_ptp_reg.h |  16 ++
 5 files changed, 280 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
index f2a23e2e474a..6b08bb8fed5b 100644
--- a/drivers/net/dsa/microchip/ksz8.c
+++ b/drivers/net/dsa/microchip/ksz8.c
@@ -2478,7 +2478,7 @@ static int ksz8463_setup(struct dsa_switch *ds)
 		if (ret)
 			goto free_girq;
 
-		ksz_ptp_set_caps(ds);
+		ksz8463_ptp_set_caps(ds);
 		ret = ksz_ptp_clock_register(ds);
 		if (ret) {
 			dev_err(dev->dev, "Failed to register PTP clock: %d\n",
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 041bb4f96365..9e234ad71b72 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1161,6 +1161,8 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.supports_mii = {false, false, true},
 		.supports_rmii = {false, false, true},
 		.internal_phy = {true, true, false},
+		.n_pins = 12,
+		.n_per_out = 12,
 	},
 
 	[KSZ8563] = {
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 0a77b46cfad2..d33034892a8f 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -26,6 +26,7 @@
  */
 #define KSZ_MAX_DRIFT_CORR 6249999
 #define KSZ_MAX_PULSE_WIDTH 125000000LL
+#define KSZ8463_MAX_PULSE_WIDTH 500000LL
 
 #define KSZ_PTP_INC_NS 40ULL  /* HW clock is incremented every 40 ns (by 40) */
 #define KSZ_PTP_SUBNS_BITS 32
@@ -63,6 +64,17 @@ static int ksz_ptp_tou_gpio(struct ksz_device *dev)
 			 LED_SRC_PTP_GPIO_1 | LED_SRC_PTP_GPIO_2);
 }
 
+static int ksz8463_ptp_tou_reset(struct ksz_device *dev, u8 unit)
+{
+	int ret;
+
+	ret = ksz_rmw16(dev, KSZ8463_TOU_SW_RST, BIT(unit), BIT(unit));
+	if (ret)
+		return ret;
+
+	return ksz_rmw16(dev, KSZ8463_TOU_SW_RST, BIT(unit), 0);
+}
+
 static int ksz_ptp_tou_reset(struct ksz_device *dev, u8 unit)
 {
 	u32 data;
@@ -120,6 +132,28 @@ static int ksz_ptp_tou_target_time_set(struct ksz_device *dev,
 	return 0;
 }
 
+static int ksz8463_ptp_tou_start(struct ksz_device *dev, u8 unit)
+{
+	u16 data;
+	int ret;
+
+	ret = ksz_rmw16(dev, KSZ8463_TOU_EN, BIT(unit), BIT(unit));
+	if (ret)
+		return ret;
+
+	ret = ksz_read16(dev, KSZ8463_TOU_ACTIVE, &data);
+	if (ret)
+		return ret;
+
+	if (!(data & BIT(unit))) {
+		dev_err(dev->dev, "%s: Trigger unit%d error!\n", __func__,
+			unit);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static int ksz_ptp_tou_start(struct ksz_device *dev, u8 unit)
 {
 	u32 data;
@@ -147,6 +181,56 @@ static int ksz_ptp_tou_start(struct ksz_device *dev, u8 unit)
 	return 0;
 }
 
+static int ksz8463_ptp_configure_perout(struct ksz_device *dev,
+					u32 cycle_width_ns, u32 pulse_width_ns,
+					struct timespec64 const *target_time,
+					u8 index)
+{
+	struct ptp_pin_desc *pin = &dev->ptp_data.pin_config[index];
+	u16 cfg_base = KSZ8463_TRIG1_CFG + KSZ8463_TRIGN_CFG_SIZE * pin->chan;
+	u16 data;
+	int ret;
+
+	/* Hardware has only 32 bit */
+	if ((target_time->tv_sec & 0xffffffff) != target_time->tv_sec)
+		return -EINVAL;
+
+	data = KSZ8463_NOTIFY_BIT |
+	       FIELD_PREP(KSZ8463_PATTERN_M, TRIG_POS_PERIOD) |
+	       pin->index;
+	ret = ksz_write16(dev, cfg_base + KSZ8463_PATTERN_OFF, data);
+	if (ret)
+		return ret;
+
+	ret = ksz_write32(dev, cfg_base + KSZ8463_CYCLE_WIDTH_OFF,
+			  cycle_width_ns);
+	if (ret)
+		return ret;
+
+	/* Set cycle count 0 - Infinite */
+	ret = ksz_write16(dev, cfg_base + KSZ8463_CYCLE_CNT_OFF, 0);
+	if (ret)
+		return ret;
+
+	/* KSZ8463 uses a 8 ns unit value to compute the pulse width */
+	data = (pulse_width_ns / 8);
+	ret = ksz_write16(dev, cfg_base + KSZ8463_PULSE_WIDTH_OFF, data);
+	if (ret)
+		return ret;
+
+	ret = ksz_write32(dev, cfg_base + KSZ8463_TARGET_NSEC,
+			  target_time->tv_nsec);
+	if (ret)
+		return ret;
+
+	ret = ksz_write32(dev, cfg_base + KSZ8463_TARGET_SEC,
+			  target_time->tv_sec);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int ksz_ptp_configure_perout(struct ksz_device *dev,
 				    u32 cycle_width_ns, u32 pulse_width_ns,
 				    struct timespec64 const *target_time,
@@ -241,6 +325,58 @@ static u64 ksz_ptp_compute_perout_pulse(struct ksz_device *dev,
 	return min_t(u64, req_pulse_width_ns, max_pulse_width);
 }
 
+static int ksz8463_ptp_enable_perout(struct ksz_device *dev,
+				     struct ptp_perout_request const *request,
+				     int on)
+{
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	u64 cycle_width_ns;
+	u64 pulse_width_ns;
+	int pin;
+	int ret;
+
+	pin = ksz_ptp_get_pin(dev, request);
+	if (pin < 0)
+		return pin;
+
+	ret = ksz8463_ptp_tou_reset(dev, request->index);
+	if (ret)
+		return ret;
+
+	if (!on) {
+		ptp_data->tou_mode = KSZ_PTP_TOU_IDLE;
+		return 0;
+	}
+	ret = ksz_ptp_compute_perout_cycle(dev, request, &cycle_width_ns);
+	if (ret)
+		return ret;
+	pulse_width_ns = ksz_ptp_compute_perout_pulse(dev, request,
+						      KSZ8463_MAX_PULSE_WIDTH);
+
+	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns,
+				       KSZ8463_TRIG_PULSE_WIDTH_M);
+	if (ret)
+		return ret;
+
+	ret = ksz8463_ptp_configure_perout(dev, cycle_width_ns, pulse_width_ns,
+					   &ptp_data->perout_target_time_first,
+					   pin);
+	if (ret)
+		return ret;
+
+	ret = ksz_ptp_tou_gpio(dev);
+	if (ret)
+		return ret;
+
+	ret = ksz8463_ptp_tou_start(dev, request->index);
+	if (ret)
+		return ret;
+
+	ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT;
+
+	return 0;
+}
+
 static int ksz_ptp_enable_perout(struct ksz_device *dev,
 				 struct ptp_perout_request const *request,
 				 int on)
@@ -845,6 +981,18 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
 	return ksz_ptp_enable_perout(dev, &request, 1);
 }
 
+static int ksz8463_ptp_restart_perout(struct ksz_device *dev)
+{
+	struct ptp_perout_request request;
+	int ret;
+
+	ret = ksz_ptp_prepare_restart_perout(dev, &request);
+	if (ret)
+		return ret;
+
+	return ksz8463_ptp_enable_perout(dev, &request, 1);
+}
+
 static int __ksz_ptp_settime(struct ksz_device *dev,
 			     const struct timespec64 *ts)
 {
@@ -871,6 +1019,41 @@ static int __ksz_ptp_settime(struct ksz_device *dev,
 	return 0;
 }
 
+static int ksz8463_ptp_settime(struct ptp_clock_info *ptp,
+			       const struct timespec64 *ts)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	ret = __ksz_ptp_settime(dev, ts);
+	if (ret)
+		goto unlock;
+
+	switch (ptp_data->tou_mode) {
+	case KSZ_PTP_TOU_IDLE:
+		break;
+
+	case KSZ_PTP_TOU_PEROUT:
+		ret = ksz8463_ptp_restart_perout(dev);
+		if (ret)
+			goto unlock;
+
+		break;
+	}
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = *ts;
+	spin_unlock_bh(&ptp_data->clock_lock);
+
+unlock:
+	mutex_unlock(&ptp_data->lock);
+
+	return ret;
+}
+
 static int ksz_ptp_settime(struct ptp_clock_info *ptp,
 			   const struct timespec64 *ts)
 {
@@ -985,6 +1168,40 @@ static int __ksz_ptp_adjtime(struct ksz_device *dev, s64 delta)
 	return 0;
 }
 
+static int ksz8463_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	struct timespec64 delta64 = ns_to_timespec64(delta);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	ret = __ksz_ptp_adjtime(dev, delta);
+	if (ret)
+		goto unlock;
+
+	switch (ptp_data->tou_mode) {
+	case KSZ_PTP_TOU_IDLE:
+		break;
+
+	case KSZ_PTP_TOU_PEROUT:
+		ret = ksz8463_ptp_restart_perout(dev);
+		if (ret)
+			goto unlock;
+
+		break;
+	}
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = timespec64_add(ptp_data->clock_time, delta64);
+	spin_unlock_bh(&ptp_data->clock_lock);
+
+unlock:
+	mutex_unlock(&ptp_data->lock);
+	return ret;
+}
+
 static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
@@ -1019,6 +1236,26 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	return ret;
 }
 
+static int ksz8463_ptp_enable(struct ptp_clock_info *ptp,
+			      struct ptp_clock_request *req, int on)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	int ret;
+
+	switch (req->type) {
+	case PTP_CLK_REQ_PEROUT:
+		mutex_lock(&ptp_data->lock);
+		ret = ksz8463_ptp_enable_perout(dev, &req->perout, on);
+		mutex_unlock(&ptp_data->lock);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return ret;
+}
+
 static int ksz_ptp_enable(struct ptp_clock_info *ptp,
 			  struct ptp_clock_request *req, int on)
 {
@@ -1095,6 +1332,27 @@ static int ksz_ptp_start_clock(struct ksz_device *dev)
 	return 0;
 }
 
+void ksz8463_ptp_set_caps(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_ptp_data *ptp_data;
+
+	ptp_data = &dev->ptp_data;
+
+	ptp_data->caps.owner		= THIS_MODULE;
+	snprintf(ptp_data->caps.name, 16, "Microchip Clock");
+	ptp_data->caps.max_adj		= KSZ_MAX_DRIFT_CORR;
+	ptp_data->caps.gettime64	= ksz_ptp_gettime;
+	ptp_data->caps.settime64	= ksz8463_ptp_settime;
+	ptp_data->caps.adjfine		= ksz_ptp_adjfine;
+	ptp_data->caps.adjtime		= ksz8463_ptp_adjtime;
+	ptp_data->caps.do_aux_work	= ksz_ptp_do_aux_work;
+	ptp_data->caps.enable		= ksz8463_ptp_enable;
+	ptp_data->caps.verify		= ksz_ptp_verify_pin;
+	ptp_data->caps.n_pins		= dev->info->n_pins;
+	ptp_data->caps.n_per_out	= dev->info->n_per_out;
+}
+
 void ksz_ptp_set_caps(struct dsa_switch *ds)
 {
 	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 89716b31596b..3838a73a8302 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -12,7 +12,7 @@
 
 #include <linux/ptp_clock_kernel.h>
 
-#define KSZ_PTP_N_GPIO		2
+#define KSZ_PTP_N_GPIO		12
 
 enum ksz_ptp_tou_mode {
 	KSZ_PTP_TOU_IDLE,
@@ -34,6 +34,7 @@ struct ksz_ptp_data {
 };
 
 void ksz_ptp_set_caps(struct dsa_switch *ds);
+void ksz8463_ptp_set_caps(struct dsa_switch *ds);
 int ksz_ptp_clock_register(struct dsa_switch *ds);
 
 void ksz_ptp_clock_unregister(struct dsa_switch *ds);
@@ -67,6 +68,7 @@ struct ksz_ptp_data {
 };
 
 static inline void ksz_ptp_set_caps(struct dsa_switch *ds) { }
+static inline void ksz8463_ptp_set_caps(struct dsa_switch *ds) { }
 static inline int ksz_ptp_clock_register(struct dsa_switch *ds)
 {
 	return 0;
diff --git a/drivers/net/dsa/microchip/ksz_ptp_reg.h b/drivers/net/dsa/microchip/ksz_ptp_reg.h
index 65ea8577af75..16f117fa5d49 100644
--- a/drivers/net/dsa/microchip/ksz_ptp_reg.h
+++ b/drivers/net/dsa/microchip/ksz_ptp_reg.h
@@ -51,6 +51,22 @@
 
 #define REG_PTP_UNIT_INDEX__4		0x0520
 
+#define KSZ8463_TOU_ACTIVE		0x202
+#define KSZ8463_TOU_EN			0x206
+#define KSZ8463_TOU_SW_RST		0x208
+
+#define KSZ8463_TRIGN_CFG_SIZE		0x20
+#define KSZ8463_TRIG1_CFG		0x220
+#define KSZ8463_TARGET_NSEC		0x000
+#define KSZ8463_TARGET_SEC		0x004
+#define KSZ8463_PATTERN_OFF		0x008
+#define KSZ8463_NOTIFY_BIT		BIT(8)
+#define KSZ8463_PATTERN_M		GENMASK(6, 4)
+#define KSZ8463_PULSE_WIDTH_OFF		0x00A
+#define KSZ8463_TRIG_PULSE_WIDTH_M	GENMASK(15, 0)
+#define KSZ8463_CYCLE_WIDTH_OFF		0x00C
+#define KSZ8463_CYCLE_CNT_OFF		0x010
+
 #define PTP_GPIO_INDEX			GENMASK(19, 16)
 #define PTP_TSI_INDEX			BIT(8)
 #define PTP_TOU_INDEX			GENMASK(1, 0)

-- 
2.55.0

Re: [PATCH net-next v3 01/10] net: dsa: microchip: add the number of pins to chip infos

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 2

- [Low] In ksz_ptp_clock_register()
  (drivers/net/dsa/microchip/ksz_ptp.c:1053,1060) both caps.n_pins and…
- [Low] The KSZ8463 entry in ksz_switch_chips[]
  (drivers/net/dsa/microchip/ksz_common.c, [KSZ8463] block around line…

--

net: dsa: microchip: add the number of pins to chip infos

This adds an n_pins field to struct ksz_chip_data and sets it to 2 for
every PTP-capable switch, replacing the hardcoded KSZ_PTP_N_GPIO used by
ksz_ptp_clock_register() for caps.n_pins and for the pin_config[]
initialization loop.

> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 1a9d6f83a0232..d01fba253af57 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1194,6 +1194,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
>  		.ptp_capable = true,
>  		.wr_table = &ksz8563_register_set,
>  		.rd_table = &ksz8563_register_set,
> +		.n_pins = 2,
>  	},
>  
>  	[KSZ8795] = {

[Severity: Low]
The commit message says ".n_pins" is set "for all the PTP-capable
switches", but the KSZ8463 entry in ksz_switch_chips[] does not get an
.n_pins value here, so it stays 0.

Is that intentional?  KSZ8463 does register a PTP clock, and the call is
not gated on info->ptp_capable:

drivers/net/dsa/microchip/ksz8.c:ksz8463_setup() {
	...
	if (dev->irq > 0) {
		...
		ret = ksz_ptp_clock_register(ds);
	...
}

With caps.n_pins coming from dev->info->n_pins, KSZ8463 goes from
advertising two PTP pins to advertising none: no ptp_pin_desc is
initialized, the pins directory of the PTP device disappears,
PTP_PIN_SETFUNC fails, and ptp_find_pin() in ksz_ptp_enable_perout()
can no longer match a pin:

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_enable_perout() {
	...
	pin = ptp_find_pin(ptp_data->clock, PTP_PF_PEROUT, request->index);
	if (pin < 0)
		return -EINVAL;
	...
}

The last patch of the series does give KSZ8463 ".n_pins = 12", so this
only affects the intermediate commits, but would it be better to set
KSZ8463 here as well so no commit in the series shrinks the pin
interface?

[ ... ]

> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index cbe98494578c3..c678fc6597dc4 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -139,6 +139,7 @@ struct ksz_chip_data {
>  	u8 sgmii_port;
>  	const struct regmap_access_table *wr_table;
>  	const struct regmap_access_table *rd_table;
> +	const u8 n_pins;
>  };
>  
>  struct ksz_irq {
> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 39cc70d65900b..c92936bf78d3d 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -1050,14 +1050,14 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
>  	ptp_data->caps.do_aux_work	= ksz_ptp_do_aux_work;
>  	ptp_data->caps.enable		= ksz_ptp_enable;
>  	ptp_data->caps.verify		= ksz_ptp_verify_pin;
> -	ptp_data->caps.n_pins		= KSZ_PTP_N_GPIO;
> +	ptp_data->caps.n_pins		= dev->info->n_pins;
>  	ptp_data->caps.n_per_out	= 3;
>  
>  	ret = ksz_ptp_start_clock(dev);
>  	if (ret)
>  		return ret;
>  
> -	for (i = 0; i < KSZ_PTP_N_GPIO; i++) {
> +	for (i = 0; i < dev->info->n_pins; i++) {
>  		struct ptp_pin_desc *ptp_pin = &ptp_data->pin_config[i];
>  
>  		snprintf(ptp_pin->name,

[Severity: Low]
Can this loop overflow ksz_ptp_data.pin_config[]?  The bound is now the
per-chip n_pins, while the destination array is still sized by the
compile-time constant:

drivers/net/dsa/microchip/ksz_ptp.h
#define KSZ_PTP_N_GPIO		2

struct ksz_ptp_data {
	...
	struct ptp_pin_desc pin_config[KSZ_PTP_N_GPIO];
	/* Serializes all operations on the PTP hardware clock */
	struct mutex lock;
	...
};

After this change nothing ties n_pins to ARRAY_SIZE(pin_config): there
is no clamp, no min(), and no BUILD_BUG_ON.  A chip entry with n_pins
larger than KSZ_PTP_N_GPIO would write past pin_config[] into lock,
clock_lock, clock_time and the perout state of struct ksz_device, and
would also hand the PTP core an n_pins larger than the array it indexes
from PTP_PIN_GETFUNC, PTP_PIN_SETFUNC, ptp_populate_pin_groups() and
ptp_find_pin().

Every n_pins initializer in this commit is 2, and the later commit that
raises KSZ8463 to 12 also changes KSZ_PTP_N_GPIO to 12 in the same diff,
so no commit in the series can trigger this today.  Would it still be
worth sizing pin_config[] from a maximum constant and adding a
BUILD_BUG_ON(dev->info->n_pins > ARRAY_SIZE(ptp_data->pin_config))
style check, so the two values cannot drift apart?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-ksz-perout-v3-0-6722a3f1ca75%40bootlin.com

Re: [PATCH net-next v3 02/10] net: dsa: microchip: add the number of periodic signals to chip infos

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 (1) · Low: 1

- [Low] KSZ8463 chip descriptor is left without .n_per_out (and, from the
  preceding patch "net: dsa: microchip: add the number of pins to chip…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the patch is
  the enabling step for it: the number of advertised periodic-output…

--

net: dsa: microchip: add the number of periodic signals to chip infos

This adds an n_per_out field to struct ksz_chip_data and makes
ksz_ptp_clock_register() advertise dev->info->n_per_out instead of the
hardcoded value 3.  The field is set to 3 in the chip descriptors of the
switches marked ptp_capable.

> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index d01fba253af57..041bb4f963659 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1195,6 +1195,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
>  		.wr_table = &ksz8563_register_set,
>  		.rd_table = &ksz8563_register_set,
>  		.n_pins = 2,
> +		.n_per_out = 3,
>  	},
>  
>  	[KSZ8795] = {

[ ... remaining ksz_switch_chips[] hunks snipped ... ]

> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index c678fc6597dc4..1155b39350c2b 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -140,6 +140,7 @@ struct ksz_chip_data {
>  	const struct regmap_access_table *wr_table;
>  	const struct regmap_access_table *rd_table;
>  	const u8 n_pins;
> +	const u8 n_per_out;
>  };
>  
>  struct ksz_irq {
> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index c92936bf78d3d..f80015250e5ab 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -1051,7 +1051,7 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
>  	ptp_data->caps.enable		= ksz_ptp_enable;
>  	ptp_data->caps.verify		= ksz_ptp_verify_pin;
>  	ptp_data->caps.n_pins		= dev->info->n_pins;
> -	ptp_data->caps.n_per_out	= 3;
> +	ptp_data->caps.n_per_out	= dev->info->n_per_out;

[Severity: Low]
Does this leave KSZ8463 advertising zero periodic outputs at this point in
the series?

The ksz_switch_chips[KSZ8463] descriptor sets neither .ptp_capable, .n_pins
nor .n_per_out, so both new fields are implicitly 0 here.  The commit message
says "Set it to 3 for all the PTP-capable switches", which is literally
consistent since KSZ8463 does not set .ptp_capable, but KSZ8463 still
registers a PHC:

drivers/net/dsa/microchip/ksz8.c:ksz8463_setup() {
	...
	if (dev->irq > 0) {
		...
		ret = ksz_ptp_clock_register(ds);
	...
}

Unlike ksz9477_setup()/ksz8_setup(), that call is gated only on dev->irq > 0
and not on dev->info->ptp_capable, so on KSZ8463 caps.n_per_out becomes 0
where the literal 3 was used before.  drivers/ptp/ptp_chardev.c then rejects
every request:

drivers/ptp/ptp_chardev.c:ptp_enable_perout() {
	...
	if (req.perout.index >= ops->n_per_out)
		return -EINVAL;
	...
}

PTP_PF_PEROUT pin assignment is refused the same way and ptp_sysfs.c hides
the period attribute.

The last patch of the series ("net: dsa: microchip: add periodic output
support for the KSZ8463") adds .n_pins = 12 and .n_per_out = 12 to that
descriptor, so only bisected intermediate trees see this.  Note also that in
this intermediate state caps.n_pins is 0 for KSZ8463 as well, so the perout
path there cannot resolve a pin anyway.  Would it be simpler to set the
KSZ8463 values in the same patch that introduces the field?

[Severity: Medium]
This isn't a bug introduced by this patch, but making the channel count a
per-chip property widens an existing mismatch: each perout index maps to a
distinct hardware Trigger Output Unit, while struct ksz_ptp_data keeps a
single tou_mode, perout_period and perout_target_time_first.

ksz_ptp_enable_perout() programs and starts the unit selected by the
request:

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_enable_perout() {
	...
	data32 = FIELD_PREP(PTP_GPIO_INDEX, pin) |
		 FIELD_PREP(PTP_TOU_INDEX, request->index);
	...
	ptp_data->perout_period.tv_sec = request->period.sec;
	...
}

but the restart path after a clock step only ever touches unit 0:

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_restart_perout() {
	...
	request.index = 0;
	request.flags = 0;

	return ksz_ptp_enable_perout(dev, &request, 1);
}

Can this leave units 1 and 2 armed with TRIG_TARGET_SEC/NANOSEC values
computed against the pre-step RTC after PTP_CLOCK_SETTIME or ADJTIME, so
their pulses fire immediately or far in the future?

There is a second effect when only a non-zero channel has a pin assigned:
ksz_ptp_settime() writes PTP_LOAD_TIME first, then
ksz_ptp_restart_perout() -> ksz_ptp_enable_perout() -> ptp_find_pin(clock,
PTP_PF_PEROUT, 0) returns -EINVAL, so settime() reports failure although the
RTC was already loaded.  ksz_ptp_verify_pin() accepts PTP_PF_PEROUT for any
pin and channel, so a user of /dev/ptpN can reach this by binding pin 0 to
PTP_PF_PEROUT channel 1 and issuing PTP_PEROUT_REQUEST with index 1.

The hardcoded index 0 is still present at the end of the series in the
extracted prepare/reset helper, while KSZ8463 then advertises 12 channels
with the same single-instance bookkeeping.  Should the driver track per
channel state, or restart all armed units?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-ksz-perout-v3-0-6722a3f1ca75%40bootlin.com

Re: [PATCH net-next v3 06/10] net: dsa: microchip: extract compute_width

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 (1) · Medium: 0 (1) · Low: 1

- [Low] The new helper `ksz_ptp_compute_perout_pulse()`
  (drivers/net/dsa/microchip/ksz_ptp.c:227) declares a `struct…

Pre-existing issues:
- [High] A non-normalized but individually non-zero periodic-output
  request is accepted and stored as an effective period of zero, after…
- [Medium] The KSZ PTP driver implements `PTP_PEROUT_DUTY_CYCLE`
  (accepted in `ksz_ptp_get_pin()`, consumed in the newly extracted…

--

net: dsa: microchip: extract compute_width

This moves the periodic-output cycle width and pulse width computations out
of ksz_ptp_enable_perout() into two new helpers,
ksz_ptp_compute_perout_cycle() and ksz_ptp_compute_perout_pulse(), so that
an upcoming ksz8463_ptp_enable_perout() can reuse them.  No functional
change is intended.

A few questions below.

> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index f97ea9d69ebc5..2f141df65eb01 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -203,12 +203,49 @@ static int ksz_ptp_get_pin(struct ksz_device *dev,
>  	return pin;
>  }
>  
> +static int ksz_ptp_compute_perout_cycle(struct ksz_device *dev,
> +					struct ptp_perout_request const *request,
> +					u64 *cycle_width_ns)
> +{
> +	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +
> +	ptp_data->perout_target_time_first.tv_sec  = request->start.sec;
> +	ptp_data->perout_target_time_first.tv_nsec = request->start.nsec;
> +
> +	ptp_data->perout_period.tv_sec = request->period.sec;
> +	ptp_data->perout_period.tv_nsec = request->period.nsec;
> +
> +	*cycle_width_ns = timespec64_to_ns(&ptp_data->perout_period);
> +	if ((*cycle_width_ns & TRIG_CYCLE_WIDTH_M) != *cycle_width_ns) {
> +		*cycle_width_ns = 0;
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

[Severity: High]
This isn't a bug introduced by this patch, since the code is moved
unchanged, but can a period that sums to zero get past this check and
later divide by zero?

The core does not normalize the request.  In ptp_perout_request() the
enable argument is computed from the raw fields:

drivers/ptp/ptp_chardev.c:ptp_perout_request() {
    ...
	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &ptp->pincfg_mux)
		return ops->enable(ops, &req, perout->period.sec || perout->period.nsec);
    ...
}

So a request with period.sec = -1 and period.nsec = 1000000000 arrives
here with on = 1.  ksz_ptp_compute_perout_cycle() stores both raw fields
into ptp_data->perout_period, and timespec64_to_ns() then yields
-1e9 + 1e9 = 0.  The guard passes because 0 & TRIG_CYCLE_WIDTH_M == 0.

ksz_ptp_compute_perout_pulse() returns 0 for the same input, and
ksz_ptp_tou_pulse_verify(0, TRIG_PULSE_WIDTH_M) also returns 0, so the
configuration succeeds and tou_mode becomes KSZ_PTP_TOU_PEROUT.

A later PTP_CLOCK_SETTIME or ADJTIME then reaches ksz_ptp_restart_perout()
with ptp_data->lock held:

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_restart_perout() {
    ...
	period_ns = timespec64_to_ns(&ptp_data->perout_period);

	if (first_ns < now_ns) {
		count = div_u64(now_ns - first_ns, period_ns);
    ...
	/* Ensure 100 ms guard time prior next event */
	while (next_ns < now_ns + 100000000)
		next_ns += period_ns;
    ...
}

With period_ns == 0, is the div_u64() a divide error, and does the guard
time loop spin forever when first_ns >= now_ns?  In both cases
ptp_data->lock is never released, so subsequent PTP operations on the
switch would block.

Since this helper is becoming the shared entry point for the KSZ8463 too,
would it make sense to reject a computed cycle width of zero here?

> +
> +static u64 ksz_ptp_compute_perout_pulse(struct ksz_device *dev,
> +					struct ptp_perout_request const *request,
> +					u64 max_pulse_width)
> +{
> +	u64 req_pulse_width_ns;
> +
> +	if (request->flags & PTP_PEROUT_DUTY_CYCLE)
> +		return request->on.sec * NSEC_PER_SEC +	request->on.nsec;

[Severity: Low]
This isn't a bug, but the dev parameter of
ksz_ptp_compute_perout_pulse() is never used in the function body, which
only reads request->flags, request->on, request->period and
max_pulse_width.  Since the chip specifics are conveyed through
max_pulse_width, could dev be dropped from the signature before the
KSZ8463 caller is added?  The only current caller,
ksz_ptp_enable_perout(), passes it needlessly.

[Severity: Medium]
This is a pre-existing issue, but is this PTP_PEROUT_DUTY_CYCLE branch
reachable at all?

ksz_ptp_set_caps() fills in caps.enable, caps.verify, caps.n_pins and
caps.n_per_out but never sets caps.supported_perout_flags, and the PTP
core has no default for it:

drivers/ptp/ptp_chardev.c:ptp_perout_request() {
    ...
	if (perout->flags & ~ops->supported_perout_flags)
		return -EOPNOTSUPP;
    ...
}

With supported_perout_flags == 0, every PTP_PEROUT_REQUEST2 carrying
PTP_PEROUT_DUTY_CYCLE is rejected before ops->enable() runs, so
ksz_ptp_get_pin() and this branch never see the flag.

Since this branch is now shared with the upcoming KSZ8463 support, should
ksz_ptp_set_caps() also set

	ptp_data->caps.supported_perout_flags = PTP_PEROUT_DUTY_CYCLE;

> +
> +	/* Use a duty cycle of 50%. Maximum pulse width supported by the
> +	 * hardware is a little bit more than 125 ms.
> +	 */
> +	req_pulse_width_ns = (request->period.sec * NSEC_PER_SEC +
> +			      request->period.nsec) / 2;
> +	return min_t(u64, req_pulse_width_ns, max_pulse_width);
> +}
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-ksz-perout-v3-0-6722a3f1ca75%40bootlin.com

Re: [PATCH net-next v3 07/10] net: dsa: microchip: extract prepare reset

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 (1) · Medium: 0 · Low: 1

- [Low] Commit subject mismatch: the subject line "net: dsa: microchip:
  extract prepare reset" does not describe the change.

Pre-existing issues:
- [High] Integer truncation causes an effectively unbounded loop in
  ksz_ptp_prepare_restart_perout()…

--

net: dsa: microchip: extract prepare reset

This patch moves the periodic output request initialization out of
ksz_ptp_restart_perout() into a new helper ksz_ptp_prepare_restart_perout(),
so a future ksz8463_ptp_restart_perout() can reuse it.  No functional change
is intended in the moved code.

> net: dsa: microchip: extract prepare reset

[Severity: Low]
Should the subject say "extract prepare restart" instead?  Nothing related
to a reset is touched here: ksz_ptp_tou_reset() and the TRIG_RESET handling
are unchanged, and the new helper is named ksz_ptp_prepare_restart_perout().

Also in the changelog:

  Extract the inialization into a dedicated function so it can be used
  later by the KSZ8463 support.

"inialization" should be "initialization".

> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 2f141df65eb01..e262bc7448f29 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -788,11 +788,11 @@ static int ksz_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
>  	return ret;
>  }
>  
> -static int ksz_ptp_restart_perout(struct ksz_device *dev)
> +static int ksz_ptp_prepare_restart_perout(struct ksz_device *dev,
> +					  struct ptp_perout_request *request)
>  {
>  	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
>  	s64 now_ns, first_ns, period_ns, next_ns;
> -	struct ptp_perout_request request;
>  	struct timespec64 next;
>  	struct timespec64 now;
>  	unsigned int count;

[Severity: High]
This isn't a bug introduced by this patch, but since the calculation is now
being moved into a helper that a second caller will use, is the truncation of
count into unsigned int in the moved code intentional?

The body that moves with the function does:

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_prepare_restart_perout() {
	unsigned int count;
	...
	if (first_ns < now_ns) {
		count = div_u64(now_ns - first_ns, period_ns);
		next_ns = first_ns + count * period_ns;
	} else {
		next_ns = first_ns;
	}

	/* Ensure 100 ms guard time prior next event */
	while (next_ns < now_ns + 100000000)
		next_ns += period_ns;
	...
}

Can count wrap here?  div_u64() returns a u64 quotient, so if the distance
between ptp_data->perout_target_time_first and the freshly written clock
value exceeds 2^32 periods, next_ns ends up far behind now_ns and the guard
time loop has to close that gap one period at a time, with no iteration cap,
no cond_resched() and no signal check.

The path is reachable from userspace:

ksz_ptp_settime()
    mutex_lock(&ptp_data->lock);
    ...
    case KSZ_PTP_TOU_PEROUT:
        ret = ksz_ptp_restart_perout(dev);
            ksz_ptp_prepare_restart_perout()

Enabling a periodic output while the PHC is still near zero with a short
period (for example 100 ms, which passes ksz_ptp_compute_perout_cycle()'s
TRIG_CYCLE_WIDTH_M check, and whose derived pulse passes
ksz_ptp_tou_pulse_verify()) and then issuing PTP_CLOCK_SETTIME with a
wall-clock value gives roughly 1.7e10 iterations, and considerably more for
the smallest accepted periods.  Since ksz_ptp_settime() holds
ptp_data->lock across the call, does this hang the ioctl and block all other
PTP operations on the device, and stall the CPU on CONFIG_PREEMPT_NONE?

Would using a u64 count (or clamping the correction to a single modulo step
instead of the loop) be preferable before the helper gains a second caller?

The same calculation is still present at the end of the series
(ac820826db205f1767ca525d6e06b8b1de2200fe), so no later patch appears to
address it.

> @@ -823,12 +823,24 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
>  
>  	/* Restart periodic output signal */
>  	next = ns_to_timespec64(next_ns);
> -	request.start.sec  = next.tv_sec;
> -	request.start.nsec = next.tv_nsec;
> -	request.period.sec  = ptp_data->perout_period.tv_sec;
> -	request.period.nsec = ptp_data->perout_period.tv_nsec;
> -	request.index = 0;
> -	request.flags = 0;
> +	request->start.sec  = next.tv_sec;
> +	request->start.nsec = next.tv_nsec;
> +	request->period.sec  = ptp_data->perout_period.tv_sec;
> +	request->period.nsec = ptp_data->perout_period.tv_nsec;
> +	request->index = 0;
> +	request->flags = 0;
> +
> +	return 0;
> +}
> +
> +static int ksz_ptp_restart_perout(struct ksz_device *dev)
> +{
> +	struct ptp_perout_request request;
> +	int ret;
> +
> +	ret = ksz_ptp_prepare_restart_perout(dev, &request);
> +	if (ret)
> +		return ret;
>  
>  	return ksz_ptp_enable_perout(dev, &request, 1);
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-ksz-perout-v3-0-6722a3f1ca75%40bootlin.com

Re: [PATCH net-next v3 10/10] net: dsa: microchip: add periodic output support for the KSZ8463

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider:

Critical: 0 · High: 1 (2) · Medium: 2 · Low: 1

- [High] KSZ8463 advertises 12 periodic-output channels (.n_pins = 12,
  .n_per_out = 12, copied into caps by ksz8463_ptp_set_caps()) while the…
- [Medium] ksz8463_ptp_enable_perout() is not transactional: it calls
  ksz8463_ptp_tou_reset(dev, request->index) (stopping an already…
- [Medium] ksz8463_ptp_configure_perout() dereferences
  dev->ptp_data.pin_config[index]…
- [Low] ksz8463_ptp_set_caps() (drivers/net/dsa/microchip/ksz_ptp.c:1335)
  does not set ptp_data->caps.supported_perout_flags, while the code…

Pre-existing issues:
- [High] The new KSZ8463 settime/adjtime paths funnel into
  ksz_ptp_prepare_restart_perout(), whose catch-up arithmetic can…
- [High] A denormalized periodic-output period is accepted as zero
  nanoseconds and later causes a kernel divide-by-zero or an infinite…

--

net: dsa: microchip: add periodic output support for the KSZ8463

This adds KSZ8463 specific periodic output handling: per-unit trigger
configuration registers, a smaller maximum pulse width, and a new set of
ksz8463_* PTP callbacks installed by ksz8463_ptp_set_caps().  The chip
data now advertises 12 pins and 12 periodic output channels.

A few questions below about the new paths.

> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 041bb4f963659..9e234ad71b723 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1161,6 +1161,8 @@ const struct ksz_chip_data ksz_switch_chips[] = {
>  		.supports_mii = {false, false, true},
>  		.supports_rmii = {false, false, true},
>  		.internal_phy = {true, true, false},
> +		.n_pins = 12,
> +		.n_per_out = 12,
>  	},

[Severity: High]
Twelve periodic output channels are advertised here, but struct
ksz_ptp_data still keeps only one set of perout state (a single tou_mode,
one perout_target_time_first, one perout_period), and the restart helper
always rebuilds a request for channel 0:

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_prepare_restart_perout() {
	...
	request->index = 0;
	request->flags = 0;

	return 0;
}

Userspace can map pin 3 to PTP_PF_PEROUT chan 3 (ptp_set_pinfunc() only
checks chan < n_per_out, and ksz_ptp_verify_pin() accepts any PEROUT
pin/chan) and then arm PTP_PEROUT_REQUEST with index 3, which makes
ksz8463_ptp_enable_perout() program trigger unit 3 and set tou_mode =
KSZ_PTP_TOU_PEROUT.

The next clock_settime() then walks:

ksz8463_ptp_settime() -> ksz8463_ptp_restart_perout() ->
  ksz_ptp_prepare_restart_perout() (index = 0) ->
  ksz8463_ptp_enable_perout() -> ksz_ptp_get_pin() ->
  ptp_find_pin(clock, PTP_PF_PEROUT, 0)

With no pin mapped to chan 0 that returns -1 and ksz_ptp_get_pin() returns
-EINVAL.  Does that mean every clock_settime()/clock_adjtime() fails for as
long as a perout is armed on a channel other than 0?

And because the failure takes the goto unlock path after
__ksz_ptp_settime() has already loaded the hardware RTC, is
ptp_data->clock_time (used by ksz_tstamp_reconstruct()) left behind until
ksz_ptp_do_aux_work() happens to run?

If some pin is mapped to chan 0, the restart resets and re-arms unit 0 with
the last stored period and target instead, while the unit that is actually
running keeps its old absolute 32-bit target and is never re-armed.  Is
that intended?

There are two related effects: enabling a second channel overwrites the
stored period/target of the first, and disabling any single channel sets
the global tou_mode to KSZ_PTP_TOU_IDLE, after which clock steps take the
IDLE branch and the units still set in KSZ8463_TOU_EN are never
resynchronised.  Should the perout state become per-channel before
n_per_out = 12 is exposed?

> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 0a77b46cfad25..d33034892a8f6 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c

[ ... ]

> @@ -120,6 +132,28 @@ static int ksz_ptp_tou_target_time_set(struct ksz_device *dev,
>  	return 0;
>  }
>  
> +static int ksz8463_ptp_tou_start(struct ksz_device *dev, u8 unit)
> +{
> +	u16 data;
> +	int ret;
> +
> +	ret = ksz_rmw16(dev, KSZ8463_TOU_EN, BIT(unit), BIT(unit));
> +	if (ret)
> +		return ret;
> +
> +	ret = ksz_read16(dev, KSZ8463_TOU_ACTIVE, &data);
> +	if (ret)
> +		return ret;
> +
> +	if (!(data & BIT(unit))) {
> +		dev_err(dev->dev, "%s: Trigger unit%d error!\n", __func__,
> +			unit);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +

[ ... ]

> @@ -147,6 +181,56 @@ static int ksz_ptp_tou_start(struct ksz_device *dev, u8 unit)
>  	return 0;
>  }
>  
> +static int ksz8463_ptp_configure_perout(struct ksz_device *dev,
> +					u32 cycle_width_ns, u32 pulse_width_ns,
> +					struct timespec64 const *target_time,
> +					u8 index)
> +{
> +	struct ptp_pin_desc *pin = &dev->ptp_data.pin_config[index];
> +	u16 cfg_base = KSZ8463_TRIG1_CFG + KSZ8463_TRIGN_CFG_SIZE * pin->chan;

[Severity: Medium]
Is pin_config safe to read here with only ptp_data->lock held?  pin->chan
selects which trigger unit's register block gets rewritten, and
drivers/ptp/ptp_private.h documents the lock for that array as:

	struct mutex pincfg_mux; /* protect concurrent info->pin_config access */

The core holds pincfg_mux around ->enable() only, not around
->settime64()/->adjtime().  So on the new paths:

CPU0: clock_settime() -> ksz8463_ptp_settime() (takes ptp_data->lock only)
        -> ksz8463_ptp_restart_perout() -> ksz8463_ptp_enable_perout()
        -> ksz_ptp_get_pin() -> ptp_find_pin() -> ksz8463_ptp_configure_perout()
           reads pin_config[index].chan / .index

CPU1: PTP_PIN_SETFUNC -> ptp_set_pinfunc() (holds pincfg_mux) writes

	if (pin1) {
		ptp_disable_pinfunc(info, func, chan);
		pin1->func = PTP_PF_NONE;
		pin1->chan = 0;
	}
	...
	pin2->func = func;
	pin2->chan = chan;

Those stores happen after ptp_disable_pinfunc() has already returned from
ksz8463_ptp_enable() and released ptp_data->lock, and for func ==
PTP_PF_NONE no ->enable() call happens at all, so ptp_data->lock does not
close the window.  Can cfg_base then point at a different, possibly
active, trigger unit than the one reset and enabled via request->index and
silently rewrite its configuration?

Note that taking pincfg_mux inside ptp_data->lock would invert the order
the core establishes in ptp_set_pinfunc() -> ptp_disable_pinfunc() ->
ksz8463_ptp_enable(); caching the pin/chan under ptp_data->lock at
enable() time looks like it would avoid that.

> +	u16 data;
> +	int ret;
> +
> +	/* Hardware has only 32 bit */
> +	if ((target_time->tv_sec & 0xffffffff) != target_time->tv_sec)
> +		return -EINVAL;
> +
> +	data = KSZ8463_NOTIFY_BIT |
> +	       FIELD_PREP(KSZ8463_PATTERN_M, TRIG_POS_PERIOD) |
> +	       pin->index;
> +	ret = ksz_write16(dev, cfg_base + KSZ8463_PATTERN_OFF, data);
> +	if (ret)
> +		return ret;

[ ... ]

> +	return 0;
> +}
> +

[ ... ]

> @@ -241,6 +325,58 @@ static u64 ksz_ptp_compute_perout_pulse(struct ksz_device *dev,
>  	return min_t(u64, req_pulse_width_ns, max_pulse_width);
>  }
>  
> +static int ksz8463_ptp_enable_perout(struct ksz_device *dev,
> +				     struct ptp_perout_request const *request,
> +				     int on)
> +{
> +	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +	u64 cycle_width_ns;
> +	u64 pulse_width_ns;
> +	int pin;
> +	int ret;
> +
> +	pin = ksz_ptp_get_pin(dev, request);
> +	if (pin < 0)
> +		return pin;
> +
> +	ret = ksz8463_ptp_tou_reset(dev, request->index);
> +	if (ret)
> +		return ret;
> +
> +	if (!on) {
> +		ptp_data->tou_mode = KSZ_PTP_TOU_IDLE;
> +		return 0;
> +	}
> +	ret = ksz_ptp_compute_perout_cycle(dev, request, &cycle_width_ns);
> +	if (ret)
> +		return ret;

[Severity: Medium]
Is there a way back out of this sequence when a request is rejected?  The
running unit is reset before the request is validated, and
ksz_ptp_compute_perout_cycle() overwrites the saved schedule before it can
reject the period:

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_compute_perout_cycle() {
	ptp_data->perout_target_time_first.tv_sec  = request->start.sec;
	...
	*cycle_width_ns = timespec64_to_ns(&ptp_data->perout_period);
	if ((*cycle_width_ns & TRIG_CYCLE_WIDTH_M) != *cycle_width_ns) {
		*cycle_width_ns = 0;
		return -EINVAL;
	}
}

So with a perout already running, a PTP_PEROUT_REQUEST carrying a period
whose ns value does not fit 32 bits (period.sec >= 5) resets trigger unit
N, stores the rejected start/period, and returns -EINVAL, while tou_mode
stays KSZ_PTP_TOU_PEROUT from the previous successful enable.  Does every
later clock_settime()/clock_adjtime() then take the PEROUT branch, call
ksz8463_ptp_restart_perout() with the rejected parameters, fail again, and
jump to unlock after the RTC has already been written but before
ptp_data->clock_time is updated?

The same applies to the -EINVAL/-ERANGE returns from
ksz_ptp_tou_pulse_verify(), the 32-bit target check in
ksz8463_ptp_configure_perout(), any regmap error, and the -EIO from
ksz8463_ptp_tou_start().

Related: ksz8463_ptp_tou_start() sets KSZ8463_TOU_EN before reading
KSZ8463_TOU_ACTIVE and does not clear the enable bit when the read fails
or ACTIVE is not set, so does an operation reported as -EIO leave the unit
enabled in hardware?

> +	pulse_width_ns = ksz_ptp_compute_perout_pulse(dev, request,
> +						      KSZ8463_MAX_PULSE_WIDTH);
> +
> +	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns,
> +				       KSZ8463_TRIG_PULSE_WIDTH_M);
> +	if (ret)
> +		return ret;
> +
> +	ret = ksz8463_ptp_configure_perout(dev, cycle_width_ns, pulse_width_ns,
> +					   &ptp_data->perout_target_time_first,
> +					   pin);
> +	if (ret)
> +		return ret;
> +
> +	ret = ksz_ptp_tou_gpio(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ksz8463_ptp_tou_start(dev, request->index);
> +	if (ret)
> +		return ret;
> +
> +	ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT;
> +
> +	return 0;
> +}
> +

[ ... ]

> @@ -845,6 +981,18 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
>  	return ksz_ptp_enable_perout(dev, &request, 1);
>  }
>  
> +static int ksz8463_ptp_restart_perout(struct ksz_device *dev)
> +{
> +	struct ptp_perout_request request;
> +	int ret;
> +
> +	ret = ksz_ptp_prepare_restart_perout(dev, &request);
> +	if (ret)
> +		return ret;

[Severity: High]
This isn't a bug introduced by this patch, but the helper reached from here
truncates the catch-up quotient to 32 bits, and this adds a second entry
point into it:

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_prepare_restart_perout() {
	unsigned int count;
	...
	if (first_ns < now_ns) {
		count = div_u64(now_ns - first_ns, period_ns);
		next_ns = first_ns + count * period_ns;
	} else {
		next_ns = first_ns;
	}

	/* Ensure 100 ms guard time prior next event */
	while (next_ns < now_ns + 100000000)
		next_ns += period_ns;
}

count is only unsigned int, so once the true number of elapsed periods
passes 2^32, next_ns is left roughly 2^32 * period_ns in the past and the
guard loop advances one period per iteration, with ptp_data->lock held, in
ioctl context, and without cond_resched().

Small periods are accepted (TRIG_CYCLE_WIDTH_M is GENMASK(31, 0), and a
1 us period yields a 500 ns pulse that passes ksz_ptp_tou_pulse_verify()),
so a perout with a 1 us period armed a day earlier gives a true quotient
around 8.6e10.  Can the next clock_settime() ->
ksz8463_ptp_settime() -> ksz8463_ptp_restart_perout() then spin for that
many iterations under the PTP mutex, producing RCU stalls or soft lockups
and blocking every other PTP operation on the device?

Write access to /dev/ptpN is enough to reach both steps, since posix
dynamic clocks only require FMODE_WRITE.  Would widening count, or
bounding the accepted period, be worth doing here?

[Severity: High]
This is a pre-existing issue in the same shared helper, but the new
KSZ8463 clock update paths make it reachable for this chip too: a
denormalized period is accepted as zero nanoseconds.

PTP_PEROUT_REQUEST does not check that struct ptp_clock_time is
normalized, and the core enables the output because the fields are
nonzero:

drivers/ptp/ptp_chardev.c:ptp_perout_request() {
	...
	return ops->enable(ops, &req, perout->period.sec || perout->period.nsec);
}

So period.sec = -1 with period.nsec = 1000000000 reaches
ksz8463_ptp_enable_perout() with on = 1, ksz_ptp_compute_perout_cycle()
stores those raw fields into ptp_data->perout_period, timespec64_to_ns()
is 0, and 0 passes both the TRIG_CYCLE_WIDTH_M test and
ksz_ptp_tou_pulse_verify().

With a stored period of 0 and tou_mode still KSZ_PTP_TOU_PEROUT from an
earlier successful enable, the next clock_settime()/clock_adjtime() reaches
ksz_ptp_prepare_restart_perout():

	if (first_ns < now_ns) {
		count = div_u64(now_ns - first_ns, period_ns);

Does this divide by zero and Oops with ptp_data->lock held?  And in the
other branch, does

	while (next_ns < now_ns + 100000000)
		next_ns += period_ns;

add zero forever?  Would a normalized, strictly positive period check in
ksz_ptp_compute_perout_cycle() cover both cases?

> +
> +	return ksz8463_ptp_enable_perout(dev, &request, 1);
> +}
> +

[ ... ]

> @@ -871,6 +1019,41 @@ static int __ksz_ptp_settime(struct ksz_device *dev,
>  	return 0;
>  }
>  
> +static int ksz8463_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{
> +	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
> +	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
> +	int ret;
> +
> +	mutex_lock(&ptp_data->lock);
> +
> +	ret = __ksz_ptp_settime(dev, ts);
> +	if (ret)
> +		goto unlock;
> +
> +	switch (ptp_data->tou_mode) {
> +	case KSZ_PTP_TOU_IDLE:
> +		break;
> +
> +	case KSZ_PTP_TOU_PEROUT:
> +		ret = ksz8463_ptp_restart_perout(dev);
> +		if (ret)
> +			goto unlock;
> +
> +		break;
> +	}
> +
> +	spin_lock_bh(&ptp_data->clock_lock);
> +	ptp_data->clock_time = *ts;
> +	spin_unlock_bh(&ptp_data->clock_lock);
> +
> +unlock:
> +	mutex_unlock(&ptp_data->lock);
> +
> +	return ret;
> +}
> +

[ ... ]

> @@ -1095,6 +1332,27 @@ static int ksz_ptp_start_clock(struct ksz_device *dev)
>  	return 0;
>  }
>  
> +void ksz8463_ptp_set_caps(struct dsa_switch *ds)
> +{
> +	struct ksz_device *dev = ds->priv;
> +	struct ksz_ptp_data *ptp_data;
> +
> +	ptp_data = &dev->ptp_data;
> +
> +	ptp_data->caps.owner		= THIS_MODULE;
> +	snprintf(ptp_data->caps.name, 16, "Microchip Clock");
> +	ptp_data->caps.max_adj		= KSZ_MAX_DRIFT_CORR;
> +	ptp_data->caps.gettime64	= ksz_ptp_gettime;
> +	ptp_data->caps.settime64	= ksz8463_ptp_settime;
> +	ptp_data->caps.adjfine		= ksz_ptp_adjfine;
> +	ptp_data->caps.adjtime		= ksz8463_ptp_adjtime;
> +	ptp_data->caps.do_aux_work	= ksz_ptp_do_aux_work;
> +	ptp_data->caps.enable		= ksz8463_ptp_enable;
> +	ptp_data->caps.verify		= ksz_ptp_verify_pin;
> +	ptp_data->caps.n_pins		= dev->info->n_pins;
> +	ptp_data->caps.n_per_out	= dev->info->n_per_out;
> +}
> +

[Severity: Low]
supported_perout_flags is not set here, while the path installed by this
function implements PTP_PEROUT_DUTY_CYCLE: ksz_ptp_get_pin() only rejects
flags outside PTP_PEROUT_DUTY_CYCLE, and
ksz_ptp_compute_perout_pulse() consumes request->on:

	if (request->flags & PTP_PEROUT_DUTY_CYCLE)
		return request->on.sec * NSEC_PER_SEC +	request->on.nsec;

The core rejects the ioctl before ->enable() runs:

drivers/ptp/ptp_chardev.c:ptp_perout_request() {
	if (perout->flags & ~ops->supported_perout_flags)
		return -EOPNOTSUPP;
}

So does every PTP_PEROUT_REQUEST2 with PTP_PEROUT_DUTY_CYCLE fail with
-EOPNOTSUPP, leaving the duty cycle handling unreachable?  The same
omission exists in ksz_ptp_set_caps(), so the KSZ8463 is no worse than the
other chips, but should the new initializer either set
supported_perout_flags = PTP_PEROUT_DUTY_CYCLE or drop the duty cycle
handling?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-ksz-perout-v3-0-6722a3f1ca75%40bootlin.com

Re: [PATCH net-next v3 10/10] net: dsa: microchip: add periodic output support for the KSZ8463

[Bastien Curutchet]

Hi all,

On 9/10/26 12:30 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 1 (2) · Medium: 2 · Low: 1
> 
> - [High] KSZ8463 advertises 12 periodic-output channels (.n_pins = 12,
>    .n_per_out = 12, copied into caps by ksz8463_ptp_set_caps()) while the…
> - [Medium] ksz8463_ptp_enable_perout() is not transactional: it calls
>    ksz8463_ptp_tou_reset(dev, request->index) (stopping an already…
> - [Medium] ksz8463_ptp_configure_perout() dereferences
>    dev->ptp_data.pin_config[index]…
> - [Low] ksz8463_ptp_set_caps() (drivers/net/dsa/microchip/ksz_ptp.c:1335)
>    does not set ptp_data->caps.supported_perout_flags, while the code…
> 
> Pre-existing issues:
> - [High] The new KSZ8463 settime/adjtime paths funnel into
>    ksz_ptp_prepare_restart_perout(), whose catch-up arithmetic can…
> - [High] A denormalized periodic-output period is accepted as zero
>    nanoseconds and later causes a kernel divide-by-zero or an infinite…
> 
> --
> 
> net: dsa: microchip: add periodic output support for the KSZ8463
> 
> This adds KSZ8463 specific periodic output handling: per-unit trigger
> configuration registers, a smaller maximum pulse width, and a new set of
> ksz8463_* PTP callbacks installed by ksz8463_ptp_set_caps().  The chip
> data now advertises 12 pins and 12 periodic output channels.
> 
> A few questions below about the new paths.
> 
>> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
>> index 041bb4f963659..9e234ad71b723 100644
>> --- a/drivers/net/dsa/microchip/ksz_common.c
>> +++ b/drivers/net/dsa/microchip/ksz_common.c
>> @@ -1161,6 +1161,8 @@ const struct ksz_chip_data ksz_switch_chips[] = {
>>   		.supports_mii = {false, false, true},
>>   		.supports_rmii = {false, false, true},
>>   		.internal_phy = {true, true, false},
>> +		.n_pins = 12,
>> +		.n_per_out = 12,
>>   	},
> 
> [Severity: High]
> Twelve periodic output channels are advertised here, but struct
> ksz_ptp_data still keeps only one set of perout state (a single tou_mode,
> one perout_target_time_first, one perout_period), and the restart helper
> always rebuilds a request for channel 0:
> 
> drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_prepare_restart_perout() {
> 	...
> 	request->index = 0;
> 	request->flags = 0;
> 
> 	return 0;
> }
> 

This is an already existing bug. It made me realize that I forgot to 
send the fix ..

The fix conflicts with this series. I can either send the fix now, wait 
for it to enter net-next and then send a new iteration of this series; 
or send the fix once this series has been merged. I checked the other 
Sashiko comments, and IMHO, this comment is the only one that is really 
relevant.

What do you prefer ?


Best regards,
Bastien