On 22/08/16 12:07, Ritesh Harjani wrote: > Hi Adrian, > > > On 8/22/2016 11:50 AM, Adrian Hunter wrote: >> On 19/08/16 16:31, Ritesh Harjani wrote: >>> Hi, >>> >>> >>> On 8/19/2016 6:34 PM, Adrian Hunter wrote: >>>> On 19/08/16 07:36, Ritesh Harjani wrote: >>>>> sdhci-msm controller may have different clk-rates for each >>>>> bus speed mode. Thus implement set_clock callback for >>>>> sdhci-msm driver. >>>>> >>>>> Signed-off-by: Sahitya Tummala <stummala@xxxxxxxxxxxxxx> >>>>> Signed-off-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxxx> >>>>> --- >>>>> drivers/mmc/host/sdhci-msm.c | 103 >>>>> ++++++++++++++++++++++++++++++++++++++++++- >>>>> 1 file changed, 102 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c >>>>> index 7c032c3..c0ad9c2 100644 >>>>> --- a/drivers/mmc/host/sdhci-msm.c >>>>> +++ b/drivers/mmc/host/sdhci-msm.c >>>>> @@ -89,6 +89,7 @@ struct sdhci_msm_host { >>>>> struct mmc_host *mmc; >>>>> bool use_14lpp_dll_reset; >>>>> struct sdhci_msm_pltfm_data *pdata; >>>>> + u32 clk_rate; >>>>> }; >>>>> >>>>> /* Platform specific tuning */ >>>>> @@ -582,6 +583,106 @@ static unsigned int sdhci_msm_get_min_clock(struct >>>>> sdhci_host *host) >>>>> return msm_host->pdata->clk_table[0]; >>>>> } >>>>> >>>>> +static unsigned int sdhci_msm_get_msm_clk_rate(struct sdhci_host *host, >>>>> + u32 req_clk) >>>>> +{ >>>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >>>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); >>>>> + int count = msm_host->pdata->clk_table_sz; >>>>> + unsigned int sel_clk = -1; >>>>> + int cnt; >>>>> + >>>>> + if (req_clk < sdhci_msm_get_min_clock(host)) { >>>>> + sel_clk = sdhci_msm_get_min_clock(host); >>>>> + return sel_clk; >>>>> + } >>>>> + >>>>> + for (cnt = 0; cnt < count; cnt++) { >>>>> + if (msm_host->pdata->clk_table[cnt] > req_clk) { >>>>> + break; >>>>> + } else if (msm_host->pdata->clk_table[cnt] == req_clk) { >>>>> + sel_clk = msm_host->pdata->clk_table[cnt]; >>>>> + break; >>>>> + } else { >>>>> + sel_clk = msm_host->pdata->clk_table[cnt]; >>>>> + } >>>>> + } >>>> >>>> 'else' is not needed after 'break' but can't this be simpler e.g. >>>> >>>> static unsigned int sdhci_msm_get_msm_clk_rate(struct sdhci_msm_host >>>> *msm_host, u32 req_clk) >>>> { >>>> int count = msm_host->pdata->clk_table_sz; >>>> unsigned int sel_clk = -1; >>>> >>>> while (count--) { >>>> sel_clk = msm_host->pdata->clk_table[count]; >>>> if (req_clk >= sel_clk) >>>> return sel_clk; >>>> } >>>> >>>> return sel_clk; >>>> } >>> >>> Ok, sure I will check and get back on this. >>> >>> >>>> >>>> >>>>> + return sel_clk; >>>>> +} >>>> >>>> Blank line needed >>> Ok done. >>> >>>> >>>>> +/** >>>>> + * __sdhci_msm_set_clock - sdhci_msm clock control. >>>>> + * >>>>> + * Description: >>>>> + * Implement MSM version of sdhci_set_clock. >>>>> + * This is required since MSM controller does not >>>>> + * use internal divider and instead directly control >>>>> + * the GCC clock as per HW recommendation. >>>>> + **/ >>>>> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock) >>>>> +{ >>>>> + u16 clk; >>>>> + unsigned long timeout; >>>>> + >>>>> + host->mmc->actual_clock = 0; >>>>> + >>>>> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL); >>>>> + >>>>> + if (clock == 0) >>>>> + return; >>>> >>>> Should set host->mmc->actual_clock to the actual rate somewhere. >>> Since MSM controller does not uses divider then there is no need of having >>> actual clock since it should be same as host->clock. >> >> Ok, so sdhci_msm_get_msm_clk_rate() is not the actual clock rate? > So I was assuming we need host->mmc->actual_clock rate if host->clock is not > the actual rate. > Ok, so I will update actual_clock to host->clock itself. > > pls, let me know if any concerns. I am confused. Isn't clk_get_rate(msm_host->clk) the actual clock rate? >> >>> >>> That's why I had kept it 0 intentionally. But I will add a comment >>> here then. >>> >>>> >>>>> + >>>>> + /* >>>>> + * MSM controller do not use clock divider. >>>>> + * Thus read SDHCI_CLOCK_CONTROL and only enable >>>>> + * clock with no divider value programmed. >>>>> + */ >>>>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); >>>>> + >>>>> + clk |= SDHCI_CLOCK_INT_EN; >>>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); >>>>> + >>>>> + /* Wait max 20 ms */ >>>>> + timeout = 20; >>>>> + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL)) >>>>> + & SDHCI_CLOCK_INT_STABLE)) { >>>>> + if (timeout == 0) { >>>>> + pr_err("%s: Internal clock never stabilised.\n", >>>>> + mmc_hostname(host->mmc)); >>>>> + return; >>>>> + } >>>>> + timeout--; >>>>> + mdelay(1); >>>>> + } >>>>> + >>>>> + clk |= SDHCI_CLOCK_CARD_EN; >>>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); >>>>> +} >>>>> + >>>>> +static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int >>>>> clock) >>>>> +{ >>>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >>>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); >>>>> + u32 msm_clock = 0; >>>>> + int rc = 0; >>>>> + >>>>> + if (!clock) >>>>> + goto out; >>>>> + >>>>> + if (clock != msm_host->clk_rate) { >>>>> + msm_clock = sdhci_msm_get_msm_clk_rate(host, clock); >>>>> + rc = clk_set_rate(msm_host->clk, msm_clock); >>>>> + if (rc) { >>>>> + pr_err("%s: failed to set clock at rate %u, requested clock >>>>> rate %u\n", >>>>> + mmc_hostname(host->mmc), msm_clock, clock); >>>>> + goto out; >>>>> + } >>>>> + msm_host->clk_rate = clock; >>>>> + pr_debug("%s: setting clock at rate %lu\n", >>>>> + mmc_hostname(host->mmc), clk_get_rate(msm_host->clk)); >>>>> + } >>>>> +out: >>>>> + __sdhci_msm_set_clock(host, clock); >>>>> +} >>>>> + >>>>> static const struct of_device_id sdhci_msm_dt_match[] = { >>>>> { .compatible = "qcom,sdhci-msm-v4" }, >>>>> {}, >>>>> @@ -592,7 +693,7 @@ MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match); >>>>> static const struct sdhci_ops sdhci_msm_ops = { >>>>> .platform_execute_tuning = sdhci_msm_execute_tuning, >>>>> .reset = sdhci_reset, >>>>> - .set_clock = sdhci_set_clock, >>>>> + .set_clock = sdhci_msm_set_clock, >>>>> .get_min_clock = sdhci_msm_get_min_clock, >>>>> .get_max_clock = sdhci_msm_get_max_clock, >>>>> .set_bus_width = sdhci_set_bus_width, >>>>> >>>> >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html