Re: [PATCH net-next 3/3] net: usb: smsc95xx: suspend PHY during USB suspend

[<Parthiban.Veerasooran@microchip.com>]

Hi Oliver

Thank you for reviewing the patch.

On 30/03/26 11:09 pm, Oliver Neukum wrote:
>> @@ -1550,6 +1550,12 @@ static int smsc95xx_suspend(struct 
>> usb_interface *intf, pm_message_t message)
>>
>>       pdata->pm_task = current;
>>
>> +     if (pdata->phydev) {
>> +             ret = phy_suspend(pdata->phydev);
>> +             if (ret)
>> +                     return ret;
>> +     }
> 
> At this point you have suspended the phy.
> Hence the device can no longer transmit
> 
>> +
>>       ret = usbnet_suspend(intf, message);
> 
> This wants to
> 
> 1. drain the queue if you do runtime PM
> 2. can return -EBUSY
> 
>>       if (ret < 0) {
>>               netdev_warn(dev->net, "usbnet_suspend error\n");
> 
> And here it will return in the error case. With the phy
> already suspended.

Thank you for pointing it out. I agree with you. I didn’t note it 
earlier since the issue did not occur during my testing. I will move the 
phy_suspend() to the appropriate place.

> 
> And, as a question of principle: Why do you suspend the phy
> in suspend(), but take no action in resume()?

In resume(), I did not call phy_resume() because the resume path already 
invokes phy_init_hw(), which internally calls 
phydev->drv->config_init(). This reinitializes and reconfigures the PHY. 
This is the reason why I didn't call phy_resume(). If there is a 
preference or expectation to use phy_resume() for symmetry or to better 
align with the PHY framework’s suspend/resume semantics, I’m happy to 
update the code accordingly.

Moreover I will revisit the implementation with internal review and then 
post the next version. Thank you for your support.

Best regards,
Parthiban V
> 
>         Regards
>                 Oliver
> 
>