| Message ID | tencent_9E44A7B2F489B91A12A11C2639C5A4B9F40A@qq.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phylink: Separating two unrelated definitions for improving code readability | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
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?
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.
On 11/20/24 07:27, 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. > > This patch defines the two separately. > > Signed-off-by: Cong Yi <yicong@kylinos.cn> ## Form letter - net-next-closed The merge window for v6.13 has begun and net-next is closed for new drivers, features, code refactoring and optimizations. We are currently accepting bug fixes only. Please repost when net-next reopens after Dec 2nd. RFC patches sent for review only are welcome at any time. See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
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;