diff mbox series

[net] net: phy: broadcom: hook up soft_reset for BCM54616S

Message ID 20220118215243.359473-1-robert.hancock@calian.com (mailing list archive)
State Accepted
Commit d15c7e875d44367005370e6a82e8f3a382a04f9b
Delegated to: Netdev Maintainers
Headers show
Series [net] net: phy: broadcom: hook up soft_reset for BCM54616S | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Robert Hancock Jan. 18, 2022, 9:52 p.m. UTC
A problem was encountered with the Bel-Fuse 1GBT-SFP05 SFP module (which
is a 1 Gbps copper module operating in SGMII mode with an internal
BCM54616S PHY device) using the Xilinx AXI Ethernet MAC core, where the
module would work properly on the initial insertion or boot of the
device, but after the device was rebooted, the link would either only
come up at 100 Mbps speeds or go up and down erratically.

I found no meaningful changes in the PHY configuration registers between
the working and non-working boots, but the status registers seemed to
have a lot of error indications set on the SERDES side of the device on
the non-working boot. I suspect the problem is that whatever happens on
the SGMII link when the device is rebooted and the FPGA logic gets
reloaded ends up putting the module's onboard PHY into a bad state.

Since commit 6e2d85ec0559 ("net: phy: Stop with excessive soft reset")
the genphy_soft_reset call is not made automatically by the PHY core
unless the callback is explicitly specified in the driver structure. For
most of these Broadcom devices, there is probably a hardware reset that
gets asserted to reset the PHY during boot, however for SFP modules
(where the BCM54616S is commonly found) no such reset line exists, so if
the board keeps the SFP cage powered up across a reboot, it will end up
with no reset occurring during reboots.

Hook up the genphy_soft_reset callback for BCM54616S to ensure that a
PHY reset is performed before the device is initialized. This appears to
fix the issue with erratic operation after a reboot with this SFP
module.

Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/net/phy/broadcom.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Florian Fainelli Jan. 18, 2022, 10:03 p.m. UTC | #1
On 1/18/22 1:52 PM, Robert Hancock wrote:
> A problem was encountered with the Bel-Fuse 1GBT-SFP05 SFP module (which
> is a 1 Gbps copper module operating in SGMII mode with an internal
> BCM54616S PHY device) using the Xilinx AXI Ethernet MAC core, where the
> module would work properly on the initial insertion or boot of the
> device, but after the device was rebooted, the link would either only
> come up at 100 Mbps speeds or go up and down erratically.
> 
> I found no meaningful changes in the PHY configuration registers between
> the working and non-working boots, but the status registers seemed to
> have a lot of error indications set on the SERDES side of the device on
> the non-working boot. I suspect the problem is that whatever happens on
> the SGMII link when the device is rebooted and the FPGA logic gets
> reloaded ends up putting the module's onboard PHY into a bad state.
> 
> Since commit 6e2d85ec0559 ("net: phy: Stop with excessive soft reset")
> the genphy_soft_reset call is not made automatically by the PHY core
> unless the callback is explicitly specified in the driver structure. For
> most of these Broadcom devices, there is probably a hardware reset that
> gets asserted to reset the PHY during boot, however for SFP modules
> (where the BCM54616S is commonly found) no such reset line exists, so if
> the board keeps the SFP cage powered up across a reboot, it will end up
> with no reset occurring during reboots.
> 
> Hook up the genphy_soft_reset callback for BCM54616S to ensure that a
> PHY reset is performed before the device is initialized. This appears to
> fix the issue with erratic operation after a reboot with this SFP
> module.
> 
> Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset")
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
patchwork-bot+netdevbpf@kernel.org Jan. 20, 2022, 11:30 a.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Tue, 18 Jan 2022 15:52:43 -0600 you wrote:
> A problem was encountered with the Bel-Fuse 1GBT-SFP05 SFP module (which
> is a 1 Gbps copper module operating in SGMII mode with an internal
> BCM54616S PHY device) using the Xilinx AXI Ethernet MAC core, where the
> module would work properly on the initial insertion or boot of the
> device, but after the device was rebooted, the link would either only
> come up at 100 Mbps speeds or go up and down erratically.
> 
> [...]

Here is the summary with links:
  - [net] net: phy: broadcom: hook up soft_reset for BCM54616S
    https://git.kernel.org/netdev/net/c/d15c7e875d44

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index bb5104ae4610..3c683e0e40e9 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -854,6 +854,7 @@  static struct phy_driver broadcom_drivers[] = {
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM54616S",
 	/* PHY_GBIT_FEATURES */
+	.soft_reset     = genphy_soft_reset,
 	.config_init	= bcm54xx_config_init,
 	.config_aneg	= bcm54616s_config_aneg,
 	.config_intr	= bcm_phy_config_intr,