Re: [PATCH net-next] net: phy: Relax error checking on sysfs_create_link()

[Florian Fainelli <f.fainelli@gmail.com>]

On May 26, 2017 8:34:18 PM PDT, Florian Fainelli <f.fainelli@gmail.com> wrote:
>Some Ethernet drivers will attach/connect to a PHY device before
>calling
>register_netdevice() which is responsible for calling
>netdev_register_kobject()
>which would do the network device's kobject initialization. In such a
>case,
>sysfs_create_link() would return -ENOENT because the network device's
>kobject
>is not ready yet, and we would fail to connect to the PHY device.
>
>In order to keep things simple and symetrical, we just take the success
>path as
>indicative of the ability to access the network device's kobject, and
>create
>the second link if that's the case.
>
>Fixes: 5568363f0cb3 ("net: phy: Create sysfs reciprocal links for
>attached_dev/phydev")
>Reported-by: Woojung Hung <Woojung.Huh@microchip.com>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>---
> drivers/net/phy/phy_device.c | 28 ++++++++++++++++++++--------
> include/linux/phy.h          |  2 ++
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/net/phy/phy_device.c
>b/drivers/net/phy/phy_device.c
>index f84414b8f2ee..523366bd705a 100644
>--- a/drivers/net/phy/phy_device.c
>+++ b/drivers/net/phy/phy_device.c
>@@ -960,15 +960,25 @@ int phy_attach_direct(struct net_device *dev,
>struct phy_device *phydev,
> 
> 	phydev->attached_dev = dev;
> 	dev->phydev = phydev;
>+
>+	/* Some Ethernet drivers try to connect to a PHY device before
>+	 * calling register_netdevice() -> netdev_register_kobject() and
>+	 * does the dev->dev.kobj initialization. Here we only check for
>+	 * success which indicates that the network device kobject is
>+	 * ready. Once we do that we still need to keep track of whether
>+	 * links were successfully set up or not for phy_detach() to
>+	 * remove them accordingly.
>+	 */
> 	err = sysfs_create_link(&phydev->mdio.dev.kobj, &dev->dev.kobj,
> 				"attached_dev");
>-	if (err)
>-		goto error;
>+	if (!err) {
>+		err = sysfs_create_link(&dev->dev.kobj, &phydev->mdio.dev.kobj,
>+					"phydev");
>+		if (err)
>+			goto error;
> 
>-	err = sysfs_create_link(&dev->dev.kobj, &phydev->mdio.dev.kobj,
>-				"phydev");
>-	if (err)
>-		goto error;
>+		phydev->sysfs_links = true;
>+	}
> 

We should not assume that this Boolean will be true or false if we enter this function a second time, v2 coming.

-- 
Florian