Messages in this thread | | | From | Arend Van Spriel <> | Date | Fri, 07 Jun 2019 15:32:20 +0200 | Subject | Re: [PATCH v2 3/3] brcmfmac: sdio: Disable auto-tuning around commands expected to fail |
| |
On June 7, 2019 2:40:04 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 7/06/19 8:12 AM, Arend Van Spriel wrote: >> On June 6, 2019 11:37:22 PM Doug Anderson <dianders@chromium.org> wrote: >>> >>> In the case of dw_mmc, which I'm most familiar with, we don't have any >>> sort of automated or timed-based retuning. ...so we'll only re-tune >>> when we see the CRC error. If I'm understanding things correctly then >>> that for dw_mmc my solution and yours behave the same. That means the >>> difference is how we deal with other retuning requests, either ones >>> that come about because of an interrupt that the host controller >>> provided or because of a timer. Did I get that right? >> >> Right. >> >>> ...and I guess the reason we have to deal specially with these cases >>> is because any time that SDIO card is "sleeping" we don't want to >>> retune because it won't work. Right? NOTE: the solution that would >>> come to my mind first to solve this would be to hold the retuning for >>> the whole time that the card was sleeping and then release it once the >>> card was awake again. ...but I guess we don't truly need to do that >>> because tuning only happens as a side effect of sending a command to >>> the card and the only command we send to the card is the "wake up" >>> command. That's why your solution to hold tuning while sending the >>> "wake up" command works, right? >> >> Yup. >> >>> --- >>> >>> OK, so assuming all the above is correct, I feel like we're actually >>> solving two problems and in fact I believe we actually need both our >>> approaches to solve everything correctly. With just your patch in >>> place there's a problem because we will clobber any external retuning >>> requests that happened while we were waking up the card. AKA, imagine >>> this: >>> >>> A) brcmf_sdio_kso_control(on=True) gets called; need_retune starts as 0 >>> >>> B) We call sdio_retune_hold_now() >>> >>> C) A retuning timer goes off or the SD Host controller tells us to retune >>> >>> D) We get to the end of brcmf_sdio_kso_control() and clear the "retune >>> needed" since need_retune was 0 at the start. >>> >>> ...so we dropped the retuning request from C), right? >>> >>> >>> What we truly need is: >>> >>> 1. CRC errors shouldn't trigger a retuning request when we're in >>> brcmf_sdio_kso_control() >>> >>> 2. A separate patch that holds any retuning requests while the SDIO >>> card is off. This patch _shouldn't_ do any clearing of retuning >>> requests, just defer them. >>> >>> >>> Does that make sense to you? If so, I can try to code it up... >> >> FWIW it does make sense to me. However, I am still not sure if our sdio >> hardware supports retuning. Have to track down an asic designer who can tell >> or dive into vhdl myself. > > The card supports re-tuning if is handles CMD19, which it does. It is not > the card that does any tuning, only the host. The card just helps by > providing a known data pattern in response to CMD19. It can be that a card > provides good enough signals that the host should not need to re-tune. I > don't know if that can be affected by the board design though.
Right. I know it supports initial tuning, but I'm not sure about subsequent retuning initiated by the host controller.
Regards, Arend
| |