Messages in this thread | | | Date | Sat, 6 Jan 2024 17:33:56 +0300 | Subject | Re: [PATCH net-next v2 1/7] net: dsa: mt7530: always trap frames to active CPU port on MT7530 | From | Arınç ÜNAL <> |
| |
On 4.01.2024 18:22, Vladimir Oltean wrote: > On Wed, Dec 27, 2023 at 07:43:41AM +0300, Arınç ÜNAL wrote: >> @@ -3075,6 +3071,38 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port, >> return 0; >> } >> >> +static void >> +mt753x_conduit_state_change(struct dsa_switch *ds, >> + const struct net_device *conduit, >> + bool operational) >> +{ >> + struct dsa_port *cpu_dp = conduit->dsa_ptr; >> + struct mt7530_priv *priv = ds->priv; >> + u8 mask; >> + int val = 0; > > Longest line first. > >> + >> + /* Set the CPU port to trap frames to for MT7530. Trapped frames will be >> + * forwarded to the numerically smallest CPU port which the DSA conduit >> + * interface its affine to is up. > > "first CPU port whose conduit interface is up" > >> + */ >> + if (priv->id != ID_MT7530 && priv->id != ID_MT7621) >> + return; >> + >> + mask = BIT(cpu_dp->index); >> + >> + if (operational) >> + priv->active_cpu_ports |= mask; >> + else >> + priv->active_cpu_ports &= ~mask; >> + >> + if (priv->active_cpu_ports) >> + val = >> + CPU_EN | >> + CPU_PORT(__ffs((unsigned long)priv->active_cpu_ports)); > > I don't think the type cast is necessary (implicit type promotion takes place). > > Also, it is customary to put {} for multi-line "if" blocks, even if they are > made up of a single expression. > > But without the type cast, it could look like this. > > if (priv->active_cpu_ports) > val = CPU_EN | CPU_PORT(__ffs(priv->active_cpu_ports));
Will do. Thanks for dealing with my rookie mistakes. :)
Arınç
| |