Re: Re: Re: [PATCH 3/3] mwifiex: debugfs: trigger device dump for usb interface

[Brian Norris <briannorris@chromium.org>]

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