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:   Sun, 4 Dec 2022 19:48:24 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc:     Piergiorgio Beruto <piergiorgio.beruto@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, Oleksij Rempel <o.rempel@pengutronix.de>
Subject: Re: [PATCH net-next 4/4] driver/ncn26000: add PLCA support
Message-ID: <Y4zreLCwdx+fyuCe@lunn.ch>
References: <cover.1670119328.git.piergiorgio.beruto@gmail.com>
 <38623984f6235a1521e6b0ad2ea958abc84ad708.1670119328.git.piergiorgio.beruto@gmail.com>
 <Y4zTqvSxLJG+G8V+@shell.armlinux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y4zTqvSxLJG+G8V+@shell.armlinux.org.uk>
Precedence: bulk

On Sun, Dec 04, 2022 at 05:06:50PM +0000, Russell King (Oracle) wrote:
> On Sun, Dec 04, 2022 at 03:32:06AM +0100, Piergiorgio Beruto wrote:
> > --- a/include/uapi/linux/mdio.h
> > +++ b/include/uapi/linux/mdio.h
> > @@ -26,6 +26,7 @@
> >  #define MDIO_MMD_C22EXT		29	/* Clause 22 extension */
> >  #define MDIO_MMD_VEND1		30	/* Vendor specific 1 */
> >  #define MDIO_MMD_VEND2		31	/* Vendor specific 2 */
> > +#define MDIO_MMD_OATC14		MDIO_MMD_VEND2
> 
> If this is in the vendor 2 register set, I doubt that this is a feature
> described by IEEE 802.3, since they allocated the entirety of this MMD
> over to manufacturers to do whatever they please with this space.
> 
> If this is correct, then these definitions have no place being in this
> generic header file, since they are likely specific to the vendors PHY.

Piergiorgio can give you the full details.

As i understand it, IEEE 802.3 defines the basic functionality, but
did not extend the standard to define the registers.

The Open Alliance member got together and added the missing parts, and
published an Open Alliance document.

Piergiorgio, i suggest you add a header file for these defines, named
to reflect that the Open Alliance defined them. And put in a comment,
explaining their origin, maybe a link to the standard. I also don't
think this needs to be a uapi header, they are not needed outside of
the kernel.

I also would not use MDIO_MMD_OATC14, but rather MDIO_MMD_VEND2. There
is no guarantee they are not being used for other things, and
MDIO_MMD_VEND2 gives a gentle warning about this.

	Andrew