Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 17 Apr 2020 11:11:45 +0100
From:   Russell King - ARM Linux admin <linux@armlinux.org.uk>
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Oleksij Rempel <o.rempel@pengutronix.de>,
        "David S. Miller" <davem@davemloft.net>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Michal Kubecek <mkubecek@suse.cz>,
        David Jander <david@protonic.nl>, kernel@pengutronix.de,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        mkl@pengutronix.de
Subject: Re: [PATCH v1] ethtool: provide UAPI for PHY master/slave
 configuration.
Message-ID: <20200417101145.GP25745@shell.armlinux.org.uk>
References: <20200415121209.12197-1-o.rempel@pengutronix.de>
 <20200415215739.GI657811@lunn.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200415215739.GI657811@lunn.ch>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 15, 2020 at 11:57:39PM +0200, Andrew Lunn wrote:
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index c8b0c34030d32..d5edf2bc40e43 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -604,6 +604,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
> >  	dev->asym_pause = 0;
> >  	dev->link = 0;
> >  	dev->interface = PHY_INTERFACE_MODE_GMII;
> > +	dev->master_slave = PORT_MODE_UNKNOWN;
> 
> phydev->master_slave is how we want the PHY to be configured. I don't
> think PORT_MODE_UNKNOWN makes any sense in that contest. 802.3 gives
> some defaults. 9.12 should be 0, meaning manual master/slave
> configuration is disabled. The majority of linux devices are end
> systems. So we should default to a single point device. So i would
> initialise PORT_MODE_SLAVE, or whatever we end up calling that.

I'm not sure that is a good idea given that we use phylib to drive
the built-in PHYs in DSA switches, which ought to prefer master mode
via the "is a multiport device" bit.

Just to be clear, there are three bits that configure 1G PHYs, which
I've framed in briefer terminology:

- 9.12: auto/manual configuration (1= manual 0= slave)
- 9.11: manual master/slave configuration (1= master, 0 = slave)
- 9.10: auto master/slave preference (1= multiport / master)

It is recommended that multiport devices (such as DSA switches) set
9.10 so they prefer to be master.

It's likely that the reason is to reduce cross-talk interference
between neighbouring ports both inside the PHY, magnetics and the
board itself. I would suspect that this becomes critical when
operating at towards the maximum cable length.

I've checked some of my DSA switches, and 9.10 appears to default to
one, as expected given what's in the specs.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up