Skip to contentSkip to conversation

net: phy: set 1Gbps as default for driver features

PATCH In review Heiner Kallweit 3 participants 9 messages Dec 18, 2018 – Dec 19, 2018

Subsystems (5)
Networking Network drivers Ethernet PHY Broadcom iproc Arm architecture +1 more subsystems
lore
Share & tools
lore

Heiner Kallweit

author maintainer
#0 lore raw

cover net: phy: set 1Gbps as default for driver features

Whether a PHY is 100Mbps or 1Gbps-capable can be autodetected, therefore it's not needed to define this manually in the driver. genphy_config_init() will remove 1Gbps from phydev->supported if not supported. Having said that PHY drivers for 100Mbps not calling genphy_config_init() still have to set the features field. As most PHY's are 1Gbps-capable let's use this as default.

Heiner Kallweit (2):
net: phy: us 1Gbps as default if driver doesn't set features
net: phy: remove feature definition from 1Gbps PHY drivers

 drivers/net/phy/at803x.c     |  2 --
 drivers/net/phy/bcm-cygnus.c |  1 -
 drivers/net/phy/bcm7xxx.c    |  1 -
 drivers/net/phy/broadcom.c   | 15 ---------------
 drivers/net/phy/cicada.c     |  2 --
 drivers/net/phy/dp83867.c    |  1 -
 drivers/net/phy/et1011c.c    |  1 -
 drivers/net/phy/icplus.c     |  1 -
 drivers/net/phy/intel-xway.c |  5 -----
 drivers/net/phy/marvell.c    | 13 -------------
 drivers/net/phy/micrel.c     |  4 ----
 drivers/net/phy/microchip.c  |  1 -
 drivers/net/phy/mscc.c       |  4 ----
 drivers/net/phy/national.c   |  1 -
 drivers/net/phy/phy_device.c | 15 +++++++++------
 drivers/net/phy/realtek.c    |  9 ---------
 drivers/net/phy/vitesse.c    | 12 ------------
 17 files changed, 9 insertions(+), 79 deletions(-)
Signature · 1 line
2.20.0
author maintainer
#1 lore raw

PATCH 1/2 net: phy: set 1Gbps as default for driver features

Whether a PHY is 100MBps or 1Gbps-capable can be autodetected, therefore it's not needed to define this manually in the driver. genphy_config_init() will remove 1Gbps from phydev->supported if not supported. Having said that PHY drivers for 100Mbps not calling genphy_config_init() still have to set the features field. As most PHY's are 1Gbps-capable let's use this as default.

Signed-off-by Heiner Kallweit <[email protected]>
---
 drivers/net/phy/phy_device.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
drivers/net/phy/phy_device.c +9 -6
index 54af2bde6..753451341 100644
@@ -2138,6 +2138,7 @@ static int phy_probe(struct device *dev)
 	struct phy_device *phydev = to_phy_device(dev);
 	struct device_driver *drv = phydev->mdio.dev.driver;
 	struct phy_driver *phydrv = to_phy_driver(drv);
+	const long unsigned int *features;
 	int err = 0;
 
 	phydev->drv = phydrv;

@@ -2153,11 +2154,14 @@ static int phy_probe(struct device *dev)
 
 	mutex_lock(&phydev->lock);
 
+	/* Use 1Gbps as default if driver defines no features */
+	features = phydrv->features ?: phy_gbit_features;
+
 	/* Start out supporting everything. Eventually,
 	 * a controller will attach, and may modify one
 	 * or both of these values
 	 */
-	linkmode_copy(phydev->supported, phydrv->features);
+	linkmode_copy(phydev->supported, features);
 	of_set_phy_supported(phydev);
 	linkmode_copy(phydev->advertising, phydev->supported);
 

@@ -2177,17 +2181,16 @@ static int phy_probe(struct device *dev)
 	 * (e.g. hardware erratum) where the driver wants to set only one
 	 * of these bits.
 	 */
-	if (test_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydrv->features) ||
-	    test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, phydrv->features)) {
+	if (test_bit(ETHTOOL_LINK_MODE_Pause_BIT, features) ||
+	    test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, features)) {
 		linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT,
 				   phydev->supported);
 		linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
 				   phydev->supported);
-		if (test_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydrv->features))
+		if (test_bit(ETHTOOL_LINK_MODE_Pause_BIT, features))
 			linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT,
 					 phydev->supported);
