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

Kever Yang kever.yang at rock-chips.com
Fri Oct 7 09:10:28 CEST 2016


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?

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