netdev - Re: [PATCH v2 3/3] brcmfmac: sdio: Disable auto-tuning around commands expected to fail

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Jun 2019 11:00:02 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Adrian Hunter <adrian.hunter@...el.com>
Cc:     Ulf Hansson <ulf.hansson@...aro.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        Arend van Spriel <arend.vanspriel@...adcom.com>,
        brcm80211-dev-list.pdl@...adcom.com,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Double Lo <double.lo@...ress.com>,
        Brian Norris <briannorris@...omium.org>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        Naveen Gupta <naveen.gupta@...ress.com>,
        Madhan Mohan R <madhanmohan.r@...ress.com>,
        Matthias Kaehlcke <mka@...omium.org>,
        Wright Feng <wright.feng@...ress.com>,
        Chi-Hsien Lin <chi-hsien.lin@...ress.com>,
        netdev <netdev@...r.kernel.org>,
        brcm80211-dev-list <brcm80211-dev-list@...ress.com>,
        "David S. Miller" <davem@...emloft.net>,
        Franky Lin <franky.lin@...adcom.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Hante Meuleman <hante.meuleman@...adcom.com>,
        YueHaibing <yuehaibing@...wei.com>,
        Michael Trimarchi <michael@...rulasolutions.com>
Subject: Re: [PATCH v2 3/3] brcmfmac: sdio: Disable auto-tuning around
 commands expected to fail

Hi,

On Fri, Jun 7, 2019 at 5:29 AM Adrian Hunter <adrian.hunter@...el.com> wrote:
>
> >> @@ -711,8 +720,16 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
> >>                         err_cnt = 0;
> >>                 }
> >>                 /* bail out upon subsequent access errors */
> >> -               if (err && (err_cnt++ > BRCMF_SDIO_MAX_ACCESS_ERRORS))
> >> -                       break;
> >> +               if (err && (err_cnt++ > BRCMF_SDIO_MAX_ACCESS_ERRORS)) {
> >> +                       if (!retune_release)
> >> +                               break;
> >> +                       /*
> >> +                        * Allow one more retry with re-tuning released in case
> >> +                        * it helps.
> >> +                        */
> >> +                       sdio_retune_release(bus->sdiodev->func1);
> >> +                       retune_release = false;
> >
> > I would be tempted to wait before adding this logic until we actually
> > see that it's needed.  Sure, doing one more transfer probably won't
> > really hurt, but until we know that it actually helps it seems like
> > we're just adding extra complexity?
>
> Depends, what is the downside of unnecessarily returning an error from
> brcmf_sdio_kso_control() in that case?

I'm not aware of any downside, but without any example of it being
needed it's just untested code that might or might not fix a problem.
For now I'm going to leave it out of my patch and if someone later
finds it needed or if you're convinced that we really need it we can
add it as a patch atop.

-Doug