Outdated Raspberry Pi 4 firmware might configure the external PHY as
rgmii although the kernel currently sets it as rgmii-rxid. This makes
connections unreliable as ID_MODE_DIS is left enabled. To avoid this,
explicitly clear that bit whenever we don't need it.
Fixes: da38802211cc ("net: bcmgenet: Add RGMII_RXID support")
Signed-off-by: Nicolas Saenz Julienne <[email protected]>
---
Changes since v1:
- Fix tags ordering
- Add targeted tree
drivers/net/ethernet/broadcom/genet/bcmmii.c | 1 +
1 file changed, 1 insertion(+)
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;
--
2.25.1
On 2/25/20 5:11 AM, Nicolas Saenz Julienne wrote:
> Outdated Raspberry Pi 4 firmware might configure the external PHY as
> rgmii although the kernel currently sets it as rgmii-rxid. This makes
> connections unreliable as ID_MODE_DIS is left enabled. To avoid this,
> explicitly clear that bit whenever we don't need it.
>
> Fixes: da38802211cc ("net: bcmgenet: Add RGMII_RXID support")
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian
From: Nicolas Saenz Julienne <[email protected]>
Date: Tue, 25 Feb 2020 14:11:59 +0100
> Outdated Raspberry Pi 4 firmware might configure the external PHY as
> rgmii although the kernel currently sets it as rgmii-rxid. This makes
> connections unreliable as ID_MODE_DIS is left enabled. To avoid this,
> explicitly clear that bit whenever we don't need it.
>
> Fixes: da38802211cc ("net: bcmgenet: Add RGMII_RXID support")
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> ---
>
> Changes since v1:
> - Fix tags ordering
> - Add targeted tree
Applied and queued up for v5.5 -stable.
Hi
On 25.02.2020 14:11, Nicolas Saenz Julienne wrote:
> Outdated Raspberry Pi 4 firmware might configure the external PHY as
> rgmii although the kernel currently sets it as rgmii-rxid. This makes
> connections unreliable as ID_MODE_DIS is left enabled. To avoid this,
> explicitly clear that bit whenever we don't need it.
>
> Fixes: da38802211cc ("net: bcmgenet: Add RGMII_RXID support")
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
I've finally bisected the network issue I have on my RPi4 used for
testing mainline builds. The bisect pointed to this patch. Once it got
applied in v5.7-rc1, the networking is broken on my RPi4 in ARM32bit
mode and kernel compiled from bcm2835_defconfig. I'm using u-boot to
tftp zImage/dtb/initrd there. After reverting this patch network is
working fine again. The strange thing is that networking works fine if
kernel is compiled from multi_v7_defconfig but I don't see any obvious
difference there.
I'm not sure if u-boot is responsible for this break, but kernel
definitely should be able to properly reset the hardware to the valid state.
I can provide more information, just let me know what is needed. Here is
the log, I hope it helps:
[ 11.881784] bcmgenet fd580000.ethernet eth0: Link is Up - 1Gbps/Full
- flow control off
[ 11.889935] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
root@target:~# ping host
PING host (192.168.100.1) 56(84) bytes of data.
From 192.168.100.53 icmp_seq=1 Destination Host Unreachable
...
> ---
>
> Changes since v1:
> - Fix tags ordering
> - Add targeted tree
>
> drivers/net/ethernet/broadcom/genet/bcmmii.c | 1 +
> 1 file changed, 1 insertion(+)
>
> 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;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi
On 07.05.2020 11:46, Marek Szyprowski wrote:
> On 25.02.2020 14:11, Nicolas Saenz Julienne wrote:
>> Outdated Raspberry Pi 4 firmware might configure the external PHY as
>> rgmii although the kernel currently sets it as rgmii-rxid. This makes
>> connections unreliable as ID_MODE_DIS is left enabled. To avoid this,
>> explicitly clear that bit whenever we don't need it.
>>
>> Fixes: da38802211cc ("net: bcmgenet: Add RGMII_RXID support")
>> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>
> I've finally bisected the network issue I have on my RPi4 used for
> testing mainline builds. The bisect pointed to this patch. Once it got
> applied in v5.7-rc1, the networking is broken on my RPi4 in ARM32bit
> mode and kernel compiled from bcm2835_defconfig. I'm using u-boot to
> tftp zImage/dtb/initrd there. After reverting this patch network is
> working fine again. The strange thing is that networking works fine if
> kernel is compiled from multi_v7_defconfig but I don't see any obvious
> difference there.
>
> I'm not sure if u-boot is responsible for this break, but kernel
> definitely should be able to properly reset the hardware to the valid
> state.
>
> I can provide more information, just let me know what is needed. Here
> is the log, I hope it helps:
>
> [ 11.881784] bcmgenet fd580000.ethernet eth0: Link is Up -
> 1Gbps/Full - flow control off
> [ 11.889935] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>
> root@target:~# ping host
> PING host (192.168.100.1) 56(84) bytes of data.
> From 192.168.100.53 icmp_seq=1 Destination Host Unreachable
> ...
Okay, I've played a bit more with this and found that enabling
CONFIG_BROADCOM_PHY fixes this network issue. I wonder if Genet driver
should simply select CONFIG_BROADCOM_PHY the same way as it selects
CONFIG_BCM7XXX_PHY.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On 5/7/2020 3:03 AM, Marek Szyprowski wrote:
> Hi
>
> On 07.05.2020 11:46, Marek Szyprowski wrote:
>> On 25.02.2020 14:11, Nicolas Saenz Julienne wrote:
>>> Outdated Raspberry Pi 4 firmware might configure the external PHY as
>>> rgmii although the kernel currently sets it as rgmii-rxid. This makes
>>> connections unreliable as ID_MODE_DIS is left enabled. To avoid this,
>>> explicitly clear that bit whenever we don't need it.
>>>
>>> Fixes: da38802211cc ("net: bcmgenet: Add RGMII_RXID support")
>>> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>>
>> I've finally bisected the network issue I have on my RPi4 used for
>> testing mainline builds. The bisect pointed to this patch. Once it got
>> applied in v5.7-rc1, the networking is broken on my RPi4 in ARM32bit
>> mode and kernel compiled from bcm2835_defconfig. I'm using u-boot to
>> tftp zImage/dtb/initrd there. After reverting this patch network is
>> working fine again. The strange thing is that networking works fine if
>> kernel is compiled from multi_v7_defconfig but I don't see any obvious
>> difference there.
>>
>> I'm not sure if u-boot is responsible for this break, but kernel
>> definitely should be able to properly reset the hardware to the valid
>> state.
>>
>> I can provide more information, just let me know what is needed. Here
>> is the log, I hope it helps:
>>
>> [ 11.881784] bcmgenet fd580000.ethernet eth0: Link is Up -
>> 1Gbps/Full - flow control off
>> [ 11.889935] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>>
>> root@target:~# ping host
>> PING host (192.168.100.1) 56(84) bytes of data.
>> From 192.168.100.53 icmp_seq=1 Destination Host Unreachable
>> ...
>
> Okay, I've played a bit more with this and found that enabling
> CONFIG_BROADCOM_PHY fixes this network issue. I wonder if Genet driver
> should simply select CONFIG_BROADCOM_PHY the same way as it selects
> CONFIG_BCM7XXX_PHY.
Historically GENET has been deployed with an internal PHY and this is
still 90% of the GENET users out there on classic Broadcom STB
platforms, not counting the 2711. For external PHYs, there is a variety
of options here, so selecting CONFIG_BROADCOM_PHY would be just one of
the possibilities, I would rather fix this with the bcm2835_defconfig
and multi_v7_defconfig update. Would that work for you?
--
Florian
Hi Florian,
On 07.05.2020 17:54, Florian Fainelli wrote:
> On 5/7/2020 3:03 AM, Marek Szyprowski wrote:
>> On 07.05.2020 11:46, Marek Szyprowski wrote:
>>> On 25.02.2020 14:11, Nicolas Saenz Julienne wrote:
>>>> Outdated Raspberry Pi 4 firmware might configure the external PHY as
>>>> rgmii although the kernel currently sets it as rgmii-rxid. This makes
>>>> connections unreliable as ID_MODE_DIS is left enabled. To avoid this,
>>>> explicitly clear that bit whenever we don't need it.
>>>>
>>>> Fixes: da38802211cc ("net: bcmgenet: Add RGMII_RXID support")
>>>> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>>> I've finally bisected the network issue I have on my RPi4 used for
>>> testing mainline builds. The bisect pointed to this patch. Once it got
>>> applied in v5.7-rc1, the networking is broken on my RPi4 in ARM32bit
>>> mode and kernel compiled from bcm2835_defconfig. I'm using u-boot to
>>> tftp zImage/dtb/initrd there. After reverting this patch network is
>>> working fine again. The strange thing is that networking works fine if
>>> kernel is compiled from multi_v7_defconfig but I don't see any obvious
>>> difference there.
>>>
>>> I'm not sure if u-boot is responsible for this break, but kernel
>>> definitely should be able to properly reset the hardware to the valid
>>> state.
>>>
>>> ...
>> Okay, I've played a bit more with this and found that enabling
>> CONFIG_BROADCOM_PHY fixes this network issue. I wonder if Genet driver
>> should simply select CONFIG_BROADCOM_PHY the same way as it selects
>> CONFIG_BCM7XXX_PHY.
> Historically GENET has been deployed with an internal PHY and this is
> still 90% of the GENET users out there on classic Broadcom STB
> platforms, not counting the 2711. For external PHYs, there is a variety
> of options here, so selecting CONFIG_BROADCOM_PHY would be just one of
> the possibilities, I would rather fix this with the bcm2835_defconfig
> and multi_v7_defconfig update. Would that work for you?
Frankly I was surprised that the Genet driver successfully probed and
registered eth0 even when no proper PHY driver was available in the
system. It even reported the link status change, but then didn't
transfer any packets. I expected at least a runtime check and error or
warning if proper PHY is not available. If this is really not possible,
I would still advise to select proper potential PHY drivers, so users
won't be confused.
The Genet driver already selects CONFIG_BCM7XXX_PHY. How common is it?
Would it really hurt do the same for CONFIG_BROADCOM_PHY? I expect that
2711 will be quite popular SoC with it soon.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Marek,
Am 07.05.20 um 12:03 schrieb Marek Szyprowski:
> Hi
>
> On 07.05.2020 11:46, Marek Szyprowski wrote:
>> On 25.02.2020 14:11, Nicolas Saenz Julienne wrote:
>>> Outdated Raspberry Pi 4 firmware might configure the external PHY as
>>> rgmii although the kernel currently sets it as rgmii-rxid. This makes
>>> connections unreliable as ID_MODE_DIS is left enabled. To avoid this,
>>> explicitly clear that bit whenever we don't need it.
>>>
>>> Fixes: da38802211cc ("net: bcmgenet: Add RGMII_RXID support")
>>> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>> I've finally bisected the network issue I have on my RPi4 used for
>> testing mainline builds. The bisect pointed to this patch. Once it got
>> applied in v5.7-rc1, the networking is broken on my RPi4 in ARM32bit
>> mode and kernel compiled from bcm2835_defconfig. I'm using u-boot to
>> tftp zImage/dtb/initrd there. After reverting this patch network is
>> working fine again. The strange thing is that networking works fine if
>> kernel is compiled from multi_v7_defconfig but I don't see any obvious
>> difference there.
>>
>> I'm not sure if u-boot is responsible for this break, but kernel
>> definitely should be able to properly reset the hardware to the valid
>> state.
>>
>> I can provide more information, just let me know what is needed. Here
>> is the log, I hope it helps:
>>
>> [ 11.881784] bcmgenet fd580000.ethernet eth0: Link is Up -
>> 1Gbps/Full - flow control off
>> [ 11.889935] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>>
>> root@target:~# ping host
>> PING host (192.168.100.1) 56(84) bytes of data.
>> From 192.168.100.53 icmp_seq=1 Destination Host Unreachable
>> ...
> Okay, I've played a bit more with this and found that enabling
> CONFIG_BROADCOM_PHY fixes this network issue. I wonder if Genet driver
> should simply select CONFIG_BROADCOM_PHY the same way as it selects
> CONFIG_BCM7XXX_PHY.
thanks for finding this issue. So it seems arm64/defconfig is also affected.
I don't have a strong opinion how to solve this.
Best regards
Stefan
>
> Best regards
On 5/7/2020 11:38 PM, Marek Szyprowski wrote:
> Hi Florian,
>
> On 07.05.2020 17:54, Florian Fainelli wrote:
>> On 5/7/2020 3:03 AM, Marek Szyprowski wrote:
>>> On 07.05.2020 11:46, Marek Szyprowski wrote:
>>>> On 25.02.2020 14:11, Nicolas Saenz Julienne wrote:
>>>>> Outdated Raspberry Pi 4 firmware might configure the external PHY as
>>>>> rgmii although the kernel currently sets it as rgmii-rxid. This makes
>>>>> connections unreliable as ID_MODE_DIS is left enabled. To avoid this,
>>>>> explicitly clear that bit whenever we don't need it.
>>>>>
>>>>> Fixes: da38802211cc ("net: bcmgenet: Add RGMII_RXID support")
>>>>> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>>>> I've finally bisected the network issue I have on my RPi4 used for
>>>> testing mainline builds. The bisect pointed to this patch. Once it got
>>>> applied in v5.7-rc1, the networking is broken on my RPi4 in ARM32bit
>>>> mode and kernel compiled from bcm2835_defconfig. I'm using u-boot to
>>>> tftp zImage/dtb/initrd there. After reverting this patch network is
>>>> working fine again. The strange thing is that networking works fine if
>>>> kernel is compiled from multi_v7_defconfig but I don't see any obvious
>>>> difference there.
>>>>
>>>> I'm not sure if u-boot is responsible for this break, but kernel
>>>> definitely should be able to properly reset the hardware to the valid
>>>> state.
>>>>
>>>> ...
>>> Okay, I've played a bit more with this and found that enabling
>>> CONFIG_BROADCOM_PHY fixes this network issue. I wonder if Genet driver
>>> should simply select CONFIG_BROADCOM_PHY the same way as it selects
>>> CONFIG_BCM7XXX_PHY.
>> Historically GENET has been deployed with an internal PHY and this is
>> still 90% of the GENET users out there on classic Broadcom STB
>> platforms, not counting the 2711. For external PHYs, there is a variety
>> of options here, so selecting CONFIG_BROADCOM_PHY would be just one of
>> the possibilities, I would rather fix this with the bcm2835_defconfig
>> and multi_v7_defconfig update. Would that work for you?
>
> Frankly I was surprised that the Genet driver successfully probed and
> registered eth0 even when no proper PHY driver was available in the
> system. It even reported the link status change, but then didn't
> transfer any packets. I expected at least a runtime check and error or
> warning if proper PHY is not available.
> If this is really not possible, I would still advise to select proper potential PHY drivers, so users
> won't be confused.
It is possible to issue a warning if we find ourselves running on a
BCM2711 SoC and we end-up using the Generic PHY driver, much like what
r8169 does (for similar reasons):
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=f32593773549
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=0c2006b29e5f62784c70209e71da7876267e0e2d
>
> The Genet driver already selects CONFIG_BCM7XXX_PHY. How common is it?
GENET is a roughly 15 years old Ethernet controller that has evolved and
is still being integrated into new chips, so we have probably hundreds
of millions of devices out there.
> Would it really hurt do the same for CONFIG_BROADCOM_PHY? I expect that
> 2711 will be quite popular SoC with it soon.
My problem with a select BROADCOM_PHY is that it will make it impossible
for me to deselect the Broadcom PHY driver. We have probably about a
hundred or so reference boards with a variety of external PHYs some
Broadcom and we have managed to get them all working out of the box with
the Generic PHY driver. If I cannot deselect the Broadcom PHY driver
there will be RGMII regressions (much like the one you reported) which I
really have no interest in solving when it can be avoided.
Does the following work for you:
diff --git a/drivers/net/ethernet/broadcom/Kconfig
b/drivers/net/ethernet/broadcom/Kconfig
index 53055ce5dfd6..8a70b9152f7c 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -69,6 +69,7 @@ config BCMGENET
select BCM7XXX_PHY
select MDIO_BCM_UNIMAC
select DIMLIB
+ imply BROADCOM_PHY if ARCH_BCM2835
help
This driver supports the built-in Ethernet MACs found in the
Broadcom BCM7xxx Set Top Box family chipset.
--
Florian