Re: net: phy: marvell: network working with generic PHY and not with marvell PHY

[Corentin Labbe <clabbe.montjoie@gmail.com>]

Le Tue, Jan 04, 2022 at 12:11:55PM +0000, Russell King (Oracle) a écrit :
> On Tue, Jan 04, 2022 at 11:41:40AM +0000, Russell King (Oracle) wrote:
> > On Tue, Jan 04, 2022 at 12:33:15PM +0100, Corentin Labbe wrote:
> > > Le Tue, Jan 04, 2022 at 11:14:46AM +0000, Russell King (Oracle) a écrit :
> > > > On Tue, Jan 04, 2022 at 11:58:01AM +0100, Corentin Labbe wrote:
> > > > > Hello
> > > > > 
> > > > > I have a gemini SSI 1328 box which has a cortina ethernet MAC with a Marvell 88E1118 as given by:
> > > > > Marvell 88E1118 gpio-0:01: attached PHY driver (mii_bus:phy_addr=gpio-0:01, irq=POLL)
> > > > > So booting with CONFIG_MARVELL_PHY=y lead to a non-working network with link set at 1Gbit
> > > > > Setting 'max-speed = <100>;' (as current state in mainline dtb) lead to a working network.
> > > > > By not working, I mean kernel started with ip=dhcp cannot get an IP.
> > > > 
> > > > How is the PHY connected to the host (which interface mode?) If it's
> > > > RGMII, it could be that the wrong RGMII interface mode is specified in
> > > > DT.
> > > > 
> > > 
> > > The PHY is set as RGMII in DT (arch/arm/boot/dts/gemini-ssi1328.dts)
> > > The only change to the mainline dtb is removing the max-speed.
> > 
> > So, it's using "rgmii" with no delay configured at the PHY with the
> > speed limited to 100Mbps. You then remove the speed limitation and
> > it doesn't work at 1Gbps.
> > 
> > I think I've seen this on other platforms (imx6 + ar8035) when the
> > RGMII delay is not correctly configured - it will work at slower
> > speeds but not 1G.
> > 
> > The RGMII spec specifies that there will be a delay - and the delay can
> > be introduced by either the MAC, PHY or by PCB track routing. It sounds
> > to me like your boot environment configures the PHY to introduce the
> > necessary delay, but then, because the DT "rgmii" mode means "no delay
> > at the PHY" when you use the Marvell driver (which respects that), the
> > Marvell driver configures the PHY for no delay, resulting in a non-
> > working situation at 1G.
> > 
> > I would suggest checking how the boot environment configures the PHY,
> > and change the "rgmii" mode in DT to match. There is a description of
> > the four RGMII modes in Documentation/networking/phy.rst that may help
> > understand what each one means.
> 
> Hmm. Sorry, I'm leading you stray. It looks like the 88E1118 code does
> not program any delays depending on the interface mode, so changing that
> will have no effect.
> 
> I suspect, looking at m88e1118_config_init(), that the write to register
> 0x15 in the MSCR page could be the problem.
> 
> 0x15 is 21, which is MII_88E1121_PHY_MSCR_REG. In other Marvell PHYs,
> bits 4 and 5 are the tx and rx delays, both of which are set. Looking
> at m88e1121_config_aneg_rgmii_delays(), this would seem to indicate
> that the PHY is being placed into rgmii-id mode.
> 
> Can you try changing:
> 
> 	err = phy_write(phydev, 0x15, 0x1070);
> 
> to:
> 
> 	err = phy_write(phydev, 0x15, 0x1040);
> 
> and see what happens? Maybe trying other combinations of bits 4 and 5
> to find a working combination.
> 

Forget my other message, using 0x1040 lead to success.
My problem was that I tried rgmii-id which net/ethernet/cortina does not support on some code path. (everything test PHY_INTERFACE_MODE_RGMII only)
So I retry tests with original phy-mode = "rgmii".

So with the following changes everything is ok:
diff --git a/arch/arm/boot/dts/gemini-ssi1328.dts b/arch/arm/boot/dts/gemini-ssi1328.dts
index 113feb1c4922..7543d117a13a 100644
--- a/arch/arm/boot/dts/gemini-ssi1328.dts
+++ b/arch/arm/boot/dts/gemini-ssi1328.dts
@@ -40,10 +40,6 @@ mdio0: mdio {
                phy0: ethernet-phy@1 {
                        reg = <1>;
                        device_type = "ethernet-phy";
-                       /* We lack the knowledge of necessary GPIO to achieve
-                        * Gigabit
-                        */
-                       max-speed = <100>;
                };
                /* WAN ICPlus IP101A */
                phy1: ethernet-phy@2 {
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 4fcfca4e1702..af7fc9d8eaa7 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -1233,7 +1233,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
                return err;
 
        /* Enable 1000 Mbit */
-       err = phy_write(phydev, 0x15, 0x1070);
+       err = phy_write(phydev, 0x15, 0x1040);
        if (err < 0)
                return err;