drivers: ethernet: vsc8541: various fixes #91726

robhancocksed · 2025-06-16T22:16:12Z

Added missing MDIO enable/disable calls around accessing PHY registers, to make sure the MDIO bus is active first
Cleaned up internal functions to use uint16_t for PHY register values
Fixed inverted reset GPIO line
Added SW reset timeout
Implemented cfg_link function required by some MAC drivers

robhancocksed · 2025-06-16T23:57:24Z

It looks like the arch.interrupt.gen_isr_table_local.riscv test is failing on the ganymed_bob and ganymed_sk boards where I modified the .dts files. However, those build errors also seem to happen on the main branch, so they seem unrelated to these changes?

maass-hamburg · 2025-06-17T08:31:11Z

boards/sensry/ganymed_bob/ganymed_bob_sy120_gbm.dts

@@ -30,7 +30,7 @@
 		reg = <0x0>;
 		status = "okay";

-		reset-gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 11 GPIO_ACTIVE_LOW>;


add a entry in the migration guide, stating that this got fixed, so users with custom boards know that they have to change it too

maass-hamburg · 2025-06-17T08:33:44Z

drivers/ethernet/phy/phy_microchip_vsc8541.c

 */
-static int phy_mc_vsc8541_cfg_link(const struct device *dev, enum phy_link_speed speeds)
+static int phy_mc_vsc8541_cfg_link(const struct device *dev, enum phy_link_speed adv_speeds)


maybe wait until #91354 is merged, it will provide phy_mii_cfg_link_autoneg() to set the autoneg registers according to the ethernet specs

check out #91572 62e692e

Rebased on top of those changes - we can now make use of some of the common phy_mii code here.

Feel free to cherry-pick b58b241 from #91572
and putting it before your changes

I fixed there some other things I noticed

maass-hamburg · 2025-06-17T08:42:21Z

drivers/ethernet/phy/phy_microchip_vsc8541.c

+		if (count++ > 1000) {
+			LOG_ERR("phy reset timed out");
+			return -ETIMEDOUT;
+		}


/* we use limited advertising, to force gigabit speed */ /* initial version of this driver supports only 1GB/s */ /* 1000MBit/s + AUTO */ ret = phy_mc_vsc8541_write(dev, PHY_REG_PAGE0_ADV, (1 << 8) | (1 << 6) | 0x01); if (ret) { return ret; } ret = phy_mc_vsc8541_write(dev, PHY_REG_PAGE0_CTRL1000, (1 << 12) | (1 << 11) | (1 << 9)); if (ret) { return ret; } /* start auto negotiation */ ret = phy_mc_vsc8541_write(dev, PHY_REG_PAGE0_BMCR, BMCR_ANENABLE | BMCR_ANRESTART); if (ret) { return ret; }

this can probably be removed, autoneg is enabled by default

maass-hamburg · 2025-06-18T06:49:36Z

drivers/ethernet/phy/phy_microchip_vsc8541.c

-	if (ret) {
-		return ret;
-	}
-
 	/* start auto negotiation */
 	ret = phy_mc_vsc8541_write(dev, PHY_REG_PAGE0_BMCR, BMCR_ANENABLE | BMCR_ANRESTART);


this linke is also no longer needed, as we don't change the autoneg regs on startup, so we don't need to restart it, autoneg is btw enabled by default

maass-hamburg · 2025-06-18T07:08:07Z

drivers/ethernet/phy/phy_microchip_vsc8541.c

+/**
+ * @brief Reconfigure the link speed
+ *
+ * @param dev device structure to phy
+ * @param adv_speeds speeds to be advertised
+ * @param flags configuration flags
+ */


these kind of descriptions for doxygen are only needed for public apis in their headers, not in the source files

maass-hamburg · 2025-06-18T10:14:19Z

drivers/ethernet/phy/phy_microchip_vsc8541.c

 */
-static int phy_mc_vsc8541_cfg_link(const struct device *dev, enum phy_link_speed speeds)
+static int phy_mc_vsc8541_cfg_link(const struct device *dev, enum phy_link_speed adv_speeds)


