drivers: ethernet: phy: add driver for ksz9131, update ksz8081 with interrupt support #90711

TonyHan11 · 2025-05-28T04:31:42Z

This PR includes:

add the driver for Microchip KSZ9131 Ethernet PHY
add interrupt mode support for KSZ8081

pdgendt · 2025-05-28T07:00:55Z

Add an entry to https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/drivers/build_all/ethernet/app.overlay so it builds in CI.

maass-hamburg · 2025-05-28T07:16:00Z

dts/bindings/ethernet/microchip,ksz9131.yaml

+  microchip,interface-type:
+    type: string
+    required: true
+    description: Which type of phy connection the phy is set up for
+    enum:
+      - "rgmii"


this seems not being used in code, please remove.

maass-hamburg · 2025-05-28T07:19:09Z

drivers/ethernet/phy/phy_microchip_ksz9131.c

+		if (mutual_capabilities & MII_ADVERTISE_100_FULL) {
+			state->speed = LINK_FULL_100BASE_T;
+		} else if (mutual_capabilities & MII_ADVERTISE_100_HALF) {
+			state->speed = LINK_HALF_100BASE_T;
+		} else if (mutual_capabilities & MII_ADVERTISE_10_FULL) {
+			state->speed = LINK_FULL_10BASE_T;
+		} else if (mutual_capabilities & MII_ADVERTISE_10_HALF) {
+			state->speed = LINK_HALF_10BASE_T;


these LINK_*_*BASE_T got changed to LINK_*_*BASE

maass-hamburg · 2025-05-28T07:19:35Z

drivers/ethernet/phy/phy_microchip_ksz9131.c

+		if (adv_speeds & LINK_FULL_10BASE_T) {
+			anar |= MII_ADVERTISE_10_FULL;
+		} else {
+			anar &= ~MII_ADVERTISE_10_FULL;
+		}
+
+		if (adv_speeds & LINK_HALF_10BASE_T) {
+			anar |= MII_ADVERTISE_10_HALF;
+		} else {
+			anar &= ~MII_ADVERTISE_10_HALF;
+		}
+
+		if (adv_speeds & LINK_FULL_100BASE_T) {
+			anar |= MII_ADVERTISE_100_FULL;
+		} else {
+			anar &= ~MII_ADVERTISE_100_FULL;
+		}
+
+		if (adv_speeds & LINK_HALF_100BASE_T) {
+			anar |= MII_ADVERTISE_100_HALF;
+		} else {
+			anar &= ~MII_ADVERTISE_100_HALF;
+		}
+
+		BREAK_IF_ERR(ret, ksz9131_write(dev, MII_ANAR, anar));
+
+		BREAK_IF_ERR(ret, ksz9131_read(dev, MII_1KTCR, &c1kt));
+
+		if (adv_speeds & LINK_FULL_1000BASE_T) {
+			c1kt |= MII_ADVERTISE_1000_FULL;
+		} else {
+			c1kt &= ~MII_ADVERTISE_1000_FULL;
+		}
+
+		if (adv_speeds & LINK_HALF_1000BASE_T) {
+			c1kt |= MII_ADVERTISE_1000_HALF;
+		} else {
+			c1kt &= ~MII_ADVERTISE_1000_HALF;
+		}


maass-hamburg · 2025-05-28T07:23:11Z

drivers/ethernet/phy/phy_microchip_ksz9131.c

+	const struct mchp_ksz9131_config *const cfg = dev->config;
+	int ret = mdio_read(cfg->mdio, cfg->phy_addr, reg_addr, value);
+
+	if (ret) {


Suggested change

if (ret) {

if (ret < 0) {

Please change everywhere, ret is negative on error.

TonyHan11 · 2025-05-28T08:15:27Z

@pdgendt An entry for ksz9131 added to tests/drivers/build_all/ethernet/app.overlay, thanks.

maass-hamburg · 2025-05-28T08:18:27Z

@TonyHan11 pls add PR description and address the things from sonarqube

TonyHan11 · 2025-05-28T08:19:09Z

@maass-hamburg updated with removing the unused property, renaming LINK_*BASED_T and fixing if (ret), thank you very much.

maass-hamburg · 2025-06-02T07:25:32Z

drivers/ethernet/phy/phy_microchip_ksz8081.c

+		return;
+	}
+#endif  /* DT_ANY_INST_HAS_PROP_STATUS_OKAY(int_gpios) */
+
 	/* TODO change this to GPIO interrupt driven */


todo can be removed

maass-hamburg · 2025-06-02T07:26:42Z

drivers/ethernet/phy/phy_microchip_ksz8081.c

+
+	/* Lock mutex */
+	ret = k_mutex_lock(&data->mutex, K_FOREVER);
+	if (ret) {


always use ret < 0 when checking for errors

maass-hamburg · 2025-06-02T07:27:07Z

drivers/ethernet/phy/phy_microchip_ksz8081.c

+#if DT_ANY_INST_HAS_PROP_STATUS_OKAY(int_gpios)
+	if (config->interrupt_gpio.port) {
+		rc = phy_mc_ksz8081_clear_interrupt(data);
+		if (rc) {


maass-hamburg · 2025-06-02T07:51:13Z

drivers/ethernet/phy/phy_microchip_ksz8081.c

+#if DT_ANY_INST_HAS_PROP_STATUS_OKAY(int_gpios)
+	do {
+		if (!config->interrupt_gpio.port) {
+			phy_mc_ksz8081_monitor_work_handler(&data->phy_monitor_work.work);
+			break;
+		}
+
+		/* Configure interrupt pin */
+		ret = gpio_pin_configure_dt(&config->interrupt_gpio, GPIO_INPUT);
+		if (ret) {
+			break;
+		}
+
+		gpio_init_callback(&data->gpio_callback, phy_mc_ksz8081_interrupt_handler,
+				   BIT(config->interrupt_gpio.pin));
+		if (ret) {
+			break;
+		}
+
+		ret = gpio_add_callback_dt(&config->interrupt_gpio, &data->gpio_callback);
+		if (ret) {
+			break;
+		}
+
+		ret = phy_mc_ksz8081_config_interrupt(dev);
+		if (ret) {
+			break;
+		}
+
+		ret = gpio_pin_interrupt_configure_dt(&config->interrupt_gpio,
+						      GPIO_INT_EDGE_TO_ACTIVE);
+	} while (0);
+	if (ret) {
+		LOG_ERR("PHY (%d) config interrupt failed", config->addr);
+	}
+#endif


maybe make this a separate function, also in zephyr we prefer using goto instead of do while (0) loops.

maass-hamburg · 2025-06-02T07:55:46Z

drivers/ethernet/phy/phy_microchip_ksz9131.c

+#define BREAK_IF_ERR(ret, fun) do {						\
+		ret = fun;							\
+		if (ret < 0) {							\
+			break;							\
+		}								\
+	} while (0)


this won't work the break will only break out of the inner do while 0 loop. Also this makes it more difficult to read, please use simple goto instead

TonyHan11 · 2025-06-03T05:40:21Z

@maass-hamburg
Thank you for your comments. Commits updated with the following changes:

using goto instead of do while (0)
update if (ret) to if (ret < 0)
fix the issues reported by SonarQube Cloud
remove comments TODO change this to GPIO interrupt driven

maass-hamburg · 2025-06-03T11:11:19Z

drivers/ethernet/phy/Kconfig

+	default y
+	depends on DT_HAS_MICROCHIP_KSZ9131_ENABLED
+	depends on MDIO
+	depends on GPIO


GPIO is probably not required without reset and irq pin.
look at config PHY_REALTEK_RTL8211F how to change

Updated, thanks.

Add support for KSZ9131 (Gigabit Ethernet Transceiver with RGMII Support). As first starter, 100MBit/s mode is tested. https://www.microchip.com/en-us/product/ksz9131 Signed-off-by: Tony Han <tony.han@microchip.com>

Add build tests for Microchip KSZ9131. Signed-off-by: Tony Han <tony.han@microchip.com>

Enable Link-Up and Link-Down interrupts. On the interrupt handling the monitor work is scheduled to update the link status and calling corresponding callback routine. Signed-off-by: Tony Han <tony.han@microchip.com>

sonarqubecloud · 2025-06-04T01:44:33Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions bot added the area: Ethernet label May 28, 2025

github-actions bot requested review from clamattia, decsny, lmajewski, maass-hamburg and pdgendt May 28, 2025 04:32

github-actions bot assigned maass-hamburg May 28, 2025

maass-hamburg requested changes May 28, 2025

View reviewed changes

TonyHan11 force-pushed the phy_ksz9131_ksz8081 branch from 6944d51 to 5055226 Compare May 28, 2025 08:09

TonyHan11 force-pushed the phy_ksz9131_ksz8081 branch from 5055226 to cfd8c64 Compare May 29, 2025 01:21

maass-hamburg requested changes Jun 2, 2025

View reviewed changes

TonyHan11 force-pushed the phy_ksz9131_ksz8081 branch 4 times, most recently from 1ac4658 to e7778f7 Compare June 3, 2025 04:41

maass-hamburg reviewed Jun 3, 2025

View reviewed changes

TonyHan11 force-pushed the phy_ksz9131_ksz8081 branch from e7778f7 to ffe8e46 Compare June 4, 2025 01:15

TonyHan11 added 4 commits June 4, 2025 09:28

drivers: ethernet: phy: add Microchip's KSZ9131 PHY support

90ebe8c

Add support for KSZ9131 (Gigabit Ethernet Transceiver with RGMII Support). As first starter, 100MBit/s mode is tested. https://www.microchip.com/en-us/product/ksz9131 Signed-off-by: Tony Han <tony.han@microchip.com>

tests: drivers: build_all: ethernet: Add KSZ9131 entry

ad9ce93

Add build tests for Microchip KSZ9131. Signed-off-by: Tony Han <tony.han@microchip.com>

drivers: ethernet: phy: ksz9131: add support for phy interrupt mode

b5c95eb

Enable Link-Up and Link-Down interrupts. On the interrupt handling the monitor work is scheduled to update the link status and calling corresponding callback routine. Signed-off-by: Tony Han <tony.han@microchip.com>

drivers: ethernet: phy: ksz8081: add support for phy interrupt mode

ffe8e46

Enable Link-Up and Link-Down interrupts. On the interrupt handling the monitor work is scheduled to update the link status and calling corresponding callback routine. Signed-off-by: Tony Han <tony.han@microchip.com>

drivers: ethernet: phy: add driver for ksz9131, update ksz8081 with interrupt support #90711

Are you sure you want to change the base?

drivers: ethernet: phy: add driver for ksz9131, update ksz8081 with interrupt support #90711

Conversation

TonyHan11 commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pdgendt commented May 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TonyHan11 commented May 28, 2025

Uh oh!

maass-hamburg commented May 28, 2025

Uh oh!

TonyHan11 commented May 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TonyHan11 commented Jun 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 4, 2025

Quality Gate passed

Uh oh!

Uh oh!

TonyHan11 commented May 28, 2025 •

edited

Loading