diff mbox series

[net,v5,2/2] net: phy: aquantia: remove usage of phy_set_max_speed

Message ID 20240930223341.3807222-3-quic_abchauha@quicinc.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Fix AQR PMA capabilities | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 2 blamed authors not CCed: ansuelsmth@gmail.com frut3k7@gmail.com; 2 maintainers not CCed: ansuelsmth@gmail.com frut3k7@gmail.com
netdev/build_clang success Errors and warnings before: 9 this patch: 9
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 Fixes tag looks correct
netdev/build_allmodconfig_warn fail Errors and warnings before: 12 this patch: 12
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns WARNING: line length of 89 exceeds 80 columns
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

Abhishek Chauhan (ABC) Sept. 30, 2024, 10:33 p.m. UTC
Remove the use of phy_set_max_speed in phy driver as the
function is mainly used in MAC driver to set the max
speed.

Instead use get_features to fix up Phy PMA capabilities for
AQR111, AQR111B0, AQR114C and AQCS109

Fixes: 038ba1dc4e54 ("net: phy: aquantia: add AQR111 and AQR111B0 PHY ID")
Fixes: 0974f1f03b07 ("net: phy: aquantia: remove false 5G and 10G speed ability for AQCS109")
Fixes: c278ec644377 ("net: phy: aquantia: add support for AQR114C PHY ID")
Link: https://lore.kernel.org/all/20240913011635.1286027-1-quic_abchauha@quicinc.com/T/
Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com>
---
Changes since v4
1. Forget about asking hardware for PMA capabilites.
2. Just set the Gbits features along with 2.5Gbps support 
   for AQCS109 Phy chip. 
3. Just set the Gbits features along with 5Gbps support 
   for AQR111, AQR111B0 and AQR114C Phy chips. 
4. re-use aqr115c_get_features inside aqr111_get_features
   for further optimizations.


Changes since v3
1. remove setting of 2500baseX bit introduced as
   part of previous patches.
2. follow reverse xmas declaration of variables.
3. remove local mask introduced as part of
   previous patch and optimize the logic.

Changes since v2
1. seperate out the changes into two different patches. 
2. use phy_gbit_features instead of initializing each and 
   every link mode bits. 
3. write seperate functions for 2.5Gbps supported phy. 
4. remove FIBRE bit which was introduced in patch 1.

Changes since v1 
1. remove usage of phy_set_max_speed in the aquantia driver code.
2. Introduce aqr_custom_get_feature which checks for the phy id and
   takes necessary actions based on max_speed supported by the phy.
3. remove aqr111_config_init as it is just a wrapper function. 

 drivers/net/phy/aquantia/aquantia_main.c | 35 ++++++++++++------------
 1 file changed, 17 insertions(+), 18 deletions(-)

Comments

Russell King (Oracle) Oct. 1, 2024, noon UTC | #1
Hi,

On Mon, Sep 30, 2024 at 03:33:41PM -0700, Abhishek Chauhan wrote:
> +static int aqr111_get_features(struct phy_device *phydev)
> +{
> +	/* PHY FIXUP */
> +	/* Phy supports Speeds up to 5G with Autoneg though the phy PMA says otherwise */
> +	aqr115c_get_features(phydev);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, phydev->supported);

More or less same as the previous. The comment could do with shortening.
I think for this linkmode_set_bit(), it's not worth using a local
"supported" variable, so just put phydev->... on the following line to
avoid the long line.

Thanks.
Abhishek Chauhan (ABC) Oct. 1, 2024, 6:06 p.m. UTC | #2
On 10/1/2024 5:00 AM, Russell King (Oracle) wrote:
> Hi,
> 
> On Mon, Sep 30, 2024 at 03:33:41PM -0700, Abhishek Chauhan wrote:
>> +static int aqr111_get_features(struct phy_device *phydev)
>> +{
>> +	/* PHY FIXUP */
>> +	/* Phy supports Speeds up to 5G with Autoneg though the phy PMA says otherwise */
>> +	aqr115c_get_features(phydev);
>> +	linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, phydev->supported);
> 
> More or less same as the previous. The comment could do with shortening.
> I think for this linkmode_set_bit(), it's not worth using a local
> "supported" variable, so just put phydev->... on the following line to
> avoid the long line.
> 
Noted!
> Thanks.
>
diff mbox series

Patch

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index bd543cdc8c1d..4a9081929b9e 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -527,12 +527,6 @@  static int aqcs109_config_init(struct phy_device *phydev)
 	if (!ret)
 		aqr107_chip_info(phydev);
 