Feel free to cherry-pick b58b241 from #91572
and putting it before your changes

I fixed there some other things I noticed

robhancocksed · 2025-06-18T19:33:19Z

@maass-hamburg I cherry-picked in the commit you mentioned, as well as one other with a mutex change, from your branch and rebased my changes on top of it. I think I have addressed your other comments as well.

robhancocksed · 2025-06-18T20:29:28Z

As noted before, those ganymed_bob and ganymed_sk test failures are not new, they also are occurring on weekly builds on the main branch.

drivers/ethernet/phy/phy_microchip_vsc8541.c

robhancocksed · 2025-07-14T17:34:38Z

Rebased to main to pick up the change in #92336 which should hopefully fix the CI build error.

maass-hamburg · 2025-07-19T20:00:49Z

@robhancocksed this needs a rebase to current main

- implement configure link - support half duplex - use defines from mii.h - fix check ret vals Signed-off-by: Fin Maaß <f.maass@vogl-electronic.com> Signed-off-by: Fin Maaß <f.maass@vogl-electronic.com>

use mutex to protect page register phy_mc_vsc8541_get_link got removed from phy_mc_vsc8541_link_cb_set so, that phy_mc_vsc8541_link_monitor (own thread) is the only one to change data->state Signed-off-by: Fin Maaß <f.maass@vogl-electronic.com>

Fixed some build warnings in the driver from previous changes by removing an unused variable and hooking up the cfg_link function. Also removes some implicit boolean conversions. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

The driver was not enabling the MDIO bus before trying to access registers. Added enabling and disabling the bus around PHY register accesses. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

The internal register read/write functions used uint32_t for the values even though the registers are only 16 bits wide, resulting in a bunch of casting. Change the internal functions to use uint16_t and wrap them for the external read/write API which uses uint32_t. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

For GPIOs driving active-low signals, such as the VSC8541's reset pin, they are supposed to be declared as active low in the device tree, and set to 1 to assert and 0 to clear. Change the driver as such so that it does not leave the PHY stuck in reset when so configured. Also changed all in-tree board DTS files for this PHY to properly declare the reset GPIO as active low. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

Mention a change to the reset-gpios handling in the vsc8541 PHY driver. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

The driver previously could enter an infinite loop if the PHY software reset failed to complete, which could happen due to hardware reset issues or MDIO bus problems. Add a timeout of 1000 iterations so we report an error in this scenario rather than causing a lockup. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

Add support for disabling autonegotiation to the cfg_link callback, as with the phy_mii driver. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

sonarqubecloud · 2025-07-21T21:19:08Z

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

robhancocksed · 2025-07-21T21:36:13Z

Rebased to main and moved the migration guide entry to 4.3 now that 4.2 has been released.

github-actions bot added area: Ethernet platform: sensry labels Jun 16, 2025

github-actions bot requested review from clamattia, decsny, lmajewski, maass-hamburg and pdgendt June 16, 2025 22:17

github-actions bot assigned maass-hamburg Jun 16, 2025

maass-hamburg requested changes Jun 17, 2025

View reviewed changes

robhancocksed force-pushed the vsc8541-fixes branch from b30a0e3 to 6dcae80 Compare June 17, 2025 17:49

github-actions bot added the Release Notes To be mentioned in the release notes label Jun 17, 2025

github-actions bot requested review from danieldegrasse, dkalowsk and kartben June 17, 2025 17:50

robhancocksed requested a review from maass-hamburg June 17, 2025 18:48

maass-hamburg requested changes Jun 18, 2025

View reviewed changes

robhancocksed force-pushed the vsc8541-fixes branch from 6dcae80 to 512218d Compare June 18, 2025 19:27

maass-hamburg previously approved these changes Jun 18, 2025

View reviewed changes

pdgendt reviewed Jun 19, 2025

View reviewed changes

drivers/ethernet/phy/phy_microchip_vsc8541.c Outdated Show resolved Hide resolved

robhancocksed dismissed maass-hamburg’s stale review via dabca3a June 19, 2025 22:10

