[PATCH net-next] net: phylink: use for_each_set_bit()

[PATCH net-next] net: phylink: use for_each_set_bit()

[Russell King (Oracle)]

Use for_each_set_bit() rather than open coding the for() test_bit()
loop.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index d2fa949ff1ea..4d0f39410ad0 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -697,21 +697,20 @@ static int phylink_validate_mask(struct phylink *pl, unsigned long *supported,
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(all_s) = { 0, };
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(s);
 	struct phylink_link_state t;
-	int intf;
+	int interface;
 
-	for (intf = 0; intf < PHY_INTERFACE_MODE_MAX; intf++) {
-		if (test_bit(intf, interfaces)) {
-			linkmode_copy(s, supported);
+	for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX) {
+		linkmode_copy(s, supported);
 
-			t = *state;
-			t.interface = intf;
-			if (!phylink_validate_mac_and_pcs(pl, s, &t)) {
-				linkmode_or(all_s, all_s, s);
-				linkmode_or(all_adv, all_adv, t.advertising);
-			}
+		t = *state;
+		t.interface = interface;
+		if (!phylink_validate_mac_and_pcs(pl, s, &t)) {
+			linkmode_or(all_s, all_s, s);
+			linkmode_or(all_adv, all_adv, t.advertising);
 		}
 	}
 
+
 	linkmode_copy(supported, all_s);
 	linkmode_copy(state->advertising, all_adv);
 
-- 
2.30.2

Re: [PATCH net-next] net: phylink: use for_each_set_bit()

[Andrew Lunn]

> +		t = *state;
> +		t.interface = interface;
> +		if (!phylink_validate_mac_and_pcs(pl, s, &t)) {
> +			linkmode_or(all_s, all_s, s);
> +			linkmode_or(all_adv, all_adv, t.advertising);
>  		}
>  	}
>  
> +
>  	linkmode_copy(supported, all_s);

Adding another blank line here was probably unintentional?

Otherwise:

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

    Andrew

Re: [PATCH net-next] net: phylink: use for_each_set_bit()

[Russell King (Oracle)]

On Fri, Nov 17, 2023 at 05:15:17PM +0100, Andrew Lunn wrote:
> > +		t = *state;
> > +		t.interface = interface;
> > +		if (!phylink_validate_mac_and_pcs(pl, s, &t)) {
> > +			linkmode_or(all_s, all_s, s);
> > +			linkmode_or(all_adv, all_adv, t.advertising);
> >  		}
> >  	}
> >  
> > +
> >  	linkmode_copy(supported, all_s);
> 
> Adding another blank line here was probably unintentional?

Hmm, indeed, thanks for spotting. I always forget whether I'll need to
send a v2 for something this trivial or whether netdev folk will fix
it up when committing it. I'm happy to resend.

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

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: phylink: use for_each_set_bit()

[Andrew Lunn]

> Hmm, indeed, thanks for spotting. I always forget whether I'll need to
> send a v2 for something this trivial or whether netdev folk will fix
> it up when committing it. I'm happy to resend.

You should resend. netdev patch acceptance is pretty bot driven, with
the normal 'fast path' not allowing the patch to be edited by the
merger.

	Andrew

Re: [PATCH net-next] net: phylink: use for_each_set_bit()

[Russell King (Oracle)]

On Fri, Nov 17, 2023 at 05:30:41PM +0100, Andrew Lunn wrote:
> > Hmm, indeed, thanks for spotting. I always forget whether I'll need to
> > send a v2 for something this trivial or whether netdev folk will fix
> > it up when committing it. I'm happy to resend.
> 
> You should resend. netdev patch acceptance is pretty bot driven, with
> the normal 'fast path' not allowing the patch to be edited by the
> merger.

I think it does happen sometimes? Or maybe I'm misremembering with other
subsystems...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!