Hi Brian, > -----Original Message----- > From: Brian Norris [mailto:briannorris@xxxxxxxxxxxx] > Sent: 2017年8月15日 8:01 > To: Xinming Hu <huxinming820@xxxxxxxxx> > Cc: Linux Wireless <linux-wireless@xxxxxxxxxxxxxxx>; Kalle Valo > <kvalo@xxxxxxxxxxxxxx>; Dmitry Torokhov <dtor@xxxxxxxxxx>; > rajatja@xxxxxxxxxx; Zhiyuan Yang <yangzy@xxxxxxxxxxx>; Tim Song > <songtao@xxxxxxxxxxx>; Cathy Luo <cluo@xxxxxxxxxxx>; Ganapathi Bhat > <gbhat@xxxxxxxxxxx>; Xinming Hu <huxm@xxxxxxxxxxx> > Subject: [EXT] Re: [PATCH 2/3] mwifiex: device dump support for usb interface > > External Email > > ---------------------------------------------------------------------- > Hi, > > On Mon, Aug 14, 2017 at 12:19:02PM +0000, Xinming Hu wrote: > > From: Xinming Hu <huxm@xxxxxxxxxxx> > > > > Firmware dump on usb interface is different with current sdio/pcie > > chipset, which is based on register operation. > > > > When firmware hang on usb interface, context dump will be upload to > > host using 0x73 firmware debug event. > > > > This patch store dump data from debug event and send to userspace > > using device coredump API. > > > > Signed-off-by: Xinming Hu <huxm@xxxxxxxxxxx> > > Signed-off-by: Cathy Luo <cluo@xxxxxxxxxxx> > > Signed-off-by: Ganapathi Bhat <gbhat@xxxxxxxxxxx> > > --- > > drivers/net/wireless/marvell/mwifiex/fw.h | 1 + > > drivers/net/wireless/marvell/mwifiex/init.c | 3 ++ > > drivers/net/wireless/marvell/mwifiex/main.h | 2 ++ > > drivers/net/wireless/marvell/mwifiex/sta_event.c | 39 > > ++++++++++++++++++++++++ > > 4 files changed, 45 insertions(+) > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h > > b/drivers/net/wireless/marvell/mwifiex/fw.h > > index 9e75522..610a3ea 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/fw.h > > +++ b/drivers/net/wireless/marvell/mwifiex/fw.h > > @@ -568,6 +568,7 @@ enum mwifiex_channel_flags { > > #define EVENT_BG_SCAN_STOPPED 0x00000065 > > #define EVENT_REMAIN_ON_CHAN_EXPIRED 0x0000005f > > #define EVENT_MULTI_CHAN_INFO 0x0000006a > > +#define EVENT_FW_DUMP_INFO 0x00000073 > > #define EVENT_TX_STATUS_REPORT 0x00000074 > > #define EVENT_BT_COEX_WLAN_PARA_CHANGE 0X00000076 > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/init.c > > b/drivers/net/wireless/marvell/mwifiex/init.c > > index e11919d..389d725 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/init.c > > +++ b/drivers/net/wireless/marvell/mwifiex/init.c > > @@ -315,6 +315,8 @@ static void mwifiex_init_adapter(struct > mwifiex_adapter *adapter) > > adapter->active_scan_triggered = false; > > setup_timer(&adapter->wakeup_timer, wakeup_timer_fn, > > (unsigned long)adapter); > > + setup_timer(&adapter->devdump_timer, mwifiex_upload_device_dump, > > + (unsigned long)adapter); > > } > > > > /* > > @@ -397,6 +399,7 @@ static void mwifiex_invalidate_lists(struct > > mwifiex_adapter *adapter) mwifiex_adapter_cleanup(struct > > mwifiex_adapter *adapter) { > > del_timer(&adapter->wakeup_timer); > > + del_timer_sync(&adapter->devdump_timer); > > mwifiex_cancel_all_pending_cmd(adapter); > > wake_up_interruptible(&adapter->cmd_wait_q.wait); > > wake_up_interruptible(&adapter->hs_activate_wait_q); > > diff --git a/drivers/net/wireless/marvell/mwifiex/main.h > > b/drivers/net/wireless/marvell/mwifiex/main.h > > index e308b8a..6b00294 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/main.h > > +++ b/drivers/net/wireless/marvell/mwifiex/main.h > > @@ -1038,6 +1038,7 @@ struct mwifiex_adapter { > > /* Device dump data/length */ > > char *devdump_data; > > int devdump_len; > > + struct timer_list devdump_timer; > > }; > > > > void mwifiex_process_tx_queue(struct mwifiex_adapter *adapter); @@ > > -1680,6 +1681,7 @@ void mwifiex_process_multi_chan_event(struct > > mwifiex_private *priv, void mwifiex_multi_chan_resync(struct > > mwifiex_adapter *adapter); int mwifiex_set_mac_address(struct > mwifiex_private *priv, > > struct net_device *dev); > > +void mwifiex_devdump_tmo_func(unsigned long function_context); > > > > #ifdef CONFIG_DEBUG_FS > > void mwifiex_debugfs_init(void); > > diff --git a/drivers/net/wireless/marvell/mwifiex/sta_event.c > > b/drivers/net/wireless/marvell/mwifiex/sta_event.c > > index 839df8a..bcf2fa2 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/sta_event.c > > +++ b/drivers/net/wireless/marvell/mwifiex/sta_event.c > > @@ -586,6 +586,40 @@ void > mwifiex_bt_coex_wlan_param_update_event(struct mwifiex_private *priv, > > adapter->coex_rx_win_size); > > } > > > > +static void > > +mwifiex_fw_dump_info_event(struct mwifiex_private *priv, > > + struct sk_buff *event_skb) > > +{ > > + struct mwifiex_adapter *adapter = priv->adapter; > > + > > + if (adapter->iface_type != MWIFIEX_USB) { > > + mwifiex_dbg(adapter, MSG, > > + "event is not on usb interface, ignore it\n"); > > + return; > > + } > > + > > + if (!adapter->devdump_data) { > > + /* When receive the first event, allocate device dump > > + * buffer, dump driver info. > > + */ > > + adapter->devdump_data = vzalloc(MWIFIEX_FW_DUMP_SIZE); > > Does this ever get freed? Sure, it will be freed by devcoredump framework, 5mins after driver call dev_coredump > > > + if (!adapter->devdump_data) { > > + mwifiex_dbg(adapter, ERROR, > > + "vzalloc devdump data failure!\n"); > > + return; > > + } > > + > > + mwifiex_drv_info_dump(adapter); > > + } > > + > > + memmove(adapter->devdump_data + adapter->devdump_len, > > + adapter->event_body, event_skb->len - > MWIFIEX_EVENT_HEADER_LEN); > > + adapter->devdump_len += (event_skb->len - > MWIFIEX_EVENT_HEADER_LEN); > > Are you going to try to check for overflow? Thanks for the point, will add the check in V2. > > > + /* If no proceeded event arrive in 10s, upload device dump data*/ > > You missed a space at the end of the comment. Should be: > > /* If no proceeded event arrive in 10s, upload device dump data. */ > > Also, is that really the only way to signal that the firmware dump has > completed? By timing out? That seems like a very bad design. Can you > implement an "end of transmission" signal? Good suggestion, Latest firmware will set the signal in the last FW_DUMP_INFO event, will apply it in V2. In case the "end of transmission" event lost in some cornel case, we would like to add a 10s timer to trigger the dump upload to userspace. Regards, Simon > > Brian > > > + mod_timer(&adapter->devdump_timer, > > + jiffies + msecs_to_jiffies(MWIFIEX_TIMER_10S)); > > +} > > + > > /* > > * This function handles events generated by firmware. > > * > > @@ -638,6 +672,7 @@ void > mwifiex_bt_coex_wlan_param_update_event(struct mwifiex_private *priv, > > * - EVENT_DELBA > > * - EVENT_BA_STREAM_TIEMOUT > > * - EVENT_AMSDU_AGGR_CTRL > > + * - EVENT_FW_DUMP_INFO > > */ > > int mwifiex_process_sta_event(struct mwifiex_private *priv) { @@ > > -1009,6 +1044,10 @@ int mwifiex_process_sta_event(struct > mwifiex_private *priv) > > adapter->event_skb->len - > > sizeof(eventcause)); > > break; > > + case EVENT_FW_DUMP_INFO: > > + mwifiex_dbg(adapter, EVENT, "event: firmware debug info\n"); > > + mwifiex_fw_dump_info_event(priv, adapter->event_skb); > > + break; > > /* Debugging event; not used, but let's not print an ERROR for it. */ > > case EVENT_UNKNOWN_DEBUG: > > mwifiex_dbg(adapter, EVENT, "event: debug\n"); > > -- > > 1.9.1 > >
