> -----Original Message----- > From: Brian Norris [mailto:briannorris@xxxxxxxxxxxx] > Sent: 2017年12月2日 9:45 > To: Xinming Hu <huxm@xxxxxxxxxxx> > Cc: Xinming Hu <huxinming820@xxxxxxxxx>; 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>; James Cao > <jcao@xxxxxxxxxxx> > Subject: [EXT] Re: Re: Re: [PATCH 3/3] mwifiex: debugfs: trigger device dump > for usb interface > > External Email > > ---------------------------------------------------------------------- > On Fri, Dec 01, 2017 at 08:36:01AM +0000, Xinming Hu wrote: > > Thanks for the suggestion, it sounds better than exporting > > mwifiex_send_cmd() directly. > > In addition to used as debugfs tirgger, the "defined point" > > if_ops.device_dump is only used in command timeout context. > > For sdio/pcie interface, register operation will be made to trigger > > firmware dump and get dump context under specific algorithm. > > For usb interface, however, this is not needed, since firmware will > > automatically send dump event to host without any trigger, and what's > > more , host is also not able to issue command in the situation. > > > > So per my understand, here we only need provide a simple way to > > trigger , instead of a totally functional complete dump entry point. > > Suppose if we make the command trigger a part of if_ops->device_dump, > > then we also need add check for "MWIFIEX_USB" in mwifiex_cmd_tmo_func. > > Ah, I see. Your explanation makes some more sense then. It would be nice if > you could include some of this in > (a) the commit message > (b) the entry point in debugfs.c, where you trigger this > > Something along the lines of "For command timeouts, USB firmware will > automatically emit firmware dump events, so we don't implement > device_dump(). For user-initiated dumps, we trigger it ourselves." > Okay, Thanks! Regards, Simon > > it also looks inelegant, and what we did looks weird, they are > > (1) export a new kernel symbol, the wrapper of mwifiex_send_command > > (2) add usb if_ops->device_dump, it send the command in mwifiex_usb, > instead of in mwifiex itself. > > (3) bypass above "if_ops->device_dump" in mwifiex_cmd_tmo_func, which is > the mainly user case. > > No, I'm not sure that solution would be much better. Your existing solution with > additional comments is probably fine. > > > I am not sure whether there is a better way on this, perhaps we need a > > trade-off on different solutions, please let us know if you have more > > suggestions. > > > > Thanks & Regards, > > SImon > > Brian