Messages in this thread | | | Date | Tue, 2 Jan 2024 11:00:42 +0100 | From | Christian Marangi <> | Subject | Re: [net-next PATCH] net: phy: at803x: better align function varibles to open parenthesis |
| |
On Tue, Jan 02, 2024 at 09:53:30AM +0000, Russell King (Oracle) wrote: > 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! >
Hi, this has been merged, I will send a followup patch to address this! (the function has to be touched anyway as it's used by another PHY Family)
-- Ansuel
| |