2023-09-29 01:40:34

by Giulio Benetti

[permalink] [raw]
Subject: [PATCH v3] net: phy: broadcom: add support for BCM5221 phy

From: Giulio Benetti <[email protected]>

This patch adds the BCM5221 PHY support by reusing brcm_fet_*()
callbacks and adding quirks for BCM5221 when needed.

Cc: Jim Reinhart <[email protected]>
Cc: James Autry <[email protected]>
Cc: Matthew Maron <[email protected]>
Signed-off-by: Giulio Benetti <[email protected]>
Signed-off-by: Giulio Benetti <[email protected]>
---
V1->V2:
Suggested by Andrew Lunn:
* handle mdix_ctrl adding bcm5221_config_aneg() and bcm5221_read_status()
* reorder PHY_ID_BCM5241 in broadcom_tbl[]
Suggested by Russell King:
* add comment on phy_read(..., MII_BRCM_FET_INTREG)
* lock mdio bus when in shadow mode
Suggested by Florian Fainelli:
* reuse brcm_fet_*() callbacks checking for phy_id == PHY_ID_BCM5221

V2->V3:
* rebase on master branch
---
drivers/net/phy/broadcom.c | 155 +++++++++++++++++++++++++++++--------
include/linux/brcmphy.h | 10 +++
2 files changed, 132 insertions(+), 33 deletions(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 04b2e6eeb195..328236cab897 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -704,16 +704,21 @@ static int brcm_fet_config_init(struct phy_device *phydev)
if (err < 0 && err != -EIO)
return err;

+ /* Read to clear status bits */
reg = phy_read(phydev, MII_BRCM_FET_INTREG);
if (reg < 0)
return reg;

/* Unmask events we are interested in and mask interrupts globally. */
- reg = MII_BRCM_FET_IR_DUPLEX_EN |
- MII_BRCM_FET_IR_SPEED_EN |
- MII_BRCM_FET_IR_LINK_EN |
- MII_BRCM_FET_IR_ENABLE |
- MII_BRCM_FET_IR_MASK;
+ if (phydev->phy_id == PHY_ID_BCM5221)
+ reg = MII_BRCM_FET_IR_ENABLE |
+ MII_BRCM_FET_IR_MASK;
+ else
+ reg = MII_BRCM_FET_IR_DUPLEX_EN |
+ MII_BRCM_FET_IR_SPEED_EN |
+ MII_BRCM_FET_IR_LINK_EN |
+ MII_BRCM_FET_IR_ENABLE |
+ MII_BRCM_FET_IR_MASK;

err = phy_write(phydev, MII_BRCM_FET_INTREG, reg);
if (err < 0)
@@ -726,42 +731,50 @@ static int brcm_fet_config_init(struct phy_device *phydev)

reg = brcmtest | MII_BRCM_FET_BT_SRE;

- err = phy_write(phydev, MII_BRCM_FET_BRCMTEST, reg);
- if (err < 0)
- return err;
+ phy_lock_mdio_bus(phydev);

- /* Set the LED mode */
- reg = phy_read(phydev, MII_BRCM_FET_SHDW_AUXMODE4);
- if (reg < 0) {
- err = reg;
- goto done;
+ err = __phy_write(phydev, MII_BRCM_FET_BRCMTEST, reg);
+ if (err < 0) {
+ phy_unlock_mdio_bus(phydev);
+ return err;
}

- reg &= ~MII_BRCM_FET_SHDW_AM4_LED_MASK;
- reg |= MII_BRCM_FET_SHDW_AM4_LED_MODE1;
+ if (phydev->phy_id != PHY_ID_BCM5221) {
+ /* Set the LED mode */
+ reg = __phy_read(phydev, MII_BRCM_FET_SHDW_AUXMODE4);
+ if (reg < 0) {
+ err = reg;
+ goto done;
+ }

- err = phy_write(phydev, MII_BRCM_FET_SHDW_AUXMODE4, reg);
- if (err < 0)
- goto done;
+ reg &= ~MII_BRCM_FET_SHDW_AM4_LED_MASK;
+ reg |= MII_BRCM_FET_SHDW_AM4_LED_MODE1;

- /* Enable auto MDIX */
- err = phy_set_bits(phydev, MII_BRCM_FET_SHDW_MISCCTRL,
- MII_BRCM_FET_SHDW_MC_FAME);
- if (err < 0)
- goto done;
+ err = __phy_write(phydev, MII_BRCM_FET_SHDW_AUXMODE4, reg);
+ if (err < 0)
+ goto done;
+
+ /* Enable auto MDIX */
+ err = __phy_set_bits(phydev, MII_BRCM_FET_SHDW_MISCCTRL,
+ MII_BRCM_FET_SHDW_MC_FAME);
+ if (err < 0)
+ goto done;
+ }

if (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE) {
/* Enable auto power down */
- err = phy_set_bits(phydev, MII_BRCM_FET_SHDW_AUXSTAT2,
- MII_BRCM_FET_SHDW_AS2_APDE);
+ err = __phy_set_bits(phydev, MII_BRCM_FET_SHDW_AUXSTAT2,
+ MII_BRCM_FET_SHDW_AS2_APDE);
}

done:
/* Disable shadow register access */
- err2 = phy_write(phydev, MII_BRCM_FET_BRCMTEST, brcmtest);
+ err2 = __phy_write(phydev, MII_BRCM_FET_BRCMTEST, brcmtest);
if (!err)
err = err2;

+ phy_unlock_mdio_bus(phydev);
+
return err;
}

@@ -840,23 +853,86 @@ static int brcm_fet_suspend(struct phy_device *phydev)

reg = brcmtest | MII_BRCM_FET_BT_SRE;

- err = phy_write(phydev, MII_BRCM_FET_BRCMTEST, reg);
- if (err < 0)
+ phy_lock_mdio_bus(phydev);
+
+ err = __phy_write(phydev, MII_BRCM_FET_BRCMTEST, reg);
+ if (err < 0) {
+ phy_unlock_mdio_bus(phydev);
return err;
+ }
+
+ if (phydev->phy_id == PHY_ID_BCM5221)
+ /* Force Low Power Mode with clock enabled */
+ reg = BCM5221_SHDW_AM4_EN_CLK_LPM | BCM5221_SHDW_AM4_FORCE_LPM;
+ else
+ /* Set standby mode */
+ reg = MII_BRCM_FET_SHDW_AM4_STANDBY;

- /* Set standby mode */
- err = phy_modify(phydev, MII_BRCM_FET_SHDW_AUXMODE4,
- MII_BRCM_FET_SHDW_AM4_STANDBY,
- MII_BRCM_FET_SHDW_AM4_STANDBY);
+ err = __phy_set_bits(phydev, MII_BRCM_FET_SHDW_AUXMODE4, reg);

/* Disable shadow register access */
- err2 = phy_write(phydev, MII_BRCM_FET_BRCMTEST, brcmtest);
+ err2 = __phy_write(phydev, MII_BRCM_FET_BRCMTEST, brcmtest);
if (!err)
err = err2;

+ phy_unlock_mdio_bus(phydev);
+
return err;
}

