Hi Dmitry, > From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx] > Sent: Tuesday, October 25, 2016 5:44 AM > To: Amitkumar Karwar > Cc: linux-wireless@xxxxxxxxxxxxxxx; Cathy Luo; Nishant Sarmukadam; > briannorris@xxxxxxxxxx; Xinming Hu > Subject: Re: [PATCH 5/5] mwifiex: wait for firmware dump completion in > remove_card > > On Mon, Oct 24, 2016 at 07:51:32PM +0530, Amitkumar Karwar wrote: > > From: Xinming Hu <huxm@xxxxxxxxxxx> > > > > This patch ensures to wait for firmware dump completion in > > mwifiex_remove_card(). > > > > For sdio interface, reset_trigger variable is used to identify if > > mwifiex_sdio_remove() is called by sdio_work during reset or the call > > is from sdio subsystem. > > > > This patch fixes a kernel crash observed during reboot when firmware > > dump operation is in process. > > > > Signed-off-by: Xinming Hu <huxm@xxxxxxxxxxx> > > Signed-off-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx> > > --- > > drivers/net/wireless/marvell/mwifiex/pcie.c | 2 ++ > > drivers/net/wireless/marvell/mwifiex/sdio.c | 15 ++++++++++++++- > > 2 files changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c > > b/drivers/net/wireless/marvell/mwifiex/pcie.c > > index 986bf07..4512e86 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c > > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c > > @@ -528,6 +528,8 @@ static void mwifiex_pcie_remove(struct pci_dev > *pdev) > > if (!adapter || !adapter->priv_num) > > return; > > > > + cancel_work_sync(&pcie_work); > > + > > if (user_rmmod && !adapter->mfg_mode) { #ifdef CONFIG_PM_SLEEP > > if (adapter->is_suspended) > > diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c > > b/drivers/net/wireless/marvell/mwifiex/sdio.c > > index 4cad1c2..f974260 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/sdio.c > > +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c > > @@ -46,6 +46,15 @@ > > */ > > static u8 user_rmmod; > > > > +/* reset_trigger variable is used to identify if > > +mwifiex_sdio_remove() > > + * is called by sdio_work during reset or the call is from sdio > subsystem. > > + * We will cancel sdio_work only if the call is from sdio subsystem. > > + */ > > +static u8 reset_triggered; > > It would be really great if the driver supported multiple devices. IOW > please avoid module-globals. You are right. As Brian pointed out in other email, I will refactor the code and get rid of global variable. > > > + > > +static void mwifiex_sdio_work(struct work_struct *work); static > > +DECLARE_WORK(sdio_work, mwifiex_sdio_work); > > + > > static struct mwifiex_if_ops sdio_ops; static unsigned long > > iface_work_flags; > > > > @@ -289,6 +298,9 @@ mwifiex_sdio_remove(struct sdio_func *func) > > if (!adapter || !adapter->priv_num) > > return; > > > > + if (!reset_triggered) > > + cancel_work_sync(&sdio_work); > > + > > mwifiex_dbg(adapter, INFO, "info: SDIO func num=%d\n", func->num); > > > > if (user_rmmod && !adapter->mfg_mode) { @@ -2290,7 +2302,9 @@ > static > > void mwifiex_recreate_adapter(struct sdio_mmc_card *card) > > * discovered and initializes them from scratch. > > */ > > > > + reset_triggered = 1; > > mwifiex_sdio_remove(func); > > + reset_triggered = 0; > > What exactly happens if I trigger mwifiex_sdio_remove() from sysfs at > the same time this code is running? Yes. There would be a race. It will be avoided with Brian's code refactoring approach. Regards, Amitkumar
