RE: [EXT] Re: [PATCH] wifi: mwifiex: fix firmware crash for AP DFS mode

[David Lin]

> 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