2014-06-26 17:42 GMT+08:00 Ulf Hansson <ulf.hansson@xxxxxxxxxx>:
>
>
> On 26 juni 2014 08:23:32 CEST, Vincent Yang <vincent.yang.fujitsu@xxxxxxxxx> wrote:
>>This patch adds manual resume for some embedded platforms with rootfs
>>stored in SD card. It references CONFIG_MMC_BLOCK_DEFERRED_RESUME in
>>kernel 3.10. It lets host controller driver to manually handle resume
>>by itself.
>>
>>[symptom]
>>This issue is found on mb86s7x platforms with rootfs stored in SD card.
>>It failed to resume form STR suspend mode because SD card cannot be
>>ready
>>in time. It take longer time (e.g., 600ms) to be ready for access.
>>The error log looks like below:
>>
>>root@localhost:~# echo mem > /sys/power/state
>>[ 30.441974] SUSPEND
>>
>>SCB Firmware : Category 01 Version 02.03 Rev. 00_
>> Config : (no configuration)
>>root@localhost:~# [ 30.702976] Buffer I/O error on device mmcblk1p2,
>>logical block 31349
>>[ 30.709678] Buffer I/O error on device mmcblk1p2, logical block
>>168073
>>[ 30.716220] Buffer I/O error on device mmcblk1p2, logical block
>>168074
>>[ 30.722759] Buffer I/O error on device mmcblk1p2, logical block
>>168075
>>[ 30.729456] Buffer I/O error on device mmcblk1p2, logical block
>>31349
>>[ 30.735916] Buffer I/O error on device mmcblk1p2, logical block
>>31350
>>[ 30.742370] Buffer I/O error on device mmcblk1p2, logical block
>>31351
>>[ 30.749025] Buffer I/O error on device mmcblk1p2, logical block
>>168075
>>[ 30.755657] Buffer I/O error on device mmcblk1p2, logical block
>>31351
>>[ 30.763130] Aborting journal on device mmcblk1p2-8.
>>[ 30.768060] JBD2: Error -5 detected when updating journal superblock
>>for mmcblk1p2-8.
>>[ 30.776085] EXT4-fs error (device mmcblk1p2):
>>ext4_journal_check_start:56: Detected aborted journal
>>[ 30.785259] EXT4-fs (mmcblk1p2): Remounting filesystem read-only
>>[ 31.370716] EXT4-fs error (device mmcblk1p2): ext4_find_entry:1309:
>>inode #2490369: comm udevd: reading directory lblock 0
>>[ 31.382485] EXT4-fs error (device mmcblk1p2): ext4_find_entry:1309:
>>inode #1048577: comm udevd: reading directory lblock 0
>>
>>[analysis]
>>In system resume path, mmc_sd_resume() is failed with error code -123
>>because at that time SD card is still not ready on mb86s7x platforms.
>
> So why does it fail? It shouldn't!
>
> I get the impression that you are solving this in the wrong way.
Hi Uffe,
On mb86s7x EVB, power for SD card is completely removed when entering
STR suspend mode (i.e., echo mem > /sys/power/state). When system
starts to resume, it turns on the power for SD card again. However, It take
longer time (e.g., 600ms) to get the power ready.
This is why mmc_sd_resume() failed with error code -123. At that time point,
power for SD card is not ready yet.
At first we fixed it by a simple method of using SDHCI_QUIRK_DELAY_AFTER_POWER:
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 169e17d..ed28896 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1283,7 +1283,7 @@ static void sdhci_set_power(struct sdhci_host
*host, unsigned char mode,
* they can apply clock after applying power
*/
if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER)
- mdelay(10);
+ mdelay(600);
}
if (host->vmmc) {
However, we found it blocks the system resume path. It can slow down
system resume and also booting.
Therefore, we would like to resume SD card manually and asynchronously
by host controller driver itself. Thus system resume path is not blocked.
Thanks a lot for your review!
Best regards,
Vincent Yang
>
> Kind regards
> Uffe
>
>>
>>[solution]
>>In order to not blocking system resume path, this patch just sets a
>>flag
>>MMC_BUSRESUME_MANUAL_RESUME when this error happened, and then host
>>controller
>>driver can understand it by this flag. Then host controller driver have
>>to
>>resume SD card manually and asynchronously.
>>
>>Signed-off-by: Vincent Yang <Vincent.Yang@xxxxxxxxxxxxxx>
>>---
>> drivers/mmc/core/core.c | 4 ++
>> drivers/mmc/core/sd.c | 4 ++
>>drivers/mmc/host/sdhci_f_sdh30.c | 89
>>++++++++++++++++++++++++++++++++++++++++
>> include/linux/mmc/host.h | 14 +++++++
>> 4 files changed, 111 insertions(+)
>>
>>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>index 764af63..51fce49 100644
>>--- a/drivers/mmc/core/core.c
>>+++ b/drivers/mmc/core/core.c
>>@@ -2648,6 +2648,10 @@ int mmc_pm_notify(struct notifier_block
>>*notify_block,
>> case PM_POST_RESTORE:
>>
>> spin_lock_irqsave(&host->lock, flags);
>>+ if (mmc_bus_manual_resume(host)) {
>>+ spin_unlock_irqrestore(&host->lock, flags);
>>+ break;
>>+ }
>> host->rescan_disable = 0;
>> spin_unlock_irqrestore(&host->lock, flags);
>> _mmc_detect_change(host, 0, false);
>>diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>index 0c44510..859390d 100644
>>--- a/drivers/mmc/core/sd.c
>>+++ b/drivers/mmc/core/sd.c
>>@@ -1133,6 +1133,10 @@ static int mmc_sd_resume(struct mmc_host *host)
>>
>> if (!(host->caps & MMC_CAP_RUNTIME_RESUME)) {
>> err = _mmc_sd_resume(host);
>>+ if ((host->caps2 & MMC_CAP2_MANUAL_RESUME) && err)
>>+ mmc_set_bus_resume_policy(host, 1);
>>+ else
>>+ mmc_set_bus_resume_policy(host, 0);
>> pm_runtime_set_active(&host->card->dev);
>> pm_runtime_mark_last_busy(&host->card->dev);
>> }
>>diff --git a/drivers/mmc/host/sdhci_f_sdh30.c
>>b/drivers/mmc/host/sdhci_f_sdh30.c
>>index 6fae509..67bcff2 100644
>>--- a/drivers/mmc/host/sdhci_f_sdh30.c
>>+++ b/drivers/mmc/host/sdhci_f_sdh30.c
>>@@ -30,6 +30,12 @@
>> #include "../core/core.h"
>>
>> #define DRIVER_NAME "f_sdh30"
>>+#define RESUME_WAIT_COUNT 100
>>+#define RESUME_WAIT_TIME 50
>>+#define RESUME_WAIT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>+#define RESUME_DETECT_COUNT 16
>>+#define RESUME_DETECT_TIME 50
>>+#define RESUME_DETECT_JIFFIES msecs_to_jiffies(RESUME_DETECT_TIME)
>>
>>
>> struct f_sdhost_priv {
>>@@ -38,8 +44,59 @@ struct f_sdhost_priv {
>> int gpio_select_1v8;
>> u32 vendor_hs200;
>> struct device *dev;
>>+ unsigned int quirks; /* Deviations from spec. */
>>+
>>+/* retry to detect mmc device when resume */
>>+#define F_SDH30_QUIRK_RESUME_DETECT_RETRY (1<<0)
>>+
>>+ struct workqueue_struct *resume_detect_wq;
>>+ struct delayed_work resume_detect_work;
>>+ unsigned int resume_detect_count;
>>+ unsigned int resume_wait_count;
>> };
>>
>>+static void sdhci_f_sdh30_resume_detect_work_func(struct work_struct
>>*work)
>>+{
>>+ struct f_sdhost_priv *priv = container_of(work, struct f_sdhost_priv,
>>+ resume_detect_work.work);
>>+ struct sdhci_host *host = dev_get_drvdata(priv->dev);
>>+ int err = 0;
>>+
>>+ if (mmc_bus_manual_resume(host->mmc)) {
>>+ pm_runtime_disable(&host->mmc->card->dev);
>>+ mmc_card_set_suspended(host->mmc->card);
>>+ err = host->mmc->bus_ops->resume(host->mmc);
>>+ if (priv->resume_detect_count-- && err)
>>+ queue_delayed_work(priv->resume_detect_wq,
>>+ &priv->resume_detect_work,
>>+ RESUME_DETECT_JIFFIES);
>>+ else
>>+ pr_debug("%s: resume detection done (count:%d, wait:%d, err:%d)\n",
>>+ mmc_hostname(host->mmc),
>>+ priv->resume_detect_count,
>>+ priv->resume_wait_count, err);
>>+ } else {
>>+ if (priv->resume_wait_count--)
>>+ queue_delayed_work(priv->resume_detect_wq,
>>+ &priv->resume_detect_work,
>>+ RESUME_WAIT_JIFFIES);
>>+ else
>>+ pr_debug("%s: resume done\n", mmc_hostname(host->mmc));
>>+ }
>>+}
>>+
>>+static void sdhci_f_sdh30_resume_detect(struct mmc_host *mmc,
>>+ int detect, int wait)
>>+{
>>+ struct sdhci_host *host = mmc_priv(mmc);
>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>+
>>+ priv->resume_detect_count = detect;
>>+ priv->resume_wait_count = wait;
>>+ queue_delayed_work(priv->resume_detect_wq,
>>+ &priv->resume_detect_work, 0);
>>+}
>>+
>> void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
>> {
>> struct f_sdhost_priv *priv = sdhci_priv(host);
>>@@ -146,6 +203,12 @@ static int sdhci_f_sdh30_probe(struct
>>platform_device *pdev)
>> }
>> }
>>
>>+ if (of_find_property(pdev->dev.of_node, "resume-detect-retry", NULL))
>>{
>>+ dev_info(dev, "Applying resume detect retry quirk\n");
>>+ priv->quirks |= F_SDH30_QUIRK_RESUME_DETECT_RETRY;
>>+ host->mmc->caps2 |= MMC_CAP2_MANUAL_RESUME;
>>+ }
>>+
>> ret = mmc_of_parse_voltage(pdev->dev.of_node, &host->ocr_mask);
>> if (ret) {
>> dev_err(dev, "%s: parse voltage error\n", __func__);
>>@@ -197,6 +260,18 @@ static int sdhci_f_sdh30_probe(struct
>>platform_device *pdev)
>> }
>> }
>>
>>+ /* Init workqueue */
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>+ priv->resume_detect_wq = create_workqueue("sdhci_f_sdh30");
>>+ if (priv->resume_detect_wq == NULL) {
>>+ ret = -ENOMEM;
>>+ dev_err(dev, "Failed to create resume detection workqueue\n");
>>+ goto err_init_wq;
>>+ }
>>+ INIT_DELAYED_WORK(&priv->resume_detect_work,
>>+ sdhci_f_sdh30_resume_detect_work_func);
>>+ }
>>+
>> ret = sdhci_add_host(host);
>> if (ret) {
>> dev_err(dev, "%s: host add error\n", __func__);
>>@@ -229,6 +304,9 @@ static int sdhci_f_sdh30_probe(struct
>>platform_device *pdev)
>> return 0;
>>
>> err_add_host:
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>+ destroy_workqueue(priv->resume_detect_wq);
>>+err_init_wq:
>> if (gpio_is_valid(priv->gpio_select_1v8)) {
>> gpio_direction_output(priv->gpio_select_1v8, 1);
>> gpio_free(priv->gpio_select_1v8);
>>@@ -268,6 +346,9 @@ static int sdhci_f_sdh30_remove(struct
>>platform_device *pdev)
>> gpio_free(priv->gpio_select_1v8);
>> }
>>
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY)
>>+ destroy_workqueue(priv->resume_detect_wq);
>>+
>> sdhci_free_host(host);
>> platform_set_drvdata(pdev, NULL);
>>
>>@@ -285,7 +366,15 @@ static int sdhci_f_sdh30_suspend(struct device
>>*dev)
>> static int sdhci_f_sdh30_resume(struct device *dev)
>> {
>> struct sdhci_host *host = dev_get_drvdata(dev);
>>+ struct f_sdhost_priv *priv = sdhci_priv(host);
>>
>>+ if (priv->quirks & F_SDH30_QUIRK_RESUME_DETECT_RETRY) {
>>+ pr_debug("%s: start resume detect\n",
>>+ mmc_hostname(host->mmc));
>>+ sdhci_f_sdh30_resume_detect(host->mmc,
>>+ RESUME_DETECT_COUNT,
>>+ RESUME_WAIT_COUNT);
>>+ }
>> return sdhci_resume_host(host);
>> }
>> #endif
>>diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>index 7960424..55221dd 100644
>>--- a/include/linux/mmc/host.h
>>+++ b/include/linux/mmc/host.h
>>@@ -283,6 +283,7 @@ struct mmc_host {
>> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
>> MMC_CAP2_HS400_1_2V)
>> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
>>+#define MMC_CAP2_MANUAL_RESUME (1 << 18) /* Resume manually when
>>error */
>>
>> mmc_pm_flag_t pm_caps; /* supported pm features */
>>
>>@@ -338,6 +339,9 @@ struct mmc_host {
>> const struct mmc_bus_ops *bus_ops; /* current bus driver */
>> unsigned int bus_refs; /* reference counter */
>>
>>+ unsigned int bus_resume_flags;
>>+#define MMC_BUSRESUME_MANUAL_RESUME (1 << 0)
>>+
>> unsigned int sdio_irqs;
>> struct task_struct *sdio_irq_thread;
>> bool sdio_irq_pending;
>>@@ -384,6 +388,16 @@ static inline void *mmc_priv(struct mmc_host
>>*host)
>> #define mmc_dev(x) ((x)->parent)
>> #define mmc_classdev(x) (&(x)->class_dev)
>> #define mmc_hostname(x) (dev_name(&(x)->class_dev))
>>+#define mmc_bus_manual_resume(host) ((host)->bus_resume_flags & \
>>+ MMC_BUSRESUME_MANUAL_RESUME)
>>+
>>+static inline void mmc_set_bus_resume_policy(struct mmc_host *host,
>>int manual)
>>+{
>>+ if (manual)
>>+ host->bus_resume_flags |= MMC_BUSRESUME_MANUAL_RESUME;
>>+ else
>>+ host->bus_resume_flags &= ~MMC_BUSRESUME_MANUAL_RESUME;
>>+}
>>
>> int mmc_power_save_host(struct mmc_host *host);
>> int mmc_power_restore_host(struct mmc_host *host);
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
