RE: [PATCH net-next 2/3] net: phy: realtek: apply SerDes lane polarity on RTL8261C/D

[Simon Polack]

Resending text-encoded:

Hi Javen,

Thanks for reviewing.

On scope: this patch only affects the parts the driver binds, PHY IDs
0x001cc898, 0x001cc899 and 0x001cc89a. The RTL8261N (0x001ccaf3) isn't
supported upstream and isn't touched.

Understood on the revision dependence. The problem remains that the
W1700K routes the lanes inverted, and without programming the polarity
its 10G ports never link. How should such a board be supported?

If there is a supported way to set lane polarity - a register the
driver writes and the PHY firmware acts on, so the control flow stays
with the hardware - I'd gladly use that instead of the direct SerDes
access. Otherwise the alternative I see is restricting the patch to
the verified variant (0x001cc899, sub-model 0x00) and leaving every
other part untouched.

Simon

Am 28. September 2026 09:17:00 MESZ schrieb Javen <javen_xu@xxxxxxxxxxxxxx>:
>>
>>Boards such as the Gemtek W1700K (Airoha AN7581) route the USXGMII lanes
>>between the SoC and the PHY inverted and describe that in the device tree with
>>the generic tx-polarity/rx-polarity properties.
>>Nothing in the RTL8261C/D driver reads them and the Airoha PCS has no
>>polarity handling of its own, so the inversion is never programmed.
>>The copper side negotiates normally while the SerDes never trains and the
>>netdev stays NO-CARRIER.
>>
>>Lane polarity on this part lives behind the same VEND1 SerDes command
>>window the RTL822x code already uses for in-band autoneg, in SerDes registers
>>0x0000 (bits 9:8) and 0x00c2 (bits 14:13). The register at
>>VEND1 0xc1 that some other Realtek 10G PHYs use for the same purpose is not
>>implemented on this die and reads back as zero after a write.
>>
>>Add a read side and a read-modify-write helper to the command window,
>>move rtl822x_serdes_write() up next to them so all three are visible from the
>>RTL8261 code, apply the polarity from config_init() and select
>>PHY_COMMON_PROPS for the property helpers. Only lanes actually described
>>in the firmware node are touched; a board without the properties keeps
>>whatever the boot loader and the PHY firmware left in place.
>>
>>The bit assignment has been verified on an RTL8261CE (sub-model 0x00, PHY
>>ID 0x001cc899): on the W1700K, SerDes register 0x0000 goes from
>>0x1403 to 0x1703 and 0x00c2 from 0x0000 to 0x6000, after which VEND1
>>0x758d reports the SerDes linked (0x0010 -> 0x001e) and the link comes up at
>>1G and 10G and passes traffic. The RTL8261C, RTL8261D and RTL8261D_VM
>>share rtl8261x_config_init() and get the same treatment, but have not been
>>tested with inverted lanes.
>>
>>Assisted-by: LLM
>>Signed-off-by: Simon Polack <spolack+git@xxxxxxxxxxx>
>>---
>> drivers/net/phy/realtek/Kconfig        |   1 +
>> drivers/net/phy/realtek/realtek_main.c | 174 +++++++++++++++++++++----
>> 2 files changed, 148 insertions(+), 27 deletions(-)
>>
>>diff --git a/drivers/net/phy/realtek/Kconfig b/drivers/net/phy/realtek/Kconfig
>>index a741b34d193e..a9272aebe26d 100644
>>--- a/drivers/net/phy/realtek/Kconfig
>>+++ b/drivers/net/phy/realtek/Kconfig
>>@@ -1,6 +1,7 @@
>> # SPDX-License-Identifier: GPL-2.0-only  config REALTEK_PHY
>>        tristate "Realtek PHYs"
>>+       select PHY_COMMON_PROPS
>>        select PHY_PACKAGE
>>        help
>>          Currently supports RTL821x/RTL822x and fast ethernet PHYs diff --git
>>a/drivers/net/phy/realtek/realtek_main.c
>>b/drivers/net/phy/realtek/realtek_main.c
>>index 1e670638dd1c..04d397f0a2d8 100644
>>--- a/drivers/net/phy/realtek/realtek_main.c
>>+++ b/drivers/net/phy/realtek/realtek_main.c
>>@@ -13,9 +13,11 @@
>> #include <linux/firmware.h>
>> #include <linux/of.h>
>> #include <linux/phy.h>
>>+#include <linux/phy/phy-common-props.h>
>> #include <linux/pm_wakeirq.h>
>> #include <linux/netdevice.h>
>> #include <linux/module.h>
>>+#include <linux/property.h>
>> #include <linux/delay.h>
>> #include <linux/clk.h>
>> #include <linux/string_choices.h>
>>@@ -164,6 +166,7 @@
>> #define   RTL822X_VND1_SERDES_INBAND_DISABLE   0x71d0
>> #define   RTL822X_VND1_SERDES_INBAND_ENABLE    0x70d0
>> #define RTL822X_VND1_SERDES_DATA               0x7589
>>+#define RTL822X_VND1_SERDES_RDATA              0x758a
>>
>> #define RTL822X_VND2_TO_PAGE(reg)              ((reg) >> 4)
>> #define RTL822X_VND2_TO_PAGE_REG(reg)          (16 + (((reg) & GENMASK(3,
>>0)) >> 1))
>>@@ -271,6 +274,18 @@
>> #define RTL8261X_INT_ALDPS_CHG         BIT(9)
>> #define RTL8261X_INT_JABBER            BIT(10)
>>
>>+/* SerDes lane polarity, behind the VEND1 SerDes command window. This
>>+is not
>>+ * the global inversion bit that other Realtek 10G PHYs use; the bit
>>+ * assignment below has only been verified on an RTL8261CE reporting
>>+PHY ID
>>+ * 0x001cc899.
>>+ */
>>+#define RTL8261X_SERDES_POL_REG0       0x0000
>>+#define  RTL8261X_SERDES_POL_REG0_TX   BIT(8)
>>+#define  RTL8261X_SERDES_POL_REG0_RX   BIT(9)
>>+#define RTL8261X_SERDES_POL_REGC2      0x00c2
>>+#define  RTL8261X_SERDES_POL_REGC2_TX  BIT(14) #define
>>+RTL8261X_SERDES_POL_REGC2_RX  BIT(13)
>>+
>
>Hi,
>
>Thanks for submitting this patch.
>
>The serdes registers modified in this patch are not unified across RTL8261 family. This patch will lead to undefined behaviors on other IC. And operating these SerDes registers requires a specific hardware control flow. The direct manipulation used in this patch will introduce unknown risk.
>
>Therefore, we kindly suggest drop this patch.
>
>BRS,
>Javen
>