Re: [1/2] mmc: dw_mmc: set to MMC_CAP_ERASE by default

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 07/15/2016 05:03 PM, Jaehoon Chung wrote:
Hi Alim

2016-07-15 19:38 GMT+09:00 Alim Akhtar <alim.akhtar@xxxxxxxxxxx
<mailto:alim.akhtar@xxxxxxxxxxx>>:



    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@xxxxxxxxxxx
                <mailto:jh80.chung@xxxxxxxxxxx>>
                Reviewed-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx
                <mailto:shawn.lin@xxxxxxxxxxxxxx>>
                ---
                    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@xxxxxxxxxxxxxxx
    <mailto:majordomo@xxxxxxxxxxxxxxx>
    More majordomo info at http://vger.kernel.org/majordomo-info.html


--
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



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux