| Message ID | 1398759651-13341-1-git-send-email-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 29, 2014 at 10:20 AM, <srinivas.kandagatla@linaro.org> wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > MCICLK going to card bus is directly driven by the clock controller, so the > driver has to set the required rates depending on the state of the card. This > bit of support is very much similar to bypass mode but there is no such thing > called bypass mode in MCICLK register of Qcom SD card controller. By default > the clock is directly driven by the clk controller. > > This patch adds clock support for Qualcomm SDCC in the driver. This bit of > code is conditioned on hw designer. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> (...) > + if (host->hw_designer == AMBA_VENDOR_QCOM) { > + host->cclk = host->mclk; > + } else if (desired >= host->mclk) { Again refrain from hard-checking the vendor everywhere. struct variant_data { bool qcom_cclk_is_mclk; (...) As per example from st_clkdiv... Then if (host->vendor->qcom_cclk_is_mclk) { (...) } > + if (ios->clock != host->mclk && > + host->hw_designer == AMBA_VENDOR_QCOM) { > + /* Qcom MCLKCLK register does not define bypass bits */ > + int rc = clk_set_rate(host->clk, ios->clock); > + if (rc < 0) { > + dev_err(mmc_dev(host->mmc), > + "Error setting clock rate (%d)\n", rc); > + } else { > + host->mclk = clk_get_rate(host->clk); > + host->cclk = host->mclk; > + } > + } For this I would define a vendor data like: struct variant_data { bool explicit_mclk_control; (...) Or something. It explains what is actually going on. > if (plat->f_max) > - mmc->f_max = min(host->mclk, plat->f_max); > + mmc->f_max = (host->hw_designer == AMBA_VENDOR_QCOM) ? > + plat->f_max : min(host->mclk, plat->f_max); So rewrite like that: if (host->vendor->explicit_mclk_control) mmc->f_max = plat->f_max; else mmc->f_max = min(host->mclk, plat->f_max); Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks Linus W, On 13/05/14 09:28, Linus Walleij wrote: >> code is conditioned on hw designer. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > (...) >> + if (host->hw_designer == AMBA_VENDOR_QCOM) { >> + host->cclk = host->mclk; >> + } else if (desired >= host->mclk) { > > Again refrain from hard-checking the vendor everywhere. > > struct variant_data { > bool qcom_cclk_is_mclk; > (...) > Got it.. Will fix it in next version. > As per example from st_clkdiv... > > Then > > if (host->vendor->qcom_cclk_is_mclk) { > (...) > } > >> + if (ios->clock != host->mclk && >> + host->hw_designer == AMBA_VENDOR_QCOM) { >> + /* Qcom MCLKCLK register does not define bypass bits */ >> + int rc = clk_set_rate(host->clk, ios->clock); >> + if (rc < 0) { >> + dev_err(mmc_dev(host->mmc), >> + "Error setting clock rate (%d)\n", rc); >> + } else { >> + host->mclk = clk_get_rate(host->clk); >> + host->cclk = host->mclk; >> + } >> + } > > For this I would define a vendor data like: > > struct variant_data { > bool explicit_mclk_control; > (...) This looks good. > > Or something. It explains what is actually going on. > >> if (plat->f_max) >> - mmc->f_max = min(host->mclk, plat->f_max); >> + mmc->f_max = (host->hw_designer == AMBA_VENDOR_QCOM) ? >> + plat->f_max : min(host->mclk, plat->f_max); > > So rewrite like that: > > if (host->vendor->explicit_mclk_control) > mmc->f_max = plat->f_max; > else > mmc->f_max = min(host->mclk, plat->f_max); > This looks much clean > Yours, > Linus Walleij > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 35aed38..da135c0 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -290,7 +290,10 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired) host->cclk = 0; if (desired) { - if (desired >= host->mclk) { + + if (host->hw_designer == AMBA_VENDOR_QCOM) { + host->cclk = host->mclk; + } else if (desired >= host->mclk) { clk = MCI_CLK_BYPASS; if (variant->st_clkdiv) clk |= MCI_ST_UX500_NEG_EDGE; @@ -1371,6 +1374,19 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) if (!ios->clock && variant->pwrreg_clkgate) pwr &= ~MCI_PWR_ON; + if (ios->clock != host->mclk && + host->hw_designer == AMBA_VENDOR_QCOM) { + /* Qcom MCLKCLK register does not define bypass bits */ + int rc = clk_set_rate(host->clk, ios->clock); + if (rc < 0) { + dev_err(mmc_dev(host->mmc), + "Error setting clock rate (%d)\n", rc); + } else { + host->mclk = clk_get_rate(host->clk); + host->cclk = host->mclk; + } + } + spin_lock_irqsave(&host->lock, flags); mmci_set_clkreg(host, ios->clock); @@ -1611,7 +1627,8 @@ static int mmci_probe(struct amba_device *dev, * of course. */ if (plat->f_max) - mmc->f_max = min(host->mclk, plat->f_max); + mmc->f_max = (host->hw_designer == AMBA_VENDOR_QCOM) ? + plat->f_max : min(host->mclk, plat->f_max); else mmc->f_max = min(host->mclk, fmax); dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max);