Re: [PATCH net-next 4/4] driver/ncn26000: add PLCA support

From: Andrew Lunn
Date: Sun Dec 04 2022 - 13:48:48 EST


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