linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: Tony Lindgren <tony@atomide.com>
Cc: "Reizer, Eyal" <eyalr@ti.com>, Kalle Valo <kvalo@codeaurora.org>,
	KISHON VIJAY ABRAHAM <kishon@ti.com>, "Mishol, Guy" <guym@ti.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	linux-omap <linux-omap@vger.kernel.org>,
	Anders Roxell <anders.roxell@linaro.org>,
	John Stultz <john.stultz@linaro.org>,
	Ricardo Salveti <rsalveti@rsalveti.net>
Subject: Re: [EXTERNAL] Re: [PATCH] wlcore: Fix bringing up wlan0 again if powered down briefly
Date: Fri, 28 Dec 2018 12:01:00 +0100	[thread overview]
Message-ID: <CAPDyKFoEOGACs4Wnm7UgKab2b1-7+v8+31DpC+p=8XxcQhSpcQ@mail.gmail.com> (raw)
In-Reply-To: <20181223160226.GK6707@atomide.com>

On Sun, 23 Dec 2018 at 17:02, Tony Lindgren <tony@atomide.com> wrote:
>
> Hi,
>
> * Reizer, Eyal <eyalr@ti.com> [181223 07:38]:
> > You are ok as long as the wlan_enable pin Does go down for a sufficient amount of time
> > turning the wl18xx device off.
> > The firmware can only be downloaded once after power on.
> > In between down/up you have to make sure the wlan_enable is fully going through on->off->on
> > and the wl18xx device is fully reset.
> > On power on the firmware is loaded by the driver and this will fail in case the reset didn't happen
>
> OK thanks for confirming that it's the enable pin that needs to toggle.
>
> Sounds like I need to get the wlcore pwrseq changes done and posted as
> the long term solution.

Just to make sure we are talking about the same things here. The
pwrseq thingy, is already implemented in the mmc core. When it comes
to Hikey, there is already a pwrseq node deployed in the DTB. So, we
should be fine.

Here are the MMC pwrseq bindings:
Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
Documentation/devicetree/bindings/mmc/mmc.txt

Here is the Hikey DTS file:
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts

>
> And for a short term fix, we should just add msleep(50) for now with
> updated patch description.
>
> Does that that sound OK to everybody?

Unfortunate, that doesn't work. Because, if user space via sysfs has
prevented runtime suspend, the SDIO card never becomes power off with
a call to pm_runtime_put*(), not even after waiting 50 ms.

If we need a short term solution, I think there are two options.

1) Call pm_runtime_force_suspend() instead of pm_runtime_put*() at
"down". This makes sure the card becomes powered off. At "up", call
pm_runtime_force_resume() instead of pm_runtime_get_sync(). Because
the runtime PM usage count it > 1, at "up", pm_runtime_force_resume()
will power up the card, rather than deferring it.

The problem with 1), is if there is an ACPI PM domain attached to the
SDIO card device. Then this doesn't work. I can't tell if that is ever
the case here.

2) At "up", after the call to pm_runtime_get_sync(), add a call to
mmc_hw_reset(). This forces the mmc core to power cycle and re-init
the SDIO card. This may in some cases be a waste, as the card may
already have been power cycled, but at least we should now always be
able to re-program the firmware.

If there is no rush, I think we may consider to move away from using
runtime PM to control power for SDIO cards. Because, what we seem to
need, is a simple interface (reference counted) to power-on/off SDIO
cards.

Kind regards
Uffe

  reply	other threads:[~2018-12-28 11:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-17 16:42 [PATCH] wlcore: Fix bringing up wlan0 again if powered down briefly Tony Lindgren
2018-12-18  1:06 ` Ricardo Salveti
2018-12-18 12:34 ` Ulf Hansson
2018-12-18 15:54   ` Tony Lindgren
2018-12-20 10:14     ` Ulf Hansson
2018-12-20 23:14       ` Tony Lindgren
2018-12-23  7:38         ` [EXTERNAL] " Reizer, Eyal
2018-12-23 16:02           ` Tony Lindgren
2018-12-28 11:01             ` Ulf Hansson [this message]
2018-12-28 19:59               ` Tony Lindgren
2019-01-02 12:01                 ` Ulf Hansson
2019-01-07 15:18                   ` Tony Lindgren
2019-01-07 16:12                     ` Kalle Valo
2019-01-14 13:47                       ` Ulf Hansson
2018-12-28  9:48           ` Ulf Hansson

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='CAPDyKFoEOGACs4Wnm7UgKab2b1-7+v8+31DpC+p=8XxcQhSpcQ@mail.gmail.com' \
    --to=ulf.hansson@linaro.org \
    --cc=anders.roxell@linaro.org \
    --cc=eyalr@ti.com \
    --cc=guym@ti.com \
    --cc=john.stultz@linaro.org \
    --cc=kishon@ti.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=rsalveti@rsalveti.net \
    --cc=tony@atomide.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).