could you add "MMC_CAP_ERASE" to the bcm2835-sdhost driver mmc->caps initialization? #1076

ghost · 2015-07-19T00:01:15Z

I've been using the new "sdhost" driver, and it's overclocking abilities are amazing. E.g., I can now run my card at 40MB/s read speeds (as compared to 18MB/s under the "mmc" driver).

However the current implementation of this new driver in the "rpi-4.0.y" tree is missing the "MMC_CAP_ERASE" capability for the host controller configuration, and as such this prevents "fstrim" from working ("fstrim" gives an "FITRIM ioctl failed: Operation not supported" error). Adding the MMC_CAP_ERASE capability fixes this issue, and the "sdhost" driver will then support the TRIM feature like the current "bcm2835-mmc" driver does.

I've enabled this capability in my own kernel build, but it'd be great to get this in upstream so that I don't need to keep building my own kernel just for this simple change...

The patch to bcm2835-sdhost.c is really simple:

diff --git a/drivers/mmc/host/bcm2835-sdhost.c b/drivers/mmc/host/bcm2835-sdhost.c
index d65870a..57a6ad3 100644
--- a/drivers/mmc/host/bcm2835-sdhost.c
+++ b/drivers/mmc/host/bcm2835-sdhost.c
@@ -1675,7 +1675,7 @@ int bcm2835_sdhost_add_host(struct bcm2835_host *host)
        /* host controller capabilities */
        mmc->caps |= /* MMC_CAP_SDIO_IRQ |*/ MMC_CAP_4_BIT_DATA |
                MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
-               MMC_CAP_NEEDS_POLL | MMC_CAP_HW_RESET |
+               MMC_CAP_NEEDS_POLL | MMC_CAP_HW_RESET | MMC_CAP_ERASE |
                (ALLOW_CMD23 * MMC_CAP_CMD23);

        spin_lock_init(&host->lock);

Thanks!

The text was updated successfully, but these errors were encountered:

pelwell · 2015-07-20T09:50:33Z

I'm happy to add it, provided it works. Which card are you using? None of mine (SanDisk class 10, Lexar class 10, no-name class 6) have the required capabilities. In fact there's a discussion in #431 where Gordon says that SD cards don't really support (and in fact don't need) trimming.

ghost · 2015-07-20T15:02:25Z

Indeed, I just verified that performing a TRIM request doesn't appear to do anything even though the card capabilities report it as being supported (!). At least this is the case on my SD card (Samsung 32GB class 6 that "supports" the required capabilities). Here's how I verified this...:

First, let's create a file, then get the file's inode number:

root@gandalf:~/tmp# echo ABCDEFG >foobar
root@gandalf:~/tmp# sync
root@gandalf:~/tmp# ls -i foobar
2646 foobar
root@gandalf:~/tmp#

Now, we'll find the physical sector of this file:

root@gandalf:~/tmp# debugfs /dev/mmcblk0p2
debugfs 1.42.9 (4-Feb-2014)
debugfs:  stat <2646>
Inode: 2646   Type: regular    Mode:  0644   Flags: 0x80000
Generation: 659272254    Version: 0x00000000:00000001
User:     0   Group:     0   Size: 8
File ACL: 0    Directory ACL: 0
Links: 1   Blockcount: 8
Fragment:  Address: 0    Number: 0    Size: 0
 ctime: 0x55ad0732:479c39fc -- Mon Jul 20 07:35:30 2015
 atime: 0x55ad0701:55fa95e8 -- Mon Jul 20 07:34:41 2015
 mtime: 0x55ad0732:479c39fc -- Mon Jul 20 07:35:30 2015
crtime: 0x55ad0701:55fa95e8 -- Mon Jul 20 07:34:41 2015
Size of extra inode fields: 28
EXTENTS:
(0):418372
debugfs:  q
root@gandalf:~/tmp#

Next, we'll read the sector to verify that it indeed contains the contents of the file:

root@gandalf:~/tmp# dd if=/dev/mmcblk0p2 ibs=4096 count=1 skip=418372 | xxd | head -1
1+0 records in
8+0 records out
4096 bytes (4.1 kB) copied, 0.00180056 s, 2.3 MB/s
0000000: 4142 4344 4546 470a 0000 0000 0000 0000  ABCDEFG.........
root@gandalf:~/tmp#

Now, let's delete the file, and we'll see that the physical contents are still there:

