On 11/15, Ritesh Harjani wrote: > On 11/15/2016 1:07 AM, Stephen Boyd wrote: > >On 11/14, Ritesh Harjani wrote: > > > >>+} > >>+ > >>+/* sdhci_msm_set_clock - Called with (host->lock) spinlock held. */ > >>+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); > >>+ int rc; > >>+ > >>+ if (!clock) { > >>+ msm_host->clk_rate = clock; > >>+ goto out; > >>+ } > >>+ > >>+ spin_unlock_irq(&host->lock); > >>+ if (clock != msm_host->clk_rate) { > > > >Why do we need to check here? Can't we call clk_set_rate() > >Unconditionally? > Since it may so happen that above layers may call for ->set_clock > function with same requested clock more than once, hence we cache > the host->clock here. > Also, since requested clock (host->clock) can be say 400Mhz but the > actual pltfm supported clock would be say 384MHz. clk_set_rate() detects the same rate being set even after it internally rounds the rate. We're not going to touch the clk hardware if 400 is requested once but 384 is what's set and then 400 is requested again. Caching the rate here in the driver can lead to problems too if the driver is out of sync with the clk hardware state, so it's best to avoid doing anything fancy here and just let the framework handle duplicates. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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