Date:   Tue, 4 Jan 2022 14:57:33 +0000
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Corentin Labbe <clabbe.montjoie@gmail.com>,
        linus.walleij@linaro.org, ulli.kroll@googlemail.com,
        kuba@kernel.org, davem@davemloft.net, hkallweit1@gmail.com,
        linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: net: phy: marvell: network working with generic PHY and not with
 marvell PHY
Message-ID: <YdRgXbpK6CFB/eCU@shell.armlinux.org.uk>
References: <YdQoOSXS98+Af1wO@Red>
 <YdQsJnfqjaFrtC0m@shell.armlinux.org.uk>
 <YdQwexJVfrdzEfZK@Red>
 <YdQydK4GhI0P5RYL@shell.armlinux.org.uk>
 <YdQ5i+//UITSbxS/@shell.armlinux.org.uk>
 <YdRVovG9mgEWffkn@Red>
 <YdRZQl6U0y19P/0+@shell.armlinux.org.uk>
 <YdRdu3jFPnGd1DsH@lunn.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YdRdu3jFPnGd1DsH@lunn.ch>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

On Tue, Jan 04, 2022 at 03:46:19PM +0100, Andrew Lunn wrote:
> > @@ -1227,16 +1227,18 @@ static int m88e1118_config_init(struct phy_device *phydev)
> >  {
> >  	int err;
> >  
> > -	/* Change address */
> > -	err = marvell_set_page(phydev, MII_MARVELL_MSCR_PAGE);
> > -	if (err < 0)
> > -		return err;
> > -
> >  	/* Enable 1000 Mbit */
> > -	err = phy_write(phydev, 0x15, 0x1070);
> > +	err = phy_write_paged(phydev, MII_MARVELL_MSCR_PAGE,
> > +			      MII_88E1121_PHY_MSCR_REG, 0x1070);
> 
> Ah, yes, keeping this makes it more backwards compatible.
> 
> It would be nice to replace the 0x1070 with #defines.
> 
> We already have:
> 
> #define MII_88E1121_PHY_MSCR_RX_DELAY	BIT(5)
> #define MII_88E1121_PHY_MSCR_TX_DELAY	BIT(4)
> #define MII_88E1121_PHY_MSCR_DELAY_MASK	(BIT(5) | BIT(4))
> 
> Bits 6 is the MSB of the default MAC speed.
> Bit 13 is the LSB of the default MAC speed. These two should default to 10b = 1000Mbps
> Bit 12 is reserved, and should be written 1.

Hmm, seems odd that these speed bits match BMCR, and I'm not sure why
the default MAC speed would have any bearing on whether gigabit mode
is enabled. If they default to 10b, then the write should have no effect
unless boot firmware has changed them.

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