Re: [PATCH net-next v2 4/8] net: ethtool: pse-pd: Expand pse commands with the PSE PoE interface

[Oleksij Rempel]

On Sun, Dec 03, 2023 at 07:45:18PM +0100, Andrew Lunn wrote:
> > @@ -143,6 +150,43 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
> >  		return -EOPNOTSUPP;
> >  	}
> >  
> > +	if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
> > +	    !tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL])
> > +		return 0;
> 
> -EINVAL? Is there a real use case for not passing either of them?
> 
> > +
> > +	if (tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
> > +	    !(pse_get_types(phydev->psec) & PSE_PODL)) {
> > +		NL_SET_ERR_MSG_ATTR(info->extack,
> > +				    tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL],
> > +				    "setting PSE PoDL admin control not supported");
> > +		return -EOPNOTSUPP;
> > +	}
> > +	if (tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL] &&
> > +	    !(pse_get_types(phydev->psec) & PSE_C33)) {
> > +		NL_SET_ERR_MSG_ATTR(info->extack,
> > +				    tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL],
> > +				    "setting PSE PoE admin control not supported");
> 
> This probably should be C33, not PoE?
> 
> I guess it depends on what the user space tools are using. 

The same problem is in the documentation. Mixing different naming
schemes is problematic. Even unmixed this "PoE" is not really suitable for most
use cases. Expanding this abbreviations make it probably more clear:
- PSE PoE - Power Source Equipment Power over Ethernet
- C33 PSE - Clause 33 Power Source Equipment

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |