All of lore.kernel.org
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Linux MMC List <linux-mmc@vger.kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Matthias Kaehlcke <mka@chromium.org>,
	Kalle Valo <kvalo@codeaurora.org>,
	Tony Lindgren <tony@atomide.com>, Wen Gong <wgong@codeaurora.org>,
	Erik Stromdahl <erik.stromdahl@gmail.com>,
	Eyal Reizer <eyalreizer@gmail.com>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	Brian Norris <briannorris@chromium.org>
Subject: Re: [PATCH v2 0/3] mmc: Fixup HW reset for SDIO cards
Date: Mon, 11 Nov 2019 16:51:10 -0800	[thread overview]
Message-ID: <CAD=FV=Wv9DgzQZZE8YvB+qjBzPsKdJvafSnFy8YAN_dN6UJbtQ@mail.gmail.com> (raw)
In-Reply-To: <20191109103046.26445-1-ulf.hansson@linaro.org>

Hi,

On Sat, Nov 9, 2019 at 2:31 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Changes in v2:
>         - Add adaptations to the mwifiex driver.
>         - Keep existing syncronous reset behaviour if the SDIO card has a single
>         func driver.
>
> It has turned out that it's not a good idea to try to power cycle and to
> re-initialize the SDIO card, as currently done through mmc_hw_reset(). This
> because there may be multiple SDIO funcs attached to the same SDIO card and
> some of the others that didn't execute the call to mmc_hw_reset(), may then
> simply experience an undefined behaviour.
>
> The following patches in this series attempts to address this problem, by
> reworking the mmc_hw_reset() behaviour for SDIO and by adopting the Marvel
> mwifiex driver to these changes.
>
> Note that, I don't have the HW at hand so the the code has only compile tested.
> Test on HW is greatly appreciated!
>
> Ulf Hansson (3):
>   mwifiex: Re-work support for SDIO HW reset
>   mmc: core: Drop check for mmc_card_is_removable() in mmc_rescan()
>   mmc: core: Re-work HW reset for SDIO cards
>
>  drivers/mmc/core/core.c                     | 12 +++-----
>  drivers/mmc/core/core.h                     |  2 ++
>  drivers/mmc/core/sdio.c                     | 28 ++++++++++++++++-
>  drivers/mmc/core/sdio_bus.c                 |  9 +++++-
>  drivers/net/wireless/marvell/mwifiex/main.c |  6 +++-
>  drivers/net/wireless/marvell/mwifiex/main.h |  1 +
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 33 ++++++++++++++-------
>  include/linux/mmc/card.h                    |  1 +
>  8 files changed, 70 insertions(+), 22 deletions(-)

I put this on rk3288-veyron-jerry atop v5.4-rc7 and I could run my
test case for a while, AKA I got over 50 cycles of:

---

for i in $(seq 1000); do
  echo "LOOP $i --------"
  echo 1 > /sys/kernel/debug/mwifiex/mlan0/reset

  while true; do
    if ! ping6 -w15 -c1 "${GW}" >/dev/null 2>&1; then
      fail=$(( fail + 1 ))
      echo "Fail WiFi ${fail}"
      if [[ ${fail} == 3 ]]; then
        exit 1
      fi
    else
      fail=0
      break
    fi
  done

  hciconfig hci0 down
  sleep 1
  if ! hciconfig hci0 up; then
    echo "Fail BT"
    exit 1
  fi

done

---

NOTE: with no patches I couldn't even get my test case to pass w/out
the BT bits and I swear that used to work before.  ...but I didn't
debug since the end result (with full card hotplug) is happy-working
for me.  I'll still use it as further argument that (IMO) full unplug
/ plug of the card is better it uses more standard code paths and is
less likely to break.  ;-)

Tested-by: Douglas Anderson <dianders@chromium.org>

WARNING: multiple messages have this Message-ID (diff)
From: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Linux MMC List
	<linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Adrian Hunter
	<adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>,
	Wen Gong <wgong-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Erik Stromdahl
	<erik.stromdahl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Eyal Reizer <eyalreizer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	linux-wireless
	<linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Brian Norris
	<briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Subject: Re: [PATCH v2 0/3] mmc: Fixup HW reset for SDIO cards
Date: Mon, 11 Nov 2019 16:51:10 -0800	[thread overview]
Message-ID: <CAD=FV=Wv9DgzQZZE8YvB+qjBzPsKdJvafSnFy8YAN_dN6UJbtQ@mail.gmail.com> (raw)
In-Reply-To: <20191109103046.26445-1-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Hi,

On Sat, Nov 9, 2019 at 2:31 AM Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
> Changes in v2:
>         - Add adaptations to the mwifiex driver.
>         - Keep existing syncronous reset behaviour if the SDIO card has a single
>         func driver.
>
> It has turned out that it's not a good idea to try to power cycle and to
> re-initialize the SDIO card, as currently done through mmc_hw_reset(). This
> because there may be multiple SDIO funcs attached to the same SDIO card and
> some of the others that didn't execute the call to mmc_hw_reset(), may then
> simply experience an undefined behaviour.
>
> The following patches in this series attempts to address this problem, by
> reworking the mmc_hw_reset() behaviour for SDIO and by adopting the Marvel
> mwifiex driver to these changes.
>
> Note that, I don't have the HW at hand so the the code has only compile tested.
> Test on HW is greatly appreciated!
>
> Ulf Hansson (3):
>   mwifiex: Re-work support for SDIO HW reset
>   mmc: core: Drop check for mmc_card_is_removable() in mmc_rescan()
>   mmc: core: Re-work HW reset for SDIO cards
>
>  drivers/mmc/core/core.c                     | 12 +++-----
>  drivers/mmc/core/core.h                     |  2 ++
>  drivers/mmc/core/sdio.c                     | 28 ++++++++++++++++-
>  drivers/mmc/core/sdio_bus.c                 |  9 +++++-
>  drivers/net/wireless/marvell/mwifiex/main.c |  6 +++-
>  drivers/net/wireless/marvell/mwifiex/main.h |  1 +
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 33 ++++++++++++++-------
>  include/linux/mmc/card.h                    |  1 +
>  8 files changed, 70 insertions(+), 22 deletions(-)

I put this on rk3288-veyron-jerry atop v5.4-rc7 and I could run my
test case for a while, AKA I got over 50 cycles of:

---

for i in $(seq 1000); do
  echo "LOOP $i --------"
  echo 1 > /sys/kernel/debug/mwifiex/mlan0/reset

  while true; do
    if ! ping6 -w15 -c1 "${GW}" >/dev/null 2>&1; then
      fail=$(( fail + 1 ))
      echo "Fail WiFi ${fail}"
      if [[ ${fail} == 3 ]]; then
        exit 1
      fi
    else
      fail=0
      break
    fi
  done

  hciconfig hci0 down
  sleep 1
  if ! hciconfig hci0 up; then
    echo "Fail BT"
    exit 1
  fi

done

---

NOTE: with no patches I couldn't even get my test case to pass w/out
the BT bits and I swear that used to work before.  ...but I didn't
debug since the end result (with full card hotplug) is happy-working
for me.  I'll still use it as further argument that (IMO) full unplug
/ plug of the card is better it uses more standard code paths and is
less likely to break.  ;-)

