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." > 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
