Skip to contentSkip to conversation

mmc: core: proper ocr negotiation during resume

PATCH NAK Prasanna NAVARATNA 2 participants 2 messages May 16, 2013 MMC, SD and SDIO

lore
Share & tools
lore

Prasanna NAVARATNA

author
#0 lore raw

Hello,

There seems some problem with ocr negotiation during resume. While going for suspend, in mmc_power_off function, it resets ocr mask to be the highest possible voltage supported for this mmc host. This value will be used at next power up without actually negotiating with card's ocr by querying, as done during initialization. This results in higher voltage during resume compared to voltage used during initialization.

To fix this issue, i propose the following patch. Please review :-

Quoted 1 line

From 305c151cb5b217ff7c622a769df120b14153c25a Mon Sep 17 00:00:00 2001

From: Prasanna NAVARATNA <[email protected]>
Date: Thu, 4 Apr 2013 19:55:19 +0530
Subject: [PATCH] mmc: core: proper ocr negotiation during resume

Save the card ocr into struct mmc_card when read from card during initialization of sd/mmc/sdio.
Druing mmc_resume_host, supply saved card's ocr to mmc_select_voltage so as to negotiate voltage appropriately.

---
 drivers/mmc/core/core.c  |    1 +
 drivers/mmc/core/mmc.c   |    3 +++
 drivers/mmc/core/sd.c    |    3 +++
 drivers/mmc/core/sdio.c  |    3 +++
 include/linux/mmc/card.h |    1 +
 5 files changed, 11 insertions(+), 0 deletions(-)
