On 30 January 2017 at 10:40, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > On 30/01/17 11:10, Ulf Hansson wrote: >> On 28 January 2017 at 09:16, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: >>> On 27/01/2017 5:12 p.m., Ulf Hansson wrote: >>>> >>>> On 26 January 2017 at 13:39, Adrian Hunter <adrian.hunter@xxxxxxxxx> >>>> wrote: >>>>> >>>>> On 26/01/17 12:50, Ulf Hansson wrote: >>>>>> >>>>>> On 11 January 2017 at 18:19, Gregory CLEMENT >>>>>> <gregory.clement@xxxxxxxxxxxxxxxxxx> wrote: >>>>>>> >>>>>>> + priv->init_card_type = MMC_TYPE_MMC; >>>>>>> + mmc->caps |= MMC_CAP_NONREMOVABLE; >>>>>>> + >>>>>>> + /* >>>>>>> + * Force to clear BUS_TEST to >>>>>>> + * skip bus_test_pre and bus_test_post >>>>>>> + */ >>>>>>> + mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST; >>>>>>> + mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ | >>>>>> >>>>>> >>>>>> This cap is a bit strange. It was added several years ago by Adrian >>>>>> Hunter, but I am wondering about the reason to why it's needed. >>>>>> >>>>> >>>>> MMC_CAP2_HC_ERASE_SZ relates to EXT-CSD ERASE_GROUP_DEF. >>>>> >>>>> I think it was added to enable people to choose whether they wanted a >>>>> large >>>>> or small erase granularity. That probably doesn't matter if the card >>>>> supports TRIM. >>>>> >>>> >>>> Huh, the erase/trim/discard code in the mmc core is really hairy. :-) >>>> >>>> In mmc_calc_max_discard() the following code/comment exists: >>>> >>>> /* >>>> * Without erase_group_def set, MMC erase timeout depends on clock >>>> * frequence which can change. In that case, the best choice is >>>> * just the preferred erase size. >>>> */ >>>> if (mmc_card_mmc(card) && !(card->ext_csd.erase_group_def & 1)) >>>> return card->pref_erase; >>>> >>>> >>>> This makes me wonder. >>>> >>>> So, when we haven't enabled the high capacity erase groups in the >>>> EXT_CSD register (ext_csd.erase_group_def), we will use the pref_erase >>>> size. >>>> >>>> In the other case, as when having MMC_CAP2_HC_ERASE_SZ set (which will >>>> set ext_csd.erase_group_def), we will instead do some calculations >>>> to find out the max discards. >>>> >>>> Are you saying that these calculations doesn't matter much - or are >>>> you saying that we always want to do them? >>> >>> >>> No, I was saying that if you have TRIM then TRIM is preferred to ERASE so >>> the erase group size does not come into play when discarding, since ERASE >>> granularity is erase groups whereas TRIM granularity is sectors. >> >> Right. Thanks for clarifying. >> >>> >>> However ERASE_GROUP_DEF also affects the size of write protect groups. >> >> In either case. >> >> What do you think of removing MMC_CAP2_HC_ERASE_SZ? I don't like these >> kind of soft polices, it's better if we can decide on a common >> behaviour - whatever that is. > > Changing the value of ERASE_GROUP_DEF could be a problem, for example the > spec. says: > > "Similarly if the host set ERASE_GROUP_DEF bit for a device that the default > write protect was already set in some of the area in the previous power > cycle, then the device may show unknown behavior when host issue write > or erase commands to the device. In application, it is mandatory for host to > use same ERASE GROUP DEF value to the device all the time." > > Whether or not there is anyone that would actually be affected is hard to know. Okay. I am going to submit a patch that just removes the behaviour for MMC_CAP2_HC_ERASE_SZ and the cap itself. The cap isn't particular widely used anyway. Let's see what people think of it. Thanks a lot for you input! 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
