Re: [PATCH net-next v9 2/3] net: ethtool: add KSZ87xx low-loss cable PHY tunables

[Andrew Lunn]

On Thu, Jun 04, 2026 at 01:41:19PM +0200, Paolo Abeni wrote:
> From: AI Reviewer <ai@xxxxxxxxxxx>
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev/
> ---
> net: ethtool: add KSZ87xx low-loss cable PHY tunables
> 
> Introduce vendor-specific PHY tunable identifiers to control the
> KSZ87xx low-loss cable erratum handling through the ethtool PHY
> tunable interface.
> 
> > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> > index 1cdfb8341df24..a2091d4e00f3f 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -291,6 +291,9 @@ enum phy_tunable_id {
> >  	ETHTOOL_PHY_DOWNSHIFT,
> >  	ETHTOOL_PHY_FAST_LINK_DOWN,
> >  	ETHTOOL_PHY_EDPD,
> > +	ETHTOOL_PHY_SHORT_CABLE_PRESET,
> > +	ETHTOOL_PHY_LPF_BW,
> > +	ETHTOOL_PHY_DSP_EQ_INIT_VALUE,
> 
> [Severity: High]
> Are these vendor-specific hardware erratum tunables appropriate for the 
> generic global ethtool UAPI? 
> 
> These tunables use generic names for device-specific KSZ87xx workarounds. 
> Since UAPI is permanent, adding hardware-tied erratum parameters without a 
> vendor prefix or standard abstraction could lead to namespace pollution. Would 
> devlink param or device tree properties be a better fit for this?

Devlink is currently not used with PHYs. I could be, but it seems
overkill when we already have PHY tunables.

Device Tree properties don't make sense, because it is the cable which
matters here. It is 1m to the next rack, or 100m? DT does not cover
this.

The concepts are generic, but the specific values for the setting
could be per device. But i also doubt this is going to get much use,
and anybody actually using it and just going to try values until it
works.

I don't see any issues here we need to address.

	Andrew