NACK/cmnt: [PATCH 1/4] UBUNTU: SAUCE: Redpine: fix wowlan issue

Shrirang Bagul shrirang.bagul at canonical.com
Tue Jan 9 04:01:52 UTC 2018


Hi Amit,

Several issues with the submission of this patch series -
- It's a good practice to have a cover letter explaining the reason for the patch
series
- BugLinks in the patch must point to public bugs against linux package.

Please refer to the SRU submission guidelines (see: https://wiki.ubuntu.com/Kernel/De
v/StablePatchFormat)
and resend the patches. I am NACKing this series.

Thanks,
Shrirang
> 
On Mon, 2018-01-08 at 19:24 +0530, Amitkumar Karwar wrote:
> From: Prameela Rani Garnepudi <prameela.j04cs at gmail.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1736097
> BugLink: https://bugs.launchpad.net/bugs/1738169

> Two issues were observed, kernel warning at S4 restore and other
> is failing to wakeup at times.
> Kernel warning is because, at hibernate resume while mac80211
> is resuming, driver is issuing mac80211 detach. The warning is
> as below:
> [  374.972073] WARNING: CPU: 1 PID: 3725 at linux-4.4.0/net/mac80211/iface.c:1000
> ieee80211_do_stop+0x6ea/0x810 [mac80211]()
> ....
> [  374.972211] CPU: 1 PID: 3725 Comm: kworker/u4:44 Tainted:
> G        W       4.4.0-98-generic #121-Ubuntu
> [  374.972213] Hardware name: Dell Inc. Edge Gateway 3002/      , BIOS 01.00.05
> 11/22/2017
> [  374.972223] Workqueue: events_unbound async_run_entry_fn
> [  374.972230]  0000000000000286 bf3948ba9db4c154 ffff88005a733ad8 ffffffff813fb2c3
> [  374.972235]  0000000000000000 ffffffffc04b8ac8 ffff88005a733b10 ffffffff810812e2
> [  374.972239]  ffff8800787c0840 ffff88006f18e700 0000000000000000 ffff88006f18ee90
> [  374.972240] Call Trace:
> [  374.972249]  [<ffffffff813fb2c3>] dump_stack+0x63/0x90
> [  374.972256]  [<ffffffff810812e2>] warn_slowpath_common+0x82/0xc0
> [  374.972260]  [<ffffffff8108142a>] warn_slowpath_null+0x1a/0x20
> [  374.972305]  [<ffffffffc045915a>] ieee80211_do_stop+0x6ea/0x810 [mac80211]
> [  374.972312]  [<ffffffff818441ee>] ? _raw_spin_unlock_bh+0x1e/0x20
> [  374.972317]  [<ffffffff817608ba>] ? dev_deactivate_many+0x20a/0x240
> [  374.972359]  [<ffffffffc045929a>] ieee80211_stop+0x1a/0x20 [mac80211]
> [  374.972365]  [<ffffffff81732a39>] __dev_close_many+0x99/0x100
> [  374.972369]  [<ffffffff81732b31>] dev_close_many+0x91/0x140
> [  374.972374]  [<ffffffff810e6171>] ? synchronize_sched_expedited+0x4e1/0x880
> [  374.972379]  [<ffffffff81734e2a>] dev_close.part.79+0x4a/0x70
> [  374.972383]  [<ffffffff81734e6a>] dev_close+0x1a/0x20
> [  374.972425]  [<ffffffffc035fac1>] cfg80211_shutdown_all_interfaces+0x41/0xa0
> [cfg80211]
> [  374.972467]  [<ffffffffc045a6c6>] ieee80211_remove_interfaces+0x56/0x1f0
> [mac80211]
> [  374.972506]  [<ffffffffc0441bca>] ieee80211_unregister_hw+0x4a/0x120 [mac80211]
> 
> This is avoided by calling ieee80211_restart_hw and reinitializing
> device as usual in sdio restore and waiting in mac80211_resume
> until device is ready.
> Other issue may be due to firmware assertion observed at times for
> the length of bgscan probe request at restore. To avoid this,
> unnecessary IEs are cut from the frame at end.
> 
> Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs at gmail.com>
> Signed-off-by: Amitkumar Karwar <amit.karwar at redpinesignals.com>
> ---
>  ubuntu/rsi/rsi_91x_hal.c      |  3 ++-
>  ubuntu/rsi/rsi_91x_mac80211.c | 15 ++++++++++++---
>  ubuntu/rsi/rsi_91x_main.c     |  1 +
>  ubuntu/rsi/rsi_91x_mgmt.c     | 24 +++++++++++++++++++++++-
>  ubuntu/rsi/rsi_91x_sdio.c     | 10 +++++++---
>  ubuntu/rsi/rsi_common.h       |  2 ++
>  ubuntu/rsi/rsi_main.h         |  2 ++
>  7 files changed, 49 insertions(+), 8 deletions(-)
> 
> diff --git a/ubuntu/rsi/rsi_91x_hal.c b/ubuntu/rsi/rsi_91x_hal.c
> index cccedaf..971d5ee 100644
> --- a/ubuntu/rsi/rsi_91x_hal.c
> +++ b/ubuntu/rsi/rsi_91x_hal.c
> @@ -188,7 +188,8 @@ int rsi_prepare_data_desc(struct rsi_common *common, struct
> sk_buff *skb)
>  			frame_desc[0] = cpu_to_le16((skb->len - FRAME_DESC_SZ) |
>  						    (RSI_WIFI_MGMT_Q << 12));
>  		}
> -			if ((skb->len - header_size) == 133) {
> +			if (((skb->len - header_size) == 133) ||
> +			    ((skb->len - header_size) == 131)) {
>  				ven_rsi_dbg(INFO_ZONE, "*** Tx EAPOL 4*****\n");
>  				frame_desc[1] |=
>  					cpu_to_le16(RSI_DESC_REQUIRE_CFM_TO_HOST);
> diff --git a/ubuntu/rsi/rsi_91x_mac80211.c b/ubuntu/rsi/rsi_91x_mac80211.c
> index 8a4100a..c32a9bb 100644
> --- a/ubuntu/rsi/rsi_91x_mac80211.c
> +++ b/ubuntu/rsi/rsi_91x_mac80211.c
> @@ -363,6 +363,11 @@ static int rsi_mac80211_hw_scan_start(struct ieee80211_hw *hw,
>  	if (common->fsm_state != FSM_MAC_INIT_DONE)
>  		return -ENODEV;
>  
> +#ifdef CONFIG_RSI_WOW
> +	if (common->wow_flags & RSI_WOW_ENABLED)
> +		return -ENETDOWN;
> +#endif
> +
>  	if (scan_req->n_channels == 0)
>  		return -EINVAL;
>  
> @@ -404,8 +409,8 @@ static int rsi_mac80211_hw_scan_start(struct ieee80211_hw *hw,
>          return 0;
>  }
>  
> -static void rsi_mac80211_hw_scan_cancel(struct ieee80211_hw *hw,
> -					struct ieee80211_vif *vif)
> +void rsi_mac80211_hw_scan_cancel(struct ieee80211_hw *hw,
> +				 struct ieee80211_vif *vif)
>  {
>  	struct rsi_hw *adapter = hw->priv;
>  	struct rsi_common *common = adapter->priv;
> @@ -444,6 +449,7 @@ static void rsi_mac80211_hw_scan_cancel(struct ieee80211_hw
> *hw,
>  	common->hw_scan_cancel = false;
>  	mutex_unlock(&common->mutex);
>  }
> +EXPORT_SYMBOL_GPL(rsi_mac80211_hw_scan_cancel);
>  #endif
>  
>  /**
> @@ -2452,8 +2458,11 @@ static int rsi_mac80211_resume(struct ieee80211_hw *hw)
>  	
>  	ven_rsi_dbg(INFO_ZONE, "%s: mac80211 resume\n", __func__);
>  
> -	if (common->hibernate_resume)
> +	if (common->hibernate_resume) {
> +		if (common->reinit_hw)
> +			wait_for_completion(&common->wlan_init_completion);
>  		return 0;
> +	}
>  
>  #ifdef CONFIG_VEN_RSI_WOW
>  	mutex_lock(&common->mutex);
> diff --git a/ubuntu/rsi/rsi_91x_main.c b/ubuntu/rsi/rsi_91x_main.c
> index f7a2e3a..2350dbb61 100644
> --- a/ubuntu/rsi/rsi_91x_main.c
> +++ b/ubuntu/rsi/rsi_91x_main.c
> @@ -415,6 +415,7 @@ struct rsi_hw *ven_rsi_91x_init(void)
>  	common->roc_timer.data = (unsigned long)common;
>  	common->roc_timer.function = (void *)&rsi_roc_timeout;
>  	init_timer(&common->roc_timer);
> +	init_completion(&common->wlan_init_completion);
>  
>  	common->init_done = true;
>  	return adapter;
> diff --git a/ubuntu/rsi/rsi_91x_mgmt.c b/ubuntu/rsi/rsi_91x_mgmt.c
> index 9976054..94fadc7 100644
> --- a/ubuntu/rsi/rsi_91x_mgmt.c
> +++ b/ubuntu/rsi/rsi_91x_mgmt.c
> @@ -2343,6 +2343,21 @@ int rsi_send_probe_request(struct rsi_common *common,
>  	}
>         
>  	if (scan_type == 1) {
> +		if (len > 120) {
> +			u16 t_len = MIN_802_11_HDR_LEN;
> +
> +			/* Cut some IEs */
> +			pos = &skb->data[MIN_802_11_HDR_LEN];
> +			while (true) {
> +				if ((t_len + pos[1] + 2) > 120) {
> +					skb_trim(skb, t_len);
> +					len = t_len;
> +					break;
> +				}
> +				t_len += pos[1] + 2;
> +				pos += (pos[1] + 2);
> +			}
> +		}
>  		common->bgscan_probe_req_len = len;	
>  		return 0;
>  	}
> @@ -2635,7 +2650,14 @@ static int rsi_handle_ta_confirm(struct rsi_common *common,
> u8 *msg)
>  			common->bb_rf_prog_count--;
>  			if (!common->bb_rf_prog_count) {
>  				common->fsm_state = FSM_MAC_INIT_DONE;
> -				return rsi_mac80211_attach(common);
> +				if (common->reinit_hw) {
> +					common->hw_data_qs_blocked = false;
> +					ieee80211_wake_queues(adapter->hw);
> +					complete(&common->wlan_init_completion);
> +					common->reinit_hw = false;
> +				} else {
> +					return rsi_mac80211_attach(common);
> +				}
>  			}
>  		} else {
>  			ven_rsi_dbg(INFO_ZONE,
> diff --git a/ubuntu/rsi/rsi_91x_sdio.c b/ubuntu/rsi/rsi_91x_sdio.c
> index 07e08f0..336f059 100644
> --- a/ubuntu/rsi/rsi_91x_sdio.c
> +++ b/ubuntu/rsi/rsi_91x_sdio.c
> @@ -1467,9 +1467,6 @@ static int rsi_sdio_reinit_device(struct rsi_hw *adapter)
>  	for (ii = 0; ii < NUM_SOFT_QUEUES; ii++)
>  		skb_queue_purge(&adapter->priv->tx_queue[ii]);
>  
> -	/* Detach MAC */
> -	ven_rsi_mac80211_detach(adapter);
> -	
>  	/* Initialize device again */
>  	sdio_claim_host(pfunction);
>  
> @@ -1500,7 +1497,14 @@ int rsi_restore(struct device *dev)
>  	adapter->priv->bt_fsm_state = BT_DEVICE_NOT_READY;
>  	adapter->priv->iface_down = true;
>  
> +	adapter->sc_nvifs = 0;
> +	rsi_mac80211_hw_scan_cancel(adapter->hw, adapter->priv->scan_vif);
> +	flush_workqueue(adapter->priv->scan_workqueue);
> +	ieee80211_stop_queues(adapter->hw);
> +	ieee80211_restart_hw(adapter->hw);
> +
>  	/* Initialize device again */
> +	adapter->priv->reinit_hw = true;
>  	rsi_sdio_reinit_device(adapter);
>  
>  #ifdef CONFIG_VEN_RSI_WOW
> diff --git a/ubuntu/rsi/rsi_common.h b/ubuntu/rsi/rsi_common.h
> index d4385de..e7f1240 100644
> --- a/ubuntu/rsi/rsi_common.h
> +++ b/ubuntu/rsi/rsi_common.h
> @@ -115,4 +115,6 @@ struct ieee80211_vif *rsi_get_vif(struct rsi_hw *adapter, u8
> *mac);
>  #ifdef CONFIG_VEN_RSI_WOW
>  int rsi_config_wowlan(struct rsi_hw *adapter, struct cfg80211_wowlan *wowlan);
>  #endif
> +void rsi_mac80211_hw_scan_cancel(struct ieee80211_hw *hw,
> +				 struct ieee80211_vif *vif);
>  #endif
> diff --git a/ubuntu/rsi/rsi_main.h b/ubuntu/rsi/rsi_main.h
> index 3f55e36..53b9245 100644
> --- a/ubuntu/rsi/rsi_main.h
> +++ b/ubuntu/rsi/rsi_main.h
> @@ -331,6 +331,8 @@ struct rsi_common {
>  	u8 ant_in_use;
>  	bool suspend_in_prog;
>  	bool hibernate_resume;
> +	bool reinit_hw;
> +	struct completion wlan_init_completion;
>  #ifdef CONFIG_VEN_RSI_WOW
>  	u8 wow_flags;
>  #endif
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180109/741c618f/attachment.sig>


More information about the kernel-team mailing list