On 08/12/2020 14:07:54+0200, Vladimir Oltean wrote:
> The index of the LAG is equal to the logical port ID that all the
> physical port members have, which is further equal to the index of the
> first physical port that is a member of the LAG.
>
> The code gets a bit carried away with logic like this:
>
> if (a == b)
> c = a;
> else
> c = b;
>
> which can be simplified, of course, into:
>
> c = b;
>
> (with a being port, b being lp, c being lag)
>
> This further makes the "lp" variable redundant, since we can use "lag"
> everywhere where "lp" (logical port) was used. So instead of a "c = b"
> assignment, we can do a complete deletion of b. Only one comment here:
>
> if (bond_mask) {
> lp = __ffs(bond_mask);
> ocelot->lags[lp] = 0;
> }
>
> lp was clobbered before, because it was used as a temporary variable to
> hold the new smallest port ID from the bond. Now that we don't have "lp"
> any longer, we'll just avoid the temporary variable and zeroize the
> bonding mask directly.
>
> Signed-off-by: Vladimir Oltean <vladimir.olt...@nxp.com>
Reviewed-by: Alexandre Belloni <alexandre.bell...@bootlin.com>
> ---
> drivers/net/ethernet/mscc/ocelot.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/mscc/ocelot.c
> b/drivers/net/ethernet/mscc/ocelot.c
> index 30dee1f957d1..080fd4ce37ea 100644
> --- a/drivers/net/ethernet/mscc/ocelot.c
> +++ b/drivers/net/ethernet/mscc/ocelot.c
> @@ -1291,28 +1291,24 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int
> port,
> struct net_device *bond)
> {
> u32 bond_mask = 0;
> - int lag, lp;
> + int lag;
>
> ocelot->ports[port]->bond = bond;
>
> bond_mask = ocelot_get_bond_mask(ocelot, bond);
>
> - lp = __ffs(bond_mask);
> + lag = __ffs(bond_mask);
>
> /* If the new port is the lowest one, use it as the logical port from
> * now on
> */
> - if (port == lp) {
> - lag = port;
> + if (port == lag) {
> ocelot->lags[port] = bond_mask;
> bond_mask &= ~BIT(port);
> - if (bond_mask) {
> - lp = __ffs(bond_mask);
> - ocelot->lags[lp] = 0;
> - }
> + if (bond_mask)
> + ocelot->lags[__ffs(bond_mask)] = 0;
> } else {
> - lag = lp;
> - ocelot->lags[lp] |= BIT(port);
> + ocelot->lags[lag] |= BIT(port);
> }
>
> ocelot_setup_lag(ocelot, lag);
> --
> 2.25.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
