Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
+180 −251
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6a39889
dts: bindings: ethernet: move adi,adin*11*.yaml
maass-hamburg Jun 13, 2025
e4ecb1a
drivers: ethernet: phy: ar8031: remove fixed link mode
maass-hamburg Jun 13, 2025
0fe8331
drivers: ethernet: phy: dts: split ethernet-phy.yaml
maass-hamburg Jun 13, 2025
dfff05a
drivers: ethernet: remove phy_configure_link() usage
maass-hamburg Jun 13, 2025
1ba3789
drivers: ethernet: phy: put shared macro in shared header
maass-hamburg Jun 13, 2025
7ab0f61
drivers: ethernet: phy: ksz8081: use default speeds
maass-hamburg Jun 13, 2025
aab1f7c
drivers: ethernet: phy: ar8031: use default speeds
maass-hamburg Jun 13, 2025
8b246fd
drivers: ethernet: phy: rtl8211f: use default speeds
maass-hamburg Jun 13, 2025
e1b6493
drivers: ethernet: phy: dp83825: use default speeds
maass-hamburg Jun 13, 2025
507d123
drivers: ethernet: phy: dp83867: use default speeds
maass-hamburg Jun 13, 2025
f4e2b8d
drivers: ethernet: phy: microchip_vsc8541: improve driver
maass-hamburg Jun 18, 2025
e819c76
drivers: ethernet: phy: microchip_vsc8541: use mutex
maass-hamburg Jun 18, 2025
5e306fb
drivers: ethernet: phy: microchip_vsc8541: use default speeds
maass-hamburg Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 4 additions & 51 deletions drivers/ethernet/eth_nxp_enet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,51 +459,12 @@ static void eth_nxp_enet_rx_thread(struct k_work *work)
ENET_EnableInterrupts(data->base, kENET_RxFrameInterrupt);
}

static int nxp_enet_phy_configure(const struct device *phy, uint8_t phy_mode)
{
enum phy_link_speed speeds = LINK_HALF_10BASE | LINK_FULL_10BASE |
LINK_HALF_100BASE | LINK_FULL_100BASE;
int ret;
struct phy_link_state state;

if (COND_CODE_1(IS_ENABLED(CONFIG_ETH_NXP_ENET_1G),
(phy_mode == NXP_ENET_RGMII_MODE), (0))) {
speeds |= (LINK_HALF_1000BASE | LINK_FULL_1000BASE);
}

/* Configure the PHY */
ret = phy_configure_link(phy, speeds, 0);
if (ret == -EALREADY) {
return 0;
} else if (ret == -ENOTSUP || ret == -ENOSYS) {
phy_get_link_state(phy, &state);

if (state.is_up) {
LOG_WRN("phy_configure_link returned %d, but link is up. "
"Speed: %s, %s-duplex",
ret,
PHY_LINK_IS_SPEED_1000M(state.speed) ? "1 Gbits" :
PHY_LINK_IS_SPEED_100M(state.speed) ? "100 Mbits" : "10 Mbits",
PHY_LINK_IS_FULL_DUPLEX(state.speed) ? "full" : "half");
} else {
LOG_ERR("phy_configure_link returned %d and link is down.", ret);
return -ENETDOWN;
}
} else if (ret) {
LOG_ERR("phy_configure_link failed with error: %d", ret);
return ret;
}

return 0;
}

static void nxp_enet_phy_cb(const struct device *phy,
struct phy_link_state *state,
void *eth_dev)
{
const struct device *dev = eth_dev;
struct nxp_enet_mac_data *data = dev->data;
const struct nxp_enet_mac_config *config = dev->config;
enet_mii_speed_t speed;
enet_mii_duplex_t duplex;

Expand All @@ -527,16 +488,13 @@ static void nxp_enet_phy_cb(const struct device *phy,
}

ENET_SetMII(data->base, speed, duplex);
}

LOG_INF("Link is %s", state->is_up ? "up" : "down");

if (!state->is_up) {
net_eth_carrier_off(data->iface);
nxp_enet_phy_configure(phy, config->phy_mode);
} else {
net_eth_carrier_on(data->iface);
} else {
net_eth_carrier_off(data->iface);
}

LOG_INF("Link is %s", state->is_up ? "up" : "down");
}

