[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <c922942a-d876-4757-be53-2505bea0e6ab@benettiengineering.com>
Date: Thu, 28 Sep 2023 20:46:36 +0200
From: Giulio Benetti <giulio.benetti@...ettiengineering.com>
To: linux-kernel@...r.kernel.org
Cc: Florian Fainelli <f.fainelli@...il.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>, Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>, Russell King
<linux@...linux.org.uk>, "David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
Giulio Benetti <giulio.benetti+tekvox@...ettiengineering.com>,
Jim Reinhart <jimr@...vox.com>, James Autry <jautry@...vox.com>,
Matthew Maron <matthewm@...vox.com>
Subject: Re: [PATCH v2] net: phy: broadcom: add support for BCM5221 phy
Pardon, I've forgotten to rebase to master branch.
I'm going to send V3.
Sorry for the noise.
Best regards
--
Giulio Benetti
CEO&CTO@...etti Engineering sas
On 28/09/23 19:57, Giulio Benetti wrote:
> From: Giulio Benetti <giulio.benetti+tekvox@...ettiengineering.com>
>
> This patch adds the BCM5221 PHY support by reusing brcm_fet_*()
> callbacks and adding quirks for BCM5221 when needed.
>
> Cc: Jim Reinhart <jimr@...vox.com>
> Cc: James Autry <jautry@...vox.com>
> Cc: Matthew Maron <matthewm@...vox.com>
> Signed-off-by: Giulio Benetti <giulio.benetti+tekvox@...ettiengineering.com>
> ---
> 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
> ---
> 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 6f5e8be73d9a..4b71207037d0 100644
> --- a/drivers/net/phy/broadcom.c
> +++ b/drivers/net/phy/broadcom.c
> @@ -648,16 +648,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)
> @@ -670,42 +675,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;
> }
>
> @@ -784,23 +797,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 int bcm54xx_phy_probe(struct phy_device *phydev)
> {
> struct bcm54xx_phy_priv *priv;
> @@ -1082,6 +1158,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,
> @@ -1154,6 +1242,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 9e77165f3ef6..ee1c8160e4d5 100644
> --- a/include/linux/brcmphy.h
> +++ b/include/linux/brcmphy.h
> @@ -12,6 +12,7 @@
> #define PHY_ID_BCM50610 0x0143bd60
> #define PHY_ID_BCM50610M 0x0143bd70
> #define PHY_ID_BCM5241 0x0143bc30
> +#define PHY_ID_BCM5221 0x004061e0
> #define PHY_ID_BCMAC131 0x0143bc70
> #define PHY_ID_BCM5481 0x0143bca0
> #define PHY_ID_BCM5395 0x0143bcf0
> @@ -272,6 +273,15 @@
> #define BCM54612E_EXP_SPARE0 (MII_BCM54XX_EXP_SEL_ETC + 0x34)
> #define BCM54612E_LED4_CLK125OUT_EN (1 << 1)
>
> +/* 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. */
> /*****************************************************************************/
Powered by blists - more mailing lists