[U-Boot] [PATCH v2] mmc: sdhci: Add the programmable clock mode support

Jaehoon Chung jh80.chung at samsung.com
Mon Oct 10 08:42:28 CEST 2016


Dear Kever,

On 10/07/2016 04:10 PM, Kever Yang wrote:
> Hi Jaehoon,
> 
> On 09/30/2016 06:48 PM, Jaehoon Chung wrote:
>> Hi Kever,
>>
>> On 09/30/2016 07:05 PM, Kever Yang wrote:
>>> Hi Wenyou,
>>>
>>>      I can not enable my emmc device  on my evb-rk3399 with this patch, could you help to fix it?
>>>
>>> Here is the debug info may help:
>>>
>>> mmc->cfg->f_max is 200000000;
>>>
>>> [with this patch]
>>> I get host->clk_mul 16 ;
>>> When driver want to set clock to 400000, always get div 1024, and then write 0x21 to SDHCI_CLOCK_CONTROL register,
>>> mmc controller can not work because of the wrong clock rate;
>>>
>>> [without this patch]
>>> we don't use host->clk_mul;
>>> When driver want to set clock to 400000, get div 250, and then write 0xfa01 to SDHCI_CLOCK_CONTROL register,
>>> mmc controller works OK.
>> If you don't use host->clk_mul, then your host controller is the lower version than V3.0..?
>>
>> host->clk_mul is assigned to dummy value? Could you check this?
>>
>> When i have checked Whenyou's patch, there is only difference about "host->clk_mul".
> 
> The host controller in rk3399 is V3.0, and with Clock Multiplier default to 16 in capabilities register.
> 
> See my comments in source code.
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> Thanks,
>>> - Kever
>>> On 09/18/2016 09:01 AM, Wenyou Yang wrote:
>>>> Add the programmable clock mode for the clock generator.
>>>>
>>>> Signed-off-by: Wenyou Yang <wenyou.yang at atmel.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>>    - Rebase on the latest u-boot-mmc.
>>>>    - Fix the typo Muliplier->Multiplier.
>>>>
>>>>    drivers/mmc/sdhci.c | 50 ++++++++++++++++++++++++++++++++++++++++----------
>>>>    include/sdhci.h     |  2 ++
>>>>    2 files changed, 42 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>>>> index 504f2d2..b2bf5a0 100644
>>>> --- a/drivers/mmc/sdhci.c
>>>> +++ b/drivers/mmc/sdhci.c
>>>> @@ -294,7 +294,7 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
>>>>    static int sdhci_set_clock(struct mmc *mmc, unsigned int clock)
>>>>    {
>>>>        struct sdhci_host *host = mmc->priv;
>>>> -    unsigned int div, clk, timeout, reg;
>>>> +    unsigned int div, clk = 0, timeout, reg;
>>>>          /* Wait max 20 ms */
>>>>        timeout = 200;
>>>> @@ -318,14 +318,36 @@ static int sdhci_set_clock(struct mmc *mmc, unsigned int clock)
>>>>            return 0;
>>>>          if (SDHCI_GET_VERSION(host) >= SDHCI_SPEC_300) {
>>>> -        /* Version 3.00 divisors must be a multiple of 2. */
>>>> -        if (mmc->cfg->f_max <= clock)
>>>> -            div = 1;
>>>> -        else {
>>>> -            for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>>>> -                if ((mmc->cfg->f_max / div) <= clock)
>>>> +        /*
>>>> +         * Check if the Host Controller supports Programmable Clock
>>>> +         * Mode.
>>>> +         */
>>>> +        if (host->clk_mul) {
>>>> +            for (div = 1; div <= 1024; div++) {
>>>> +                if ((mmc->cfg->f_max * host->clk_mul / div)
>>>> +                    <= clock)
>>>>                        break;
>>>>                }
>>>> +
>>>> +            /*
>>>> +             * Set Programmable Clock Mode in the Clock
>>>> +             * Control register.
>>>> +             */
>>>> +            clk = SDHCI_PROG_CLOCK_MODE;
>>>> +            div--;
> 
> 
> According to this patch, my f_max is 200MHz, if we want to get the 400KHz with clk_mul 16, then the divider
> would be 200M*16/400K = 4000 which is lager than 1024.
> 
> What's the benefit for us to enable the Programmable Clock Mode(PCM) with multiplier?
> Can we have option here to still using the Divided Clock Mode instead of PCM?
> Even if we have to use PCM mode, the clock generator must have some limitation for the clock, right?
> Can we add some error check/report? eg. My input 200MHz, after multiplier with 16 it become 3.2GHz,
> I believe most of PLL can not do it, so I suppose I still get fail even if I can get the correct divider when I want 50MHz
> instead of 400KHz with the same clock source.
> 
> BTW, what's your value of f_max and clk_mul? Do you use one input for both 400KHz and 52MHz or 100MHz/200MHz?

I think We can fix this issue for checking clk_mul...I will send the patch then could you test the patch?

Best Regards,
Jaehoon Chung

> 
> Thanks,
> - Kever
> 
>>>> +        } else {
>>>> +            /* Version 3.00 divisors must be a multiple of 2. */
>>>> +            if (mmc->cfg->f_max <= clock) {
>>>> +                div = 1;
>>>> +            } else {
>>>> +                for (div = 2;
>>>> +                     div < SDHCI_MAX_DIV_SPEC_300;
>>>> +                     div += 2) {
>>>> +                    if ((mmc->cfg->f_max / div) <= clock)
>>>> +                        break;
>>>> +                }
>>>> +            }
>>>> +            div >>= 1;
>>>>            }
>>>>        } else {
>>>>            /* Version 2.00 divisors must be a power of 2. */
>>>> @@ -333,13 +355,13 @@ static int sdhci_set_clock(struct mmc *mmc, unsigned int clock)
>>>>                if ((mmc->cfg->f_max / div) <= clock)
>>>>                    break;
>>>>            }
>>>> +        div >>= 1;
>>>>        }
>>>> -    div >>= 1;
>>>>          if (host->set_clock)
>>>>            host->set_clock(host->index, div);
>>>>    -    clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>>> +    clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>>>        clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>>>>            << SDHCI_DIVIDER_HI_SHIFT;
>>>>        clk |= SDHCI_CLOCK_INT_EN;
>>>> @@ -513,7 +535,7 @@ static const struct mmc_ops sdhci_ops = {
>>>>    int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
>>>>            u32 max_clk, u32 min_clk)
>>>>    {
>>>> -    u32 caps;
>>>> +    u32 caps, caps_1;
>>>>          caps = sdhci_readl(host, SDHCI_CAPABILITIES);
>>>>    @@ -577,6 +599,14 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
>>>>          cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
>>>>    +    /*
>>>> +     * In case of Host Controller v3.00, find out whether clock
>>>> +     * multiplier is supported.
>>>> +     */
>>>> +    caps_1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>>>> +    host->clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >>
>>>> +            SDHCI_CLOCK_MUL_SHIFT;
>>>> +
>>>>        return 0;
>>>>    }
>>>>    diff --git a/include/sdhci.h b/include/sdhci.h
>>>> index 6844c73..144570f 100644
>>>> --- a/include/sdhci.h
>>>> +++ b/include/sdhci.h
>>>> @@ -97,6 +97,7 @@
>>>>    #define  SDHCI_DIV_MASK    0xFF
>>>>    #define  SDHCI_DIV_MASK_LEN    8
>>>>    #define  SDHCI_DIV_HI_MASK    0x300
>>>> +#define  SDHCI_PROG_CLOCK_MODE  0x0020
>>>>    #define  SDHCI_CLOCK_CARD_EN    0x0004
>>>>    #define  SDHCI_CLOCK_INT_STABLE    0x0002
>>>>    #define  SDHCI_CLOCK_INT_EN    0x0001
>>>> @@ -242,6 +243,7 @@ struct sdhci_host {
>>>>        unsigned int quirks;
>>>>        unsigned int host_caps;
>>>>        unsigned int version;
>>>> +    unsigned int clk_mul;   /* Clock Multiplier value */
>>>>        unsigned int clock;
>>>>        struct mmc *mmc;
>>>>        const struct sdhci_ops *ops;
>>>
>>>
>>>
>>>
>>
>>
>>
> 
> 
> 
> 
> 



More information about the U-Boot mailing list