static void eth_nxp_enet_iface_init(struct net_if *iface)
Expand Down Expand Up @@ -813,11 +771,6 @@ static int eth_nxp_enet_init(const struct device *dev)

ENET_ActiveRead(data->base);

err = nxp_enet_phy_configure(config->phy_dev, config->phy_mode);
if (err) {
return err;
}

LOG_DBG("%s MAC %02x:%02x:%02x:%02x:%02x:%02x",
dev->name,
data->mac_addr[0], data->mac_addr[1],
Expand Down
7 changes: 0 additions & 7 deletions drivers/ethernet/eth_xilinx_axienet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,13 +526,6 @@ static int xilinx_axienet_probe(const struct device *dev)
XILINX_AXIENET_RECEIVER_CONFIGURATION_FLOW_CONTROL_OFFSET,
XILINX_AXIENET_RECEIVER_CONFIGURATION_FLOW_CONTROL_EN_MASK);

/* at time of writing, hardware does not support half duplex */
err = phy_configure_link(config->phy,
LINK_FULL_10BASE | LINK_FULL_100BASE | LINK_FULL_1000BASE, 0);
if (err) {
LOG_WRN("Could not configure PHY: %d", -err);
}

LOG_INF("RX Checksum offloading %s",
config->have_rx_csum_offload ? "requested" : "disabled");
LOG_INF("TX Checksum offloading %s",
Expand Down
5 changes: 5 additions & 0 deletions drivers/ethernet/phy/phy_microchip_ksz8081.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ struct mc_ksz8081_config {
uint8_t addr;
const struct device *mdio_dev;
enum ksz8081_interface phy_iface;
enum phy_link_speed default_speeds;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about using conventional name supported_speeds or link_speeds?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only used for configuring the phy on init, so these are the speeds that the phy will use by default, you can still configure the speeds later with phy_configure_link() and also set speeds that were not in default_speeds. By the way this is also how it is in phy_mii and this pr is extending it's functionality to the other phys

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only used for configuring the phy on init, so these are the speeds that the phy will use by default, you can still configure the speeds later with phy_configure_link() and also set speeds that were not in default_speeds. By the way this is also how it is in phy_mii and this pr is extending it's functionality to the other phys

Sorry, I missed the reference to the phy_mii PR earlier. My rationale is as follows:

Currently, default_speeds is generated based on what the specific PHY driver defines in the device tree or YAML, and it is used as a parameter for phy_configure_link().

For example:
default-speeds:
default: ["10BASE Half-Duplex", "10BASE Full-Duplex", "100BASE Half-Duplex",
"100BASE Full-Duplex"]

In practice, users or customers interpret these values as the supported speeds of the specific PHY. Referring to them as "default" is not ideal, as "default" suggests a fallback or initial value, rather than the complete set of speeds the PHY is capable of.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the user doesn't provide default-speeds, we will do auto negotiation with all supported speeds of the phy. That btw is also the default behavior of the phy (based on the registers) if we don't configure it.

also the phy_mii phy was doing phy_configure_link() with all supported speeds before #89210 and to preserve that behavior, these got set as the default, if the user didn't changed the prop in the dts.

#if DT_ANY_INST_HAS_PROP_STATUS_OKAY(reset_gpios)
const struct gpio_dt_spec reset_gpio;
#endif
Expand Down Expand Up @@ -489,6 +490,9 @@ static int phy_mc_ksz8081_init(const struct device *dev)
k_work_init_delayable(&data->phy_monitor_work,
phy_mc_ksz8081_monitor_work_handler);

/* Advertise default speeds */
phy_mc_ksz8081_cfg_link(dev, config->default_speeds, 0);

return 0;
}

Expand Down Expand Up @@ -519,6 +523,7 @@ static DEVICE_API(ethphy, mc_ksz8081_phy_api) = {
.addr = DT_INST_REG_ADDR(n), \
.mdio_dev = DEVICE_DT_GET(DT_INST_PARENT(n)), \
.phy_iface = DT_INST_ENUM_IDX(n, microchip_interface_type), \
.default_speeds = PHY_INST_GENERATE_DEFAULT_SPEEDS(n), \
RESET_GPIO(n) \
INTERRUPT_GPIO(n) \
}; \
Expand Down
Loading