Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 2 Jan 2024 09:53:30 +0000 | From | "Russell King (Oracle)" <> | Subject | Re: [net-next PATCH] net: phy: at803x: better align function varibles to open parenthesis |
| |
On Mon, Dec 18, 2023 at 12:50:11AM +0100, Christian Marangi wrote: > - if (qca808x_cdt_fault_length_valid(pair_a)) > - ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_A, > - qca808x_cdt_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_A)); > - if (qca808x_cdt_fault_length_valid(pair_b)) > - ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_B, > - qca808x_cdt_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_B)); > - if (qca808x_cdt_fault_length_valid(pair_c)) > - ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_C, > - qca808x_cdt_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_C)); > - if (qca808x_cdt_fault_length_valid(pair_d)) > - ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_D, > - qca808x_cdt_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_D)); > + if (qca808x_cdt_fault_length_valid(pair_a)) { > + val = qca808x_cdt_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_A); > + ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_A, val); > + } > + if (qca808x_cdt_fault_length_valid(pair_b)) { > + val = qca808x_cdt_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_B); > + ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_B, val); > + } > + if (qca808x_cdt_fault_length_valid(pair_c)) { > + val = qca808x_cdt_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_C); > + ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_C, val); > + } > + if (qca808x_cdt_fault_length_valid(pair_d)) { > + val = qca808x_cdt_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_D); > + ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_D, val); > + }
Maybe a static function for these?
static void qca808x_get_cdt_length(struct phy_device *phydev, u8 pair) { ethnl_cable_test_fault_length(phydev, pair, qca808x_cdt_fault_length(phydev, pair)); }
or going via 'val'. Either way, repeating the same two lines multiple times seems a bit suboptimal.
Whether or not you do this:
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Thanks!
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
| ![\](/images/icornerr.gif) |