Hi Simon,
On Wed, Nov 15, 2017 at 11:31:05AM +0000, Xinming Hu wrote:
> > -----Original Message-----
> > From: Brian Norris [mailto:[email protected]]
> >
> > 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
