Re: [PATCH] net: macb: Remove redundant assignment to w0 and queue

From: Nicolas Ferre
Date: Thu Apr 29 2021 - 12:32:20 EST


On 28/04/2021 at 21:21, Jakub Kicinski wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On Wed, 28 Apr 2021 18:03:08 +0800 Jiapeng Chong wrote:
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 0f6a6cb..5f1dbc2 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3248,7 +3248,6 @@ static void gem_prog_cmp_regs(struct macb *bp, struct ethtool_rx_flow_spec *fs)
/* ignore field if any masking set */
if (tp4sp_m->ip4src == 0xFFFFFFFF) {
/* 1st compare reg - IP source address */
- w0 = 0;
w1 = 0;
w0 = tp4sp_v->ip4src;
w1 = GEM_BFINS(T2DISMSK, 1, w1); /* 32-bit compare */
@@ -3262,7 +3261,6 @@ static void gem_prog_cmp_regs(struct macb *bp, struct ethtool_rx_flow_spec *fs)
/* ignore field if any masking set */
if (tp4sp_m->ip4dst == 0xFFFFFFFF) {
/* 2nd compare reg - IP destination address */
- w0 = 0;
w1 = 0;
w0 = tp4sp_v->ip4dst;
w1 = GEM_BFINS(T2DISMSK, 1, w1); /* 32-bit compare */

Looks like this was written like that on purpose.

@@ -4829,7 +4827,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
{
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
- struct macb_queue *queue = bp->queues;
+ struct macb_queue *queue;
unsigned long flags;
unsigned int q;
int err;
@@ -4916,7 +4914,7 @@ static int __maybe_unused macb_resume(struct device *dev)
{
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
- struct macb_queue *queue = bp->queues;
+ struct macb_queue *queue;
unsigned long flags;
unsigned int q;
int err;

This chunk looks good!

Would you mind splitting the patch into two (1 - w0 assignments, and
2 - queue assignments) and reposting? We can merge the latter, the
former is up to the driver maintainer to decide.

Good move Jakub, thanks for having suggested this as we are highlighting a bug!

Best regards,
Nicolas

--
Nicolas Ferre