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

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

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

- [High] Per-chip PTP capability fields `n_pins`/`n_per_out` are only
  populated for KSZ8563 (and, at the end of the series, KSZ8463), so…

--

net: dsa: microchip: extract PTP callbacks configuration from PTP registration

This moves the ptp_clock_info fields assignment out of
ksz_ptp_clock_register() into a new ksz_ptp_set_caps(), and calls the new
helper from each of the ksz8463/ksz8/ksz9477/lan937x setup paths before
registering the PTP clock.

[Severity: High]
Are the per-chip n_pins and n_per_out values actually populated for all
PTP capable switches?  ksz_ptp_set_caps() publishes dev->info->n_pins and
dev->info->n_per_out directly, but in ksz_common.c only the [KSZ8563]
entry initializes them:

	[KSZ8563] = {
		...
		.ptp_capable = true,
		...
		.n_pins = 2,
		.n_per_out = 3,

The other .ptp_capable = true entries ([KSZ9477], [KSZ9563], [KSZ8567],
[KSZ9567], [LAN9370], [LAN9371], [LAN9372], [LAN9373], [LAN9374]) leave
both fields at 0.

Before this rework the code hardcoded caps.n_pins = KSZ_PTP_N_GPIO (2) and
caps.n_per_out = 3 for every one of these chips, so does this leave them
advertising zero pins and zero periodic output channels?

With n_per_out == 0, ptp_chardev.c rejects every request:

	if (perout->index >= ops->n_per_out)
		return -EINVAL;

and with n_pins == 0 both PTP_PIN_SETFUNC and PTP_PIN_GETFUNC take the
same path:

	if (pd.index >= ops->n_pins)
		return -EINVAL;

+}
+
+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)

Related to the same question: the pin descriptor loop later in
ksz_ptp_clock_register() is bounded by dev->info->n_pins, so for the chips
above no ptp_pin_desc entry gets a name, index or func at all:

	for (i = 0; i < dev->info->n_pins; i++) {
		struct ptp_pin_desc *ptp_pin = &ptp_data->pin_config[i];

		snprintf(ptp_pin->name,
			 sizeof(ptp_pin->name), "ksz_ptp_pin_%02d", i);
		ptp_pin->index = i;
		ptp_pin->func = PTP_PF_NONE;
	}

[Severity: High]
lan937x_setup() registers the PTP clock unconditionally, so does every
LAN9370-LAN9374 device end up registering with n_pins = 0 and
n_per_out = 0 here?

Reading ksz_common.c at the end of the series (c35b1061) the initializers
are still only present for [KSZ8463] and [KSZ8563], so nothing later
appears to restore the values for the remaining chips.  Should the
.n_pins / .n_per_out initializers be added to the other .ptp_capable
entries?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831-ksz-perout-v1-0-14202db763b3%40bootlin.com