[PATCH] net: phylink: Separating two unrelated definitions for improving code readability

[PATCH] net: phylink: Separating two unrelated definitions for improving code readability

[Cong Yi]

From: Cong Yi <yicong@kylinos.cn>

After the support of PCS state machine, phylink and pcs two
unrelated state enum definitions are put together, which brings
some confusion to code reading.

This patch defines the two separately.

Signed-off-by: Cong Yi <yicong@kylinos.cn>
---
 drivers/net/phy/phylink.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 3e9957b6aa14..1c65fd29538d 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -30,11 +30,13 @@
 	(ADVERTISED_TP | ADVERTISED_MII | ADVERTISED_FIBRE | \
 	 ADVERTISED_BNC | ADVERTISED_AUI | ADVERTISED_Backplane)
 
-enum {
+enum phylink_disable_state {
 	PHYLINK_DISABLE_STOPPED,
 	PHYLINK_DISABLE_LINK,
 	PHYLINK_DISABLE_MAC_WOL,
+};
 
+enum phylink_pcs_state {
 	PCS_STATE_DOWN = 0,
 	PCS_STATE_STARTING,
 	PCS_STATE_STARTED,
@@ -76,7 +78,7 @@ struct phylink {
 	struct phylink_link_state phy_state;
 	struct work_struct resolve;
 	unsigned int pcs_neg_mode;
-	unsigned int pcs_state;
+	enum phylink_pcs_state pcs_state;
 
 	bool link_failed;
 	bool using_mac_select_pcs;
-- 
2.25.1

Re: [PATCH] net: phylink: Separating two unrelated definitions for improving code readability

[Russell King (Oracle)]

On Wed, Nov 20, 2024 at 02:27:53PM +0800, Cong Yi wrote:
> From: Cong Yi <yicong@kylinos.cn>
> 
> After the support of PCS state machine, phylink and pcs two
> unrelated state enum definitions are put together, which brings
> some confusion to code reading.

Hmm, so the definitions being prefixed with "PCS_STATE_" and the
variable being called "pcs_state" isn't enough?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] net: phylink: Separating two unrelated definitions for improving code readability

[Cong Yi]

Hi, Russell King:

Thank you for your reply!
Yes, as you say, there is no problem with the definitions themselves
being named. When I just read from Linux-5.4 to 6.6, I thought
that PCS_STATE_ and PHYLINK_DISABLE- were associated in some way.
After reading the code carefully, I found that there was no correlation。
In order to avoid similar confusion, I sent this patch.