Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation

[cantabile <cantabile.desu@gmail.com>]

On 27/02/18 22:42, Luis R. Rodriguez wrote:
> I'd be curious if someone who can trigger the situation can test what
> happens if you use:
> 
> diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c b/drivers/net/wireless/mediatek/mt7601u/mcu.c
> index 65a8004418ea..04cbffd225a1 100644
> --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
> @@ -421,7 +421,7 @@ static int mt7601u_load_firmware(struct mt7601u_dev *dev)
>   					 MT_USB_DMA_CFG_TX_BULK_EN));
>   
>   	if (firmware_running(dev))
> -		return 0;
> +		pr_info("Firmware already loaded but going to reload...");
>   
>   	ret = request_firmware(&fw, MT7601U_FIRMWARE, dev->dev);
>   	if (ret)
>   
> 
> 
> Curious, will it really fail?

This change brings no new messages from mt7601u in dmesg (other than 
this pr_info), and the device works fine, as far as I can tell.

> 
> Note that I see mt7601u_stop() just calls mt7601u_mac_stop(). The big cleanup
> happens via mt7601u_cleanup(), but I see mt7601u_disconnect() calls it.
> 
> Just curious, does that not get called on shutdown?
> 
> diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c b/drivers/net/wireless/mediatek/mt7601u/usb.c
> index b9e4f6793138..126ef2ba77c2 100644
> --- a/drivers/net/wireless/mediatek/mt7601u/usb.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
> @@ -311,6 +311,7 @@ static void mt7601u_disconnect(struct usb_interface *usb_intf)
>   {
>   	struct mt7601u_dev *dev = usb_get_intfdata(usb_intf);
>   
> +	pr_info("Calling mt7601u_disconnect()...");
>   	ieee80211_unregister_hw(dev->hw);
>   	mt7601u_cleanup(dev);
> 
> If it does, one option is mt7601u_cleanup() can use some love to really shut down
> the device more... But its not clear to me what else could be done and I'm very
> inclined to believe its not sensible.
> 
"Calling mt7601u_disconnect" does not appear in journalctl after a reboot.

> The idea of an optimization of *not* having to load firmware one more time if
> it already has it since power hasn't been shut off on the device seems sensible
> to me.
> 
> Give the few deltas above a quick test just to fill in curiosity if you like
> and to be complete -- I'll post RFCs shortly for you Cantabile to test, is
> that your name?

Yes.