diff mbox

[v1,07/11] mmc: mmci: move ST specific register extensions access under condition.

Message ID 1398759627-13256-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>

This patch moves some of the ST specific register extensions access under
condition, so that other SOCs like Qualcomm or ARM would not a side effect of
writing to those reserved/different purpose bits.

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

Comments

Linus Walleij May 13, 2014, 8:08 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>
>
> This patch moves some of the ST specific register extensions access under
> condition, so that other SOCs like Qualcomm or ARM would not a side effect of
> writing to those reserved/different purpose bits.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
(...)

>         /* Keep ST Micro busy mode if enabled */
> -       datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
> +       if (host->hw_designer == AMBA_VENDOR_ST)
> +               datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;

Do not hard-check the hw_designer everywhere, follow the pattern
if storing special stuff in the variant data.

struct variant_data {
        u32 datactrl_mask_busymode;
(...)

static struct variant_data variant_u300 = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_nomadik = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_ux500 = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_ux500v2 = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)

Then end up like this:

>         /* Keep ST Micro busy mode if enabled */
> -       datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
> +      datactrl |= host->datactrl_reg & host->vendor->datactrl_mask_busymode;

OK we know this should have been done like this from the beginning
but that is not an excuse not to fix it up now.

> -       if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
> +       if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 &&
> +           host->hw_designer == AMBA_VENDOR_ST)
>                 datactrl |= MCI_ST_DPSM_DDRMODE;

Same pattern here.

Actually I think this is only available on Ux500v2 (Ulf? Can you verify
this) so it is probably plain wrong to do it for other variants.

struct variant_data {
        u32 datactrl_ddrmode;
(...)
static struct variant_data variant_ux500v2 = {
        .datactrl_ddrmode = MCI_ST_DPSM_DDRMODE,
(...)

Then end up like this:

>        if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
> -                datactrl |= MCI_ST_DPSM_DDRMODE;
> +               datactrl |= host->vendor->datactrl_ddrmode;

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:33 a.m. UTC | #2 
On 13/05/14 09:08, Linus Walleij wrote:
>> >         /* Keep ST Micro busy mode if enabled */
>> >-       datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
>> >+       if (host->hw_designer == AMBA_VENDOR_ST)
>> >+               datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
> Do not hard-check the hw_designer everywhere, follow the pattern
> if storing special stuff in the variant data.
>
Got it, I will fix this across other patches too.

> struct variant_data {
>          u32 datactrl_mask_busymode;
> (...)
>
> static struct variant_data variant_u300 = {
>          .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
> (...)
> static struct variant_data variant_nomadik = {
--
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 f73dc48..306e0c8 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -269,7 +269,8 @@  static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
 static void mmci_write_datactrlreg(struct mmci_host *host, u32 datactrl)
 {
 	/* Keep ST Micro busy mode if enabled */
-	datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
+	if (host->hw_designer == AMBA_VENDOR_ST)
+		datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
 
 	if (host->datactrl_reg != datactrl) {
 		host->datactrl_reg = datactrl;
@@ -815,7 +816,8 @@  static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 			mmci_write_clkreg(host, clk);
 		}
 
-	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
+	if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 &&
+	    host->hw_designer == AMBA_VENDOR_ST)
 		datactrl |= MCI_ST_DPSM_DDRMODE;
 
 	/*