root@gandalf:~/tmp# rm foobar
root@gandalf:~/tmp# sync
root@gandalf:~/tmp# dd if=/dev/mmcblk0p2 ibs=4096 count=1 skip=418372 | xxd | head -1
1+0 records in
8+0 records out
0000000: 4142 4344 4546 470a 0000 0000 0000 0000  ABCDEFG.........
4096 bytes (4.1 kB) copied, 0.00782803 s, 523 kB/s
root@gandalf:~/tmp#

What will the magical "fstrim" command do? (The moment we've all been waiting for...)

root@gandalf:~/tmp# fstrim -v /
/: 48234496 bytes were trimmed
root@gandalf:~/tmp# sync
root@gandalf:~/tmp# dd if=/dev/mmcblk0p2 ibs=4096 count=1 skip=418372 | xxd | head -1
1+0 records in
8+0 records out
4096 bytes (4.1 kB) copied, 0.00204582 s, 2.0 MB/s
0000000: 4142 4344 4546 470a 0000 0000 0000 0000  ABCDEFG.........
root@gandalf:~/tmp#

Absolutely nothing!

So...it would appear that "fstrim" actually doesn't do anything for this particular card. However keep in mind that we can't actually say that other cards might not actually support this functionality correctly.

Before I say "nevermind", I am thinking that the right thing to do regardless would be to support this in the driver by default. If a particular card's capabilities indicates that it supports this function, shouldn't the driver allow/enable it? This way the driver is properly supporting (and not arbitrarily limiting) what a particular card's firmware implementation actually may do.

Thanks!

See: #1076

pelwell · 2015-07-21T10:30:04Z

I've added the ERASE capability to the sdhost driver - it will be in the next release.

See: #1076

ghost · 2015-07-22T03:53:28Z

Great, thanks!

See: #1076

kernel: BCM2835_V4L2: Add support for V4L2_EXPOSURE_METERING_MATRIX See: raspberrypi/linux#1068 kernel: dmaengine: bcm2708-dmaengine: Fix memory leak when stopping a running transfer See: raspberrypi/linux#1072 kernel: BCM270X_DT: mz61581: Revert to spi-bcm2708 See: raspberrypi/linux#1077 kernel: bcm2708/2835-i2s: Fix for PCM register ranges in device trees See: raspberrypi/linux#1079 kernel: bcm2835-sdhost: Add the ERASE capability See: raspberrypi/linux#1076 kernel: bcm2835-sdhost: Ignore CRC7 for MMC CMD1 kernel: BCM270X_DT: Add unit address to gpio node name kernel: spi-bcm2835: merge upstream patches allowing DMA transfers See: raspberrypi/linux#1085 kernel: BCM270X_DT: Use i2c_arm for rtc and bmp085 overlays kernel: BCM2708_DT: CM dtparams for audio, watchdog and RNG firmware: video_decode: Don't wait for a valid timestamp to output frames See: raspberrypi/firmware#451

kernel: BCM2835_V4L2: Add support for V4L2_EXPOSURE_METERING_MATRIX See: raspberrypi/linux#1068 kernel: dmaengine: bcm2708-dmaengine: Fix memory leak when stopping a running transfer See: raspberrypi/linux#1072 kernel: BCM270X_DT: mz61581: Revert to spi-bcm2708 See: raspberrypi/linux#1077 kernel: bcm2708/2835-i2s: Fix for PCM register ranges in device trees See: raspberrypi/linux#1079 kernel: bcm2835-sdhost: Add the ERASE capability See: raspberrypi/linux#1076 kernel: bcm2835-sdhost: Ignore CRC7 for MMC CMD1 kernel: BCM270X_DT: Add unit address to gpio node name kernel: spi-bcm2835: merge upstream patches allowing DMA transfers See: raspberrypi/linux#1085 kernel: BCM270X_DT: Use i2c_arm for rtc and bmp085 overlays kernel: BCM2708_DT: CM dtparams for audio, watchdog and RNG firmware: video_decode: Don't wait for a valid timestamp to output frames See: #451

See: #1076

See: raspberrypi#1076

See: #1076

BCM2835 has two SD card interfaces. This driver uses the other one. bcm2835-sdhost: Error handling fix, and code clarification bcm2835-sdhost: Adding overclocking option Allow a different clock speed to be substitued for a requested 50MHz. This option is exposed using the "overclock_50" DT parameter. Note that the sdhost interface is restricted to integer divisions of core_freq, and the highest sensible option for a core_freq of 250MHz is 84 (250/3 = 83.3MHz), the next being 125 (250/2) which is much too high. Use at your own risk. bcm2835-sdhost: Round up the overclock, so 62 works for 62.5Mhz Also only warn once for each overclock setting. bcm2835-sdhost: Improve error handling and recovery 1) Expose the hw_reset method to the MMC framework, removing many internal calls by the driver. 2) Reduce overclock setting on error. 3) Increase timeout to cope with high capacity cards. 4) Add properties and parameters to control pio_limit and debug. 5) Reduce messages at probe time. bcm2835-sdhost: Further improve overclock back-off bcm2835-sdhost: Clear HBLC for PIO mode Also update pio_limit default in overlay README. bcm2835-sdhost: Add the ERASE capability See: #1076 bcm2835-sdhost: Ignore CRC7 for MMC CMD1 It seems that the sdhost interface returns CRC7 errors for CMD1, which is the MMC-specific SEND_OP_COND. Returning these errors to the MMC layer causes a downward spiral, but ignoring them seems to be harmless. bcm2835-mmc/sdhost: Remove ARCH_BCM2835 differences The bcm2835-mmc driver (and -sdhost driver that copied from it) contains code to handle SDIO interrupts in a threaded interrupt handler rather than waking the MMC framework thread. The change follows a patch from Russell King that adds the facility as the preferred way of working. However, the new code path is only present in ARCH_BCM2835 builds, which I have taken to be a way of testing the waters rather than making the change across the board; I can't see any technical reason why it wouldn't be enabled for MACH_BCM270X builds. So this patch standardises on the ARCH_BCM2835 code, removing the old code paths.

