* [PATCH 0/1] Bluetooth: btmrvl: Reset SDIO card on hang @ 2020-03-20 2:01 Abhishek Pandit-Subedi 2020-03-20 2:01 ` [PATCH 1/1] Bluetooth: btmrvl: Detect hangs and force a reset of the SDIO card Abhishek Pandit-Subedi 0 siblings, 1 reply; 4+ messages in thread From: Abhishek Pandit-Subedi @ 2020-03-20 2:01 UTC (permalink / raw) To: marcel, linux-bluetooth, ulf.hansson Cc: chromeos-bluetooth-upstreaming, mka, dianders, Abhishek Pandit-Subedi, Johan Hedberg, linux-kernel Hi Marcel, This patch adds an error recovery mechanism to the btmrvl driver. We have been using it on ChromeOS on kernel v4.19 and it has been on the stable channel release of ChromeOS for a few months now. A side effect of this change is that other functions on the same card will be affected by the reset. The necessary patches to support this reset have already been added to the mwifiex driver by Ulf: 11239229 New [v3,1/3] mwifiex: Re-work support for SDIO HW reset 11239233 New [v3,2/3] mmc: core: Drop check for mmc_card_is_removable() in mmc_rescan() 11239237 New [v3,3/3] mmc: core: Re-work HW reset for SDIO cards You can see more information at crbug.com/1004473 Thanks Abhishek Matthias Kaehlcke (1): Bluetooth: btmrvl: Detect hangs and force a reset of the SDIO card drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++++ drivers/bluetooth/btmrvl_sdio.h | 1 + 2 files changed, 25 insertions(+) -- 2.25.1.696.g5e7596f4ac-goog ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] Bluetooth: btmrvl: Detect hangs and force a reset of the SDIO card 2020-03-20 2:01 [PATCH 0/1] Bluetooth: btmrvl: Reset SDIO card on hang Abhishek Pandit-Subedi @ 2020-03-20 2:01 ` Abhishek Pandit-Subedi 2020-03-20 20:00 ` Doug Anderson 0 siblings, 1 reply; 4+ messages in thread From: Abhishek Pandit-Subedi @ 2020-03-20 2:01 UTC (permalink / raw) To: marcel, linux-bluetooth, ulf.hansson Cc: chromeos-bluetooth-upstreaming, mka, dianders, Abhishek Pandit-Subedi, Johan Hedberg, linux-kernel From: Matthias Kaehlcke <mka@chromium.org> When scanning for BLE devices for a longer period (e.g. because a BLE device is paired, but not connected) the Marvell 8997 often ends up in a borked state, which manifests through failures on certain SDIO transactions. When such a SDIO failure is detected force a reset of the SDIO card to initialize it from scratch. Since the SDIO bus is shared with the WiFi part of the chip this also involves a reset of WiFi. Signed-off-by: Matthias Kaehlcke <mka@chromium.org> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> --- drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++++ drivers/bluetooth/btmrvl_sdio.h | 1 + 2 files changed, 25 insertions(+) diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c index 0f3a020703ab..69a8b6b3c11c 100644 --- a/drivers/bluetooth/btmrvl_sdio.c +++ b/drivers/bluetooth/btmrvl_sdio.c @@ -22,6 +22,8 @@ #include <linux/slab.h> #include <linux/suspend.h> +#include <linux/mmc/core.h> +#include <linux/mmc/card.h> #include <linux/mmc/sdio_ids.h> #include <linux/mmc/sdio_func.h> #include <linux/module.h> @@ -59,6 +61,23 @@ static const struct of_device_id btmrvl_sdio_of_match_table[] = { { } }; +static void btmrvl_sdio_card_reset_work(struct work_struct *work) +{ + struct btmrvl_sdio_card *card = + container_of(work, struct btmrvl_sdio_card, reset_work); + struct sdio_func *func = card->func; + + sdio_claim_host(func); + mmc_hw_reset(func->card->host); + sdio_release_host(func); +} + +static void btmrvl_sdio_card_reset(struct btmrvl_sdio_card *card) +{ + BT_ERR("resetting SDIO card!"); + schedule_work(&card->reset_work); +} + static irqreturn_t btmrvl_wake_irq_bt(int irq, void *priv) { struct btmrvl_sdio_card *card = priv; @@ -774,6 +793,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv) ret = btmrvl_sdio_read_rx_len(card, &buf_len); if (ret < 0) { BT_ERR("read rx_len failed"); + btmrvl_sdio_card_reset(card); ret = -EIO; goto exit; } @@ -809,6 +829,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv) num_blocks * blksz); if (ret < 0) { BT_ERR("readsb failed: %d", ret); + btmrvl_sdio_card_reset(card); ret = -EIO; goto exit; } @@ -913,6 +934,7 @@ static int btmrvl_sdio_read_to_clear(struct btmrvl_sdio_card *card, u8 *ireg) ret = sdio_readsb(card->func, adapter->hw_regs, 0, SDIO_BLOCK_SIZE); if (ret) { BT_ERR("sdio_readsb: read int hw_regs failed: %d", ret); + btmrvl_sdio_card_reset(card); return ret; } @@ -1591,6 +1613,8 @@ static int btmrvl_sdio_probe(struct sdio_func *func, card->supports_fw_dump = data->supports_fw_dump; } + INIT_WORK(&card->reset_work, btmrvl_sdio_card_reset_work); + if (btmrvl_sdio_register_dev(card) < 0) { BT_ERR("Failed to register BT device!"); return -ENODEV; diff --git a/drivers/bluetooth/btmrvl_sdio.h b/drivers/bluetooth/btmrvl_sdio.h index 3a522d23ee6e..d35a2f1b7046 100644 --- a/drivers/bluetooth/btmrvl_sdio.h +++ b/drivers/bluetooth/btmrvl_sdio.h @@ -103,6 +103,7 @@ struct btmrvl_sdio_card { struct btmrvl_private *priv; struct device_node *plt_of_node; struct btmrvl_plt_wake_cfg *plt_wake_cfg; + struct work_struct reset_work; }; struct btmrvl_sdio_device { -- 2.25.1.696.g5e7596f4ac-goog ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] Bluetooth: btmrvl: Detect hangs and force a reset of the SDIO card 2020-03-20 2:01 ` [PATCH 1/1] Bluetooth: btmrvl: Detect hangs and force a reset of the SDIO card Abhishek Pandit-Subedi @ 2020-03-20 20:00 ` Doug Anderson 2020-03-20 20:56 ` Abhishek Pandit-Subedi 0 siblings, 1 reply; 4+ messages in thread From: Doug Anderson @ 2020-03-20 20:00 UTC (permalink / raw) To: Abhishek Pandit-Subedi Cc: Marcel Holtmann, BlueZ, Ulf Hansson, chromeos-bluetooth-upstreaming, Matthias Kaehlcke, Johan Hedberg, LKML Hi, On Thu, Mar 19, 2020 at 7:02 PM Abhishek Pandit-Subedi <abhishekpandit@chromium.org> wrote: > > From: Matthias Kaehlcke <mka@chromium.org> > > When scanning for BLE devices for a longer period (e.g. because a > BLE device is paired, but not connected) the Marvell 8997 often > ends up in a borked state, which manifests through failures on > certain SDIO transactions. > > When such a SDIO failure is detected force a reset of the SDIO > card to initialize it from scratch. Since the SDIO bus is shared > with the WiFi part of the chip this also involves a reset of WiFi. > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > --- > > drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++++ > drivers/bluetooth/btmrvl_sdio.h | 1 + > 2 files changed, 25 insertions(+) > > diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c > index 0f3a020703ab..69a8b6b3c11c 100644 > --- a/drivers/bluetooth/btmrvl_sdio.c > +++ b/drivers/bluetooth/btmrvl_sdio.c > @@ -22,6 +22,8 @@ > #include <linux/slab.h> > #include <linux/suspend.h> > > +#include <linux/mmc/core.h> > +#include <linux/mmc/card.h> > #include <linux/mmc/sdio_ids.h> > #include <linux/mmc/sdio_func.h> > #include <linux/module.h> > @@ -59,6 +61,23 @@ static const struct of_device_id btmrvl_sdio_of_match_table[] = { > { } > }; > > +static void btmrvl_sdio_card_reset_work(struct work_struct *work) > +{ > + struct btmrvl_sdio_card *card = > + container_of(work, struct btmrvl_sdio_card, reset_work); > + struct sdio_func *func = card->func; > + > + sdio_claim_host(func); > + mmc_hw_reset(func->card->host); The fact that you don't check the return value here seems like a problem. See specifically how commit cdb2256f795e ("mwifiex: Re-work support for SDIO HW reset") handles it. This is a distinct difference between the solution that we landed in Chrome OS 4.19 and what landed upstream. In Chrome OS 4.19 we went the simple approach and said that there was only one way to reset the card: treat it as a full unplug / replug of the card. ...but upstream adopted a different solution. For upstream if there is only a single function on the SD card it will not trigger a full unplug/replug and it's up to the function driver to re-init itself. This wasn't such a big deal for the WiFi driver since it already had a way to re-init itself (mwifiex_reinit_sw). I don't know how hard it will be for Bluetooth, but that needs to be part of this patch I think? -Doug ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] Bluetooth: btmrvl: Detect hangs and force a reset of the SDIO card 2020-03-20 20:00 ` Doug Anderson @ 2020-03-20 20:56 ` Abhishek Pandit-Subedi 0 siblings, 0 replies; 4+ messages in thread From: Abhishek Pandit-Subedi @ 2020-03-20 20:56 UTC (permalink / raw) To: Doug Anderson Cc: Marcel Holtmann, BlueZ, Ulf Hansson, ChromeOS Bluetooth Upstreaming, Matthias Kaehlcke, Johan Hedberg, LKML Thanks for the heads up Doug. I'll resend the patch once I handle the case where the reset is immediate and not a full unplug/replug. In the meantime, please do not merge this patch. Thanks Abhishek On Fri, Mar 20, 2020 at 1:00 PM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Mar 19, 2020 at 7:02 PM Abhishek Pandit-Subedi > <abhishekpandit@chromium.org> wrote: > > > > From: Matthias Kaehlcke <mka@chromium.org> > > > > When scanning for BLE devices for a longer period (e.g. because a > > BLE device is paired, but not connected) the Marvell 8997 often > > ends up in a borked state, which manifests through failures on > > certain SDIO transactions. > > > > When such a SDIO failure is detected force a reset of the SDIO > > card to initialize it from scratch. Since the SDIO bus is shared > > with the WiFi part of the chip this also involves a reset of WiFi. > > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > > --- > > > > drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++++ > > drivers/bluetooth/btmrvl_sdio.h | 1 + > > 2 files changed, 25 insertions(+) > > > > diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c > > index 0f3a020703ab..69a8b6b3c11c 100644 > > --- a/drivers/bluetooth/btmrvl_sdio.c > > +++ b/drivers/bluetooth/btmrvl_sdio.c > > @@ -22,6 +22,8 @@ > > #include <linux/slab.h> > > #include <linux/suspend.h> > > > > +#include <linux/mmc/core.h> > > +#include <linux/mmc/card.h> > > #include <linux/mmc/sdio_ids.h> > > #include <linux/mmc/sdio_func.h> > > #include <linux/module.h> > > @@ -59,6 +61,23 @@ static const struct of_device_id btmrvl_sdio_of_match_table[] = { > > { } > > }; > > > > +static void btmrvl_sdio_card_reset_work(struct work_struct *work) > > +{ > > + struct btmrvl_sdio_card *card = > > + container_of(work, struct btmrvl_sdio_card, reset_work); > > + struct sdio_func *func = card->func; > > + > > + sdio_claim_host(func); > > + mmc_hw_reset(func->card->host); > > The fact that you don't check the return value here seems like a > problem. See specifically how commit cdb2256f795e ("mwifiex: Re-work > support for SDIO HW reset") handles it. > > This is a distinct difference between the solution that we landed in > Chrome OS 4.19 and what landed upstream. In Chrome OS 4.19 we went > the simple approach and said that there was only one way to reset the > card: treat it as a full unplug / replug of the card. ...but upstream > adopted a different solution. For upstream if there is only a single > function on the SD card it will not trigger a full unplug/replug and > it's up to the function driver to re-init itself. This wasn't such a > big deal for the WiFi driver since it already had a way to re-init > itself (mwifiex_reinit_sw). I don't know how hard it will be for > Bluetooth, but that needs to be part of this patch I think? > > -Doug ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-20 20:56 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-20 2:01 [PATCH 0/1] Bluetooth: btmrvl: Reset SDIO card on hang Abhishek Pandit-Subedi 2020-03-20 2:01 ` [PATCH 1/1] Bluetooth: btmrvl: Detect hangs and force a reset of the SDIO card Abhishek Pandit-Subedi 2020-03-20 20:00 ` Doug Anderson 2020-03-20 20:56 ` Abhishek Pandit-Subedi
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).