Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 7 Dec 2022 15:16:00 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
Cc:     Jakub Kicinski <kuba@kernel.org>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, Oleksij Rempel <o.rempel@pengutronix.de>
Subject: Re: [PATCH v5 net-next 1/5] net/ethtool: add netlink interface for
 the PLCA RS
Message-ID: <Y5CgIL+cu4Fv43vy@lunn.ch>
References: <cover.1670371013.git.piergiorgio.beruto@gmail.com>
 <350e640b5c3c7b9c25f6fd749dc0237e79e1c573.1670371013.git.piergiorgio.beruto@gmail.com>
 <20221206195014.10d7ec82@kernel.org>
 <Y5CQY0pI+4DobFSD@gvm01>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <Y5CQY0pI+4DobFSD@gvm01>
Precedence: bulk

> > TBH I can't parse the "ETHTOOL_A_PLCA_VERSION is reported as 0Axx
> > where.." sentence. Specifically I'm confused about what the 0A is.
> How about this: "When this standard is supported, the upper byte of
> ``ETHTOOL_A_PLCA_VERSION`` shall be 0x0A (see Table A.1.0 — IDVER 
> bits assignment).

I think the 0x0A is pointless and should not be included here. If the
register does not contain 0x0A, the device does not follow the open
alliance standard, and hence the lower part of the register is
meaningless.

This is why i suggested -ENODEV should actually be returned on invalid
values in this register.

> > >   * struct ethtool_phy_ops - Optional PHY device options
> > >   * @get_sset_count: Get number of strings that @get_strings will write.
> > >   * @get_strings: Return a set of strings that describe the requested objects
> > >   * @get_stats: Return extended statistics about the PHY device.
> > > + * @get_plca_cfg: Return PLCA configuration.
> > > + * @set_plca_cfg: Set PLCA configuration.
> > 
> > missing get status in kdoc
> Fixed. Good catch.

Building with W=1 C=2 will tell you about kerneldoc issues. Ideally we
want all network code to be clean with these two options.

     Andrew