| Message ID | 1400146669-30302-1-git-send-email-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 --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 */