Messages in this thread | | | Date | Fri, 30 Sep 2022 23:05:06 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH] net: mv643xx_eth: support MII/GMII/RGMII modes |
| |
On Sat, Oct 01, 2022 at 04:47:42AM +0800, Yangfl wrote: > Andrew Lunn <andrew@lunn.ch> 于2022年10月1日周六 04:28写道: > > > > On Sat, Oct 01, 2022 at 03:49:23AM +0800, David Yang wrote: > > > On device reset all ports are automatically set to RGMII mode. MII > > > mode must be explicitly enabled. > > > > > > If SoC has two Ethernet controllers, by setting both of them into MII > > > mode, the first controller enters GMII mode, while the second > > > controller is effectively disabled. This requires configuring (and > > > maybe enabling) the second controller in the device tree, even though > > > it cannot be used. > > > > > > Signed-off-by: David Yang <mmyangfl@gmail.com> > > > --- > > > drivers/net/ethernet/marvell/mv643xx_eth.c | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c > > > index b6be0552a..e2216ce5e 100644 > > > --- a/drivers/net/ethernet/marvell/mv643xx_eth.c > > > +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c > > > @@ -108,6 +108,7 @@ static char mv643xx_eth_driver_version[] = "1.4"; > > > #define TXQ_COMMAND 0x0048 > > > #define TXQ_FIX_PRIO_CONF 0x004c > > > #define PORT_SERIAL_CONTROL1 0x004c > > > +#define RGMII_EN 0x00000008 > > > #define CLK125_BYPASS_EN 0x00000010 > > > #define TX_BW_RATE 0x0050 > > > #define TX_BW_MTU 0x0058 > > > @@ -1245,6 +1246,21 @@ static void mv643xx_eth_adjust_link(struct net_device *dev) > > > > > > out_write: > > > wrlp(mp, PORT_SERIAL_CONTROL, pscr); > > > + > > > + /* If two Ethernet controllers present in the SoC, MII modes follow the > > > + * following matrix: > > > + * > > > + * Port0 Mode Port1 Mode Port0 RGMII_EN Port1 RGMII_EN > > > + * RGMII RGMII 1 1 > > > + * RGMII MII/MMII 1 0 > > > + * MII/MMII RGMII 0 1 > > > + * GMII N/A 0 0 > > > + * > > > + * To enable GMII on Port 0, Port 1 must also disable RGMII_EN too. > > > + */ > > > + if (!phy_interface_is_rgmii(dev->phydev)) > > > + wrlp(mp, PORT_SERIAL_CONTROL1, > > > + rdlp(mp, PORT_SERIAL_CONTROL1) & ~RGMII_EN); > > > > I could be reading this wrong, but doesn't this break the third line: > > > > > + * MII/MMII RGMII 0 1 > > > > Port 1 probes first, phy_interface is rgmii, so nothing happens, port1 > > RGMII_EN is left true. > > > > Port 0 then probes, MII/MMII is not RGMII, so port1 RGMII_EN is > > cleared, breaking port1. > > > > I think you need to be more specific with the comparison. > > > > Andrew > > Oh, I see. So you mean "phy-mode" property should belong to > controller, not port? I thought one controller can have at most one > port.
If you look at mv643xx_eth_shared_of_probe(), it appears a controller can have multiple ports. And:
if (dev_num == 3) { dev_err(&pdev->dev, "too many ports registered\n"); return -EINVAL; }
I don't know the details?
Andrew
| |