Hi,
On Fri, Jun 7, 2019 at 5:29 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> 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
