From patchwork Fri Feb 21 11:02:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Saenz Julienne X-Patchwork-Id: 236706 List-Id: U-Boot discussion From: nsaenzjulienne at suse.de (Nicolas Saenz Julienne) Date: Fri, 21 Feb 2020 12:02:37 +0100 Subject: [PATCH] net: bcmgenet: Don't set ID_MODE_DIS when not using RGMII In-Reply-To: References: <20200220163630.29806-1-nsaenzjulienne@suse.de> Message-ID: <8a34ebdb26b7d303a9c8a587cf66d9db3d8fd42c.camel@suse.de> Florian, On Fri, 2020-02-21 at 11:54 +0100, Nicolas Saenz Julienne wrote: > Hi Matthias, > > On Thu, 2020-02-20 at 19:58 +0100, Matthias Brugger wrote: > > On 20/02/2020 17:36, Nicolas Saenz Julienne wrote: > > > As per Linux's driver, ID_MODE_DIS is only set when the PHY interface is > > > RGMII. Don't enable it for the rest of setups. > > > > > > This has been seen to misconfigure RPi4's PHY when booting Linux. > > > > > > Signed-off-by: Nicolas Saenz Julienne > > > --- > > > drivers/net/bcmgenet.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c > > > index 8f4848aec6..e971b556ac 100644 > > > --- a/drivers/net/bcmgenet.c > > > +++ b/drivers/net/bcmgenet.c > > > @@ -448,7 +448,10 @@ static int bcmgenet_adjust_link(struct > > > bcmgenet_eth_priv *priv) > > > } > > > > > > clrsetbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, OOB_DISABLE, > > > - RGMII_LINK | RGMII_MODE_EN | ID_MODE_DIS); > > > + RGMII_LINK | RGMII_MODE_EN); > > > + > > > + if (phy_dev->interface == PHY_INTERFACE_MODE_RGMII) > > > + setbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, ID_MODE_DIS); > > > > Is this given because by different DTS? Shouldn't that be uniform on the > > RPi4? > > The interface type is read from DT, the 'phy-mode' property. In the case of > the > RPi4 it's 'rgmii-rxid'. > > The downstream DT used to be configured differently ('rgmii' and using > 'ethernet-phy-ieee802.3-c22'), that's why you might have seen the board > working > at some point with this driver. But as we updated the DT to match upstream's > we > switched to 'rgmii-rxid' which is being misconfigured as 'rgmii' in u-boot. So > you have u-boot configuring 'rgmii' while Linux configures 'rgmii-rxid', which > fails to clear the ID_MODE_DIS bit. This, I imagine, blocks the delay > configuration process from the PHY (I don't have any documentation). With this in mind, would it make sens to do this in the Linux driver? It all comes down to wheter we trust bootloader's config or not. Regards, Nicolas -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: This is a digitally signed message part URL: diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c index 6392a2530183..10244941a7a6 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c @@ -294,6 +294,7 @@ int bcmgenet_mii_config(struct net_device *dev, bool init) */ if (priv->ext_phy) { reg = bcmgenet_ext_readl(priv, EXT_RGMII_OOB_CTRL); + reg &= ~ID_MODE_DIS; reg |= id_mode_dis; if (GENET_IS_V1(priv) || GENET_IS_V2(priv) || GENET_IS_V3(priv)) reg |= RGMII_MODE_EN_V123;