Re: [PATCH net-next v7 1/3] net: dsa: microchip: implement KSZ87xx Module 3 low-loss cable errata

[Nicolai Buchwitz]

Hi Fidelio,

A couple more from testing on a KSZ8794, in case they help while you
look at the register-document question.

On 24.5.2026 12:44, Fidelio Lawson wrote:
> Implement the "Module 3: Equalizer fix for short cables" erratum from
> Microchip document DS80000687C for KSZ87xx switches.
>
> The issue affects short or low-loss cable links (e.g. CAT5e/CAT6),
> where the PHY receiver equalizer may amplify high-amplitude signals
> excessively, resulting in internal distortion and link establishment
> failures.
>
> KSZ87xx devices require a workaround for the Module 3 low-loss cable
> condition, controlled through the switch TABLE_LINK_MD_V indirect
> registers.
>
> This change models the erratum handling as vendor-specific Clause 22 
> PHY
> registers, virtualized by the KSZ8 DSA driver and accessed via
> ksz8_r_phy() / ksz8_w_phy(). The following controls are provided:
>
> - A boolean “short-cable” preset, which applies a documented and
>   conservative configuration (LPF 62 MHz bandwidth and DSP EQ initial
>   value 0), and is the recommended interface for typical use cases.
>
> - Separate LPF bandwidth and DSP EQ initial value controls intended for
>   advanced or experimental tuning. These are orthogonal and 
> independent,
>   and override the corresponding settings without requiring any 
> specific
>   ordering.
>
> The preset and tunables act as simple setters with no implicit state
> machine or invalid combinations, keeping the API predictable and 
> aligned
> with the KISS principle.
>
> The erratum affects the shared PHY analog front-end and therefore 
> applies
> globally to the switch.
>
> Fixes: e66f840c08a2 ("net: dsa: ksz: Add Microchip KSZ8795 DSA driver")
> Signed-off-by: Fidelio Lawson <fidelio.lawson@xxxxxxxxxx>
> ---

> [...]

> +
>  /**
>   * ksz8_r_phy_ctrl - Translates and reads from the SMI interface to a 
> MIIM PHY
>   *		     Control register (Reg. 31).
> @@ -1046,6 +1077,22 @@ static int ksz8_r_phy(struct ksz_device *dev, 
> u16 phy, u16 reg, u16 *val)
>  		if (ret)
>  			return ret;
>
> +		break;
> +	case PHY_REG_KSZ87XX_SHORT_CABLE:
> +		if (!ksz_is_ksz87xx(dev))
> +			return -EOPNOTSUPP;
> +		data = !!(dev->lpf_bw == KSZ87XX_PHY_LPF_62MHZ &&
> +				dev->eq_init == KSZ87XX_DSP_EQ_INIT_LOW_LOSS);

The cached state lives on struct ksz_device, but ksz8_r_phy() is
invoked per port. Setting the preset on one port is reported back as
active on the others:

  phytool write swp1/1/0x1a 1
  phytool read  swp1/2/0x1a -> 0x0001
  phytool read  swp1/3/0x1a -> 0x0001

The commit message already notes that the erratum applies switch-wide.
So writes from non-primary ports could either be rejected or at least 
produce
a dev_info_once() so userspace can see the setting is shared?

> +		break;
> +	case PHY_REG_KSZ87XX_LPF_BW:
> +		if (!ksz_is_ksz87xx(dev))
> +			return -EOPNOTSUPP;
> +		data = dev->lpf_bw;
> +		break;
> +	case PHY_REG_KSZ87XX_EQ_INIT:
> +		if (!ksz_is_ksz87xx(dev))
> +			return -EOPNOTSUPP;
> +		data = dev->eq_init;

dev->eq_init starts at zero from the kzalloc, but the header added by
this patch defines KSZ87XX_DSP_EQ_INIT_FACTORY = 0x0F as the hardware
default. After a fresh boot on my setup (no writes), the read returns 0:

  phytool read swp1/1/0x1c -> 0x0000

Should probe set dev->eq_init = KSZ87XX_DSP_EQ_INIT_FACTORY (and
dev->lpf_bw = KSZ87XX_PHY_LPF_90MHZ), or read the indirect registers
once on probe, so the cache reflects what the hardware actually holds?

> [...]

Thanks
Nicolai