Add support for MDI-X status and configuration for GPY211 chips
Signed-off-by: Raju Lakkaraju <[email protected]>
---
drivers/net/phy/mxl-gpy.c | 79 +++++++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 24bae27eedef..a7b11a86bef5 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -29,6 +29,10 @@
#define PHY_ID_GPY241BM 0x67C9DE80
#define PHY_ID_GPY245B 0x67C9DEC0
+#define PHY_CTL1 0x13
+#define PHY_CTL1_MDICD BIT(3)
+#define PHY_CTL1_MDIAB BIT(2)
+#define PHY_CTL1_AMDIX BIT(0)
#define PHY_MIISTAT 0x18 /* MII state */
#define PHY_IMASK 0x19 /* interrupt mask */
#define PHY_ISTAT 0x1A /* interrupt status */
@@ -59,6 +63,13 @@
#define PHY_FWV_MAJOR_MASK GENMASK(11, 8)
#define PHY_FWV_MINOR_MASK GENMASK(7, 0)
+#define PHY_PMA_MGBT_POLARITY 0x82
+#define PHY_MDI_MDI_X_MASK GENMASK(1, 0)
+#define PHY_MDI_MDI_X_NORMAL 0x3
+#define PHY_MDI_MDI_X_AB 0x2
+#define PHY_MDI_MDI_X_CD 0x1
+#define PHY_MDI_MDI_X_CROSS 0x0
+
/* SGMII */
#define VSPEC1_SGMII_CTRL 0x08
#define VSPEC1_SGMII_CTRL_ANEN BIT(12) /* Aneg enable */
@@ -289,6 +300,36 @@ static bool gpy_sgmii_aneg_en(struct phy_device *phydev)
return (ret & VSPEC1_SGMII_CTRL_ANEN) ? true : false;
}
+static int gpy_config_mdix(struct phy_device *phydev, u8 ctrl)
+{
+ int ret;
+ u16 val;
+
+ switch (ctrl) {
+ case ETH_TP_MDI_AUTO:
+ val = PHY_CTL1_AMDIX;
+ break;
+ case ETH_TP_MDI_X:
+ val = (PHY_CTL1_MDIAB | PHY_CTL1_MDICD);
+ break;
+ case ETH_TP_MDI:
+ val = 0;
+ break;
+ default:
+ return 0;
+ }
+
+ ret = phy_modify(phydev, PHY_CTL1, PHY_CTL1_AMDIX | PHY_CTL1_MDIAB |
+ PHY_CTL1_MDICD, val);
+ if (ret < 0) {
+ phydev_err(phydev, "Error: MMD register access failed: %d\n",
+ ret);
+ return ret;
+ }
+
+ return genphy_c45_restart_aneg(phydev);
+}
+
static int gpy_config_aneg(struct phy_device *phydev)
{
bool changed = false;
@@ -304,6 +345,10 @@ static int gpy_config_aneg(struct phy_device *phydev)
: genphy_c45_pma_setup_forced(phydev);
}
+ ret = gpy_config_mdix(phydev, phydev->mdix_ctrl);
+ if (ret < 0)
+ return ret;
+
ret = genphy_c45_an_config_aneg(phydev);
if (ret < 0)
return ret;
@@ -370,6 +415,38 @@ static int gpy_config_aneg(struct phy_device *phydev)
VSPEC1_SGMII_CTRL_ANRS, VSPEC1_SGMII_CTRL_ANRS);
}
+static void gpy_update_mdix(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = phy_read(phydev, PHY_CTL1);
+ if (ret < 0) {
+ phydev_err(phydev, "Error: MDIO register access failed: %d\n",
+ ret);
+ return;
+ }
+
+ if (ret & PHY_CTL1_AMDIX)
+ phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+ else
+ if (ret & PHY_CTL1_MDICD || ret & PHY_CTL1_MDIAB)
+ phydev->mdix_ctrl = ETH_TP_MDI_X;
+ else
+ phydev->mdix_ctrl = ETH_TP_MDI;
+
+ ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, PHY_PMA_MGBT_POLARITY);
+ if (ret < 0) {
+ phydev_err(phydev, "Error: MMD register access failed: %d\n",
+ ret);
+ return;
+ }
+
+ if ((ret & PHY_MDI_MDI_X_MASK) < PHY_MDI_MDI_X_NORMAL)
+ phydev->mdix = ETH_TP_MDI_X;
+ else
+ phydev->mdix = ETH_TP_MDI;
+}
+
static void gpy_update_interface(struct phy_device *phydev)
{
int ret;
@@ -413,6 +490,8 @@ static void gpy_update_interface(struct phy_device *phydev)
if (phydev->speed == SPEED_2500 || phydev->speed == SPEED_1000)
genphy_read_master_slave(phydev);
+
+ gpy_update_mdix(phydev);
}
static int gpy_read_status(struct phy_device *phydev)
--
2.25.1
Hi,
On Fri, Oct 21, 2022 at 03:33:05PM +0530, Raju Lakkaraju wrote:
> @@ -370,6 +415,38 @@ static int gpy_config_aneg(struct phy_device *phydev)
> VSPEC1_SGMII_CTRL_ANRS, VSPEC1_SGMII_CTRL_ANRS);
> }
>
> +static void gpy_update_mdix(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = phy_read(phydev, PHY_CTL1);
> + if (ret < 0) {
> + phydev_err(phydev, "Error: MDIO register access failed: %d\n",
> + ret);
> + return;
> + }
> +
> + if (ret & PHY_CTL1_AMDIX)
> + phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
> + else
> + if (ret & PHY_CTL1_MDICD || ret & PHY_CTL1_MDIAB)
> + phydev->mdix_ctrl = ETH_TP_MDI_X;
> + else
> + phydev->mdix_ctrl = ETH_TP_MDI;
I think this would be better formatted as:
if (...)
...
else if (...)
...
else
...
We don't indent unless there's braces, and if there's braces, coding
style advises braces on both sides of the "else". So, much better to
use the formatting I suggest above.
Apart from that, nothing stands out as being wrong in this patch.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> +static void gpy_update_mdix(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = phy_read(phydev, PHY_CTL1);
> + if (ret < 0) {
> + phydev_err(phydev, "Error: MDIO register access failed: %d\n",
> + ret);
> + return;
> + }
> @@ -413,6 +490,8 @@ static void gpy_update_interface(struct phy_device *phydev)
>
> if (phydev->speed == SPEED_2500 || phydev->speed == SPEED_1000)
> genphy_read_master_slave(phydev);
> +
> + gpy_update_mdix(phydev);
Do you know why gpy_update_interface() is a void function? It is
called from gpy_read_status() which does return error codes. And it
seems like gpy_read_status() would benefit from returning -EINVAL, etc.
Andrew
Hi Andrew,
Thank you for review comments.
The 10/21/2022 16:01, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> > +static void gpy_update_mdix(struct phy_device *phydev)
> > +{
> > + int ret;
> > +
> > + ret = phy_read(phydev, PHY_CTL1);
> > + if (ret < 0) {
> > + phydev_err(phydev, "Error: MDIO register access failed: %d\n",
> > + ret);
> > + return;
> > + }
>
> > @@ -413,6 +490,8 @@ static void gpy_update_interface(struct phy_device *phydev)
> >
> > if (phydev->speed == SPEED_2500 || phydev->speed == SPEED_1000)
> > genphy_read_master_slave(phydev);
> > +
> > + gpy_update_mdix(phydev);
>
> Do you know why gpy_update_interface() is a void function? It is
> called from gpy_read_status() which does return error codes. And it
> seems like gpy_read_status() would benefit from returning -EINVAL, etc.
Do you want me to change gpy_update_interface() return type ?
Can I do those changes as part of this commit or need to fix on "net"
branch ?
>
> Andrew
--
--------
Thanks,
Raju
> > Do you know why gpy_update_interface() is a void function? It is
> > called from gpy_read_status() which does return error codes. And it
> > seems like gpy_read_status() would benefit from returning -EINVAL, etc.
>
> Do you want me to change gpy_update_interface() return type ?
> Can I do those changes as part of this commit or need to fix on "net"
> branch ?
I would say it is not a fix, so do it on net-next. But do it as a
separate patch please.
Andrew