net: bcmgenet: Don't set ID_MODE_DIS when not using RGMII

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 <nsaenzjulienne at suse.de>
> > > ---
> > >  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: <https://lists.denx.de/pipermail/u-boot/attachments/20200221/b3b0b39d/attachment.sig>