* [PATCH net-next v2] net: phy: Add JLSemi JL21xx Gigabit Ethernet PHY driver
@ 2026-10-17 16:53 Pufan Jin
2026-10-18 17:03 ` netdev-bot+sashiko
0 siblings, 1 reply; 2+ messages in thread
From: Pufan Jin @ 2026-10-17 16:53 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Heiner Kallweit,
Jakub Kicinski, Paolo Abeni, Russell King
Cc: linux-kernel, netdev
Add a new PHY driver for JLSemi (Jinglue Semiconductor (Shanghai)
Co., Ltd.) Gigabit Ethernet PHYs, specifically supporting the JL2101,
JL2121, and generic JL21xx models.
The driver implements:
- Paged register access via register 0x1f
- RGMII TX and RX internal delay configuration with Device Tree
support via phy_get_internal_delay()
- Link downshift and resolved speed, duplex, and master/slave mode
reporting from vendor-specific status registers
- Interrupt handling for link state change and PME wakeup events
- Hardware LED control via the Linux LED class / netdev triggers
- Wake-on-LAN (WoL) with magic packet matching
- Autonomous EEE disable to prevent interoperability packet drops
- Power management (suspend / resume)
Link: https://www.jlsemi.com/
Signed-off-by: Pufan Jin <cyan2254650260@gmail.com>
---
Changes in v2:
- Drop redundant phy_interface_is_rgmii() check in jlsemi_config_init()
(Andrew Lunn)
- Change speed check in jlsemi_read_status() to '== SPEED_1000' since
JL21xx only supports up to 1000M and 100BASE-TX does not use Master/Slave
clock resolution (Andrew Lunn)
- Check device_can_wakeup() in get_wol/set_wol, and configure wakeup
capability and wake IRQ in probe() when 'wakeup-source' is present
(Andrew Lunn)
- Drop redundant .features = PHY_GBIT_FEATURES (Andrew Lunn)
- Use PHY_ID_MATCH_VENDOR() for generic driver and mdio table (Andrew Lunn)
MAINTAINERS | 6 +
drivers/net/phy/Kconfig | 9 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/jlsemi.c | 555 +++++++++++++++++++++++++++++++++++++++
4 files changed, 571 insertions(+)
create mode 100644 drivers/net/phy/jlsemi.c
diff --git a/MAINTAINERS b/MAINTAINERS
index f53047aa8..4f738a573 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13976,6 +13976,12 @@ T: git https://github.com/kleikamp/linux-shaggy.git
F: Documentation/admin-guide/jfs.rst
F: fs/jfs/
+JLSEMI PHY DRIVER
+M: Pufan Jin <cyan2254650260@gmail.com>
+L: netdev@vger.kernel.org
+S: Maintained
+F: drivers/net/phy/jlsemi.c
+
JME NETWORK DRIVER
M: Guo-Fu Tseng <cooldavid@cooldavid.org>
L: netdev@vger.kernel.org
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index b4ef927fd..535a48084 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -270,6 +270,15 @@ config INTEL_XWAY_PHY
PEF 7061, PEF 7071 and PEF 7072 or integrated into the Intel
SoCs xRX200, xRX300, xRX330, xRX350 and xRX550.
+config JLSEMI_PHY
+ tristate "JLSemi Ethernet PHYs"
+ help
+ Supports the JLSemi Gigabit Ethernet PHYs, including the
+ JL2121 and JL2101 models found on embedded system carrier boards
+ and single-board computers.
+
+ If unsure, say N.
+
config LSI_ET1011C_PHY
tristate "LSI ET1011C PHY"
help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 25c4a3c24..2d7758e86 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_DP83TG720_PHY) += dp83tg720.o
obj-$(CONFIG_FIXED_PHY) += fixed_phy.o
obj-$(CONFIG_ICPLUS_PHY) += icplus.o
obj-$(CONFIG_INTEL_XWAY_PHY) += intel-xway.o
+obj-$(CONFIG_JLSEMI_PHY) += jlsemi.o
obj-$(CONFIG_LSI_ET1011C_PHY) += et1011c.o
obj-$(CONFIG_LXT_PHY) += lxt.o
obj-$(CONFIG_MARVELL_10G_PHY) += marvell10g.o
diff --git a/drivers/net/phy/jlsemi.c b/drivers/net/phy/jlsemi.c
new file mode 100644
index 000000000..c83739ad1
--- /dev/null
+++ b/drivers/net/phy/jlsemi.c
@@ -0,0 +1,555 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for JLSemi Gigabit Ethernet PHYs (JL2121 / JL2101)
+ *
+ * Copyright (C) 2026 Pufan Jin <cyan2254650260@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/ethtool.h>
+#include <linux/ethtool_netlink.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/phy.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/property.h>
+
+#define PHY_ID_JL2101 0x937c4032
+#define PHY_ID_JL2121 0x937c4030
+#define PHY_ID_JLSEMI 0x937c4000
+#define JLSEMI_PAGE_SELECT 0x1f
+
+/* Status page and register */
+#define JLSEMI_STATUS_PAGE 0xa43
+#define JLSEMI_PHYSR 0x1a
+#define JLSEMI_PHYSR_DUPLEX BIT(3)
+#define JLSEMI_PHYSR_SPEED_MASK GENMASK(5, 4)
+#define JLSEMI_PHYSR_SPEED_1000 0x20
+#define JLSEMI_PHYSR_SPEED_100 0x10
+#define JLSEMI_PHYSR_SPEED_10 0x00
+#define JLSEMI_PHYSR_MASTER BIT(11)
+
+/* RGMII delay page and registers */
+#define JLSEMI_RGMII_PAGE 0xd08
+#define JLSEMI_TXCR 0x11
+#define JLSEMI_TX_DELAY BIT(8)
+#define JLSEMI_RXCR 0x15
+#define JLSEMI_RX_DELAY BIT(3)
+
+/* Power and EEE configuration */
+#define JLSEMI_PHYCR2 0x19
+#define JLSEMI_PHYCR2_PHY_EEE_ENABLE BIT(5)
+
+/* Interrupt page and registers */
+#define JLSEMI_INSR 0x1d
+#define JLSEMI_INER_PAGE 0xa42
+#define JLSEMI_INER 0x12
+#define JLSEMI_INER_LINK_STATUS BIT(4)
+#define JLSEMI_INER_PME BIT(7)
+
+/* Wake-on-LAN settings */
+#define JLSEMI_WOL_PAGE 0xd8a
+#define JLSEMI_WOL_SETTINGS_EVENTS 16
+#define JLSEMI_WOL_EVENT_MAGIC BIT(12)
+#define JLSEMI_WOL_RST_RMSQ 17
+#define JLSEMI_WOL_RG_RSTB BIT(15)
+#define JLSEMI_WOL_RMSQ 0x1fff
+
+/* Physical MAC address for WoL */
+#define JLSEMI_PHYSICAL_ADDR_PAGE 0xd8c
+#define JLSEMI_PHYSICAL_ADDR_WORD0 16
+#define JLSEMI_PHYSICAL_ADDR_WORD1 17
+#define JLSEMI_PHYSICAL_ADDR_WORD2 18
+
+/* LED control */
+#define JLSEMI_LED_COUNT 3
+#define JLSEMI_LEDCR_PAGE 0xd04
+#define JLSEMI_LEDCR 0x10
+#define JLSEMI_LEDCR_MODE BIT(15)
+#define JLSEMI_LEDCR_ACT_TXRX BIT(4)
+#define JLSEMI_LEDCR_LINK_1000 BIT(3)
+#define JLSEMI_LEDCR_LINK_100 BIT(1)
+#define JLSEMI_LEDCR_LINK_10 BIT(0)
+#define JLSEMI_LEDCR_MASK GENMASK(4, 0)
+#define JLSEMI_LEDCR_SHIFT 5
+
+struct jlsemi_priv {
+ u16 iner;
+};
+
+static int jlsemi_read_page(struct phy_device *phydev)
+{
+ return __phy_read(phydev, JLSEMI_PAGE_SELECT);
+}
+
+static int jlsemi_write_page(struct phy_device *phydev, int page)
+{
+ return __phy_write(phydev, JLSEMI_PAGE_SELECT, page);
+}
+
+static int jlsemi_probe(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ struct jlsemi_priv *priv;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ phydev->priv = priv;
+
+ if (device_property_read_bool(dev, "wakeup-source") &&
+ phy_interrupt_is_valid(phydev)) {
+ device_set_wakeup_capable(dev, true);
+ devm_pm_set_wake_irq(dev, phydev->irq);
+ }
+ return 0;
+}
+
+static int jlsemi_config_rgmii_delay(struct phy_device *phydev)
+{
+ u16 val_txdly = 0, val_rxdly = 0;
+ s32 rx_delay = -1, tx_delay = -1;
+ int ret;
+
+ if (!phy_interface_is_rgmii(phydev))
+ return 0;
+
+ if (phydev->interface != PHY_INTERFACE_MODE_RGMII_TXID)
+ rx_delay = phy_get_internal_delay(phydev, NULL, 0, true);
+
+ if (phydev->interface != PHY_INTERFACE_MODE_RGMII_RXID)
+ tx_delay = phy_get_internal_delay(phydev, NULL, 0, false);
+
+ switch (phydev->interface) {
+ case PHY_INTERFACE_MODE_RGMII:
+ val_txdly = 0;
+ val_rxdly = 0;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ val_txdly = JLSEMI_TX_DELAY;
+ val_rxdly = JLSEMI_RX_DELAY;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ val_txdly = 0;
+ val_rxdly = JLSEMI_RX_DELAY;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ val_txdly = JLSEMI_TX_DELAY;
+ val_rxdly = 0;
+ break;
+ default:
+ return 0;
+ }
+
+ if (rx_delay >= 0)
+ val_rxdly = rx_delay > 0 ? JLSEMI_RX_DELAY : 0;
+ if (tx_delay >= 0)
+ val_txdly = tx_delay > 0 ? JLSEMI_TX_DELAY : 0;
+
+ ret = phy_modify_paged_changed(phydev, JLSEMI_RGMII_PAGE,
+ JLSEMI_TXCR, JLSEMI_TX_DELAY, val_txdly);
+ if (ret < 0)
+ return ret;
+
+ return phy_modify_paged_changed(phydev, JLSEMI_RGMII_PAGE,
+ JLSEMI_RXCR, JLSEMI_RX_DELAY,
+ val_rxdly);
+}
+
+static int jlsemi_disable_autonomous_eee(struct phy_device *phydev)
+{
+ return phy_modify(phydev, JLSEMI_PHYCR2,
+ JLSEMI_PHYCR2_PHY_EEE_ENABLE, 0);
+}
+
+static int jlsemi_config_init(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = jlsemi_disable_autonomous_eee(phydev);
+ if (ret < 0)
+ return ret;
+
+ return jlsemi_config_rgmii_delay(phydev);
+}
+
+static int jlsemi_ack_interrupt(struct phy_device *phydev)
+{
+ int err = phy_read(phydev, JLSEMI_INSR);
+
+ return (err < 0) ? err : 0;
+}
+
+static int jlsemi_config_intr(struct phy_device *phydev)
+{
+ struct jlsemi_priv *priv = phydev->priv;
+ int err;
+ u16 val;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ err = jlsemi_ack_interrupt(phydev);
+ if (err)
+ return err;
+
+ val = JLSEMI_INER_LINK_STATUS;
+ err = phy_write_paged(phydev, JLSEMI_INER_PAGE, JLSEMI_INER, val);
+ if (!err)
+ priv->iner = val;
+ } else {
+ val = 0;
+ err = phy_write_paged(phydev, JLSEMI_INER_PAGE, JLSEMI_INER, val);
+ if (!err)
+ priv->iner = val;
+
+ err = jlsemi_ack_interrupt(phydev);
+ }
+
+ return err;
+}
+
+static irqreturn_t jlsemi_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ irq_status = phy_read(phydev, JLSEMI_INSR);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (irq_status & JLSEMI_INER_LINK_STATUS) {
+ phy_trigger_machine(phydev);
+ return IRQ_HANDLED;
+ }
+
+ if (irq_status & JLSEMI_INER_PME) {
+ pm_wakeup_event(&phydev->mdio.dev, 0);
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
+static int jlsemi_read_status(struct phy_device *phydev)
+{
+ int err, val;
+
+ err = genphy_read_status(phydev);
+ if (err < 0 || !phydev->link)
+ return err;
+
+ val = phy_read_paged(phydev, JLSEMI_STATUS_PAGE, JLSEMI_PHYSR);
+ if (val < 0)
+ return val;
+
+ if (val & JLSEMI_PHYSR_DUPLEX)
+ phydev->duplex = DUPLEX_FULL;
+ else
+ phydev->duplex = DUPLEX_HALF;
+
+ switch (val & JLSEMI_PHYSR_SPEED_MASK) {
+ case JLSEMI_PHYSR_SPEED_1000:
+ phydev->speed = SPEED_1000;
+ break;
+ case JLSEMI_PHYSR_SPEED_100:
+ phydev->speed = SPEED_100;
+ break;
+ case JLSEMI_PHYSR_SPEED_10:
+ phydev->speed = SPEED_10;
+ break;
+ default:
+ phydev->speed = SPEED_UNKNOWN;
+ break;
+ }
+
+ if (phydev->speed == SPEED_1000) {
+ if (val & JLSEMI_PHYSR_MASTER)
+ phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
+ else
+ phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
+ } else {
+ phydev->master_slave_state = MASTER_SLAVE_STATE_UNSUPPORTED;
+ }
+
+ return 0;
+}
+
+static void jlsemi_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
+{
+ int val;
+
+ if (!device_can_wakeup(&phydev->mdio.dev)) {
+ wol->supported = 0;
+ wol->wolopts = 0;
+ return;
+ }
+
+ wol->supported = WAKE_MAGIC;
+ wol->wolopts = 0;
+
+ val = phy_read_paged(phydev, JLSEMI_WOL_PAGE, JLSEMI_WOL_SETTINGS_EVENTS);
+ if (val < 0)
+ return;
+
+ if (val & JLSEMI_WOL_EVENT_MAGIC)
+ wol->wolopts |= WAKE_MAGIC;
+}
+
+static int jlsemi_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
+{
+ const u8 *mac_addr;
+ int oldpage, ret;
+
+ if (!device_can_wakeup(&phydev->mdio.dev))
+ return -EOPNOTSUPP;
+
+ if (wol->wolopts & ~WAKE_MAGIC)
+ return -EINVAL;
+
+ if (wol->wolopts & WAKE_MAGIC) {
+ if (!phydev->attached_dev)
+ return -EINVAL;
+
+ mac_addr = phydev->attached_dev->dev_addr;
+
+ oldpage = phy_select_page(phydev, JLSEMI_PHYSICAL_ADDR_PAGE);
+ if (oldpage < 0)
+ return oldpage;
+
+ __phy_write(phydev, JLSEMI_PHYSICAL_ADDR_WORD0,
+ mac_addr[1] << 8 | mac_addr[0]);
+ __phy_write(phydev, JLSEMI_PHYSICAL_ADDR_WORD1,
+ mac_addr[3] << 8 | mac_addr[2]);
+ __phy_write(phydev, JLSEMI_PHYSICAL_ADDR_WORD2,
+ mac_addr[5] << 8 | mac_addr[4]);
+
+ ret = __phy_write(phydev, JLSEMI_PAGE_SELECT, JLSEMI_WOL_PAGE);
+ if (ret < 0)
+ goto restore;
+
+ __phy_write(phydev, JLSEMI_WOL_SETTINGS_EVENTS,
+ JLSEMI_WOL_EVENT_MAGIC);
+ __phy_write(phydev, JLSEMI_WOL_RST_RMSQ,
+ JLSEMI_WOL_RMSQ);
+restore:
+ phy_restore_page(phydev, oldpage, ret);
+ } else {
+ oldpage = phy_select_page(phydev, JLSEMI_WOL_PAGE);
+ if (oldpage < 0)
+ return oldpage;
+
+ __phy_write(phydev, JLSEMI_WOL_SETTINGS_EVENTS, 0);
+ __phy_clear_bits(phydev, JLSEMI_WOL_RST_RMSQ,
+ JLSEMI_WOL_RG_RSTB);
+
+ phy_restore_page(phydev, oldpage, 0);
+ }
+
+ device_set_wakeup_enable(&phydev->mdio.dev, !!(wol->wolopts & WAKE_MAGIC));
+
+ return 0;
+}
+
+static int jlsemi_suspend(struct phy_device *phydev)
+{
+ int ret;
+
+ if (device_may_wakeup(&phydev->mdio.dev)) {
+ ret = phy_write_paged(phydev, JLSEMI_INER_PAGE, JLSEMI_INER,
+ JLSEMI_INER_PME);
+ if (ret < 0)
+ return ret;
+
+ jlsemi_ack_interrupt(phydev);
+ return 0;
+ }
+
+ return genphy_suspend(phydev);
+}
+
+static int jlsemi_resume(struct phy_device *phydev)
+{
+ struct jlsemi_priv *priv = phydev->priv;
+ int ret;
+
+ ret = genphy_resume(phydev);
+ if (ret < 0)
+ return ret;
+
+ return phy_write_paged(phydev, JLSEMI_INER_PAGE, JLSEMI_INER,
+ priv ? priv->iner : 0);
+}
+
+static int jlsemi_led_hw_is_supported(struct phy_device *phydev, u8 index,
+ unsigned long rules)
+{
+ const unsigned long mask = BIT(TRIGGER_NETDEV_LINK) |
+ BIT(TRIGGER_NETDEV_LINK_10) |
+ BIT(TRIGGER_NETDEV_LINK_100) |
+ BIT(TRIGGER_NETDEV_LINK_1000) |
+ BIT(TRIGGER_NETDEV_RX) |
+ BIT(TRIGGER_NETDEV_TX);
+
+ if (index >= JLSEMI_LED_COUNT)
+ return -EINVAL;
+
+ if (rules & ~mask)
+ return -EOPNOTSUPP;
+
+ if (!(rules & BIT(TRIGGER_NETDEV_RX)) ^ !(rules & BIT(TRIGGER_NETDEV_TX)))
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
+static int jlsemi_led_hw_control_get(struct phy_device *phydev, u8 index,
+ unsigned long *rules)
+{
+ int val;
+
+ if (index >= JLSEMI_LED_COUNT)
+ return -EINVAL;
+
+ val = phy_read_paged(phydev, JLSEMI_LEDCR_PAGE, JLSEMI_LEDCR);
+ if (val < 0)
+ return val;
+
+ val >>= JLSEMI_LEDCR_SHIFT * index;
+ val &= JLSEMI_LEDCR_MASK;
+
+ if (val & JLSEMI_LEDCR_LINK_10)
+ __set_bit(TRIGGER_NETDEV_LINK_10, rules);
+
+ if (val & JLSEMI_LEDCR_LINK_100)
+ __set_bit(TRIGGER_NETDEV_LINK_100, rules);
+
+ if (val & JLSEMI_LEDCR_LINK_1000)
+ __set_bit(TRIGGER_NETDEV_LINK_1000, rules);
+
+ if ((val & JLSEMI_LEDCR_LINK_10) &&
+ (val & JLSEMI_LEDCR_LINK_100) &&
+ (val & JLSEMI_LEDCR_LINK_1000)) {
+ __set_bit(TRIGGER_NETDEV_LINK, rules);
+ }
+
+ if (val & JLSEMI_LEDCR_ACT_TXRX) {
+ __set_bit(TRIGGER_NETDEV_RX, rules);
+ __set_bit(TRIGGER_NETDEV_TX, rules);
+ }
+
+ return 0;
+}
+
+static int jlsemi_led_hw_control_set(struct phy_device *phydev, u8 index,
+ unsigned long rules)
+{
+ const u16 mask = JLSEMI_LEDCR_MASK << (JLSEMI_LEDCR_SHIFT * index);
+ u16 reg = 0;
+
+ if (index >= JLSEMI_LED_COUNT)
+ return -EINVAL;
+
+ if (test_bit(TRIGGER_NETDEV_LINK, &rules) ||
+ test_bit(TRIGGER_NETDEV_LINK_10, &rules)) {
+ reg |= JLSEMI_LEDCR_LINK_10;
+ }
+
+ if (test_bit(TRIGGER_NETDEV_LINK, &rules) ||
+ test_bit(TRIGGER_NETDEV_LINK_100, &rules)) {
+ reg |= JLSEMI_LEDCR_LINK_100;
+ }
+
+ if (test_bit(TRIGGER_NETDEV_LINK, &rules) ||
+ test_bit(TRIGGER_NETDEV_LINK_1000, &rules)) {
+ reg |= JLSEMI_LEDCR_LINK_1000;
+ }
+
+ if (test_bit(TRIGGER_NETDEV_RX, &rules) ||
+ test_bit(TRIGGER_NETDEV_TX, &rules)) {
+ reg |= JLSEMI_LEDCR_ACT_TXRX;
+ }
+
+ reg <<= JLSEMI_LEDCR_SHIFT * index;
+ reg |= JLSEMI_LEDCR_MODE;
+
+ return phy_modify_paged(phydev, JLSEMI_LEDCR_PAGE, JLSEMI_LEDCR,
+ mask, reg);
+}
+
+static struct phy_driver jlsemi_drivers[] = {
+ {
+ PHY_ID_MATCH_EXACT(PHY_ID_JL2101),
+ .name = "JLSemi JL2101 Gigabit Ethernet",
+ .flags = PHY_ALWAYS_CALL_SUSPEND,
+ .probe = jlsemi_probe,
+ .soft_reset = genphy_soft_reset,
+ .config_init = jlsemi_config_init,
+ .read_status = jlsemi_read_status,
+ .config_intr = jlsemi_config_intr,
+ .handle_interrupt = jlsemi_handle_interrupt,
+ .set_wol = jlsemi_set_wol,
+ .get_wol = jlsemi_get_wol,
+ .suspend = jlsemi_suspend,
+ .resume = jlsemi_resume,
+ .read_page = jlsemi_read_page,
+ .write_page = jlsemi_write_page,
+ .led_hw_is_supported = jlsemi_led_hw_is_supported,
+ .led_hw_control_get = jlsemi_led_hw_control_get,
+ .led_hw_control_set = jlsemi_led_hw_control_set,
+ .disable_autonomous_eee = jlsemi_disable_autonomous_eee,
+ },
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_JL2121),
+ .name = "JLSemi JL2121 Gigabit Ethernet",
+ .flags = PHY_ALWAYS_CALL_SUSPEND,
+ .probe = jlsemi_probe,
+ .soft_reset = genphy_soft_reset,
+ .config_init = jlsemi_config_init,
+ .read_status = jlsemi_read_status,
+ .config_intr = jlsemi_config_intr,
+ .handle_interrupt = jlsemi_handle_interrupt,
+ .set_wol = jlsemi_set_wol,
+ .get_wol = jlsemi_get_wol,
+ .suspend = jlsemi_suspend,
+ .resume = jlsemi_resume,
+ .read_page = jlsemi_read_page,
+ .write_page = jlsemi_write_page,
+ .led_hw_is_supported = jlsemi_led_hw_is_supported,
+ .led_hw_control_get = jlsemi_led_hw_control_get,
+ .led_hw_control_set = jlsemi_led_hw_control_set,
+ .disable_autonomous_eee = jlsemi_disable_autonomous_eee,
+ },
+ {
+ PHY_ID_MATCH_VENDOR(PHY_ID_JLSEMI),
+ .name = "JLSemi Generic Gigabit Ethernet",
+ .flags = PHY_ALWAYS_CALL_SUSPEND,
+ .probe = jlsemi_probe,
+ .soft_reset = genphy_soft_reset,
+ .config_init = jlsemi_config_init,
+ .read_status = jlsemi_read_status,
+ .config_intr = jlsemi_config_intr,
+ .handle_interrupt = jlsemi_handle_interrupt,
+ .suspend = jlsemi_suspend,
+ .resume = jlsemi_resume,
+ .read_page = jlsemi_read_page,
+ .write_page = jlsemi_write_page,
+ .disable_autonomous_eee = jlsemi_disable_autonomous_eee,
+ },
+};
+
+module_phy_driver(jlsemi_drivers);
+
+static const struct mdio_device_id __maybe_unused jlsemi_tbl[] = {
+ { PHY_ID_MATCH_EXACT(PHY_ID_JL2101) },
+ { PHY_ID_MATCH_MODEL(PHY_ID_JL2121) },
+ { PHY_ID_MATCH_VENDOR(PHY_ID_JLSEMI) },
+ { /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(mdio, jlsemi_tbl);
+
+MODULE_DESCRIPTION("JLSemi Ethernet PHY driver");
+MODULE_AUTHOR("Pufan Jin <cyan2254650260@gmail.com>");
+MODULE_LICENSE("GPL");
--
2.56.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH net-next v2] net: phy: Add JLSemi JL21xx Gigabit Ethernet PHY driver
2026-10-17 16:53 [PATCH net-next v2] net: phy: Add JLSemi JL21xx Gigabit Ethernet PHY driver Pufan Jin
@ 2026-10-18 17:03 ` netdev-bot+sashiko
0 siblings, 0 replies; 2+ messages in thread
From: netdev-bot+sashiko @ 2026-10-18 17:03 UTC (permalink / raw)
To: cyan2254650260
Cc: andrew, davem, edumazet, hkallweit1, kuba, pabeni, linux,
linux-kernel, netdev
Thank you for your contribution! Sashiko AI review found 12 potential
issue(s) to consider.
Critical: 0 · High: 3 · Medium: 6 · Low: 3
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] WoL support is incomplete: the WoL engine is left in reset and
is never released, and a WoL configuration left by firmware is never…
- [High] jlsemi_set_wol() leaks the MDIO bus mutex when page selection
fails.
- [High] MAC-based Wake-on-LAN breaks: every jlsemi entry sets
PHY_ALWAYS_CALL_SUSPEND, and jlsemi_suspend() checks only…
- [Medium] The new driver is presented as original work.
- [Medium] jlsemi_config_rgmii_delay() calls
phy_get_internal_delay(phydev, NULL, 0, ...) with no delay table.
- [Medium] The generic PHY_ID_MATCH_VENDOR(PHY_ID_JLSEMI) entry uses
jlsemi_probe() and jlsemi_suspend(), but it has no .get_wol/.set_wol.
- [Medium] jlsemi_led_hw_control_get() returns a set of netdev trigger
rules that the netdev trigger itself rejects.
- [Medium] jlsemi_set_wol() reports success even when programming fails.
- [Medium] net: phy: jlsemi: jlsemi_config_init() unconditionally
disables autonomous EEE
- [Low] On the interrupt-disable path of jlsemi_config_intr(), `err =
phy_write_paged(..., JLSEMI_INER, 0)` is followed unconditionally by…
- [Low] jlsemi_handle_interrupt() reads INSR once and returns IRQ_HANDLED
as soon as it sees JLSEMI_INER_LINK_STATUS, without testing…
- [Low] jlsemi_probe() calls device_set_wakeup_capable(dev, true) and
then devm_pm_set_wake_irq(dev, phydev->irq) without checking the…
--
net: phy: Add JLSemi JL21xx Gigabit Ethernet PHY driver
This commit adds a new phylib driver for the JLSemi JL2101, JL2121 and
generic JL21xx Gigabit Ethernet PHYs. It implements paged register access,
RGMII delay configuration, vendor status reporting, interrupts, LED
hardware control, magic packet Wake-on-LAN and suspend/resume.
Link: https://www.jlsemi.com/
> diff --git a/drivers/net/phy/jlsemi.c b/drivers/net/phy/jlsemi.c
> new file mode 100644
> index 0000000000000..c83739ad182b4
> --- /dev/null
> +++ b/drivers/net/phy/jlsemi.c
> @@ -0,0 +1,555 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for JLSemi Gigabit Ethernet PHYs (JL2121 / JL2101)
> + *
> + * Copyright (C) 2026 Pufan Jin <cyan2254650260@gmail.com>
> + */
[Severity: Medium]
Most of this driver looks like a copy of the RTL8211F support in
drivers/net/phy/realtek/realtek_main.c, with the RTL8211F_ prefix renamed
to JLSEMI_.
The register map is the same: 0xa43/0x1a, 0xd08/0x11/0x15, PHYCR2 0x19
bit 5, 0xa42/0x12, 0xd04/0x10, 0xd8a/16-17 and 0xd8c/16-18.
jlsemi_led_hw_control_get() and jlsemi_led_hw_control_set() are identical
to rtl8211f_led_hw_control_get() and rtl8211f_led_hw_control_set(). The
WoL, config_intr, handle_interrupt and disable_autonomous_eee code follow
the same pattern.
Neither the commit message nor this header says that JL21xx is
register-compatible with RTL8211F, and neither credits the original code.
Would it be better to add these IDs to the Realtek driver instead? If a
separate driver is preferred, could the commit message explain why, and
credit the source?
As a separate copy, fixes to the RTL8211F code will not reach this driver.
Some of the comments below look like exactly that.
[ ... ]
> +static int jlsemi_probe(struct phy_device *phydev)
> +{
[ ... ]
> + if (device_property_read_bool(dev, "wakeup-source") &&
> + phy_interrupt_is_valid(phydev)) {
> + device_set_wakeup_capable(dev, true);
> + devm_pm_set_wake_irq(dev, phydev->irq);
> + }
> + return 0;
> +}
[Severity: Low]
Should the return value of devm_pm_set_wake_irq() be checked here?
dev_pm_set_wake_irq() can fail, for example with -ENOMEM. The device has
already been marked wakeup capable at that point. jlsemi_get_wol() would
still advertise WAKE_MAGIC and jlsemi_set_wol() would still succeed.
jlsemi_suspend() would then keep the PHY powered with PME armed, but the
IRQ was never set up as a wake source.
> +static int jlsemi_config_rgmii_delay(struct phy_device *phydev)
> +{
[ ... ]
> + if (phydev->interface != PHY_INTERFACE_MODE_RGMII_TXID)
> + rx_delay = phy_get_internal_delay(phydev, NULL, 0, true);
> +
> + if (phydev->interface != PHY_INTERFACE_MODE_RGMII_RXID)
> + tx_delay = phy_get_internal_delay(phydev, NULL, 0, false);
> +
[ ... ]
> + if (rx_delay >= 0)
> + val_rxdly = rx_delay > 0 ? JLSEMI_RX_DELAY : 0;
> + if (tx_delay >= 0)
> + val_txdly = tx_delay > 0 ? JLSEMI_TX_DELAY : 0;
[Severity: Medium]
With a NULL table and size 0, phy_get_internal_delay() returns the raw
picosecond value from rx-internal-delay-ps or tx-internal-delay-ps. This
code reduces that value to a single on/off bit.
A DT value such as rx-internal-delay-ps = <500> would therefore select the
fixed hardware delay (around 2ns), with no warning and no -EINVAL. Is that
intended?
The handling also depends on phy-mode in an inconsistent way:
phy-mode = "rgmii": either property can turn on a PHY delay.
phy-mode = "rgmii-txid": rx-internal-delay-ps is never read.
phy-mode = "rgmii-rxid": tx-internal-delay-ps is ignored.
The ethernet-phy.yaml binding defines these properties as picosecond
values for PHYs with configurable delays. The RTL8211F code takes the
delay only from phy-mode.
Could this do the same? Alternatively, it could reject any value other
than 0 and the delay the hardware supports.
[ ... ]
> +static int jlsemi_config_intr(struct phy_device *phydev)
> +{
[ ... ]
> + } else {
> + val = 0;
> + err = phy_write_paged(phydev, JLSEMI_INER_PAGE, JLSEMI_INER, val);
> + if (!err)
> + priv->iner = val;
> +
> + err = jlsemi_ack_interrupt(phydev);
> + }
[Severity: Low]
On the disable path, jlsemi_ack_interrupt() overwrites the
phy_write_paged() error before anything checks it.
If the INER write fails but the INSR read succeeds, can this return 0
while the link status interrupt is still enabled in the PHY?
The enable branch returns early on error, and rtl8211f_config_intr()
returns when the INER write fails.
> +static irqreturn_t jlsemi_handle_interrupt(struct phy_device *phydev)
> +{
[ ... ]
> + if (irq_status & JLSEMI_INER_LINK_STATUS) {
> + phy_trigger_machine(phydev);
> + return IRQ_HANDLED;
> + }
[Severity: Low]
If INSR has both LINK_STATUS and PME set, this returns before the PME bit
is checked.
The ack-by-read in jlsemi_ack_interrupt() suggests INSR is cleared on
read. If so, is pm_wakeup_event() skipped for that wakeup?
The actual wake from suspend goes through the wake IRQ. The main effect is
on wakeup source accounting and on aborting a suspend that is in progress.
The upstream Realtek handler has the same structure.
[ ... ]
> +static int jlsemi_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
> +{
[ ... ]
> + oldpage = phy_select_page(phydev, JLSEMI_PHYSICAL_ADDR_PAGE);
> + if (oldpage < 0)
> + return oldpage;
[Severity: High]
Does this leak the MDIO bus mutex?
phy_select_page() takes bus->mdio_lock in phy_save_page() before it reads
or writes the page register, and either of those can fail:
drivers/net/phy/phy-core.c:phy_save_page() {
phy_lock_mdio_bus(phydev);
return __phy_read_page(phydev);
}
The kerneldoc for phy_select_page() says phy_restore_page() must always be
called afterwards, whether or not the call succeeded. Returning oldpage
directly skips the only unlock.
Every later MDIO access on the bus would then block forever. That includes
the PHY state machine, the threaded phy_interrupt() handler and other PHYs
on the same bus. The path is an MDIO error during:
ethtool -s <dev> wol g|d
-> phy_ethtool_set_wol()
-> jlsemi_set_wol()
The disable branch below has the same early return. The Realtek code jumps
to phy_restore_page() on this error instead.
> +
> + __phy_write(phydev, JLSEMI_PHYSICAL_ADDR_WORD0,
> + mac_addr[1] << 8 | mac_addr[0]);
[ ... ]
> + __phy_write(phydev, JLSEMI_WOL_SETTINGS_EVENTS,
> + JLSEMI_WOL_EVENT_MAGIC);
> + __phy_write(phydev, JLSEMI_WOL_RST_RMSQ,
> + JLSEMI_WOL_RMSQ);
[Severity: High]
JLSEMI_WOL_RMSQ is 0x1fff, so this write also clears bit 15
(JLSEMI_WOL_RG_RSTB) and puts the WoL block into reset. The disable branch
clears the same bit on purpose.
Does anything set RG_RSTB back to 1?
jlsemi_suspend() only arms INER_PME and never touches reg 17. After
"ethtool -s <dev> wol g" the WoL engine seems to stay in reset through
suspend, so no PME would be raised.
The RTL8211F code was fixed for this in commit b826bf795564 ("net: phy:
realtek: fix RTL8211F wake-on-lan support"). After that commit, set_wol
asserts the WoL reset and rtl8211f_suspend() toggles RG_RSTB low and then
high. The commit notes that without this a PME interrupt may not be
raised, even when another packet is received.
The same commit also clears WOL_SETTINGS_EVENTS in rtl8211f_probe(),
because the WoL configuration survives reboots. jlsemi_probe() does not
clear page 0xd8a reg 16.
Could a WoL setup left by firmware or a previous kernel stay armed here?
jlsemi_get_wol() would then report it as WAKE_MAGIC, even though
device_set_wakeup_enable() was never called.
> +restore:
> + phy_restore_page(phydev, oldpage, ret);
> + } else {
> + oldpage = phy_select_page(phydev, JLSEMI_WOL_PAGE);
> + if (oldpage < 0)
> + return oldpage;
> +
> + __phy_write(phydev, JLSEMI_WOL_SETTINGS_EVENTS, 0);
> + __phy_clear_bits(phydev, JLSEMI_WOL_RST_RMSQ,
> + JLSEMI_WOL_RG_RSTB);
> +
> + phy_restore_page(phydev, oldpage, 0);
> + }
> +
> + device_set_wakeup_enable(&phydev->mdio.dev, !!(wol->wolopts & WAKE_MAGIC));
> +
> + return 0;
> +}
[Severity: Medium]
The return value of phy_restore_page() is discarded. If the __phy_write()
of JLSEMI_PAGE_SELECT fails and jumps to restore, the code still calls
device_set_wakeup_enable(..., true) and returns 0.
None of the MAC address writes or the WOL_SETTINGS_EVENTS and RST_RMSQ
writes are checked either. The disable branch passes a hard-coded 0 to
phy_restore_page() and also ignores its result.
Can "ethtool -s <dev> wol g" or "wol d" then report success while the
hardware is only partly programmed? jlsemi_get_wol() reads the hardware,
but jlsemi_suspend() follows device_may_wakeup(), so the two could
disagree.
The return value of device_set_wakeup_enable() is also ignored. With
CONFIG_PM_SLEEP, device_wakeup_enable() can fail with -ENOMEM.
> +static int jlsemi_suspend(struct phy_device *phydev)
> +{
> + int ret;
> +
> + if (device_may_wakeup(&phydev->mdio.dev)) {
[Severity: Medium]
jlsemi_config_init() calls jlsemi_disable_autonomous_eee() on every
phy_init_hw(). All three driver entries also register the same
function as .disable_autonomous_eee.
As far as I can tell, phylib only intends that callback to run once the
MAC has said it handles LPI itself. phy_support_eee() sets
phydev->autonomous_eee_disabled and calls the hook. phy_init_hw() then
re-applies it after .config_init() whenever that flag is set.
Calling it from jlsemi_config_init() as well turns autonomous EEE off
for every MAC. That includes MACs that never call phy_support_eee() and
cannot generate LPI themselves. On those boards the PHY was the only
thing that could put the link into low power idle, so the EEE power
savings are lost. On MACs that do call phy_support_eee(), PHYCR2 is
simply modified twice on each phy_init_hw().
The commit message says this is done to prevent interoperability packet
drops. Is that a problem in the PHY that requires autonomous EEE to be
off with every MAC? If so, a comment saying so would help, and the
.disable_autonomous_eee hook would then add nothing. If not, could the
call in jlsemi_config_init() be dropped and the job left to phylib?
[ ... ]
> + }
> +
> + return genphy_suspend(phydev);
> +}
[Severity: High]
All three driver entries set PHY_ALWAYS_CALL_SUSPEND, which bypasses this
check in phy_suspend():
drivers/net/phy/phy_device.c:phy_suspend() {
...
phydev->wol_enabled = phy_may_wakeup(phydev) ||
(netdev && netdev->ethtool->wol_enabled);
if (phydev->wol_enabled && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
return -EBUSY;
...
}
Suppose the MAC handles WoL, because the PHY has no wakeup-source or PHY
wakeup is not enabled. device_may_wakeup() is then false, and
genphy_suspend() sets BMCR_PDOWN.
Doesn't that take the link down, so the MAC can never receive the magic
packet?
Before this patch these PHYs bound to genphy. In this case phy_suspend()
returned -EBUSY and left the PHY powered. That makes this a regression for
boards that use MAC WoL, for example stmmac PMT.
rtl821x_suspend() checks phydev->wol_enabled before it calls
genphy_suspend().
[ ... ]
> +static int jlsemi_led_hw_control_get(struct phy_device *phydev, u8 index,
> + unsigned long *rules)
> +{
[ ... ]
> + if ((val & JLSEMI_LEDCR_LINK_10) &&
> + (val & JLSEMI_LEDCR_LINK_100) &&
> + (val & JLSEMI_LEDCR_LINK_1000)) {
> + __set_bit(TRIGGER_NETDEV_LINK, rules);
> + }
[Severity: Medium]
jlsemi_led_hw_control_set() sets all three speed bits in LEDCR for a
TRIGGER_NETDEV_LINK request. When all three bits are set, this function
reports LINK_10, LINK_100 and LINK_1000 together with
TRIGGER_NETDEV_LINK.
netdev_trig_activate() copies this bitmap straight into
trigger_data->mode. However, netdev_led_attr_store() rejects LINK combined
with any of the speed-specific link rules:
drivers/leds/trigger/ledtrig-netdev.c:netdev_led_attr_store() {
...
if (test_bit(TRIGGER_NETDEV_LINK, &mode) &&
(test_bit(TRIGGER_NETDEV_LINK_10, &mode) || ...
return -EINVAL;
...
}
Would later sysfs writes that keep link set then fail with -EINVAL until
link is cleared first? Examples are toggling rx or tx, or one of the
link_N attributes.
Should this report either LINK or the individual speeds, but not both?
[ ... ]
> + {
> + PHY_ID_MATCH_VENDOR(PHY_ID_JLSEMI),
> + .name = "JLSemi Generic Gigabit Ethernet",
> + .flags = PHY_ALWAYS_CALL_SUSPEND,
> + .probe = jlsemi_probe,
[ ... ]
> + .handle_interrupt = jlsemi_handle_interrupt,
> + .suspend = jlsemi_suspend,
> + .resume = jlsemi_resume,
[Severity: Medium]
The generic entry uses jlsemi_probe() and jlsemi_suspend(), but it has no
.get_wol or .set_wol.
With wakeup-source in DT and a valid IRQ, jlsemi_probe() still marks the
device wakeup capable and registers the wake IRQ. phy_can_wakeup() then
returns true and phylink_phy_supports_wol() treats the PHY as WoL capable,
but no wake event can ever be programmed.
If power/wakeup is then enabled through sysfs, jlsemi_suspend() skips
genphy_suspend() and switches INER to PME only, which masks the link
interrupt. Nothing has been set up that could wake the system.
Should the wakeup setup in probe be limited to the entries that implement
WoL?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261017165316.146793-1-cyan2254650260%40gmail.com
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-18 17:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-17 16:53 [PATCH net-next v2] net: phy: Add JLSemi JL21xx Gigabit Ethernet PHY driver Pufan Jin
2026-10-18 17:03 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®