diff mbox series

[RFC,net-next,1/5] net: dsa: b53: clean up if() condition to be more readable

Message ID E1nFdPI-006Wh0-HW@rmk-PC.armlinux.org.uk (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: b53: convert to phylink_generic_validate() and mark as non-legacy | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: linux@armlinux.org.uk
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 18 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Russell King (Oracle) Feb. 3, 2022, 2:48 p.m. UTC
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(-)

Comments

Florian Fainelli Feb. 16, 2022, 5:52 p.m. UTC | #1
On 2/3/22 6:48 AM, 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>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Did I mention that I always struggled and still do with double negations
and not just while programming?
diff mbox series

Patch

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);
 	}