diff mbox series

[net-next] net: phy: vitesse: implement MDI-X configuration in vsc73xx

Message ID 20240822145336.409867-1-paweldembicki@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: phy: vitesse: implement MDI-X configuration in vsc73xx | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 16 this patch: 16
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 16
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 124 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Pawel Dembicki Aug. 22, 2024, 2:53 p.m. UTC 
This commit introduces MDI-X configuration support in vsc73xx phys.

Vsc73xx supports only auto mode or forced MDI.

Vsc73xx have auto MDI-X disabled by default in forced speed mode.
This commit enables it.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
 drivers/net/phy/vitesse.c | 88 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

Comments

Andrew Lunn Aug. 22, 2024, 3:58 p.m. UTC | #1 
> +static int vsc73xx_mdix_set(struct phy_device *phydev, u8 mdix)
> +{
> +	int ret;
> +	u16 val;
> +
> +	val = phy_read(phydev, MII_VSC73XX_PHY_BYPASS_CTRL);
> +
> +	switch (mdix) {
> +	case ETH_TP_MDI:
> +		val |= MII_VSC73XX_PBC_FOR_SPD_AUTO_MDIX_DIS |
> +		       MII_VSC73XX_PBC_PAIR_SWAP_DIS |
> +		       MII_VSC73XX_PBC_POL_INV_DIS;
> +		break;
> +	case ETH_TP_MDI_X:
> +		/* When MDI-X auto configuration is disabled, is possible
> +		 * to force only MDI mode. Let's use autoconfig for forced
> +		 * MDIX mode.
> +		 */
> +	default:
> +		val &= ~(MII_VSC73XX_PBC_FOR_SPD_AUTO_MDIX_DIS |

This could be a little bit more readable if rather than default: you
used case ETH_TP_MDI_AUTO: . Then after this code, add a real default:
which returns -EINVAL,

    Andrew

---
pw-bot: cr
Pawel Dembicki Aug. 23, 2024, 8:46 a.m. UTC | #2 
czw., 22 sie 2024 o 17:58 Andrew Lunn <andrew@lunn.ch> napisał(a):
>
> > +static int vsc73xx_mdix_set(struct phy_device *phydev, u8 mdix)
> > +{
> > +     int ret;
> > +     u16 val;
> > +
> > +     val = phy_read(phydev, MII_VSC73XX_PHY_BYPASS_CTRL);
> > +
> > +     switch (mdix) {
> > +     case ETH_TP_MDI:
> > +             val |= MII_VSC73XX_PBC_FOR_SPD_AUTO_MDIX_DIS |
> > +                    MII_VSC73XX_PBC_PAIR_SWAP_DIS |
> > +                    MII_VSC73XX_PBC_POL_INV_DIS;
> > +             break;
> > +     case ETH_TP_MDI_X:
> > +             /* When MDI-X auto configuration is disabled, is possible
> > +              * to force only MDI mode. Let's use autoconfig for forced
> > +              * MDIX mode.
> > +              */
> > +     default:
> > +             val &= ~(MII_VSC73XX_PBC_FOR_SPD_AUTO_MDIX_DIS |
>
> This could be a little bit more readable if rather than default: you
> used case ETH_TP_MDI_AUTO: . Then after this code, add a real default:
> which returns -EINVAL,
>

How should I handle ETH_TP_MDI_INVALID? Should I do it like in
marvell.c or rockchip.c, or leave it as the default?
Andrew Lunn Aug. 26, 2024, 2:13 a.m. UTC | #3 
On Fri, Aug 23, 2024 at 10:46:44AM +0200, Paweł Dembicki wrote:
> czw., 22 sie 2024 o 17:58 Andrew Lunn <andrew@lunn.ch> napisał(a):
> >
> > > +static int vsc73xx_mdix_set(struct phy_device *phydev, u8 mdix)
> > > +{
> > > +     int ret;
> > > +     u16 val;
> > > +
> > > +     val = phy_read(phydev, MII_VSC73XX_PHY_BYPASS_CTRL);
> > > +
> > > +     switch (mdix) {
> > > +     case ETH_TP_MDI:
> > > +             val |= MII_VSC73XX_PBC_FOR_SPD_AUTO_MDIX_DIS |
> > > +                    MII_VSC73XX_PBC_PAIR_SWAP_DIS |
> > > +                    MII_VSC73XX_PBC_POL_INV_DIS;
> > > +             break;
> > > +     case ETH_TP_MDI_X:
> > > +             /* When MDI-X auto configuration is disabled, is possible
> > > +              * to force only MDI mode. Let's use autoconfig for forced
> > > +              * MDIX mode.
> > > +              */
> > > +     default:
> > > +             val &= ~(MII_VSC73XX_PBC_FOR_SPD_AUTO_MDIX_DIS |
> >
> > This could be a little bit more readable if rather than default: you
> > used case ETH_TP_MDI_AUTO: . Then after this code, add a real default:
> > which returns -EINVAL,
> >
> 
> How should I handle ETH_TP_MDI_INVALID? Should I do it like in
> marvell.c or rockchip.c, or leave it as the default?

ETH_TP_MDI_INVALID generally means there is no support for controlling
MDI. But you are adding support. The most useful default is
ETH_TP_MDI_AUTO. So i would do that here. I would also set
phydev->mdio_ctrl to whatever the hardware defaults to in .probe, or
.config_init

	Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
index 54eb4e8377c4..38a025521119 100644
--- a/drivers/net/phy/vitesse.c
+++ b/drivers/net/phy/vitesse.c
@@ -71,6 +71,19 @@ 
 #define MII_VSC73XX_DOWNSHIFT_MAX		5
 #define MII_VSC73XX_DOWNSHIFT_INVAL		1
 
+/* VSC73XX PHY_BYPASS_CTRL register*/
+#define MII_VSC73XX_PHY_BYPASS_CTRL		MII_DCOUNTER
+#define MII_VSC73XX_PBC_TX_DIS			BIT(15)
+#define MII_VSC73XX_PBC_FOR_SPD_AUTO_MDIX_DIS	BIT(7)
+#define MII_VSC73XX_PBC_PAIR_SWAP_DIS		BIT(5)
+#define MII_VSC73XX_PBC_POL_INV_DIS		BIT(4)
+#define MII_VSC73XX_PBC_PARALLEL_DET_DIS	BIT(3)
+#define MII_VSC73XX_PBC_AUTO_NP_EXCHANGE_DIS	BIT(1)
+
+/* VSC73XX PHY_AUX_CTRL_STAT register */
+#define MII_VSC73XX_PHY_AUX_CTRL_STAT	MII_NCONFIG
+#define MII_VSC73XX_PACS_NO_MDI_X_IND	BIT(13)
+
 /* Vitesse VSC8601 Extended PHY Control Register 1 */
 #define MII_VSC8601_EPHY_CTL		0x17
 #define MII_VSC8601_EPHY_CTL_RGMII_SKEW	(1 << 8)
@@ -319,6 +332,73 @@  static int vsc739x_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int vsc73xx_mdix_set(struct phy_device *phydev, u8 mdix)
+{
+	int ret;
+	u16 val;
+
+	val = phy_read(phydev, MII_VSC73XX_PHY_BYPASS_CTRL);
+
+	switch (mdix) {
+	case ETH_TP_MDI:
+		val |= MII_VSC73XX_PBC_FOR_SPD_AUTO_MDIX_DIS |
+		       MII_VSC73XX_PBC_PAIR_SWAP_DIS |
+		       MII_VSC73XX_PBC_POL_INV_DIS;
+		break;
+	case ETH_TP_MDI_X:
+		/* When MDI-X auto configuration is disabled, is possible
+		 * to force only MDI mode. Let's use autoconfig for forced
+		 * MDIX mode.
+		 */
+	default:
+		val &= ~(MII_VSC73XX_PBC_FOR_SPD_AUTO_MDIX_DIS |
+			 MII_VSC73XX_PBC_PAIR_SWAP_DIS |
+			 MII_VSC73XX_PBC_POL_INV_DIS);
+		break;
+	}
+
+	ret = phy_write(phydev, MII_VSC73XX_PHY_BYPASS_CTRL, val);
+	if (ret)
+		return ret;
+
+	return genphy_restart_aneg(phydev);
+}
+
+static int vsc73xx_config_aneg(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = vsc73xx_mdix_set(phydev, phydev->mdix_ctrl);
+	if (ret)
+		return ret;
+
+	return genphy_config_aneg(phydev);
+}
+
+static int vsc73xx_mdix_get(struct phy_device *phydev, u8 *mdix)
+{
+	u16 reg_val;
+
+	reg_val = phy_read(phydev, MII_VSC73XX_PHY_AUX_CTRL_STAT);
+	if (reg_val & MII_VSC73XX_PACS_NO_MDI_X_IND)
+		*mdix = ETH_TP_MDI;
+	else
+		*mdix = ETH_TP_MDI_X;
+
+	return 0;
+}
+
+static int vsc73xx_read_status(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = vsc73xx_mdix_get(phydev, &phydev->mdix);
+	if (ret < 0)
+		return ret;
+
+	return genphy_read_status(phydev);
+}
+
 /* This adds a skew for both TX and RX clocks, so the skew should only be
  * applied to "rgmii-id" interfaces. It may not work as expected
  * on "rgmii-txid", "rgmii-rxid" or "rgmii" interfaces.
@@ -516,6 +596,8 @@  static struct phy_driver vsc82xx_driver[] = {
 	.phy_id_mask    = 0x000ffff0,
 	/* PHY_GBIT_FEATURES */
 	.config_init    = vsc738x_config_init,
+	.config_aneg    = vsc73xx_config_aneg,
+	.read_status	= vsc73xx_read_status,
 	.read_page      = vsc73xx_read_page,
 	.write_page     = vsc73xx_write_page,
 	.get_tunable    = vsc73xx_get_tunable,
@@ -526,6 +608,8 @@  static struct phy_driver vsc82xx_driver[] = {
 	.phy_id_mask    = 0x000ffff0,
 	/* PHY_GBIT_FEATURES */
 	.config_init    = vsc738x_config_init,
+	.config_aneg    = vsc73xx_config_aneg,
+	.read_status	= vsc73xx_read_status,
 	.read_page      = vsc73xx_read_page,
 	.write_page     = vsc73xx_write_page,
 	.get_tunable    = vsc73xx_get_tunable,
@@ -536,6 +620,8 @@  static struct phy_driver vsc82xx_driver[] = {
 	.phy_id_mask    = 0x000ffff0,
 	/* PHY_GBIT_FEATURES */
 	.config_init    = vsc739x_config_init,
+	.config_aneg    = vsc73xx_config_aneg,
+	.read_status	= vsc73xx_read_status,
 	.read_page      = vsc73xx_read_page,
 	.write_page     = vsc73xx_write_page,
 	.get_tunable    = vsc73xx_get_tunable,
@@ -546,6 +632,8 @@  static struct phy_driver vsc82xx_driver[] = {
 	.phy_id_mask    = 0x000ffff0,
 	/* PHY_GBIT_FEATURES */
 	.config_init    = vsc739x_config_init,
+	.config_aneg    = vsc73xx_config_aneg,
+	.read_status	= vsc73xx_read_status,
 	.read_page      = vsc73xx_read_page,
 	.write_page     = vsc73xx_write_page,
 	.get_tunable    = vsc73xx_get_tunable,