* [PATCH v2 0/1] mmc: sdhci-brcmstb: check R1_STATUS for erase/trim/discard
@ 2024-06-03 22:08 Kamal Dasu
2024-06-03 22:08 ` [PATCH v2 1/1] " Kamal Dasu
0 siblings, 1 reply; 4+ messages in thread
From: Kamal Dasu @ 2024-06-03 22:08 UTC (permalink / raw)
To: ulf.hansson, linux-kernel, linux-mmc, ludovic.barre
Cc: f.fainelli, bcm-kernel-feedback-list, Kamal Dasu
[-- Attachment #1: Type: text/plain, Size: 747 bytes --]
v1 change
Modified the core mmc_ops to not use card_busy signal checkin fixes the
issue observed on brcmstb mmc host controller when using BLKSECDISCARD ioctl
during erase operation.
v2 change
The v1 changes [1] modified core mmc file to fix the issue however based on
testing it appears to be related to brcmstb hardware behavior with the card
busy signal not getting pulled up after secure discard erase operation. The
fix would be more appropriate in sdhci-brcmstb host controller driver to
inhibit using card_busy() signal.
[1] https://lore.kernel.org/linux-mmc/202404232
Kamal Dasu (1):
mmc: sdhci-brcmstb: check R1_STATUS for erase/trim/discard
drivers/mmc/host/sdhci-brcmstb.c | 4 ++++
1 file changed, 4 insertions(+)
--
2.17.1
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4203 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] mmc: sdhci-brcmstb: check R1_STATUS for erase/trim/discard
2024-06-03 22:08 [PATCH v2 0/1] mmc: sdhci-brcmstb: check R1_STATUS for erase/trim/discard Kamal Dasu
@ 2024-06-03 22:08 ` Kamal Dasu
2024-06-04 11:13 ` Ulf Hansson
0 siblings, 1 reply; 4+ messages in thread
From: Kamal Dasu @ 2024-06-03 22:08 UTC (permalink / raw)
To: ulf.hansson, linux-kernel, linux-mmc, ludovic.barre
Cc: f.fainelli, bcm-kernel-feedback-list, Kamal Dasu
[-- Attachment #1: Type: text/plain, Size: 3628 bytes --]
When erase/trim/discard completion was converted to mmc_poll_for_busy(),
optional ->card_busy() host ops support was added as part of
dd0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for erase/trim/discard").
sdhci card->busy() could return busy for long periods to cause
mmc_do_erase() to block during discard operation as shown below
during mkfs.f2fs :
Info: [/dev/mmcblk1p9] Discarding device
[ 39.597258] sysrq: Show Blocked State
[ 39.601183] task:mkfs.f2fs state:D stack:0 pid:1561 tgid:1561 ppid:1542 flags:0x0000000d
[ 39.610609] Call trace:
[ 39.613098] __switch_to+0xd8/0xf4
[ 39.616582] __schedule+0x440/0x4f4
[ 39.620137] schedule+0x2c/0x48
[ 39.623341] schedule_hrtimeout_range_clock+0xe0/0x114
[ 39.628562] schedule_hrtimeout_range+0x10/0x18
[ 39.633169] usleep_range_state+0x5c/0x90
[ 39.637253] __mmc_poll_for_busy+0xec/0x128
[ 39.641514] mmc_poll_for_busy+0x48/0x70
[ 39.645511] mmc_do_erase+0x1ec/0x210
[ 39.649237] mmc_erase+0x1b4/0x1d4
[ 39.652701] mmc_blk_mq_issue_rq+0x35c/0x6ac
[ 39.657037] mmc_mq_queue_rq+0x18c/0x214
[ 39.661022] blk_mq_dispatch_rq_list+0x3a8/0x528
[ 39.665722] __blk_mq_sched_dispatch_requests+0x3a0/0x4ac
[ 39.671198] blk_mq_sched_dispatch_requests+0x28/0x5c
[ 39.676322] blk_mq_run_hw_queue+0x11c/0x12c
[ 39.680668] blk_mq_flush_plug_list+0x200/0x33c
[ 39.685278] blk_add_rq_to_plug+0x68/0xd8
[ 39.689365] blk_mq_submit_bio+0x3a4/0x458
[ 39.693539] __submit_bio+0x1c/0x80
[ 39.697096] submit_bio_noacct_nocheck+0x94/0x174
[ 39.701875] submit_bio_noacct+0x1b0/0x22c
[ 39.706042] submit_bio+0xac/0xe8
[ 39.709424] blk_next_bio+0x4c/0x5c
[ 39.712973] blkdev_issue_secure_erase+0x118/0x170
[ 39.717835] blkdev_common_ioctl+0x374/0x728
[ 39.722175] blkdev_ioctl+0x8c/0x2b0
[ 39.725816] vfs_ioctl+0x24/0x40
[ 39.729117] __arm64_sys_ioctl+0x5c/0x8c
[ 39.733114] invoke_syscall+0x68/0xec
[ 39.736839] el0_svc_common.constprop.0+0x70/0xd8
[ 39.741609] do_el0_svc+0x18/0x20
[ 39.744981] el0_svc+0x68/0x94
[ 39.748107] el0t_64_sync_handler+0x88/0x124
[ 39.752455] el0t_64_sync+0x168/0x16c
This problem is obsereved with BLKSECDISCARD ioctl on brcmstb mmc
controllers. Fix makes mmc_host_ops.card_busy NULL and forces
MMC_SEND_STATUS and R1_STATUS check in mmc_busy_cb() function.
Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
---
drivers/mmc/host/sdhci-brcmstb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
index 9053526fa212..150fb477b7cc 100644
--- a/drivers/mmc/host/sdhci-brcmstb.c
+++ b/drivers/mmc/host/sdhci-brcmstb.c
@@ -24,6 +24,7 @@
#define BRCMSTB_MATCH_FLAGS_NO_64BIT BIT(0)
#define BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT BIT(1)
#define BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE BIT(2)
+#define BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY BIT(4)
#define BRCMSTB_PRIV_FLAGS_HAS_CQE BIT(0)
#define BRCMSTB_PRIV_FLAGS_GATE_CLOCK BIT(1)
@@ -384,6 +385,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
+ if (!(match_priv->flags & BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY))
+ host->mmc_host_ops.card_busy = NULL;
+
/* Change the base clock frequency if the DT property exists */
if (device_property_read_u32(&pdev->dev, "clock-frequency",
&priv->base_freq_hz) != 0)
--
2.17.1
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4203 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2 1/1] mmc: sdhci-brcmstb: check R1_STATUS for erase/trim/discard
2024-06-03 22:08 ` [PATCH v2 1/1] " Kamal Dasu
@ 2024-06-04 11:13 ` Ulf Hansson
2024-06-04 15:42 ` Kamal Dasu
0 siblings, 1 reply; 4+ messages in thread
From: Ulf Hansson @ 2024-06-04 11:13 UTC (permalink / raw)
To: Kamal Dasu
Cc: linux-kernel, linux-mmc, ludovic.barre, f.fainelli,
bcm-kernel-feedback-list
On Tue, 4 Jun 2024 at 00:09, Kamal Dasu <kamal.dasu@broadcom.com> wrote:
>
> When erase/trim/discard completion was converted to mmc_poll_for_busy(),
> optional ->card_busy() host ops support was added as part of
> dd0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for erase/trim/discard").
I can't find the above commit hash. You probably want "0d84c3e6a5b2"?
> sdhci card->busy() could return busy for long periods to cause
> mmc_do_erase() to block during discard operation as shown below
> during mkfs.f2fs :
>
> Info: [/dev/mmcblk1p9] Discarding device
> [ 39.597258] sysrq: Show Blocked State
> [ 39.601183] task:mkfs.f2fs state:D stack:0 pid:1561 tgid:1561 ppid:1542 flags:0x0000000d
> [ 39.610609] Call trace:
> [ 39.613098] __switch_to+0xd8/0xf4
> [ 39.616582] __schedule+0x440/0x4f4
> [ 39.620137] schedule+0x2c/0x48
> [ 39.623341] schedule_hrtimeout_range_clock+0xe0/0x114
> [ 39.628562] schedule_hrtimeout_range+0x10/0x18
> [ 39.633169] usleep_range_state+0x5c/0x90
> [ 39.637253] __mmc_poll_for_busy+0xec/0x128
> [ 39.641514] mmc_poll_for_busy+0x48/0x70
> [ 39.645511] mmc_do_erase+0x1ec/0x210
> [ 39.649237] mmc_erase+0x1b4/0x1d4
> [ 39.652701] mmc_blk_mq_issue_rq+0x35c/0x6ac
> [ 39.657037] mmc_mq_queue_rq+0x18c/0x214
> [ 39.661022] blk_mq_dispatch_rq_list+0x3a8/0x528
> [ 39.665722] __blk_mq_sched_dispatch_requests+0x3a0/0x4ac
> [ 39.671198] blk_mq_sched_dispatch_requests+0x28/0x5c
> [ 39.676322] blk_mq_run_hw_queue+0x11c/0x12c
> [ 39.680668] blk_mq_flush_plug_list+0x200/0x33c
> [ 39.685278] blk_add_rq_to_plug+0x68/0xd8
> [ 39.689365] blk_mq_submit_bio+0x3a4/0x458
> [ 39.693539] __submit_bio+0x1c/0x80
> [ 39.697096] submit_bio_noacct_nocheck+0x94/0x174
> [ 39.701875] submit_bio_noacct+0x1b0/0x22c
> [ 39.706042] submit_bio+0xac/0xe8
> [ 39.709424] blk_next_bio+0x4c/0x5c
> [ 39.712973] blkdev_issue_secure_erase+0x118/0x170
> [ 39.717835] blkdev_common_ioctl+0x374/0x728
> [ 39.722175] blkdev_ioctl+0x8c/0x2b0
> [ 39.725816] vfs_ioctl+0x24/0x40
> [ 39.729117] __arm64_sys_ioctl+0x5c/0x8c
> [ 39.733114] invoke_syscall+0x68/0xec
> [ 39.736839] el0_svc_common.constprop.0+0x70/0xd8
> [ 39.741609] do_el0_svc+0x18/0x20
> [ 39.744981] el0_svc+0x68/0x94
> [ 39.748107] el0t_64_sync_handler+0x88/0x124
> [ 39.752455] el0t_64_sync+0x168/0x16c
>
> This problem is obsereved with BLKSECDISCARD ioctl on brcmstb mmc
> controllers. Fix makes mmc_host_ops.card_busy NULL and forces
> MMC_SEND_STATUS and R1_STATUS check in mmc_busy_cb() function.
>
> Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
We probably want a fixes/stable tag for this too, right?
Fixes: 0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for
erase/trim/discard")
I have amended the commit message and applied this for fixes, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-brcmstb.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> index 9053526fa212..150fb477b7cc 100644
> --- a/drivers/mmc/host/sdhci-brcmstb.c
> +++ b/drivers/mmc/host/sdhci-brcmstb.c
> @@ -24,6 +24,7 @@
> #define BRCMSTB_MATCH_FLAGS_NO_64BIT BIT(0)
> #define BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT BIT(1)
> #define BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE BIT(2)
> +#define BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY BIT(4)
>
> #define BRCMSTB_PRIV_FLAGS_HAS_CQE BIT(0)
> #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK BIT(1)
> @@ -384,6 +385,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
> host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>
> + if (!(match_priv->flags & BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY))
> + host->mmc_host_ops.card_busy = NULL;
> +
> /* Change the base clock frequency if the DT property exists */
> if (device_property_read_u32(&pdev->dev, "clock-frequency",
> &priv->base_freq_hz) != 0)
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2 1/1] mmc: sdhci-brcmstb: check R1_STATUS for erase/trim/discard
2024-06-04 11:13 ` Ulf Hansson
@ 2024-06-04 15:42 ` Kamal Dasu
0 siblings, 0 replies; 4+ messages in thread
From: Kamal Dasu @ 2024-06-04 15:42 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-kernel, linux-mmc, ludovic.barre, f.fainelli,
bcm-kernel-feedback-list
[-- Attachment #1: Type: text/plain, Size: 4674 bytes --]
On Tue, Jun 4, 2024 at 7:14 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Tue, 4 Jun 2024 at 00:09, Kamal Dasu <kamal.dasu@broadcom.com> wrote:
> >
> > When erase/trim/discard completion was converted to mmc_poll_for_busy(),
> > optional ->card_busy() host ops support was added as part of
> > dd0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for erase/trim/discard").
>
> I can't find the above commit hash. You probably want "0d84c3e6a5b2"?
>
> > sdhci card->busy() could return busy for long periods to cause
> > mmc_do_erase() to block during discard operation as shown below
> > during mkfs.f2fs :
> >
> > Info: [/dev/mmcblk1p9] Discarding device
> > [ 39.597258] sysrq: Show Blocked State
> > [ 39.601183] task:mkfs.f2fs state:D stack:0 pid:1561 tgid:1561 ppid:1542 flags:0x0000000d
> > [ 39.610609] Call trace:
> > [ 39.613098] __switch_to+0xd8/0xf4
> > [ 39.616582] __schedule+0x440/0x4f4
> > [ 39.620137] schedule+0x2c/0x48
> > [ 39.623341] schedule_hrtimeout_range_clock+0xe0/0x114
> > [ 39.628562] schedule_hrtimeout_range+0x10/0x18
> > [ 39.633169] usleep_range_state+0x5c/0x90
> > [ 39.637253] __mmc_poll_for_busy+0xec/0x128
> > [ 39.641514] mmc_poll_for_busy+0x48/0x70
> > [ 39.645511] mmc_do_erase+0x1ec/0x210
> > [ 39.649237] mmc_erase+0x1b4/0x1d4
> > [ 39.652701] mmc_blk_mq_issue_rq+0x35c/0x6ac
> > [ 39.657037] mmc_mq_queue_rq+0x18c/0x214
> > [ 39.661022] blk_mq_dispatch_rq_list+0x3a8/0x528
> > [ 39.665722] __blk_mq_sched_dispatch_requests+0x3a0/0x4ac
> > [ 39.671198] blk_mq_sched_dispatch_requests+0x28/0x5c
> > [ 39.676322] blk_mq_run_hw_queue+0x11c/0x12c
> > [ 39.680668] blk_mq_flush_plug_list+0x200/0x33c
> > [ 39.685278] blk_add_rq_to_plug+0x68/0xd8
> > [ 39.689365] blk_mq_submit_bio+0x3a4/0x458
> > [ 39.693539] __submit_bio+0x1c/0x80
> > [ 39.697096] submit_bio_noacct_nocheck+0x94/0x174
> > [ 39.701875] submit_bio_noacct+0x1b0/0x22c
> > [ 39.706042] submit_bio+0xac/0xe8
> > [ 39.709424] blk_next_bio+0x4c/0x5c
> > [ 39.712973] blkdev_issue_secure_erase+0x118/0x170
> > [ 39.717835] blkdev_common_ioctl+0x374/0x728
> > [ 39.722175] blkdev_ioctl+0x8c/0x2b0
> > [ 39.725816] vfs_ioctl+0x24/0x40
> > [ 39.729117] __arm64_sys_ioctl+0x5c/0x8c
> > [ 39.733114] invoke_syscall+0x68/0xec
> > [ 39.736839] el0_svc_common.constprop.0+0x70/0xd8
> > [ 39.741609] do_el0_svc+0x18/0x20
> > [ 39.744981] el0_svc+0x68/0x94
> > [ 39.748107] el0t_64_sync_handler+0x88/0x124
> > [ 39.752455] el0t_64_sync+0x168/0x16c
> >
> > This problem is obsereved with BLKSECDISCARD ioctl on brcmstb mmc
> > controllers. Fix makes mmc_host_ops.card_busy NULL and forces
> > MMC_SEND_STATUS and R1_STATUS check in mmc_busy_cb() function.
> >
> > Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
>
> We probably want a fixes/stable tag for this too, right?
>
> Fixes: 0d84c3e6a5b2 ("mmc: core: Convert to mmc_poll_for_busy() for
> erase/trim/discard")
>
> I have amended the commit message and applied this for fixes, thanks!
>
Thank you Ulffe.
> Kind regards
> Uffe
>
>
> > ---
> > drivers/mmc/host/sdhci-brcmstb.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> > index 9053526fa212..150fb477b7cc 100644
> > --- a/drivers/mmc/host/sdhci-brcmstb.c
> > +++ b/drivers/mmc/host/sdhci-brcmstb.c
> > @@ -24,6 +24,7 @@
> > #define BRCMSTB_MATCH_FLAGS_NO_64BIT BIT(0)
> > #define BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT BIT(1)
> > #define BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE BIT(2)
> > +#define BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY BIT(4)
> >
> > #define BRCMSTB_PRIV_FLAGS_HAS_CQE BIT(0)
> > #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK BIT(1)
> > @@ -384,6 +385,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> > if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
> > host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> >
> > + if (!(match_priv->flags & BRCMSTB_MATCH_FLAGS_USE_CARD_BUSY))
> > + host->mmc_host_ops.card_busy = NULL;
> > +
> > /* Change the base clock frequency if the DT property exists */
> > if (device_property_read_u32(&pdev->dev, "clock-frequency",
> > &priv->base_freq_hz) != 0)
> > --
> > 2.17.1
> >
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4203 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-06-04 15:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-03 22:08 [PATCH v2 0/1] mmc: sdhci-brcmstb: check R1_STATUS for erase/trim/discard Kamal Dasu
2024-06-03 22:08 ` [PATCH v2 1/1] " Kamal Dasu
2024-06-04 11:13 ` Ulf Hansson
2024-06-04 15:42 ` Kamal Dasu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox