[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <30673ad6023785016bddec34eb97f4259580340a.camel@perches.com>
Date: Wed, 20 Dec 2023 02:28:34 -0800
From: Joe Perches <joe@...ches.com>
To: Christian Marangi <ansuelsmth@...il.com>, Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>, Russell King
<linux@...linux.org.uk>, "David S. Miller" <davem@...emloft.net>, Eric
Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo
Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [net-next PATCH v2] net: phy: at803x: better align function
varibles to open parenthesis
On Tue, 2023-12-19 at 21:21 +0100, Christian Marangi wrote:
> Better align function variables to open parenthesis as suggested by
> checkpatch script for qca808x function to make code cleaner.
>
> For cable_test_get_status function some additional rework was needed to
> handle too long functions.
[]
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
[]
> @@ -2070,18 +2072,22 @@ static int qca808x_cable_test_get_status(struct phy_device *phydev, bool *finish
> ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_D,
> qca808x_cable_test_result_trans(pair_d));
>
> - 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);
> + }
trivia:
Using the preprocessor can make this look simpler so there's
no wall of text and no duplicated identifiers.
Something like:
#define test_length(phydev, locase, upcase) \
if (qca808x_cdt_fault_length(locase)) { \
int len = qca808x_cdt_fault_length(phydev, ETHTOOL_A_CABLE_ ## upcase); \
\
ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_ ## upcase, len); \
}
so these could be
test_length(phydev, pair_a, PAIR_A);
test_length(phydev, pair_b, PAIR_B);
test_length(phydev, pair_d, PAIR_C);
test_length(phydev, pair_d, PAIR_D);
Powered by blists - more mailing lists