See: #1076

BCM2835 has two SD card interfaces. This driver uses the other one. bcm2835-sdhost: Error handling fix, and code clarification bcm2835-sdhost: Adding overclocking option Allow a different clock speed to be substitued for a requested 50MHz. This option is exposed using the "overclock_50" DT parameter. Note that the sdhost interface is restricted to integer divisions of core_freq, and the highest sensible option for a core_freq of 250MHz is 84 (250/3 = 83.3MHz), the next being 125 (250/2) which is much too high. Use at your own risk. bcm2835-sdhost: Round up the overclock, so 62 works for 62.5Mhz Also only warn once for each overclock setting. bcm2835-sdhost: Improve error handling and recovery 1) Expose the hw_reset method to the MMC framework, removing many internal calls by the driver. 2) Reduce overclock setting on error. 3) Increase timeout to cope with high capacity cards. 4) Add properties and parameters to control pio_limit and debug. 5) Reduce messages at probe time. bcm2835-sdhost: Further improve overclock back-off bcm2835-sdhost: Clear HBLC for PIO mode Also update pio_limit default in overlay README. bcm2835-sdhost: Add the ERASE capability See: #1076 bcm2835-sdhost: Ignore CRC7 for MMC CMD1 It seems that the sdhost interface returns CRC7 errors for CMD1, which is the MMC-specific SEND_OP_COND. Returning these errors to the MMC layer causes a downward spiral, but ignoring them seems to be harmless. bcm2835-mmc/sdhost: Remove ARCH_BCM2835 differences The bcm2835-mmc driver (and -sdhost driver that copied from it) contains code to handle SDIO interrupts in a threaded interrupt handler rather than waking the MMC framework thread. The change follows a patch from Russell King that adds the facility as the preferred way of working. However, the new code path is only present in ARCH_BCM2835 builds, which I have taken to be a way of testing the waters rather than making the change across the board; I can't see any technical reason why it wouldn't be enabled for MACH_BCM270X builds. So this patch standardises on the ARCH_BCM2835 code, removing the old code paths. bcm2835-sdhost: Don't log timeout errors unless debug=1 The MMC card-discovery process generates timeouts. This is expected behaviour, so reporting it to the user serves no purpose. Suppress the reporting of timeout errors unless the debug flag is on. bcm2835-sdhost: Add workaround for odd behaviour on some cards For reasons not understood, the sdhost driver fails when reading sectors very near the end of some SD cards. The problem could be related to the similar issue that reading the final sector of any card as part of a multiple read never completes, and the workaround is an extension of the mechanism introduced to solve that problem which ensures those sectors are always read singly. bcm2835-sdhost: Major revision This is a significant revision of the bcm2835-sdhost driver. It improves on the original in a number of ways: 1) Through the use of CMD23 for reads it appears to avoid problems reading some sectors on certain high speed cards. 2) Better atomicity to prevent crashes. 3) Higher performance. 4) Activity logging included, for easier diagnosis in the event of a problem. Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: Restore ATOMIC flag to PIO sg mapping Allocation problems have been seen in a wireless driver, and this is the only change which might have been responsible. SQUASH: bcm2835-sdhost: Only claim one DMA channel With both MMC controllers enabled there are few DMA channels left. The bcm2835-sdhost driver only uses DMA in one direction at a time, so it doesn't need to claim two channels. See: #1327 Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: Workaround for "slow" sectors Some cards have been seen to cause timeouts after certain sectors are read. This workaround enforces a minimum delay between the stop after reading one of those sectors and a subsequent data command. Using CMD23 (SET_BLOCK_COUNT) avoids this problem, so good cards will not be penalised by this workaround. Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: Firmware manages the clock divisor The bcm2835-sdhost driver hands control of the CDIV clock divisor register to matching firmware, allowing it to adjust to a changing core clock. This removes the need to use the performance governor or to enable io_is_busy on the on-demand governor in order to get the best SD performance. N.B. As SD clocks must be an integer divisor of the core clock, it is possible that the SD clock for "turbo" mode can be different (even lower) than "normal" mode. Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: Reset the clock in task context Since reprogramming the clock can now involve a round-trip to the firmware it must not be done at atomic context, and a tasklet is not a task. Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: Don't exit cmd wait loop on error The FAIL flag can be set in the CMD register before command processing is complete, leading to spurious "failed to complete" errors. This has the effect of promoting harmless CRC7 errors during CMD1 processing into errors that can delay and even prevent booting. Also: 1) Convert the last KERN_ERROR message in the register dumping to KERN_INFO. 2) Remove an unnecessary reset call from bcm2835_sdhost_add_host. See: #1492 Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: mmc_card_blockaddr fix Get the definition of mmc_card_blockaddr from drivers/mmc/core/card.h. Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: New timer API mmc: bcm2835-sdhost: Support underclocking Support underclocking of the SD bus in two ways: 1. using the max-frequency DT property (which currently has no DT parameter), and 2. using the exiting sd_overclock parameter. The two methods differ slightly - in the former the MMC subsystem is aware of the underclocking, while in the latter it isn't - but the end results should be the same. See: #2350 Signed-off-by: Phil Elwell <phil@raspberrypi.org> mmc: bcm2835-sdhost: Add include highmem.h (needed for kmap_atomic) is pulled in by one of the other include files, but only with some CONFIG settings. Make the inclusion explicit to cater for cases where the CONFIG setting is absent. See: #2366 Signed-off-by: Phil Elwell <phil@raspberrypi.org> mmc/bcm2835-sdhost: Recover from MMC_SEND_EXT_CSD If the user issues an "mmc extcsd read", the SD controller receives what it thinks is a SEND_IF_COND command with an unexpected data block. The resulting operations leave the FSM stuck in READWAIT, a state which persists until the MMC framework resets the controller, by which point the root filesystem is likely to have been unmounted. A less heavyweight solution is to detect the condition and nudge the FSM by asserting the (self-clearing) FORCE_DATA_MODE bit. N.B. This workaround was essentially discovered by accident and without a full understanding the inner workings of the controller, so it is fortunate that the "fix" only modifies error paths. See: #2728 Signed-off-by: Phil Elwell <phil@raspberrypi.org> mmc: bcm2835-sdhost: Fix warnings on arm64 Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: Allow for sg entries that cross pages The dma_complete handling code calculates a virtual address for a page then adds an offset, but if the offset is more than a page and HIGHMEM is in use then the summed address could be in an unmapped (or just incorrect) page. The upstream SDHOST driver allows for this possibility - copy the code that does so. Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: Fix DMA channel leak on error/remove Signed-off-by: Phil Elwell <phil@raspberrypi.org> mmc: bcm2835-sdhost: Support 64-bit physical addresses Signed-off-by: Phil Elwell <phil@raspberrypi.org> bcm2835-sdhost: Replace obsolete struct timeval struct timeval has been retired due to the impending linux 32-bit tv_sec rollover (only 18 years to go) - timespec64 is the obvious replacement. Signed-off-by: Phil Elwell <phil@raspberrypi.com> mmc: sdhost: Pass DT pointer to rpi_firmware_get Using the rpi_firmware API as intended allows proper reference counting of the firmware device and means we can remove a downstream patch to the firmware driver. Signed-off-by: Phil Elwell <phil@raspberrypi.com> bcm2835-sdhost: Use DT to configure logging Retrieve the system timer base address directly from DT. Signed-off-by: Phil Elwell <phil@raspberrypi.com> bcm2835-sdhost: Use phys addresses for slave DMA config Contrary to what struct snd_dmaengine_dai_dma_data suggests, the configuration of addresses of DMA slave interfaces should be done in CPU physical addresses. Signed-off-by: Phil Elwell <phil@raspberrypi.com> bcm2835-sdhost: Fail gracefully with bad dtb The logging timestamps depend on the existence of a bcm2835-system-timer node. If this node doesn't exist, leave the logging disabled rather than crashing. Signed-off-by: Phil Elwell <phil@raspberrypi.com>

