| Message ID | E1nFfwa-006X66-Dd@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RFC,net-next,1/5] net: dsa: b53: clean up if() condition to be more readable | expand |
Sorry for the unintentional re-send, please ignore the second set. On Thu, Feb 03, 2022 at 05:30:52PM +0000, Russell King (Oracle) wrote: > I've stared at this if() statement for a while trying to work out if > it really does correspond with the comment above, and it does seem to. > However, let's make it more readable and phrase it in the same way as > the comment. > > Also add a FIXME into the comment - we appear to deny Gigabit modes for > 802.3z interface modes, but 802.3z interface modes only operate at > gigabit and above. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > drivers/net/dsa/b53/b53_common.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c > index a3b98992f180..7d62b0aeaae9 100644 > --- a/drivers/net/dsa/b53/b53_common.c > +++ b/drivers/net/dsa/b53/b53_common.c > @@ -1327,11 +1327,14 @@ void b53_phylink_validate(struct dsa_switch *ds, int port, > > /* With the exclusion of 5325/5365, MII, Reverse MII and 802.3z, we > * support Gigabit, including Half duplex. > + * > + * FIXME: this is weird - 802.3z is always Gigabit, but we exclude > + * it here. Why? This makes no sense. > */ > - if (state->interface != PHY_INTERFACE_MODE_MII && > - state->interface != PHY_INTERFACE_MODE_REVMII && > - !phy_interface_mode_is_8023z(state->interface) && > - !(is5325(dev) || is5365(dev))) { > + if (!(state->interface == PHY_INTERFACE_MODE_MII || > + state->interface == PHY_INTERFACE_MODE_REVMII || > + phy_interface_mode_is_8023z(state->interface) || > + is5325(dev) || is5365(dev))) { > phylink_set(mask, 1000baseT_Full); > phylink_set(mask, 1000baseT_Half); > } > -- > 2.30.2 > >
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index a3b98992f180..7d62b0aeaae9 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -1327,11 +1327,14 @@ void b53_phylink_validate(struct dsa_switch *ds, int port, /* With the exclusion of 5325/5365, MII, Reverse MII and 802.3z, we * support Gigabit, including Half duplex. + * + * FIXME: this is weird - 802.3z is always Gigabit, but we exclude + * it here. Why? This makes no sense. */ - if (state->interface != PHY_INTERFACE_MODE_MII && - state->interface != PHY_INTERFACE_MODE_REVMII && - !phy_interface_mode_is_8023z(state->interface) && - !(is5325(dev) || is5365(dev))) { + if (!(state->interface == PHY_INTERFACE_MODE_MII || + state->interface == PHY_INTERFACE_MODE_REVMII || + phy_interface_mode_is_8023z(state->interface) || + is5325(dev) || is5365(dev))) { phylink_set(mask, 1000baseT_Full); phylink_set(mask, 1000baseT_Half); }
I've stared at this if() statement for a while trying to work out if it really does correspond with the comment above, and it does seem to. However, let's make it more readable and phrase it in the same way as the comment. Also add a FIXME into the comment - we appear to deny Gigabit modes for 802.3z interface modes, but 802.3z interface modes only operate at gigabit and above. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/dsa/b53/b53_common.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)