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

2014-05-13 Thread Linus Walleij
On Tue, Apr 29, 2014 at 10:20 AM,  srinivas.kandaga...@linaro.org wrote:

 From: Srinivas Kandagatla srinivas.kandaga...@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.kandaga...@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 {
boolqcom_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 {
boolexplicit_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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-05-13 Thread Srinivas Kandagatla

Thanks Linus W,

On 13/05/14 09:28, Linus Walleij wrote:

code is conditioned on hw designer.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@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 {
 boolqcom_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 {
 boolexplicit_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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html