robhancocksed force-pushed the vsc8541-fixes branch from 512218d to dabca3a Compare June 19, 2025 22:10

robhancocksed requested review from pdgendt and maass-hamburg June 19, 2025 22:23

maass-hamburg reviewed Jun 23, 2025

View reviewed changes

drivers/ethernet/phy/phy_microchip_vsc8541.c Outdated Show resolved Hide resolved

robhancocksed force-pushed the vsc8541-fixes branch from dabca3a to bdecf2d Compare June 24, 2025 19:10

robhancocksed requested a review from maass-hamburg June 24, 2025 19:10

maass-hamburg previously approved these changes Jun 24, 2025

View reviewed changes

maass-hamburg mentioned this pull request Jul 7, 2025

zephyr: sw_isr_table: Fix compilation error #92336

Merged

robhancocksed force-pushed the vsc8541-fixes branch from bdecf2d to 63b968a Compare July 14, 2025 17:32

danieldegrasse added the backport v4.2-branch Request backport to the v4.2-branch label Jul 15, 2025

danieldegrasse modified the milestones: v4.2.0, v4.3.0 Jul 15, 2025

maass-hamburg and others added 9 commits July 21, 2025 15:08

drivers: ethernet: phy: microchip_vsc8541: improve driver

8e5a2f1

- implement configure link - support half duplex - use defines from mii.h - fix check ret vals Signed-off-by: Fin Maaß <f.maass@vogl-electronic.com> Signed-off-by: Fin Maaß <f.maass@vogl-electronic.com>

drivers: ethernet: phy: vsc8541: add MDIO enable/disable

5f09d46

The driver was not enabling the MDIO bus before trying to access registers. Added enabling and disabling the bus around PHY register accesses. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

doc: migration guide: ethernet: Mention vsc8541 reset changes

ae562a2

Mention a change to the reset-gpios handling in the vsc8541 PHY driver. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

drivers: ethernet: phy: vsc8541: allow disabling autonegotiation

147ecfe

Add support for disabling autonegotiation to the cfg_link callback, as with the phy_mii driver. Signed-off-by: Robert Hancock <robert.hancock@calian.com>

robhancocksed dismissed stale reviews from pdgendt and maass-hamburg via 147ecfe July 21, 2025 21:16

robhancocksed force-pushed the vsc8541-fixes branch from 63b968a to 147ecfe Compare July 21, 2025 21:16

zephyrbot requested a review from tswaehn July 21, 2025 21:17

robhancocksed requested review from pdgendt and maass-hamburg July 22, 2025 21:35

maass-hamburg approved these changes Jul 22, 2025

View reviewed changes

pdgendt approved these changes Jul 23, 2025

View reviewed changes

kartben merged commit 45eedaa into zephyrproject-rtos:main Jul 23, 2025
40 of 42 checks passed

github-actions bot mentioned this pull request Jul 23, 2025

[Backport v4.2-branch] drivers: ethernet: vsc8541: various fixes #93554

Closed

maass-hamburg mentioned this pull request Jul 23, 2025

drivers: ethernet: phy: use default speeds props on the other phys #91572

Merged

robhancocksed deleted the vsc8541-fixes branch August 5, 2025 18:02

drivers: ethernet: vsc8541: various fixes #91726

drivers: ethernet: vsc8541: various fixes #91726

Uh oh!

Conversation

robhancocksed commented Jun 16, 2025

Uh oh!

robhancocksed commented Jun 16, 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!

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!

Choose a reason for hiding this comment

Uh oh!

robhancocksed commented Jun 18, 2025

Uh oh!

robhancocksed commented Jun 18, 2025

Uh oh!

Uh oh!

Uh oh!

robhancocksed commented Jul 14, 2025

Uh oh!

maass-hamburg commented Jul 19, 2025

Uh oh!

sonarqubecloud bot commented Jul 21, 2025

Quality Gate passed

Uh oh!

robhancocksed commented Jul 21, 2025

Uh oh!

Uh oh!

Uh oh!