-	/* AQCS109 belongs to a chip family partially supporting 10G and 5G.
-	 * PMA speed ability bits are the same for all members of the family,
-	 * AQCS109 however supports speeds up to 2.5G only.
-	 */
-	phy_set_max_speed(phydev, SPEED_2500);
-
 	return aqr107_set_downshift(phydev, MDIO_AN_VEND_PROV_DOWNSHIFT_DFLT);
 }
 
@@ -731,6 +725,16 @@  static int aqr115c_get_features(struct phy_device *phydev)
 	return 0;
 }
 
+static int aqr111_get_features(struct phy_device *phydev)
+{
+	/* PHY FIXUP */
+	/* Phy supports Speeds up to 5G with Autoneg though the phy PMA says otherwise */
+	aqr115c_get_features(phydev);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, phydev->supported);
+
+	return 0;
+}
+
 static int aqr113c_config_init(struct phy_device *phydev)
 {
 	int ret;
@@ -767,15 +771,6 @@  static int aqr107_probe(struct phy_device *phydev)
 	return aqr_hwmon_probe(phydev);
 }
 
-static int aqr111_config_init(struct phy_device *phydev)
-{
-	/* AQR111 reports supporting speed up to 10G,
-	 * however only speeds up to 5G are supported.
-	 */
-	phy_set_max_speed(phydev, SPEED_5000);
-
-	return aqr107_config_init(phydev);
-}
 
 static struct phy_driver aqr_driver[] = {
 {
@@ -853,6 +848,7 @@  static struct phy_driver aqr_driver[] = {
 	.get_sset_count	= aqr107_get_sset_count,
 	.get_strings	= aqr107_get_strings,
 	.get_stats	= aqr107_get_stats,
+	.get_features   = aqr115c_get_features,
 	.link_change_notify = aqr107_link_change_notify,
 	.led_brightness_set = aqr_phy_led_brightness_set,
 	.led_hw_is_supported = aqr_phy_led_hw_is_supported,
@@ -865,7 +861,7 @@  static struct phy_driver aqr_driver[] = {
 	.name		= "Aquantia AQR111",
 	.probe		= aqr107_probe,
 	.get_rate_matching = aqr107_get_rate_matching,
-	.config_init	= aqr111_config_init,
+	.config_init	= aqr107_config_init,
 	.config_aneg    = aqr_config_aneg,
 	.config_intr	= aqr_config_intr,
 	.handle_interrupt = aqr_handle_interrupt,
@@ -877,6 +873,7 @@  static struct phy_driver aqr_driver[] = {
 	.get_sset_count	= aqr107_get_sset_count,
 	.get_strings	= aqr107_get_strings,
 	.get_stats	= aqr107_get_stats,
+	.get_features   = aqr111_get_features,
 	.link_change_notify = aqr107_link_change_notify,
 	.led_brightness_set = aqr_phy_led_brightness_set,
 	.led_hw_is_supported = aqr_phy_led_hw_is_supported,
@@ -889,7 +886,7 @@  static struct phy_driver aqr_driver[] = {
 	.name		= "Aquantia AQR111B0",
 	.probe		= aqr107_probe,
 	.get_rate_matching = aqr107_get_rate_matching,
-	.config_init	= aqr111_config_init,
+	.config_init	= aqr107_config_init,
 	.config_aneg    = aqr_config_aneg,
 	.config_intr	= aqr_config_intr,
 	.handle_interrupt = aqr_handle_interrupt,
@@ -901,6 +898,7 @@  static struct phy_driver aqr_driver[] = {
 	.get_sset_count	= aqr107_get_sset_count,
 	.get_strings	= aqr107_get_strings,
 	.get_stats	= aqr107_get_stats,
+	.get_features   = aqr111_get_features,
 	.link_change_notify = aqr107_link_change_notify,
 	.led_brightness_set = aqr_phy_led_brightness_set,
 	.led_hw_is_supported = aqr_phy_led_hw_is_supported,
@@ -1010,7 +1008,7 @@  static struct phy_driver aqr_driver[] = {
 	.name           = "Aquantia AQR114C",
 	.probe          = aqr107_probe,
 	.get_rate_matching = aqr107_get_rate_matching,
-	.config_init    = aqr111_config_init,
+	.config_init    = aqr107_config_init,
 	.config_aneg    = aqr_config_aneg,
 	.config_intr    = aqr_config_intr,
 	.handle_interrupt = aqr_handle_interrupt,
@@ -1022,6 +1020,7 @@  static struct phy_driver aqr_driver[] = {
 	.get_sset_count = aqr107_get_sset_count,
 	.get_strings    = aqr107_get_strings,
 	.get_stats      = aqr107_get_stats,
+	.get_features   = aqr111_get_features,
 	.link_change_notify = aqr107_link_change_notify,
 	.led_brightness_set = aqr_phy_led_brightness_set,
 	.led_hw_is_supported = aqr_phy_led_hw_is_supported,