diff mbox series

[net-next,6/6] net: phy: added ethtool master-slave configuration support

Message ID 20220304094401.31375-7-arun.ramadoss@microchip.com (mailing list archive)
State Accepted
Commit 8a1b415d70b78e002a8f18bf6da99c6d7ec8055b
Delegated to: Netdev Maintainers
Headers show
Series Add support for LAN937x T1 Phy Driver | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 114 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Arun Ramadoss March 4, 2022, 9:44 a.m. UTC 
To configure the T1 phy as master or slave using the ethtool -s <dev>
master-slave <forced-master/forced-slave>, the config_aneg and read
status functions are added.

Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/phy/microchip_t1.c | 90 ++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

Comments

Andrew Lunn March 4, 2022, 1:13 p.m. UTC | #1 
> +static int lan87xx_read_master_slave(struct phy_device *phydev)
> +{
> +	int rc = 0;
> +
> +	phydev->master_slave_get = MASTER_SLAVE_CFG_UNKNOWN;
> +	phydev->master_slave_state = MASTER_SLAVE_STATE_UNKNOWN;
> +
> +	rc = phy_read(phydev, MII_CTRL1000);
> +	if (rc < 0)
> +		return rc;
> +
> +	if (rc & CTL1000_AS_MASTER)
> +		phydev->master_slave_get = MASTER_SLAVE_CFG_MASTER_FORCE;
> +	else
> +		phydev->master_slave_get = MASTER_SLAVE_CFG_SLAVE_FORCE;
> +
> +	rc = phy_read(phydev, MII_STAT1000);
> +	if (rc < 0)
> +		return rc;
> +
> +	if (rc & LPA_1000MSRES)
> +		phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
> +	else
> +		phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
> +
> +	return rc;
> +}

It looks like you can just call genphy_read_master_slave()? Or am i
missing some subtle difference?

