diff mbox series

[net] net: phy: broadcom: Fix brcm_fet_config_init()

Message ID 20220324215451.1151297-1-f.fainelli@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] net: phy: broadcom: Fix brcm_fet_config_init() | 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 12 of 12 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, 26 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Florian Fainelli March 24, 2022, 9:54 p.m. UTC
A Broadcom AC201 PHY (same entry as 5241) would be flagged by the
Broadcom UniMAC MDIO controller as not completing the turn around
properly since the PHY expects 65 MDC clock cycles to complete a write
cycle, and the MDIO controller was only sending 64 MDC clock cycles as
determined by looking at a scope shot.

This would make the subsequent read fail with the UniMAC MDIO controller
command field having MDIO_READ_FAIL set and we would abort the
brcm_fet_config_init() function and thus not probe the PHY at all.

After issuing a software reset, wait for at least 1ms which is well
above the 1us reset delay advertised by the datasheet and issue a dummy
read to let the PHY turn around the line properly. This read
specifically ignores -EIO which would be returned by MDIO controllers
checking for the line being turned around.

If we have a genuine read failure, the next read of the interrupt
status register would pick it up anyway.

Fixes: d7a2ed9248a3 ("broadcom: Add AC131 phy support")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/broadcom.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Florian Fainelli March 24, 2022, 10:31 p.m. UTC | #1
On 3/24/22 14:54, Florian Fainelli wrote:
> A Broadcom AC201 PHY (same entry as 5241) would be flagged by the
> Broadcom UniMAC MDIO controller as not completing the turn around
> properly since the PHY expects 65 MDC clock cycles to complete a write
> cycle, and the MDIO controller was only sending 64 MDC clock cycles as
> determined by looking at a scope shot.
> 
> This would make the subsequent read fail with the UniMAC MDIO controller
> command field having MDIO_READ_FAIL set and we would abort the
> brcm_fet_config_init() function and thus not probe the PHY at all.
> 
> After issuing a software reset, wait for at least 1ms which is well
> above the 1us reset delay advertised by the datasheet and issue a dummy
> read to let the PHY turn around the line properly. This read
> specifically ignores -EIO which would be returned by MDIO controllers
> checking for the line being turned around.
> 
> If we have a genuine read failure, the next read of the interrupt
> status register would pick it up anyway.
> 
> Fixes: d7a2ed9248a3 ("broadcom: Add AC131 phy support")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---

I will make this a v2 which explicitly includes delay.h to have a 
definition for usleep_range9) so this commit would apply cleanly to 
earlier kernels as well. Please do not apply just yet. Thanks!
diff mbox series

Patch

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 3c683e0e40e9..ae6abe146236 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -602,6 +602,26 @@  static int brcm_fet_config_init(struct phy_device *phydev)
 	if (err < 0)
 		return err;
 
+	/* The datasheet indicates the PHY needs up to 1us to complete a reset,
+	 * build some slack here.
+	 */
+	usleep_range(1000, 2000);
+
+	/* The PHY requires 65 MDC clock cycles to complete a write operation
+	 * and turnaround the line properly.
+	 *
+	 * We ignore -EIO here as the MDIO controller (e.g.: mdio-bcm-unimac)
+	 * may flag the lack of turn-around as a read failure. This is
+	 * particularly true with this combination since the MDIO controller
+	 * only used 64 MDC cycles. This is not a critical failure in this
+	 * specific case and it has no functional impact otherwise, so we let
+	 * that one go through. If there is a genuine bus error, the next read
+	 * of MII_BRCM_FET_INTREG will error out.
+	 */
+	err = phy_read(phydev, MII_BMCR);
+	if (err < 0 && err != -EIO)
+		return err;
+
 	reg = phy_read(phydev, MII_BRCM_FET_INTREG);
 	if (reg < 0)
 		return reg;