pelwell pushed a commit that referenced this issue Jul 20, 2015

bcm2835-sdhost: Add the ERASE capability

67ed9de

See: #1076

pelwell pushed a commit that referenced this issue Jul 21, 2015

bcm2835-sdhost: Add the ERASE capability

a5b4fab

See: #1076

pelwell pushed a commit that referenced this issue Jul 21, 2015

bcm2835-sdhost: Add the ERASE capability

d57693f

See: #1076

ghost closed this as completed Jul 22, 2015

pelwell pushed a commit that referenced this issue Jul 24, 2015

bcm2835-sdhost: Add the ERASE capability

fc3fb87

See: #1076

pelwell pushed a commit that referenced this issue Jul 27, 2015

bcm2835-sdhost: Add the ERASE capability

f8b0d0e

See: #1076

gohai pushed a commit to gohai/linux-vc4 that referenced this issue Jul 29, 2015

bcm2835-sdhost: Add the ERASE capability

51bcd86

See: raspberrypi#1076

pelwell pushed a commit that referenced this issue Aug 4, 2015

bcm2835-sdhost: Add the ERASE capability

5ea690a

See: #1076

pelwell pushed a commit that referenced this issue Aug 5, 2015

bcm2835-sdhost: Add the ERASE capability

26be449

See: #1076

