Hi Simon, On Wed, Nov 15, 2017 at 11:31:05AM +0000, Xinming Hu wrote: > > -----Original Message----- > > From: Brian Norris [mailto:briannorris@xxxxxxxxxxxx] > > > > On Mon, Aug 14, 2017 at 12:19:03PM +0000, Xinming Hu wrote: > > > diff --git a/drivers/net/wireless/marvell/mwifiex/debugfs.c > > > b/drivers/net/wireless/marvell/mwifiex/debugfs.c > > > index 6f4239b..5d476de 100644 > > > --- a/drivers/net/wireless/marvell/mwifiex/debugfs.c > > > +++ b/drivers/net/wireless/marvell/mwifiex/debugfs.c > > > @@ -168,10 +168,11 @@ > > > { > > > struct mwifiex_private *priv = file->private_data; > > > > > > - if (!priv->adapter->if_ops.device_dump) > > > - return -EIO; > > > - > > > - priv->adapter->if_ops.device_dump(priv->adapter); > > > + if (priv->adapter->iface_type == MWIFIEX_USB) > > > + mwifiex_send_cmd(priv, HostCmd_CMD_FW_DUMP_EVENT, > > > + HostCmd_ACT_GEN_SET, 0, NULL, true); > > > > Why couldn't you just implement the device_dump() callback? > > Currently mwifiex_send_cmd function is not exported to external modules, it is designed for module mwifiex internal use. If you really don't want to export mwifiex_send_cmd(), you can export some other wrapper which does the logic for you. The point is that there's a well defined point for determining how to dump the firmware based on which interface you're using. You should use it. > If we export mwifiex_send_cmd, and call it in mwifiex_usb. There will be a loop, So? There are all sorts of interdependencies between mwifiex.ko and mwifiex_usb.ko (or in your words, "loops"). > .Device_dump (module mwifiex_usb) --> mwifiex_send_cmd(module mwifiex) --> .host_to_card (module mwifiex_usb) > > This seems not an elegant design, right? No less elegant than scattering: if (!USB) driver->this(); else that(); all over the place. > Regards, > Simon > > > > > + else > > > + priv->adapter->if_ops.device_dump(priv->adapter); > > > > > > return 0; > > > } Brian
