On 2 May 2018 at 11:26, Michał Pecio <michal.pecio@xxxxxxxxx> wrote: > Hi, > > I'm trying to get erasing to work on that controller. > > I searched for past mailing list discussions on the topic and found two > commits which enabled erase in the related PCI driver: > > 9bce7fd6f834 mmc: rtsx_pci: Enable MMC_CAP_ERASE to allow erase/discard/trim requests > 27f4bf7d7454 mmc: rtsx_pci: Use the provided busy timeout from the mmc core Yes, these are needed for the USB variant as well. > > One just adds a flag, the other increases the timeout for R1b responses > (whatever they are, I am *not* familiar with that low-level SD stuff). > > The old timeout was 3 seconds and, as the commit message argues, wasn't > sufficient for erase commands. I assume (perhaps wrongly) that it is > insuficcient because erasing can easily take longer than 3 seconds. > > So I headed to rtsx_usb_sdmmc.c and found identical timeout code > in sd_send_cmd_get_rsp() and similar host caps initialization. > > I started with adding MMC_CAP_ERASE, fully expecting to get nasty red > messages in dmesg and "operation timed out" from blkdiscard. > > However, dmesg was clean and blkdiscard returned sucessfully after 17 > seconds. The card was erased. > > So I added printks around the function call which consumes that timeout > variable: > > + if (rsp_type == SD_RSP_TYPE_R1b) > + printk(KERN_INFO "rtsx_usb_get_rsp()...\n"); > err = rtsx_usb_get_rsp(ucr, len, timeout); > + if (rsp_type == SD_RSP_TYPE_R1b) > + printk(KERN_INFO "rtsx_usb_get_rsp(): %d\n", err); > > And got this, repeating for 17 seconds: > > [ +0,000850] rtsx_usb_get_rsp()... > [ +0,028390] rtsx_usb_get_rsp(): 0 > [ +0,000856] rtsx_usb_get_rsp()... > [ +0,028133] rtsx_usb_get_rsp(): 0 > [ +0,000863] rtsx_usb_get_rsp()... > [ +0,040398] rtsx_usb_get_rsp(): 0 > > So my questions are: > > 1. Is the timeout code broken or am I missing something? > 2. Is it relevant at all to erasing? > 3. Can we just add MMC_CAP_ERASE to this driver and be done with it? > This seems to work for me, as described above. Thanks for sharing the details from your tests! Yes, we do need a little more than MMC_CAP_ERASE. I have had patches for this in my pipe for a while, but never got to the point of posting them as I needed someone to test them on HW. If you could run a round of tests and preferably add you tested by tag, that would be great so I can queue them up. I guess you even want them for stable? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