> +static int lan87xx_config_aneg(struct phy_device *phydev)
> +{
> +	u16 ctl = 0;
> +	int rc;
> +
> +	switch (phydev->master_slave_set) {
> +	case MASTER_SLAVE_CFG_MASTER_FORCE:
> +		ctl |= CTL1000_AS_MASTER;
> +		break;
> +	case MASTER_SLAVE_CFG_SLAVE_FORCE:
> +		break;
> +	case MASTER_SLAVE_CFG_UNKNOWN:
> +	case MASTER_SLAVE_CFG_UNSUPPORTED:
> +		return 0;
> +	default:
> +		phydev_warn(phydev, "Unsupported Master/Slave mode\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	rc = phy_modify_changed(phydev, MII_CTRL1000, CTL1000_AS_MASTER, ctl);
> +	if (rc == 1)
> +		rc = genphy_soft_reset(phydev);
> +
> +	return rc;
> +}

Please use genphy_setup_master_slave()

       Andrew
Arun Ramadoss March 4, 2022, 1:31 p.m. UTC | #2 
Hi Andrew,

On Fri, 2022-03-04 at 14:13 +0100, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> > +static int lan87xx_read_master_slave(struct phy_device *phydev)
> > +{
> > +     int rc = 0;
> > +
> > +     phydev->master_slave_get = MASTER_SLAVE_CFG_UNKNOWN;
> > +     phydev->master_slave_state = MASTER_SLAVE_STATE_UNKNOWN;
> > +
> > +     rc = phy_read(phydev, MII_CTRL1000);
> > +     if (rc < 0)
> > +             return rc;
> > +
> > +     if (rc & CTL1000_AS_MASTER)
> > +             phydev->master_slave_get =
> > MASTER_SLAVE_CFG_MASTER_FORCE;
> > +     else
> > +             phydev->master_slave_get =
> > MASTER_SLAVE_CFG_SLAVE_FORCE;
> > +
> > +     rc = phy_read(phydev, MII_STAT1000);
> > +     if (rc < 0)
> > +             return rc;
> > +
> > +     if (rc & LPA_1000MSRES)
> > +             phydev->master_slave_state =
> > MASTER_SLAVE_STATE_MASTER;
> > +     else
> > +             phydev->master_slave_state =
> > MASTER_SLAVE_STATE_SLAVE;
> > +
> > +     return rc;
> > +}
> 
> It looks like you can just call genphy_read_master_slave()? Or am i
> missing some subtle difference?

Thanks for the comment.

genphy_read_master_slave() and genphy_setup_master_slave() functions
first check for whether phy is gigabit capable. If no, it returns.
LAN87XX is not gigabit capable, so I replicated the genphy function and
removed only the gigabit capable check. I took nxp-tja11xx code as
reference, which has similar implementation.

> 
> > +static int lan87xx_config_aneg(struct phy_device *phydev)
> > +{
> > +     u16 ctl = 0;
> > +     int rc;
> > +
> > +     switch (phydev->master_slave_set) {
> > +     case MASTER_SLAVE_CFG_MASTER_FORCE:
> > +             ctl |= CTL1000_AS_MASTER;
> > +             break;
> > +     case MASTER_SLAVE_CFG_SLAVE_FORCE:
> > +             break;
> > +     case MASTER_SLAVE_CFG_UNKNOWN:
> > +     case MASTER_SLAVE_CFG_UNSUPPORTED:
> > +             return 0;
> > +     default:
> > +             phydev_warn(phydev, "Unsupported Master/Slave
> > mode\n");
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     rc = phy_modify_changed(phydev, MII_CTRL1000,
> > CTL1000_AS_MASTER, ctl);
> > +     if (rc == 1)
> > +             rc = genphy_soft_reset(phydev);
> > +
> > +     return rc;
> > +}
> 
> Please use genphy_setup_master_slave()
> 
>        Andrew
Andrew Lunn March 4, 2022, 1:51 p.m. UTC | #3 
> > It looks like you can just call genphy_read_master_slave()? Or am i
> > missing some subtle difference?
> 
> Thanks for the comment.
> 
> genphy_read_master_slave() and genphy_setup_master_slave() functions
> first check for whether phy is gigabit capable. If no, it returns.
> LAN87XX is not gigabit capable, so I replicated the genphy function and
> removed only the gigabit capable check. I took nxp-tja11xx code as
> reference, which has similar implementation.

O.K, please refactor genphy_read_master_slave() and split it into two.
You can then call the inner function which does not perform the speed
check. Maybe you can also change nxp-tja11xx in the same way.

       Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/microchip_t1.c b/drivers/net/phy/microchip_t1.c
index 6a7836c2961a..8292f7305805 100644
--- a/drivers/net/phy/microchip_t1.c
+++ b/drivers/net/phy/microchip_t1.c
@@ -79,6 +79,9 @@ 
 #define T1_EQ_WT_FD_LCK_FRZ_CFG		0x6D
 #define T1_PST_EQ_LCK_STG1_FRZ_CFG	0x6E
 
+#define T1_MODE_STAT_REG		0x11
+#define T1_LINK_UP_MSK			BIT(0)
+
 #define DRIVER_AUTHOR	"Nisar Sayed <nisar.sayed@microchip.com>"
 #define DRIVER_DESC	"Microchip LAN87XX/LAN937x T1 PHY driver"
 
@@ -671,6 +674,89 @@  static int lan87xx_cable_test_get_status(struct phy_device *phydev,
 	return 0;
 }
 
+static int lan87xx_read_master_slave(struct phy_device *phydev)
+{
+	int rc = 0;
+
+	phydev->master_slave_get = MASTER_SLAVE_CFG_UNKNOWN;
+	phydev->master_slave_state = MASTER_SLAVE_STATE_UNKNOWN;
+
+	rc = phy_read(phydev, MII_CTRL1000);
+	if (rc < 0)
+		return rc;
+
+	if (rc & CTL1000_AS_MASTER)
+		phydev->master_slave_get = MASTER_SLAVE_CFG_MASTER_FORCE;
+	else
+		phydev->master_slave_get = MASTER_SLAVE_CFG_SLAVE_FORCE;
+
+	rc = phy_read(phydev, MII_STAT1000);
+	if (rc < 0)
+		return rc;
+
+	if (rc & LPA_1000MSRES)
+		phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
+	else
+		phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
+
+	return rc;
+}
+
+static int lan87xx_read_status(struct phy_device *phydev)
+{
+	int rc = 0;
+
+	rc = phy_read(phydev, T1_MODE_STAT_REG);
+	if (rc < 0)
+		return rc;
+
+	if (rc & T1_LINK_UP_MSK)
+		phydev->link = 1;
+	else
+		phydev->link = 0;
+
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	rc = lan87xx_read_master_slave(phydev);
+	if (rc < 0)
+		return rc;
+
+	rc = genphy_read_status_fixed(phydev);
+	if (rc < 0)
+		return rc;
+
+	return rc;
+}
+
+static int lan87xx_config_aneg(struct phy_device *phydev)
+{
+	u16 ctl = 0;
+	int rc;
+
+	switch (phydev->master_slave_set) {
+	case MASTER_SLAVE_CFG_MASTER_FORCE:
+		ctl |= CTL1000_AS_MASTER;
+		break;
+	case MASTER_SLAVE_CFG_SLAVE_FORCE:
+		break;
+	case MASTER_SLAVE_CFG_UNKNOWN:
+	case MASTER_SLAVE_CFG_UNSUPPORTED:
+		return 0;
+	default:
+		phydev_warn(phydev, "Unsupported Master/Slave mode\n");
+		return -EOPNOTSUPP;
+	}
+
+	rc = phy_modify_changed(phydev, MII_CTRL1000, CTL1000_AS_MASTER, ctl);
+	if (rc == 1)
+		rc = genphy_soft_reset(phydev);
+
+	return rc;
+}
+
 static struct phy_driver microchip_t1_phy_driver[] = {
 	{
 		PHY_ID_MATCH_MODEL(PHY_ID_LAN87XX),
@@ -682,6 +768,8 @@  static struct phy_driver microchip_t1_phy_driver[] = {
 		.handle_interrupt = lan87xx_handle_interrupt,
 		.suspend        = genphy_suspend,
 		.resume         = genphy_resume,
+		.config_aneg    = lan87xx_config_aneg,
+		.read_status	= lan87xx_read_status,
 		.cable_test_start = lan87xx_cable_test_start,
 		.cable_test_get_status = lan87xx_cable_test_get_status,
 	},
@@ -692,6 +780,8 @@  static struct phy_driver microchip_t1_phy_driver[] = {
 		.config_init	= lan87xx_config_init,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.config_aneg    = lan87xx_config_aneg,
+		.read_status	= lan87xx_read_status,
 		.cable_test_start = lan87xx_cable_test_start,
 		.cable_test_get_status = lan87xx_cable_test_get_status,
 	}