drivers/mmc/core/core.c +1 -0
index ad7decc..e61b5ab 100644
@@ -2680,6 +2680,7 @@ int mmc_resume_host(struct mmc_host *host)
 	if (host->bus_ops && !host->bus_dead) {
 		if (!mmc_card_keep_power(host)) {
 			mmc_power_up(host);
+			host->ocr = host->card ? host->card->ocr : host->ocr;
 			mmc_select_voltage(host, host->ocr);
 			/*
 			 * Tell runtime PM core we just powered up the card,
drivers/mmc/core/mmc.c +3 -0
index 0cbd1ef..9b4b3a5 100644
@@ -1566,6 +1566,9 @@ int mmc_attach_mmc(struct mmc_host *host)
 		ocr &= ~0x7F;
 	}
 
+	/* Save the card OCR */
+	host->card->ocr = ocr;
+
 	host->ocr = mmc_select_voltage(host, ocr);
 
 	/*
drivers/mmc/core/sd.c +3 -0
index 9e645e1..965504b 100644
@@ -1185,6 +1185,9 @@ int mmc_attach_sd(struct mmc_host *host)
 		ocr &= ~MMC_VDD_165_195;
 	}
 
+	/* Save the card OCR */
+	host->card->ocr = ocr;
+
 	host->ocr = mmc_select_voltage(host, ocr);
 
 	/*
drivers/mmc/core/sdio.c +3 -0
index aa0719a..3f9e08d 100644
@@ -1090,6 +1090,9 @@ int mmc_attach_sdio(struct mmc_host *host)
 		ocr &= ~0x7F;
 	}
 
+	/* Save the card OCR */
+	host->card->ocr = ocr;
+
 	host->ocr = mmc_select_voltage(host, ocr);
 
 	/*
include/linux/mmc/card.h +1 -0
index f31725b..1bbec2f 100644
@@ -273,6 +273,7 @@ struct mmc_card {
 	u32			raw_cid[4];	/* raw card CID */
 	u32			raw_csd[4];	/* raw card CSD */
 	u32			raw_scr[2];	/* raw card SCR */
+	u32			ocr;		/* card OCR */
 	struct mmc_cid		cid;		/* card identification */
 	struct mmc_csd		csd;		/* card specific */
 	struct mmc_ext_csd	ext_csd;	/* mmc v4 extended card specific */
Signature · 1 line
1.7.6
NAK
#1 lore raw
Quoted 16 lines · Prasanna NAVARATNA

Hello,

There seems some problem with ocr negotiation during resume. While going for suspend, in mmc_power_off function, it resets ocr mask to be the highest possible voltage supported for this mmc host. This value will be used at next power up without actually negotiating with card's ocr by querying, as done during initialization. This results in higher voltage during resume compared to voltage used during initialization.

To fix this issue, i propose the following patch. Please review :-

From 305c151cb5b217ff7c622a769df120b14153c25a Mon Sep 17 00:00:00 2001
From: Prasanna NAVARATNA <[email protected]>
Date: Thu, 4 Apr 2013 19:55:19 +0530
Subject: [PATCH] mmc: core: proper ocr negotiation during resume

Quoted 4 lines

Save the card ocr into struct mmc_card when read from card during initialization of sd/mmc/sdio.
Druing mmc_resume_host, supply saved card's ocr to mmc_select_voltage so as to negotiate voltage appropriately.

We have already discussed this issue on the mailing list I believe. Your patch will not solve anything on it's own.

Quoted 13 lines
---
 drivers/mmc/core/core.c  |    1 +
 drivers/mmc/core/mmc.c   |    3 +++
 drivers/mmc/core/sd.c    |    3 +++
 drivers/mmc/core/sdio.c  |    3 +++
 include/linux/mmc/card.h |    1 +
 5 files changed, 11 insertions(+), 0 deletions(-)
drivers/mmc/core/core.c +0 -0
index ad7decc..e61b5ab 100644

drivers/mmc/core/core.c int mmc_resume_host(struct mmc_host *host) · line 2684

Quoted 5 lines
drivers/mmc/core/core.c +1 -0
@@ -2680,6 +2680,7 @@ int mmc_resume_host(struct mmc_host *host)
        if (host->bus_ops && !host->bus_dead) {
                if (!mmc_card_keep_power(host)) {
                        mmc_power_up(host);
+                       host->ocr = host->card ? host->card->ocr : host->ocr;
                        mmc_select_voltage(host, host->ocr);

mmc_select_voltage will manipulate the vdd/ocr, but at this point the card is already powered due to state is MMC_POWER_ON.

Quoted 7 lines
drivers/mmc/core/core.c +0 -0
                        /*
                         * Tell runtime PM core we just powered up the card,
drivers/mmc/core/mmc.c +0 -0
index 0cbd1ef..9b4b3a5 100644

drivers/mmc/core/mmc.c int mmc_attach_mmc(struct mmc_host *host) · line 1572

Quoted 7 lines
drivers/mmc/core/mmc.c +3 -0
@@ -1566,6 +1566,9 @@ int mmc_attach_mmc(struct mmc_host *host)
                ocr &= ~0x7F;
        }
 
+       /* Save the card OCR */
+       host->card->ocr = ocr;
+
        host->ocr = mmc_select_voltage(host, ocr);

dito

Quoted 7 lines
drivers/mmc/core/mmc.c +0 -0
        /*
drivers/mmc/core/sd.c +0 -0
index 9e645e1..965504b 100644

drivers/mmc/core/sd.c int mmc_attach_sd(struct mmc_host *host) · line 1191

Quoted 7 lines
drivers/mmc/core/sd.c +3 -0
@@ -1185,6 +1185,9 @@ int mmc_attach_sd(struct mmc_host *host)
                ocr &= ~MMC_VDD_165_195;
        }
 
+       /* Save the card OCR */
+       host->card->ocr = ocr;
+
        host->ocr = mmc_select_voltage(host, ocr);

dito

Quoted 7 lines
drivers/mmc/core/sd.c +0 -0
        /*
drivers/mmc/core/sdio.c +0 -0
index aa0719a..3f9e08d 100644

drivers/mmc/core/sdio.c int mmc_attach_sdio(struct mmc_host *host) · line 1096

Quoted 7 lines
drivers/mmc/core/sdio.c +3 -0
@@ -1090,6 +1090,9 @@ int mmc_attach_sdio(struct mmc_host *host)
                ocr &= ~0x7F;
        }
 
+       /* Save the card OCR */
+       host->card->ocr = ocr;
+
        host->ocr = mmc_select_voltage(host, ocr);

dito

Quoted 18 lines
drivers/mmc/core/sdio.c +0 -0
        /*
include/linux/mmc/card.h +1 -1
index f31725b..1bbec2f 100644
@@ -273,6 +273,7 @@ struct mmc_card {
        u32                     raw_cid[4];     /* raw card CID */
        u32                     raw_csd[4];     /* raw card CSD */
        u32                     raw_scr[2];     /* raw card SCR */
+       u32                     ocr;            /* card OCR */
        struct mmc_cid          cid;            /* card identification */
        struct mmc_csd          csd;            /* card specific */
        struct mmc_ext_csd      ext_csd;        /* mmc v4 extended card specific */
--

1.7.6

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Nack!

This is not the proper way to solve this issue. We need a way to negotiate _AND_ use the correct ocr mask already at the first card initialization sequence, this is not done today. When that is solved, we don't need mmc_select_voltage as a part of the mmc_resume_host sequence I believe.

Kind regards
Ulf Hansson