-		if (test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
-			     phydrv->features))
+		if (test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, features))
 			linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
 					 phydev->supported);
 	} else {
Signature · 1 line
2.20.0
author maintainer
#2 lore raw

PATCH 2/2 net: phy: remove feature definition from 1Gbps PHY drivers

In reply to Heiner Kallweit

Now that 1Gbps is the default for features, let's remove the setting from quite some drivers. For the realtek driver also remove the basic mode definitions because it uses genphy_config_init() and the missing 1Gbps capability will be autodetected.

Signed-off-by Heiner Kallweit <[email protected]>
---
 drivers/net/phy/at803x.c     |  2 --
 drivers/net/phy/bcm-cygnus.c |  1 -
 drivers/net/phy/bcm7xxx.c    |  1 -
 drivers/net/phy/broadcom.c   | 15 ---------------
 drivers/net/phy/cicada.c     |  2 --
 drivers/net/phy/dp83867.c    |  1 -
 drivers/net/phy/et1011c.c    |  1 -
 drivers/net/phy/icplus.c     |  1 -
 drivers/net/phy/intel-xway.c |  5 -----
 drivers/net/phy/marvell.c    | 13 -------------
 drivers/net/phy/micrel.c     |  4 ----
 drivers/net/phy/microchip.c  |  1 -
 drivers/net/phy/mscc.c       |  4 ----
 drivers/net/phy/national.c   |  1 -
 drivers/net/phy/realtek.c    |  9 ---------
 drivers/net/phy/vitesse.c    | 12 ------------
 16 files changed, 73 deletions(-)
drivers/net/phy/at803x.c +0 -2
index f9432d053..53b9f578c 100644
@@ -378,7 +378,6 @@ static struct phy_driver at803x_driver[] = {
 	.get_wol		= at803x_get_wol,
 	.suspend		= at803x_suspend,
 	.resume			= at803x_resume,
-	.features		= PHY_GBIT_FEATURES,
 	.ack_interrupt		= at803x_ack_interrupt,
 	.config_intr		= at803x_config_intr,
 }, {

@@ -407,7 +406,6 @@ static struct phy_driver at803x_driver[] = {
 	.get_wol		= at803x_get_wol,
 	.suspend		= at803x_suspend,
 	.resume			= at803x_resume,
-	.features		= PHY_GBIT_FEATURES,
 	.aneg_done		= at803x_aneg_done,
 	.ack_interrupt		= &at803x_ack_interrupt,
 	.config_intr		= &at803x_config_intr,
drivers/net/phy/bcm-cygnus.c +0 -1
index e757b09f1..3f454d6b5 100644
@@ -134,7 +134,6 @@ static struct phy_driver bcm_cygnus_phy_driver[] = {
 	.phy_id        = PHY_ID_BCM_CYGNUS,
 	.phy_id_mask   = 0xfffffff0,
 	.name          = "Broadcom Cygnus PHY",
-	.features      = PHY_GBIT_FEATURES,
 	.config_init   = bcm_cygnus_config_init,
 	.ack_interrupt = bcm_phy_ack_intr,
 	.config_intr   = bcm_phy_config_intr,
drivers/net/phy/bcm7xxx.c +0 -1
index 712224cc4..533f0313a 100644
@@ -609,7 +609,6 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
 	.phy_id		= (_oui),					\
 	.phy_id_mask	= 0xfffffff0,					\
 	.name		= _name,					\
-	.features	= PHY_GBIT_FEATURES,				\
 	.flags		= PHY_IS_INTERNAL,				\
 	.config_init	= bcm7xxx_28nm_config_init,			\
 	.resume		= bcm7xxx_28nm_resume,				\
drivers/net/phy/broadcom.c +0 -15
index aa73c5cc5..8c07d1bc9 100644
@@ -601,7 +601,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM5411,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM5411",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,

@@ -609,7 +608,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM5421,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM5421",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,

@@ -617,7 +615,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM54210E,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM54210E",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,

@@ -625,7 +622,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM5461,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM5461",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,

@@ -633,7 +629,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM54612E,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM54612E",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,

@@ -641,7 +636,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM54616S,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM54616S",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.config_aneg	= bcm54616s_config_aneg,
 	.ack_interrupt	= bcm_phy_ack_intr,

@@ -650,7 +644,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM5464,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM5464",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,

@@ -658,7 +651,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM5481,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM5481",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.config_aneg	= bcm5481_config_aneg,
 	.ack_interrupt	= bcm_phy_ack_intr,

@@ -667,7 +659,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id         = PHY_ID_BCM54810,
 	.phy_id_mask    = 0xfffffff0,
 	.name           = "Broadcom BCM54810",
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = bcm54xx_config_init,
 	.config_aneg    = bcm5481_config_aneg,
 	.ack_interrupt  = bcm_phy_ack_intr,

@@ -676,7 +667,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM5482,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM5482",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm5482_config_init,
 	.read_status	= bcm5482_read_status,
 	.ack_interrupt	= bcm_phy_ack_intr,

@@ -685,7 +675,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM50610,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM50610",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,

@@ -693,7 +682,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM50610M,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM50610M",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,

@@ -701,7 +689,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id		= PHY_ID_BCM57780,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM57780",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= bcm54xx_config_init,
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,

@@ -726,7 +713,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM5395",
 	.flags		= PHY_IS_INTERNAL,
-	.features	= PHY_GBIT_FEATURES,
 	.get_sset_count	= bcm_phy_get_sset_count,
 	.get_strings	= bcm_phy_get_strings,
 	.get_stats	= bcm53xx_phy_get_stats,

@@ -735,7 +721,6 @@ static struct phy_driver broadcom_drivers[] = {
 	.phy_id         = PHY_ID_BCM89610,
 	.phy_id_mask    = 0xfffffff0,
 	.name           = "Broadcom BCM89610",
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = bcm54xx_config_init,
 	.ack_interrupt  = bcm_phy_ack_intr,
 	.config_intr    = bcm_phy_config_intr,
drivers/net/phy/cicada.c +0 -2
index fea61c81b..e02065469 100644
@@ -107,7 +107,6 @@ static struct phy_driver cis820x_driver[] = {
 	.phy_id		= 0x000fc410,
 	.name		= "Cicada Cis8201",
 	.phy_id_mask	= 0x000ffff0,
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= &cis820x_config_init,
 	.ack_interrupt	= &cis820x_ack_interrupt,
 	.config_intr	= &cis820x_config_intr,

@@ -115,7 +114,6 @@ static struct phy_driver cis820x_driver[] = {
 	.phy_id		= 0x000fc440,
 	.name		= "Cicada Cis8204",
 	.phy_id_mask	= 0x000fffc0,
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= &cis820x_config_init,
 	.ack_interrupt	= &cis820x_ack_interrupt,
 	.config_intr	= &cis820x_config_intr,
drivers/net/phy/dp83867.c +0 -1
index da6a67d47..c57dee081 100644
@@ -333,7 +333,6 @@ static struct phy_driver dp83867_driver[] = {
 		.phy_id		= DP83867_PHY_ID,
 		.phy_id_mask	= 0xfffffff0,
 		.name		= "TI DP83867",
-		.features	= PHY_GBIT_FEATURES,
 
 		.config_init	= dp83867_config_init,
 		.soft_reset	= dp83867_phy_reset,
drivers/net/phy/et1011c.c +0 -1
index 565e49e7f..e9bbedce1 100644
@@ -91,7 +91,6 @@ static struct phy_driver et1011c_driver[] = { {
 	.phy_id		= 0x0282f014,
 	.name		= "ET1011C",
 	.phy_id_mask	= 0xfffffff0,
-	.features	= PHY_GBIT_FEATURES,
 	.config_aneg	= et1011c_config_aneg,
 	.read_status	= et1011c_read_status,
 } };
drivers/net/phy/icplus.c +0 -1
index 7d5938b87..55c114d4e 100644
@@ -326,7 +326,6 @@ static struct phy_driver icplus_driver[] = {
 	.phy_id		= 0x02430d90,
 	.name		= "ICPlus IP1001",
 	.phy_id_mask	= 0x0ffffff0,
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= &ip1001_config_init,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
drivers/net/phy/intel-xway.c +0 -5
index fc0f5024a..e306af592 100644
@@ -241,7 +241,6 @@ static struct phy_driver xway_gphy[] = {
 		.phy_id		= PHY_ID_PHY11G_1_3,
 		.phy_id_mask	= 0xffffffff,
 		.name		= "Intel XWAY PHY11G (PEF 7071/PEF 7072) v1.3",
-		.features	= PHY_GBIT_FEATURES,
 		.config_init	= xway_gphy_config_init,
 		.config_aneg	= xway_gphy14_config_aneg,
 		.ack_interrupt	= xway_gphy_ack_interrupt,

@@ -265,7 +264,6 @@ static struct phy_driver xway_gphy[] = {
 		.phy_id		= PHY_ID_PHY11G_1_4,
 		.phy_id_mask	= 0xffffffff,
 		.name		= "Intel XWAY PHY11G (PEF 7071/PEF 7072) v1.4",
-		.features	= PHY_GBIT_FEATURES,
 		.config_init	= xway_gphy_config_init,
 		.config_aneg	= xway_gphy14_config_aneg,
 		.ack_interrupt	= xway_gphy_ack_interrupt,

@@ -289,7 +287,6 @@ static struct phy_driver xway_gphy[] = {
 		.phy_id		= PHY_ID_PHY11G_1_5,
 		.phy_id_mask	= 0xffffffff,
 		.name		= "Intel XWAY PHY11G (PEF 7071/PEF 7072) v1.5 / v1.6",
-		.features	= PHY_GBIT_FEATURES,
 		.config_init	= xway_gphy_config_init,
 		.ack_interrupt	= xway_gphy_ack_interrupt,
 		.did_interrupt	= xway_gphy_did_interrupt,

@@ -311,7 +308,6 @@ static struct phy_driver xway_gphy[] = {
 		.phy_id		= PHY_ID_PHY11G_VR9_1_1,
 		.phy_id_mask	= 0xffffffff,
 		.name		= "Intel XWAY PHY11G (xRX v1.1 integrated)",
-		.features	= PHY_GBIT_FEATURES,
 		.config_init	= xway_gphy_config_init,
 		.ack_interrupt	= xway_gphy_ack_interrupt,
 		.did_interrupt	= xway_gphy_did_interrupt,

@@ -333,7 +329,6 @@ static struct phy_driver xway_gphy[] = {
 		.phy_id		= PHY_ID_PHY11G_VR9_1_2,
 		.phy_id_mask	= 0xffffffff,
 		.name		= "Intel XWAY PHY11G (xRX v1.2 integrated)",
-		.features	= PHY_GBIT_FEATURES,
 		.config_init	= xway_gphy_config_init,
 		.ack_interrupt	= xway_gphy_ack_interrupt,
 		.did_interrupt	= xway_gphy_did_interrupt,
drivers/net/phy/marvell.c +0 -13
index a9c7c7f41..11bef469e 100644
@@ -2010,7 +2010,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1101,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1101",
-		.features = PHY_GBIT_FEATURES,
 		.probe = marvell_probe,
 		.config_init = &marvell_config_init,
 		.config_aneg = &m88e1101_config_aneg,

@@ -2028,7 +2027,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1112,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1112",
-		.features = PHY_GBIT_FEATURES,
 		.probe = marvell_probe,
 		.config_init = &m88e1111_config_init,
 		.config_aneg = &marvell_config_aneg,

@@ -2046,7 +2044,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1111,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1111",
-		.features = PHY_GBIT_FEATURES,
 		.probe = marvell_probe,
 		.config_init = &m88e1111_config_init,
 		.config_aneg = &marvell_config_aneg,

@@ -2065,7 +2062,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1118,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1118",
-		.features = PHY_GBIT_FEATURES,
 		.probe = marvell_probe,
 		.config_init = &m88e1118_config_init,
 		.config_aneg = &m88e1118_config_aneg,

@@ -2083,7 +2079,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1121R,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1121R",
-		.features = PHY_GBIT_FEATURES,
 		.probe = &m88e1121_probe,
 		.config_init = &marvell_config_init,
 		.config_aneg = &m88e1121_config_aneg,

@@ -2103,7 +2098,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1318S,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1318S",
-		.features = PHY_GBIT_FEATURES,
 		.probe = marvell_probe,
 		.config_init = &m88e1318_config_init,
 		.config_aneg = &m88e1318_config_aneg,

@@ -2125,7 +2119,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1145,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1145",
-		.features = PHY_GBIT_FEATURES,
 		.probe = marvell_probe,
 		.config_init = &m88e1145_config_init,
 		.config_aneg = &m88e1101_config_aneg,

@@ -2144,7 +2137,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1149R,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1149R",
-		.features = PHY_GBIT_FEATURES,
 		.probe = marvell_probe,
 		.config_init = &m88e1149_config_init,
 		.config_aneg = &m88e1118_config_aneg,

@@ -2162,7 +2154,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1240,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1240",
-		.features = PHY_GBIT_FEATURES,
 		.probe = marvell_probe,
 		.config_init = &m88e1111_config_init,
 		.config_aneg = &marvell_config_aneg,

@@ -2180,7 +2171,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1116R,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1116R",
-		.features = PHY_GBIT_FEATURES,
 		.probe = marvell_probe,
 		.config_init = &m88e1116r_config_init,
 		.ack_interrupt = &marvell_ack_interrupt,

@@ -2220,7 +2210,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1540,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1540",
-		.features = PHY_GBIT_FEATURES,
 		.probe = m88e1510_probe,
 		.config_init = &marvell_config_init,
 		.config_aneg = &m88e1510_config_aneg,

@@ -2241,7 +2230,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1545",
 		.probe = m88e1510_probe,
-		.features = PHY_GBIT_FEATURES,
 		.config_init = &marvell_config_init,
 		.config_aneg = &m88e1510_config_aneg,
 		.read_status = &marvell_read_status,

@@ -2280,7 +2268,6 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E6390,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E6390",
-		.features = PHY_GBIT_FEATURES,
 		.probe = m88e6390_probe,
 		.config_init = &marvell_config_init,
 		.config_aneg = &m88e1510_config_aneg,
drivers/net/phy/micrel.c +0 -4
index c33384710..b3ee43127 100644
@@ -1049,7 +1049,6 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id		= PHY_ID_KSZ9021,
 	.phy_id_mask	= 0x000ffffe,
 	.name		= "Micrel KSZ9021 Gigabit PHY",
-	.features	= PHY_GBIT_FEATURES,
 	.driver_data	= &ksz9021_type,
 	.probe		= kszphy_probe,
 	.config_init	= ksz9021_config_init,

@@ -1066,7 +1065,6 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id		= PHY_ID_KSZ9031,
 	.phy_id_mask	= MICREL_PHY_ID_MASK,
 	.name		= "Micrel KSZ9031 Gigabit PHY",
-	.features	= PHY_GBIT_FEATURES,
 	.driver_data	= &ksz9021_type,
 	.probe		= kszphy_probe,
 	.config_init	= ksz9031_config_init,

@@ -1082,7 +1080,6 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id		= PHY_ID_KSZ9131,
 	.phy_id_mask	= MICREL_PHY_ID_MASK,
 	.name		= "Microchip KSZ9131 Gigabit PHY",
-	.features	= PHY_GBIT_FEATURES,
 	.driver_data	= &ksz9021_type,
 	.probe		= kszphy_probe,
 	.config_init	= ksz9131_config_init,

@@ -1125,7 +1122,6 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id		= PHY_ID_KSZ9477,
 	.phy_id_mask	= MICREL_PHY_ID_MASK,
 	.name		= "Microchip KSZ9477",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= kszphy_config_init,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
drivers/net/phy/microchip.c +0 -1
index 7557bebd5..2f4a2360e 100644
@@ -345,7 +345,6 @@ static struct phy_driver microchip_phy_driver[] = {
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Microchip LAN88xx",
 
-	.features	= PHY_GBIT_FEATURES,
 
 	.probe		= lan88xx_probe,
 	.remove		= lan88xx_remove,
drivers/net/phy/mscc.c +0 -4
index cfe680f78..3615f36be 100644
@@ -1853,7 +1853,6 @@ static struct phy_driver vsc85xx_driver[] = {
 	.phy_id		= PHY_ID_VSC8531,
 	.name		= "Microsemi VSC8531",
 	.phy_id_mask    = 0xfffffff0,
-	.features	= PHY_GBIT_FEATURES,
 	.soft_reset	= &genphy_soft_reset,
 	.config_init    = &vsc85xx_config_init,
 	.config_aneg    = &vsc85xx_config_aneg,

@@ -1903,7 +1902,6 @@ static struct phy_driver vsc85xx_driver[] = {
 	.phy_id		= PHY_ID_VSC8541,
 	.name		= "Microsemi VSC8541 SyncE",
 	.phy_id_mask    = 0xfffffff0,
-	.features	= PHY_GBIT_FEATURES,
 	.soft_reset	= &genphy_soft_reset,
 	.config_init    = &vsc85xx_config_init,
 	.config_aneg    = &vsc85xx_config_aneg,

@@ -1928,7 +1926,6 @@ static struct phy_driver vsc85xx_driver[] = {
 	.phy_id		= PHY_ID_VSC8574,
 	.name		= "Microsemi GE VSC8574 SyncE",
 	.phy_id_mask	= 0xfffffff0,
-	.features	= PHY_GBIT_FEATURES,
 	.soft_reset	= &genphy_soft_reset,
 	.config_init    = &vsc8584_config_init,
 	.config_aneg    = &vsc85xx_config_aneg,

@@ -1954,7 +1951,6 @@ static struct phy_driver vsc85xx_driver[] = {
 	.phy_id		= PHY_ID_VSC8584,
 	.name		= "Microsemi GE VSC8584 SyncE",
 	.phy_id_mask	= 0xfffffff0,
-	.features	= PHY_GBIT_FEATURES,
 	.soft_reset	= &genphy_soft_reset,
 	.config_init    = &vsc8584_config_init,
 	.config_aneg    = &vsc85xx_config_aneg,
drivers/net/phy/national.c +0 -1
index 139bed2c8..0a202ae57 100644
@@ -133,7 +133,6 @@ static struct phy_driver dp83865_driver[] = { {
 	.phy_id = DP83865_PHY_ID,
 	.phy_id_mask = 0xfffffff0,
 	.name = "NatSemi DP83865",
-	.features = PHY_GBIT_FEATURES,
 	.config_init = ns_config_init,
 	.ack_interrupt = ns_ack_interrupt,
 	.config_intr = ns_config_intr,
drivers/net/phy/realtek.c +0 -9
index c6010fb1a..29a6f6c22 100644
@@ -215,11 +215,9 @@ static struct phy_driver realtek_drvs[] = {
 	{
 		PHY_ID_MATCH_EXACT(0x00008201),
 		.name           = "RTL8201CP Ethernet",
-		.features       = PHY_BASIC_FEATURES,
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc816),
 		.name		= "RTL8201F Fast Ethernet",
-		.features	= PHY_BASIC_FEATURES,
 		.ack_interrupt	= &rtl8201_ack_interrupt,
 		.config_intr	= &rtl8201_config_intr,
 		.suspend	= genphy_suspend,

@@ -229,14 +227,12 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc910),
 		.name		= "RTL8211 Gigabit Ethernet",
-		.features	= PHY_GBIT_FEATURES,
 		.config_aneg	= rtl8211_config_aneg,
 		.read_mmd	= &genphy_read_mmd_unsupported,
 		.write_mmd	= &genphy_write_mmd_unsupported,
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc912),
 		.name		= "RTL8211B Gigabit Ethernet",
-		.features	= PHY_GBIT_FEATURES,
 		.ack_interrupt	= &rtl821x_ack_interrupt,
 		.config_intr	= &rtl8211b_config_intr,
 		.read_mmd	= &genphy_read_mmd_unsupported,

@@ -246,14 +242,12 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc913),
 		.name		= "RTL8211C Gigabit Ethernet",
-		.features	= PHY_GBIT_FEATURES,
 		.config_init	= rtl8211c_config_init,
 		.read_mmd	= &genphy_read_mmd_unsupported,
 		.write_mmd	= &genphy_write_mmd_unsupported,
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc914),
 		.name		= "RTL8211DN Gigabit Ethernet",
-		.features	= PHY_GBIT_FEATURES,
 		.ack_interrupt	= rtl821x_ack_interrupt,
 		.config_intr	= rtl8211e_config_intr,
 		.suspend	= genphy_suspend,

@@ -261,7 +255,6 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc915),
 		.name		= "RTL8211E Gigabit Ethernet",
-		.features	= PHY_GBIT_FEATURES,
 		.ack_interrupt	= &rtl821x_ack_interrupt,
 		.config_intr	= &rtl8211e_config_intr,
 		.suspend	= genphy_suspend,

@@ -269,7 +262,6 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc916),
 		.name		= "RTL8211F Gigabit Ethernet",
-		.features	= PHY_GBIT_FEATURES,
 		.config_init	= &rtl8211f_config_init,
 		.ack_interrupt	= &rtl8211f_ack_interrupt,
 		.config_intr	= &rtl8211f_config_intr,

@@ -280,7 +272,6 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc961),
 		.name		= "RTL8366RB Gigabit Ethernet",
-		.features	= PHY_GBIT_FEATURES,
 		.config_init	= &rtl8366rb_config_init,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
drivers/net/phy/vitesse.c +0 -12
index 0646af458..c86301798 100644
@@ -396,7 +396,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id         = PHY_ID_VSC8234,
 	.name           = "Vitesse VSC8234",
 	.phy_id_mask    = 0x000ffff0,
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = &vsc824x_config_init,
 	.config_aneg    = &vsc82x4_config_aneg,
 	.ack_interrupt  = &vsc824x_ack_interrupt,

@@ -405,7 +404,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id		= PHY_ID_VSC8244,
 	.name		= "Vitesse VSC8244",
 	.phy_id_mask	= 0x000fffc0,
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= &vsc824x_config_init,
 	.config_aneg	= &vsc82x4_config_aneg,
 	.ack_interrupt	= &vsc824x_ack_interrupt,

@@ -414,7 +412,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id		= PHY_ID_VSC8514,
 	.name		= "Vitesse VSC8514",
 	.phy_id_mask	= 0x000ffff0,
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= &vsc824x_config_init,
 	.config_aneg	= &vsc82x4_config_aneg,
 	.ack_interrupt	= &vsc824x_ack_interrupt,

@@ -423,7 +420,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id         = PHY_ID_VSC8572,
 	.name           = "Vitesse VSC8572",
 	.phy_id_mask    = 0x000ffff0,
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = &vsc824x_config_init,
 	.config_aneg    = &vsc82x4_config_aneg,
 	.ack_interrupt  = &vsc824x_ack_interrupt,

@@ -432,7 +428,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id         = PHY_ID_VSC8601,
 	.name           = "Vitesse VSC8601",
 	.phy_id_mask    = 0x000ffff0,
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = &vsc8601_config_init,
 	.ack_interrupt  = &vsc824x_ack_interrupt,
 	.config_intr    = &vsc82xx_config_intr,

@@ -440,7 +435,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id         = PHY_ID_VSC7385,
 	.name           = "Vitesse VSC7385",
 	.phy_id_mask    = 0x000ffff0,
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = vsc738x_config_init,
 	.config_aneg    = vsc73xx_config_aneg,
 	.read_page      = vsc73xx_read_page,

@@ -449,7 +443,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id         = PHY_ID_VSC7388,
 	.name           = "Vitesse VSC7388",
 	.phy_id_mask    = 0x000ffff0,
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = vsc738x_config_init,
 	.config_aneg    = vsc73xx_config_aneg,
 	.read_page      = vsc73xx_read_page,

@@ -458,7 +451,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id         = PHY_ID_VSC7395,
 	.name           = "Vitesse VSC7395",
 	.phy_id_mask    = 0x000ffff0,
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = vsc739x_config_init,
 	.config_aneg    = vsc73xx_config_aneg,
 	.read_page      = vsc73xx_read_page,

@@ -467,7 +459,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id         = PHY_ID_VSC7398,
 	.name           = "Vitesse VSC7398",
 	.phy_id_mask    = 0x000ffff0,
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = vsc739x_config_init,
 	.config_aneg    = vsc73xx_config_aneg,
 	.read_page      = vsc73xx_read_page,

@@ -476,7 +467,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id         = PHY_ID_VSC8662,
 	.name           = "Vitesse VSC8662",
 	.phy_id_mask    = 0x000ffff0,
-	.features       = PHY_GBIT_FEATURES,
 	.config_init    = &vsc824x_config_init,
 	.config_aneg    = &vsc82x4_config_aneg,
 	.ack_interrupt  = &vsc824x_ack_interrupt,

@@ -486,7 +476,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id		= PHY_ID_VSC8221,
 	.phy_id_mask	= 0x000ffff0,
 	.name		= "Vitesse VSC8221",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= &vsc8221_config_init,
 	.ack_interrupt	= &vsc824x_ack_interrupt,
 	.config_intr	= &vsc82xx_config_intr,

@@ -495,7 +484,6 @@ static struct phy_driver vsc82xx_driver[] = {
 	.phy_id		= PHY_ID_VSC8211,
 	.phy_id_mask	= 0x000ffff0,
 	.name		= "Vitesse VSC8211",
-	.features	= PHY_GBIT_FEATURES,
 	.config_init	= &vsc8221_config_init,
 	.ack_interrupt	= &vsc824x_ack_interrupt,
 	.config_intr	= &vsc82xx_config_intr,
Signature · 1 line
2.20.0
maintainer
#3 lore raw
In reply to Heiner Kallweit
Quoted 6 lines · Heiner Kallweit

Whether a PHY is 100Mbps or 1Gbps-capable can be autodetected, therefore it's not needed to define this manually in the driver. genphy_config_init() will remove 1Gbps from phydev->supported if not supported. Having said that PHY drivers for 100Mbps not calling genphy_config_init() still have to set the features field. As most PHY's are 1Gbps-capable let's use this as default.

Hi Heiner

I'm not sure i like this. Today most PHYs are 1G. But multi-gige PHYs are starting to appear. In 5 years time, i expect most new PHYs will be 2.5G and 5G capable, maybe 10G. We then end up with the odd situation that 10M, 100M and 2.5G, 5G and 10G all need features, but 1G not.

I would prefer to keep it consistent and always have a features.

Andrew

author maintainer
#4 lore raw
Quoted 8 lines · Andrew Lunn
Quoted 6 lines · Heiner Kallweit

Whether a PHY is 100Mbps or 1Gbps-capable can be autodetected, therefore it's not needed to define this manually in the driver. genphy_config_init() will remove 1Gbps from phydev->supported if not supported. Having said that PHY drivers for 100Mbps not calling genphy_config_init() still have to set the features field. As most PHY's are 1Gbps-capable let's use this as default.

Quoted 8 lines

Hi Heiner

I'm not sure i like this. Today most PHYs are 1G. But multi-gige PHYs are starting to appear. In 5 years time, i expect most new PHYs will be 2.5G and 5G capable, maybe 10G. We then end up with the odd situation that 10M, 100M and 2.5G, 5G and 10G all need features, but 1G not.

I see the point and thought about this too. My conclusion:
Basic idea is that we shouldn’t force driver authors to specify things we can autodetect in phylib. And AFAICS supported speeds can be autodetected based on the information in clause 22 and clause 45 (speed ability) registers. What remains as a challenge is to autodetect C22 vs. C22-capable C45 vs. C45. So far it seems that C45 PHY’s can be used only via DT configuration, MDIO bus scan (mdiobus_scan can’t deal with C45) doesn't work with C45.

Coming back to why 1Gbps as default (at least for now):
Technically I could have used also 10Gbps as default, genphy_config_init() masks everything above 1Gbps anyway. I think also for 2.5 Gbps and 5Gbps PHY’s would don’t have to specify the supported speeds. What we need is an extension to genphy_config_init() which can deal with C22 only so far. Maybe mv3310_config_init() from the marvell10g driver can be at least partially reused.

Eventually defining 1Gbps as default isn’t something we would need to roll back later once PHYs above 1Gbps are the standard. We can deal with this in phylib w/o affecting existing drivers. Therefore I think the proposed change is future-proof.

Quoted 4 lines

I would prefer to keep it consistent and always have a features.

Andrew

Heiner

maintainer
#5 lore raw

From: Heiner Kallweit <[email protected]>
Date: Tue, 18 Dec 2018 19:09:36 +0100

Quoted 4 lines

Eventually defining 1Gbps as default isn’t something we would need to roll back later once PHYs above 1Gbps are the standard. We can deal with this in phylib w/o affecting existing drivers. Therefore I think the proposed change is future-proof.

Andrew, what do you think given Heiner's explanation of his thinking?

maintainer
#6 lore raw
Quoted 9 lines · David Miller

From: Heiner Kallweit <[email protected]>
Date: Tue, 18 Dec 2018 19:09:36 +0100

Quoted 4 lines

Eventually defining 1Gbps as default isn’t something we would need to roll back later once PHYs above 1Gbps are the standard. We can deal with this in phylib w/o affecting existing drivers. Therefore I think the proposed change is future-proof.

Andrew, what do you think given Heiner's explanation of his thinking?

Ah, sorry, i replied P2P to Heiner and Florian. Please give us a bit of time to discuss this, we can let you know before net-next closes.

Andrew

author maintainer
#7 lore raw
Quoted 9 lines · Andrew Lunn
Quoted 8 lines · David Miller

From: Heiner Kallweit <[email protected]>
Date: Tue, 18 Dec 2018 19:09:36 +0100

Quoted 4 lines

Eventually defining 1Gbps as default isn’t something we would need to roll back later once PHYs above 1Gbps are the standard. We can deal with this in phylib w/o affecting existing drivers. Therefore I think the proposed change is future-proof.

Quoted 5 lines
Quoted 1 line

Andrew, what do you think given Heiner's explanation of his thinking?

Ah, sorry, i replied P2P to Heiner and Florian. Please give us a bit of time to discuss this, we can let you know before net-next closes.

David, we agreed to first work on clause 45 speed autodetection in phylib and then come up with patches including the scope of what I submitted now. So you can disregard the current patch series.

Quoted 2 lines

Andrew

Heiner

maintainer
#8 lore raw

From: Heiner Kallweit <[email protected]>
Date: Wed, 19 Dec 2018 20:30:20 +0100

Quoted 10 lines · Andrew Lunn
Quoted 9 lines · David Miller
Quoted 8 lines

From: Heiner Kallweit <[email protected]>
Date: Tue, 18 Dec 2018 19:09:36 +0100

Quoted 4 lines

Eventually defining 1Gbps as default isn’t something we would need to roll back later once PHYs above 1Gbps are the standard. We can deal with this in phylib w/o affecting existing drivers. Therefore I think the proposed change is future-proof.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

Andrew, what do you think given Heiner's explanation of his thinking?

Ah, sorry, i replied P2P to Heiner and Florian. Please give us a bit of time to discuss this, we can let you know before net-next closes.

David, we agreed to first work on clause 45 speed autodetection in phylib and then come up with patches including the scope of what I submitted now. So you can disregard the current patch series.

Ok, thanks for the update.