* [net-next PATCH] net: phy: at803x: better align function varibles to open parenthesis
@ 2023-12-17 23:50 Christian Marangi
2023-12-18 9:47 ` Andrew Lunn
2024-01-02 9:53 ` Russell King (Oracle)
0 siblings, 2 replies; 4+ messages in thread
From: Christian Marangi @ 2023-12-17 23:50 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
Cc: Christian Marangi
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.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/net/phy/at803x.c | 65 ++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 29 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index d5dc927618ab..5e3fbaa7c26c 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -1781,27 +1781,27 @@ static int qca808x_phy_fast_retrain_config(struct phy_device *phydev)
return ret;
phy_write_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_TOP_OPTION1,
- QCA808X_TOP_OPTION1_DATA);
+ QCA808X_TOP_OPTION1_DATA);
phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_20DB,
- QCA808X_MSE_THRESHOLD_20DB_VALUE);
+ QCA808X_MSE_THRESHOLD_20DB_VALUE);
phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_17DB,
- QCA808X_MSE_THRESHOLD_17DB_VALUE);
+ QCA808X_MSE_THRESHOLD_17DB_VALUE);
phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_27DB,
- QCA808X_MSE_THRESHOLD_27DB_VALUE);
+ QCA808X_MSE_THRESHOLD_27DB_VALUE);
phy_write_mmd(phydev, MDIO_MMD_PMAPMD, QCA808X_PHY_MMD1_MSE_THRESHOLD_28DB,
- QCA808X_MSE_THRESHOLD_28DB_VALUE);
+ QCA808X_MSE_THRESHOLD_28DB_VALUE);
phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_1,
- QCA808X_MMD3_DEBUG_1_VALUE);
+ QCA808X_MMD3_DEBUG_1_VALUE);
phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_4,
- QCA808X_MMD3_DEBUG_4_VALUE);
+ QCA808X_MMD3_DEBUG_4_VALUE);
phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_5,
- QCA808X_MMD3_DEBUG_5_VALUE);
+ QCA808X_MMD3_DEBUG_5_VALUE);
phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_3,
- QCA808X_MMD3_DEBUG_3_VALUE);
+ QCA808X_MMD3_DEBUG_3_VALUE);
phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_6,
- QCA808X_MMD3_DEBUG_6_VALUE);
+ QCA808X_MMD3_DEBUG_6_VALUE);
phy_write_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_DEBUG_2,
- QCA808X_MMD3_DEBUG_2_VALUE);
+ QCA808X_MMD3_DEBUG_2_VALUE);
return 0;
}
@@ -1838,13 +1838,14 @@ static int qca808x_config_init(struct phy_device *phydev)
/* Active adc&vga on 802.3az for the link 1000M and 100M */
ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_ADDR_CLD_CTRL7,
- QCA808X_8023AZ_AFE_CTRL_MASK, QCA808X_8023AZ_AFE_EN);
+ QCA808X_8023AZ_AFE_CTRL_MASK, QCA808X_8023AZ_AFE_EN);
if (ret)
return ret;
/* Adjust the threshold on 802.3az for the link 1000M */
ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
- QCA808X_PHY_MMD3_AZ_TRAINING_CTRL, QCA808X_MMD3_AZ_TRAINING_VAL);
+ QCA808X_PHY_MMD3_AZ_TRAINING_CTRL,
+ QCA808X_MMD3_AZ_TRAINING_VAL);
if (ret)
return ret;
@@ -1870,7 +1871,8 @@ static int qca808x_config_init(struct phy_device *phydev)
/* Configure adc threshold as 100mv for the link 10M */
return at803x_debug_reg_mask(phydev, QCA808X_PHY_DEBUG_ADC_THRESHOLD,
- QCA808X_ADC_THRESHOLD_MASK, QCA808X_ADC_THRESHOLD_100MV);
+ QCA808X_ADC_THRESHOLD_MASK,
+ QCA808X_ADC_THRESHOLD_100MV);
}
static int qca808x_read_status(struct phy_device *phydev)
@@ -1883,7 +1885,7 @@ static int qca808x_read_status(struct phy_device *phydev)
return ret;
linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->lp_advertising,
- ret & MDIO_AN_10GBT_STAT_LP2_5G);
+ ret & MDIO_AN_10GBT_STAT_LP2_5G);
ret = genphy_read_status(phydev);
if (ret)
@@ -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);
+ }
*finished = true;
@@ -2148,8 +2154,9 @@ static void qca808x_link_change_notify(struct phy_device *phydev)
* the interface device address is always phy address added by 1.
*/
mdiobus_c45_modify_changed(phydev->mdio.bus, phydev->mdio.addr + 1,
- MDIO_MMD_PMAPMD, QCA8081_PHY_SERDES_MMD1_FIFO_CTRL,
- QCA8081_PHY_FIFO_RSTN, phydev->link ? QCA8081_PHY_FIFO_RSTN : 0);
+ MDIO_MMD_PMAPMD, QCA8081_PHY_SERDES_MMD1_FIFO_CTRL,
+ QCA8081_PHY_FIFO_RSTN,
+ phydev->link ? QCA8081_PHY_FIFO_RSTN : 0);
}
static struct phy_driver at803x_driver[] = {
--
2.40.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [net-next PATCH] net: phy: at803x: better align function varibles to open parenthesis
2023-12-17 23:50 [net-next PATCH] net: phy: at803x: better align function varibles to open parenthesis Christian Marangi
@ 2023-12-18 9:47 ` Andrew Lunn
2024-01-02 9:53 ` Russell King (Oracle)
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Lunn @ 2023-12-18 9:47 UTC (permalink / raw)
To: Christian Marangi
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
On Mon, Dec 18, 2023 at 12:50:11AM +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.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net-next PATCH] net: phy: at803x: better align function varibles to open parenthesis
2023-12-17 23:50 [net-next PATCH] net: phy: at803x: better align function varibles to open parenthesis Christian Marangi
2023-12-18 9:47 ` Andrew Lunn
@ 2024-01-02 9:53 ` Russell King (Oracle)
2024-01-02 10:00 ` Christian Marangi
1 sibling, 1 reply; 4+ messages in thread
From: Russell King (Oracle) @ 2024-01-02 9:53 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
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!
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net-next PATCH] net: phy: at803x: better align function varibles to open parenthesis
2024-01-02 9:53 ` Russell King (Oracle)
@ 2024-01-02 10:00 ` Christian Marangi
0 siblings, 0 replies; 4+ messages in thread
From: Christian Marangi @ 2024-01-02 10:00 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-01-02 10:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-17 23:50 [net-next PATCH] net: phy: at803x: better align function varibles to open parenthesis Christian Marangi
2023-12-18 9:47 ` Andrew Lunn
2024-01-02 9:53 ` Russell King (Oracle)
2024-01-02 10:00 ` Christian Marangi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).