diff mbox

[v1,09/11] mmc: mmci: Add clock support for Qualcomm.

Message ID 1398759651-13341-1-git-send-email-srinivas.kandagatla@linaro.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Srinivas Kandagatla April 29, 2014, 8:20 a.m. UTC
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>
---
 drivers/mmc/host/mmci.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

Comments

Linus Walleij May 13, 2014, 8:28 a.m. UTC | #1
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-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srinivas Kandagatla May 13, 2014, 9:39 a.m. UTC | #2
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-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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);