Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   <Nisar.Sayed@microchip.com>
To:     <agraf@suse.de>, <Woojung.Huh@microchip.com>
CC:     <UNGLinuxDriver@microchip.com>, <netdev@vger.kernel.org>,
        <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <tbogendoerfer@suse.de>, <phil@raspberrypi.org>
Subject: RE: [PATCH] lan78xx: Connect phy early
Thread-Topic: [PATCH] lan78xx: Connect phy early
Thread-Index: AQHTu6RtRKCcnT2O9k2lhCneqxr8AaPQ3VhQ
Date:   Thu, 15 Mar 2018 07:36:29 +0000
Message-ID: <CE371C1263339941885964188A0225FA3838A8@CHN-SV-EXMX03.mchp-main.com>
References: <20180314145456.69029-1-agraf@suse.de>
In-Reply-To: <20180314145456.69029-1-agraf@suse.de>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Alexander,

Thanks for the patch.

> @@ -2575,13 +2571,7 @@ static int lan78xx_stop(struct net_device *net)
>  	if (timer_pending(&dev->stat_monitor))
>  		del_timer_sync(&dev->stat_monitor);
> 
> -	phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0);
> -	phy_unregister_fixup_for_uid(PHY_LAN8835, 0xfffffff0);
> -
>  	phy_stop(net->phydev);
> -	phy_disconnect(net->phydev);
> -
> -	net->phydev = NULL;
> 
>  	clear_bit(EVENT_DEV_OPEN, &dev->flags);
>  	netif_stop_queue(net);

Please do add valid "phydev"  check before phy_stop, since "phy_disconnect" should be called before "unregister_netdev"

+ if (net->phydev)
	phy_stop(net->phydev);

> @@ -3481,6 +3471,11 @@ static void lan78xx_disconnect(struct
> usb_interface *intf)
>  	net = dev->net;
>  	unregister_netdev(net);
> 
> +	phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0);
> +	phy_unregister_fixup_for_uid(PHY_LAN8835, 0xfffffff0);
> +
> +	phy_disconnect(net->phydev);
> +
>  	cancel_delayed_work_sync(&dev->wq);
> 
>  	usb_scuttle_anchored_urbs(&dev->deferred);

Please move "unregister_netdev" after "phy_disconnect", otherwise "phy_disconnect" will fail while we disconnect USB.

> @@ -3634,6 +3629,10 @@ static int lan78xx_probe(struct usb_interface
> *intf,
>  	pm_runtime_set_autosuspend_delay(&udev->dev,
>  					 DEFAULT_AUTOSUSPEND_DELAY);
> 
> +	ret = lan78xx_phy_init(dev);
> +	if (ret < 0)
> +		return ret;
> +
>  	return 0;
> 
>  out3:
> --
> 2.12.3

We should "goto out4" instead of "return" upon "lan78xx_phy_init" fail

+ out4:
+	unregister_netdev(netdev);

In addition to current changes, you might have to take care of the following.

In function "lan78xx_reset_resume"

-  lan78xx_phy_init(dev);
+ phy_start(dev->net->phydev);

Thanks,
Sd.Nisar