+static int bcm5221_config_aneg(struct phy_device *phydev)
+{
+ int ret, val;
+
+ ret = genphy_config_aneg(phydev);
+ if (ret)
+ return ret;
+
+ switch (phydev->mdix_ctrl) {
+ case ETH_TP_MDI:
+ val = BCM5221_AEGSR_MDIX_DIS;
+ break;
+ case ETH_TP_MDI_X:
+ val = BCM5221_AEGSR_MDIX_DIS | BCM5221_AEGSR_MDIX_MAN_SWAP;
+ break;
+ case ETH_TP_MDI_AUTO:
+ val = 0;
+ break;
+ default:
+ return 0;
+ }
+
+ return phy_modify(phydev, BCM5221_AEGSR, BCM5221_AEGSR_MDIX_MAN_SWAP |
+ BCM5221_AEGSR_MDIX_DIS,
+ val);
+}
+
+static int bcm5221_read_status(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Read MDIX status */
+ ret = phy_read(phydev, BCM5221_AEGSR);
+ if (ret < 0)
+ return ret;
+
+ if (ret & BCM5221_AEGSR_MDIX_DIS) {
+ if (ret & BCM5221_AEGSR_MDIX_MAN_SWAP)
+ phydev->mdix_ctrl = ETH_TP_MDI_X;
+ else
+ phydev->mdix_ctrl = ETH_TP_MDI;
+ } else {
+ phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+ }
+
+ if (ret & BCM5221_AEGSR_MDIX_STATUS)
+ phydev->mdix = ETH_TP_MDI_X;
+ else
+ phydev->mdix = ETH_TP_MDI;
+
+ return genphy_read_status(phydev);
+}
+
static void bcm54xx_phy_get_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
@@ -1221,6 +1297,18 @@ static struct phy_driver broadcom_drivers[] = {
.handle_interrupt = brcm_fet_handle_interrupt,
.suspend = brcm_fet_suspend,
.resume = brcm_fet_config_init,
+}, {
+ .phy_id = PHY_ID_BCM5221,
+ .phy_id_mask = 0xfffffff0,
+ .name = "Broadcom BCM5221",
+ /* PHY_BASIC_FEATURES */
+ .config_init = brcm_fet_config_init,
+ .config_intr = brcm_fet_config_intr,
+ .handle_interrupt = brcm_fet_handle_interrupt,
+ .suspend = brcm_fet_suspend,
+ .resume = brcm_fet_config_init,
+ .config_aneg = bcm5221_config_aneg,
+ .read_status = bcm5221_read_status,
}, {
.phy_id = PHY_ID_BCM5395,
.phy_id_mask = 0xfffffff0,
@@ -1296,6 +1384,7 @@ static struct mdio_device_id __maybe_unused broadcom_tbl[] = {
{ PHY_ID_BCM50610M, 0xfffffff0 },
{ PHY_ID_BCM57780, 0xfffffff0 },
{ PHY_ID_BCMAC131, 0xfffffff0 },
+ { PHY_ID_BCM5221, 0xfffffff0 },
{ PHY_ID_BCM5241, 0xfffffff0 },
{ PHY_ID_BCM5395, 0xfffffff0 },
{ PHY_ID_BCM53125, 0xfffffff0 },
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index c55810a43541..1394ba302367 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -11,6 +11,7 @@

#define PHY_ID_BCM50610 0x0143bd60
#define PHY_ID_BCM50610M 0x0143bd70
+#define PHY_ID_BCM5221 0x004061e0
#define PHY_ID_BCM5241 0x0143bc30
#define PHY_ID_BCMAC131 0x0143bc70
#define PHY_ID_BCM5481 0x0143bca0
@@ -331,6 +332,15 @@

#define BCM54XX_WOL_INT_STATUS (MII_BCM54XX_EXP_SEL_WOL + 0x94)

+/* BCM5221 Registers */
+#define BCM5221_AEGSR 0x1C
+#define BCM5221_AEGSR_MDIX_STATUS BIT(13)
+#define BCM5221_AEGSR_MDIX_MAN_SWAP BIT(12)
+#define BCM5221_AEGSR_MDIX_DIS BIT(11)
+
+#define BCM5221_SHDW_AM4_EN_CLK_LPM BIT(2)
+#define BCM5221_SHDW_AM4_FORCE_LPM BIT(1)
+
/*****************************************************************************/
/* Fast Ethernet Transceiver definitions. */
/*****************************************************************************/
--
2.34.1


2023-10-05 14:10:09

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v3] net: phy: broadcom: add support for BCM5221 phy

On Thu, Sep 28, 2023 at 08:59:49PM +0200, Giulio Benetti wrote:
> + /* Read to clear status bits */
> reg = phy_read(phydev, MII_BRCM_FET_INTREG);
> if (reg < 0)
> return reg;
>
> /* Unmask events we are interested in and mask interrupts globally. */
> - reg = MII_BRCM_FET_IR_DUPLEX_EN |
> - MII_BRCM_FET_IR_SPEED_EN |
> - MII_BRCM_FET_IR_LINK_EN |
> - MII_BRCM_FET_IR_ENABLE |
> - MII_BRCM_FET_IR_MASK;

Note this style of formatting, where MII_BRCM_xxx line up with the
first...

> + if (phydev->phy_id == PHY_ID_BCM5221)
> + reg = MII_BRCM_FET_IR_ENABLE |
> + MII_BRCM_FET_IR_MASK;

That should be repeated here.

> + else
> + reg = MII_BRCM_FET_IR_DUPLEX_EN |
> + MII_BRCM_FET_IR_SPEED_EN |
> + MII_BRCM_FET_IR_LINK_EN |
> + MII_BRCM_FET_IR_ENABLE |
> + MII_BRCM_FET_IR_MASK;

and here.

>
> err = phy_write(phydev, MII_BRCM_FET_INTREG, reg);
> if (err < 0)
> @@ -726,42 +731,50 @@ static int brcm_fet_config_init(struct phy_device *phydev)
>
> reg = brcmtest | MII_BRCM_FET_BT_SRE;
>
> - err = phy_write(phydev, MII_BRCM_FET_BRCMTEST, reg);
> - if (err < 0)
> - return err;
> + phy_lock_mdio_bus(phydev);
>
> - /* Set the LED mode */
> - reg = phy_read(phydev, MII_BRCM_FET_SHDW_AUXMODE4);
> - if (reg < 0) {
> - err = reg;
> - goto done;
> + err = __phy_write(phydev, MII_BRCM_FET_BRCMTEST, reg);
> + if (err < 0) {
> + phy_unlock_mdio_bus(phydev);
> + return err;
> }
>
> - reg &= ~MII_BRCM_FET_SHDW_AM4_LED_MASK;
> - reg |= MII_BRCM_FET_SHDW_AM4_LED_MODE1;
> + if (phydev->phy_id != PHY_ID_BCM5221) {
> + /* Set the LED mode */
> + reg = __phy_read(phydev, MII_BRCM_FET_SHDW_AUXMODE4);
> + if (reg < 0) {
> + err = reg;
> + goto done;
> + }
>
> - err = phy_write(phydev, MII_BRCM_FET_SHDW_AUXMODE4, reg);
> - if (err < 0)
> - goto done;
> + reg &= ~MII_BRCM_FET_SHDW_AM4_LED_MASK;
> + reg |= MII_BRCM_FET_SHDW_AM4_LED_MODE1;
>
> - /* Enable auto MDIX */
> - err = phy_set_bits(phydev, MII_BRCM_FET_SHDW_MISCCTRL,
> - MII_BRCM_FET_SHDW_MC_FAME);
> - if (err < 0)
> - goto done;
> + err = __phy_write(phydev, MII_BRCM_FET_SHDW_AUXMODE4, reg);

I think this can be simplified to:

err = __phy_modify(phydev, MII_BRCM_FET_SHDW_AUXMODE4,
MII_BRCM_FET_SHDW_AM4_LED_MASK,
MII_BRCM_FET_SHDW_AM4_LED_MODE1);

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2023-10-05 16:11:57

by Paolo Abeni

[permalink] [raw]
Subject: Re: [PATCH v3] net: phy: broadcom: add support for BCM5221 phy

On Thu, 2023-09-28 at 20:59 +0200, Giulio Benetti wrote:
> From: Giulio Benetti <[email protected]>
>
> This patch adds the BCM5221 PHY support by reusing brcm_fet_*()
> callbacks and adding quirks for BCM5221 when needed.
>
> Cc: Jim Reinhart <[email protected]>
> Cc: James Autry <[email protected]>
> Cc: Matthew Maron <[email protected]>
> Signed-off-by: Giulio Benetti <[email protected]>
> Signed-off-by: Giulio Benetti <[email protected]>
> ---
> V1->V2:
> Suggested by Andrew Lunn:
> * handle mdix_ctrl adding bcm5221_config_aneg() and bcm5221_read_status()
> * reorder PHY_ID_BCM5241 in broadcom_tbl[]
> Suggested by Russell King:
> * add comment on phy_read(..., MII_BRCM_FET_INTREG)
> * lock mdio bus when in shadow mode
> Suggested by Florian Fainelli:
> * reuse brcm_fet_*() callbacks checking for phy_id == PHY_ID_BCM5221
>
> V2->V3:
> * rebase on master branch

LGTM, but waiting an extra bit for explicit ack from Florian.

Cheers,

Paolo