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

[Russell King (Oracle)]

On Sun, Dec 04, 2022 at 09:09:08PM +0100, Piergiorgio Beruto wrote:
> On Sun, Dec 04, 2022 at 07:48:24PM +0100, Andrew Lunn wrote:
> > 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.
> Thanks Andrew for commenting on this one. This is right, in the IEEE
> 802.3cg group we could not allocate an MMD for the PLCA reconciliation
> sublayer because of an 'unfriendly' wording in Clause 45 ruling out
> Reconciliation Sublayers from what can be configured via registers.
> Clause 45 says you can have registers for the PHY, while it should have
> said 'Physical Layer" and there is a subtle difference between the two
> words. PLCA, for example, is part of the Physical Layer but not of the
> PHY. Since we could not change that wording, we had to define
> configuration parameters in Clause 30, and let organizations outside the
> IEEE define memory maps for PHYs that integrate PLCA.
> 
> The OPEN Alliance SIG (see the reference in the patches) defined
> registers for the PLCA RS in MMD31, which is in fact vendor-specific
> from an IEEE perspective, but part of it is now standardized in the OPEN
> Alliance. So unfortunately we have to live with this somehow.
> 
> So ok, I can separate these definitions into a different non-UAPI header
> as Andrew is suggesting. I'll do this in the next patchset.

Sounds like yet another clause 45 mess :(

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!