MIME-Version: 1.0
References: <CAHYQr0qffa_G2ZVnSsDfjqSgxuBBZB+8e6ZODpAWV_TMhqzFYg@mail.gmail.com>
 <CAHYQr0qhqce2QcwMLRMWnqhZTL4KzHKJLzt6tfCNLHUcEt9pyQ@mail.gmail.com>
 <20181212014556.GC39861@atomide.com> <c8625c42f04a4721b7efb7d7b6b3931c@ti.com>
 <20181212183116.GH39861@atomide.com> <CAHYQr0ojkf3CvFb_zuEK4Vuz1w-rm9Sb=+dk5BkqYkWoxy8+aQ@mail.gmail.com>
 <9060b4655e064665a1812d9fae00f1ec@ti.com> <CAHYQr0r4eQNSqZEr-vbCiVN6G_O1CXxX9cxemANSRY-jqDMZ=A@mail.gmail.com>
 <20181213144522.GO39861@atomide.com> <CAHYQr0qg31hqyzm9uK03qz=DmUkA9idqTrhLaP9A6nuXPEW73w@mail.gmail.com>
 <20181214232857.GZ39861@atomide.com>
In-Reply-To: <20181214232857.GZ39861@atomide.com>
From:   Ricardo Salveti <rsalveti@rsalveti.net>
Date:   Sat, 15 Dec 2018 01:37:19 -0200
Message-ID: <CAHYQr0r90pLVzMus6n7Pdj5MELeQPK7nt9LnFWNHsoBM=eKhkg@mail.gmail.com>
Subject: Re: [EXTERNAL] Re: wlcore getting stuck on hikey after the runtime PM
 autosuspend support change
To:     Tony Lindgren <tony@atomide.com>
Cc:     eyalr@ti.com, John Stultz <john.stultz@linaro.org>,
        linux-wireless@vger.kernel.org, anders.roxell@linaro.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Fri, Dec 14, 2018 at 9:29 PM Tony Lindgren <tony@atomide.com> wrote:
> * Ricardo Salveti <rsalveti@rsalveti.net> [181214 12:42]:
> > Basically since commit 60f36637bbbd ("wlcore: sdio: allow pm to handle
> > sdio power") PM is now handling the sdio power off/on process, and if
> > wl12xx_sdio_power_on gets called right after wl12xx_sdio_power_off (if
> > down/up), the device will not go to the required power off/on sequence
> > (since PM will abort the suspend process), and the firmware loading
> > process will fail. I would guess the problem only happens with
> > autosuspend because of the extra delay it causes (pm_runtime_put
> > always returns -EBUSY on wl12xx_sdio_power_off with autosuspend).
>
> OK thanks for the update, that's interesting.
>
> > Is there a way to force the suspend on wl12xx_sdio_power_off, or
> > should we partially restore the old behavior?
>
> Well usually we could do pm_runtime_put_sync_suspend() but
> here it won't help as the pm_runtime_put() is already in
> progress by the SDIO subsystem and that's why we get -EBUSY.
>
> Does adding a little wait at the end of wl12xx_sdio_power_off()
> before return? Maybe something like:
>
>         /* Make sure the card gets powered off */
>         while (error == -EBUSY && !pm_runtime_suspended(&card->dev) &&
>                 retries--) {
>                 msleep(100);
>         }

Yes, this is enough to force the power off process and fixes the hang
I was having.

Also tried playing a bit with the msleep value and can confirm it
usually needs to wait between 30-40 milliseconds before powering the
card off.

Do you mind sending a patch with the above changes then? I can help
testing with the boards I have.

Thanks,
-- 
Ricardo Salveti de Araujo