diff mbox series

[RFC] phy: make phy_set_max_speed() *void*

Message ID a2296c4e-884b-334a-570f-901831bfea3c@omp.ru (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [RFC] phy: make phy_set_max_speed() *void* | expand

Checks

Context Check Description
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
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: 405 this patch: 405
netdev/cc_maintainers warning 3 maintainers not CCed: biju.das.jz@bp.renesas.com prabhakar.mahadev-lad.rj@bp.renesas.com geert+renesas@glider.be
netdev/build_clang success Errors and warnings before: 312 this patch: 312
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: 395 this patch: 395
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 106 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Sergey Shtylyov Jan. 17, 2022, 9:18 p.m. UTC
After following the call tree of phy_set_max_speed(), it became clear
that this function never returns anything but 0, so we can change its
result type to *void* and drop the result checks from the three drivers
that actually bothered to do it...

Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

---
This patch is against the DaveM's 'net-next.git' repo.

 drivers/net/ethernet/renesas/ravb_main.c |    8 +-------
 drivers/net/ethernet/renesas/sh_eth.c    |   10 ++--------
 drivers/net/phy/aquantia_main.c          |    4 +---
 drivers/net/phy/phy-core.c               |   22 ++++++++--------------
 include/linux/phy.h                      |    2 +-
 5 files changed, 13 insertions(+), 33 deletions(-)

Comments

Andrew Lunn Jan. 17, 2022, 10:08 p.m. UTC | #1
On Tue, Jan 18, 2022 at 12:18:58AM +0300, Sergey Shtylyov wrote:
> After following the call tree of phy_set_max_speed(), it became clear
> that this function never returns anything but 0, so we can change its
> result type to *void* and drop the result checks from the three drivers
> that actually bothered to do it...
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

Seems reasonable. net-next is closed at the moment, so please repost
once it opens.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Sergey Shtylyov Jan. 18, 2022, 12:47 p.m. UTC | #2
On 1/18/22 1:08 AM, Andrew Lunn wrote:

>> After following the call tree of phy_set_max_speed(), it became clear
>> that this function never returns anything but 0, so we can change its
>> result type to *void* and drop the result checks from the three drivers
>> that actually bothered to do it...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>> analysis tool.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> Seems reasonable.

   No need to seprate into severla patches? :-)

> net-next is closed at the moment, so please repost

   That's why RFC is mainly here. :-)

> once it opens.

   Sure.

> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

   T!

>     Andrew

MBR, Sergey
diff mbox series

Patch

Index: net-next/drivers/net/ethernet/renesas/ravb_main.c
===================================================================
--- net-next.orig/drivers/net/ethernet/renesas/ravb_main.c
+++ net-next/drivers/net/ethernet/renesas/ravb_main.c
@@ -1432,11 +1432,7 @@  static int ravb_phy_init(struct net_devi
 	 * at this time.
 	 */
 	if (soc_device_match(r8a7795es10)) {
-		err = phy_set_max_speed(phydev, SPEED_100);
-		if (err) {
-			netdev_err(ndev, "failed to limit PHY to 100Mbit/s\n");
-			goto err_phy_disconnect;
-		}
+		phy_set_max_speed(phydev, SPEED_100);
 
 		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
 	}
@@ -1457,8 +1453,6 @@  static int ravb_phy_init(struct net_devi
 
 	return 0;
 
-err_phy_disconnect:
-	phy_disconnect(phydev);
 err_deregister_fixed_link:
 	if (of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
Index: net-next/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net-next/drivers/net/ethernet/renesas/sh_eth.c
@@ -2026,14 +2026,8 @@  static int sh_eth_phy_init(struct net_de
 	}
 
 	/* mask with MAC supported features */
-	if (mdp->cd->register_type != SH_ETH_REG_GIGABIT) {
-		int err = phy_set_max_speed(phydev, SPEED_100);
-		if (err) {
-			netdev_err(ndev, "failed to limit PHY to 100 Mbit/s\n");
-			phy_disconnect(phydev);
-			return err;
-		}
-	}
+	if (mdp->cd->register_type != SH_ETH_REG_GIGABIT)
+		phy_set_max_speed(phydev, SPEED_100);
 
 	phy_attached_info(phydev);
 
Index: net-next/drivers/net/phy/aquantia_main.c
===================================================================
--- net-next.orig/drivers/net/phy/aquantia_main.c
+++ net-next/drivers/net/phy/aquantia_main.c
@@ -533,9 +533,7 @@  static int aqcs109_config_init(struct ph
 	 * PMA speed ability bits are the same for all members of the family,
 	 * AQCS109 however supports speeds up to 2.5G only.
 	 */
-	ret = phy_set_max_speed(phydev, SPEED_2500);
-	if (ret)
-		return ret;
+	phy_set_max_speed(phydev, SPEED_2500);
 
 	return aqr107_set_downshift(phydev, MDIO_AN_VEND_PROV_DOWNSHIFT_DFLT);
 }
Index: net-next/drivers/net/phy/phy-core.c
===================================================================
--- net-next.orig/drivers/net/phy/phy-core.c
+++ net-next/drivers/net/phy/phy-core.c
@@ -243,7 +243,7 @@  size_t phy_speeds(unsigned int *speeds,
 	return count;
 }
 
-static int __set_linkmode_max_speed(u32 max_speed, unsigned long *addr)
+static void __set_linkmode_max_speed(u32 max_speed, unsigned long *addr)
 {
 	const struct phy_setting *p;
 	int i;
@@ -254,13 +254,11 @@  static int __set_linkmode_max_speed(u32
 		else
 			break;
 	}
-
-	return 0;
 }
 
-static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
+static void __set_phy_supported(struct phy_device *phydev, u32 max_speed)
 {
-	return __set_linkmode_max_speed(max_speed, phydev->supported);
+	__set_linkmode_max_speed(max_speed, phydev->supported);
 }
 
 /**
@@ -273,17 +271,11 @@  static int __set_phy_supported(struct ph
  * is connected to a 1G PHY. This function allows the MAC to indicate its
  * maximum speed, and so limit what the PHY will advertise.
  */
-int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
+void phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
 {
-	int err;
-
-	err = __set_phy_supported(phydev, max_speed);
-	if (err)
-		return err;
+	__set_phy_supported(phydev, max_speed);
 
 	phy_advertise_supported(phydev);
-
-	return 0;
 }
 EXPORT_SYMBOL(phy_set_max_speed);
 
@@ -440,7 +432,9 @@  int phy_speed_down_core(struct phy_devic
 	if (min_common_speed == SPEED_UNKNOWN)
 		return -EINVAL;
 
-	return __set_linkmode_max_speed(min_common_speed, phydev->advertising);
+	__set_linkmode_max_speed(min_common_speed, phydev->advertising);
+
+	return 0;
 }
 
 static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
Index: net-next/include/linux/phy.h
===================================================================
--- net-next.orig/include/linux/phy.h
+++ net-next/include/linux/phy.h
@@ -1661,7 +1661,7 @@  int phy_disable_interrupts(struct phy_de
 void phy_request_interrupt(struct phy_device *phydev);
 void phy_free_interrupt(struct phy_device *phydev);
 void phy_print_status(struct phy_device *phydev);
-int phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
+void phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
 void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode);
 void phy_advertise_supported(struct phy_device *phydev);
 void phy_support_sym_pause(struct phy_device *phydev);