netdev - Re: [PATCH 1/3] brcmfmac: re-enable command decode in sdio

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]

Message-ID: <CAD=FV=Wr4WsO7Xmei5GB4X91L_sDN331B1_oH+CPZOeFUkxyMg@mail.gmail.com>
Date:   Tue, 4 Jun 2019 09:01:10 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Wright Feng <Wright.Feng@...ress.com>
Cc:     Arend Van Spriel <arend.vanspriel@...adcom.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        Madhan Mohan R <MadhanMohan.R@...ress.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Hante Meuleman <hante.meuleman@...adcom.com>,
        "David S. Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Chi-Hsien Lin <Chi-Hsien.Lin@...ress.com>,
        Brian Norris <briannorris@...omium.org>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        YueHaibing <yuehaibing@...wei.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        "brcm80211-dev-list.pdl@...adcom.com" 
        <brcm80211-dev-list.pdl@...adcom.com>,
        Matthias Kaehlcke <mka@...omium.org>,
        Naveen Gupta <Naveen.Gupta@...ress.com>,
        brcm80211-dev-list <brcm80211-dev-list@...ress.com>,
        Double Lo <Double.Lo@...ress.com>,
        Franky Lin <franky.lin@...adcom.com>
Subject: Re: [PATCH 1/3] brcmfmac: re-enable command decode in sdio_aos for
 BRCM 4354

Hi,

On Mon, Jun 3, 2019 at 8:20 PM Wright Feng <Wright.Feng@...ress.com> wrote:
>
> On 2019/5/29 上午 12:11, Arend Van Spriel wrote:
> > On May 28, 2019 6:09:21 PM Arend Van Spriel
> > <arend.vanspriel@...adcom.com> wrote:
> >
> >> On May 28, 2019 5:52:10 PM Doug Anderson <dianders@...omium.org> wrote:
> >>
> >>> Hi,
> >>>
> >>> On Tue, May 28, 2019 at 5:18 AM Kalle Valo <kvalo@...eaurora.org> wrote:
> >>>>
> >>>> Douglas Anderson <dianders@...omium.org> wrote:
> >>>>
> >>>> > In commit 29f6589140a1 ("brcmfmac: disable command decode in
> >>>> > sdio_aos") we disabled something called "command decode in sdio_aos"
> >>>> > for a whole bunch of Broadcom SDIO WiFi parts.
> >>>> >
> >>>> > After that patch landed I find that my kernel log on
> >>>> > rk3288-veyron-minnie and rk3288-veyron-speedy is filled with:
> >>>> >   brcmfmac: brcmf_sdio_bus_sleep: error while changing bus sleep
> >>>> state -110
> >>>> >
> >>>> > This seems to happen every time the Broadcom WiFi transitions out of
> >>>> > sleep mode.  Reverting the part of the commit that affects the
> >>>> WiFi on
> >>>> > my boards fixes the problem for me, so that's what this patch does.
> >>>> >
> >>>> > Note that, in general, the justification in the original commit
> >>>> seemed
> >>>> > a little weak.  It looked like someone was testing on a SD card
> >>>> > controller that would sometimes die if there were CRC errors on the
> >>>> > bus.  This used to happen back in early days of dw_mmc (the
> >>>> controller
> >>>> > on my boards), but we fixed it.  Disabling a feature on all boards
> >>>> > just because one SD card controller is broken seems bad.  ...so
> >>>> > instead of just this patch possibly the right thing to do is to fully
> >>>> > revert the original commit.
> >>>> >
> Since the commit 29f6589140a1 ("brcmfmac: disable command decode in
> sdio_aos") causes the regression on other SD card controller, it is
> better to revert it as you mentioned.
> Actually, without the commit, we hit MMC timeout(-110) and hanged
> instead of CRC error in our test.

Any chance I can convince you to provide some official tags like
Reviewed-by or Tested-by on the revert?

> Would you please share the analysis of
> dw_mmc issue which you fixed? We'd like to compare whether we got the
> same issue.

I'm not sure there's any single magic commit I can point to.  When I
started working on dw_mmc it was terrible at handling error cases and
would often crash / hang / stop all future communication upon certain
classes or efforts.  There were dozens of problems we've had to fix
over the years.  These problems showed up when we started supporting
HS200 / UHS since the tuning phase really stress the error handling of
the host controller.

I searched and by the time we were supporting Broadcom SDIO cards the
error handling was already pretty good.  ...but if we hadn't already
made the error handling more robust for UHS tuning then we would have
definitely hit these types of problems.  The only problem I remember
having to solve in dw_mmc that was unique to Broadcom was commit
0bdbd0e88cf6 ("mmc: dw_mmc: Don't start commands while busy").  Any
chance that could be what you're hitting?

What host controller are you having problems with?

-Doug