On Sat, Oct 05, 2019 at 07:54:15PM +0800, Kai-Heng Feng wrote: > > > > On Oct 5, 2019, at 19:46, Simon Horman <horms@xxxxxxxxxxxx> wrote: > > > > On Fri, Oct 04, 2019 at 08:51:04PM +0800, Kai-Heng Feng wrote: > >> r8152 may fail to establish network connection after resume from system > >> suspend. > >> > >> If the USB port connects to r8152 lost its power during system suspend, > >> the MAC address was written before is lost. The reason is that The MAC > >> address doesn't get written again in its reset_resume callback. > >> > >> So let's set MAC address again in reset_resume callback. Also remove > >> unnecessary lock as no other locking attempt will happen during > >> reset_resume. > > > > This is two separate seemingly unrelated, other than locality in the code, > > changes. One is a fix, the other seems to be a cleanup. Perhaps they would > > be better addressed in separate patches. > > rtl8152_set_mac_address() which gets called by set_ethernet_addr(), also holds the same mutex. > So this is more then a cleanup and I should mention it in commit log. Thanks, I agree that is a good idea. > Kai-Heng > > > > >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > >> --- > >> drivers/net/usb/r8152.c | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c > >> index 08726090570e..cee9fef925cd 100644 > >> --- a/drivers/net/usb/r8152.c > >> +++ b/drivers/net/usb/r8152.c > >> @@ -4799,10 +4799,9 @@ static int rtl8152_reset_resume(struct usb_interface *intf) > >> struct r8152 *tp = usb_get_intfdata(intf); > >> > >> clear_bit(SELECTIVE_SUSPEND, &tp->flags); > >> - mutex_lock(&tp->control); > >> tp->rtl_ops.init(tp); > >> queue_delayed_work(system_long_wq, &tp->hw_phy_work, 0); > >> - mutex_unlock(&tp->control); > >> + set_ethernet_addr(tp); > >> return rtl8152_resume(intf); > >> } > >> > >> -- > >> 2.17.1 > >> >