From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932937AbdA0PNt (ORCPT ); Fri, 27 Jan 2017 10:13:49 -0500 Received: from mail-wm0-f48.google.com ([74.125.82.48]:38691 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932858AbdA0PMr (ORCPT ); Fri, 27 Jan 2017 10:12:47 -0500 MIME-Version: 1.0 In-Reply-To: <2e67332d-d8c3-fd24-1f81-72675a09927a@intel.com> References: <3546f143091c121face8ecbcbf7f6a4c41d2cec7.1484154449.git-series.gregory.clement@free-electrons.com> <2e67332d-d8c3-fd24-1f81-72675a09927a@intel.com> From: Ulf Hansson Date: Fri, 27 Jan 2017 16:12:24 +0100 Message-ID: Subject: Re: [PATCH v5 06/12] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality To: Adrian Hunter Cc: Gregory CLEMENT , "linux-mmc@vger.kernel.org" , Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , Thomas Petazzoni , "linux-arm-kernel@lists.infradead.org" , Mike Turquette , Stephen Boyd , linux-clk , "linux-kernel@vger.kernel.org" , Rob Herring , "devicetree@vger.kernel.org" , Ziji Hu , Jimmy Xu , Jisheng Zhang , Nadav Haklai , Ryan Gao , Doug Jones , Victor Gu , "Wei(SOCP) Liu" , Wilson Ding , Yehuda Yitschak , Marcin Wojtas , Hanna Hawa , Kostya Porotchkin Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26 January 2017 at 13:39, Adrian Hunter wrote: > On 26/01/17 12:50, Ulf Hansson wrote: >> On 11 January 2017 at 18:19, Gregory CLEMENT >> 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? Kind regards Uffe From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ulf Hansson Subject: Re: [PATCH v5 06/12] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality Date: Fri, 27 Jan 2017 16:12:24 +0100 Message-ID: References: <3546f143091c121face8ecbcbf7f6a4c41d2cec7.1484154449.git-series.gregory.clement@free-electrons.com> <2e67332d-d8c3-fd24-1f81-72675a09927a@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <2e67332d-d8c3-fd24-1f81-72675a09927a@intel.com> Sender: linux-clk-owner@vger.kernel.org To: Adrian Hunter Cc: Gregory CLEMENT , "linux-mmc@vger.kernel.org" , Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , Thomas Petazzoni , "linux-arm-kernel@lists.infradead.org" , Mike Turquette , Stephen Boyd , linux-clk , "linux-kernel@vger.kernel.org" , Rob Herring , "devicetree@vger.kernel.org" , Ziji Hu , Jimmy Xu , Jisheng Zhang , Nadav Haklai List-Id: devicetree@vger.kernel.org On 26 January 2017 at 13:39, Adrian Hunter wrote: > On 26/01/17 12:50, Ulf Hansson wrote: >> On 11 January 2017 at 18:19, Gregory CLEMENT >> 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? Kind regards Uffe From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <2e67332d-d8c3-fd24-1f81-72675a09927a@intel.com> References: <3546f143091c121face8ecbcbf7f6a4c41d2cec7.1484154449.git-series.gregory.clement@free-electrons.com> <2e67332d-d8c3-fd24-1f81-72675a09927a@intel.com> From: Ulf Hansson Date: Fri, 27 Jan 2017 16:12:24 +0100 Message-ID: Subject: Re: [PATCH v5 06/12] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality To: Adrian Hunter Cc: Gregory CLEMENT , "linux-mmc@vger.kernel.org" , Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , Thomas Petazzoni , "linux-arm-kernel@lists.infradead.org" , Mike Turquette , Stephen Boyd , linux-clk , "linux-kernel@vger.kernel.org" , Rob Herring , "devicetree@vger.kernel.org" , Ziji Hu , Jimmy Xu , Jisheng Zhang , Nadav Haklai , Ryan Gao , Doug Jones , Victor Gu , "Wei(SOCP) Liu" , Wilson Ding , Yehuda Yitschak , Marcin Wojtas , Hanna Hawa , Kostya Porotchkin Content-Type: text/plain; charset=UTF-8 List-ID: On 26 January 2017 at 13:39, Adrian Hunter wrote: > On 26/01/17 12:50, Ulf Hansson wrote: >> On 11 January 2017 at 18:19, Gregory CLEMENT >> 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? Kind regards Uffe From mboxrd@z Thu Jan 1 00:00:00 1970 From: ulf.hansson@linaro.org (Ulf Hansson) Date: Fri, 27 Jan 2017 16:12:24 +0100 Subject: [PATCH v5 06/12] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality In-Reply-To: <2e67332d-d8c3-fd24-1f81-72675a09927a@intel.com> References: <3546f143091c121face8ecbcbf7f6a4c41d2cec7.1484154449.git-series.gregory.clement@free-electrons.com> <2e67332d-d8c3-fd24-1f81-72675a09927a@intel.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 26 January 2017 at 13:39, Adrian Hunter wrote: > On 26/01/17 12:50, Ulf Hansson wrote: >> On 11 January 2017 at 18:19, Gregory CLEMENT >> 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? Kind regards Uffe