diff mbox series

[RFC,v1,5/9] net: phy: micrel: ksz886x add MDI-X support

Message ID 20210505092025.8785-6-o.rempel@pengutronix.de (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series provide cable test support for the ksz886x | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers warning 1 maintainers not CCed: hkallweit1@gmail.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 125 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Oleksij Rempel May 5, 2021, 9:20 a.m. UTC
Add support for MDI-X status and configuration

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/micrel.c | 101 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

Comments

Andrew Lunn May 5, 2021, 12:37 p.m. UTC | #1
> +/* Device specific MII_BMCR (Reg 0) bits */
> +/* 1 = HP Auto MDI/MDI-X mode, 0 = Microchip Auto MDI/MDI-X mode */
> +#define KSZ886X_BMCR_HP_MDIX			BIT(5)
> +/* 1 = Force MDI (transmit on RXP/RXM pins), 0 = Normal operation
> + * (transmit on TXP/TXM pins)
> + */
> +#define KSZ886X_BMCR_FORCE_MDI			BIT(4)
> +/* 1 = Disable auto MDI-X */
> +#define KSZ886X_BMCR_DISABLE_AUTO_MDIX		BIT(3)
> +#define KSZ886X_BMCR_DISABLE_FAR_END_FAULT	BIT(2)
> +#define KSZ886X_BMCR_DISABLE_TRANSMIT		BIT(1)
> +#define KSZ886X_BMCR_DISABLE_LED		BIT(0)

Do these have the same values as what you added in patch 1?

> +static int ksz886x_config_mdix(struct phy_device *phydev, u8 ctrl)
> +{
> +	u16 val;
> +
> +	switch (ctrl) {
> +	case ETH_TP_MDI:
> +		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX;
> +		break;
> +	case ETH_TP_MDI_X:
> +		/* Note: The naming of the bit KSZ886X_BMCR_FORCE_MDI is bit
> +		 * counter intuitive, the "-X" in "1 = Force MDI" in the data
> +		 * sheet seems to be missing:
> +		 * 1 = Force MDI (sic!) (transmit on RX+/RX- pins)
> +		 * 0 = Normal operation (transmit on TX+/TX- pins)
> +		 */
> +		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX | KSZ886X_BMCR_FORCE_MDI;
> +		break;
> +	case ETH_TP_MDI_AUTO:
> +		val = 0;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return phy_modify(phydev, MII_BMCR,
> +			  KSZ886X_BMCR_HP_MDIX | KSZ886X_BMCR_FORCE_MDI |
> +			  KSZ886X_BMCR_DISABLE_AUTO_MDIX,
> +			  KSZ886X_BMCR_HP_MDIX | val);
> +}

Maybe this will also work for the PHY driver embedded in ksz8795.c?
Maybe as another patchset, see if that PHY driver can be moved out of the DSA driver,
and share some code with this driver?

    Andrew
Oleksij Rempel May 10, 2021, 9:10 a.m. UTC | #2
On Wed, May 05, 2021 at 02:37:35PM +0200, Andrew Lunn wrote:
> > +/* Device specific MII_BMCR (Reg 0) bits */
> > +/* 1 = HP Auto MDI/MDI-X mode, 0 = Microchip Auto MDI/MDI-X mode */
> > +#define KSZ886X_BMCR_HP_MDIX			BIT(5)
> > +/* 1 = Force MDI (transmit on RXP/RXM pins), 0 = Normal operation
> > + * (transmit on TXP/TXM pins)
> > + */
> > +#define KSZ886X_BMCR_FORCE_MDI			BIT(4)
> > +/* 1 = Disable auto MDI-X */
> > +#define KSZ886X_BMCR_DISABLE_AUTO_MDIX		BIT(3)
> > +#define KSZ886X_BMCR_DISABLE_FAR_END_FAULT	BIT(2)
> > +#define KSZ886X_BMCR_DISABLE_TRANSMIT		BIT(1)
> > +#define KSZ886X_BMCR_DISABLE_LED		BIT(0)
> 
> Do these have the same values as what you added in patch 1?

ACK, i'll move it

> > +static int ksz886x_config_mdix(struct phy_device *phydev, u8 ctrl)
> > +{
> > +	u16 val;
> > +
> > +	switch (ctrl) {
> > +	case ETH_TP_MDI:
> > +		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX;
> > +		break;
> > +	case ETH_TP_MDI_X:
> > +		/* Note: The naming of the bit KSZ886X_BMCR_FORCE_MDI is bit
> > +		 * counter intuitive, the "-X" in "1 = Force MDI" in the data
> > +		 * sheet seems to be missing:
> > +		 * 1 = Force MDI (sic!) (transmit on RX+/RX- pins)
> > +		 * 0 = Normal operation (transmit on TX+/TX- pins)
> > +		 */
> > +		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX | KSZ886X_BMCR_FORCE_MDI;
> > +		break;
> > +	case ETH_TP_MDI_AUTO:
> > +		val = 0;
> > +		break;
> > +	default:
> > +		return 0;
> > +	}
> > +
> > +	return phy_modify(phydev, MII_BMCR,
> > +			  KSZ886X_BMCR_HP_MDIX | KSZ886X_BMCR_FORCE_MDI |
> > +			  KSZ886X_BMCR_DISABLE_AUTO_MDIX,
> > +			  KSZ886X_BMCR_HP_MDIX | val);
> > +}
> 
> Maybe this will also work for the PHY driver embedded in ksz8795.c?
> Maybe as another patchset, see if that PHY driver can be moved out of the DSA driver,
> and share some code with this driver?

Hm, i'm sure it can be done, but right now i have no access to ksz8795
hardware. I'll keep it in mind.

Regards,
Oleksij
diff mbox series

Patch

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index f03188ed953a..ea30cd6bd7bc 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -28,6 +28,19 @@ 
 #include <linux/clk.h>
 #include <linux/delay.h>
 
+/* Device specific MII_BMCR (Reg 0) bits */
+/* 1 = HP Auto MDI/MDI-X mode, 0 = Microchip Auto MDI/MDI-X mode */
+#define KSZ886X_BMCR_HP_MDIX			BIT(5)
+/* 1 = Force MDI (transmit on RXP/RXM pins), 0 = Normal operation
+ * (transmit on TXP/TXM pins)
+ */
+#define KSZ886X_BMCR_FORCE_MDI			BIT(4)
+/* 1 = Disable auto MDI-X */
+#define KSZ886X_BMCR_DISABLE_AUTO_MDIX		BIT(3)
+#define KSZ886X_BMCR_DISABLE_FAR_END_FAULT	BIT(2)
+#define KSZ886X_BMCR_DISABLE_TRANSMIT		BIT(1)
+#define KSZ886X_BMCR_DISABLE_LED		BIT(0)
+
 /* Operation Mode Strap Override */
 #define MII_KSZPHY_OMSO				0x16
 #define KSZPHY_OMSO_FACTORY_TEST		BIT(15)
@@ -62,6 +75,7 @@ 
 /* bitmap of PHY register to set interrupt mode */
 #define KSZPHY_CTRL_INT_ACTIVE_HIGH		BIT(9)
 #define KSZPHY_RMII_REF_CLK_SEL			BIT(7)
+#define KSZ886X_CTRL_MDIX_STAT			BIT(4)
 
 /* Write/read to/from extended registers */
 #define MII_KSZPHY_EXTREG                       0x0b
@@ -1048,6 +1062,91 @@  static int ksz8873mll_config_aneg(struct phy_device *phydev)
 	return 0;
 }
 
+static int ksz886x_config_mdix(struct phy_device *phydev, u8 ctrl)
+{
+	u16 val;
+
+	switch (ctrl) {
+	case ETH_TP_MDI:
+		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX;
+		break;
+	case ETH_TP_MDI_X:
+		/* Note: The naming of the bit KSZ886X_BMCR_FORCE_MDI is bit
+		 * counter intuitive, the "-X" in "1 = Force MDI" in the data
+		 * sheet seems to be missing:
+		 * 1 = Force MDI (sic!) (transmit on RX+/RX- pins)
+		 * 0 = Normal operation (transmit on TX+/TX- pins)
+		 */
+		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX | KSZ886X_BMCR_FORCE_MDI;
+		break;
+	case ETH_TP_MDI_AUTO:
+		val = 0;
+		break;
+	default:
+		return 0;
+	}
+
+	return phy_modify(phydev, MII_BMCR,
+			  KSZ886X_BMCR_HP_MDIX | KSZ886X_BMCR_FORCE_MDI |
+			  KSZ886X_BMCR_DISABLE_AUTO_MDIX,
+			  KSZ886X_BMCR_HP_MDIX | val);
+}
+
+static int ksz886x_config_aneg(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_config_aneg(phydev);
+	if (ret)
+		return ret;
+
+	/* The MDI-X configuration is automatically changed by the PHY after
+	 * switching from autoneg off to on. So, take MDI-X configuration under
+	 * own control and set it after autoneg configuration was done.
+	 */
+	return ksz886x_config_mdix(phydev, phydev->mdix_ctrl);
+}
+
+static int ksz886x_mdix_update(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read(phydev, MII_BMCR);
+	if (ret < 0)
+		return ret;
+
+	if (ret & KSZ886X_BMCR_DISABLE_AUTO_MDIX) {
+		if (ret & KSZ886X_BMCR_FORCE_MDI)
+			phydev->mdix_ctrl = ETH_TP_MDI_X;
+		else
+			phydev->mdix_ctrl = ETH_TP_MDI;
+	} else {
+		phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+	}
+
+	ret = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (ret < 0)
+		return ret;
+
+	if (ret & KSZ886X_CTRL_MDIX_STAT)
+		phydev->mdix = ETH_TP_MDI;
+	else
+		phydev->mdix = ETH_TP_MDI_X;
+
+	return 0;
+}
+
+static int ksz886x_read_status(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = ksz886x_mdix_update(phydev);
+	if (ret < 0)
+		return ret;
+
+	return genphy_read_status(phydev);
+}
+
 static int ksz886x_resume(struct phy_device *phydev)
 {
 	int ret;
@@ -1420,6 +1519,8 @@  static struct phy_driver ksphy_driver[] = {
 	.name		= "Micrel KSZ8851 Ethernet MAC or KSZ886X Switch",
 	/* PHY_BASIC_FEATURES */
 	.config_init	= kszphy_config_init,
+	.config_aneg	= ksz886x_config_aneg,
+	.read_status	= ksz886x_read_status,
 	.suspend	= genphy_suspend,
 	.resume		= ksz886x_resume,
 }, {