Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Mon, 4 Dec 2023 16:29:12 +0100
From:   Oleksij Rempel <o.rempel@pengutronix.de>
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Kory Maincent <kory.maincent@bootlin.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Luis Chamberlain <mcgrof@kernel.org>,
        Russ Weight <russ.weight@linux.dev>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-doc@vger.kernel.org, devicetree@vger.kernel.org,
        Dent Project <dentproject@linuxfoundation.org>
Subject: Re: [PATCH net-next v2 4/8] net: ethtool: pse-pd: Expand pse
 commands with the PSE PoE interface
Message-ID: <20231204152912.GF981228@pengutronix.de>
References: <20231201-feature_poe-v2-0-56d8cac607fa@bootlin.com>
 <20231201-feature_poe-v2-4-56d8cac607fa@bootlin.com>
 <e0b143dc-ca7e-4762-bd0b-3acffad0932b@lunn.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <e0b143dc-ca7e-4762-bd0b-3acffad0932b@lunn.ch>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

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 |