[PATCH v2] net: ethernet: ag71xx: Remove dead code

[PATCH v2] net: ethernet: ag71xx: Remove dead code

[Muhammad Usama Anjum]

The err variable isn't being used anywhere other than getting
initialized to 0 and then it is being checked in if condition. The
condition can never be true. Remove the err and deadcode.

Move the rx_dropped counter above when skb isn't found.

Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- Move the rx_dropped counter above when skb isn't found.
---
 drivers/net/ethernet/atheros/ag71xx.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index db2a8ade62055..2effceeb191db 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -1619,7 +1619,6 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit)
 		unsigned int i = ring->curr & ring_mask;
 		struct ag71xx_desc *desc = ag71xx_ring_desc(ring, i);
 		int pktlen;
-		int err = 0;
 
 		if (ag71xx_desc_empty(desc))
 			break;
@@ -1643,20 +1642,16 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit)
 		skb = napi_build_skb(ring->buf[i].rx.rx_buf, ag71xx_buffer_size(ag));
 		if (!skb) {
 			skb_free_frag(ring->buf[i].rx.rx_buf);
+			ndev->stats.rx_dropped++;
 			goto next;
 		}
 
 		skb_reserve(skb, offset);
 		skb_put(skb, pktlen);
 
-		if (err) {
-			ndev->stats.rx_dropped++;
-			kfree_skb(skb);
-		} else {
-			skb->dev = ndev;
-			skb->ip_summed = CHECKSUM_NONE;
-			list_add_tail(&skb->list, &rx_list);
-		}
+		skb->dev = ndev;
+		skb->ip_summed = CHECKSUM_NONE;
+		list_add_tail(&skb->list, &rx_list);
 
 next:
 		ring->buf[i].rx.rx_buf = NULL;
-- 
2.39.2

Re: [PATCH v2] net: ethernet: ag71xx: Remove dead code

[Andrew Lunn]

On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote:
> The err variable isn't being used anywhere other than getting
> initialized to 0 and then it is being checked in if condition. The
> condition can never be true. Remove the err and deadcode.
> 
> Move the rx_dropped counter above when skb isn't found.
> 
> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

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

    Andrew

Re: [PATCH v2] net: ethernet: ag71xx: Remove dead code

[Vadim Fedorenko]

On 11/09/2024 14:58, Muhammad Usama Anjum wrote:
> The err variable isn't being used anywhere other than getting
> initialized to 0 and then it is being checked in if condition. The
> condition can never be true. Remove the err and deadcode.
> 
> Move the rx_dropped counter above when skb isn't found.
> 
> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - Move the rx_dropped counter above when skb isn't found.
> ---
>   drivers/net/ethernet/atheros/ag71xx.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
> index db2a8ade62055..2effceeb191db 100644
> --- a/drivers/net/ethernet/atheros/ag71xx.c
> +++ b/drivers/net/ethernet/atheros/ag71xx.c
> @@ -1619,7 +1619,6 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit)
>   		unsigned int i = ring->curr & ring_mask;
>   		struct ag71xx_desc *desc = ag71xx_ring_desc(ring, i);
>   		int pktlen;
> -		int err = 0;
>   
>   		if (ag71xx_desc_empty(desc))
>   			break;
> @@ -1643,20 +1642,16 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit)
>   		skb = napi_build_skb(ring->buf[i].rx.rx_buf, ag71xx_buffer_size(ag));
>   		if (!skb) {
>   			skb_free_frag(ring->buf[i].rx.rx_buf);
> +			ndev->stats.rx_dropped++;
>   			goto next;
>   		}
>   
>   		skb_reserve(skb, offset);
>   		skb_put(skb, pktlen);
>   
> -		if (err) {
> -			ndev->stats.rx_dropped++;
> -			kfree_skb(skb);
> -		} else {
> -			skb->dev = ndev;
> -			skb->ip_summed = CHECKSUM_NONE;
> -			list_add_tail(&skb->list, &rx_list);
> -		}
> +		skb->dev = ndev;
> +		skb->ip_summed = CHECKSUM_NONE;
> +		list_add_tail(&skb->list, &rx_list);
>   
>   next:
>   		ring->buf[i].rx.rx_buf = NULL;


Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Re: [PATCH v2] net: ethernet: ag71xx: Remove dead code

[Oleksij Rempel]

On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote:
> The err variable isn't being used anywhere other than getting
> initialized to 0 and then it is being checked in if condition. The
> condition can never be true. Remove the err and deadcode.
> 
> Move the rx_dropped counter above when skb isn't found.
> 
> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
 
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>

Thank you!

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2] net: ethernet: ag71xx: Remove dead code

[Eric Dumazet]

On Wed, Sep 11, 2024 at 8:20 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote:
> > The err variable isn't being used anywhere other than getting
> > initialized to 0 and then it is being checked in if condition. The
> > condition can never be true. Remove the err and deadcode.
> >
> > Move the rx_dropped counter above when skb isn't found.
> >
> > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>
> Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
>
> Thank you!
>
> Regards,
> Oleksij

I do not see any credits given to  Qianqiang Liu, who is desperate to get his
first linux patch...

https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/

Re: [PATCH v2] net: ethernet: ag71xx: Remove dead code

[Qianqiang Liu]

Hi Eric,

> I do not see any credits given to  Qianqiang Liu, who is desperate to get his
> first linux patch...
> 
> https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/

Yes, you are right! I'm a kernel newbie.
But Linux is a FOSS software, and anyone can contribute, right?
Actually, I have two patches that were merged into the linux-next branch:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=93497752dfed196b41d2804503e80b9a04318adb
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=cd0920ebab6bce93ac5054d621c0633f6a4d640b

-- 
Best,
Qianqiang Liu

Re: [PATCH v2] net: ethernet: ag71xx: Remove dead code

[Jakub Kicinski]

On Thu, 12 Sep 2024 17:56:11 +0200 Eric Dumazet wrote:
> On Wed, Sep 11, 2024 at 8:20 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote:  
> > > The err variable isn't being used anywhere other than getting
> > > initialized to 0 and then it is being checked in if condition. The
> > > condition can never be true. Remove the err and deadcode.
> > >
> > > Move the rx_dropped counter above when skb isn't found.
> > >
> > > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>  
> >
> > Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> I do not see any credits given to  Qianqiang Liu, who is desperate to get his
> first linux patch...
> 
> https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/

Right, odd, is there a reason you took over from Qianqiang Liu?
Otherwise I'd prefer if they could send the next version.
Last thing we need is arguments about ownership of trivial
patches.

This v2 has an unnecessary Fixes tag, this is not a fix.

Re: [PATCH v2] net: ethernet: ag71xx: Remove dead code

[Jakub Kicinski]

On Thu, 12 Sep 2024 17:42:22 -0700 Jakub Kicinski wrote:
> On Thu, 12 Sep 2024 17:56:11 +0200 Eric Dumazet wrote:
> > I do not see any credits given to  Qianqiang Liu, who is desperate to get his
> > first linux patch...
> > 
> > https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/  
> 
> Right, odd, is there a reason you took over from Qianqiang Liu?
> Otherwise I'd prefer if they could send the next version.
> Last thing we need is arguments about ownership of trivial
> patches.
> 
> This v2 has an unnecessary Fixes tag, this is not a fix.

Oh, I guess it may be a v2 of your own change:

https://lore.kernel.org/all/20240911105924.4028423-1-usama.anjum@collabora.com/

Are you both using the same tool? 5 year old code and suddenly we get
the same fix two times.