* [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
@ 2019-11-12 13:48 Eugeniu Rosca
2019-11-12 20:49 ` Wolfram Sang
0 siblings, 1 reply; 13+ messages in thread
From: Eugeniu Rosca @ 2019-11-12 13:48 UTC (permalink / raw)
To: Wolfram Sang, Yoshihiro Shimoda, Niklas Söderlund, Ulf Hansson
Cc: Geert Uytterhoeven, Simon Horman, linux-mmc, linux-kernel,
linux-renesas-soc, Eugeniu Rosca, Eugeniu Rosca,
Harish Jenny K N, Andrew Gabbasov
From: Harish Jenny K N <harish_kandiga@mentor.com>
Enable MMC_CAP_ERASE capability in the driver to allow
erase/discard/trim requests.
Suggested-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
[erosca: Forward-port and test on v5.4-rc7 using H3ULCB-KF:
"blkdiscard /dev/mmcblk0" passes with this patch applied
and complains otherwise:
"BLKDISCARD ioctl failed: Operation not supported"]
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
---
drivers/mmc/host/renesas_sdhi_internal_dmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index a66f8d6d61d1..61fcbf51c947 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -105,7 +105,7 @@ static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
.capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
+ MMC_CAP_ERASE | MMC_CAP_CMD23,
.capabilities2 = MMC_CAP2_NO_WRITE_PROTECT | MMC_CAP2_MERGE_CAPABLE,
.bus_shift = 2,
.scc_offset = 0x1000,
--
2.24.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-12 13:48 [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs Eugeniu Rosca
@ 2019-11-12 20:49 ` Wolfram Sang
2019-11-14 10:56 ` Ulf Hansson
0 siblings, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2019-11-12 20:49 UTC (permalink / raw)
To: Eugeniu Rosca
Cc: Wolfram Sang, Yoshihiro Shimoda, Niklas Söderlund,
Ulf Hansson, Geert Uytterhoeven, Simon Horman, linux-mmc,
linux-kernel, linux-renesas-soc, Eugeniu Rosca, Harish Jenny K N,
Andrew Gabbasov
[-- Attachment #1: Type: text/plain, Size: 927 bytes --]
On Tue, Nov 12, 2019 at 02:48:08PM +0100, Eugeniu Rosca wrote:
> From: Harish Jenny K N <harish_kandiga@mentor.com>
>
> Enable MMC_CAP_ERASE capability in the driver to allow
> erase/discard/trim requests.
>
> Suggested-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> [erosca: Forward-port and test on v5.4-rc7 using H3ULCB-KF:
> "blkdiscard /dev/mmcblk0" passes with this patch applied
> and complains otherwise:
> "BLKDISCARD ioctl failed: Operation not supported"]
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Looks good to me. Just a generic question, probably more for Ulf:
Why does this CAP_ERASE exist? As I understand, the driver only needs to
set the flag and no further handling is required. Why would a driver not
set this flag and not support erase/trim commands?
Kind regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-12 20:49 ` Wolfram Sang
@ 2019-11-14 10:56 ` Ulf Hansson
2019-11-14 11:37 ` Eugeniu Rosca
0 siblings, 1 reply; 13+ messages in thread
From: Ulf Hansson @ 2019-11-14 10:56 UTC (permalink / raw)
To: Wolfram Sang
Cc: Eugeniu Rosca, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
On Tue, 12 Nov 2019 at 21:49, Wolfram Sang <wsa@the-dreams.de> wrote:
>
> On Tue, Nov 12, 2019 at 02:48:08PM +0100, Eugeniu Rosca wrote:
> > From: Harish Jenny K N <harish_kandiga@mentor.com>
> >
> > Enable MMC_CAP_ERASE capability in the driver to allow
> > erase/discard/trim requests.
> >
> > Suggested-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> > Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> > [erosca: Forward-port and test on v5.4-rc7 using H3ULCB-KF:
> > "blkdiscard /dev/mmcblk0" passes with this patch applied
> > and complains otherwise:
> > "BLKDISCARD ioctl failed: Operation not supported"]
> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
>
> Looks good to me. Just a generic question, probably more for Ulf:
>
> Why does this CAP_ERASE exist? As I understand, the driver only needs to
> set the flag and no further handling is required. Why would a driver not
> set this flag and not support erase/trim commands?
I am working on removing the cap, altogether. Step by step, this is
getting closer now.
The main problem has been about busy detect timeouts, as an erase
command may have a very long busy timeout. On the host side, they
typically need to respect the cmd->busy_timeout for the request, and
if it can't because of some HW limitation, it needs to set
mmc->max_busy_timeout.
Once that is fixed for all, we can drop CAP_ERASE.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-14 10:56 ` Ulf Hansson
@ 2019-11-14 11:37 ` Eugeniu Rosca
2019-11-14 12:48 ` Ulf Hansson
0 siblings, 1 reply; 13+ messages in thread
From: Eugeniu Rosca @ 2019-11-14 11:37 UTC (permalink / raw)
To: Ulf Hansson
Cc: Wolfram Sang, Eugeniu Rosca, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
Hi everyone,
On Thu, Nov 14, 2019 at 11:56:23AM +0100, Ulf Hansson wrote:
> On Tue, 12 Nov 2019 at 21:49, Wolfram Sang <wsa@the-dreams.de> wrote:
> >
> > On Tue, Nov 12, 2019 at 02:48:08PM +0100, Eugeniu Rosca wrote:
> > > From: Harish Jenny K N <harish_kandiga@mentor.com>
> > >
> > > Enable MMC_CAP_ERASE capability in the driver to allow
> > > erase/discard/trim requests.
> > >
> > > Suggested-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> > > Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> > > [erosca: Forward-port and test on v5.4-rc7 using H3ULCB-KF:
> > > "blkdiscard /dev/mmcblk0" passes with this patch applied
> > > and complains otherwise:
> > > "BLKDISCARD ioctl failed: Operation not supported"]
> > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> >
> > Looks good to me. Just a generic question, probably more for Ulf:
> >
> > Why does this CAP_ERASE exist? As I understand, the driver only needs to
> > set the flag and no further handling is required. Why would a driver not
> > set this flag and not support erase/trim commands?
>
> I am working on removing the cap, altogether. Step by step, this is
> getting closer now.
>
> The main problem has been about busy detect timeouts, as an erase
> command may have a very long busy timeout. On the host side, they
> typically need to respect the cmd->busy_timeout for the request, and
> if it can't because of some HW limitation, it needs to set
> mmc->max_busy_timeout.
FWIW we've discussed such concerns internally, based on past commits
which either disable [1-2] busy timeouts or increase their value [3].
To get a feeling if this is relevant for R-Car3, I've run blkdiscard on
a 64 GiB eMMC without noticing any issues on v5.4-rc7. Hopefully this
is sufficient as testing?
>
> Once that is fixed for all, we can drop CAP_ERASE.
>
> Kind regards
> Uffe
[1] 93caf8e69eac76 ("omap_hsmmc: add erase capability")
[2] b13d1f0f9ad64b ("mmc: omap: Add erase capability")
[3] ec30f11e821f2d ("mmc: rtsx_usb: Use the provided busy timeout from the mmc core")
--
Best Regards,
Eugeniu
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-14 11:37 ` Eugeniu Rosca
@ 2019-11-14 12:48 ` Ulf Hansson
2019-11-14 20:15 ` Wolfram Sang
2019-11-14 22:07 ` Eugeniu Rosca
0 siblings, 2 replies; 13+ messages in thread
From: Ulf Hansson @ 2019-11-14 12:48 UTC (permalink / raw)
To: Eugeniu Rosca
Cc: Wolfram Sang, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
On Thu, 14 Nov 2019 at 12:37, Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
>
> Hi everyone,
>
> On Thu, Nov 14, 2019 at 11:56:23AM +0100, Ulf Hansson wrote:
> > On Tue, 12 Nov 2019 at 21:49, Wolfram Sang <wsa@the-dreams.de> wrote:
> > >
> > > On Tue, Nov 12, 2019 at 02:48:08PM +0100, Eugeniu Rosca wrote:
> > > > From: Harish Jenny K N <harish_kandiga@mentor.com>
> > > >
> > > > Enable MMC_CAP_ERASE capability in the driver to allow
> > > > erase/discard/trim requests.
> > > >
> > > > Suggested-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> > > > Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> > > > [erosca: Forward-port and test on v5.4-rc7 using H3ULCB-KF:
> > > > "blkdiscard /dev/mmcblk0" passes with this patch applied
> > > > and complains otherwise:
> > > > "BLKDISCARD ioctl failed: Operation not supported"]
> > > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > >
> > > Looks good to me. Just a generic question, probably more for Ulf:
> > >
> > > Why does this CAP_ERASE exist? As I understand, the driver only needs to
> > > set the flag and no further handling is required. Why would a driver not
> > > set this flag and not support erase/trim commands?
> >
> > I am working on removing the cap, altogether. Step by step, this is
> > getting closer now.
> >
> > The main problem has been about busy detect timeouts, as an erase
> > command may have a very long busy timeout. On the host side, they
> > typically need to respect the cmd->busy_timeout for the request, and
> > if it can't because of some HW limitation, it needs to set
> > mmc->max_busy_timeout.
>
> FWIW we've discussed such concerns internally, based on past commits
> which either disable [1-2] busy timeouts or increase their value [3].
>
> To get a feeling if this is relevant for R-Car3, I've run blkdiscard on
> a 64 GiB eMMC without noticing any issues on v5.4-rc7. Hopefully this
> is sufficient as testing?
Let's first take a step back, because I don't know how the HW busy
detection works for your controller.
I have noticed there is TMIO_STAT_CMD_BUSY bit being set for some
variants, which seems to cause renesas_sdhi_wait_idle() to loop for a
pre-defined number of loops/timeout. This looks scary, but I can't
tell if it's really a problem.
BTW, do you know what TMIO_STAT_CMD_BUSY actually is monitoring?
I have also noticed that MMC_CAP_WAIT_WHILE_BUSY isn't set for any of
the renesas/tmio variant hosts. Is that simply because the HW doesn't
support this? Or because implementation is missing?
If you want to run a test that stretches the behaviour on the timeout
path, I would rather use an SD-card (the older the better). For eMMCs
the erase likely translates to a trim/discard, which is far more
quicker than a real erase - as is what happens on an old SD card.
>
> >
> > Once that is fixed for all, we can drop CAP_ERASE.
> >
> > Kind regards
> > Uffe
>
> [1] 93caf8e69eac76 ("omap_hsmmc: add erase capability")
> [2] b13d1f0f9ad64b ("mmc: omap: Add erase capability")
> [3] ec30f11e821f2d ("mmc: rtsx_usb: Use the provided busy timeout from the mmc core")
>
> --
> Best Regards,
> Eugeniu
Kind regards
Uffe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-14 12:48 ` Ulf Hansson
@ 2019-11-14 20:15 ` Wolfram Sang
2019-11-15 9:19 ` Ulf Hansson
2019-11-14 22:07 ` Eugeniu Rosca
1 sibling, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2019-11-14 20:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Eugeniu Rosca, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]
Hi Ulf,
thanks again for the heads up.
> Let's first take a step back, because I don't know how the HW busy
> detection works for your controller.
>
> I have noticed there is TMIO_STAT_CMD_BUSY bit being set for some
> variants, which seems to cause renesas_sdhi_wait_idle() to loop for a
> pre-defined number of loops/timeout. This looks scary, but I can't
> tell if it's really a problem.
That should be okay. The datasheet mentions that some registers can only
be accessed when either CBSY or SCLKDIVEN bits signal non-busyness.
renesas_sdhi_wait_idle() is for that.
> BTW, do you know what TMIO_STAT_CMD_BUSY actually is monitoring?
0: A command sequence has been completed.
1: A command sequence is being executed.
> I have also noticed that MMC_CAP_WAIT_WHILE_BUSY isn't set for any of
> the renesas/tmio variant hosts. Is that simply because the HW doesn't
> support this? Or because implementation is missing?
Good thing we use public development. I recalled we discussed this
before but I needed a search engine to find it again:
https://patchwork.kernel.org/patch/8114821/
Summary: The HW (at least since Gen2) has HW support for busy timeout
detection but I never came around to implement it (and even forgot about
it :( ). So, we still use a workqueue for it.
Kind regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-14 12:48 ` Ulf Hansson
2019-11-14 20:15 ` Wolfram Sang
@ 2019-11-14 22:07 ` Eugeniu Rosca
2019-11-15 9:27 ` Ulf Hansson
1 sibling, 1 reply; 13+ messages in thread
From: Eugeniu Rosca @ 2019-11-14 22:07 UTC (permalink / raw)
To: Ulf Hansson
Cc: Eugeniu Rosca, Wolfram Sang, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
Hi Ulf,
On Thu, Nov 14, 2019 at 01:48:41PM +0100, Ulf Hansson wrote:
[..]
>
> Let's first take a step back, because I don't know how the HW busy
> detection works for your controller.
>
> I have noticed there is TMIO_STAT_CMD_BUSY bit being set for some
> variants, which seems to cause renesas_sdhi_wait_idle() to loop for a
> pre-defined number of loops/timeout. This looks scary, but I can't
> tell if it's really a problem.
>
> BTW, do you know what TMIO_STAT_CMD_BUSY actually is monitoring?
>
> I have also noticed that MMC_CAP_WAIT_WHILE_BUSY isn't set for any of
> the renesas/tmio variant hosts. Is that simply because the HW doesn't
> support this? Or because implementation is missing?
Hopefully Wolfram just addressed that?
> If you want to run a test that stretches the behaviour on the timeout
> path, I would rather use an SD-card (the older the better). For eMMCs
> the erase likely translates to a trim/discard, which is far more
> quicker than a real erase - as is what happens on an old SD card.
Running 'blkdiscard' with different SD cards on H3ULCB, I don't see any
signs of misbehavior:
root@rcar-gen3:~# blkdiscard -V
blkdiscard from util-linux 2.32.1
root@rcar-gen3:~# lsblk
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
mmcblk0 179:0 0 59.2G 0 disk
mmcblk0boot0 179:8 0 4M 1 disk
mmcblk0boot1 179:16 0 4M 1 disk
mmcblk1 179:24 0 30G 0 disk
# Erasing 32 GiB uSD Card
root@rcar-gen3:~# time blkdiscard -v /dev/mmcblk1
/dev/mmcblk1: Discarded 32227983360 bytes from the offset 0
real 0m1.198s
user 0m0.001s
sys 0m0.122s
# Erasing 64 GiB eMMC
root@rcar-gen3:~# time blkdiscard -v /dev/mmcblk0
/dev/mmcblk0: Discarded 63585648640 bytes from the offset 0
real 0m8.703s
user 0m0.002s
sys 0m1.909s
I guess that by decreasing below erase sizes, I could further increase
the execution time, but these sysfs properties are read-only:
cat /sys/devices/platform/soc/ee100000.sd/mmc_host/mmc1/mmc1:59b4/preferred_erase_size
4194304
cat /sys/devices/platform/soc/ee100000.sd/mmc_host/mmc1/mmc1:59b4/erase_size
512
--
Best Regards,
Eugeniu
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-14 20:15 ` Wolfram Sang
@ 2019-11-15 9:19 ` Ulf Hansson
2019-11-15 10:12 ` Wolfram Sang
0 siblings, 1 reply; 13+ messages in thread
From: Ulf Hansson @ 2019-11-15 9:19 UTC (permalink / raw)
To: Wolfram Sang
Cc: Eugeniu Rosca, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
On Thu, 14 Nov 2019 at 21:15, Wolfram Sang <wsa@the-dreams.de> wrote:
>
> Hi Ulf,
>
> thanks again for the heads up.
>
> > Let's first take a step back, because I don't know how the HW busy
> > detection works for your controller.
> >
> > I have noticed there is TMIO_STAT_CMD_BUSY bit being set for some
> > variants, which seems to cause renesas_sdhi_wait_idle() to loop for a
> > pre-defined number of loops/timeout. This looks scary, but I can't
> > tell if it's really a problem.
>
> That should be okay. The datasheet mentions that some registers can only
> be accessed when either CBSY or SCLKDIVEN bits signal non-busyness.
> renesas_sdhi_wait_idle() is for that.
>
> > BTW, do you know what TMIO_STAT_CMD_BUSY actually is monitoring?
>
> 0: A command sequence has been completed.
> 1: A command sequence is being executed.
Alright, thanks for clarifying!
>
> > I have also noticed that MMC_CAP_WAIT_WHILE_BUSY isn't set for any of
> > the renesas/tmio variant hosts. Is that simply because the HW doesn't
> > support this? Or because implementation is missing?
>
> Good thing we use public development. I recalled we discussed this
> before but I needed a search engine to find it again:
>
> https://patchwork.kernel.org/patch/8114821/
>
> Summary: The HW (at least since Gen2) has HW support for busy timeout
> detection but I never came around to implement it (and even forgot about
> it :( ). So, we still use a workqueue for it.
I had a vague memory about this discussion as well, thanks for giving
the pointers to it.
I think using a workqueue for scheduling a reset work with a timeout
of 5 s, as in your case.
However, as a heads up, if/when implementing support for busy
detection and adding MMC_CAP_WAIT_WHILE_BUSY, needs to update that
timeout according to cmd->busy_timeout, which is provided by the core.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-14 22:07 ` Eugeniu Rosca
@ 2019-11-15 9:27 ` Ulf Hansson
2019-11-15 12:51 ` Eugeniu Rosca
0 siblings, 1 reply; 13+ messages in thread
From: Ulf Hansson @ 2019-11-15 9:27 UTC (permalink / raw)
To: Eugeniu Rosca
Cc: Wolfram Sang, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
On Thu, 14 Nov 2019 at 23:07, Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
>
> Hi Ulf,
>
> On Thu, Nov 14, 2019 at 01:48:41PM +0100, Ulf Hansson wrote:
>
> [..]
> >
> > Let's first take a step back, because I don't know how the HW busy
> > detection works for your controller.
> >
> > I have noticed there is TMIO_STAT_CMD_BUSY bit being set for some
> > variants, which seems to cause renesas_sdhi_wait_idle() to loop for a
> > pre-defined number of loops/timeout. This looks scary, but I can't
> > tell if it's really a problem.
> >
> > BTW, do you know what TMIO_STAT_CMD_BUSY actually is monitoring?
> >
> > I have also noticed that MMC_CAP_WAIT_WHILE_BUSY isn't set for any of
> > the renesas/tmio variant hosts. Is that simply because the HW doesn't
> > support this? Or because implementation is missing?
>
> Hopefully Wolfram just addressed that?
>
> > If you want to run a test that stretches the behaviour on the timeout
> > path, I would rather use an SD-card (the older the better). For eMMCs
> > the erase likely translates to a trim/discard, which is far more
> > quicker than a real erase - as is what happens on an old SD card.
>
> Running 'blkdiscard' with different SD cards on H3ULCB, I don't see any
> signs of misbehavior:
>
> root@rcar-gen3:~# blkdiscard -V
> blkdiscard from util-linux 2.32.1
>
> root@rcar-gen3:~# lsblk
> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> mmcblk0 179:0 0 59.2G 0 disk
> mmcblk0boot0 179:8 0 4M 1 disk
> mmcblk0boot1 179:16 0 4M 1 disk
> mmcblk1 179:24 0 30G 0 disk
>
> # Erasing 32 GiB uSD Card
> root@rcar-gen3:~# time blkdiscard -v /dev/mmcblk1
> /dev/mmcblk1: Discarded 32227983360 bytes from the offset 0
>
> real 0m1.198s
> user 0m0.001s
> sys 0m0.122s
>
> # Erasing 64 GiB eMMC
> root@rcar-gen3:~# time blkdiscard -v /dev/mmcblk0
> /dev/mmcblk0: Discarded 63585648640 bytes from the offset 0
>
> real 0m8.703s
> user 0m0.002s
> sys 0m1.909s
>
> I guess that by decreasing below erase sizes, I could further increase
> the execution time, but these sysfs properties are read-only:
>
> cat /sys/devices/platform/soc/ee100000.sd/mmc_host/mmc1/mmc1:59b4/preferred_erase_size
> 4194304
> cat /sys/devices/platform/soc/ee100000.sd/mmc_host/mmc1/mmc1:59b4/erase_size
> 512
>
This test and due to the discussions with Wolfram and you in this
thread, I would actually suggest that you enable MMC_CAP_ERASE for all
tmio variants, rather than just for this particular one.
In other words, set the cap in tmio_mmc_host_probe() should be fine,
as it seems none of the tmio variants supports HW busy detection at
this point.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-15 9:19 ` Ulf Hansson
@ 2019-11-15 10:12 ` Wolfram Sang
2019-11-15 10:38 ` Ulf Hansson
0 siblings, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2019-11-15 10:12 UTC (permalink / raw)
To: Ulf Hansson
Cc: Eugeniu Rosca, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
[-- Attachment #1: Type: text/plain, Size: 196 bytes --]
> I think using a workqueue for scheduling a reset work with a timeout
> of 5 s, as in your case.
Sorry, I didn't get it. You think what exactly? Is it good / bad / ok
for now / ok in general?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-15 10:12 ` Wolfram Sang
@ 2019-11-15 10:38 ` Ulf Hansson
0 siblings, 0 replies; 13+ messages in thread
From: Ulf Hansson @ 2019-11-15 10:38 UTC (permalink / raw)
To: Wolfram Sang
Cc: Eugeniu Rosca, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
On Fri, 15 Nov 2019 at 11:12, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>
> > I think using a workqueue for scheduling a reset work with a timeout
> > of 5 s, as in your case.
>
> Sorry, I didn't get it. You think what exactly? Is it good / bad / ok
> for now / ok in general?
Sorry.
It's good for now!
If/when you start implementing support for HW busy detection, then you
need to adjust to the timeout value according to cmd->busy_timeout
from the core.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-15 9:27 ` Ulf Hansson
@ 2019-11-15 12:51 ` Eugeniu Rosca
2019-11-15 13:54 ` Eugeniu Rosca
0 siblings, 1 reply; 13+ messages in thread
From: Eugeniu Rosca @ 2019-11-15 12:51 UTC (permalink / raw)
To: Masahiro Yamada, Ulf Hansson
Cc: Eugeniu Rosca, Wolfram Sang, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
Hello Yamada-san,
On Fri, Nov 15, 2019 at 10:27:25AM +0100, Ulf Hansson wrote:
> On Thu, 14 Nov 2019 at 23:07, Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
> >
> > Hi Ulf,
> >
> > On Thu, Nov 14, 2019 at 01:48:41PM +0100, Ulf Hansson wrote:
> >
> > [..]
> > >
> > > Let's first take a step back, because I don't know how the HW busy
> > > detection works for your controller.
> > >
> > > I have noticed there is TMIO_STAT_CMD_BUSY bit being set for some
> > > variants, which seems to cause renesas_sdhi_wait_idle() to loop for a
> > > pre-defined number of loops/timeout. This looks scary, but I can't
> > > tell if it's really a problem.
> > >
> > > BTW, do you know what TMIO_STAT_CMD_BUSY actually is monitoring?
> > >
> > > I have also noticed that MMC_CAP_WAIT_WHILE_BUSY isn't set for any of
> > > the renesas/tmio variant hosts. Is that simply because the HW doesn't
> > > support this? Or because implementation is missing?
> >
> > Hopefully Wolfram just addressed that?
> >
> > > If you want to run a test that stretches the behaviour on the timeout
> > > path, I would rather use an SD-card (the older the better). For eMMCs
> > > the erase likely translates to a trim/discard, which is far more
> > > quicker than a real erase - as is what happens on an old SD card.
> >
> > Running 'blkdiscard' with different SD cards on H3ULCB, I don't see any
> > signs of misbehavior:
> >
> > root@rcar-gen3:~# blkdiscard -V
> > blkdiscard from util-linux 2.32.1
> >
> > root@rcar-gen3:~# lsblk
> > NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> > mmcblk0 179:0 0 59.2G 0 disk
> > mmcblk0boot0 179:8 0 4M 1 disk
> > mmcblk0boot1 179:16 0 4M 1 disk
> > mmcblk1 179:24 0 30G 0 disk
> >
> > # Erasing 32 GiB uSD Card
> > root@rcar-gen3:~# time blkdiscard -v /dev/mmcblk1
> > /dev/mmcblk1: Discarded 32227983360 bytes from the offset 0
> >
> > real 0m1.198s
> > user 0m0.001s
> > sys 0m0.122s
> >
> > # Erasing 64 GiB eMMC
> > root@rcar-gen3:~# time blkdiscard -v /dev/mmcblk0
> > /dev/mmcblk0: Discarded 63585648640 bytes from the offset 0
> >
> > real 0m8.703s
> > user 0m0.002s
> > sys 0m1.909s
> >
> > I guess that by decreasing below erase sizes, I could further increase
> > the execution time, but these sysfs properties are read-only:
> >
> > cat /sys/devices/platform/soc/ee100000.sd/mmc_host/mmc1/mmc1:59b4/preferred_erase_size
> > 4194304
> > cat /sys/devices/platform/soc/ee100000.sd/mmc_host/mmc1/mmc1:59b4/erase_size
> > 512
> >
>
> This test and due to the discussions with Wolfram and you in this
> thread, I would actually suggest that you enable MMC_CAP_ERASE for all
> tmio variants, rather than just for this particular one.
>
> In other words, set the cap in tmio_mmc_host_probe() should be fine,
> as it seems none of the tmio variants supports HW busy detection at
> this point.
Just for your information, following Ulf's suggestion, we are going to
enable MMC_CAP_ERASE in the TMIO mmc core driver, affecting UniPhier
SD/eMMC Host Controller. Hope to see your Ack/NAK on this in the
upcoming patch. TIA.
>
> Kind regards
> Uffe
--
Best Regards,
Eugeniu
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs
2019-11-15 12:51 ` Eugeniu Rosca
@ 2019-11-15 13:54 ` Eugeniu Rosca
0 siblings, 0 replies; 13+ messages in thread
From: Eugeniu Rosca @ 2019-11-15 13:54 UTC (permalink / raw)
To: Masahiro Yamada, Ulf Hansson
Cc: Eugeniu Rosca, Wolfram Sang, Wolfram Sang, Yoshihiro Shimoda,
Niklas Söderlund, Geert Uytterhoeven, Simon Horman,
linux-mmc, Linux Kernel Mailing List, Linux-Renesas,
Eugeniu Rosca, Harish Jenny K N, Andrew Gabbasov
[to facilitate version tracking]
Superseded by:
https://lore.kernel.org/linux-renesas-soc/20191115134430.12621-1-erosca@de.adit-jv.com/
("[PATCH v2] mmc: tmio: Add MMC_CAP_ERASE to allow erase/discard/trim requests")
--
Best Regards,
Eugeniu
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-11-15 13:54 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12 13:48 [PATCH] mmc: renesas_sdhi_internal_dmac: Add MMC_CAP_ERASE to Gen3 SoCs Eugeniu Rosca
2019-11-12 20:49 ` Wolfram Sang
2019-11-14 10:56 ` Ulf Hansson
2019-11-14 11:37 ` Eugeniu Rosca
2019-11-14 12:48 ` Ulf Hansson
2019-11-14 20:15 ` Wolfram Sang
2019-11-15 9:19 ` Ulf Hansson
2019-11-15 10:12 ` Wolfram Sang
2019-11-15 10:38 ` Ulf Hansson
2019-11-14 22:07 ` Eugeniu Rosca
2019-11-15 9:27 ` Ulf Hansson
2019-11-15 12:51 ` Eugeniu Rosca
2019-11-15 13:54 ` Eugeniu Rosca
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).