[PATCH 0/2] Kill some dead code in the Renesas Ethernet drivers

[PATCH 0/2] Kill some dead code in the Renesas Ethernet drivers

[Sergey Shtylyov]

Here are 2 patches against DaveM's 'net-next.git' repo. The Renesas drivers
call their ndo_stop() methods directly and they always return 0, making the
result checks pointless, hence remove them...

Sergey Shtylyov (2):
  ravb: ravb_close() always returns 0
  sh_eth: sh_eth_close() always returns 0

 drivers/net/ethernet/renesas/ravb_main.c | 4 +---
 drivers/net/ethernet/renesas/sh_eth.c    | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

-- 
2.26.3

[PATCH 1/2] ravb: ravb_close() always returns 0

[Sergey Shtylyov]

ravb_close() always returns 0, hence the check in ravb_wol_restore() is
pointless (however, we cannot change the prototype of ravb_close() as it
implements the driver's ndo_stop() method).

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

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
 drivers/net/ethernet/renesas/ravb_main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index b215cde68e10..02fa8cfc2b7b 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2863,9 +2863,7 @@ static int ravb_wol_restore(struct net_device *ndev)
 	/* Disable MagicPacket */
 	ravb_modify(ndev, ECMR, ECMR_MPDE, 0);
 
-	ret = ravb_close(ndev);
-	if (ret < 0)
-		return ret;
+	ravb_close(ndev);
 
 	return disable_irq_wake(priv->emac_irq);
 }
-- 
2.26.3

[PATCH 2/2] sh_eth: sh_eth_close() always returns 0

[Sergey Shtylyov]

sh_eth_close() always returns 0, hence the check in sh_eth_wol_restore()
is pointless (however we cannot change the prototype of sh_eth_close() as
it implements the driver's ndo_stop() method).

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

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
 drivers/net/ethernet/renesas/sh_eth.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index d947a628e166..b21e101b4484 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3450,9 +3450,7 @@ static int sh_eth_wol_restore(struct net_device *ndev)
 	 * both be reset and all registers restored. This is what
 	 * happens during suspend and resume without WoL enabled.
 	 */
-	ret = sh_eth_close(ndev);
-	if (ret < 0)
-		return ret;
+	sh_eth_close(ndev);
 	ret = sh_eth_open(ndev);
 	if (ret < 0)
 		return ret;
-- 
2.26.3

Re: [PATCH 1/2] ravb: ravb_close() always returns 0

[Jakub Kicinski]

On Fri, 28 Jan 2022 23:38:37 +0300 Sergey Shtylyov wrote:
> ravb_close() always returns 0, hence the check in ravb_wol_restore() is
> pointless (however, we cannot change the prototype of ravb_close() as it
> implements the driver's ndo_stop() method).
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> ---
>  drivers/net/ethernet/renesas/ravb_main.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index b215cde68e10..02fa8cfc2b7b 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2863,9 +2863,7 @@ static int ravb_wol_restore(struct net_device *ndev)
>  	/* Disable MagicPacket */
>  	ravb_modify(ndev, ECMR, ECMR_MPDE, 0);
>  
> -	ret = ravb_close(ndev);
> -	if (ret < 0)
> -		return ret;
> +	ravb_close(ndev);
>  
>  	return disable_irq_wake(priv->emac_irq);
>  }

drivers/net/ethernet/renesas/ravb_main.c:2857:13: warning: unused variable ‘ret’ [-Wunused-variable]
 2857 |         int ret;
      |             ^~~

Re: [PATCH 1/2] ravb: ravb_close() always returns 0

[Sergey Shtylyov]

Hello!

On 1/29/22 12:51 AM, Jakub Kicinski wrote:

>> ravb_close() always returns 0, hence the check in ravb_wol_restore() is
>> pointless (however, we cannot change the prototype of ravb_close() as it
>> implements the driver's ndo_stop() method).
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>> analysis tool.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>> ---
>>  drivers/net/ethernet/renesas/ravb_main.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index b215cde68e10..02fa8cfc2b7b 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -2863,9 +2863,7 @@ static int ravb_wol_restore(struct net_device *ndev)
>>  	/* Disable MagicPacket */
>>  	ravb_modify(ndev, ECMR, ECMR_MPDE, 0);
>>  
>> -	ret = ravb_close(ndev);
>> -	if (ret < 0)
>> -		return ret;
>> +	ravb_close(ndev);
>>  
>>  	return disable_irq_wake(priv->emac_irq);
>>  }
> 
> drivers/net/ethernet/renesas/ravb_main.c:2857:13: warning: unused variable ‘ret’ [-Wunused-variable]
>  2857 |         int ret;
>       |             ^~~

   Oops, sorry about that!
   This patch was created during the merge window and when it finally closed, I rushed
to send this series before the end of week, and forgot to sanity check it (thinking it
has been checked already)... :-/
   I'll fix and resend...

MBR, Sergey