From: Alim Akhtar <alim.akhtar@samsung.com>
To: Jaehoon Chung <jh80.chung@gmail.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
Ulf Hansson <ulf.hansson@linaro.org>,
Shawn Lin <shawn.lin@rock-chips.com>
Subject: Re: [1/2] mmc: dw_mmc: set to MMC_CAP_ERASE by default
Date: Fri, 15 Jul 2016 17:16:16 +0530 [thread overview]
Message-ID: <5788CD08.2060004@samsung.com> (raw)
In-Reply-To: <CAELcNGTYJvaSJCK_ALm7vaoErkgrdL03y1GbNTtDAoXDso0R6w@mail.gmail.com>
On 07/15/2016 05:03 PM, Jaehoon Chung wrote:
> Hi Alim
>
> 2016-07-15 19:38 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com
> <mailto:alim.akhtar@samsung.com>>:
>
>
>
> On 07/15/2016 10:21 AM, Jaehoon Chung wrote:
>
> On 07/15/2016 01:38 PM, Alim Akhtar wrote:
>
> Hi Jaehoon
>
> On 07/15/2016 07:24 AM, Jaehoon Chung wrote:
>
> This flag needs to use the trim/discard/erase commands.
> dwmmc controller enables this flag by default.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com
> <mailto:jh80.chung@samsung.com>>
> Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com
> <mailto:shawn.lin@rock-chips.com>>
> ---
> drivers/mmc/host/dw_mmc.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c
> b/drivers/mmc/host/dw_mmc.c
> index 9fab5ed..d16de19 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2604,6 +2604,12 @@ static int
> dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> if (host->pdata->caps)
> mmc->caps = host->pdata->caps;
>
> + /*
> + * Support MMC_CAP_ERASE by default.
> + * It needs to use trim/discard/erase commands.
> + */
> + mmc->caps |= MMC_CAP_ERASE;
> +
>
> Just a thought, probably this should be move to
> mmc_of_parse() and let the board/platform configure this via
> device-tree.
>
>
> I don't think so...I think best solution is supported by default.
> I didn't see the platform/board that don't need to use
> MMC_CAP_ERASE.
>
> If MMC_CAP_ERASE should be moved into mmc_of_parse(), it also
> needs to modify the almost all device-trees.
> If setting by default will have side effect,
> i will consider about abandoning this patch or adding other
> things to prevent side-effect. :)
>
> My point was, MMC_CAP_ERASE is a generic capability not dw_mmc
> specific. Suppose some other controller wants to enable this, then
> they need to add this CAP to their files/drivers. And probably thats
> why mmc_of_parse() was introduced at common place.
>
>
> I understood what you said..If I understood right, it can be located
> into mmc_of_parse(), right?
> Then other drivers are also used by default..but some SoCs can have a
> problem.
No, if someone wants they should pass mmc-cap-erase and enable it.
In case those SoC has some issue with ERASE command, then they will not
set it in their DTS file.
And ofcourse to enable it, dts files need to be modified.
> So if it is located into mmc_of_parse(), i will add the
> "mmc-cap-no-erase" as property.
>
I don't see a use case for this, if you can explain more?
> How about this?
>
> Best Regards,
> Jaehoon Chung
>
>
> Since you are adding this, lets add in a common place for a large use.
> Anyway this was just a though, if you don't like this, I am ok. :-)
>
>
> Best Regards,
> Jaehoon Chung
>
>
> if (host->pdata->pm_caps)
> mmc->pm_caps = host->pdata->pm_caps;
>
>
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> <mailto:majordomo@vger.kernel.org>
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
next prev parent reply other threads:[~2016-07-15 11:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-15 1:54 [PATCH 1/2] mmc: dw_mmc: set to MMC_CAP_ERASE by default Jaehoon Chung
2016-07-15 1:54 ` [PATCH 2/2] mmc: dw_mmc: rockchip: unset the MMC_CAP_ERASE flag Jaehoon Chung
2016-07-15 2:09 ` Shawn Lin
2016-07-18 11:20 ` Ulf Hansson
2016-07-15 2:11 ` [PATCH 1/2] mmc: dw_mmc: set to MMC_CAP_ERASE by default Shawn Lin
2016-07-15 4:38 ` [1/2] " Alim Akhtar
2016-07-15 4:51 ` Jaehoon Chung
2016-07-15 10:38 ` Alim Akhtar
[not found] ` <CAELcNGTYJvaSJCK_ALm7vaoErkgrdL03y1GbNTtDAoXDso0R6w@mail.gmail.com>
2016-07-15 11:46 ` Alim Akhtar [this message]
2016-07-18 2:15 ` Jaehoon Chung
2016-07-18 11:20 ` [PATCH 1/2] " Ulf Hansson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5788CD08.2060004@samsung.com \
--to=alim.akhtar@samsung.com \
--cc=jh80.chung@gmail.com \
--cc=jh80.chung@samsung.com \
--cc=linux-mmc@vger.kernel.org \
--cc=shawn.lin@rock-chips.com \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).