netdev - Re: [EXTERNAL] [PATCH] net: phy: don't issue a module request if a driver is available

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4771715.vXUDI8C0e8@fedora.fritz.box>
Date: Thu, 02 Jan 2025 11:21:36 +0100
From: Francesco Valla <francesco@...la.it>
To: Andrew Lunn <andrew+netdev@...n.ch>,
 Heiner Kallweit <hkallweit1@...il.com>, Suman Ghosh <sumang@...vell.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 Russell King <linux@...linux.org.uk>
Subject:
 Re: [EXTERNAL] [PATCH] net: phy: don't issue a module request if a driver is
 available

Hi Suman,

On Thursday, 2 January 2025 at 10:35:40 Suman Ghosh <sumang@...vell.com> wrote:
> > 	mutex_init(&dev->lock);
> > 	INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
> >
> >-	/* Request the appropriate module unconditionally; don't
> >-	 * bother trying to do so only if it isn't already loaded,
> >-	 * because that gets complicated. A hotplug event would have
> >-	 * done an unconditional modprobe anyway.
> >-	 * We don't do normal hotplug because it won't work for MDIO
> >+	/* We don't do normal hotplug because it won't work for MDIO
> > 	 * -- because it relies on the device staying around for long
> > 	 * enough for the driver to get loaded. With MDIO, the NIC
> > 	 * driver will get bored and give up as soon as it finds that @@ -
> >724,7 +745,8 @@ struct phy_device *phy_device_create(struct mii_bus
> >*bus, int addr, u32 phy_id,
> > 		int i;
> >
> > 		for (i = 1; i < num_ids; i++) {
> >-			if (c45_ids->device_ids[i] == 0xffffffff)
> >+			if (c45_ids->device_ids[i] == 0xffffffff ||
> >+			    phy_driver_exists(c45_ids->device_ids[i]))
> > 				continue;
> >
> > 			ret = phy_request_driver_module(dev, @@ -732,7 +754,7 @@
> >struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32
> >phy_id,
> > 			if (ret)
> > 				break;
> > 		}
> >-	} else {
> >+	} else if (!phy_driver_exists(phy_id)) {
> [Suman] Can we add this phy_driver_exists() API call before the if/else check?
> 

Not really, as in case of C45 PHYs we have to check for drivers using (multiple)
IDs, which are different from phy_id.

> > 		ret = phy_request_driver_module(dev, phy_id);
> > 	}
> >
> 


Thank you!

Regards,
Francesco