| Message ID | 20250411062426.8820-2-michael@fossekall.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: realtek: Add support for PHY LEDs on | expand |
On Fri, Apr 11, 2025 at 08:24:23AM +0200, Michael Klein wrote:
> Group macro definitions by chip number in lexicographic order.
Even after your patch, the registers aren't sorted as you describe.
My recommendation would be:
- group register field definitions below register number definitions.
let's call these register definition blocks.
- group register definition blocks by prefix (so blocks for the same PHY
are together.)
- within each PHY block, order by page number and then register number.
- keep definitions that aren't registers separate.
The reason is, when someone comes along and adds more definitions, they
aren't going to be using the register definition to determine whether
it already exists or not, they're going to be using the register number
and/or bitfield.
So something like the below. It could do with more definitions for
register numbers and/or page numbers.
diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 893c82479671..e61a29c54f78 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -17,6 +17,16 @@
#include "realtek.h"
+#define RTL8201F_IER 0x13
+
+#define RTL8201F_ISR 0x1e
+#define RTL8201F_ISR_ANERR BIT(15)
+#define RTL8201F_ISR_DUPLEX BIT(13)
+#define RTL8201F_ISR_LINK BIT(11)
+#define RTL8201F_ISR_MASK (RTL8201F_ISR_ANERR | \
+ RTL8201F_ISR_DUPLEX | \
+ RTL8201F_ISR_LINK)
+
#define RTL821x_PHYSR 0x11
#define RTL821x_PHYSR_DUPLEX BIT(13)
#define RTL821x_PHYSR_SPEED GENMASK(15, 14)
@@ -31,13 +41,26 @@
#define RTL821x_EXT_PAGE_SELECT 0x1e
#define RTL821x_PAGE_SELECT 0x1f
+/* 0x1c */
+#define RTL8211E_CTRL_DELAY BIT(13)
+#define RTL8211E_TX_DELAY BIT(12)
+#define RTL8211E_RX_DELAY BIT(11)
+
+/* page 0xa43 */
#define RTL8211F_PHYCR1 0x18
+#define RTL8211F_ALDPS_PLL_OFF BIT(1)
+#define RTL8211F_ALDPS_ENABLE BIT(2)
+#define RTL8211F_ALDPS_XTAL_OFF BIT(12)
+
+/* page 0xa43 */
#define RTL8211F_PHYCR2 0x19
#define RTL8211F_CLKOUT_EN BIT(0)
#define RTL8211F_PHYCR2_PHY_EEE_ENABLE BIT(5)
+/* page 0xa43 */
#define RTL8211F_INSR 0x1d
+/* page 0xd04 */
#define RTL8211F_LEDCR 0x10
#define RTL8211F_LEDCR_MODE BIT(15)
#define RTL8211F_LEDCR_ACT_TXRX BIT(4)
@@ -47,25 +70,11 @@
#define RTL8211F_LEDCR_MASK GENMASK(4, 0)
#define RTL8211F_LEDCR_SHIFT 5
+/* page 0xd08 reg 0x11 */
#define RTL8211F_TX_DELAY BIT(8)
-#define RTL8211F_RX_DELAY BIT(3)
-
-#define RTL8211F_ALDPS_PLL_OFF BIT(1)
-#define RTL8211F_ALDPS_ENABLE BIT(2)
-#define RTL8211F_ALDPS_XTAL_OFF BIT(12)
-
-#define RTL8211E_CTRL_DELAY BIT(13)
-#define RTL8211E_TX_DELAY BIT(12)
-#define RTL8211E_RX_DELAY BIT(11)
-#define RTL8201F_ISR 0x1e
-#define RTL8201F_ISR_ANERR BIT(15)
-#define RTL8201F_ISR_DUPLEX BIT(13)
-#define RTL8201F_ISR_LINK BIT(11)
-#define RTL8201F_ISR_MASK (RTL8201F_ISR_ANERR | \
- RTL8201F_ISR_DUPLEX | \
- RTL8201F_ISR_LINK)
-#define RTL8201F_IER 0x13
+/* page 0xd08 reg 0x15 */
+#define RTL8211F_RX_DELAY BIT(3)
#define RTL822X_VND1_SERDES_OPTION 0x697a
#define RTL822X_VND1_SERDES_OPTION_MODE_MASK GENMASK(5, 0)
diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c index 893c82479671..f6e402bf78bf 100644 --- a/drivers/net/phy/realtek/realtek_main.c +++ b/drivers/net/phy/realtek/realtek_main.c @@ -17,6 +17,16 @@ #include "realtek.h" +#define RTL8201F_IER 0x13 + +#define RTL8201F_ISR 0x1e +#define RTL8201F_ISR_ANERR BIT(15) +#define RTL8201F_ISR_DUPLEX BIT(13) +#define RTL8201F_ISR_LINK BIT(11) +#define RTL8201F_ISR_MASK (RTL8201F_ISR_ANERR | \ + RTL8201F_ISR_DUPLEX | \ + RTL8201F_ISR_LINK) + #define RTL821x_PHYSR 0x11 #define RTL821x_PHYSR_DUPLEX BIT(13) #define RTL821x_PHYSR_SPEED GENMASK(15, 14) @@ -31,6 +41,10 @@ #define RTL821x_EXT_PAGE_SELECT 0x1e #define RTL821x_PAGE_SELECT 0x1f +#define RTL8211E_CTRL_DELAY BIT(13) +#define RTL8211E_TX_DELAY BIT(12) +#define RTL8211E_RX_DELAY BIT(11) + #define RTL8211F_PHYCR1 0x18 #define RTL8211F_PHYCR2 0x19 #define RTL8211F_CLKOUT_EN BIT(0) @@ -47,6 +61,8 @@ #define RTL8211F_LEDCR_MASK GENMASK(4, 0) #define RTL8211F_LEDCR_SHIFT 5 +#define RTL8211F_LED_COUNT 3 + #define RTL8211F_TX_DELAY BIT(8) #define RTL8211F_RX_DELAY BIT(3) @@ -54,19 +70,6 @@ #define RTL8211F_ALDPS_ENABLE BIT(2) #define RTL8211F_ALDPS_XTAL_OFF BIT(12) -#define RTL8211E_CTRL_DELAY BIT(13) -#define RTL8211E_TX_DELAY BIT(12) -#define RTL8211E_RX_DELAY BIT(11) - -#define RTL8201F_ISR 0x1e -#define RTL8201F_ISR_ANERR BIT(15) -#define RTL8201F_ISR_DUPLEX BIT(13) -#define RTL8201F_ISR_LINK BIT(11) -#define RTL8201F_ISR_MASK (RTL8201F_ISR_ANERR | \ - RTL8201F_ISR_DUPLEX | \ - RTL8201F_ISR_LINK) -#define RTL8201F_IER 0x13 - #define RTL822X_VND1_SERDES_OPTION 0x697a #define RTL822X_VND1_SERDES_OPTION_MODE_MASK GENMASK(5, 0) #define RTL822X_VND1_SERDES_OPTION_MODE_2500BASEX_SGMII 0 @@ -112,8 +115,6 @@ #define RTL_8221B_VN_CG 0x001cc84a #define RTL_8251B 0x001cc862 -#define RTL8211F_LED_COUNT 3 - MODULE_DESCRIPTION("Realtek PHY driver"); MODULE_AUTHOR("Johnson Leung"); MODULE_LICENSE("GPL");
Group macro definitions by chip number in lexicographic order. Signed-off-by: Michael Klein <michael@fossekall.de> --- drivers/net/phy/realtek/realtek_main.c | 31 +++++++++++++------------- 1 file changed, 16 insertions(+), 15 deletions(-)