> From: Francesco Dolcini <francesco@xxxxxxxxxx> > Sent: Thursday, September 12, 2024 3:56 PM > To: David Lin <yu-hao.lin@xxxxxxx> > Cc: Francesco Dolcini <francesco@xxxxxxxxxx>; l.stach@xxxxxxxxxxxxxx; > linux-wireless@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > briannorris@xxxxxxxxxxxx; kvalo@xxxxxxxxxx; Pete Hsieh > <tsung-hsien.hsieh@xxxxxxx> > Subject: Re: [EXT] Re: [PATCH] wifi: mwifiex: fix firmware crash for AP DFS > mode > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On Thu, Sep 12, 2024 at 02:22:09AM +0000, David Lin wrote: > > > From: Francesco Dolcini <francesco@xxxxxxxxxx> > > > Sent: Wednesday, September 11, 2024 5:33 PM > > > To: David Lin <yu-hao.lin@xxxxxxx>; l.stach@xxxxxxxxxxxxxx > > > Cc: linux-wireless@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > > > briannorris@xxxxxxxxxxxx; kvalo@xxxxxxxxxx; francesco@xxxxxxxxxx; > > > Pete Hsieh <tsung-hsien.hsieh@xxxxxxx> > > > Subject: [EXT] Re: [PATCH] wifi: mwifiex: fix firmware crash for AP > > > DFS mode > > > > > > Caution: This is an external email. Please take care when clicking > > > links or opening attachments. When in doubt, report the message > > > using the 'Report this email' button > > > > > > > > > +Lucas (in case he missed this patch) > > > > > > On Fri, Aug 30, 2024 at 04:07:19PM +0800, David Lin wrote: > > > > Firmware crashes when AP works on a DFS channel and radar > > > > detection > > > occurs. > > > > This patch fixes the issue, also add "fake_radar_detect" entry to > > > > mimic radar detection for testing purpose. > > > > > > Do we want such kind of "fake" code in the driver? > > > > > > I do not agree that we mix an actual bug fix with additional testing > > > code, and if I understand correctly the commit message this is what we are > doing here. > > > > > > > This file can be used to test this patch on other chips without really > > radar detection from HW. > > please move the fake test code to a separate patch so that it can be discussed > separetely from the actual fix > O.K. I will remove this debugfs file for patch v2. > > > > --- a/drivers/net/wireless/marvell/mwifiex/11h.c > > > > +++ b/drivers/net/wireless/marvell/mwifiex/11h.c > > ... > > > > > + > > > > + if (priv->uap_stop_tx) { > > > > + if (!netif_carrier_ok(priv->netdev)) > > > > > > is this if needed? why? can't you just call netif_carrier_on() in every case? > > > > If netif_carrier_ok(), there is no need to call netif_carrier_on(). > > yes, ok. this I know. But it seems not needed, and one line less of code is better > than having one additional useless line of code. > > My question is, is it required to have it? for what reason? My undestanding is > that you should just remove it, but maybe I am missing something. > I check netif_carrier_on(), it will check "test_and_clear_bit(__LINK_STATE_NOCARRIER, &dev->state)" before turning on the carrier. I will remove this check for patch v2 . I will also remove all the checks for nxpwifi, but for original code of mwifiex, there should be no plan to remove this kind of check. David