Tested-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

  parent reply	other threads:[~2019-11-12  0:51 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-09 10:30 [PATCH v2 0/3] mmc: Fixup HW reset for SDIO cards Ulf Hansson
2019-11-09 10:30 ` Ulf Hansson
2019-11-09 10:30 ` [PATCH v2 1/3] mwifiex: Re-work support for SDIO HW reset Ulf Hansson
2019-11-09 10:30   ` Ulf Hansson
2019-11-12  0:33   ` Doug Anderson
2019-11-12  0:33     ` Doug Anderson
2019-11-12 12:13     ` Ulf Hansson
2019-11-12 12:13       ` Ulf Hansson
2019-11-12 18:04       ` Doug Anderson
2019-11-12 18:04         ` Doug Anderson
2019-11-13 15:00         ` Ulf Hansson
2019-11-13 15:00           ` Ulf Hansson
2019-11-09 10:30 ` [PATCH v2 2/3] mmc: core: Drop check for mmc_card_is_removable() in mmc_rescan() Ulf Hansson
2019-11-09 10:30   ` Ulf Hansson
2019-11-09 10:30 ` [PATCH v2 3/3] mmc: core: Re-work HW reset for SDIO cards Ulf Hansson
2019-11-09 10:30   ` Ulf Hansson
2019-11-12  0:33   ` Doug Anderson
2019-11-12  0:33     ` Doug Anderson
2019-11-12 12:19     ` Ulf Hansson
2019-11-12 12:19       ` Ulf Hansson
2019-11-20  6:28       ` Kalle Valo
2019-11-20  6:28         ` Kalle Valo
2019-11-20  6:28       ` Kalle Valo
2019-11-20  6:28         ` Kalle Valo
     [not found]       ` <87zhgr5af6.fsf@codeaurora.org>
2019-11-20  7:10         ` wgong
2019-11-20  7:10           ` wgong
2019-11-20  7:10         ` wgong
2019-11-20  7:10           ` wgong-sgV2jX0FEOL9JmXXK+q4OQ
     [not found]         ` <6e6b53b28581a8f1a2944ca0bc65311e@codeaurora.org>
2019-11-20  7:20           ` Kalle Valo
2019-11-20  7:20             ` Kalle Valo
2019-11-20  7:20           ` Kalle Valo
2019-11-20  7:20             ` Kalle Valo
     [not found]           ` <0101016e87aeb8b6-761ad812-5da7-4b0d-8cae-c69633d90de0-000000@us-west-2.amazonses.com>
2019-11-20 12:10             ` Ulf Hansson
2019-11-20 12:10               ` Ulf Hansson
2019-11-20 12:10               ` Ulf Hansson
2019-11-20 16:41               ` Kalle Valo
2019-11-20 16:41                 ` Kalle Valo
2019-11-20 16:41               ` Kalle Valo
2019-11-20 16:41                 ` Kalle Valo
     [not found]               ` <87zhgqmref.fsf@kamboji.qca.qualcomm.com>
2019-11-21  2:29                 ` wgong
2019-11-21  2:29                   ` wgong-sgV2jX0FEOL9JmXXK+q4OQ
2019-11-21  2:29                 ` wgong
2019-11-21  2:29                   ` wgong
2019-11-11 22:08 ` [PATCH v2 0/3] mmc: Fixup " Tony Lindgren
2019-11-11 22:08   ` Tony Lindgren
2019-11-12 12:23   ` Ulf Hansson
2019-11-12 12:23     ` Ulf Hansson
2019-11-12 18:00     ` Tony Lindgren
2019-11-12 18:00       ` Tony Lindgren
2019-11-12  0:51 ` Doug Anderson [this message]
2019-11-12  0:51   ` Doug Anderson
2019-11-12 12:27   ` Ulf Hansson
2019-11-12 12:27     ` Ulf Hansson
2019-11-12 17:42     ` Doug Anderson
2019-11-12 17:42       ` Doug Anderson
2019-11-13 15:11       ` Ulf Hansson
2019-11-13 15:11         ` 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='CAD=FV=Wv9DgzQZZE8YvB+qjBzPsKdJvafSnFy8YAN_dN6UJbtQ@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=adrian.hunter@intel.com \
    --cc=briannorris@chromium.org \
    --cc=erik.stromdahl@gmail.com \
    --cc=eyalreizer@gmail.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=tony@atomide.com \
    --cc=ulf.hansson@linaro.org \
    --cc=wgong@codeaurora.org \
    /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 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.