]> git.ipfire.org Git - thirdparty/openwrt.git/commitdiff
realtek: pcs: rtl93xx: share IP mode register write 23213/head
authorJonas Jelonek <jelonek.jonas@gmail.com>
Mon, 27 Apr 2026 10:34:27 +0000 (10:34 +0000)
committerRobert Marko <robimarko@gmail.com>
Thu, 7 May 2026 10:21:18 +0000 (12:21 +0200)
RTL930x and RTL931x program the same physical SerDes IP mode field
(page 0x1f reg 0x09, bits 11:7 hold the 5-bit mode value, bit 6 is
the "force mode" enable), but did so via two unrelated code paths:
RTL930x kept the force bit separate from the value in a __set helper,
while RTL931x had it baked into each switch-case constant.

Add a shared rtpcs_93xx_sds_set_ip_mode() that takes the rtpcs_sds_mode
enum, looks up the 5-bit value from the existing sds_hw_mode_vals
table, and writes value | force-bit in one place.

Convert both variants:
 - RTL930x: drop __rtpcs_930x_sds_set_ip_mode and the manual table
   lookup; __rtpcs_930x_sds_get_ip_mode is replaced by the shared
   rtpcs_93xx_sds_get_ip_mode, which reverse-looks the raw register
   value up in sds_hw_mode_vals[] and returns the matching enum
   rtpcs_sds_mode (or -ENOENT for an unmapped raw value). The
   wrapper that orchestrates power, CMU, state machine and rx-reset
   around the mode write is renamed to rtpcs_930x_sds_apply_ip_mode
   for clarity.
 - RTL931x: drop the per-mode switch and the leftover pr_info debug
   print; rename the symerr-clear + MAC-OFF + IP-mode-write wrapper
   to rtpcs_931x_sds_apply_ip_mode.

rtpcs_930x_sds_reconfigure_to_pll() now goes through the new shared
get/set helpers: it saves the current IP mode as an enum on entry
and restores it via the enum-taking setter after the PLL reconfigure.
This changes behavior by mapping the raw mode setting through the
hardware mode table, effectively blocking unknown modes which might be
set by bootloader or somewhere else. This is intended and might uncover
unknown behavior instead of hiding it.

As a side-effect, QSGMII is now properly set too for RTL931x. Most code
paths anyway already had support for this mode, but it was missing from
the mode setting.

Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
Link: https://github.com/openwrt/openwrt/pull/23213
Signed-off-by: Robert Marko <robimarko@gmail.com>
target/linux/realtek/files-6.18/drivers/net/pcs/pcs-rtl-otto.c

index 7a8020de73093672c0c4dc559667d5c188bced28..b006da9f9909dc99a259bb3fc7fe157a67eb2056 100644 (file)
@@ -82,6 +82,7 @@
 #define RTPCS_93XX_SDS_MODE_QSGMII             0x06
 #define RTPCS_93XX_SDS_MODE_USXGMII            0x0d
 #define RTPCS_93XX_SDS_MODE_XSGMII             0x10
+#define RTPCS_93XX_SDS_MODE_HISGMII            0x12
 #define RTPCS_93XX_SDS_MODE_2500BASEX          0x16
 #define RTPCS_93XX_SDS_MODE_10GBASER           0x1a
 #define RTPCS_93XX_SDS_MODE_OFF                        0x1f
@@ -1196,6 +1197,7 @@ static const s16 rtpcs_93xx_sds_hw_mode_vals[RTPCS_SDS_MODE_MAX] = {
        [RTPCS_SDS_MODE_2500BASEX]              = RTPCS_93XX_SDS_MODE_2500BASEX,
        [RTPCS_SDS_MODE_10GBASER]               = RTPCS_93XX_SDS_MODE_10GBASER,
        [RTPCS_SDS_MODE_QSGMII]                 = RTPCS_93XX_SDS_MODE_QSGMII,
+       [RTPCS_SDS_MODE_HISGMII]                = RTPCS_93XX_SDS_MODE_HISGMII,
        [RTPCS_SDS_MODE_XSGMII]                 = RTPCS_93XX_SDS_MODE_XSGMII,
        [RTPCS_SDS_MODE_USXGMII_10GSXGMII]      = RTPCS_93XX_SDS_MODE_USXGMII,
        [RTPCS_SDS_MODE_USXGMII_10GDXGMII]      = RTPCS_93XX_SDS_MODE_USXGMII,
@@ -1483,6 +1485,42 @@ static int rtpcs_93xx_sds_set_mac_mode(struct rtpcs_serdes *sds, enum rtpcs_sds_
        return 0;
 }
 
+/*
+ * Read/write the SerDes IP mode register: page 0x1f reg 0x09, bits 11:7
+ * hold the 5-bit mode value, bit 6 is the "force mode" enable. The same
+ * physical field is used on RTL930x and RTL931x.
+ */
+static int rtpcs_93xx_sds_get_ip_mode(struct rtpcs_serdes *sds)
+{
+       const s16 *vals = sds->ctrl->cfg->sds_hw_mode_vals;
+       int raw;
+
+       raw = rtpcs_sds_read_bits(sds, 0x1f, 0x09, 11, 7);
+       if (raw < 0)
+               return raw;
+
+       for (int i = 0; i < RTPCS_SDS_MODE_MAX; i++)
+               if (vals[i] == raw)
+                       return i;
+
+       return -ENOENT;
+}
+
+static int rtpcs_93xx_sds_set_ip_mode(struct rtpcs_serdes *sds, enum rtpcs_sds_mode hw_mode)
+{
+       int raw;
+
+       if (hw_mode >= RTPCS_SDS_MODE_MAX)
+               return -EINVAL;
+
+       raw = sds->ctrl->cfg->sds_hw_mode_vals[hw_mode];
+       if (raw < 0)
+               return -EOPNOTSUPP;
+
+       /* BIT(0) is force mode enable bit */
+       return rtpcs_sds_write_bits(sds, 0x1f, 0x09, 11, 6, raw << 1 | BIT(0));
+}
+
 /* RTL930X */
 
 /* This mapping is not coherent so it cannot be expressed arithmetically */
@@ -1661,17 +1699,6 @@ static int rtpcs_930x_sds_wait_clock_ready(struct rtpcs_serdes *sds)
        return -EBUSY;
 }
 
