[PATCH] net: bcmgenet: Don't set ID_MODE_DIS when not using RGMII
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@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);
writel(speed << CMD_SPEED_SHIFT, (priv->mac_reg + UMAC_CMD));
On Thu, 2020-02-20 at 17:36 +0100, 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@suse.de
I forgot to add:
Fixes: d53e3fa385 ("net: Add support for Broadcom GENETv5 Ethernet controller")
On Thu, 2020-02-20 at 18:48 +0100, Nicolas Saenz Julienne wrote:
On Thu, 2020-02-20 at 17:36 +0100, 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@suse.de
I forgot to add:
Fixes: d53e3fa385 ("net: Add support for Broadcom GENETv5 Ethernet controller")
Ping :)
Regards, Nicolas
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@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?
BTW Joe, will you take patches for this driver through your branch? For now I delegated it to me, but I'm fine either way.
Regards, Matthias
writel(speed << CMD_SPEED_SHIFT, (priv->mac_reg + UMAC_CMD));
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@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).
Regards, Nicolas
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@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?
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;
It all comes down to wheter we trust bootloader's config or not.
Regards, Nicolas
On 2/20/20 8:36 AM, 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@suse.de
Does the failure look like the following: you have a driver for the Broadcom PHY used on the Pi4 in u-boot, and the phy_dev->interface value is being used to configure the Ethernet PHY chip in a certain way.
Later when you boot Linux, you do not have CONFIG_BROADCOM_PHY enabled so the Generic PHY driver gets used instead, and there is a disagreement between the AMAC and PHY as to whom should be adding the delays?
At any rate:
Reviewed-by: Florian Fainelli f.fainelli@gmail.com
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);writel(speed << CMD_SPEED_SHIFT, (priv->mac_reg + UMAC_CMD));
Hi Florian,
On Thu, 2020-02-20 at 11:05 -0800, Florian Fainelli wrote:
On 2/20/20 8:36 AM, 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@suse.de
Does the failure look like the following: you have a driver for the Broadcom PHY used on the Pi4 in u-boot, and the phy_dev->interface value is being used to configure the Ethernet PHY chip in a certain way.
Later when you boot Linux, you do not have CONFIG_BROADCOM_PHY enabled so the Generic PHY driver gets used instead, and there is a disagreement between the AMAC and PHY as to whom should be adding the delays?
I added an explanation to Matthias' response. I think it fits yours, modulo my limited knowledge in the area :)
At any rate:
Reviewed-by: Florian Fainelli f.fainelli@gmail.com
Thanks!
Regards, Nicolas
participants (3)
-
Florian Fainelli -
Matthias Brugger -
Nicolas Saenz Julienne