pelwell pushed a commit that referenced this issue Aug 17, 2015

bcm2835-sdhost: Add the ERASE capability

42f461f

See: #1076

pelwell pushed a commit that referenced this issue Aug 20, 2015

bcm2835-sdhost: Add the ERASE capability

36d180d

See: #1076

pelwell pushed a commit that referenced this issue Sep 2, 2015

bcm2835-sdhost: Add the ERASE capability

754d1a7

See: #1076

pelwell pushed a commit that referenced this issue Sep 14, 2015

bcm2835-sdhost: Add the ERASE capability

1072f6b

See: #1076

pelwell pushed a commit that referenced this issue Sep 14, 2015

bcm2835-sdhost: Add the ERASE capability

25121f1

See: #1076

pelwell pushed a commit that referenced this issue Sep 22, 2015

bcm2835-sdhost: Add the ERASE capability

6dc7c83

See: #1076

pelwell pushed a commit that referenced this issue Sep 22, 2015

bcm2835-sdhost: Add the ERASE capability

a13b6ec

See: #1076

pelwell pushed a commit that referenced this issue Oct 1, 2015

bcm2835-sdhost: Add the ERASE capability

113f3e4

See: #1076

pelwell pushed a commit that referenced this issue Oct 3, 2015

bcm2835-sdhost: Add the ERASE capability

0c7da07

See: #1076

pelwell pushed a commit that referenced this issue Oct 3, 2015

bcm2835-sdhost: Add the ERASE capability

615320a

See: #1076

pelwell pushed a commit that referenced this issue Oct 6, 2015

bcm2835-sdhost: Add the ERASE capability

04458cc

See: #1076

pelwell pushed a commit that referenced this issue Oct 7, 2015

bcm2835-sdhost: Add the ERASE capability

a7b0eb8

See: #1076

pelwell pushed a commit that referenced this issue Oct 8, 2015

bcm2835-sdhost: Add the ERASE capability

d48da1e

See: #1076

This issue was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

could you add "MMC_CAP_ERASE" to the bcm2835-sdhost driver mmc->caps initialization? #1076

could you add "MMC_CAP_ERASE" to the bcm2835-sdhost driver mmc->caps initialization? #1076

ghost commented Jul 19, 2015

pelwell commented Jul 20, 2015

ghost commented Jul 20, 2015

pelwell commented Jul 21, 2015

ghost commented Jul 22, 2015

could you add "MMC_CAP_ERASE" to the bcm2835-sdhost driver mmc->caps initialization? #1076

could you add "MMC_CAP_ERASE" to the bcm2835-sdhost driver mmc->caps initialization? #1076

Comments

ghost commented Jul 19, 2015

pelwell commented Jul 20, 2015

ghost commented Jul 20, 2015

pelwell commented Jul 21, 2015

ghost commented Jul 22, 2015