-static int __rtpcs_930x_sds_get_ip_mode(struct rtpcs_serdes *sds)
-{
-       return rtpcs_sds_read_bits(sds, 0x1f, 0x09, 11, 7);
-}
-
-static int __rtpcs_930x_sds_set_ip_mode(struct rtpcs_serdes *sds, u32 mode)
-{
-       /* BIT(0) is force mode enable bit */
-       return rtpcs_sds_write_bits(sds, 0x1f, 0x09, 11, 6, (mode & 0x1f) << 1 | BIT(0));
-}
-
 static void rtpcs_930x_sds_set_power(struct rtpcs_serdes *sds, bool on)
 {
        int power_down = on ? 0x0 : 0x3;
@@ -1685,9 +1712,11 @@ static int rtpcs_930x_sds_reconfigure_to_pll(struct rtpcs_serdes *sds, enum rtpc
 {
        enum rtpcs_sds_pll_speed speed;
        enum rtpcs_sds_pll_type old_pll;
-       int mode, ret;
+       int hw_mode, ret;
 
-       mode = __rtpcs_930x_sds_get_ip_mode(sds);
+       hw_mode = rtpcs_93xx_sds_get_ip_mode(sds);
+       if (hw_mode < 0)
+               return hw_mode;
 
        ret = rtpcs_930x_sds_get_pll_select(sds, &old_pll);
        if (ret < 0)
@@ -1698,7 +1727,7 @@ static int rtpcs_930x_sds_reconfigure_to_pll(struct rtpcs_serdes *sds, enum rtpc
                return ret;
 
        rtpcs_930x_sds_set_power(sds, false);
-       __rtpcs_930x_sds_set_ip_mode(sds, RTPCS_93XX_SDS_MODE_OFF);
+       rtpcs_93xx_sds_set_ip_mode(sds, RTPCS_SDS_MODE_OFF);
 
        ret = rtpcs_93xx_sds_set_pll_config(sds, pll, speed);
        if (ret < 0)
@@ -1708,7 +1737,7 @@ static int rtpcs_930x_sds_reconfigure_to_pll(struct rtpcs_serdes *sds, enum rtpc
        if (ret < 0)
                return ret;
 
-       __rtpcs_930x_sds_set_ip_mode(sds, mode);
+       rtpcs_93xx_sds_set_ip_mode(sds, hw_mode);
        if (rtpcs_930x_sds_wait_clock_ready(sds))
                pr_err("%s: SDS %d could not sync clock\n", __func__, sds->id);
 
@@ -1753,10 +1782,10 @@ static int rtpcs_930x_sds_init_state_machine(struct rtpcs_serdes *sds,
        return ret;
 }
 
-static int rtpcs_930x_sds_set_ip_mode(struct rtpcs_serdes *sds,
-                                     enum rtpcs_sds_mode hw_mode)
+static int rtpcs_930x_sds_apply_ip_mode(struct rtpcs_serdes *sds,
+                                       enum rtpcs_sds_mode hw_mode)
 {
-       int mode_val, ret;
+       int ret;
 
        /*
         * TODO: Usually one would expect that it is enough to modify the SDS_MODE_SEL_*
@@ -1765,18 +1794,8 @@ static int rtpcs_930x_sds_set_ip_mode(struct rtpcs_serdes *sds,
         * if this sequence should quit early in case of errors.
         */
 
-       if (hw_mode >= RTPCS_SDS_MODE_MAX)
-               return -EINVAL;
-
-       mode_val = sds->ctrl->cfg->sds_hw_mode_vals[hw_mode];
-       if (mode_val < 0) {
-               pr_err("%s: SDS %d does not support mode %d\n", __func__,
-                      sds->id, hw_mode);
-               return mode_val;
-       }
-
        rtpcs_930x_sds_set_power(sds, false);
-       ret = __rtpcs_930x_sds_set_ip_mode(sds, RTPCS_93XX_SDS_MODE_OFF);
+       ret = rtpcs_93xx_sds_set_ip_mode(sds, RTPCS_SDS_MODE_OFF);
        if (ret < 0)
                return ret;
 
@@ -1788,7 +1807,7 @@ static int rtpcs_930x_sds_set_ip_mode(struct rtpcs_serdes *sds,
                pr_err("%s: SDS %d could not configure PLL for mode %d: %d\n", __func__,
                       sds->id, hw_mode, ret);
 
-       ret = __rtpcs_930x_sds_set_ip_mode(sds, mode_val);
+       ret = rtpcs_93xx_sds_set_ip_mode(sds, hw_mode);
        if (ret < 0)
                return ret;
 
@@ -1820,7 +1839,7 @@ static int rtpcs_930x_sds_set_mode(struct rtpcs_serdes *sds, enum rtpcs_sds_mode
        case RTPCS_SDS_MODE_1000BASEX:
        case RTPCS_SDS_MODE_2500BASEX:
        case RTPCS_SDS_MODE_10GBASER:
-               return rtpcs_930x_sds_set_ip_mode(sds, hw_mode);
+               return rtpcs_930x_sds_apply_ip_mode(sds, hw_mode);
 
        default:
                break;
@@ -3213,14 +3232,12 @@ static int rtpcs_931x_sds_power(struct rtpcs_serdes *sds, bool power_on)
 }
 
 /*
- * rtpcs_931x_sds_set_ip_mode
- *
- * Set the SerDes mode in the SerDes IP block's registers.
+ * Set the SerDes mode in the SerDes IP block's registers, after clearing
+ * the symbol error counter and forcing the MAC mode off.
  */
-static int rtpcs_931x_sds_set_ip_mode(struct rtpcs_serdes *sds,
-                                     enum rtpcs_sds_mode hw_mode)
+static int rtpcs_931x_sds_apply_ip_mode(struct rtpcs_serdes *sds,
+                                       enum rtpcs_sds_mode hw_mode)
 {
-       u32 mode_val;
        int ret;
 
        /* clear symbol error count before changing mode */
@@ -3229,51 +3246,7 @@ static int rtpcs_931x_sds_set_ip_mode(struct rtpcs_serdes *sds,
        if (ret)
                return ret;
 
-       switch (hw_mode) {
-       case RTPCS_SDS_MODE_OFF:
-               mode_val = 0x3f;
-               break;
-
-       case RTPCS_SDS_MODE_SGMII:
-               mode_val = 0x5;
-               break;
-
-       case RTPCS_SDS_MODE_1000BASEX:
-               /* serdes mode FIBER1G */
-               mode_val = 0x9;
-               break;
-
-       case RTPCS_SDS_MODE_2500BASEX:
-               /* available SDK code doesn't have this value. based on brute-forcing
-                * the SerDes mode register field until the link is working
-                */
-               mode_val = 0x2d;
-               break;
-
-       case RTPCS_SDS_MODE_10GBASER:
-               mode_val = 0x35;
-               break;
-/*      case MII_10GR1000BX_AUTO:
-                mode_val = 0x39;
-                break; */
-
-       case RTPCS_SDS_MODE_USXGMII_10GSXGMII:
-       case RTPCS_SDS_MODE_USXGMII_10GDXGMII:
-       case RTPCS_SDS_MODE_USXGMII_10GQXGMII:
-       case RTPCS_SDS_MODE_USXGMII_5GSXGMII:
-       case RTPCS_SDS_MODE_USXGMII_5GDXGMII:
-       case RTPCS_SDS_MODE_USXGMII_2_5GSXGMII:
-               mode_val = 0x1b;
-               break;
-       case RTPCS_SDS_MODE_HISGMII:
-               mode_val = 0x25;
-               break;
-       default:
-               return -ENOTSUPP;
-       }
-
-       pr_info("%s writing analog SerDes Mode value %02x\n", __func__, mode_val);
-       return rtpcs_sds_write_bits(sds, 0x1f, 0x9, 11, 6, mode_val);
+       return rtpcs_93xx_sds_set_ip_mode(sds, hw_mode);
 }
 
 static int rtpcs_931x_sds_set_mode(struct rtpcs_serdes *sds,
@@ -3284,7 +3257,7 @@ static int rtpcs_931x_sds_set_mode(struct rtpcs_serdes *sds,
        if (hw_mode == RTPCS_SDS_MODE_XSGMII)
                return rtpcs_93xx_sds_set_mac_mode(sds, hw_mode);
 
-       ret = rtpcs_931x_sds_set_ip_mode(sds, hw_mode);
+       ret = rtpcs_931x_sds_apply_ip_mode(sds, hw_mode);
        if (ret)
                return ret;