[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