diff mbox

[v2,12/14] mmc: mmci: add support for fbclk to latch data and cmd.

Message ID 1400146669-30302-1-git-send-email-srinivas.kandagatla@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Srinivas Kandagatla May 15, 2014, 9:37 a.m. UTC
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds support to fbclk that is used to latch data and
cmd on some controllers like SD Card controller in Qcom SOC.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/mmc/host/mmci.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Linus Walleij May 23, 2014, 9:12 a.m. UTC | #1
On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds support to fbclk that is used to latch data and
> cmd on some controllers like SD Card controller in Qcom SOC.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

(...)

Isn't this overkill?

@@ -189,6 +192,7 @@ static struct variant_data variant_qcom = {
         .clkreg                 = MCI_CLK_ENABLE,
-        .clkreg_enable          = MCI_QCOM_CLK_FLOWENA,
+       .clkreg_enable          = MCI_QCOM_CLK_FLOWENA |
MCI_QCOM_CLK_FEEDBACK_CLK,
         .clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,

Isn't this achieveing exactly the same thing without the extra fields?
You unconditionally do it at every enable anyway, don't you?

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
Srinivas Kandagatla May 23, 2014, 9:20 a.m. UTC | #2
On 23/05/14 10:12, Linus Walleij wrote:
> On Thu, May 15, 2014 at 11:37 AM,  <srinivas.kandagatla@linaro.org> wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds support to fbclk that is used to latch data and
>> cmd on some controllers like SD Card controller in Qcom SOC.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> (...)
>
> Isn't this overkill?

I totally agree.

Initially I did do it the way you suggested, but wanted to be more 
explicit in what its actually doing and I was also not sure if its Ok to 
add more than one flag in clkreg_enable.

>
> @@ -189,6 +192,7 @@ static struct variant_data variant_qcom = {
>           .clkreg                 = MCI_CLK_ENABLE,
> -        .clkreg_enable          = MCI_QCOM_CLK_FLOWENA,
> +       .clkreg_enable          = MCI_QCOM_CLK_FLOWENA |
> MCI_QCOM_CLK_FEEDBACK_CLK,
>           .clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
>

> Isn't this achieveing exactly the same thing without the extra fields?
> You unconditionally do it at every enable anyway, don't you?
I will fix it and send a new version.

Thanks,
srini
>
> 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 mbox

Patch

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 05ae654..bc7b80d 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -54,6 +54,8 @@  static unsigned int fmax = 515633;
  * @clkreg_enable: enable value for MMCICLOCK register
  * @clkreg_8bit_bus_enable: enable value for 8 bit bus
  * @clkreg_neg_edge_enable: enable value for inverted data/cmd output
+ * @clkreg_fbclk_latch: enable value to select feedback clock to
+ *			latch data and command comming in.
  * @datalength_bits: number of bits in the MMCIDATALENGTH register
  * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY
  *	      is asserted (likewise for RX)
@@ -79,6 +81,7 @@  struct variant_data {
 	unsigned int		clkreg_enable;
 	unsigned int		clkreg_8bit_bus_enable;
 	unsigned int		clkreg_neg_edge_enable;
+	unsigned int		clkreg_fbclk_latch;
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
@@ -189,6 +192,7 @@  static struct variant_data variant_qcom = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_QCOM_CLK_FLOWENA,
 	.clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
+	.clkreg_fbclk_latch	= MCI_QCOM_CLK_FEEDBACK_CLK,
 	.datactrl_mask_ddrmode	= MCI_QCOM_CLK_DDR_MODE,
 	.data_cmd_enable	= MCI_QCOM_CSPM_DATCMD,
 	.blksz_datactrl4	= true,
@@ -343,6 +347,7 @@  static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 			host->cclk = host->mclk / (2 * (clk + 1));
 		}
 
+		clk |= variant->clkreg_fbclk_latch;
 		clk |= variant->clkreg_enable;
 		clk |= MCI_CLK_ENABLE;
 		/* This hasn't proven to be worthwhile */