From: Doug Anderson <dianders@chromium.org> To: Arend Van Spriel <arend.vanspriel@broadcom.com> Cc: Ulf Hansson <ulf.hansson@linaro.org>, Kalle Valo <kvalo@codeaurora.org>, Adrian Hunter <adrian.hunter@intel.com>, "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>, Double Lo <double.lo@cypress.com>, Brian Norris <briannorris@chromium.org>, Madhan Mohan R <madhanmohan.r@cypress.com>, Matthias Kaehlcke <mka@chromium.org>, Wright Feng <wright.feng@cypress.com>, Chi-Hsien Lin <chi-hsien.lin@cypress.com>, brcm80211-dev-list.pdl@broadcom.com, Franky Lin <franky.lin@broadcom.com>, netdev <netdev@vger.kernel.org>, linux-wireless <linux-wireless@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, Hante Meuleman <hante.meuleman@broadcom.com>, Naveen Gupta <naveen.gupta@cypress.com>, brcm80211-dev-list@cypress.com, YueHaibing <yuehaibing@huawei.com>, "David S. Miller" <davem@davemloft.net> Subject: Re: [PATCH 1/3] brcmfmac: re-enable command decode in sdio_aos for BRCM 4354 Date: Mon, 20 May 2019 11:20:22 -0700 [thread overview] Message-ID: <CAD=FV=Uvc1wUQe-W1Jvm_gQ722pFm2a4OWvJDNVtkyQynFe4Gw@mail.gmail.com> (raw) In-Reply-To: <e3f54bcb-8d10-1336-1458-2bd11cfc1010@broadcom.com> Hi, On Mon, May 20, 2019 at 1:09 AM Arend Van Spriel <arend.vanspriel@broadcom.com> wrote: > > On 5/18/2019 12:54 AM, Douglas Anderson 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. > > This sounds very similar to the issue we had during integration of wifi > on rk3288 chromebooks years ago. I'm working on those same Chromebooks. ;-) I'm working on trying to make them well on newer kernels. ...but I guess you're saying that the problem faced by the people who wanted commit 29f6589140a1 ("brcmfmac: disable command decode in sdio_aos") are similar to the problems we saw in the past on those Chromebooks. I'd tend to agree. In general it's difficult to get a SD Host Controller to be fully robust in the fact of any/all errors on the bus. While dw_mmc is pretty robust these days I'm assuming that some other host controllers aren't. > > 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. > > I am leaning towards a full revert, but let's wait for more background info. I'd be fine with a full revert too. Presumably that will break someone but maybe they need to come up with a better solution? -Doug
WARNING: multiple messages have this Message-ID (diff)
From: Doug Anderson <dianders@chromium.org> To: Arend Van Spriel <arend.vanspriel@broadcom.com> Cc: Ulf Hansson <ulf.hansson@linaro.org>, Kalle Valo <kvalo@codeaurora.org>, Adrian Hunter <adrian.hunter@intel.com>, "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>, Double Lo <double.lo@cypress.com>, Brian Norris <briannorris@chromium.org>, Madhan Mohan R <madhanmohan.r@cypress.com>, Matthias Kaehlcke <mka@chromium.org>, Wright Feng <wright.feng@cypress.com>, Chi-Hsien Lin <chi-hsien.lin@cypress.com>, brcm80211-dev-list.pdl@broadcom.com, Franky Lin <franky.lin@broadcom.com>, netdev <netdev@vger.kernel.org>, linux-wireless <linux-wireless@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, Hante Meuleman <hante.meuleman@broadcom.com>, Naveen Gupta <naveen.gupta@cypress.com>, brcm80211-dev-list@cypress.com, YueHaibing <yuehaibing@> Subject: Re: [PATCH 1/3] brcmfmac: re-enable command decode in sdio_aos for BRCM 4354 Date: Mon, 20 May 2019 11:20:22 -0700 [thread overview] Message-ID: <CAD=FV=Uvc1wUQe-W1Jvm_gQ722pFm2a4OWvJDNVtkyQynFe4Gw@mail.gmail.com> (raw) In-Reply-To: <e3f54bcb-8d10-1336-1458-2bd11cfc1010@broadcom.com> Hi, On Mon, May 20, 2019 at 1:09 AM Arend Van Spriel <arend.vanspriel@broadcom.com> wrote: > > On 5/18/2019 12:54 AM, Douglas Anderson 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. > > This sounds very similar to the issue we had during integration of wifi > on rk3288 chromebooks years ago. I'm working on those same Chromebooks. ;-) I'm working on trying to make them well on newer kernels. ...but I guess you're saying that the problem faced by the people who wanted commit 29f6589140a1 ("brcmfmac: disable command decode in sdio_aos") are similar to the problems we saw in the past on those Chromebooks. I'd tend to agree. In general it's difficult to get a SD Host Controller to be fully robust in the fact of any/all errors on the bus. While dw_mmc is pretty robust these days I'm assuming that some other host controllers aren't. > > 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. > > I am leaning towards a full revert, but let's wait for more background info. I'd be fine with a full revert too. Presumably that will break someone but maybe they need to come up with a better solution? -Doug
next prev parent reply other threads:[~2019-05-20 18:20 UTC|newest] Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-17 22:54 [PATCH 0/3] brcmfmac: sdio: Deal better w/ transmission errors waking from sleep Douglas Anderson 2019-05-17 22:54 ` Douglas Anderson 2019-05-17 22:54 ` [PATCH 1/3] brcmfmac: re-enable command decode in sdio_aos for BRCM 4354 Douglas Anderson 2019-05-20 8:09 ` Arend Van Spriel 2019-05-20 18:20 ` Doug Anderson [this message] 2019-05-20 18:20 ` Doug Anderson [not found] ` <20190517225420.176893-2-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2019-05-28 12:18 ` Kalle Valo 2019-05-28 15:51 ` Doug Anderson 2019-05-28 15:51 ` Doug Anderson 2019-05-28 16:09 ` Arend Van Spriel 2019-05-28 16:09 ` Arend Van Spriel 2019-05-28 16:11 ` Arend Van Spriel 2019-05-28 16:11 ` Arend Van Spriel 2019-06-04 3:20 ` Wright Feng 2019-06-04 3:20 ` Wright Feng 2019-06-04 16:01 ` Doug Anderson 2019-06-04 16:01 ` Doug Anderson 2019-06-04 16:48 ` Arend Van Spriel 2019-06-04 16:48 ` Arend Van Spriel 2019-05-29 14:51 ` Kalle Valo 2019-05-29 14:51 ` Kalle Valo 2019-05-28 12:18 ` Kalle Valo 2019-05-28 12:18 ` Kalle Valo 2019-05-17 22:54 ` [PATCH 2/3] mmc: core: API for temporarily disabling auto-retuning due to errors Douglas Anderson 2019-05-17 22:54 ` Douglas Anderson 2019-05-19 9:06 ` Wolfram Sang 2019-05-19 9:06 ` Wolfram Sang 2019-05-20 8:46 ` Arend Van Spriel 2019-05-20 8:52 ` Wolfram Sang 2019-05-20 8:52 ` Wolfram Sang 2019-05-26 18:42 ` Arend Van Spriel 2019-05-26 18:42 ` Arend Van Spriel 2019-05-28 10:04 ` Adrian Hunter 2019-05-28 11:21 ` Arend Van Spriel 2019-05-28 11:45 ` Adrian Hunter 2019-05-28 15:42 ` Doug Anderson 2019-05-17 22:54 ` [PATCH 3/3] brcmfmac: sdio: Disable auto-tuning around commands expected to fail Douglas Anderson 2019-05-18 15:09 ` [PATCH 0/3] brcmfmac: sdio: Deal better w/ transmission errors waking from sleep Avri Altman 2019-05-18 15:09 ` Avri Altman 2019-05-21 0:23 ` Brian Norris 2019-05-21 0:23 ` Brian Norris 2019-05-20 8:55 ` Arend Van Spriel 2019-05-20 8:55 ` Arend Van Spriel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAD=FV=Uvc1wUQe-W1Jvm_gQ722pFm2a4OWvJDNVtkyQynFe4Gw@mail.gmail.com' \ --to=dianders@chromium.org \ --cc=adrian.hunter@intel.com \ --cc=arend.vanspriel@broadcom.com \ --cc=brcm80211-dev-list.pdl@broadcom.com \ --cc=brcm80211-dev-list@cypress.com \ --cc=briannorris@chromium.org \ --cc=chi-hsien.lin@cypress.com \ --cc=davem@davemloft.net \ --cc=double.lo@cypress.com \ --cc=franky.lin@broadcom.com \ --cc=hante.meuleman@broadcom.com \ --cc=kvalo@codeaurora.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-rockchip@lists.infradead.org \ --cc=linux-wireless@vger.kernel.org \ --cc=madhanmohan.r@cypress.com \ --cc=mka@chromium.org \ --cc=naveen.gupta@cypress.com \ --cc=netdev@vger.kernel.org \ --cc=ulf.hansson@linaro.org \ --cc=wright.feng@cypress.com \ --cc=yuehaibing@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.