Hi Andrew,
On Tue, 8 Jul 2025 16:01:06 +0200 Andrew Lunn andrew@lunn.ch wrote:
+static int nsim_phy_state_link_set(void *data, u64 val) +{
- struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
- ns_phy->link = !!val;
- phy_trigger_machine(ns_phy->phy);
- return 0;
+}
+static int nsim_phy_state_link_get(void *data, u64 *val) +{
- struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
- *val = ns_phy->link;
- return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(nsim_phy_state_link_fops, nsim_phy_state_link_get,
nsim_phy_state_link_set, "%llu\n");
+static void nsim_phy_debugfs_create(struct nsim_dev_port *port,
struct nsim_phy_device *ns_phy)
+{
- char phy_dir_name[sizeof("phy") + 10];
- sprintf(phy_dir_name, "phy%u", ns_phy->phy->phyindex);
- /* create debugfs stuff */
- ns_phy->phy_dir = debugfs_create_dir(phy_dir_name, port->ddir);
- debugfs_create_file("link", 0600, ns_phy->phy_dir, ns_phy, &nsim_phy_state_link_fops);
Maybe this can be converted into:
debugfs_create_bool("link", 0600, ns_phy->phy_dir, &ns_phy->link);
You loose the phy_trigger_machine(), but that might actually be good. PHYs are async to any operation you take on them. It can take up to 1 second due to the polling before any change is reported. So any user space tools which expect an immediate state change are broken. So leaving the PHY state machine to poll the PHY to notice the link has changed is a better simulation.
That's true indeed... Simpler and more accurate, I'll add that in V3.
Thanks for the feedback :)
Maxime